fix: respect queue-order.json in DB-backed state derivation (#2649)

getActiveMilestoneId and deriveStateFromDb sorted milestones by ID
(localeCompare / milestoneIdSort) while the dispatch guard in
dispatch-guard.ts sorted by queue-order.json via findMilestoneIds.
When a user reordered milestones via /gsd queue to prioritize a
later-numbered milestone, the state machine ignored the reordering
and dispatched to the earlier-numbered one. The dispatch guard then
blocked completion because the queue-ordered-first milestone was
incomplete — producing a deadlock.

Replace the lexicographic sort with sortByQueueOrder(loadQueueOrder())
in both the getActiveMilestoneId DB path and the deriveStateFromDb
milestone sort. This aligns all three subsystems (state derivation,
dispatch, and dispatch guard) on the same ordering.

Closes #2556
This commit is contained in:
mastertyko 2026-03-26 15:14:23 +01:00 committed by GitHub
parent af7da35384
commit b03694fb4f
2 changed files with 56 additions and 5 deletions

View file

@ -34,7 +34,8 @@ import {
gsdRoot,
} from './paths.js';
import { milestoneIdSort, findMilestoneIds } from './milestone-ids.js';
import { findMilestoneIds } from './milestone-ids.js';
import { loadQueueOrder, sortByQueueOrder } from './queue-order.js';
import { nativeBatchParseGsdFiles, type BatchParsedFile } from './native-parser-bridge.js';
import { join, resolve } from 'path';
@ -149,8 +150,14 @@ export async function getActiveMilestoneId(basePath: string): Promise<string | n
if (isDbAvailable()) {
const allMilestones = getAllMilestones();
if (allMilestones.length > 0) {
const sorted = [...allMilestones].sort((a, b) => a.id.localeCompare(b.id));
for (const m of sorted) {
// Respect queue-order.json so /gsd queue reordering is honored (#2556).
// Without this, the DB path uses lexicographic sort while the dispatch
// guard uses queue order — causing a deadlock.
const customOrder = loadQueueOrder(basePath);
const sortedIds = sortByQueueOrder(allMilestones.map(m => m.id), customOrder);
const byId = new Map(allMilestones.map(m => [m.id, m]));
for (const id of sortedIds) {
const m = byId.get(id)!;
if (m.status === "complete" || m.status === "done" || m.status === "parked") continue;
return m.id;
}
@ -304,8 +311,12 @@ export async function deriveStateFromDb(basePath: string): Promise<GSDState> {
} as MilestoneRow);
}
}
// Re-sort so milestones are in canonical order after injection
allMilestones.sort((a, b) => milestoneIdSort(a.id, b.id));
// Re-sort so milestones follow queue order (same as dispatch guard) (#2556)
const customOrder = loadQueueOrder(basePath);
const sortedIds = sortByQueueOrder(allMilestones.map(m => m.id), customOrder);
const byId = new Map(allMilestones.map(m => [m.id, m]));
allMilestones.length = 0;
for (const id of sortedIds) allMilestones.push(byId.get(id)!);
// Parallel worker isolation: when locked, filter to just the locked milestone
const milestoneLock = process.env.GSD_MILESTONE_LOCK;

View file

@ -292,4 +292,44 @@ test('E2E: depends_on inline format preserved after partial removal', () => {
}
});
test('E2E: DB-backed path respects queue order (#2556)', async () => {
// Regression test for #2556: getActiveMilestoneId and deriveStateFromDb
// used lexicographic sort instead of queue order, causing a deadlock when
// the dispatch guard (which respects queue order) blocked completion.
const base = createFixtureBase();
try {
const { openDatabase, closeDatabase, insertMilestone, isDbAvailable } = await import('../gsd-db.ts');
const dbPath = join(base, '.gsd', 'gsd.db');
// Create milestone directories (required for findMilestoneIds)
writeMilestoneDir(base, 'M006');
writeContext(base, 'M006', '', 'Earlier milestone');
writeMilestoneDir(base, 'M008');
writeContext(base, 'M008', '', 'Later milestone');
// Open DB and insert milestones
openDatabase(dbPath);
try {
insertMilestone({ id: 'M006', title: 'Earlier', status: 'active' });
insertMilestone({ id: 'M008', title: 'Later', status: 'active' });
// Set queue order: M008 should come FIRST (user reordered via /gsd queue)
saveQueueOrder(base, ['M008', 'M006']);
// deriveState should pick M008 (queue-first), not M006 (ID-first)
invalidateStateCache();
const state = await deriveState(base);
assert.equal(
state.activeMilestone?.id,
'M008',
'DB-backed deriveState must respect queue order — M008 is queued first',
);
} finally {
if (isDbAvailable()) closeDatabase();
}
} finally {
cleanup(base);
}
});
});