From a56707a07b2ed074ba738916990bd4347e794ff1 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Thu, 23 Apr 2026 08:22:42 -0400 Subject: [PATCH] fix(#2613): preserve STATE.md frontmatter on write path (option 2) (#2622) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#2613): preserve STATE.md frontmatter on write path (option 2) `readModifyWriteStateMd` strips frontmatter before invoking the modifier, so `syncStateFrontmatter` received body-only content and `existingFm` was always `{}`. The preservation branch never fired, and every mutation re-derived `status` (to `'unknown'` when body had no `Status:` line) and `progress.*` (to 0/0 when the shipped milestone's phase directories were archived), silently overwriting authoritative frontmatter values. Option 2 — write-side analogue of #2495 READ fix: `buildStateFrontmatter` reads the current STATE.md frontmatter from disk as a preservation backstop. Status preserved when derived is `'unknown'` and existing is non-unknown. Progress preserved when disk scan returns all zeros AND existing has non-zero counts. Legitimate body-driven status changes and non-zero disk counts still win. Milestone/milestone_name already preserved via `getMilestoneInfo`'s #2495 fix — regression test added to lock that in. Adds 5 regression tests covering status preservation, progress preservation, milestone preservation, legitimate status updates, and disk-scan-wins-when-non-zero. Closes #2613 * fix(sdk): double-cast WorkflowConfig to Record in loadGateConfig TypeScript error on main (introduced in #2611) blocks the install-smoke CI job: `WorkflowConfig` has no string index signature, so the direct cast to `Record` fails type-check. The SDK build fails, `installSdkIfNeeded()` cannot install `gsd-sdk` from source, and the smoke job reports a false-positive installer regression. src/query/check-decision-coverage.ts(236,16): error TS2352: Conversion of type 'WorkflowConfig' to type 'Record' may be a mistake because neither type sufficiently overlaps with the other. Apply the double-cast via `unknown` as the compiler suggests. Behavior is unchanged — this was already a cast. --- sdk/src/query/check-decision-coverage.ts | 2 +- sdk/src/query/state-mutation.test.ts | 218 +++++++++++++++++++++++ sdk/src/query/state.ts | 33 ++++ 3 files changed, 252 insertions(+), 1 deletion(-) diff --git a/sdk/src/query/check-decision-coverage.ts b/sdk/src/query/check-decision-coverage.ts index f34eb839a..d92fc1269 100644 --- a/sdk/src/query/check-decision-coverage.ts +++ b/sdk/src/query/check-decision-coverage.ts @@ -233,7 +233,7 @@ function planSectionsMention(planSections: PlanSections[], decision: ParsedDecis async function loadGateConfig(projectDir: string, workstream?: string): Promise { try { const cfg = await loadConfig(projectDir, workstream); - const wf = (cfg.workflow ?? {}) as Record; + const wf = (cfg.workflow ?? {}) as unknown as Record; const v = wf.context_coverage_gate; if (typeof v === 'boolean') return v; // Tolerate stringified booleans coming from environment-variable-style configs, diff --git a/sdk/src/query/state-mutation.test.ts b/sdk/src/query/state-mutation.test.ts index 6acf85766..1e91d0a92 100644 --- a/sdk/src/query/state-mutation.test.ts +++ b/sdk/src/query/state-mutation.test.ts @@ -448,3 +448,221 @@ describe('stateRecordSession', () => { expect(content).toContain('Completed 11-01-PLAN.md'); }); }); + +// ─── Bug #2613: write-side frontmatter preservation ───────────────────────── + +describe('Bug #2613: STATE.md frontmatter preservation through mutations', () => { + let tmpDir: string; + + beforeEach(async () => { + tmpDir = await mkdtemp(join(tmpdir(), 'gsd-state-2613-')); + }); + + afterEach(async () => { + await rm(tmpDir, { recursive: true, force: true }); + }); + + it('record-session preserves milestone + milestone_name when ROADMAP has a different current milestone', async () => { + // STATE.md declares v12.0 / Focus (shipped). ROADMAP's heading-parseable + // current is v11.0 / Research-Depth. Before the fix, re-derivation pulled + // v11.0 / Research-Depth into STATE.md's frontmatter on every mutation. + const stateContent = `--- +gsd_state_version: 1.0 +milestone: v12.0 +milestone_name: Focus +status: shipped +--- + +# Project State + +## Session Continuity + +Last session: 2026-04-20T00:00:00Z +Stopped at: v12.0 SHIPPED +Resume file: None +`; + const roadmapContent = `# Roadmap + +## Phases + +## v11.0 Research-Depth Scoring (In Progress) + +### Phase 55 +- stuff + +## v12.0 Focus — ✅ SHIPPED 2026-04-20 + +### Phase 60 +- shipped stuff +`; + const planningDir = join(tmpDir, '.planning'); + await mkdir(join(planningDir, 'phases'), { recursive: true }); + await writeFile(join(planningDir, 'STATE.md'), stateContent, 'utf-8'); + await writeFile(join(planningDir, 'ROADMAP.md'), roadmapContent, 'utf-8'); + await writeFile(join(planningDir, 'config.json'), '{}', 'utf-8'); + + const { stateRecordSession } = await import('./state-mutation.js'); + await stateRecordSession( + ['--stopped-at', 'regression test', '--resume-file', '.planning/MILESTONES.md'], + tmpDir, + ); + + const after = await readFile(join(planningDir, 'STATE.md'), 'utf-8'); + const { extractFrontmatter } = await import('./frontmatter.js'); + const fm = extractFrontmatter(after); + expect(fm.milestone).toBe('v12.0'); + expect(fm.milestone_name).toBe('Focus'); + }); + + it('record-session preserves status from existing frontmatter when body has no Status field', async () => { + // STATE.md frontmatter declares status: shipped. Body has no "Status:" line. + // Before the fix, derived status defaulted to 'unknown' and the frontmatter + // value was lost because existingFm was {} at the preservation branch. + const stateContent = `--- +gsd_state_version: 1.0 +milestone: v12.0 +milestone_name: Focus +status: shipped +--- + +# Project State + +## Session Continuity + +Last session: 2026-04-20T00:00:00Z +Stopped at: v12.0 SHIPPED +Resume file: None +`; + const planningDir = join(tmpDir, '.planning'); + await mkdir(join(planningDir, 'phases'), { recursive: true }); + await writeFile(join(planningDir, 'STATE.md'), stateContent, 'utf-8'); + await writeFile(join(planningDir, 'ROADMAP.md'), '# Roadmap\n\n## v12.0 Focus\n', 'utf-8'); + await writeFile(join(planningDir, 'config.json'), '{}', 'utf-8'); + + const { stateRecordSession } = await import('./state-mutation.js'); + await stateRecordSession( + ['--stopped-at', 'regression test', '--resume-file', 'None'], + tmpDir, + ); + + const after = await readFile(join(planningDir, 'STATE.md'), 'utf-8'); + const { extractFrontmatter } = await import('./frontmatter.js'); + const fm = extractFrontmatter(after); + expect(fm.status).toBe('shipped'); + }); + + it('record-session preserves progress from frontmatter when disk scan returns zero counts', async () => { + // Shipped milestone: phase directories have been archived, so disk scan + // returns total_plans=0. Existing frontmatter has authoritative counts + // (5/5, 12/12, 100%). Before the fix, disk scan stomped the counts to 0/0. + const stateContent = `--- +gsd_state_version: 1.0 +milestone: v12.0 +milestone_name: Focus +status: shipped +progress: + total_phases: 5 + completed_phases: 5 + total_plans: 12 + completed_plans: 12 + percent: 100 +--- + +# Project State + +## Session Continuity + +Last session: 2026-04-20T00:00:00Z +Stopped at: v12.0 SHIPPED +Resume file: None +`; + const planningDir = join(tmpDir, '.planning'); + await mkdir(join(planningDir, 'phases'), { recursive: true }); + await writeFile(join(planningDir, 'STATE.md'), stateContent, 'utf-8'); + await writeFile(join(planningDir, 'ROADMAP.md'), '# Roadmap\n\n## v12.0 Focus\n', 'utf-8'); + await writeFile(join(planningDir, 'config.json'), '{}', 'utf-8'); + + const { stateRecordSession } = await import('./state-mutation.js'); + await stateRecordSession( + ['--stopped-at', 'regression test', '--resume-file', 'None'], + tmpDir, + ); + + const after = await readFile(join(planningDir, 'STATE.md'), 'utf-8'); + const { extractFrontmatter } = await import('./frontmatter.js'); + const fm = extractFrontmatter(after); + const progress = fm.progress as Record; + expect(Number(progress.total_plans)).toBe(12); + expect(Number(progress.completed_plans)).toBe(12); + expect(Number(progress.percent)).toBe(100); + }); + + it('regression guard: state.update Status still updates frontmatter status when body is mutated', async () => { + // Legitimate status change must still propagate. If the body's Status + // field becomes "executing", derived status is 'executing' and option 2 + // must NOT overwrite it with the frontmatter's prior 'shipped'. + await setupTestProject(tmpDir); + + const { stateUpdate } = await import('./state-mutation.js'); + await stateUpdate(['Status', 'executing'], tmpDir); + + const after = await readFile(join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); + const { extractFrontmatter } = await import('./frontmatter.js'); + const fm = extractFrontmatter(after); + expect(fm.status).toBe('executing'); + }); + + it('regression guard: disk-scanned progress wins when scan returns non-zero counts', async () => { + // Mid-milestone: disk has real phase directories with plans + summaries. + // Disk is the ground truth — frontmatter progress must not override it. + const stateContent = `--- +gsd_state_version: 1.0 +milestone: v3.0 +milestone_name: SDK-First Migration +status: executing +progress: + total_phases: 99 + completed_phases: 99 + total_plans: 99 + completed_plans: 99 + percent: 99 +--- + +# Project State + +## Current Position + +Status: Executing +Last activity: today + +## Session Continuity + +Last session: 2026-04-08T05:00:00Z +Stopped at: work +Resume file: None +`; + const planningDir = join(tmpDir, '.planning'); + const phasesDir = join(planningDir, 'phases'); + await mkdir(phasesDir, { recursive: true }); + // Real phase with 1 plan and 1 summary — disk scan must report these. + const phase10 = join(phasesDir, '10-foo'); + await mkdir(phase10, { recursive: true }); + await writeFile(join(phase10, '10-01-PLAN.md'), 'plan', 'utf-8'); + await writeFile(join(phase10, '10-01-SUMMARY.md'), 'summary', 'utf-8'); + await writeFile(join(planningDir, 'STATE.md'), stateContent, 'utf-8'); + await writeFile(join(planningDir, 'ROADMAP.md'), '# Roadmap\n\n### Phase 10: Foo\n', 'utf-8'); + await writeFile(join(planningDir, 'config.json'), '{}', 'utf-8'); + + const { stateRecordSession } = await import('./state-mutation.js'); + await stateRecordSession(['--stopped-at', 'x', '--resume-file', 'None'], tmpDir); + + const after = await readFile(join(planningDir, 'STATE.md'), 'utf-8'); + const { extractFrontmatter } = await import('./frontmatter.js'); + const fm = extractFrontmatter(after); + const progress = fm.progress as Record; + // Disk ground truth — not the stale 99/99 from frontmatter. + expect(Number(progress.total_plans)).toBe(1); + expect(Number(progress.completed_plans)).toBe(1); + expect(Number(progress.percent)).toBe(100); + }); +}); diff --git a/sdk/src/query/state.ts b/sdk/src/query/state.ts index e683a5269..2125841b1 100644 --- a/sdk/src/query/state.ts +++ b/sdk/src/query/state.ts @@ -89,6 +89,19 @@ export async function buildStateFrontmatter(bodyContent: string, projectDir: str const stoppedAt = stateExtractField(bodyContent, 'Stopped At') || stateExtractField(bodyContent, 'Stopped at'); const pausedAt = stateExtractField(bodyContent, 'Paused At'); + // Bug #2613: read existing STATE.md frontmatter as preservation backstop. + // The write path through `readModifyWriteStateMd` strips frontmatter before + // invoking the modifier, so callers of `buildStateFrontmatter` only see the + // body. Without reading frontmatter here, status defaults to 'unknown' when + // body has no Status field, and progress is stomped to 0/0 when the current + // milestone's phase directories have been archived. Matches the #2495 READ + // pattern: STATE.md is authoritative, re-derive only when absent. + let existingFm: Record = {}; + try { + const raw = await readFile(planningPaths(projectDir, workstream).state, 'utf-8'); + existingFm = extractFrontmatter(raw); + } catch { /* STATE.md missing on first write — no preservation needed */ } + let milestone: string | null = null; let milestoneName: string | null = null; try { @@ -160,6 +173,12 @@ export async function buildStateFrontmatter(bodyContent: string, projectDir: str normalizedStatus = 'executing'; } + // Bug #2613: status preservation — if body has no Status field and existing + // frontmatter has a non-unknown status, prefer existing. + if (normalizedStatus === 'unknown' && typeof existingFm.status === 'string' && existingFm.status && existingFm.status !== 'unknown') { + normalizedStatus = existingFm.status; + } + const fm: Record = { gsd_state_version: '1.0' }; if (milestone) fm.milestone = milestone; @@ -181,6 +200,20 @@ export async function buildStateFrontmatter(bodyContent: string, projectDir: str if (progressPercent !== null) progress.percent = progressPercent; if (Object.keys(progress).length > 0) fm.progress = progress; + // Bug #2613: progress preservation — when disk scan returns zero counts + // (archived/shipped milestone) and existing frontmatter has non-zero counts, + // prefer existing. Legitimate mid-milestone updates see non-zero disk counts + // and fall through, keeping disk as ground truth. + const existingProgress = existingFm.progress as Record | undefined; + if (existingProgress && typeof existingProgress === 'object') { + const derivedTotalPlans = Number(progress.total_plans ?? 0); + const derivedCompletedPlans = Number(progress.completed_plans ?? 0); + const existingTotalPlans = Number(existingProgress.total_plans ?? 0); + if (derivedTotalPlans === 0 && derivedCompletedPlans === 0 && existingTotalPlans > 0) { + fm.progress = existingProgress; + } + } + return fm; }