From 245b6469338dc8c596a7ccc8ac241ee9913df039 Mon Sep 17 00:00:00 2001 From: Dominik Simonik Date: Tue, 6 Oct 2026 10:23:29 +0200 Subject: [PATCH] feat(hooks): let analyze and format take their files as arguments The scripts read the edited file from tool_input.file_path, which is how Claude Code reports an edit. A host that reports edits differently can now pass the files as arguments instead, through the shared hook_file_paths helper, so the scripts never learn a second payload shape. Both handle several files per run, and analyze reports every failure before exiting 2. Both gain tests that stub dart and assert which files it was asked to handle, like the existing suites stub very_good. Co-Authored-By: Claude Fable 5.1 --- .github/workflows/ci.yaml | 4 + README.md | 4 +- hooks/scripts/analyze.sh | 34 ++++---- hooks/scripts/analyze_test.sh | 136 ++++++++++++++++++++++++++++++++ hooks/scripts/format.sh | 25 +++--- hooks/scripts/format_test.sh | 105 ++++++++++++++++++++++++ hooks/scripts/vgv-cli-common.sh | 12 +++ 7 files changed, 287 insertions(+), 33 deletions(-) create mode 100755 hooks/scripts/analyze_test.sh create mode 100755 hooks/scripts/format_test.sh diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index edfc10f..c54c36e 100644 --- a/.github/workflows/ci.yaml +++ b/.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 diff --git a/README.md b/README.md index d5981e5..fa38601 100644 --- a/README.md +++ b/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 — Claude must fix issues before continuing). Takes the files from `tool_input.file_path`, or as arguments when a host that reports edits differently passes them that way | +| **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 diff --git a/hooks/scripts/analyze.sh b/hooks/scripts/analyze.sh index bba41ca..39c8130 100755 --- a/hooks/scripts/analyze.sh +++ b/hooks/scripts/analyze.sh @@ -1,25 +1,25 @@ #!/bin/bash +# PostToolUse hook: run `dart analyze` on each edited Dart file and block on any issue. +# The files come from the arguments, or from the payload when there are none; see +# hook_file_paths in vgv-cli-common.sh. set -euo pipefail -# Read the hook payload from stdin -input=$(cat) +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +source "$SCRIPT_DIR/vgv-cli-common.sh" -# Check jq availability -if ! command -v jq &>/dev/null; then +# Only the payload needs jq to read +if [ "$#" -eq 0 ] && ! command -v jq &>/dev/null; then echo "analyze hook: jq not found, skipping" >&2 exit 0 fi -# Extract file path from the tool input -file_path=$(jq -r '.tool_input.file_path // empty' <<< "$input") - -# 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 the single file -output=$(dart analyze "$file_path" 2>&1) || { - echo "$output" >&2 - exit 2 -} \ No newline at end of file +# Analyze every Dart file, so one run reports every issue, then block if any failed +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 < <(hook_file_paths "$@") +exit "$status" diff --git a/hooks/scripts/analyze_test.sh b/hooks/scripts/analyze_test.sh new file mode 100755 index 0000000..f020030 --- /dev/null +++ b/hooks/scripts/analyze_test.sh @@ -0,0 +1,136 @@ +#!/bin/bash +# Tests for analyze.sh +# +# Usage: bash hooks/scripts/analyze_test.sh +# +# The hook runs `dart analyze` on each Dart file it is given: the paths passed as +# arguments, or tool_input.file_path from the JSON payload on stdin when there are none. +# 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 the coreutils the hook needs besides jq, and nothing else. Built from +# symlinks 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" +for tool in cat dirname; do ln -s "$(command -v "$tool")" "$NOJQ_DIR/$tool"; done +NOJQ_PATH="$STUB_DIR:$NOJQ_DIR" + +# Run the hook on a payload, with any hook arguments after the PATH. 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}" + shift; [ "$#" -eq 0 ] || shift + : > "$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