mirror of
https://github.com/glittercowboy/get-shit-done
synced 2026-04-25 17:25:23 +02:00
/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
This commit is contained in:
Tom Boucher
@@ -4,7 +4,6 @@ description: Insert urgent work as decimal phase (e.g., 72.1) between existing p
|
||||
argument-hint: <after> <description>
|
||||
allowed-tools:
|
||||
- Read
|
||||
- Write
|
||||
- Bash
|
||||
---
|
||||
|
||||
|
||||
@@ -63,19 +63,35 @@ Extract from result: `phase_number`, `after_phase`, `name`, `slug`, `directory`.
|
||||
</step>
|
||||
|
||||
<step name="update_project_state">
|
||||
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).
|
||||
</step>
|
||||
|
||||
<step name="completion">
|
||||
@@ -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
|
||||
</success_criteria>
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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<string>([
|
||||
'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);
|
||||
|
||||
@@ -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<string, unknown>;
|
||||
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<string, unknown>).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<string, unknown>).added).toBe(true);
|
||||
|
||||
const second = await stateAddRoadmapEvolution(argv, tmpDir);
|
||||
const data = second.data as Record<string, unknown>;
|
||||
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', () => {
|
||||
|
||||
@@ -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`).
|
||||
|
||||
Reference in New Issue
Block a user