From 303fd26b4578146b59e97fb909b760361593a369 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 24 Apr 2026 20:20:02 -0400 Subject: [PATCH] fix(#2662): add state.add-roadmap-evolution SDK handler; insert-phase uses it (#2683) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /gsd-insert-phase step 4 instructed the agent to directly Edit/Write .planning/STATE.md to append a Roadmap Evolution entry. Projects that ship a protect-files.sh PreToolUse hook (a recommended hardening pattern) blocked the raw write, silently leaving STATE.md out of sync with ROADMAP.md. Adds a dedicated SDK handler state.add-roadmap-evolution (plus space alias) that: - Reads STATE.md through the shared readModifyWriteStateMd lockfile path (matches sibling mutation handlers — atomic against concurrent writers). - Locates ### Roadmap Evolution under ## Accumulated Context, or creates both sections as needed. - Dedupes on exact-line match so idempotent retries are no-ops ({ added: false, reason: "duplicate" }). - Validates --phase / --action presence and action membership, throwing GSDError(Validation) for bad input (no silent { ok: false } swallow). Workflow change (insert-phase.md step 4): - Replaces the raw Edit/Write instructions for STATE.md with gsd-sdk query state.patch (for the next-phase pointer) and gsd-sdk query state.add-roadmap-evolution (for the evolution log). - Updates success criteria to check handler responses. - Drops "Write" from commands/gsd/insert-phase.md allowed-tools (no step in the workflow needs it any more). Tests (vitest, sdk/src/query/state-mutation.test.ts): subsection creation when missing; append-preserving-order when present; duplicate -> reason=duplicate; idempotence over two calls; three validation cases covering missing --phase, missing --action, and invalid action. This is the first SDK handler dedicated to STATE.md Roadmap Evolution mutations. Other workflows with similar raw STATE.md edits (/gsd-pause-work, /gsd-resume-work, /gsd-new-project, /gsd-complete-milestone, /gsd-add-phase) remain on raw Edit/Write and will need follow-up issues to migrate — out of scope for this fix. Closes #2662 --- commands/gsd/insert-phase.md | 1 - get-shit-done/workflows/insert-phase.md | 39 ++++-- sdk/src/query/QUERY-HANDLERS.md | 2 + sdk/src/query/index.ts | 5 +- sdk/src/query/state-mutation.test.ts | 160 ++++++++++++++++++++++++ sdk/src/query/state-mutation.ts | 140 +++++++++++++++++++++ 6 files changed, 334 insertions(+), 13 deletions(-) diff --git a/commands/gsd/insert-phase.md b/commands/gsd/insert-phase.md index e7c06a3f..2e6a7f62 100644 --- a/commands/gsd/insert-phase.md +++ b/commands/gsd/insert-phase.md @@ -4,7 +4,6 @@ description: Insert urgent work as decimal phase (e.g., 72.1) between existing p argument-hint: allowed-tools: - Read - - Write - Bash --- diff --git a/get-shit-done/workflows/insert-phase.md b/get-shit-done/workflows/insert-phase.md index c584718f..8805a6bd 100644 --- a/get-shit-done/workflows/insert-phase.md +++ b/get-shit-done/workflows/insert-phase.md @@ -63,19 +63,35 @@ Extract from result: `phase_number`, `after_phase`, `name`, `slug`, `directory`. -Update STATE.md to reflect the inserted phase: +Update STATE.md to reflect the inserted phase via SDK handlers (never raw +`Edit`/`Write` — projects may ship a `protect-files.sh` PreToolUse hook that +blocks direct STATE.md writes): -1. Read `.planning/STATE.md` -2. Update STATE.md's next-phase pointers to the newly inserted phase `{decimal_phase}`: - - Update structured field(s) used by tooling (e.g. `current_phase:`) to `{decimal_phase}`. - - Update human-readable recommendation text (e.g. `## Current Phase`, `Next recommended run:`) to `{decimal_phase}`. - - If multiple pointer locations exist, update all of them in the same edit. -3. Under "## Accumulated Context" → "### Roadmap Evolution" add entry: - ``` - - Phase {decimal_phase} inserted after Phase {after_phase}: {description} (URGENT) +1. Update STATE.md's next-phase pointer(s) to the newly inserted phase + `{decimal_phase}`: + + ```bash + gsd-sdk query state.patch '{"Current Phase":"{decimal_phase}","Next recommended run":"/gsd:plan-phase {decimal_phase}"}' ``` -If "Roadmap Evolution" section doesn't exist, create it. + (Adjust field names to whatever pointers STATE.md exposes — the handler + reports which fields it matched.) + +2. Append a Roadmap Evolution entry via the dedicated handler. It creates the + `### Roadmap Evolution` subsection under `## Accumulated Context` if missing + and dedupes identical entries: + + ```bash + gsd-sdk query state.add-roadmap-evolution \ + --phase {decimal_phase} \ + --action inserted \ + --after {after_phase} \ + --note "{description}" \ + --urgent + ``` + + Expected response shape: `{ added: true, entry: "- Phase ... (URGENT)" }` + (or `{ added: false, reason: "duplicate", entry: ... }` on replay). @@ -129,6 +145,7 @@ Phase insertion is complete when: - [ ] `gsd-sdk query phase.insert` executed successfully - [ ] Phase directory created - [ ] Roadmap updated with new phase entry (includes "(INSERTED)" marker) -- [ ] STATE.md updated with roadmap evolution note +- [ ] `gsd-sdk query state.add-roadmap-evolution ...` returned `{ added: true }` or `{ added: false, reason: "duplicate" }` +- [ ] `gsd-sdk query state.patch` returned matched next-phase pointer field(s) - [ ] User informed of next steps and dependency implications diff --git a/sdk/src/query/QUERY-HANDLERS.md b/sdk/src/query/QUERY-HANDLERS.md index b8224cb4..98bbfa72 100644 --- a/sdk/src/query/QUERY-HANDLERS.md +++ b/sdk/src/query/QUERY-HANDLERS.md @@ -62,6 +62,8 @@ No `gsd-tools.cjs` mirror — agents use these instead of shell `ls`/`find`/`gre Handlers for `**state.signal-waiting`**, `**state.signal-resume**`, `**state.validate**`, `**state.sync**` (supports `--verify` dry-run), and `**state.prune**` live in `state-mutation.ts`, with dotted and `state …` space aliases in `index.ts`. +**`state.add-roadmap-evolution`** (bug #2662) — appends one entry to the `### Roadmap Evolution` subsection under `## Accumulated Context` in STATE.md, creating the subsection if missing. argv: `--phase`, `--action` (`inserted|removed|moved|edited|added`), optional `--note`, `--after` (for `inserted`), and `--urgent` flag. Returns `{ added: true, entry }` or `{ added: false, reason: 'duplicate', entry }`. Throws `GSDError(Validation)` when `--phase` / `--action` are missing or action is not in the allowed set. Canonical replacement for raw `Edit`/`Write` on STATE.md in `insert-phase.md` / `add-phase.md` workflows — required when projects ship a `protect-files.sh` PreToolUse hook that blocks direct STATE.md writes. + **`state.json` vs `state.load` (different CJS commands):** - **`state.json`** / `state json` — port of **`cmdStateJson`** (`state.ts` `stateJson`): rebuilt STATE.md frontmatter JSON. Read-only golden: `read-only-parity.integration.test.ts` compares to CJS `state json` with **`last_updated`** stripped. diff --git a/sdk/src/query/index.ts b/sdk/src/query/index.ts index 67e6248d..c296a05a 100644 --- a/sdk/src/query/index.ts +++ b/sdk/src/query/index.ts @@ -32,7 +32,7 @@ import { stateRecordMetric, stateUpdateProgress, stateAddDecision, stateAddBlocker, stateResolveBlocker, stateRecordSession, stateSignalWaiting, stateSignalResume, stateValidate, stateSync, statePrune, - stateMilestoneSwitch, + stateMilestoneSwitch, stateAddRoadmapEvolution, } from './state-mutation.js'; import { configSet, configSetModelProfile, configNewProject, configEnsureSection, @@ -135,6 +135,7 @@ export const QUERY_MUTATION_COMMANDS = new Set([ 'state.sync', 'state sync', 'state.prune', 'state prune', 'state.milestone-switch', 'state milestone-switch', + 'state.add-roadmap-evolution', 'state add-roadmap-evolution', 'frontmatter.set', 'frontmatter.merge', 'frontmatter.validate', 'frontmatter validate', 'config-set', 'config-set-model-profile', 'config-new-project', 'config-ensure-section', 'commit', 'check-commit', 'commit-to-subrepo', @@ -324,7 +325,9 @@ export function createRegistry( registry.register('state.sync', stateSync); registry.register('state.prune', statePrune); registry.register('state.milestone-switch', stateMilestoneSwitch); + registry.register('state.add-roadmap-evolution', stateAddRoadmapEvolution); registry.register('state milestone-switch', stateMilestoneSwitch); + registry.register('state add-roadmap-evolution', stateAddRoadmapEvolution); registry.register('state signal-waiting', stateSignalWaiting); registry.register('state signal-resume', stateSignalResume); registry.register('state validate', stateValidate); diff --git a/sdk/src/query/state-mutation.test.ts b/sdk/src/query/state-mutation.test.ts index 46e1fe99..9efdb0ce 100644 --- a/sdk/src/query/state-mutation.test.ts +++ b/sdk/src/query/state-mutation.test.ts @@ -420,6 +420,166 @@ describe('stateAddDecision', () => { }); }); +// ─── stateAddRoadmapEvolution (bug #2662) ────────────────────────────────── + +describe('stateAddRoadmapEvolution', () => { + let tmpDir: string; + + beforeEach(async () => { + tmpDir = await mkdtemp(join(tmpdir(), 'gsd-state-evo-')); + }); + + afterEach(async () => { + await rm(tmpDir, { recursive: true, force: true }); + }); + + it('creates the Roadmap Evolution subsection when missing and appends the entry', async () => { + await setupTestProject(tmpDir); // MINIMAL_STATE has no Roadmap Evolution. + const { stateAddRoadmapEvolution } = await import('./state-mutation.js'); + + const result = await stateAddRoadmapEvolution( + ['--phase', '72.1', '--action', 'inserted', '--after', '72', + '--note', 'Fix critical auth bug', '--urgent'], + tmpDir, + ); + const data = result.data as Record; + expect(data.added).toBe(true); + expect(data.entry).toBe('- Phase 72.1 inserted after Phase 72: Fix critical auth bug (URGENT)'); + + const content = await readFile(join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); + expect(content).toContain('### Roadmap Evolution'); + expect(content).toContain('- Phase 72.1 inserted after Phase 72: Fix critical auth bug (URGENT)'); + // Subsection sits under Accumulated Context. + const idxAccum = content.indexOf('## Accumulated Context'); + const idxEvo = content.indexOf('### Roadmap Evolution'); + expect(idxAccum).toBeGreaterThan(-1); + expect(idxEvo).toBeGreaterThan(idxAccum); + }); + + it('appends to an existing Roadmap Evolution subsection preserving prior entries', async () => { + const stateWithEvo = `--- +gsd_state_version: 1.0 +milestone: v3.0 +milestone_name: SDK-First Migration +status: executing +--- + +# Project State + +## Current Position + +Phase: 10 — EXECUTING + +## Accumulated Context + +### Decisions + +None yet. + +### Roadmap Evolution + +- Phase 5 added: Baseline work +- Phase 6 added: Follow-up + +## Session Continuity + +Last session: 2026-04-07T10:00:00.000Z +`; + await setupTestProject(tmpDir, stateWithEvo); + const { stateAddRoadmapEvolution } = await import('./state-mutation.js'); + + const result = await stateAddRoadmapEvolution( + ['--phase', '72.1', '--action', 'inserted', '--after', '72', '--note', 'Urgent fix', '--urgent'], + tmpDir, + ); + expect((result.data as Record).added).toBe(true); + + const content = await readFile(join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); + expect(content).toContain('- Phase 5 added: Baseline work'); + expect(content).toContain('- Phase 6 added: Follow-up'); + expect(content).toContain('- Phase 72.1 inserted after Phase 72: Urgent fix (URGENT)'); + + // Order preserved: existing entries come before the new one. + const idx5 = content.indexOf('Phase 5 added'); + const idx6 = content.indexOf('Phase 6 added'); + const idxNew = content.indexOf('Phase 72.1 inserted'); + expect(idx5).toBeLessThan(idx6); + expect(idx6).toBeLessThan(idxNew); + }); + + it('dedupes exact-match entries and reports reason=duplicate', async () => { + await setupTestProject(tmpDir); + const { stateAddRoadmapEvolution } = await import('./state-mutation.js'); + + const argv = ['--phase', '72.1', '--action', 'inserted', '--after', '72', '--note', 'Fix X', '--urgent']; + + const first = await stateAddRoadmapEvolution(argv, tmpDir); + expect((first.data as Record).added).toBe(true); + + const second = await stateAddRoadmapEvolution(argv, tmpDir); + const data = second.data as Record; + expect(data.added).toBe(false); + expect(data.reason).toBe('duplicate'); + + // Entry appears exactly once. + const content = await readFile(join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); + const matches = content.match(/Phase 72\.1 inserted after Phase 72: Fix X \(URGENT\)/g) || []; + expect(matches.length).toBe(1); + }); + + it('is idempotent: calling twice with same input leaves a single entry', async () => { + await setupTestProject(tmpDir); + const { stateAddRoadmapEvolution } = await import('./state-mutation.js'); + const argv = ['--phase', '9', '--action', 'added', '--note', 'new work']; + + await stateAddRoadmapEvolution(argv, tmpDir); + await stateAddRoadmapEvolution(argv, tmpDir); + + const content = await readFile(join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); + const matches = content.match(/Phase 9 added: new work/g) || []; + expect(matches.length).toBe(1); + }); + + it('throws GSDError(Validation) when phase is missing', async () => { + await setupTestProject(tmpDir); + const { stateAddRoadmapEvolution } = await import('./state-mutation.js'); + const { GSDError, ErrorClassification } = await import('../errors.js'); + + await expect(stateAddRoadmapEvolution( + ['--action', 'inserted'], + tmpDir, + )).rejects.toSatisfy((err: unknown) => { + return err instanceof GSDError && err.classification === ErrorClassification.Validation; + }); + }); + + it('throws GSDError(Validation) when action is missing', async () => { + await setupTestProject(tmpDir); + const { stateAddRoadmapEvolution } = await import('./state-mutation.js'); + const { GSDError, ErrorClassification } = await import('../errors.js'); + + await expect(stateAddRoadmapEvolution( + ['--phase', '72.1'], + tmpDir, + )).rejects.toSatisfy((err: unknown) => { + return err instanceof GSDError && err.classification === ErrorClassification.Validation; + }); + }); + + it('throws GSDError(Validation) on invalid action', async () => { + await setupTestProject(tmpDir); + const { stateAddRoadmapEvolution } = await import('./state-mutation.js'); + const { GSDError, ErrorClassification } = await import('../errors.js'); + + await expect(stateAddRoadmapEvolution( + ['--phase', '72.1', '--action', 'frobnicated'], + tmpDir, + )).rejects.toSatisfy((err: unknown) => { + return err instanceof GSDError && err.classification === ErrorClassification.Validation; + }); + }); +}); + // ─── stateRecordSession ───────────────────────────────────────────────────── describe('stateRecordSession', () => { diff --git a/sdk/src/query/state-mutation.ts b/sdk/src/query/state-mutation.ts index 5af4aebe..b6929647 100644 --- a/sdk/src/query/state-mutation.ts +++ b/sdk/src/query/state-mutation.ts @@ -886,6 +886,146 @@ export const stateResolveBlocker: QueryHandler = async (args, projectDir, workst } }; }; +// ─── state.add-roadmap-evolution ───────────────────────────────────────── + +const VALID_ROADMAP_EVOLUTION_ACTIONS = new Set([ + 'inserted', 'removed', 'moved', 'edited', 'added', +]); + +/** + * Format a canonical Roadmap Evolution entry line. + * + * Shapes match existing workflow templates (`insert-phase.md`, `add-phase.md`): + * - inserted: `- Phase {phase} inserted after Phase {after}: {note} (URGENT)` + * - added: `- Phase {phase} added: {note}` + * - removed: `- Phase {phase} removed: {note}` + * - moved: `- Phase {phase} moved: {note}` + * - edited: `- Phase {phase} edited: {note}` + */ +function formatRoadmapEvolutionEntry(opts: { + phase: string; + action: string; + note?: string | null; + after?: string | null; + urgent?: boolean; +}): string { + const { phase, action, note, after, urgent } = opts; + const trimmedNote = note ? note.trim() : ''; + let line: string; + if (action === 'inserted') { + const afterClause = after ? ` after Phase ${after}` : ''; + line = `- Phase ${phase} inserted${afterClause}`; + if (trimmedNote) line += `: ${trimmedNote}`; + if (urgent) line += ' (URGENT)'; + } else { + // added | removed | moved | edited + line = `- Phase ${phase} ${action}`; + if (trimmedNote) line += `: ${trimmedNote}`; + } + return line; +} + +/** + * Query handler for `state.add-roadmap-evolution`. + * + * Appends a single entry to the `### Roadmap Evolution` subsection under + * `## Accumulated Context` in STATE.md. Creates the subsection if missing. + * Deduplicates on exact line match against existing entries. + * + * Canonical replacement for the raw `Edit`/`Write` instructions in + * `insert-phase.md` / `add-phase.md` step "update_project_state" so that + * projects with a `protect-files.sh` PreToolUse hook blocking direct + * STATE.md writes still update the Roadmap Evolution log. + * + * argv: `--phase`, `--action` (inserted|removed|moved|edited|added), + * `--note` (optional), `--after` (optional, for `inserted`), + * `--urgent` (boolean flag, appends "(URGENT)" when action=inserted). + * + * Returns `{ added: true, entry }` on success, or + * `{ added: false, reason: 'duplicate', entry }` when an identical line + * already exists. + * + * Throws `GSDError` with `ErrorClassification.Validation` when required + * inputs are missing or `--action` is not in the allowed set. + * + * Atomicity: goes through `readModifyWriteStateMd` which holds a lockfile + * across read -> transform -> write. Matches sibling mutation handlers. + */ +export const stateAddRoadmapEvolution: QueryHandler = async (args, projectDir, workstream) => { + const parsed = parseNamedArgs(args, ['phase', 'action', 'note', 'after'], ['urgent']); + const phase = (parsed.phase as string | null) ?? null; + const action = (parsed.action as string | null) ?? null; + const note = (parsed.note as string | null) ?? null; + const after = (parsed.after as string | null) ?? null; + const urgent = Boolean(parsed.urgent); + + if (!phase) { + throw new GSDError('phase required for state.add-roadmap-evolution', ErrorClassification.Validation); + } + if (!action) { + throw new GSDError('action required for state.add-roadmap-evolution', ErrorClassification.Validation); + } + if (!VALID_ROADMAP_EVOLUTION_ACTIONS.has(action)) { + throw new GSDError( + `invalid action "${action}" (expected one of: ${Array.from(VALID_ROADMAP_EVOLUTION_ACTIONS).join(', ')})`, + ErrorClassification.Validation, + ); + } + + const entry = formatRoadmapEvolutionEntry({ phase, action, note, after, urgent }); + + let added = false; + let duplicate = false; + + await readModifyWriteStateMd(projectDir, (content) => { + // Match `### Roadmap Evolution` subsection up to the next heading or EOF. + const subsectionPattern = /(###\s*Roadmap Evolution\s*\n)([\s\S]*?)(?=\n###?\s|\n##[^#]|$)/i; + const match = content.match(subsectionPattern); + + if (match) { + let sectionBody = match[2]; + // Dedupe: exact line match against any existing entry line. + const existingLines = sectionBody.split('\n').map(l => l.trim()); + if (existingLines.some(l => l === entry.trim())) { + duplicate = true; + return content; + } + // Strip placeholder "None" / "None yet." lines. + sectionBody = sectionBody.replace(/^None(?:\s+yet)?\.?\s*$/gim, ''); + sectionBody = sectionBody.trimEnd() + '\n' + entry + '\n'; + content = content.replace(subsectionPattern, (_m, header: string) => `${header}${sectionBody}`); + added = true; + return content; + } + + // Subsection missing — create it. + const accumulatedPattern = /(##\s*Accumulated Context\s*\n)/i; + const newSubsection = `\n### Roadmap Evolution\n\n${entry}\n`; + + if (accumulatedPattern.test(content)) { + // Insert immediately after the "## Accumulated Context" header. + content = content.replace(accumulatedPattern, (_m, header: string) => `${header}${newSubsection}`); + added = true; + return content; + } + + // No Accumulated Context section either — append both at EOF. + const suffix = `\n## Accumulated Context\n${newSubsection}`; + content = content.trimEnd() + suffix + '\n'; + added = true; + return content; + }, workstream); + + if (duplicate) { + return { data: { added: false, reason: 'duplicate', entry } }; + } + if (added) { + return { data: { added: true, entry } }; + } + // Unreachable given the logic above, but defensive. + return { data: { added: false, reason: 'unknown', entry } }; +}; + /** * Query handler for state.record-session command. * argv: `--stopped-at`, `--resume-file` (see `cmdStateRecordSession` in `state.cjs`).