fix: hook orchestration — finalize runtime records, add supervision, fix retry

Hooks were dispatched (runtime record created with phase="dispatched") but
never properly tracked through completion. Four issues fixed:

1. Hook runtime records now finalized: handleAgentEnd writes phase="finalized"
   and clears the record when a hook completes. Previously records stayed at
   "dispatched" forever because verifyExpectedArtifact returned false for
   hook types.

2. Supervision timer for hooks: hook dispatch now sets a hard timeout so
   stuck hooks don't hang auto-mode indefinitely.

3. Hook retry removes completion key: when a hook requests retry via
   retry_on, the trigger unit's completion key is removed from the
   idempotency set so dispatchNextUnit will re-dispatch it.

4. Hook closeout in dispatchNextUnit: hook units are properly closed out
   (pushed to completedUnits, runtime cleared) without polluting the
   idempotency set. verifyExpectedArtifact returns true for hook/ types.

Fixes #140 (comment 4063396798)

src/resources/extensions/gsd/auto.ts

@@ -939,17 +939,37 @@ export async function handleAgentEnd(
     // produced its expected artifact. If so, persist the completion key now so the
     // idempotency check at the top of dispatchNextUnit() skips it — even if
     // deriveState() still returns this unit as active (e.g. branch mismatch).
-    try {
-      if (verifyExpectedArtifact(currentUnit.type, currentUnit.id, basePath)) {
-        const completionKey = `${currentUnit.type}/${currentUnit.id}`;
-        if (!completedKeySet.has(completionKey)) {
-          persistCompletedKey(basePath, completionKey);
-          completedKeySet.add(completionKey);
+    //
+    // IMPORTANT: For non-hook units, defer persistence until after the hook check.
+    // If a post-unit hook requests a retry, we need to remove the completion key
+    // so dispatchNextUnit re-dispatches the trigger unit.
+    let triggerArtifactVerified = false;
+    if (!currentUnit.type.startsWith("hook/")) {
+      try {
+        triggerArtifactVerified = verifyExpectedArtifact(currentUnit.type, currentUnit.id, basePath);
+        if (triggerArtifactVerified) {
+          const completionKey = `${currentUnit.type}/${currentUnit.id}`;
+          if (!completedKeySet.has(completionKey)) {
+            persistCompletedKey(basePath, completionKey);
+            completedKeySet.add(completionKey);
+          }
+          invalidateStateCache();
         }
-        invalidateStateCache();
+      } catch {
+        // Non-fatal — worst case we fall through to normal dispatch which has its own checks
+      }
+    } else {
+      // Hook unit completed — finalize its runtime record and clear it
+      try {
+        writeUnitRuntimeRecord(basePath, currentUnit.type, currentUnit.id, currentUnit.startedAt, {
+          phase: "finalized",
+          progressCount: 1,
+          lastProgressKind: "hook-completed",
+        });
+        clearUnitRuntimeRecord(basePath, currentUnit.type, currentUnit.id);
+      } catch {
+        // Non-fatal
       }
-    } catch {
-      // Non-fatal — worst case we fall through to normal dispatch which has its own checks
     }
   }
 
@@ -1005,6 +1025,31 @@ export async function handleAgentEnd(
       writeLock(basePath, hookUnit.unitType, hookUnit.unitId, completedUnits.length, sessionFile);
       // Persist hook state so cycle counts survive crashes
       persistHookState(basePath);
+
+      // Start supervision timers for hook units — hooks can get stuck just
+      // like normal units, and without a watchdog auto-mode would hang forever.
+      clearUnitTimeout();
+      const supervisor = resolveAutoSupervisorConfig();
+      const hookHardTimeoutMs = (supervisor.hard_timeout_minutes ?? 30) * 60 * 1000;
+      unitTimeoutHandle = setTimeout(async () => {
+        unitTimeoutHandle = null;
+        if (!active) return;
+        if (currentUnit) {
+          writeUnitRuntimeRecord(basePath, hookUnit.unitType, hookUnit.unitId, currentUnit.startedAt, {
+            phase: "timeout",
+            timeoutAt: Date.now(),
+          });
+        }
+        ctx.ui.notify(
+          `Hook ${hookUnit.hookName} exceeded ${supervisor.hard_timeout_minutes ?? 30}min timeout. Pausing auto-mode.`,
+          "warning",
+        );
+        resetHookState();
+        await pauseAuto(ctx, pi);
+      }, hookHardTimeoutMs);
+
+      // Guard against race with timeout/pause before sending
+      if (!active) return;
       pi.sendMessage(
         { customType: "gsd-auto", content: hookUnit.prompt, display: verbose },
         { triggerTurn: true },
@@ -1016,6 +1061,11 @@ export async function handleAgentEnd(
     if (isRetryPending()) {
       const trigger = consumeRetryTrigger();
       if (trigger) {
+        // Remove the trigger unit's completion key so dispatchNextUnit
+        // will re-dispatch it instead of skipping it as already-complete.
+        const triggerKey = `${trigger.unitType}/${trigger.unitId}`;
+        completedKeySet.delete(triggerKey);
+        removePersistedKey(basePath, triggerKey);
         ctx.ui.notify(
           `Hook requested retry of ${trigger.unitType} ${trigger.unitId}.`,
           "info",
@@ -2207,12 +2257,19 @@ async function dispatchNextUnit(
     // Only mark the previous unit as completed if:
     // 1. We're not about to re-dispatch the same unit (retry scenario)
     // 2. The expected artifact actually exists on disk
+    // For hook units, skip artifact verification — hooks don't produce standard
+    // artifacts and their runtime records were already finalized in handleAgentEnd.
     const closeoutKey = `${currentUnit.type}/${currentUnit.id}`;
     const incomingKey = `${unitType}/${unitId}`;
-    const artifactVerified = verifyExpectedArtifact(currentUnit.type, currentUnit.id, basePath);
+    const isHookUnit = currentUnit.type.startsWith("hook/");
+    const artifactVerified = isHookUnit || verifyExpectedArtifact(currentUnit.type, currentUnit.id, basePath);
     if (closeoutKey !== incomingKey && artifactVerified) {
-      persistCompletedKey(basePath, closeoutKey);
-      completedKeySet.add(closeoutKey);
+      if (!isHookUnit) {
+        // Only persist completion keys for real units — hook keys are
+        // ephemeral and should not pollute the idempotency set.
+        persistCompletedKey(basePath, closeoutKey);
+        completedKeySet.add(closeoutKey);
+      }
 
       completedUnits.push({
         type: currentUnit.type,
@@ -3772,6 +3829,10 @@ export function verifyExpectedArtifact(unitType: string, unitId: string, base: s
   // Clear stale directory listing cache so artifact checks see fresh disk state (#431)
   clearPathCache();
 
+  // Hook units have no standard artifact — always pass. Their lifecycle
+  // is managed by the hook engine, not the artifact verification system.
+  if (unitType.startsWith("hook/")) return true;
+
   // fix-merge has no file artifact — verify by checking git state
   if (unitType === "fix-merge") {
     const unmerged = runGit(base, ["diff", "--name-only", "--diff-filter=U"], { allowFailure: true });

src/resources/extensions/gsd/tests/idle-recovery.test.ts

@@ -574,4 +574,25 @@ const ROADMAP_COMPLETE = `# M001: Test Milestone
   }
 }
 
+// ═══ verifyExpectedArtifact: hook unit types ═════════════════════════════════
+
+console.log("\n=== verifyExpectedArtifact: hook types always return true ===");
+
+{
+  const base = createFixtureBase();
+  try {
+    // Hook units don't have standard artifacts — they should always pass
+    const result1 = verifyExpectedArtifact("hook/code-review", "M001/S01/T01", base);
+    assertTrue(result1, "hook/code-review should always return true");
+
+    const result2 = verifyExpectedArtifact("hook/simplify", "M001/S01/T02", base);
+    assertTrue(result2, "hook/simplify should always return true");
+
+    const result3 = verifyExpectedArtifact("hook/custom-hook", "M001/S01", base);
+    assertTrue(result3, "hook/custom-hook at slice level should return true");
+  } finally {
+    rmSync(base, { recursive: true, force: true });
+  }
+}
+
 report();