feat(hooks): analyze and format every file an apply_patch touched

.github/workflows/ci.yaml

@@ -82,3 +82,7 @@ jobs:
         run: bash hooks/scripts/check-vgv-cli_test.sh
       - name: Warn missing MCP hook tests
         run: bash hooks/scripts/warn-missing-mcp_test.sh
+      - name: Analyze hook tests
+        run: bash hooks/scripts/analyze_test.sh
+      - name: Format hook tests
+        run: bash hooks/scripts/format_test.sh

README.md

@@ -73,8 +73,8 @@ This plugin includes SessionStart, PreToolUse, and PostToolUse hooks that valida
 | **Check VGV CLI** (`check-vgv-cli.sh`) | PreToolUse (`mcp__.*very-good-cli__.*`) | Verifies from the payload that the caller is a Very Good CLI tool and stands aside otherwise, so the decision never lands on an unrelated tool; for its own tools, auto-approves the call in every run mode via a PreToolUse `allow` decision, so they never dead-end when the tool isn't on `permissions.allow` (including under `skipAutoPermissionPrompt`); denies with an install/upgrade message if the CLI is missing or < 1.3.0 |
 | **Block CLI Workarounds** (`block-cli-workarounds.sh`) | PreToolUse (`Bash`) | Blocks direct CLI bypass of Very Good CLI commands through the host's shell tool; inspects the command only when the payload identifies a shell call, so an unrelated tool carrying a `command` argument is left alone; when the CLI is present but cannot run because `dart` is missing from `PATH`, the denial says so instead of redirecting to an MCP server that cannot start; exits 2 on failure (blocking) |
 | **Allow Read-only Git** (`allow-readonly-git.sh`) | PreToolUse (`Bash`, `flutter-reviewer` agent only) | Restricts the `flutter-reviewer` agent's Bash to single-line `git diff`/`git status`, without the `--output` or `--ext-diff` options, and denies anything else (blocking). Scoped via the agent's frontmatter, not `hooks.json` |
-| **Analyze** (`analyze.sh`) | PostToolUse (`Edit`/`Write`) | Runs `dart analyze` on the modified `.dart` file; exits 2 on failure (blocking — Claude must fix issues before continuing) |
-| **Format** (`format.sh`) | PostToolUse (`Edit`/`Write`) | Runs `dart format` on the modified `.dart` file; always exits 0 (non-blocking — formatting is applied silently) |
+| **Analyze** (`analyze.sh`) | PostToolUse (`Edit`/`Write`) | Runs `dart analyze` on each modified `.dart` file; exits 2 on failure (blocking — the agent must fix issues before continuing). Reads the file from `tool_input.file_path` (Claude Code) or from the `A`/`M` lines of an `apply_patch` response (Codex), so one patch touching several files analyzes each |
+| **Format** (`format.sh`) | PostToolUse (`Edit`/`Write`) | Runs `dart format` on each modified `.dart` file, found the same way as Analyze; always exits 0 (non-blocking — formatting is applied silently) |
 
 ### Prerequisites
 

hooks/scripts/analyze.sh

@@ -10,16 +10,18 @@ if ! command -v jq &>/dev/null; then
   exit 0
 fi
 
-# Extract file path from the tool input
-file_path=$(jq -r '.tool_input.file_path // empty' <<< "$input")
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+source "$SCRIPT_DIR/vgv-cli-common.sh"
 
-# Skip if no file path or not a Dart file
-if [[ -z "$file_path" || "$file_path" != *.dart ]]; then
-  exit 0
-fi
+# Run dart analyze on each modified Dart file. A failure on any file blocks, but every
+# file is still analyzed so the agent sees all of the issues at once.
+status=0
+while IFS= read -r file_path; do
+  [[ "$file_path" == *.dart ]] || continue
+  output=$(dart analyze "$file_path" 2>&1) || {
+    echo "$output" >&2
+    status=2
+  }
+done < <(payload_file_paths <<< "$input")
 
-# Run dart analyze on the single file
-output=$(dart analyze "$file_path" 2>&1) || {
-  echo "$output" >&2
-  exit 2
-}
+exit "$status"

