fix(#4243): anchor stateReplaceField bold form; pin frontmatter round-trip (#4453)

* test(#4243): failing-first regressions for bold-field anchoring and frontmatter round-trip

* fix(#4243): anchor stateReplaceField bold form to line start

The bold branch of stateReplaceField carried no ^ and no /m flag, so a bold
label quoted mid-sentence inside prose — the issue's **Status:** inside an
Accumulated Context bullet — captured the rewrite and destroyed the rest of
its line, silently, whenever a whole-body caller fed the function every
section (beginPhaseCore's tryField, advancePlanCore's Status/Current Plan
writes). The plain branch was always line-anchored; only the bold branch
lagged.

Anchored to ^([ \t]*\*\*Field:\*\*[ \t]*) with /im, reusing #4010's
same-line confinement idiom for the leading class (deliberately not the
issue's suggested ^\s* — it can consume the newlines before the label into
the match) and #4186's recognition-by-anchoring discipline. Frontmatter
half of the issue (unknown-key drops, invented milestone defaults) is
already fixed on next by #2202/#3216/#4129; pinned here with the issue's
requested regression fixtures.

* test(#4243): pin survival contract, not derived percent, in frontmatter rows

Bench RED run caught two assertion defects in the pin rows: the unknown
progress subkey re-parses as a quoted scalar ('77' vs 77), and percent is a
declared derived subkey - omitted under the #3573 no-roadmap withhold,
recomputed when measured (#4129) - so pinning its value over-pins derived
semantics. The rows now pin what the issue demands: unknown/custom keys
survive, stored counters are kept under the withhold, milestone identity is
never reset to invented defaults.

* chore(#4243): changeset for the anchored bold-field fix

* chore(#4243): backfill PR number in changeset
This commit is contained in:
Tom Boucher
2026-09-07 00:03:15 -04:00
committed by GitHub
parent 8c8eda46b0
commit 33e393ba4c
4 changed files with 451 additions and 1 deletions

View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 4453
---
**`state begin-phase` no longer rewrites prose that merely quotes a bold field label** — a `**Status:**` (or any served field label) quoted mid-sentence inside prose captured the field rewrite and silently destroyed the rest of its line; the bold form is now anchored to line start, so only the real field updates. Frontmatter round-trip through begin-phase (custom keys, progress subkeys, milestone identity without a ROADMAP) is pinned with regression tests. (#4243)

View File

@@ -553,7 +553,22 @@ export function stateReplaceField(content: string, fieldName: string, newValue:
// `(.*)` captured the following line and the rebuild discarded it — the #4010
// data-loss. ADR-3180 §7.7 makes stateExtractField the same-line-confined owner;
// this aligns the writer to it.
const boldPattern = new RegExp(`(\\*\\*${escaped}:\\*\\*[ \\t]*)(.*)`, 'i');
//
// #4243: the bold form is also ANCHORED to line start, with same-line leading
// whitespace only. The pre-fix pattern carried no `^` and no `m` flag, so a
// bold label quoted MID-SENTENCE inside prose — an Accumulated Context bullet
// mentioning `**Status:**` — captured the rewrite and destroyed the rest of
// its line, silently, whenever a whole-body caller fed this function every
// section (beginPhaseCore's tryField, advancePlanCore's Status/Current Plan
// writes). The plain branch below was always line-anchored; only the bold
// branch lagged. Anchoring reuses #4010's same-line confinement idiom (the
// leading class is `[ \t]*`, deliberately NOT the `\s*` the issue suggested —
// `^\s*\*\*` can consume the newlines before the label into the match and
// drop them on rebuild) and #4186's recognition-by-anchoring discipline: a
// write target must BE the whole declared line shape, never a substring
// guess inside prose. `$` is explicit-and-inert (`.` never crosses line
// terminators) and documents that the match ends at end-of-line.
const boldPattern = new RegExp(`^([ \\t]*\\*\\*${escaped}:\\*\\*[ \\t]*)(.*)$`, 'im');
if (boldPattern.test(content)) {
return content.replace(boldPattern, (_match, prefix: string) => joinFieldReplacement(prefix, newValue));
}

View File

@@ -221,6 +221,187 @@ describe('stateReplaceField — empty field preserves the following line (#4010)
});
});
// #4243: the bold branch of stateReplaceField was UNANCHORED
// (`(\*\*Field:\*\*[ \t]*)(.*)` with no ^ and no /m), so a bold label quoted
// MID-SENTENCE inside prose — the issue's `**Status:**` inside an Accumulated
// Context bullet — captured the rewrite and destroyed the rest of the line,
// silently, while the real field went stale or was updated elsewhere. The fix
// anchors the bold form to line start with same-line leading whitespace
// (`^([ \t]*\*\*Field:\*\*[ \t]*)`, 'im'), reusing #4010's same-line
// confinement idiom and #4186's recognition-by-anchoring discipline. These rows
// pin the corruption shapes; rows further down pin the negative space
// (legitimate line-start bold updates are byte-identical, branch order and
// first-occurrence-wins unchanged).
describe('stateReplaceField — anchored bold form leaves prose lookalikes untouched (#4243)', () => {
// The issue's verbatim prose line: a bold label quoted for documentation
// purposes inside a bullet, with the real field in the plain template form.
const ISSUE_PROSE_LINE =
'- [Phase 170]: archived files gained a `**Status:**Ready to execute` marker. Must not change.';
// ROW 1 — the failing-first regression from the issue. The lookalike must
// survive byte-identically and the REAL plain field must take the update.
test('issue repro: mid-sentence **Status:** lookalike survives, real plain field updates', () => {
const input = [
'## Current Position',
'',
'Phase: 5 of 9',
'Plan: 2 of 6',
'Status: Ready to execute',
'Last activity: 2026-08-01 — did a thing',
'',
'## Accumulated Context',
'',
'### Decisions',
'',
ISSUE_PROSE_LINE,
'',
].join('\n');
const result = stateReplaceField(input, 'Status', 'Executing Phase 901');
assert.notEqual(result, null, 'the real plain field must still match');
assert.ok(
result.includes(ISSUE_PROSE_LINE),
`prose lookalike must survive byte-identically, got:\n${result}`,
);
assert.ok(
/^Status: Executing Phase 901$/m.test(result),
'the real plain Status line must take the update',
);
assert.ok(
!result.includes('Executing Phase 901` marker'),
'the rewrite must not bleed into the prose occurrence',
);
});
test('lookalike ordered BEFORE the real bold field: prose survives, bold field updates', () => {
const input = [
'## Accumulated Context',
'',
ISSUE_PROSE_LINE,
'',
'## Current Position',
'',
'**Status:** Ready to execute',
'',
].join('\n');
const result = stateReplaceField(input, 'Status', 'Executing Phase 901');
assert.notEqual(result, null);
assert.ok(result.includes(ISSUE_PROSE_LINE), `prose lookalike must survive, got:\n${result}`);
assert.ok(
/^\*\*Status:\*\* Executing Phase 901$/m.test(result),
'the real line-start bold field must take the update',
);
});
test('mid-word lookalike with no real field: returns null (honest absence), never a rewrite', () => {
const input = 'Prose mentions text**Status:**tail mid-word and nothing else.';
assert.equal(stateReplaceField(input, 'Status', 'Executing Phase 901'), null);
});
// A list-item bold label (`- **Status:** value`) is prose-shaped for the
// writer: no STATE.md writer emits body fields as list items, and treating
// a bullet as a field write target is exactly the #4243 corruption class.
// The read side's own vocabulary (bold anywhere) is untouched; the writer
// reports honest absence instead.
test('list-item bold label is not a write target: returns null, bullet untouched', () => {
const input = '- **Status:** resolved in the archived review';
assert.equal(stateReplaceField(input, 'Status', 'new'), null);
});
// Every field the regex serves: a document whose ONLY occurrence of the
// label is a mid-sentence lookalike must yield null — no served field may
// be rewritten from prose.
test('mid-sentence lookalike yields null for every served field', () => {
const servedFields = [
'Status', 'Phase', 'Plan', 'Current Plan', 'Current Phase', 'Current Phase Name',
'Last Activity', 'Last Activity Description', 'Total Phases', 'Total Plans in Phase',
'Progress', 'Completed Phases', 'Stopped At',
];
for (const field of servedFields) {
const input = `Some prose sentence quoting a **${field}:** label mid-sentence, plus trailing words.`;
assert.equal(
stateReplaceField(input, field, 'NEW'),
null,
`mid-sentence **${field}:** lookalike must not match (got a rewrite)`,
);
}
});
test('lookalike plus real plain field: only the real plain line changes (representative fields)', () => {
const cases = [
{ field: 'Status', plain: 'Status: Ready to execute' },
{ field: 'Phase', plain: 'Phase: 5 of 9' },
{ field: 'Last Activity', plain: 'Last Activity: 2026-08-01 — did a thing' },
];
for (const { field, plain } of cases) {
const lookalike = `- notes: the **${field}:** label was archived here. Keep it.`;
const input = [plain, '', '## Accumulated Context', '', lookalike, ''].join('\n');
const result = stateReplaceField(input, field, 'NEW VALUE');
assert.notEqual(result, null, `${field}: real plain field must match`);
assert.ok(
result.includes(lookalike),
`${field}: lookalike line must survive byte-identically, got:\n${result}`,
);
}
});
// Negative space: an INDENTED line-start bold field is still a field (the
// doc's form ranking reads bold anywhere in the section; the writer keeps
// same-line indentation writable), and the indent is preserved.
test('indented line-start bold field still updates, indent preserved', () => {
const input = ' **Status:** old';
const result = stateReplaceField(input, 'Status', 'new');
assert.equal(result, ' **Status:** new');
});
// Negative space + fix-shape pin: leading blank lines before the label are
// NOT swallowed. The anchor's leading class is same-line whitespace only
// (`[ \t]*`, #4010's idiom); the issue's suggested `^\s*` variant would
// consume the newlines into the match and drop them on rebuild.
test('leading blank lines before a bold label survive byte-identically', () => {
const input = '\n\n**Status:** Ready';
const result = stateReplaceField(input, 'Status', 'Executing Phase 5');
assert.equal(result, '\n\n**Status:** Executing Phase 5');
});
test('CRLF document: lookalike survives with CRLF intact, real plain field updates', () => {
const input = [
'Status: Ready to execute',
'',
'## Accumulated Context',
'',
ISSUE_PROSE_LINE,
'',
].join('\r\n');
const result = stateReplaceField(input, 'Status', 'Executing Phase 901');
assert.notEqual(result, null);
assert.ok(result.includes(ISSUE_PROSE_LINE), `prose lookalike must survive, got:\n${result}`);
assert.ok(result.includes('\r\n'), 'CRLF endings must be preserved');
assert.ok(/^Status: Executing Phase 901\r?$/m.test(result), 'real plain field must update');
});
// Negative space: branch ORDER is unchanged — a line-start bold field still
// beats the plain form, and only the first bold occurrence is replaced.
test('line-start bold still beats the plain form (branch order unchanged)', () => {
const input = '**Status:** old bold\nStatus: old plain';
const result = stateReplaceField(input, 'Status', 'new');
assert.equal(result, '**Status:** new\nStatus: old plain');
});
test('two line-start bold occurrences: only the first is replaced', () => {
const input = '**Status:** first\n**Status:** second';
const result = stateReplaceField(input, 'Status', 'new');
assert.equal(result, '**Status:** new\n**Status:** second');
});
// #4010 same-line adjacency under the anchor: an empty bold field's value
// lands on its own line and the following line survives.
test('anchored bold branch keeps the #4010 empty-field boundary', () => {
const input = ' **Status:**\n **Current Plan:** 2 of 5';
const result = stateReplaceField(input, 'Status', 'Executing Phase 5');
assert.equal(result, ' **Status:** Executing Phase 5\n **Current Plan:** 2 of 5');
});
});
describe('stateExtractField (#2880)', () => {
test('extracts from a two-cell row', () => {
const input = '| Current Phase | 3 |';

View File

@@ -3892,6 +3892,255 @@ Progress: [..........] 0%
});
});
// ─────────────────────────────────────────────────────────────────────────────
// #4243 — begin-phase: prose bold-lookalikes stay untouched (anchored bold
// form in stateReplaceField) and frontmatter round-trips unknown keys
// ─────────────────────────────────────────────────────────────────────────────
describe('#4243: begin-phase leaves prose lookalikes untouched, preserves unknown frontmatter', () => {
const ISSUE_PROSE_LINE =
'- [Phase 170]: archived files gained a `**Status:**Ready to execute` marker. Must not change.';
let tmpDir;
beforeEach(() => {
tmpDir = createFixture();
});
afterEach(() => {
cleanup(tmpDir);
});
// The issue's suggested regression fixture 1, verbatim shape: a bold
// `**Status:**` inside prose (## Accumulated Context), the real field in
// the plain template form. begin-phase must rewrite ONLY the real field.
test('issue fixture 1: prose **Status:** lookalike is byte-identical, real field updates', () => {
writeState(tmpDir, [
'# Project State',
'',
'## Current Position',
'Phase: 5 of 9 (Fifth)',
'Plan: 2 of 6 in current phase',
'Status: Ready to execute',
'Last activity: 2026-08-01 — did a thing',
'',
'Progress: [████░░░░░░] 40%',
'',
'## Accumulated Context',
'',
'### Decisions',
'',
ISSUE_PROSE_LINE,
'',
].join('\n'));
const result = runGsdTools(
['state', 'begin-phase', '--phase', '901', '--plans', '3'],
tmpDir,
);
assert.ok(result.success, `begin-phase failed: ${result.error}`);
const content = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8');
assert.ok(
content.includes(ISSUE_PROSE_LINE),
`prose lookalike must survive begin-phase byte-identically, got:\n${content}`,
);
const pos = sectionMatchOf(content, 'Current Position');
assert.ok(pos, 'Current Position section should exist');
assert.match(pos[1], /^Status: Executing Phase 901$/m);
});
// Corruption shape 1b: the lookalike section ordered BEFORE ## Current
// Position, real field in the bold form — first-match-in-document-order is
// the prose under the unanchored pattern.
test('lookalike before Current Position: prose survives, real bold field updates', () => {
writeState(tmpDir, [
'# Project State',
'',
'## Accumulated Context',
'',
ISSUE_PROSE_LINE,
'',
'## Current Position',
'',
'Phase: 5 of 9',
'Plan: 2 of 6',
'**Status:** Ready to execute',
'Last activity: 2026-08-01 — did a thing',
'',
].join('\n'));
const result = runGsdTools(
['state', 'begin-phase', '--phase', '901', '--plans', '3'],
tmpDir,
);
assert.ok(result.success, `begin-phase failed: ${result.error}`);
const content = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8');
assert.ok(
content.includes(ISSUE_PROSE_LINE),
`prose lookalike must survive begin-phase byte-identically, got:\n${content}`,
);
assert.match(content, /^\*\*Status:\*\* Executing Phase 901$/m);
});
// Corruption shape 1c: lookalikes for OTHER served fields — a mid-sentence
// `**Last Activity:**` must not capture the Last-activity refresh.
test('mid-sentence **Last Activity:** lookalike survives, real field refreshes', () => {
const lookalike = 'An earlier note mentions **Last Activity:** thresholds for archival. Keep.';
writeState(tmpDir, [
'# Project State',
'',
'## Current Position',
'Phase: 5 of 9',
'Plan: 2 of 6',
'Status: Ready to execute',
'Last activity: 2026-08-01 — did a thing',
'',
'## Accumulated Context',
'',
lookalike,
'',
].join('\n'));
const result = runGsdTools(
['state', 'begin-phase', '--phase', '901', '--plans', '3'],
tmpDir,
);
assert.ok(result.success, `begin-phase failed: ${result.error}`);
const content = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8');
assert.ok(content.includes(lookalike), `lookalike must survive, got:\n${content}`);
const pos = sectionMatchOf(content, 'Current Position');
assert.ok(pos, 'Current Position section should exist');
assert.match(pos[1], /^Last activity: \d{4}-\d{2}-\d{2}/m);
});
// The issue's suggested regression fixture 2, no-ROADMAP arm: a custom
// frontmatter key, a populated progress block (with a custom subkey), a
// curated stopped_at, and milestone identity — all must survive begin-phase
// without a ROADMAP.md, and milestone/milestone_name must NOT be reset to
// invented defaults. (Already-correct behavior on next via the #2202
// carry-forward + #3216 milestone-identity fix + the #4129 ratchet; pinned
// here so the class cannot regress.)
const FRONTMATTER_FIXTURE = [
'---',
"gsd_state_version: '1.0'",
'milestone: v2.1',
'milestone_name: Real Curated Name',
'status: planning',
"stopped_at: '2026-08-01 — curated stop note'",
'custom_key: hand-added-by-agent',
'progress:',
' total_phases: 9',
' completed_phases: 4',
' total_plans: 30',
' completed_plans: 12',
' percent: 40',
' custom_subkey: 77',
'---',
'',
'# Project State',
'',
'## Current Position',
'Phase: 5 of 9 (Fifth)',
'Plan: 2 of 6 in current phase',
'Status: Ready to execute',
'Last activity: 2026-08-01 — did a thing',
'',
].join('\n');
test('issue fixture 2 (no ROADMAP): unknown keys, progress subkeys, stopped_at, milestone survive', () => {
writeState(tmpDir, FRONTMATTER_FIXTURE);
const result = runGsdTools(
['state', 'begin-phase', '--phase', '901', '--name', 'Nine-Oh-One', '--plans', '3'],
tmpDir,
);
assert.ok(result.success, `begin-phase failed: ${result.error}`);
const content = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8');
const fm = frontmatterLib.extractFrontmatter(content);
assert.equal(fm['custom_key'], 'hand-added-by-agent', 'custom frontmatter key must survive');
assert.equal(fm['milestone'], 'v2.1', 'milestone must not be reset to an invented default');
assert.equal(fm['milestone_name'], 'Real Curated Name', 'curated milestone_name must survive');
assert.ok(String(fm['stopped_at']).includes('curated stop note'), 'stopped_at must survive');
const progress = fm['progress'];
assert.ok(progress && typeof progress === 'object', 'progress block must survive');
// Numeric-tolerant: reconstructFrontmatter may serialize an unknown subkey
// as a quoted scalar, so it re-parses as a string — the VALUE surviving is
// the contract, not the YAML scalar shape.
assert.equal(Number(progress['custom_subkey']), 77, 'custom progress subkey must survive');
// With ROADMAP.md absent and a milestone asserted, the #3573/#4094 withhold
// keeps all four counters at their STORED values (pinned below) and omits
// percent (an unmeasured scan must not assert one, #3233). `percent` is a
// DECLARED derived subkey governed by that recorded semantics — unlike the
// custom subkey above, its absence here is the documented behavior, so this
// row deliberately does not pin its value in either arm.
assert.equal(Number(progress['total_phases']), 9, 'stored total_phases kept under the #3573 withhold');
assert.equal(Number(progress['completed_phases']), 4, 'stored completed_phases kept under the #3573 withhold');
assert.equal(Number(progress['total_plans']), 30, 'stored total_plans kept under the #3573 withhold');
assert.equal(Number(progress['completed_plans']), 12, 'stored completed_plans kept under the #3573 withhold');
// Known keys still take the begin-phase update.
assert.equal(fm['status'], 'executing');
assert.equal(String(fm['current_phase']), '901');
});
test('issue fixture 2 (with ROADMAP): unknown keys and progress subkeys survive', () => {
writeState(tmpDir, FRONTMATTER_FIXTURE);
fs.writeFileSync(
path.join(tmpDir, '.planning', 'ROADMAP.md'),
['# Roadmap', '', '## v2.1 — Real Curated Name', '', '### Phase 5: Fifth', '', 'complete.', ''].join('\n'),
);
const result = runGsdTools(
['state', 'begin-phase', '--phase', '901', '--name', 'Nine-Oh-One', '--plans', '3'],
tmpDir,
);
assert.ok(result.success, `begin-phase failed: ${result.error}`);
const content = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8');
const fm = frontmatterLib.extractFrontmatter(content);
assert.equal(fm['custom_key'], 'hand-added-by-agent', 'custom frontmatter key must survive');
assert.equal(fm['milestone'], 'v2.1');
assert.equal(fm['milestone_name'], 'Real Curated Name');
const progress = fm['progress'];
assert.ok(progress && typeof progress === 'object', 'progress block must survive');
assert.equal(Number(progress['custom_subkey']), 77, 'custom progress subkey must survive');
assert.equal(fm['status'], 'executing');
});
test('milestone without milestone_name, no ROADMAP: no invented identity appears', () => {
writeState(tmpDir, [
'---',
"gsd_state_version: '1.0'",
'milestone: v2.1',
'custom_key: keep-me',
'---',
'',
'# Project State',
'',
'## Current Position',
'Phase: 5 of 9',
'Plan: 2 of 6',
'Status: Ready to execute',
'',
].join('\n'));
const result = runGsdTools(
['state', 'begin-phase', '--phase', '901', '--plans', '3'],
tmpDir,
);
assert.ok(result.success, `begin-phase failed: ${result.error}`);
const content = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8');
const fm = frontmatterLib.extractFrontmatter(content);
assert.equal(fm['milestone'], 'v2.1', 'milestone must survive without a ROADMAP');
assert.equal(fm['milestone_name'], undefined, 'no fabricated milestone_name may appear');
assert.equal(fm['custom_key'], 'keep-me');
});
});
// ─────────────────────────────────────────────────────────────────────────────
// Bug #1589 — progress counters not updated during plan execution
// ─────────────────────────────────────────────────────────────────────────────