From 6306f425932acf33289355f38866d5f824b0f2aa Mon Sep 17 00:00:00 2001 From: Dominik Simonik Date: Tue, 13 Oct 2026 12:11:48 +0200 Subject: [PATCH 1/2] fix(green-gate): read uncovered lines from the test result, not lcov The MCP `test` tool exposes `show_uncovered` as of very_good_cli 1.5.0, so its result carries the per-file `Lines not covered:` block the coverage gate used to reconstruct by parsing `coverage/lcov.info`. - Pass `show_uncovered: true` alongside the other coverage parameters - Rank fix targets from the result; the lcov parse is now the fallback for a result without the block - Soften the "Bash is reserved for parsing lcov" carve-out accordingly Closes #146 --- skills/green-gate/SKILL.md | 75 +++++++++++++++--------- skills/green-gate/references/coverage.md | 33 +++++++++-- 2 files changed, 74 insertions(+), 34 deletions(-) diff --git a/skills/green-gate/SKILL.md b/skills/green-gate/SKILL.md index 972a076..c75dd97 100644 --- a/skills/green-gate/SKILL.md +++ b/skills/green-gate/SKILL.md @@ -40,7 +40,8 @@ Apply these to all green-gate work: through a shell command. The Bash test path (`very_good test`, `flutter test`, `dart test`) is hook-blocked by `block-cli-workarounds.sh` and will be denied, and `dart analyze` / `dart format` via Bash are redundant with the MCP tools. - **Bash is reserved for parsing `coverage/lcov.info` — nothing else.** This MCP-only + **The only Bash the loop runs is the fallback `coverage/lcov.info` parse**, and + only when the test result carries no `Lines not covered:` block. This MCP-only rule is a Claude Code constraint enforced by that hook; the hook does not run on other hosts. **Cross-harness fallback:** on a host without the hook and without the MCP servers connected, run the equivalent `dart analyze`, `dart format`, and @@ -49,7 +50,7 @@ Apply these to all green-gate work: tools, which arguments, or what order the gates run in and does not want a run yet, answer from this skill: the same tool calls (`mcp__dart__analyze_files` with `applyFixes: true`, `mcp__dart__dart_format`, `mcp__very-good-cli__test` - with the coverage triple), in gate order, with the precedence rule that makes + with the coverage parameters), in gate order, with the precedence rule that makes the order matter. Never substitute an improvised shell plan of `flutter analyze` / `flutter test --coverage` for the tools the loop actually runs. - **Never cache green** — re-evaluate every gate every round. Fixing analyze or @@ -58,10 +59,13 @@ analyze` / `flutter test --coverage` for the tools the loop actually runs. - **Exit only on observed numbers** — the loop terminates only after a single final iteration in which analyze is clean, format reports zero changes, all tests pass, and `min_coverage` is satisfied, all observed in the same round. -- **Pass `coverage: true`, `min_coverage`, and `check_ignore: true` together** — - omitting `coverage: true` silently produces no `lcov.info` (mimics a - misconfiguration); omitting `check_ignore: true` makes the `// coverage:ignore` - remedy a no-op. +- **Pass `coverage: true`, `min_coverage`, `check_ignore: true`, and + `show_uncovered: true` together** — omitting `coverage: true` silently produces + no `lcov.info` (mimics a misconfiguration); omitting `check_ignore: true` makes + the `// coverage:ignore` remedy a no-op; omitting `show_uncovered: true` drops + the per-file uncovered lines from the result and sends the loop to the lcov file + for what the tool would have handed back. `show_uncovered` arrived in + `very_good_cli` 1.5.0, the plugin's minimum. - **Defer test-writing to the `testing` skill** — when a fix requires authoring tests, follow that skill for structure, mocking, and naming. - **Fix root causes, not symptoms** — never weaken a gate to pass it (do not @@ -89,9 +93,10 @@ For each package root (see **Recursive / Monorepo**), run this algorithm: with the coverage parameters from **Test Gate**. If tests fail, fix, record the fingerprint, go to step 7. 5. **Coverage** — only if tests pass this round. The MCP `min_coverage` result is - authoritative pass/fail. Parse `coverage/lcov.info` for the displayed - percentage and per-file fix targets (advisory). If below target, author tests - for the ranked under-covered files (via the `testing` skill), go to step 7. + authoritative pass/fail, and its `Lines not covered:` block gives the per-file + fix targets; parse `coverage/lcov.info` only when that block is absent. If + below target, author tests for the ranked under-covered files (via the + `testing` skill), go to step 7. 6. **Exit** — if all four gates are green in _this same iteration_, confirm success with the observed numbers and stop. This is the only exit-green path. 7. **Re-verify** — increment the iteration counter, recompute the failure @@ -123,7 +128,7 @@ Fingerprint keys per gate: | **Analyze** | Sorted set of `diagnosticCode @ file:line` | | **Format** | Set of files the format gate rewrote (empty = green) | | **Test** | Set of failing test IDs / names | -| **Coverage** | Observed percentage + sorted set of under-covered `SF` files | +| **Coverage** | Observed percentage + sorted set of under-covered file paths | Definitions: @@ -141,7 +146,7 @@ Fixed order: **analyze → format → test → coverage**. Two rules govern it: 1. **A downstream gate is not assessed until the upstream gate is green this round.** A red analyzer can make tests fail to compile; coverage is - meaningless when tests do not pass — never parse `lcov.info` after a compile + meaningless when tests do not pass — never diagnose coverage after a compile failure. Format runs after analyze so its reformatting does not churn over code that analyze's `applyFixes` is about to rewrite. 2. **Every gate is re-evaluated every round; green is never cached.** Exit @@ -188,7 +193,8 @@ Fixed order: **analyze → format → test → coverage**. Two rules govern it: ## Test Gate - Run `mcp__very-good-cli__test` with `coverage: true`, `min_coverage: `, - `check_ignore: true`, and the `exclude_coverage` glob (below). + `check_ignore: true`, `show_uncovered: true`, and the `exclude_coverage` glob + (below). - **Dart vs Flutter** — omit `dart` to let the tool auto-detect (Flutter is run when a Flutter project is detected). Pass `dart: true` only for a pure Dart package the tool would otherwise misclassify. @@ -206,9 +212,18 @@ Fixed order: **analyze → format → test → coverage**. Two rules govern it: ## Coverage Gate -The MCP `min_coverage` result is **authoritative** for pass/fail. The -`coverage/lcov.info` parse is **advisory** — it supplies the displayed -percentage and per-file fix targets. +The MCP `min_coverage` result is **authoritative** for pass/fail, and with +`show_uncovered: true` it also carries the per-file fix targets: + +```text +Expected coverage >= 100.00% but actual is 33.33%. +Lines not covered: + - lib/src/weather_repository.dart: 4, 5 +``` + +Rank the files from that block. The `coverage/lcov.info` parse is the +**fallback**, for a result that has no such block: lcov is left behind by a run +that may not be the one being diagnosed, while the result cannot be stale. - **Default target is `100`** (VGV's house standard), **overridable per invocation** (e.g. `80`) for legacy or non-template packages. Generated files @@ -217,13 +232,13 @@ percentage and per-file fix targets. - **Default `exclude_coverage`** — `**/*.{g,freezed,gen}.dart` plus generated and l10n directories. One glob string (brace expansion; fall back to `**/*.g.dart` if a CLI build does not honor it). -- **Pass `coverage: true`, `min_coverage`, and `check_ignore: true` together** — - see **Core Standards**. -- **When `coverage/lcov.info` is missing or below target**, follow the decision - tree and parsing rules in [`references/coverage.md`](references/coverage.md) — - it covers the three absence causes, stale-lcov handling, the lcov record fields - (`SF`/`LF`/`LH`/`DA`), the advisory-mirrors-the-gate rule, and the - `check_ignore` Dart-only limitation. +- **Pass `coverage: true`, `min_coverage`, `check_ignore: true`, and + `show_uncovered: true` together** — see **Core Standards**. +- **When the result carries no `Lines not covered:` block**, fall back to + `coverage/lcov.info` per [`references/coverage.md`](references/coverage.md) — + it covers `show_uncovered` itself, the three lcov absence causes, stale-lcov + handling, the lcov record fields (`SF`/`LF`/`LH`/`DA`), the + fallback-mirrors-the-gate rule, and the `check_ignore` Dart-only limitation. --- @@ -231,9 +246,10 @@ percentage and per-file fix targets. - **Fix only failing items** — address the diagnostics, tests, or under-covered files surfaced this round. Do not refactor unrelated code (YAGNI). -- **Coverage fixes = author tests** — for each ranked under-covered `SF` file, - write tests following the `testing` skill. Prioritize files by uncovered - line count (`LF - LH`). +- **Coverage fixes = author tests** — for each ranked under-covered file, write + tests following the `testing` skill. Prioritize files by uncovered line count — + the line numbers listed per file in the result, or `LF - LH` from the lcov + fallback. - **Bound files per round** — fix a coherent batch, then re-verify. Re-running the gates after each batch is what makes "no progress" detectable and prevents fixing one gate while silently breaking another. @@ -257,7 +273,7 @@ Stop and surface to the user when: When escalating, name the gate that is red — analyze, format, test, or coverage — then give its fingerprint entries verbatim (`diagnosticCode @ file:line` for -analyze, failing test names for test, under-covered `SF` paths for coverage), +analyze, failing test names for test, under-covered file paths for coverage), the files touched, the iteration count, and the one decision the user must make. A summary count ("4 errors remain") is not a report; the user cannot act on it. @@ -277,7 +293,8 @@ report, and wait. - **Shared package-root discovery** — walk for `pubspec.yaml` files. The `analyze_files` `roots` set must match the package set `mcp__very-good-cli__test --recursive` (`recursive: true`) covers. -- **lcov path is `/coverage/lcov.info`** — resolved per discovered root. +- **Fallback lcov path is `/coverage/lcov.info`** — resolved per + discovered root. - **Single `min_coverage` applies to all packages** — documented limitation: the tool schema has no per-package coverage override. State the shared target when confirming coverage. @@ -287,7 +304,7 @@ report, and wait. ## Additional Resources - [`references/coverage.md`](references/coverage.md) — green-gate's coverage-gate - detail (default target, exclude globs, lcov fields, decision tree, - `check_ignore`, stale lcov). + detail (default target, exclude globs, `show_uncovered`, the lcov fallback and + its fields, decision tree, `check_ignore`, stale lcov). - The `testing` skill — test conventions, plus its coverage reference for closing per-file gaps. diff --git a/skills/green-gate/references/coverage.md b/skills/green-gate/references/coverage.md index a282298..3d62d8c 100644 --- a/skills/green-gate/references/coverage.md +++ b/skills/green-gate/references/coverage.md @@ -1,9 +1,10 @@ # Coverage Gate — Details Companion to the **Coverage Gate** section of `green-gate/SKILL.md`. The MCP -`min_coverage` result is **authoritative** for pass/fail; the -`coverage/lcov.info` parse is **advisory** (the displayed percentage and -per-file fix targets). +`min_coverage` result is **authoritative** for pass/fail, and with +`show_uncovered: true` its `Lines not covered:` block is the per-file fix-target +list. Everything below about `coverage/lcov.info` is the **fallback** path, for a +result that carries no such block. ## Default target @@ -15,6 +16,28 @@ so the target is achievable rather than a ceiling on every generated line. It is **overridable per invocation** (e.g. `80`) for legacy or non-template packages while the loop raises coverage toward it. +## show_uncovered + +Pass `show_uncovered: true` on every `mcp__very-good-cli__test` call. The result +then names every uncovered line per file — the same granularity the loop would +otherwise reconstruct from lcov: + +```text +Expected coverage >= 100.00% but actual is 33.33%. +Lines not covered: + - lib/src/weather_repository.dart: 4, 5 +``` + +- It applies to **both Dart and Flutter** runs, unlike `check_ignore`. +- The block honors `exclude_coverage`, so it already mirrors the authoritative + gate — no exclusion set to re-apply by hand. +- It appears whenever lines are uncovered: inside the `min_coverage` failure when + the gate is red, and as informational output when the gate passes with lines + still uncovered. A clean 100% run prints no block, which is not a fallback + trigger. +- It arrived in `very_good_cli` 1.5.0, the plugin's minimum. A `test` tool schema + without the parameter means an older CLI — use the lcov fallback. + ## exclude_coverage Default: `**/*.{g,freezed,gen}.dart` plus generated and l10n directories. This is @@ -22,9 +45,9 @@ one glob string (not a repeated flag); multiple patterns require brace expansion If a CLI build does not honor brace expansion, fall back to the single most impactful glob `**/*.g.dart` and note the limitation in the report. -## Advisory parse must mirror the gate +## Fallback parse must mirror the gate -The lcov parser applies the **identical exclusion set** used in +When parsing lcov, apply the **identical exclusion set** used in `exclude_coverage`, or the displayed percentage will not match the authoritative gate. Read `SF` (source file), `LF` (lines found), `LH` (lines hit) per file; use `DA` (line, hit-count) records to point at specific uncovered lines. From 23132e20fc4b2da142f00224b05fd8d163ba3c97 Mon Sep 17 00:00:00 2001 From: Dominik Simonik Date: Tue, 13 Oct 2026 12:11:54 +0200 Subject: [PATCH 2/2] feat(hooks): require very_good_cli >= 1.5.0 green-gate now relies on `show_uncovered` on the MCP `test` tool, which the CLI only exposes from 1.5.0. Raise the single floor in `vgv-cli-common.sh` so the version checks match what the skills call, and update the test fixtures and the docs that quote the floor. --- AGENTS.md | 2 +- CLAUDE.md | 4 ++-- CONTRIBUTING.md | 2 +- README.md | 6 +++--- hooks/scripts/check-vgv-cli_test.sh | 8 ++++---- hooks/scripts/vgv-cli-common.sh | 4 ++-- hooks/scripts/warn-missing-mcp_test.sh | 10 +++++----- 7 files changed, 18 insertions(+), 18 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index b052fef..d5eb251 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -39,7 +39,7 @@ hooks/ analyze.sh # Runs dart analyze on modified .dart files block-cli-workarounds.sh # Prevents direct CLI bypass via Bash block-cli-workarounds_test.sh # Tests for block-cli-workarounds.sh - check-vgv-cli.sh # Validates VGV CLI installed and >= 1.3.0 + check-vgv-cli.sh # Validates VGV CLI installed and >= 1.5.0 check-vgv-cli_test.sh # Tests for check-vgv-cli.sh format.sh # Runs dart format on modified .dart files vgv-cli-common.sh # Shared utilities for VGV CLI hook scripts diff --git a/CLAUDE.md b/CLAUDE.md index e2a900a..0190449 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -10,13 +10,13 @@ The `hooks/` directory contains SessionStart, PreToolUse, and PostToolUse hooks These run **when a session begins**: -- `warn-missing-mcp.sh` — checks if Very Good CLI is installed and >= 1.3.0; outputs a warning to Claude's context if missing, outdated, or present but unable to run because `dart` is not on the `PATH` hooks inherit (non-blocking) +- `warn-missing-mcp.sh` — checks if Very Good CLI is installed and >= 1.5.0; outputs a warning to Claude's context if missing, outdated, or present but unable to run because `dart` is not on the `PATH` hooks inherit (non-blocking) ### PreToolUse Hooks These run **before** a tool call is executed: -- `mcp__.*very-good-cli__.*` matcher → `check-vgv-cli.sh` — reads `tool_name` from the payload and exits 0 unless the caller is a Very Good CLI tool, so a host that does not apply the matcher cannot have the decision land on an unrelated tool; for its own tools it auto-approves the 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, and stands aside when the CLI is present but its version cannot be read (`dart` missing from `PATH`), leaving normal permission handling to apply. 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 +- `mcp__.*very-good-cli__.*` matcher → `check-vgv-cli.sh` — reads `tool_name` from the payload and exits 0 unless the caller is a Very Good CLI tool, so a host that does not apply the matcher cannot have the decision land on an unrelated tool; for its own tools it auto-approves the 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.5.0, and stands aside when the CLI is present but its version cannot be read (`dart` missing from `PATH`), leaving normal permission handling to apply. 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 host's shell tool (`Bash`, or `Shell` on other hosts); exits 0 when the payload names a non-shell tool, so an unrelated tool carrying a `command` argument is never inspected; when the CLI is present but cannot run because `dart` is missing from `PATH`, the denial says so rather than redirecting to an MCP server that cannot start; every denial also says the whole shell call was refused, so the agent re-runs any command chained with the blocked one instead of assuming it ran; exits 2 on failure (blocking) The first two PreToolUse hooks are plugin-level (defined in `hooks.json`) and share common utilities diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 17476dc..395a777 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -205,7 +205,7 @@ session and exercise it before you commit. - **Claude Code CLI** installed (`npm install -g @anthropic-ai/claude-code`). - **Dart SDK** and **jq** on your `PATH` — the hooks need both. -- **Very Good CLI** ≥ 1.3.0 (`dart pub global activate very_good_cli`) for the +- **Very Good CLI** ≥ 1.5.0 (`dart pub global activate very_good_cli`) for the Very Good CLI MCP server tools. See the README [Hooks](README.md#hooks) and [MCP Integration](README.md#mcp-integration) diff --git a/README.md b/README.md index d5981e5..61e5fa9 100644 --- a/README.md +++ b/README.md @@ -69,8 +69,8 @@ This plugin includes SessionStart, PreToolUse, and PostToolUse hooks that valida | Hook | Trigger | Behavior | | ---- | ------- | -------- | -| **Warn Missing MCP** (`warn-missing-mcp.sh`) | SessionStart | Warns if the Very Good CLI is missing, older than 1.3.0, or present but unable to run because `dart` is not on the `PATH` hooks inherit; non-blocking | -| **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 | +| **Warn Missing MCP** (`warn-missing-mcp.sh`) | SessionStart | Warns if the Very Good CLI is missing, older than 1.5.0, or present but unable to run because `dart` is not on the `PATH` hooks inherit; non-blocking | +| **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.5.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) | @@ -186,7 +186,7 @@ The Very Good CLI MCP server exposes Very Good CLI commands to Claude. **Prerequisites:** -- Very Good CLI v1.3.0+ installed: `dart pub global activate very_good_cli` +- Very Good CLI v1.5.0+ installed: `dart pub global activate very_good_cli` - `very_good` must be on your PATH **How it works:** diff --git a/hooks/scripts/check-vgv-cli_test.sh b/hooks/scripts/check-vgv-cli_test.sh index bf25b3b..2a049af 100644 --- a/hooks/scripts/check-vgv-cli_test.sh +++ b/hooks/scripts/check-vgv-cli_test.sh @@ -93,15 +93,15 @@ assert_decision aside "server-less MCP name" '{"tool_name":"MCP:tes echo "" echo "--- Very Good CLI tools, CLI present and current ---" -stub_cli 1.5.0 +stub_cli 1.6.0 assert_decision allow "version above minimum" "$VGV_TOOL" assert_decision allow "marketplace-namespaced tool" "$VGV_PLUGIN_TOOL" -stub_cli 1.3.0 +stub_cli 1.5.0 assert_decision allow "version exactly at minimum" "$VGV_TOOL" echo "" echo "--- Very Good CLI tools, CLI missing or outdated ---" -stub_cli 1.2.9 +stub_cli 1.4.9 assert_decision deny "version below minimum" "$VGV_TOOL" stub_cli 0.9.0 assert_decision deny "major version below minimum" "$VGV_TOOL" @@ -110,7 +110,7 @@ assert_decision deny "CLI not installed" "$VGV_TOOL" echo "" echo "--- CLI reachable only through the pub-cache fallback ---" -stub_cli 1.5.0 --in-pub-cache +stub_cli 1.6.0 --in-pub-cache assert_decision allow "resolved from PUB_CACHE, not PATH" "$VGV_TOOL" echo "" diff --git a/hooks/scripts/vgv-cli-common.sh b/hooks/scripts/vgv-cli-common.sh index 31e53ef..f56ec88 100755 --- a/hooks/scripts/vgv-cli-common.sh +++ b/hooks/scripts/vgv-cli-common.sh @@ -1,9 +1,9 @@ #!/bin/bash # Shared helpers for Very Good CLI version checks and hook deny responses. -MIN_VERSION="1.3.0" +MIN_VERSION="1.5.0" MIN_MAJOR=1 -MIN_MINOR=3 +MIN_MINOR=5 MIN_PATCH=0 deny() { diff --git a/hooks/scripts/warn-missing-mcp_test.sh b/hooks/scripts/warn-missing-mcp_test.sh index 3090a51..f7915c3 100755 --- a/hooks/scripts/warn-missing-mcp_test.sh +++ b/hooks/scripts/warn-missing-mcp_test.sh @@ -80,19 +80,19 @@ assert_warns() { echo "=== warn-missing-mcp tests ===" echo "" echo "--- CLI present and current (no warning) ---" -stub_cli 1.5.0 +stub_cli 1.6.0 assert_silent "version above minimum" -stub_cli 1.3.0 +stub_cli 1.5.0 assert_silent "version exactly at minimum" -stub_cli 1.5.0 --in-pub-cache +stub_cli 1.6.0 --in-pub-cache assert_silent "resolved from PUB_CACHE, not PATH" echo "" echo "--- CLI missing or outdated ---" no_cli assert_warns "not installed" "CLI not installed" -stub_cli 1.2.9 -assert_warns "1.2.9 is too old" "version below minimum" +stub_cli 1.4.9 +assert_warns "1.4.9 is too old" "version below minimum" echo "" echo "--- Version unreadable (shim present, dart missing) ---"