diff --git a/docs/gateway/protocol.md b/docs/gateway/protocol.md index 09f5f718d00..a13f9117138 100644 --- a/docs/gateway/protocol.md +++ b/docs/gateway/protocol.md @@ -381,16 +381,18 @@ implemented in `src/gateway/server-methods/*.ts`. #### Approval families -- `exec.approval.request` and `exec.approval.resolve` cover one-shot exec - approval requests. +- `exec.approval.request`, `exec.approval.get`, `exec.approval.list`, and + `exec.approval.resolve` cover one-shot exec approval requests plus pending + approval lookup/replay. - `exec.approval.waitDecision` waits on one pending exec approval and returns the final decision (or `null` on timeout). - `exec.approvals.get` and `exec.approvals.set` manage gateway exec approval policy snapshots. - `exec.approvals.node.get` and `exec.approvals.node.set` manage node-local exec approval policy via node relay commands. -- `plugin.approval.request`, `plugin.approval.waitDecision`, and - `plugin.approval.resolve` cover plugin-defined approval flows. +- `plugin.approval.request`, `plugin.approval.list`, + `plugin.approval.waitDecision`, and `plugin.approval.resolve` cover + plugin-defined approval flows. #### Other major families diff --git a/src/infra/exec-approval-channel-runtime.test.ts b/src/infra/exec-approval-channel-runtime.test.ts index 70991bf5a74..5a004527a9c 100644 --- a/src/infra/exec-approval-channel-runtime.test.ts +++ b/src/infra/exec-approval-channel-runtime.test.ts @@ -491,6 +491,56 @@ describe("createExecApprovalChannelRuntime", () => { expect(deliverRequested).toHaveBeenCalledTimes(1); }); + it("does not replay approvals after stop wins once hello is already complete", async () => { + const replayDeferred = createDeferred< + Array<{ + id: string; + request: { command: string }; + createdAtMs: number; + expiresAtMs: number; + }> + >(); + mockGatewayClientRequests.mockImplementation(async (method) => { + if (method === "exec.approval.list") { + return replayDeferred.promise; + } + return { ok: true }; + }); + const deliverRequested = vi.fn(async (request) => [{ id: request.id }]); + const runtime = createExecApprovalChannelRuntime({ + label: "test/replay-stop-after-ready", + clientDisplayName: "Test Replay Stop", + cfg: {} as never, + isConfigured: () => true, + shouldHandle: () => true, + deliverRequested, + finalizeResolved: async () => undefined, + }); + + const startPromise = runtime.start(); + await vi.waitFor(() => { + expect(mockGatewayClientRequests).toHaveBeenCalledWith("exec.approval.list", {}); + }); + + const stopPromise = runtime.stop(); + replayDeferred.resolve([ + { + id: "abc", + request: { + command: "echo abc", + }, + createdAtMs: 1000, + expiresAtMs: 2000, + }, + ]); + + await startPromise; + await stopPromise; + + expect(deliverRequested).not.toHaveBeenCalled(); + expect(mockGatewayClientStops).toHaveBeenCalled(); + }); + it("clears pending state when delivery throws", async () => { const deliverRequested = vi .fn<() => Promise>>() diff --git a/src/infra/exec-approval-channel-runtime.ts b/src/infra/exec-approval-channel-runtime.ts index c806606bcd4..57274dfaba0 100644 --- a/src/infra/exec-approval-channel-runtime.ts +++ b/src/infra/exec-approval-channel-runtime.ts @@ -89,6 +89,8 @@ export function createExecApprovalChannelRuntime< let shouldRun = false; let startPromise: Promise | null = null; + const shouldKeepRunning = (): boolean => shouldRun; + const spawn = (label: string, promise: Promise): void => { void promise.catch((err: unknown) => { const message = formatErrorMessage(err); @@ -96,6 +98,15 @@ export function createExecApprovalChannelRuntime< }); }; + const stopClientIfInactive = (client: GatewayClient): boolean => { + if (shouldKeepRunning()) { + return false; + } + gatewayClient = null; + client.stop(); + return true; + }; + const clearPendingEntry = ( approvalId: string, ): PendingApprovalEntry | null => { @@ -122,7 +133,13 @@ export function createExecApprovalChannelRuntime< }); }; - const handleRequested = async (request: TRequest): Promise => { + const handleRequested = async ( + request: TRequest, + opts?: { ignoreIfInactive?: boolean }, + ): Promise => { + if (opts?.ignoreIfInactive && !shouldKeepRunning()) { + return; + } if (!adapter.shouldHandle(request)) { return; } @@ -202,11 +219,17 @@ export function createExecApprovalChannelRuntime< const handleGatewayEvent = (evt: EventFrame): void => { if (evt.event === "exec.approval.requested" && eventKinds.has("exec")) { - spawn("error handling approval request", handleRequested(evt.payload as TRequest)); + spawn( + "error handling approval request", + handleRequested(evt.payload as TRequest, { ignoreIfInactive: true }), + ); return; } if (evt.event === "plugin.approval.requested" && eventKinds.has("plugin")) { - spawn("error handling approval request", handleRequested(evt.payload as TRequest)); + spawn( + "error handling approval request", + handleRequested(evt.payload as TRequest, { ignoreIfInactive: true }), + ); return; } if (evt.event === "exec.approval.resolved" && eventKinds.has("exec")) { @@ -278,10 +301,22 @@ export function createExecApprovalChannelRuntime< try { client.start(); await ready; + if (stopClientIfInactive(client)) { + return; + } for (const method of resolveApprovalReplayMethods(eventKinds)) { + if (stopClientIfInactive(client)) { + return; + } const pendingRequests = await client.request>(method, {}); + if (stopClientIfInactive(client)) { + return; + } for (const request of pendingRequests) { - await handleRequested(request); + if (stopClientIfInactive(client)) { + return; + } + await handleRequested(request, { ignoreIfInactive: true }); } } started = true;