diff --git a/src/resources/extensions/gsd/db-writer.ts b/src/resources/extensions/gsd/db-writer.ts index 5ff97479a..3230e6d7c 100644 --- a/src/resources/extensions/gsd/db-writer.ts +++ b/src/resources/extensions/gsd/db-writer.ts @@ -272,6 +272,10 @@ export interface SaveRequirementFields { /** * Save a new requirement to DB and regenerate REQUIREMENTS.md. * Auto-assigns the next ID via nextRequirementId(). + * + * The ID computation and insert are wrapped in a single transaction + * to prevent parallel race conditions (same pattern as saveDecisionToDb). + * * Returns the assigned ID. */ export async function saveRequirementToDb( @@ -281,24 +285,37 @@ export async function saveRequirementToDb( try { const db = await import('./gsd-db.js'); - const id = await nextRequirementId(); + // Atomic ID assignment + insert inside a transaction. + const id = db.transaction(() => { + const adapter = db._getAdapter(); + if (!adapter) throw new GSDError(GSD_STALE_STATE, "gsd-db: No database open"); - const requirement: Requirement = { - id, - class: fields.class, - status: fields.status ?? 'active', - description: fields.description, - why: fields.why, - source: fields.source, - primary_owner: fields.primary_owner ?? '', - supporting_slices: fields.supporting_slices ?? '', - validation: fields.validation ?? '', - notes: fields.notes ?? '', - full_content: '', - superseded_by: null, - }; + const row = adapter + .prepare('SELECT MAX(CAST(SUBSTR(id, 2) AS INTEGER)) as max_num FROM requirements') + .get(); + const maxNum = row ? (row['max_num'] as number | null) : null; + const nextId = (maxNum == null || isNaN(maxNum)) + ? 'R001' + : `R${String(maxNum + 1).padStart(3, '0')}`; - db.upsertRequirement(requirement); + const requirement: Requirement = { + id: nextId, + class: fields.class, + status: fields.status ?? 'active', + description: fields.description, + why: fields.why, + source: fields.source, + primary_owner: fields.primary_owner ?? '', + supporting_slices: fields.supporting_slices ?? '', + validation: fields.validation ?? '', + notes: fields.notes ?? '', + full_content: '', + superseded_by: null, + }; + + db.upsertRequirement(requirement); + return nextId; + }); // Fetch all requirements for full file regeneration const adapter = db._getAdapter(); @@ -358,6 +375,11 @@ export interface SaveDecisionFields { /** * Save a new decision to DB and regenerate DECISIONS.md. * Auto-assigns the next ID via nextDecisionId(). + * + * The ID computation (SELECT MAX) and insert are wrapped in a single + * transaction to prevent parallel tool calls from computing the same ID + * and silently overwriting each other (#3326, #3339, #3459). + * * Returns the assigned ID. */ export async function saveDecisionToDb( @@ -367,18 +389,33 @@ export async function saveDecisionToDb( try { const db = await import('./gsd-db.js'); - const id = await nextDecisionId(); + // Atomic ID assignment + insert inside a transaction to prevent + // parallel calls from racing on the same MAX(id) value. + const id = db.transaction(() => { + const adapter = db._getAdapter(); + if (!adapter) throw new GSDError(GSD_STALE_STATE, "gsd-db: No database open"); - db.upsertDecision({ - id, - when_context: fields.when_context ?? '', - scope: fields.scope, - decision: fields.decision, - choice: fields.choice, - rationale: fields.rationale, - revisable: fields.revisable ?? 'Yes', - made_by: fields.made_by ?? 'agent', - superseded_by: null, + const row = adapter + .prepare('SELECT MAX(CAST(SUBSTR(id, 2) AS INTEGER)) as max_num FROM decisions') + .get(); + const maxNum = row ? (row['max_num'] as number | null) : null; + const nextId = (maxNum == null || isNaN(maxNum)) + ? 'D001' + : `D${String(maxNum + 1).padStart(3, '0')}`; + + db.upsertDecision({ + id: nextId, + when_context: fields.when_context ?? '', + scope: fields.scope, + decision: fields.decision, + choice: fields.choice, + rationale: fields.rationale, + revisable: fields.revisable ?? 'Yes', + made_by: fields.made_by ?? 'agent', + superseded_by: null, + }); + + return nextId; }); // Fetch all decisions (including superseded for the full register) diff --git a/src/resources/extensions/gsd/tests/db-writer.test.ts b/src/resources/extensions/gsd/tests/db-writer.test.ts index 922e070b5..cbdd44422 100644 --- a/src/resources/extensions/gsd/tests/db-writer.test.ts +++ b/src/resources/extensions/gsd/tests/db-writer.test.ts @@ -358,6 +358,47 @@ describe('db-writer', () => { } }); + // ═══════════════════════════════════════════════════════════════════════════ + // Parallel save race condition regression (#3326, #3339, #3459) + // ═══════════════════════════════════════════════════════════════════════════ + + test('parallel saveDecisionToDb calls produce unique IDs', async () => { + const tmpDir = makeTmpDir(); + const dbPath = path.join(tmpDir, '.gsd', 'gsd.db'); + openDatabase(dbPath); + + try { + // Fire 5 saves concurrently — before the fix, all would get D001 + const results = await Promise.all([ + saveDecisionToDb({ scope: 'a', decision: 'd1', choice: 'c1', rationale: 'r1' }, tmpDir), + saveDecisionToDb({ scope: 'b', decision: 'd2', choice: 'c2', rationale: 'r2' }, tmpDir), + saveDecisionToDb({ scope: 'c', decision: 'd3', choice: 'c3', rationale: 'r3' }, tmpDir), + saveDecisionToDb({ scope: 'd', decision: 'd4', choice: 'c4', rationale: 'r4' }, tmpDir), + saveDecisionToDb({ scope: 'e', decision: 'd5', choice: 'c5', rationale: 'r5' }, tmpDir), + ]); + + const ids = results.map((r) => r.id); + const uniqueIds = new Set(ids); + + // All 5 IDs must be unique + assert.equal(uniqueIds.size, 5, `Expected 5 unique IDs, got ${uniqueIds.size}: ${ids.join(', ')}`); + + // IDs should be D001-D005 (order may vary due to concurrency) + for (const id of ids) { + assert.match(id, /^D\d{3}$/, `ID ${id} should match D### pattern`); + } + + // Verify all 5 exist in DB + for (const id of ids) { + const row = getDecisionById(id); + assert.ok(row, `Decision ${id} should exist in DB`); + } + } finally { + closeDatabase(); + cleanupDir(tmpDir); + } + }); + // ═══════════════════════════════════════════════════════════════════════════ // updateRequirementInDb Tests // ═══════════════════════════════════════════════════════════════════════════