* test(#948): add regression tests for no-op write guard and record-session auto-create (#944) Red before fix: 11/15 tests fail. Green after: 15/15. Covers zero-match patch byte-identity, milestone_name preservation, stopped_at frontmatter-wins, record-session auto-create fallback, and adversarial fixtures (CRLF, empty body, non-canonical labels). Also registers bug-948-state-noop-write-guard.test.cjs in the state bucket of lint-test-file-count.allowlist.json. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#948): guard STATE.md no-op writes; preserve milestone_name/stopped_at (closes #944) Shared root cause: `readModifyWriteStateMd` wrote STATE.md unconditionally even when the transform produced no change, and `syncStateFrontmatter` re-derived frontmatter from the possibly-stale body on every write. Three coordinated fixes in src/state.cts: 1. readModifyWriteStateMd: add no-op guard — when transform result === input content, skip the write entirely (no platformWriteSync, no last_updated bump, no frontmatter re-derive). Fixes #948 zero-match phantom write and the #944 phantom last_updated bump. 2. syncStateFrontmatter: extend existing-frontmatter preserve logic — fall back to existingFm['milestone_name'] / existingFm['milestone'] when the derived value is the template placeholder 'milestone' (getMilestoneInfo returns this literal when it cannot match the version in ROADMAP.md); prefer existingFm['stopped_at'] / existingFm['paused_at'] over a body-derived value (the frontmatter value, written by the canonical record-session path, wins over stale historical body lines). Mirrors the fallback already in cmdStateJson. 3. cmdStateRecordSession: when --stopped-at / --resume-file are supplied but body labels are absent, DWIM auto-create a canonical ## Session section (mirroring how add-decision / add-blocker / record-metric auto-create their sections). Never return a silent recorded:false when the caller supplied values. SDK check: no sdk/src/state.ts exists in this repo (the comment in cmdStateSnapshot references a sibling concern in the TypeScript SDK codebase, which is a separate repo not present here). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore: add changeset for PR #952 (fix #948/#944) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#948): correct stopped_at preserve rule; adjust test for sync behaviour The "always prefer frontmatter stopped_at" rule in syncStateFrontmatter was too aggressive — it broke phase.complete which intentionally updates stopped_at in the body and expects syncStateFrontmatter to pick it up. The primary fix (no-op guard in readModifyWriteStateMd) already prevents the stale-body-overwrites-frontmatter scenario from #948: the file is not written when the transform produces no change, so syncStateFrontmatter never runs on a zero-match patch. The body-derived value can only win when an actual write occurs, which means the body was legitimately updated. Reverted to the original #905 rule for stopped_at/paused_at: fall back to existing frontmatter only when the derived value is absent (empty/null). Also adjusted the sync-suite test to assert what state sync actually does (milestone_name preservation) rather than a stopped_at-wins property that state sync does not have by design. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#944): update existing session block in place (adversarial review) HIGH finding: the DWIM auto-create in cmdStateRecordSession was appending a second ## Session block unconditionally, even when one already existed with non-canonical content (e.g. a markdown table). Both buildStateFrontmatter and cmdStateSnapshot read only the FIRST ## Session block via regex, so the newly-written Stopped at / Resume file values landed in the second, invisible block — frontmatter stopped_at stayed stale and state-snapshot returned nulls. Fix: check for an existing ## Session heading. When one is present, normalize that section in place by replacing its body with canonical **Last session:** / **Stopped at:** / **Resume file:** bold-label lines. Only append a brand-new section when NO ## Session heading exists. LOW finding: the auto-create scaffold emits **Last session:** but cmdStateSnapshot only matched **Last Date:**, so session.last_date was null after auto-create despite a valid timestamp being written. Fix: extend the lastDateMatch regex in cmdStateSnapshot to also accept **Last session:** / Last session: (the form the scaffold writes). Tests: 3 new tests added to bug-948-state-noop-write-guard.test.cjs that confirmed failure against the previous HEAD and pass after this fix: - exactly one ## Session block after record-session with non-canonical existing block - state-snapshot sees correct stopped_at via first Session block (not a duplicate) - state-snapshot session.last_date is non-null after auto-create on body-less file Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#944): improve in-place section replace to cleanly remove old body content The previous regex `/(^## Session[ \t]*$)([\s\S]*?)(?=\n^## |\n*$)/im` with a lazy match consumed nothing after the heading, so old non-canonical body content (e.g. table rows) remained after the new canonical lines. While functionally correct (parsers found the canonical lines first in the FIRST ## Session block), it left stale content in the section. Replace with a negative-lookahead per-line pattern that consumes all content from the heading up to (but not including) the next ## heading, producing a clean section with only the canonical bold-label lines. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
5
.changeset/clever-yaks-sprint.md
Normal file
5
.changeset/clever-yaks-sprint.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 952
|
||||
---
|
||||
**`state patch` and `state record-session` no longer corrupt STATE.md** — a no-match patch no longer rewrites the file (was resetting `milestone_name` and resurrecting a stale `stopped_at`), and `record-session` now persists `--stopped-at`/`--resume-file` even when the body lacks the exact labels.
|
||||
@@ -98,6 +98,7 @@
|
||||
"bug-3454-state-dollar-backreference-growth.test.cjs",
|
||||
"bug-397-state-preserve-executor-authored.test.cjs",
|
||||
"bug-905-state-syncstatefrontmatter-preserve-scalars.test.cjs",
|
||||
"bug-948-state-noop-write-guard.test.cjs",
|
||||
"state-acquirestatelock-non-eexist.test.cjs",
|
||||
"state-prune.test.cjs",
|
||||
"state.test.cjs"
|
||||
|
||||
125
src/state.cts
125
src/state.cts
@@ -713,6 +713,7 @@ function cmdStateRecordSession(cwd: string, options: StateRecordSessionOptions,
|
||||
|
||||
const now = realClock.nowIso();
|
||||
const updated: string[] = [];
|
||||
let sessionCreated = false;
|
||||
|
||||
readModifyWriteStateMd(statePath, (content) => {
|
||||
// Update Last session / Last Date
|
||||
@@ -755,11 +756,85 @@ function cmdStateRecordSession(cwd: string, options: StateRecordSessionOptions,
|
||||
}
|
||||
}
|
||||
|
||||
// Bug #944: DWIM normalize/auto-create — when the caller supplied --stopped-at or
|
||||
// --resume-file but the body lacks the canonical labels (in-place replace
|
||||
// returned a miss), persist the values durably. Mirrors the DWIM pattern used
|
||||
// by add-decision, add-blocker, and record-metric. Never silently drop
|
||||
// caller-supplied values.
|
||||
//
|
||||
// Guard: only act when the caller actually supplied a value. When no
|
||||
// --stopped-at / --resume-file are given and the body already had no session
|
||||
// labels (nothing was updated), we return recorded:false — the existing
|
||||
// behaviour for a no-op call that didn't supply any values.
|
||||
//
|
||||
// Correctness invariant: both buildStateFrontmatter and cmdStateSnapshot read
|
||||
// only the FIRST `## Session` block (via a /##\s*Session\s*\n…/i regex).
|
||||
// If we blindly append a second `## Session` block when one already exists, the
|
||||
// newly-written Stopped at / Resume file end up in the second (invisible) block.
|
||||
// Fix: when a `## Session` heading already exists, normalize THAT block in place
|
||||
// (insert / replace canonical bold-label lines within the existing section).
|
||||
// Only append a brand-new section when NO `## Session` heading exists at all.
|
||||
const callerSuppliedValues = !!(options.stopped_at || (options.resume_file !== undefined && options.resume_file !== null));
|
||||
const needsStoppedAt = options.stopped_at && !updated.includes('Stopped At');
|
||||
const needsResumeFile = options.resume_file !== undefined && options.resume_file !== null && !updated.includes('Resume File');
|
||||
const needsLastSession = !updated.includes('Last session') && !updated.includes('Last Date');
|
||||
|
||||
if (callerSuppliedValues && (needsStoppedAt || needsResumeFile || needsLastSession)) {
|
||||
const resumeValue = (options.resume_file !== undefined && options.resume_file !== null)
|
||||
? options.resume_file
|
||||
: 'None';
|
||||
const stoppedAtValue = options.stopped_at || 'None';
|
||||
|
||||
// Determine whether a ## Session heading already exists in the body.
|
||||
const existingSessionHeading = /^## Session\s*$/im.test(content);
|
||||
|
||||
if (existingSessionHeading) {
|
||||
// Normalize in place: replace the ENTIRE BODY of the existing ## Session
|
||||
// section (heading + all content up to the next ## heading or EOF) with
|
||||
// canonical bold-label lines. The negative-lookahead per-line pattern
|
||||
// `(?!^## )[\s\S]` consumes every line that doesn't start with "## ",
|
||||
// which correctly stops at the next section boundary without consuming it.
|
||||
// A trailing blank line is added so the next ## heading keeps its spacing.
|
||||
content = content.replace(
|
||||
/^(## Session[ \t]*\n(?:(?!^## )[\s\S])*)/m,
|
||||
[
|
||||
'## Session',
|
||||
'',
|
||||
`**Last session:** ${now}`,
|
||||
`**Stopped at:** ${stoppedAtValue}`,
|
||||
`**Resume file:** ${resumeValue}`,
|
||||
'',
|
||||
'',
|
||||
].join('\n'),
|
||||
);
|
||||
} else {
|
||||
// No ## Session heading exists at all — append a new canonical section.
|
||||
const scaffold = [
|
||||
'',
|
||||
'## Session',
|
||||
'',
|
||||
`**Last session:** ${now}`,
|
||||
`**Stopped at:** ${stoppedAtValue}`,
|
||||
`**Resume file:** ${resumeValue}`,
|
||||
'',
|
||||
].join('\n');
|
||||
content = content.trimEnd() + '\n' + scaffold;
|
||||
}
|
||||
|
||||
sessionCreated = true;
|
||||
|
||||
if (needsLastSession) updated.push('Last session');
|
||||
if (needsStoppedAt) updated.push('Stopped At');
|
||||
if (needsResumeFile) updated.push('Resume File');
|
||||
}
|
||||
|
||||
return content;
|
||||
}, cwd);
|
||||
|
||||
if (updated.length > 0) {
|
||||
output({ recorded: true, updated }, raw, 'true');
|
||||
const result: Record<string, unknown> = { recorded: true, updated };
|
||||
if (sessionCreated) result['created'] = true;
|
||||
output(result, raw, 'true');
|
||||
} else {
|
||||
output({ recorded: false, reason: 'No session fields found in STATE.md' }, raw, 'false');
|
||||
}
|
||||
@@ -850,8 +925,12 @@ function cmdStateSnapshot(cwd: string, raw: boolean): void {
|
||||
const sessionMatch = body.match(/##\s*Session\s*\n([\s\S]*?)(?=\n##|$)/i);
|
||||
if (sessionMatch) {
|
||||
const sessionSection = sessionMatch[1];
|
||||
// Accept both `**Last Date:**` (canonical template form) and `**Last session:**`
|
||||
// (the form written by the DWIM auto-create / normalize path added for #944).
|
||||
const lastDateMatch = sessionSection.match(/\*\*Last Date:\*\*\s*(.+)/i)
|
||||
|| sessionSection.match(/^Last Date:\s*(.+)/im);
|
||||
|| sessionSection.match(/^Last Date:\s*(.+)/im)
|
||||
|| sessionSection.match(/\*\*Last session:\*\*\s*(.+)/i)
|
||||
|| sessionSection.match(/^Last session:\s*(.+)/im);
|
||||
const stoppedAtMatch = sessionSection.match(/\*\*Stopped At:\*\*\s*(.+)/i)
|
||||
|| sessionSection.match(/^Stopped At:\s*(.+)/im);
|
||||
const resumeFileMatch = sessionSection.match(/\*\*Resume File:\*\*\s*(.+)/i)
|
||||
@@ -1073,12 +1152,42 @@ function syncStateFrontmatter(content: string, cwd: string | undefined): string
|
||||
derivedFm['status'] = existingFm['status'];
|
||||
}
|
||||
|
||||
// Bug #948: preserve `milestone_name` / `milestone` when the derived value
|
||||
// is the template placeholder 'milestone'. getMilestoneInfo returns the
|
||||
// literal string 'milestone' when it cannot match the version from the roadmap
|
||||
// (e.g. no ROADMAP.md, roadmap lacks the heading for the stored version, or the
|
||||
// milestone version read from STATE.md itself triggers the lookup before the
|
||||
// file is fully written). A placeholder must never overwrite a real name that the
|
||||
// existing frontmatter already holds; only an empty derived value falls through
|
||||
// to this guard (the primary #905 preserve path below handles that).
|
||||
const MILESTONE_NAME_PLACEHOLDER = 'milestone';
|
||||
if (
|
||||
derivedFm['milestone_name'] === MILESTONE_NAME_PLACEHOLDER &&
|
||||
existingFm['milestone_name'] &&
|
||||
existingFm['milestone_name'] !== MILESTONE_NAME_PLACEHOLDER
|
||||
) {
|
||||
derivedFm['milestone_name'] = existingFm['milestone_name'];
|
||||
// Keep the stored milestone version consistent with the preserved name.
|
||||
if (existingFm['milestone']) {
|
||||
derivedFm['milestone'] = existingFm['milestone'];
|
||||
}
|
||||
}
|
||||
|
||||
// Bug #905: preserve scalar fields that buildStateFrontmatter can only derive
|
||||
// from body annotations (Current Phase:, Current Plan:, etc.). When those
|
||||
// annotations are absent — e.g. after an agent or tool rewrites the body —
|
||||
// buildStateFrontmatter returns no value for those keys. Mirror the same
|
||||
// fallback pattern used in cmdStateJson so the existing frontmatter values
|
||||
// survive every writeStateMd call.
|
||||
//
|
||||
// For stopped_at / paused_at: the original #905 "fall back when derived is
|
||||
// absent" rule is preserved here. The stale-body-overwrites-frontmatter
|
||||
// scenario from #948 is prevented by the no-op guard in
|
||||
// readModifyWriteStateMd: when the transform produces no change the file is
|
||||
// never written, so syncStateFrontmatter never even runs. Attempting to
|
||||
// "always prefer frontmatter" here breaks legitimate callers like phase.complete
|
||||
// that intentionally write a new stopped_at value to the body and expect
|
||||
// syncStateFrontmatter to pick it up.
|
||||
if (!derivedFm['stopped_at'] && existingFm['stopped_at']) {
|
||||
derivedFm['stopped_at'] = existingFm['stopped_at'];
|
||||
}
|
||||
@@ -1247,6 +1356,18 @@ function readModifyWriteStateMd(statePath: string, transformFn: (content: string
|
||||
// restore it when resync is false.
|
||||
const preFm = resync ? null : extractFrontmatter(content) as Record<string, unknown>;
|
||||
const modified = transformFn(content);
|
||||
|
||||
// Bug #948: no-op guard — if the transform produced no change, do NOT write
|
||||
// the file. An unconditional write would bump `last_updated`, reset
|
||||
// `milestone_name` to the template placeholder, and resurrect stale
|
||||
// body-derived `stopped_at` values via syncStateFrontmatter. Skipping the
|
||||
// write when content is unchanged is safe because every caller that mutates
|
||||
// content already returns the mutated string, and callers that detect a
|
||||
// no-op explicitly return the original content unchanged.
|
||||
if (modified === content) {
|
||||
return;
|
||||
}
|
||||
|
||||
let synced = syncStateFrontmatter(modified, cwd);
|
||||
|
||||
if (!resync && preFm && preFm['progress']) {
|
||||
|
||||
698
tests/bug-948-state-noop-write-guard.test.cjs
Normal file
698
tests/bug-948-state-noop-write-guard.test.cjs
Normal file
@@ -0,0 +1,698 @@
|
||||
'use strict';
|
||||
/**
|
||||
* Regression guard for bugs #948 and #944.
|
||||
*
|
||||
* #948 (data loss): a `state patch` whose fields all fail to match still
|
||||
* rewrites STATE.md — bumping `last_updated`, resetting `milestone_name` to
|
||||
* the template placeholder, and resurrecting a stale `stopped_at` from an
|
||||
* old body `## Session` block (body-derived value overwrites a newer
|
||||
* frontmatter value written by `record-session`).
|
||||
*
|
||||
* #944: `state record-session --stopped-at X --resume-file Y` silently
|
||||
* drops the supplied values when the STATE.md body lacks the exact session
|
||||
* labels the in-place replace expects, returning `{"recorded": false}` at
|
||||
* exit 0 and only bumping `last_updated`.
|
||||
*
|
||||
* Shared root cause: `readModifyWriteStateMd` always writes STATE.md even
|
||||
* when the transform produced no change, and `syncStateFrontmatter`
|
||||
* re-derives frontmatter (including milestone_name / stopped_at) from the
|
||||
* possibly-stale body on every write.
|
||||
*
|
||||
* Fixes:
|
||||
* 1. No-op guard in `readModifyWriteStateMd`: when transform output ===
|
||||
* input, skip the write entirely.
|
||||
* 2. `syncStateFrontmatter` preserves existing `milestone_name` / `milestone`
|
||||
* when the derived value is the template placeholder `'milestone'`.
|
||||
* 3. `syncStateFrontmatter` prefers existing frontmatter `stopped_at` /
|
||||
* `paused_at` over a body-derived value (frontmatter wins).
|
||||
* 4. `cmdStateRecordSession` auto-creates a canonical `## Session` section
|
||||
* when `--stopped-at` / `--resume-file` are supplied but no labels exist.
|
||||
*/
|
||||
|
||||
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, parseFrontmatter } = require('./helpers.cjs');
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// Fixture builders
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* STATE.md with:
|
||||
* - real `milestone_name` in frontmatter (e.g. "My Real Milestone")
|
||||
* - newer frontmatter `stopped_at` (written by a prior `record-session`)
|
||||
* - stale `## Session` body section with an OLDER "Stopped at" line
|
||||
*
|
||||
* When a zero-match `state patch` runs on this file, NONE of these values
|
||||
* should be disturbed — the file must be byte-identical afterward.
|
||||
*/
|
||||
function buildStateMdWithStaleSectionAndRealFrontmatter(opts) {
|
||||
const {
|
||||
milestoneName = 'My Real Milestone',
|
||||
fmStoppedAt = 'Phase 3, Plan 2 — newer value',
|
||||
bodyStoppedAt = 'Phase 1, Plan 1 — stale historical value',
|
||||
lastUpdated = '2026-01-01T00:00:00.000Z',
|
||||
} = opts || {};
|
||||
|
||||
return [
|
||||
'---',
|
||||
'gsd_state_version: 1.0',
|
||||
'milestone: v2.0',
|
||||
`milestone_name: ${milestoneName}`,
|
||||
'status: executing',
|
||||
`stopped_at: ${fmStoppedAt}`,
|
||||
`last_updated: ${lastUpdated}`,
|
||||
'progress:',
|
||||
' total_phases: 5',
|
||||
' completed_phases: 2',
|
||||
' total_plans: 10',
|
||||
' completed_plans: 4',
|
||||
' percent: 40',
|
||||
'---',
|
||||
'',
|
||||
'# GSD State',
|
||||
'',
|
||||
'## Current Position',
|
||||
'',
|
||||
'Status: Executing Phase 3',
|
||||
'Last Activity: 2026-01-01',
|
||||
'',
|
||||
'## Session',
|
||||
'',
|
||||
`**Last session:** 2026-01-01T00:00:00.000Z`,
|
||||
`**Stopped at:** ${bodyStoppedAt}`,
|
||||
'**Resume file:** None',
|
||||
'',
|
||||
'## Accumulated Context',
|
||||
'',
|
||||
'### Decisions',
|
||||
'',
|
||||
'- [Phase 1]: Use Node 22',
|
||||
'',
|
||||
].join('\n');
|
||||
}
|
||||
|
||||
/**
|
||||
* STATE.md with NO session section at all — no "## Session" heading,
|
||||
* no Stopped at / Resume file labels. This is the #944 scenario.
|
||||
*/
|
||||
function buildStateMdWithoutSessionSection() {
|
||||
return [
|
||||
'---',
|
||||
'gsd_state_version: 1.0',
|
||||
'milestone: v1.0',
|
||||
'milestone_name: Foundation',
|
||||
'status: executing',
|
||||
'last_updated: 2026-01-01T00:00:00.000Z',
|
||||
'---',
|
||||
'',
|
||||
'# GSD State',
|
||||
'',
|
||||
'## Current Position',
|
||||
'',
|
||||
'Status: Executing Phase 1',
|
||||
'Last Activity: 2026-01-01',
|
||||
'',
|
||||
'## Accumulated Context',
|
||||
'',
|
||||
'### Decisions',
|
||||
'',
|
||||
'- [Phase 1]: Use TypeScript',
|
||||
'',
|
||||
].join('\n');
|
||||
}
|
||||
|
||||
/**
|
||||
* STATE.md with a canonical session section (the success path — must not regress).
|
||||
*/
|
||||
function buildStateMdWithCanonicalSessionSection() {
|
||||
return [
|
||||
'---',
|
||||
'gsd_state_version: 1.0',
|
||||
'milestone: v1.0',
|
||||
'milestone_name: Foundation',
|
||||
'status: executing',
|
||||
'last_updated: 2026-01-01T00:00:00.000Z',
|
||||
'---',
|
||||
'',
|
||||
'# GSD State',
|
||||
'',
|
||||
'## Session',
|
||||
'',
|
||||
'**Last session:** 2026-01-01T00:00:00.000Z',
|
||||
'**Stopped at:** Phase 1, Plan 1',
|
||||
'**Resume file:** None',
|
||||
'',
|
||||
'## Accumulated Context',
|
||||
'',
|
||||
'### Decisions',
|
||||
'',
|
||||
'- Use TypeScript',
|
||||
'',
|
||||
].join('\n');
|
||||
}
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// Bug #948: zero-match patch must leave STATE.md byte-identical
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
describe('#948: zero-match state patch must not rewrite STATE.md', () => {
|
||||
let tmpDir;
|
||||
|
||||
beforeEach(() => {
|
||||
tmpDir = createTempProject();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
test('STATE.md is byte-identical after a zero-match patch', () => {
|
||||
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
|
||||
const original = buildStateMdWithStaleSectionAndRealFrontmatter({});
|
||||
fs.writeFileSync(statePath, original);
|
||||
|
||||
// Patch a field that does NOT exist in the file — zero matches expected.
|
||||
const result = runGsdTools('state patch --NonExistentFieldXYZ "some value"', tmpDir);
|
||||
assert.ok(result.success, `state patch should exit 0: ${result.error}`);
|
||||
|
||||
const patchOutput = JSON.parse(result.output);
|
||||
assert.deepStrictEqual(patchOutput.updated, [], 'updated should be empty');
|
||||
assert.ok(Array.isArray(patchOutput.failed), 'failed should be an array');
|
||||
|
||||
const after = fs.readFileSync(statePath, 'utf-8');
|
||||
assert.strictEqual(after, original, 'STATE.md must be byte-identical after zero-match patch');
|
||||
});
|
||||
|
||||
test('milestone_name is preserved after zero-match patch (not reset to template placeholder)', () => {
|
||||
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
|
||||
const original = buildStateMdWithStaleSectionAndRealFrontmatter({
|
||||
milestoneName: 'My Real Milestone',
|
||||
});
|
||||
fs.writeFileSync(statePath, original);
|
||||
|
||||
runGsdTools('state patch --NonExistentField "value"', tmpDir);
|
||||
|
||||
const after = fs.readFileSync(statePath, 'utf-8');
|
||||
const fm = parseFrontmatter(after);
|
||||
assert.strictEqual(fm['milestone_name'], 'My Real Milestone',
|
||||
'milestone_name must not be reset to template placeholder by zero-match patch');
|
||||
});
|
||||
|
||||
test('stopped_at frontmatter value is preserved after zero-match patch (via byte-identity)', () => {
|
||||
// The no-op guard prevents ANY rewrite when nothing changed, so the
|
||||
// frontmatter stopped_at is preserved because the file is never touched.
|
||||
// The stale body value cannot win because syncStateFrontmatter is never called.
|
||||
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
|
||||
const original = buildStateMdWithStaleSectionAndRealFrontmatter({
|
||||
fmStoppedAt: 'Phase 3, Plan 2 — newer value',
|
||||
bodyStoppedAt: 'Phase 1, Plan 1 — stale historical value',
|
||||
});
|
||||
fs.writeFileSync(statePath, original);
|
||||
|
||||
runGsdTools('state patch --NonExistentField "value"', tmpDir);
|
||||
|
||||
// The byte-identity test already covers this; this test confirms the key
|
||||
// field specifically is intact.
|
||||
const after = fs.readFileSync(statePath, 'utf-8');
|
||||
assert.strictEqual(after, original,
|
||||
'STATE.md must be byte-identical — stopped_at cannot be overwritten via a no-op patch');
|
||||
});
|
||||
|
||||
test('last_updated is not bumped by a zero-match patch', () => {
|
||||
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
|
||||
const original = buildStateMdWithStaleSectionAndRealFrontmatter({
|
||||
lastUpdated: '2026-01-01T00:00:00.000Z',
|
||||
});
|
||||
fs.writeFileSync(statePath, original);
|
||||
|
||||
runGsdTools('state patch --NonExistentField "value"', tmpDir);
|
||||
|
||||
const after = fs.readFileSync(statePath, 'utf-8');
|
||||
const fm = parseFrontmatter(after);
|
||||
assert.strictEqual(fm['last_updated'], '2026-01-01T00:00:00.000Z',
|
||||
'last_updated must not be bumped when no fields were changed');
|
||||
});
|
||||
|
||||
test('a matching patch STILL updates STATE.md correctly (no regression)', () => {
|
||||
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
|
||||
const fixture = [
|
||||
'---',
|
||||
'gsd_state_version: 1.0',
|
||||
'milestone: v1.0',
|
||||
'milestone_name: Foundation',
|
||||
'status: executing',
|
||||
'last_updated: 2026-01-01T00:00:00.000Z',
|
||||
'---',
|
||||
'',
|
||||
'# GSD State',
|
||||
'',
|
||||
'**Status:** In Progress',
|
||||
'**Last Activity:** 2026-01-01',
|
||||
'',
|
||||
].join('\n');
|
||||
fs.writeFileSync(statePath, fixture);
|
||||
|
||||
const result = runGsdTools('state patch --Status "Phase complete — ready for verification"', tmpDir);
|
||||
assert.ok(result.success, `state patch should succeed: ${result.error}`);
|
||||
|
||||
const patchOutput = JSON.parse(result.output);
|
||||
assert.ok(patchOutput.updated.includes('Status'), 'Status should be in updated list');
|
||||
|
||||
const after = fs.readFileSync(statePath, 'utf-8');
|
||||
assert.ok(after.includes('Phase complete — ready for verification'),
|
||||
'matching patch should update the field');
|
||||
});
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// Bug #948: syncStateFrontmatter — milestone_name placeholder preservation
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
describe('#948: syncStateFrontmatter preserves milestone_name when derived is template placeholder', () => {
|
||||
let tmpDir;
|
||||
|
||||
beforeEach(() => {
|
||||
tmpDir = createTempProject();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
test('state sync preserves real milestone_name when disk yields only template placeholder', () => {
|
||||
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
|
||||
// Frontmatter has a real name, but no ROADMAP.md exists so getMilestoneInfo
|
||||
// will fall back to the 'milestone' placeholder — must not overwrite.
|
||||
const content = [
|
||||
'---',
|
||||
'gsd_state_version: 1.0',
|
||||
'milestone: v2.5',
|
||||
'milestone_name: Very Real Project Name',
|
||||
'status: executing',
|
||||
'---',
|
||||
'',
|
||||
'# GSD State',
|
||||
'',
|
||||
'Status: Executing Phase 1',
|
||||
'Last Activity: 2026-01-01',
|
||||
'',
|
||||
].join('\n');
|
||||
fs.writeFileSync(statePath, content);
|
||||
|
||||
const result = runGsdTools('state sync', tmpDir);
|
||||
assert.ok(result.success, `state sync failed: ${result.error}`);
|
||||
|
||||
const after = fs.readFileSync(statePath, 'utf-8');
|
||||
const fm = parseFrontmatter(after);
|
||||
assert.strictEqual(fm['milestone_name'], 'Very Real Project Name',
|
||||
'milestone_name must not be reset to template placeholder by state sync');
|
||||
});
|
||||
|
||||
test('state sync runs successfully and preserves milestone_name (no corruption)', () => {
|
||||
// state sync always rebuilds frontmatter from the body — the no-op guard
|
||||
// applies to commands whose transform produces no change. state sync always
|
||||
// writes because last_updated changes. This test verifies that a full sync
|
||||
// cycle does not corrupt milestone_name when the placeholder is derived.
|
||||
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
|
||||
const content = [
|
||||
'---',
|
||||
'gsd_state_version: 1.0',
|
||||
'milestone: v2.5',
|
||||
'milestone_name: Very Real Project Name',
|
||||
'status: executing',
|
||||
'---',
|
||||
'',
|
||||
'# GSD State',
|
||||
'',
|
||||
'Status: Executing Phase 1',
|
||||
'Last Activity: 2026-01-01',
|
||||
'',
|
||||
].join('\n');
|
||||
fs.writeFileSync(statePath, content);
|
||||
|
||||
const result = runGsdTools('state sync', tmpDir);
|
||||
assert.ok(result.success, `state sync failed: ${result.error}`);
|
||||
|
||||
const after = fs.readFileSync(statePath, 'utf-8');
|
||||
const fm = parseFrontmatter(after);
|
||||
assert.strictEqual(fm['milestone_name'], 'Very Real Project Name',
|
||||
'state sync must not reset milestone_name to template placeholder');
|
||||
});
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// Bug #944: record-session with no session section must persist supplied values
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
describe('#944: record-session persists values even when body lacks session labels', () => {
|
||||
let tmpDir;
|
||||
|
||||
beforeEach(() => {
|
||||
tmpDir = createTempProject();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
test('stopped-at and resume-file are present in STATE.md after record-session with no prior section', () => {
|
||||
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
|
||||
fs.writeFileSync(statePath, buildStateMdWithoutSessionSection());
|
||||
|
||||
const PINNED_MS = Date.parse('2026-06-09T12:00:00.000Z');
|
||||
const result = runGsdTools(
|
||||
'state record-session --stopped-at "Phase 2, Plan 3" --resume-file ".planning/phases/02/02-03-PLAN.md"',
|
||||
tmpDir,
|
||||
{ GSD_TEST_MODE: '1', GSD_NOW_MS: String(PINNED_MS) },
|
||||
);
|
||||
assert.ok(result.success, `state record-session should exit 0: ${result.error}`);
|
||||
|
||||
const output = JSON.parse(result.output);
|
||||
assert.strictEqual(output.recorded, true,
|
||||
'recorded must be true when values were supplied and persisted');
|
||||
assert.ok(!output.reason || output.reason !== 'No session fields found in STATE.md',
|
||||
'must not return the silent no-op reason when values were supplied');
|
||||
|
||||
const after = fs.readFileSync(statePath, 'utf-8');
|
||||
assert.ok(after.includes('Phase 2, Plan 3'),
|
||||
'--stopped-at value must appear in STATE.md');
|
||||
assert.ok(after.includes('.planning/phases/02/02-03-PLAN.md'),
|
||||
'--resume-file value must appear in STATE.md');
|
||||
});
|
||||
|
||||
test('command does not silently no-op when values are supplied (recorded must not be false)', () => {
|
||||
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
|
||||
fs.writeFileSync(statePath, buildStateMdWithoutSessionSection());
|
||||
|
||||
const result = runGsdTools(
|
||||
'state record-session --stopped-at "Phase 5, Plan 1"',
|
||||
tmpDir,
|
||||
);
|
||||
assert.ok(result.success, `should exit 0: ${result.error}`);
|
||||
|
||||
const output = JSON.parse(result.output);
|
||||
// The key contract: if values were supplied, recorded must be true.
|
||||
assert.notStrictEqual(output.recorded, false,
|
||||
'recorded must not be false when --stopped-at was explicitly supplied');
|
||||
});
|
||||
|
||||
test('STATE.md with non-canonical session labels still persists supplied values', () => {
|
||||
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
|
||||
// Session section exists but uses non-canonical label shapes (table, alternate caps)
|
||||
const nonCanonical = [
|
||||
'---',
|
||||
'gsd_state_version: 1.0',
|
||||
'milestone: v1.0',
|
||||
'milestone_name: Foundation',
|
||||
'status: executing',
|
||||
'last_updated: 2026-01-01T00:00:00.000Z',
|
||||
'---',
|
||||
'',
|
||||
'# GSD State',
|
||||
'',
|
||||
'## Session Info',
|
||||
'',
|
||||
'| Field | Value |',
|
||||
'|-------|-------|',
|
||||
'| Last Session | 2026-01-01 |',
|
||||
'| Stopped Here | Phase 1, Plan 1 |',
|
||||
'',
|
||||
].join('\n');
|
||||
fs.writeFileSync(statePath, nonCanonical);
|
||||
|
||||
const PINNED_MS = Date.parse('2026-06-09T15:00:00.000Z');
|
||||
const result = runGsdTools(
|
||||
'state record-session --stopped-at "Phase 3, Plan 2" --resume-file "none.md"',
|
||||
tmpDir,
|
||||
{ GSD_TEST_MODE: '1', GSD_NOW_MS: String(PINNED_MS) },
|
||||
);
|
||||
assert.ok(result.success, `should exit 0: ${result.error}`);
|
||||
|
||||
const output = JSON.parse(result.output);
|
||||
assert.strictEqual(output.recorded, true,
|
||||
'recorded must be true when values are persisted via auto-create fallback');
|
||||
|
||||
const after = fs.readFileSync(statePath, 'utf-8');
|
||||
assert.ok(after.includes('Phase 3, Plan 2'),
|
||||
'--stopped-at value must be present in STATE.md');
|
||||
assert.ok(after.includes('none.md'),
|
||||
'--resume-file value must be present in STATE.md');
|
||||
});
|
||||
|
||||
test('record-session with no args against a body-less file returns recorded:false (no regression)', () => {
|
||||
// When NO values are supplied and no session fields can be found/updated,
|
||||
// recorded:false is the correct behaviour — we only changed the contract
|
||||
// when the caller supplies values.
|
||||
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
|
||||
fs.writeFileSync(statePath, buildStateMdWithoutSessionSection());
|
||||
|
||||
const result = runGsdTools('state record-session', tmpDir);
|
||||
assert.ok(result.success, `should exit 0: ${result.error}`);
|
||||
|
||||
const output = JSON.parse(result.output);
|
||||
assert.strictEqual(output.recorded, false,
|
||||
'recorded should still be false when no session fields exist AND no values were supplied');
|
||||
});
|
||||
|
||||
test('canonical session section still updates in place (no regression)', () => {
|
||||
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
|
||||
fs.writeFileSync(statePath, buildStateMdWithCanonicalSessionSection());
|
||||
|
||||
const PINNED_MS = Date.parse('2026-06-09T18:00:00.000Z');
|
||||
const result = runGsdTools(
|
||||
'state record-session --stopped-at "Phase 2, Plan 4" --resume-file ".planning/phases/02/02-04-PLAN.md"',
|
||||
tmpDir,
|
||||
{ GSD_TEST_MODE: '1', GSD_NOW_MS: String(PINNED_MS) },
|
||||
);
|
||||
assert.ok(result.success, `should exit 0: ${result.error}`);
|
||||
|
||||
const output = JSON.parse(result.output);
|
||||
assert.strictEqual(output.recorded, true, 'recorded should be true');
|
||||
|
||||
const after = fs.readFileSync(statePath, 'utf-8');
|
||||
assert.ok(after.includes('Phase 2, Plan 4'), 'stopped-at should be updated');
|
||||
assert.ok(after.includes('.planning/phases/02/02-04-PLAN.md'), 'resume-file should be updated');
|
||||
});
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// Adversarial fixtures: malformed frontmatter, missing fields, CRLF
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
describe('#948/#944: adversarial fixture variants', () => {
|
||||
let tmpDir;
|
||||
|
||||
beforeEach(() => {
|
||||
tmpDir = createTempProject();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
test('zero-match patch on CRLF STATE.md leaves file unchanged', () => {
|
||||
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
|
||||
// Build with CRLF line endings
|
||||
const original = buildStateMdWithStaleSectionAndRealFrontmatter({}).replace(/\n/g, '\r\n');
|
||||
fs.writeFileSync(statePath, original);
|
||||
|
||||
runGsdTools('state patch --NonExistentFieldXYZ "value"', tmpDir);
|
||||
|
||||
const after = fs.readFileSync(statePath, 'utf-8');
|
||||
assert.strictEqual(after, original, 'CRLF file must be byte-identical after zero-match patch');
|
||||
});
|
||||
|
||||
test('zero-match patch on STATE.md with missing frontmatter fields does not corrupt', () => {
|
||||
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
|
||||
const minimal = [
|
||||
'---',
|
||||
'gsd_state_version: 1.0',
|
||||
'---',
|
||||
'',
|
||||
'# GSD State',
|
||||
'',
|
||||
'**Status:** In Progress',
|
||||
'',
|
||||
].join('\n');
|
||||
fs.writeFileSync(statePath, minimal);
|
||||
|
||||
const result = runGsdTools('state patch --NonExistentField "value"', tmpDir);
|
||||
assert.ok(result.success, `should exit 0: ${result.error}`);
|
||||
|
||||
const patchOutput = JSON.parse(result.output);
|
||||
assert.deepStrictEqual(patchOutput.updated, [], 'no fields should be updated');
|
||||
});
|
||||
|
||||
test('record-session with empty body still records when values supplied', () => {
|
||||
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
|
||||
// Body is entirely empty (only frontmatter)
|
||||
const emptyBody = [
|
||||
'---',
|
||||
'gsd_state_version: 1.0',
|
||||
'status: planning',
|
||||
'---',
|
||||
'',
|
||||
].join('\n');
|
||||
fs.writeFileSync(statePath, emptyBody);
|
||||
|
||||
const result = runGsdTools(
|
||||
'state record-session --stopped-at "Phase 1, Plan 1"',
|
||||
tmpDir,
|
||||
);
|
||||
assert.ok(result.success, `should exit 0: ${result.error}`);
|
||||
|
||||
const output = JSON.parse(result.output);
|
||||
assert.strictEqual(output.recorded, true,
|
||||
'should persist even into a body-less STATE.md');
|
||||
|
||||
const after = fs.readFileSync(statePath, 'utf-8');
|
||||
assert.ok(after.includes('Phase 1, Plan 1'),
|
||||
'--stopped-at value must appear in STATE.md');
|
||||
});
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// Adversarial review findings: in-place update for existing ## Session heading
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
describe('#944 adversarial: existing ## Session heading must be updated in place, not duplicated', () => {
|
||||
let tmpDir;
|
||||
|
||||
beforeEach(() => {
|
||||
tmpDir = createTempProject();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
/**
|
||||
* HIGH finding: when a `## Session` heading already exists but uses
|
||||
* non-canonical rows (e.g. a markdown table), the DWIM code was appending
|
||||
* a second `## Session` block instead of normalizing the existing one.
|
||||
* buildStateFrontmatter / cmdStateSnapshot both read only the FIRST match,
|
||||
* so the newly-written Stopped at / Resume file end up in an ignored block.
|
||||
*/
|
||||
test('record-session with existing non-canonical ## Session block: exactly one ## Session block afterward', () => {
|
||||
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
|
||||
const nonCanonicalWithHeading = [
|
||||
'---',
|
||||
'gsd_state_version: 1.0',
|
||||
'milestone: v1.0',
|
||||
'milestone_name: Foundation',
|
||||
'status: executing',
|
||||
'last_updated: 2026-01-01T00:00:00.000Z',
|
||||
'---',
|
||||
'',
|
||||
'# GSD State',
|
||||
'',
|
||||
'## Session',
|
||||
'',
|
||||
'| Field | Value |',
|
||||
'|-------|-------|',
|
||||
'| Last Session | 2026-01-01 |',
|
||||
'| Stopped Here | Phase 1, Plan 1 |',
|
||||
'',
|
||||
'## Accumulated Context',
|
||||
'',
|
||||
'- Decision: use TypeScript',
|
||||
'',
|
||||
].join('\n');
|
||||
fs.writeFileSync(statePath, nonCanonicalWithHeading);
|
||||
|
||||
const PINNED_MS = Date.parse('2026-06-09T20:00:00.000Z');
|
||||
const result = runGsdTools(
|
||||
'state record-session --stopped-at "Phase 4, Plan 2" --resume-file "resume.md"',
|
||||
tmpDir,
|
||||
{ GSD_TEST_MODE: '1', GSD_NOW_MS: String(PINNED_MS) },
|
||||
);
|
||||
assert.ok(result.success, `record-session should exit 0: ${result.error}`);
|
||||
|
||||
const after = fs.readFileSync(statePath, 'utf-8');
|
||||
|
||||
// (a) exactly ONE ## Session block — no duplicate
|
||||
const sessionHeadingCount = (after.match(/^## Session\s*$/gm) || []).length;
|
||||
assert.strictEqual(sessionHeadingCount, 1,
|
||||
'exactly ONE ## Session block must exist after record-session (no duplicate appended)');
|
||||
|
||||
// (b) supplied values are present in the file
|
||||
assert.ok(after.includes('Phase 4, Plan 2'),
|
||||
'--stopped-at value must be present in STATE.md');
|
||||
assert.ok(after.includes('resume.md'),
|
||||
'--resume-file value must be present in STATE.md');
|
||||
});
|
||||
|
||||
test('record-session with existing non-canonical ## Session block: state-snapshot sees supplied stopped_at', () => {
|
||||
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
|
||||
const nonCanonicalWithHeading = [
|
||||
'---',
|
||||
'gsd_state_version: 1.0',
|
||||
'milestone: v1.0',
|
||||
'milestone_name: Foundation',
|
||||
'status: executing',
|
||||
'last_updated: 2026-01-01T00:00:00.000Z',
|
||||
'---',
|
||||
'',
|
||||
'# GSD State',
|
||||
'',
|
||||
'## Session',
|
||||
'',
|
||||
'| Field | Value |',
|
||||
'|-------|-------|',
|
||||
'| Last Session | 2026-01-01 |',
|
||||
'| Stopped Here | Phase 1, Plan 1 |',
|
||||
'',
|
||||
].join('\n');
|
||||
fs.writeFileSync(statePath, nonCanonicalWithHeading);
|
||||
|
||||
const PINNED_MS = Date.parse('2026-06-09T20:30:00.000Z');
|
||||
runGsdTools(
|
||||
'state record-session --stopped-at "Phase 4, Plan 2" --resume-file "resume.md"',
|
||||
tmpDir,
|
||||
{ GSD_TEST_MODE: '1', GSD_NOW_MS: String(PINNED_MS) },
|
||||
);
|
||||
|
||||
// (c) state-snapshot must see the written stopped_at in the session block
|
||||
// (via buildStateFrontmatter frontmatter OR body Session section, first match)
|
||||
const snapshotResult = runGsdTools('state-snapshot', tmpDir);
|
||||
assert.ok(snapshotResult.success, `state-snapshot should exit 0: ${snapshotResult.error}`);
|
||||
const snapshot = JSON.parse(snapshotResult.output);
|
||||
assert.strictEqual(
|
||||
snapshot.session && snapshot.session.stopped_at,
|
||||
'Phase 4, Plan 2',
|
||||
`state-snapshot session.stopped_at must reflect "Phase 4, Plan 2", got: ${JSON.stringify(snapshot.session)}`,
|
||||
);
|
||||
});
|
||||
|
||||
/**
|
||||
* LOW finding: auto-created scaffold writes `**Last session:**` but
|
||||
* cmdStateSnapshot only matched `**Last Date:**`, so session.last_date
|
||||
* was null after auto-create despite a valid timestamp being written.
|
||||
* Fix: teach the snapshot parser to also accept `**Last session:**`.
|
||||
*/
|
||||
test('state-snapshot returns non-null session.last_date after auto-create on body-less file', () => {
|
||||
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
|
||||
fs.writeFileSync(statePath, buildStateMdWithoutSessionSection());
|
||||
|
||||
const PINNED_MS = Date.parse('2026-06-09T21:00:00.000Z');
|
||||
const recResult = runGsdTools(
|
||||
'state record-session --stopped-at "Phase 1, Plan 1"',
|
||||
tmpDir,
|
||||
{ GSD_TEST_MODE: '1', GSD_NOW_MS: String(PINNED_MS) },
|
||||
);
|
||||
assert.ok(recResult.success, `record-session should exit 0: ${recResult.error}`);
|
||||
|
||||
const snapshotResult = runGsdTools('state-snapshot', tmpDir);
|
||||
assert.ok(snapshotResult.success, `state-snapshot should exit 0: ${snapshotResult.error}`);
|
||||
const snapshot = JSON.parse(snapshotResult.output);
|
||||
assert.notStrictEqual(
|
||||
snapshot.session && snapshot.session.last_date,
|
||||
null,
|
||||
`state-snapshot session.last_date must not be null after auto-create; got: ${JSON.stringify(snapshot.session)}`,
|
||||
);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user