From 8c9265d4e5ee9a9ac6298ec1d09e969b3ed98703 Mon Sep 17 00:00:00 2001 From: Cody Anderson <70287898+arakasi1@users.noreply.github.com> Date: Mon, 31 Aug 2026 17:08:11 -0600 Subject: [PATCH] fix(#3724): warning-only Dimension 3b findings no longer force the revision loop (#3758) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#3724): stop advisory Dimension 3b findings from forcing the revision loop Dimension 3b (undeclared/temporal coupling, #1954) is spec'd "never a blocker" but tagged severity: warning — the tier plan-phase's revision loop counts as must-fix — and the planner is never taught the rule, so every multi-wave phase touching shared mutable state replans at least once, and intentionally coupled plans re-flag identically every iteration to the stall prompt. Three coordinated changes: - gsd-plan-checker: retag 3b to severity: info, the tier references/revision-loop.md already exempts by design; recognize a coupling_justified frontmatter declaration in the Do-NOT-flag list so deliberate pairs converge. Additions are offset by trimming 3b motivation prose — the checker sits 45 bytes under its LARGE hard cap. - plan-phase step 12: INFO-only accept — an issues block with zero BLOCKER/WARNING entries accepts the plan and surfaces the advisories instead of re-entering the revision loop. Real blockers and warnings still gate unconditionally. - gsd-planner: slim pointer in assign_waves to the new progressive-disclosure reference gsd-core/references/planner-coupling.md (the planner sits 19 chars under its own cap), which carries the shared-mutable-state rule and the coupling_justified escape hatch so first-pass plans avoid the finding when the coupling is unintentional. Documented the coupling_justified field in docs/reference/plan-md.md. Growth acks per #2914; inventory manifest and install-tree fixtures regenerated for the new reference file. Closes #3724 Claude-Session: https://claude.ai/code/session_01GshUzpGjoxiw6uNRiFMHvM * test(#3724): pin Dimension 3b at severity: info The severity retag makes the old assertion (severity: warning) stale; lock the advisory tier from both directions — info must be present, warning must not — so a future edit cannot silently re-arm the revision-loop trigger. Refs #3724 Claude-Session: https://claude.ai/code/session_01GshUzpGjoxiw6uNRiFMHvM * chore(#3724): changeset fragment for PR #3758 Refs #3724 Claude-Session: https://claude.ai/code/session_01GshUzpGjoxiw6uNRiFMHvM * docs(#3724): roster planner-coupling.md in docs/INVENTORY.md The new reference was enumerated in the manifest and all 19 install-tree fixtures but missing its row in the Modular Planner Decomposition table — the roster half the manifest-sync test cannot check. (Review Blocker.) Refs #3724 Claude-Session: https://claude.ai/code/session_01GshUzpGjoxiw6uNRiFMHvM * test(#3724): cover all four acceptance criteria (review round 1) - plan-checker-coupling: the 3b severity assertion is now a PARITY check deriving the exempt tier from revision-loop.md's flow instead of hardcoding info — editing either side alone reds the suite. New describe pins the other three criteria: plan-phase's INFO-only accept clause (proven failing-first), the BLOCKER + WARNING count staying intact, the coupling_justified Do-NOT-flag exemption + fix_hint, and the planner pointer + planner-coupling.md content. - ack fragment: $comment's plan-phase figure corrected to +79B; the 2775 pin note carried forward into the gsd-planner.md entry, updated for upstream's #3761/#3764 Rule-paragraph anchor (which this diff leaves verbatim). The parallel-dependent-plans re-anchor this commit originally carried was superseded by upstream #3764 during review; this branch no longer touches that file. Refs #3724 Claude-Session: https://claude.ai/code/session_01GshUzpGjoxiw6uNRiFMHvM * fix(#3724): review round 2 — align the stance enumeration, complete the template contract MAJOR: 's severity enumeration gains the INFO bullet so it agrees with Dimension 3b's 'ALWAYS INFO' mandate instead of contradicting it. Funded by extracting the inline block to the new progressive- disclosure reference gsd-core/references/plan-checker-examples.md (@-inlined from the same spot; #1949 precedent), which also restores the 3b motivation clause round 1 traded away (Nit 4) and nets the agent file SMALLER than base (49107 -> 48486) — the extraction the byte pressure was owed. MINOR: gsd-core/templates/phase-prompt.md now carries coupling_justified, and the field's shape becomes one 'plan-id: reason' string per coupled peer so a plan justified against two peers can express it; docs/reference/plan-md.md's Type column names the shape. NIT: the 3409 ack's plan-phase entry no longer calls the #1168 workflow ratchet an 'XL tier'. Acks and derived artifacts updated accordingly (checker entry removed — a shrink needs no ack; INVENTORY roster row + regen:derived for the new file). Claude-Session: https://claude.ai/code/session_01GshUzpGjoxiw6uNRiFMHvM * test(#3724): derive the 3b negative severity assertion (review round 2) Every severity token in the 3b span must BE the tier revision-loop.md exempts, replacing the hardcoded severity:warning negative — if the loop's exemption ever moves, the failure names the real conflict instead of blaming the agent file with a mutually-unsatisfiable pair. Claude-Session: https://claude.ai/code/session_01GshUzpGjoxiw6uNRiFMHvM * fix(#3724): refit the planner coupling pointer under the char cap Upstream #3299 (PR #3390) grew agents/gsd-planner.md to 49146 chars at the base, leaving 5 chars of headroom where the +16-char pointer was measured against 13 more. The pointer prose shortens to 'Non-file coupling:' — 49150 chars, back under the strict 49152-char cap — and the ack figures follow. The @-path the tests pin is unchanged. Claude-Session: https://claude.ai/code/session_01GshUzpGjoxiw6uNRiFMHvM * fix(#3724): re-home the plan-phase ack after the #3823 spent-fragment sweep Upstream #3078/#3823 deleted all fully-spent ack fragments, including 3409-unreachable-guard-arms.json, which carried this PR's plan-phase.md +79B append. Per the collision remedy that sweep added: take the deletion and home the still-live entry in this PR's own fragment. Figures re-measured at this merge base (90871 -> 90950 LF bytes). Claude-Session: https://claude.ai/code/session_01GshUzpGjoxiw6uNRiFMHvM * fix(#3724): absorb the spent #3172 plan-phase fragment into this PR's ack Upstream #3825 shipped 3172-stated-failing-direction.json naming only plan-phase.md, now spent at the base — colliding with this PR's live plan-phase entry. Per the #3003 pattern the fully-spent single-path fragment is deleted and this fragment stays the path's one source; figures re-measured at this base (93073 -> 93152 LF bytes). Claude-Session: https://claude.ai/code/session_01GshUzpGjoxiw6uNRiFMHvM * fix(#3724): review round 3 — true up the ack figures, restore the wave comment The fragment's absolute sizes are re-measured and anchored to base e40e9670 (planner 47259 -> 47330 chars, checker 45537 -> 44916 B, plan-phase 91186 -> 91265 LF bytes), with a note that absolutes rot as next moves — the deltas are the durable claims. The round-1 removal of the '# Implicit dependency: files_modified overlap forces a later wave.' pseudocode comment offset headroom base drift had already returned, so it is restored (findings 2-3). Changeset gains the (#3724) backlink (finding 4). Claude-Session: https://claude.ai/code/session_01GshUzpGjoxiw6uNRiFMHvM * fix(#3724): review round 4 — close the verify-work surface, harden the boundaries BLOCKER: verify-work.md's verify_gap_plans is the second multi-plan consumer of the checker's sentinels, and its ISSUES FOUND handler entered revision_loop with zero severity parsing — the guaranteed replan #3724 fixed in plan-phase, alive on the gap-closure surface. The handler now counts BLOCKER + WARNING and accepts INFO-only returns with advisories displayed. The checker's INFO stance bullet is reworded to the claim that is true everywhere ('revision gates count only BLOCKER + WARNING'). Minor 1: plan-phase's iteration_count >= 3 arm recounts severities, so an INFO-only third check accepts instead of halting on a '0 issues remain' user gate. Minor 2: the coupling_justified exemption now requires the entry to NAME the other plan, closing the blanket-suppression reading. Nit 1: INVENTORY row states the extraction buys cap headroom, not context. Nit 2: the advisory display gains a concrete format on both surfaces. Ack fragment re-anchored at base ddde001a: verify-work.md +264B (new entry), plan-phase.md +395B, checker still net negative (-512B). Claude-Session: https://claude.ai/code/session_01GshUzpGjoxiw6uNRiFMHvM * test(#3724): pin the verify-work accept and the iteration-cap boundary (review round 4) Two wiring assertions: verify_gap_plans' ISSUES FOUND handler gates on BLOCKER + WARNING and accepts INFO-only blocks, and plan-phase's iteration_count >= 3 arm recounts severities instead of gating advisories — the limit+1 boundary of the gate this PR fixes. Claude-Session: https://claude.ai/code/session_01GshUzpGjoxiw6uNRiFMHvM * fix(#3724): review round 5 — fail closed at the gates, surface the advisory Blocker 1: the checker's step-10 status rule routes an INFO-only result to ## ISSUES FOUND (with a new ### Advisories (info) template section and a severity-aware recommendation) so the orchestrator receives the block and displays the advisory instead of silently accepting a bare PASSED. Blockers 2+3: all three gate surfaces (plan-phase step 12 both arms, verify-work verify_gap_plans) carry one canonical clause verbatim — an entry whose severity is missing or unrecognized counts as a BLOCKER (fail closed) — making the accept condition an explicit-INFO whitelist while keeping issue_count coherent for stall math. Major 1: the INFO stance bullet scopes its claim to the plan-phase and verify-work gates (quick mode's loop still revises on any ISSUES FOUND). Major 2: INVENTORY row and ack $comment state the extraction's real trade (readability, +0.6 KB eager runtime context), not a cap remedy. Minor 1: plan-md.md marks coupling_justified as prompt convention, unvalidated. Nit 1: ack absolutes re-anchored at base 1e67ec97; checker now +120B and acked. Claude-Session: https://claude.ai/code/session_01GshUzpGjoxiw6uNRiFMHvM * test(#3724): pin the round-5 contract — fail-closed parity, INFO-only return shape New: three-surface verbatim parity test for the fail-closed clause (Blockers 2+3); checker return-contract test for the INFO-only ## ISSUES FOUND route and advisories section (Blocker 1). All seven newly pinned tokens are absent at f3a5682d, so each new assertion fails pre-fix. Updated: accept-clause regexes track the explicit-INFO whitelist wording; the severity sweep scopes to the span's fenced yaml examples via yamlSeverityTiers (round-5 Minor 3, applied to the blocker negative too); the iteration-cap comment states it is a prose pin, not an executed boundary check (Minor 4); splitLines call sites document the line-pin coupling (Nit 2). Claude-Session: https://claude.ai/code/session_01GshUzpGjoxiw6uNRiFMHvM * fix(#3724): adopt next's line wrap in the 3b motivation clause — drops a wrap-only hunk from the diff Byte-identical content; the wrap difference was an artifact of the round-1 base adaptation predating upstream's #3003 landing. Claude-Session: https://claude.ai/code/session_01GshUzpGjoxiw6uNRiFMHvM * fix(#3724): review round 7 — gate every checker consumer, not just the two audited ones Blocker: quick/steps/plan-checker-loop.md (issue-named in #3724) gets the same canonical fail-closed clause and explicit-INFO whitelist accept as plan-phase/verify-work — an INFO-only result proceeds instead of entering quick mode's revision loop. Major: import.md plan_validate handles the checker return by severity (INFO-only never blocks an import) and is added to agent-contracts.md's consumer enumeration, which had omitted it. The checker's INFO stance bullet drops the quick-mode carve-out — the claim is universally true again now that every consuming gate is severity-aware. Minor: an applied coupling_justified exemption is surfaced as its own info advisory so a stale one-sided declaration stays observable. Nit: plan-phase's revision-iteration Display line is explicitly conditioned on not having already proceeded to step 13. Claude-Session: https://claude.ai/code/session_01GshUzpGjoxiw6uNRiFMHvM Emitted-Drift-Ack-Growth: import.md — #3724 round 7: the plan_validate step's checker-return handler becomes severity-aware — counts BLOCKER + WARNING failing closed and accepts an explicitly-INFO-only return with advisories displayed instead of blocking the import * test(#3724): pin the round-7 surfaces — five-gate parity, quick/import accepts, exemption visibility The verbatim fail-closed parity test extends to quick/steps/plan-checker-loop.md and import.md plan_validate; new assertions pin quick mode's INFO-only proceed, import's never-blocks accept, import.md's presence in agent-contracts.md's consumer row, and the surfaced coupling_justified exemption advisory. All four newly pinned token families are absent at the pre-fix head, so each new assertion fails first. Claude-Session: https://claude.ai/code/session_01GshUzpGjoxiw6uNRiFMHvM --------- Co-authored-by: Tom Boucher --- .changeset/gallant-newts-squeak.md | 5 + agents/gsd-plan-checker.md | 64 ++-- agents/gsd-planner.md | 2 + docs/INVENTORY-MANIFEST.json | 2 + docs/INVENTORY.md | 2 + docs/reference/plan-md.md | 1 + gsd-core/references/agent-contracts.md | 2 +- gsd-core/references/plan-checker-examples.md | 40 +++ gsd-core/references/planner-coupling.md | 42 +++ gsd-core/templates/phase-prompt.md | 4 + gsd-core/workflows/import.md | 4 +- gsd-core/workflows/plan-phase.md | 6 +- .../quick/steps/plan-checker-loop.md | 2 +- gsd-core/workflows/verify-work.md | 2 +- tests/fixtures/install-tree/antigravity.json | 2 + tests/fixtures/install-tree/augment.json | 2 + tests/fixtures/install-tree/claude-local.json | 2 + tests/fixtures/install-tree/claude.json | 2 + tests/fixtures/install-tree/cline.json | 2 + tests/fixtures/install-tree/codebuddy.json | 2 + tests/fixtures/install-tree/codex.json | 2 + tests/fixtures/install-tree/copilot.json | 2 + tests/fixtures/install-tree/cursor.json | 2 + tests/fixtures/install-tree/hermes.json | 2 + tests/fixtures/install-tree/kilo.json | 2 + tests/fixtures/install-tree/kimi-code.json | 2 + tests/fixtures/install-tree/kimi.json | 2 + tests/fixtures/install-tree/opencode.json | 2 + tests/fixtures/install-tree/pi.json | 2 + tests/fixtures/install-tree/qwen.json | 2 + tests/fixtures/install-tree/trae.json | 2 + tests/fixtures/install-tree/windsurf.json | 2 + tests/fixtures/install-tree/zcode.json | 2 + tests/plan-checker-coupling.test.cjs | 300 +++++++++++++++++- 34 files changed, 461 insertions(+), 55 deletions(-) create mode 100644 .changeset/gallant-newts-squeak.md create mode 100644 gsd-core/references/plan-checker-examples.md create mode 100644 gsd-core/references/planner-coupling.md diff --git a/.changeset/gallant-newts-squeak.md b/.changeset/gallant-newts-squeak.md new file mode 100644 index 000000000..b8324a379 --- /dev/null +++ b/.changeset/gallant-newts-squeak.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3758 +--- +**Advisory plan-checker findings no longer force a replan** — Dimension 3b (undeclared same-wave coupling, #1954) is retagged to the advisory `info` tier, plan-phase accepts INFO-only checker results instead of entering the revision loop, and planners can declare deliberate coupling with a new optional `coupling_justified` plan-frontmatter field that the checker recognizes — so multi-wave phases stop paying a guaranteed extra planner pass and intentionally coupled plans converge instead of stalling. (#3724) diff --git a/agents/gsd-plan-checker.md b/agents/gsd-plan-checker.md index e62291737..2be126f89 100644 --- a/agents/gsd-plan-checker.md +++ b/agents/gsd-plan-checker.md @@ -39,6 +39,7 @@ You are NOT the executor or verifier — you verify plans WILL work before execu **Required finding classification:** Every issue must carry an explicit severity: - **BLOCKER** — the phase goal will not be achieved if this is not fixed before execution - **WARNING** — quality or maintainability is degraded; fix recommended but execution can proceed +- **INFO** — advisory; every consuming gate counts only BLOCKER + WARNING, so INFO alone never forces a revision or blocks acceptance (#3724) Issues without a severity classification are not valid output. @@ -231,19 +232,24 @@ Execution; strong-but-local coupling inside one plan is fine): **Do NOT flag:** both sides only READ it, or it is immutable; the pair already overlaps in `files_modified` or `files_deleted` (report that once, on the file axis); the plans sit in a different wave, which already orders them; two tasks inside one plan; a vague same-subsystem claim naming no -resource; incompatible *transformations* of one entity — that is Dimension 9. +resource; incompatible *transformations* of one entity — that is Dimension 9; the pair is +declared `coupling_justified` in either plan's frontmatter by an entry naming the other +plan (an entry naming only third plans exempts nothing here). -**Severity: ALWAYS WARNING, never a blocker.** Coupling is sometimes intentional; the finding -lets the planner declare the edge, move a plan to a later wave, or justify the pair. +**Severity: ALWAYS INFO, never a blocker.** Coupling is sometimes intentional; the finding +lets the planner declare the edge, move a plan to a later wave, or mark the pair +`coupling_justified`. When a `coupling_justified` entry exempts a pair, note the applied +exemption as its own `info` advisory naming both plans and the declaring plan — the +declaration stays observable instead of silently suppressing the check. ```yaml issue: dimension: dependency_correctness - severity: warning + severity: info description: "Plans 02 and 03 are both Wave 1 with no depends_on, but 02 writes config key auth.session_ttl and 03 reads it" plans: ["02", "03"] - fix_hint: "Declare depends_on, move 03 to a later wave, or justify either order" + fix_hint: "Declare depends_on, move 03 to a later wave, or set coupling_justified" ``` ## Dimension 4: Key Links Planned @@ -861,9 +867,9 @@ Thresholds: 2-3 tasks/plan good, 4 warning, 5+ blocker (split required). ## Step 10: Determine Overall Status -**passed:** All requirements covered, all tasks complete, dependency graph valid, key links planned, scope within budget, must_haves properly derived. +**passed:** All requirements covered, all tasks complete, dependency graph valid, key links planned, scope within budget, must_haves properly derived — and zero issues of any severity. An INFO-only result is NOT `passed`. -**issues_found:** One or more blockers or warnings. Plans need revision. +**issues_found:** One or more issues of ANY severity, including INFO-only. Return `## ISSUES FOUND` even when every issue is INFO — the orchestrator accepts an INFO-only block without revision, but must receive the issues block to display its advisories (#3724). Plans need revision only when blockers or warnings are present. Severities: `blocker` (must fix), `warning` (should fix), `info` (suggestions). @@ -871,40 +877,7 @@ Severities: `blocker` (must fix), `warning` (should fix), `info` (suggestions). -## Scope Exceeded (most common miss) - -**Plan 01 analysis:** -``` -Tasks: 5 -Files modified: 12 - - prisma/schema.prisma - - src/app/api/auth/login/route.ts - - src/app/api/auth/logout/route.ts - - src/app/api/auth/refresh/route.ts - - src/middleware.ts - - src/lib/auth.ts - - src/lib/jwt.ts - - src/components/LoginForm.tsx - - src/components/LogoutButton.tsx - - src/app/login/page.tsx - - src/app/dashboard/page.tsx - - src/types/auth.ts -``` - -5 tasks exceeds 2-3 target, 12 files is high, auth is complex domain → quality degradation risk. - -```yaml -issue: - dimension: scope_sanity - severity: blocker - description: "Plan 01 has 5 tasks with 12 files - exceeds context budget" - plan: "01" - metrics: - tasks: 5 - files: 12 - estimated_context: "~80%" - fix_hint: "Split into: 01 (schema + API), 02 (middleware + lib), 03 (UI components)" -``` +@~/.claude/gsd-core/references/plan-checker-examples.md @@ -993,13 +966,20 @@ Plans verified. Run `/gsd:execute-phase {phase}` to proceed. - Plan: {plan} - Fix: {fix_hint} +### Advisories (info) + +**1. [{dimension}] {description}** +- Plan: {plan} +- Fix: {fix_hint} + ### Structured Issues (YAML issues list using format from Issue Format above) ### Recommendation -{N} blocker(s) require revision. Returning to planner with feedback. +{N} blocker(s), {M} warning(s) require revision. Returning to planner with feedback. +(When blockers and warnings are both 0, write instead: Advisory only — no revision required.) ``` diff --git a/agents/gsd-planner.md b/agents/gsd-planner.md index 609e81b7a..f09cfc85c 100644 --- a/agents/gsd-planner.md +++ b/agents/gsd-planner.md @@ -754,6 +754,8 @@ for each plan B in plan_order: ``` **Rule:** Same-wave plans must have zero `files_modified`/`files_deleted` overlap. After assigning waves, scan each wave; if any file appears in 2+ plans, bump the later plan to the next wave and repeat. + +Non-file coupling: @~/.claude/gsd-core/references/planner-coupling.md diff --git a/docs/INVENTORY-MANIFEST.json b/docs/INVENTORY-MANIFEST.json index 1752f2bdf..cc567c85d 100644 --- a/docs/INVENTORY-MANIFEST.json +++ b/docs/INVENTORY-MANIFEST.json @@ -251,8 +251,10 @@ "nyquist-compliance.md", "offer-next.md", "phase-argument-parsing.md", + "plan-checker-examples.md", "planner-antipatterns.md", "planner-chunked.md", + "planner-coupling.md", "planner-failing-direction.md", "planner-gap-closure.md", "planner-graphify-auto-update.md", diff --git a/docs/INVENTORY.md b/docs/INVENTORY.md index c16fb471d..8a36ccd6c 100644 --- a/docs/INVENTORY.md +++ b/docs/INVENTORY.md @@ -379,6 +379,7 @@ Full roster at `gsd-core/references/*.md`. References are shared knowledge docum | `api-coverage.md` | API-coverage gate reference (full-coverage-by-default) for the `ai-integration` capability's `verify:pre` blocking gate (#1562) — matrix format, trigger, tuning, detector CLI, and the seal-time outcome table naming every pass/block arm including `scope_unavailable` (#3909). | | `ai-frameworks.md` | AI framework decision-matrix reference for `gsd-framework-selector`. | | `executor-examples.md` | Worked examples for the gsd-executor agent. | +| `plan-checker-examples.md` | Worked example for the gsd-plan-checker agent (Scope Exceeded), moved out of the inline `` block to keep worked examples structurally separate from the agent contract, matching the other agents' reference layout. `@`-inlined at load (eager), so this costs ~0.6 KB of runtime context versus keeping it inline — a readability trade, not a size-cap remedy (#3724). | | `doc-conflict-engine.md` | Shared conflict-detection contract for ingest/import workflows. | | `execute-mvp-tdd.md` | Runtime gate semantics for execute-phase under MVP+TDD — pre-task failing-test verification, end-of-phase blocking review. | | `mvp-concepts.md` | Cross-reference index for the six MVP-related reference files; maps each file to its purpose and which workflow loads it. | @@ -428,6 +429,7 @@ The `gsd-planner` agent is decomposed into a core agent plus reference modules t | `planner-interface-context.md` | Interface context rules for executors — how to extract key interfaces/types/exports from existing code and document new interfaces that downstream plans will consume. | | `planner-load-graph-context.md` | Planner's load_graph_context step: knowledge-graph freshness + dependency-context query via the gsd_run launcher (extracted from gsd-planner.md). | | `planner-verify-command-grounding.md` | Verify Command Grounding rules (#2401): inherit `prior_verify_commands` verbatim when the story repeats, prefer `npm --prefix run