From 01aea09f74335b278c2d281a21a2a340e44e61e2 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 30 Mar 2026 15:51:26 -0400 Subject: [PATCH] fix: add gsd_requirement_save and upsert path for requirement updates (#3249) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: add gsd_requirement_save tool and upsert path for gsd_requirement_update (#2919) gsd_requirement_update returned not_found for all requirements because requirements written to REQUIREMENTS.md were never inserted into the DB, and no create path existed. This adds: - saveRequirementToDb() + nextRequirementId() in db-writer.ts (symmetric to saveDecisionToDb/nextDecisionId) - gsd_requirement_save tool in db-tools.ts with auto-assigned IDs - Upsert behavior in updateRequirementInDb() — creates a skeleton row when the requirement ID is not in the DB instead of throwing Co-Authored-By: Claude Opus 4.6 * fix: add null check before reverting requirement on disk write failure The `existing` variable from `getRequirementById` can be null when the requirement was newly created (not previously in DB). Guard the revert call to avoid passing null to `upsertRequirement`. Fixes TypeScript error: 'Requirement | null' is not assignable to 'Requirement' Co-Authored-By: Claude Opus 4.6 --------- Co-authored-by: Claude Opus 4.6 --- .../extensions/gsd/bootstrap/db-tools.ts | 93 ++++++++++- src/resources/extensions/gsd/db-writer.ts | 147 +++++++++++++++++- .../extensions/gsd/tests/db-writer.test.ts | 19 +-- .../extensions/gsd/tests/gsd-tools.test.ts | 137 ++++++++++++++-- .../extensions/gsd/tests/tool-naming.test.ts | 3 +- 5 files changed, 359 insertions(+), 40 deletions(-) diff --git a/src/resources/extensions/gsd/bootstrap/db-tools.ts b/src/resources/extensions/gsd/bootstrap/db-tools.ts index 8e6e490d2..d4ebe91dc 100644 --- a/src/resources/extensions/gsd/bootstrap/db-tools.ts +++ b/src/resources/extensions/gsd/bootstrap/db-tools.ts @@ -121,14 +121,6 @@ export function registerDbTools(pi: ExtensionAPI): void { }; } try { - const db = await import("../gsd-db.js"); - const existing = db.getRequirementById(params.id); - if (!existing) { - return { - content: [{ type: "text" as const, text: `Error: Requirement ${params.id} not found.` }], - details: { operation: "update_requirement", id: params.id, error: "not_found" } as any, - }; - } const { updateRequirementInDb } = await import("../db-writer.js"); const updates: Record = {}; if (params.status !== undefined) updates.status = params.status; @@ -196,6 +188,91 @@ export function registerDbTools(pi: ExtensionAPI): void { pi.registerTool(requirementUpdateTool); registerAlias(pi, requirementUpdateTool, "gsd_update_requirement", "gsd_requirement_update"); + // ─── gsd_requirement_save ───────────────────────────────────────────── + + const requirementSaveExecute = async (_toolCallId: string, params: any, _signal: AbortSignal | undefined, _onUpdate: unknown, _ctx: unknown) => { + const dbAvailable = await ensureDbOpen(); + if (!dbAvailable) { + return { + content: [{ type: "text" as const, text: "Error: GSD database is not available. Cannot save requirement." }], + details: { operation: "save_requirement", error: "db_unavailable" } as any, + }; + } + try { + const { saveRequirementToDb } = await import("../db-writer.js"); + const result = await saveRequirementToDb( + { + class: params.class, + status: params.status, + description: params.description, + why: params.why, + source: params.source, + primary_owner: params.primary_owner, + supporting_slices: params.supporting_slices, + validation: params.validation, + notes: params.notes, + }, + process.cwd(), + ); + return { + content: [{ type: "text" as const, text: `Saved requirement ${result.id}` }], + details: { operation: "save_requirement", id: result.id } as any, + }; + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + logError("tool", `gsd_requirement_save tool failed: ${msg}`, { tool: "gsd_requirement_save", error: String(err) }); + return { + content: [{ type: "text" as const, text: `Error saving requirement: ${msg}` }], + details: { operation: "save_requirement", error: msg } as any, + }; + } + }; + + const requirementSaveTool = { + name: "gsd_requirement_save", + label: "Save Requirement", + description: + "Record a new requirement to the GSD database and regenerate REQUIREMENTS.md. " + + "Requirement IDs are auto-assigned — never provide an ID manually.", + promptSnippet: "Record a new GSD requirement to the database (auto-assigns ID, regenerates REQUIREMENTS.md)", + promptGuidelines: [ + "Use gsd_requirement_save when recording a new functional, non-functional, or operational requirement.", + "Requirement IDs are auto-assigned (R001, R002, ...) — never guess or provide an ID.", + "class, description, why, and source are required. All other fields are optional.", + "The tool writes to the DB and regenerates .gsd/REQUIREMENTS.md automatically.", + ], + parameters: Type.Object({ + class: Type.String({ description: "Requirement class (e.g. 'functional', 'non-functional', 'operational')" }), + description: Type.String({ description: "Short description of the requirement" }), + why: Type.String({ description: "Why this requirement matters" }), + source: Type.String({ description: "Origin of the requirement (e.g. 'user-research', 'design', 'M001')" }), + status: Type.Optional(Type.String({ description: "Status (default: 'active')" })), + primary_owner: Type.Optional(Type.String({ description: "Primary owning slice" })), + supporting_slices: Type.Optional(Type.String({ description: "Supporting slices" })), + validation: Type.Optional(Type.String({ description: "Validation criteria" })), + notes: Type.Optional(Type.String({ description: "Additional notes" })), + }), + execute: requirementSaveExecute, + renderCall(args: any, theme: any) { + let text = theme.fg("toolTitle", theme.bold("requirement_save ")); + if (args.class) text += theme.fg("accent", `[${args.class}] `); + if (args.description) text += theme.fg("muted", args.description); + return new Text(text, 0, 0); + }, + renderResult(result: any, _options: any, theme: any) { + const d = result.details; + if (result.isError || d?.error) { + return new Text(theme.fg("error", `Error: ${d?.error ?? "unknown"}`), 0, 0); + } + let text = theme.fg("success", `Requirement ${d?.id ?? ""} saved`); + text += theme.fg("dim", ` → REQUIREMENTS.md`); + return new Text(text, 0, 0); + }, + }; + + pi.registerTool(requirementSaveTool); + registerAlias(pi, requirementSaveTool, "gsd_save_requirement", "gsd_requirement_save"); + // ─── gsd_summary_save (formerly gsd_save_summary) ────────────────────── const summarySaveExecute = async (_toolCallId: string, params: any, _signal: AbortSignal | undefined, _onUpdate: unknown, _ctx: unknown) => { diff --git a/src/resources/extensions/gsd/db-writer.ts b/src/resources/extensions/gsd/db-writer.ts index 489b0d915..5ff97479a 100644 --- a/src/resources/extensions/gsd/db-writer.ts +++ b/src/resources/extensions/gsd/db-writer.ts @@ -227,6 +227,122 @@ export async function nextDecisionId(): Promise { } } +// ─── Next Requirement ID ───────────────────────────────────────────────── + +/** + * Compute the next requirement ID from the current DB state. + * Queries MAX(CAST(SUBSTR(id, 2) AS INTEGER)) from requirements table. + * Returns R001 if no requirements exist. Zero-pads to 3 digits. + */ +export async function nextRequirementId(): Promise { + try { + const db = await import('./gsd-db.js'); + const adapter = db._getAdapter(); + if (!adapter) return 'R001'; + + 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; + if (maxNum == null || isNaN(maxNum)) return 'R001'; + + const next = maxNum + 1; + return `R${String(next).padStart(3, '0')}`; + } catch (err) { + logError('manifest', 'nextRequirementId failed', { fn: 'nextRequirementId', error: String((err as Error).message) }); + return 'R001'; + } +} + +// ─── Save Requirement to DB + Regenerate Markdown ──────────────────────── + +export interface SaveRequirementFields { + class: string; + status?: string; + description: string; + why: string; + source: string; + primary_owner?: string; + supporting_slices?: string; + validation?: string; + notes?: string; +} + +/** + * Save a new requirement to DB and regenerate REQUIREMENTS.md. + * Auto-assigns the next ID via nextRequirementId(). + * Returns the assigned ID. + */ +export async function saveRequirementToDb( + fields: SaveRequirementFields, + basePath: string, +): Promise<{ id: string }> { + try { + const db = await import('./gsd-db.js'); + + const id = await nextRequirementId(); + + 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, + }; + + db.upsertRequirement(requirement); + + // Fetch all requirements for full file regeneration + const adapter = db._getAdapter(); + let allRequirements: Requirement[] = []; + if (adapter) { + const rows = adapter.prepare('SELECT * FROM requirements ORDER BY id').all(); + allRequirements = rows.map(row => ({ + id: row['id'] as string, + class: row['class'] as string, + status: row['status'] as string, + description: row['description'] as string, + why: row['why'] as string, + source: row['source'] as string, + primary_owner: row['primary_owner'] as string, + supporting_slices: row['supporting_slices'] as string, + validation: row['validation'] as string, + notes: row['notes'] as string, + full_content: row['full_content'] as string, + superseded_by: (row['superseded_by'] as string) ?? null, + })); + } + + const nonSuperseded = allRequirements.filter(r => r.superseded_by == null); + const md = generateRequirementsMd(nonSuperseded); + const filePath = resolveGsdRootFile(basePath, 'REQUIREMENTS'); + try { + await saveFile(filePath, md); + } catch (diskErr) { + logError('manifest', 'disk write failed, rolling back DB row', { fn: 'saveRequirementToDb', error: String((diskErr as Error).message) }); + const rollbackAdapter = db._getAdapter(); + rollbackAdapter?.prepare('DELETE FROM requirements WHERE id = :id').run({ ':id': id }); + throw diskErr; + } + invalidateStateCache(); + clearPathCache(); + clearParseCache(); + + return { id }; + } catch (err) { + logError('manifest', 'saveRequirementToDb failed', { fn: 'saveRequirementToDb', error: String((err as Error).message) }); + throw err; + } +} + // ─── Save Decision to DB + Regenerate Markdown ──────────────────────────── export interface SaveDecisionFields { @@ -344,15 +460,30 @@ export async function updateRequirementInDb( const db = await import('./gsd-db.js'); const existing = db.getRequirementById(id); - if (!existing) { - throw new GSDError(GSD_STALE_STATE, `Requirement ${id} not found`); - } - // Merge updates into existing + // If requirement doesn't exist in DB, create a skeleton and merge updates. + // This handles the case where requirements were written to REQUIREMENTS.md + // but never imported into the database (see #2919). + const base: Requirement = existing ?? { + id, + class: '', + status: 'active', + description: '', + why: '', + source: '', + primary_owner: '', + supporting_slices: '', + validation: '', + notes: '', + full_content: '', + superseded_by: null, + }; + + // Merge updates into existing (or skeleton) const merged: Requirement = { - ...existing, + ...base, ...updates, - id: existing.id, // ID cannot be changed + id: base.id, // ID cannot be changed }; db.upsertRequirement(merged); @@ -388,7 +519,9 @@ export async function updateRequirementInDb( await saveFile(filePath, md); } catch (diskErr) { logError('manifest', 'disk write failed, reverting DB row', { fn: 'updateRequirementInDb', error: String((diskErr as Error).message) }); - db.upsertRequirement(existing); + if (existing) { + db.upsertRequirement(existing); + } throw diskErr; } // Invalidate file-read caches so deriveState() sees the updated markdown. diff --git a/src/resources/extensions/gsd/tests/db-writer.test.ts b/src/resources/extensions/gsd/tests/db-writer.test.ts index 180e8578b..922e070b5 100644 --- a/src/resources/extensions/gsd/tests/db-writer.test.ts +++ b/src/resources/extensions/gsd/tests/db-writer.test.ts @@ -416,23 +416,18 @@ describe('db-writer', () => { } }); - test('updateRequirementInDb — not found', async () => { + test('updateRequirementInDb — upserts when not found (#2919)', async () => { const tmpDir = makeTmpDir(); const dbPath = path.join(tmpDir, '.gsd', 'gsd.db'); openDatabase(dbPath); try { - let threw = false; - try { - await updateRequirementInDb('R999', { status: 'validated' }, tmpDir); - } catch (err) { - threw = true; - assert.ok( - (err as Error).message.includes('R999'), - 'error message mentions the missing ID', - ); - } - assert.ok(threw, 'throws when requirement not found'); + // Previously threw; now upserts a skeleton requirement with the provided updates + await updateRequirementInDb('R999', { status: 'validated' }, tmpDir); + const created = getRequirementById('R999'); + assert.ok(created !== null, 'R999 should be created by upsert'); + assert.deepStrictEqual(created!.status, 'validated', 'Upserted requirement should have validated status'); + assert.deepStrictEqual(created!.id, 'R999', 'Upserted requirement should keep the provided ID'); } finally { closeDatabase(); cleanupDir(tmpDir); diff --git a/src/resources/extensions/gsd/tests/gsd-tools.test.ts b/src/resources/extensions/gsd/tests/gsd-tools.test.ts index ef1dedd11..e86f69b98 100644 --- a/src/resources/extensions/gsd/tests/gsd-tools.test.ts +++ b/src/resources/extensions/gsd/tests/gsd-tools.test.ts @@ -21,8 +21,10 @@ import { import { saveDecisionToDb, updateRequirementInDb, + saveRequirementToDb, saveArtifactToDb, nextDecisionId, + nextRequirementId, } from '../db-writer.ts'; import type { Requirement } from '../types.ts'; @@ -160,18 +162,11 @@ describe('gsd-tools', () => { assert.ok(mdContent.includes('R001'), 'REQUIREMENTS.md should contain R001'); assert.ok(mdContent.includes('validated'), 'REQUIREMENTS.md should reflect updated status'); - // Updating non-existent requirement throws - let threwForMissing = false; - try { - await updateRequirementInDb('R999', { status: 'deferred' }, tmpDir); - } catch (err) { - threwForMissing = true; - assert.ok( - (err as Error).message.includes('R999'), - 'Error should mention the missing requirement ID', - ); - } - assert.ok(threwForMissing, 'Should throw for non-existent requirement'); + // Updating non-existent requirement upserts (creates it) — see #2919 + await updateRequirementInDb('R999', { status: 'deferred' }, tmpDir); + const upserted = getRequirementById('R999'); + assert.ok(upserted !== null, 'R999 should be created by upsert'); + assert.deepStrictEqual(upserted!.status, 'deferred', 'Upserted requirement should have the updated status'); closeDatabase(); } finally { @@ -263,6 +258,124 @@ describe('gsd-tools', () => { assert.deepStrictEqual(fallbackId, 'D001', 'nextDecisionId should return D001 when DB unavailable'); }); + test('gsd_requirement_save creates new requirement', async () => { + const tmpDir = makeTmpDir(); + try { + const dbPath = path.join(tmpDir, '.gsd', 'gsd.db'); + openDatabase(dbPath); + + // (a) saveRequirementToDb creates a new requirement with auto-assigned ID + const result = await saveRequirementToDb( + { + class: 'functional', + status: 'active', + description: 'Must support dark mode', + why: 'Accessibility requirement', + source: 'user-research', + }, + tmpDir, + ); + + assert.deepStrictEqual(result.id, 'R001', 'First requirement should be R001'); + + // Verify DB row exists + const row = getRequirementById('R001'); + assert.ok(row !== null, 'Requirement R001 should exist in DB'); + assert.deepStrictEqual(row!.class, 'functional', 'Class should match'); + assert.deepStrictEqual(row!.description, 'Must support dark mode', 'Description should match'); + assert.deepStrictEqual(row!.status, 'active', 'Status should match'); + + // Verify REQUIREMENTS.md was generated + const mdPath = path.join(tmpDir, '.gsd', 'REQUIREMENTS.md'); + assert.ok(fs.existsSync(mdPath), 'REQUIREMENTS.md should be created'); + const mdContent = fs.readFileSync(mdPath, 'utf-8'); + assert.ok(mdContent.includes('R001'), 'REQUIREMENTS.md should contain R001'); + assert.ok(mdContent.includes('dark mode'), 'REQUIREMENTS.md should contain description'); + + // (b) Auto-assigns correct next ID + const result2 = await saveRequirementToDb( + { + class: 'non-functional', + status: 'active', + description: 'Must load in under 2 seconds', + why: 'Performance SLA', + source: 'design', + }, + tmpDir, + ); + assert.deepStrictEqual(result2.id, 'R002', 'Second requirement should be R002'); + + closeDatabase(); + } finally { + cleanupDir(tmpDir); + } + }); + + test('nextRequirementId computes correct next ID', async () => { + const tmpDir = makeTmpDir(); + try { + const dbPath = path.join(tmpDir, '.gsd', 'gsd.db'); + openDatabase(dbPath); + + // No requirements yet + const id1 = await nextRequirementId(); + assert.deepStrictEqual(id1, 'R001', 'Should return R001 when no requirements exist'); + + // Add one requirement + upsertRequirement({ + id: 'R001', + class: 'functional', + status: 'active', + description: 'Test', + why: '', + source: '', + primary_owner: '', + supporting_slices: '', + validation: '', + notes: '', + full_content: '', + superseded_by: null, + }); + + const id2 = await nextRequirementId(); + assert.deepStrictEqual(id2, 'R002', 'Should return R002 after R001 exists'); + + closeDatabase(); + } finally { + cleanupDir(tmpDir); + } + }); + + test('gsd_requirement_update upserts when requirement not in DB', async () => { + const tmpDir = makeTmpDir(); + try { + const dbPath = path.join(tmpDir, '.gsd', 'gsd.db'); + openDatabase(dbPath); + + // Requirement R025 does NOT exist in DB — simulates the bug scenario + // where requirements exist in REQUIREMENTS.md but were never imported. + // updateRequirementInDb should create the row instead of throwing. + await updateRequirementInDb( + 'R025', + { status: 'validated', validation: 'Integration tests pass' }, + tmpDir, + ); + + const created = getRequirementById('R025'); + assert.ok(created !== null, 'R025 should be created by upsert'); + assert.deepStrictEqual(created!.status, 'validated', 'Status should be set'); + assert.deepStrictEqual(created!.validation, 'Integration tests pass', 'Validation should be set'); + + // Verify REQUIREMENTS.md was generated + const mdPath = path.join(tmpDir, '.gsd', 'REQUIREMENTS.md'); + assert.ok(fs.existsSync(mdPath), 'REQUIREMENTS.md should be created'); + + closeDatabase(); + } finally { + cleanupDir(tmpDir); + } + }); + test('Tool result format', async () => { const tmpDir = makeTmpDir(); try { diff --git a/src/resources/extensions/gsd/tests/tool-naming.test.ts b/src/resources/extensions/gsd/tests/tool-naming.test.ts index 772a4eed6..b7e333cff 100644 --- a/src/resources/extensions/gsd/tests/tool-naming.test.ts +++ b/src/resources/extensions/gsd/tests/tool-naming.test.ts @@ -24,6 +24,7 @@ function makeMockPi() { const RENAME_MAP: Array<{ canonical: string; alias: string }> = [ { canonical: "gsd_decision_save", alias: "gsd_save_decision" }, { canonical: "gsd_requirement_update", alias: "gsd_update_requirement" }, + { canonical: "gsd_requirement_save", alias: "gsd_save_requirement" }, { canonical: "gsd_summary_save", alias: "gsd_save_summary" }, { canonical: "gsd_milestone_generate_id", alias: "gsd_generate_milestone_id" }, { canonical: "gsd_task_complete", alias: "gsd_complete_task" }, @@ -44,7 +45,7 @@ console.log('\n── Tool naming: registration count ──'); const pi = makeMockPi(); registerDbTools(pi); -assert.deepStrictEqual(pi.tools.length, 27, 'Should register exactly 27 tools (13 canonical + 13 aliases + 1 gate tool)'); +assert.deepStrictEqual(pi.tools.length, 29, 'Should register exactly 29 tools (14 canonical + 14 aliases + 1 gate tool)'); // ─── Both names exist for each pair ──────────────────────────────────────────