hooks/scripts/analyze_test.sh

@@ -0,0 +1,133 @@
+#!/bin/bash
+# Tests for analyze.sh
+#
+# Usage: bash hooks/scripts/analyze_test.sh
+#
+# The hook reads a JSON payload from stdin and runs `dart analyze` on each Dart file the
+# payload names. Claude Code names one in tool_input.file_path; Codex's apply_patch names
+# each in tool_response as "A path" / "M path", relative to cwd. Every case runs against a
+# stubbed dart on a PATH that contains nothing else, and asserts on exactly which files the
+# stub was asked to analyze, so results do not depend on a Dart SDK being installed.
+
+set -euo pipefail
+
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+HOOK="$SCRIPT_DIR/analyze.sh"
+
+# Invoked by absolute path: `env -i PATH=...` resolves the command it runs with the PATH
+# it was just given, and the no-jq PATH below deliberately holds almost nothing.
+BASH_BIN="$(command -v bash)"
+
+PASSED=0
+FAILED=0
+
+STUB_DIR="$(mktemp -d)"
+trap 'rm -rf "$STUB_DIR"' EXIT
+
+BASE_PATH="$(dirname "$(command -v jq)"):/usr/bin:/bin"
+
+# A stubbed dart that logs its arguments, one call per line, and exits with the status in
+# dart.exit (default 0). On a non-zero exit it prints an issue the way `dart analyze` does.
+cat > "$STUB_DIR/dart" <<'STUB'
+#!/bin/sh
+echo "$*" >> "$(dirname "$0")/dart.log"
+status=$(cat "$(dirname "$0")/dart.exit" 2>/dev/null || echo 0)
+[ "$status" -ne 0 ] && echo "  error - lib/a.dart:1:1 - Undefined name 'x'. - undefined_identifier"
+exit "$status"
+STUB
+chmod +x "$STUB_DIR/dart"
+
+dart_exits() { echo "$1" > "$STUB_DIR/dart.exit"; }
+
+# A PATH with cat, the one coreutil the hook uses before its jq check, and nothing else.
+# Built from a symlink rather than named as /bin, because on a merged-/usr Linux /bin is
+# /usr/bin and so still has jq on it.
+NOJQ_DIR="$STUB_DIR/nojq"
+mkdir -p "$NOJQ_DIR" && ln -s "$(command -v cat)" "$NOJQ_DIR/cat"
+NOJQ_PATH="$STUB_DIR:$NOJQ_DIR"
+
+# Run the hook on a payload. Leaves the exit status in LAST_STATUS, stderr in LAST_STDERR,
+# and every `dart` invocation (one per line, e.g. "analyze /w/lib/a.dart") in LAST_CALLS.
+LAST_STATUS=0
+LAST_STDERR=""
+LAST_CALLS=""
+run_hook() {
+  local payload="$1" path="${2:-$STUB_DIR:$BASE_PATH}"
+  : > "$STUB_DIR/dart.log"
+  LAST_STATUS=0
+  LAST_STDERR=$(printf '%s' "$payload" | env -i PATH="$path" "$BASH_BIN" "$HOOK" 2>&1 >/dev/null) || LAST_STATUS=$?
+  LAST_CALLS=$(cat "$STUB_DIR/dart.log")
+}
+
+pass() { printf "  \033[32mPASS\033[0m  %s\n" "$1"; PASSED=$((PASSED + 1)); }
+fail() { printf "  \033[31mFAIL\033[0m  %s\n    %s\n" "$1" "$2"; FAILED=$((FAILED + 1)); }
+
+# Usage: assert_calls <label> <expected calls, newline separated>
+assert_calls() {
+  local label="$1" expected="$2"
+  if [ "$LAST_CALLS" = "$expected" ]; then pass "$label"; else fail "$label" "dart was called with: $(printf '%s' "$LAST_CALLS" | tr '\n' '|')"; fi
+}
+
+# Usage: assert_status <label> <expected exit>
+assert_status() {
+  local label="$1" expected="$2"
+  if [ "$LAST_STATUS" -eq "$expected" ]; then pass "$label"; else fail "$label" "exit was $LAST_STATUS, expected $expected"; fi
+}
+
+# The real response text Codex sends for an apply_patch, captured from codex-cli 0.153.4.
+codex_response() { printf 'Exit code: 0\nWall time: 0.1 seconds\nOutput:\nSuccess. Updated the following files:\n%s\n' "$1"; }
+
+echo "=== analyze tests ==="
+
+echo ""
+echo "--- Claude Code: one file in tool_input.file_path ---"
+run_hook "$(jq -n '{cwd:"/w", tool_name:"Edit", tool_input:{file_path:"/w/lib/a.dart", old_string:"x", new_string:"y"}, tool_response:{filePath:"/w/lib/a.dart", success:true}}')"
+assert_calls  "analyzes the edited file"                         "analyze /w/lib/a.dart"
+assert_status "exits 0 when analysis passes"                     0
+
+run_hook "$(jq -n '{cwd:"/w", tool_name:"Write", tool_input:{file_path:"/w/README.md", content:"# hi"}, tool_response:{success:true}}')"
+assert_calls  "skips a file that is not Dart"                    ""
+assert_status "exits 0 for a non-Dart file"                      0
+
+echo ""
+echo "--- Codex: files named in tool_response ---"
+run_hook "$(jq -n --arg r "$(codex_response 'M /abs/lib/x.dart')" '{cwd:"/w", tool_name:"apply_patch", tool_input:{command:"*** Begin Patch\n*** Update File: /abs/lib/x.dart\n*** End Patch"}, tool_response:$r}')"
+assert_calls  "analyzes an absolute path as given"              "analyze /abs/lib/x.dart"
+
+run_hook "$(jq -n --arg r "$(codex_response 'A lib/new.dart')" '{cwd:"/w", tool_name:"apply_patch", tool_input:{command:"..."}, tool_response:$r}')"
+assert_calls  "resolves a relative path against the payload cwd" "analyze /w/lib/new.dart"
+
+run_hook "$(jq -n --arg r "$(codex_response $'A lib/new.dart\nM lib/old.dart\nD lib/gone.dart\nM lib/renamed.dart')" '{cwd:"/w", tool_name:"apply_patch", tool_input:{command:"..."}, tool_response:$r}')"
+assert_calls  "analyzes added, modified and renamed files, not deleted" $'analyze /w/lib/new.dart\nanalyze /w/lib/old.dart\nanalyze /w/lib/renamed.dart'
+
+run_hook "$(jq -n --arg r "$(codex_response $'M lib/a.dart\nM lib/b.md')" '{cwd:"/w", tool_name:"apply_patch", tool_input:{command:"..."}, tool_response:$r}')"
+assert_calls  "skips a non-Dart file in a patch"                 "analyze /w/lib/a.dart"
+
+echo ""
+echo "--- Payloads that name no file ---"
+run_hook "$(jq -n '{cwd:"/w", tool_name:"Bash", tool_input:{command:"ls"}, tool_response:"Exit code: 0\nOutput:\nlib\n"}')"
+assert_calls  "runs nothing for a shell call"                    ""
+assert_status "exits 0 for a shell call"                         0
+
+echo ""
+echo "--- Failures block ---"
+dart_exits 1
+run_hook "$(jq -n '{cwd:"/w", tool_input:{file_path:"/w/lib/a.dart"}}')"
+assert_status "exits 2 when analysis fails"                      2
+if [[ "$LAST_STDERR" == *"undefined_identifier"* ]]; then pass "forwards the analyzer output on stderr"; else fail "forwards the analyzer output on stderr" "stderr was: $LAST_STDERR"; fi
+
+run_hook "$(jq -n --arg r "$(codex_response $'M lib/a.dart\nM lib/b.dart')" '{cwd:"/w", tool_input:{command:"..."}, tool_response:$r}')"
+assert_calls  "still analyzes every file after one fails"        $'analyze /w/lib/a.dart\nanalyze /w/lib/b.dart'
+assert_status "exits 2 when any file fails"                      2
+dart_exits 0
+
+echo ""
+echo "--- Without jq ---"
+run_hook "$(jq -n '{tool_input:{file_path:"/w/lib/a.dart"}}')" "$NOJQ_PATH"
+assert_calls  "runs nothing when jq is missing"                  ""
+assert_status "exits 0 when jq is missing"                       0
+if [[ "$LAST_STDERR" == *"jq not found, skipping"* ]]; then pass "says why it skipped"; else fail "says why it skipped" "stderr was: $LAST_STDERR"; fi
+
+echo ""
+echo "=== Results: $PASSED passed, $FAILED failed ==="
+[ "$FAILED" -eq 0 ] || exit 1

