Repository navigation
Conversation
Every finding now carries a stable rule ID and a severity from the new ruleSeverities map. Errors exit 2 and block the hook; warnings are printed but exit 0. Output marks errors with ✗ and warnings with ⚠ and ends with an error, warning, and suppressed count. A `# ffca-ignore: <rule-id> — <reason>` comment in a pubspec suppresses that rule for the package. A suppression without a reason or with an unknown rule ID is an error and suppresses nothing; one that matches no finding is a warning in --all mode. Add the presentation-missing-own-domain warning held back from #24. The hook returns warnings to Claude as non-blocking additionalContext, covered by the new hooks/validate_layers_test.sh in the Hook Tests job. Closes #21 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Vxxv9fVFGupLXEnmx5bSq
…rity # Conflicts: # AGENTS.md # CLAUDE.md # README.md # hooks/validate_layers_test.sh
…rity Brings the review fixes from #29: deterministic cycle attribution and 4-space pubspec indentation. The cycle and indentation tests now assert the rule IDs this branch adds. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Vxxv9fVFGupLXEnmx5bSq
From a review of #29, #30 and #31: - Check imports of undeclared workspace packages against the rules table. A Dart workspace resolves them, so `product_presentation` could import `cart_data` without either mode noticing. A forbidden layer reports that layer rule; an allowed one reports the new undeclared-dependency error. - On a pubspec edit, run import checks and suppression validation for the edited package only, so a clean edit never blocks on a dependent's existing Dart findings. - Skip missing-primary-barrel in incremental mode while a package has no lib/, so a new package can be scaffolded pubspec first. - Read directives with a scanner that stops at the first declaration, follows multi-line directives, and reads every URI, so imports inside strings are ignored and both sides of a conditional import are checked. - Check every barrel at the root of a data package's lib/, following relative exports one level, for exported DTOs and mappers. - Report a `//` suppression naming a pubspec-only rule, and group suppression findings under the file the comment is in. - Fix CLAUDE.md, which said dart only runs for pubspec edits, and cite the barrel rule's actual sources. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Vxxv9fVFGupLXEnmx5bSq
| # Exit 0 with output means warnings only. Stderr on exit 0 never reaches | ||
| # Claude, so hand the warnings over as additional context, without blocking. | ||
| if [[ -n "$output" ]]; then | ||
| jq -n --arg ctx "FFCA validator warnings (non-blocking):"$'\n'"$output" \ |
There was a problem hiding this comment.
Before this change, output from a run that exited 0 was dropped. Now anything dart run prints (first-run/analytics notices, implicit Resolving dependencies... because scripts/ has a pubspec, SDK warnings) gets sent to Claude as "FFCA validator warnings". Can we forward only real findings? For example, only emit the JSON when $output contains FFCA findings, as the test's run_hook already checks. Or have the validator write findings to a dedicated stream so the hook doesn't depend on 2>&1.
| if (violations.isEmpty) return 0; // silent pass | ||
| _report(violations, root); | ||
| return 2; | ||
| final result = _applySuppressions(violations, scope, reportUnused: false); |
There was a problem hiding this comment.
Since warnings now go to Claude on every pubspec edit, could incremental mode only report warnings for the edited package (or only findings the edit could have caused)? Today, editing a shared package re-surfaces every existing warning across all its dependents each time, which seems likely to distract Claude into unrelated changes. Errors in dependents still make sense to report.
|
|
||
| /// `# ffca-ignore: <rule-id> <separator> <reason>`, where the separator is an | ||
| /// em dash, an en dash, a hyphen, or a colon. | ||
| final _suppressionPattern = RegExp(r'#\s*ffca-ignore:\s*([A-Za-z0-9_-]+)(.*)$'); |
There was a problem hiding this comment.
Small edge case: because the ID class includes -, a hyphen separator with no space before it (domain-uses-flutter- reason) is swallowed into the rule ID and reported as an unknown rule. Requiring the ID to end in [a-z0-9] (e.g. ([a-z0-9]+(?:-[a-z0-9]+)*)) would make the hyphen separator work as documented.
| final String rule; | ||
| final String fix; | ||
|
|
||
| Severity get severity => ruleSeverities[id]!; |
There was a problem hiding this comment.
ruleSeverities[id]! turns a mistyped or unregistered rule id into a runtime crash, and the hook reads a crash as "skipping", so a missing map entry silently disables blocking for that rule. Could the IDs be an enum (with .id string and severity), or could we add a test that every id: passed to _Violation is a key of ruleSeverities?
2 participants
Description
Closes #21. Stacked on #29: the base is
feat/feature-ownership-checks, so this diff shows only the #21 changes. Merge #29 first; GitHub then retargets this PR tomain.Rule IDs and severities. Every finding has a stable rule ID, and a
ruleSeveritiesmap next to therulestable gives each one a severity: 14 errors and 3 warnings. Errors exit 2 and block the hook. Warnings are printed but exit 0. Output looks like this:Suppressions.
# ffca-ignore: <rule-id> — <reason>in a pubspec suppresses that rule for the package. The reason is required:suppression-missing-reasonerror, suppresses nothingsuppression-unknown-ruleerrorunused-suppressionwarning,--allonly, since incremental mode does not compute every findingNew warning.
presentation-missing-own-domain, held back from #24: a presentation package that does not depend on its feature's existing_domain. Presentation-only features have no domain, so they are not affected.Hook. Stderr on exit 0 never reaches Claude, so
validate_layers.shnow returns warnings asadditionalContextJSON without blocking.hooks/validate_layers_test.sh, added in #17, gains awarnoutcome and two cases againstwarning_workspace: a warning reaches Claude as context, and a suppressed finding stays silent.Fixtures. The new
warning_workspacemust exit 0: it holds a warning, an error suppressed with a reason, and an unused suppression.invalid_workspacegainsomicron(no reason) andpi(unknown rule ID).beta_presentationnow depends onbeta_domainso it does not pick up an accidental warning.Docs. The README gains a rules, severities, and suppressions section with the full table of rule IDs.
CLAUDE.mdcovers the warning path and the hook test,AGENTS.mdlists the new files, and the auditor agent maps errors tofailand warnings towarnand lists suppressions with their reasons.Out of scope. Suppressions apply per package, not per dependency line.
// ffca-ignorein Dart files waits for #25, since no check reads Dart source yet.Review fixes. Merges #29's review fixes (cycle attribution and 4-space pubspecs); the two new tests assert this branch's rule IDs. The review's suppression findings concern Dart-file suppressions and are fixed in #31.
Verified locally:
dart analyze,dart format,dart test(48 tests, 11 new), both hook test scripts (validate_layers_test.sh: 17 cases, 2 new), markdownlint, and cspell.mainis merged in through #29, resolving the overlap with #17.Type of Change
feat)fix)refactor)docs)ci)chore)🤖 Generated with Claude Code
https://claude.ai/code/session_017Vxxv9fVFGupLXEnmx5bSq