From 07139fa3a0ff25703bba4e4cb86866ddc65e3d27 Mon Sep 17 00:00:00 2001 From: mark-wint Date: Tue, 29 Sep 2026 11:48:02 -0400 Subject: [PATCH] fix: layered-architecture lets the app declare the data packages its bootstrap constructs The Dependency Graph section required a repositories-only app pubspec while App Bootstrap had main_.dart import and construct the data clients. depend_on_referenced_packages rejects that combination, so an agent following the skill kept the pubspec clean and had the repository build or default its own client, breaking the injection standard. The app pubspec now declares the data packages its bootstrap constructs, and the layer boundary is an import rule: only the entrypoints and bootstrap import a data package. Injection covers SDK objects as well as data clients, and a repository with no external source is valid. The pubspec reference gains an Import Boundary section, and a new eval case covers a request to keep the app pubspec to repositories only. Refs #164 Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 2 +- evals/README.md | 6 +-- evals/layered-architecture/NOTES.md | 17 +++++++ .../case.yaml | 4 ++ .../fixture.sh | 1 + .../graders/app-pubspec-declares-client.md | 5 ++ .../graders/constructs-client-in-main.md | 7 +++ .../graders/repository-builds-no-client.md | 6 +++ .../graders/requires-client.md | 4 ++ .../graders/skill-fired.md | 7 +++ .../prompt.md | 26 ++++++++++ skills/layered-architecture/SKILL.md | 51 ++++++++++++------- .../references/pubspec.md | 48 +++++++++++++++-- 13 files changed, 159 insertions(+), 25 deletions(-) create mode 100644 evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/case.yaml create mode 120000 evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/fixture.sh create mode 100644 evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/graders/app-pubspec-declares-client.md create mode 100644 evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/graders/constructs-client-in-main.md create mode 100644 evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/graders/repository-builds-no-client.md create mode 100644 evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/graders/requires-client.md create mode 100644 evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/graders/skill-fired.md create mode 100644 evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/prompt.md diff --git a/AGENTS.md b/AGENTS.md index 2afbd9f..04d9574 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -15,7 +15,7 @@ agents/ flutter-reviewer.md # Read-only Flutter code reviewer subagent docs/ # Gitignored, local only plan/ # Planning and design documents -evals/ # `claude plugin eval` suite — all 15 skills, 100 cases +evals/ # `claude plugin eval` suite — all 15 skills, 101 cases README.md # Case format, grader reference, how to add a case _fixture/ fixture.sh # The only copy of the neutral Flutter skeleton; every case symlinks here diff --git a/evals/README.md b/evals/README.md index 8643b46..14b83dc 100644 --- a/evals/README.md +++ b/evals/README.md @@ -4,7 +4,7 @@ Does Claude route to the skill, and does the output follow it? Cases run through [`claude plugin eval`](https://code.claude.com/docs/en/plugin-evals) against real models. ```bash -claude plugin eval . --scaffold # all 100 cases, both arms +claude plugin eval . --scaffold # all 101 cases, both arms claude plugin eval . --scaffold --tag bloc # one skill claude plugin eval . --scaffold --ablation none # with-plugin arm only, half the cost ``` @@ -105,7 +105,7 @@ made to a [mocked MCP tool](#mocking-the-mcp-servers). ### Routing graders -Ninety-nine of the hundred cases carry one: +A hundred of the 101 cases carry one: ```markdown --- @@ -305,7 +305,7 @@ plugin and 0.00 without, on three runs each. ```bash E="claude plugin eval . --scaffold" -$E # all 100, both arms +$E # all 101, both arms $E --tag bloc --tag testing # two skills $E --case bloc-writes-sealed-events-and-states # one case $E --ablation none # with-plugin arm only, half the cost diff --git a/evals/layered-architecture/NOTES.md b/evals/layered-architecture/NOTES.md index c3269f9..54335a4 100644 --- a/evals/layered-architecture/NOTES.md +++ b/evals/layered-architecture/NOTES.md @@ -63,6 +63,23 @@ sharing one definition does remove duplication. **Discriminates.** A bare model reaches for a service locator or a global singleton, and lists the repository packages as hosted pub dependencies. +### layered-architecture-injects-client-the-app-pubspec-declares + +**Discriminates.** The prompt asks for an app pubspec that lists only the repository. A bare +model honors that by giving `WeatherRepository` a `baseUrl` and building the client inside +it, through a redirecting `: this._(WeatherApiClient(baseUrl: baseUrl))` or an initializer +list, and says outright that it does so to keep `weather_api_client` out of the app. The +skill requires the client in the constructor, constructs it in `main_development.dart`, and +declares `weather_api_client` in the app pubspec because that entrypoint imports it. + +**Grader notes.** `repository-builds-no-client` and `app-pubspec-declares-client` carry the +case and weigh 2 each. `repository-builds-no-client` catches a `??` default, an +initializer-list construction, and a redirecting constructor, and stays clear of the +`final weatherApiClient = WeatherApiClient(...)` line in `main`. +`app-pubspec-declares-client` anchors on `path: packages/` so the repository pubspec's +`path: ../weather_api_client` cannot satisfy it, and allows a leading `+` because the +plugin arm often answers with a diff. + ### layered-architecture-stays-out-of-single-file-work **Discriminates.** No `packages/`, `_api_client`, `_repository` or `RepositoryProvider` diff --git a/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/case.yaml b/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/case.yaml new file mode 100644 index 0000000..431edf5 --- /dev/null +++ b/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/case.yaml @@ -0,0 +1,4 @@ +schema_version: "1.1" +name: layered-architecture-injects-client-the-app-pubspec-declares +context: + scaffold_script: fixture.sh diff --git a/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/fixture.sh b/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/fixture.sh new file mode 120000 index 0000000..56543ee --- /dev/null +++ b/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/fixture.sh @@ -0,0 +1 @@ +../../_fixture/fixture.sh \ No newline at end of file diff --git a/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/graders/app-pubspec-declares-client.md b/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/graders/app-pubspec-declares-client.md new file mode 100644 index 0000000..ff0bec6 --- /dev/null +++ b/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/graders/app-pubspec-declares-client.md @@ -0,0 +1,5 @@ +--- +type: regex +pattern: 'weather_api_client:\s*\+?\s*path:\s*(\./)?packages/weather_api_client' +weight: 2 +--- diff --git a/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/graders/constructs-client-in-main.md b/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/graders/constructs-client-in-main.md new file mode 100644 index 0000000..6b9e21a --- /dev/null +++ b/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/graders/constructs-client-in-main.md @@ -0,0 +1,7 @@ +--- +type: llm +--- + +PASS if lib/main_development.dart constructs a WeatherApiClient with the dev base URL and passes it into the WeatherRepository constructor. + +FAIL if main_development.dart constructs WeatherRepository without passing it a WeatherApiClient, or if the WeatherApiClient is constructed inside the repository package, for example in a factory constructor or a default parameter value. diff --git a/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/graders/repository-builds-no-client.md b/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/graders/repository-builds-no-client.md new file mode 100644 index 0000000..b6a0dea --- /dev/null +++ b/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/graders/repository-builds-no-client.md @@ -0,0 +1,6 @@ +--- +type: regex +pattern: '(\?\?|[:,]\s*_\w+\s*=|this\.\w*\()\s*WeatherApiClient\(' +match: not_contains +weight: 2 +--- diff --git a/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/graders/requires-client.md b/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/graders/requires-client.md new file mode 100644 index 0000000..7ed4bb8 --- /dev/null +++ b/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/graders/requires-client.md @@ -0,0 +1,4 @@ +--- +type: regex +pattern: 'required\s+WeatherApiClient\s+\w+' +--- diff --git a/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/graders/skill-fired.md b/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/graders/skill-fired.md new file mode 100644 index 0000000..5791dd4 --- /dev/null +++ b/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/graders/skill-fired.md @@ -0,0 +1,7 @@ +--- +type: tool_used +tool: Skill +input_match: '"skill"\s*:\s*"(?:[\w-]+:)?layered-architecture"' +weight: 3 +arm: both +--- diff --git a/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/prompt.md b/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/prompt.md new file mode 100644 index 0000000..c20bfbb --- /dev/null +++ b/evals/layered-architecture/layered-architecture-injects-client-the-app-pubspec-declares/prompt.md @@ -0,0 +1,26 @@ +--- +max_turns: 12 +timeout_seconds: 600 +allowed_tools: [Read, Glob, Grep, Skill] +tags: [layered-architecture] +description: "Bootstrap injection under a request to keep the app pubspec to repositories only: the repository requires its client and never builds one, main_development.dart constructs it, and the app pubspec declares the data package." +--- + +My monorepo has a data layer package at `packages/weather_api_client`. It is not in your working directory, so treat this as its exact public API: + +```dart +class WeatherApiClient { + WeatherApiClient({required String baseUrl}); + + Future getWeather(String city); +} + +class WeatherResponse { + const WeatherResponse({required this.city, required this.temperature}); + + final String city; + final double temperature; +} +``` + +Create `packages/weather_repository` with a `WeatherRepository` whose `getWeather(String city)` maps the response to a `Weather` domain model, and wire it into the app so features can reach it. I want the app's pubspec to list only `weather_repository` so it stays clean. The dev API is at `https://api.dev.example.com`. Show the Dart code for the repository and `lib/main_development.dart`, and the app pubspec changes. diff --git a/skills/layered-architecture/SKILL.md b/skills/layered-architecture/SKILL.md index 3b4296c..3b53adb 100644 --- a/skills/layered-architecture/SKILL.md +++ b/skills/layered-architecture/SKILL.md @@ -26,7 +26,7 @@ Layered monorepo architecture for Flutter apps — four layers organized as inde Apply these standards to ALL layered architecture work: -- **Four layers** — Data, Repository, Business Logic, Presentation — every feature spans exactly these four layers +- **Four layers** — Data, Repository, Business Logic, Presentation — a feature spans all four whenever its repository reads an external source - **Unidirectional dependencies** — Presentation → Business Logic → Repository → Data — never skip or invert a layer - **Data and Repository layers live in `packages/`** — each is an independent Dart package with its own `pubspec.yaml` - **Business Logic and Presentation live in `lib/`** — organized by feature within the app @@ -36,8 +36,9 @@ Apply these standards to ALL layered architecture work: - **One repository per domain** — `user_repository`, `weather_repository`, `auth_repository` - **Path dependencies for local packages** — never `git:` or pub version references for packages in the same repo - **Barrel exports at every package boundary** — `src/` is never imported directly by consumers -- **Repositories accept data layer dependencies via constructor injection** — never instantiate clients internally +- **Repositories take every external source through the constructor** — a data client or an SDK object such as `FirebaseAuth.instance`, built in bootstrap and passed in, never constructed or defaulted inside the repository - **App bootstrap wires all layers** — `main_.dart` creates clients and repositories, provides them via `RepositoryProvider` +- **Only the app's entrypoints and bootstrap import a data package** — blocs, widgets, and app tests reach data through a repository > **Cross-harness fallback.** This skill scaffolds and tests packages via the Very Good CLI MCP server. On a host without this plugin's Bash hooks and without that MCP server connected, run the equivalent `very_good create dart_package …`, `very_good packages get`, and `very_good test` commands directly. @@ -46,7 +47,7 @@ Apply these standards to ALL layered architecture work: | Layer | Responsibility | Location | Depends On | Example | | --- | --- | --- | --- | --- | | **Data** | External communication — API calls, local storage, platform plugins | `packages/_api_client/` | External packages only | `user_api_client`, `local_storage_client` | -| **Repository** | Data orchestration — combines data sources, transforms models, caches | `packages/_repository/` | Data layer packages | `user_repository`, `weather_repository` | +| **Repository** | Data orchestration — combines data sources, transforms models, caches | `packages/_repository/` | Zero or more data layer packages | `user_repository`, `weather_repository` | | **Business Logic** | State management — processes user actions, emits state changes | `lib//bloc/` or `lib//cubit/` | Repository layer | `LoginBloc`, `ProfileCubit` | | **Presentation** | UI — widgets, pages, views, layout | `lib//view/` | Business Logic layer | `LoginPage`, `ProfileView` | @@ -171,14 +172,14 @@ See [worked-example.md](references/worked-example.md) for the complete `user_api ## Repository Layer -The repository layer orchestrates data sources and exposes domain models. Each repository composes one or more data clients, transforms response models into domain models, and provides a clean API for the business logic layer. +The repository layer orchestrates data sources and exposes domain models. Each repository composes the data clients it needs, transforms response models into domain models, and provides a clean API for the business logic layer. **Rules:** - No inter-repository dependencies — repositories are isolated - No Flutter SDK — the `very_good_cli` MCP server `create dart_package` tool - Domain models live in the repository package — not in data packages - Transform data models into domain models — never leak API response shapes upstream -- Accept all data clients via constructor injection +- A repository with no external source, such as in-memory session state, takes no constructor arguments ### Pattern: Domain Model + Repository Transformation @@ -231,13 +232,16 @@ See [worked-example.md](references/worked-example.md) for the complete `user_rep ## Dependency Graph -Path dependencies in each `pubspec.yaml` are what enforce the architecture. A data package -declares external packages only. A repository package declares a path dependency on its data -package. The root app declares **repository packages only** — data packages arrive as -transitive dependencies. +Path dependencies in each `pubspec.yaml` point one direction. A data package declares +external packages only. A repository package declares a path dependency on its data +packages. The root app declares its repository packages and every data package its bootstrap +constructs. `main_.dart` imports each client to inject it, and +`depend_on_referenced_packages` in `package:very_good_analysis` requires every imported +package to be declared. -**The app never depends on a data package directly.** That is the boundary: business logic -and presentation cannot bypass the repository layer, because they cannot import past it. +**Imports hold the layer boundary.** Once the app declares a data package, the lint no +longer stops a bloc from importing it. The boundary is the import rule in Core Standards: +only the app's entrypoints and bootstrap import a data package. ```yaml # packages/user_api_client/pubspec.yaml — external packages only @@ -249,13 +253,16 @@ dependencies: user_api_client: path: ../user_api_client -# pubspec.yaml — repository packages only; data packages are transitive +# pubspec.yaml: repositories, plus the data packages bootstrap constructs dependencies: + user_api_client: + path: packages/user_api_client user_repository: path: packages/user_repository ``` -See [references/pubspec.md](references/pubspec.md) for the three files in full. +See [references/pubspec.md](references/pubspec.md) for the three files in full and for +checking the import boundary, including which files count as entrypoints and bootstrap. ## Data Flow @@ -287,12 +294,19 @@ Future _onLoadRequested( ## App Bootstrap -`main_.dart` constructs every data client and repository, then passes them to the -`App` widget, which exposes them through `MultiRepositoryProvider`. Flavors change only -configuration — base URLs, API keys — never the wiring shape. +`main_.dart` imports and constructs every data client and repository, then passes +the repositories to the `App` widget, which exposes them through `MultiRepositoryProvider`. +Flavors change only configuration — base URLs, API keys — never the wiring shape. ```dart // lib/main_development.dart +import 'package:auth_api_client/auth_api_client.dart'; +import 'package:auth_repository/auth_repository.dart'; +import 'package:flutter/material.dart'; +import 'package:my_app/app/app.dart'; +import 'package:user_api_client/user_api_client.dart'; +import 'package:user_repository/user_repository.dart'; + void main() { WidgetsFlutterBinding.ensureInitialized(); @@ -358,11 +372,12 @@ the `App` widget with `MultiRepositoryProvider`. ### Connecting a Repository to a Feature -1. Add path dependency on the repository package to root `pubspec.yaml` -2. Create the repository in `main_.dart` and pass it to `App` +1. Add path dependencies on the repository package and on each data package it takes to root `pubspec.yaml` +2. Create the data clients and the repository in `main_.dart` and pass the repository to `App` 3. Add `RepositoryProvider.value` in `App`'s `MultiRepositoryProvider` 4. Create the Bloc/Cubit with the repository injected — see the **bloc** skill 5. Build the Page/View with `BlocProvider` and `BlocBuilder` — see the **bloc** skill +6. Grep `lib/` and `test/` for `package:/` imports. The work is done when every hit is an entrypoint or bootstrap file, as the pubspec reference's Import Boundary section defines them ## Additional Resources diff --git a/skills/layered-architecture/references/pubspec.md b/skills/layered-architecture/references/pubspec.md index 0097f63..5c01b65 100644 --- a/skills/layered-architecture/references/pubspec.md +++ b/skills/layered-architecture/references/pubspec.md @@ -61,10 +61,16 @@ dependencies: flutter: sdk: flutter flutter_bloc: ^9.1.0 + auth_api_client: + path: packages/auth_api_client auth_repository: path: packages/auth_repository + user_api_client: + path: packages/user_api_client user_repository: path: packages/user_repository + weather_api_client: + path: packages/weather_api_client weather_repository: path: packages/weather_repository @@ -105,8 +111,8 @@ dev_dependencies: ## Layer Boundaries in `pubspec.yaml` -Each layer's `pubspec.yaml` is what actually enforces the architecture. Path dependencies -point one direction only. +Path dependencies point one direction only. The root app is the one package that declares +both layers, because its bootstrap constructs the data clients it injects into repositories. ### Data Package (`packages/user_api_client/pubspec.yaml`) @@ -134,9 +140,45 @@ dependencies: flutter: sdk: flutter flutter_bloc: ^9.1.0 - # Repository packages only — data packages are transitive + # Repositories, plus each data package main_.dart constructs + auth_api_client: + path: packages/auth_api_client auth_repository: path: packages/auth_repository + user_api_client: + path: packages/user_api_client user_repository: path: packages/user_repository ``` + +## The Import Boundary + +With the data packages declared, `depend_on_referenced_packages` no longer flags a bloc that +imports one. The boundary is the import rule: only the app's entrypoints and bootstrap import +a data package. + +The entrypoints and bootstrap are the files that construct the clients. In a Very Good CLI +app they are `lib/main_.dart` and `lib/bootstrap.dart`. A single-flavor app has +`lib/main.dart`. Some apps group them in a directory, such as `lib/main/main_development.dart` +and `lib/main/bootstrap/bootstrap.dart`. Treat whichever files the app uses as the allowed set. + +Check the boundary after wiring a repository. Grep `lib/` and `test/` for each data package's +imports: + +```text +^(import|export) 'package:user_api_client/ +``` + +The check passes when every hit is an entrypoint or bootstrap file. Fix any other hit in +the code: + +- A bloc or widget that names a data-layer model, enum, or exception gets a domain type + instead. The repository defines it and maps to it, the way `UserRepository` catches + `UserApiException` and throws its own `UserNotFoundException`. +- A bloc or widget that calls the client calls a repository method instead. +- An app test mocks the repository, never the client. + +When the data layer is a third-party SDK used without a wrapper package, such as +`package:firebase_auth` or `package:firebase_storage`, the same rule covers its imports. Its +package name does not mark it as data layer, so add each SDK the repositories take to the +grep by name.