diff --git a/src/trpc/hooks-api.ts b/src/trpc/hooks-api.ts index bdba55011..7325a0684 100644 --- a/src/trpc/hooks-api.ts +++ b/src/trpc/hooks-api.ts @@ -23,6 +23,8 @@ export interface CreateHooksApiDependencies { deleteTaskTurnCheckpointRef?: (input: { cwd: string; ref: string }) => Promise; } +const COMPLETION_HOOK_EVENT_NAMES = new Set(["TaskComplete", "stop", "subagentstop", "afteragent"]); + function canTransitionTaskForHookEvent(summary: RuntimeTaskSessionSummary, event: RuntimeHookEvent): boolean { if (event === "activity") { return false; @@ -30,10 +32,27 @@ function canTransitionTaskForHookEvent(summary: RuntimeTaskSessionSummary, event if (event === "to_review") { return summary.state === "running"; } - return ( - summary.state === "awaiting_review" && - (summary.reviewReason === "attention" || summary.reviewReason === "hook" || summary.reviewReason === "error") - ); + if (summary.state !== "awaiting_review") { + return false; + } + + // attention/error reviews can always return to running. + if (summary.reviewReason === "attention" || summary.reviewReason === "error") { + return true; + } + + // Hook-triggered reviews: disallow to_in_progress when the hook that + // triggered review was a completion event (TaskComplete, stop, etc.). + // This prevents auto-commit PreToolUse from flipping completed tasks back to in_progress. + if (summary.reviewReason === "hook") { + const hookEventName = summary.latestHookActivity?.hookEventName ?? null; + if (hookEventName && COMPLETION_HOOK_EVENT_NAMES.has(hookEventName)) { + return false; + } + return true; + } + + return false; } export function createHooksApi(deps: CreateHooksApiDependencies): RuntimeTrpcContext["hooksApi"] { diff --git a/test/runtime/trpc/hooks-api.test.ts b/test/runtime/trpc/hooks-api.test.ts index 80c629853..35a5ddb94 100644 --- a/test/runtime/trpc/hooks-api.test.ts +++ b/test/runtime/trpc/hooks-api.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it, vi } from "vitest"; -import type { RuntimeTaskSessionSummary } from "../../../src/core/api-contract"; +import type { RuntimeTaskHookActivity, RuntimeTaskSessionSummary } from "../../../src/core/api-contract"; import type { TerminalSessionManager } from "../../../src/terminal/session-manager"; import { createHooksApi } from "../../../src/trpc/hooks-api"; @@ -22,6 +22,30 @@ function createSummary(overrides: Partial = {}): Runt }; } +function createHookActivity(overrides: Partial = {}): RuntimeTaskHookActivity { + return { + activityText: null, + toolName: null, + toolInputSummary: null, + finalMessage: null, + hookEventName: null, + notificationType: null, + source: null, + ...overrides, + }; +} + +function createAwaitingReviewSummary( + reviewReason: RuntimeTaskSessionSummary["reviewReason"], + hookEventName: string | null, +): RuntimeTaskSessionSummary { + return createSummary({ + state: "awaiting_review", + reviewReason, + latestHookActivity: hookEventName !== null ? createHookActivity({ hookEventName }) : null, + }); +} + describe("createHooksApi", () => { it("treats ineligible hook transitions as successful no-ops", async () => { const manager = { @@ -145,4 +169,89 @@ describe("createHooksApi", () => { ref: "refs/kanban/checkpoints/task-1/turn/1", }); }); + + describe("to_in_progress guard for hook-triggered reviews", () => { + function makeManager(summary: RuntimeTaskSessionSummary) { + return { + getSummary: vi.fn(() => summary), + transitionToReview: vi.fn(), + transitionToRunning: vi.fn(() => summary), + applyHookActivity: vi.fn(), + } as unknown as TerminalSessionManager; + } + + async function ingestToInProgress(summary: RuntimeTaskSessionSummary) { + const manager = makeManager(summary); + const api = createHooksApi({ + getWorkspacePathById: vi.fn(() => "/tmp/repo"), + ensureTerminalManagerForWorkspace: vi.fn(async () => manager), + broadcastRuntimeWorkspaceStateUpdated: vi.fn(), + broadcastTaskReadyForReview: vi.fn(), + }); + + const response = await api.ingest({ + taskId: "task-1", + workspaceId: "workspace-1", + event: "to_in_progress", + }); + + return { response, manager }; + } + + it("blocks to_in_progress when reviewReason=hook and latestHookActivity.hookEventName=TaskComplete", async () => { + const summary = createAwaitingReviewSummary("hook", "TaskComplete"); + const { response, manager } = await ingestToInProgress(summary); + expect(response).toEqual({ ok: true }); + expect(manager.transitionToRunning).not.toHaveBeenCalled(); + }); + + it("blocks to_in_progress when reviewReason=hook and latestHookActivity.hookEventName=stop", async () => { + const summary = createAwaitingReviewSummary("hook", "stop"); + const { response, manager } = await ingestToInProgress(summary); + expect(response).toEqual({ ok: true }); + expect(manager.transitionToRunning).not.toHaveBeenCalled(); + }); + + it("blocks to_in_progress when reviewReason=hook and latestHookActivity.hookEventName=afteragent", async () => { + const summary = createAwaitingReviewSummary("hook", "afteragent"); + const { response, manager } = await ingestToInProgress(summary); + expect(response).toEqual({ ok: true }); + expect(manager.transitionToRunning).not.toHaveBeenCalled(); + }); + + it("blocks to_in_progress when reviewReason=hook and latestHookActivity.hookEventName=subagentstop", async () => { + const summary = createAwaitingReviewSummary("hook", "subagentstop"); + const { response, manager } = await ingestToInProgress(summary); + expect(response).toEqual({ ok: true }); + expect(manager.transitionToRunning).not.toHaveBeenCalled(); + }); + + it("allows to_in_progress when reviewReason=hook and latestHookActivity.hookEventName=PreToolUse (ask_followup_question)", async () => { + const summary = createAwaitingReviewSummary("hook", "PreToolUse"); + const { response, manager } = await ingestToInProgress(summary); + expect(response).toEqual({ ok: true }); + expect(manager.transitionToRunning).toHaveBeenCalled(); + }); + + it("allows to_in_progress when reviewReason=hook and latestHookActivity=null (backward compat)", async () => { + const summary = createAwaitingReviewSummary("hook", null); + const { response, manager } = await ingestToInProgress(summary); + expect(response).toEqual({ ok: true }); + expect(manager.transitionToRunning).toHaveBeenCalled(); + }); + + it("allows to_in_progress when reviewReason=attention (user returned)", async () => { + const summary = createAwaitingReviewSummary("attention", null); + const { response, manager } = await ingestToInProgress(summary); + expect(response).toEqual({ ok: true }); + expect(manager.transitionToRunning).toHaveBeenCalled(); + }); + + it("allows to_in_progress when reviewReason=error (error recovery)", async () => { + const summary = createAwaitingReviewSummary("error", null); + const { response, manager } = await ingestToInProgress(summary); + expect(response).toEqual({ ok: true }); + expect(manager.transitionToRunning).toHaveBeenCalled(); + }); + }); });