mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-24 11:19:51 +00:00
fix(codex): bridge computer use elicitations
This commit is contained in:
@@ -89,6 +89,24 @@ function buildCurrentCodexApprovalElicitation() {
|
||||
};
|
||||
}
|
||||
|
||||
function buildComputerUseApprovalElicitation(overrides: Record<string, unknown> = {}) {
|
||||
return {
|
||||
threadId: "thread-1",
|
||||
turnId: "turn-1",
|
||||
serverName: "computer-use",
|
||||
mode: "form",
|
||||
message: "Allow Codex to use Notes?",
|
||||
_meta: {
|
||||
persist: ["always"],
|
||||
},
|
||||
requestedSchema: {
|
||||
type: "object",
|
||||
properties: {},
|
||||
},
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function buildPluginApprovalElicitation(overrides: Record<string, unknown> = {}) {
|
||||
return {
|
||||
threadId: "thread-1",
|
||||
@@ -290,6 +308,201 @@ describe("Codex app-server elicitation bridge", () => {
|
||||
expect(approvalRequest.description).toContain("Repository: openclaw/openclaw");
|
||||
});
|
||||
|
||||
it("routes Computer Use app approvals through plugin approvals", async () => {
|
||||
mockCallGatewayTool
|
||||
.mockResolvedValueOnce({ id: "plugin:approval-computer-use", status: "accepted" })
|
||||
.mockResolvedValueOnce({ id: "plugin:approval-computer-use", decision: "allow-once" });
|
||||
|
||||
const result = await handleCodexAppServerElicitationRequest({
|
||||
requestParams: buildComputerUseApprovalElicitation(),
|
||||
paramsForRun: createParams(),
|
||||
threadId: "thread-1",
|
||||
turnId: "turn-1",
|
||||
pluginAppPolicyContext: createPluginAppPolicyContext({ apps: [] }),
|
||||
computerUseMcpServerName: "computer-use",
|
||||
});
|
||||
|
||||
expect(result).toEqual({
|
||||
action: "accept",
|
||||
content: null,
|
||||
_meta: null,
|
||||
});
|
||||
expect(mockCallGatewayTool.mock.calls.map(([method]) => method)).toEqual([
|
||||
"plugin.approval.request",
|
||||
"plugin.approval.waitDecision",
|
||||
]);
|
||||
});
|
||||
|
||||
it("maps Computer Use allow-always decisions onto persistent metadata", async () => {
|
||||
mockCallGatewayTool
|
||||
.mockResolvedValueOnce({ id: "plugin:approval-computer-use-always", status: "accepted" })
|
||||
.mockResolvedValueOnce({
|
||||
id: "plugin:approval-computer-use-always",
|
||||
decision: "allow-always",
|
||||
});
|
||||
|
||||
const result = await handleCodexAppServerElicitationRequest({
|
||||
requestParams: buildComputerUseApprovalElicitation(),
|
||||
paramsForRun: createParams(),
|
||||
threadId: "thread-1",
|
||||
turnId: "turn-1",
|
||||
pluginAppPolicyContext: createPluginAppPolicyContext({ apps: [] }),
|
||||
computerUseMcpServerName: "computer-use",
|
||||
});
|
||||
|
||||
expect(result).toEqual({
|
||||
action: "accept",
|
||||
content: null,
|
||||
_meta: {
|
||||
persist: "always",
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it("does not handle non-Computer Use elicitations without approval metadata", async () => {
|
||||
const result = await handleCodexAppServerElicitationRequest({
|
||||
requestParams: buildComputerUseApprovalElicitation({ serverName: "desktop-control" }),
|
||||
paramsForRun: createParams(),
|
||||
threadId: "thread-1",
|
||||
turnId: "turn-1",
|
||||
pluginAppPolicyContext: createPluginAppPolicyContext({ apps: [] }),
|
||||
computerUseMcpServerName: "computer-use",
|
||||
});
|
||||
|
||||
expect(result).toBeUndefined();
|
||||
expect(mockCallGatewayTool).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("routes configured custom Computer Use server names through plugin approvals", async () => {
|
||||
mockCallGatewayTool
|
||||
.mockResolvedValueOnce({ id: "plugin:approval-custom-computer-use", status: "accepted" })
|
||||
.mockResolvedValueOnce({
|
||||
id: "plugin:approval-custom-computer-use",
|
||||
decision: "allow-once",
|
||||
});
|
||||
|
||||
const result = await handleCodexAppServerElicitationRequest({
|
||||
requestParams: buildComputerUseApprovalElicitation({ serverName: "desktop-control" }),
|
||||
paramsForRun: createParams(),
|
||||
threadId: "thread-1",
|
||||
turnId: "turn-1",
|
||||
pluginAppPolicyContext: createPluginAppPolicyContext({ apps: [] }),
|
||||
computerUseMcpServerName: "desktop-control",
|
||||
});
|
||||
|
||||
expect(result).toEqual({
|
||||
action: "accept",
|
||||
content: null,
|
||||
_meta: null,
|
||||
});
|
||||
const approvalRequest = gatewayToolArg(0, 2) as { description: string };
|
||||
expect(approvalRequest.description).toContain("MCP server: desktop-control");
|
||||
});
|
||||
|
||||
it("declines approved Computer Use app approvals with unmappable non-empty schemas", async () => {
|
||||
const warnSpy = vi.spyOn(embeddedAgentLog, "warn").mockImplementation(() => undefined);
|
||||
mockCallGatewayTool
|
||||
.mockResolvedValueOnce({ id: "plugin:approval-computer-use-fields", status: "accepted" })
|
||||
.mockResolvedValueOnce({ id: "plugin:approval-computer-use-fields", decision: "allow-once" });
|
||||
|
||||
const result = await handleCodexAppServerElicitationRequest({
|
||||
requestParams: buildComputerUseApprovalElicitation({
|
||||
requestedSchema: {
|
||||
type: "object",
|
||||
properties: {
|
||||
appName: {
|
||||
type: "string",
|
||||
title: "App name",
|
||||
},
|
||||
},
|
||||
required: ["appName"],
|
||||
},
|
||||
}),
|
||||
paramsForRun: createParams(),
|
||||
threadId: "thread-1",
|
||||
turnId: "turn-1",
|
||||
pluginAppPolicyContext: createPluginAppPolicyContext({ apps: [] }),
|
||||
computerUseMcpServerName: "computer-use",
|
||||
});
|
||||
|
||||
expect(result).toEqual({ action: "decline", content: null, _meta: null });
|
||||
expect(mockCallGatewayTool.mock.calls.map(([method]) => method)).toEqual([
|
||||
"plugin.approval.request",
|
||||
"plugin.approval.waitDecision",
|
||||
]);
|
||||
expect(warnSpy).toHaveBeenCalledWith(
|
||||
"codex MCP approval elicitation approved without a mappable response",
|
||||
expect.objectContaining({
|
||||
fields: ["appName"],
|
||||
outcome: "approved-once",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("does not bridge Computer Use elicitations without an approval form schema", async () => {
|
||||
const result = await handleCodexAppServerElicitationRequest({
|
||||
requestParams: buildComputerUseApprovalElicitation({
|
||||
requestedSchema: "not-a-schema",
|
||||
}),
|
||||
paramsForRun: createParams(),
|
||||
threadId: "thread-1",
|
||||
turnId: "turn-1",
|
||||
pluginAppPolicyContext: createPluginAppPolicyContext({ apps: [] }),
|
||||
computerUseMcpServerName: "computer-use",
|
||||
});
|
||||
|
||||
expect(result).toBeUndefined();
|
||||
expect(mockCallGatewayTool).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not bridge Computer Use elicitations outside form mode", async () => {
|
||||
const result = await handleCodexAppServerElicitationRequest({
|
||||
requestParams: buildComputerUseApprovalElicitation({
|
||||
mode: "notification",
|
||||
}),
|
||||
paramsForRun: createParams(),
|
||||
threadId: "thread-1",
|
||||
turnId: "turn-1",
|
||||
pluginAppPolicyContext: createPluginAppPolicyContext({ apps: [] }),
|
||||
computerUseMcpServerName: "computer-use",
|
||||
});
|
||||
|
||||
expect(result).toBeUndefined();
|
||||
expect(mockCallGatewayTool).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("falls back to a Computer Use approval title and sanitizes server names", async () => {
|
||||
mockCallGatewayTool
|
||||
.mockResolvedValueOnce({ id: "plugin:approval-computer-use-title", status: "accepted" })
|
||||
.mockResolvedValueOnce({ id: "plugin:approval-computer-use-title", decision: "allow-once" });
|
||||
|
||||
const result = await handleCodexAppServerElicitationRequest({
|
||||
requestParams: buildComputerUseApprovalElicitation({
|
||||
message: "\u001b[31m",
|
||||
serverName: "computer-use\u009b31m",
|
||||
_meta: null,
|
||||
}),
|
||||
paramsForRun: createParams(),
|
||||
threadId: "thread-1",
|
||||
turnId: "turn-1",
|
||||
pluginAppPolicyContext: createPluginAppPolicyContext({ apps: [] }),
|
||||
computerUseMcpServerName: "computer-use\u009b31m",
|
||||
});
|
||||
|
||||
expect(result).toEqual({
|
||||
action: "accept",
|
||||
content: null,
|
||||
_meta: null,
|
||||
});
|
||||
const approvalRequest = gatewayToolArg(0, 2) as {
|
||||
title: string;
|
||||
description: string;
|
||||
};
|
||||
expect(approvalRequest.title).toBe("Computer Use approval");
|
||||
expect(approvalRequest.description).toContain("MCP server: computer-use");
|
||||
expect(approvalRequest.description).not.toContain("\u009b");
|
||||
});
|
||||
|
||||
it("strips control and invisible formatting from approval display text", async () => {
|
||||
mockCallGatewayTool
|
||||
.mockResolvedValueOnce({ id: "plugin:approval-sanitized", status: "accepted" })
|
||||
|
||||
@@ -43,6 +43,7 @@ const MCP_TOOL_APPROVAL_TOOL_PARAMS_DISPLAY_KEY = "tool_params_display";
|
||||
const MCP_TOOL_APPROVAL_SOURCE_KEY = "source";
|
||||
const MCP_TOOL_APPROVAL_CONNECTOR_SOURCE = "connector";
|
||||
const CODEX_APPS_SERVER_NAME = "codex_apps";
|
||||
const COMPUTER_USE_APPROVAL_TITLE = "Computer Use approval";
|
||||
const PLUGIN_APP_ID_META_KEYS = ["app_id", "appId", "codex_app_id", "codexAppId"];
|
||||
const PLUGIN_CONNECTOR_ID_META_KEYS = ["connector_id", "connectorId"];
|
||||
const PLUGIN_NAME_META_KEYS = ["plugin_name", "pluginName", "codex_plugin_name", "codexPluginName"];
|
||||
@@ -82,6 +83,7 @@ export async function handleCodexAppServerElicitationRequest(params: {
|
||||
threadId: string;
|
||||
turnId: string;
|
||||
pluginAppPolicyContext?: PluginAppPolicyContext;
|
||||
computerUseMcpServerName?: string;
|
||||
signal?: AbortSignal;
|
||||
}): Promise<JsonValue | undefined> {
|
||||
const requestParams = isJsonObject(params.requestParams) ? params.requestParams : undefined;
|
||||
@@ -110,7 +112,9 @@ export async function handleCodexAppServerElicitationRequest(params: {
|
||||
return buildPluginPolicyElicitationResponse(pluginResolution.entry, requestParams);
|
||||
}
|
||||
|
||||
const approvalPrompt = readBridgeableApprovalElicitation(requestParams);
|
||||
const approvalPrompt =
|
||||
readComputerUseApprovalElicitation(requestParams, params.computerUseMcpServerName) ??
|
||||
readBridgeableApprovalElicitation(requestParams);
|
||||
if (!approvalPrompt) {
|
||||
return undefined;
|
||||
}
|
||||
@@ -350,6 +354,45 @@ function readBridgeableApprovalElicitation(
|
||||
};
|
||||
}
|
||||
|
||||
function readComputerUseApprovalElicitation(
|
||||
requestParams: JsonObject | undefined,
|
||||
expectedServerName: string | undefined,
|
||||
): BridgeableApprovalElicitation | undefined {
|
||||
const serverName = readString(requestParams, "serverName");
|
||||
if (
|
||||
!serverName ||
|
||||
!expectedServerName ||
|
||||
serverName !== expectedServerName ||
|
||||
readString(requestParams, "mode") !== "form" ||
|
||||
!isJsonObject(requestParams?.requestedSchema)
|
||||
) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const requestedSchema = requestParams.requestedSchema;
|
||||
if (
|
||||
readString(requestedSchema, "type") !== "object" ||
|
||||
!isJsonObject(requestedSchema.properties)
|
||||
) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const meta = isJsonObject(requestParams?.["_meta"]) ? requestParams["_meta"] : {};
|
||||
const title =
|
||||
sanitizeDisplayText(readString(requestParams, "message") ?? "") || COMPUTER_USE_APPROVAL_TITLE;
|
||||
return {
|
||||
title,
|
||||
description: buildApprovalDescription({
|
||||
title,
|
||||
meta,
|
||||
requestedSchema,
|
||||
serverName: sanitizeOptionalDisplayText(serverName),
|
||||
}),
|
||||
requestedSchema,
|
||||
meta,
|
||||
};
|
||||
}
|
||||
|
||||
function buildApprovalDescription(params: {
|
||||
title: string;
|
||||
meta: JsonObject;
|
||||
|
||||
@@ -6861,7 +6861,7 @@ describe("runCodexAppServerAttempt", () => {
|
||||
expect(result.timedOut).toBe(false);
|
||||
});
|
||||
|
||||
it("routes MCP approval elicitations through the native bridge", async () => {
|
||||
it("routes Computer Use MCP elicitations through the native bridge", async () => {
|
||||
let notify: (notification: CodexServerNotification) => Promise<void> = async () => undefined;
|
||||
let handleRequest:
|
||||
| ((request: { id: string; method: string; params?: unknown }) => Promise<unknown>)
|
||||
@@ -6874,6 +6874,65 @@ describe("runCodexAppServerAttempt", () => {
|
||||
_meta: null,
|
||||
});
|
||||
const request = vi.fn(async (method: string) => {
|
||||
if (method === "plugin/list") {
|
||||
return {
|
||||
marketplaces: [
|
||||
{
|
||||
name: "openai-bundled",
|
||||
path: "/marketplaces/openai-bundled",
|
||||
plugins: [
|
||||
{
|
||||
id: "computer-use@openai-bundled",
|
||||
name: "computer-use",
|
||||
source: {
|
||||
type: "local",
|
||||
path: "/marketplaces/openai-bundled/plugins/computer-use",
|
||||
},
|
||||
installed: true,
|
||||
enabled: true,
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
marketplaceLoadErrors: [],
|
||||
featuredPluginIds: [],
|
||||
};
|
||||
}
|
||||
if (method === "plugin/read") {
|
||||
return {
|
||||
plugin: {
|
||||
marketplaceName: "openai-bundled",
|
||||
marketplacePath: "/marketplaces/openai-bundled",
|
||||
summary: {
|
||||
id: "computer-use@openai-bundled",
|
||||
name: "computer-use",
|
||||
source: {
|
||||
type: "local",
|
||||
path: "/marketplaces/openai-bundled/plugins/computer-use",
|
||||
},
|
||||
installed: true,
|
||||
enabled: true,
|
||||
},
|
||||
description: null,
|
||||
skills: [],
|
||||
apps: [],
|
||||
mcpServers: ["computer-use"],
|
||||
},
|
||||
};
|
||||
}
|
||||
if (method === "mcpServerStatus/list") {
|
||||
return {
|
||||
data: [
|
||||
{
|
||||
name: "desktop-control",
|
||||
tools: {
|
||||
"computer-use.get_app_state": {},
|
||||
},
|
||||
},
|
||||
],
|
||||
nextCursor: null,
|
||||
};
|
||||
}
|
||||
if (method === "thread/start") {
|
||||
return threadStartResult("thread-1");
|
||||
}
|
||||
@@ -6905,6 +6964,15 @@ describe("runCodexAppServerAttempt", () => {
|
||||
|
||||
const run = runCodexAppServerAttempt(
|
||||
createParams(path.join(tempDir, "session.jsonl"), path.join(tempDir, "workspace")),
|
||||
{
|
||||
pluginConfig: {
|
||||
computerUse: {
|
||||
enabled: true,
|
||||
marketplaceName: "openai-bundled",
|
||||
mcpServerName: "desktop-control",
|
||||
},
|
||||
},
|
||||
},
|
||||
);
|
||||
await vi.waitFor(() => expect(handleRequest).toBeTypeOf("function"));
|
||||
|
||||
@@ -6914,7 +6982,7 @@ describe("runCodexAppServerAttempt", () => {
|
||||
params: {
|
||||
threadId: "thread-1",
|
||||
turnId: "turn-1",
|
||||
serverName: "codex_apps__github",
|
||||
serverName: "desktop-control",
|
||||
mode: "form",
|
||||
},
|
||||
});
|
||||
@@ -6925,10 +6993,23 @@ describe("runCodexAppServerAttempt", () => {
|
||||
_meta: null,
|
||||
});
|
||||
const [bridgeCall] = mockCall(bridgeSpy, "elicitation bridge") as [
|
||||
{ threadId?: string; turnId?: string },
|
||||
{
|
||||
requestParams?: { serverName?: string };
|
||||
computerUseMcpServerName?: string;
|
||||
threadId?: string;
|
||||
turnId?: string;
|
||||
},
|
||||
];
|
||||
expect(bridgeCall.threadId).toBe("thread-1");
|
||||
expect(bridgeCall.turnId).toBe("turn-1");
|
||||
expect(bridgeCall.requestParams?.serverName).toBe("desktop-control");
|
||||
expect(bridgeCall.computerUseMcpServerName).toBe("desktop-control");
|
||||
const requestCalls = request.mock.calls as unknown as Array<[string, unknown, unknown?]>;
|
||||
const threadStart = requestCalls.find(([method]) => method === "thread/start");
|
||||
const threadStartParams = threadStart?.[1] as
|
||||
| { approvalPolicy?: { granular?: { mcp_elicitations?: boolean } } }
|
||||
| undefined;
|
||||
expect(threadStartParams?.approvalPolicy?.granular?.mcp_elicitations).toBe(true);
|
||||
|
||||
await notify({
|
||||
method: "turn/completed",
|
||||
|
||||
@@ -80,6 +80,7 @@ import { ensureCodexComputerUse } from "./computer-use.js";
|
||||
import {
|
||||
isCodexAppServerApprovalPolicyAllowedByRequirements,
|
||||
readCodexPluginConfig,
|
||||
resolveCodexComputerUseConfig,
|
||||
resolveCodexPluginsPolicy,
|
||||
resolveCodexAppServerRuntimeOptions,
|
||||
withMcpElicitationsApprovalPolicy,
|
||||
@@ -801,6 +802,7 @@ export async function runCodexAppServerAttempt(
|
||||
const attemptStartedAt = Date.now();
|
||||
const attemptClientFactory = options.clientFactory ?? defaultCodexAppServerClientFactory;
|
||||
const pluginConfig = readCodexPluginConfig(options.pluginConfig);
|
||||
const computerUseConfig = resolveCodexComputerUseConfig({ pluginConfig });
|
||||
const configuredAppServer = resolveCodexAppServerRuntimeOptions({ pluginConfig });
|
||||
const resolvedWorkspace = resolveUserPath(params.workspaceDir);
|
||||
await fs.mkdir(resolvedWorkspace, { recursive: true });
|
||||
@@ -1228,6 +1230,9 @@ export async function runCodexAppServerAttempt(
|
||||
const resolvedPluginPolicy = pluginThreadConfigRequired
|
||||
? resolveCodexPluginsPolicy(pluginThreadConfigPluginConfig)
|
||||
: undefined;
|
||||
const computerUseMcpElicitationDelegationRequired = computerUseConfig.enabled;
|
||||
const mcpElicitationDelegationRequired =
|
||||
resolvedPluginPolicy?.enabled === true || computerUseMcpElicitationDelegationRequired;
|
||||
const enabledPluginConfigKeys = resolvedPluginPolicy
|
||||
? resolvedPluginPolicy.pluginPolicies
|
||||
.filter((plugin) => plugin.enabled)
|
||||
@@ -1247,13 +1252,12 @@ export async function runCodexAppServerAttempt(
|
||||
appServer,
|
||||
}),
|
||||
);
|
||||
pluginAppServer =
|
||||
resolvedPluginPolicy?.enabled === true
|
||||
? {
|
||||
...appServer,
|
||||
approvalPolicy: withMcpElicitationsApprovalPolicy(appServer.approvalPolicy),
|
||||
}
|
||||
: appServer;
|
||||
pluginAppServer = mcpElicitationDelegationRequired
|
||||
? {
|
||||
...appServer,
|
||||
approvalPolicy: withMcpElicitationsApprovalPolicy(appServer.approvalPolicy),
|
||||
}
|
||||
: appServer;
|
||||
({ client, thread } = await withCodexStartupTimeout({
|
||||
timeoutMs: startupTimeoutMs,
|
||||
signal: runAbortController.signal,
|
||||
@@ -1270,7 +1274,7 @@ export async function runCodexAppServerAttempt(
|
||||
startupClientForCleanup = startupClient;
|
||||
await ensureCodexComputerUse({
|
||||
client: startupClient,
|
||||
pluginConfig: options.pluginConfig,
|
||||
pluginConfig,
|
||||
timeoutMs: appServer.requestTimeoutMs,
|
||||
signal: runAbortController.signal,
|
||||
});
|
||||
@@ -2041,6 +2045,9 @@ export async function runCodexAppServerAttempt(
|
||||
threadId: thread.threadId,
|
||||
turnId,
|
||||
pluginAppPolicyContext: thread.pluginAppPolicyContext,
|
||||
...(computerUseConfig.enabled
|
||||
? { computerUseMcpServerName: computerUseConfig.mcpServerName }
|
||||
: {}),
|
||||
signal: runAbortController.signal,
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user