Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ scripts/
fixtures/
valid_workspace/ # FFCA repo that must pass
invalid_workspace/ # One package per violation the validator must catch
warning_workspace/ # Warnings and suppressed errors only, must exit 0
not_ffca/ # Non-FFCA repo the validator must skip
skills/
ffca-architecture/SKILL.md # Orientation, layer rules, naming
Expand Down
4 changes: 2 additions & 2 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@

`hooks/hooks.json` defines one PostToolUse hook and one PreToolUse hook.

- `Edit|MultiEdit|Write` matcher → `validate_layers.sh`. The matcher is a filter, not a guarantee, so the hook reads `tool_name` from the payload and exits 0 unless it is `Edit`, `MultiEdit`, or `Write`. When the edited file is a `pubspec.yaml` inside an FFCA-shaped repo, meaning a `features/` folder exists above it, the hook runs `dart run scripts/validate_layers.dart --file <pubspec>`. The validator checks the edited package and its direct dependents. Exit 2 means a layer violation. The hook prints the rule and the fix to stderr and exits 2, which blocks Claude until the dependency is fixed.
- `Edit|MultiEdit|Write` matcher → `validate_layers.sh`. The matcher is a filter, not a guarantee, so the hook reads `tool_name` from the payload and exits 0 unless it is `Edit`, `MultiEdit`, or `Write`. When the edited file is a `pubspec.yaml` inside an FFCA-shaped repo, meaning a `features/` folder exists above it, the hook runs `dart run scripts/validate_layers.dart --file <pubspec>`. The validator checks the edited package and its direct dependents. Exit 2 means at least one error-severity finding. The hook prints the findings to stderr and exits 2, which blocks Claude until the dependency is fixed. When the validator exits 0 with output, every finding is a warning: the hook returns it as `additionalContext` JSON and exits 0, so Claude sees it without being blocked. A `# ffca-ignore: <rule-id> — <reason>` comment in a pubspec suppresses that rule for that package.
- Any other file, or a repo with no `features/` folder, exits 0 silently.
- The hook skips with exit 0 when `jq` or `dart` is missing from `PATH`. The `dart` check runs only for FFCA pubspec edits. A validator failure other than exit 2 is reported to stderr but never blocks.

- `mcp__.*very-good-cli__.*` matcher → `check_vgv_cli.sh`. For a Very Good CLI MCP tool, it returns `allow` when `very_good --version` is 1.3.0 or newer and `deny` with an install or upgrade message when the CLI is missing or older. It stands aside with exit 0 for any other tool, when the version cannot be read, or when `jq` is missing. `allowed-tools` grants only last for the invoking turn, so this hook is what keeps the tools approved afterwards.

`hooks/check_vgv_cli_test.sh` covers the PreToolUse hook against a stubbed `very_good` and runs in CI under the **Hook Tests** job.

`scripts/test/validate_layers_test.dart` covers the validator and runs in CI under the **Layer Validator** job. Add a fixture and a case there when changing a rule.
`scripts/test/validate_layers_test.dart` covers the validator and runs in CI under the **Layer Validator** job. Add a fixture and a case there when changing a rule, and add a new rule's ID and severity to `ruleSeverities` and to the rules table in the **Hooks** section of `README.md`.

`hooks/validate_layers_test.sh` covers the PostToolUse hook against the same fixtures and runs in CI under the **Hook Tests** job.
36 changes: 35 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ A PostToolUse hook runs on every `Edit`, `MultiEdit`, or `Write` and ignores any

| Hook | Event | Behavior |
| --- | --- | --- |
| **Validate layers** (`validate_layers.sh`) | PostToolUse (`Edit`/`MultiEdit`/`Write`) | Runs the FFCA validator incrementally on the edited package and its direct dependents. Exits 2 on a violation (blocking: Claude must fix the dependency before continuing), printing the rule and the fix. Passes silently otherwise |
| **Validate layers** (`validate_layers.sh`) | PostToolUse (`Edit`/`MultiEdit`/`Write`) | Runs the FFCA validator incrementally on the edited package and its direct dependents. Exits 2 on an error (blocking: Claude must fix the dependency before continuing), printing the rule and the fix. Passes warnings to Claude as non-blocking context. Passes silently otherwise |
| **Check VGV CLI** (`check_vgv_cli.sh`) | PreToolUse (`mcp__.*very-good-cli__.*`) | Auto-approves Very Good CLI MCP tool calls when the CLI is installed at 1.3.0 or newer, so they work in every run mode. Denies with an install or upgrade message when the CLI is missing or outdated. Stands aside for any other tool, or when the CLI version cannot be read |

