FN-7608: block executors on pending approvals instead of allowing workarounds
Executors could previously treat a pending approval as a normal turn end and go hunt for ungated workarounds instead of stopping. This change makes wait-for-approval a hard suspend point. - wait-for-approval now suspends the in-flight executor session via awaitAbortInFlightTaskWork - Dedupe identical pending approvals so repeated waits don't pile up - Executor prompts now carve out awaiting-approval as a legitimate turn end (agent-prompts.ts) - Extend provisioning-gate and agent-action-gate coverage for the new suspend/carveout behavior - Add changeset (patch) documenting the fix for release notes - Update docs/agents.md and docs/architecture.md to describe the new blocking behavior Files changed: .changeset/fn-7608-awaiting-approval-blocking.md | 7 ++ docs/agents.md | 1 + docs/architecture.md | 1 + packages/core/src/agent-prompts.ts | 5 + .../engine/src/__tests__/agent-action-gate.test.ts | 82 +++++++++++++ .../executor-approval-gate-suspend.test.ts | 128 +++++++++++++++++++++ .../executor-approval-prompt-carveout.test.ts | 61 ++++++++++ packages/engine/src/agent-heartbeat.ts | 13 +++ packages/engine/src/executor.ts | 28 +++++ packages/engine/src/pi.ts | 22 +++- .../sandbox/__tests__/provisioning-gate.test.ts | 29 +++++ packages/engine/src/sandbox/provisioning-gate.ts | 11 ++ 12 files changed, 384 insertions(+), 4 deletions(-) Fusion-Task-Id: FN-7608 Fusion-Task-Lineage: 9e42d8ee-bda7-4ef1-b159-46c2100bbc48 Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
This commit is contained in:
gsxdsm
7
.changeset/fn-7608-awaiting-approval-blocking.md
Normal file
7
.changeset/fn-7608-awaiting-approval-blocking.md
Normal file
@@ -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.
|
||||
@@ -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=<requester>`); 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`.
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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<string, { id: string; status: "pending" | "approved" | "denied" }>();
|
||||
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",
|
||||
|
||||
@@ -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<string, Set<(...args: any[]) => 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<void>((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;
|
||||
});
|
||||
});
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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 });
|
||||
|
||||
@@ -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");
|
||||
|
||||
@@ -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.`,
|
||||
);
|
||||
},
|
||||
};
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user