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

fix(#464): make phase completion planning writes transactional (#465)

Browse Source 
* fix(#464): make phase completion planning writes transactional

* fix(#464): add phase completion changeset

* test(#464): avoid source-grep rollback assertion

* fix(#464): address phase completion rollback review

* fix(#191): remove retired sdk tsconfig reference
This commit is contained in:
Colin Johnson
2026-05-30 17:44:36 -04:00
committed by GitHub
parent 79002a00cb
commit 2bc295b32d
9 changed files with 495 additions and 255 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
.changeset/jolly-wolves-hop.md Normal file
View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 465
---
**`phase complete` no longer leaves roadmap and state inconsistent after a failed state publish** — phase completion now publishes planning files from one locked transaction and rolls back earlier writes when a later write fails.

4
.gitignore vendored
View File

@@ -89,3 +89,7 @@ claude-test-command.md
.stryker-tmp/
.stryker-incremental.json
reports/mutation/
# Local Crabbox machine configuration
.crabbox.yaml
.crabbox.local.yaml

535
get-shit-done/bin/lib/phase.cjs
View File

@@ -18,7 +18,7 @@ const { escapeRegex, loadConfig, normalizePhaseName, phaseMarkdownRegexSource, c
const { platformWriteSync, platformReadSync, platformEnsureDir } = require('./shell-command-projection.cjs');
const { planningDir, withPlanningLock } = require('./planning-workspace.cjs');
const { extractFrontmatter } = require('./frontmatter.cjs');
const { writeStateMd, readModifyWriteStateMd, stateExtractField, stateReplaceField, stateReplaceFieldWithFallback, updatePerformanceMetricsSection } = require('./state.cjs');
const { readModifyWriteStateMd, stateExtractField, stateReplaceField, stateReplaceFieldWithFallback, syncStateFrontmatter, withStateLock, updatePerformanceMetricsSection } = require('./state.cjs');
const { formatGsdSlash, resolveRuntime } = require('./runtime-slash.cjs');
// Pure-computation helpers for cmdPhaseComplete (issue #4 fix).
const { deriveProgressFromRoadmap, clampPercent } = require('./phase-lifecycle.cjs');
@@ -1207,6 +1207,31 @@ function cmdPhaseRemove(cwd, targetPhase, options, raw) {
}, raw);
}
function writePlanningFileSet(writes) {
const applied = [];
try {
for (const write of writes) {
if (write.before === write.after) continue;
platformWriteSync(write.filePath, write.after);
applied.push(write);
}
} catch (err) {
for (const write of applied.reverse()) {
try {
platformWriteSync(write.filePath, write.before);
} catch (rollbackErr) {
err.rollbackError = rollbackErr;
err.message += `\nWARNING: rollback failed while restoring ${write.filePath} ` +
`(${rollbackErr.message}). Planning files under .planning/ may be left in an ` +
`inconsistent, partially rolled back state. Inspect ROADMAP.md / REQUIREMENTS.md / ` +
`STATE.md before re-running phase complete.`;
break;
}
}
throw err;
}
}
function cmdPhaseComplete(cwd, phaseNum, raw) {
if (!phaseNum) {
error('phase number required for phase complete');
@@ -1249,279 +1274,289 @@ function cmdPhaseComplete(cwd, phaseNum, raw) {
}
} catch {}
// Update ROADMAP.md and REQUIREMENTS.md atomically under lock
if (fs.existsSync(roadmapPath)) {
withPlanningLock(cwd, () => {
let roadmapContent = fs.readFileSync(roadmapPath, 'utf-8');
// Checkbox: - [ ] Phase N: → - [x] Phase N: (...completed DATE)
// #3537: padding-tolerant fragment so the caller-resolved padded id
// matches un-padded ROADMAP prose.
const phaseEscaped = phaseMarkdownRegexSource(phaseNum);
const checkboxPattern = new RegExp(
`(-\\s*\\[)[ ](\\]\\s*.*Phase\\s+${phaseEscaped}[:\\s][^\\n]*)`,
'i'
);
roadmapContent = roadmapContent.replace(checkboxPattern, `$1x$2 (completed ${today})`);
// Progress table: update Status to Complete, add date (handles 4 or 5 column tables)
const tableRowPattern = new RegExp(
`^(\\|\\s*${phaseEscaped}\\.?\\s[^|]*(?:\\|[^\\n]*))$`,
'im'
);
roadmapContent = roadmapContent.replace(tableRowPattern, (fullRow) => {
const cells = fullRow.split('|').slice(1, -1);
if (cells.length === 5) {
// 5-col: Phase | Milestone | Plans | Status | Completed
cells[2] = ` ${summaryCount}/${planCount} `;
cells[3] = ' Complete ';
cells[4] = ` ${today} `;
} else if (cells.length === 4) {
// 4-col: Phase | Plans | Status | Completed
cells[1] = ` ${summaryCount}/${planCount} `;
cells[2] = ' Complete ';
cells[3] = ` ${today} `;
}
return '|' + cells.join('|') + '|';
});
// Update plan count in phase section.
// Use direct .replace() rather than replaceInCurrentMilestone() so this
// works when the current milestone section is itself inside a <details>
// block (the standard /gsd:new-project layout). replaceInCurrentMilestone
// scopes to content after the last </details>, which misses content inside
// the current milestone's own <details> wrapper (#2005).
// The phase-scoped heading pattern is specific enough to avoid matching
// archived phases (which belong to different milestones).
const planCountPattern = new RegExp(
`(#{2,4}\\s*Phase\\s+${phaseEscaped}[\\s\\S]*?\\*\\*Plans:\\*\\*\\s*)[^\\n]+`,
'i'
);
roadmapContent = roadmapContent.replace(
planCountPattern,
`$1${summaryCount}/${planCount} plans complete`
);
// Mark completed plan checkboxes (safety net for missed per-plan updates)
// Handles both plain IDs ("- [ ] 01-01-PLAN.md") and bold-wrapped IDs ("- [ ] **01-01**")
for (const summaryFile of phaseInfo.summaries) {
const planId = summaryFile.replace('-SUMMARY.md', '').replace('SUMMARY.md', '');
if (!planId) continue;
const planEscaped = escapeRegex(planId);
const planCheckboxPattern = new RegExp(
`(-\\s*\\[) (\\]\\s*(?:\\*\\*)?${planEscaped}(?:\\*\\*)?)`,
'i'
);
roadmapContent = roadmapContent.replace(planCheckboxPattern, '$1x$2');
}
platformWriteSync(roadmapPath, roadmapContent);
// Update REQUIREMENTS.md traceability for this phase's requirements
const reqPath = path.join(planningDir(cwd), 'REQUIREMENTS.md');
if (fs.existsSync(reqPath)) {
// Extract the current phase section from roadmap (scoped to avoid cross-phase matching).
// #3537: padding-tolerant fragment so an un-padded `Phase 2.7:` heading
// is found when caller resolved to padded `02.7`.
const phaseEsc = phaseMarkdownRegexSource(phaseNum);
const currentMilestoneRoadmap = extractCurrentMilestone(roadmapContent, cwd);
const phaseSectionMatch = currentMilestoneRoadmap.match(
new RegExp(`(#{2,4}\\s*Phase\\s+${phaseEsc}[:\\s][\\s\\S]*?)(?=#{2,4}\\s*Phase\\s+|$)`, 'i')
);
const sectionText = phaseSectionMatch ? phaseSectionMatch[1] : '';
// Accept all bold/colon variants (#2769) — the previous pattern only
// matched **Requirements:** (colon inside bold) and silently skipped
// **Requirements**: (colon outside), preventing the matching REQ-IDs
// from being ticked off in REQUIREMENTS.md on phase completion.
const reqMatch = sectionText.match(/\*\*Requirements:?\*\*[^\S\n]*:?[^\S\n]*([^\n]+)/i);
let reqContent = fs.readFileSync(reqPath, 'utf-8');
if (reqMatch) {
const reqIds = reqMatch[1].replace(/[\[\]]/g, '').split(/[,\s]+/).map(r => r.trim()).filter(Boolean);
for (const reqId of reqIds) {
const reqEscaped = escapeRegex(reqId);
// Update checkbox: - [ ] **REQ-ID** → - [x] **REQ-ID**
reqContent = reqContent.replace(
new RegExp(`(-\\s*\\[)[ ](\\]\\s*\\*\\*${reqEscaped}\\*\\*)`, 'gi'),
'$1x$2'
);
// Update traceability table: | REQ-ID | Phase N | Pending/In Progress | → | REQ-ID | Phase N | Complete |
reqContent = reqContent.replace(
new RegExp(`(\\|\\s*${reqEscaped}\\s*\\|[^|]+\\|)\\s*(?:Pending|In Progress)\\s*(\\|)`, 'gi'),
'$1 Complete $2'
);
}
}
// Scan body for all **REQ-ID** patterns, warn about any missing from the Traceability table.
// Always runs regardless of whether the roadmap has a Requirements: line.
const bodyReqIds = [];
const bodyReqPattern = /\*\*([A-Z][A-Z0-9]*-\d+)\*\*/g;
let bodyMatch;
while ((bodyMatch = bodyReqPattern.exec(reqContent)) !== null) {
const id = bodyMatch[1];
if (!bodyReqIds.includes(id)) bodyReqIds.push(id);
}
// Collect REQ-IDs present in the Traceability section only, to avoid
// picking up IDs from other tables in the document.
const traceabilityHeadingMatch = reqContent.match(/^#{1,6}\s+Traceability\b/im);
const traceabilitySection = traceabilityHeadingMatch
? reqContent.slice(traceabilityHeadingMatch.index)
: '';
const tableReqIds = new Set();
const tableRowPattern = /^\|\s*([A-Z][A-Z0-9]*-\d+)\s*\|/gm;
let tableMatch;
while ((tableMatch = tableRowPattern.exec(traceabilitySection)) !== null) {
tableReqIds.add(tableMatch[1]);
}
const unregistered = bodyReqIds.filter(id => !tableReqIds.has(id));
if (unregistered.length > 0) {
warnings.push(
`REQUIREMENTS.md: ${unregistered.length} REQ-ID(s) found in body but missing from Traceability table: ${unregistered.join(', ')} — add them manually to keep traceability in sync`
);
}
platformWriteSync(reqPath, reqContent);
requirementsUpdated = true;
}
});
}
// Find next phase — check both filesystem AND roadmap
// Phases may be defined in ROADMAP.md but not yet scaffolded to disk,
// so a filesystem-only scan would incorrectly report is_last_phase:true
let nextPhaseNum = null;
let nextPhaseName = null;
let isLastPhase = true;
try {
const isDirInMilestone = getMilestonePhaseFilter(cwd);
const entries = fs.readdirSync(phasesDir, { withFileTypes: true });
const dirs = entries.filter(e => e.isDirectory()).map(e => e.name)
.filter(isDirInMilestone)
.sort((a, b) => comparePhaseNum(a, b));
// Update ROADMAP.md, REQUIREMENTS.md, and STATE.md from one locked snapshot.
// A previous split-lock sequence could publish ROADMAP/REQUIREMENTS and then
// fail before STATE advanced, leaving planning files disagreeing about the
// current phase.
withPlanningLock(cwd, () => {
const runPhaseCompleteTransaction = () => {
const writes = [];
let roadmapContent = null;
// Find the next phase directory after current
// Skip backlog phases (999.x) — they are parked ideas, not sequential work (#2129)
for (const dir of dirs) {
const dm = dir.match(/^(\d+[A-Z]?(?:\.\d+)*)-?(.*)/i);
if (dm) {
if (/^999(?:\.|$)/.test(dm[1])) continue;
if (comparePhaseNum(dm[1], phaseNum) > 0) {
nextPhaseNum = dm[1];
nextPhaseName = dm[2] || null;
isLastPhase = false;
break;
if (fs.existsSync(roadmapPath)) {
const originalRoadmapContent = fs.readFileSync(roadmapPath, 'utf-8');
roadmapContent = originalRoadmapContent;
// Checkbox: - [ ] Phase N: → - [x] Phase N: (...completed DATE)
// #3537: padding-tolerant fragment so the caller-resolved padded id
// matches un-padded ROADMAP prose.
const phaseEscaped = phaseMarkdownRegexSource(phaseNum);
const checkboxPattern = new RegExp(
`(-\\s*\\[)[ ](\\]\\s*.*Phase\\s+${phaseEscaped}[:\\s][^\\n]*)`,
'i'
);
roadmapContent = roadmapContent.replace(checkboxPattern, `$1x$2 (completed ${today})`);
// Progress table: update Status to Complete, add date (handles 4 or 5 column tables)
const tableRowPattern = new RegExp(
`^(\\|\\s*${phaseEscaped}\\.?\\s[^|]*(?:\\|[^\\n]*))$`,
'im'
);
roadmapContent = roadmapContent.replace(tableRowPattern, (fullRow) => {
const cells = fullRow.split('|').slice(1, -1);
if (cells.length === 5) {
// 5-col: Phase | Milestone | Plans | Status | Completed
cells[2] = ` ${summaryCount}/${planCount} `;
cells[3] = ' Complete ';
cells[4] = ` ${today} `;
} else if (cells.length === 4) {
// 4-col: Phase | Plans | Status | Completed
cells[1] = ` ${summaryCount}/${planCount} `;
cells[2] = ' Complete ';
cells[3] = ` ${today} `;
}
return '|' + cells.join('|') + '|';
});
// Update plan count in phase section.
// Use direct .replace() rather than replaceInCurrentMilestone() so this
// works when the current milestone section is itself inside a <details>
// block (the standard /gsd:new-project layout). replaceInCurrentMilestone
// scopes to content after the last </details>, which misses content inside
// the current milestone's own <details> wrapper (#2005).
// The phase-scoped heading pattern is specific enough to avoid matching
// archived phases (which belong to different milestones).
const planCountPattern = new RegExp(
`(#{2,4}\\s*Phase\\s+${phaseEscaped}[\\s\\S]*?\\*\\*Plans:\\*\\*\\s*)[^\\n]+`,
'i'
);
roadmapContent = roadmapContent.replace(
planCountPattern,
`$1${summaryCount}/${planCount} plans complete`
);
// Mark completed plan checkboxes (safety net for missed per-plan updates)
// Handles both plain IDs ("- [ ] 01-01-PLAN.md") and bold-wrapped IDs ("- [ ] **01-01**")
for (const summaryFile of phaseInfo.summaries) {
const planId = summaryFile.replace('-SUMMARY.md', '').replace('SUMMARY.md', '');
if (!planId) continue;
const planEscaped = escapeRegex(planId);
const planCheckboxPattern = new RegExp(
`(-\\s*\\[) (\\]\\s*(?:\\*\\*)?${planEscaped}(?:\\*\\*)?)`,
'i'
);
roadmapContent = roadmapContent.replace(planCheckboxPattern, '$1x$2');
}
writes.push({ filePath: roadmapPath, before: originalRoadmapContent, after: roadmapContent });
// Update REQUIREMENTS.md traceability for this phase's requirements
const reqPath = path.join(planningDir(cwd), 'REQUIREMENTS.md');
if (fs.existsSync(reqPath)) {
// Extract the current phase section from roadmap (scoped to avoid cross-phase matching).
// #3537: padding-tolerant fragment so an un-padded `Phase 2.7:` heading
// is found when caller resolved to padded `02.7`.
const phaseEsc = phaseMarkdownRegexSource(phaseNum);
const currentMilestoneRoadmap = extractCurrentMilestone(roadmapContent, cwd);
const phaseSectionMatch = currentMilestoneRoadmap.match(
new RegExp(`(#{2,4}\\s*Phase\\s+${phaseEsc}[:\\s][\\s\\S]*?)(?=#{2,4}\\s*Phase\\s+|$)`, 'i')
);
const sectionText = phaseSectionMatch ? phaseSectionMatch[1] : '';
// Accept all bold/colon variants (#2769) — the previous pattern only
// matched **Requirements:** (colon inside bold) and silently skipped
// **Requirements**: (colon outside), preventing the matching REQ-IDs
// from being ticked off in REQUIREMENTS.md on phase completion.
const reqMatch = sectionText.match(/\*\*Requirements:?\*\*[^\S\n]*:?[^\S\n]*([^\n]+)/i);
const originalReqContent = fs.readFileSync(reqPath, 'utf-8');
let reqContent = originalReqContent;
if (reqMatch) {
const reqIds = reqMatch[1].replace(/[\[\]]/g, '').split(/[,\s]+/).map(r => r.trim()).filter(Boolean);
for (const reqId of reqIds) {
const reqEscaped = escapeRegex(reqId);
// Update checkbox: - [ ] **REQ-ID** → - [x] **REQ-ID**
reqContent = reqContent.replace(
new RegExp(`(-\\s*\\[)[ ](\\]\\s*\\*\\*${reqEscaped}\\*\\*)`, 'gi'),
'$1x$2'
);
// Update traceability table: | REQ-ID | Phase N | Pending/In Progress | → | REQ-ID | Phase N | Complete |
reqContent = reqContent.replace(
new RegExp(`(\\|\\s*${reqEscaped}\\s*\\|[^|]+\\|)\\s*(?:Pending|In Progress)\\s*(\\|)`, 'gi'),
'$1 Complete $2'
);
}
}
// Scan body for all **REQ-ID** patterns, warn about any missing from the Traceability table.
// Always runs regardless of whether the roadmap has a Requirements: line.
const bodyReqIds = [];
const bodyReqPattern = /\*\*([A-Z][A-Z0-9]*-\d+)\*\*/g;
let bodyMatch;
while ((bodyMatch = bodyReqPattern.exec(reqContent)) !== null) {
const id = bodyMatch[1];
if (!bodyReqIds.includes(id)) bodyReqIds.push(id);
}
// Collect REQ-IDs present in the Traceability section only, to avoid
// picking up IDs from other tables in the document.
const traceabilityHeadingMatch = reqContent.match(/^#{1,6}\s+Traceability\b/im);
const traceabilitySection = traceabilityHeadingMatch
? reqContent.slice(traceabilityHeadingMatch.index)
: '';
const tableReqIds = new Set();
const tableRowPattern = /^\|\s*([A-Z][A-Z0-9]*-\d+)\s*\|/gm;
let tableMatch;
while ((tableMatch = tableRowPattern.exec(traceabilitySection)) !== null) {
tableReqIds.add(tableMatch[1]);
}
const unregistered = bodyReqIds.filter(id => !tableReqIds.has(id));
if (unregistered.length > 0) {
warnings.push(
`REQUIREMENTS.md: ${unregistered.length} REQ-ID(s) found in body but missing from Traceability table: ${unregistered.join(', ')} — add them manually to keep traceability in sync`
);
}
writes.push({ filePath: reqPath, before: originalReqContent, after: reqContent });
requirementsUpdated = true;
}
}
}
} catch { /* intentionally empty */ }
// Fallback: if filesystem found no next phase, check ROADMAP.md
// for phases that are defined but not yet planned (no directory on disk)
if (isLastPhase && fs.existsSync(roadmapPath)) {
try {
const roadmapForPhases = extractCurrentMilestone(fs.readFileSync(roadmapPath, 'utf-8'), cwd);
const phasePattern = /#{2,4}\s*Phase\s+(\d+[A-Z]?(?:\.\d+)*)\s*:\s*([^\n]+)/gi;
let pm;
while ((pm = phasePattern.exec(roadmapForPhases)) !== null) {
if (comparePhaseNum(pm[1], phaseNum) > 0) {
nextPhaseNum = pm[1];
nextPhaseName = pm[2].replace(/\(INSERTED\)/i, '').trim().toLowerCase().replace(/\s+/g, '-');
isLastPhase = false;
break;
// Find next phase — check both filesystem AND roadmap
// Phases may be defined in ROADMAP.md but not yet scaffolded to disk,
// so a filesystem-only scan would incorrectly report is_last_phase:true
try {
const isDirInMilestone = getMilestonePhaseFilter(cwd);
const entries = fs.readdirSync(phasesDir, { withFileTypes: true });
const dirs = entries.filter(e => e.isDirectory()).map(e => e.name)
.filter(isDirInMilestone)
.sort((a, b) => comparePhaseNum(a, b));
// Find the next phase directory after current
// Skip backlog phases (999.x) — they are parked ideas, not sequential work (#2129)
for (const dir of dirs) {
const dm = dir.match(/^(\d+[A-Z]?(?:\.\d+)*)-?(.*)/i);
if (dm) {
if (/^999(?:\.|$)/.test(dm[1])) continue;
if (comparePhaseNum(dm[1], phaseNum) > 0) {
nextPhaseNum = dm[1];
nextPhaseName = dm[2] || null;
isLastPhase = false;
break;
}
}
}
}
} catch { /* intentionally empty */ }
}
} catch { /* intentionally empty */ }
// Update STATE.md atomically — hold lock across read-modify-write (#P4.4).
// Previously read outside the lock; a crash between the ROADMAP update
// (locked above) and this write left ROADMAP/STATE inconsistent.
if (fs.existsSync(statePath)) {
readModifyWriteStateMd(statePath, (stateContent) => {
// Update Current Phase — preserve "X of Y (Name)" compound format
const phaseValue = nextPhaseNum || phaseNum;
const existingPhaseField = stateExtractField(stateContent, 'Current Phase')
|| stateExtractField(stateContent, 'Phase');
let newPhaseValue = String(phaseValue);
if (existingPhaseField) {
const totalMatch = existingPhaseField.match(/of\s+(\d+)/);
const nameMatch = existingPhaseField.match(/\(([^)]+)\)/);
if (totalMatch) {
const total = totalMatch[1];
const nameStr = nextPhaseName ? ` (${nextPhaseName.replace(/-/g, ' ')})` : (nameMatch ? ` (${nameMatch[1]})` : '');
newPhaseValue = `${phaseValue} of ${total}${nameStr}`;
// Fallback: if filesystem found no next phase, check ROADMAP.md
// for phases that are defined but not yet planned (no directory on disk)
if (isLastPhase && roadmapContent !== null) {
try {
const roadmapForPhases = extractCurrentMilestone(roadmapContent, cwd);
const phasePattern = /#{2,4}\s*Phase\s+(\d+[A-Z]?(?:\.\d+)*)\s*:\s*([^\n]+)/gi;
let pm;
while ((pm = phasePattern.exec(roadmapForPhases)) !== null) {
if (comparePhaseNum(pm[1], phaseNum) > 0) {
nextPhaseNum = pm[1];
nextPhaseName = pm[2].replace(/\(INSERTED\)/i, '').trim().toLowerCase().replace(/\s+/g, '-');
isLastPhase = false;
break;
}
}
} catch { /* intentionally empty */ }
}
// Update STATE.md while the planning lock is still held.
if (fs.existsSync(statePath)) {
const originalStateContent = platformReadSync(statePath) || '';
let stateContent = originalStateContent;
// Update Current Phase — preserve "X of Y (Name)" compound format
const phaseValue = nextPhaseNum || phaseNum;
const existingPhaseField = stateExtractField(stateContent, 'Current Phase')
|| stateExtractField(stateContent, 'Phase');
let newPhaseValue = String(phaseValue);
if (existingPhaseField) {
const totalMatch = existingPhaseField.match(/of\s+(\d+)/);
const nameMatch = existingPhaseField.match(/\(([^)]+)\)/);
if (totalMatch) {
const total = totalMatch[1];
const nameStr = nextPhaseName ? ` (${nextPhaseName.replace(/-/g, ' ')})` : (nameMatch ? ` (${nameMatch[1]})` : '');
newPhaseValue = `${phaseValue} of ${total}${nameStr}`;
}
}
}
stateContent = stateReplaceFieldWithFallback(stateContent, 'Current Phase', 'Phase', newPhaseValue);
stateContent = stateReplaceFieldWithFallback(stateContent, 'Current Phase', 'Phase', newPhaseValue);
// Update Current Phase Name
if (nextPhaseName) {
stateContent = stateReplaceFieldWithFallback(stateContent, 'Current Phase Name', null, nextPhaseName.replace(/-/g, ' '));
}
// Update Current Phase Name
if (nextPhaseName) {
stateContent = stateReplaceFieldWithFallback(stateContent, 'Current Phase Name', null, nextPhaseName.replace(/-/g, ' '));
}
// Update Status
stateContent = stateReplaceFieldWithFallback(stateContent, 'Status', null,
isLastPhase ? 'Milestone complete' : 'Ready to plan');
// Update Status
stateContent = stateReplaceFieldWithFallback(stateContent, 'Status', null,
isLastPhase ? 'Milestone complete' : 'Ready to plan');
// Update Current Plan
stateContent = stateReplaceFieldWithFallback(stateContent, 'Current Plan', 'Plan', 'Not started');
// Update Current Plan
stateContent = stateReplaceFieldWithFallback(stateContent, 'Current Plan', 'Plan', 'Not started');
// Update Last Activity
stateContent = stateReplaceFieldWithFallback(stateContent, 'Last Activity', 'Last activity', today);
// Update Last Activity
stateContent = stateReplaceFieldWithFallback(stateContent, 'Last Activity', 'Last activity', today);
// Update Last Activity Description
stateContent = stateReplaceFieldWithFallback(stateContent, 'Last Activity Description', null,
`Phase ${phaseNum} complete${nextPhaseNum ? `, transitioned to Phase ${nextPhaseNum}` : ''}`);
// Update Last Activity Description
stateContent = stateReplaceFieldWithFallback(stateContent, 'Last Activity Description', null,
`Phase ${phaseNum} complete${nextPhaseNum ? `, transitioned to Phase ${nextPhaseNum}` : ''}`);
// Update Completed Phases counter — derive from ROADMAP instead of blind +1.
// Fix for issue #4: the original code did parseInt(completedRaw, 10) + 1 on every
// call, making phase complete non-idempotent (double-call = double-increment).
// Now we read the freshly-updated ROADMAP to count Complete rows, then use
// deriveProgressFromRoadmap() from phase-lifecycle.generated.cjs (generated from
// sdk/src/query/phase-lifecycle.ts "Root cause 1 fix" block).
// References: issue #4, ADR-3524, gen-phase-lifecycle.mjs.
const completedRaw = stateExtractField(stateContent, 'Completed Phases');
if (completedRaw !== null) {
// Derive from ROADMAP if available (idempotent); fall back to existing value.
let newCompleted = parseInt(completedRaw, 10);
let derivedTotalPhases = null;
if (fs.existsSync(roadmapPath)) {
try {
const freshRoadmap = fs.readFileSync(roadmapPath, 'utf-8');
const derived = deriveProgressFromRoadmap(freshRoadmap);
// Update Completed Phases counter — derive from the same ROADMAP snapshot
// that will be published in this transaction, not a separately-read file.
const completedRaw = stateExtractField(stateContent, 'Completed Phases');
if (completedRaw !== null) {
// Derive from ROADMAP if available (idempotent); fall back to existing value.
let newCompleted = parseInt(completedRaw, 10);
let derivedTotalPhases = null;
if (roadmapContent !== null) {
const derived = deriveProgressFromRoadmap(roadmapContent);
if (derived.completedPhases !== null) newCompleted = derived.completedPhases;
if (derived.totalPhases !== null) derivedTotalPhases = derived.totalPhases;
} catch { /* fall through to existing value */ }
}
stateContent = stateReplaceField(stateContent, 'Completed Phases', String(newCompleted)) || stateContent;
}
stateContent = stateReplaceField(stateContent, 'Completed Phases', String(newCompleted)) || stateContent;
// Recalculate percent — use clampPercent to prevent >100% (#4 unclamped bug).
const totalRaw = stateExtractField(stateContent, 'Total Phases');
const totalPhases = derivedTotalPhases
|| (totalRaw ? parseInt(totalRaw, 10) : null);
if (totalPhases && totalPhases > 0) {
const newPercent = clampPercent(newCompleted, totalPhases);
stateContent = stateReplaceField(stateContent, 'Progress', `${newPercent}%`) || stateContent;
stateContent = stateContent.replace(
/(percent:\s*)\d+/,
`$1${newPercent}`
);
// Recalculate percent — use clampPercent to prevent >100% (#4 unclamped bug).
const totalRaw = stateExtractField(stateContent, 'Total Phases');
const totalPhases = derivedTotalPhases
|| (totalRaw ? parseInt(totalRaw, 10) : null);
if (totalPhases && totalPhases > 0) {
const newPercent = clampPercent(newCompleted, totalPhases);
stateContent = stateReplaceField(stateContent, 'Progress', `${newPercent}%`) || stateContent;
stateContent = stateContent.replace(
/(percent:\s*)\d+/,
`$1${newPercent}`
);
}
}
// Gate 4: Update Performance Metrics section (#1627)
stateContent = updatePerformanceMetricsSection(stateContent, cwd, phaseNum, planCount, summaryCount);
stateContent = syncStateFrontmatter(stateContent, cwd);
writes.push({ filePath: statePath, before: originalStateContent, after: stateContent });
}
// Gate 4: Update Performance Metrics section (#1627)
stateContent = updatePerformanceMetricsSection(stateContent, cwd, phaseNum, planCount, summaryCount);
writePlanningFileSet(writes);
};
return stateContent;
}, cwd);
}
if (fs.existsSync(statePath)) {
withStateLock(statePath, runPhaseCompleteTransaction);
} else {
runPhaseCompleteTransaction();
}
});
// Auto-prune STATE.md on phase boundary when configured (#2087)
let autoPruned = false;

10
get-shit-done/bin/lib/planning-workspace.cjs
View File

@@ -100,7 +100,7 @@ function withPlanningLock(cwd, fn, clock) {
// Ensure .planning/ exists
try { platformEnsureDir(planningDir(cwd)); } catch { /* ok */ }
function runWithHeldLock() {
function acquireLock() {
// Atomic create — fails if file exists
fs.writeFileSync(lockPath, JSON.stringify({
pid: process.pid,
@@ -109,8 +109,9 @@ function withPlanningLock(cwd, fn, clock) {
}), { flag: 'wx' });
_heldPlanningLocks.add(lockPath);
}
// Lock acquired — run the function
function runWithHeldLock() {
try {
return fn();
} finally {
@@ -120,12 +121,16 @@ function withPlanningLock(cwd, fn, clock) {
}
while (clock.now() - start < lockTimeout) {
let lockWasAcquired = false;
try {
acquireLock();
lockWasAcquired = true;
return runWithHeldLock();
} catch (err) {
// Transient filesystem errors (Docker overlay-fs, NFS, OS signals, AV scanners)
// are recoverable — wait and retry rather than propagating.
// See PLANNING_LOCK_RETRY_ERRNOS for the full list and rationale.
if (lockWasAcquired) throw err;
if (PLANNING_LOCK_RETRY_ERRNOS.has(err.code)) {
clock.sleep(100);
continue;
@@ -150,6 +155,7 @@ function withPlanningLock(cwd, fn, clock) {
// Timeout — stale-lock recovery, then re-acquire atomically before entering critical section.
try { fs.unlinkSync(lockPath); } catch { /* ok */ }
acquireLock();
return runWithHeldLock();
}

11
get-shit-done/bin/lib/state.cjs
View File

@@ -1050,6 +1050,15 @@ function releaseStateLock(lockPath) {
try { fs.unlinkSync(lockPath); } catch { /* lock already gone */ }
}
function withStateLock(statePath, fn) {
const lockPath = acquireStateLock(statePath);
try {
return fn();
} finally {
releaseStateLock(lockPath);
}
}
/**
* Write STATE.md with synchronized YAML frontmatter.
* All STATE.md writes should use this instead of raw writeFileSync.
@@ -2001,6 +2010,8 @@ module.exports = {
releaseStateLock,
writeStateMd,
readModifyWriteStateMd,
syncStateFrontmatter,
withStateLock,
updatePerformanceMetricsSection,
cmdStateLoad,
cmdStateGet,

146
tests/4-phase-complete-cjs-regression.test.cjs
View File

@@ -41,6 +41,7 @@ const { cmdPhaseComplete } = phaseModule;
/**
* Creates a minimal fixture project with:
* - ROADMAP.md with a 4-column progress table (Phase | Plans | Status | Completed)
* - REQUIREMENTS.md with a phase-scoped REQ-ID and Traceability row
* - STATE.md with Completed Phases: 0 and Total Phases: 2 (Progress 0%)
* - Phase 01 directory with one plan+summary (to satisfy phase complete guard)
* - Phase 02 directory (next phase)
@@ -61,6 +62,7 @@ function createFixture(prefix = 'gsd-4-regression-') {
'',
'### Phase 01: Foundation',
'**Goal:** Build the foundation',
'**Requirements:** REQ-1',
'**Plans:** 1 plans',
'',
'### Phase 02: API',
@@ -76,6 +78,22 @@ function createFixture(prefix = 'gsd-4-regression-') {
].join('\n');
fs.writeFileSync(path.join(planningDir, 'ROADMAP.md'), roadmap);
const requirements = [
'# Requirements',
'',
'## Functional Requirements',
'',
'- [ ] **REQ-1** Foundation must be complete.',
'',
'## Traceability',
'',
'| Requirement | Phase | Status |',
'|-------------|-------|--------|',
'| REQ-1 | Phase 01 | Pending |',
'',
].join('\n');
fs.writeFileSync(path.join(planningDir, 'REQUIREMENTS.md'), requirements);
// STATE.md: Completed Phases: 0, Total Phases: 2, Progress: 0%
// Uses body-field format (bold **Field:** value) so the CJS handler's
// stateExtractField/stateReplaceField path is exercised.
@@ -111,6 +129,38 @@ function readStateMd(tmpDir) {
return fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf8');
}
function roadmapCompletionSnapshot(roadmapContent) {
const snapshot = {
phaseCheckboxes: [],
progressRows: [],
};
for (const line of roadmapContent.split(/\r?\n/)) {
let match = line.match(/^- \[([ x])\] Phase ([^:]+): (.*)$/);
if (match) {
snapshot.phaseCheckboxes.push({
checked: match[1] === 'x',
phase: match[2].trim(),
title: match[3].replace(/\s+\(completed [^)]+\)$/, '').trim(),
});
continue;
}
match = line.match(/^\|\s*(\d+[A-Z]?(?:\.\d+)*)\.?\s*([^|]*)\|\s*([^|]*)\|\s*([^|]*)\|\s*([^|]*)\|$/i);
if (match) {
snapshot.progressRows.push({
phase: match[1].trim(),
title: match[2].trim(),
plans: match[3].trim(),
status: match[4].trim(),
completed: match[5].trim(),
});
}
}
return snapshot;
}
function extractField(stateContent, fieldName) {
const escaped = fieldName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
const boldMatch = stateContent.match(new RegExp(`\\*\\*${escaped}:\\*\\*[ \\t]*(.+)`, 'i'));
@@ -207,6 +257,102 @@ describe('issue #4 (CJS): cmdPhaseComplete — idempotency (blind-increment bug)
`STATE after second call (body: ${completedAfter2Body}, fm: ${completedAfter2Fm}):\n${stateAfter2}`,
);
});
test('rolls back ROADMAP when STATE write fails during phase completion', (t) => {
const roadmapPath = path.join(tmpDir, '.planning', 'ROADMAP.md');
const reqPath = path.join(tmpDir, '.planning', 'REQUIREMENTS.md');
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
const originalRoadmap = fs.readFileSync(roadmapPath, 'utf8');
const originalReq = fs.readFileSync(reqPath, 'utf8');
const originalState = fs.readFileSync(statePath, 'utf8');
const originalWriteFileSync = fs.writeFileSync;
t.mock.method(fs, 'writeFileSync', function injectedStateWriteFailure(target, ...args) {
const targetPath = String(target);
const isStatePublish = targetPath === statePath || targetPath === `${statePath}.tmp.${process.pid}`;
if (isStatePublish) {
const err = new Error('injected STATE.md write failure');
err.code = 'EIO';
throw err;
}
return originalWriteFileSync.call(this, target, ...args);
});
assert.throws(
() => capturePhaseComplete(tmpDir, '1'),
/injected STATE\.md write failure/,
);
const roadmapAfter = fs.readFileSync(roadmapPath, 'utf8');
const reqAfter = fs.readFileSync(reqPath, 'utf8');
const stateAfter = fs.readFileSync(statePath, 'utf8');
assert.deepEqual(
roadmapCompletionSnapshot(roadmapAfter),
roadmapCompletionSnapshot(originalRoadmap),
'ROADMAP.md should roll back to its original completion state',
);
assert.equal(reqAfter, originalReq, 'REQUIREMENTS.md should roll back when STATE.md write fails');
assert.equal(stateAfter, originalState, 'STATE.md should remain unchanged after injected write failure');
});
test('rolls back ROADMAP when REQUIREMENTS write fails during phase completion', (t) => {
const roadmapPath = path.join(tmpDir, '.planning', 'ROADMAP.md');
const reqPath = path.join(tmpDir, '.planning', 'REQUIREMENTS.md');
const originalRoadmap = fs.readFileSync(roadmapPath, 'utf8');
const originalReq = fs.readFileSync(reqPath, 'utf8');
const originalWriteFileSync = fs.writeFileSync;
t.mock.method(fs, 'writeFileSync', function injectedRequirementsWriteFailure(target, ...args) {
const targetPath = String(target);
if (targetPath === reqPath || targetPath === `${reqPath}.tmp.${process.pid}`) {
const err = new Error('injected REQUIREMENTS.md write failure');
err.code = 'EIO';
throw err;
}
return originalWriteFileSync.call(this, target, ...args);
});
assert.throws(
() => capturePhaseComplete(tmpDir, '1'),
/injected REQUIREMENTS\.md write failure/,
);
assert.deepEqual(
roadmapCompletionSnapshot(fs.readFileSync(roadmapPath, 'utf8')),
roadmapCompletionSnapshot(originalRoadmap),
'ROADMAP.md should roll back when the REQUIREMENTS write fails',
);
assert.equal(fs.readFileSync(reqPath, 'utf8'), originalReq, 'REQUIREMENTS.md should be unchanged');
});
test('reports rollback failure when restoring an earlier planning file fails', (t) => {
const roadmapPath = path.join(tmpDir, '.planning', 'ROADMAP.md');
const reqPath = path.join(tmpDir, '.planning', 'REQUIREMENTS.md');
const originalWriteFileSync = fs.writeFileSync;
let requirementsWriteFailed = false;
t.mock.method(fs, 'writeFileSync', function injectedRollbackFailure(target, ...args) {
const targetPath = String(target);
if (targetPath === reqPath || targetPath === `${reqPath}.tmp.${process.pid}`) {
requirementsWriteFailed = true;
const err = new Error('injected REQUIREMENTS.md write failure');
err.code = 'EIO';
throw err;
}
if (requirementsWriteFailed && (targetPath === roadmapPath || targetPath === `${roadmapPath}.tmp.${process.pid}`)) {
const err = new Error('injected ROADMAP.md rollback failure');
err.code = 'EIO';
throw err;
}
return originalWriteFileSync.call(this, target, ...args);
});
assert.throws(
() => capturePhaseComplete(tmpDir, '1'),
/injected REQUIREMENTS\.md write failure[\s\S]*WARNING: rollback failed while restoring[\s\S]*injected ROADMAP\.md rollback failure/,
);
});
});
// ── T2: Progress percent must never exceed 100% ──────────────────────────────

16
tests/bugs-1656-1657.test.cjs
View File

@@ -90,4 +90,20 @@ describe('#1657 / #191: installer/package metadata retires sdk seam', () => {
'root package.json files must not include sdk paths'
);
});
test('root tsconfig project references resolve to existing paths (#191)', () => {
const tsconfigPath = path.join(__dirname, '..', 'tsconfig.json');
if (!fs.existsSync(tsconfigPath)) return;
const tsconfig = JSON.parse(fs.readFileSync(tsconfigPath, 'utf-8'));
const references = tsconfig.references || [];
for (const ref of references) {
const refPath = String(ref.path || '');
assert.notEqual(refPath, 'sdk', 'root tsconfig must not reference retired sdk project');
assert.ok(
fs.existsSync(path.join(__dirname, '..', refPath)),
`root tsconfig reference must exist: ${refPath}`
);
}
});
});

19
tests/planning-workspace.test.cjs
View File

@@ -125,6 +125,25 @@ describe('planning-workspace: lock seam', () => {
fs.rmSync(tmpDir, { recursive: true, force: true });
}
});
test('does not retry errors thrown by locked work', () => {
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-planning-lock-work-error-'));
let attempts = 0;
try {
assert.throws(() => {
withPlanningLock(tmpDir, () => {
attempts += 1;
const err = new Error('write failed inside critical section');
err.code = 'EIO';
throw err;
});
}, /write failed inside critical section/);
assert.strictEqual(attempts, 1);
assert.ok(!fs.existsSync(path.join(tmpDir, '.planning', '.lock')));
} finally {
fs.rmSync(tmpDir, { recursive: true, force: true });
}
});
});
describe('core compatibility adapter: planning workspace functions', () => {

4
tsconfig.json
View File

@@ -1,6 +1,4 @@
{
"files": [],
"references": [
{ "path": "sdk" }
]
"references": []
}