Skip to content

Commit 6cc08d5

Browse files
ishulginishulgin
committed
Add more e2e mcp approval tests
1 parent 4d11bb7 commit 6cc08d5

1 file changed

Lines changed: 119 additions & 18 deletions

File tree

‎src/__tests__/CodexACPAgent/e2e/acp-e2e-mcp-approval.test.ts‎

Lines changed: 119 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
type PermissionResponder,
1111
type SpawnedAgentFixture,
1212
} from "./acp-e2e-test-utils";
13+
import os from "node:os";
1314

1415
const MCP_SERVER_NAME = "integration-mcp";
1516
const MCP_ECHO_MESSAGE = "mcp approval e2e";
@@ -37,11 +38,32 @@ function createMcpPermissionResponse(optionId: McpApprovalOptionIdValue | null):
3738
return {outcome: {outcome: "selected", optionId}};
3839
}
3940

40-
function createMcpPermissionResponder(optionId: McpApprovalOptionIdValue): PermissionResponder {
41-
return (request) => createMcpPermissionResponse(isMcpPermissionRequest(request) ? optionId : null);
41+
function createMcpPermissionResponder(...optionIds: McpApprovalOptionIdValue[]): PermissionResponder {
42+
const queue = [...optionIds];
43+
return (request) => createMcpPermissionResponse(
44+
isMcpPermissionRequest(request)
45+
? queue.shift() ?? McpApprovalOptionId.Decline
46+
: null,
47+
);
4248
}
4349

44-
describeE2E("E2E MCP approval tests", () => {
50+
async function expectEchoToolReply(fixture: SpawnedAgentFixture, sessionId: string, message: string): Promise<void> {
51+
await fixture.expectPromptText(
52+
sessionId,
53+
`Use the ${MCP_SERVER_NAME} MCP echo tool with message "${message}". Reply with exactly the tool result and no extra text.`,
54+
(text) => expect(text).toContain(`You said: ${message}`),
55+
);
56+
}
57+
58+
function expectMcpPermissionRequestCount(fixture: SpawnedAgentFixture, sessionId: string, count: number): void {
59+
const requests = fixture.readPermissionRequests(sessionId, "execute");
60+
expect(requests.length).toBe(count);
61+
for (const request of requests) {
62+
expect(isMcpPermissionRequest(request)).toBe(true);
63+
}
64+
}
65+
66+
describeE2E("E2E MCP approval tests (configured in session)", () => {
4567
let fixture: SpawnedAgentFixture;
4668

4769
beforeEach(async () => {
@@ -52,30 +74,24 @@ describeE2E("E2E MCP approval tests", () => {
5274
await fixture.dispose();
5375
});
5476

55-
function expectMcpToolPermissionRequest(sessionId: string): void {
56-
const requests = fixture.readPermissionRequests(sessionId, "execute");
57-
expect(requests.length).toBe(1);
58-
expect(isMcpPermissionRequest(requests[0]!)).toBe(true);
77+
async function createMcpSession(): Promise<{ sessionId: string; invocationMarkerPath: string }> {
78+
const invocationMarkerPath = path.join(fixture.workspaceDir, `mcp-tool-invocation-${crypto.randomUUID()}.txt`);
79+
const sessionId = (await fixture.createSession([createMcpServer(invocationMarkerPath)])).sessionId;
80+
return {sessionId, invocationMarkerPath};
5981
}
6082

6183
it("executes an approved MCP tool call", async () => {
6284
fixture.setPermissionResponder(createMcpPermissionResponder(McpApprovalOptionId.AllowOnce));
63-
const invocationMarkerPath = path.join(fixture.workspaceDir, `mcp-tool-invocation-${crypto.randomUUID()}.txt`);
64-
const sessionId = (await fixture.createSession([createMcpServer(invocationMarkerPath)])).sessionId;
85+
const {sessionId, invocationMarkerPath} = await createMcpSession();
6586

66-
await fixture.expectPromptText(
67-
sessionId,
68-
`Use the ${MCP_SERVER_NAME} MCP echo tool with message "${MCP_ECHO_MESSAGE}". Reply with exactly the tool result and no extra text.`,
69-
(text) => expect(text).toContain(`You said: ${MCP_ECHO_MESSAGE}`),
70-
);
87+
await expectEchoToolReply(fixture, sessionId, MCP_ECHO_MESSAGE);
7188
expect(fs.readFileSync(invocationMarkerPath, "utf8")).toBe(MCP_ECHO_MESSAGE);
72-
expectMcpToolPermissionRequest(sessionId);
89+
expectMcpPermissionRequestCount(fixture, sessionId, 1);
7390
});
7491

7592
it("ends turn when MCP tool call is rejected", async () => {
7693
fixture.setPermissionResponder(createMcpPermissionResponder(McpApprovalOptionId.Decline));
77-
const invocationMarkerPath = path.join(fixture.workspaceDir, `mcp-tool-invocation-${crypto.randomUUID()}.txt`);
78-
const sessionId = (await fixture.createSession([createMcpServer(invocationMarkerPath)])).sessionId;
94+
const {sessionId, invocationMarkerPath} = await createMcpSession();
7995

8096
expectEndTurn(await fixture.connection.prompt({
8197
sessionId,
@@ -85,6 +101,91 @@ describeE2E("E2E MCP approval tests", () => {
85101
}],
86102
}));
87103
expect(fs.existsSync(invocationMarkerPath)).toBe(false);
88-
expectMcpToolPermissionRequest(sessionId);
104+
expectMcpPermissionRequestCount(fixture, sessionId, 1);
105+
});
106+
107+
it("skips subsequent approvals in the same session when allow_session is selected", async () => {
108+
fixture.setPermissionResponder(createMcpPermissionResponder(McpApprovalOptionId.AllowSession));
109+
const {sessionId, invocationMarkerPath} = await createMcpSession();
110+
111+
await expectEchoToolReply(fixture, sessionId, "session approval first");
112+
await expectEchoToolReply(fixture, sessionId, "session approval second");
113+
114+
expect(fs.readFileSync(invocationMarkerPath, "utf8")).toBe("session approval second");
115+
expectMcpPermissionRequestCount(fixture, sessionId, 1);
116+
});
117+
118+
it("requests subsequent approvals after session restart when allow_session is selected", async () => {
119+
fixture.setPermissionResponder(createMcpPermissionResponder(McpApprovalOptionId.AllowSession, McpApprovalOptionId.AllowOnce));
120+
const {sessionId, invocationMarkerPath} = await createMcpSession();
121+
122+
await expectEchoToolReply(fixture, sessionId, MCP_ECHO_MESSAGE);
123+
expectMcpPermissionRequestCount(fixture, sessionId, 1);
124+
125+
fixture = await fixture.restart();
126+
await fixture.connection.loadSession({
127+
sessionId,
128+
cwd: fixture.workspaceDir,
129+
mcpServers: [createMcpServer(invocationMarkerPath)],
130+
});
131+
132+
await expectEchoToolReply(fixture, sessionId, MCP_ECHO_MESSAGE);
133+
expectMcpPermissionRequestCount(fixture, sessionId, 2);
134+
});
135+
});
136+
137+
describeE2E("E2E MCP approval tests (configured in toml)", () => {
138+
let invocationMarkerPath: string;
139+
let fixture: SpawnedAgentFixture;
140+
141+
beforeEach(async () => {
142+
fixture = await createAuthenticatedFixture();
143+
invocationMarkerPath = path.join(os.tmpdir(), `mcp-tool-invocation-${crypto.randomUUID()}.txt`)
144+
});
145+
146+
afterEach(async () => {
147+
await fixture.dispose();
148+
fs.rmSync(invocationMarkerPath, { force: true });
149+
});
150+
151+
152+
beforeEach(async () => {
153+
await fixture.dispose();
154+
fixture = await createAuthenticatedFixture(undefined, [createMcpServer(invocationMarkerPath)]);
155+
});
156+
157+
it("skips subsequent approvals in the same session when allow_always is selected", async () => {
158+
fixture.setPermissionResponder(
159+
createMcpPermissionResponder(McpApprovalOptionId.AllowAlways),
160+
);
161+
const sessionId = (await fixture.createSession()).sessionId;
162+
163+
await expectEchoToolReply(fixture, sessionId, "always approval first");
164+
await expectEchoToolReply(fixture, sessionId, "always approval second");
165+
166+
expect(fs.readFileSync(invocationMarkerPath, "utf8")).toBe("always approval second");
167+
expectMcpPermissionRequestCount(fixture, sessionId, 1);
168+
});
169+
170+
it.skip("skips subsequent approvals after session restart when allow_always is selected", async () => {
171+
fixture.setPermissionResponder(
172+
createMcpPermissionResponder(McpApprovalOptionId.AllowAlways),
173+
);
174+
const firstSessionId = (await fixture.createSession()).sessionId;
175+
176+
await expectEchoToolReply(fixture, firstSessionId, "always approval first");
177+
178+
fixture = await fixture.restart();
179+
fixture.setPermissionResponder((request) => {
180+
if (isMcpPermissionRequest(request)) {
181+
throw new Error("unexpected MCP approval after allow_always restart");
182+
}
183+
return createMcpPermissionResponse(null);
184+
});
185+
const newSessionId = (await fixture.createSession()).sessionId;
186+
await expectEchoToolReply(fixture, newSessionId, "always approval second");
187+
188+
expect(fs.readFileSync(invocationMarkerPath, "utf8")).toBe("always approval second");
189+
expect(fixture.readPermissionRequests(newSessionId, "execute").length).toBe(0);
89190
});
90191
});

0 commit comments

Comments
 (0)