diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index ece6a46..30c2f49 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -85,3 +85,12 @@ jobs: run: npm install -g @anthropic-ai/claude-code - name: Validate Plugin run: claude plugin validate . + script-tests: + name: 🛡️ Script Tests + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - 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 c6a5100..c69aafa 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,4 @@ .DS_Store *.log node_modules/ -docs/ \ No newline at end of file +docs/ diff --git a/CLAUDE.md b/CLAUDE.md index 357da65..995793f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -12,11 +12,14 @@ 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 @@ -86,6 +89,26 @@ 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` + ## Maintaining Existing Skills, Hooks, and MCP Tools Most documentation drift comes from changing existing assets without updating the @@ -119,7 +142,13 @@ These run **before** a tool call is executed: - `mcp__.*very-good-cli__.*` matcher → `check-vgv-cli.sh` — auto-approves the Very Good CLI MCP tool call by returning a PreToolUse `allow` decision, so it is always permitted regardless of run mode (interactive, headless, or `skipAutoPermissionPrompt`) and never dead-ends when the tool isn't on `permissions.allow`; denies with an install/upgrade message if the CLI is missing or < 1.3.0. The `.*` in the matcher covers both the bare `mcp__very-good-cli__*` server (repo-root `.mcp.json`) and the plugin-namespaced `mcp__plugin__very-good-cli__*` form used when installed from a marketplace - `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 5a30f66..7f131f9 100644 --- a/README.md +++ b/README.md @@ -55,6 +55,14 @@ For more details, see the [Very Good Claude Marketplace][marketplace_link]. | [**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 | | [**Green Gate**](skills/green-gate/SKILL.md) | Autonomous verify-fix-rerun loop that drives a package to green across four quality gates — analyze, format, test, and coverage — exiting only when a final iteration proves all four pass with observed numbers (default 100% coverage, overridable) | +## 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"). + +| 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. @@ -64,6 +72,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__.*`) | Auto-approves Very Good CLI MCP tool calls 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 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..760c7ee --- /dev/null +++ b/agents/flutter-reviewer.md @@ -0,0 +1,133 @@ +--- +name: flutter-reviewer +description: > + 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 + - 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. When an orchestrator +dispatches you, it 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 + +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 | +| --------------------------------- | ---------------------------------------- | ------------------------------------ | -------------- | +| 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, 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`, 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 +`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. + +## Dispatch contract + +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 ab65d64..2ebb3a6 100644 --- a/config/cspell.json +++ b/config/cspell.json @@ -2,6 +2,7 @@ "language": "en", "words": [ "activatable", + "adversarial", "antipattern", "Automator", "Bidirectionality", @@ -39,8 +40,11 @@ "pasteable", "prefs", "pubspec", + "redirections", "serialization", "stdio", + "subagent", + "subagents", "subclassing", "vsync", "WCAG", 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