Skip to content

Commit 777a113

Browse files
kevinslinkevinslin
authored
fix(codex): await computer use elicitation bridge (#85117)
* fix(codex): bridge computer use elicitations * fix(codex): preserve computer use approval boundary * fix(codex): await app-server elicitation bridge
1 parent bc9e601 commit 777a113

5 files changed

Lines changed: 103 additions & 7 deletions

File tree

‎docs/channels/slack.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -742,6 +742,11 @@ The default manifest enables the Slack App Home **Home** tab and subscribes to `
742742
"description": "Show or set exec defaults",
743743
"usage_hint": "host=<auto|sandbox|gateway|node> security=<deny|allowlist|full> ask=<off|on-miss|always> node=<id>"
744744
},
745+
{
746+
"command": "/approve",
747+
"description": "Approve or deny pending approval requests",
748+
"usage_hint": "<id> <decision>"
749+
},
745750
{
746751
"command": "/model",
747752
"description": "Show or set the model",

‎extensions/codex/src/app-server/elicitation-bridge.test.ts‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -439,7 +439,14 @@ describe("Codex app-server elicitation bridge", () => {
439439
);
440440
});
441441

442-
it("does not bridge Computer Use elicitations without an approval form schema", async () => {
442+
it("normalizes missing Computer Use schemas to the empty object schema", async () => {
443+
mockCallGatewayTool
444+
.mockResolvedValueOnce({ id: "plugin:approval-computer-use-schema", status: "accepted" })
445+
.mockResolvedValueOnce({
446+
id: "plugin:approval-computer-use-schema",
447+
decision: "allow-once",
448+
});
449+
443450
const result = await handleCodexAppServerElicitationRequest({
444451
requestParams: buildComputerUseApprovalElicitation({
445452
requestedSchema: "not-a-schema",
@@ -451,8 +458,11 @@ describe("Codex app-server elicitation bridge", () => {
451458
computerUseMcpServerName: "computer-use",
452459
});
453460

454-
expect(result).toBeUndefined();
455-
expect(mockCallGatewayTool).not.toHaveBeenCalled();
461+
expect(result).toEqual({
462+
action: "accept",
463+
content: null,
464+
_meta: null,
465+
});
456466
});
457467

458468
it("does not bridge Computer Use elicitations outside form mode", async () => {

‎extensions/codex/src/app-server/elicitation-bridge.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ const MCP_TOOL_APPROVAL_SOURCE_KEY = "source";
4444
const MCP_TOOL_APPROVAL_CONNECTOR_SOURCE = "connector";
4545
const CODEX_APPS_SERVER_NAME = "codex_apps";
4646
const COMPUTER_USE_APPROVAL_TITLE = "Computer Use approval";
47+
const EMPTY_OBJECT_SCHEMA: JsonObject = { type: "object", properties: {} };
4748
const PLUGIN_APP_ID_META_KEYS = ["app_id", "appId", "codex_app_id", "codexAppId"];
4849
const PLUGIN_CONNECTOR_ID_META_KEYS = ["connector_id", "connectorId"];
4950
const PLUGIN_NAME_META_KEYS = ["plugin_name", "pluginName", "codex_plugin_name", "codexPluginName"];
@@ -363,13 +364,14 @@ function readComputerUseApprovalElicitation(
363364
!serverName ||
364365
!expectedServerName ||
365366
serverName !== expectedServerName ||
366-
readString(requestParams, "mode") !== "form" ||
367-
!isJsonObject(requestParams?.requestedSchema)
367+
readString(requestParams, "mode") !== "form"
368368
) {
369369
return undefined;
370370
}
371371

372-
const requestedSchema = requestParams.requestedSchema;
372+
const requestedSchema = isJsonObject(requestParams?.requestedSchema)
373+
? requestParams.requestedSchema
374+
: EMPTY_OBJECT_SCHEMA;
373375
if (
374376
readString(requestedSchema, "type") !== "object" ||
375377
!isJsonObject(requestedSchema.properties)

‎extensions/codex/src/app-server/run-attempt.test.ts‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3822,6 +3822,85 @@ describe("runCodexAppServerAttempt", () => {
38223822
expect(result.promptError).toBeNull();
38233823
});
38243824

3825+
it("keeps turn request activity active until elicitation handling resolves", async () => {
3826+
const harness = createStartedThreadHarness();
3827+
const bridgedResponse = {
3828+
action: "accept",
3829+
content: null,
3830+
_meta: null,
3831+
} as const;
3832+
let resolveBridge!: (value: typeof bridgedResponse) => void;
3833+
const bridgePromise = new Promise<typeof bridgedResponse>((resolve) => {
3834+
resolveBridge = resolve;
3835+
});
3836+
vi.spyOn(elicitationBridge, "handleCodexAppServerElicitationRequest").mockImplementation(
3837+
async () => await bridgePromise,
3838+
);
3839+
const params = createParams(
3840+
path.join(tempDir, "session.jsonl"),
3841+
path.join(tempDir, "workspace"),
3842+
);
3843+
params.timeoutMs = 500;
3844+
const onRunProgress = vi.fn();
3845+
params.onRunProgress = onRunProgress;
3846+
3847+
const run = runCodexAppServerAttempt(params, {
3848+
turnCompletionIdleTimeoutMs: 1_000,
3849+
turnAssistantCompletionIdleTimeoutMs: 1_000,
3850+
turnTerminalIdleTimeoutMs: 1_000,
3851+
});
3852+
await harness.waitForMethod("turn/start");
3853+
3854+
const response = harness.handleServerRequest({
3855+
id: "request-pending-elicitation",
3856+
method: "mcpServer/elicitation/request",
3857+
params: {
3858+
threadId: "thread-1",
3859+
turnId: "turn-1",
3860+
mode: "form",
3861+
message: "Approve?",
3862+
requestedSchema: { type: "object", properties: {} },
3863+
serverName: "server-1",
3864+
_meta: null,
3865+
},
3866+
});
3867+
await vi.waitFor(
3868+
() =>
3869+
expect(onRunProgress).toHaveBeenCalledWith(
3870+
expect.objectContaining({
3871+
reason: "request:mcpServer/elicitation/request:start",
3872+
}),
3873+
),
3874+
fastWait,
3875+
);
3876+
await new Promise((resolve) => setTimeout(resolve, 60));
3877+
expect(
3878+
onRunProgress.mock.calls.some(
3879+
([event]) =>
3880+
(event as { reason?: string }).reason ===
3881+
"request:mcpServer/elicitation/request:response",
3882+
),
3883+
).toBe(false);
3884+
3885+
resolveBridge(bridgedResponse);
3886+
await expect(response).resolves.toEqual(bridgedResponse);
3887+
await vi.waitFor(
3888+
() =>
3889+
expect(onRunProgress).toHaveBeenCalledWith(
3890+
expect.objectContaining({
3891+
reason: "request:mcpServer/elicitation/request:response",
3892+
}),
3893+
),
3894+
fastWait,
3895+
);
3896+
await harness.completeTurn({ threadId: "thread-1", turnId: "turn-1" });
3897+
3898+
const result = await run;
3899+
expect(result.aborted).toBe(false);
3900+
expect(result.timedOut).toBe(false);
3901+
expect(result.promptError).toBeNull();
3902+
});
3903+
38253904
it("counts pending user input requests as turn attempt progress", async () => {
38263905
const harness = createStartedThreadHarness();
38273906
const params = createParams(

‎extensions/codex/src/app-server/run-attempt.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2139,7 +2139,7 @@ export async function runCodexAppServerAttempt(
21392139
armCompletionWatchOnResponse = true;
21402140
markCurrentTurnRequestProgress();
21412141
}
2142-
return handleCodexAppServerElicitationRequest({
2142+
return await handleCodexAppServerElicitationRequest({
21432143
requestParams: request.params,
21442144
paramsForRun: params,
21452145
threadId: thread.threadId,

0 commit comments

Comments
 (0)