diff --git a/src/resources/extensions/gsd/pre-execution-checks.ts b/src/resources/extensions/gsd/pre-execution-checks.ts index f0a4b692e..7d682b6bf 100644 --- a/src/resources/extensions/gsd/pre-execution-checks.ts +++ b/src/resources/extensions/gsd/pre-execution-checks.ts @@ -237,9 +237,12 @@ export async function checkPackageExistence( */ export function normalizeFilePath(filePath: string): string { if (!filePath) return filePath; - + + // Strip backtick wrapping from LLM-generated paths (#3649) + let normalized = filePath.replace(/`/g, ""); + // Normalize path separators to forward slashes - let normalized = filePath.replace(/\\/g, "/"); + normalized = normalized.replace(/\\/g, "/"); // Remove leading ./ while (normalized.startsWith("./")) { @@ -272,10 +275,13 @@ function getExpectedOutputsUpTo(tasks: TaskRow[], taskIndex: number): Set = { function validatePlanSlice(content: string): ContentViolation[] { const violations: ContentViolation[] = []; - // Must have at least 2 task entries (checkbox pattern) + // Must have at least 1 task entry — single-task slices are valid (#3649) const taskCount = (content.match(/- \[[ x]\] \*\*T\d+/g) || []).length; - if (taskCount < 2) { + if (taskCount < 1) { violations.push({ severity: "warning", - reason: `Slice plan has only ${taskCount} task(s) — expected at least 2`, + reason: `Slice plan has ${taskCount} task(s) — expected at least 1`, }); } diff --git a/src/resources/extensions/gsd/tests/pre-exec-backtick-strip.test.ts b/src/resources/extensions/gsd/tests/pre-exec-backtick-strip.test.ts new file mode 100644 index 000000000..ffdeae7c8 --- /dev/null +++ b/src/resources/extensions/gsd/tests/pre-exec-backtick-strip.test.ts @@ -0,0 +1,68 @@ +/** + * Regression test for #3626 / #3649 — pre-execution-checks false positives + * + * Two sources of false positives were fixed: + * 1. normalizeFilePath did not strip backtick wrapping from LLM-generated + * paths like `src/foo.ts`, causing file-existence checks to fail (#3649). + * 2. checkFilePathConsistency checked both task.files and task.inputs, but + * task.files ("files likely touched") intentionally includes files that + * will be created by the task, so they don't need to pre-exist (#3626). + */ + +import { describe, it } from 'node:test' +import assert from 'node:assert/strict' +import { normalizeFilePath, checkFilePathConsistency } from '../pre-execution-checks.ts' +import { readFileSync } from 'node:fs' +import { resolve } from 'node:path' + +const src = readFileSync( + resolve(process.cwd(), 'src', 'resources', 'extensions', 'gsd', 'pre-execution-checks.ts'), + 'utf-8', +) + +describe('normalizeFilePath backtick stripping (#3649)', () => { + it('strips backticks from file paths', () => { + assert.equal(normalizeFilePath('`src/foo.ts`'), 'src/foo.ts') + }) + + it('strips backticks even when mixed with other normalization', () => { + assert.equal(normalizeFilePath('`./src//bar.ts`'), 'src/bar.ts') + }) + + it('leaves normal paths unchanged', () => { + assert.equal(normalizeFilePath('src/foo.ts'), 'src/foo.ts') + }) + + it('handles empty string', () => { + assert.equal(normalizeFilePath(''), '') + }) +}) + +describe('checkFilePathConsistency checks task.inputs not task.files (#3626)', () => { + it('source uses only task.inputs in filesToCheck', () => { + // Verify the fix structurally: the spread should be [...task.inputs] only + const fnStart = src.indexOf('export function checkFilePathConsistency(') + assert.ok(fnStart !== -1, 'checkFilePathConsistency function must exist') + + // Find the filesToCheck assignment + const filesToCheckLine = src.indexOf('filesToCheck', fnStart) + assert.ok(filesToCheckLine !== -1, 'filesToCheck assignment must exist') + + // Extract the line + const lineEnd = src.indexOf('\n', filesToCheckLine) + const line = src.slice(filesToCheckLine, lineEnd) + + // Must include task.inputs + assert.ok( + line.includes('task.inputs'), + 'filesToCheck must reference task.inputs', + ) + + // Must NOT include task.files + assert.ok( + !line.includes('task.files'), + 'filesToCheck must NOT reference task.files — files likely touched include ' + + 'files the task will create, so they do not need to pre-exist', + ) + }) +}) diff --git a/src/resources/extensions/gsd/tests/pre-execution-checks.test.ts b/src/resources/extensions/gsd/tests/pre-execution-checks.test.ts index c9b18f24a..86fc8c5f6 100644 --- a/src/resources/extensions/gsd/tests/pre-execution-checks.test.ts +++ b/src/resources/extensions/gsd/tests/pre-execution-checks.test.ts @@ -193,7 +193,7 @@ describe("checkFilePathConsistency", () => { } }); - test("fails when files don't exist and not in prior outputs", () => { + test("fails when inputs don't exist and not in prior outputs", () => { tempDir = join(tmpdir(), `pre-exec-test-${Date.now()}`); mkdirSync(tempDir, { recursive: true }); @@ -201,8 +201,8 @@ describe("checkFilePathConsistency", () => { const tasks = [ createTask({ id: "T01", - files: ["nonexistent.ts"], - inputs: [], + files: [], + inputs: ["nonexistent.ts"], expected_output: [], }), ]; @@ -218,7 +218,7 @@ describe("checkFilePathConsistency", () => { } }); - test("checks both files and inputs arrays", () => { + test("checks only inputs array, not files array", () => { tempDir = join(tmpdir(), `pre-exec-test-${Date.now()}`); mkdirSync(tempDir, { recursive: true }); @@ -232,10 +232,13 @@ describe("checkFilePathConsistency", () => { }), ]; + // Only inputs are checked — files ("files likely touched") are excluded + // because they may include files the task will create (#3626) const results = checkFilePathConsistency(tasks, tempDir); - assert.equal(results.length, 2); - assert.ok(results.some((r) => r.target === "missing-file.ts")); + assert.equal(results.length, 1); assert.ok(results.some((r) => r.target === "missing-input.ts")); + // missing-file.ts should NOT produce a failure + assert.ok(!results.some((r) => r.target === "missing-file.ts")); } finally { rmSync(tempDir, { recursive: true, force: true }); } @@ -825,8 +828,8 @@ describe("runPreExecutionChecks", () => { const tasks = [ createTask({ id: "T01", - files: ["nonexistent.ts"], - inputs: [], + files: [], + inputs: ["nonexistent.ts"], expected_output: [], }), ]; diff --git a/src/resources/extensions/gsd/tests/pre-execution-fail-closed.test.ts b/src/resources/extensions/gsd/tests/pre-execution-fail-closed.test.ts index 927fe4b7a..f2fec376d 100644 --- a/src/resources/extensions/gsd/tests/pre-execution-fail-closed.test.ts +++ b/src/resources/extensions/gsd/tests/pre-execution-fail-closed.test.ts @@ -225,9 +225,9 @@ describe("Pre-execution fail-closed behavior", () => { planning: { description: "References missing file", estimate: "1h", - files: ["nonexistent-file.ts"], + files: [], verify: "npm test", - inputs: [], + inputs: ["nonexistent-file.ts"], expectedOutput: [], observabilityImpact: "", }, diff --git a/src/resources/extensions/gsd/tests/pre-execution-pause-wiring.test.ts b/src/resources/extensions/gsd/tests/pre-execution-pause-wiring.test.ts index d4de0727a..7a540d86b 100644 --- a/src/resources/extensions/gsd/tests/pre-execution-pause-wiring.test.ts +++ b/src/resources/extensions/gsd/tests/pre-execution-pause-wiring.test.ts @@ -189,9 +189,9 @@ function createFailingTasks(): void { planning: { description: "This task references a non-existent file", estimate: "1h", - files: ["nonexistent-file-that-does-not-exist.ts"], + files: [], verify: "npm test", - inputs: [], + inputs: ["nonexistent-file-that-does-not-exist.ts"], expectedOutput: [], observabilityImpact: "", },