fix(codex): harden app-server approvals

This commit is contained in:
Peter Steinberger
2026-04-23 02:14:37 +01:00
parent de95e414d1
commit 1cbd5a9470
14 changed files with 985 additions and 118 deletions

View File

@@ -234,6 +234,25 @@ describe("Codex app-server approval bridge", () => {
expect(description).toContain("High-risk targets:");
});
it("ignores approval requests that are missing explicit thread or turn ids", async () => {
const params = createParams();
const result = await handleCodexAppServerApprovalRequest({
method: "item/commandExecution/requestApproval",
requestParams: {
itemId: "cmd-2",
command: "pnpm test",
},
paramsForRun: params,
threadId: "thread-1",
turnId: "turn-1",
});
expect(result).toBeUndefined();
expect(mockCallGatewayTool).not.toHaveBeenCalled();
expect(params.onAgentEvent).not.toHaveBeenCalled();
});
it("maps app-server approval response families separately", () => {
expect(
buildApprovalResponse(

View File

@@ -1,34 +1,18 @@
import {
callGatewayTool,
type AgentApprovalEventData,
type EmbeddedRunAttemptParams,
type ExecApprovalDecision,
} from "openclaw/plugin-sdk/agent-harness";
import {
mapExecDecisionToOutcome,
requestPluginApproval,
type AppServerApprovalOutcome,
waitForPluginApprovalDecision,
} from "./plugin-approval-roundtrip.js";
import { isJsonObject, type JsonObject, type JsonValue } from "./protocol.js";
const DEFAULT_CODEX_APPROVAL_TIMEOUT_MS = 120_000;
const PERMISSION_DESCRIPTION_MAX_LENGTH = 700;
const PERMISSION_SAMPLE_LIMIT = 2;
const PERMISSION_VALUE_MAX_LENGTH = 48;
export type AppServerApprovalOutcome =
| "approved-once"
| "approved-session"
| "denied"
| "unavailable"
| "cancelled";
type ApprovalRequestResult = {
id?: string;
status?: string;
decision?: ExecApprovalDecision | null;
};
type ApprovalWaitResult = {
id?: string;
decision?: ExecApprovalDecision | null;
};
export async function handleCodexAppServerApprovalRequest(params: {
method: string;
requestParams: JsonValue | undefined;
@@ -52,29 +36,14 @@ export async function handleCodexAppServerApprovalRequest(params: {
});
try {
const timeoutMs = DEFAULT_CODEX_APPROVAL_TIMEOUT_MS;
const requestResult: ApprovalRequestResult | undefined = await callGatewayTool(
"plugin.approval.request",
{ timeoutMs: timeoutMs + 10_000 },
{
pluginId: "openclaw-codex-app-server",
title: context.title,
description: context.description,
severity: context.severity,
toolName: context.toolName,
toolCallId: context.itemId,
agentId: params.paramsForRun.agentId,
sessionKey: params.paramsForRun.sessionKey,
turnSourceChannel:
params.paramsForRun.messageChannel ?? params.paramsForRun.messageProvider,
turnSourceTo: params.paramsForRun.currentChannelId,
turnSourceAccountId: params.paramsForRun.agentAccountId,
turnSourceThreadId: params.paramsForRun.currentThreadTs,
timeoutMs,
twoPhase: true,
},
{ expectFinal: false },
);
const requestResult = await requestPluginApproval({
paramsForRun: params.paramsForRun,
title: context.title,
description: context.description,
severity: context.severity,
toolName: context.toolName,
toolCallId: context.itemId,
});
const approvalId = requestResult?.id;
if (!approvalId) {
@@ -84,6 +53,7 @@ export async function handleCodexAppServerApprovalRequest(params: {
status: "unavailable",
title: context.title,
...context.eventDetails,
...approvalEventScope(params.method, "denied"),
message: "Codex app-server approval route unavailable.",
});
return buildApprovalResponse(params.method, context.requestParams, "denied");
@@ -102,11 +72,7 @@ export async function handleCodexAppServerApprovalRequest(params: {
const decision = Object.prototype.hasOwnProperty.call(requestResult, "decision")
? requestResult.decision
: await waitForApprovalDecision({
approvalId,
timeoutMs,
signal: params.signal,
});
: await waitForPluginApprovalDecision({ approvalId, signal: params.signal });
const outcome = mapExecDecisionToOutcome(decision);
emitApprovalEvent(params.paramsForRun, {
@@ -124,6 +90,7 @@ export async function handleCodexAppServerApprovalRequest(params: {
approvalId,
approvalSlug: approvalId,
...context.eventDetails,
...approvalEventScope(params.method, outcome),
message: approvalResolutionMessage(outcome),
});
return buildApprovalResponse(params.method, context.requestParams, outcome);
@@ -135,6 +102,7 @@ export async function handleCodexAppServerApprovalRequest(params: {
status: cancelled ? "failed" : "unavailable",
title: context.title,
...context.eventDetails,
...approvalEventScope(params.method, cancelled ? "cancelled" : "denied"),
message: cancelled
? "Codex app-server approval cancelled because the run stopped."
: `Codex app-server approval route failed: ${formatErrorMessage(error)}`,
@@ -176,18 +144,12 @@ function matchesCurrentTurn(
turnId: string,
): boolean {
if (!requestParams) {
return true;
return false;
}
const requestThreadId =
readString(requestParams, "threadId") ?? readString(requestParams, "conversationId");
const requestTurnId = readString(requestParams, "turnId");
if (requestThreadId && requestThreadId !== threadId) {
return false;
}
if (requestTurnId && requestTurnId !== turnId) {
return false;
}
return true;
return requestThreadId === threadId && requestTurnId === turnId;
}
function buildApprovalContext(params: {
@@ -248,37 +210,6 @@ function buildApprovalContext(params: {
};
}
async function waitForApprovalDecision(params: {
approvalId: string;
timeoutMs: number;
signal?: AbortSignal;
}): Promise<ExecApprovalDecision | null | undefined> {
const waitPromise: Promise<ApprovalWaitResult | undefined> = callGatewayTool(
"plugin.approval.waitDecision",
{ timeoutMs: params.timeoutMs + 10_000 },
{ id: params.approvalId },
);
if (!params.signal) {
return (await waitPromise)?.decision;
}
let onAbort: (() => void) | undefined;
const abortPromise = new Promise<never>((_, reject) => {
if (params.signal!.aborted) {
reject(params.signal!.reason);
return;
}
onAbort = () => reject(params.signal!.reason);
params.signal!.addEventListener("abort", onAbort, { once: true });
});
try {
return (await Promise.race([waitPromise, abortPromise]))?.decision;
} finally {
if (onAbort) {
params.signal.removeEventListener("abort", onAbort);
}
}
}
function commandApprovalDecision(
requestParams: JsonObject | undefined,
outcome: AppServerApprovalOutcome,
@@ -528,27 +459,12 @@ function hasAvailableDecision(requestParams: JsonObject | undefined, decision: s
return available.includes(decision);
}
function mapExecDecisionToOutcome(
decision: ExecApprovalDecision | null | undefined,
): AppServerApprovalOutcome {
if (decision === "allow-once") {
return "approved-once";
}
if (decision === "allow-always") {
return "approved-session";
}
if (decision === null || decision === undefined) {
return "unavailable";
}
return "denied";
}
function approvalResolutionMessage(outcome: AppServerApprovalOutcome): string {
if (outcome === "approved-session") {
return "Codex app-server approval granted for the session.";
}
if (outcome === "approved-once") {
return "Codex app-server approval granted once.";
return "Codex app-server approval granted for this turn.";
}
if (outcome === "cancelled") {
return "Codex app-server approval cancelled.";
@@ -559,6 +475,19 @@ function approvalResolutionMessage(outcome: AppServerApprovalOutcome): string {
return "Codex app-server approval denied.";
}
function approvalScopeForOutcome(outcome: AppServerApprovalOutcome): "turn" | "session" {
return outcome === "approved-session" ? "session" : "turn";
}
function approvalEventScope(
method: string,
outcome: AppServerApprovalOutcome,
): Pick<AgentApprovalEventData, "scope"> {
return method === "item/permissions/requestApproval"
? { scope: approvalScopeForOutcome(outcome) }
: {};
}
function approvalKindForMethod(method: string): AgentApprovalEventData["kind"] {
if (method.includes("commandExecution") || method.includes("execCommand")) {
return "exec";

View File

@@ -31,7 +31,6 @@ describe("CodexAppServerClient", () => {
resetSharedCodexAppServerClientForTests();
vi.useRealTimers();
vi.restoreAllMocks();
vi.useRealTimers();
for (const client of clients) {
client.close();
}
@@ -253,4 +252,26 @@ describe("CodexAppServerClient", () => {
expect(isCodexAppServerApprovalRequest("evil/Approval")).toBe(false);
expect(isCodexAppServerApprovalRequest("item/tool/requestApproval")).toBe(false);
});
it("fails closed for unhandled request_user_input prompts", async () => {
const harness = createClientHarness();
clients.push(harness.client);
harness.send({
id: "input-1",
method: "item/tool/requestUserInput",
params: {
threadId: "thread-1",
turnId: "turn-1",
itemId: "tool-1",
questions: [],
},
});
await vi.waitFor(() => expect(harness.writes.length).toBe(1));
expect(JSON.parse(harness.writes[0] ?? "{}")).toEqual({
id: "input-1",
result: { answers: {} },
});
});
});

View File

@@ -0,0 +1,284 @@
import {
callGatewayTool,
embeddedAgentLog,
type EmbeddedRunAttemptParams,
} from "openclaw/plugin-sdk/agent-harness";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { handleCodexAppServerElicitationRequest } from "./elicitation-bridge.js";
vi.mock("openclaw/plugin-sdk/agent-harness", async (importOriginal) => ({
...(await importOriginal<typeof import("openclaw/plugin-sdk/agent-harness")>()),
callGatewayTool: vi.fn(),
}));
const mockCallGatewayTool = vi.mocked(callGatewayTool);
function createParams(): EmbeddedRunAttemptParams {
return {
sessionKey: "agent:main:session-1",
agentId: "main",
messageChannel: "telegram",
currentChannelId: "chat-1",
agentAccountId: "default",
currentThreadTs: "thread-ts",
} as unknown as EmbeddedRunAttemptParams;
}
function buildApprovalElicitation() {
return {
threadId: "thread-1",
turnId: "turn-1",
serverName: "codex_apps__github",
mode: "form",
message: "Approve app tool call?",
_meta: {
codex_approval_kind: "mcp_tool_call",
persist: ["session", "always"],
},
requestedSchema: {
type: "object",
properties: {
approve: {
type: "boolean",
title: "Approve this tool call",
},
persist: {
type: "string",
title: "Persist choice",
enum: ["session", "always"],
},
},
required: ["approve"],
},
};
}
describe("Codex app-server elicitation bridge", () => {
beforeEach(() => {
mockCallGatewayTool.mockReset();
vi.restoreAllMocks();
});
it("routes MCP tool approval elicitations through plugin approvals", async () => {
mockCallGatewayTool
.mockResolvedValueOnce({ id: "plugin:approval-1", status: "accepted" })
.mockResolvedValueOnce({ id: "plugin:approval-1", decision: "allow-once" });
const result = await handleCodexAppServerElicitationRequest({
requestParams: buildApprovalElicitation(),
paramsForRun: createParams(),
threadId: "thread-1",
turnId: "turn-1",
});
expect(result).toEqual({
action: "accept",
content: {
approve: true,
},
_meta: null,
});
expect(mockCallGatewayTool.mock.calls.map(([method]) => method)).toEqual([
"plugin.approval.request",
"plugin.approval.waitDecision",
]);
});
it("maps allow-always decisions onto session-scoped persistence when offered", async () => {
mockCallGatewayTool
.mockResolvedValueOnce({ id: "plugin:approval-2", status: "accepted" })
.mockResolvedValueOnce({ id: "plugin:approval-2", decision: "allow-always" });
const result = await handleCodexAppServerElicitationRequest({
requestParams: buildApprovalElicitation(),
paramsForRun: createParams(),
threadId: "thread-1",
turnId: "turn-1",
});
expect(result).toEqual({
action: "accept",
content: {
approve: true,
persist: "session",
},
_meta: null,
});
});
it("does not inherit persist defaults for one-time approvals", async () => {
mockCallGatewayTool
.mockResolvedValueOnce({ id: "plugin:approval-5", status: "accepted" })
.mockResolvedValueOnce({ id: "plugin:approval-5", decision: "allow-once" });
const result = await handleCodexAppServerElicitationRequest({
requestParams: {
...buildApprovalElicitation(),
requestedSchema: {
type: "object",
properties: {
approve: {
type: "boolean",
title: "Approve this tool call",
},
persist: {
type: "string",
title: "Persist choice",
enum: ["session", "always"],
default: "always",
},
},
required: ["approve"],
},
},
paramsForRun: createParams(),
threadId: "thread-1",
turnId: "turn-1",
});
expect(result).toEqual({
action: "accept",
content: {
approve: true,
},
_meta: null,
});
});
it("truncates long approval titles and descriptions before requesting approval", async () => {
mockCallGatewayTool
.mockResolvedValueOnce({ id: "plugin:approval-4", status: "accepted" })
.mockResolvedValueOnce({ id: "plugin:approval-4", decision: "allow-once" });
const result = await handleCodexAppServerElicitationRequest({
requestParams: {
...buildApprovalElicitation(),
message: "Approve ".repeat(20).trim(),
requestedSchema: {
type: "object",
properties: {
approve: {
type: "boolean",
title: "Approve this tool call",
description: "Explain ".repeat(60).trim(),
},
},
required: ["approve"],
},
},
paramsForRun: createParams(),
threadId: "thread-1",
turnId: "turn-1",
});
expect(result).toEqual({
action: "accept",
content: {
approve: true,
},
_meta: null,
});
expect(mockCallGatewayTool).toHaveBeenCalledWith(
"plugin.approval.request",
expect.any(Object),
expect.objectContaining({
title: expect.any(String),
description: expect.any(String),
}),
{ expectFinal: false },
);
const approvalRequest = mockCallGatewayTool.mock.calls[0]?.[2] as {
title: string;
description: string;
};
expect(approvalRequest.title.length).toBeLessThanOrEqual(80);
expect(approvalRequest.description.length).toBeLessThanOrEqual(256);
});
it("fails closed when the approval route is unavailable", async () => {
mockCallGatewayTool.mockResolvedValueOnce({ id: "plugin:approval-3", decision: null });
const result = await handleCodexAppServerElicitationRequest({
requestParams: buildApprovalElicitation(),
paramsForRun: createParams(),
threadId: "thread-1",
turnId: "turn-1",
});
expect(result).toEqual({
action: "decline",
content: null,
_meta: null,
});
});
it("ignores non-approval elicitation requests", async () => {
const result = await handleCodexAppServerElicitationRequest({
requestParams: {
threadId: "thread-1",
turnId: "turn-1",
serverName: "codex_apps__github",
mode: "form",
message: "Choose a template",
_meta: {},
requestedSchema: {
type: "object",
properties: {
template: {
type: "string",
enum: ["simple", "fancy"],
},
},
required: ["template"],
},
},
paramsForRun: createParams(),
threadId: "thread-1",
turnId: "turn-1",
});
expect(result).toBeUndefined();
expect(mockCallGatewayTool).not.toHaveBeenCalled();
});
it("logs and declines approved elicitations that do not expose an approval field", async () => {
const warn = vi.spyOn(embeddedAgentLog, "warn").mockImplementation(() => undefined);
mockCallGatewayTool
.mockResolvedValueOnce({ id: "plugin:approval-6", status: "accepted" })
.mockResolvedValueOnce({ id: "plugin:approval-6", decision: "allow-once" });
const result = await handleCodexAppServerElicitationRequest({
requestParams: {
...buildApprovalElicitation(),
requestedSchema: {
type: "object",
properties: {
confirmChoice: {
type: "string",
title: "Confirmation choice",
enum: ["yes", "no"],
},
},
required: ["confirmChoice"],
},
},
paramsForRun: createParams(),
threadId: "thread-1",
turnId: "turn-1",
});
expect(result).toEqual({
action: "decline",
content: null,
_meta: null,
});
expect(warn).toHaveBeenCalledWith(
"codex MCP approval elicitation approved without a mappable response",
expect.objectContaining({
approvalKind: "mcp_tool_call",
fields: ["confirmChoice"],
outcome: "approved-once",
}),
);
});
});

View File

@@ -0,0 +1,345 @@
import { embeddedAgentLog, type EmbeddedRunAttemptParams } from "openclaw/plugin-sdk/agent-harness";
import {
mapExecDecisionToOutcome,
requestPluginApproval,
type AppServerApprovalOutcome,
waitForPluginApprovalDecision,
} from "./plugin-approval-roundtrip.js";
import { isJsonObject, type JsonObject, type JsonValue } from "./protocol.js";
type ApprovalPropertyContext = {
name: string;
schema: JsonObject;
required: boolean;
};
type BridgeableApprovalElicitation = {
title: string;
description: string;
requestedSchema: JsonObject;
meta: JsonObject;
};
export async function handleCodexAppServerElicitationRequest(params: {
requestParams: JsonValue | undefined;
paramsForRun: EmbeddedRunAttemptParams;
threadId: string;
turnId: string;
signal?: AbortSignal;
}): Promise<JsonValue | undefined> {
const requestParams = isJsonObject(params.requestParams) ? params.requestParams : undefined;
if (!matchesCurrentTurn(requestParams, params.threadId, params.turnId)) {
return undefined;
}
const approvalPrompt = readBridgeableApprovalElicitation(requestParams);
if (!approvalPrompt) {
return undefined;
}
const outcome = await requestPluginApprovalOutcome({
paramsForRun: params.paramsForRun,
title: approvalPrompt.title,
description: approvalPrompt.description,
signal: params.signal,
});
return buildElicitationResponse(approvalPrompt.requestedSchema, approvalPrompt.meta, outcome);
}
function matchesCurrentTurn(
requestParams: JsonObject | undefined,
threadId: string,
turnId: string,
): boolean {
if (!requestParams) {
return false;
}
const requestThreadId = readString(requestParams, "threadId");
if (requestThreadId !== threadId) {
return false;
}
const rawTurnId = requestParams.turnId;
if (rawTurnId !== null && rawTurnId !== undefined && rawTurnId !== turnId) {
return false;
}
return true;
}
function readBridgeableApprovalElicitation(
requestParams: JsonObject | undefined,
): BridgeableApprovalElicitation | undefined {
if (
!requestParams ||
readString(requestParams, "mode") !== "form" ||
!isJsonObject(requestParams._meta) ||
requestParams._meta.codex_approval_kind !== "mcp_tool_call" ||
!isJsonObject(requestParams.requestedSchema)
) {
return undefined;
}
const requestedSchema = requestParams.requestedSchema;
if (
readString(requestedSchema, "type") !== "object" ||
!isJsonObject(requestedSchema.properties) ||
Object.keys(requestedSchema.properties).length === 0
) {
return undefined;
}
const title = readString(requestParams, "message") ?? "Codex MCP tool approval";
const propertyLines = Object.entries(requestedSchema.properties)
.map(([name, value]) => {
const schema = isJsonObject(value) ? value : undefined;
if (!schema) {
return undefined;
}
const propTitle = readString(schema, "title") ?? name;
const description = readString(schema, "description");
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,
};
}
async function requestPluginApprovalOutcome(params: {
paramsForRun: EmbeddedRunAttemptParams;
title: string;
description: string;
signal?: AbortSignal;
}): Promise<AppServerApprovalOutcome> {
try {
const requestResult = await requestPluginApproval({
paramsForRun: params.paramsForRun,
title: params.title,
description: params.description,
severity: "warning",
toolName: "codex_mcp_tool_approval",
});
const approvalId = requestResult?.id;
if (!approvalId) {
return "unavailable";
}
const decision = Object.prototype.hasOwnProperty.call(requestResult, "decision")
? requestResult.decision
: await waitForPluginApprovalDecision({ approvalId, signal: params.signal });
return mapExecDecisionToOutcome(decision);
} catch {
return params.signal?.aborted ? "cancelled" : "denied";
}
}
function buildElicitationResponse(
requestedSchema: JsonObject,
meta: JsonObject,
outcome: AppServerApprovalOutcome,
): JsonValue {
if (outcome === "cancelled") {
return { action: "cancel", content: null, _meta: null };
}
if (outcome === "denied" || outcome === "unavailable") {
return { action: "decline", content: null, _meta: null };
}
const content = buildAcceptedContent(requestedSchema, meta, outcome);
if (!content) {
embeddedAgentLog.warn("codex MCP approval elicitation approved without a mappable response", {
approvalKind: meta.codex_approval_kind,
fields: Object.keys(requestedSchema.properties ?? {}),
outcome,
});
return { action: "decline", content: null, _meta: null };
}
return { action: "accept", content, _meta: null };
}
function buildAcceptedContent(
requestedSchema: JsonObject,
meta: JsonObject,
outcome: AppServerApprovalOutcome,
): JsonObject | undefined {
const properties = isJsonObject(requestedSchema.properties)
? requestedSchema.properties
: undefined;
if (!properties) {
return undefined;
}
const required = Array.isArray(requestedSchema.required)
? new Set(
requestedSchema.required.filter((entry): entry is string => typeof entry === "string"),
)
: new Set<string>();
const content: JsonObject = {};
let sawApprovalField = false;
for (const [name, value] of Object.entries(properties)) {
const schema = isJsonObject(value) ? value : undefined;
if (!schema) {
continue;
}
const property = { name, schema, required: required.has(name) };
const next =
readApprovalFieldValue(property, outcome) ??
readPersistFieldValue(property, meta, outcome) ??
readFallbackFieldValue(property, outcome);
if (next === undefined) {
if (isApprovalField(property)) {
sawApprovalField = true;
}
if (property.required) {
return undefined;
}
continue;
}
if (isApprovalField(property)) {
sawApprovalField = true;
}
content[name] = next;
}
return sawApprovalField ? content : undefined;
}
function readApprovalFieldValue(
property: ApprovalPropertyContext,
outcome: AppServerApprovalOutcome,
): JsonValue | undefined {
if (!isApprovalField(property)) {
return undefined;
}
const type = readString(property.schema, "type");
if (type === "boolean") {
return true;
}
const options = readEnumOptions(property.schema);
if (options.length === 0) {
return undefined;
}
const sessionChoice = options.find((option) => isSessionApprovalOption(option));
const acceptChoice = options.find((option) => isPositiveApprovalOption(option));
if (outcome === "approved-session") {
return sessionChoice?.value ?? acceptChoice?.value;
}
return acceptChoice?.value ?? sessionChoice?.value;
}
function readPersistFieldValue(
property: ApprovalPropertyContext,
meta: JsonObject,
outcome: AppServerApprovalOutcome,
): JsonValue | undefined {
if (!isPersistField(property) || outcome !== "approved-session") {
return undefined;
}
const persistHints = readPersistHints(meta);
const options = readEnumOptions(property.schema);
if (options.length === 0) {
return undefined;
}
for (const preferred of persistHints) {
const match = options.find(
(option) => option.value === preferred || option.label === preferred,
);
if (match) {
return match.value;
}
}
return options.find((option) => option.value === "session" || option.label === "session")?.value;
}
function readDefaultValue(schema: JsonObject): JsonValue | undefined {
return schema.default as JsonValue | undefined;
}
function readFallbackFieldValue(
property: ApprovalPropertyContext,
outcome: AppServerApprovalOutcome,
): JsonValue | undefined {
if (outcome === "approved-once" && isPersistField(property)) {
return undefined;
}
return readDefaultValue(property.schema);
}
function isApprovalField(property: ApprovalPropertyContext): boolean {
const haystack = propertyText(property).toLowerCase();
return /\b(approve|approval|allow|accept|decision)\b/.test(haystack);
}
function isPersistField(property: ApprovalPropertyContext): boolean {
const haystack = propertyText(property).toLowerCase();
return /\b(persist|session|always|scope)\b/.test(haystack);
}
function propertyText(property: ApprovalPropertyContext): string {
return [
property.name,
readString(property.schema, "title"),
readString(property.schema, "description"),
]
.filter(Boolean)
.join(" ");
}
function readPersistHints(meta: JsonObject): string[] {
const raw = meta.persist;
if (typeof raw === "string") {
return [raw];
}
if (Array.isArray(raw)) {
return raw.filter((entry): entry is string => typeof entry === "string");
}
return ["session", "always"];
}
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");
const labels = Array.isArray(schema.enumNames)
? schema.enumNames.filter((entry): entry is string => typeof entry === "string")
: [];
return values.map((value, index) => ({ value, label: labels[index] ?? value }));
}
if (Array.isArray(schema.oneOf)) {
return schema.oneOf
.map((entry) => {
const option = isJsonObject(entry) ? entry : undefined;
const value = readString(option, "const");
if (!value) {
return undefined;
}
return { value, label: readString(option, "title") ?? value };
})
.filter((entry): entry is { value: string; label: string } => Boolean(entry));
}
return [];
}
function isPositiveApprovalOption(option: { value: string; label: string }): boolean {
const haystack = `${option.value} ${option.label}`.toLowerCase();
return /\b(allow|approve|accept|yes|continue|proceed|true)\b/.test(haystack);
}
function isSessionApprovalOption(option: { value: string; label: string }): boolean {
const haystack = `${option.value} ${option.label}`.toLowerCase();
return (
/\b(session|always|persistent)\b/.test(haystack) && /\b(allow|approve|accept)\b/.test(haystack)
);
}
function readString(record: JsonObject | undefined, key: string): string | undefined {
const value = record?.[key];
return typeof value === "string" && value.trim() ? value : undefined;
}

View File

@@ -292,6 +292,25 @@ describe("CodexAppServerEventProjector", () => {
expect(result.assistantTexts).toEqual([]);
});
it("ignores notifications that omit top-level thread and turn ids", async () => {
const projector = await createProjector();
await projector.handleNotification({
method: "turn/completed",
params: {
turn: {
id: TURN_ID,
status: "completed",
items: [{ type: "agentMessage", id: "msg-1", text: "wrong turn" }],
},
},
});
const result = projector.buildResult(buildEmptyToolTelemetry());
expect(result.assistantTexts).toEqual([]);
expect(result.lastAssistant).toBeUndefined();
});
it("preserves sessions_yield detection in attempt results", () => {
const projector = new CodexAppServerEventProjector(
{

View File

@@ -577,7 +577,7 @@ export class CodexAppServerEventProjector {
private isNotificationForTurn(params: JsonObject): boolean {
const threadId = readString(params, "threadId");
const turnId = readString(params, "turnId");
return (!threadId || threadId === this.threadId) && (!turnId || turnId === this.turnId);
return threadId === this.threadId && turnId === this.turnId;
}
}

View File

@@ -0,0 +1,106 @@
import { callGatewayTool, type EmbeddedRunAttemptParams } from "openclaw/plugin-sdk/agent-harness";
export const DEFAULT_CODEX_APPROVAL_TIMEOUT_MS = 120_000;
const MAX_PLUGIN_APPROVAL_TITLE_LENGTH = 80;
const MAX_PLUGIN_APPROVAL_DESCRIPTION_LENGTH = 256;
type ExecApprovalDecision = "allow-once" | "allow-always" | "deny";
export type AppServerApprovalOutcome =
| "approved-once"
| "approved-session"
| "denied"
| "unavailable"
| "cancelled";
type ApprovalRequestResult = {
id?: string;
decision?: ExecApprovalDecision | null;
};
type ApprovalWaitResult = {
id?: string;
decision?: ExecApprovalDecision | null;
};
export async function requestPluginApproval(params: {
paramsForRun: EmbeddedRunAttemptParams;
title: string;
description: string;
severity: "info" | "warning";
toolName: string;
toolCallId?: string;
}): Promise<ApprovalRequestResult | undefined> {
const timeoutMs = DEFAULT_CODEX_APPROVAL_TIMEOUT_MS;
return callGatewayTool(
"plugin.approval.request",
{ timeoutMs: timeoutMs + 10_000 },
{
pluginId: "openclaw-codex-app-server",
title: truncateForGateway(params.title, MAX_PLUGIN_APPROVAL_TITLE_LENGTH),
description: truncateForGateway(params.description, MAX_PLUGIN_APPROVAL_DESCRIPTION_LENGTH),
severity: params.severity,
toolName: params.toolName,
toolCallId: params.toolCallId,
agentId: params.paramsForRun.agentId,
sessionKey: params.paramsForRun.sessionKey,
turnSourceChannel: params.paramsForRun.messageChannel ?? params.paramsForRun.messageProvider,
turnSourceTo: params.paramsForRun.currentChannelId,
turnSourceAccountId: params.paramsForRun.agentAccountId,
turnSourceThreadId: params.paramsForRun.currentThreadTs,
timeoutMs,
twoPhase: true,
},
{ expectFinal: false },
) as Promise<ApprovalRequestResult | undefined>;
}
export async function waitForPluginApprovalDecision(params: {
approvalId: string;
signal?: AbortSignal;
}): Promise<ExecApprovalDecision | null | undefined> {
const timeoutMs = DEFAULT_CODEX_APPROVAL_TIMEOUT_MS;
const waitPromise: Promise<ApprovalWaitResult | undefined> = callGatewayTool(
"plugin.approval.waitDecision",
{ timeoutMs: timeoutMs + 10_000 },
{ id: params.approvalId },
);
if (!params.signal) {
return (await waitPromise)?.decision;
}
let onAbort: (() => void) | undefined;
const abortPromise = new Promise<never>((_, reject) => {
if (params.signal!.aborted) {
reject(params.signal!.reason);
return;
}
onAbort = () => reject(params.signal!.reason);
params.signal!.addEventListener("abort", onAbort, { once: true });
});
try {
return (await Promise.race([waitPromise, abortPromise]))?.decision;
} finally {
if (onAbort) {
params.signal.removeEventListener("abort", onAbort);
}
}
}
export function mapExecDecisionToOutcome(
decision: ExecApprovalDecision | null | undefined,
): AppServerApprovalOutcome {
if (decision === "allow-once") {
return "approved-once";
}
if (decision === "allow-always") {
return "approved-session";
}
if (decision === null || decision === undefined) {
return "unavailable";
}
return "denied";
}
function truncateForGateway(value: string, maxLength: number): string {
return value.length <= maxLength ? value : `${value.slice(0, Math.max(0, maxLength - 3))}...`;
}

View File

@@ -14,6 +14,7 @@ import {
} from "../../../../src/plugins/hook-runner-global.js";
import { createMockPluginRegistry } from "../../../../src/plugins/hooks.test-helpers.js";
import { CODEX_GPT5_BEHAVIOR_CONTRACT } from "../../prompt-overlay.js";
import * as elicitationBridge from "./elicitation-bridge.js";
import type { CodexServerNotification } from "./protocol.js";
import { runCodexAppServerAttempt, __testing } from "./run-attempt.js";
import { writeCodexAppServerBinding } from "./session-binding.js";
@@ -104,6 +105,9 @@ function createAppServerHarness(
interval: 1,
});
},
async notify(notification: CodexServerNotification) {
await notify(notification);
},
async completeTurn(params: { threadId: string; turnId: string }) {
await notify({
method: "turn/completed",
@@ -114,9 +118,6 @@ function createAppServerHarness(
},
});
},
async notify(notification: CodexServerNotification) {
await notify(notification);
},
};
}
@@ -621,6 +622,50 @@ describe("runCodexAppServerAttempt", () => {
});
});
it("does not complete on unscoped turn/completed notifications", async () => {
const harness = createStartedThreadHarness();
const run = runCodexAppServerAttempt(
createParams(path.join(tempDir, "session.jsonl"), path.join(tempDir, "workspace")),
);
let resolved = false;
void run.then(() => {
resolved = true;
});
await harness.waitForMethod("turn/start");
await harness.notify({
method: "turn/completed",
params: {
turn: {
id: "turn-1",
status: "completed",
items: [{ type: "agentMessage", id: "msg-wrong", text: "wrong completion" }],
},
},
});
await new Promise((resolve) => setTimeout(resolve, 25));
expect(resolved).toBe(false);
await harness.notify({
method: "turn/completed",
params: {
threadId: "thread-1",
turnId: "turn-1",
turn: {
id: "turn-1",
status: "completed",
items: [{ type: "agentMessage", id: "msg-right", text: "final completion" }],
},
},
});
await expect(run).resolves.toMatchObject({
assistantTexts: ["final completion"],
aborted: false,
timedOut: false,
});
});
it("releases completion when a projector callback throws during turn/completed", async () => {
// Regression for openclaw/openclaw#67996: a throw inside the projector's
// turn/completed handler must not strand resolveCompletion, otherwise the
@@ -676,6 +721,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: { approve: true },
_meta: null,
});
const request = vi.fn(async (method: string) => {
if (method === "thread/start") {
return { thread: { id: "thread-1" }, model: "gpt-5.4-codex", modelProvider: "openai" };
}
if (method === "turn/start") {
return { turn: { id: "turn-1", status: "inProgress" } };
}
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"));
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: { approve: true },
_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("times out app-server startup before thread setup can hang forever", async () => {
__testing.setCodexAppServerClientFactoryForTests(() => new Promise<never>(() => undefined));
const params = createParams(

View File

@@ -31,6 +31,7 @@ import {
import { isCodexAppServerApprovalRequest, type CodexAppServerClient } from "./client.js";
import { resolveCodexAppServerRuntimeOptions } from "./config.js";
import { createCodexDynamicToolBridge } from "./dynamic-tools.js";
import { handleCodexAppServerElicitationRequest } from "./elicitation-bridge.js";
import { CodexAppServerEventProjector } from "./event-projector.js";
import {
isJsonObject,
@@ -175,7 +176,8 @@ export async function runCodexAppServerAttempt(
// inside projector.handleNotification still releases the session lane.
// See openclaw/openclaw#67996.
const isTurnCompletion =
notification.method === "turn/completed" && isTurnNotification(notification.params, turnId);
notification.method === "turn/completed" &&
isTurnNotification(notification.params, thread.threadId, turnId);
try {
await projector.handleNotification(notification);
} catch (error) {
@@ -203,6 +205,15 @@ export async function runCodexAppServerAttempt(
if (!turnId) {
return undefined;
}
if (request.method === "mcpServer/elicitation/request") {
return handleCodexAppServerElicitationRequest({
requestParams: request.params,
paramsForRun: params,
threadId: thread.threadId,
turnId,
signal: runAbortController.signal,
});
}
if (request.method !== "item/tool/call") {
if (isCodexAppServerApprovalRequest(request.method)) {
return handleApprovalRequest({
@@ -562,16 +573,15 @@ function readDynamicToolCallParams(
};
}
function isTurnNotification(value: JsonValue | undefined, turnId: string): boolean {
function isTurnNotification(
value: JsonValue | undefined,
threadId: string,
turnId: string,
): boolean {
if (!isJsonObject(value)) {
return false;
}
const directTurnId = readString(value, "turnId");
if (directTurnId === turnId) {
return true;
}
const turn = isJsonObject(value.turn) ? value.turn : undefined;
return readString(turn ?? {}, "id") === turnId;
return readString(value, "threadId") === threadId && readString(value, "turnId") === turnId;
}
function readString(record: JsonObject, key: string): string | undefined {

View File

@@ -108,6 +108,7 @@ export type GetReplyOptions = {
command?: string;
host?: string;
reason?: string;
scope?: "turn" | "session";
message?: string;
}) => Promise<void> | void;
/** Called when command output streams or completes. */

View File

@@ -762,6 +762,7 @@ describe("runAgentTurnWithFallback", () => {
command: undefined,
host: undefined,
reason: undefined,
scope: undefined,
message: undefined,
});
expect(onCommandOutput).toHaveBeenCalledWith({

View File

@@ -86,6 +86,10 @@ const GPT_CHAT_BREVITY_ACK_MAX_SENTENCES = 3;
const GPT_CHAT_BREVITY_SOFT_MAX_CHARS = 900;
const GPT_CHAT_BREVITY_SOFT_MAX_SENTENCES = 6;
function readApprovalScopeValue(value: unknown): "turn" | "session" | undefined {
return value === "turn" || value === "session" ? value : undefined;
}
export type RuntimeFallbackAttempt = {
provider: string;
model: string;
@@ -1124,6 +1128,7 @@ export async function runAgentTurnWithFallback(params: {
command: readStringValue(evt.data.command),
host: readStringValue(evt.data.host),
reason: readStringValue(evt.data.reason),
scope: readApprovalScopeValue(evt.data.scope),
message: readStringValue(evt.data.message),
});
}

View File

@@ -68,6 +68,7 @@ export type AgentApprovalEventData = {
command?: string;
host?: string;
reason?: string;
scope?: "turn" | "session";
message?: string;
};