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

refactor(#1791): ADR-1769 Phase 6 — patch migration + curated current_phase_name preserve (#1695) (#1792)

Browse Source 
Migrate cmdStatePatch (state.cts) onto the STATE.md Transition Module substrate
and close the curated-field clobber bug class #1743/#1695 (ADR-1769, epic #1769).

- Add {kind: 'patch'} to StateTransitionIntent, with patchCore in
  src/state-transition.cts. Applies each caller-supplied {field:value} pair via
  stateReplaceField over the full content (body + frontmatter), tracking
  updated vs. failed. data.updated/data.failed mirror the CLI output shape.
- Collapse cmdStatePatch to a transitionCore dispatch. Field-name validation
  (security) and the resync-progress decision stay in the adapter.
- #1695/#1743 fix: extend the #1230 delta heuristic in readModifyWriteStateMd to
  the curated current_phase_name, table-driven via
  getFieldClassification('current_phase_name').preservation === 'preserve-always'.
  When a write does NOT change the body Phase: source line, the curated
  frontmatter value wins over syncStateFrontmatter's body re-derivation (which
  harvests a wrong parenthetical aside — #1695). begin/planned/complete-phase
  rewrite their body Phase line, so the delta does not fire for them and
  current_phase_name still advances.
- Regression: bug-1695-state-patch-clobbers-phase-name.test.cjs — unrelated
  patch preserves curated current_phase_name; patching the Phase source advances.

All 70 transition + 493 regression tests pass (state/phase/milestone + #905/#397/#3242 lineages).

Closes #1791
This commit is contained in:
Tom Boucher
2026-06-27 16:21:28 -04:00
committed by GitHub
parent 3ceb83329d
commit 064f63b299
8 changed files with 301 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
docs/adr/1769-state-md-transition-module.md
View File

@@ -213,5 +213,5 @@ alongside, leave callbacks — parallel worlds don't converge (ADR-857's failure
| 3 | `completePhase` + `phase.cts:1770` | #1784 | — |
| 4 | `plannedPhase` + `milestoneSwitch` | #1786 | — |
| 5 | `milestoneComplete` + `milestone.cts:352` | #1789 | — |
| 6 | `patch` | TBD | #1743, #1695 |
| 6 | `patch` | #1791 | #1743, #1695 |
| 7 | `sync`, `prune`, `update` | TBD | #1760, #1761 |

47
gsd-core/bin/lib/state-transition.cjs
View File

@@ -125,6 +125,8 @@ function transitionCore(content, intent, deps) {
return milestoneSwitchCore(content, intent, deps);
case 'milestoneComplete':
return milestoneCompleteCore(content, intent, deps);
case 'patch':
return patchCore(content, intent);
}
}
// ----------------------------------------------------------------------------
@@ -878,9 +880,8 @@ function milestoneCompleteCore(content, intent, deps) {
updated.push('Last Activity Description');
}
// ## Current Position reset — stop resume/progress flows pointing at closed
// execution instructions. allow-adhoc-markdown: pre-seam section write-modify;
// pending collectSection migration #1372.
const positionPattern = /(##\s*Current Position\s*\n)([\s\S]*?)(?=\n##|$)/i;
// execution instructions.
const positionPattern = /(##\s*Current Position\s*\n)([\s\S]*?)(?=\n##|$)/i; // allow-adhoc-markdown: pre-seam section write-modify carried from milestone.cts; pending collectSection migration #1372
const closedPositionBody = `\nPhase: Milestone ${version} complete\n` +
`Plan: —\n` +
`Status: Awaiting next milestone\n` +
@@ -893,8 +894,7 @@ function milestoneCompleteCore(content, intent, deps) {
}
updated.push('Current Position');
// ## Operator Next Steps — normalize stale tails that can persist after close.
// allow-adhoc-markdown: pre-seam section write-modify; pending collectSection migration #1372.
const operatorPattern = /(##\s*Operator Next Steps\s*\n)([\s\S]*?)(?=\n##|$)/i;
const operatorPattern = /(##\s*Operator Next Steps\s*\n)([\s\S]*?)(?=\n##|$)/i; // allow-adhoc-markdown: pre-seam section write-modify carried from milestone.cts; pending collectSection migration #1372
if (operatorPattern.test(body)) {
body = body.replace(operatorPattern, `$1\n- Start the next milestone with ${intent.nextMilestoneCommand}\n\n`);
}
@@ -904,3 +904,40 @@ function milestoneCompleteCore(content, intent, deps) {
updated.push('Operator Next Steps');
return { content: reassemble(body), updated };
}
// ----------------------------------------------------------------------------
// patch — intent implementation (Phase 6)
// ----------------------------------------------------------------------------
/**
* Apply a `patch` transition to STATE.md content.
*
* Migrates `cmdStatePatch` (state.cts) onto the substrate. Applies each
* caller-supplied `{field: value}` pair via `stateReplaceField` over the full
* content (body + frontmatter — patch can target either), tracking which fields
* were updated vs. not found.
*
* The curated-field preservation that fixes #1743/#1695 is NOT in this core —
* it lives in `readModifyWriteStateMd`'s post-sync delta (table-driven via
* `getFieldClassification('current_phase_name').preservation === 'preserve-always'`).
* `patch` consulting the table "refuses to overwrite" curated fields implicitly:
* when the patch does not change a curated field's body source line, the
* existing frontmatter value wins over the sync re-derivation. The adapter
* still owns field-name validation (security) and the resync-progress decision.
*
* `data.updated` / `data.failed` mirror the pre-migration CLI output shape.
*/
function patchCore(content, intent) {
const updated = [];
const failed = [];
let result = content;
for (const [field, value] of Object.entries(intent.patches)) {
const replaced = (0, state_document_cjs_1.stateReplaceField)(result, field, value);
if (replaced !== null) {
result = replaced;
updated.push(field);
}
else {
failed.push(field);
}
}
return { content: result, updated, data: { updated, failed } };
}

1
scripts/lint-regression-test-names.allowlist.json
View File

@@ -5,6 +5,7 @@
"bug-1367-claude-local-flat-command-layout.test.cjs",
"bug-14-progress-auto-flag-dropped.test.cjs",
"bug-167-query-meta-command.test.cjs",
"bug-1695-state-patch-clobbers-phase-name.test.cjs",
"bug-17-askuserquestion-option-cap.test.cjs",
"bug-170-workflow-fallback-install-hint.test.cjs",
"bug-1736-local-install-commands.test.cjs",

1
scripts/lint-test-file-count.allowlist.json
View File

@@ -99,6 +99,7 @@
},
"state": {
"files": [
"bug-1695-state-patch-clobbers-phase-name.test.cjs",
"bug-21-state-md-template-frontmatter.test.cjs",
"bug-2630-state-frontmatter-milestone-switch.test.cjs",
"bug-3127-state-begin-phase-idempotent.test.cjs",

50
src/state-transition.cts
View File

@@ -179,8 +179,9 @@ export type StateTransitionIntent =
version: string;
/** Resolved runtime slash command for the Operator Next Steps hint (e.g. '/gsd:new-milestone'). */
nextMilestoneCommand: string;
};
// Phases 6–7 add the remaining intent kinds to this discriminated union.
}
| { kind: 'patch'; patches: Record<string, string> };
// Phase 7 adds the remaining intent kinds to this discriminated union.
export type StateTransitionResult = {
content: string;
@@ -221,6 +222,8 @@ export function transitionCore(
return milestoneSwitchCore(content, intent, deps);
case 'milestoneComplete':
return milestoneCompleteCore(content, intent, deps);
case 'patch':
return patchCore(content, intent);
}
}
@@ -1128,3 +1131,46 @@ function milestoneCompleteCore(
return { content: reassemble(body), updated };
}
// ----------------------------------------------------------------------------
// patch — intent implementation (Phase 6)
// ----------------------------------------------------------------------------
/**
* Apply a `patch` transition to STATE.md content.
*
* Migrates `cmdStatePatch` (state.cts) onto the substrate. Applies each
* caller-supplied `{field: value}` pair via `stateReplaceField` over the full
* content (body + frontmatter — patch can target either), tracking which fields
* were updated vs. not found.
*
* The curated-field preservation that fixes #1743/#1695 is NOT in this core —
* it lives in `readModifyWriteStateMd`'s post-sync delta (table-driven via
* `getFieldClassification('current_phase_name').preservation === 'preserve-always'`).
* `patch` consulting the table "refuses to overwrite" curated fields implicitly:
* when the patch does not change a curated field's body source line, the
* existing frontmatter value wins over the sync re-derivation. The adapter
* still owns field-name validation (security) and the resync-progress decision.
*
* `data.updated` / `data.failed` mirror the pre-migration CLI output shape.
*/
function patchCore(
content: string,
intent: { kind: 'patch'; patches: Record<string, string> },
): StateTransitionResult {
const updated: string[] = [];
const failed: string[] = [];
let result = content;
for (const [field, value] of Object.entries(intent.patches)) {
const replaced = stateReplaceField(result, field, value);
if (replaced !== null) {
result = replaced;
updated.push(field);
} else {
failed.push(field);
}
}
return { content: result, updated, data: { updated, failed } };
}

55
src/state.cts
View File

@@ -32,7 +32,7 @@ const { extractFrontmatter, reconstructFrontmatter } = frontmatter;
import scanPhasePlans = require('./plan-scan.cjs');
// eslint-disable-next-line @typescript-eslint/no-require-imports
import stateTransitionMod = require('./state-transition.cjs');
const { transitionCore } = stateTransitionMod;
const { transitionCore, getFieldClassification } = stateTransitionMod;
type StateTransitionIntent = stateTransitionMod.StateTransitionIntent;
type StateTransitionDeps = stateTransitionMod.StateTransitionDeps;
import {
@@ -397,21 +397,19 @@ function cmdStatePatch(cwd: string, patches: Record<string, string>, raw: boolea
const statePath = planningPaths(cwd).state;
try {
const results: { updated: string[]; failed: string[] } = { updated: [], failed: [] };
const shouldResync = shouldResyncStateProgress(Object.keys(patches));
// Use atomic read-modify-write to prevent lost updates from concurrent agents
// ADR-1769 Phase 6: dispatches to the STATE.md Transition Module. The
// per-patch stateReplaceField loop is the pure `patchCore` in
// src/state-transition.cts. readModifyWriteStateMd still owns the lock, the
// #1230/#1264 post-sync preservation, AND the #1695 curated-current_phase_name
// delta (table-driven) that this phase adds. Field-name validation (security)
// and the resync-progress decision stay in this adapter.
let results: { updated: string[]; failed: string[] } = { updated: [], failed: [] };
readModifyWriteStateMd(statePath, (content) => {
for (const [field, value] of Object.entries(patches)) {
const result = stateReplaceField(content, field, value);
if (result) {
content = result;
results.updated.push(field);
} else {
results.failed.push(field);
}
}
return content;
const result = transitionCore(content, { kind: 'patch', patches }, { clock: realClock, progressProvider: () => null });
results = (result.data as { updated: string[]; failed: string[] }) ?? results;
return result.content;
}, cwd, { resync: shouldResync });
output(results, raw, results.updated.length > 0 ? 'true' : 'false');
@@ -1892,6 +1890,14 @@ function readModifyWriteStateMd(statePath: string, transformFn: (content: string
const preSessionScope = preSessionMatch ? preSessionMatch[1] : preBody;
const preBodyStoppedAt = stateExtractField(preSessionScope, 'Stopped At') || stateExtractField(preSessionScope, 'Stopped at');
// ADR-1769 Phase 6 / #1743 / #1695: snapshot the body source for the curated
// current_phase_name (the `Phase:` line parseProsePhaseField harvests). When
// this write does NOT change that line, the curated frontmatter value must
// win over syncStateFrontmatter's body re-derivation (which can harvest a
// wrong parenthetical aside — #1695). Gated by the field-classification
// table's preserve-always row so the rule lives in one place.
const preBodyPhaseSource = stateExtractField(preBody, 'Phase');
const modified = transformFn(content);
// Bug #948: no-op guard — if the transform produced no change, do NOT write
@@ -1922,6 +1928,9 @@ function readModifyWriteStateMd(statePath: string, transformFn: (content: string
const postSessionMatch = matchSessionSection(postBody);
const postSessionScope = postSessionMatch ? postSessionMatch[1] : postBody;
const postBodyStoppedAt = stateExtractField(postSessionScope, 'Stopped At') || stateExtractField(postSessionScope, 'Stopped at');
// ADR-1769 Phase 6 / #1695: post-transform body Phase source for the
// current_phase_name delta comparison.
const postBodyPhaseSource = stateExtractField(postBody, 'Phase');
let mutated = false;
const postFm = extractFrontmatter(synced) as Record<string, unknown>;
@@ -1963,6 +1972,26 @@ function readModifyWriteStateMd(statePath: string, transformFn: (content: string
mutated = true;
}
// ADR-1769 Phase 6 / #1743 / #1695: same delta heuristic for the curated
// current_phase_name. Gated by the field-classification table (preserve-always).
// When this write did NOT change the body `Phase:` source line, the curated
// frontmatter current_phase_name wins over syncStateFrontmatter's body
// re-derivation (parseProsePhaseField can harvest a wrong parenthetical
// aside — #1695). begin/planned/complete-phase rewrite their body Phase line,
// so the delta does not fire for them and current_phase_name still advances.
const phaseNameCls = getFieldClassification('current_phase_name');
if (
phaseNameCls !== null &&
phaseNameCls.preservation === 'preserve-always' &&
postBodyPhaseSource === preBodyPhaseSource &&
typeof preFmSnapshot['current_phase_name'] === 'string' &&
preFmSnapshot['current_phase_name'].length > 0 &&
postFm['current_phase_name'] !== preFmSnapshot['current_phase_name']
) {
postFm['current_phase_name'] = preFmSnapshot['current_phase_name'];
mutated = true;
}
if (mutated) {
const yamlStr = reconstructFrontmatter(postFm as unknown as Frontmatter);
const body = stripFrontmatter(synced);

113
tests/bug-1695-state-patch-clobbers-phase-name.test.cjs Normal file
View File

@@ -0,0 +1,113 @@
'use strict';
// Regression test for issue #1695 — `state patch` of an unrelated field clobbers
// the curated `current_phase_name` frontmatter scalar.
//
// Root cause: readModifyWriteStateMd({resync:false}) still runs syncStateFrontmatter,
// which re-derives EVERY body-derived scalar from body prose. The #1264 restore
// covers `progress` only and #1230 covers `status`/`stopped_at`; `current_phase_name`
// was left exposed, and parseProsePhaseField's paren-over-dash preference made the
// re-derived value wrong (harvesting a parenthetical aside as the phase name).
//
// ADR-1769 Phase 6 fix: extend the #1230 delta heuristic to current_phase_name
// (gated by the field-classification table's preserve-always row). When the
// transform did NOT change the body Current Phase / Phase source line, the curated
// frontmatter value wins.
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');
const { extractFrontmatter } = require('../gsd-core/bin/lib/frontmatter.cjs');
function buildStateWithCuratedPhaseName({ phaseName = 'Native Global Hotkey', aside = 'next; Phase 15 landed, UAT deferred' } = {}) {
return [
'---',
'gsd_state_version: 1.0',
'milestone: v1.0',
'milestone_name: Test',
'current_phase: "16"',
`current_phase_name: "${phaseName}"`,
'status: executing',
'progress:',
' total_phases: 20',
' completed_phases: 15',
' total_plans: 40',
' completed_plans: 30',
' percent: 75',
'---',
'',
'# GSD State',
'',
'## Configuration',
'Current Phase: 16',
'Total Plans in Phase: 4',
'Current Plan: 2',
'Status: Executing Phase 16',
'Last Activity: 2026-06-20',
'',
'## Current Position',
'',
`Phase: 16 — ${phaseName} (${aside})`,
'Plan: 2 of 4',
'Status: Executing Phase 16',
'Last activity: 2026-06-20 — mid-flight',
'',
].join('\n');
}
function readFm(statePath) {
return extractFrontmatter(fs.readFileSync(statePath, 'utf-8'));
}
describe('#1695: state patch of an unrelated field preserves curated current_phase_name', () => {
let tmpDir;
beforeEach(() => { tmpDir = createTempProject(); });
afterEach(() => { cleanup(tmpDir); });
test('patching Status does NOT clobber the curated current_phase_name', () => {
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
fs.writeFileSync(statePath, buildStateWithCuratedPhaseName());
const result = runGsdTools(['query', 'state.patch', JSON.stringify({ Status: 'Paused for review' })], tmpDir);
assert.ok(result.success, `state patch failed: ${result.error}`);
const fm = readFm(statePath);
assert.strictEqual(
fm.current_phase_name,
'Native Global Hotkey',
`current_phase_name must be preserved on an unrelated patch; got ${JSON.stringify(fm.current_phase_name)} (the paren-over-dash re-derivation clobbered it — #1695)`,
);
});
test('patching Current Plan does NOT clobber the curated current_phase_name', () => {
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
fs.writeFileSync(statePath, buildStateWithCuratedPhaseName());
const result = runGsdTools(['query', 'state.patch', JSON.stringify({ 'Current Plan': '3' })], tmpDir);
assert.ok(result.success, `state patch failed: ${result.error}`);
const fm = readFm(statePath);
assert.strictEqual(fm.current_phase_name, 'Native Global Hotkey');
});
test('explicitly patching the body Phase name-source line still advances (delta does not over-pin)', () => {
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
fs.writeFileSync(statePath, buildStateWithCuratedPhaseName());
// Patching the body 'Phase' field (the parseProsePhaseField source for
// current_phase_name) changes the source line, so the #1230 delta must NOT
// fire — syncStateFrontmatter re-derives current_phase_name from the new line.
// (Acceptance criterion from #1743: the guard must not pin a scalar whose body
// source genuinely changed.)
const result = runGsdTools(['query', 'state.patch', JSON.stringify({ Phase: '17 — Brand New Phase Name' })], tmpDir);
assert.ok(result.success, `state patch failed: ${result.error}`);
const fm = readFm(statePath);
// current_phase_name should be re-derived from the new body 'Phase' line
// (not pinned to the old curated value).
assert.notStrictEqual(fm.current_phase_name, 'Native Global Hotkey',
`current_phase_name must advance when the body Phase source changed; got ${JSON.stringify(fm.current_phase_name)}`);
});
});

53
tests/state-transition.test.cjs
View File

@@ -1032,3 +1032,56 @@ describe('ADR-1769 Phase 5: milestoneComplete transition — closure write', ()
assert.ok(/^milestone: v1\.0/m.test(result.content), 'frontmatter milestone preserved');
});
});
// ADR-1769 Phase 6: patch
describe('ADR-1769 Phase 6: patch transition — field updates', () => {
const deps = { clock: fixedClock, progressProvider: noProgress };
test('applies each patched field and reports the updated set', () => {
const input = [
'# Project State',
'',
'**Status:** Planning',
'**Current Plan:** 2',
'**Total Plans in Phase:** 5',
'',
].join('\n');
const result = transitionCore(
input,
{ kind: 'patch', patches: { Status: 'Paused', 'Current Plan': '3' } },
deps,
);
assert.strictEqual(stateExtractField(result.content, 'Status'), 'Paused');
assert.strictEqual(stateExtractField(result.content, 'Current Plan'), '3');
assert.deepStrictEqual(result.data && result.data.updated, ['Status', 'Current Plan']);
});
test('reports failed fields (no matching field in content)', () => {
const input = '# Project State\n\n**Status:** Planning\n';
const result = transitionCore(
input,
{ kind: 'patch', patches: { Status: 'Paused', Nonexistent: 'x' } },
deps,
);
assert.deepStrictEqual(result.data && result.data.updated, ['Status']);
assert.deepStrictEqual(result.data && result.data.failed, ['Nonexistent']);
});
test('leaves content unchanged when no patch matches (no-op)', () => {
const input = '# Project State\n\n**Status:** Planning\n';
const result = transitionCore(input, { kind: 'patch', patches: { Nonexistent: 'x' } }, deps);
assert.strictEqual(result.content, input);
assert.deepStrictEqual(result.data && result.data.updated, []);
assert.deepStrictEqual(result.data && result.data.failed, ['Nonexistent']);
});
test('patching a frontmatter YAML key directly updates the YAML line', () => {
// patch operates on the full content (body + frontmatter), so a lowercase
// frontmatter key like `stopped_at` is matched and replaced.
const input = ['---', 'status: executing', 'stopped_at: 2026-01-01', '---', '', '# State', ''].join('\n');
const result = transitionCore(input, { kind: 'patch', patches: { stopped_at: '2026-06-27' } }, deps);
assert.ok(/^stopped_at: 2026-06-27$/m.test(result.content), 'YAML stopped_at must be patched');
assert.deepStrictEqual(result.data && result.data.updated, ['stopped_at']);
});
});