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 <noreply@anthropic.com>
This commit is contained in:
5
.changeset/397-state-preserve-executor-authored.md
Normal file
5
.changeset/397-state-preserve-executor-authored.md
Normal file
@@ -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.
|
||||
@@ -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(
|
||||
|
||||
@@ -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 };
|
||||
|
||||
@@ -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, {
|
||||
|
||||
375
tests/bug-397-state-preserve-executor-authored.test.cjs
Normal file
375
tests/bug-397-state-preserve-executor-authored.test.cjs
Normal file
@@ -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 });
|
||||
}
|
||||
});
|
||||
|
||||
});
|
||||
Reference in New Issue
Block a user