From cf16a7ad127c615113dc42b085b7a4b88f8c81f3 Mon Sep 17 00:00:00 2001 From: Dominik Simonik Date: Tue, 29 Sep 2026 08:44:32 +0200 Subject: [PATCH 1/2] fix: scope validate_layers hook on tool_name The hook now exits 0 unless tool_name is Edit, MultiEdit, or Write, per the hook scoping rule (a matcher is a filter, not a guarantee). The dart check moves next to the validator run so unrelated edits stay silent. Adds hooks/validate_layers_test.sh and a Script Tests CI job. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yaml | 3 + README.md | 4 +- hooks/hooks.json | 2 +- hooks/validate_layers.sh | 20 ++++-- hooks/validate_layers_test.sh | 119 ++++++++++++++++++++++++++++++++++ 5 files changed, 140 insertions(+), 8 deletions(-) create mode 100755 hooks/validate_layers_test.sh diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index b46d1ce..49c92d0 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -61,8 +61,11 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v7 + - uses: dart-lang/setup-dart@v1 - name: Check VGV CLI hook tests run: bash hooks/check_vgv_cli_test.sh + - name: Validate layers hook tests + run: bash hooks/validate_layers_test.sh skills-lint: name: 🔍 Skills Lint diff --git a/README.md b/README.md index a08bf19..ab0a918 100644 --- a/README.md +++ b/README.md @@ -114,11 +114,11 @@ Skills activate automatically when Claude detects an FFCA repo or an FFCA-shaped ## Hooks -A PostToolUse hook runs on every `Edit` or `Write`. When the edited file is a `pubspec.yaml` inside an FFCA-shaped repo, it validates the package's layer dependencies. A PreToolUse hook gates every Very Good CLI MCP tool call. +A PostToolUse hook runs on every `Edit`, `MultiEdit`, or `Write` and ignores any other tool call. When the edited file is a `pubspec.yaml` inside an FFCA-shaped repo, it validates the package's layer dependencies. A PreToolUse hook gates every Very Good CLI MCP tool call. | Hook | Event | Behavior | | --- | --- | --- | -| **Validate layers** (`validate_layers.sh`) | PostToolUse (`Edit`/`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 a violation (blocking: Claude must fix the dependency before continuing), printing the rule and the fix. 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 is also runnable directly for CI and audits, across the whole workspace: diff --git a/hooks/hooks.json b/hooks/hooks.json index 2543f4f..8499b5c 100644 --- a/hooks/hooks.json +++ b/hooks/hooks.json @@ -15,7 +15,7 @@ ], "PostToolUse": [ { - "matcher": "Edit|Write", + "matcher": "Edit|MultiEdit|Write", "hooks": [ { "type": "command", diff --git a/hooks/validate_layers.sh b/hooks/validate_layers.sh index 5dc91fa..a235895 100755 --- a/hooks/validate_layers.sh +++ b/hooks/validate_layers.sh @@ -9,15 +9,19 @@ set -euo pipefail # Read the hook payload from stdin. input=$(cat) -# Graceful skip if jq or dart is unavailable. +# Graceful skip if jq is unavailable. if ! command -v jq &>/dev/null; then echo "validate_layers hook: jq not found, skipping" >&2 exit 0 fi -if ! command -v dart &>/dev/null; then - echo "validate_layers hook: dart not found, skipping" >&2 - exit 0 -fi + +# The matcher is a filter, not a guarantee: confirm from the payload that this +# is a file-editing tool call before doing anything else. +tool_name=$(jq -r '.tool_name // empty' <<<"$input") +case "$tool_name" in + Edit | MultiEdit | Write) ;; + *) exit 0 ;; +esac # Extract the edited file path. file_path=$(jq -r '.tool_input.file_path // empty' <<<"$input") @@ -45,6 +49,12 @@ if [[ "$file_path" == /* ]]; then [[ "$ffca" -eq 1 ]] || exit 0 fi +# Graceful skip if dart is unavailable. +if ! command -v dart &>/dev/null; then + echo "validate_layers hook: dart not found, skipping" >&2 + exit 0 +fi + # Run the validator in incremental mode and propagate its exit code. set +e output=$(dart run "${CLAUDE_PLUGIN_ROOT}/scripts/validate_layers.dart" --file "$file_path" 2>&1) diff --git a/hooks/validate_layers_test.sh b/hooks/validate_layers_test.sh new file mode 100755 index 0000000..9aedae3 --- /dev/null +++ b/hooks/validate_layers_test.sh @@ -0,0 +1,119 @@ +#!/bin/bash +# Tests for validate_layers.sh +# +# 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 +# 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 +# asdf or fvm resolve dart from it. The missing-tool cases run on a PATH that holds only +# the few utilities the hook needs. + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +PLUGIN_ROOT="$(dirname "$SCRIPT_DIR")" +HOOK="$SCRIPT_DIR/validate_layers.sh" +FIXTURES="$PLUGIN_ROOT/scripts/test/fixtures" +BASH_BIN="$(command -v bash)" + +for tool in jq dart; do + if ! command -v "$tool" &>/dev/null; then + echo "validate_layers_test: $tool is required to run these tests" >&2 + exit 1 + fi +done + +PASSED=0 +FAILED=0 + +STUB_DIR="$(mktemp -d)" +trap 'rm -rf "$STUB_DIR"' EXIT + +# Minimal PATHs for the missing-tool cases: core utilities only, then the same plus jq. +mkdir -p "$STUB_DIR/core" "$STUB_DIR/jq" +for util in cat dirname basename; do + ln -s "$(command -v "$util")" "$STUB_DIR/core/$util" +done +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" +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:" for anything +# else, so a crashing hook or validator cannot pass as a pass. +# Usage: run_hook [path] +run_hook() { + local input="$1" path="${2:-$PATH}" + local stderr 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=$? + if [ "$status" -eq 2 ]; then + echo "block" + elif [ "$status" -ne 0 ]; then + echo "exit:$status" + elif [[ "$stderr" == *"validator exited"* ]]; then + echo "error" + elif [[ "$stderr" == *"not found, skipping"* ]]; then + echo "skip" + else + echo "pass" + fi +} + +assert_outcome() { + local expected="$1" label="$2" input="$3" path="${4:-$PATH}" + local result + result=$(run_hook "$input" "$path") + if [ "$result" = "$expected" ]; then + printf " \033[32mPASS\033[0m %-6s %s\n" "$expected" "$label" + PASSED=$((PASSED + 1)) + else + printf " \033[31mFAIL\033[0m expected %s but got %s: %s\n" "$expected" "$result" "$label" + FAILED=$((FAILED + 1)) + fi +} + +echo "=== validate_layers tests ===" +echo "" +echo "--- Tools that are not this hook's business (pass) ---" +# A violating pubspec proves the hook stood aside instead of validating. +assert_outcome pass "Read on a violating pubspec" "$(payload Read "$VIOLATING")" +assert_outcome pass "Bash tool call" '{"tool_name":"Bash","tool_input":{"command":"ls"}}' +assert_outcome pass "NotebookEdit on a violating pubspec" "$(payload NotebookEdit "$VIOLATING")" +assert_outcome pass "empty payload" '{}' +assert_outcome pass "null tool_name" "{\"tool_name\":null,\"tool_input\":{\"file_path\":\"$VIOLATING\"}}" + +echo "" +echo "--- Files the hook does not validate (pass) ---" +assert_outcome pass "non-pubspec file" "$(payload Edit "$NOT_PUBSPEC")" +assert_outcome pass "missing file_path" '{"tool_name":"Write","tool_input":{}}' +assert_outcome pass "pubspec in a non-FFCA repo" "$(payload Edit "$NOT_FFCA")" + +echo "" +echo "--- FFCA pubspec edits ---" +assert_outcome pass "Edit on a valid pubspec" "$(payload Edit "$VALID")" +assert_outcome pass "Write on a valid pubspec" "$(payload Write "$VALID")" +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")" + +echo "" +echo "--- Missing prerequisites (skip) ---" +assert_outcome skip "jq not installed" "$(payload Edit "$VIOLATING")" "$STUB_DIR/core" +assert_outcome skip "dart not installed" "$(payload Edit "$VIOLATING")" "$STUB_DIR/core:$STUB_DIR/jq" + +echo "" +echo "=== Results: $PASSED passed, $FAILED failed ===" + +if [ "$FAILED" -gt 0 ]; then + exit 1 +fi From 44fcc4f4fc359c65454556bbb9ff0471fd4eb85a Mon Sep 17 00:00:00 2001 From: Dominik Simonik Date: Wed, 30 Sep 2026 10:57:35 +0200 Subject: [PATCH 2/2] docs: document validate_layers hook scoping and tests Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 3 ++- CLAUDE.md | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 06ba364..c98ff3b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -24,6 +24,7 @@ hooks/ check_vgv_cli.sh # Checks the Very Good CLI version and auto-approves its MCP tools check_vgv_cli_test.sh # Tests for check_vgv_cli.sh validate_layers.sh # Runs the validator on edited pubspec.yaml files + validate_layers_test.sh # Tests for validate_layers.sh references/ ffca/ # Byte mirror of the VGV Engineering FFCA pages, single source of truth README.md # Manifest: which upstream page each file mirrors @@ -106,7 +107,7 @@ Documentation drifts when an asset changes and the docs describing it do not. Up - **The FFCA architecture changes.** Update the VGV Engineering page first, then run `dart run scripts/sync_reference.dart` to re-sync `references/ffca/`. Check every skill, the agent, and the code templates for section names that moved or rules that changed. If a dependency rule changed, update the rules table at the top of `scripts/validate_layers.dart` and its tests. - **A skill's scope or triggers change.** Update `description` and the matching row in the `README.md` Skills table. - **The validator's rules or flags change.** Update `scripts/test/validate_layers_test.dart` and its fixtures, the **Hooks** section of `README.md`, and the `## Hooks` section of `CLAUDE.md` if the hook's behavior changes. -- **A hook changes in `hooks/hooks.json`.** Update the **Hooks** section of `README.md` and the `## Hooks` section of `CLAUDE.md`. If it is `check_vgv_cli.sh`, update `hooks/check_vgv_cli_test.sh`. +- **A hook changes in `hooks/hooks.json`.** Update the **Hooks** section of `README.md` and the `## Hooks` section of `CLAUDE.md`. If it is `check_vgv_cli.sh` or `validate_layers.sh`, update its `_test.sh` file. - **An MCP tool is added, renamed, or removed.** Check every skill's `allowed-tools` and the **MCP Integration** section of `README.md`. Nothing validates those names. ## Checks diff --git a/CLAUDE.md b/CLAUDE.md index 055faeb..20d2198 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -7,12 +7,14 @@ `hooks/hooks.json` defines one PostToolUse hook and one PreToolUse hook. -- `Edit|Write` matcher → `validate_layers.sh`. 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 `. 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 `. 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. - 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`. A validator failure other than exit 2 is reported to stderr but never blocks. +- 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. + +`hooks/validate_layers_test.sh` covers the PostToolUse hook against the same fixtures and runs in CI under the **Hook Tests** job.