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

* 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

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% ──────────────────────────────

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}`
);
}
});
});

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', () => {