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

fix(#2956): scope Phase extraction to ## Current Position (3rd gen of #2444/#2567) (#2961)

Browse Source 
* test(#2956): fail-first regressions for Phase scoped to ## Current Position

Third generation of #2444 / #2567. Stopped At / Paused At were scoped to
## Session; Phase (canonically in ## Current Position per templates/state.md)
was left unscoped, so a historical Phase: / **Phase:** line in an archive
section overwrites current_phase on every write. Since current_phase is
routing input for gsd-progress / --next, the rewind routes work to the wrong
phase.

Six failing-first regressions + one round-trip:
- shape B: bold **Phase:** 19 archive BELOW the section
- shape C: plain archive Phase: 19 ABOVE the section
- bootstrap h3 ### Current Position variant
- CRLF variant
- Phase token in decisions prose (over-broad-fix guard)
- Paused At read-path parity with the write seam (## Session)
- write-then-read round trip stays at 22 (read/write agreement)

Folded into tests/state.test.cjs (lint:regression-test-names bans a
new tests/bug-NNNN-*.test.cjs file).

* fix(#2956): scope Phase extraction to ## Current Position at both seams

Third generation of #2444 / #2567. Stopped At / Paused At were scoped to
## Session by those fixes; Phase (canonically in ## Current Position per
templates/state.md) was left unscoped, so a historical Phase: / **Phase:**
line in an archive section silently overwrote current_phase on every write.
Because current_phase is routing input for gsd-progress / --next, the rewind
routes work to the wrong phase.

Fix mirrors the proven #2444 seam exactly:
- new matchCurrentPositionSection helper (collectSection-based, CRLF-tolerant,
  level-flexible for the bootstrap ### Current Position h3 variant), sited next
  to matchSessionSection.
- read path (cmdStateSnapshot): extract Phase from matchCurrentPositionSection
  ?? body. Also scope Paused At to matchSessionSection ?? body so the read seam
  agrees with the write seam (which already scoped Paused At to ## Session).
- write path (buildStateFrontmatter): extract Phase from
  matchCurrentPositionSection ?? bodyContent.

stateExtractField itself is untouched (its bold/plain precedence is load-bearing
for other fields — the #3265 test depends on it), and preferNewerLastActivity is
untouched (Last Activity has no canonical section; its date-direction guard is
deliberate). Fall back to full-body when no ## Current Position section exists
so files without the heading keep current behaviour.

* chore(#2956): add changeset fragment (pr:0 placeholder, backfill after PR)

* test(#2956): make round-trip test actually trigger the write-path resync

The write-then-read round-trip test used 'state update Status "Executing"' on a
fixture with no Status field, so the update was a no-op (updated:false) and no
frontmatter resync ran through buildStateFrontmatter — the assertion on the
written frontmatter then failed not because the fix is wrong, but because no
write happened. Add a **Status:** field so the update performs a real field
update (updated:true) and forces the resync. Verified locally: pre-fix this
writes current_phase:19 (the archive value); post-fix it writes 22.

The code fix is correct (5 of 7 RED tests passed; the 2 failures were this
defective test). This is the 'fix the bad test' half of the TDD-loop rule.

* chore(#2956): backfill changeset PR number 2961

---------

Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
2026-08-01 00:02:41 -04:00
committed by GitHub
parent 9ac0dfad58
commit 73418c516f
3 changed files with 268 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
.changeset/curious-seals-zip.md Normal file
View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 2961
---
**`current_phase` no longer rewinds to an archived phase when STATE.md carries a historical `Phase:` line** — a stale `Phase:` or `**Phase:**` line in an archive section of a long-lived STATE.md silently overwrote `current_phase` on every state write, and because `current_phase` drives `gsd-progress` and `--next` routing the rewind sent work to the wrong phase. Phase extraction is now scoped to the `## Current Position` section (mirroring the existing `## Session` scoping for Stopped At / Paused At). (#2956)

50
src/state.cts
View File

@@ -1307,6 +1307,31 @@ function matchSessionSection(body: string): string | null {
return section ? section.body : null;
}
/**
* Match the "Current Position" section body from a STATE.md body. #2956: this
* is the Phase analogue of matchSessionSection. `Phase` canonically lives under
* `## Current Position` (gsd-core/templates/state.md), so — like Stopped At /
* Paused At under `## Session` — it must be extracted from THAT section, not
* from the first `Phase:` / `**Phase:**` line anywhere in the body. Without the
* scope, a historical `Phase:` line in an archive section silently overwrites
* `current_phase` on every write, and because `current_phase` is routing input
* for gsd-progress / --next the rewind routes work to the wrong phase.
*
* Level-flexible: the canonical template uses an h2 `## Current Position`, the
* bootstrap template an h3 `### Current Position` (templates/state.md). Both
* must match — mirroring how matchSessionSection recognises `## Session` and
* `## Session Continuity`. Exact 'current position' text match (case-insensitive)
* excludes unrelated headings. Built on the same `collectSection` seam as
* matchSessionSection, so it inherits that seam's CRLF tolerance (#2444 fix).
* Returns the section body, or null (caller falls back to full-body search).
*/
function matchCurrentPositionSection(body: string): string | null {
const isCurrentPosition = (h: HeadingToken): boolean =>
(h.level === 2 || h.level === 3) && h.text.trim().toLowerCase() === 'current position';
const section = collectSection(body, isCurrentPosition, { levelBounded: true });
return section ? section.body : null;
}
/**
* #2567: prevent a stale archive "Last activity:" line from overwriting a
* newer frontmatter value. `stateExtractField` matches the first body
@@ -1392,7 +1417,14 @@ function cmdStateSnapshot(cwd: string, raw: boolean): void {
};
// Extract basic fields — frontmatter keys take precedence over body
const prosePhase = parseProsePhaseField(stateExtractField(body, 'Phase'));
// #2956: scope `Phase` extraction to ## Current Position so a historical
// Phase: / **Phase:** line in an archive section cannot overwrite the current
// value. Phase canonically lives in ## Current Position (templates/state.md),
// so it is scopeable exactly like Stopped At under ## Session. Fall back to
// full-body search only when no ## Current Position section exists, so files
// with no section heading keep their current behaviour.
const currentPositionScope = matchCurrentPositionSection(body) ?? body;
const prosePhase = parseProsePhaseField(stateExtractField(currentPositionScope, 'Phase'));
const currentPhase = fmScalar('current_phase') ?? stateExtractField(body, 'Current Phase') ?? prosePhase.phase;
const currentPhaseName = fmScalar('current_phase_name') ?? stateExtractField(body, 'Current Phase Name') ?? prosePhase.name;
const totalPhasesRaw = fmScalar('total_phases') ?? stateExtractField(body, 'Total Phases');
@@ -1404,7 +1436,12 @@ function cmdStateSnapshot(cwd: string, raw: boolean): void {
const proseLastActivity = parseProseLastActivityField(rawLastActivity);
const lastActivity = fmScalar('last_activity') ?? proseLastActivity.date ?? rawLastActivity;
const lastActivityDesc = fmScalar('last_activity_desc') ?? stateExtractField(body, 'Last Activity Description') ?? proseLastActivity.description;
const pausedAt = fmScalar('paused_at') ?? stateExtractField(body, 'Paused At');
// #2956: Paused At canonically lives in ## Session (see the comment above
// preferNewerLastActivity and the write seam in buildStateFrontmatter). The
// write seam already scopes it to ## Session; this read seam must agree, so a
// stale "Paused At:" in a Session Continuity Archive cannot win here either.
const sessionScope = matchSessionSection(body) ?? body;
const pausedAt = fmScalar('paused_at') ?? stateExtractField(sessionScope, 'Paused At');
// Parse numeric fields
const totalPhases = totalPhasesRaw ? parseInt(totalPhasesRaw, 10) : null;
@@ -1552,7 +1589,14 @@ function extractRetiredPhaseNumbers(scope: string): Set<string> {
* reliably via `state json` instead of fragile regex parsing.
*/
function buildStateFrontmatter(bodyContent: string, cwd: string | undefined): Record<string, unknown> {
const prosePhase = parseProsePhaseField(stateExtractField(bodyContent, 'Phase'));
// #2956: scope `Phase` extraction to ## Current Position (mirrors the read
// path in cmdStateSnapshot and the Stopped At / Paused At ## Session scoping
// below). Phase canonically lives in ## Current Position (templates/state.md);
// without the scope, a historical Phase: / **Phase:** line in an archive
// section overwrites current_phase here, and the next read surfaces it. Fall
// back to full-body search when no ## Current Position section exists.
const currentPositionScope = matchCurrentPositionSection(bodyContent) ?? bodyContent;
const prosePhase = parseProsePhaseField(stateExtractField(currentPositionScope, 'Phase'));
const currentPhase = stateExtractField(bodyContent, 'Current Phase') ?? prosePhase.phase;
const currentPhaseName = stateExtractField(bodyContent, 'Current Phase Name') ?? prosePhase.name;
const currentPlan = stateExtractField(bodyContent, 'Current Plan');

216
tests/state.test.cjs
View File

@@ -162,6 +162,177 @@ describe('state-snapshot command', () => {
assert.strictEqual(output.paused_at, 'Phase 3, Plan 1, Task 2 - mid-implementation', 'paused_at extracted');
});
// ─── Regression: #2956 — Phase must be scoped to ## Current Position ──────
// Third generation of #2444 / #2567. Stopped At / Paused At were scoped to
// ## Session; Phase (which canonically lives in ## Current Position per
// gsd-core/templates/state.md) was left unscoped, so a historical Phase: /
// **Phase:** line in an archive section silently overwrote current_phase on
// every write. Because current_phase is routing input for gsd-progress / --next,
// the rewind routes work to the wrong phase — not merely a stale display.
test('#2956 scopes Phase to ## Current Position — bold archive line below the section (shape B)', () => {
// Bold **Phase:** 19 in an archive BELOW ## Current Position. The unscoped
// extractor's bold pattern wins outright (it is tried first, unanchored),
// so the read returns 19 instead of 22. Scoping to the section fixes it.
const stateContent = [
'# Project State',
'',
'## Current Position',
'',
'Phase: 22 (Documentation hygiene) — COMPLETE',
'',
'## Archive — earlier milestones',
'',
'**Phase:** 19 **complete — shipped v2.43.0**',
'',
].join('\n');
fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), stateContent);
const result = runGsdTools('state-snapshot', tmpDir);
assert.ok(result.success, `Command failed: ${result.error}`);
const output = JSON.parse(result.output);
assert.strictEqual(output.current_phase, '22', 'current_phase must come from ## Current Position, not the bold archive line');
});
test('#2956 scopes Phase to ## Current Position — plain archive line above the section (shape C)', () => {
// Plain archive Phase: 19 ABOVE ## Current Position. Without scoping the
// plain pattern (^Phase:, /im) matches the first line-start occurrence in
// document order — the archive line — and returns 19 instead of 22.
const stateContent = [
'# Project State',
'',
'## Archive — earlier milestones',
'',
'Phase: 19 **complete — shipped v2.43.0**',
'',
'## Current Position',
'',
'Phase: 22 (Documentation hygiene) — COMPLETE',
'',
].join('\n');
fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), stateContent);
const result = runGsdTools('state-snapshot', tmpDir);
assert.ok(result.success, `Command failed: ${result.error}`);
const output = JSON.parse(result.output);
assert.strictEqual(output.current_phase, '22', 'current_phase must come from ## Current Position, not the plain archive line above it');
});
test('#2956 scopes Phase to ### Current Position (bootstrap h3 variant)', () => {
// gsd-core/templates/state.md ships a bootstrap layout that uses a level-3
// ### Current Position heading. The section matcher must recognise BOTH h2
// and h3, mirroring how matchSessionSection recognises ## Session and
// ## Session Continuity — matching only h2 would silently drop the h3 shape.
const stateContent = [
'# Project State',
'',
'### Current Position',
'',
'Phase: 22 (Documentation hygiene)',
'',
'### Archive',
'',
'**Phase:** 19',
'',
].join('\n');
fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), stateContent);
const result = runGsdTools('state-snapshot', tmpDir);
assert.ok(result.success, `Command failed: ${result.error}`);
const output = JSON.parse(result.output);
assert.strictEqual(output.current_phase, '22', 'current_phase must resolve from the h3 ### Current Position section');
});
test('#2956 scopes Phase to ## Current Position under CRLF', () => {
// The #2444 seam was CRLF-fixed by migrating onto collectSection (which
// strips the trailing \r before heading-text extraction). The Phase scope
// must inherit that CRLF tolerance — a hand-rolled [ \t]*\n boundary would
// silently fail to match a ## Current Position\r\n heading.
const stateContent = [
'# Project State',
'',
'## Current Position',
'',
'Phase: 22 (Documentation hygiene)',
'',
'## Archive — earlier milestones',
'',
'**Phase:** 19',
'',
].join('\r\n');
fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), stateContent);
const result = runGsdTools('state-snapshot', tmpDir);
assert.ok(result.success, `Command failed: ${result.error}`);
const output = JSON.parse(result.output);
assert.strictEqual(output.current_phase, '22', 'current_phase must come from ## Current Position under CRLF');
});
test('#2956 ignores a Phase token in decisions prose outside ## Current Position', () => {
// A decisions-table row or prose mention of "Phase 19" elsewhere is NOT the
// current phase. Scoping it out is correct, not a regression — this is the
// over-broad-fix guard.
const stateContent = [
'# Project State',
'',
'## Current Position',
'',
'Phase: 22 (Documentation hygiene)',
'',
'### Decisions',
'',
'| Decided to defer Phase 19 to the next milestone | 2026-07-01 |',
'',
].join('\n');
fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), stateContent);
const result = runGsdTools('state-snapshot', tmpDir);
assert.ok(result.success, `Command failed: ${result.error}`);
const output = JSON.parse(result.output);
assert.strictEqual(output.current_phase, '22', 'a Phase token in decisions prose must not leak into current_phase');
});
test('#2956 scopes Paused At to ## Session on the read path (parity with the write seam)', () => {
// The WRITE seam (buildStateFrontmatter src/state.cts ~1576) already scopes
// Paused At to ## Session. The READ seam (cmdStateSnapshot) read it
// unscoped — a parity gap. A stale "Paused At:" in a Session Continuity
// Archive below the real ## Session must not win on the read path.
const stateContent = [
'# Project State',
'',
'## Current Position',
'',
'Phase: 22 (Documentation hygiene)',
'',
'## Session',
'',
'**Paused At:** Phase 22, Plan 1, Task 2 - mid-implementation',
'',
'## Session Continuity Archive',
'',
'**Paused At:** Phase 19, Plan 3 (stale)',
'',
].join('\n');
fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), stateContent);
const result = runGsdTools('state-snapshot', tmpDir);
assert.ok(result.success, `Command failed: ${result.error}`);
const output = JSON.parse(result.output);
assert.strictEqual(output.paused_at, 'Phase 22, Plan 1, Task 2 - mid-implementation', 'paused_at must come from ## Session, not the archive');
});
describe('--cwd override', () => {
let outsideDir;
@@ -588,6 +759,51 @@ describe('STATE.md frontmatter sync', () => {
assert.ok(content.includes('status: paused'), 'frontmatter should reflect latest status');
});
test('#2956 write-then-read does not rewind current_phase past an archive Phase line', () => {
// The write seam (buildStateFrontmatter) and the read seam (cmdStateSnapshot)
// must agree: a state write that re-syncs frontmatter must not pick up the
// archive **Phase:** 19 line and write current_phase: 19, which the next
// state-snapshot read would then surface. Round-trip must stay at 22.
//
// The fixture carries a **Status:** field so `state update Status` performs
// a real field update (updated:true) and forces the frontmatter resync
// through buildStateFrontmatter — without an existing Status field the
// update is a no-op (updated:false) and no write occurs.
const stateContent = [
'# Project State',
'',
'## Current Position',
'',
'Phase: 22 (Documentation hygiene)',
'',
'**Status:** Ready',
'',
'## Archive — earlier milestones',
'',
'**Phase:** 19 **complete — shipped v2.43.0**',
'',
].join('\n');
fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), stateContent);
// state update forces a frontmatter sync through buildStateFrontmatter.
const writeResult = runGsdTools('state update Status "Executing"', tmpDir);
assert.ok(writeResult.success, `write failed: ${writeResult.error}`);
const writeOutput = JSON.parse(writeResult.output);
assert.strictEqual(writeOutput.updated, true, 'state update must perform a real field update to force the resync');
// The persisted frontmatter must not have rewound to the archive phase.
const written = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8');
assert.ok(/current_phase:\s*22\b/m.test(written), 'written frontmatter current_phase must be 22 (not the archive 19)');
assert.ok(!/^current_phase:\s*19\b/m.test(written), 'written frontmatter must NOT carry the archive phase 19');
// And a fresh read must agree.
const readResult = runGsdTools('state-snapshot', tmpDir);
assert.ok(readResult.success, `read failed: ${readResult.error}`);
const output = JSON.parse(readResult.output);
assert.strictEqual(output.current_phase, '22', 'read-after-write current_phase must stay 22 (round-trip agreement)');
});
test('preserves frontmatter status when body Status field is missing', () => {
// Simulate: frontmatter has status: executing, but body lost Status: field
fs.writeFileSync(