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