diff --git a/src/resources/extensions/gsd/auto.ts b/src/resources/extensions/gsd/auto.ts index ba866014d..3dfe517a0 100644 --- a/src/resources/extensions/gsd/auto.ts +++ b/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 }); diff --git a/src/resources/extensions/gsd/tests/idle-recovery.test.ts b/src/resources/extensions/gsd/tests/idle-recovery.test.ts index 60d952c27..4f63dcb99 100644 --- a/src/resources/extensions/gsd/tests/idle-recovery.test.ts +++ b/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();