The validator checks package naming under `features/`, the layer dependency rules, that nothing depends on an app, and cycles. It also checks that each feature's packages fit together: a data package depends on its own `_domain` under `dependencies`, a data package never exists without a `_domain` in its feature, and a `_domain` package never depends on the Flutter SDK.
Expand All @@ -129,6 +129,39 @@ The validator is also runnable directly for CI and audits, across the whole work
dart run scripts/validate_layers.dart --all
```

### Rules, severities, and suppressions

Every finding carries a rule ID in brackets. An **error** exits 2 and blocks the hook. A **warning** is reported but exits 0.

| Rule ID | Severity | Finding |
| --- | --- | --- |
| `package-name` | error | A package under `features/{f}/` is not named `{f}_domain`, `{f}_data`, `{f}_data_{backend}`, or `{f}_presentation` |
| `depends-on-app` | error | Any package depends on an app |
| `shared-depends-on-feature` | error | A shared package depends on a feature package |
| `presentation-depends-on-data` | error | A presentation package depends on a data package |
| `domain-depends-on-data` | error | A domain package depends on a data package |
| `domain-depends-on-presentation` | error | A domain package depends on a presentation package |
| `data-depends-on-presentation` | error | A data package depends on a presentation package |
| `data-depends-on-feature-data` | error | A data package depends on another feature's data package |
| `layer-dependency` | error | Any other dependency the rules table does not allow |
| `dependency-cycle` | error | Packages depend on each other in a cycle |
| `data-missing-own-domain` | error | A data package does not depend on its own `_domain` under `dependencies` |
| `data-without-domain` | error | A data package's feature has no `_domain` package |
| `domain-uses-flutter` | error | A domain package depends on the Flutter SDK |
| `presentation-missing-own-domain` | warning | A presentation package does not depend on its feature's existing `_domain` |
| `suppression-missing-reason` | error | An `ffca-ignore` comment has no reason |
| `suppression-unknown-rule` | error | An `ffca-ignore` comment names a rule that does not exist or cannot be suppressed |
| `unused-suppression` | warning | An `ffca-ignore` comment matches no finding (`--all` only) |

To accept a finding as a deliberate exception, add a comment with the rule ID and a reason to the pubspec of the package it is reported on:

```yaml
# ffca-ignore: data-missing-own-domain — legacy repository, moving to orders_domain in #123
name: orders_data
```

A suppression covers every finding with that rule ID in that package. The reason is required, so each exception explains itself in review. The last three rules cannot be suppressed. `--all` reports how many findings were suppressed, and flags suppressions that no longer match anything so they can be removed.

### Prerequisites

- **Dart SDK** must be available on your `PATH`. The validator imports only `dart:io`, so it runs with just the SDK, no `dart pub get` required.
Expand All @@ -138,6 +171,7 @@ Run the hook tests with:

```bash
bash hooks/check_vgv_cli_test.sh
bash hooks/validate_layers_test.sh
```

## MCP Integration
Expand Down
2 changes: 1 addition & 1 deletion agents/ffca-layer-auditor.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ From the repo root, run the validator in full-graph mode and treat every line it
dart run ${CLAUDE_PLUGIN_ROOT}/scripts/validate_layers.dart --all
```

