fix: add gsd_requirement_save and upsert path for requirement updates (#3249)
* 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
GitHub
parent
2ea668ee09
commit
01aea09f74
5 changed files with 359 additions and 40 deletions
|
|
@ -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<string, string | undefined> = {};
|
||||
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) => {
|
||||
|
|
|
|||
|
|
@ -227,6 +227,122 @@ export async function nextDecisionId(): Promise<string> {
|
|||
}
|
||||
}
|
||||
|
||||
// ─── 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<string> {
|
||||
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.
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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 ──────────────────────────────────────────
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue