fix: parse hook/* completed-unit keys correctly in forensics + doctor (#2826) (#3252)

The `key.indexOf("/")` split broke compound hook types like
"hook/telegram-progress/M007/S01", yielding unitType="hook" instead of
"hook/telegram-progress". This bypassed the `startsWith("hook/")` guard
in verifyExpectedArtifact, producing false-positive missing-artifact
errors for every hook unit.

Extract a shared `splitCompletedKey()` helper that handles the two-segment
hook prefix and use it in both `detectMissingArtifacts` (forensics.ts) and
the orphaned-key check (doctor-runtime-checks.ts).

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

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/<hookName>/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");

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/<hookName>"), 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/<hookName>/<unitId...>"
+    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;

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/<hookName> 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",
+    );
+  });
+});