diff --git a/src/agents/bash-tools.exec-approval-followup.test.ts b/src/agents/bash-tools.exec-approval-followup.test.ts new file mode 100644 index 00000000000..b7cc5fb2ef2 --- /dev/null +++ b/src/agents/bash-tools.exec-approval-followup.test.ts @@ -0,0 +1,75 @@ +import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; + +vi.mock("./tools/gateway.js", () => ({ + callGatewayTool: vi.fn(async () => ({ ok: true })), +})); + +let callGatewayTool: typeof import("./tools/gateway.js").callGatewayTool; +let sendExecApprovalFollowup: typeof import("./bash-tools.exec-approval-followup.js").sendExecApprovalFollowup; + +describe("sendExecApprovalFollowup", () => { + beforeAll(async () => { + ({ callGatewayTool } = await import("./tools/gateway.js")); + ({ sendExecApprovalFollowup } = await import("./bash-tools.exec-approval-followup.js")); + }); + + beforeEach(() => { + vi.mocked(callGatewayTool).mockClear(); + }); + + it("keeps follow-up delivery internal when no outbound route is available", async () => { + await expect( + sendExecApprovalFollowup({ + approvalId: "approval-1", + sessionKey: "main", + resultText: "Exec finished", + }), + ).resolves.toBe(true); + + expect(callGatewayTool).toHaveBeenCalledWith( + "agent", + { timeoutMs: 60_000 }, + expect.objectContaining({ + sessionKey: "main", + deliver: false, + bestEffortDeliver: false, + channel: undefined, + to: undefined, + accountId: undefined, + threadId: undefined, + idempotencyKey: "exec-approval-followup:approval-1", + }), + { expectFinal: true }, + ); + }); + + it("preserves outbound delivery when the originating route is known", async () => { + await expect( + sendExecApprovalFollowup({ + approvalId: "approval-2", + sessionKey: "main", + turnSourceChannel: "telegram", + turnSourceTo: "12345", + turnSourceAccountId: "ops", + turnSourceThreadId: 77, + resultText: "Exec finished", + }), + ).resolves.toBe(true); + + expect(callGatewayTool).toHaveBeenCalledWith( + "agent", + { timeoutMs: 60_000 }, + expect.objectContaining({ + sessionKey: "main", + deliver: true, + bestEffortDeliver: true, + channel: "telegram", + to: "12345", + accountId: "ops", + threadId: "77", + idempotencyKey: "exec-approval-followup:approval-2", + }), + { expectFinal: true }, + ); + }); +}); diff --git a/src/agents/bash-tools.exec-approval-followup.ts b/src/agents/bash-tools.exec-approval-followup.ts index af24f07fb50..40f6ce38e61 100644 --- a/src/agents/bash-tools.exec-approval-followup.ts +++ b/src/agents/bash-tools.exec-approval-followup.ts @@ -35,6 +35,7 @@ export async function sendExecApprovalFollowup( const channel = params.turnSourceChannel?.trim(); const to = params.turnSourceTo?.trim(); + const canRouteOutbound = Boolean(channel && to); const threadId = params.turnSourceThreadId != null && params.turnSourceThreadId !== "" ? String(params.turnSourceThreadId) @@ -46,12 +47,12 @@ export async function sendExecApprovalFollowup( { sessionKey, message: buildExecApprovalFollowupPrompt(resultText), - deliver: true, - bestEffortDeliver: true, - channel: channel && to ? channel : undefined, - to: channel && to ? to : undefined, - accountId: channel && to ? params.turnSourceAccountId?.trim() || undefined : undefined, - threadId: channel && to ? threadId : undefined, + deliver: canRouteOutbound, + bestEffortDeliver: canRouteOutbound, + channel: canRouteOutbound ? channel : undefined, + to: canRouteOutbound ? to : undefined, + accountId: canRouteOutbound ? params.turnSourceAccountId?.trim() || undefined : undefined, + threadId: canRouteOutbound ? threadId : undefined, idempotencyKey: `exec-approval-followup:${params.approvalId}`, }, { expectFinal: true }, diff --git a/ui/src/ui/controllers/exec-approval.test.ts b/ui/src/ui/controllers/exec-approval.test.ts new file mode 100644 index 00000000000..fb264531e1b --- /dev/null +++ b/ui/src/ui/controllers/exec-approval.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, it, vi } from "vitest"; +import { + addExecApproval, + pruneExecApprovalQueue, + type ExecApprovalRequest, +} from "./exec-approval.ts"; + +function createApproval(params: { + id: string; + createdAtMs: number; + expiresAtMs: number; +}): ExecApprovalRequest { + return { + id: params.id, + createdAtMs: params.createdAtMs, + expiresAtMs: params.expiresAtMs, + request: { command: `echo ${params.id}` }, + }; +} + +describe("exec approval queue", () => { + it("keeps the newest approval at the front of the queue", () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-03-20T00:00:00.000Z")); + + const first = createApproval({ + id: "approval-1", + createdAtMs: Date.now(), + expiresAtMs: Date.now() + 60_000, + }); + const second = createApproval({ + id: "approval-2", + createdAtMs: Date.now() + 1_000, + expiresAtMs: Date.now() + 61_000, + }); + + expect(addExecApproval([first], second)).toEqual([second, first]); + }); + + it("prunes expired approvals before showing a new one", () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-03-20T00:00:10.000Z")); + + const expired = createApproval({ + id: "expired", + createdAtMs: Date.now() - 20_000, + expiresAtMs: Date.now() - 1, + }); + const active = createApproval({ + id: "active", + createdAtMs: Date.now(), + expiresAtMs: Date.now() + 60_000, + }); + + expect(pruneExecApprovalQueue([expired, active])).toEqual([active]); + expect(addExecApproval([expired], active)).toEqual([active]); + }); +}); diff --git a/ui/src/ui/controllers/exec-approval.ts b/ui/src/ui/controllers/exec-approval.ts index a0b4acddebe..aade2882b65 100644 --- a/ui/src/ui/controllers/exec-approval.ts +++ b/ui/src/ui/controllers/exec-approval.ts @@ -88,8 +88,7 @@ export function addExecApproval( entry: ExecApprovalRequest, ): ExecApprovalRequest[] { const next = pruneExecApprovalQueue(queue).filter((item) => item.id !== entry.id); - next.push(entry); - return next; + return [entry, ...next]; } export function removeExecApproval(