diff --git a/.changeset/fn-7608-awaiting-approval-blocking.md b/.changeset/fn-7608-awaiting-approval-blocking.md new file mode 100644 index 0000000000..d0eb70c055 --- /dev/null +++ b/.changeset/fn-7608-awaiting-approval-blocking.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Executors now block on pending approvals instead of probing for ungated workarounds. +category: fix +dev: wait-for-approval now suspends the in-flight executor session via awaitAbortInFlightTaskWork and dedupes identical pending approvals; executor prompts carve out awaiting-approval as a legitimate turn end. diff --git a/docs/agents.md b/docs/agents.md index c2435d159a..9b0009670a 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -187,6 +187,7 @@ Approval pause/resume lifecycle (FN-3548): - Permanent-agent gating short-circuits `block` and `require-approval` actions before tool execution and returns structured non-success tool results. - For `require-approval`, the engine creates/reuses a durable approval request and pauses execution with canonical `pauseReason: "awaiting-approval"`. - If task-backed, the owning task is paused (`Task.paused=true`, `pausedByAgentId=`); the requesting agent is paused (`state="paused"`, `pauseReason="awaiting-approval"`). The task-detail **Paused by agent** indicator is context only: operators may still manually pause or unpause an agent-assigned task, and unpause clears the task pause latch. +- FN-7608: for `TaskExecutor`-backed sessions, a `wait-for-approval` gate outcome does more than mark the task/agent paused in the store — it actually suspends the in-flight executor session. `TaskExecutor.buildActionGateContext()`'s `pauseForApproval` fires `awaitAbortInFlightTaskWork(taskId, "awaiting-approval:...")` (fire-and-forget, not awaited inline, to avoid a self-deadlock against the tool call that triggered it) so the running LLM turn is aborted rather than continuing to probe for ungated workarounds. `wrapToolsWithActionGate()` invokes `pauseForApproval` on both the newly-created-request path and the reused-pending path (a repeated identical gated call reuses the pending request via `approvalDedupeKey` and still triggers the pause/suspend). Both canonical executor prompts (engine `EXECUTOR_SYSTEM_PROMPT`, core `EXECUTOR_PROMPT_TEXT`) carry a byte-identical carve-out stating that waiting on a pending approval is a legitimate turn end and that re-issuing the gated call, probing read-only equivalents, or routing around the block via other tools is forbidden. Heartbeat-driven sessions have no persistent in-flight session object to abort (each tick is a short bounded cycle), so pausing the task/agent there is already sufficient. - Dedupe semantics by `approvalDedupeKey`: `pending` reuses the same request, `approved` allows exactly one execution and then marks request `completed`, `denied` stays blocked, `completed` requires a fresh request. - HTTP decision endpoint resumes best-effort: `POST /api/approvals/:id/decision` with `{ decision: "approve" | "deny", comment? }` unpauses matching task/agent when they are paused for `awaiting-approval`. - Approval API surface: `GET /api/approvals` (supports status/limit/offset and returns `{ requests, total, pendingCount }`), `GET /api/approvals/:id` (includes request context + audit/history), `POST /api/approvals/:id/decision`. diff --git a/docs/architecture.md b/docs/architecture.md index 329a035bb7..b878076706 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -667,6 +667,7 @@ Runtime action-gate flow (v1): - Non-ephemeral agents receive `AgentActionGateContext` from executor/heartbeat session creation. - `block` and `require-approval` dispositions intercept before tool side effects. - `require-approval` persists durable requests via `ApprovalRequestStore`, reusing pending requests by dedupe key in `targetAction.context.approvalDedupeKey`. +- FN-7608: a `wait-for-approval` gate outcome is a REAL session-suspending state, not just a soft per-call rejection. `wrapToolsWithActionGate()` invokes `gateContext.pauseForApproval(...)` for BOTH the newly-created-request sub-case and the reused-pending sub-case (previously only the former), and `TaskExecutor.buildActionGateContext()`'s `pauseForApproval` closure pauses the task in the store AND fires `awaitAbortInFlightTaskWork(taskId, "awaiting-approval:...")` to abort the in-flight executor session for that task — fire-and-forget (not awaited inline), because `session.abort()` internally awaits `agent.waitForIdle()`, which cannot resolve until the very tool call invoking `pauseForApproval` returns (awaiting it inline would deadlock). `HeartbeatMonitor`'s gate context has no persistent in-flight session surface to abort (each heartbeat tick is a short bounded request/response cycle), so pausing the task/agent there is sufficient — see the `FNXC:AgentGating` comment at its `pauseForApproval` closure. Both canonical executor prompts (`EXECUTOR_SYSTEM_PROMPT` in `executor.ts`, `EXECUTOR_PROMPT_TEXT` in `agent-prompts.ts`) carry a byte-identical carve-out: waiting on a pending approval is a legitimate turn end, and probing for ungated workarounds (re-issuing the gated call, read-only equivalents, `fn_web_fetch`/`fn_task_attach` bypasses) is forbidden. ### Concurrency, recovery, and resiliency - `AgentSemaphore` (`concurrency.ts`) — slot acquisition. Multi-project runtimes share a single manager-owned semaphore for the cross-project `globalMaxConcurrent` cap, while each `InProcessRuntime` wraps that pool in a scoped semaphore that tracks only that project's held slots. Engine stop, `pauseProject`, and `stopAll` abort in-flight agents, wait the configured stop drain window, then return any residual scoped slots to the shared pool without using a blanket `reconcileActiveCount(0)`, so other projects' active slots are preserved and stopped projects do not starve global capacity. diff --git a/packages/core/src/agent-prompts.ts b/packages/core/src/agent-prompts.ts index 14014c0046..d2eacae40d 100644 --- a/packages/core/src/agent-prompts.ts +++ b/packages/core/src/agent-prompts.ts @@ -24,6 +24,9 @@ import type { AgentCapability, AgentPromptTemplate, AgentPromptsConfig } from ". /* FNXC:ExecutorPrompt 2026-06-21-03:59: Agents must not run the full/workspace-wide test suite by default; targeted/package-scoped verification is the norm, full runs require explicit task/workflow opt-in. + +FNXC:ExecutorPrompt 2026-07-05-00:35: +FN-7608: a `require-approval` gate previously only parked the single tool call (soft rejection + task/agent paused in the store) while the turn-ending rules below forbade ending a turn without another tool call, so the model was effectively instructed to hunt for ungated workarounds (re-issuing the same bash, probing read-only equivalents, fn_web_fetch/fn_task_attach bypasses) instead of stopping. The engine now actually suspends the in-flight session when a gate resolves to wait-for-approval (see executor.ts buildActionGateContext.pauseForApproval), so the prompt must carve out waiting on a pending approval as a legitimate turn end and explicitly forbid probing for alternatives. This clause must stay byte-identical with EXECUTOR_SYSTEM_PROMPT in packages/engine/src/executor.ts. */ const EXECUTOR_PROMPT_TEXT = `You are a task execution agent for "fn", an AI-orchestrated task board. @@ -43,6 +46,8 @@ You MUST NOT end a turn by writing prose that asks the user a question, summariz - "Ready to move on to step N. Want me to continue?" - Any markdown progress summary at the end of a turn instead of a tool call +**Exception — pending approval.** If a tool call reports that the action requires approval (a permission gate) and the task has been paused awaiting a decision, STOP. Waiting on a pending approval IS a legitimate turn end: the engine suspends this session automatically once the gate fires, so ending the turn here is expected, not a violation of the rule above. Do NOT re-issue the same gated call, probe for a read-only or "equivalent" alternative, fetch the gated resource through another tool (e.g. \`fn_web_fetch\`, \`fn_task_attach\`), or otherwise search for an ungated path around the blocked action — an approval gate is fully blocking, not something to route around or "make progress another way" against. Execution resumes on its own once the request is approved or denied. + If you have just finished a step's work, immediately call \`fn_task_update\` to mark the step done and continue with the next pending step in the SAME turn. Do not pause to summarize. The user is not watching this conversation in real-time. They will read the final result. Asking permission wastes a full retry cycle and may orphan committed work. diff --git a/packages/engine/src/__tests__/agent-action-gate.test.ts b/packages/engine/src/__tests__/agent-action-gate.test.ts index 10bcc12565..db8c44e5c7 100644 --- a/packages/engine/src/__tests__/agent-action-gate.test.ts +++ b/packages/engine/src/__tests__/agent-action-gate.test.ts @@ -271,6 +271,88 @@ describe("agent-action-gate", () => { expect(execute).toHaveBeenCalledTimes(1); }); + /* + FNXC:AgentGating 2026-07-05-00:25: + FN-7608 regression coverage: an identical gated `bash` call issued twice + (same command — the original symptom was `pnpm install` re-issued while the + first request sat pending) must NOT mint a second approval request, and + BOTH the newly-created and reused-pending sub-cases must invoke + pauseForApproval so the executor's session-abort wiring actually runs every + time a gate resolves to wait-for-approval — not only on first creation. + Uses a tiny in-memory fake backed by the real findLatestByDedupeKey + contract shape (id + status) to drive resolveGateOutcome's dedupe reuse. + */ + it("dedupes identical pending bash approvals and pauses on both the created and reused-pending path", async () => { + const execute = vi.fn().mockResolvedValue({ ok: true }); + const tool = { name: "bash", label: "Bash", description: "", parameters: {}, execute }; + const { wrapToolsWithActionGate } = await import("../pi.js"); + + const requests = new Map(); + let nextId = 0; + const createApprovalRequest = vi.fn(async (decision: { approvalDedupeKey: string }) => { + const id = `apr-${++nextId}`; + requests.set(decision.approvalDedupeKey, { id, status: "pending" }); + return { id }; + }); + const findApprovalByDedupeKey = vi.fn(async (dedupeKey: string) => requests.get(dedupeKey) ?? null); + const pauseForApproval = vi.fn(); + + const gated = wrapToolsWithActionGate([tool as any], { + agentId: "agent-1", + agentName: "Agent", + isEphemeral: false, + taskId: "FN-1", + permissionPolicy: approvalPolicy, + createApprovalRequest, + findApprovalByDedupeKey, + pauseForApproval, + }); + + const first = await (gated[0] as any).execute("call-1", { command: "pnpm install" }); + expect(first.isError).toBe(true); + expect(createApprovalRequest).toHaveBeenCalledTimes(1); + expect(pauseForApproval).toHaveBeenCalledTimes(1); + const firstApprovalId = first.decision.metadata.approvalRequestId; + expect(firstApprovalId).toBe("apr-1"); + + // Second identical call: same dedupe key, request still pending -> must + // reuse the existing request (no second createApprovalRequest call) but + // must STILL invoke pauseForApproval so the session is suspended again. + const second = await (gated[0] as any).execute("call-2", { command: "pnpm install" }); + expect(second.isError).toBe(true); + expect(createApprovalRequest).toHaveBeenCalledTimes(1); + expect(pauseForApproval).toHaveBeenCalledTimes(2); + expect(second.decision.metadata.approvalRequestId).toBe(firstApprovalId); + expect(execute).not.toHaveBeenCalled(); + + // A denied request must block and must never retry/execute. + requests.set(second.decision.metadata.dedupeKey, { id: firstApprovalId, status: "denied" }); + const third = await (gated[0] as any).execute("call-3", { command: "pnpm install" }); + expect(third.isError).toBe(true); + expect(third.error).toMatch(/denied/i); + expect(execute).not.toHaveBeenCalled(); + expect(createApprovalRequest).toHaveBeenCalledTimes(1); + + // An approved request executes exactly once, then completes. + requests.set(second.decision.metadata.dedupeKey, { id: firstApprovalId, status: "approved" }); + const markApprovalCompleted = vi.fn(); + const approvedGate = wrapToolsWithActionGate([tool as any], { + agentId: "agent-1", + agentName: "Agent", + isEphemeral: false, + taskId: "FN-1", + permissionPolicy: approvalPolicy, + createApprovalRequest, + findApprovalByDedupeKey, + pauseForApproval, + markApprovalCompleted, + }); + const fourth = await (approvedGate[0] as any).execute("call-4", { command: "pnpm install" }); + expect(fourth).toEqual({ ok: true }); + expect(execute).toHaveBeenCalledTimes(1); + expect(markApprovalCompleted).toHaveBeenCalledWith(firstApprovalId); + }); + it.each([ "fn_task_create", "fn_delegate_task", diff --git a/packages/engine/src/__tests__/executor-approval-gate-suspend.test.ts b/packages/engine/src/__tests__/executor-approval-gate-suspend.test.ts new file mode 100644 index 0000000000..3efc51ad3d --- /dev/null +++ b/packages/engine/src/__tests__/executor-approval-gate-suspend.test.ts @@ -0,0 +1,128 @@ +import "./executor-test-helpers.js"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { TaskExecutor } from "../executor.js"; +import { resetExecutorMocks } from "./executor-test-helpers.js"; + +/* +FNXC:AgentGating 2026-07-05-00:30: +FN-7608 regression coverage: TaskExecutor.buildActionGateContext's +`pauseForApproval` closure must both pause the task in the store AND +synchronously trigger a session-suspending abort of the in-flight session for +that task (via the existing awaitAbortInFlightTaskWork hard-cancel surface). +Before this fix, pauseTask() marked the row paused but the running LLM turn +kept going -- the executor prompt forbade ending a turn without a tool call, +so the agent hunted for ungated workarounds while the task only *looked* +paused in the store. Assert both effects fire, and that a rejected/failed +abort call is swallowed (never breaks pauseForApproval's own control flow). +*/ +function createEventedStore() { + const listeners = new Map void>>(); + return { + on: vi.fn((event: string, listener: (...args: any[]) => void) => { + const set = listeners.get(event) ?? new Set(); + set.add(listener); + listeners.set(event, set); + }), + off: vi.fn((event: string, listener: (...args: any[]) => void) => { + listeners.get(event)?.delete(listener); + }), + getSettings: vi.fn().mockResolvedValue({ globalPause: false, enginePaused: false }), + listTasks: vi.fn().mockResolvedValue([]), + pauseTask: vi.fn().mockResolvedValue(undefined), + logEntry: vi.fn().mockResolvedValue(undefined), + } as any; +} + +describe("TaskExecutor.buildActionGateContext pauseForApproval", () => { + beforeEach(() => { + resetExecutorMocks(); + vi.clearAllMocks(); + }); + + it("pauses the task and synchronously kicks off an in-flight session abort", async () => { + const store = createEventedStore(); + const executor = new TaskExecutor(store, "/tmp/test"); + + const abortSpy = vi.spyOn(executor, "awaitAbortInFlightTaskWork").mockResolvedValue(undefined); + + const gateContext = (executor as any).buildActionGateContext("FN-1", null, undefined); + expect(gateContext).toBeTruthy(); + + const decision = { + disposition: "require-approval", + category: "command_execution", + toolName: "bash", + operation: "shell command", + summary: "bash: shell command", + resourceType: "command", + approvalDedupeKey: "executor-FN-1|FN-1|bash|command_execution|command||shell command", + metadata: {}, + }; + + await gateContext.pauseForApproval({ approvalRequestId: "apr-1", decision }); + + expect(store.pauseTask).toHaveBeenCalledWith("FN-1", true, undefined, expect.objectContaining({ pausedByAgentId: expect.any(String) })); + expect(store.logEntry).toHaveBeenCalled(); + // Session suspension must be triggered synchronously (called, not merely + // scheduled for some later tick) as part of this same pauseForApproval + // invocation. + expect(abortSpy).toHaveBeenCalledTimes(1); + expect(abortSpy).toHaveBeenCalledWith("FN-1", expect.stringContaining("awaiting-approval")); + }); + + it("does not let a rejected session-abort break pauseForApproval's control flow", async () => { + const store = createEventedStore(); + const executor = new TaskExecutor(store, "/tmp/test"); + + vi.spyOn(executor, "awaitAbortInFlightTaskWork").mockRejectedValue(new Error("boom")); + + const gateContext = (executor as any).buildActionGateContext("FN-2", null, undefined); + const decision = { + disposition: "require-approval", + category: "command_execution", + toolName: "bash", + operation: "shell command", + summary: "bash: shell command", + resourceType: "command", + approvalDedupeKey: "k", + metadata: {}, + }; + + await expect(gateContext.pauseForApproval({ approvalRequestId: "apr-2", decision })).resolves.toBeUndefined(); + expect(store.pauseTask).toHaveBeenCalledTimes(1); + + // Let the fire-and-forget rejected promise's .catch() handler settle + // before the test ends, so no unhandled rejection leaks. + await new Promise((resolve) => setImmediate(resolve)); + }); + + it("fires the abort call without blocking on it (fire-and-forget, not awaited inline)", async () => { + const store = createEventedStore(); + const executor = new TaskExecutor(store, "/tmp/test"); + + let resolveAbort: () => void = () => {}; + const abortPromise = new Promise((resolve) => { + resolveAbort = resolve; + }); + vi.spyOn(executor, "awaitAbortInFlightTaskWork").mockReturnValue(abortPromise); + + const gateContext = (executor as any).buildActionGateContext("FN-3", null, undefined); + const decision = { + disposition: "require-approval", + category: "command_execution", + toolName: "bash", + operation: "shell command", + summary: "bash: shell command", + resourceType: "command", + approvalDedupeKey: "k3", + metadata: {}, + }; + + // If pauseForApproval awaited the abort call inline, this would hang + // forever since abortPromise never resolves during the test body. + await expect(gateContext.pauseForApproval({ approvalRequestId: "apr-3", decision })).resolves.toBeUndefined(); + + resolveAbort(); + await abortPromise; + }); +}); diff --git a/packages/engine/src/__tests__/executor-approval-prompt-carveout.test.ts b/packages/engine/src/__tests__/executor-approval-prompt-carveout.test.ts new file mode 100644 index 0000000000..b4f30c6e87 --- /dev/null +++ b/packages/engine/src/__tests__/executor-approval-prompt-carveout.test.ts @@ -0,0 +1,61 @@ +import { describe, expect, it } from "vitest"; +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; +import { resolveAgentPrompt } from "@fusion/core"; + +/* +FNXC:ExecutorPrompt 2026-07-05-00:40: +FN-7608 regression coverage: both canonical executor prompts (EXECUTOR_SYSTEM_PROMPT +in engine/executor.ts and EXECUTOR_PROMPT_TEXT in core/agent-prompts.ts, exposed via +resolveAgentPrompt("executor")) must carry the pending-approval turn-ending carve-out, +and the shared clause text must stay byte-identical between the two copies so they +never drift. EXECUTOR_SYSTEM_PROMPT is a private module constant (not exported), so +the engine copy is asserted by reading the source file directly. +*/ +const CARVEOUT_MARKER = "Exception — pending approval."; + +function readExecutorSourcePrompt(): string { + const executorTsPath = fileURLToPath(new URL("../executor.ts", import.meta.url)); + return readFileSync(executorTsPath, "utf8"); +} + +function readAgentPromptsSource(): string { + const agentPromptsTsPath = fileURLToPath(new URL("../../../core/src/agent-prompts.ts", import.meta.url)); + return readFileSync(agentPromptsTsPath, "utf8"); +} + +describe("executor prompt pending-approval carve-out (FN-7608)", () => { + it("EXECUTOR_SYSTEM_PROMPT (engine) contains the carve-out marker", () => { + const source = readExecutorSourcePrompt(); + expect(source).toContain(CARVEOUT_MARKER); + expect(source).toContain("Waiting on a pending approval IS a legitimate turn end"); + }); + + it("EXECUTOR_PROMPT_TEXT (core, via resolveAgentPrompt) contains the carve-out marker", () => { + const prompt = resolveAgentPrompt("executor"); + expect(prompt).toContain(CARVEOUT_MARKER); + expect(prompt).toContain("Waiting on a pending approval IS a legitimate turn end"); + }); + + it("keeps the shared carve-out clause byte-identical between both prompt copies", () => { + // Compare raw .ts SOURCE text on both sides (not the runtime-resolved + // core prompt), since the core copy's runtime string has already been + // through template-literal escape resolution (e.g. `\\\`` -> `\``) while + // reading executor.ts's source keeps its literal escape sequences — + // comparing source-to-source avoids a false mismatch from that. + const engineSource = readExecutorSourcePrompt(); + const coreSource = readAgentPromptsSource(); + + function extractClause(text: string): string { + const start = text.indexOf(`**${CARVEOUT_MARKER}**`); + expect(start).toBeGreaterThan(-1); + const end = text.indexOf("\n\nIf you have just finished", start); + expect(end).toBeGreaterThan(start); + return text.slice(start, end); + } + + const engineClause = extractClause(engineSource); + const coreClause = extractClause(coreSource); + expect(engineClause).toBe(coreClause); + }); +}); diff --git a/packages/engine/src/agent-heartbeat.ts b/packages/engine/src/agent-heartbeat.ts index 5537e83069..fbdc30fc6a 100644 --- a/packages/engine/src/agent-heartbeat.ts +++ b/packages/engine/src/agent-heartbeat.ts @@ -1200,6 +1200,19 @@ export class HeartbeatMonitor { const latest = this.getApprovalRequestStore().findLatestByDedupeKey({ requesterActorId: agent.id, taskId, dedupeKey }); return latest?.status === "pending" ? { id: latest.id } : null; }, + /* + FNXC:AgentGating 2026-07-05-00:15: + FN-7608: unlike TaskExecutor.buildActionGateContext, HeartbeatMonitor has + no `activeSessions`/session-abort surface of its own -- permanent-agent + heartbeat ticks are short stateless request/response cycles (each tick + runs one bounded pi turn and returns; there is no long-lived in-flight + session object to synchronously abort mid-turn). Pausing the task and + agent here already prevents the NEXT heartbeat tick from running (the + scheduler skips paused agents/tasks), so there is nothing further to + suspend -- this closure intentionally has no session-abort call. If + HeartbeatMonitor ever grows a persistent in-flight session surface, wire + awaitAbortInFlightTaskWork-equivalent suspension here too. + */ pauseForApproval: async ({ approvalRequestId, decision }) => { if (taskId && this.taskStore) { await this.taskStore.pauseTask(taskId, true, undefined, { pausedByAgentId: agent.id }); diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 5be6ce3532..af74a03f8f 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -1315,6 +1315,9 @@ const reviewStepParams = Type.Object({ /* FNXC:ExecutorPrompt 2026-06-21-03:59: Agents must not run the full/workspace-wide test suite by default; targeted/package-scoped verification is the norm, full runs require explicit task/workflow opt-in. + +FNXC:ExecutorPrompt 2026-07-05-00:35: +FN-7608: a `require-approval` gate previously only parked the single tool call (soft rejection + task/agent paused in the store) while the turn-ending rules below forbade ending a turn without another tool call, so the model was effectively instructed to hunt for ungated workarounds (re-issuing the same bash, probing read-only equivalents, fn_web_fetch/fn_task_attach bypasses) instead of stopping. The engine now actually suspends the in-flight session when a gate resolves to wait-for-approval (see executor.ts buildActionGateContext.pauseForApproval), so the prompt must carve out waiting on a pending approval as a legitimate turn end and explicitly forbid probing for alternatives. This clause must stay byte-identical with EXECUTOR_PROMPT_TEXT in packages/core/src/agent-prompts.ts. */ const EXECUTOR_SYSTEM_PROMPT = `You are a task execution agent for "fn", an AI-orchestrated task board. @@ -1338,6 +1341,8 @@ You MUST NOT end a turn by writing prose that asks the user a question, summariz - "Ready to move on to step N. Want me to continue?" - Any markdown progress summary at the end of a turn instead of a tool call +**Exception — pending approval.** If a tool call reports that the action requires approval (a permission gate) and the task has been paused awaiting a decision, STOP. Waiting on a pending approval IS a legitimate turn end: the engine suspends this session automatically once the gate fires, so ending the turn here is expected, not a violation of the rule above. Do NOT re-issue the same gated call, probe for a read-only or "equivalent" alternative, fetch the gated resource through another tool (e.g. \`fn_web_fetch\`, \`fn_task_attach\`), or otherwise search for an ungated path around the blocked action — an approval gate is fully blocking, not something to route around or "make progress another way" against. Execution resumes on its own once the request is approved or denied. + If you have just finished a step's work, immediately call \`fn_task_update\` to mark the step done and continue with the next pending step in the SAME turn. Do not pause to summarize. The user is not watching this conversation in real-time. They will read the final result. Asking permission wastes a full retry cycle and may orphan committed work. @@ -2275,6 +2280,29 @@ export class TaskExecutor { undefined, this.getRunContextFor(taskId), ); + /* + FNXC:AgentGating 2026-07-05-00:10: + FN-7608: pauseTask() alone does not stop the in-flight LLM turn -- the + gated tool call only returns a soft rejection, and the executor system + prompt forbids ending a turn without another tool call, so the agent + kept hunting for ungated workarounds (re-issuing the same bash, probing + read-only tools, fn_web_fetch/fn_task_attach bypasses) while the task + sat "paused" only in the store. Make wait-for-approval a REAL + session-suspending state by aborting the in-flight session here, using + the same synchronous abort surface hard-cancel uses + (awaitAbortInFlightTaskWork). This call is deliberately NOT awaited: + awaitAbortInFlightTaskWork's agent-session branch awaits + session.abort(), which internally awaits agent.waitForIdle() -- since + pauseForApproval runs from inside this very tool call, the agent + cannot become idle until our own execute() resolves, so awaiting the + abort inline here would deadlock. Firing it (fire-and-forget, errors + swallowed to a warn per the FN-7335 best-effort-breadcrumb pattern) + lets the abort proceed the moment this tool's rejection unwinds back + to the agent loop. + */ + void this.awaitAbortInFlightTaskWork(taskId, `awaiting-approval:${decision.toolName}`).catch((error) => { + executorLog.warn(`${taskId}: failed to suspend in-flight session while awaiting approval: ${error instanceof Error ? error.message : String(error)}`); + }); } if (agent && this.options.agentStore) { await this.options.agentStore.updateAgentState(agent.id, "paused"); diff --git a/packages/engine/src/pi.ts b/packages/engine/src/pi.ts index b454e35fb7..ac1a84e302 100644 --- a/packages/engine/src/pi.ts +++ b/packages/engine/src/pi.ts @@ -1958,13 +1958,27 @@ export function wrapToolsWithActionGate( ); } + /* + FNXC:AgentGating 2026-07-05-00:00: + FN-7608: pauseForApproval (which pauses the task AND suspends the + in-flight session — see executor.ts buildActionGateContext) must run + for BOTH the newly-created-request sub-case and the reused-pending + sub-case. Previously it only ran when a fresh approval request was + minted, so a second identical gated call that resolved to an already- + pending request (dedupe working as designed) never paused/suspended + the session — the agent's turn kept going and it hunted for ungated + workarounds instead of stopping. resolveGateOutcome() already + guarantees no duplicate request is created on the reused-pending path + (gateOutcome.approvalRequestId is set from the existing pending row), + so this only ever pauses once per distinct approval request. + */ let approvalRequestId = gateOutcome.approvalRequestId; if (!approvalRequestId) { const created = await gateContext.createApprovalRequest(decision, params) as { id?: string } | null; approvalRequestId = created?.id; - if (approvalRequestId) { - await gateContext.pauseForApproval?.({ approvalRequestId, decision }); - } + } + if (approvalRequestId) { + await gateContext.pauseForApproval?.({ approvalRequestId, decision }); } return buildGateRejection( @@ -1976,7 +1990,7 @@ export function wrapToolsWithActionGate( dedupeKey: decision.approvalDedupeKey, }, }, - `Action requires approval (request ${approvalRequestId ?? "pending"}). Agent and task have been paused; will resume once a decision is made.`, + `Action requires approval (request ${approvalRequestId ?? "pending"}). Task paused and session suspended awaiting decision; do not attempt alternatives.`, ); }, }; diff --git a/packages/engine/src/sandbox/__tests__/provisioning-gate.test.ts b/packages/engine/src/sandbox/__tests__/provisioning-gate.test.ts index 7095ec0cbc..efb86af052 100644 --- a/packages/engine/src/sandbox/__tests__/provisioning-gate.test.ts +++ b/packages/engine/src/sandbox/__tests__/provisioning-gate.test.ts @@ -83,6 +83,35 @@ describe("requireSandboxProvisioningApproval", () => { expect(result).toEqual({ outcome: "execute-once-then-complete", approvalRequestId: "apr-1" }); }); + /* + FNXC:AgentGating 2026-07-05-00:22: + FN-7608 parity coverage: a reused-PENDING approval (dedupe hit) must also + re-invoke pauseForApproval, mirroring the same fix applied to + wrapToolsWithActionGate (pi.ts) — previously only the newly-created path + called pauseForApproval here. + */ + it("re-invokes pauseForApproval when reusing an existing pending approval", async () => { + const pauseForApproval = vi.fn(async () => undefined); + await expect( + requireSandboxProvisioningApproval({ + backendId: "bubblewrap", + operation: "install", + description: "Install bubblewrap", + context: { + taskId: "FN-4641", + requester: { actorId: "agent-1", actorType: "agent", actorName: "Executor" }, + settings: undefined, + createApprovalRequest: vi.fn(async () => null), + findApprovalByDedupeKey: vi.fn(async () => ({ id: "apr-1", status: "pending" as const })), + pauseForApproval, + }, + }), + ).rejects.toBeInstanceOf(SandboxProvisioningPendingError); + + expect(pauseForApproval).toHaveBeenCalledTimes(1); + expect(pauseForApproval).toHaveBeenCalledWith(expect.objectContaining({ approvalRequestId: "apr-1" })); + }); + it("throws block when prior approval was denied", async () => { await expect( requireSandboxProvisioningApproval({ diff --git a/packages/engine/src/sandbox/provisioning-gate.ts b/packages/engine/src/sandbox/provisioning-gate.ts index f6977c57f0..97f348a9a0 100644 --- a/packages/engine/src/sandbox/provisioning-gate.ts +++ b/packages/engine/src/sandbox/provisioning-gate.ts @@ -129,6 +129,17 @@ export async function requireSandboxProvisioningApproval(input: { } if (gateOutcome.approvalRequestId) { + /* + FNXC:AgentGating 2026-07-05-00:20: + FN-7608: a reused-pending approval must also re-run pauseForApproval, not + just the newly-created path below -- otherwise a repeated identical + provisioning request after the first pause (e.g. task/agent resumed some + other way) would silently re-block without re-pausing, mirroring the same + gap fixed in wrapToolsWithActionGate (pi.ts). + */ + if (context.pauseForApproval) { + await context.pauseForApproval({ approvalRequestId: gateOutcome.approvalRequestId, decision }); + } throw new SandboxProvisioningPendingError({ message: "Sandbox provisioning approval pending", approvalRequestId: gateOutcome.approvalRequestId,