This is the authoritative check for the pubspec dependency graph: package naming under `features/`, the layer dependency rules, the no-dependency-on-an-app rule, workspace-wide cycles, and feature ownership (a data package depends on its own domain, a data package has a domain in its feature, a domain package never depends on the Flutter SDK). It prints each violation with its rule and fix and exits 2 if any are found, 0 if the graph is clean. Do not re-derive these checks by hand; the script is the source of truth for them. Read `references/ffca/project_structure.md`, sections *Dependency rules* and *Naming conventions*, only if you need to explain a finding.
This is the authoritative check for the pubspec dependency graph: package naming under `features/`, the layer dependency rules, the no-dependency-on-an-app rule, workspace-wide cycles, and feature ownership (a data package depends on its own domain, a data package has a domain in its feature, a domain package never depends on the Flutter SDK). It prints each finding with its rule ID, severity (`✗` error, `⚠` warning), and fix, then a summary line with the error, warning, and suppressed counts. It exits 2 if any error is found, and 0 when there are only warnings or nothing at all. An error makes the package `fail`; a warning makes it `warn`. A finding suppressed with an `# ffca-ignore:` comment is an accepted exception: do not report it as a violation, but list the suppression and its reason in the package's notes. Do not re-derive these checks by hand; the script is the source of truth for them. Read `references/ffca/project_structure.md`, sections *Dependency rules* and *Naming conventions*, only if you need to explain a finding.

## Step 2: source-level checks (qualitative)

Expand Down
8 changes: 8 additions & 0 deletions hooks/validate_layers.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ set -euo pipefail
# Mirrors the analyze hook in vgv-ai-flutter-plugin: parse the payload with jq,
# filter to the files we care about, skip gracefully when prerequisites are
# missing, then run the validator and propagate its exit code (2 blocks Claude).
# Warnings exit 0 and reach Claude as non-blocking additional context.

# Read the hook payload from stdin.
input=$(cat)
Expand Down Expand Up @@ -74,4 +75,11 @@ if [[ "$code" -ne 0 ]]; then
exit 0
fi

# 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" \

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

'{hookSpecificOutput: {hookEventName: "PostToolUse", additionalContext: $ctx}}'
fi

exit 0
26 changes: 20 additions & 6 deletions hooks/validate_layers_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
# Usage: bash hooks/validate_layers_test.sh
#
# The hook reads a JSON payload from stdin and either passes (exit 0, silent), skips
# (exit 0 with a "skipping" note on stderr) or blocks (exit 2). Any other exit is its
# (exit 0 with a "skipping" note on stderr), warns (exit 0 with the validator's
# warnings as additionalContext JSON on stdout) or blocks (exit 2). Any other exit is its
# own outcome, never mistaken for a pass, and so is the validator itself failing to run.
# Cases run against the validator fixtures in scripts/test/fixtures, so the runner needs
# dart and jq installed. HOME is passed through because SDK version managers such as
Expand Down Expand Up @@ -42,20 +43,24 @@ ln -s "$(command -v jq)" "$STUB_DIR/jq/jq"
VALID="$FIXTURES/valid_workspace/features/cart/cart_domain/pubspec.yaml"
VIOLATING="$FIXTURES/invalid_workspace/features/alpha/alpha_domain/pubspec.yaml"
NOT_FFCA="$FIXTURES/not_ffca/packages/some_pkg/pubspec.yaml"
WARNING="$FIXTURES/warning_workspace/features/nu/nu_presentation/pubspec.yaml"
SUPPRESSED="$FIXTURES/warning_workspace/features/xi/xi_data/pubspec.yaml"
NOT_PUBSPEC="$FIXTURES/invalid_workspace/features/alpha/alpha_domain/lib/alpha_domain.dart"

payload() { printf '{"tool_name":"%s","tool_input":{"file_path":"%s"}}' "$1" "$2"; }

