diff --git a/src/resources/extensions/gsd/doctor-runtime-checks.ts b/src/resources/extensions/gsd/doctor-runtime-checks.ts index 1137981a7..d2af2bd9a 100644 --- a/src/resources/extensions/gsd/doctor-runtime-checks.ts +++ b/src/resources/extensions/gsd/doctor-runtime-checks.ts @@ -119,10 +119,11 @@ export async function checkRuntimeHealth( for (const key of keys) { // Key format: "unitType/unitId" e.g. "execute-task/M001/S01/T01" - const slashIdx = key.indexOf("/"); - if (slashIdx === -1) continue; - const unitType = key.slice(0, slashIdx); - const unitId = key.slice(slashIdx + 1); + // Hook units have compound types: "hook//unitId" + const { splitCompletedKey } = await import("./forensics.js"); + const parsed = splitCompletedKey(key); + if (!parsed) continue; + const { unitType, unitId } = parsed; // Only validate artifact-producing unit types const { verifyExpectedArtifact } = await import("./auto-recovery.js"); diff --git a/src/resources/extensions/gsd/forensics.ts b/src/resources/extensions/gsd/forensics.ts index afe5df0b0..dedf2b331 100644 --- a/src/resources/extensions/gsd/forensics.ts +++ b/src/resources/extensions/gsd/forensics.ts @@ -650,15 +650,42 @@ function detectTimeouts(traces: UnitTrace[], anomalies: ForensicAnomaly[]): void } } +/** + * Parse a completed-unit key into its unitType and unitId. + * + * Hook units use a compound slash-delimited type ("hook/"), so a + * naive `key.indexOf("/")` would split "hook/telegram-progress/M007/S01" into + * unitType="hook" (wrong) instead of "hook/telegram-progress". + * + * Returns `null` for malformed keys that cannot be split. + */ +export function splitCompletedKey(key: string): { unitType: string; unitId: string } | null { + if (key.startsWith("hook/")) { + // Hook unit types are two segments: "hook//" + const secondSlash = key.indexOf("/", 5); // skip past "hook/" + if (secondSlash === -1) return null; // malformed — no unitId after hook name + return { + unitType: key.slice(0, secondSlash), + unitId: key.slice(secondSlash + 1), + }; + } + + const slashIdx = key.indexOf("/"); + if (slashIdx === -1) return null; + return { + unitType: key.slice(0, slashIdx), + unitId: key.slice(slashIdx + 1), + }; +} + function detectMissingArtifacts(completedKeys: string[], basePath: string, activeMilestone: string | null, anomalies: ForensicAnomaly[]): void { // Also check the worktree path for artifacts — they may exist there but not at root const wtBasePath = activeMilestone ? getAutoWorktreePath(basePath, activeMilestone) : null; for (const key of completedKeys) { - const slashIdx = key.indexOf("/"); - if (slashIdx === -1) continue; - const unitType = key.slice(0, slashIdx); - const unitId = key.slice(slashIdx + 1); + const parsed = splitCompletedKey(key); + if (!parsed) continue; + const { unitType, unitId } = parsed; const rootHasArtifact = verifyExpectedArtifact(unitType, unitId, basePath); const wtHasArtifact = wtBasePath ? verifyExpectedArtifact(unitType, unitId, wtBasePath) : false; diff --git a/src/resources/extensions/gsd/tests/hook-key-parsing.test.ts b/src/resources/extensions/gsd/tests/hook-key-parsing.test.ts new file mode 100644 index 000000000..42424ad50 --- /dev/null +++ b/src/resources/extensions/gsd/tests/hook-key-parsing.test.ts @@ -0,0 +1,107 @@ +import { describe, it } from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { join, dirname } from "node:path"; +import { fileURLToPath } from "node:url"; + +const __dirname = dirname(fileURLToPath(import.meta.url)); +const gsdDir = join(__dirname, ".."); + +/** + * Regression tests for #2826: hook/* completed-unit keys were parsed + * incorrectly by forensics + doctor, causing false-positive missing-artifact + * errors for all hook units. + * + * The root cause: `key.indexOf("/")` splits "hook/telegram-progress/M007/S01" + * into unitType="hook" + unitId="telegram-progress/M007/S01" instead of + * unitType="hook/telegram-progress" + unitId="M007/S01". + */ + +describe("splitCompletedKey (#2826)", () => { + it("is exported from forensics.ts", () => { + const source = readFileSync(join(gsdDir, "forensics.ts"), "utf-8"); + assert.ok( + source.includes("export function splitCompletedKey"), + "forensics.ts must export splitCompletedKey helper", + ); + }); + + it("splits simple unit types correctly", async () => { + const { splitCompletedKey } = await import("../forensics.ts"); + const result = splitCompletedKey("execute-task/M007/S01/T01"); + assert.deepStrictEqual(result, { + unitType: "execute-task", + unitId: "M007/S01/T01", + }); + }); + + it("splits hook unit types preserving the compound hook/ prefix", async () => { + const { splitCompletedKey } = await import("../forensics.ts"); + const result = splitCompletedKey("hook/telegram-progress/M007/S01"); + assert.deepStrictEqual(result, { + unitType: "hook/telegram-progress", + unitId: "M007/S01", + }); + }); + + it("splits hook unit types with task-level unitId", async () => { + const { splitCompletedKey } = await import("../forensics.ts"); + const result = splitCompletedKey("hook/telegram-progress/M007/S02/T01"); + assert.deepStrictEqual(result, { + unitType: "hook/telegram-progress", + unitId: "M007/S02/T01", + }); + }); + + it("returns null for malformed keys without a slash", async () => { + const { splitCompletedKey } = await import("../forensics.ts"); + assert.strictEqual(splitCompletedKey("noslash"), null); + }); + + it("returns null for malformed hook keys with only 'hook/' and no more segments", async () => { + const { splitCompletedKey } = await import("../forensics.ts"); + // "hook/someName" has no unitId segment after the hook name + assert.strictEqual(splitCompletedKey("hook/someName"), null); + }); +}); + +describe("forensics detectMissingArtifacts uses splitCompletedKey (#2826)", () => { + it("does not use indexOf for key splitting", () => { + const source = readFileSync(join(gsdDir, "forensics.ts"), "utf-8"); + // Extract only the detectMissingArtifacts function body + const fnStart = source.indexOf("function detectMissingArtifacts"); + assert.ok(fnStart !== -1, "detectMissingArtifacts must exist"); + const fnBody = source.slice(fnStart, source.indexOf("\n}\n", fnStart) + 3); + + assert.ok( + !fnBody.includes('key.indexOf("/")'), + "detectMissingArtifacts must not use key.indexOf('/') — use splitCompletedKey instead", + ); + assert.ok( + fnBody.includes("splitCompletedKey"), + "detectMissingArtifacts must use splitCompletedKey helper", + ); + }); +}); + +describe("doctor-runtime-checks uses splitCompletedKey (#2826)", () => { + it("does not use indexOf for key splitting in orphaned-key check", () => { + const source = readFileSync( + join(gsdDir, "doctor-runtime-checks.ts"), + "utf-8", + ); + // Find the orphaned completed-units section + const sectionStart = source.indexOf("Orphaned completed-units"); + assert.ok(sectionStart !== -1, "orphaned completed-units section must exist"); + const sectionBody = source.slice(sectionStart, source.indexOf("} catch", sectionStart)); + + assert.ok( + !sectionBody.includes('key.indexOf("/")'), + "doctor orphaned-key check must not use key.indexOf('/') — use splitCompletedKey instead", + ); + assert.ok( + sectionBody.includes("splitCompletedKey"), + "doctor orphaned-key check must use splitCompletedKey helper", + ); + }); +});