From 5c7f3457ec613ed3749c0e23b163bee9d23ceb79 Mon Sep 17 00:00:00 2001 From: Oscar Martin Date: Tue, 30 Jun 2026 11:57:55 +0200 Subject: [PATCH 1/5] feat: add flutter-reviewer read-only subagent --- .github/workflows/ci.yaml | 13 +++ .gitignore | 3 +- CLAUDE.md | 37 ++++++- README.md | 9 ++ agents/flutter-reviewer.md | 126 +++++++++++++++++++++++ config/cspell.json | 5 + hooks/scripts/allow-readonly-git.sh | 40 +++++++ hooks/scripts/allow-readonly-git_test.sh | 87 ++++++++++++++++ test/agents_tools_guard.sh | 92 +++++++++++++++++ test/agents_tools_guard_test.sh | 115 +++++++++++++++++++++ 10 files changed, 525 insertions(+), 2 deletions(-) create mode 100644 agents/flutter-reviewer.md create mode 100755 hooks/scripts/allow-readonly-git.sh create mode 100755 hooks/scripts/allow-readonly-git_test.sh create mode 100755 test/agents_tools_guard.sh create mode 100755 test/agents_tools_guard_test.sh diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index ece6a46..41f662e 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -85,3 +85,16 @@ jobs: run: npm install -g @anthropic-ai/claude-code - name: Validate Plugin run: claude plugin validate . + shell-tests: + name: 🛡️ Shell Tests + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - name: Agent tools guard + run: bash test/agents_tools_guard.sh + - name: Agent tools guard self-test + run: bash test/agents_tools_guard_test.sh + - name: Read-only git hook tests + run: bash hooks/scripts/allow-readonly-git_test.sh + - name: Block CLI workarounds hook tests + run: bash hooks/scripts/block-cli-workarounds_test.sh diff --git a/.gitignore b/.gitignore index 5cf1910..c69aafa 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ .DS_Store *.log -node_modules/ \ No newline at end of file +node_modules/ +docs/ diff --git a/CLAUDE.md b/CLAUDE.md index e9ba6f6..06e305f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -12,17 +12,22 @@ VGV AI Flutter Plugin is a Claude Code plugin that provides best-practices skill .mcp.json # MCP server configuration (Dart and Very Good CLI) .claude-plugin/ plugin.json # Plugin manifest (name, version, keywords) +agents/ + flutter-reviewer.md # Read-only Flutter code reviewer subagent docs/ plan/ # Planning and design documents hooks/ hooks.json # Hook definitions (PreToolUse and PostToolUse) scripts/ + allow-readonly-git.sh # Restricts flutter-reviewer Bash to git diff/status analyze.sh # Runs dart analyze on modified .dart files block-cli-workarounds.sh # Prevents direct CLI bypass via Bash check-vgv-cli.sh # Validates VGV CLI installed and >= 1.3.0 format.sh # Runs dart format on modified .dart files vgv-cli-common.sh # Shared utilities for VGV CLI hook scripts warn-missing-mcp.sh # Warns at session start if VGV CLI is missing/outdated +test/ + agents_tools_guard.sh # Asserts agent tools frontmatter has no write tool skills/ accessibility/SKILL.md accessibility/references/ @@ -83,6 +88,30 @@ Every `SKILL.md` follows this structure: 5. Add any new domain terms to the `words` list in `config/cspell.json` 6. Update the repository structure in `CLAUDE.md` +## Adding a New Agent + +Agents are subagents that Claude Code dispatches as isolated, specialized helpers (e.g., reviewers). +They live in `agents/.md` at the plugin root and are **auto-discovered** — unlike skills, no +`.claude-plugin/plugin.json` change is required. An `agents/.md` file registers as +`vgv-ai-flutter-plugin:`. + +1. Create `agents/.md` with YAML frontmatter: + - `name` _(required)_ — must match the file name; lowercase letters, numbers, and hyphens only + - `description` _(required)_ — when Claude should dispatch the agent + - `tools` _(optional)_ — comma-separated bare tool names. The `tools` field cannot scope Bash by + command; for a read-only agent, omit write tools (`Edit`, `Write`, `NotebookEdit`) and restrict + Bash with an agent-scoped PreToolUse hook (see `flutter-reviewer.md`) + - `skills` _(optional)_ — bare skill names to preload at startup (full skill content is injected) + - `model` _(optional)_ — `inherit` to use the session model + - `hooks` _(optional)_ — agent-scoped hooks, e.g. a PreToolUse `Bash` hook +2. Add an **Agents** table row in `README.md` (agent name links to the `agents/.md` file) +3. Add any new domain terms to the `words` list in `config/cspell.json` +4. Update the repository structure in `CLAUDE.md` + +The `test/agents_tools_guard.sh` guard runs in CI and automatically scans every `agents/*.md`: it +fails if any agent grants a write tool (`Edit`/`Write`/`NotebookEdit`), or grants `Bash` without +referencing the `allow-readonly-git.sh` hook. No per-agent wiring is needed. + ## Maintaining Existing Skills, Hooks, and MCP Tools Most documentation drift comes from changing existing assets without updating the @@ -116,7 +145,13 @@ These run **before** a tool call is executed: - `mcp__very-good-cli__.*` matcher → `check-vgv-cli.sh` — validates that the Very Good CLI is installed and at version >= 1.3.0; exits 2 on failure (blocking) - `Bash` matcher → `block-cli-workarounds.sh` — prevents direct CLI bypass of VGV CLI commands through the Bash tool; exits 2 on failure (blocking) -Both PreToolUse scripts share common utilities from `vgv-cli-common.sh`. +The first two PreToolUse hooks are plugin-level (defined in `hooks.json`) and share common utilities +from `vgv-cli-common.sh`. The following hook is **agent-scoped** — it is declared in the +`flutter-reviewer` agent's frontmatter, not in `hooks.json`, so it only fires for that agent: + +- `Bash` matcher → `allow-readonly-git.sh` — restricts the `flutter-reviewer` agent's Bash to + `git diff` / `git status` only; exits 2 on anything else, including compound-command bypass + (blocking). Enforces the agent's read-only contract. ### PostToolUse Hooks diff --git a/README.md b/README.md index 8d143af..29a7e63 100644 --- a/README.md +++ b/README.md @@ -54,6 +54,14 @@ For more details, see the [Very Good Claude Marketplace][marketplace_link]. | [**Dart/Flutter SDK Upgrade**](skills/dart-flutter-sdk-upgrade/SKILL.md) | Bump Dart and Flutter SDK constraints across packages — CI workflow versions, pubspec.yaml environment constraints, and PR preparation for SDK upgrades | | [**Very Good Analysis Upgrade**](skills/very-good-analysis-upgrade/SKILL.md) | Upgrade the `very_good_analysis` lint package across Dart/Flutter projects — version bump in `pubspec.yaml`, minimal lint fixes for new rules, and PR preparation | +## Agents + +This plugin ships subagents that Claude Code can dispatch as isolated, specialized reviewers. Unlike skills, agents are **not** invoked as slash commands — Claude dispatches them automatically, or you can ask Claude to run one by name (e.g. "review my changes with the flutter-reviewer agent"). They also pair with the [Wingspan][marketplace_link] adversarial-critic round, which can dispatch them. + +| Agent | Description | +| ----- | ----------- | +| [**Flutter Reviewer**](agents/flutter-reviewer.md) | Read-only reviewer of changed Dart code against the preloaded `bloc`, `testing`, `static-security`, and `accessibility` standards — emits a `location \| problem \| fix \| standard` findings table. Never edits files; Bash is hook-restricted to `git diff`/`git status` | + ## Hooks This plugin includes SessionStart, PreToolUse, and PostToolUse hooks that validate the Very Good CLI, guard against CLI bypass, and automatically run Dart analysis and formatting on `.dart` files. @@ -63,6 +71,7 @@ This plugin includes SessionStart, PreToolUse, and PostToolUse hooks that valida | **Warn Missing MCP** (`warn-missing-mcp.sh`) | SessionStart | Warns if the Very Good CLI is missing or older than 1.3.0; non-blocking | | **Check VGV CLI** (`check-vgv-cli.sh`) | PreToolUse (`mcp__very-good-cli__.*`) | Validates the Very Good CLI is installed and ≥ 1.3.0; exits 2 on failure (blocking) | | **Block CLI Workarounds** (`block-cli-workarounds.sh`) | PreToolUse (`Bash`) | Blocks direct CLI bypass of Very Good CLI commands through the Bash tool; 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 `git diff`/`git status`; exits 2 on 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) | diff --git a/agents/flutter-reviewer.md b/agents/flutter-reviewer.md new file mode 100644 index 0000000..3a13aac --- /dev/null +++ b/agents/flutter-reviewer.md @@ -0,0 +1,126 @@ +--- +name: flutter-reviewer +description: > + Read-only Flutter code reviewer. Dispatch after writing or changing Dart code, or from the + Wingspan adversarial-critic round, to review changed code against VGV bloc, testing, security, + and accessibility standards. Never edits files. +tools: Read, Glob, Grep, Bash, mcp__dart__analyze_files +skills: + - bloc + - testing + - static-security + - accessibility +model: inherit +hooks: + PreToolUse: + - matcher: "Bash" + hooks: + - type: command + command: "bash ${CLAUDE_PLUGIN_ROOT}/hooks/scripts/allow-readonly-git.sh" +--- + +# Flutter Reviewer Agent + +You are a read-only Flutter code reviewer for Very Good Ventures. You review changed Dart code +against four preloaded VGV standards and report findings as a markdown table. You pair with the +Wingspan adversarial-critic round, which dispatches you and consumes your table verbatim. + +## Read-only contract + +You **never** edit files. You have no `Edit`, `Write`, or `NotebookEdit` tools, and you do not need +them. Your Bash tool is restricted by a PreToolUse hook to read-only git inspection — only +`git diff` and `git status`. Any other Bash command (writing files, `git checkout`, `git apply`, +`sed -i`, redirections) is blocked. Do not attempt to work around this; it is intentional. + +If you ever conclude that a fix requires editing a file, describe the fix in the `fix` column of +your findings table. Do not apply it. + +## Preloaded standards + +The full content of four VGV skills is injected into your context at startup. These are your only +standards source: + +- **`bloc`** — Bloc/Cubit state management conventions. +- **`testing`** — unit, widget, and golden test conventions. +- **`static-security`** — Flutter static security review. +- **`accessibility`** — WCAG-aligned Flutter accessibility. + +Every finding you report must trace back to one of these four standards. If a problem does not map +to one of them, do not report it (see "What not to report"). + +## Diff scoping + +Scope your review to changed Dart code only. Never review the whole repository. + +Determine the change set adaptively, from the repository root: + +1. **Uncommitted changes first.** Run `git status` and `git diff` (staged and unstaged). If there + are uncommitted `.dart` changes, review those. +2. **Otherwise, branch-vs-base.** If the working tree is clean, fall back to the branch's changes + against its merge base: `git diff ...HEAD` (typically `main...HEAD`). Use `git status` and + `git diff` to enumerate the changed files. +3. **Include untracked `.dart` files.** `git status` surfaces untracked files; review untracked + `.dart` files as new code. +4. **Monorepo / subdirectory.** Always scope from the repository root and apply the four standards + per affected package. + +Read the changed files with `Read`/`Grep` to review their full context, not just the diff hunks. +You may use `mcp__dart__analyze_files` to corroborate a skill-based judgment, but analyzer output is +not itself a findings source (see "What not to report"). + +### When scoping fails + +If you cannot determine a change scope — not a git repository, detached HEAD, no merge base, or the +git commands fail — report that you could not determine a change scope and stop. Do not guess and do +not review the whole repository. + +## Output + +Report findings as a single markdown table, one row per finding: + +```markdown +| location | problem | fix | standard | +| --------------------------------- | ---------------------------------------- | ------------------------------------ | -------------- | +| lib/counter/counter_cubit.dart:12 | Mutable state field breaks immutability | Mark state class fields `final` | bloc | +| test/counter/counter_test.dart:30 | Tautological assertion `expect(x, x)` | Assert against the expected value | testing | +``` + +Rules: + +- `location` — `path:line` of the finding. +- `problem` — what is wrong, concisely. +- `fix` — the change you recommend. Describe it; never apply it. +- `standard` — exactly one of `bloc`, `testing`, `static-security`, `accessibility`. Every row must + name one of these four. +- Align the pipe characters vertically (VGV markdown convention). + +### No changed Dart files + +If the change scope contains no `.dart` files (clean tree, or only non-Dart changes), report +`No changed Dart files to review.` and stop. Never emit an empty table and never invent findings. + +### Out-of-domain changes + +Your four standards do not cover every domain. If changed Dart code touches areas outside them — +for example navigation, theming, internationalization, or layered architecture — you have no loaded +standard to cite, so you stay silent on findings there. Add a one-line note after the table listing +the changed areas that fall outside your four standards, so a clean review is not mistaken for full +coverage. For example: + +> Note: changes in `lib/routing/` and `lib/theme/` are outside the loaded standards (bloc, testing, +> static-security, accessibility) and were not reviewed. + +### What not to report + +- **Analyzer-only findings.** Raw `dart analyze` errors (unused imports, dead null-aware operators, + etc.) do not trace to any of your four loaded standards, so they are out of scope for your table. + Do not report them and do not introduce a `dart-analyzer` pseudo-standard. Such errors are caught + separately by the plugin's PostToolUse `analyze.sh` hook when code is written, not here. Use the + analyzer only to corroborate a skill-based judgment. +- **Untraceable findings.** If a finding cannot name one of the four loaded standards, omit it. + +## Wingspan dispatch contract + +When dispatched from the Wingspan adversarial-critic round, you self-scope via the adaptive diff +procedure above — the caller does not pass you a file list — and Wingspan consumes your findings +table verbatim. diff --git a/config/cspell.json b/config/cspell.json index 1ef1fba..3c631a9 100644 --- a/config/cspell.json +++ b/config/cspell.json @@ -2,6 +2,7 @@ "language": "en", "words": [ "activatable", + "adversarial", "antipattern", "Automator", "Bidirectionality", @@ -38,13 +39,17 @@ "pasteable", "prefs", "pubspec", + "redirections", "serialization", "stdio", + "subagent", + "subagents", "subclassing", "vsync", "WCAG", "widgetbook", "Widgetbook", + "Wingspan", "xxlg" ], "flagWords": [] diff --git a/hooks/scripts/allow-readonly-git.sh b/hooks/scripts/allow-readonly-git.sh new file mode 100755 index 0000000..33811b7 --- /dev/null +++ b/hooks/scripts/allow-readonly-git.sh @@ -0,0 +1,40 @@ +#!/bin/bash +# PreToolUse hook (flutter-reviewer agent): restrict Bash to read-only git inspection. +# Allows only `git diff` and `git status`. Denies everything else (file writes, +# git checkout/apply, redirections, compound-command bypass). +# +# Uses the shared deny() helper (JSON permissionDecision) for consistency with +# the other PreToolUse Bash hook (block-cli-workarounds.sh). + +# Skip gracefully if jq is unavailable, matching the repo convention. +if ! command -v jq &>/dev/null; then + exit 0 +fi + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +source "$SCRIPT_DIR/vgv-cli-common.sh" + +DENY_REASON="flutter-reviewer is read-only: only 'git diff' and 'git status' are allowed." + +INPUT=$(cat) +COMMAND=$(echo "$INPUT" | jq -r '.tool_input.command // empty') + +if [ -z "$COMMAND" ]; then + exit 0 +fi + +# Reject shell operators outright. A compound command (`;`, `&&`, `||`, `|`, +# redirections, command substitution) could smuggle a mutating command past a +# first-token check, so anything not a single bare git command is denied. +case "$COMMAND" in + *";"* | *"&"* | *"|"* | *">"* | *"<"* | *'`'* | *'$('*) + deny "$DENY_REASON" + ;; +esac + +# Allow only `git diff …` and `git status …` (with optional leading whitespace). +if echo "$COMMAND" | grep -Eq '^[[:space:]]*git[[:space:]]+(diff|status)([[:space:]]|$)'; then + exit 0 +fi + +deny "$DENY_REASON" diff --git a/hooks/scripts/allow-readonly-git_test.sh b/hooks/scripts/allow-readonly-git_test.sh new file mode 100755 index 0000000..097ea1a --- /dev/null +++ b/hooks/scripts/allow-readonly-git_test.sh @@ -0,0 +1,87 @@ +#!/bin/bash +# Tests for allow-readonly-git.sh +# +# Usage: bash hooks/scripts/allow-readonly-git_test.sh +# +# The hook reads a JSON payload from stdin containing tool_input.command, +# then prints a deny JSON on stdout if denied, or exits silently if allowed. +# We check stdout for the deny marker to determine the result. + +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +HOOK="$SCRIPT_DIR/allow-readonly-git.sh" + +PASSED=0 +FAILED=0 + +run_hook() { + local cmd="$1" + local payload + payload=$(jq -n --arg c "$cmd" '{"tool_input":{"command":$c}}') + local output + output=$(echo "$payload" | bash "$HOOK" 2>/dev/null) || true + if echo "$output" | grep -q '"permissionDecision"'; then + echo "blocked" + else + echo "allowed" + fi +} + +assert_blocked() { + local cmd="$1" + local result + result=$(run_hook "$cmd") + if [ "$result" = "blocked" ]; then + printf " \033[32mPASS\033[0m blocked: %s\n" "$cmd" + PASSED=$((PASSED + 1)) + else + printf " \033[31mFAIL\033[0m expected blocked but allowed: %s\n" "$cmd" + FAILED=$((FAILED + 1)) + fi +} + +assert_allowed() { + local cmd="$1" + local result + result=$(run_hook "$cmd") + if [ "$result" = "allowed" ]; then + printf " \033[32mPASS\033[0m allowed: %s\n" "$cmd" + PASSED=$((PASSED + 1)) + else + printf " \033[31mFAIL\033[0m expected allowed but blocked: %s\n" "$cmd" + FAILED=$((FAILED + 1)) + fi +} + +echo "=== allow-readonly-git tests ===" +echo "" +echo "--- Should be ALLOWED ---" +assert_allowed "git diff" +assert_allowed "git status" +assert_allowed "git status -s" +assert_allowed "git diff --stat" +assert_allowed "git diff main...HEAD" +assert_allowed " git diff HEAD~1" + +echo "" +echo "--- Should be BLOCKED ---" +assert_blocked "git checkout ." +assert_blocked "git apply patch.diff" +assert_blocked "git commit -m wip" +assert_blocked "git diff > out.txt" +assert_blocked "git status; rm -rf x" +assert_blocked "git diff && rm x" +assert_blocked "git diff | tee out.txt" +assert_blocked 'git diff $(rm x)' +assert_blocked "rm -rf /" +assert_blocked "sed -i s/a/b/ file" +assert_blocked "echo hi > file" +assert_blocked "diff a b" + +echo "" +echo "=== Results: $PASSED passed, $FAILED failed ===" + +if [ "$FAILED" -gt 0 ]; then + exit 1 +fi diff --git a/test/agents_tools_guard.sh b/test/agents_tools_guard.sh new file mode 100755 index 0000000..8888ac2 --- /dev/null +++ b/test/agents_tools_guard.sh @@ -0,0 +1,92 @@ +#!/bin/bash +# Guard test: enforce the read-only contract for agents. +# +# For every agents/*.md: +# 1. The `tools` frontmatter must not grant a write tool (Edit, Write, +# NotebookEdit). Both the inline form (`tools: Read, Glob`) and the YAML +# block-list form (`tools:\n - Read\n - Glob`) are parsed. +# 2. If the agent grants Bash, its frontmatter must reference the read-only +# git hook (allow-readonly-git.sh); otherwise Bash would be unrestricted. +# +# Exits 1 on any violation, 0 otherwise. + +set -euo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +AGENTS_DIR="${AGENTS_DIR:-$REPO_ROOT/agents}" + +WRITE_TOOLS="Edit Write NotebookEdit" +READONLY_GIT_HOOK="allow-readonly-git.sh" + +if [ ! -d "$AGENTS_DIR" ]; then + echo "No agents/ directory found; nothing to guard." + exit 0 +fi + +shopt -s nullglob +agent_files=("$AGENTS_DIR"/*.md) +if [ ${#agent_files[@]} -eq 0 ]; then + echo "No agent files found in agents/; nothing to guard." + exit 0 +fi + +# Print the space-separated list of tool names declared in an agent's `tools` +# frontmatter, handling both the inline and the YAML block-list forms. +extract_tools() { + local file="$1" + awk ' + # Track the frontmatter block (between the first two --- fences). + NR == 1 && $0 == "---" { in_fm = 1; next } + in_fm && $0 == "---" { exit } + !in_fm { next } + + # Inline form: `tools: Read, Glob, Bash` + /^tools:[[:space:]]*[^[:space:]]/ { + sub(/^tools:[[:space:]]*/, "") + gsub(/,/, " ") + print + next + } + + # Block-list form: `tools:` followed by ` - Read` lines. + /^tools:[[:space:]]*$/ { collecting = 1; next } + collecting && /^[[:space:]]*-[[:space:]]*/ { + line = $0 + sub(/^[[:space:]]*-[[:space:]]*/, "", line) + printf "%s ", line + next + } + collecting && /^[^[:space:]]/ { collecting = 0 } + ' "$file" +} + +failed=0 + +for file in "${agent_files[@]}"; do + base="$(basename "$file")" + tools="$(extract_tools "$file")" + + # 1. No write tools. + for write_tool in $WRITE_TOOLS; do + if echo "$tools" | grep -Eqw "$write_tool"; then + echo "FAIL: $base grants write tool '$write_tool' in its tools frontmatter." + failed=1 + fi + done + + # 2. Bash must be paired with the read-only git hook. + if echo "$tools" | grep -Eqw "Bash"; then + if ! grep -q "$READONLY_GIT_HOOK" "$file"; then + echo "FAIL: $base grants Bash but does not reference $READONLY_GIT_HOOK in its hooks." + failed=1 + fi + fi +done + +if [ "$failed" -ne 0 ]; then + echo "Agent tools guard failed: agents must stay read-only." + exit 1 +fi + +echo "Agent tools guard passed: no agent grants a write tool, and any Bash is hook-restricted." +exit 0 diff --git a/test/agents_tools_guard_test.sh b/test/agents_tools_guard_test.sh new file mode 100755 index 0000000..0c96aa5 --- /dev/null +++ b/test/agents_tools_guard_test.sh @@ -0,0 +1,115 @@ +#!/bin/bash +# Tests for agents_tools_guard.sh +# +# Usage: bash test/agents_tools_guard_test.sh +# +# Drives the guard against synthetic agents/ directories (via the AGENTS_DIR +# override) and asserts it exits 0 on clean input and 1 on a violation, for +# both the inline and YAML block-list `tools:` forms. + +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +GUARD="$SCRIPT_DIR/agents_tools_guard.sh" + +PASSED=0 +FAILED=0 + +WORKDIR="$(mktemp -d)" +trap 'rm -rf "$WORKDIR"' EXIT + +# Write a single agent file into a fresh agents dir and run the guard against it. +# Usage: run_guard +# Echoes the guard's exit code. +run_guard() { + local contents="$1" + local dir="$WORKDIR/agents" + rm -rf "$dir" + mkdir -p "$dir" + printf '%s\n' "$contents" > "$dir/sample.md" + AGENTS_DIR="$dir" bash "$GUARD" >/dev/null 2>&1 + echo "$?" +} + +assert_exit() { + local label="$1" expected="$2" actual="$3" + if [ "$actual" = "$expected" ]; then + printf " \033[32mPASS\033[0m %s (exit %s)\n" "$label" "$actual" + PASSED=$((PASSED + 1)) + else + printf " \033[31mFAIL\033[0m %s: expected exit %s, got %s\n" "$label" "$expected" "$actual" + FAILED=$((FAILED + 1)) + fi +} + +echo "=== agents_tools_guard tests ===" +echo "" + +# Clean: inline read-only tools, Bash paired with the hook. +clean_inline='--- +name: sample +tools: Read, Glob, Grep, Bash, mcp__dart__analyze_files +hooks: + PreToolUse: + - matcher: "Bash" + hooks: + - type: command + command: "bash ${CLAUDE_PLUGIN_ROOT}/hooks/scripts/allow-readonly-git.sh" +--- +body' +assert_exit "clean inline tools + Bash hook" 0 "$(run_guard "$clean_inline")" + +# Violation: inline form with a write tool. +bad_inline='--- +name: sample +tools: Read, Write, Glob +--- +body' +assert_exit "inline form with Write" 1 "$(run_guard "$bad_inline")" + +# Violation: YAML block-list form with a write tool (the false-negative class). +bad_list='--- +name: sample +tools: + - Read + - Edit + - Glob +--- +body' +assert_exit "block-list form with Edit" 1 "$(run_guard "$bad_list")" + +# Clean: YAML block-list form, all read-only, no Bash. +clean_list='--- +name: sample +tools: + - Read + - Glob + - Grep +--- +body' +assert_exit "block-list form read-only" 0 "$(run_guard "$clean_list")" + +# Violation: Bash granted but no read-only git hook referenced. +bash_no_hook='--- +name: sample +tools: Read, Bash +--- +body' +assert_exit "Bash without hook" 1 "$(run_guard "$bash_no_hook")" + +# Violation: NotebookEdit in block-list form. +bad_notebook='--- +name: sample +tools: + - Read + - NotebookEdit +--- +body' +assert_exit "block-list form with NotebookEdit" 1 "$(run_guard "$bad_notebook")" + +echo "" +echo "=== Results: $PASSED passed, $FAILED failed ===" + +if [ "$FAILED" -gt 0 ]; then + exit 1 +fi From 529f3a19513ae833fad62caf0bfd8aa50589dd12 Mon Sep 17 00:00:00 2001 From: Oscar Martin Date: Tue, 30 Jun 2026 12:09:38 +0200 Subject: [PATCH 2/5] chore: update rules for output --- agents/flutter-reviewer.md | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/agents/flutter-reviewer.md b/agents/flutter-reviewer.md index 3a13aac..a83931c 100644 --- a/agents/flutter-reviewer.md +++ b/agents/flutter-reviewer.md @@ -76,7 +76,10 @@ not review the whole repository. ## Output -Report findings as a single markdown table, one row per finding: +Output **exactly one** markdown table, one row per finding. Do **not** split findings into multiple +tables, do **not** group them by file, and do **not** introduce section headings or extra columns +around the table. The table has exactly these four columns, in this order — `location`, `problem`, +`fix`, `standard`: ```markdown | location | problem | fix | standard | @@ -87,13 +90,18 @@ Report findings as a single markdown table, one row per finding: Rules: -- `location` — `path:line` of the finding. +- `location` — `path:line` of the finding, in a single column. Always include the file path on every + row; never move the path into a heading and never reduce this column to a bare line number. - `problem` — what is wrong, concisely. - `fix` — the change you recommend. Describe it; never apply it. -- `standard` — exactly one of `bloc`, `testing`, `static-security`, `accessibility`. Every row must - name one of these four. +- `standard` — exactly one of `bloc`, `testing`, `static-security`, `accessibility`, in its own + column on every row. Every row must name one of these four. Never convey the standard through a + section heading instead of this column. - Align the pipe characters vertically (VGV markdown convention). +A one-line note after the table (per "Out-of-domain changes" below) is allowed. Any other prose, +grouping, or additional tables is not. + ### No changed Dart files If the change scope contains no `.dart` files (clean tree, or only non-Dart changes), report From 9673688aab2d86faea6f4d1d96ce1173d7ed6333 Mon Sep 17 00:00:00 2001 From: Oscar Martin Date: Tue, 30 Jun 2026 12:28:13 +0200 Subject: [PATCH 3/5] ci: rename shell-tests job to script-tests --- .github/workflows/ci.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 41f662e..e4fff36 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -85,8 +85,8 @@ jobs: run: npm install -g @anthropic-ai/claude-code - name: Validate Plugin run: claude plugin validate . - shell-tests: - name: 🛡️ Shell Tests + script-tests: + name: 🛡️ Script Tests runs-on: ubuntu-latest steps: - uses: actions/checkout@v7 From 810617d02b1a223cea0bd5b9ccd82220417a6b30 Mon Sep 17 00:00:00 2001 From: Oscar Martin Date: Wed, 1 Jul 2026 16:50:34 +0200 Subject: [PATCH 4/5] chore: drop Wingspan reference --- README.md | 2 +- agents/flutter-reviewer.md | 17 ++++++++--------- config/cspell.json | 1 - 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 29a7e63..753dcf8 100644 --- a/README.md +++ b/README.md @@ -56,7 +56,7 @@ For more details, see the [Very Good Claude Marketplace][marketplace_link]. ## Agents -This plugin ships subagents that Claude Code can dispatch as isolated, specialized reviewers. Unlike skills, agents are **not** invoked as slash commands — Claude dispatches them automatically, or you can ask Claude to run one by name (e.g. "review my changes with the flutter-reviewer agent"). They also pair with the [Wingspan][marketplace_link] adversarial-critic round, which can dispatch them. +This plugin ships subagents that Claude Code can dispatch as isolated, specialized reviewers. Unlike skills, agents are **not** invoked as slash commands — Claude dispatches them automatically, or you can ask Claude to run one by name (e.g. "review my changes with the flutter-reviewer agent"). | Agent | Description | | ----- | ----------- | diff --git a/agents/flutter-reviewer.md b/agents/flutter-reviewer.md index a83931c..760c7ee 100644 --- a/agents/flutter-reviewer.md +++ b/agents/flutter-reviewer.md @@ -1,9 +1,8 @@ --- name: flutter-reviewer description: > - Read-only Flutter code reviewer. Dispatch after writing or changing Dart code, or from the - Wingspan adversarial-critic round, to review changed code against VGV bloc, testing, security, - and accessibility standards. Never edits files. + Read-only Flutter code reviewer. Dispatch after writing or changing Dart code to review + changed code against VGV bloc, testing, security, and accessibility standards. Never edits files. tools: Read, Glob, Grep, Bash, mcp__dart__analyze_files skills: - bloc @@ -22,8 +21,8 @@ hooks: # Flutter Reviewer Agent You are a read-only Flutter code reviewer for Very Good Ventures. You review changed Dart code -against four preloaded VGV standards and report findings as a markdown table. You pair with the -Wingspan adversarial-critic round, which dispatches you and consumes your table verbatim. +against four preloaded VGV standards and report findings as a markdown table. When an orchestrator +dispatches you, it consumes your table verbatim. ## Read-only contract @@ -127,8 +126,8 @@ coverage. For example: analyzer only to corroborate a skill-based judgment. - **Untraceable findings.** If a finding cannot name one of the four loaded standards, omit it. -## Wingspan dispatch contract +## Dispatch contract -When dispatched from the Wingspan adversarial-critic round, you self-scope via the adaptive diff -procedure above — the caller does not pass you a file list — and Wingspan consumes your findings -table verbatim. +When dispatched by an orchestrator or critic round, you self-scope via the adaptive diff procedure +above — the caller does not pass you a file list — and the caller consumes your findings table +verbatim. diff --git a/config/cspell.json b/config/cspell.json index 3c631a9..7dd42b5 100644 --- a/config/cspell.json +++ b/config/cspell.json @@ -49,7 +49,6 @@ "WCAG", "widgetbook", "Widgetbook", - "Wingspan", "xxlg" ], "flagWords": [] From 8fde0d879809a1dcde6ffa47ef46ff9606d27920 Mon Sep 17 00:00:00 2001 From: Oscar Martin Date: Thu, 2 Jul 2026 10:08:19 +0200 Subject: [PATCH 5/5] chore: remove agent tools guard --- .github/workflows/ci.yaml | 4 -- CLAUDE.md | 6 -- test/agents_tools_guard.sh | 92 ------------------------- test/agents_tools_guard_test.sh | 115 -------------------------------- 4 files changed, 217 deletions(-) delete mode 100755 test/agents_tools_guard.sh delete mode 100755 test/agents_tools_guard_test.sh diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index e4fff36..30c2f49 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -90,10 +90,6 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v7 - - name: Agent tools guard - run: bash test/agents_tools_guard.sh - - name: Agent tools guard self-test - run: bash test/agents_tools_guard_test.sh - name: Read-only git hook tests run: bash hooks/scripts/allow-readonly-git_test.sh - name: Block CLI workarounds hook tests diff --git a/CLAUDE.md b/CLAUDE.md index 6658da6..2bbbced 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -26,8 +26,6 @@ hooks/ format.sh # Runs dart format on modified .dart files vgv-cli-common.sh # Shared utilities for VGV CLI hook scripts warn-missing-mcp.sh # Warns at session start if VGV CLI is missing/outdated -test/ - agents_tools_guard.sh # Asserts agent tools frontmatter has no write tool skills/ accessibility/SKILL.md accessibility/references/ @@ -108,10 +106,6 @@ They live in `agents/.md` at the plugin root and are **auto-discovered** 3. Add any new domain terms to the `words` list in `config/cspell.json` 4. Update the repository structure in `CLAUDE.md` -The `test/agents_tools_guard.sh` guard runs in CI and automatically scans every `agents/*.md`: it -fails if any agent grants a write tool (`Edit`/`Write`/`NotebookEdit`), or grants `Bash` without -referencing the `allow-readonly-git.sh` hook. No per-agent wiring is needed. - ## Maintaining Existing Skills, Hooks, and MCP Tools Most documentation drift comes from changing existing assets without updating the diff --git a/test/agents_tools_guard.sh b/test/agents_tools_guard.sh deleted file mode 100755 index 8888ac2..0000000 --- a/test/agents_tools_guard.sh +++ /dev/null @@ -1,92 +0,0 @@ -#!/bin/bash -# Guard test: enforce the read-only contract for agents. -# -# For every agents/*.md: -# 1. The `tools` frontmatter must not grant a write tool (Edit, Write, -# NotebookEdit). Both the inline form (`tools: Read, Glob`) and the YAML -# block-list form (`tools:\n - Read\n - Glob`) are parsed. -# 2. If the agent grants Bash, its frontmatter must reference the read-only -# git hook (allow-readonly-git.sh); otherwise Bash would be unrestricted. -# -# Exits 1 on any violation, 0 otherwise. - -set -euo pipefail - -REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" -AGENTS_DIR="${AGENTS_DIR:-$REPO_ROOT/agents}" - -WRITE_TOOLS="Edit Write NotebookEdit" -READONLY_GIT_HOOK="allow-readonly-git.sh" - -if [ ! -d "$AGENTS_DIR" ]; then - echo "No agents/ directory found; nothing to guard." - exit 0 -fi - -shopt -s nullglob -agent_files=("$AGENTS_DIR"/*.md) -if [ ${#agent_files[@]} -eq 0 ]; then - echo "No agent files found in agents/; nothing to guard." - exit 0 -fi - -# Print the space-separated list of tool names declared in an agent's `tools` -# frontmatter, handling both the inline and the YAML block-list forms. -extract_tools() { - local file="$1" - awk ' - # Track the frontmatter block (between the first two --- fences). - NR == 1 && $0 == "---" { in_fm = 1; next } - in_fm && $0 == "---" { exit } - !in_fm { next } - - # Inline form: `tools: Read, Glob, Bash` - /^tools:[[:space:]]*[^[:space:]]/ { - sub(/^tools:[[:space:]]*/, "") - gsub(/,/, " ") - print - next - } - - # Block-list form: `tools:` followed by ` - Read` lines. - /^tools:[[:space:]]*$/ { collecting = 1; next } - collecting && /^[[:space:]]*-[[:space:]]*/ { - line = $0 - sub(/^[[:space:]]*-[[:space:]]*/, "", line) - printf "%s ", line - next - } - collecting && /^[^[:space:]]/ { collecting = 0 } - ' "$file" -} - -failed=0 - -for file in "${agent_files[@]}"; do - base="$(basename "$file")" - tools="$(extract_tools "$file")" - - # 1. No write tools. - for write_tool in $WRITE_TOOLS; do - if echo "$tools" | grep -Eqw "$write_tool"; then - echo "FAIL: $base grants write tool '$write_tool' in its tools frontmatter." - failed=1 - fi - done - - # 2. Bash must be paired with the read-only git hook. - if echo "$tools" | grep -Eqw "Bash"; then - if ! grep -q "$READONLY_GIT_HOOK" "$file"; then - echo "FAIL: $base grants Bash but does not reference $READONLY_GIT_HOOK in its hooks." - failed=1 - fi - fi -done - -if [ "$failed" -ne 0 ]; then - echo "Agent tools guard failed: agents must stay read-only." - exit 1 -fi - -echo "Agent tools guard passed: no agent grants a write tool, and any Bash is hook-restricted." -exit 0 diff --git a/test/agents_tools_guard_test.sh b/test/agents_tools_guard_test.sh deleted file mode 100755 index 0c96aa5..0000000 --- a/test/agents_tools_guard_test.sh +++ /dev/null @@ -1,115 +0,0 @@ -#!/bin/bash -# Tests for agents_tools_guard.sh -# -# Usage: bash test/agents_tools_guard_test.sh -# -# Drives the guard against synthetic agents/ directories (via the AGENTS_DIR -# override) and asserts it exits 0 on clean input and 1 on a violation, for -# both the inline and YAML block-list `tools:` forms. - -set -uo pipefail - -SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -GUARD="$SCRIPT_DIR/agents_tools_guard.sh" - -PASSED=0 -FAILED=0 - -WORKDIR="$(mktemp -d)" -trap 'rm -rf "$WORKDIR"' EXIT - -# Write a single agent file into a fresh agents dir and run the guard against it. -# Usage: run_guard -# Echoes the guard's exit code. -run_guard() { - local contents="$1" - local dir="$WORKDIR/agents" - rm -rf "$dir" - mkdir -p "$dir" - printf '%s\n' "$contents" > "$dir/sample.md" - AGENTS_DIR="$dir" bash "$GUARD" >/dev/null 2>&1 - echo "$?" -} - -assert_exit() { - local label="$1" expected="$2" actual="$3" - if [ "$actual" = "$expected" ]; then - printf " \033[32mPASS\033[0m %s (exit %s)\n" "$label" "$actual" - PASSED=$((PASSED + 1)) - else - printf " \033[31mFAIL\033[0m %s: expected exit %s, got %s\n" "$label" "$expected" "$actual" - FAILED=$((FAILED + 1)) - fi -} - -echo "=== agents_tools_guard tests ===" -echo "" - -# Clean: inline read-only tools, Bash paired with the hook. -clean_inline='--- -name: sample -tools: Read, Glob, Grep, Bash, mcp__dart__analyze_files -hooks: - PreToolUse: - - matcher: "Bash" - hooks: - - type: command - command: "bash ${CLAUDE_PLUGIN_ROOT}/hooks/scripts/allow-readonly-git.sh" ---- -body' -assert_exit "clean inline tools + Bash hook" 0 "$(run_guard "$clean_inline")" - -# Violation: inline form with a write tool. -bad_inline='--- -name: sample -tools: Read, Write, Glob ---- -body' -assert_exit "inline form with Write" 1 "$(run_guard "$bad_inline")" - -# Violation: YAML block-list form with a write tool (the false-negative class). -bad_list='--- -name: sample -tools: - - Read - - Edit - - Glob ---- -body' -assert_exit "block-list form with Edit" 1 "$(run_guard "$bad_list")" - -# Clean: YAML block-list form, all read-only, no Bash. -clean_list='--- -name: sample -tools: - - Read - - Glob - - Grep ---- -body' -assert_exit "block-list form read-only" 0 "$(run_guard "$clean_list")" - -# Violation: Bash granted but no read-only git hook referenced. -bash_no_hook='--- -name: sample -tools: Read, Bash ---- -body' -assert_exit "Bash without hook" 1 "$(run_guard "$bash_no_hook")" - -# Violation: NotebookEdit in block-list form. -bad_notebook='--- -name: sample -tools: - - Read - - NotebookEdit ---- -body' -assert_exit "block-list form with NotebookEdit" 1 "$(run_guard "$bad_notebook")" - -echo "" -echo "=== Results: $PASSED passed, $FAILED failed ===" - -if [ "$FAILED" -gt 0 ]; then - exit 1 -fi