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..f2aa216 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 — 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 diff --git a/hooks/scripts/analyze.sh b/hooks/scripts/analyze.sh index bba41ca..178850a 100755 --- a/hooks/scripts/analyze.sh +++ b/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 -} \ No newline at end of file +exit "$status" diff --git a/hooks/scripts/analyze_test.sh b/hooks/scripts/analyze_test.sh new file mode 100755 index 0000000..a8f7297 --- /dev/null +++ b/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