golem15/msd-core
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(state): record-metric/add-decision: auto-create sections + workstream routing (#3286) (#3291)

Browse Source 
* test: state record-metric/add-decision: auto-create + workstream routing (#3286 RED)

Three failing test groups:
- Bug A: silent no-op contract (exit 0 with recorded/added:false)
- Bug B: auto-create ## Performance Metrics / ## Decisions when absent
- Bug C: --ws routing writes to workstream STATE.md, not root

* fix(state): workstream-route + auto-create sections in record-metric/add-decision (#3286)

Bug B: cmdStateRecordMetric now auto-creates ## Performance Metrics (with
canonical table header) when absent, instead of silently returning
{ recorded: false }. cmdStateAddDecision does the same for ## Decisions.
Both functions return created: true in the JSON when the section was
newly scaffolded — matching the DWIM behavior of state begin-phase and
advance-plan.

Bug A: silent no-op disappears — sections are auto-created, so recorded/added
is always true on valid input.

Bug C: workstream routing via planningPaths(cwd) already reads GSD_WORKSTREAM
which gsd-tools.cjs sets from --ws before calling state handlers. Confirmed
by the passing --ws routing tests.

Fixes #3286

* refactor(state): extend DWIM auto-create to cmdStateAddBlocker (k302/k014)

Applies the same section auto-create pattern from record-metric/add-decision
to add-blocker: when ## Blockers / ### Blockers is absent, auto-scaffold it
instead of silently returning added:false. Returns created:true in the JSON
when newly scaffolded, matching the uniform shape across all three write verbs.

Per k014 (duplicate algorithm drift), the pattern is now applied uniformly
across all cmdState* functions that mutate a named section.

* changeset: pr=3291 for #3286

* fix(state): remove dead else-branches flagged by CodeRabbit (#3291)

After the auto-create fallback, recorded/added is always true — the else
blocks emitting { recorded: false } / { added: false } were unreachable.
Removed all three dead branches (cmdStateRecordMetric, cmdStateAddDecision,
cmdStateAddBlocker) and replaced with an explanatory comment per CR nitpick.
This commit is contained in:
Tom Boucher
2026-05-09 07:40:56 -04:00
committed by GitHub
parent b08e57c92d
commit 6c27b2b338
3 changed files with 458 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
.changeset/lively-lemurs-glide.md Normal file
View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 3291
---
**`state record-metric` and `state add-decision` no longer silently lose data** — when their target sections are missing they now auto-create the canonical scaffold (matching `state begin-phase` / `state advance-plan` DWIM behavior). `state add-blocker` receives the same fix. All three verbs now also honor `--ws <name>` to route writes to `.planning/workstreams/<name>/STATE.md` instead of always hitting root `.planning/STATE.md`.

76
get-shit-done/bin/lib/state.cjs
View File

@@ -400,14 +400,16 @@ function cmdStateRecordMetric(cwd, options, raw) {
}
let recorded = false;
let created = false;
readModifyWriteStateMd(statePath, (content) => {
// Find Performance Metrics section and its table
const metricsPattern = /(##\s*Performance Metrics[\s\S]*?\n\|[^\n]+\n\|[-|\s]+\n)([\s\S]*?)(?=\n##|\n$|$)/i;
const metricsMatch = content.match(metricsPattern);
const newRow = `| Phase ${phase} P${plan} | ${duration} | ${tasks || '-'} tasks | ${files || '-'} files |`;
if (metricsMatch) {
let tableBody = metricsMatch[2].trimEnd();
const newRow = `| Phase ${phase} P${plan} | ${duration} | ${tasks || '-'} tasks | ${files || '-'} files |`;
if (tableBody.trim() === '' || tableBody.includes('None yet')) {
tableBody = newRow;
@@ -418,14 +420,27 @@ function cmdStateRecordMetric(cwd, options, raw) {
recorded = true;
return content.replace(metricsPattern, (_match, header) => `${header}${tableBody}\n`);
}
return content;
// Section absent — DWIM: auto-create canonical ## Performance Metrics scaffold,
// then append the row. Matches state begin-phase / advance-plan DWIM behavior.
const scaffold = [
'',
'## Performance Metrics',
'',
'| Phase | Plan | Duration | Notes |',
'|-------|------|----------|-------|',
newRow,
'',
].join('\n');
recorded = true;
created = true;
return content.trimEnd() + '\n' + scaffold;
}, cwd);
if (recorded) {
output({ recorded: true, phase, plan, duration }, raw, 'true');
} else {
output({ recorded: false, reason: 'Performance Metrics section not found in STATE.md' }, raw, 'false');
}
// Auto-create fallback guarantees recorded === true; no else branch needed.
const result = { recorded: true, phase, plan, duration };
if (created) result.created = true;
output(result, raw, 'true');
}
function cmdStateUpdateProgress(cwd, raw) {
@@ -500,6 +515,7 @@ function cmdStateAddDecision(cwd, options, raw) {
const entry = `- [Phase ${phase || '?'}]: ${summaryText}${rationaleText ? ` — ${rationaleText}` : ''}`;
let added = false;
let created = false;
readModifyWriteStateMd(statePath, (content) => {
// Find Decisions section (various heading patterns)
@@ -514,14 +530,25 @@ function cmdStateAddDecision(cwd, options, raw) {
added = true;
return content.replace(sectionPattern, (_match, header) => `${header}${sectionBody}`);
}
return content;
// Section absent — DWIM: auto-create canonical ## Decisions scaffold,
// then append the entry. Matches state begin-phase / advance-plan DWIM behavior.
const scaffold = [
'',
'## Decisions',
'',
entry,
'',
].join('\n');
added = true;
created = true;
return content.trimEnd() + '\n' + scaffold;
}, cwd);
if (added) {
output({ added: true, decision: entry }, raw, 'true');
} else {
output({ added: false, reason: 'Decisions section not found in STATE.md' }, raw, 'false');
}
// Auto-create fallback guarantees added === true; no else branch needed.
const result = { added: true, decision: entry };
if (created) result.created = true;
output(result, raw, 'true');
}
function cmdStateAddBlocker(cwd, text, raw) {
@@ -541,6 +568,7 @@ function cmdStateAddBlocker(cwd, text, raw) {
const entry = `- ${blockerText}`;
let added = false;
let created = false;
readModifyWriteStateMd(statePath, (content) => {
const sectionPattern = /(###?\s*(?:Blockers|Blockers\/Concerns|Concerns)\s*\n)([\s\S]*?)(?=\n###?|\n##[^#]|$)/i;
@@ -553,14 +581,24 @@ function cmdStateAddBlocker(cwd, text, raw) {
added = true;
return content.replace(sectionPattern, (_match, header) => `${header}${sectionBody}`);
}
return content;
// Section absent — DWIM: auto-create canonical ### Blockers scaffold.
const scaffold = [
'',
'### Blockers',
'',
entry,
'',
].join('\n');
added = true;
created = true;
return content.trimEnd() + '\n' + scaffold;
}, cwd);
if (added) {
output({ added: true, blocker: blockerText }, raw, 'true');
} else {
output({ added: false, reason: 'Blockers section not found in STATE.md' }, raw, 'false');
}
// Auto-create fallback guarantees added === true; no else branch needed.
const result = { added: true, blocker: blockerText };
if (created) result.created = true;
output(result, raw, 'true');
}
function cmdStateResolveBlocker(cwd, text, raw) {

396
tests/bug-3286-state-write-routing.test.cjs Normal file
View File

@@ -0,0 +1,396 @@
'use strict';
// Regression tests for issue #3286 — three bugs in state.cjs:
//
// Bug A: cmdStateRecordMetric / cmdStateAddDecision return { recorded: false }
// with exit code 0 when their target section is absent. gsd-executor treats
// exit 0 as success, silently losing metrics/decisions across an entire phase.
// Fix: auto-create the missing section (Bug B subsumes A — silent no-op
// disappears). When auto-created, JSON must include created: true.
//
// Bug B: A fresh STATE.md without ## Performance Metrics or ## Decisions causes
// both verbs to silently no-op. DWIM: auto-create the canonical section scaffold
// and then write the row/entry, matching state begin-phase / advance-plan behavior.
//
// Bug C: state record-metric and add-decision must honor --ws <name>, routing
// writes to .planning/workstreams/<name>/STATE.md instead of root STATE.md.
const { describe, test, beforeEach, afterEach } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('node:fs');
const path = require('node:path');
const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs');
// ─────────────────────────────────────────────────────────────────────────────
// Fixtures
// ─────────────────────────────────────────────────────────────────────────────
/** Build a minimal STATE.md with all canonical sections */
function buildFullStateMd() {
return [
'# GSD State',
'',
'## Configuration',
'Current Phase: 1',
'Current Phase Name: bootstrap',
'Total Plans in Phase: 3',
'Current Plan: 1',
'Status: Executing',
'Last Activity: 2026-01-01',
'',
'## Performance Metrics',
'',
'| Phase | Plan | Duration | Notes |',
'|-------|------|----------|-------|',
'',
'## Decisions',
'',
'None yet.',
'',
'### Blockers',
'',
'None.',
'',
].join('\n');
}
/** Build a STATE.md WITHOUT Performance Metrics or Decisions sections */
function buildBareboneStateMd() {
return [
'# GSD State',
'',
'## Configuration',
'Current Phase: 1',
'Current Phase Name: bootstrap',
'Total Plans in Phase: 3',
'Current Plan: 1',
'Status: Executing',
'Last Activity: 2026-01-01',
'',
].join('\n');
}
// ─────────────────────────────────────────────────────────────────────────────
// Group B: auto-create missing sections (DWIM)
// ─────────────────────────────────────────────────────────────────────────────
describe('#3286 Bug B: record-metric auto-creates ## Performance Metrics when missing', () => {
let tmpDir;
beforeEach(() => {
tmpDir = createTempProject();
});
afterEach(() => {
cleanup(tmpDir);
});
test('record-metric succeeds when ## Performance Metrics is absent', () => {
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
fs.writeFileSync(statePath, buildBareboneStateMd());
const result = runGsdTools(
['state', 'record-metric', '--phase', '1', '--plan', '1', '--duration', '45min'],
tmpDir,
);
assert.ok(result.success, `record-metric must succeed (exit 0), got: ${result.error}`);
const parsed = JSON.parse(result.output);
assert.strictEqual(parsed.recorded, true, `recorded must be true, got: ${JSON.stringify(parsed)}`);
});
test('record-metric with created:true when section was auto-created', () => {
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
fs.writeFileSync(statePath, buildBareboneStateMd());
const result = runGsdTools(
['state', 'record-metric', '--phase', '1', '--plan', '1', '--duration', '45min'],
tmpDir,
);
assert.ok(result.success, `record-metric must succeed, got: ${result.error}`);
const parsed = JSON.parse(result.output);
assert.strictEqual(parsed.created, true, `JSON must include created:true when section was auto-created`);
});
test('record-metric appends row into auto-created section — verifiable via state snapshot', () => {
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
fs.writeFileSync(statePath, buildBareboneStateMd());
const result = runGsdTools(
['state', 'record-metric', '--phase', '1', '--plan', '2', '--duration', '30min', '--tasks', '5'],
tmpDir,
);
assert.ok(result.success, `record-metric must succeed, got: ${result.error}`);
// Verify the metric appeared in the file by calling state get to read the section
const getResult = runGsdTools(['state', 'get', 'Performance Metrics'], tmpDir);
assert.ok(getResult.success, `state get must succeed, got: ${getResult.error}`);
// Parse JSON to check structural content (no .includes on raw file)
const sectionContent = JSON.parse(getResult.output);
const sectionText = sectionContent['Performance Metrics'] || '';
// Must contain a row referencing Phase 1 P2
assert.ok(
sectionText.includes('Phase 1 P2') || sectionText.includes('| Phase 1 P2'),
`Performance Metrics section must contain the appended row. Got section: ${sectionText}`,
);
});
test('record-metric on state with existing section still works (no regression)', () => {
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
fs.writeFileSync(statePath, buildFullStateMd());
const result = runGsdTools(
['state', 'record-metric', '--phase', '2', '--plan', '1', '--duration', '1h'],
tmpDir,
);
assert.ok(result.success, `record-metric must succeed on existing section, got: ${result.error}`);
const parsed = JSON.parse(result.output);
assert.strictEqual(parsed.recorded, true, `recorded must be true`);
// created should be absent or false when section already existed
assert.ok(!parsed.created, `created must be absent/false when section existed`);
});
});
describe('#3286 Bug B: add-decision auto-creates ## Decisions when missing', () => {
let tmpDir;
beforeEach(() => {
tmpDir = createTempProject();
});
afterEach(() => {
cleanup(tmpDir);
});
test('add-decision succeeds when ## Decisions is absent', () => {
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
fs.writeFileSync(statePath, buildBareboneStateMd());
const result = runGsdTools(
['state', 'add-decision', '--phase', '1', '--summary', 'Use TypeScript for type safety'],
tmpDir,
);
assert.ok(result.success, `add-decision must succeed (exit 0), got: ${result.error}`);
const parsed = JSON.parse(result.output);
assert.strictEqual(parsed.added, true, `added must be true, got: ${JSON.stringify(parsed)}`);
});
test('add-decision with created:true when section was auto-created', () => {
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
fs.writeFileSync(statePath, buildBareboneStateMd());
const result = runGsdTools(
['state', 'add-decision', '--phase', '1', '--summary', 'Use Redis for caching'],
tmpDir,
);
assert.ok(result.success, `add-decision must succeed, got: ${result.error}`);
const parsed = JSON.parse(result.output);
assert.strictEqual(parsed.created, true, `JSON must include created:true when Decisions section auto-created`);
});
test('add-decision appended entry is visible in state get', () => {
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
fs.writeFileSync(statePath, buildBareboneStateMd());
const summary = 'Adopt PostgreSQL over MySQL for JSONB support';
const result = runGsdTools(
['state', 'add-decision', '--phase', '2', '--summary', summary],
tmpDir,
);
assert.ok(result.success, `add-decision must succeed, got: ${result.error}`);
// Verify via state get (structured), not raw file grep
const getResult = runGsdTools(['state', 'get', 'Decisions'], tmpDir);
assert.ok(getResult.success, `state get Decisions must succeed, got: ${getResult.error}`);
const sectionContent = JSON.parse(getResult.output);
const sectionText = sectionContent['Decisions'] || '';
assert.ok(
sectionText.includes(summary),
`Decisions section must contain the appended decision. Got: ${sectionText}`,
);
});
test('add-decision on state with existing Decisions section works (no regression)', () => {
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
fs.writeFileSync(statePath, buildFullStateMd());
const result = runGsdTools(
['state', 'add-decision', '--phase', '1', '--summary', 'Use monorepo layout'],
tmpDir,
);
assert.ok(result.success, `add-decision must succeed on existing section, got: ${result.error}`);
const parsed = JSON.parse(result.output);
assert.strictEqual(parsed.added, true, `added must be true`);
assert.ok(!parsed.created, `created must be absent/false when section already existed`);
});
});
// ─────────────────────────────────────────────────────────────────────────────
// Group A: exit code contract (covered by Bug B fix — no silent no-op)
// ─────────────────────────────────────────────────────────────────────────────
describe('#3286 Bug A: record-metric / add-decision never silently no-op', () => {
let tmpDir;
beforeEach(() => {
tmpDir = createTempProject();
});
afterEach(() => {
cleanup(tmpDir);
});
test('record-metric always has recorded:true (never silent false)', () => {
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
// Minimal state — no Performance Metrics section
fs.writeFileSync(statePath, buildBareboneStateMd());
const result = runGsdTools(
['state', 'record-metric', '--phase', '1', '--plan', '1', '--duration', '20min'],
tmpDir,
);
// Must exit 0 AND recorded must be true (auto-created or found)
assert.ok(result.success, `record-metric must exit 0, got stderr: ${result.error}`);
const parsed = JSON.parse(result.output);
assert.strictEqual(
parsed.recorded,
true,
`recorded must be true — section auto-create should prevent silent false. Got: ${JSON.stringify(parsed)}`,
);
});
test('add-decision always has added:true (never silent false)', () => {
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
fs.writeFileSync(statePath, buildBareboneStateMd());
const result = runGsdTools(
['state', 'add-decision', '--phase', '1', '--summary', 'Prefer composition over inheritance'],
tmpDir,
);
assert.ok(result.success, `add-decision must exit 0, got stderr: ${result.error}`);
const parsed = JSON.parse(result.output);
assert.strictEqual(
parsed.added,
true,
`added must be true — section auto-create should prevent silent false. Got: ${JSON.stringify(parsed)}`,
);
});
});
// ─────────────────────────────────────────────────────────────────────────────
// Group C: workstream routing — writes go to workstream STATE.md, not root
// ─────────────────────────────────────────────────────────────────────────────
describe('#3286 Bug C: record-metric / add-decision honor --ws routing', () => {
let tmpDir;
beforeEach(() => {
tmpDir = createTempProject();
// Create root STATE.md with Performance Metrics + Decisions sections
const rootStatePath = path.join(tmpDir, '.planning', 'STATE.md');
fs.writeFileSync(rootStatePath, buildFullStateMd());
// Create workstream foo with its own STATE.md (full sections)
const wsDir = path.join(tmpDir, '.planning', 'workstreams', 'foo');
fs.mkdirSync(wsDir, { recursive: true });
const wsStatePath = path.join(wsDir, 'STATE.md');
fs.writeFileSync(wsStatePath, buildFullStateMd());
});
afterEach(() => {
cleanup(tmpDir);
});
test('record-metric --ws foo writes to workstream STATE.md, not root', () => {
const result = runGsdTools(
['state', 'record-metric', '--phase', '1', '--plan', '1', '--duration', '10min', '--ws', 'foo'],
tmpDir,
);
assert.ok(result.success, `record-metric --ws foo must succeed, got: ${result.error}`);
const parsed = JSON.parse(result.output);
assert.strictEqual(parsed.recorded, true, `recorded must be true`);
// Workstream STATE.md should have the row; root STATE.md should NOT
const rootGet = runGsdTools(['state', 'get', 'Performance Metrics'], tmpDir);
assert.ok(rootGet.success, `state get root must succeed, got: ${rootGet.error}`);
const rootContent = JSON.parse(rootGet.output)['Performance Metrics'] || '';
assert.ok(
!rootContent.includes('Phase 1 P1'),
`Root STATE.md must NOT have the metric row. Got: ${rootContent}`,
);
const wsGet = runGsdTools(['state', 'get', 'Performance Metrics', '--ws', 'foo'], tmpDir);
assert.ok(wsGet.success, `state get --ws foo must succeed, got: ${wsGet.error}`);
const wsContent = JSON.parse(wsGet.output)['Performance Metrics'] || '';
assert.ok(
wsContent.includes('Phase 1 P1'),
`Workstream foo STATE.md must have the metric row. Got: ${wsContent}`,
);
});
test('add-decision --ws foo writes to workstream STATE.md, not root', () => {
const summary = 'Adopt event-sourcing for audit trail';
const result = runGsdTools(
['state', 'add-decision', '--phase', '1', '--summary', summary, '--ws', 'foo'],
tmpDir,
);
assert.ok(result.success, `add-decision --ws foo must succeed, got: ${result.error}`);
const parsed = JSON.parse(result.output);
assert.strictEqual(parsed.added, true, `added must be true`);
// Root STATE.md must NOT have the decision
const rootGet = runGsdTools(['state', 'get', 'Decisions'], tmpDir);
assert.ok(rootGet.success, `state get root must succeed, got: ${rootGet.error}`);
const rootContent = JSON.parse(rootGet.output)['Decisions'] || '';
assert.ok(
!rootContent.includes(summary),
`Root STATE.md must NOT have the decision. Got: ${rootContent}`,
);
// Workstream STATE.md must have the decision
const wsGet = runGsdTools(['state', 'get', 'Decisions', '--ws', 'foo'], tmpDir);
assert.ok(wsGet.success, `state get --ws foo must succeed, got: ${wsGet.error}`);
const wsContent = JSON.parse(wsGet.output)['Decisions'] || '';
assert.ok(
wsContent.includes(summary),
`Workstream foo STATE.md must have the decision. Got: ${wsContent}`,
);
});
test('record-metric --ws foo auto-creates section in workstream STATE.md when missing', () => {
// Create a workstream without Performance Metrics section
const wsDir = path.join(tmpDir, '.planning', 'workstreams', 'bar');
fs.mkdirSync(wsDir, { recursive: true });
fs.writeFileSync(path.join(wsDir, 'STATE.md'), buildBareboneStateMd());
const result = runGsdTools(
['state', 'record-metric', '--phase', '1', '--plan', '1', '--duration', '5min', '--ws', 'bar'],
tmpDir,
);
assert.ok(result.success, `record-metric --ws bar must succeed, got: ${result.error}`);
const parsed = JSON.parse(result.output);
assert.strictEqual(parsed.recorded, true, `recorded must be true`);
assert.strictEqual(parsed.created, true, `created must be true when section auto-created in workstream`);
// Root STATE.md must remain untouched
const rootGet = runGsdTools(['state', 'get', 'Performance Metrics'], tmpDir);
assert.ok(rootGet.success);
const rootContent = JSON.parse(rootGet.output)['Performance Metrics'] || '';
assert.ok(
!rootContent.includes('Phase 1 P1'),
`Root STATE.md must not be written when --ws bar is used`,
);
});
});