Merge pull request #3677 from Tibsfox/fix/pre-execution-checks-false-positives

fix(gsd): fix pre-execution-checks false positives from backticks and task.files

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<string
 }
 
 /**
- * Check that all files referenced in task.files and task.inputs either:
+ * Check that all files referenced in task.inputs either:
  *   1. Exist on disk, OR
  *   2. Are in a prior task's expected_output
- * 
+ *
+ * task.files ("files likely touched") is excluded — it intentionally includes
+ * files the task will create, so they don't need to pre-exist (#3626).
+ *
  * All paths are normalized before comparison to ensure ./src/a.ts matches src/a.ts.
  */
 export function checkFilePathConsistency(
@@ -287,7 +293,7 @@ export function checkFilePathConsistency(
   for (let i = 0; i < tasks.length; i++) {
     const task = tasks[i];
     const priorOutputs = getExpectedOutputsUpTo(tasks, i);
-    const filesToCheck = [...task.files, ...task.inputs];
+    const filesToCheck = [...task.inputs];
 
     for (const file of filesToCheck) {
       // Skip empty strings

src/resources/extensions/gsd/safety/content-validator.ts

@@ -54,12 +54,12 @@ const VALIDATORS: Record<string, ContentValidatorFn> = {
 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`,
     });
   }
 

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',
+    )
+  })
+})

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: [],
         }),
       ];

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: "",
       },

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: "",
     },