Skip to content

Commit 54b121c

Browse files
committed
fix: Ignore Guardian warning notifications
These ended up being quite noisy, the tool calls themselves already had the correct information.
1 parent f99ad4c commit 54b121c

5 files changed

Lines changed: 43 additions & 17 deletions

src/CodexEventHandler.ts

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import type {
1414
CommandExecutionOutputDeltaNotification,
1515
ConfigWarningNotification,
1616
ErrorNotification,
17-
GuardianWarningNotification,
1817
ItemGuardianApprovalReviewCompletedNotification,
1918
ItemGuardianApprovalReviewStartedNotification,
2019
ItemCompletedNotification,
@@ -131,7 +130,7 @@ export class CodexEventHandler {
131130
case "warning":
132131
return this.createWarningEvent(notification.params);
133132
case "guardianWarning":
134-
return this.createGuardianWarningEvent(notification.params);
133+
return null;
135134
case "item/autoApprovalReview/started":
136135
return this.handleGuardianApprovalReviewStarted(notification.params);
137136
case "item/autoApprovalReview/completed":
@@ -237,16 +236,6 @@ export class CodexEventHandler {
237236
};
238237
}
239238

240-
private createGuardianWarningEvent(event: GuardianWarningNotification): UpdateSessionEvent {
241-
return {
242-
sessionUpdate: "agent_message_chunk",
243-
content: {
244-
type: "text",
245-
text: `Guardian warning: ${event.message}\n\n`
246-
}
247-
};
248-
}
249-
250239
private createModelReroutedEvent(event: ModelReroutedNotification): UpdateSessionEvent {
251240
return {
252241
sessionUpdate: "agent_thought_chunk",

src/CodexToolCallMapper.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,9 @@ function createGuardianApprovalReviewContent(
321321
if (review.riskLevel) {
322322
lines.push(`Risk: ${review.riskLevel}`);
323323
}
324+
if (review.userAuthorization) {
325+
lines.push(`Authorization: ${review.userAuthorization}`);
326+
}
324327
if (review.rationale?.trim()) {
325328
lines.push(`Rationale: ${review.rationale}`);
326329
}

src/__tests__/CodexACPAgent/data/guardian-approval-review-completed-without-start.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
"type": "content",
1515
"content": {
1616
"type": "text",
17-
"text": "Status: Denied\nAction: network access to api.example.com\nRisk: high\nRationale: The network target is not permitted."
17+
"text": "Status: Denied\nAction: network access to api.example.com\nRisk: high\nAuthorization: low\nRationale: The network target is not permitted."
1818
}
1919
}
2020
],

src/__tests__/CodexACPAgent/data/guardian-approval-review-flow.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
"type": "content",
1515
"content": {
1616
"type": "text",
17-
"text": "Status: In progress\nAction: exec /bin/ls -l\nRisk: medium\nRationale: Checking whether this command should run automatically."
17+
"text": "Status: In progress\nAction: exec /bin/ls -l\nRisk: medium\nAuthorization: unknown\nRationale: Checking whether this command should run automatically."
1818
}
1919
}
2020
],
@@ -59,7 +59,7 @@
5959
"type": "content",
6060
"content": {
6161
"type": "text",
62-
"text": "Status: Approved\nAction: exec /bin/ls -l\nRisk: low\nRationale: The command only lists files."
62+
"text": "Status: Approved\nAction: exec /bin/ls -l\nRisk: low\nAuthorization: medium\nRationale: The command only lists files."
6363
}
6464
}
6565
],

src/__tests__/CodexACPAgent/guardian-approval-review-events.test.ts

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,10 +73,27 @@ describe("CodexEventHandler - Guardian approval review events", () => {
7373
},
7474
},
7575
};
76+
const duplicateWarning: ServerNotification = {
77+
method: "guardianWarning",
78+
params: {
79+
threadId: sessionId,
80+
message: "Automatic approval review approved (risk: low, authorization: medium): The command only lists files.",
81+
},
82+
};
7683

77-
await setupPromptAndSendNotifications(mockFixture, sessionId, sessionState, [started, completed]);
84+
await setupPromptAndSendNotifications(mockFixture, sessionId, sessionState, [
85+
started,
86+
duplicateWarning,
87+
completed,
88+
]);
7889

79-
await expect(mockFixture.getAcpConnectionDump([])).toMatchFileSnapshot(
90+
const dump = mockFixture.getAcpConnectionDump([]);
91+
expect(dump).not.toContain("agent_message_chunk");
92+
expect(dump).not.toContain("Guardian warning");
93+
expect(dump).not.toContain("Automatic approval review approved");
94+
expect(dump).toContain("tool_call_update");
95+
expect(dump).toContain("Authorization: medium");
96+
await expect(dump).toMatchFileSnapshot(
8097
"data/guardian-approval-review-flow.json"
8198
);
8299
});
@@ -114,4 +131,21 @@ describe("CodexEventHandler - Guardian approval review events", () => {
114131
"data/guardian-approval-review-completed-without-start.json"
115132
);
116133
});
134+
135+
it("ignores Guardian warnings that are not approval review summaries", async () => {
136+
const warning: ServerNotification = {
137+
method: "guardianWarning",
138+
params: {
139+
threadId: sessionId,
140+
message: "Automatic approval review rejected too many approval requests for this turn (3 consecutive, 5 in the last 10 reviews); interrupting the turn.",
141+
},
142+
};
143+
144+
vi.spyOn(mockFixture.getCodexAcpAgent(), "getSessionState").mockReturnValue(sessionState);
145+
mockFixture.sendServerNotification(warning);
146+
await mockFixture.getCodexAcpClient().waitForSessionNotifications(sessionId);
147+
148+
const dump = mockFixture.getAcpConnectionDump([]);
149+
expect(dump).toBe("");
150+
});
117151
});

0 commit comments

Comments
 (0)