diff --git a/.changeset/eager-rams-jump.md b/.changeset/eager-rams-jump.md new file mode 100644 index 000000000..694fc54be --- /dev/null +++ b/.changeset/eager-rams-jump.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4008 +--- +**`gsd-roadmapper` no longer contradicts itself on write-vs-approve ordering** — the agent's role, output format, and completion checklist now match its write-first execution flow (write for durability, return `## ROADMAP CREATED` with a preview; the orchestrator presents and owns the approval gate), and the orphaned `## ROADMAP DRAFT` template that matched no orchestrator branch is gone. (#3797) diff --git a/agents/gsd-roadmapper.md b/agents/gsd-roadmapper.md index a34cbecb6..b2e28b0ac 100644 --- a/agents/gsd-roadmapper.md +++ b/agents/gsd-roadmapper.md @@ -42,7 +42,7 @@ This ensures project-specific patterns, conventions, and best practices are appl - Apply goal-backward thinking at phase level - Create success criteria (2-5 observable behaviors per phase) - Initialize STATE.md (project memory) -- Return structured draft for user approval +- Write ROADMAP.md and STATE.md immediately (durability — artifacts persist even if context is lost), then return a structured summary for the orchestrator to present; approval is the orchestrator's gate, revision is a re-run (#3797) @@ -446,12 +446,18 @@ Key sections: - Accumulated Context (decisions, todos, blockers) - Session Continuity -## Draft Presentation Format +## Summary Preview Format -When presenting to user for approval: +The post-write `## ROADMAP CREATED` return carries this preview block (renamed from the pre-#3797 draft format — the orchestrator branches only on `ROADMAP CREATED`/`ROADMAP BLOCKED`, presents the roadmap, and owns the approval gate): ```markdown -## ROADMAP DRAFT +## ROADMAP CREATED + +**Files written:** +- .planning/ROADMAP.md +- .planning/STATE.md + +### Roadmap Preview **Phases:** [N] **Granularity:** [from config] @@ -483,11 +489,10 @@ When presenting to user for approval: ✓ All [X] v1 requirements mapped ✓ No orphaned requirements -### Awaiting - -Approve roadmap or provide feedback for revision. ``` +The orchestrator presents this roadmap and collects approval or feedback; revisions are applied on re-run (Step 9). + @@ -751,10 +756,9 @@ Roadmap is complete when: - [ ] ROADMAP.md structure complete - [ ] STATE.md structure complete - [ ] REQUIREMENTS.md traceability update prepared -- [ ] Draft presented for user approval -- [ ] User feedback incorporated (if any) -- [ ] Files written (after approval) -- [ ] Structured return provided to orchestrator +- [ ] Files written immediately (durability — Step 7) +- [ ] Structured summary (## ROADMAP CREATED + preview) returned for orchestrator presentation and approval +- [ ] User feedback incorporated on re-run (if any) Quality indicators: diff --git a/gsd-core/references/agent-contracts.md b/gsd-core/references/agent-contracts.md index cc0321c20..8a51153da 100644 --- a/gsd-core/references/agent-contracts.md +++ b/gsd-core/references/agent-contracts.md @@ -19,7 +19,7 @@ This doc describes what IS, not what should be. Casing inconsistencies are docum | gsd-research-synthesizer | Multi-research synthesis | `## SYNTHESIS COMPLETE`, `## SYNTHESIS BLOCKED` (unconsumed: blocked-research return — spawners detect failure via the #222 SUMMARY.md-on-disk check, no dispatch branch keys on the marker) | `gsd-core/workflows/new-milestone.md`, `gsd-core/workflows/new-project.md` | sentinel-match | | gsd-debugger | Debug investigation | `## DEBUG COMPLETE`, `## ROOT CAUSE FOUND`, `## CHECKPOINT REACHED`, `## INVESTIGATION INCONCLUSIVE`, `## TDD CHECKPOINT`, `## FIX REJECTED BY GUARDRAIL` | `agents/gsd-debug-session-manager.md`, `gsd-core/workflows/diagnose-issues.md`, `gsd-core/workflows/plan-phase.md`, `agents/gsd-executor.md` | sentinel-match | | gsd-debug-session-manager | Debug checkpoint loop | `## DEBUG SESSION COMPLETE`, `## CONTINUE_REQUIRED` | `gsd-core/workflows/debug.md` | sentinel-match | -| gsd-roadmapper | Roadmap creation/revision | `## ROADMAP CREATED`, `## ROADMAP REVISED`, `## ROADMAP BLOCKED`, `## ROADMAP DRAFT` (unconsumed: draft-presentation format the shipped execution flow never invokes — Step 8 returns `## ROADMAP CREATED`; retained for interactive draft review) | `gsd-core/workflows/new-milestone.md`, `gsd-core/workflows/new-project.md` | sentinel-match | +| gsd-roadmapper | Roadmap creation/revision | `## ROADMAP CREATED`, `## ROADMAP REVISED`, `## ROADMAP BLOCKED` | `gsd-core/workflows/new-milestone.md`, `gsd-core/workflows/new-project.md` | sentinel-match | | gsd-ui-auditor | UI review | `## UI REVIEW COMPLETE` | `gsd-core/workflows/ui-review.md` | sentinel-match | | gsd-dom-verifier | Live-DOM UAT verification | No marker (writes `{phase}-DOM-VERIFY.md` directly; the frontmatter `outcome` / `reason` scalars carry the verdict, and `could_not_look` is never conflated with `nothing_to_report`) | `{phase}-DOM-VERIFY.md` artifact, written by the `live-dom-uat` capability's `execute:wave:post` step dispatched from `gsd-core/workflows/execute-phase.md` | artifact+query | | gsd-ui-checker | UI validation | `## ISSUES FOUND`, `## UI-SPEC VERIFIED` | `gsd-core/workflows/plan-phase.md`, `gsd-core/workflows/quick/steps/plan-checker-loop.md`, `gsd-core/workflows/ui-phase.md`, `gsd-core/workflows/verify-work.md`, `agents/gsd-plan-checker.md` | sentinel-match | @@ -57,7 +57,7 @@ This doc describes what IS, not what should be. Casing inconsistencies are docum - `structured-return` -- the agent has no way to write files (no `Write` tool) or simply doesn't; it returns parseable sections, a table, or JSON inline, and the caller reads that return text directly 5. Markers must appear as H2 headings (`## `) at the start of a line in the agent's final output 6. The `Consumed by` / `Kind` columns are machine-enforced by `check:contract-drift` (`scripts/check-contract-drift.cjs`), which cross-checks this table against what each `agents/*.md` file actually emits in-fence and what every `gsd-core/workflows/**`, `commands/**`, and `agents/**` file actually consumes. Update this table whenever an agent's return contract changes -- a stale row is a violation the check will report, not something to leave for later. -7. A marker entry annotated `(unconsumed: )` is emitted deliberately but matched by no workflow, command, or agent — e.g. `## ROADMAP DRAFT`, a presentation format a human approves interactively. The check still verifies the marker is declared **and** emitted, and still counts it for case-collision purposes; only the consumer requirement is waived. Use it for display formats, never to silence a real orphan. +7. A marker entry annotated `(unconsumed: )` is emitted deliberately but matched by no workflow, command, or agent — a display/presentation format no orchestrator branch consumes (the `## ROADMAP DRAFT` header was the canonical case until #3797 folded its content into `## ROADMAP CREATED`). The check still verifies the marker is declared **and** emitted, and still counts it for case-collision purposes; only the consumer requirement is waived. Use it for display formats, never to silence a real orphan. ## Key Handoff Contracts diff --git a/tests/no-bare-gsd-tools-command-position.test.cjs b/tests/no-bare-gsd-tools-command-position.test.cjs index 0816c4035..c73aad3e3 100644 --- a/tests/no-bare-gsd-tools-command-position.test.cjs +++ b/tests/no-bare-gsd-tools-command-position.test.cjs @@ -105,7 +105,7 @@ const BARE_COMMAND_RE = new RegExp( const PROSE_ALLOWLIST = [ { file: 'agents/gsd-executor.md', line: 795, reason: 'describes the SDK return envelope of `gsd-tools query commit`; not an instruction to run the bare word' }, { file: 'agents/gsd-phase-researcher.md', line: 33, reason: 'package-legitimacy provenance rule names the command as the source of an OK verdict; descriptive' }, - { file: 'agents/gsd-roadmapper.md', line: 642, reason: 'parenthetical "e.g." naming SDK queries a user *could* run; not an agent instruction' }, + { file: 'agents/gsd-roadmapper.md', line: 647, reason: 'parenthetical "e.g." naming SDK queries a user *could* run; not an agent instruction' }, { file: 'agents/gsd-intel-updater.md', line: 40, reason: 'cross-platform note names the `gsd-tools intel ` CLI surface descriptively ("CLI invocations go through..."); not an agent instruction' }, { file: 'gsd-core/workflows/execute-plan.md', line: 419, reason: 'describes the downstream SDK validation step (`validated downstream by ...`); names the mechanism, does not instruct the agent to type it' }, ]; diff --git a/tests/roadmapper-contract-guard.test.cjs b/tests/roadmapper-contract-guard.test.cjs new file mode 100644 index 000000000..1508790d3 --- /dev/null +++ b/tests/roadmapper-contract-guard.test.cjs @@ -0,0 +1,57 @@ +'use strict'; + +// ───────────────────────────────────────────────────────────────────────────── +// #3797 — gsd-roadmapper must follow ONE contract: write-first. +// +// The agent's role blurb, output format, and completion checklist described +// an approve-first flow ("Return structured draft for user approval", +// "Awaiting / Approve roadmap", "Files written (after approval)") while its +// execution flow said "Write Files Immediately ... Write files first, then +// return" with reactive-only revision. The approval gate belongs to the +// ORCHESTRATOR (new-project.md reads the written ROADMAP.md, presents it, +// and gates on approval/auto-mode itself — a subagent cannot host the user +// loop). The agent's write-first execution flow is the intentional contract +// (explicit durability rationale; the #2255 write-guard arming in the same +// Step 7); the approve-first text is a leftover and must not return. +// ───────────────────────────────────────────────────────────────────────────── + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const ROADMAPPER = path.join(__dirname, '..', 'agents', 'gsd-roadmapper.md'); + +// agents/*.md is shipped agent text — the bytes ARE what the runtime loads; +// a structural scan over it tests the deployed contract. +test('#3797: the roadmapper follows one write-first contract (approval is the orchestrator\'s)', () => { + const md = fs.readFileSync(ROADMAPPER, 'utf-8'); + assert.ok( + !/for (user )?approval/i.test(md), + '#3797: the agent cannot host a user approval loop — approve-first phrasing must not return', + ); + assert.ok( + !/Awaiting \/ Approve/.test(md), + '#3797: the Awaiting/Approve output footer is the old contract; revision is the orchestrator\'s gate plus Step 9 re-runs', + ); + assert.ok( + !/^## ROADMAP DRAFT/m.test(md), + '#3797: the agent returns ## ROADMAP CREATED — a DRAFT header matches no orchestrator branch (they branch on CREATED/BLOCKED only)', + ); + assert.ok( + /Files written immediately/.test(md), + 'the checklist must pin the write-first ordering', + ); + assert.ok( + /^## Step 7: Write Files Immediately$/m.test(md), + 'the write-first execution flow (durability rationale) is the intended contract and must stay', + ); + assert.ok( + !/\(after approval\)/.test(md), + '#3797: the checklist must not claim files are written after an approval the agent never hosts', + ); + assert.ok( + /^## Step 9: Handle Revision \(if needed\)$/m.test(md), + 'reactive revision (Step 9) stays — it is the revision path under the write-first contract too', + ); +});