fix(gsd): address Codex adversarial review findings for #3565

- verificationEvidence coercion now uses sentinel values (exitCode: -1,
  verdict: "unknown") instead of fabricating passing results
- String coercion for requirements fields now parses "ID — detail"
  delimiter format to preserve semantic payload
- Added regression tests for sentinel values and delimiter parsing

Closes #3565
This commit is contained in:
Jeremy 2026-04-05 13:30:09 -05:00
parent 0742cf3493
commit 6046a31c6f
2 changed files with 92 additions and 28 deletions

View file

@ -707,7 +707,7 @@ export function registerDbTools(pi: ExtensionAPI): void {
// Coerce string items to objects for verificationEvidence (#3541).
const coerced = { ...params };
coerced.verificationEvidence = (params.verificationEvidence ?? []).map((v: any) =>
typeof v === "string" ? { command: v, exitCode: 0, verdict: "pass", durationMs: 0 } : v,
typeof v === "string" ? { command: v, exitCode: -1, verdict: "unknown (coerced from string)", durationMs: 0 } : v,
);
const { handleCompleteTask } = await import("../tools/complete-task.js");
@ -798,22 +798,37 @@ export function registerDbTools(pi: ExtensionAPI): void {
try {
// Coerce string items to objects for fields where LLMs sometimes pass
// plain strings instead of the expected { key, value } shape (#3541).
// Parses "key — value" or "key - value" format when possible.
const splitPair = (s: string): [string, string] => {
const m = s.match(/^(.+?)\s*(?:—|-)\s+(.+)$/);
return m ? [m[1].trim(), m[2].trim()] : [s.trim(), ""];
};
const coerced = { ...params };
coerced.filesModified = (params.filesModified ?? []).map((f: any) =>
typeof f === "string" ? { path: f, description: "" } : f,
);
coerced.requires = (params.requires ?? []).map((r: any) =>
typeof r === "string" ? { slice: r, provides: "" } : r,
);
coerced.requirementsAdvanced = (params.requirementsAdvanced ?? []).map((r: any) =>
typeof r === "string" ? { id: r, how: "" } : r,
);
coerced.requirementsValidated = (params.requirementsValidated ?? []).map((r: any) =>
typeof r === "string" ? { id: r, proof: "" } : r,
);
coerced.requirementsInvalidated = (params.requirementsInvalidated ?? []).map((r: any) =>
typeof r === "string" ? { id: r, what: "" } : r,
);
coerced.filesModified = (params.filesModified ?? []).map((f: any) => {
if (typeof f !== "string") return f;
const [path, description] = splitPair(f);
return { path, description };
});
coerced.requires = (params.requires ?? []).map((r: any) => {
if (typeof r !== "string") return r;
const [slice, provides] = splitPair(r);
return { slice, provides };
});
coerced.requirementsAdvanced = (params.requirementsAdvanced ?? []).map((r: any) => {
if (typeof r !== "string") return r;
const [id, how] = splitPair(r);
return { id, how };
});
coerced.requirementsValidated = (params.requirementsValidated ?? []).map((r: any) => {
if (typeof r !== "string") return r;
const [id, proof] = splitPair(r);
return { id, proof };
});
coerced.requirementsInvalidated = (params.requirementsInvalidated ?? []).map((r: any) => {
if (typeof r !== "string") return r;
const [id, what] = splitPair(r);
return { id, what };
});
const { handleCompleteSlice } = await import("../tools/complete-slice.js");
const result = await handleCompleteSlice(coerced, process.cwd());

View file

@ -422,32 +422,81 @@ console.log('\n=== complete-slice: handler accepts string-coerced arrays (#3541)
insertSlice({ id: 'S01', milestoneId: 'M001' });
insertTask({ id: 'T01', sliceId: 'S01', milestoneId: 'M001', status: 'complete', title: 'Task 1' });
// Simulate LLM passing strings instead of objects — coerced before handler
// Simulate the coercion logic from sliceCompleteExecute: parse "key — value" format
const splitPair = (s: string): [string, string] => {
const m = s.match(/^(.+?)\s*(?:—|-)\s+(.+)$/);
return m ? [m[1].trim(), m[2].trim()] : [s.trim(), ""];
};
const params = makeValidSliceParams();
const coerced = { ...params };
coerced.filesModified = ['src/foo.ts', 'src/bar.ts'].map((f: string) =>
({ path: f, description: '' }),
);
coerced.requires = ['S00'].map((r: string) =>
({ slice: r, provides: '' }),
);
coerced.requirementsAdvanced = ['R001'].map((r: string) =>
({ id: r, how: '' }),
);
// Plain strings without delimiter — second field should be empty
coerced.filesModified = ['src/foo.ts', 'src/bar.ts'].map((f: string) => {
const [p, d] = splitPair(f);
return { path: p, description: d };
});
assertEq(coerced.filesModified[0].path, 'src/foo.ts', 'plain string: path preserved');
assertEq(coerced.filesModified[0].description, '', 'plain string: description empty');
// Strings with "—" delimiter — should parse both parts
coerced.requirementsAdvanced = ['R001 — Handler validates task completion'].map((r: string) => {
const [id, how] = splitPair(r);
return { id, how };
});
assertEq(coerced.requirementsAdvanced[0].id, 'R001', 'delimited string: id parsed');
assertEq(coerced.requirementsAdvanced[0].how, 'Handler validates task completion', 'delimited string: how parsed');
// Strings with "- " delimiter — should also parse
coerced.requirementsValidated = ['R002 - Tests pass'].map((r: string) => {
const [id, proof] = splitPair(r);
return { id, proof };
});
assertEq(coerced.requirementsValidated[0].id, 'R002', 'hyphen delimiter: id parsed');
assertEq(coerced.requirementsValidated[0].proof, 'Tests pass', 'hyphen delimiter: proof parsed');
coerced.requires = ['S00'].map((r: string) => {
const [slice, provides] = splitPair(r);
return { slice, provides };
});
coerced.requirementsInvalidated = [];
const result = await handleCompleteSlice(coerced, basePath);
assertTrue(!('error' in result), 'handler should succeed with coerced string arrays');
if (!('error' in result)) {
// Verify SUMMARY.md renders without crashing on coerced fields
const summaryContent = fs.readFileSync(result.summaryPath, 'utf-8');
assertMatch(summaryContent, /src\/foo\.ts/, 'summary should list coerced file path');
assertMatch(summaryContent, /R001/, 'summary should list coerced requirement');
assertMatch(summaryContent, /R001/, 'summary should list coerced requirement id');
assertMatch(summaryContent, /Handler validates task completion/, 'summary should list parsed requirement detail');
}
cleanupDir(basePath);
cleanup(dbPath);
}
// ═══════════════════════════════════════════════════════════════════════════
// complete-slice: verificationEvidence coercion uses sentinel values (#3541)
// ═══════════════════════════════════════════════════════════════════════════
console.log('\n=== complete-slice: verificationEvidence sentinel values (#3541) ===');
{
// Verify the coercion logic produces non-passing sentinel values
const coerce = (v: any) =>
typeof v === "string" ? { command: v, exitCode: -1, verdict: "unknown (coerced from string)", durationMs: 0 } : v;
const coerced = coerce("npm test");
assertEq(coerced.command, 'npm test', 'sentinel: command preserved');
assertEq(coerced.exitCode, -1, 'sentinel: exitCode is -1, not 0');
assertEq(coerced.verdict, 'unknown (coerced from string)', 'sentinel: verdict is unknown, not pass');
assertEq(coerced.durationMs, 0, 'sentinel: durationMs is 0');
// Object inputs pass through unchanged
const obj = { command: "npm test", exitCode: 0, verdict: "pass", durationMs: 1234 };
const passthrough = coerce(obj);
assertEq(passthrough.exitCode, 0, 'object passthrough: exitCode unchanged');
assertEq(passthrough.verdict, 'pass', 'object passthrough: verdict unchanged');
}
// ═══════════════════════════════════════════════════════════════════════════
// complete-slice: step 13 specifies write tool for PROJECT.md (#2946)
// ═══════════════════════════════════════════════════════════════════════════