From bbabd9bec4d0ff05751a66608be519fbf84d9da1 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 9 May 2026 19:57:08 +0100 Subject: [PATCH] test: tighten exec approval followup assertions --- .../bash-tools.exec-approval-followup.test.ts | 157 +++++++++--------- 1 file changed, 83 insertions(+), 74 deletions(-) diff --git a/src/agents/bash-tools.exec-approval-followup.test.ts b/src/agents/bash-tools.exec-approval-followup.test.ts index 0d66d9a01ae..d6e4199cc3e 100644 --- a/src/agents/bash-tools.exec-approval-followup.test.ts +++ b/src/agents/bash-tools.exec-approval-followup.test.ts @@ -19,6 +19,40 @@ afterEach(() => { vi.resetAllMocks(); }); +function requireRecord(value: unknown, label: string): Record { + if (!value || typeof value !== "object" || Array.isArray(value)) { + throw new Error(`expected ${label}`); + } + return value as Record; +} + +function requireFirstMockCall(mock: unknown, label: string): unknown[] { + const call = (mock as { mock?: { calls?: unknown[][] } }).mock?.calls?.[0]; + if (!call) { + throw new Error(`expected ${label}`); + } + return call; +} + +function expectGatewayAgentFollowup(expected: Record) { + const call = requireFirstMockCall(callGatewayTool, "callGatewayTool call"); + expect(call[0]).toBe("agent"); + requireRecord(call[1], "gateway tool context"); + const params = requireRecord(call[2], "gateway tool params"); + for (const [key, value] of Object.entries(expected)) { + expect(params[key]).toBe(value); + } + expect(call[3]).toEqual({ expectFinal: true }); +} + +function expectDirectSend(expected: Record) { + const call = requireFirstMockCall(sendMessage, "sendMessage call"); + const params = requireRecord(call[0], "sendMessage params"); + for (const [key, value] of Object.entries(expected)) { + expect(params[key]).toBe(value); + } +} + describe("exec approval followup", () => { it("uses an explicit denial prompt when the command did not run", () => { const prompt = buildExecApprovalFollowupPrompt( @@ -44,17 +78,12 @@ describe("exec approval followup", () => { resultText: "Exec completed: echo ok", }); - expect(callGatewayTool).toHaveBeenCalledWith( - "agent", - expect.any(Object), - expect.objectContaining({ - sessionKey: "agent:main:main", - deliver: false, - channel: undefined, - to: undefined, - }), - { expectFinal: true }, - ); + expectGatewayAgentFollowup({ + sessionKey: "agent:main:main", + deliver: false, + channel: undefined, + to: undefined, + }); expect(sendMessage).not.toHaveBeenCalled(); }); @@ -91,21 +120,16 @@ describe("exec approval followup", () => { resultText: "slack exec approval smoke", }); - expect(callGatewayTool).toHaveBeenCalledWith( - "agent", - expect.any(Object), - expect.objectContaining({ - sessionKey: target.sessionKey, - deliver: true, - bestEffortDeliver: true, - channel: target.channel, - to: target.to, - accountId: target.accountId, - threadId: target.threadId, - idempotencyKey: `exec-approval-followup:req-${target.channel}`, - }), - { expectFinal: true }, - ); + expectGatewayAgentFollowup({ + sessionKey: target.sessionKey, + deliver: true, + bestEffortDeliver: true, + channel: target.channel, + to: target.to, + accountId: target.accountId, + threadId: target.threadId, + idempotencyKey: `exec-approval-followup:req-${target.channel}`, + }); expect(sendMessage).not.toHaveBeenCalled(); }); @@ -119,16 +143,14 @@ describe("exec approval followup", () => { resultText: "Exec finished (gateway id=req-no-session, session=sess_1, code 0)\nall good", }); - expect(sendMessage).toHaveBeenCalledWith( - expect.objectContaining({ - channel: "discord", - to: "123", - accountId: "default", - threadId: "456", - content: "all good", - idempotencyKey: "exec-approval-followup:req-no-session", - }), - ); + expectDirectSend({ + channel: "discord", + to: "123", + accountId: "default", + threadId: "456", + content: "all good", + idempotencyKey: "exec-approval-followup:req-no-session", + }); expect(callGatewayTool).not.toHaveBeenCalled(); }); @@ -144,15 +166,13 @@ describe("exec approval followup", () => { direct: true, }); - expect(sendMessage).toHaveBeenCalledWith( - expect.objectContaining({ - channel: "telegram", - to: "123", - accountId: "default", - content: "pasteable diagnostics report", - idempotencyKey: "exec-approval-followup:req-direct", - }), - ); + expectDirectSend({ + channel: "telegram", + to: "123", + accountId: "default", + content: "pasteable diagnostics report", + idempotencyKey: "exec-approval-followup:req-direct", + }); expect(callGatewayTool).not.toHaveBeenCalled(); }); @@ -170,12 +190,10 @@ describe("exec approval followup", () => { "Exec finished (gateway id=req-session-resume-failed, session=sess_1, code 0)\nall good", }); - expect(sendMessage).toHaveBeenCalledWith( - expect.objectContaining({ - content: "all good", - idempotencyKey: "exec-approval-followup:req-session-resume-failed", - }), - ); + expectDirectSend({ + content: "all good", + idempotencyKey: "exec-approval-followup:req-session-resume-failed", + }); }); it("uses a generic summary when a no-session completion has no user-visible output", async () => { @@ -188,12 +206,10 @@ describe("exec approval followup", () => { resultText: "Exec finished (gateway id=req-no-session-empty, session=sess_2, code 0)", }); - expect(sendMessage).toHaveBeenCalledWith( - expect.objectContaining({ - content: "Background command finished.", - idempotencyKey: "exec-approval-followup:req-no-session-empty", - }), - ); + expectDirectSend({ + content: "Background command finished.", + idempotencyKey: "exec-approval-followup:req-no-session-empty", + }); }); it("uses safe denied copy when session resume fails", async () => { @@ -209,13 +225,11 @@ describe("exec approval followup", () => { resultText: "Exec denied (gateway id=req-denied-resume-failed, approval-timeout): uname -a", }); - expect(sendMessage).toHaveBeenCalledWith( - expect.objectContaining({ - content: - "Automatic session resume failed, so sending the status directly.\n\nCommand did not run: approval timed out.", - idempotencyKey: "exec-approval-followup:req-denied-resume-failed", - }), - ); + expectDirectSend({ + content: + "Automatic session resume failed, so sending the status directly.\n\nCommand did not run: approval timed out.", + idempotencyKey: "exec-approval-followup:req-denied-resume-failed", + }); }); it("suppresses denied followups for subagent sessions", async () => { @@ -261,16 +275,11 @@ describe("exec approval followup", () => { resultText: "Exec completed: systemctl status gateway", }); - expect(callGatewayTool).toHaveBeenCalledWith( - "agent", - expect.any(Object), - expect.objectContaining({ - sessionKey: "agent:main:telegram:-100123", - deliver: false, - channel: "telegram", - }), - { expectFinal: true }, - ); + expectGatewayAgentFollowup({ + sessionKey: "agent:main:telegram:-100123", + deliver: false, + channel: "telegram", + }); expect(sendMessage).not.toHaveBeenCalled(); });