hooks/scripts/format.sh

@@ -10,13 +10,11 @@ if ! command -v jq &>/dev/null; then
   exit 0
 fi
 
-# Extract file path from the tool input
-file_path=$(jq -r '.tool_input.file_path // empty' <<< "$input")
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+source "$SCRIPT_DIR/vgv-cli-common.sh"
 
-# Skip if no file path or not a Dart file
-if [[ -z "$file_path" || "$file_path" != *.dart ]]; then
-  exit 0
-fi
-
-# Run dart format on the single file (auto-fix, always exit 0)
-dart format "$file_path" &>/dev/null || true
+# Run dart format on each modified Dart file (auto-fix, always exit 0)
+while IFS= read -r file_path; do
+  [[ "$file_path" == *.dart ]] || continue
+  dart format "$file_path" &>/dev/null || true
+done < <(payload_file_paths <<< "$input")

hooks/scripts/format_test.sh

@@ -0,0 +1,101 @@
+#!/bin/bash
+# Tests for format.sh
+#
+# Usage: bash hooks/scripts/format_test.sh
+#
+# The hook reads a JSON payload from stdin and runs `dart format` on each Dart file the
+# payload names, from tool_input.file_path (Claude Code) or tool_response (Codex's
+# apply_patch). It never blocks. Every case runs against a stubbed dart on a PATH that
+# contains nothing else and asserts on which files the stub was asked to format.
+
+set -euo pipefail
+
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+HOOK="$SCRIPT_DIR/format.sh"
+
+# Invoked by absolute path: `env -i PATH=...` resolves the command it runs with the PATH
+# it was just given, and the no-jq PATH below deliberately holds almost nothing.
+BASH_BIN="$(command -v bash)"
+
+PASSED=0
+FAILED=0
+
+STUB_DIR="$(mktemp -d)"
+trap 'rm -rf "$STUB_DIR"' EXIT
+
+BASE_PATH="$(dirname "$(command -v jq)"):/usr/bin:/bin"
+
+cat > "$STUB_DIR/dart" <<'STUB'
+#!/bin/sh
+echo "$*" >> "$(dirname "$0")/dart.log"
+exit "$(cat "$(dirname "$0")/dart.exit" 2>/dev/null || echo 0)"
+STUB
+chmod +x "$STUB_DIR/dart"
+
+dart_exits() { echo "$1" > "$STUB_DIR/dart.exit"; }
+
+# A PATH with cat, the one coreutil the hook uses before its jq check, and nothing else.
+# Built from a symlink rather than named as /bin, because on a merged-/usr Linux /bin is
+# /usr/bin and so still has jq on it.
+NOJQ_DIR="$STUB_DIR/nojq"
+mkdir -p "$NOJQ_DIR" && ln -s "$(command -v cat)" "$NOJQ_DIR/cat"
+NOJQ_PATH="$STUB_DIR:$NOJQ_DIR"
+
+LAST_STATUS=0
+LAST_CALLS=""
+run_hook() {
+  local payload="$1" path="${2:-$STUB_DIR:$BASE_PATH}"
+  : > "$STUB_DIR/dart.log"
+  LAST_STATUS=0
+  printf '%s' "$payload" | env -i PATH="$path" "$BASH_BIN" "$HOOK" >/dev/null 2>&1 || LAST_STATUS=$?
+  LAST_CALLS=$(cat "$STUB_DIR/dart.log")
+}
+
+pass() { printf "  \033[32mPASS\033[0m  %s\n" "$1"; PASSED=$((PASSED + 1)); }
+fail() { printf "  \033[31mFAIL\033[0m  %s\n    %s\n" "$1" "$2"; FAILED=$((FAILED + 1)); }
+
+assert_calls() {
+  local label="$1" expected="$2"
+  if [ "$LAST_CALLS" = "$expected" ]; then pass "$label"; else fail "$label" "dart was called with: $(printf '%s' "$LAST_CALLS" | tr '\n' '|')"; fi
+}
+
+assert_status() {
+  local label="$1" expected="$2"
+  if [ "$LAST_STATUS" -eq "$expected" ]; then pass "$label"; else fail "$label" "exit was $LAST_STATUS, expected $expected"; fi
+}
+
+codex_response() { printf 'Exit code: 0\nWall time: 0.1 seconds\nOutput:\nSuccess. Updated the following files:\n%s\n' "$1"; }
+
+echo "=== format tests ==="
+
+echo ""
+echo "--- Claude Code ---"
+run_hook "$(jq -n '{cwd:"/w", tool_name:"Edit", tool_input:{file_path:"/w/lib/a.dart"}, tool_response:{success:true}}')"
+assert_calls  "formats the edited file"                          "format /w/lib/a.dart"
+assert_status "exits 0"                                          0
+
+run_hook "$(jq -n '{cwd:"/w", tool_input:{file_path:"/w/pubspec.yaml"}}')"
+assert_calls  "skips a file that is not Dart"                    ""
+
+echo ""
+echo "--- Codex ---"
+run_hook "$(jq -n --arg r "$(codex_response $'A lib/new.dart\nM lib/old.dart\nD lib/gone.dart')" '{cwd:"/w", tool_name:"apply_patch", tool_input:{command:"..."}, tool_response:$r}')"
+assert_calls  "formats added and modified files, not deleted, relative to cwd" $'format /w/lib/new.dart\nformat /w/lib/old.dart'
+assert_status "exits 0"                                          0
+
+echo ""
+echo "--- Never blocks ---"
+dart_exits 1
+run_hook "$(jq -n '{cwd:"/w", tool_input:{file_path:"/w/lib/a.dart"}}')"
+assert_status "exits 0 even when dart format fails"              0
+dart_exits 0
+
+echo ""
+echo "--- Without jq ---"
+run_hook "$(jq -n '{tool_input:{file_path:"/w/lib/a.dart"}}')" "$NOJQ_PATH"
+assert_calls  "runs nothing when jq is missing"                  ""
+assert_status "exits 0 when jq is missing"                       0
+
+echo ""
+echo "=== Results: $PASSED passed, $FAILED failed ==="
+[ "$FAILED" -eq 0 ] || exit 1

hooks/scripts/vgv-cli-common.sh

@@ -98,3 +98,28 @@ check_vgv_cli() {
   fi
   echo "ok"
 }
+
+# The files a tool call touched, one absolute path per line.
+#
+# Claude Code's Edit and Write name the one file in tool_input.file_path. Codex edits
+# through apply_patch instead: it sends the whole patch as text and names each file only
+# in tool_response, so only after the tool has run, as "A path" for an added file or
+# "M path" for a modified one. A
+# rename reports its new name as "M". A deleted file reports "D path" and is left out,
+# since there is nothing on disk to analyze or format. Those paths are relative to cwd
+# when the model wrote them that way, so each is resolved against the payload's cwd.
+#
+# tool_response is an object on Claude Code and a string on Codex. `strings` keeps only
+# the latter, so the scan never runs against an object.
+payload_file_paths() {
+  jq -r '
+    (.cwd // ".") as $cwd
+    | def absolute: if startswith("/") then . else $cwd + "/" + . end;
+    [
+      (.tool_input.file_path // empty | absolute),
+      ((.tool_response | strings) // "" | [scan("(?m)^[AM] (.+)$")[]] | .[] | absolute)
+    ]
+    | unique
+    | .[]
+  '
+}