# Prints "pass" (exit 0, silent), "skip" (exit 0, missing-tool note), "block" (exit 2),
# "error" (exit 0 after the validator itself failed) or "exit:<status>" for anything
# else, so a crashing hook or validator cannot pass as a pass.
# Prints "pass" (exit 0, silent), "skip" (exit 0, missing-tool note), "warn" (exit 0,
# findings in additionalContext), "block" (exit 2), "error" (exit 0 after the validator
# itself failed) or "exit:<status>" for anything else, so a crashing hook or validator
# cannot pass as a pass.
# Usage: run_hook <payload> [path]
run_hook() {
local input="$1" path="${2:-$PATH}"
local stderr status=0
local stderr stdout status=0
stderr=$(printf '%s' "$input" \
| env -i PATH="$path" HOME="$HOME" CLAUDE_PLUGIN_ROOT="$PLUGIN_ROOT" \
"$BASH_BIN" "$HOOK" 2>&1 >/dev/null) || status=$?
"$BASH_BIN" "$HOOK" 2>&1 >"$STUB_DIR/stdout") || status=$?
stdout=$(cat "$STUB_DIR/stdout")
if [ "$status" -eq 2 ]; then
echo "block"
elif [ "$status" -ne 0 ]; then
Expand All @@ -64,6 +69,13 @@ run_hook() {
echo "error"
elif [[ "$stderr" == *"not found, skipping"* ]]; then
echo "skip"
elif [ -n "$stdout" ]; then
if jq -r '.hookSpecificOutput.additionalContext // empty' <<<"$stdout" \
| grep -q 'FFCA findings'; then
echo "warn"
else
echo "exit:malformed"
fi
else
echo "pass"
fi
Expand Down Expand Up @@ -105,6 +117,8 @@ assert_outcome pass "Write on a valid pubspec" "$(payload Write "$VA
assert_outcome block "Edit on a violating pubspec" "$(payload Edit "$VIOLATING")"
assert_outcome block "Write on a violating pubspec" "$(payload Write "$VIOLATING")"
assert_outcome block "MultiEdit on a violating pubspec" "$(payload MultiEdit "$VIOLATING")"
assert_outcome warn "Edit on a pubspec with a warning" "$(payload Edit "$WARNING")"
assert_outcome pass "Edit on a suppressed finding" "$(payload Edit "$SUPPRESSED")"

echo ""
echo "--- Missing prerequisites (skip) ---"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,5 @@ name: beta_presentation
resolution: workspace
environment:
sdk: ^3.11.0
dependencies:
beta_domain: any
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# A suppression with no reason suppresses nothing and is itself an error.
# ffca-ignore: data-missing-own-domain
name: omicron_data
resolution: workspace
environment:
sdk: ^3.11.0
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
name: omicron_domain
resolution: workspace
environment:
sdk: ^3.11.0
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# A suppression naming a rule that does not exist.
# ffca-ignore: domain-uses-widgets — not a rule ID
name: pi_domain
resolution: workspace
environment:
sdk: ^3.11.0
3 changes: 3 additions & 0 deletions scripts/test/fixtures/invalid_workspace/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@ workspace:
- features/lambda/lambda_domain
- features/mu/mu_data
- features/mu/mu_domain
- features/omicron/omicron_data
- features/omicron/omicron_domain
- features/orders/orders_service
- features/pi/pi_domain
- features/theta/theta_data
- features/theta/theta_domain
- features/zeta/zeta_domain
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
name: nu_domain
resolution: workspace
environment:
sdk: ^3.11.0
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Presentation layer that does not depend on its own feature's domain.
name: nu_presentation
resolution: workspace
environment:
sdk: ^3.11.0
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# A suppression that no longer matches any finding.
# ffca-ignore: domain-uses-flutter — Flutter dependency already removed
name: rho_domain
resolution: workspace
environment:
sdk: ^3.11.0
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# An error suppressed with a reason.
# ffca-ignore: data-missing-own-domain — legacy repository, moving to xi_domain
name: xi_data
resolution: workspace
environment:
sdk: ^3.11.0
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
name: xi_domain
resolution: workspace
environment:
sdk: ^3.11.0
10 changes: 10 additions & 0 deletions scripts/test/fixtures/warning_workspace/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# Warnings and suppressed errors only: --all must exit 0.
name: _warning_workspace
environment:
sdk: ^3.11.0
workspace:
- features/nu/nu_domain
- features/nu/nu_presentation
- features/rho/rho_domain
- features/xi/xi_data
- features/xi/xi_domain
Loading