mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 07:20:43 +00:00
fix: accept Codex MCP approval elicitations (#68807)
This commit is contained in:
@@ -31,6 +31,7 @@ Docs: https://docs.openclaw.ai
|
||||
|
||||
### Fixes
|
||||
|
||||
- Codex harness: route Codex-tagged MCP tool approval elicitations through OpenClaw plugin approvals, including current empty-schema app-server requests, while leaving generic user-input prompts fail-closed. (#68807) Thanks @kesslerio.
|
||||
- Providers/OpenAI: harden Voice Call realtime transcription against OpenAI Realtime session-update drift, forward language and prompt hints, and add live coverage for realtime STT.
|
||||
- Providers/Moonshot: stop strict-sanitizing Kimi's native tool_call IDs (shaped like `functions.<name>:<index>`) on the OpenAI-compatible transport, so multi-turn agentic flows through Kimi K2.6 no longer break after 2-3 tool-calling rounds when the serving layer fails to match mangled IDs against the original tool definitions. Adds a `sanitizeToolCallIds` opt-out to the shared `openai-compatible` replay family helper and wires Moonshot to it. Fixes #62319. (#70030) Thanks @LeoDu0314.
|
||||
- Dependencies/security: override transitive `uuid` to `14.0.0`, clearing the runtime advisory across dependencies.
|
||||
|
||||
@@ -509,6 +509,11 @@ OpenClaw still builds the tool list and receives dynamic tool results from the
|
||||
harness. Text, images, video, music, TTS, approvals, and messaging-tool output
|
||||
continue through the normal OpenClaw delivery path.
|
||||
|
||||
Codex MCP tool approval elicitations are routed through OpenClaw's plugin
|
||||
approval flow when Codex marks `_meta.codex_approval_kind` as
|
||||
`"mcp_tool_call"`; other elicitation and free-form input requests still fail
|
||||
closed.
|
||||
|
||||
When the selected model uses the Codex harness, native thread compaction is
|
||||
delegated to Codex app-server. OpenClaw keeps a transcript mirror for channel
|
||||
history, search, `/new`, `/reset`, and future model or harness switching. The
|
||||
|
||||
@@ -53,6 +53,26 @@ function buildApprovalElicitation() {
|
||||
};
|
||||
}
|
||||
|
||||
function buildCurrentCodexApprovalElicitation() {
|
||||
return {
|
||||
...buildApprovalElicitation(),
|
||||
_meta: {
|
||||
codex_approval_kind: "mcp_tool_call",
|
||||
persist: ["session", "always"],
|
||||
connector_name: "GitHub",
|
||||
tool_title: "Create pull request",
|
||||
tool_description: "Creates a pull request in the selected repository.",
|
||||
tool_params_display: [
|
||||
{ name: "repo", display_name: "Repository", value: "openclaw/openclaw" },
|
||||
],
|
||||
},
|
||||
requestedSchema: {
|
||||
type: "object",
|
||||
properties: {},
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
describe("Codex app-server elicitation bridge", () => {
|
||||
beforeEach(() => {
|
||||
mockCallGatewayTool.mockReset();
|
||||
@@ -84,7 +104,75 @@ describe("Codex app-server elicitation bridge", () => {
|
||||
]);
|
||||
});
|
||||
|
||||
it("maps allow-always decisions onto session-scoped persistence when offered", async () => {
|
||||
it("accepts current Codex MCP approval elicitations with an empty form schema", async () => {
|
||||
mockCallGatewayTool
|
||||
.mockResolvedValueOnce({ id: "plugin:approval-current", status: "accepted" })
|
||||
.mockResolvedValueOnce({ id: "plugin:approval-current", decision: "allow-once" });
|
||||
|
||||
const result = await handleCodexAppServerElicitationRequest({
|
||||
requestParams: buildCurrentCodexApprovalElicitation(),
|
||||
paramsForRun: createParams(),
|
||||
threadId: "thread-1",
|
||||
turnId: "turn-1",
|
||||
});
|
||||
|
||||
expect(result).toEqual({
|
||||
action: "accept",
|
||||
content: null,
|
||||
_meta: null,
|
||||
});
|
||||
expect(mockCallGatewayTool).toHaveBeenCalledWith(
|
||||
"plugin.approval.request",
|
||||
expect.any(Object),
|
||||
expect.objectContaining({
|
||||
description: expect.stringContaining("App: GitHub"),
|
||||
}),
|
||||
{ expectFinal: false },
|
||||
);
|
||||
const approvalRequest = mockCallGatewayTool.mock.calls[0]?.[2] as {
|
||||
description: string;
|
||||
};
|
||||
expect(approvalRequest.description).toContain("Tool: Create pull request");
|
||||
expect(approvalRequest.description).toContain("Repository: openclaw/openclaw");
|
||||
});
|
||||
|
||||
it("accepts approval elicitations with a null turn id when the thread matches", async () => {
|
||||
mockCallGatewayTool
|
||||
.mockResolvedValueOnce({ id: "plugin:approval-null-turn", status: "accepted" })
|
||||
.mockResolvedValueOnce({ id: "plugin:approval-null-turn", decision: "allow-once" });
|
||||
|
||||
const result = await handleCodexAppServerElicitationRequest({
|
||||
requestParams: {
|
||||
...buildCurrentCodexApprovalElicitation(),
|
||||
turnId: null,
|
||||
},
|
||||
paramsForRun: createParams(),
|
||||
threadId: "thread-1",
|
||||
turnId: "turn-1",
|
||||
});
|
||||
|
||||
expect(result).toEqual({
|
||||
action: "accept",
|
||||
content: null,
|
||||
_meta: null,
|
||||
});
|
||||
});
|
||||
|
||||
it("ignores unscoped approval elicitations without the active thread id", async () => {
|
||||
const { turnId, serverName, mode, message, _meta, requestedSchema } =
|
||||
buildCurrentCodexApprovalElicitation();
|
||||
const result = await handleCodexAppServerElicitationRequest({
|
||||
requestParams: { turnId, serverName, mode, message, _meta, requestedSchema },
|
||||
paramsForRun: createParams(),
|
||||
threadId: "thread-1",
|
||||
turnId: "turn-1",
|
||||
});
|
||||
|
||||
expect(result).toBeUndefined();
|
||||
expect(mockCallGatewayTool).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("maps allow-always decisions onto persistent approval metadata when offered", async () => {
|
||||
mockCallGatewayTool
|
||||
.mockResolvedValueOnce({ id: "plugin:approval-2", status: "accepted" })
|
||||
.mockResolvedValueOnce({ id: "plugin:approval-2", decision: "allow-always" });
|
||||
@@ -100,9 +188,32 @@ describe("Codex app-server elicitation bridge", () => {
|
||||
action: "accept",
|
||||
content: {
|
||||
approve: true,
|
||||
persist: "session",
|
||||
persist: "always",
|
||||
},
|
||||
_meta: {
|
||||
persist: "always",
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it("maps allow-always decisions onto metadata for current empty-schema approvals", async () => {
|
||||
mockCallGatewayTool
|
||||
.mockResolvedValueOnce({ id: "plugin:approval-current-always", status: "accepted" })
|
||||
.mockResolvedValueOnce({ id: "plugin:approval-current-always", decision: "allow-always" });
|
||||
|
||||
const result = await handleCodexAppServerElicitationRequest({
|
||||
requestParams: buildCurrentCodexApprovalElicitation(),
|
||||
paramsForRun: createParams(),
|
||||
threadId: "thread-1",
|
||||
turnId: "turn-1",
|
||||
});
|
||||
|
||||
expect(result).toEqual({
|
||||
action: "accept",
|
||||
content: null,
|
||||
_meta: {
|
||||
persist: "always",
|
||||
},
|
||||
_meta: null,
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -20,6 +20,14 @@ type BridgeableApprovalElicitation = {
|
||||
meta: JsonObject;
|
||||
};
|
||||
|
||||
const MCP_TOOL_APPROVAL_KIND = "mcp_tool_call";
|
||||
const MCP_TOOL_APPROVAL_KIND_KEY = "codex_approval_kind";
|
||||
const MCP_TOOL_APPROVAL_CONNECTOR_NAME_KEY = "connector_name";
|
||||
const MCP_TOOL_APPROVAL_TOOL_TITLE_KEY = "tool_title";
|
||||
const MCP_TOOL_APPROVAL_TOOL_DESCRIPTION_KEY = "tool_description";
|
||||
const MCP_TOOL_APPROVAL_TOOL_PARAMS_DISPLAY_KEY = "tool_params_display";
|
||||
const MAX_DISPLAY_PARAM_VALUE_LENGTH = 120;
|
||||
|
||||
export async function handleCodexAppServerElicitationRequest(params: {
|
||||
requestParams: JsonValue | undefined;
|
||||
paramsForRun: EmbeddedRunAttemptParams;
|
||||
@@ -71,7 +79,7 @@ function readBridgeableApprovalElicitation(
|
||||
!requestParams ||
|
||||
readString(requestParams, "mode") !== "form" ||
|
||||
!isJsonObject(requestParams._meta) ||
|
||||
requestParams._meta.codex_approval_kind !== "mcp_tool_call" ||
|
||||
requestParams._meta[MCP_TOOL_APPROVAL_KIND_KEY] !== MCP_TOOL_APPROVAL_KIND ||
|
||||
!isJsonObject(requestParams.requestedSchema)
|
||||
) {
|
||||
return undefined;
|
||||
@@ -80,14 +88,54 @@ function readBridgeableApprovalElicitation(
|
||||
const requestedSchema = requestParams.requestedSchema;
|
||||
if (
|
||||
readString(requestedSchema, "type") !== "object" ||
|
||||
!isJsonObject(requestedSchema.properties) ||
|
||||
Object.keys(requestedSchema.properties).length === 0
|
||||
!isJsonObject(requestedSchema.properties)
|
||||
) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const title = readString(requestParams, "message") ?? "Codex MCP tool approval";
|
||||
const propertyLines = Object.entries(requestedSchema.properties)
|
||||
return {
|
||||
title,
|
||||
description: buildApprovalDescription({
|
||||
title,
|
||||
meta: requestParams._meta,
|
||||
requestedSchema,
|
||||
serverName: readString(requestParams, "serverName"),
|
||||
}),
|
||||
requestedSchema,
|
||||
meta: requestParams._meta,
|
||||
};
|
||||
}
|
||||
|
||||
function buildApprovalDescription(params: {
|
||||
title: string;
|
||||
meta: JsonObject;
|
||||
requestedSchema: JsonObject;
|
||||
serverName: string | undefined;
|
||||
}): string {
|
||||
const summaryLines = [
|
||||
readString(params.meta, MCP_TOOL_APPROVAL_CONNECTOR_NAME_KEY) &&
|
||||
`App: ${readString(params.meta, MCP_TOOL_APPROVAL_CONNECTOR_NAME_KEY)}`,
|
||||
readString(params.meta, MCP_TOOL_APPROVAL_TOOL_TITLE_KEY) &&
|
||||
`Tool: ${readString(params.meta, MCP_TOOL_APPROVAL_TOOL_TITLE_KEY)}`,
|
||||
params.serverName && `MCP server: ${params.serverName}`,
|
||||
readString(params.meta, MCP_TOOL_APPROVAL_TOOL_DESCRIPTION_KEY),
|
||||
].filter((line): line is string => Boolean(line));
|
||||
const paramLines = readDisplayParamLines(params.meta);
|
||||
const propertyLines = readPropertyDescriptionLines(params.requestedSchema);
|
||||
return [
|
||||
params.title,
|
||||
summaryLines.join("\n"),
|
||||
paramLines.length > 0 ? ["Parameters:", ...paramLines].join("\n") : "",
|
||||
propertyLines.length > 0 ? ["Fields:", ...propertyLines].join("\n") : "",
|
||||
]
|
||||
.filter(Boolean)
|
||||
.join("\n\n");
|
||||
}
|
||||
|
||||
function readPropertyDescriptionLines(requestedSchema: JsonObject): string[] {
|
||||
const properties = isJsonObject(requestedSchema.properties) ? requestedSchema.properties : {};
|
||||
return Object.entries(properties)
|
||||
.map(([name, value]) => {
|
||||
const schema = isJsonObject(value) ? value : undefined;
|
||||
if (!schema) {
|
||||
@@ -98,15 +146,33 @@ function readBridgeableApprovalElicitation(
|
||||
return description ? `- ${propTitle}: ${description}` : `- ${propTitle}`;
|
||||
})
|
||||
.filter((line): line is string => Boolean(line));
|
||||
}
|
||||
|
||||
return {
|
||||
title,
|
||||
description: [title, propertyLines.length > 0 ? ["Fields:", ...propertyLines].join("\n") : ""]
|
||||
.filter(Boolean)
|
||||
.join("\n\n"),
|
||||
requestedSchema,
|
||||
meta: requestParams._meta,
|
||||
};
|
||||
function readDisplayParamLines(meta: JsonObject): string[] {
|
||||
const displayParams = meta[MCP_TOOL_APPROVAL_TOOL_PARAMS_DISPLAY_KEY];
|
||||
if (!Array.isArray(displayParams)) {
|
||||
return [];
|
||||
}
|
||||
return displayParams
|
||||
.map((entry) => {
|
||||
const param = isJsonObject(entry) ? entry : undefined;
|
||||
if (!param) {
|
||||
return undefined;
|
||||
}
|
||||
const name = readString(param, "display_name") ?? readString(param, "name");
|
||||
if (!name) {
|
||||
return undefined;
|
||||
}
|
||||
return `- ${name}: ${formatDisplayParamValue(param.value)}`;
|
||||
})
|
||||
.filter((line): line is string => Boolean(line));
|
||||
}
|
||||
|
||||
function formatDisplayParamValue(value: JsonValue | undefined): string {
|
||||
const formatted = typeof value === "string" ? value : JSON.stringify(value ?? null);
|
||||
return formatted.length <= MAX_DISPLAY_PARAM_VALUE_LENGTH
|
||||
? formatted
|
||||
: `${formatted.slice(0, MAX_DISPLAY_PARAM_VALUE_LENGTH - 3)}...`;
|
||||
}
|
||||
|
||||
async function requestPluginApprovalOutcome(params: {
|
||||
@@ -152,14 +218,21 @@ function buildElicitationResponse(
|
||||
|
||||
const content = buildAcceptedContent(requestedSchema, meta, outcome);
|
||||
if (!content) {
|
||||
if (hasNoSchemaProperties(requestedSchema)) {
|
||||
return {
|
||||
action: "accept",
|
||||
content: null,
|
||||
_meta: buildAcceptedMeta(meta, outcome),
|
||||
};
|
||||
}
|
||||
embeddedAgentLog.warn("codex MCP approval elicitation approved without a mappable response", {
|
||||
approvalKind: meta.codex_approval_kind,
|
||||
approvalKind: meta[MCP_TOOL_APPROVAL_KIND_KEY],
|
||||
fields: Object.keys(requestedSchema.properties ?? {}),
|
||||
outcome,
|
||||
});
|
||||
return { action: "decline", content: null, _meta: null };
|
||||
}
|
||||
return { action: "accept", content, _meta: null };
|
||||
return { action: "accept", content, _meta: buildAcceptedMeta(meta, outcome) };
|
||||
}
|
||||
|
||||
function buildAcceptedContent(
|
||||
@@ -248,15 +321,14 @@ function readPersistFieldValue(
|
||||
if (options.length === 0) {
|
||||
return undefined;
|
||||
}
|
||||
for (const preferred of persistHints) {
|
||||
const preferred = choosePersistHint(persistHints);
|
||||
if (preferred) {
|
||||
const match = options.find(
|
||||
(option) => option.value === preferred || option.label === preferred,
|
||||
);
|
||||
if (match) {
|
||||
return match.value;
|
||||
}
|
||||
return match?.value;
|
||||
}
|
||||
return options.find((option) => option.value === "session" || option.label === "session")?.value;
|
||||
return undefined;
|
||||
}
|
||||
|
||||
function readDefaultValue(schema: JsonObject): JsonValue | undefined {
|
||||
@@ -304,6 +376,29 @@ function readPersistHints(meta: JsonObject): string[] {
|
||||
return ["session", "always"];
|
||||
}
|
||||
|
||||
function buildAcceptedMeta(meta: JsonObject, outcome: AppServerApprovalOutcome): JsonObject | null {
|
||||
if (outcome !== "approved-session") {
|
||||
return null;
|
||||
}
|
||||
const persist = choosePersistHint(readPersistHints(meta));
|
||||
return persist ? { persist } : null;
|
||||
}
|
||||
|
||||
function choosePersistHint(persistHints: string[]): "always" | "session" | undefined {
|
||||
if (persistHints.includes("always")) {
|
||||
return "always";
|
||||
}
|
||||
if (persistHints.includes("session")) {
|
||||
return "session";
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
function hasNoSchemaProperties(requestedSchema: JsonObject): boolean {
|
||||
const properties = isJsonObject(requestedSchema.properties) ? requestedSchema.properties : {};
|
||||
return Object.keys(properties).length === 0;
|
||||
}
|
||||
|
||||
function readEnumOptions(schema: JsonObject): Array<{ value: string; label: string }> {
|
||||
if (Array.isArray(schema.enum)) {
|
||||
const values = schema.enum.filter((entry): entry is string => typeof entry === "string");
|
||||
|
||||
@@ -533,6 +533,87 @@ describe("runCodexAppServerAttempt", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("routes MCP approval 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>)
|
||||
| undefined;
|
||||
const bridgeSpy = vi
|
||||
.spyOn(elicitationBridge, "handleCodexAppServerElicitationRequest")
|
||||
.mockResolvedValue({
|
||||
action: "accept",
|
||||
content: null,
|
||||
_meta: null,
|
||||
});
|
||||
const request = vi.fn(async (method: string) => {
|
||||
if (method === "thread/start") {
|
||||
return threadStartResult();
|
||||
}
|
||||
if (method === "turn/start") {
|
||||
return turnStartResult();
|
||||
}
|
||||
return {};
|
||||
});
|
||||
__testing.setCodexAppServerClientFactoryForTests(
|
||||
async () =>
|
||||
({
|
||||
request,
|
||||
addNotificationHandler: (handler: typeof notify) => {
|
||||
notify = handler;
|
||||
return () => undefined;
|
||||
},
|
||||
addRequestHandler: (
|
||||
handler: (request: {
|
||||
id: string;
|
||||
method: string;
|
||||
params?: unknown;
|
||||
}) => Promise<unknown>,
|
||||
) => {
|
||||
handleRequest = handler;
|
||||
return () => undefined;
|
||||
},
|
||||
}) as never,
|
||||
);
|
||||
|
||||
const run = runCodexAppServerAttempt(
|
||||
createParams(path.join(tempDir, "session.jsonl"), path.join(tempDir, "workspace")),
|
||||
);
|
||||
await vi.waitFor(() => expect(handleRequest).toBeTypeOf("function"), { interval: 1 });
|
||||
|
||||
const result = await handleRequest?.({
|
||||
id: "request-elicitation-1",
|
||||
method: "mcpServer/elicitation/request",
|
||||
params: {
|
||||
threadId: "thread-1",
|
||||
turnId: "turn-1",
|
||||
serverName: "codex_apps__github",
|
||||
mode: "form",
|
||||
},
|
||||
});
|
||||
|
||||
expect(result).toEqual({
|
||||
action: "accept",
|
||||
content: null,
|
||||
_meta: null,
|
||||
});
|
||||
expect(bridgeSpy).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
threadId: "thread-1",
|
||||
turnId: "turn-1",
|
||||
}),
|
||||
);
|
||||
|
||||
await notify({
|
||||
method: "turn/completed",
|
||||
params: {
|
||||
threadId: "thread-1",
|
||||
turnId: "turn-1",
|
||||
turn: { id: "turn-1", status: "completed" },
|
||||
},
|
||||
});
|
||||
await run;
|
||||
});
|
||||
|
||||
it("does not leak unhandled rejections when shutdown closes before interrupt", async () => {
|
||||
const unhandledRejections: unknown[] = [];
|
||||
const onUnhandledRejection = (reason: unknown) => {
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
export const EXPECTED_CODEX_MODELS_COMMAND_TEXT = [
|
||||
"Codex models:",
|
||||
"Available Codex models",
|
||||
"Available models, local cache:",
|
||||
"Available agent target:",
|
||||
"Available agent targets:",
|
||||
"opened an interactive trust prompt",
|
||||
|
||||
Reference in New Issue
Block a user