From 54b121cbd8daba663a3acfe701d32a87ebe7aced Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Mon, 15 Jun 2026 14:36:20 +0200 Subject: [PATCH] fix: Ignore Guardian warning notifications These ended up being quite noisy, the tool calls themselves already had the correct information. --- src/CodexEventHandler.ts | 13 +------ src/CodexToolCallMapper.ts | 3 ++ ...proval-review-completed-without-start.json | 2 +- .../data/guardian-approval-review-flow.json | 4 +- .../guardian-approval-review-events.test.ts | 38 ++++++++++++++++++- 5 files changed, 43 insertions(+), 17 deletions(-) diff --git a/src/CodexEventHandler.ts b/src/CodexEventHandler.ts index 5b1d520a..c685bc1d 100644 --- a/src/CodexEventHandler.ts +++ b/src/CodexEventHandler.ts @@ -14,7 +14,6 @@ import type { CommandExecutionOutputDeltaNotification, ConfigWarningNotification, ErrorNotification, - GuardianWarningNotification, ItemGuardianApprovalReviewCompletedNotification, ItemGuardianApprovalReviewStartedNotification, ItemCompletedNotification, @@ -131,7 +130,7 @@ export class CodexEventHandler { case "warning": return this.createWarningEvent(notification.params); case "guardianWarning": - return this.createGuardianWarningEvent(notification.params); + return null; case "item/autoApprovalReview/started": return this.handleGuardianApprovalReviewStarted(notification.params); case "item/autoApprovalReview/completed": @@ -237,16 +236,6 @@ export class CodexEventHandler { }; } - private createGuardianWarningEvent(event: GuardianWarningNotification): UpdateSessionEvent { - return { - sessionUpdate: "agent_message_chunk", - content: { - type: "text", - text: `Guardian warning: ${event.message}\n\n` - } - }; - } - private createModelReroutedEvent(event: ModelReroutedNotification): UpdateSessionEvent { return { sessionUpdate: "agent_thought_chunk", diff --git a/src/CodexToolCallMapper.ts b/src/CodexToolCallMapper.ts index 06454aab..6e733612 100644 --- a/src/CodexToolCallMapper.ts +++ b/src/CodexToolCallMapper.ts @@ -321,6 +321,9 @@ function createGuardianApprovalReviewContent( if (review.riskLevel) { lines.push(`Risk: ${review.riskLevel}`); } + if (review.userAuthorization) { + lines.push(`Authorization: ${review.userAuthorization}`); + } if (review.rationale?.trim()) { lines.push(`Rationale: ${review.rationale}`); } diff --git a/src/__tests__/CodexACPAgent/data/guardian-approval-review-completed-without-start.json b/src/__tests__/CodexACPAgent/data/guardian-approval-review-completed-without-start.json index 3f9dd03d..d27929c8 100644 --- a/src/__tests__/CodexACPAgent/data/guardian-approval-review-completed-without-start.json +++ b/src/__tests__/CodexACPAgent/data/guardian-approval-review-completed-without-start.json @@ -14,7 +14,7 @@ "type": "content", "content": { "type": "text", - "text": "Status: Denied\nAction: network access to api.example.com\nRisk: high\nRationale: The network target is not permitted." + "text": "Status: Denied\nAction: network access to api.example.com\nRisk: high\nAuthorization: low\nRationale: The network target is not permitted." } } ], diff --git a/src/__tests__/CodexACPAgent/data/guardian-approval-review-flow.json b/src/__tests__/CodexACPAgent/data/guardian-approval-review-flow.json index d92e765e..7bd7846d 100644 --- a/src/__tests__/CodexACPAgent/data/guardian-approval-review-flow.json +++ b/src/__tests__/CodexACPAgent/data/guardian-approval-review-flow.json @@ -14,7 +14,7 @@ "type": "content", "content": { "type": "text", - "text": "Status: In progress\nAction: exec /bin/ls -l\nRisk: medium\nRationale: Checking whether this command should run automatically." + "text": "Status: In progress\nAction: exec /bin/ls -l\nRisk: medium\nAuthorization: unknown\nRationale: Checking whether this command should run automatically." } } ], @@ -59,7 +59,7 @@ "type": "content", "content": { "type": "text", - "text": "Status: Approved\nAction: exec /bin/ls -l\nRisk: low\nRationale: The command only lists files." + "text": "Status: Approved\nAction: exec /bin/ls -l\nRisk: low\nAuthorization: medium\nRationale: The command only lists files." } } ], diff --git a/src/__tests__/CodexACPAgent/guardian-approval-review-events.test.ts b/src/__tests__/CodexACPAgent/guardian-approval-review-events.test.ts index 820a69f5..2f2e4402 100644 --- a/src/__tests__/CodexACPAgent/guardian-approval-review-events.test.ts +++ b/src/__tests__/CodexACPAgent/guardian-approval-review-events.test.ts @@ -73,10 +73,27 @@ describe("CodexEventHandler - Guardian approval review events", () => { }, }, }; + const duplicateWarning: ServerNotification = { + method: "guardianWarning", + params: { + threadId: sessionId, + message: "Automatic approval review approved (risk: low, authorization: medium): The command only lists files.", + }, + }; - await setupPromptAndSendNotifications(mockFixture, sessionId, sessionState, [started, completed]); + await setupPromptAndSendNotifications(mockFixture, sessionId, sessionState, [ + started, + duplicateWarning, + completed, + ]); - await expect(mockFixture.getAcpConnectionDump([])).toMatchFileSnapshot( + const dump = mockFixture.getAcpConnectionDump([]); + expect(dump).not.toContain("agent_message_chunk"); + expect(dump).not.toContain("Guardian warning"); + expect(dump).not.toContain("Automatic approval review approved"); + expect(dump).toContain("tool_call_update"); + expect(dump).toContain("Authorization: medium"); + await expect(dump).toMatchFileSnapshot( "data/guardian-approval-review-flow.json" ); }); @@ -114,4 +131,21 @@ describe("CodexEventHandler - Guardian approval review events", () => { "data/guardian-approval-review-completed-without-start.json" ); }); + + it("ignores Guardian warnings that are not approval review summaries", async () => { + const warning: ServerNotification = { + method: "guardianWarning", + params: { + threadId: sessionId, + message: "Automatic approval review rejected too many approval requests for this turn (3 consecutive, 5 in the last 10 reviews); interrupting the turn.", + }, + }; + + vi.spyOn(mockFixture.getCodexAcpAgent(), "getSessionState").mockReturnValue(sessionState); + mockFixture.sendServerNotification(warning); + await mockFixture.getCodexAcpClient().waitForSessionNotifications(sessionId); + + const dump = mockFixture.getAcpConnectionDump([]); + expect(dump).toBe(""); + }); });