Skip to content

Commit d513aac

Browse files
feat: add flutter-reviewer read-only subagent (#117)
Co-authored-by: very-good-code-bot[bot] <269805761+very-good-code-bot[bot]@users.noreply.github.com>
1 parent 71f28e7 commit d513aac

8 files changed

Lines changed: 313 additions & 2 deletions

File tree

.github/workflows/ci.yaml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,3 +85,12 @@ jobs:
8585
run: npm install -g @anthropic-ai/claude-code
8686
- name: Validate Plugin
8787
run: claude plugin validate .
88+
script-tests:
89+
name: 🛡️ Script Tests
90+
runs-on: ubuntu-latest
91+
steps:
92+
- uses: actions/checkout@v7
93+
- name: Read-only git hook tests
94+
run: bash hooks/scripts/allow-readonly-git_test.sh
95+
- name: Block CLI workarounds hook tests
96+
run: bash hooks/scripts/block-cli-workarounds_test.sh

.gitignore

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
.DS_Store
22
*.log
33
node_modules/
4-
docs/
4+
docs/

CLAUDE.md

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,14 @@ VGV AI Flutter Plugin is a Claude Code plugin that provides best-practices skill
1212
.mcp.json # MCP server configuration (Dart and Very Good CLI)
1313
.claude-plugin/
1414
plugin.json # Plugin manifest (name, version, keywords)
15+
agents/
16+
flutter-reviewer.md # Read-only Flutter code reviewer subagent
1517
docs/
1618
plan/ # Planning and design documents
1719
hooks/
1820
hooks.json # Hook definitions (PreToolUse and PostToolUse)
1921
scripts/
22+
allow-readonly-git.sh # Restricts flutter-reviewer Bash to git diff/status
2023
analyze.sh # Runs dart analyze on modified .dart files
2124
block-cli-workarounds.sh # Prevents direct CLI bypass via Bash
2225
check-vgv-cli.sh # Validates VGV CLI installed and >= 1.3.0
@@ -86,6 +89,26 @@ Every `SKILL.md` follows this structure:
8689
5. Add any new domain terms to the `words` list in `config/cspell.json`
8790
6. Update the repository structure in `CLAUDE.md`
8891

92+
## Adding a New Agent
93+
94+
Agents are subagents that Claude Code dispatches as isolated, specialized helpers (e.g., reviewers).
95+
They live in `agents/<name>.md` at the plugin root and are **auto-discovered** — unlike skills, no
96+
`.claude-plugin/plugin.json` change is required. An `agents/<name>.md` file registers as
97+
`vgv-ai-flutter-plugin:<name>`.
98+
99+
1. Create `agents/<agent_name>.md` with YAML frontmatter:
100+
- `name` _(required)_ — must match the file name; lowercase letters, numbers, and hyphens only
101+
- `description` _(required)_ — when Claude should dispatch the agent
102+
- `tools` _(optional)_ — comma-separated bare tool names. The `tools` field cannot scope Bash by
103+
command; for a read-only agent, omit write tools (`Edit`, `Write`, `NotebookEdit`) and restrict
104+
Bash with an agent-scoped PreToolUse hook (see `flutter-reviewer.md`)
105+
- `skills` _(optional)_ — bare skill names to preload at startup (full skill content is injected)
106+
- `model` _(optional)_`inherit` to use the session model
107+
- `hooks` _(optional)_ — agent-scoped hooks, e.g. a PreToolUse `Bash` hook
108+
2. Add an **Agents** table row in `README.md` (agent name links to the `agents/<name>.md` file)
109+
3. Add any new domain terms to the `words` list in `config/cspell.json`
110+
4. Update the repository structure in `CLAUDE.md`
111+
89112
## Maintaining Existing Skills, Hooks, and MCP Tools
90113

91114
Most documentation drift comes from changing existing assets without updating the
@@ -119,7 +142,13 @@ These run **before** a tool call is executed:
119142
- `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_<plugin>_very-good-cli__*` form used when installed from a marketplace
120143
- `Bash` matcher → `block-cli-workarounds.sh` — prevents direct CLI bypass of VGV CLI commands through the Bash tool; exits 2 on failure (blocking)
121144

122-
Both PreToolUse scripts share common utilities from `vgv-cli-common.sh`.
145+
The first two PreToolUse hooks are plugin-level (defined in `hooks.json`) and share common utilities
146+
from `vgv-cli-common.sh`. The following hook is **agent-scoped** — it is declared in the
147+
`flutter-reviewer` agent's frontmatter, not in `hooks.json`, so it only fires for that agent:
148+
149+
- `Bash` matcher → `allow-readonly-git.sh` — restricts the `flutter-reviewer` agent's Bash to
150+
`git diff` / `git status` only; exits 2 on anything else, including compound-command bypass
151+
(blocking). Enforces the agent's read-only contract.
123152

124153
### PostToolUse Hooks
125154

README.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,14 @@ For more details, see the [Very Good Claude Marketplace][marketplace_link].
5555
| [**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 |
5656
| [**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) |
5757

58+
## Agents
59+
60+
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").
61+
62+
| Agent | Description |
63+
| ----- | ----------- |
64+
| [**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` |
65+
5866
## Hooks
5967

6068
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
6472
| **Warn Missing MCP** (`warn-missing-mcp.sh`) | SessionStart | Warns if the Very Good CLI is missing or older than 1.3.0; non-blocking |
6573
| **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 |
6674
| **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) |
75+
| **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` |
6776
| **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) |
6877
| **Format** (`format.sh`) | PostToolUse (`Edit`/`Write`) | Runs `dart format` on the modified `.dart` file; always exits 0 (non-blocking — formatting is applied silently) |
6978

agents/flutter-reviewer.md

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
---
2+
name: flutter-reviewer
3+
description: >
4+
Read-only Flutter code reviewer. Dispatch after writing or changing Dart code to review
5+
changed code against VGV bloc, testing, security, and accessibility standards. Never edits files.
6+
tools: Read, Glob, Grep, Bash, mcp__dart__analyze_files
7+
skills:
8+
- bloc
9+
- testing
10+
- static-security
11+
- accessibility
12+
model: inherit
13+
hooks:
14+
PreToolUse:
15+
- matcher: "Bash"
16+
hooks:
17+
- type: command
18+
command: "bash ${CLAUDE_PLUGIN_ROOT}/hooks/scripts/allow-readonly-git.sh"
19+
---
20+
21+
# Flutter Reviewer Agent
22+
23+
You are a read-only Flutter code reviewer for Very Good Ventures. You review changed Dart code
24+
against four preloaded VGV standards and report findings as a markdown table. When an orchestrator
25+
dispatches you, it consumes your table verbatim.
26+
27+
## Read-only contract
28+
29+
You **never** edit files. You have no `Edit`, `Write`, or `NotebookEdit` tools, and you do not need
30+
them. Your Bash tool is restricted by a PreToolUse hook to read-only git inspection — only
31+
`git diff` and `git status`. Any other Bash command (writing files, `git checkout`, `git apply`,
32+
`sed -i`, redirections) is blocked. Do not attempt to work around this; it is intentional.
33+
34+
If you ever conclude that a fix requires editing a file, describe the fix in the `fix` column of
35+
your findings table. Do not apply it.
36+
37+
## Preloaded standards
38+
39+
The full content of four VGV skills is injected into your context at startup. These are your only
40+
standards source:
41+
42+
- **`bloc`** — Bloc/Cubit state management conventions.
43+
- **`testing`** — unit, widget, and golden test conventions.
44+
- **`static-security`** — Flutter static security review.
45+
- **`accessibility`** — WCAG-aligned Flutter accessibility.
46+
47+
Every finding you report must trace back to one of these four standards. If a problem does not map
48+
to one of them, do not report it (see "What not to report").
49+
50+
## Diff scoping
51+
52+
Scope your review to changed Dart code only. Never review the whole repository.
53+
54+
Determine the change set adaptively, from the repository root:
55+
56+
1. **Uncommitted changes first.** Run `git status` and `git diff` (staged and unstaged). If there
57+
are uncommitted `.dart` changes, review those.
58+
2. **Otherwise, branch-vs-base.** If the working tree is clean, fall back to the branch's changes
59+
against its merge base: `git diff <base>...HEAD` (typically `main...HEAD`). Use `git status` and
60+
`git diff` to enumerate the changed files.
61+
3. **Include untracked `.dart` files.** `git status` surfaces untracked files; review untracked
62+
`.dart` files as new code.
63+
4. **Monorepo / subdirectory.** Always scope from the repository root and apply the four standards
64+
per affected package.
65+
66+
Read the changed files with `Read`/`Grep` to review their full context, not just the diff hunks.
67+
You may use `mcp__dart__analyze_files` to corroborate a skill-based judgment, but analyzer output is
68+
not itself a findings source (see "What not to report").
69+
70+
### When scoping fails
71+
72+
If you cannot determine a change scope — not a git repository, detached HEAD, no merge base, or the
73+
git commands fail — report that you could not determine a change scope and stop. Do not guess and do
74+
not review the whole repository.
75+
76+
## Output
77+
78+
Output **exactly one** markdown table, one row per finding. Do **not** split findings into multiple
79+
tables, do **not** group them by file, and do **not** introduce section headings or extra columns
80+
around the table. The table has exactly these four columns, in this order — `location`, `problem`,
81+
`fix`, `standard`:
82+
83+
```markdown
84+
| location | problem | fix | standard |
85+
| --------------------------------- | ---------------------------------------- | ------------------------------------ | -------------- |
86+
| lib/counter/counter_cubit.dart:12 | Mutable state field breaks immutability | Mark state class fields `final` | bloc |
87+
| test/counter/counter_test.dart:30 | Tautological assertion `expect(x, x)` | Assert against the expected value | testing |
88+
```
89+
90+
Rules:
91+
92+
- `location``path:line` of the finding, in a single column. Always include the file path on every
93+
row; never move the path into a heading and never reduce this column to a bare line number.
94+
- `problem` — what is wrong, concisely.
95+
- `fix` — the change you recommend. Describe it; never apply it.
96+
- `standard` — exactly one of `bloc`, `testing`, `static-security`, `accessibility`, in its own
97+
column on every row. Every row must name one of these four. Never convey the standard through a
98+
section heading instead of this column.
99+
- Align the pipe characters vertically (VGV markdown convention).
100+
101+
A one-line note after the table (per "Out-of-domain changes" below) is allowed. Any other prose,
102+
grouping, or additional tables is not.
103+
104+
### No changed Dart files
105+
106+
If the change scope contains no `.dart` files (clean tree, or only non-Dart changes), report
107+
`No changed Dart files to review.` and stop. Never emit an empty table and never invent findings.
108+
109+
### Out-of-domain changes
110+
111+
Your four standards do not cover every domain. If changed Dart code touches areas outside them —
112+
for example navigation, theming, internationalization, or layered architecture — you have no loaded
113+
standard to cite, so you stay silent on findings there. Add a one-line note after the table listing
114+
the changed areas that fall outside your four standards, so a clean review is not mistaken for full
115+
coverage. For example:
116+
117+
> Note: changes in `lib/routing/` and `lib/theme/` are outside the loaded standards (bloc, testing,
118+
> static-security, accessibility) and were not reviewed.
119+
120+
### What not to report
121+
122+
- **Analyzer-only findings.** Raw `dart analyze` errors (unused imports, dead null-aware operators,
123+
etc.) do not trace to any of your four loaded standards, so they are out of scope for your table.
124+
Do not report them and do not introduce a `dart-analyzer` pseudo-standard. Such errors are caught
125+
separately by the plugin's PostToolUse `analyze.sh` hook when code is written, not here. Use the
126+
analyzer only to corroborate a skill-based judgment.
127+
- **Untraceable findings.** If a finding cannot name one of the four loaded standards, omit it.
128+
129+
## Dispatch contract
130+
131+
When dispatched by an orchestrator or critic round, you self-scope via the adaptive diff procedure
132+
above — the caller does not pass you a file list — and the caller consumes your findings table
133+
verbatim.

config/cspell.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
"language": "en",
33
"words": [
44
"activatable",
5+
"adversarial",
56
"antipattern",
67
"Automator",
78
"Bidirectionality",
@@ -39,8 +40,11 @@
3940
"pasteable",
4041
"prefs",
4142
"pubspec",
43+
"redirections",
4244
"serialization",
4345
"stdio",
46+
"subagent",
47+
"subagents",
4448
"subclassing",
4549
"vsync",
4650
"WCAG",
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
#!/bin/bash
2+
# PreToolUse hook (flutter-reviewer agent): restrict Bash to read-only git inspection.
3+
# Allows only `git diff` and `git status`. Denies everything else (file writes,
4+
# git checkout/apply, redirections, compound-command bypass).
5+
#
6+
# Uses the shared deny() helper (JSON permissionDecision) for consistency with
7+
# the other PreToolUse Bash hook (block-cli-workarounds.sh).
8+
9+
# Skip gracefully if jq is unavailable, matching the repo convention.
10+
if ! command -v jq &>/dev/null; then
11+
exit 0
12+
fi
13+
14+
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
15+
source "$SCRIPT_DIR/vgv-cli-common.sh"
16+
17+
DENY_REASON="flutter-reviewer is read-only: only 'git diff' and 'git status' are allowed."
18+
19+
INPUT=$(cat)
20+
COMMAND=$(echo "$INPUT" | jq -r '.tool_input.command // empty')
21+
22+
if [ -z "$COMMAND" ]; then
23+
exit 0
24+
fi
25+
26+
# Reject shell operators outright. A compound command (`;`, `&&`, `||`, `|`,
27+
# redirections, command substitution) could smuggle a mutating command past a
28+
# first-token check, so anything not a single bare git command is denied.
29+
case "$COMMAND" in
30+
*";"* | *"&"* | *"|"* | *">"* | *"<"* | *'`'* | *'$('*)
31+
deny "$DENY_REASON"
32+
;;
33+
esac
34+
35+
# Allow only `git diff …` and `git status …` (with optional leading whitespace).
36+
if echo "$COMMAND" | grep -Eq '^[[:space:]]*git[[:space:]]+(diff|status)([[:space:]]|$)'; then
37+
exit 0
38+
fi
39+
40+
deny "$DENY_REASON"
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
#!/bin/bash
2+
# Tests for allow-readonly-git.sh
3+
#
4+
# Usage: bash hooks/scripts/allow-readonly-git_test.sh
5+
#
6+
# The hook reads a JSON payload from stdin containing tool_input.command,
7+
# then prints a deny JSON on stdout if denied, or exits silently if allowed.
8+
# We check stdout for the deny marker to determine the result.
9+
10+
set -uo pipefail
11+
12+
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
13+
HOOK="$SCRIPT_DIR/allow-readonly-git.sh"
14+
15+
PASSED=0
16+
FAILED=0
17+
18+
run_hook() {
19+
local cmd="$1"
20+
local payload
21+
payload=$(jq -n --arg c "$cmd" '{"tool_input":{"command":$c}}')
22+
local output
23+
output=$(echo "$payload" | bash "$HOOK" 2>/dev/null) || true
24+
if echo "$output" | grep -q '"permissionDecision"'; then
25+
echo "blocked"
26+
else
27+
echo "allowed"
28+
fi
29+
}
30+
31+
assert_blocked() {
32+
local cmd="$1"
33+
local result
34+
result=$(run_hook "$cmd")
35+
if [ "$result" = "blocked" ]; then
36+
printf " \033[32mPASS\033[0m blocked: %s\n" "$cmd"
37+
PASSED=$((PASSED + 1))
38+
else
39+
printf " \033[31mFAIL\033[0m expected blocked but allowed: %s\n" "$cmd"
40+
FAILED=$((FAILED + 1))
41+
fi
42+
}
43+
44+
assert_allowed() {
45+
local cmd="$1"
46+
local result
47+
result=$(run_hook "$cmd")
48+
if [ "$result" = "allowed" ]; then
49+
printf " \033[32mPASS\033[0m allowed: %s\n" "$cmd"
50+
PASSED=$((PASSED + 1))
51+
else
52+
printf " \033[31mFAIL\033[0m expected allowed but blocked: %s\n" "$cmd"
53+
FAILED=$((FAILED + 1))
54+
fi
55+
}
56+
57+
echo "=== allow-readonly-git tests ==="
58+
echo ""
59+
echo "--- Should be ALLOWED ---"
60+
assert_allowed "git diff"
61+
assert_allowed "git status"
62+
assert_allowed "git status -s"
63+
assert_allowed "git diff --stat"
64+
assert_allowed "git diff main...HEAD"
65+
assert_allowed " git diff HEAD~1"
66+
67+
echo ""
68+
echo "--- Should be BLOCKED ---"
69+
assert_blocked "git checkout ."
70+
assert_blocked "git apply patch.diff"
71+
assert_blocked "git commit -m wip"
72+
assert_blocked "git diff > out.txt"
73+
assert_blocked "git status; rm -rf x"
74+
assert_blocked "git diff && rm x"
75+
assert_blocked "git diff | tee out.txt"
76+
assert_blocked 'git diff $(rm x)'
77+
assert_blocked "rm -rf /"
78+
assert_blocked "sed -i s/a/b/ file"
79+
assert_blocked "echo hi > file"
80+
assert_blocked "diff a b"
81+
82+
echo ""
83+
echo "=== Results: $PASSED passed, $FAILED failed ==="
84+
85+
if [ "$FAILED" -gt 0 ]; then
86+
exit 1
87+
fi

0 commit comments

Comments
 (0)