singularity/singularity-forge
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

fix(state): DB-authoritative milestone completeness (#4179)

Browse source 
Read-side twin of #4175. `deriveStateFromDb` had a SUMMARY-file fallback
that could mark a milestone complete even when the DB row said otherwise,
allowing an orphan SUMMARY.md (crashed complete-milestone turn, partial
merge, manual edit) to cascade into a false auto-merge.

- buildCompletenessSet: drop SUMMARY fallback; only DB status decides.
- buildRegistryAndFindActive: remove SUMMARY from the completeness check;
  still consult SUMMARY as a title fallback for DB-certified milestones.
- allSlicesDone branch: drop `!summaryFile` clause so a terminal-validation
  + orphan-SUMMARY path flows through to `completing-milestone` instead of
  short-circuiting, letting complete-milestone re-run idempotently.

Regression tests: orphan SUMMARY with in-flight slice stays active; orphan
SUMMARY with all-slices-done + validation-terminal lands at
completing-milestone (does not report as already complete).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Jeremy 2026-04-14 07:43:29 -05:00
parent d07b9bf473
commit 4a2045d290
2 changed files with 92 additions and 22 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

38
src/resources/extensions/gsd/state.ts
View file

@ -366,6 +366,10 @@ function buildCompletenessSet(basePath: string, milestones: MilestoneRow[]) {
const completeMilestoneIds = new Set<string>();
const parkedMilestoneIds = new Set<string>();
// DB-authoritative: a milestone is only "complete" when its DB row says so.
// SUMMARY-file presence is NOT a completion signal here — an orphan SUMMARY
// (crashed complete-milestone turn, partial merge, manual edit) must not
// flip derived state to complete and cascade into a false auto-merge (#4179).
for (const m of milestones) {
const parkedFile = resolveMilestoneFile(basePath, m.id, "PARKED");
if (parkedFile || m.status === 'parked') {
@ -376,11 +380,6 @@ function buildCompletenessSet(basePath: string, milestones: MilestoneRow[]) {
completeMilestoneIds.add(m.id);
continue;
}
const summaryFile = resolveMilestoneFile(basePath, m.id, "SUMMARY");
if (summaryFile) {
completeMilestoneIds.add(m.id);
continue;
}
}
return { completeMilestoneIds, parkedMilestoneIds };
}
@ -409,18 +408,22 @@ async function buildRegistryAndFindActive(
if (isGhostMilestone(basePath, m.id)) continue;
}
const summaryFile = resolveMilestoneFile(basePath, m.id, "SUMMARY");
if (completeMilestoneIds.has(m.id) || (summaryFile !== null)) {
// DB-authoritative completeness (#4179): only trust completeMilestoneIds,
// which is itself derived from DB status. SUMMARY-file presence alone must
// not imply completion. The summary file may still be consulted below as a
// title source for legitimately-complete milestones whose DB row has no title.
if (completeMilestoneIds.has(m.id)) {
let title = stripMilestonePrefix(m.title) || m.id;
if (summaryFile && !m.title) {
const summaryContent = await loadFile(summaryFile);
if (summaryContent) {
title = parseSummary(summaryContent).title || m.id;
if (!m.title) {
const summaryFile = resolveMilestoneFile(basePath, m.id, "SUMMARY");
if (summaryFile) {
const summaryContent = await loadFile(summaryFile);
if (summaryContent) {
title = parseSummary(summaryContent).title || m.id;
}
}
}
registry.push({ id: m.id, title, status: 'complete' });
completeMilestoneIds.add(m.id);
continue;
}
@ -461,7 +464,14 @@ async function buildRegistryAndFindActive(
const validationContent = validationFile ? await loadFile(validationFile) : null;
const validationTerminal = validationContent ? isValidationTerminal(validationContent) : false;
if (!validationTerminal || (validationTerminal && !summaryFile)) {
// DB-authoritative (#4179): completeness is already decided by
// completeMilestoneIds above. If we reached this branch, the DB says
// the milestone is NOT complete — so any SUMMARY file on disk is an
// orphan (crashed complete-milestone, partial merge, manual edit) and
// must not short-circuit this path. When validation is terminal, fall
// through to the default active-push below so `complete-milestone` can
// re-run idempotently.
if (!validationTerminal) {
activeMilestone = { id: m.id, title };
activeMilestoneSlices = slices;
activeMilestoneFound = true;

76
src/resources/extensions/gsd/tests/derive-state-helpers.test.ts
View file

@ -307,27 +307,87 @@ describe('derive-state-helpers', () => {
}
});
// ─── buildCompletenessSet: SUMMARY-on-disk marks complete ───────────
test('buildCompletenessSet: milestone with SUMMARY on disk treated as complete', async () => {
// ─── buildCompletenessSet: DB status is authoritative ──────────────
test('buildCompletenessSet: DB status=complete marks milestone complete', async () => {
const base = createFixtureBase();
try {
// M001 has summary on disk but DB status is still 'active'
writeFile(base, 'milestones/M001/M001-ROADMAP.md', ROADMAP_CONTENT);
writeFile(base, 'milestones/M001/M001-SUMMARY.md', '# M001 Summary\n\nDone.');
// M002 is the real active milestone
writeFile(base, 'milestones/M002/M002-CONTEXT.md', '# M002\n\nActive.');
openDatabase(':memory:');
insertMilestone({ id: 'M001', title: 'First', status: 'active' });
insertMilestone({ id: 'M001', title: 'First', status: 'complete' });
insertMilestone({ id: 'M002', title: 'Second', status: 'active' });
invalidateStateCache();
const state = await deriveStateFromDb(base);
// M001 should be complete (summary on disk), M002 should be active
const m1 = state.registry.find(e => e.id === 'M001');
assert.equal(m1?.status, 'complete', 'summary-disk: M001 marked complete via disk SUMMARY');
assert.equal(state.activeMilestone?.id, 'M002', 'summary-disk: M002 is active');
assert.equal(m1?.status, 'complete', 'DB status=complete → registry entry complete');
assert.equal(state.activeMilestone?.id, 'M002', 'M002 is the active milestone');
} finally {
closeDatabase();
cleanup(base);
}
});
// ─── Regression #4179: orphan SUMMARY must NOT flip DB-active milestone ───
// A crashed complete-milestone turn (or stale/manual SUMMARY.md) can leave
// a milestone SUMMARY on disk while the DB row still reads 'active'. The
// read-side of state derivation must NOT treat the orphan SUMMARY as a
// completion signal, or the auto-loop advances and merges work that was
// never actually finished (same failure class as #4175, read-side twin).
test('buildCompletenessSet (#4179): orphan SUMMARY on disk does not mark DB-active milestone complete', async () => {
const base = createFixtureBase();
try {
writeFile(base, 'milestones/M001/M001-ROADMAP.md', ROADMAP_CONTENT);
writeFile(base, 'milestones/M001/M001-SUMMARY.md', '# M001 Orphan Summary\n\nLeft over from crashed turn.');
openDatabase(':memory:');
insertMilestone({ id: 'M001', title: 'First', status: 'active' });
// Slice still in-flight — auto should resume, not merge.
insertSlice({ id: 'S01', milestoneId: 'M001', title: 'First', status: 'active', risk: 'low', depends: [] });
insertSlice({ id: 'S02', milestoneId: 'M001', title: 'Second', status: 'pending', risk: 'low', depends: ['S01'] });
insertTask({ id: 'T01', sliceId: 'S01', milestoneId: 'M001', title: 'In-flight', status: 'pending' });
invalidateStateCache();
const state = await deriveStateFromDb(base);
const m1 = state.registry.find(e => e.id === 'M001');
assert.notEqual(m1?.status, 'complete', 'orphan SUMMARY must not mark milestone complete');
assert.equal(m1?.status, 'active', 'M001 remains active — DB is authoritative');
assert.equal(state.activeMilestone?.id, 'M001', 'M001 is still the active milestone');
assert.notEqual(state.phase, 'completing-milestone', 'must not short-circuit into completion');
} finally {
closeDatabase();
cleanup(base);
}
});
// Regression #4179 (companion): DB-active milestone with all slices done +
// validation terminal + orphan SUMMARY must still flow through completing-milestone
// (re-runs complete-milestone), not be reported as already-complete.
test('buildRegistryAndFindActive (#4179): orphan SUMMARY with validation-terminal falls through to completing-milestone', async () => {
const base = createFixtureBase();
try {
writeFile(base, 'milestones/M001/M001-ROADMAP.md', ROADMAP_CONTENT);
writeFile(base, 'milestones/M001/slices/S01/S01-PLAN.md', PLAN_CONTENT);
writeFile(base, 'milestones/M001/slices/S02/S02-PLAN.md', PLAN_CONTENT);
writeFile(base, 'milestones/M001/M001-VALIDATION.md', '---\nverdict: passed\n---\n# Validation\nAll good.');
writeFile(base, 'milestones/M001/M001-SUMMARY.md', '# M001 Orphan Summary\n\nLeft over.');
openDatabase(':memory:');
insertMilestone({ id: 'M001', title: 'First', status: 'active' });
insertSlice({ id: 'S01', milestoneId: 'M001', title: 'First', status: 'complete', risk: 'low', depends: [] });
insertSlice({ id: 'S02', milestoneId: 'M001', title: 'Second', status: 'complete', risk: 'low', depends: ['S01'] });
invalidateStateCache();
const state = await deriveStateFromDb(base);
const m1 = state.registry.find(e => e.id === 'M001');
assert.equal(m1?.status, 'active', 'M001 stays active despite orphan SUMMARY + validation-terminal');
assert.equal(state.activeMilestone?.id, 'M001', 'M001 is still the active milestone');
assert.equal(state.phase, 'completing-milestone', 'phase flows through completing-milestone (re-run)');
} finally {
closeDatabase();
cleanup(base);