From 6cba62b39dc33a2d18eb1a2418c88e03612361e5 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Wed, 27 May 2026 23:03:15 -0400 Subject: [PATCH] fix(#397): preserve executor-authored STATE.md fields (template-default-only replacement) (#422) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduces KNOWN_TEMPLATE_DEFAULTS and KNOWN_STATUS_PATTERNS in state-document.cjs to enumerate every string a GSD handler writes. stateReplaceFieldIfTemplate consults this table and only replaces the field when the existing value is a known template default (or absent) — executor-authored values are left untouched. Wire-in: - record-session: Resume File now only overwritten when caller passes --resume-file OR existing value is 'None'. Router no longer defaults resume_file to 'None' before calling the handler. - advance-plan (both branches): Status and Last Activity guarded via stateReplaceFieldIfTemplate. - updateCurrentPositionFields: Status and Last activity in the Current Position section guarded; bare ISO date shape is the trigger for replacement, prose narrative is preserved. - planned-phase: Status and Last Activity guarded the same way. Regression tests (7 cases) in tests/bug-397-state-preserve-executor-authored.test.cjs cover each data-loss shape; all 106 existing state.test.cjs tests still pass. Co-authored-by: Claude Sonnet 4.6 --- .../397-state-preserve-executor-authored.md | 5 + .../bin/lib/state-command-router.cjs | 5 +- get-shit-done/bin/lib/state-document.cjs | 143 ++++++- get-shit-done/bin/lib/state.cjs | 104 ++++- ...-state-preserve-executor-authored.test.cjs | 375 ++++++++++++++++++ 5 files changed, 610 insertions(+), 22 deletions(-) create mode 100644 .changeset/397-state-preserve-executor-authored.md create mode 100644 tests/bug-397-state-preserve-executor-authored.test.cjs diff --git a/.changeset/397-state-preserve-executor-authored.md b/.changeset/397-state-preserve-executor-authored.md new file mode 100644 index 000000000..87c3695ba --- /dev/null +++ b/.changeset/397-state-preserve-executor-authored.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 397 +--- +State handlers (`record-session`, `advance-plan`, `planned-phase`) now preserve executor-authored STATE.md field values instead of overwriting them with template defaults. Introduces explicit field-ownership via `KNOWN_TEMPLATE_DEFAULTS` and `stateReplaceFieldIfTemplate`. Fixes a data-loss shape where hand-authored Resume File / Status / Last Activity values were silently lost on the next state-handler call. diff --git a/get-shit-done/bin/lib/state-command-router.cjs b/get-shit-done/bin/lib/state-command-router.cjs index c0762b326..ef4b404f9 100644 --- a/get-shit-done/bin/lib/state-command-router.cjs +++ b/get-shit-done/bin/lib/state-command-router.cjs @@ -158,7 +158,10 @@ function routeStateCommand({ state, args, cwd, raw, error }) { null, () => { const { 'stopped-at': stopped_at, 'resume-file': resume_file } = parseNamedArgs(args, ['stopped-at', 'resume-file']); - state.cmdStateRecordSession(cwd, { stopped_at, resume_file: resume_file || 'None' }, raw); + // Pass resume_file as-is (undefined when --resume-file was not provided) so + // cmdStateRecordSession can distinguish "caller explicitly passed a value" from + // "option was not supplied" and apply the template-default-only replacement guard. + state.cmdStateRecordSession(cwd, { stopped_at, resume_file }, raw); }, ), 'begin-phase': cjsFallbackHandler( diff --git a/get-shit-done/bin/lib/state-document.cjs b/get-shit-done/bin/lib/state-document.cjs index eff4b2e9b..bb813a2d7 100644 --- a/get-shit-done/bin/lib/state-document.cjs +++ b/get-shit-done/bin/lib/state-document.cjs @@ -124,4 +124,145 @@ function normalizeProgressNumbers(progress) { return normalized; } -module.exports = { stateExtractField, stateReplaceField, stateReplaceFieldWithFallback, normalizeStateStatus, computeProgressPercent, shouldPreserveExistingProgress, normalizeProgressNumbers }; +/** + * KNOWN_TEMPLATE_DEFAULTS — per-field table of string values that were written + * by a GSD handler (not by an executor / human). A value that appears in this + * list is safe to overwrite on the next handler call. Any other value was + * authored by the executor and must be preserved (Knuth invariant: + * handler-owns-transition-between-known-template-defaults). + * + * Keys must match the canonical field name as it appears in STATE.md. + * Comparison is case-insensitive so "None" and "none" both match. + * + * For Status, exact strings are supplemented by a pattern list + * (KNOWN_STATUS_PATTERNS) that matches handler-generated values whose exact + * text is variable (e.g. "Executing Phase 5"). + */ +const KNOWN_TEMPLATE_DEFAULTS = { + 'Resume File': ['None'], + 'Status': [ + 'Ready to execute', + 'Phase complete — ready for verification', + 'Ready to plan', + 'Defining requirements', + 'Planning complete', + // Legacy / abbreviated handler values present in older STATE.md files + 'Executing', + 'In progress', + 'Planning', + 'Verifying', + 'Completed', + 'Done', + 'Active', + 'Paused', + 'unknown', + ], + // Last Activity is a date field; ISO date-only strings (YYYY-MM-DD) are the + // handler-generated form. We detect them by shape rather than an exhaustive + // list because the date changes every day. + // NOTE: entries here are matched by isStateTemplateDefault using the date regex + // in addition to exact string equality. + 'Last Activity': [], + 'Last activity': [], +}; + +/** + * Regex patterns that match handler-generated Status values whose text includes + * a variable component (e.g. phase number). Checked after the KNOWN_TEMPLATE_DEFAULTS + * exact-match list in isStateTemplateDefault. + */ +const KNOWN_STATUS_PATTERNS = [ + /^Executing Phase\s+\d+/i, + /^Planning Phase\s+\d+/i, + /^Phase\s+\d+\s+complete/i, + /^Verifying Phase\s+\d+/i, + /^Phase complete/i, +]; + +/** + * Returns true when the given value is a known template default for the field, + * meaning a GSD handler wrote it and a subsequent handler may replace it. + * + * A value is considered a template default when: + * (a) it appears in KNOWN_TEMPLATE_DEFAULTS[field] (exact, case-insensitive), OR + * (b) it matches the ISO date-only shape (YYYY-MM-DD) for Last Activity fields + * (handlers always write bare dates; executors write narrative prose). + * + * @param {string} field - Canonical field name (case-sensitive key lookup attempted + * first, then case-insensitive fallback). + * @param {string} value - The current value extracted from STATE.md. + * @returns {boolean} + */ +function isStateTemplateDefault(field, value) { + if (value === null || value === undefined) return true; // absent → initial write + const v = String(value).trim(); + if (v === '') return true; // blank → treat as absent + + // Look up the defaults list, trying exact key first then case-insensitive. + let defaults = KNOWN_TEMPLATE_DEFAULTS[field]; + if (!defaults) { + const fieldLower = field.toLowerCase(); + const matchKey = Object.keys(KNOWN_TEMPLATE_DEFAULTS).find(k => k.toLowerCase() === fieldLower); + defaults = matchKey ? KNOWN_TEMPLATE_DEFAULTS[matchKey] : null; + } + + if (defaults && defaults.some(d => d.toLowerCase() === v.toLowerCase())) { + return true; + } + + const fieldLower = field.toLowerCase(); + + // Status: also check pattern list for variable handler-generated values + // (e.g. "Executing Phase 5", "Planning Phase 3"). + if (fieldLower === 'status') { + if (KNOWN_STATUS_PATTERNS.some(p => p.test(v))) return true; + } + + // Last Activity / Last activity: bare ISO date (YYYY-MM-DD) is handler-generated. + if (fieldLower === 'last activity') { + if (/^\d{4}-\d{2}-\d{2}$/.test(v)) return true; + } + + return false; +} + +/** + * Replaces a field in STATE.md content only when the existing value is a known + * template default (or the field is absent). If the existing value is + * executor-authored, the content is returned unchanged. + * + * When `newValue` is null or undefined the function is a no-op (returns content). + * + * @param {string} content - Full STATE.md text. + * @param {string} field - Field name as it appears in STATE.md. + * @param {string[]} knownDefaults - The defaults list to check against (typically + * KNOWN_TEMPLATE_DEFAULTS[field]). + * @param {string} newValue - Value to write when replacement is permitted. + * @returns {string} - Updated content (or original if skipped). + */ +function stateReplaceFieldIfTemplate(content, field, knownDefaults, newValue) { + if (newValue === null || newValue === undefined) return content; + const existing = stateExtractField(content, field); + // Build a temporary KNOWN_TEMPLATE_DEFAULTS-compatible lookup so we can reuse + // the isStateTemplateDefault logic for the provided knownDefaults array. + const tempField = '__tmp__'; + const tempDefaults = { [tempField]: knownDefaults || [] }; + // Inline check: absent/blank → always write; in list → write; else → skip. + if (existing === null || existing === undefined || existing.trim() === '') { + return stateReplaceField(content, field, newValue) || content; + } + const v = existing.trim(); + const inList = (knownDefaults || []).some(d => d.toLowerCase() === v.toLowerCase()); + const fieldLower = field.toLowerCase(); + // Special-case: Status pattern list for variable handler-generated values. + const matchesStatusPattern = (fieldLower === 'status') && KNOWN_STATUS_PATTERNS.some(p => p.test(v)); + // Special-case: Last Activity bare ISO date (YYYY-MM-DD) is handler-generated. + const isDateShape = (fieldLower === 'last activity') && /^\d{4}-\d{2}-\d{2}$/.test(v); + if (inList || matchesStatusPattern || isDateShape) { + return stateReplaceField(content, field, newValue) || content; + } + // Executor-authored — preserve. + return content; +} + +module.exports = { stateExtractField, stateReplaceField, stateReplaceFieldWithFallback, normalizeStateStatus, computeProgressPercent, shouldPreserveExistingProgress, normalizeProgressNumbers, KNOWN_TEMPLATE_DEFAULTS, KNOWN_STATUS_PATTERNS, isStateTemplateDefault, stateReplaceFieldIfTemplate }; diff --git a/get-shit-done/bin/lib/state.cjs b/get-shit-done/bin/lib/state.cjs index 37b41b7eb..661a89cbe 100644 --- a/get-shit-done/bin/lib/state.cjs +++ b/get-shit-done/bin/lib/state.cjs @@ -16,6 +16,9 @@ const { shouldPreserveExistingProgress, stateExtractField, stateReplaceField, + KNOWN_TEMPLATE_DEFAULTS, + KNOWN_STATUS_PATTERNS, + stateReplaceFieldIfTemplate, } = require('./state-document.cjs'); // Cache disk scan results from buildStateFrontmatter per cwd per process (#1967). @@ -256,12 +259,34 @@ function updateCurrentPositionFields(content, fields) { if (!posMatch) return content; let posBody = posMatch[2]; + const statusDefaults = KNOWN_TEMPLATE_DEFAULTS['Status']; + const lastActivityDefaults = KNOWN_TEMPLATE_DEFAULTS['Last Activity']; if (fields.status && /^Status:/m.test(posBody)) { - posBody = posBody.replace(/^Status:.*$/m, `Status: ${fields.status}`); + // Only replace when the existing Current Position Status is a known template default. + const existingStatusMatch = posBody.match(/^Status:\s*(.+)$/m); + const existingStatus = existingStatusMatch ? existingStatusMatch[1].trim() : null; + const isInList = existingStatus && statusDefaults.some(d => d.toLowerCase() === existingStatus.toLowerCase()); + const matchesPattern = existingStatus && KNOWN_STATUS_PATTERNS.some(p => p.test(existingStatus)); + const isDefault = !existingStatus || isInList || matchesPattern; + if (isDefault) { + posBody = posBody.replace(/^Status:.*$/m, `Status: ${fields.status}`); + } } if (fields.lastActivity && /^Last activity:/im.test(posBody)) { - posBody = posBody.replace(/^Last activity:.*$/im, `Last activity: ${fields.lastActivity}`); + // Only replace when the existing Current Position Last activity is a known template + // default (a bare ISO date). Executor-authored narrative prose is preserved. + const existingActivityMatch = posBody.match(/^Last activity:\s*(.+)$/im); + const existingActivity = existingActivityMatch ? existingActivityMatch[1].trim() : null; + // A bare ISO date (YYYY-MM-DD with nothing after) is handler-generated. + // A date with a narrative suffix (e.g. "2026-02-15 -- blocked by infra...") + // was authored by the executor and must be preserved. + const isDateShape = existingActivity && /^\d{4}-\d{2}-\d{2}$/.test(existingActivity); + const inList = existingActivity && lastActivityDefaults.some(d => d.toLowerCase() === existingActivity.toLowerCase()); + const isDefault = !existingActivity || isDateShape || inList; + if (isDefault) { + posBody = posBody.replace(/^Last activity:.*$/im, `Last activity: ${fields.lastActivity}`); + } } if (fields.plan && /^Plan:/m.test(posBody)) { posBody = posBody.replace(/^Plan:.*$/m, `Plan: ${fields.plan}`); @@ -302,9 +327,16 @@ function cmdStateAdvancePlan(cwd, raw) { return content; } + const statusDefaults = KNOWN_TEMPLATE_DEFAULTS['Status']; + const lastActivityDefaults = KNOWN_TEMPLATE_DEFAULTS['Last Activity']; + if (currentPlan >= totalPlans) { - content = stateReplaceFieldWithFallback(content, 'Status', null, 'Phase complete — ready for verification'); - content = stateReplaceFieldWithFallback(content, 'Last Activity', 'Last activity', today); + // Phase-complete branch — only replace Status/Last Activity when the existing + // value is a known template default (Knuth invariant: preserve executor-authored). + content = stateReplaceFieldIfTemplate(content, 'Status', statusDefaults, 'Phase complete — ready for verification'); + content = stateReplaceFieldIfTemplate(content, 'Last Activity', lastActivityDefaults, today); + // stateReplaceFieldWithFallback tries 'Last activity' alias too + content = stateReplaceFieldIfTemplate(content, 'Last activity', lastActivityDefaults, today); content = updateCurrentPositionFields(content, { status: 'Phase complete — ready for verification', lastActivity: today }); result = { advanced: false, reason: 'last_plan', current_plan: currentPlan, total_plans: totalPlans, status: 'ready_for_verification' }; } else { @@ -318,8 +350,11 @@ function cmdStateAdvancePlan(cwd, raw) { planDisplayValue = `${newPlan} of ${totalPlans}`; content = stateReplaceField(content, 'Current Plan', String(newPlan)) || content; } - content = stateReplaceFieldWithFallback(content, 'Status', null, 'Ready to execute'); - content = stateReplaceFieldWithFallback(content, 'Last Activity', 'Last activity', today); + // Normal advance — only replace Status/Last Activity when the existing value is + // a known template default (Knuth invariant: preserve executor-authored). + content = stateReplaceFieldIfTemplate(content, 'Status', statusDefaults, 'Ready to execute'); + content = stateReplaceFieldIfTemplate(content, 'Last Activity', lastActivityDefaults, today); + content = stateReplaceFieldIfTemplate(content, 'Last activity', lastActivityDefaults, today); content = updateCurrentPositionFields(content, { status: 'Ready to execute', lastActivity: today, plan: planDisplayValue }); result = { advanced: true, previous_plan: currentPlan, current_plan: newPlan, total_plans: totalPlans }; } @@ -610,11 +645,32 @@ function cmdStateRecordSession(cwd, options, raw) { if (result) { content = result; updated.push('Stopped At'); } } - // Update Resume file - const resumeFile = options.resume_file || 'None'; - result = stateReplaceField(content, 'Resume File', resumeFile); - if (!result) result = stateReplaceField(content, 'Resume file', resumeFile); - if (result) { content = result; updated.push('Resume File'); } + // Update Resume File — only when the caller explicitly passed a value OR the + // existing value is a known template default. An executor-authored path must + // not be silently replaced with 'None' just because --resume-file was omitted + // (Knuth invariant: handler-owns-transition-between-known-template-defaults). + const resumeFileDefaults = KNOWN_TEMPLATE_DEFAULTS['Resume File']; + if (options.resume_file !== undefined && options.resume_file !== null) { + // Caller explicitly passed a value — always honour it. + result = stateReplaceField(content, 'Resume File', options.resume_file); + if (!result) result = stateReplaceField(content, 'Resume file', options.resume_file); + if (result) { content = result; updated.push('Resume File'); } + } else { + // No explicit value — only set 'None' when existing value is also a known default + // (i.e. not executor-authored). + const newRf = stateReplaceFieldIfTemplate(content, 'Resume File', resumeFileDefaults, 'None'); + if (newRf !== content) { + content = newRf; + updated.push('Resume File'); + } else { + // Try alternate capitalisation + const newRfAlt = stateReplaceFieldIfTemplate(content, 'Resume file', resumeFileDefaults, 'None'); + if (newRfAlt !== content) { + content = newRfAlt; + updated.push('Resume File'); + } + } + } return content; }, cwd); @@ -1325,23 +1381,31 @@ function cmdStatePlannedPhase(cwd, phaseNumber, planCount, raw) { const today = new Date().toISOString().split('T')[0]; const updated = []; - // Update Status - let result = stateReplaceField(content, 'Status', 'Ready to execute'); - if (result) { content = result; updated.push('Status'); } + const statusDefaults = KNOWN_TEMPLATE_DEFAULTS['Status']; + const lastActivityDefaults = KNOWN_TEMPLATE_DEFAULTS['Last Activity']; + + // Update Status — only when the existing value is a known template default + // (Knuth invariant: preserve executor-authored values). + const newContent = stateReplaceFieldIfTemplate(content, 'Status', statusDefaults, 'Ready to execute'); + if (newContent !== content) { content = newContent; updated.push('Status'); } // Update Total Plans in Phase if (planCount !== null && planCount !== undefined) { - result = stateReplaceField(content, 'Total Plans in Phase', String(planCount)); + let result = stateReplaceField(content, 'Total Plans in Phase', String(planCount)); if (result) { content = result; updated.push('Total Plans in Phase'); } } - // Update Last Activity - result = stateReplaceField(content, 'Last Activity', today); - if (result) { content = result; updated.push('Last Activity'); } + // Update Last Activity — only when the existing value is a known template default + { + const after = stateReplaceFieldIfTemplate(content, 'Last Activity', lastActivityDefaults, today); + if (after !== content) { content = after; updated.push('Last Activity'); } + } // Update Last Activity Description - result = stateReplaceField(content, 'Last Activity Description', `Phase ${phaseNumber} planning complete — ${planCount || '?'} plans ready`); - if (result) { content = result; updated.push('Last Activity Description'); } + { + let result = stateReplaceField(content, 'Last Activity Description', `Phase ${phaseNumber} planning complete — ${planCount || '?'} plans ready`); + if (result) { content = result; updated.push('Last Activity Description'); } + } // Update Current Position section content = updateCurrentPositionFields(content, { diff --git a/tests/bug-397-state-preserve-executor-authored.test.cjs b/tests/bug-397-state-preserve-executor-authored.test.cjs new file mode 100644 index 000000000..98406ab9b --- /dev/null +++ b/tests/bug-397-state-preserve-executor-authored.test.cjs @@ -0,0 +1,375 @@ +'use strict'; +// allow-test-rule: reads runtime STATE.md written to temp dir — behavioral output test, not source-grep + +// Regression tests for bug #397. +// +// STATE.md fields edited by an executor (e.g. a hand-authored Resume File path, +// a custom Status value, or a custom Last Activity entry) were silently overwritten +// by the next call to record-session, advance-plan, or planned-phase because the +// handlers used unconditional stateReplaceField / stateReplaceFieldWithFallback +// calls, even when the option was not passed by the caller. +// +// Fix: introduce KNOWN_TEMPLATE_DEFAULTS (a per-field table of string values that +// are safe to replace because they came from a template) and +// stateReplaceFieldIfTemplate (a helper that only replaces when the current value +// is a template default or absent). Handlers must consult this table rather than +// writing unconditionally. +// +// The 7 baseline cases verified here: +// +// 1. record-session WITHOUT --resume-file when Resume File is executor-authored +// → preserved (must NOT be replaced with 'None') +// 2. record-session WITHOUT --resume-file when Resume File is 'None' +// → remains 'None' (template-default → template-default is fine) +// 3. record-session WITH --resume-file → explicit caller value wins (always) +// 4. advance-plan phase-complete when Status is executor-authored → preserved +// 5. advance-plan phase-complete when Status is a known default → replaced +// 6. advance-plan advance when Last Activity is executor-authored → preserved +// 7. updateCurrentPositionFields with executor-authored Current Position values +// → preserved + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); + +const ROOT = path.join(__dirname, '..'); +const TOOLS_PATH = path.join(ROOT, 'get-shit-done', 'bin', 'gsd-tools.cjs'); + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +function makeTempPlanning(stateContent) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-397-')); + const planningDir = path.join(dir, '.planning'); + fs.mkdirSync(planningDir, { recursive: true }); + fs.writeFileSync(path.join(planningDir, 'STATE.md'), stateContent, 'utf8'); + return dir; +} + +function readState(dir) { + return fs.readFileSync(path.join(dir, '.planning', 'STATE.md'), 'utf8'); +} + +function runGsdState(args, cwd) { + const { execFileSync } = require('child_process'); + const env = { + ...process.env, + GSD_SESSION_KEY: '', + CODEX_THREAD_ID: '', + CLAUDE_SESSION_ID: '', + CLAUDE_CODE_SSE_PORT: '', + OPENCODE_SESSION_ID: '', + }; + try { + execFileSync(process.execPath, [TOOLS_PATH, 'state', ...args], { + cwd, + encoding: 'utf-8', + stdio: ['pipe', 'pipe', 'pipe'], + env, + }); + return { success: true }; + } catch (err) { + return { success: false, error: err.stderr?.toString().trim() || err.message }; + } +} + +// --------------------------------------------------------------------------- +// Fixtures +// --------------------------------------------------------------------------- + +// Case 1: Resume File is executor-authored (not a template default) +const STATE_EXECUTOR_RESUME_FILE = `# GSD State + +## Configuration +Current Phase: 2 +Total Plans in Phase: 4 +Current Plan: 1 +Status: Ready to execute + +## Current Position +Phase: 2 +Plan: 1 of 4 +Status: Ready to execute +Last activity: 2026-01-01 + +## Session Continuity +Last session: 2026-01-01T00:00:00.000Z +Last Date: 2026-01-01T00:00:00.000Z +Resume File: /home/user/my-custom-context.md +Stopped At: Phase 2 Plan 1 complete +`; + +// Case 2: Resume File is 'None' (the known template default) +const STATE_DEFAULT_RESUME_FILE = `# GSD State + +## Configuration +Current Phase: 2 +Total Plans in Phase: 4 +Current Plan: 1 +Status: Ready to execute + +## Current Position +Phase: 2 +Plan: 1 of 4 +Status: Ready to execute +Last activity: 2026-01-01 + +## Session Continuity +Last session: 2026-01-01T00:00:00.000Z +Last Date: 2026-01-01T00:00:00.000Z +Resume File: None +Stopped At: Phase 2 Plan 1 complete +`; + +// Cases 4 & 7: Status is executor-authored in both Configuration and Current Position. +// Current Plan=2, Total Plans=2 → triggers the phase-complete branch of advance-plan. +const STATE_EXECUTOR_STATUS = `# GSD State + +## Configuration +Current Phase: 3 +Total Plans in Phase: 2 +Current Plan: 2 +Status: Awaiting QA sign-off before proceeding + +## Current Position +Phase: 3 +Plan: 2 of 2 +Status: Awaiting QA sign-off before proceeding +Last activity: 2026-01-01 + +## Session Continuity +Last session: 2026-01-01T00:00:00.000Z +Last Date: 2026-01-01T00:00:00.000Z +Resume File: None +`; + +// Case 5: Status IS a known template default ('Ready to execute'). +// Current Plan=2, Total Plans=2 → triggers the phase-complete branch. +const STATE_DEFAULT_STATUS = `# GSD State + +## Configuration +Current Phase: 3 +Total Plans in Phase: 2 +Current Plan: 2 +Status: Ready to execute + +## Current Position +Phase: 3 +Plan: 2 of 2 +Status: Ready to execute +Last activity: 2026-01-01 + +## Session Continuity +Last session: 2026-01-01T00:00:00.000Z +Last Date: 2026-01-01T00:00:00.000Z +Resume File: None +`; + +// Case 6: The only Last Activity field in the document is executor-authored +// (a narrative, not a bare ISO date). Current Plan=1, Total=3 → advance branch. +const STATE_EXECUTOR_LAST_ACTIVITY = `# GSD State + +## Configuration +Current Phase: 2 +Total Plans in Phase: 3 +Current Plan: 1 +Status: Ready to execute +Last Activity: Unblocked after infra fix — merged PR #88 manually + +## Current Position +Phase: 2 +Plan: 1 of 3 +Status: Ready to execute +Last activity: 2026-01-01 + +## Session Continuity +Last session: 2026-01-01T00:00:00.000Z +Last Date: 2026-01-01T00:00:00.000Z +Resume File: None +`; + +// Case 7: Current Position has executor-authored Status and Last activity. +// Current Plan=2, Total=3 → advance branch. +const STATE_EXECUTOR_CURRENT_POSITION = `# GSD State + +## Configuration +Current Phase: 4 +Total Plans in Phase: 3 +Current Plan: 2 +Status: Ready to execute + +## Current Position +Phase: 4 +Plan: 2 of 3 +Status: On hold — waiting for upstream dependency merge +Last activity: 2026-02-15 -- blocked by infra; resume after merge + +## Session Continuity +Last session: 2026-01-01T00:00:00.000Z +Last Date: 2026-01-01T00:00:00.000Z +Resume File: None +`; + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe('bug #397: executor-authored STATE.md fields must be preserved', () => { + + // Case 1: record-session without --resume-file, Resume File is executor-authored + test('case 1: record-session without --resume-file preserves executor-authored Resume File', () => { + const dir = makeTempPlanning(STATE_EXECUTOR_RESUME_FILE); + try { + const r = runGsdState(['record-session', '--stopped-at', 'Plan 1 complete'], dir); + assert.ok(r.success, `record-session failed: ${r.error}`); + const after = readState(dir); + const rfMatch = after.match(/Resume File:\s*(.+)/i); + assert.ok(rfMatch, 'Resume File field not found in STATE.md after record-session'); + assert.strictEqual( + rfMatch[1].trim(), + '/home/user/my-custom-context.md', + `record-session overwrote executor-authored Resume File with '${rfMatch[1].trim()}'`, + ); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + // Case 2: record-session without --resume-file, Resume File is 'None' (template default) + test('case 2: record-session without --resume-file keeps "None" when it is already "None"', () => { + const dir = makeTempPlanning(STATE_DEFAULT_RESUME_FILE); + try { + const r = runGsdState(['record-session', '--stopped-at', 'Plan complete'], dir); + assert.ok(r.success, `record-session failed: ${r.error}`); + const after = readState(dir); + const rfMatch = after.match(/Resume File:\s*(.+)/i); + assert.ok(rfMatch, 'Resume File field not found in STATE.md after record-session'); + assert.strictEqual( + rfMatch[1].trim(), + 'None', + `Expected 'None' to remain when it was already 'None', got: ${rfMatch[1].trim()}`, + ); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + // Case 3: record-session WITH --resume-file — explicit value always wins + test('case 3: record-session with --resume-file sets the explicit value', () => { + const dir = makeTempPlanning(STATE_EXECUTOR_RESUME_FILE); + try { + const r = runGsdState(['record-session', '--resume-file', '/tmp/new-resume.md'], dir); + assert.ok(r.success, `record-session failed: ${r.error}`); + const after = readState(dir); + const rfMatch = after.match(/Resume File:\s*(.+)/i); + assert.ok(rfMatch, 'Resume File field not found in STATE.md after record-session'); + assert.strictEqual( + rfMatch[1].trim(), + '/tmp/new-resume.md', + `Expected explicit --resume-file value to be written, got: ${rfMatch[1].trim()}`, + ); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + // Case 4: advance-plan phase-complete, Status is executor-authored → preserved + test('case 4: advance-plan (phase-complete) preserves executor-authored Status', () => { + const dir = makeTempPlanning(STATE_EXECUTOR_STATUS); + try { + // Current Plan=2, Total=2 → phase-complete branch + const r = runGsdState(['advance-plan'], dir); + assert.ok(r.success, `advance-plan failed: ${r.error}`); + const after = readState(dir); + // The Configuration-level Status must not be clobbered + const statusMatch = after.match(/^Status:\s*(.+)/m); + assert.ok(statusMatch, 'Status field not found after advance-plan'); + assert.strictEqual( + statusMatch[1].trim(), + 'Awaiting QA sign-off before proceeding', + `advance-plan overwrote executor-authored Status: got '${statusMatch[1].trim()}'`, + ); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + // Case 5: advance-plan phase-complete, Status is a known default → replaced + test('case 5: advance-plan (phase-complete) replaces known-default Status with phase-complete value', () => { + const dir = makeTempPlanning(STATE_DEFAULT_STATUS); + try { + // Current Plan=2, Total=2 → phase-complete branch + const r = runGsdState(['advance-plan'], dir); + assert.ok(r.success, `advance-plan failed: ${r.error}`); + const after = readState(dir); + const statusMatch = after.match(/^Status:\s*(.+)/m); + assert.ok(statusMatch, 'Status field not found after advance-plan'); + // 'Ready to execute' is a known default and should be replaced + assert.notStrictEqual( + statusMatch[1].trim(), + 'Ready to execute', + `Status should have been updated from 'Ready to execute' after phase-complete, but was not`, + ); + assert.ok( + statusMatch[1].includes('Phase complete') || statusMatch[1].includes('ready for verification'), + `Expected phase-complete Status text, got: '${statusMatch[1].trim()}'`, + ); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + // Case 6: advance-plan normal advance, top-level Last Activity is executor-authored → preserved + test('case 6: advance-plan (normal advance) preserves executor-authored Last Activity', () => { + const dir = makeTempPlanning(STATE_EXECUTOR_LAST_ACTIVITY); + try { + // Current Plan=1, Total=3 → advance branch + const r = runGsdState(['advance-plan'], dir); + assert.ok(r.success, `advance-plan failed: ${r.error}`); + const after = readState(dir); + // The top-level Last Activity (in Configuration section) must be preserved + const laMatch = after.match(/^Last Activity:\s*(.+)/im); + assert.ok(laMatch, 'Last Activity field not found after advance-plan'); + assert.strictEqual( + laMatch[1].trim(), + 'Unblocked after infra fix — merged PR #88 manually', + `advance-plan overwrote executor-authored Last Activity: got '${laMatch[1].trim()}'`, + ); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + // Case 7: advance-plan preserves executor-authored Status and Last activity in Current Position + test('case 7: advance-plan preserves executor-authored Current Position Status and Last activity', () => { + const dir = makeTempPlanning(STATE_EXECUTOR_CURRENT_POSITION); + try { + // Current Plan=2, Total=3 → advance branch + const r = runGsdState(['advance-plan'], dir); + assert.ok(r.success, `advance-plan failed: ${r.error}`); + const after = readState(dir); + const posMatch = after.match(/##\s*Current Position\s*\n([\s\S]*?)(?=\n##|$)/i); + assert.ok(posMatch, 'Current Position section not found after advance-plan'); + const posBody = posMatch[1]; + const posStatusMatch = posBody.match(/^Status:\s*(.+)/m); + assert.ok(posStatusMatch, 'Status field not found in Current Position section'); + assert.strictEqual( + posStatusMatch[1].trim(), + 'On hold — waiting for upstream dependency merge', + `advance-plan overwrote executor-authored Current Position Status: got '${posStatusMatch[1].trim()}'`, + ); + const posActivityMatch = posBody.match(/^Last activity:\s*(.+)/im); + assert.ok(posActivityMatch, 'Last activity field not found in Current Position section'); + assert.ok( + posActivityMatch[1].includes('blocked by infra'), + `advance-plan overwrote executor-authored Current Position Last activity: got '${posActivityMatch[1].trim()}'`, + ); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + +});