Skip to content

Commit 79239d5

Browse files
NikolaiSviridovNikolaiSviridov
committed
fix: ApprovalOptionId duplication
1 parent fe1c965 commit 79239d5

9 files changed

Lines changed: 29 additions & 33 deletions

‎src/ApprovalOptionId.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ export const ApprovalOptionId = {
55
AllowCommandPrefixRule: "allow_command_prefix_rule",
66
ApplyNetworkPolicyAmendment: "apply_network_policy_amendment",
77
RejectOnce: "reject_once",
8-
Cancel: "cancel",
98
} as const;
109

1110
export type ApprovalOptionId = typeof ApprovalOptionId[keyof typeof ApprovalOptionId];

‎src/CodexApprovalHandler.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ export class CodexApprovalHandler implements ApprovalHandler {
163163
if (decision === "cancel") {
164164
return {
165165
option: {
166-
optionId: ApprovalOptionId.Cancel,
166+
optionId: ApprovalOptionId.RejectOnce,
167167
name: "No, and tell Codex what to do differently",
168168
kind: "reject_once"
169169
},

‎src/CodexElicitationHandler.ts‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,10 @@ import { ApprovalOptionId } from "./ApprovalOptionId";
1414

1515
// Standard elicitation options (non-tool-call approval).
1616
const ELICITATION_OPTIONS: acp.PermissionOption[] = [
17-
{ optionId: "accept", name: "Accept", kind: "allow_once" },
18-
{ optionId: "decline", name: "Decline", kind: "reject_once" },
17+
{ optionId: ApprovalOptionId.AllowOnce, name: "Accept", kind: "allow_once" },
18+
{ optionId: ApprovalOptionId.RejectOnce, name: "Decline", kind: "reject_once" },
1919
];
2020

21-
// Option ID unique to elicitation persist choices — not part of the shared ApprovalOptionId set.
22-
const OPTION_ALLOW_SESSION = "allow_session";
23-
2421
type PersistValue = "session" | "always";
2522

2623
function buildToolApprovalOption(
@@ -82,7 +79,7 @@ function buildToolApprovalOptions(persistOptions: Set<PersistValue>): acp.Permis
8279
// Only surface scopes the server explicitly offered.
8380
if (persistOptions.has("session")) {
8481
options.push(buildToolApprovalOption(
85-
OPTION_ALLOW_SESSION,
82+
ApprovalOptionId.AllowForSession,
8683
"Allow for this session",
8784
"allow_always",
8885
"Run the tool and remember this choice for this session."
@@ -97,7 +94,7 @@ function buildToolApprovalOptions(persistOptions: Set<PersistValue>): acp.Permis
9794
));
9895
}
9996
options.push(buildToolApprovalOption(
100-
"decline",
97+
ApprovalOptionId.RejectOnce,
10198
"Cancel",
10299
"reject_once",
103100
"Cancel this tool call."
@@ -160,7 +157,7 @@ export class CodexElicitationHandler implements ElicitationHandler {
160157
const response = await this.connection.requestPermission(request);
161158
if (correlatedCallId !== undefined && response.outcome.outcome !== "cancelled") {
162159
const optionId = response.outcome.optionId;
163-
if (optionId !== "decline") {
160+
if (optionId !== ApprovalOptionId.RejectOnce) {
164161
await this.connection.sessionUpdate({
165162
sessionId: this.sessionState.sessionId,
166163
update: { sessionUpdate: "tool_call_update", toolCallId: correlatedCallId, status: "in_progress" },
@@ -255,7 +252,7 @@ export class CodexElicitationHandler implements ElicitationHandler {
255252
}
256253

257254
const optionId = response.outcome.optionId;
258-
if (optionId === OPTION_ALLOW_SESSION) {
255+
if (optionId === ApprovalOptionId.AllowForSession) {
259256
// This _meta is part of Codex's MCP tool approval response contract.
260257
// It tells app-server to remember this approval for the current session.
261258
return { action: "accept", content: null, _meta: { persist: "session" } };
@@ -271,7 +268,7 @@ export class CodexElicitationHandler implements ElicitationHandler {
271268
// It tells app-server to persist this MCP tool approval across sessions.
272269
return { action: "accept", content: null, _meta: { persist: "always" } };
273270
}
274-
if (optionId === ApprovalOptionId.AllowOnce || optionId === "accept") {
271+
if (optionId === ApprovalOptionId.AllowOnce) {
275272
return { action: "accept", content: null, _meta: null };
276273
}
277274
return { action: "decline", content: null, _meta: null };

‎src/__tests__/CodexACPAgent/data/elicitation-form-accept.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,12 @@
3333
},
3434
"options": [
3535
{
36-
"optionId": "accept",
36+
"optionId": "allow_once",
3737
"name": "Accept",
3838
"kind": "allow_once"
3939
},
4040
{
41-
"optionId": "decline",
41+
"optionId": "reject_once",
4242
"name": "Decline",
4343
"kind": "reject_once"
4444
}

‎src/__tests__/CodexACPAgent/data/elicitation-tool-approval-all-persist.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
"_meta": "_meta"
3434
},
3535
{
36-
"optionId": "allow_session",
36+
"optionId": "allow_for_session",
3737
"name": "Allow for this session",
3838
"kind": "allow_always",
3939
"_meta": "_meta"
@@ -45,7 +45,7 @@
4545
"_meta": "_meta"
4646
},
4747
{
48-
"optionId": "decline",
48+
"optionId": "reject_once",
4949
"name": "Cancel",
5050
"kind": "reject_once",
5151
"_meta": "_meta"

‎src/__tests__/CodexACPAgent/data/elicitation-tool-approval-no-persist.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
"_meta": "_meta"
3434
},
3535
{
36-
"optionId": "decline",
36+
"optionId": "reject_once",
3737
"name": "Cancel",
3838
"kind": "reject_once",
3939
"_meta": "_meta"

‎src/__tests__/CodexACPAgent/data/elicitation-tool-approval-session-only.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,13 @@
3333
"_meta": "_meta"
3434
},
3535
{
36-
"optionId": "allow_session",
36+
"optionId": "allow_for_session",
3737
"name": "Allow for this session",
3838
"kind": "allow_always",
3939
"_meta": "_meta"
4040
},
4141
{
42-
"optionId": "decline",
42+
"optionId": "reject_once",
4343
"name": "Cancel",
4444
"kind": "reject_once",
4545
"_meta": "_meta"

‎src/__tests__/CodexACPAgent/data/elicitation-url-accept.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,12 @@
2323
},
2424
"options": [
2525
{
26-
"optionId": "accept",
26+
"optionId": "allow_once",
2727
"name": "Accept",
2828
"kind": "allow_once"
2929
},
3030
{
31-
"optionId": "decline",
31+
"optionId": "reject_once",
3232
"name": "Decline",
3333
"kind": "reject_once"
3434
}

‎src/__tests__/CodexACPAgent/elicitation-events.test.ts‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,9 @@ describe('Elicitation Events', () => {
5151
}
5252

5353
describe('Form mode elicitation', () => {
54-
it('should map accept to accept', async () => {
54+
it('should map allow_once to accept', async () => {
5555
const { promptPromise, completeTurn } = setupSessionWithPendingPrompt();
56-
fixture.setPermissionResponse({ outcome: { outcome: 'selected', optionId: 'accept' } });
56+
fixture.setPermissionResponse({ outcome: { outcome: 'selected', optionId: 'allow_once' } });
5757

5858
const params: McpServerElicitationRequestParams = {
5959
threadId: sessionId, turnId: 'turn-1', serverName: 'test-server',
@@ -68,9 +68,9 @@ describe('Elicitation Events', () => {
6868
await promptPromise;
6969
});
7070

71-
it('should map decline to decline', async () => {
71+
it('should map reject_once to decline', async () => {
7272
const { promptPromise, completeTurn } = setupSessionWithPendingPrompt();
73-
fixture.setPermissionResponse({ outcome: { outcome: 'selected', optionId: 'decline' } });
73+
fixture.setPermissionResponse({ outcome: { outcome: 'selected', optionId: 'reject_once' } });
7474

7575
const params: McpServerElicitationRequestParams = {
7676
threadId: sessionId, turnId: 'turn-1', serverName: 'test-server',
@@ -115,7 +115,7 @@ describe('Elicitation Events', () => {
115115

116116
it('should build correct ACP permission request for form mode', async () => {
117117
const { promptPromise, completeTurn } = setupSessionWithPendingPrompt();
118-
fixture.setPermissionResponse({ outcome: { outcome: 'selected', optionId: 'accept' } });
118+
fixture.setPermissionResponse({ outcome: { outcome: 'selected', optionId: 'allow_once' } });
119119

120120
const params: McpServerElicitationRequestParams = {
121121
threadId: sessionId, turnId: 'turn-1', serverName: 'my-mcp-server',
@@ -173,7 +173,7 @@ describe('Elicitation Events', () => {
173173
_meta: { description: 'Run the tool and continue.' },
174174
},
175175
{
176-
optionId: 'allow_session',
176+
optionId: 'allow_for_session',
177177
name: 'Allow for this session',
178178
kind: 'allow_always',
179179
_meta: { description: 'Run the tool and remember this choice for this session.' },
@@ -185,7 +185,7 @@ describe('Elicitation Events', () => {
185185
_meta: { description: 'Run the tool and remember this choice for future tool calls.' },
186186
},
187187
{
188-
optionId: 'decline',
188+
optionId: 'reject_once',
189189
name: 'Cancel',
190190
kind: 'reject_once',
191191
_meta: { description: 'Cancel this tool call.' },
@@ -215,9 +215,9 @@ describe('Elicitation Events', () => {
215215
await promptPromise;
216216
});
217217

218-
it('should map allow_session to accept with persist:session meta', async () => {
218+
it('should map allow_for_session to accept with persist:session meta', async () => {
219219
const { promptPromise, completeTurn } = setupSessionWithPendingPrompt();
220-
fixture.setPermissionResponse({ outcome: { outcome: 'selected', optionId: 'allow_session' } });
220+
fixture.setPermissionResponse({ outcome: { outcome: 'selected', optionId: 'allow_for_session' } });
221221

222222
const params: McpServerElicitationRequestParams = {
223223
threadId: sessionId, turnId: 'turn-1', serverName: 'tool-server',
@@ -465,9 +465,9 @@ describe('Elicitation Events', () => {
465465
});
466466

467467
describe('URL mode elicitation', () => {
468-
it('should map accept to accept for URL mode', async () => {
468+
it('should map allow_once to accept for URL mode', async () => {
469469
const { promptPromise, completeTurn } = setupSessionWithPendingPrompt();
470-
fixture.setPermissionResponse({ outcome: { outcome: 'selected', optionId: 'accept' } });
470+
fixture.setPermissionResponse({ outcome: { outcome: 'selected', optionId: 'allow_once' } });
471471

472472
const params: McpServerElicitationRequestParams = {
473473
threadId: sessionId, turnId: 'turn-1', serverName: 'auth-server',
@@ -501,7 +501,7 @@ describe('Elicitation Events', () => {
501501

502502
it('should build correct ACP permission request for URL mode', async () => {
503503
const { promptPromise, completeTurn } = setupSessionWithPendingPrompt();
504-
fixture.setPermissionResponse({ outcome: { outcome: 'selected', optionId: 'accept' } });
504+
fixture.setPermissionResponse({ outcome: { outcome: 'selected', optionId: 'allow_once' } });
505505

506506
const params: McpServerElicitationRequestParams = {
507507
threadId: sessionId, turnId: 'turn-1', serverName: 'auth-server',

0 commit comments

Comments
 (0)