plan-agent 8.7.0 · merged 1b4f657

Red-green-verify plans

The implementation-plan skill now decides whether a plan should force its tests to fail before any implementation exists — and shapes the plan's steps into RED / GREEN / VERIFY / SHIP phases when it should.

6files
+291insertions
4commits
13threads resolved
2open items

At a glance

A plan could name its tests and still be implemented backwards: build the feature, then write a test that passes against whatever got built. That test proves the implementation matches itself, not the objective. Nothing in the spec format forced the failure to come first.

No renderer, parser, or build change was needed. ### Phase: groupings, per-step Verify: markers, and build stopping at each phase boundary all shipped in 8.6.0. This release is the guidance that uses them — five markdown files and a version bump. That is why the diff is 291 lines and touches no .mjs.

Architecture and code paths

Nothing executes here. The deliverable is text the model reads at a specific point in a workflow, so the "code path" is the read order — and the contract it has to keep with machinery that already exists.

Where the decision happens

Detection lives in SKILL.md Step 2, sub-step 1 — after right-sizing classifies the work, before any step is drafted. It has to be there: phases are headings inside ## Steps, so the shape must be settled before the steps exist.

flowchart TD
    A["Step 2.1
classify the work"] --> B{"--tdd or --no-tdd
passed?"}
    B -->|"--no-tdd"| S["single-pass steps"]
    B -->|"--tdd"| F["force phases
RED step 1 stands up the runner"]
    B -->|neither| C{"steps touch
application source?"}
    C -->|"no — Tier 2"| S
    C -->|yes| D{"Step 0b ran?"}
    D -->|"no — --quick"| E{"cheap runner probe
test script / pytest.ini / *_test.go"}
    E -->|hit| G["red-green-verify"]
    E -->|miss| S
    D -->|yes| H{"runner found?"}
    H -->|yes| G
    H -->|"no / borderline"| I["AskUserQuestion once"]
    I --> G
    I --> S
Diagram source (mermaid) — renders in the hosted artifact. The two branches that did not exist in the first commit: the --quick probe and the --tdd override. Both were review findings.

What a detected plan emits

Four ### Phase: headings inside ## Steps, over flat global numbering. That flatness is load-bearing: it is what keeps [x] markers valid when phases are added to an in-progress plan, and why build resumes at the first unmarked step rather than at a phase start.

  1. 01
    Phase: RED

    Test files only, no implementation source. The Verify: line demands the failure output, not a claim. Failing for the right reason is the assertion — a test that errors on a missing import has not gone red, it has not run.

  2. 02
    Phase: GREEN

    Minimum change that passes, re-running after every edit. The 8-iteration cap is written into the phase's last step — a cap that lives only in the guideline has nothing to stop the loop. That step says report the exact blocker, never success.

  3. 03
    Phase: VERIFY

    Full suite, lint, and typecheck where the project has one. The browser pass — layout, targets ≥ 44×44px, zero hydration warnings — is UI-only. A backend or library plan omits it rather than inventing an affected page.

  4. 04
    Phase: SHIP

    Conditional on the user having asked to ship. build Step 6 commits only on request; a SHIP phase that committed unconditionally would override that from inside the plan. When in doubt, omit — a missing SHIP costs one prompt, an unwanted one costs a commit nobody asked for.

The read order

Start at SKILL.md Step 2.1 for the trigger, then guidelines/red-green-verify.md for everything the phases contain. right-sizing.md only arbitrates a collision (below). Do not start in the guideline — it reads as unconditional until you have seen the gate.

Decisions

Guidance, not machinery

Every phase mechanism already existed. Adding a renderer flag or a build mode would have duplicated rules in two places and bumped two skills. The plan document carries the discipline, and build enforces it for free by walking steps and stopping at boundaries.

Detected, not flagged

An opt-in flag would be forgotten. Always-on would force a RED phase onto docs-only plans, where a grep -q check is the only thing that could fail. Detection keys on the Tier 1 signal Step 5c already computes, and asks once when the call is genuinely close.

Both browser MCP surfaces, prefixed

A reviewer asked to drop mcp__Claude_Browser__* for mcp__claude-in-chrome__*, on the premise the former appears nowhere else in the repo. It does — git-agent/skills/ship-autonomous/SKILL.md:4 declares five of its tools. The repo uses both. The real point inside the finding was precision: bare read_page is ambiguous with two servers connected, so the names now carry their prefix and the guideline says to write whichever the target repo has.

Agent-driven checks and committed scripts are not interchangeable

The first draft said "a script driving the MCP tools." Those are model-side calls; no .mjs file can invoke read_page. The RED bullet now splits explicitly: an agent-driven pass (the step names the calls, the Verify: line is the reported measurement) or a committed script (Playwright/Puppeteer, which is then an ordinary RED test with a Run: command).

RGV wins the headings

Phases now have two unrelated rationales — context budget (the Phased profile) and discipline (this shape). They cannot share one heading run: ### Phase: Parse beside ### Phase: RED leaves a reader unable to tell what a boundary means. RGV takes the headings; context seams live inside it. If that makes GREEN too large for one window, the objective was two plans.

Tradeoffs and rejected options

RejectedWhyWhat would revisit it
Enforce in build — refuse GREEN before a recorded RED failure Duplicates the rules in two skills and bumps both. build already walks steps and stops at boundaries. Evidence that plans get restructured mid-implementation to skip RED.
Always-on for every plan Forces a RED phase onto docs-only work with nothing runnable to fail. Never — Tier 2 has no failure to author.
Opt-in --tdd only MINOR bump and zero risk, but a flag nobody remembers is a feature nobody uses. If detection proves noisy in practice.
Align the GREEN cap to tdd-loop's five iterations Eight is what this workflow specified; the two skills are independent. A deliberate cross-plugin reconciliation.
AbortController + setTimeout in the driver sample Guards against Node <18, which reached EOL in April 2025. Three extra lines for no live runtime. A consumer pinned to an EOL Node.

Learnings

Three things were walked, not weighed — they only surfaced by running the thing.

The poll loop did not poll

The shipped Node driver went through two wrong versions. First it declared res with var inside a try in the loop — it worked by hoisting, but on timeout printed server never came up: undefined. Fixing the scoping exposed the real bug underneath: fetch resolves for 503, so the loop broke on the first transient response and reported "unhealthy" against a server that was merely still booting.

A poll loop that stops at the first answer is not polling. Both versions looked correct on the page and only failed against a dev server with a realistic boot sequence.

let res
for (let i = 0; i < 60; i++) {
  // A booting server answers 503 before it answers 200, and fetch does not
  // throw on either — so poll until res.ok, not until the first response.
  try { res = await fetch(url, { signal: AbortSignal.timeout(1000) }) } catch {}
  if (res?.ok) break
  await new Promise(r => setTimeout(r, 500))   // also bounds a hung request
}
if (!res?.ok) throw new Error(`never healthy in 30s (last: ${res?.status ?? 'no response'})`)

A reviewer's premise is not a finding

Two bots independently claimed mcp__Claude_Browser__* appears nowhere else in the repo. One grep disproved it. The finding still had a real point inside it — but acting on the stated premise would have switched the guideline to the wrong surface. Verify the premise before acting on the conclusion.

Three bots, one PR, no memory between runs

Copilot, Codex, and CodeRabbit each re-reviewed on every push, re-firing against older commits after fixes had landed. CodeRabbit hit its own rate limit. The productive shape was one triage pass on merit — four of ten findings were real defects, the rest polish — then a single fix commit, then stop. Copilot's last three runs all returned "no new comments."

Tests and verification

No test files ship in this diff — it is guidance. Verification drove the three real surfaces instead.

The driver sample, extracted verbatim and run

Pulled out of the shipped markdown with awk on the fence — not retyped — and run against a dev server that binds after 1.2s then 503s three times:

> dev
> node server.mjs
[dev] listening on 3000
[dev] 503 #1
[dev] 503 #2
[dev] 503 #3
[dev] 200 #4
EXIT=0

Failure paths: permanently-unhealthy server → never healthy in 30s (last: 503) at 31s elapsed, child killed, port freed. Crashing dev script → (last: no response). Exit codes deterministic at 1 / 0, which is what lets the guideline claim the exit code is the Verify: line.

The render pipeline

The agent surface

Loaded the plugin headlessly and asked it to read back the three rules the review round added. It returned them unprompted by their wording — the strongest available evidence that a guidance change works.

SHIP: Only when all three prior phases are green **and** the user explicitly
      asked to ship — otherwise omit it and let VERIFY be last.
VERIFY-BROWSER: Only when the plan touches UI — backend/CLI/library plans omit it.
QUICK: One cheap runner check; no hit means no RGV — unless --tdd was passed,
      which overrides and makes RED step 1 stand up the runner.
Knowingly untested. No end-to-end run of implementation-plan producing an RGV plan from a bare objective — the interactive path needs AskUserQuestion. Detection was verified by reading back the rules, not by observing a plan get classified. Also unrun: test-skill-behavior-baselines.sh, which spawns headless claude sessions and exceeded the timeout; it covers five unrelated skills.

Review follow-ups and tech debt

Tracked issue 530

1. extract-plan-spec.mjs silently drops all progress state. A spec with 1. [x] … and - [x] … renders correctly to HTML, but extracting back returns bare 1. and - bullets. Reproduced identically on an unphased plan, so it is pre-existing, not a regression from the phase work. Impact is narrow today — the .md spec is the source of truth and extract only runs for legacy HTML-only plans — but build resumes at the first unmarked step, so a legacy in-progress plan re-specced this way silently redoes finished work. Phased RGV plans are explicitly designed to span context windows, which is exactly when resumption happens.

2. RED evidence does not survive a phase checkpoint. references/phase-checkpoints.md persists step markers and the Decisions ledger at a boundary, not captured stdout — but SHIP requires the exact RED failure output in the PR body. A session that compacts after RED cannot produce it.

Untracked

Known ceiling. Detection reads a runner's presence, never its health. A repo with a test script that has not passed in months still reads as RGV-eligible. Upgrade path: probe exit status rather than existence — but that costs a subprocess in Step 2, which is why it was not done.

Files touched

PathWhy
guidelines/red-green-verify.md new 172 lines. The four phases, the applies/skip/ask rules, the foreground Node driver, the UI-only scoping. Everything else points here.
skills/implementation-plan/SKILL.md Detection in Step 2.1; --tdd/--no-tdd in the flag list and argument-hint; guidelines-library entry; Step 5c note tying ## Tests to the RED steps.
guidelines/right-sizing.md Calibration row, and the precedence rule for when both phasing rationales apply.
kit/plugins/plan-agent/README.md Flag table rows, the detection paragraph, and both argument summaries — the second one at line 586 was missed in the first commit.
kit/plugins/plan-agent/CHANGELOG.md 8.7.0 entry.
.claude-plugin/marketplace.json 8.6.0 → 8.7.0. Version lives only here for relative-path plugins; a CI guard fails the PR if it does not exceed base.