diff --git a/CHANGELOG.md b/CHANGELOG.md index dae1b57fede..6131b15b48f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,7 @@ Docs: https://docs.openclaw.ai - QA/Mantis: pass the runtime env through desktop-browser Crabbox and artifact-copy child commands, so embedded Mantis callers can provide Crabbox credentials without mutating the parent process. Thanks @vincentkoc. - QA/Mantis: return the copied Slack desktop screenshot path even when remote Slack QA fails, so the CLI still prints the failure screenshot artifact. Thanks @vincentkoc. - QA/Mantis: accept Blacksmith Testbox `tbx_...` lease ids from desktop smoke warmup, so provider overrides do not fail before inspect/run. Thanks @vincentkoc. +- QA/Codex harness: add targeted live Docker/Testbox diagnostics, auth preflight checks, cache mount fixes, and app-server protocol checkout discovery so maintainer harness failures are easier to reproduce. Thanks @vincentkoc. - Plugins/update: treat official externalized bundled npm migrations and ClawHub-to-npm fallbacks as trusted source-linked installs, so prerelease-only official plugin packages can migrate from bundled builds without being rejected as unsafe prerelease resolutions. Thanks @vincentkoc. - Plugins/update: move ClawHub-preferred externalized plugin installs back to ClawHub after an earlier npm fallback once the ClawHub package becomes available. Thanks @vincentkoc. - Plugins/update: clean stale bundled load paths for already-externalized pinned npm and ClawHub plugin installs, so release-channel sync does not leave removed bundled paths ahead of the installed external package. Thanks @vincentkoc. @@ -128,6 +129,9 @@ Docs: https://docs.openclaw.ai - Web search: scope explicit bundled `web_search` provider runtime loading through manifest ownership, so selecting DuckDuckGo/Gemini/etc. does not import unrelated bundled providers or log their optional dependency failures. Thanks @vincentkoc. - Plugins/discovery: demote the source-only TypeScript runtime check on already-installed `origin: "global"` plugin packages from a config-blocking error to a warning and let the runtime fall through to the TypeScript source via jiti, so a single broken installed package no longer blocks `plugins install` for unrelated plugins; install-time rejection of newly-installed source-only packages is unchanged. Thanks @romneyda. - Providers/OpenAI Codex: stop the OAuth progress spinner before showing the manual redirect paste prompt, so callback timeouts do not spam `Browser callback did not finish` across terminals. +- Providers/OpenAI Codex: fail closed on malformed `/codex` control commands and diagnostics confirmations before changing bindings, permissions, model overrides, active turns, or feedback uploads. Thanks @vincentkoc. +- Providers/OpenAI Codex: sanitize Codex app-server command readouts, failure replies, approval prompts, elicitation prompts, and `request_user_input` text before posting them back into chat. Thanks @vincentkoc. +- Providers/OpenAI Codex: preserve local bound-turn image paths, reject stale same-thread turn notifications, enforce option-only user input prompts, and return failed dynamic tool results to Codex as unsuccessful tool calls. Thanks @vincentkoc. - Providers/DeepSeek: expose DeepSeek V4 `xhigh` and `max` thinking levels through the lightweight provider-policy surface, so Control UI `/think` pickers keep showing the max reasoning options when the runtime plugin registry is not active. Fixes #77139. Thanks @bittoby. - Release/beta smoke: resolve the dispatched Telegram beta E2E run from `gh run list` when `gh workflow run` returns no run URL, so the maintainer helper does not fail immediately after dispatch. Thanks @vincentkoc. - Media/images: keep HEIC/HEIF attachments fail-closed when optional Sharp conversion is unavailable instead of sending originals that still need conversion. Thanks @vincentkoc. @@ -220,6 +224,7 @@ Docs: https://docs.openclaw.ai - OpenAI/Google Meet: fail realtime voice connection attempts when the socket closes before `session.updated`, avoiding stuck Meet joins waiting on a bridge that never became ready. Thanks @vincentkoc. - Google Meet: avoid treating repeated participant words as multiple assistant-overlap matches when suppressing realtime echo transcripts. Thanks @vincentkoc. - Google Meet: make `mode: "agent"` the default Chrome talk-back path, using realtime transcription for input and regular OpenClaw TTS for speech output, while keeping direct realtime voice answers available as `mode: "bidi"` and accepting `mode: "realtime"` as an agent-mode compatibility alias. +- Codex harness: keep `codex_app_server.*` telemetry publication owned by the harness instead of republishing the same callback event from core runners. Thanks @vincentkoc. - Slack/Discord: suppress standalone tool-progress chatter when partial preview streaming has `streaming.preview.toolProgress: false`, matching the documented quiet-preview behavior. Thanks @vincentkoc. - Matrix: bind native approval reaction targets before publishing option reactions, so fast approver reactions on threaded prompts are not dropped while the approval handler finishes setup. Thanks @vincentkoc. - Google Meet: make realtime talk-back agent-driven by default with `realtime.strategy: "agent"`, keep the previous direct bidirectional model behavior available as `realtime.strategy: "bidi"`, route the Meet tab speaker output to `BlackHole 2ch` automatically for local Chrome realtime joins, coalesce nearby speech transcript fragments before consulting the agent, and avoid cutting off agent speech from server VAD or stale playback pipe errors. diff --git a/extensions/codex/src/app-server/approval-bridge.test.ts b/extensions/codex/src/app-server/approval-bridge.test.ts index 1ef725f04bb..3276244b19d 100644 --- a/extensions/codex/src/app-server/approval-bridge.test.ts +++ b/extensions/codex/src/app-server/approval-bridge.test.ts @@ -118,6 +118,83 @@ describe("Codex app-server approval bridge", () => { ); }); + it("describes command approval permission and policy amendments", async () => { + const params = createParams(); + mockCallGatewayTool + .mockResolvedValueOnce({ id: "plugin:approval-command-permissions", status: "accepted" }) + .mockResolvedValueOnce({ + id: "plugin:approval-command-permissions", + decision: "allow-always", + }); + + const result = await handleCodexAppServerApprovalRequest({ + method: "item/commandExecution/requestApproval", + requestParams: { + threadId: "thread-1", + turnId: "turn-1", + itemId: "cmd-permissions", + command: "npm install", + additionalPermissions: { + network: { enabled: true }, + fileSystem: { + write: ["/"], + }, + }, + proposedExecpolicyAmendment: ["npm install"], + proposedNetworkPolicyAmendments: [{ host: "registry.npmjs.org", action: "allow" }], + }, + paramsForRun: params, + threadId: "thread-1", + turnId: "turn-1", + }); + + expect(result).toEqual({ decision: "acceptForSession" }); + const [, , requestPayload] = mockCallGatewayTool.mock.calls[0] ?? []; + const description = (requestPayload as { description: string }).description; + expect(description).toContain("Command: npm install"); + expect(description).toContain("Additional permissions: network, fileSystem"); + expect(description).toContain("High-risk targets: network access, filesystem root"); + expect(description).toContain("Network enabled: true"); + expect(description).toContain("File system write: /"); + expect(description).toContain("Proposed exec policy: npm install"); + expect(description).toContain("Proposed network policy: allow registry.npmjs.org"); + }); + + it("keeps command approval permission details visible after long command previews", async () => { + const params = createParams(); + mockCallGatewayTool + .mockResolvedValueOnce({ id: "plugin:approval-long-command-permissions", status: "accepted" }) + .mockResolvedValueOnce({ + id: "plugin:approval-long-command-permissions", + decision: "allow-always", + }); + + await handleCodexAppServerApprovalRequest({ + method: "item/commandExecution/requestApproval", + requestParams: { + threadId: "thread-1", + turnId: "turn-1", + itemId: "cmd-long-permissions", + command: `${"npm install ".repeat(500)} --unsafe-perm`, + additionalPermissions: { + network: { enabled: true }, + fileSystem: { + write: ["/"], + }, + }, + }, + paramsForRun: params, + threadId: "thread-1", + turnId: "turn-1", + }); + + const [, , requestPayload] = mockCallGatewayTool.mock.calls[0] ?? []; + const description = (requestPayload as { description: string }).description; + expect(description).toContain("[preview truncated or unsafe content omitted]"); + expect(description).toContain("Additional permissions: network, fileSystem"); + expect(description).toContain("High-risk targets: network access, filesystem root"); + }); + it("sanitizes command previews before forwarding approval text and events", async () => { const params = createParams(); mockCallGatewayTool @@ -155,6 +232,44 @@ describe("Codex app-server approval bridge", () => { ); }); + it("escapes command approval previews before forwarding approval text and events", async () => { + const params = createParams(); + mockCallGatewayTool + .mockResolvedValueOnce({ id: "plugin:approval-escaped-command", status: "accepted" }) + .mockResolvedValueOnce({ id: "plugin:approval-escaped-command", decision: "allow-once" }); + + await handleCodexAppServerApprovalRequest({ + method: "item/commandExecution/requestApproval", + requestParams: { + threadId: "thread-1", + turnId: "turn-1", + itemId: "cmd-escaped", + command: "printf '<@U123> [trusted](https://evil) @here'", + }, + paramsForRun: params, + threadId: "thread-1", + turnId: "turn-1", + }); + + const [, , requestPayload] = mockCallGatewayTool.mock.calls[0] ?? []; + const description = (requestPayload as { description: string }).description; + expect(description).toContain( + "printf '<\uff20U123> \uff3btrusted\uff3d\uff08https://evil\uff09 \uff20here'", + ); + expect(description).not.toContain("<@U123>"); + expect(description).not.toContain("[trusted](https://evil)"); + expect(description).not.toContain("@here"); + expect(params.onAgentEvent).toHaveBeenCalledWith( + expect.objectContaining({ + stream: "approval", + data: expect.objectContaining({ + command: + "printf '<\uff20U123> \uff3btrusted\uff3d\uff08https://evil\uff09 \uff20here'", + }), + }), + ); + }); + it("preserves visible OSC-8 link labels in command previews", async () => { const params = createParams(); mockCallGatewayTool @@ -615,6 +730,59 @@ describe("Codex app-server approval bridge", () => { expect(description).toContain("readPaths: ~/.ssh/id_rsa, /etc/hosts"); }); + it("describes current protocol network and filesystem permission grants", async () => { + const params = createParams(); + mockCallGatewayTool + .mockResolvedValueOnce({ id: "plugin:approval-current-permissions", status: "accepted" }) + .mockResolvedValueOnce({ id: "plugin:approval-current-permissions", decision: "allow-once" }); + + const result = await handleCodexAppServerApprovalRequest({ + method: "item/permissions/requestApproval", + requestParams: { + threadId: "thread-1", + turnId: "turn-1", + itemId: "perm-current", + permissions: { + network: { enabled: true }, + fileSystem: { + read: ["/Users/simone/.ssh/id_rsa"], + write: ["/"], + entries: [ + { path: "/workspace/project", access: "read" }, + { path: "/tmp/output", access: "write" }, + { path: "/ignored", access: "none" }, + ], + }, + }, + }, + paramsForRun: params, + threadId: "thread-1", + turnId: "turn-1", + }); + + expect(result).toEqual({ + permissions: { + network: { enabled: true }, + fileSystem: { + read: ["/Users/simone/.ssh/id_rsa"], + write: ["/"], + entries: [ + { path: "/workspace/project", access: "read" }, + { path: "/tmp/output", access: "write" }, + { path: "/ignored", access: "none" }, + ], + }, + }, + scope: "turn", + }); + const [, , requestPayload] = mockCallGatewayTool.mock.calls[0] ?? []; + const description = (requestPayload as { description: string }).description; + expect(description).toContain("Network enabled: true"); + expect(description).toContain("File system read: ~/.ssh/id_rsa; write: /"); + expect(description).toContain("entries: read /workspace/project, write /tmp/output (+1 more)"); + expect(description).toContain("High-risk targets: network access, filesystem root"); + }); + it("compacts Windows home paths in permission descriptions", async () => { const params = createParams(); mockCallGatewayTool diff --git a/extensions/codex/src/app-server/approval-bridge.ts b/extensions/codex/src/app-server/approval-bridge.ts index 7f9b278c10b..7c8fd2f4e9c 100644 --- a/extensions/codex/src/app-server/approval-bridge.ts +++ b/extensions/codex/src/app-server/approval-bridge.ts @@ -3,6 +3,7 @@ import { formatApprovalDisplayPath, type EmbeddedRunAttemptParams, } from "openclaw/plugin-sdk/agent-harness-runtime"; +import { formatCodexDisplayText } from "../command-formatters.js"; import { approvalRequestExplicitlyUnavailable, mapExecDecisionToOutcome, @@ -15,6 +16,7 @@ import { isJsonObject, type JsonObject, type JsonValue } from "./protocol.js"; const PERMISSION_DESCRIPTION_MAX_LENGTH = 700; const PERMISSION_SAMPLE_LIMIT = 2; const PERMISSION_VALUE_MAX_LENGTH = 48; +const COMMAND_PREVIEW_WITH_DETAILS_MAX_LENGTH = 80; const APPROVAL_PREVIEW_SCAN_MAX_LENGTH = 4096; const APPROVAL_PREVIEW_OMITTED = "[preview truncated or unsafe content omitted]"; const ANSI_OSC_SEQUENCE_RE = new RegExp( @@ -136,7 +138,9 @@ export async function handleCodexAppServerApprovalRequest(params: { ...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)}`, + : `Codex app-server approval route failed: ${formatCodexDisplayText( + formatErrorMessage(error), + )}`, }); return buildApprovalResponse( params.method, @@ -192,9 +196,13 @@ function buildApprovalContext(params: { readString(params.requestParams, "itemId") ?? readString(params.requestParams, "callId") ?? readString(params.requestParams, "approvalId"); + const commandDetailLines = + params.method === "item/commandExecution/requestApproval" + ? describeCommandApprovalDetails(params.requestParams) + : []; const commandPreview = sanitizeApprovalPreview( readDisplayCommandPreview(params.requestParams), - 180, + commandDetailLines.length > 0 ? COMMAND_PREVIEW_WITH_DETAILS_MAX_LENGTH : 180, ); const reasonPreview = sanitizeApprovalPreview( readStringPreview(params.requestParams, "reason"), @@ -229,7 +237,11 @@ function buildApprovalContext(params: { const description = permissionLines.length > 0 ? joinDescriptionLinesWithinLimit(permissionLines, PERMISSION_DESCRIPTION_MAX_LENGTH) - : [subject, params.paramsForRun.sessionKey && `Session: ${params.paramsForRun.sessionKey}`] + : [ + subject, + ...commandDetailLines, + params.paramsForRun.sessionKey && `Session: ${params.paramsForRun.sessionKey}`, + ] .filter(Boolean) .join("\n"); return { @@ -310,6 +322,35 @@ function unsupportedApprovalResponse(): JsonValue { function describeRequestedPermissions(requestParams: JsonObject | undefined): string[] { const permissions = requestedPermissions(requestParams); + return describePermissionProfile(permissions, "Permissions"); +} + +function describeCommandApprovalDetails(requestParams: JsonObject | undefined): string[] { + const lines: string[] = []; + const additionalPermissions = isJsonObject(requestParams?.additionalPermissions) + ? requestParams.additionalPermissions + : undefined; + if (additionalPermissions) { + lines.push(...describePermissionProfile(additionalPermissions, "Additional permissions")); + } + const execpolicySummary = summarizeStringArray( + requestParams?.proposedExecpolicyAmendment, + "Proposed exec policy", + sanitizePermissionScalar, + ); + if (execpolicySummary) { + lines.push(execpolicySummary); + } + const networkAmendmentSummary = summarizeNetworkPolicyAmendments( + requestParams?.proposedNetworkPolicyAmendments, + ); + if (networkAmendmentSummary) { + lines.push(networkAmendmentSummary); + } + return lines; +} + +function describePermissionProfile(permissions: JsonObject, label: string): string[] { const lines: string[] = []; const kinds: string[] = []; const risks = new Set(); @@ -320,41 +361,61 @@ function describeRequestedPermissions(requestParams: JsonObject | undefined): st kinds.push("fileSystem"); } if (kinds.length > 0) { - lines.push(`Permissions: ${kinds.join(", ")}`); + lines.push(`${label}: ${kinds.join(", ")}`); } let networkSummary: string | undefined; if (isJsonObject(permissions.network)) { - networkSummary = summarizePermissionRecord(permissions.network, risks, [ - { - key: "allowHosts", - label: "allowHosts", - sanitize: sanitizePermissionHostValue, - risksFor: permissionHostRisks, - }, - ]); + const summaries = [ + summarizeNetworkEnabledPermission(permissions.network, risks), + summarizePermissionRecord(permissions.network, risks, [ + { + key: "allowHosts", + label: "allowHosts", + sanitize: sanitizePermissionHostValue, + risksFor: permissionHostRisks, + }, + ]), + ].filter((summary): summary is string => Boolean(summary)); + networkSummary = summaries.length > 0 ? summaries.join("; ") : undefined; } let fileSystemSummary: string | undefined; if (isJsonObject(permissions.fileSystem)) { - fileSystemSummary = summarizePermissionRecord(permissions.fileSystem, risks, [ - { - key: "roots", - label: "roots", - sanitize: sanitizePermissionPathValue, - risksFor: permissionPathRisks, - }, - { - key: "readPaths", - label: "readPaths", - sanitize: sanitizePermissionPathValue, - risksFor: permissionPathRisks, - }, - { - key: "writePaths", - label: "writePaths", - sanitize: sanitizePermissionPathValue, - risksFor: permissionPathRisks, - }, - ]); + const summaries = [ + summarizePermissionRecord(permissions.fileSystem, risks, [ + { + key: "read", + label: "read", + sanitize: sanitizePermissionPathValue, + risksFor: permissionPathRisks, + }, + { + key: "write", + label: "write", + sanitize: sanitizePermissionPathValue, + risksFor: permissionPathRisks, + }, + { + key: "roots", + label: "roots", + sanitize: sanitizePermissionPathValue, + risksFor: permissionPathRisks, + }, + { + key: "readPaths", + label: "readPaths", + sanitize: sanitizePermissionPathValue, + risksFor: permissionPathRisks, + }, + { + key: "writePaths", + label: "writePaths", + sanitize: sanitizePermissionPathValue, + risksFor: permissionPathRisks, + }, + ]), + summarizeFileSystemEntries(permissions.fileSystem, risks), + ].filter((summary): summary is string => Boolean(summary)); + fileSystemSummary = summaries.length > 0 ? summaries.join("; ") : undefined; } if (risks.size > 0) { lines.push(`High-risk targets: ${[...risks].join(", ")}`); @@ -375,6 +436,55 @@ type PermissionArrayDescriptor = { risksFor: (value: string) => readonly string[]; }; +function summarizeNetworkEnabledPermission( + permission: JsonObject, + risks: Set, +): string | undefined { + const enabled = permission.enabled; + if (typeof enabled !== "boolean") { + return undefined; + } + if (enabled) { + risks.add("network access"); + } + return `enabled: ${enabled}`; +} + +function summarizeFileSystemEntries( + permission: JsonObject, + risks: Set, +): string | undefined { + const entries = permission.entries; + if (!Array.isArray(entries)) { + return undefined; + } + const samples: string[] = []; + let count = 0; + for (const entry of entries) { + const item = isJsonObject(entry) ? entry : undefined; + const path = typeof item?.path === "string" ? item.path.trim() : ""; + const access = typeof item?.access === "string" ? item.access.trim() : ""; + if (!path || !access) { + continue; + } + count += 1; + if (access !== "none") { + for (const risk of permissionPathRisks(path)) { + risks.add(risk); + } + } + if (samples.length < PERMISSION_SAMPLE_LIMIT) { + samples.push(`${sanitizePermissionScalar(access)} ${sanitizePermissionPathValue(path)}`); + } + } + if (count === 0) { + return undefined; + } + const remaining = count - samples.length; + const remainderSuffix = remaining > 0 ? ` (+${remaining} more)` : ""; + return `entries: ${samples.join(", ")}${remainderSuffix}`; +} + function summarizePermissionRecord( permission: JsonObject, risks: Set, @@ -416,6 +526,53 @@ function summarizePermissionArray( return `${descriptor.label}: ${sampleValues.join(", ")}${remainderSuffix}`; } +function summarizeStringArray( + value: JsonValue | undefined, + label: string, + sanitize: (value: string) => string, +): string | undefined { + if (!Array.isArray(value)) { + return undefined; + } + const values = value + .filter((entry): entry is string => typeof entry === "string") + .map((entry) => sanitize(entry)) + .filter(Boolean); + if (values.length === 0) { + return undefined; + } + const samples = values.slice(0, PERMISSION_SAMPLE_LIMIT); + const remaining = values.length - samples.length; + const remainderSuffix = remaining > 0 ? ` (+${remaining} more)` : ""; + return `${label}: ${samples.join(", ")}${remainderSuffix}`; +} + +function summarizeNetworkPolicyAmendments(value: JsonValue | undefined): string | undefined { + if (!Array.isArray(value)) { + return undefined; + } + const samples: string[] = []; + let count = 0; + for (const entry of value) { + const amendment = isJsonObject(entry) ? entry : undefined; + const host = typeof amendment?.host === "string" ? amendment.host : ""; + const action = typeof amendment?.action === "string" ? amendment.action : ""; + if (!host || !action) { + continue; + } + count += 1; + if (samples.length < PERMISSION_SAMPLE_LIMIT) { + samples.push(`${sanitizePermissionScalar(action)} ${sanitizePermissionHostValue(host)}`); + } + } + if (count === 0) { + return undefined; + } + const remaining = count - samples.length; + const remainderSuffix = remaining > 0 ? ` (+${remaining} more)` : ""; + return `Proposed network policy: ${samples.join(", ")}${remainderSuffix}`; +} + function readStringArray(record: JsonObject, key: string): string[] { const value = record[key]; return Array.isArray(value) @@ -693,7 +850,7 @@ function sanitizeApprovalPreview( if (!sanitized) { return { omitted: true }; } - return { text: truncate(sanitized, maxLength), omitted: source.clipped }; + return { text: formatCodexDisplayText(truncate(sanitized, maxLength)), omitted: source.clipped }; } function sanitizeVisibleScalar(value: string): string { diff --git a/extensions/codex/src/app-server/dynamic-tools.test.ts b/extensions/codex/src/app-server/dynamic-tools.test.ts index 14460326d06..9176c7f864f 100644 --- a/extensions/codex/src/app-server/dynamic-tools.test.ts +++ b/extensions/codex/src/app-server/dynamic-tools.test.ts @@ -314,7 +314,7 @@ describe("createCodexDynamicToolBridge", () => { details: { status: "failed", exitCode: 1 }, }); - await bridge.handleToolCall({ + const result = await bridge.handleToolCall({ threadId: "thread-1", turnId: "turn-1", callId: "call-1", @@ -323,6 +323,10 @@ describe("createCodexDynamicToolBridge", () => { arguments: { command: "false" }, }); + expect(result).toEqual({ + success: false, + contentItems: [{ type: "inputText", text: "failed output" }], + }); expect(handler).toHaveBeenCalledWith( expect.objectContaining({ isError: true }), expect.objectContaining({ runtime: "codex" }), @@ -641,7 +645,7 @@ describe("createCodexDynamicToolBridge", () => { }); expect(result).toEqual({ - success: true, + success: false, contentItems: [{ type: "inputText", text: "blocked by policy" }], }); expect(execute).not.toHaveBeenCalled(); diff --git a/extensions/codex/src/app-server/dynamic-tools.ts b/extensions/codex/src/app-server/dynamic-tools.ts index 285fe2979e2..dfb02bd5e2d 100644 --- a/extensions/codex/src/app-server/dynamic-tools.ts +++ b/extensions/codex/src/app-server/dynamic-tools.ts @@ -119,13 +119,14 @@ export function createCodexDynamicToolBridge(params: { args, result: middlewareResult, }); + const resultIsError = rawIsError || isToolResultError(result); collectToolTelemetry({ toolName: tool.name, args, result, mediaTrustResult: rawResult, telemetry, - isError: rawIsError || isToolResultError(result), + isError: resultIsError, }); void runAgentHarnessAfterToolCallHook({ toolName: tool.name, @@ -140,7 +141,7 @@ export function createCodexDynamicToolBridge(params: { }); return { contentItems: result.content.flatMap(convertToolContent), - success: true, + success: !resultIsError, }; } catch (error) { collectToolTelemetry({ diff --git a/extensions/codex/src/app-server/elicitation-bridge.test.ts b/extensions/codex/src/app-server/elicitation-bridge.test.ts index d21f581d8e2..1139f2514b1 100644 --- a/extensions/codex/src/app-server/elicitation-bridge.test.ts +++ b/extensions/codex/src/app-server/elicitation-bridge.test.ts @@ -243,6 +243,67 @@ describe("Codex app-server elicitation bridge", () => { expect(approvalRequest.description).not.toContain("\u202e"); }); + it("escapes approval display text before forwarding approval prompts", async () => { + mockCallGatewayTool + .mockResolvedValueOnce({ id: "plugin:approval-escaped", status: "accepted" }) + .mockResolvedValueOnce({ id: "plugin:approval-escaped", decision: "allow-once" }); + + await handleCodexAppServerElicitationRequest({ + requestParams: { + ...buildCurrentCodexApprovalElicitation(), + message: "Approve <@U123>", + serverName: "server @here", + _meta: { + codex_approval_kind: "mcp_tool_call", + connector_name: "GitHub [trusted](https://evil)", + tool_title: "Create <@U123>", + tool_description: "Use @here", + tool_params_display: [ + { + name: "repo", + display_name: "Repository [trusted](https://evil)", + value: "<@U123>", + }, + ], + }, + requestedSchema: { + type: "object", + properties: { + approve: { + type: "boolean", + title: "Approve <@U123>", + description: "Confirm @here", + }, + }, + required: ["approve"], + }, + }, + paramsForRun: createParams(), + threadId: "thread-1", + turnId: "turn-1", + }); + + const approvalRequest = mockCallGatewayTool.mock.calls[0]?.[2] as { + title: string; + description: string; + }; + expect(approvalRequest.title).toBe("Approve <\uff20U123>"); + expect(approvalRequest.description).toContain( + "GitHub \uff3btrusted\uff3d\uff08https://evil\uff09", + ); + expect(approvalRequest.description).toContain("Tool: Create <\uff20U123>"); + expect(approvalRequest.description).toContain("MCP server: server \uff20here"); + expect(approvalRequest.description).toContain( + "Repository \uff3btrusted\uff3d\uff08https://evil\uff09: <\uff20U123>", + ); + expect(approvalRequest.description).toContain( + "- Approve <\uff20U123>: Confirm \uff20here", + ); + expect(approvalRequest.description).not.toContain("<@U123>"); + expect(approvalRequest.description).not.toContain("[trusted](https://evil)"); + expect(approvalRequest.description).not.toContain("@here"); + }); + it("falls back to stable names when display labels sanitize to empty", async () => { mockCallGatewayTool .mockResolvedValueOnce({ id: "plugin:approval-label-fallback", status: "accepted" }) diff --git a/extensions/codex/src/app-server/elicitation-bridge.ts b/extensions/codex/src/app-server/elicitation-bridge.ts index 017c3d7c4d9..a91aa8a4305 100644 --- a/extensions/codex/src/app-server/elicitation-bridge.ts +++ b/extensions/codex/src/app-server/elicitation-bridge.ts @@ -2,6 +2,7 @@ import { embeddedAgentLog, type EmbeddedRunAttemptParams, } from "openclaw/plugin-sdk/agent-harness-runtime"; +import { formatCodexDisplayText } from "../command-formatters.js"; import { approvalRequestExplicitlyUnavailable, mapExecDecisionToOutcome, @@ -283,7 +284,8 @@ function sanitizeDisplayText(value: string): string { .replace(CONTROL_CHARACTER_RE, " ") .replace(/\s+/g, " ") .trim(); - return clipped ? `${sanitized}...` : sanitized; + const escaped = sanitized ? formatCodexDisplayText(sanitized) : ""; + return clipped && escaped ? `${escaped}...` : escaped; } function truncateDisplayText(value: string, maxLength: number): string { diff --git a/extensions/codex/src/app-server/openclaw-owned-tool-runtime-contract.test.ts b/extensions/codex/src/app-server/openclaw-owned-tool-runtime-contract.test.ts index ad9bcbc6379..1961573d915 100644 --- a/extensions/codex/src/app-server/openclaw-owned-tool-runtime-contract.test.ts +++ b/extensions/codex/src/app-server/openclaw-owned-tool-runtime-contract.test.ts @@ -188,7 +188,7 @@ describe("OpenClaw-owned tool runtime contract — Codex app-server adapter", () }); expect(result).toEqual({ - success: true, + success: false, contentItems: [{ type: "inputText", text: "blocked by policy" }], }); expect(execute).not.toHaveBeenCalled(); diff --git a/extensions/codex/src/app-server/user-input-bridge.test.ts b/extensions/codex/src/app-server/user-input-bridge.test.ts index ec39264cbba..7ef88c1cc93 100644 --- a/extensions/codex/src/app-server/user-input-bridge.test.ts +++ b/extensions/codex/src/app-server/user-input-bridge.test.ts @@ -98,6 +98,87 @@ describe("Codex app-server user input bridge", () => { }); }); + it("rejects free-form option replies when Other is disabled", async () => { + const params = createParams(); + const bridge = createCodexUserInputBridge({ + paramsForRun: params, + threadId: "thread-1", + turnId: "turn-1", + }); + + const response = bridge.handleRequest({ + id: "input-options", + params: { + threadId: "thread-1", + turnId: "turn-1", + itemId: "tool-1", + questions: [ + { + id: "mode", + header: "Mode", + question: "Pick a mode", + isOther: false, + isSecret: false, + options: [{ label: "Fast", description: "Use less reasoning" }], + }, + ], + }, + }); + + await vi.waitFor(() => expect(params.onBlockReply).toHaveBeenCalledTimes(1)); + expect(bridge.handleQueuedMessage("banana")).toBe(true); + + await expect(response).resolves.toEqual({ + answers: { mode: { answers: [] } }, + }); + }); + + it("escapes prompt question and option text before chat display", async () => { + const params = createParams(); + const bridge = createCodexUserInputBridge({ + paramsForRun: params, + threadId: "thread-1", + turnId: "turn-1", + }); + + const response = bridge.handleRequest({ + id: "input-escaped", + params: { + threadId: "thread-1", + turnId: "turn-1", + itemId: "tool-1", + questions: [ + { + id: "mode", + header: "Mode <@U123>", + question: "Pick [trusted](https://evil) @here", + isOther: false, + isSecret: false, + options: [{ label: "Fast <@U123>", description: "Use [less](https://evil)" }], + }, + ], + }, + }); + + await vi.waitFor(() => expect(params.onBlockReply).toHaveBeenCalledTimes(1)); + const payload = vi.mocked(params.onBlockReply!).mock.calls[0]?.[0]; + expect(payload).toEqual(expect.objectContaining({ text: expect.any(String) })); + const text = payload?.text ?? ""; + expect(text).toContain("Mode <\uff20U123>"); + expect(text).toContain("Pick \uff3btrusted\uff3d\uff08https://evil\uff09 \uff20here"); + expect(text).toContain( + "Fast <\uff20U123> - Use \uff3bless\uff3d\uff08https://evil\uff09", + ); + expect(text).not.toContain("<@U123>"); + expect(text).not.toContain("[trusted](https://evil)"); + expect(text).not.toContain("@here"); + + expect(bridge.handleQueuedMessage("1")).toBe(true); + await expect(response).resolves.toEqual({ + answers: { mode: { answers: ["Fast <@U123>"] } }, + }); + }); + it("clears pending prompts when Codex resolves the server request itself", async () => { const params = createParams(); const bridge = createCodexUserInputBridge({ @@ -134,4 +215,27 @@ describe("Codex app-server user input bridge", () => { await expect(response).resolves.toEqual({ answers: {} }); expect(bridge.handleQueuedMessage("too late")).toBe(false); }); + + it("resolves malformed empty question prompts without waiting for chat input", async () => { + const params = createParams(); + const bridge = createCodexUserInputBridge({ + paramsForRun: params, + threadId: "thread-1", + turnId: "turn-1", + }); + + await expect( + bridge.handleRequest({ + id: "input-empty", + params: { + threadId: "thread-1", + turnId: "turn-1", + itemId: "tool-1", + questions: [], + }, + }), + ).resolves.toEqual({ answers: {} }); + expect(params.onBlockReply).not.toHaveBeenCalled(); + expect(bridge.handleQueuedMessage("late answer")).toBe(false); + }); }); diff --git a/extensions/codex/src/app-server/user-input-bridge.ts b/extensions/codex/src/app-server/user-input-bridge.ts index 360c46651ee..880308ab6ba 100644 --- a/extensions/codex/src/app-server/user-input-bridge.ts +++ b/extensions/codex/src/app-server/user-input-bridge.ts @@ -2,6 +2,7 @@ import { embeddedAgentLog, type EmbeddedRunAttemptParams, } from "openclaw/plugin-sdk/agent-harness-runtime"; +import { formatCodexDisplayText } from "../command-formatters.js"; import { isJsonObject, type CodexServerNotification, @@ -70,6 +71,9 @@ export function createCodexUserInputBridge(params: { if (requestParams.threadId !== params.threadId || requestParams.turnId !== params.turnId) { return undefined; } + if (requestParams.questions.length === 0) { + return emptyUserInputResponse(); + } resolvePending(emptyUserInputResponse()); @@ -205,16 +209,26 @@ function formatUserInputPrompt(questions: UserInputQuestion[]): string { const lines = ["Codex needs input:"]; questions.forEach((question, index) => { if (questions.length > 1) { - lines.push("", `${index + 1}. ${question.header}`, question.question); + lines.push( + "", + `${index + 1}. ${formatCodexDisplayText(question.header)}`, + formatCodexDisplayText(question.question), + ); } else { - lines.push("", question.header, question.question); + lines.push( + "", + formatCodexDisplayText(question.header), + formatCodexDisplayText(question.question), + ); } if (question.isSecret) { lines.push("This channel may show your reply to other participants."); } question.options?.forEach((option, optionIndex) => { lines.push( - `${optionIndex + 1}. ${option.label}${option.description ? ` - ${option.description}` : ""}`, + `${optionIndex + 1}. ${formatCodexDisplayText(option.label)}${ + option.description ? ` - ${formatCodexDisplayText(option.description)}` : "" + }`, ); }); if (question.isOther) { @@ -229,7 +243,8 @@ function buildUserInputResponse(questions: UserInputQuestion[], inputText: strin if (questions.length === 1) { const question = questions[0]; if (question) { - answers[question.id] = { answers: [normalizeAnswer(inputText, question)] }; + const answer = normalizeAnswer(inputText, question); + answers[question.id] = { answers: answer ? [answer] : [] }; } return { answers }; } @@ -246,12 +261,13 @@ function buildUserInputResponse(questions: UserInputQuestion[], inputText: strin keyed.get(question.question.toLowerCase()) ?? keyed.get(String(index + 1)); const answer = key ?? fallbackLines[index] ?? ""; - answers[question.id] = { answers: answer ? [normalizeAnswer(answer, question)] : [] }; + const normalized = answer ? normalizeAnswer(answer, question) : undefined; + answers[question.id] = { answers: normalized ? [normalized] : [] }; }); return { answers }; } -function normalizeAnswer(answer: string, question: UserInputQuestion): string { +function normalizeAnswer(answer: string, question: UserInputQuestion): string | undefined { const trimmed = answer.trim(); const options = question.options ?? []; const optionIndex = /^\d+$/.test(trimmed) ? Number(trimmed) - 1 : -1; @@ -260,7 +276,13 @@ function normalizeAnswer(answer: string, question: UserInputQuestion): string { return indexed.label; } const exact = options.find((option) => option.label.toLowerCase() === trimmed.toLowerCase()); - return exact?.label ?? trimmed; + if (exact) { + return exact.label; + } + if (options.length > 0 && !question.isOther) { + return undefined; + } + return trimmed || undefined; } function parseKeyedAnswers(inputText: string): Map { diff --git a/extensions/codex/src/command-formatters.ts b/extensions/codex/src/command-formatters.ts index a6f935dd429..bf8855e5598 100644 --- a/extensions/codex/src/command-formatters.ts +++ b/extensions/codex/src/command-formatters.ts @@ -19,25 +19,41 @@ export function formatCodexStatus(probes: CodexStatusProbes): string { lines.push( `Models: ${ probes.models.value.models - .map((model) => model.id) + .map((model) => formatCodexDisplayText(model.id)) .slice(0, 8) .join(", ") || "none" }`, ); } else { - lines.push(`Models: ${probes.models.error}`); + lines.push(`Models: ${formatCodexDisplayText(probes.models.error)}`); } lines.push( - `Account: ${probes.account.ok ? summarizeAccount(probes.account.value) : probes.account.error}`, + `Account: ${ + probes.account.ok + ? formatCodexAccountSummary(probes.account.value) + : formatCodexDisplayText(probes.account.error) + }`, ); lines.push( - `Rate limits: ${probes.limits.ok ? summarizeArrayLike(probes.limits.value) : probes.limits.error}`, + `Rate limits: ${ + probes.limits.ok + ? summarizeRateLimits(probes.limits.value) + : formatCodexDisplayText(probes.limits.error) + }`, ); lines.push( - `MCP servers: ${probes.mcps.ok ? summarizeArrayLike(probes.mcps.value) : probes.mcps.error}`, + `MCP servers: ${ + probes.mcps.ok + ? summarizeArrayLike(probes.mcps.value) + : formatCodexDisplayText(probes.mcps.error) + }`, ); lines.push( - `Skills: ${probes.skills.ok ? summarizeArrayLike(probes.skills.value) : probes.skills.error}`, + `Skills: ${ + probes.skills.ok + ? summarizeArrayLike(probes.skills.value) + : formatCodexDisplayText(probes.skills.error) + }`, ); return lines.join("\n"); } @@ -48,7 +64,9 @@ export function formatModels(result: CodexAppServerModelListResult): string { } const lines = [ "Codex models:", - ...result.models.map((model) => `- ${model.id}${model.isDefault ? " (default)" : ""}`), + ...result.models.map( + (model) => `- ${formatCodexDisplayText(model.id)}${model.isDefault ? " (default)" : ""}`, + ), ]; if (result.truncated) { lines.push("- More models available; output truncated."); @@ -72,10 +90,10 @@ export function formatThreads(response: JsonValue | undefined): string { readString(record, "model"), readString(record, "cwd"), readString(record, "updatedAt") ?? readString(record, "lastUpdatedAt"), - ].filter(Boolean); - return `- ${id}${title ? ` - ${title}` : ""}${ - details.length > 0 ? ` (${details.join(", ")})` : "" - }\n Resume: /codex resume ${id}`; + ].filter((value): value is string => Boolean(value)); + return `- ${formatCodexDisplayText(id)}${title ? ` - ${formatCodexDisplayText(title)}` : ""}${ + details.length > 0 ? ` (${details.map(formatCodexDisplayText).join(", ")})` : "" + }\n Resume: ${formatCodexResumeHint(id)}`; }), ].join("\n"); } @@ -85,8 +103,8 @@ export function formatAccount( limits: SafeValue, ): string { return [ - `Account: ${account.ok ? summarizeAccount(account.value) : account.error}`, - `Rate limits: ${limits.ok ? summarizeArrayLike(limits.value) : limits.error}`, + `Account: ${account.ok ? formatCodexAccountSummary(account.value) : formatCodexDisplayText(account.error)}`, + `Rate limits: ${limits.ok ? summarizeRateLimits(limits.value) : formatCodexDisplayText(limits.error)}`, ].join("\n"); } @@ -94,19 +112,21 @@ export function formatComputerUseStatus(status: CodexComputerUseStatus): string const lines = [ `Computer Use: ${status.ready ? "ready" : status.enabled ? "not ready" : "disabled"}`, ]; - lines.push(`Plugin: ${status.pluginName} (${computerUsePluginState(status)})`); lines.push( - `MCP server: ${status.mcpServerName}${ + `Plugin: ${formatCodexDisplayText(status.pluginName)} (${computerUsePluginState(status)})`, + ); + lines.push( + `MCP server: ${formatCodexDisplayText(status.mcpServerName)}${ status.mcpServerAvailable ? ` (${status.tools.length} tools)` : " (unavailable)" }`, ); if (status.marketplaceName) { - lines.push(`Marketplace: ${status.marketplaceName}`); + lines.push(`Marketplace: ${formatCodexDisplayText(status.marketplaceName)}`); } if (status.tools.length > 0) { - lines.push(`Tools: ${status.tools.slice(0, 8).join(", ")}`); + lines.push(`Tools: ${status.tools.slice(0, 8).map(formatCodexDisplayText).join(", ")}`); } - lines.push(status.message); + lines.push(formatCodexDisplayText(status.message)); return lines.join("\n"); } @@ -126,11 +146,85 @@ export function formatList(response: JsonValue | undefined, label: string): stri `${label}:`, ...entries.slice(0, 25).map((entry) => { const record = isJsonObject(entry) ? entry : {}; - return `- ${readString(record, "name") ?? readString(record, "id") ?? JSON.stringify(entry)}`; + return `- ${formatCodexDisplayText( + readString(record, "name") ?? readString(record, "id") ?? JSON.stringify(entry), + )}`; }), ].join("\n"); } +const CODEX_RESUME_SAFE_THREAD_ID_PATTERN = /^[A-Za-z0-9._:-]+$/; + +function formatCodexResumeHint(threadId: string): string { + const safe = formatCodexTextForDisplay(threadId); + if (!CODEX_RESUME_SAFE_THREAD_ID_PATTERN.test(safe)) { + return "copy the thread id above and run /codex resume "; + } + return `/codex resume ${safe}`; +} + +export function formatCodexDisplayText(value: string): string { + return escapeCodexChatText(formatCodexTextForDisplay(value)); +} + +function formatCodexAccountSummary(value: JsonValue | undefined): string { + const safe = formatCodexTextForDisplay(summarizeAccount(value)); + return isLikelyEmailAddress(safe) + ? escapeCodexChatTextPreservingAt(safe) + : escapeCodexChatText(safe); +} + +function formatCodexTextForDisplay(value: string): string { + let safe = ""; + for (const character of value) { + const codePoint = character.codePointAt(0); + safe += codePoint != null && isUnsafeDisplayCodePoint(codePoint) ? "?" : character; + } + safe = safe.trim(); + return safe || ""; +} + +function escapeCodexChatText(value: string): string { + return value + .replaceAll("&", "&") + .replaceAll("<", "<") + .replaceAll(">", ">") + .replaceAll("@", "\uff20") + .replaceAll("`", "\uff40") + .replaceAll("[", "\uff3b") + .replaceAll("]", "\uff3d") + .replaceAll("(", "\uff08") + .replaceAll(")", "\uff09") + .replaceAll("*", "\u2217") + .replaceAll("_", "\uff3f") + .replaceAll("~", "\uff5e") + .replaceAll("|", "\uff5c"); +} + +function escapeCodexChatTextPreservingAt(value: string): string { + return escapeCodexChatText(value).replaceAll("\uff20", "@"); +} + +function isLikelyEmailAddress(value: string): boolean { + return /^[^\s@<>()[\]`]+@[^\s@<>()[\]`]+\.[^\s@<>()[\]`]+$/.test(value); +} + +function isUnsafeDisplayCodePoint(codePoint: number): boolean { + return ( + codePoint <= 0x001f || + (codePoint >= 0x007f && codePoint <= 0x009f) || + codePoint === 0x00ad || + codePoint === 0x061c || + codePoint === 0x180e || + (codePoint >= 0x200b && codePoint <= 0x200f) || + (codePoint >= 0x202a && codePoint <= 0x202e) || + (codePoint >= 0x2060 && codePoint <= 0x206f) || + codePoint === 0xfeff || + (codePoint >= 0xfff9 && codePoint <= 0xfffb) || + (codePoint >= 0xe0000 && codePoint <= 0xe007f) + ); +} + export function buildHelp(): string { return [ "Codex commands:", @@ -182,6 +276,28 @@ function summarizeArrayLike(value: JsonValue | undefined): string { return `${entries.length}`; } +function summarizeRateLimits(value: JsonValue | undefined): string { + const entries = extractArray(value); + if (entries.length > 0) { + return `${entries.length}`; + } + if (!isJsonObject(value)) { + return "none returned"; + } + const keyed = value.rateLimitsByLimitId; + if (isJsonObject(keyed)) { + const count = Object.values(keyed).filter(isMeaningfulRateLimitSnapshot).length; + if (count > 0) { + return `${count}`; + } + } + return isMeaningfulRateLimitSnapshot(value.rateLimits) ? "1" : "none returned"; +} + +function isMeaningfulRateLimitSnapshot(value: JsonValue | undefined): boolean { + return isJsonObject(value) && Object.values(value).some((entry) => entry != null); +} + function extractArray(value: JsonValue | undefined): JsonValue[] { if (Array.isArray(value)) { return value; diff --git a/extensions/codex/src/command-handlers.ts b/extensions/codex/src/command-handlers.ts index 761b6ac1813..b6d141859c2 100644 --- a/extensions/codex/src/command-handlers.ts +++ b/extensions/codex/src/command-handlers.ts @@ -18,6 +18,7 @@ import { buildHelp, formatAccount, formatComputerUseStatus, + formatCodexDisplayText, formatCodexStatus, formatList, formatModels, @@ -120,7 +121,8 @@ type ParsedComputerUseArgs = { type ParsedDiagnosticsArgs = | { action: "request"; note: string } | { action: "confirm"; token: string } - | { action: "cancel"; token: string }; + | { action: "cancel"; token: string } + | { action: "usage" }; type CodexDiagnosticsTarget = { threadId: string; @@ -185,11 +187,17 @@ export async function handleCodexSubcommand( return { text: buildHelp() }; } if (normalized === "status") { + if (rest.length > 0) { + return { text: "Usage: /codex status" }; + } return { text: formatCodexStatus(await deps.readCodexStatusProbes(options.pluginConfig, ctx.config)), }; } if (normalized === "models") { + if (rest.length > 0) { + return { text: "Usage: /codex models" }; + } return { text: formatModels( await deps.listCodexAppServerModels( @@ -202,31 +210,40 @@ export async function handleCodexSubcommand( return { text: await buildThreads(deps, options.pluginConfig, rest.join(" ")) }; } if (normalized === "resume") { - return { text: await resumeThread(deps, ctx, options.pluginConfig, rest[0]) }; + return { text: await resumeThread(deps, ctx, options.pluginConfig, rest) }; } if (normalized === "bind") { return await bindConversation(deps, ctx, options.pluginConfig, rest); } if (normalized === "detach" || normalized === "unbind") { + if (rest.length > 0) { + return { text: "Usage: /codex detach" }; + } return { text: await detachConversation(deps, ctx) }; } if (normalized === "binding") { + if (rest.length > 0) { + return { text: "Usage: /codex binding" }; + } return { text: await describeConversationBinding(deps, ctx) }; } if (normalized === "stop") { + if (rest.length > 0) { + return { text: "Usage: /codex stop" }; + } return { text: await stopConversationTurn(deps, ctx, options.pluginConfig) }; } if (normalized === "steer") { return { text: await steerConversationTurn(deps, ctx, options.pluginConfig, rest.join(" ")) }; } if (normalized === "model") { - return { text: await setConversationModel(deps, ctx, options.pluginConfig, rest.join(" ")) }; + return { text: await setConversationModel(deps, ctx, options.pluginConfig, rest) }; } if (normalized === "fast") { - return { text: await setConversationFastMode(deps, ctx, options.pluginConfig, rest[0]) }; + return { text: await setConversationFastMode(deps, ctx, options.pluginConfig, rest) }; } if (normalized === "permissions") { - return { text: await setConversationPermissions(deps, ctx, options.pluginConfig, rest[0]) }; + return { text: await setConversationPermissions(deps, ctx, options.pluginConfig, rest) }; } if (normalized === "compact") { return { @@ -236,6 +253,7 @@ export async function handleCodexSubcommand( options.pluginConfig, CODEX_CONTROL_METHODS.compact, "compaction", + rest, ), }; } @@ -247,6 +265,7 @@ export async function handleCodexSubcommand( options.pluginConfig, CODEX_CONTROL_METHODS.review, "review", + rest, ), }; } @@ -265,6 +284,9 @@ export async function handleCodexSubcommand( }; } if (normalized === "mcp") { + if (rest.length > 0) { + return { text: "Usage: /codex mcp" }; + } return { text: formatList( await deps.codexControlRequest(options.pluginConfig, CODEX_CONTROL_METHODS.listMcpServers, { @@ -275,6 +297,9 @@ export async function handleCodexSubcommand( }; } if (normalized === "skills") { + if (rest.length > 0) { + return { text: "Usage: /codex skills" }; + } return { text: formatList( await deps.codexControlRequest(options.pluginConfig, CODEX_CONTROL_METHODS.listSkills, {}), @@ -283,6 +308,9 @@ export async function handleCodexSubcommand( }; } if (normalized === "account") { + if (rest.length > 0) { + return { text: "Usage: /codex account" }; + } const [account, limits] = await Promise.all([ deps.safeCodexControlRequest(options.pluginConfig, CODEX_CONTROL_METHODS.account, { refreshToken: false, @@ -295,7 +323,7 @@ export async function handleCodexSubcommand( ]); return { text: formatAccount(account, limits) }; } - return { text: `Unknown Codex command: ${subcommand}\n\n${buildHelp()}` }; + return { text: `Unknown Codex command: ${formatCodexDisplayText(subcommand)}\n\n${buildHelp()}` }; } async function handleComputerUseCommand( @@ -327,17 +355,17 @@ async function bindConversation( pluginConfig: unknown, args: string[], ): Promise { - if (!ctx.sessionFile) { - return { - text: "Cannot bind Codex because this command did not include an OpenClaw session file.", - }; - } const parsed = parseBindArgs(args); if (parsed.help) { return { text: "Usage: /codex bind [thread-id] [--cwd ] [--model ] [--provider ]", }; } + if (!ctx.sessionFile) { + return { + text: "Cannot bind Codex because this command did not include an OpenClaw session file.", + }; + } const workspaceDir = parsed.cwd ?? deps.resolveCodexDefaultWorkspaceDir(pluginConfig); const existingBinding = await deps.readCodexAppServerBinding(ctx.sessionFile); const authProfileId = existingBinding?.authProfileId; @@ -356,7 +384,7 @@ async function bindConversation( const data = await deps.startCodexConversationThread(startParams); const binding = await deps.readCodexAppServerBinding(ctx.sessionFile); const threadId = binding?.threadId ?? parsed.threadId ?? "new thread"; - const summary = `Codex app-server thread ${threadId} in ${workspaceDir}`; + const summary = `Codex app-server thread ${formatCodexDisplayText(threadId)} in ${formatCodexDisplayText(workspaceDir)}`; let request: Awaited>; try { request = await ctx.requestConversationBinding({ @@ -369,13 +397,17 @@ async function bindConversation( throw error; } if (request.status === "bound") { - return { text: `Bound this conversation to Codex thread ${threadId} in ${workspaceDir}.` }; + return { + text: `Bound this conversation to Codex thread ${formatCodexDisplayText( + threadId, + )} in ${formatCodexDisplayText(workspaceDir)}.`, + }; } if (request.status === "pending") { return request.reply; } await deps.clearCodexAppServerBinding(ctx.sessionFile); - return { text: request.message }; + return { text: formatCodexDisplayText(request.message) }; } async function detachConversation( @@ -408,13 +440,13 @@ async function describeConversationBinding( const active = deps.readCodexConversationActiveTurn(data.sessionFile); return [ "Codex conversation binding:", - `- Thread: ${threadBinding?.threadId ?? "unknown"}`, - `- Workspace: ${data.workspaceDir}`, - `- Model: ${threadBinding?.model ?? "default"}`, + `- Thread: ${formatCodexDisplayText(threadBinding?.threadId ?? "unknown")}`, + `- Workspace: ${formatCodexDisplayText(data.workspaceDir)}`, + `- Model: ${formatCodexDisplayText(threadBinding?.model ?? "default")}`, `- Fast: ${threadBinding?.serviceTier === "fast" ? "on" : "off"}`, `- Permissions: ${threadBinding ? formatPermissionsMode(threadBinding) : "default"}`, - `- Active run: ${active ? active.turnId : "none"}`, - `- Session: ${data.sessionFile}`, + `- Active run: ${formatCodexDisplayText(active ? active.turnId : "none")}`, + `- Session: ${formatCodexDisplayText(data.sessionFile)}`, ].join("\n"); } @@ -434,10 +466,11 @@ async function resumeThread( deps: CodexCommandDeps, ctx: PluginCommandContext, pluginConfig: unknown, - threadId: string | undefined, + args: string[], ): Promise { + const [threadId] = args; const normalizedThreadId = threadId?.trim(); - if (!normalizedThreadId) { + if (!normalizedThreadId || args.length !== 1) { return "Usage: /codex resume "; } if (!ctx.sessionFile) { @@ -459,7 +492,9 @@ async function resumeThread( model: isJsonObject(response) ? readString(response, "model") : undefined, modelProvider: isJsonObject(response) ? readString(response, "modelProvider") : undefined, }); - return `Attached this OpenClaw session to Codex thread ${effectiveThreadId}.`; + return `Attached this OpenClaw session to Codex thread ${formatCodexDisplayText( + effectiveThreadId, + )}.`; } async function stopConversationTurn( @@ -497,16 +532,22 @@ async function setConversationModel( deps: CodexCommandDeps, ctx: PluginCommandContext, pluginConfig: unknown, - model: string, + args: string[], ): Promise { + if (args.length > 1) { + return "Usage: /codex model "; + } const sessionFile = await resolveControlSessionFile(ctx); if (!sessionFile) { return "Cannot set Codex model because this command did not include an OpenClaw session file."; } + const [model = ""] = args; const normalized = model.trim(); if (!normalized) { const binding = await deps.readCodexAppServerBinding(sessionFile); - return binding?.model ? `Codex model: ${binding.model}` : "Usage: /codex model "; + return binding?.model + ? `Codex model: ${formatCodexDisplayText(binding.model)}` + : "Usage: /codex model "; } return await deps.setCodexConversationModel({ sessionFile, @@ -519,12 +560,16 @@ async function setConversationFastMode( deps: CodexCommandDeps, ctx: PluginCommandContext, pluginConfig: unknown, - value: string | undefined, + args: string[], ): Promise { + if (args.length > 1) { + return "Usage: /codex fast [on|off|status]"; + } const sessionFile = await resolveControlSessionFile(ctx); if (!sessionFile) { return "Cannot set Codex fast mode because this command did not include an OpenClaw session file."; } + const value = args[0]; const parsed = parseCodexFastModeArg(value); if (value && parsed == null && value.trim().toLowerCase() !== "status") { return "Usage: /codex fast [on|off|status]"; @@ -540,12 +585,16 @@ async function setConversationPermissions( deps: CodexCommandDeps, ctx: PluginCommandContext, pluginConfig: unknown, - value: string | undefined, + args: string[], ): Promise { + if (args.length > 1) { + return "Usage: /codex permissions [default|yolo|status]"; + } const sessionFile = await resolveControlSessionFile(ctx); if (!sessionFile) { return "Cannot set Codex permissions because this command did not include an OpenClaw session file."; } + const value = args[0]; const parsed = parseCodexPermissionsModeArg(value); if (value && !parsed && value.trim().toLowerCase() !== "status") { return "Usage: /codex permissions [default|yolo|status]"; @@ -573,6 +622,9 @@ async function handleCodexDiagnosticsFeedback( return { text: "Only an owner can send Codex diagnostics." }; } const parsed = parseDiagnosticsArgs(args); + if (parsed.action === "usage") { + return { text: formatDiagnosticsUsage(commandPrefix) }; + } if (parsed.action === "confirm") { return { text: await confirmCodexDiagnosticsFeedback(deps, ctx, pluginConfig, parsed.token), @@ -998,17 +1050,41 @@ function normalizeDiagnosticsReason(note: string): string | undefined { } function parseDiagnosticsArgs(args: string): ParsedDiagnosticsArgs { - const [action, token] = splitArgs(args); + const [action, token, ...extra] = splitArgs(args); const normalizedAction = action?.toLowerCase(); - if ((normalizedAction === "confirm" || normalizedAction === "--confirm") && token) { + if ( + (normalizedAction === "confirm" || normalizedAction === "--confirm") && + token && + extra.length === 0 + ) { return { action: "confirm", token }; } - if ((normalizedAction === "cancel" || normalizedAction === "--cancel") && token) { + if ( + (normalizedAction === "cancel" || normalizedAction === "--cancel") && + token && + extra.length === 0 + ) { return { action: "cancel", token }; } + if ( + normalizedAction === "confirm" || + normalizedAction === "--confirm" || + normalizedAction === "cancel" || + normalizedAction === "--cancel" + ) { + return { action: "usage" }; + } return { action: "request", note: args }; } +function formatDiagnosticsUsage(commandPrefix: string): string { + return [ + `Usage: ${commandPrefix} [note]`, + `Usage: ${commandPrefix} confirm `, + `Usage: ${commandPrefix} cancel `, + ].join("\n"); +} + function createCodexDiagnosticsConfirmation(params: { targets: CodexDiagnosticsTarget[]; note?: string; @@ -1396,7 +1472,11 @@ async function startThreadAction( pluginConfig: unknown, method: typeof CODEX_CONTROL_METHODS.compact | typeof CODEX_CONTROL_METHODS.review, label: string, + args: string[], ): Promise { + if (args.length > 0) { + return `Usage: /codex ${label === "compaction" ? "compact" : label}`; + } const sessionFile = await resolveControlSessionFile(ctx); if (!sessionFile) { return `Cannot start Codex ${label} because this command did not include an OpenClaw session file.`; @@ -1413,11 +1493,60 @@ async function startThreadAction( } else { await deps.codexControlRequest(pluginConfig, method, { threadId: binding.threadId }); } - return `Started Codex ${label} for thread ${binding.threadId}.`; + return `Started Codex ${label} for thread ${formatCodexDisplayText(binding.threadId)}.`; } function splitArgs(value: string | undefined): string[] { - return (value ?? "").trim().split(/\s+/).filter(Boolean); + const input = value ?? ""; + const args: string[] = []; + let current = ""; + let quote: '"' | "'" | undefined; + let escaping = false; + let tokenStarted = false; + for (const char of input) { + if (escaping) { + current += char; + escaping = false; + tokenStarted = true; + continue; + } + if (char === "\\" && quote !== "'") { + escaping = true; + tokenStarted = true; + continue; + } + if (quote) { + if (char === quote) { + quote = undefined; + } else { + current += char; + } + tokenStarted = true; + continue; + } + if (char === '"' || char === "'") { + quote = char; + tokenStarted = true; + continue; + } + if (/\s/.test(char)) { + if (tokenStarted) { + args.push(current); + current = ""; + tokenStarted = false; + } + continue; + } + current += char; + tokenStarted = true; + } + if (escaping) { + current += "\\"; + } + if (tokenStarted) { + args.push(current); + } + return args; } function parseBindArgs(args: string[]): ParsedBindArgs { @@ -1429,17 +1558,32 @@ function parseBindArgs(args: string[]): ParsedBindArgs { continue; } if (arg === "--cwd") { - parsed.cwd = args[index + 1]; + const value = readRequiredOptionValue(args, index); + if (!value || parsed.cwd !== undefined) { + parsed.help = true; + continue; + } + parsed.cwd = value; index += 1; continue; } if (arg === "--model") { - parsed.model = args[index + 1]; + const value = readRequiredOptionValue(args, index); + if (!value || parsed.model !== undefined) { + parsed.help = true; + continue; + } + parsed.model = value; index += 1; continue; } if (arg === "--provider" || arg === "--model-provider") { - parsed.provider = args[index + 1]; + const value = readRequiredOptionValue(args, index); + if (!value || parsed.provider !== undefined) { + parsed.help = true; + continue; + } + parsed.provider = value; index += 1; continue; } @@ -1462,6 +1606,7 @@ function parseComputerUseArgs(args: string[]): ParsedComputerUseArgs { overrides: {}, hasOverrides: false, }; + let sawAction = false; for (let index = 0; index < args.length; index += 1) { const arg = args[index]; if (arg === "--help" || arg === "-h") { @@ -1469,12 +1614,17 @@ function parseComputerUseArgs(args: string[]): ParsedComputerUseArgs { continue; } if (arg === "status" || arg === "install") { + if (sawAction) { + parsed.help = true; + continue; + } + sawAction = true; parsed.action = arg; continue; } if (arg === "--source" || arg === "--marketplace-source") { const value = readRequiredOptionValue(args, index); - if (!value) { + if (!value || parsed.overrides.marketplaceSource !== undefined) { parsed.help = true; continue; } @@ -1484,7 +1634,7 @@ function parseComputerUseArgs(args: string[]): ParsedComputerUseArgs { } if (arg === "--marketplace-path" || arg === "--path") { const value = readRequiredOptionValue(args, index); - if (!value) { + if (!value || parsed.overrides.marketplacePath !== undefined) { parsed.help = true; continue; } @@ -1494,7 +1644,7 @@ function parseComputerUseArgs(args: string[]): ParsedComputerUseArgs { } if (arg === "--marketplace") { const value = readRequiredOptionValue(args, index); - if (!value) { + if (!value || parsed.overrides.marketplaceName !== undefined) { parsed.help = true; continue; } @@ -1504,7 +1654,7 @@ function parseComputerUseArgs(args: string[]): ParsedComputerUseArgs { } if (arg === "--plugin") { const value = readRequiredOptionValue(args, index); - if (!value) { + if (!value || parsed.overrides.pluginName !== undefined) { parsed.help = true; continue; } @@ -1514,7 +1664,7 @@ function parseComputerUseArgs(args: string[]): ParsedComputerUseArgs { } if (arg === "--server" || arg === "--mcp-server") { const value = readRequiredOptionValue(args, index); - if (!value) { + if (!value || parsed.overrides.mcpServerName !== undefined) { parsed.help = true; continue; } @@ -1531,7 +1681,8 @@ function parseComputerUseArgs(args: string[]): ParsedComputerUseArgs { function readRequiredOptionValue(args: string[], index: number): string | undefined { const value = args[index + 1]; - if (!value || value.startsWith("-")) { + const normalized = value?.trim(); + if (!normalized || normalized.startsWith("-")) { return undefined; } return value; diff --git a/extensions/codex/src/commands.test.ts b/extensions/codex/src/commands.test.ts index ab09902df6b..e669ae7ed1b 100644 --- a/extensions/codex/src/commands.test.ts +++ b/extensions/codex/src/commands.test.ts @@ -105,6 +105,13 @@ describe("codex command", () => { await fs.rm(tempDir, { recursive: true, force: true }); }); + it("escapes unknown subcommands before chat display", async () => { + const result = await handleCodexCommand(createContext("<@U123> [trusted](https://evil) @here")); + + expect(result.text).toContain("Unknown Codex command: <\uff20U123>"); + expect(result.text).not.toContain("<@U123>"); + }); + it("attaches the current session to an existing Codex thread", async () => { const sessionFile = path.join(tempDir, "session.jsonl"); const requests: Array<{ method: string; params: unknown }> = []; @@ -138,6 +145,42 @@ describe("codex command", () => { ); }); + it("rejects malformed resume commands before attaching a Codex thread", async () => { + const sessionFile = path.join(tempDir, "session.jsonl"); + const codexControlRequest = vi.fn(); + const writeCodexAppServerBinding = vi.fn(); + + await expect( + handleCodexCommand(createContext("resume thread-123 extra", sessionFile), { + deps: createDeps({ codexControlRequest, writeCodexAppServerBinding }), + }), + ).resolves.toEqual({ + text: "Usage: /codex resume ", + }); + expect(codexControlRequest).not.toHaveBeenCalled(); + expect(writeCodexAppServerBinding).not.toHaveBeenCalled(); + }); + + it("escapes resumed Codex thread ids before chat display", async () => { + const sessionFile = path.join(tempDir, "session.jsonl"); + const unsafe = "thread-123 <@U123> [trusted](https://evil)"; + const deps = createDeps({ + codexControlRequest: vi.fn(async () => ({ + thread: { id: unsafe, cwd: "/repo" }, + })), + }); + + const result = await handleCodexCommand(createContext("resume thread-123", sessionFile), { + deps, + }); + + expect(result.text).toContain( + "thread-123 <\uff20U123> \uff3btrusted\uff3d\uff08https://evil\uff09", + ); + expect(result.text).not.toContain("<@U123>"); + expect(result.text).not.toContain("[trusted](https://evil)"); + }); + it("shows model ids from Codex app-server", async () => { const config = { auth: { order: { "openai-codex": ["openai-codex:work"] } } }; const deps = createDeps({ @@ -183,6 +226,49 @@ describe("codex command", () => { }); }); + it("escapes Codex app-server model ids before chat display", async () => { + const deps = createDeps({ + listCodexAppServerModels: vi.fn(async () => ({ + models: [ + { + id: "gpt-5.4 <@U123> [trusted](https://evil)", + model: "gpt-5.4", + inputModalities: ["text"], + supportedReasoningEfforts: ["medium"], + }, + ], + })), + }); + + const result = await handleCodexCommand(createContext("models"), { deps }); + + expect(result.text).toContain( + "gpt-5.4 <\uff20U123> \uff3btrusted\uff3d\uff08https://evil\uff09", + ); + expect(result.text).not.toContain("<@U123>"); + expect(result.text).not.toContain("[trusted](https://evil)"); + }); + + it("escapes markdown underscores in Codex app-server readouts", async () => { + const deps = createDeps({ + listCodexAppServerModels: vi.fn(async () => ({ + models: [ + { + id: "unsafe_model_name", + model: "unsafe_model_name", + inputModalities: ["text"], + supportedReasoningEfforts: ["medium"], + }, + ], + })), + }); + + const result = await handleCodexCommand(createContext("models"), { deps }); + + expect(result.text).toContain("unsafe\uff3fmodel\uff3fname"); + expect(result.text).not.toContain("unsafe_model_name"); + }); + it("reports status unavailable when every Codex probe fails", async () => { const config = { auth: { order: { "openai-codex": ["openai-codex:work"] } } }; const offline = { ok: false as const, error: "offline" }; @@ -211,6 +297,184 @@ describe("codex command", () => { expect(deps.readCodexStatusProbes).toHaveBeenCalledWith(undefined, config); }); + it("escapes Codex status probe errors before chat display", async () => { + const unsafe = "<@U123> [trusted](https://evil) @here"; + const offline = { ok: false as const, error: unsafe }; + const deps = createDeps({ + readCodexStatusProbes: vi.fn(async () => ({ + models: offline, + account: offline, + limits: offline, + mcps: offline, + skills: offline, + })), + }); + + const result = await handleCodexCommand(createContext("status"), { deps }); + + expect(result.text).toContain( + "<\uff20U123> \uff3btrusted\uff3d\uff08https://evil\uff09 \uff20here", + ); + expect(result.text).not.toContain("<@U123>"); + expect(result.text).not.toContain("[trusted](https://evil)"); + expect(result.text).not.toContain("@here"); + }); + + it("escapes successful Codex status model ids and account summaries", async () => { + const unsafe = "<@U123> [trusted](https://evil) @here"; + const deps = createDeps({ + readCodexStatusProbes: vi.fn(async () => ({ + models: { + ok: true as const, + value: { + models: [ + { + id: unsafe, + model: unsafe, + inputModalities: ["text"], + supportedReasoningEfforts: ["medium"], + }, + ], + }, + }, + account: { + ok: true as const, + value: { + account: { + type: "chatgpt" as const, + email: unsafe, + planType: "plus" as const, + }, + requiresOpenaiAuth: false, + }, + }, + limits: { + ok: true as const, + value: { + rateLimits: { + limitId: null, + limitName: null, + primary: null, + secondary: null, + credits: null, + planType: null, + rateLimitReachedType: null, + }, + rateLimitsByLimitId: null, + }, + }, + mcps: { ok: true as const, value: { data: [], nextCursor: null } }, + skills: { ok: true as const, value: { data: [] } }, + })), + }); + + const result = await handleCodexCommand(createContext("status"), { deps }); + + expect(result.text).toContain( + "<\uff20U123> \uff3btrusted\uff3d\uff08https://evil\uff09 \uff20here", + ); + expect(result.text).not.toContain("<@U123>"); + expect(result.text).not.toContain("[trusted](https://evil)"); + expect(result.text).not.toContain("@here"); + }); + + it("summarizes generated Codex rate-limit payloads", async () => { + const limits = { + ok: true as const, + value: { + rateLimits: { + limitId: "codex", + limitName: "Codex", + primary: { usedPercent: 42, windowDurationMins: 300, resetsAt: null }, + secondary: null, + credits: null, + planType: null, + rateLimitReachedType: null, + }, + rateLimitsByLimitId: { + codex: { + limitId: "codex", + limitName: "Codex", + primary: { usedPercent: 42, windowDurationMins: 300, resetsAt: null }, + secondary: null, + credits: null, + planType: null, + rateLimitReachedType: null, + }, + }, + }, + }; + const deps = createDeps({ + readCodexStatusProbes: vi.fn(async () => ({ + models: { ok: false as const, error: "offline" }, + account: { ok: false as const, error: "offline" }, + limits, + mcps: { ok: true as const, value: { data: [], nextCursor: null } }, + skills: { ok: true as const, value: { data: [] } }, + })), + safeCodexControlRequest: vi + .fn() + .mockResolvedValueOnce({ + ok: true as const, + value: { account: { email: "codex@example.com" } }, + }) + .mockResolvedValueOnce(limits), + }); + + await expect(handleCodexCommand(createContext("status"), { deps })).resolves.toMatchObject({ + text: expect.stringContaining("Rate limits: 1"), + }); + await expect(handleCodexCommand(createContext("account"), { deps })).resolves.toMatchObject({ + text: expect.stringContaining("Rate limits: 1"), + }); + }); + + it("rejects extra operands for read-only Codex commands", async () => { + const readCodexStatusProbes = vi.fn(); + const listCodexAppServerModels = vi.fn(); + const safeCodexControlRequest = vi.fn(); + const codexControlRequest = vi.fn(); + const getCurrentConversationBinding = vi.fn(); + const deps = createDeps({ + codexControlRequest, + listCodexAppServerModels, + readCodexStatusProbes, + safeCodexControlRequest, + }); + + await expect(handleCodexCommand(createContext("status now"), { deps })).resolves.toEqual({ + text: "Usage: /codex status", + }); + await expect(handleCodexCommand(createContext("models all"), { deps })).resolves.toEqual({ + text: "Usage: /codex models", + }); + await expect(handleCodexCommand(createContext("account refresh"), { deps })).resolves.toEqual({ + text: "Usage: /codex account", + }); + await expect(handleCodexCommand(createContext("mcp list"), { deps })).resolves.toEqual({ + text: "Usage: /codex mcp", + }); + await expect(handleCodexCommand(createContext("skills list"), { deps })).resolves.toEqual({ + text: "Usage: /codex skills", + }); + await expect( + handleCodexCommand( + createContext("binding current", undefined, { + getCurrentConversationBinding, + }), + { deps }, + ), + ).resolves.toEqual({ + text: "Usage: /codex binding", + }); + + expect(readCodexStatusProbes).not.toHaveBeenCalled(); + expect(listCodexAppServerModels).not.toHaveBeenCalled(); + expect(safeCodexControlRequest).not.toHaveBeenCalled(); + expect(codexControlRequest).not.toHaveBeenCalled(); + expect(getCurrentConversationBinding).not.toHaveBeenCalled(); + }); + it("formats generated account/read responses", async () => { const safeCodexControlRequest = vi .fn() @@ -235,6 +499,44 @@ describe("codex command", () => { }); }); + it("escapes Codex account probe errors before chat display", async () => { + const unsafe = "<@U123> [trusted](https://evil) @here"; + const safeCodexControlRequest = vi + .fn() + .mockResolvedValueOnce({ ok: false as const, error: unsafe }) + .mockResolvedValueOnce({ ok: false as const, error: unsafe }); + + const result = await handleCodexCommand(createContext("account"), { + deps: createDeps({ safeCodexControlRequest }), + }); + + expect(result.text).toContain( + "<\uff20U123> \uff3btrusted\uff3d\uff08https://evil\uff09 \uff20here", + ); + expect(result.text).not.toContain("<@U123>"); + expect(result.text).not.toContain("[trusted](https://evil)"); + expect(result.text).not.toContain("@here"); + }); + + it("escapes successful Codex account fallback summaries before chat display", async () => { + const unsafe = "<@U123> [trusted](https://evil) @here"; + const safeCodexControlRequest = vi + .fn() + .mockResolvedValueOnce({ ok: true as const, value: { account: { id: unsafe } } }) + .mockResolvedValueOnce({ ok: true as const, value: [] }); + + const result = await handleCodexCommand(createContext("account"), { + deps: createDeps({ safeCodexControlRequest }), + }); + + expect(result.text).toContain( + "<\uff20U123> \uff3btrusted\uff3d\uff08https://evil\uff09 \uff20here", + ); + expect(result.text).not.toContain("<@U123>"); + expect(result.text).not.toContain("[trusted](https://evil)"); + expect(result.text).not.toContain("@here"); + }); + it("formats generated Amazon Bedrock account responses", async () => { const safeCodexControlRequest = vi .fn() @@ -295,6 +597,43 @@ describe("codex command", () => { }); }); + it("rejects malformed compact and review commands before starting thread actions", async () => { + const sessionFile = path.join(tempDir, "session.jsonl"); + const codexControlRequest = vi.fn(); + + await expect( + handleCodexCommand(createContext("compact now", sessionFile), { + deps: createDeps({ codexControlRequest }), + }), + ).resolves.toEqual({ + text: "Usage: /codex compact", + }); + await expect( + handleCodexCommand(createContext("review staged", sessionFile), { + deps: createDeps({ codexControlRequest }), + }), + ).resolves.toEqual({ + text: "Usage: /codex review", + }); + expect(codexControlRequest).not.toHaveBeenCalled(); + }); + + it("escapes started thread-action ids before chat display", async () => { + const sessionFile = path.join(tempDir, "session.jsonl"); + await fs.writeFile( + `${sessionFile}.codex-app-server.json`, + JSON.stringify({ schemaVersion: 1, threadId: "thread-123 <@U123>", cwd: "/repo" }), + ); + const codexControlRequest = vi.fn(async () => ({})); + + const result = await handleCodexCommand(createContext("compact", sessionFile), { + deps: createDeps({ codexControlRequest }), + }); + + expect(result.text).toContain("thread-123 <\uff20U123>"); + expect(result.text).not.toContain("<@U123>"); + }); + it("checks Codex Computer Use setup", async () => { const readCodexComputerUseStatus = vi.fn(async () => computerUseReadyStatus()); @@ -308,7 +647,7 @@ describe("codex command", () => { "Plugin: computer-use (installed)", "MCP server: computer-use (1 tools)", "Marketplace: desktop-tools", - "Tools: list_apps", + "Tools: list\uff3fapps", "Computer Use is ready.", ].join("\n"), }); @@ -318,6 +657,34 @@ describe("codex command", () => { }); }); + it("escapes Codex Computer Use status fields before chat display", async () => { + const readCodexComputerUseStatus = vi.fn(async () => ({ + ...computerUseReadyStatus(), + pluginName: "<@U123>", + mcpServerName: "computer-use [server](https://evil)", + marketplaceName: "desktop_tools", + tools: ["list_apps", "[click](https://evil)"], + message: "Computer Use is ready @here.", + })); + + const result = await handleCodexCommand(createContext("computer-use status"), { + deps: createDeps({ readCodexComputerUseStatus }), + }); + + expect(result.text).toContain("Plugin: <\uff20U123> (installed)"); + expect(result.text).toContain( + "MCP server: computer-use \uff3bserver\uff3d\uff08https://evil\uff09 (2 tools)", + ); + expect(result.text).toContain("Marketplace: desktop\uff3ftools"); + expect(result.text).toContain( + "Tools: list\uff3fapps, \uff3bclick\uff3d\uff08https://evil\uff09", + ); + expect(result.text).toContain("Computer Use is ready \uff20here."); + expect(result.text).not.toContain("<@U123>"); + expect(result.text).not.toContain("[click](https://evil)"); + expect(result.text).not.toContain("@here"); + }); + it("formats disabled installed Codex Computer Use plugins", async () => { const readCodexComputerUseStatus = vi.fn(async () => ({ ...computerUseReadyStatus(), @@ -377,6 +744,21 @@ describe("codex command", () => { expect(installCodexComputerUse).not.toHaveBeenCalled(); }); + it("rejects ambiguous Computer Use actions before setup checks", async () => { + const readCodexComputerUseStatus = vi.fn(async () => computerUseReadyStatus()); + const installCodexComputerUse = vi.fn(async () => computerUseReadyStatus()); + + await expect( + handleCodexCommand(createContext("computer-use status install"), { + deps: createDeps({ readCodexComputerUseStatus, installCodexComputerUse }), + }), + ).resolves.toEqual({ + text: expect.stringContaining("Usage: /codex computer-use"), + }); + expect(readCodexComputerUseStatus).not.toHaveBeenCalled(); + expect(installCodexComputerUse).not.toHaveBeenCalled(); + }); + it("explains compaction when no Codex thread is attached", async () => { const sessionFile = path.join(tempDir, "session.jsonl"); @@ -481,6 +863,53 @@ describe("codex command", () => { ); }); + it("rejects malformed diagnostics confirmation commands without consuming the token", async () => { + const sessionFile = path.join(tempDir, "session.jsonl"); + await fs.writeFile( + `${sessionFile}.codex-app-server.json`, + JSON.stringify({ schemaVersion: 1, threadId: "thread-confirm-args", cwd: "/repo" }), + ); + const safeCodexControlRequest = vi.fn(async () => ({ + ok: true as const, + value: { threadId: "thread-confirm-args" }, + })); + const deps = createDeps({ safeCodexControlRequest }); + + const request = await handleCodexCommand(createContext("diagnostics", sessionFile), { deps }); + const token = readDiagnosticsConfirmationToken(request); + + await expect( + handleCodexCommand(createContext(`diagnostics confirm ${token} extra`, sessionFile), { + deps, + }), + ).resolves.toEqual({ + text: [ + "Usage: /codex diagnostics [note]", + "Usage: /codex diagnostics confirm ", + "Usage: /codex diagnostics cancel ", + ].join("\n"), + }); + await expect( + handleCodexCommand(createContext(`diagnostics cancel ${token} extra`, sessionFile), { + deps, + }), + ).resolves.toEqual({ + text: [ + "Usage: /codex diagnostics [note]", + "Usage: /codex diagnostics confirm ", + "Usage: /codex diagnostics cancel ", + ].join("\n"), + }); + expect(safeCodexControlRequest).not.toHaveBeenCalled(); + + await expect( + handleCodexCommand(createContext(`diagnostics confirm ${token}`, sessionFile), { deps }), + ).resolves.toMatchObject({ + text: expect.stringContaining("Codex diagnostics sent to OpenAI servers:"), + }); + expect(safeCodexControlRequest).toHaveBeenCalledTimes(1); + }); + it("previews exec-approved diagnostics upload without exposing Codex ids", async () => { const sessionFile = path.join(tempDir, "session.jsonl"); await fs.writeFile( @@ -1386,6 +1815,86 @@ describe("codex command", () => { }); }); + it("escapes Codex thread fields and avoids unsafe resume commands", async () => { + const codexControlRequest = vi.fn(async () => ({ + data: [ + { + id: "thread-123\n`bad`", + title: "<@U123> [trusted](https://evil) @here", + model: "gpt_5", + cwd: "/repo_(x)", + }, + ], + })); + const deps = createDeps({ codexControlRequest }); + + const result = await handleCodexCommand(createContext("threads"), { deps }); + + expect(result.text).toContain("thread-123?\uff40bad\uff40"); + expect(result.text).toContain( + "<\uff20U123> \uff3btrusted\uff3d\uff08https://evil\uff09 \uff20here", + ); + expect(result.text).toContain("(gpt\uff3f5, /repo\uff3f\uff08x\uff09)"); + expect(result.text).toContain( + "Resume: copy the thread id above and run /codex resume ", + ); + expect(result.text).not.toContain("<@U123>"); + expect(result.text).not.toContain("[trusted](https://evil)"); + expect(result.text).not.toContain("Resume: /codex resume thread-123"); + }); + + it("escapes Codex MCP and skill list entries before chat display", async () => { + const codexControlRequest = vi + .fn() + .mockResolvedValueOnce({ data: [{ name: "<@U123> [mcp](https://evil)" }] }) + .mockResolvedValueOnce({ data: [{ id: "skill_1 @here" }] }); + const deps = createDeps({ codexControlRequest }); + + const mcp = await handleCodexCommand(createContext("mcp"), { deps }); + const skills = await handleCodexCommand(createContext("skills"), { deps }); + + expect(mcp.text).toContain("<\uff20U123> \uff3bmcp\uff3d\uff08https://evil\uff09"); + expect(skills.text).toContain("skill\uff3f1 \uff20here"); + expect(`${mcp.text}\n${skills.text}`).not.toContain("<@U123>"); + expect(`${mcp.text}\n${skills.text}`).not.toContain("[mcp](https://evil)"); + expect(`${mcp.text}\n${skills.text}`).not.toContain("@here"); + }); + + it("returns sanitized command failures instead of leaking app-server errors", async () => { + const sessionFile = path.join(tempDir, "session.jsonl"); + await fs.writeFile( + `${sessionFile}.codex-app-server.json`, + JSON.stringify({ schemaVersion: 1, threadId: "thread-123", cwd: "/repo" }), + ); + const failure = () => { + throw new Error("app-server failed <@U123> [trusted](https://evil) @here"); + }; + const expectSanitizedFailure = (result: PluginCommandResult) => { + expect(result.text).toContain( + "Codex command failed: app-server failed <\uff20U123> \uff3btrusted\uff3d\uff08https://evil\uff09 \uff20here", + ); + expect(result.text).not.toContain("<@U123>"); + expect(result.text).not.toContain("[trusted](https://evil)"); + expect(result.text).not.toContain("@here"); + }; + + for (const [args, deps] of [ + ["models", createDeps({ listCodexAppServerModels: vi.fn(failure) })], + ["threads", createDeps({ codexControlRequest: vi.fn(failure) })], + ["mcp", createDeps({ codexControlRequest: vi.fn(failure) })], + ["skills", createDeps({ codexControlRequest: vi.fn(failure) })], + ["resume thread-123", createDeps({ codexControlRequest: vi.fn(failure) })], + ["compact", createDeps({ codexControlRequest: vi.fn(failure) })], + ["review", createDeps({ codexControlRequest: vi.fn(failure) })], + ["bind", createDeps({ startCodexConversationThread: vi.fn(failure) })], + ["stop", createDeps({ stopCodexConversationTurn: vi.fn(failure) })], + ["steer keep going", createDeps({ steerCodexConversationTurn: vi.fn(failure) })], + ["model gpt-5.4", createDeps({ setCodexConversationModel: vi.fn(failure) })], + ] as const) { + expectSanitizedFailure(await handleCodexCommand(createContext(args, sessionFile), { deps })); + } + }); + it("binds the current conversation to a Codex app-server thread", async () => { const sessionFile = path.join(tempDir, "session.jsonl"); await fs.writeFile( @@ -1458,6 +1967,170 @@ describe("codex command", () => { }); }); + it("binds quoted workspace paths that contain spaces", async () => { + const sessionFile = path.join(tempDir, "session.jsonl"); + const startCodexConversationThread = vi.fn(async () => ({ + kind: "codex-app-server-session" as const, + version: 1 as const, + sessionFile, + workspaceDir: "/repo with space", + })); + const requestConversationBinding = vi.fn(async () => ({ + status: "bound" as const, + binding: { + bindingId: "binding-1", + pluginId: "codex", + pluginRoot: "/plugin", + channel: "test", + accountId: "default", + conversationId: "conversation", + boundAt: 1, + }, + })); + + await expect( + handleCodexCommand( + createContext('bind thread-123 --cwd "/repo with space"', sessionFile, { + requestConversationBinding, + }), + { + deps: createDeps({ + startCodexConversationThread, + resolveCodexDefaultWorkspaceDir: vi.fn(() => "/default"), + }), + }, + ), + ).resolves.toEqual({ + text: "Bound this conversation to Codex thread thread-123 in /repo with space.", + }); + expect(startCodexConversationThread).toHaveBeenCalledWith({ + pluginConfig: undefined, + config: {}, + sessionFile, + workspaceDir: "/repo with space", + threadId: "thread-123", + model: undefined, + modelProvider: undefined, + }); + }); + + it("escapes bound Codex thread ids and workspace paths before chat display", async () => { + const sessionFile = path.join(tempDir, "session.jsonl"); + const unsafeThread = "thread-123 <@U123>"; + const unsafeWorkspace = "/repo [trusted](https://evil)"; + const startCodexConversationThread = vi.fn(async () => ({ + kind: "codex-app-server-session" as const, + version: 1 as const, + sessionFile, + workspaceDir: unsafeWorkspace, + })); + const requestConversationBinding = vi.fn(async () => ({ + status: "bound" as const, + binding: { + bindingId: "binding-1", + pluginId: "codex", + pluginRoot: "/plugin", + channel: "test", + accountId: "default", + conversationId: "conversation", + boundAt: 1, + }, + })); + + const result = await handleCodexCommand( + createContext(`bind "${unsafeThread}" --cwd "${unsafeWorkspace}"`, sessionFile, { + requestConversationBinding, + }), + { + deps: createDeps({ + startCodexConversationThread, + resolveCodexDefaultWorkspaceDir: vi.fn(() => "/default"), + }), + }, + ); + + expect(result.text).toContain("thread-123 <\uff20U123>"); + expect(result.text).toContain("/repo \uff3btrusted\uff3d\uff08https://evil\uff09"); + expect(result.text).not.toContain("<@U123>"); + expect(result.text).not.toContain("[trusted](https://evil)"); + expect(requestConversationBinding).toHaveBeenCalledWith( + expect.objectContaining({ + summary: + "Codex app-server thread thread-123 <\uff20U123> in /repo \uff3btrusted\uff3d\uff08https://evil\uff09", + }), + ); + }); + + it("rejects bind options with missing, blank, or repeated values before starting Codex", async () => { + const sessionFile = path.join(tempDir, "session.jsonl"); + const startCodexConversationThread = vi.fn(); + const requestConversationBinding = vi.fn(); + + await expect( + handleCodexCommand( + createContext("bind thread-123 --cwd --model gpt-5.4", sessionFile, { + requestConversationBinding, + }), + { + deps: createDeps({ + startCodexConversationThread, + resolveCodexDefaultWorkspaceDir: vi.fn(() => "/default"), + }), + }, + ), + ).resolves.toEqual({ + text: "Usage: /codex bind [thread-id] [--cwd ] [--model ] [--provider ]", + }); + await expect( + handleCodexCommand( + createContext('bind thread-123 --cwd ""', sessionFile, { + requestConversationBinding, + }), + { + deps: createDeps({ + startCodexConversationThread, + resolveCodexDefaultWorkspaceDir: vi.fn(() => "/default"), + }), + }, + ), + ).resolves.toEqual({ + text: "Usage: /codex bind [thread-id] [--cwd ] [--model ] [--provider ]", + }); + await expect( + handleCodexCommand( + createContext("bind thread-123 --cwd /repo --cwd /other", sessionFile, { + requestConversationBinding, + }), + { + deps: createDeps({ + startCodexConversationThread, + resolveCodexDefaultWorkspaceDir: vi.fn(() => "/default"), + }), + }, + ), + ).resolves.toEqual({ + text: "Usage: /codex bind [thread-id] [--cwd ] [--model ] [--provider ]", + }); + expect(startCodexConversationThread).not.toHaveBeenCalled(); + expect(requestConversationBinding).not.toHaveBeenCalled(); + }); + + it("rejects malformed bind arguments before requiring a session file", async () => { + const startCodexConversationThread = vi.fn(); + + await expect( + handleCodexCommand(createContext("bind thread-123 --cwd", undefined), { + deps: createDeps({ + startCodexConversationThread, + resolveCodexDefaultWorkspaceDir: vi.fn(() => "/default"), + }), + }), + ).resolves.toEqual({ + text: "Usage: /codex bind [thread-id] [--cwd ] [--model ] [--provider ]", + }); + expect(startCodexConversationThread).not.toHaveBeenCalled(); + }); + it("returns the binding approval reply when conversation bind needs approval", async () => { const sessionFile = path.join(tempDir, "session.jsonl"); const reply = { text: "Approve this?" }; @@ -1494,7 +2167,7 @@ describe("codex command", () => { createContext("bind", sessionFile, { requestConversationBinding: async () => ({ status: "error", - message: "binding unsupported", + message: "binding unsupported <@U123> [trusted](https://evil)", }), }), { @@ -1510,7 +2183,9 @@ describe("codex command", () => { }), }, ), - ).resolves.toEqual({ text: "binding unsupported" }); + ).resolves.toEqual({ + text: "binding unsupported <\uff20U123> \uff3btrusted\uff3d\uff08https://evil\uff09", + }); expect(clearCodexAppServerBinding).toHaveBeenCalledWith(sessionFile); }); @@ -1548,6 +2223,25 @@ describe("codex command", () => { expect(clearCodexAppServerBinding).toHaveBeenCalledWith(sessionFile); }); + it("rejects malformed detach commands before clearing bindings", async () => { + const sessionFile = path.join(tempDir, "session.jsonl"); + const clearCodexAppServerBinding = vi.fn(); + const detachConversationBinding = vi.fn(); + + await expect( + handleCodexCommand( + createContext("detach now", sessionFile, { + detachConversationBinding, + }), + { deps: createDeps({ clearCodexAppServerBinding }) }, + ), + ).resolves.toEqual({ + text: "Usage: /codex detach", + }); + expect(detachConversationBinding).not.toHaveBeenCalled(); + expect(clearCodexAppServerBinding).not.toHaveBeenCalled(); + }); + it("stops the active bound Codex turn", async () => { const sessionFile = path.join(tempDir, "session.jsonl"); const stopCodexConversationTurn = vi.fn(async () => ({ @@ -1566,6 +2260,18 @@ describe("codex command", () => { }); }); + it("rejects malformed stop commands before interrupting Codex", async () => { + const sessionFile = path.join(tempDir, "session.jsonl"); + const stopCodexConversationTurn = vi.fn(); + + await expect( + handleCodexCommand(createContext("stop now", sessionFile), { + deps: createDeps({ stopCodexConversationTurn }), + }), + ).resolves.toEqual({ text: "Usage: /codex stop" }); + expect(stopCodexConversationTurn).not.toHaveBeenCalled(); + }); + it("steers the active bound Codex turn", async () => { const sessionFile = path.join(tempDir, "session.jsonl"); const steerCodexConversationTurn = vi.fn(async () => ({ @@ -1625,6 +2331,86 @@ describe("codex command", () => { }); }); + it("escapes current bound model status before chat display", async () => { + const sessionFile = path.join(tempDir, "session.jsonl"); + await fs.writeFile( + `${sessionFile}.codex-app-server.json`, + JSON.stringify({ + schemaVersion: 1, + threadId: "thread-model", + cwd: "/repo", + model: "model_<@U123>_[trusted](https://evil)", + }), + ); + + const result = await handleCodexCommand(createContext("model", sessionFile), { + deps: createDeps(), + }); + + expect(result.text).toContain( + "model\uff3f<\uff20U123>\uff3f\uff3btrusted\uff3d\uff08https://evil\uff09", + ); + expect(result.text).not.toContain("<@U123>"); + expect(result.text).not.toContain("[trusted](https://evil)"); + }); + + it("rejects malformed model commands before persisting the model", async () => { + const sessionFile = path.join(tempDir, "session.jsonl"); + const setCodexConversationModel = vi.fn(); + + await expect( + handleCodexCommand(createContext("model gpt-5.4 extra", sessionFile), { + deps: createDeps({ setCodexConversationModel }), + }), + ).resolves.toEqual({ text: "Usage: /codex model " }); + expect(setCodexConversationModel).not.toHaveBeenCalled(); + }); + + it("rejects extra fast and permissions arguments", async () => { + const sessionFile = path.join(tempDir, "session.jsonl"); + const setCodexConversationFastMode = vi.fn(); + const setCodexConversationPermissions = vi.fn(); + const deps = createDeps({ + setCodexConversationFastMode, + setCodexConversationPermissions, + }); + + await expect( + handleCodexCommand(createContext("fast on now", sessionFile), { deps }), + ).resolves.toEqual({ text: "Usage: /codex fast [on|off|status]" }); + await expect( + handleCodexCommand(createContext("permissions yolo now", sessionFile), { deps }), + ).resolves.toEqual({ text: "Usage: /codex permissions [default|yolo|status]" }); + + expect(setCodexConversationFastMode).not.toHaveBeenCalled(); + expect(setCodexConversationPermissions).not.toHaveBeenCalled(); + }); + + it("rejects malformed control arguments before requiring a session file", async () => { + const deps = createDeps({ + setCodexConversationModel: vi.fn(), + setCodexConversationFastMode: vi.fn(), + setCodexConversationPermissions: vi.fn(), + }); + + await expect( + handleCodexCommand(createContext("model gpt-5.4 extra"), { deps }), + ).resolves.toEqual({ + text: "Usage: /codex model ", + }); + await expect(handleCodexCommand(createContext("fast on now"), { deps })).resolves.toEqual({ + text: "Usage: /codex fast [on|off|status]", + }); + await expect( + handleCodexCommand(createContext("permissions yolo now"), { deps }), + ).resolves.toEqual({ + text: "Usage: /codex permissions [default|yolo|status]", + }); + expect(deps.setCodexConversationModel).not.toHaveBeenCalled(); + expect(deps.setCodexConversationFastMode).not.toHaveBeenCalled(); + expect(deps.setCodexConversationPermissions).not.toHaveBeenCalled(); + }); + it("uses current plugin binding data for follow-up control commands", async () => { const hostSessionFile = path.join(tempDir, "host-session.jsonl"); const pluginSessionFile = path.join(tempDir, "plugin-session.jsonl"); @@ -1717,10 +2503,50 @@ describe("codex command", () => { "- Fast: on", "- Permissions: full access", "- Active run: turn-1", - `- Session: ${sessionFile}`, + `- Session: ${sessionFile.replaceAll("_", "\uff3f")}`, ].join("\n"), }); }); + + it("escapes active binding fields before chat display", async () => { + const sessionFile = path.join(tempDir, "session.jsonl"); + await fs.writeFile( + `${sessionFile}.codex-app-server.json`, + JSON.stringify({ + schemaVersion: 1, + threadId: "thread-123 <@U123>", + cwd: "/repo", + model: "gpt [trusted](https://evil)", + }), + ); + + const result = await handleCodexCommand( + createContext("binding", sessionFile, { + getCurrentConversationBinding: async () => ({ + bindingId: "binding-1", + pluginId: "codex", + pluginRoot: "/plugin", + channel: "test", + accountId: "default", + conversationId: "conversation", + boundAt: 1, + data: { + kind: "codex-app-server-session", + version: 1, + sessionFile, + workspaceDir: "/repo <@U123>", + }, + }), + }), + { deps: createDeps() }, + ); + + expect(result.text).toContain("Thread: thread-123 <\uff20U123>"); + expect(result.text).toContain("Workspace: /repo <\uff20U123>"); + expect(result.text).toContain("Model: gpt \uff3btrusted\uff3d\uff08https://evil\uff09"); + expect(result.text).not.toContain("<@U123>"); + expect(result.text).not.toContain("[trusted](https://evil)"); + }); }); function computerUseReadyStatus(): CodexComputerUseStatus { diff --git a/extensions/codex/src/commands.ts b/extensions/codex/src/commands.ts index e5dc83023a3..8fb715bdd3f 100644 --- a/extensions/codex/src/commands.ts +++ b/extensions/codex/src/commands.ts @@ -3,6 +3,8 @@ import type { PluginCommandContext, PluginCommandResult, } from "openclaw/plugin-sdk/plugin-entry"; +import { describeControlFailure } from "./app-server/capabilities.js"; +import { formatCodexDisplayText } from "./command-formatters.js"; import type { CodexCommandDeps } from "./command-handlers.js"; export function createCodexCommand(options: { @@ -28,5 +30,11 @@ export async function handleCodexCommand( options: { pluginConfig?: unknown; deps?: Partial } = {}, ): Promise { const { handleCodexSubcommand } = await import("./command-handlers.js"); - return await handleCodexSubcommand(ctx, options); + try { + return await handleCodexSubcommand(ctx, options); + } catch (error) { + return { + text: `Codex command failed: ${formatCodexDisplayText(describeControlFailure(error))}`, + }; + } } diff --git a/extensions/codex/src/conversation-binding.test.ts b/extensions/codex/src/conversation-binding.test.ts index 5339145b8f7..675d3463050 100644 --- a/extensions/codex/src/conversation-binding.test.ts +++ b/extensions/codex/src/conversation-binding.test.ts @@ -240,7 +240,7 @@ describe("codex conversation binding", () => { request: vi.fn(async (method: string) => { if (method === "turn/start") { throw new Error( - "unexpected status 401 Unauthorized: Missing bearer or basic authentication in header", + "unexpected status 401 Unauthorized: Missing bearer <@U123> [trusted](https://evil) @here", ); } throw new Error(`unexpected method: ${method}`); @@ -283,12 +283,91 @@ describe("codex conversation binding", () => { expect(result).toEqual({ handled: true, reply: { - text: "Codex app-server turn failed: unexpected status 401 Unauthorized: Missing bearer or basic authentication in header", + text: "Codex app-server turn failed: unexpected status 401 Unauthorized: Missing bearer <\uff20U123> \uff3btrusted\uff3d\uff08https://evil\uff09 \uff20here", }, }); + const replyText = result?.reply?.text ?? ""; + expect(replyText).not.toContain("<@U123>"); + expect(replyText).not.toContain("[trusted](https://evil)"); + expect(replyText).not.toContain("@here"); expect(unhandledRejections).toEqual([]); } finally { process.off("unhandledRejection", onUnhandledRejection); } }); + + it("falls back to content when the channel body for agent is blank", async () => { + const sessionFile = path.join(tempDir, "session.jsonl"); + await fs.writeFile( + `${sessionFile}.codex-app-server.json`, + JSON.stringify({ + schemaVersion: 1, + threadId: "thread-1", + cwd: tempDir, + }), + ); + let notificationHandler: ((notification: unknown) => void) | undefined; + const turnStartParams: Record[] = []; + sharedClientMocks.getSharedCodexAppServerClient.mockResolvedValue({ + request: vi.fn(async (method: string, requestParams: Record) => { + if (method === "turn/start") { + turnStartParams.push(requestParams); + setImmediate(() => + notificationHandler?.({ + method: "turn/completed", + params: { + threadId: "thread-1", + turn: { + id: "turn-1", + status: "completed", + items: [{ type: "agentMessage", id: "item-1", text: "done" }], + }, + }, + }), + ); + return { turn: { id: "turn-1" } }; + } + throw new Error(`unexpected method: ${method}`); + }), + addNotificationHandler: vi.fn((handler: (notification: unknown) => void) => { + notificationHandler = handler; + return () => undefined; + }), + addRequestHandler: vi.fn(() => () => undefined), + }); + + const result = await handleCodexConversationInboundClaim( + { + content: "use the fallback prompt", + bodyForAgent: "", + channel: "telegram", + isGroup: false, + commandAuthorized: true, + }, + { + channelId: "telegram", + pluginBinding: { + bindingId: "binding-1", + pluginId: "codex", + pluginRoot: tempDir, + channel: "telegram", + accountId: "default", + conversationId: "5185575566", + boundAt: Date.now(), + data: { + kind: "codex-app-server-session", + version: 1, + sessionFile, + workspaceDir: tempDir, + }, + }, + }, + { timeoutMs: 50 }, + ); + + expect(result).toEqual({ handled: true, reply: { text: "done" } }); + expect(turnStartParams[0]?.input).toMatchObject([ + { type: "text", text: "use the fallback prompt" }, + ]); + }); }); diff --git a/extensions/codex/src/conversation-binding.ts b/extensions/codex/src/conversation-binding.ts index a0e0a17b197..c8919e8f1b0 100644 --- a/extensions/codex/src/conversation-binding.ts +++ b/extensions/codex/src/conversation-binding.ts @@ -26,6 +26,7 @@ import { type CodexAppServerAuthProfileLookup, } from "./app-server/session-binding.js"; import { getSharedCodexAppServerClient } from "./app-server/shared-client.js"; +import { formatCodexDisplayText } from "./command-formatters.js"; import { createCodexConversationBindingData, readCodexConversationBindingData, @@ -130,7 +131,7 @@ export async function handleCodexConversationInboundClaim( if (event.commandAuthorized !== true) { return { handled: true }; } - const prompt = (event.bodyForAgent ?? event.content ?? "").trim(); + const prompt = event.bodyForAgent?.trim() || event.content?.trim() || ""; if (!prompt) { return { handled: true }; } @@ -149,7 +150,7 @@ export async function handleCodexConversationInboundClaim( return { handled: true, reply: { - text: `Codex app-server turn failed: ${formatErrorMessage(error)}`, + text: `Codex app-server turn failed: ${formatCodexDisplayText(formatErrorMessage(error))}`, }, }; } diff --git a/extensions/codex/src/conversation-control.test.ts b/extensions/codex/src/conversation-control.test.ts index 0b33fefa35f..e5e9a7fced7 100644 --- a/extensions/codex/src/conversation-control.test.ts +++ b/extensions/codex/src/conversation-control.test.ts @@ -102,4 +102,25 @@ describe("codex conversation controls", () => { }); expect(binding?.modelProvider).toBeUndefined(); }); + + it("escapes model names returned from Codex before chat display", async () => { + const sessionFile = path.join(tempDir, "session.jsonl"); + await writeCodexAppServerBinding(sessionFile, { + threadId: "thread-1", + cwd: tempDir, + model: "gpt-5.4", + modelProvider: "openai", + }); + sharedClientMocks.getSharedCodexAppServerClient.mockResolvedValue({ + request: vi.fn(async () => ({ + thread: { id: "thread-1", cwd: tempDir }, + model: "gpt-5.5 <@U123> [trusted](https://evil)", + modelProvider: "openai", + })), + }); + + await expect(setCodexConversationModel({ sessionFile, model: "gpt-5.5" })).resolves.toBe( + "Codex model set to gpt-5.5 <\uff20U123> \uff3btrusted\uff3d\uff08https://evil\uff09.", + ); + }); }); diff --git a/extensions/codex/src/conversation-control.ts b/extensions/codex/src/conversation-control.ts index 04e0b7f76ea..8fedfb295d0 100644 --- a/extensions/codex/src/conversation-control.ts +++ b/extensions/codex/src/conversation-control.ts @@ -10,6 +10,7 @@ import { writeCodexAppServerBinding, } from "./app-server/session-binding.js"; import { getSharedCodexAppServerClient } from "./app-server/shared-client.js"; +import { formatCodexDisplayText } from "./command-formatters.js"; type ActiveTurn = { sessionFile: string; @@ -128,7 +129,7 @@ export async function setCodexConversationModel(params: { sandbox: binding.sandbox, serviceTier: binding.serviceTier ?? runtime.serviceTier, }); - return `Codex model set to ${response.model ?? model}.`; + return `Codex model set to ${formatCodexDisplayText(response.model ?? model)}.`; } export async function setCodexConversationFastMode(params: { diff --git a/extensions/codex/src/conversation-turn-collector.test.ts b/extensions/codex/src/conversation-turn-collector.test.ts index 49b61f652ab..47f4018dea0 100644 --- a/extensions/codex/src/conversation-turn-collector.test.ts +++ b/extensions/codex/src/conversation-turn-collector.test.ts @@ -23,6 +23,43 @@ describe("codex conversation turn collector", () => { await expect(completion).resolves.toEqual({ replyText: "hello world" }); }); + it("buffers pre-start notifications and replays only the selected turn", async () => { + const collector = createCodexConversationTurnCollector("thread-1"); + + collector.handleNotification({ + method: "turn/completed", + params: { + threadId: "thread-1", + turn: { + id: "turn-stale", + status: "completed", + items: [{ type: "agentMessage", id: "wrong", text: "stale answer" }], + }, + }, + }); + collector.handleNotification({ + method: "item/agentMessage/delta", + params: { threadId: "thread-1", turnId: "turn-1", itemId: "right", delta: "fresh " }, + }); + collector.handleNotification({ + method: "turn/completed", + params: { + threadId: "thread-1", + turn: { + id: "turn-1", + status: "completed", + items: [{ type: "agentMessage", id: "right", text: "fresh answer" }], + }, + }, + }); + + collector.setTurnId("turn-1"); + + await expect(collector.wait({ timeoutMs: 1_000 })).resolves.toEqual({ + replyText: "fresh answer", + }); + }); + it("uses completed agent message items when deltas are absent", async () => { const collector = createCodexConversationTurnCollector("thread-1"); collector.setTurnId("turn-1"); diff --git a/extensions/codex/src/conversation-turn-collector.ts b/extensions/codex/src/conversation-turn-collector.ts index b9cc4e7a548..ba82956b1ca 100644 --- a/extensions/codex/src/conversation-turn-collector.ts +++ b/extensions/codex/src/conversation-turn-collector.ts @@ -4,6 +4,8 @@ import { type JsonObject, } from "./app-server/protocol.js"; +const MAX_PENDING_NOTIFICATIONS_PER_TURN = 100; + export function createCodexConversationTurnCollector(threadId: string) { let turnId: string | undefined; let completed = false; @@ -11,6 +13,7 @@ export function createCodexConversationTurnCollector(threadId: string) { let timeout: ReturnType | undefined; const assistantTextByItem = new Map(); const assistantOrder: string[] = []; + const pendingNotificationsByTurnId = new Map(); let resolveCompletion: ((value: { replyText: string }) => void) | undefined; let rejectCompletion: ((error: Error) => void) | undefined; @@ -46,59 +49,80 @@ export function createCodexConversationTurnCollector(threadId: string) { clearWaitState(); }; + const handleNotification = (notification: CodexServerNotification) => { + const params = isJsonObject(notification.params) ? notification.params : undefined; + if (!params || readString(params, "threadId") !== threadId) { + return; + } + if (!turnId) { + const pendingTurnId = readNotificationTurnId(params); + if (pendingTurnId) { + const pending = pendingNotificationsByTurnId.get(pendingTurnId) ?? []; + if (pending.length < MAX_PENDING_NOTIFICATIONS_PER_TURN) { + pending.push(notification); + pendingNotificationsByTurnId.set(pendingTurnId, pending); + } + } + return; + } + if (!isNotificationForTurn(params, threadId, turnId)) { + return; + } + if (notification.method === "item/agentMessage/delta") { + const itemId = readString(params, "itemId") ?? readString(params, "id") ?? "assistant"; + const delta = readTextString(params, "delta"); + if (!delta) { + return; + } + rememberItem(itemId); + assistantTextByItem.set(itemId, `${assistantTextByItem.get(itemId) ?? ""}${delta}`); + return; + } + if (notification.method === "item/completed") { + const item = isJsonObject(params.item) ? params.item : undefined; + if (item?.type === "agentMessage") { + const itemId = readString(item, "id") ?? readString(params, "itemId") ?? "assistant"; + const text = readTextString(item, "text"); + if (text) { + rememberItem(itemId); + assistantTextByItem.set(itemId, text); + } + } + return; + } + if (notification.method === "turn/completed") { + const turn = isJsonObject(params.turn) ? params.turn : undefined; + const status = readString(turn, "status"); + if (status === "failed") { + failedError = + readString(readRecord(turn?.error), "message") ?? "codex app-server turn failed"; + } + const items = Array.isArray(turn?.items) ? turn.items : []; + for (const item of items) { + if (!isJsonObject(item) || item.type !== "agentMessage") { + continue; + } + const itemId = readString(item, "id") ?? `assistant-${assistantOrder.length + 1}`; + const text = readTextString(item, "text"); + if (text) { + rememberItem(itemId); + assistantTextByItem.set(itemId, text); + } + } + finish(); + } + }; + return { setTurnId(nextTurnId: string) { turnId = nextTurnId; - }, - handleNotification(notification: CodexServerNotification) { - const params = isJsonObject(notification.params) ? notification.params : undefined; - if (!params || !isNotificationForTurn(params, threadId, turnId)) { - return; - } - if (notification.method === "item/agentMessage/delta") { - const itemId = readString(params, "itemId") ?? readString(params, "id") ?? "assistant"; - const delta = readTextString(params, "delta"); - if (!delta) { - return; - } - rememberItem(itemId); - assistantTextByItem.set(itemId, `${assistantTextByItem.get(itemId) ?? ""}${delta}`); - return; - } - if (notification.method === "item/completed") { - const item = isJsonObject(params.item) ? params.item : undefined; - if (item?.type === "agentMessage") { - const itemId = readString(item, "id") ?? readString(params, "itemId") ?? "assistant"; - const text = readTextString(item, "text"); - if (text) { - rememberItem(itemId); - assistantTextByItem.set(itemId, text); - } - } - return; - } - if (notification.method === "turn/completed") { - const turn = isJsonObject(params.turn) ? params.turn : undefined; - const status = readString(turn, "status"); - if (status === "failed") { - failedError = - readString(readRecord(turn?.error), "message") ?? "codex app-server turn failed"; - } - const items = Array.isArray(turn?.items) ? turn.items : []; - for (const item of items) { - if (!isJsonObject(item) || item.type !== "agentMessage") { - continue; - } - const itemId = readString(item, "id") ?? `assistant-${assistantOrder.length + 1}`; - const text = readTextString(item, "text"); - if (text) { - rememberItem(itemId); - assistantTextByItem.set(itemId, text); - } - } - finish(); + const pending = pendingNotificationsByTurnId.get(nextTurnId) ?? []; + pendingNotificationsByTurnId.clear(); + for (const notification of pending) { + handleNotification(notification); } }, + handleNotification, wait(params: { timeoutMs: number }): Promise<{ replyText: string }> { if (completed) { return failedError @@ -141,6 +165,10 @@ function isNotificationForTurn( return readString(turn, "id") === turnId; } +function readNotificationTurnId(params: JsonObject): string | undefined { + return readString(params, "turnId") ?? readString(readRecord(params.turn), "id"); +} + function readRecord(value: unknown): Record | undefined { return value && typeof value === "object" && !Array.isArray(value) ? (value as Record) diff --git a/extensions/codex/src/conversation-turn-input.test.ts b/extensions/codex/src/conversation-turn-input.test.ts index f5e86691ba9..917a4ca8434 100644 --- a/extensions/codex/src/conversation-turn-input.test.ts +++ b/extensions/codex/src/conversation-turn-input.test.ts @@ -41,4 +41,101 @@ describe("codex conversation turn input", () => { { type: "image", url: "https://example.test/photo.webp?sig=1" }, ]); }); + + it("keeps protocol-relative image urls remote", () => { + expect( + buildCodexConversationTurnInput({ + prompt: "look", + event: { + content: "look", + channel: "webchat", + isGroup: false, + metadata: { + mediaUrl: "//cdn.example.test/photo.webp", + }, + }, + }), + ).toEqual([ + { type: "text", text: "look", text_elements: [] }, + { type: "image", url: "//cdn.example.test/photo.webp" }, + ]); + }); + + it("decodes local file URLs for Codex local image input", () => { + expect( + buildCodexConversationTurnInput({ + prompt: "look", + event: { + content: "look", + channel: "webchat", + isGroup: false, + metadata: { + mediaPath: "file:///tmp/OpenClaw%20QA/photo.png", + mediaType: "image/png", + }, + }, + }), + ).toEqual([ + { type: "text", text: "look", text_elements: [] }, + { type: "localImage", path: "/tmp/OpenClaw QA/photo.png" }, + ]); + }); + + it("drops malformed local file URLs instead of throwing", () => { + expect( + buildCodexConversationTurnInput({ + prompt: "look", + event: { + content: "look", + channel: "webchat", + isGroup: false, + metadata: { + mediaPath: "file:///tmp/%zz/photo.png", + mediaType: "image/png", + }, + }, + }), + ).toEqual([{ type: "text", text: "look", text_elements: [] }]); + }); + + it("treats local media URLs as Codex local image input", () => { + expect( + buildCodexConversationTurnInput({ + prompt: "look", + event: { + content: "look", + channel: "webchat", + isGroup: false, + metadata: { + mediaUrls: ["/tmp/staged-photo.png", "file:///tmp/OpenClaw%20QA/second.jpg"], + mediaTypes: ["image/png", "image/jpeg"], + }, + }, + }), + ).toEqual([ + { type: "text", text: "look", text_elements: [] }, + { type: "localImage", path: "/tmp/staged-photo.png" }, + { type: "localImage", path: "/tmp/OpenClaw QA/second.jpg" }, + ]); + }); + + it("treats Windows media paths as Codex local image input", () => { + expect( + buildCodexConversationTurnInput({ + prompt: "look", + event: { + content: "look", + channel: "webchat", + isGroup: false, + metadata: { + mediaUrl: "C:\\OpenClaw QA\\photo.png", + mediaType: "image/png", + }, + }, + }), + ).toEqual([ + { type: "text", text: "look", text_elements: [] }, + { type: "localImage", path: "C:\\OpenClaw QA\\photo.png" }, + ]); + }); }); diff --git a/extensions/codex/src/conversation-turn-input.ts b/extensions/codex/src/conversation-turn-input.ts index aaff58453e1..0f30bafcee1 100644 --- a/extensions/codex/src/conversation-turn-input.ts +++ b/extensions/codex/src/conversation-turn-input.ts @@ -1,4 +1,5 @@ import path from "node:path"; +import { fileURLToPath } from "node:url"; import type { PluginHookInboundClaimEvent } from "openclaw/plugin-sdk/plugin-entry"; import type { CodexUserInput } from "./app-server/protocol.js"; @@ -48,8 +49,10 @@ function toCodexImageInput(media: InboundMedia): CodexUserInput | undefined { if (!isImageMedia(media)) { return undefined; } - if (media.path) { - return { type: "localImage", path: normalizeFileUrl(media.path) }; + const localPath = media.path ?? readLocalMediaPath(media.url); + if (localPath) { + const normalized = normalizeFileUrl(localPath); + return normalized ? { type: "localImage", path: normalized } : undefined; } return media.url ? { type: "image", url: media.url } : undefined; } @@ -65,8 +68,31 @@ function isImageMedia(media: InboundMedia): boolean { return IMAGE_EXTENSIONS.has(path.extname(candidate.split(/[?#]/, 1)[0] ?? "").toLowerCase()); } -function normalizeFileUrl(value: string): string { - return value.startsWith("file://") ? new URL(value).pathname : value; +function normalizeFileUrl(value: string): string | undefined { + if (!value.startsWith("file://")) { + return value; + } + try { + return fileURLToPath(value); + } catch { + return undefined; + } +} + +function readLocalMediaPath(value: string | undefined): string | undefined { + if (!value) { + return undefined; + } + if (value.startsWith("file://")) { + return value; + } + if (value.startsWith("//")) { + return undefined; + } + if (path.isAbsolute(value) || path.win32.isAbsolute(value)) { + return value; + } + return /^[a-z][a-z0-9+.-]*:/i.test(value) ? undefined : value; } function readStringArray(value: unknown): string[] { diff --git a/scripts/check-changed.mjs b/scripts/check-changed.mjs index 144a2aaa72d..54f89c7bab9 100644 --- a/scripts/check-changed.mjs +++ b/scripts/check-changed.mjs @@ -33,6 +33,52 @@ export function createChangedCheckChildEnv(baseEnv = process.env) { }; } +function isTruthyEnvFlag(value) { + const normalized = String(value ?? "") + .trim() + .toLowerCase(); + return normalized !== "" && normalized !== "0" && normalized !== "false" && normalized !== "no"; +} + +export function shouldDelegateChangedCheckToTestbox(argv = [], env = process.env) { + if (!isTruthyEnvFlag(env.OPENCLAW_TESTBOX)) { + return false; + } + if (isTruthyEnvFlag(env.OPENCLAW_TESTBOX_REMOTE_RUN)) { + return false; + } + if (isTruthyEnvFlag(env.CI) || isTruthyEnvFlag(env.GITHUB_ACTIONS)) { + return false; + } + if (argv.includes("--dry-run")) { + return false; + } + return true; +} + +export function buildChangedCheckTestboxArgs(argv = []) { + return [ + "testbox:run", + "--", + "OPENCLAW_TESTBOX=1", + "OPENCLAW_TESTBOX_REMOTE_RUN=1", + "pnpm", + "check:changed", + ...argv, + ]; +} + +export async function runChangedCheckViaTestbox(argv = [], env = process.env) { + console.error( + "[check:changed] OPENCLAW_TESTBOX=1 set; delegating to Blacksmith Testbox via `pnpm testbox:run`.", + ); + return await runManagedCommand({ + bin: "pnpm", + args: buildChangedCheckTestboxArgs(argv), + env, + }); +} + export function createChangedCheckPlan(result, options = {}) { const commands = []; const baseEnv = createChangedCheckChildEnv(options.env ?? process.env); @@ -283,21 +329,26 @@ function isDirectRun() { } if (isDirectRun()) { - const args = parseArgs(process.argv.slice(2)); - const paths = - args.paths.length > 0 - ? args.paths - : args.staged - ? listStagedChangedPaths() - : listChangedPathsFromGit({ base: args.base, head: args.head }); - const result = detectChangedLanesForPaths({ - paths, - base: args.base, - head: args.head, - staged: args.staged, - }); - process.exitCode = await runChangedCheck(result, { - ...args, - explicitPaths: args.paths.length > 0, - }); + const argv = process.argv.slice(2); + if (shouldDelegateChangedCheckToTestbox(argv, process.env)) { + process.exitCode = await runChangedCheckViaTestbox(argv, process.env); + } else { + const args = parseArgs(argv); + const paths = + args.paths.length > 0 + ? args.paths + : args.staged + ? listStagedChangedPaths() + : listChangedPathsFromGit({ base: args.base, head: args.head }); + const result = detectChangedLanesForPaths({ + paths, + base: args.base, + head: args.head, + staged: args.staged, + }); + process.exitCode = await runChangedCheck(result, { + ...args, + explicitPaths: args.paths.length > 0, + }); + } } diff --git a/scripts/check-codex-app-server-protocol.ts b/scripts/check-codex-app-server-protocol.ts index e543e5a281e..65431874ef7 100644 --- a/scripts/check-codex-app-server-protocol.ts +++ b/scripts/check-codex-app-server-protocol.ts @@ -1,11 +1,9 @@ import fs from "node:fs/promises"; import path from "node:path"; +import { resolveCodexAppServerProtocolSource } from "./lib/codex-app-server-protocol-source.js"; -const codexRepo = process.env.OPENCLAW_CODEX_REPO - ? path.resolve(process.env.OPENCLAW_CODEX_REPO) - : path.resolve(process.cwd(), "../codex"); -const schemaRoot = path.join(codexRepo, "codex-rs/app-server-protocol/schema/typescript"); -const sourceSchemaRoot = path.join(codexRepo, "codex-rs/app-server-protocol/schema"); +const { sourceRoot: sourceSchemaRoot } = await resolveCodexAppServerProtocolSource(process.cwd()); +const schemaRoot = path.join(sourceSchemaRoot, "typescript"); const generatedRoot = path.resolve( process.cwd(), "extensions/codex/src/app-server/protocol-generated", @@ -104,12 +102,14 @@ if (failures.length > 0) { for (const failure of failures) { console.error(`- ${failure}`); } - console.error("Run `pnpm codex-app-server:protocol:sync` after refreshing ../codex."); + console.error( + `Run \`pnpm codex-app-server:protocol:sync\` after refreshing the Codex checkout at ${path.resolve(sourceSchemaRoot, "../../..")}.`, + ); process.exit(1); } console.log( - `Codex app-server generated protocol matches OpenClaw bridge assumptions: ${schemaRoot}`, + `Codex app-server generated protocol matches OpenClaw bridge assumptions: ${sourceSchemaRoot}`, ); async function compareGeneratedProtocolMirror(): Promise { @@ -130,14 +130,12 @@ async function compareGeneratedProtocolMirror(): Promise { ); const target = await fs.readFile(path.join(targetTsRoot, file), "utf8"); if (source !== target) { - failures.push( - `protocol-generated/typescript/${file}: differs from normalized ../codex schema`, - ); + failures.push(`protocol-generated/typescript/${file}: differs from normalized source schema`); } } for (const file of targetFiles) { if (!sourceSet.has(file)) { - failures.push(`protocol-generated/typescript/${file}: no longer present in ../codex schema`); + failures.push(`protocol-generated/typescript/${file}: no longer present in source schema`); } } @@ -161,7 +159,7 @@ async function compareGeneratedProtocolMirror(): Promise { continue; } if (source !== target) { - failures.push(`protocol-generated/json/${schema}: differs from ../codex schema`); + failures.push(`protocol-generated/json/${schema}: differs from source schema`); } } } diff --git a/scripts/lib/codex-app-server-protocol-source.ts b/scripts/lib/codex-app-server-protocol-source.ts new file mode 100644 index 00000000000..1c13c50e161 --- /dev/null +++ b/scripts/lib/codex-app-server-protocol-source.ts @@ -0,0 +1,74 @@ +import fs from "node:fs/promises"; +import path from "node:path"; + +const PROTOCOL_SCHEMA_RELATIVE_PATH = "codex-rs/app-server-protocol/schema"; + +export async function resolveCodexAppServerProtocolSource(repoRoot: string): Promise<{ + codexRepo: string; + sourceRoot: string; +}> { + const candidates = await collectCodexRepoCandidates(repoRoot); + const checked: string[] = []; + + for (const candidate of candidates) { + const codexRepo = path.resolve(candidate); + if (checked.includes(codexRepo)) { + continue; + } + checked.push(codexRepo); + const sourceRoot = path.join(codexRepo, PROTOCOL_SCHEMA_RELATIVE_PATH); + if (await isDirectory(path.join(sourceRoot, "typescript"))) { + return { codexRepo, sourceRoot }; + } + } + + throw new Error( + [ + "Codex app-server protocol schema not found.", + "Set OPENCLAW_CODEX_REPO to a checkout of openai/codex, or keep a sibling `codex` checkout next to the primary OpenClaw checkout.", + `Checked: ${checked.join(", ") || ""}`, + ].join("\n"), + ); +} + +async function collectCodexRepoCandidates(repoRoot: string): Promise { + const candidates = [ + process.env.OPENCLAW_CODEX_REPO, + path.resolve(repoRoot, "../codex"), + await resolvePrimaryWorktreeSiblingCodex(repoRoot), + ]; + return candidates.filter((candidate): candidate is string => Boolean(candidate)); +} + +async function resolvePrimaryWorktreeSiblingCodex(repoRoot: string): Promise { + const gitFilePath = path.join(repoRoot, ".git"); + let gitFile: string; + try { + gitFile = await fs.readFile(gitFilePath, "utf8"); + } catch { + return undefined; + } + + const match = /^gitdir:\s*(.+)$/m.exec(gitFile); + if (!match) { + return undefined; + } + + const gitDir = path.resolve(repoRoot, match[1].trim()); + const worktreeMarker = `${path.sep}.git${path.sep}worktrees${path.sep}`; + const markerIndex = gitDir.indexOf(worktreeMarker); + if (markerIndex < 0) { + return undefined; + } + + const primaryWorktreeRoot = gitDir.slice(0, markerIndex); + return path.join(path.dirname(primaryWorktreeRoot), "codex"); +} + +async function isDirectory(candidate: string): Promise { + try { + return (await fs.stat(candidate)).isDirectory(); + } catch { + return false; + } +} diff --git a/scripts/qa-coverage-report.ts b/scripts/qa-coverage-report.ts new file mode 100644 index 00000000000..dfe9bcba67e --- /dev/null +++ b/scripts/qa-coverage-report.ts @@ -0,0 +1,56 @@ +import { runQaCoverageReportCommand } from "../extensions/qa-lab/src/cli.runtime.ts"; + +type Options = { + json?: boolean; + output?: string; + repoRoot?: string; +}; + +function takeValue(args: string[], index: number, flag: string): string { + const value = args[index + 1]; + if (!value || value.startsWith("-")) { + throw new Error(`${flag} requires a value.`); + } + return value; +} + +function parseArgs(args: string[]): Options { + const opts: Options = {}; + for (let index = 0; index < args.length; index += 1) { + const arg = args[index]; + switch (arg) { + case "--help": + case "-h": + process.stdout.write(`Usage: openclaw qa coverage [options] + +Options: + --json Print machine-readable JSON + --output Write the report to a file + --repo-root Repository root to target + -h, --help Display help +`); + process.exit(0); + case "--json": + opts.json = true; + break; + case "--output": + opts.output = takeValue(args, index, arg); + index += 1; + break; + case "--repo-root": + opts.repoRoot = takeValue(args, index, arg); + index += 1; + break; + default: + throw new Error(`Unknown qa coverage option: ${arg}`); + } + } + return opts; +} + +const opts = parseArgs(process.argv.slice(2)); +await runQaCoverageReportCommand({ + ...(opts.json ? { json: true } : {}), + ...(opts.output ? { output: opts.output } : {}), + ...(opts.repoRoot ? { repoRoot: opts.repoRoot } : {}), +}); diff --git a/scripts/run-node.mjs b/scripts/run-node.mjs index 7b485805527..46226255357 100644 --- a/scripts/run-node.mjs +++ b/scripts/run-node.mjs @@ -796,6 +796,7 @@ const shouldUseExistingDistForGatewayClient = (deps, buildRequirement) => statMtime(deps.distEntry, deps.fs) != null; const isQaParityReportCommand = (args) => args[0] === "qa" && args[1] === "parity-report"; +const isQaCoverageReportCommand = (args) => args[0] === "qa" && args[1] === "coverage"; const shouldRunQaParityReportFromSource = (deps, buildRequirement) => buildRequirement.reason === "missing_private_qa_dist" && @@ -803,6 +804,12 @@ const shouldRunQaParityReportFromSource = (deps, buildRequirement) => deps.env.OPENCLAW_FORCE_BUILD !== "1" && statMtime(path.join(deps.cwd, "extensions", "qa-lab", "src", "cli.runtime.ts"), deps.fs) != null; +const shouldRunQaCoverageReportFromSource = (deps, buildRequirement) => + buildRequirement.reason === "missing_private_qa_dist" && + isQaCoverageReportCommand(deps.args) && + deps.env.OPENCLAW_FORCE_BUILD !== "1" && + statMtime(path.join(deps.cwd, "extensions", "qa-lab", "src", "cli.runtime.ts"), deps.fs) != null; + const runQaParityReportFromSource = async (deps) => { const sourceEntrypoint = path.join(deps.cwd, "scripts", "qa-parity-report.ts"); const nodeProcess = deps.spawn( @@ -823,6 +830,26 @@ const runQaParityReportFromSource = async (deps) => { return res.exitCode ?? 1; }; +const runQaCoverageReportFromSource = async (deps) => { + const sourceEntrypoint = path.join(deps.cwd, "scripts", "qa-coverage-report.ts"); + const nodeProcess = deps.spawn( + deps.execPath, + ["--import", "tsx", sourceEntrypoint, ...deps.args.slice(2)], + { + cwd: deps.cwd, + env: deps.env, + stdio: deps.outputTee ? ["inherit", "pipe", "pipe"] : "inherit", + }, + ); + pipeSpawnedOutput(nodeProcess, deps); + const res = await waitForSpawnedProcess(nodeProcess, deps); + const interruptedExitCode = getInterruptedSpawnExitCode(res); + if (interruptedExitCode !== null) { + return interruptedExitCode; + } + return res.exitCode ?? 1; +}; + export async function runNodeMain(params = {}) { const deps = { spawn: params.spawn ?? spawn, @@ -862,6 +889,7 @@ export async function runNodeMain(params = {}) { buildRequirement, ); const useQaParityReportSource = shouldRunQaParityReportFromSource(deps, buildRequirement); + const useQaCoverageReportSource = shouldRunQaCoverageReportFromSource(deps, buildRequirement); if (useExistingGatewayClientDist) { buildRequirement = { shouldBuild: false, reason: "gateway_client_existing_dist" }; } @@ -870,6 +898,11 @@ export async function runNodeMain(params = {}) { exitCode = await runQaParityReportFromSource(deps); return await closeRunNodeOutputTee(deps, exitCode); } + if (useQaCoverageReportSource) { + logRunner("Running QA coverage report from source without rebuilding private QA dist.", deps); + exitCode = await runQaCoverageReportFromSource(deps); + return await closeRunNodeOutputTee(deps, exitCode); + } if (!buildRequirement.shouldBuild) { if (!useExistingGatewayClientDist) { const runtimePostBuildRequirement = resolveRuntimePostBuildRequirement(deps); diff --git a/scripts/run-oxlint.mjs b/scripts/run-oxlint.mjs index e89b87768d2..924fb02ebdb 100644 --- a/scripts/run-oxlint.mjs +++ b/scripts/run-oxlint.mjs @@ -52,16 +52,24 @@ export function filterSparseMissingOxlintTargets( } = {}, ) { if (!isSparseCheckoutEnabled({ cwd })) { - return { args, hadExplicitTargets: false, remainingExplicitTargets: 0, skippedTargets: [] }; + return { + args, + hadExplicitTargets: false, + remainingExplicitTargets: 0, + skippedTargets: [], + skippedConfigs: [], + }; } const filteredArgs = []; const skippedTargets = []; + const skippedConfigs = []; let hadExplicitTargets = false; let remainingExplicitTargets = 0; let consumeNextValue = false; - for (const arg of args) { + for (let index = 0; index < args.length; index += 1) { + const arg = args[index]; if (consumeNextValue) { filteredArgs.push(arg); consumeNextValue = false; @@ -74,6 +82,29 @@ export function filterSparseMissingOxlintTargets( } if (arg.startsWith("--")) { + if (arg === "--tsconfig") { + const value = args[index + 1]; + if (value !== undefined) { + index += 1; + if (!fileExists(path.resolve(cwd, value)) && isTrackedPath({ cwd, target: value })) { + skippedConfigs.push(value); + continue; + } + filteredArgs.push(arg, value); + continue; + } + } + if (arg.startsWith("--tsconfig=")) { + const value = arg.slice("--tsconfig=".length); + if ( + value && + !fileExists(path.resolve(cwd, value)) && + isTrackedPath({ cwd, target: value }) + ) { + skippedConfigs.push(value); + continue; + } + } filteredArgs.push(arg); if (!arg.includes("=") && OXLINT_VALUE_FLAGS.has(arg)) { consumeNextValue = true; @@ -97,7 +128,13 @@ export function filterSparseMissingOxlintTargets( filteredArgs.push(arg); } - return { args: filteredArgs, hadExplicitTargets, remainingExplicitTargets, skippedTargets }; + return { + args: filteredArgs, + hadExplicitTargets, + remainingExplicitTargets, + skippedTargets, + skippedConfigs, + }; } function getSparseCheckoutEnabled({ cwd }) { @@ -159,6 +196,12 @@ export async function main(argv = process.argv.slice(2), runtimeEnv = process.en `[oxlint] sparse checkout is missing tracked target(s); skipping ${sparseTargets.skippedTargets.join(", ")}`, ); } + if (sparseTargets.skippedConfigs.length > 0) { + console.error( + `[oxlint] sparse checkout is missing tracked config(s); skipping oxlint: ${sparseTargets.skippedConfigs.join(", ")}`, + ); + return; + } if (sparseTargets.hadExplicitTargets && sparseTargets.remainingExplicitTargets === 0) { console.error("[oxlint] no present sparse-checkout targets remain; skipping oxlint."); return; diff --git a/scripts/sync-codex-app-server-protocol.ts b/scripts/sync-codex-app-server-protocol.ts index 526bc3b7bde..379d150d7bc 100644 --- a/scripts/sync-codex-app-server-protocol.ts +++ b/scripts/sync-codex-app-server-protocol.ts @@ -1,11 +1,8 @@ import fs from "node:fs/promises"; import path from "node:path"; +import { resolveCodexAppServerProtocolSource } from "./lib/codex-app-server-protocol-source.js"; -const codexRepo = process.env.OPENCLAW_CODEX_REPO - ? path.resolve(process.env.OPENCLAW_CODEX_REPO) - : path.resolve(process.cwd(), "../codex"); - -const sourceRoot = path.join(codexRepo, "codex-rs/app-server-protocol/schema"); +const { sourceRoot } = await resolveCodexAppServerProtocolSource(process.cwd()); const targetRoot = path.resolve( process.cwd(), "extensions/codex/src/app-server/protocol-generated", diff --git a/scripts/test-live-codex-harness-docker.sh b/scripts/test-live-codex-harness-docker.sh index b7cc1406672..7d322c1fe33 100644 --- a/scripts/test-live-codex-harness-docker.sh +++ b/scripts/test-live-codex-harness-docker.sh @@ -22,9 +22,15 @@ DOCKER_USER="${OPENCLAW_DOCKER_USER:-node}" DOCKER_HOME_MOUNT=() DOCKER_TRUSTED_HARNESS_MOUNT=() DOCKER_TRUSTED_HARNESS_CONTAINER_DIR="" +DOCKER_CACHE_CONTAINER_DIR="/tmp/openclaw-cache" +DOCKER_CLI_TOOLS_CONTAINER_DIR="/tmp/openclaw-npm-global" DOCKER_EXTRA_ENV_FILES=() DOCKER_AUTH_PRESTAGED=0 +openclaw_live_codex_harness_is_ci() { + [[ -n "${CI:-}" && "${CI:-}" != "false" ]] || [[ -n "${GITHUB_ACTIONS:-}" && "${GITHUB_ACTIONS:-}" != "false" ]] +} + openclaw_live_codex_harness_append_build_extension() { local extension="${1:?extension required}" local current="${OPENCLAW_DOCKER_BUILD_EXTENSIONS:-${OPENCLAW_EXTENSIONS:-}}" @@ -50,6 +56,13 @@ if [[ "$CODEX_HARNESS_AUTH_MODE" == "api-key" && -z "${OPENAI_API_KEY:-}" ]]; th echo "ERROR: OPENCLAW_LIVE_CODEX_HARNESS_AUTH=api-key requires OPENAI_API_KEY." >&2 exit 1 fi +if [[ "$CODEX_HARNESS_AUTH_MODE" != "api-key" && ! -s "$HOME/.codex/auth.json" ]]; then + echo "ERROR: OPENCLAW_LIVE_CODEX_HARNESS_AUTH=codex-auth requires ~/.codex/auth.json before building the live Docker image." >&2 + if [[ -n "${OPENAI_API_KEY:-}" ]]; then + echo "If this is a Testbox/API-key run, set OPENCLAW_LIVE_CODEX_HARNESS_AUTH=api-key and run through openclaw-testbox-env." >&2 + fi + exit 1 +fi cleanup_temp_dirs() { if ((${#TEMP_DIRS[@]} > 0)); then @@ -60,7 +73,7 @@ trap cleanup_temp_dirs EXIT if [[ -n "${OPENCLAW_DOCKER_CLI_TOOLS_DIR:-}" ]]; then CLI_TOOLS_DIR="${OPENCLAW_DOCKER_CLI_TOOLS_DIR}" -elif [[ "${CI:-}" == "true" || "${GITHUB_ACTIONS:-}" == "true" ]]; then +elif openclaw_live_codex_harness_is_ci; then CLI_TOOLS_DIR="$(mktemp -d "${RUNNER_TEMP:-/tmp}/openclaw-docker-cli-tools.XXXXXX")" TEMP_DIRS+=("$CLI_TOOLS_DIR") else @@ -68,7 +81,7 @@ else fi if [[ -n "${OPENCLAW_DOCKER_CACHE_HOME_DIR:-}" ]]; then CACHE_HOME_DIR="${OPENCLAW_DOCKER_CACHE_HOME_DIR}" -elif [[ "${CI:-}" == "true" || "${GITHUB_ACTIONS:-}" == "true" ]]; then +elif openclaw_live_codex_harness_is_ci; then CACHE_HOME_DIR="$(mktemp -d "${RUNNER_TEMP:-/tmp}/openclaw-docker-cache.XXXXXX")" TEMP_DIRS+=("$CACHE_HOME_DIR") else @@ -77,7 +90,10 @@ fi mkdir -p "$CLI_TOOLS_DIR" mkdir -p "$CACHE_HOME_DIR" -if [[ "${CI:-}" == "true" || "${GITHUB_ACTIONS:-}" == "true" ]]; then +if openclaw_live_codex_harness_is_ci; then + chmod 0777 "$CLI_TOOLS_DIR" "$CACHE_HOME_DIR" || true +fi +if openclaw_live_codex_harness_is_ci; then DOCKER_USER="$(id -u):$(id -g)" DOCKER_HOME_DIR="$(mktemp -d "${RUNNER_TEMP:-/tmp}/openclaw-docker-home.XXXXXX")" TEMP_DIRS+=("$DOCKER_HOME_DIR") @@ -146,6 +162,11 @@ export XDG_CACHE_HOME="${XDG_CACHE_HOME:-$HOME/.cache}" export COREPACK_HOME="${COREPACK_HOME:-$XDG_CACHE_HOME/node/corepack}" export NPM_CONFIG_CACHE="${NPM_CONFIG_CACHE:-$XDG_CACHE_HOME/npm}" export npm_config_cache="$NPM_CONFIG_CACHE" +if [ "${OPENCLAW_LIVE_CODEX_HARNESS_DEBUG:-}" = "1" ]; then + id + mount | grep -E 'openclaw-cache|openclaw-npm|/home/node' || true + ls -ld "$HOME" "$XDG_CACHE_HOME" "$NPM_CONFIG_PREFIX" 2>/dev/null || true +fi # Force the Codex harness to use the staged `~/.codex` auth files. This lane # is not meant to exercise raw OpenAI API-key routing unless the lane # explicitly opts into API-key auth for CI. @@ -254,6 +275,12 @@ DOCKER_RUN_ARGS=(docker run --rm -t \ --entrypoint bash \ -e COREPACK_ENABLE_DOWNLOAD_PROMPT=0 \ -e HOME=/home/node \ + -e NPM_CONFIG_PREFIX="$DOCKER_CLI_TOOLS_CONTAINER_DIR" \ + -e npm_config_prefix="$DOCKER_CLI_TOOLS_CONTAINER_DIR" \ + -e XDG_CACHE_HOME="$DOCKER_CACHE_CONTAINER_DIR" \ + -e COREPACK_HOME="$DOCKER_CACHE_CONTAINER_DIR/node/corepack" \ + -e NPM_CONFIG_CACHE="$DOCKER_CACHE_CONTAINER_DIR/npm" \ + -e npm_config_cache="$DOCKER_CACHE_CONTAINER_DIR/npm" \ -e NODE_OPTIONS=--disable-warning=ExperimentalWarning \ -e OPENCLAW_AGENT_HARNESS_FALLBACK=none \ -e OPENCLAW_DOCKER_AUTH_PRESTAGED="$DOCKER_AUTH_PRESTAGED" \ @@ -287,14 +314,22 @@ openclaw_live_append_array DOCKER_RUN_ARGS DOCKER_EXTRA_ENV_FILES openclaw_live_append_array DOCKER_RUN_ARGS DOCKER_HOME_MOUNT openclaw_live_append_array DOCKER_RUN_ARGS DOCKER_TRUSTED_HARNESS_MOUNT DOCKER_RUN_ARGS+=(\ - -v "$CACHE_HOME_DIR":/home/node/.cache \ + -v "$CACHE_HOME_DIR":"$DOCKER_CACHE_CONTAINER_DIR" \ -v "$ROOT_DIR":/src:ro \ -v "$CONFIG_DIR":/home/node/.openclaw \ -v "$WORKSPACE_DIR":/home/node/.openclaw/workspace \ - -v "$CLI_TOOLS_DIR":/home/node/.npm-global) + -v "$CLI_TOOLS_DIR":"$DOCKER_CLI_TOOLS_CONTAINER_DIR") openclaw_live_append_array DOCKER_RUN_ARGS EXTERNAL_AUTH_MOUNTS openclaw_live_append_array DOCKER_RUN_ARGS PROFILE_MOUNT DOCKER_RUN_ARGS+=(\ "$LIVE_IMAGE_NAME" \ -lc "$LIVE_TEST_CMD") +if [[ "${OPENCLAW_LIVE_CODEX_HARNESS_DEBUG:-}" == "1" ]]; then + echo "==> Docker debug: host ids and mounted dirs" + id + ls -ld "$CACHE_HOME_DIR" "$CLI_TOOLS_DIR" "${DOCKER_HOME_DIR:-$HOME}" 2>/dev/null || true + printf '==> Docker debug args:' + printf ' %q' "${DOCKER_RUN_ARGS[@]}" + printf '\n' +fi "${DOCKER_RUN_ARGS[@]}" diff --git a/src/agents/agent-command.ts b/src/agents/agent-command.ts index e47193dedc6..d2b0ae4f488 100644 --- a/src/agents/agent-command.ts +++ b/src/agents/agent-command.ts @@ -1031,14 +1031,6 @@ async function agentCommandInternal( currentTurnUserMessagePersisted = true; }, onAgentEvent: (evt) => { - if (evt.stream.startsWith("codex_app_server.")) { - emitAgentEvent({ - runId, - stream: evt.stream, - data: evt.data ?? {}, - ...(evt.sessionKey ? { sessionKey: evt.sessionKey } : {}), - }); - } if ( evt.stream === "lifecycle" && typeof evt.data?.phase === "string" && diff --git a/src/auto-reply/reply/agent-runner-execution.test.ts b/src/auto-reply/reply/agent-runner-execution.test.ts index 0274293bab8..cdcc4b29b71 100644 --- a/src/auto-reply/reply/agent-runner-execution.test.ts +++ b/src/auto-reply/reply/agent-runner-execution.test.ts @@ -1176,7 +1176,7 @@ describe("runAgentTurnWithFallback", () => { }); }); - it("publishes Codex app-server telemetry to agent event subscribers", async () => { + it("leaves Codex app-server telemetry publication to the harness", async () => { const agentEvents = await import("../../infra/agent-events.js"); const emitAgentEvent = vi.mocked(agentEvents.emitAgentEvent); state.runEmbeddedPiAgentMock.mockImplementationOnce(async (params: EmbeddedAgentParams) => { @@ -1217,15 +1217,12 @@ describe("runAgentTurnWithFallback", () => { }); expect(result.kind).toBe("success"); - expect(emitAgentEvent).toHaveBeenCalledWith({ - runId: "run-codex", - stream: "codex_app_server.guardian", - sessionKey: "agent:main:subagent:codex-child", - data: { - phase: "blocked", - message: "command requires approval", - }, - }); + expect(emitAgentEvent).not.toHaveBeenCalledWith( + expect.objectContaining({ + runId: "run-codex", + stream: "codex_app_server.guardian", + }), + ); }); it("emits an embedded lifecycle terminal backstop when the runner returns without one", async () => { diff --git a/src/auto-reply/reply/agent-runner-execution.ts b/src/auto-reply/reply/agent-runner-execution.ts index f80869bed86..984e41bc05d 100644 --- a/src/auto-reply/reply/agent-runner-execution.ts +++ b/src/auto-reply/reply/agent-runner-execution.ts @@ -1509,14 +1509,6 @@ export async function runAgentTurnWithFallback(params: { onReasoningEnd: params.opts?.onReasoningEnd, onAgentEvent: async (evt) => { lifecycleBackstop.note(evt); - if (evt.stream.startsWith("codex_app_server.")) { - emitAgentEvent({ - runId, - stream: evt.stream, - data: evt.data, - ...(evt.sessionKey ? { sessionKey: evt.sessionKey } : {}), - }); - } // Signal run start only after the embedded agent emits real activity. const hasLifecyclePhase = evt.stream === "lifecycle" && typeof evt.data.phase === "string"; diff --git a/src/auto-reply/reply/followup-runner.ts b/src/auto-reply/reply/followup-runner.ts index 7df8162bf08..cc75197816e 100644 --- a/src/auto-reply/reply/followup-runner.ts +++ b/src/auto-reply/reply/followup-runner.ts @@ -15,7 +15,7 @@ import { import type { SessionEntry } from "../../config/sessions.js"; import type { TypingMode } from "../../config/types.js"; import { logVerbose } from "../../globals.js"; -import { emitAgentEvent, registerAgentRunContext } from "../../infra/agent-events.js"; +import { registerAgentRunContext } from "../../infra/agent-events.js"; import { formatErrorMessage } from "../../infra/errors.js"; import { defaultRuntime } from "../../runtime.js"; import { isInternalMessageChannel } from "../../utils/message-channel.js"; @@ -332,14 +332,6 @@ export function createFollowupRunner(params: { bootstrapPromptWarningSignaturesSeen.length - 1 ], onAgentEvent: (evt) => { - if (evt.stream.startsWith("codex_app_server.")) { - emitAgentEvent({ - runId, - stream: evt.stream, - data: evt.data, - ...(evt.sessionKey ? { sessionKey: evt.sessionKey } : {}), - }); - } if (evt.stream !== "compaction") { return; } diff --git a/src/commands/agent.test.ts b/src/commands/agent.test.ts index ab8f3d9bf66..74eabf2ea52 100644 --- a/src/commands/agent.test.ts +++ b/src/commands/agent.test.ts @@ -588,6 +588,44 @@ describe("agentCommand", () => { }); }); + it("does not publish Codex app-server events from the core command callback", async () => { + await withTempHome(async (home) => { + const store = path.join(home, "sessions.json"); + mockConfig(home, store); + + const codexEvents: Array<{ runId: string; phase?: string }> = []; + const stop = onAgentEvent((evt) => { + if (evt.stream !== "codex_app_server.lifecycle") { + return; + } + codexEvents.push({ + runId: evt.runId, + phase: typeof evt.data?.phase === "string" ? evt.data.phase : undefined, + }); + }); + + vi.mocked(runEmbeddedPiAgent).mockImplementationOnce(async (params) => { + ( + params as { + onAgentEvent?: (evt: { stream: string; data: Record }) => void; + } + ).onAgentEvent?.({ + stream: "codex_app_server.lifecycle", + data: { phase: "startup" }, + }); + return { + payloads: [{ text: "hello" }], + meta: { agentMeta: { provider: "p", model: "m" } }, + } as never; + }); + + await agentCommand({ message: "hi", to: "+1555", thinking: "low" }, runtime); + stop(); + + expect(codexEvents).toHaveLength(0); + }); + }); + it("uses default fallback list for auto session model overrides", async () => { await withTempHome(async (home) => { const store = path.join(home, "sessions.json"); diff --git a/src/gateway/test-helpers.server.ts b/src/gateway/test-helpers.server.ts index e604300dd6a..c47d9b7bf32 100644 --- a/src/gateway/test-helpers.server.ts +++ b/src/gateway/test-helpers.server.ts @@ -28,6 +28,8 @@ import { } from "../routing/session-key.js"; import { normalizeLowercaseStringOrEmpty } from "../shared/string-coerce.js"; import { normalizeOptionalString } from "../shared/string-coerce.js"; +import { resetTaskRegistryForTests } from "../tasks/runtime-internal.js"; +import { resetTaskFlowRegistryForTests } from "../tasks/task-flow-runtime-internal.js"; import { captureEnv } from "../test-utils/env.js"; import { getDeterministicFreePortBlock } from "../test-utils/ports.js"; import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES } from "../utils/message-channel.js"; @@ -254,6 +256,8 @@ async function resetGatewayTestState(options: { uniqueConfigRoot: boolean }) { } applyGatewaySkipEnv(); delete process.env.OPENCLAW_GATEWAY_TOKEN; + resetTaskRegistryForTests({ persist: false }); + resetTaskFlowRegistryForTests({ persist: false }); const stateDir = process.env.OPENCLAW_STATE_DIR; if (stateDir) { await fs.rm(stateDir, { @@ -365,6 +369,8 @@ async function cleanupGatewayTestHome(options: { restoreEnv: boolean }) { vi.useRealTimers(); clearGatewaySubagentRuntime(); resetLogger(); + resetTaskRegistryForTests({ persist: false }); + resetTaskFlowRegistryForTests({ persist: false }); if (options.restoreEnv) { gatewayEnvSnapshot?.restore(); gatewayEnvSnapshot = undefined; diff --git a/src/infra/run-node.test.ts b/src/infra/run-node.test.ts index b621158853b..d4bac6516ae 100644 --- a/src/infra/run-node.test.ts +++ b/src/infra/run-node.test.ts @@ -834,6 +834,46 @@ describe("run-node script", () => { }); }); + it("runs QA coverage report from source without rebuilding private QA dist", async () => { + await withTempDir({ prefix: "openclaw-run-node-" }, async (tmp) => { + await setupTrackedProject(tmp, { + files: { + "extensions/qa-lab/src/cli.runtime.ts": "export {};\n", + }, + buildPaths: [DIST_ENTRY, BUILD_STAMP], + }); + + const spawnCalls: string[][] = []; + const spawn = (cmd: string, args: string[]) => { + spawnCalls.push([cmd, ...args]); + return createExitedProcess(0); + }; + + const exitCode = await runNodeMain({ + cwd: tmp, + args: ["qa", "coverage", "--json"], + env: { + ...process.env, + OPENCLAW_RUNNER_LOG: "0", + }, + spawn, + execPath: process.execPath, + platform: process.platform, + }); + + expect(exitCode).toBe(0); + expect(spawnCalls).toEqual([ + [ + process.execPath, + "--import", + "tsx", + path.join(tmp, "scripts", "qa-coverage-report.ts"), + "--json", + ], + ]); + }); + }); + it("skips runtime postbuild restaging when the runtime stamp is current", async () => { await withTempDir({ prefix: "openclaw-run-node-" }, async (tmp) => { await setupTrackedProject(tmp, { diff --git a/test/scripts/changed-lanes.test.ts b/test/scripts/changed-lanes.test.ts index b3034f6dcb1..89dcc202a2a 100644 --- a/test/scripts/changed-lanes.test.ts +++ b/test/scripts/changed-lanes.test.ts @@ -8,8 +8,10 @@ import { isPackageScriptOnlyChange, } from "../../scripts/changed-lanes.mjs"; import { + buildChangedCheckTestboxArgs, createChangedCheckChildEnv, createChangedCheckPlan, + shouldDelegateChangedCheckToTestbox, } from "../../scripts/check-changed.mjs"; import { cleanupTempDirs, makeTempRepoRoot } from "../helpers/temp-repo.js"; @@ -215,6 +217,44 @@ describe("scripts/changed-lanes", () => { }); }); + it("delegates local Testbox-mode changed gates before running locally", () => { + expect( + shouldDelegateChangedCheckToTestbox(["--base", "origin/main"], { + OPENCLAW_TESTBOX: "1", + PATH: "/usr/bin", + }), + ).toBe(true); + + expect(buildChangedCheckTestboxArgs(["--base", "origin/main", "--head", "HEAD"])).toEqual([ + "testbox:run", + "--", + "OPENCLAW_TESTBOX=1", + "OPENCLAW_TESTBOX_REMOTE_RUN=1", + "pnpm", + "check:changed", + "--base", + "origin/main", + "--head", + "HEAD", + ]); + }); + + it("does not delegate dry-run, CI, or already-remote changed gates", () => { + expect(shouldDelegateChangedCheckToTestbox(["--dry-run"], { OPENCLAW_TESTBOX: "1" })).toBe( + false, + ); + expect( + shouldDelegateChangedCheckToTestbox([], { OPENCLAW_TESTBOX: "1", GITHUB_ACTIONS: "true" }), + ).toBe(false); + expect(shouldDelegateChangedCheckToTestbox([], { OPENCLAW_TESTBOX: "1", CI: "1" })).toBe(false); + expect( + shouldDelegateChangedCheckToTestbox([], { + OPENCLAW_TESTBOX: "1", + OPENCLAW_TESTBOX_REMOTE_RUN: "1", + }), + ).toBe(false); + }); + it("runs changed-check lint lanes under the parent heavy-check lock", () => { const result = detectChangedLanes(["extensions/discord/src/index.ts"]); const plan = createChangedCheckPlan(result, { env: { PATH: "/usr/bin" } }); diff --git a/test/scripts/codex-app-server-protocol-source.test.ts b/test/scripts/codex-app-server-protocol-source.test.ts new file mode 100644 index 00000000000..d9c47e93a42 --- /dev/null +++ b/test/scripts/codex-app-server-protocol-source.test.ts @@ -0,0 +1,58 @@ +import fs from "node:fs"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { resolveCodexAppServerProtocolSource } from "../../scripts/lib/codex-app-server-protocol-source.js"; +import { createScriptTestHarness } from "./test-helpers.js"; + +const { createTempDir } = createScriptTestHarness(); +const originalOpenClawCodexRepo = process.env.OPENCLAW_CODEX_REPO; + +afterEach(() => { + if (originalOpenClawCodexRepo === undefined) { + delete process.env.OPENCLAW_CODEX_REPO; + } else { + process.env.OPENCLAW_CODEX_REPO = originalOpenClawCodexRepo; + } +}); + +describe("codex app-server protocol source resolver", () => { + it("uses OPENCLAW_CODEX_REPO when provided", async () => { + const root = createTempDir("openclaw-protocol-source-root-"); + const codexRepo = createTempDir("openclaw-protocol-source-codex-"); + createProtocolSchema(codexRepo); + process.env.OPENCLAW_CODEX_REPO = codexRepo; + + await expect(resolveCodexAppServerProtocolSource(root)).resolves.toEqual({ + codexRepo, + sourceRoot: path.join(codexRepo, "codex-rs/app-server-protocol/schema"), + }); + }); + + it("finds the primary checkout sibling from a git worktree", async () => { + const parentDir = createTempDir("openclaw-protocol-source-parent-"); + const primaryOpenClaw = path.join(parentDir, "openclaw"); + const codexRepo = path.join(parentDir, "codex"); + const worktreeRoot = createTempDir("openclaw-protocol-source-worktree-"); + fs.mkdirSync(path.join(primaryOpenClaw, ".git", "worktrees", "codex-harness"), { + recursive: true, + }); + fs.mkdirSync(worktreeRoot, { recursive: true }); + fs.writeFileSync( + path.join(worktreeRoot, ".git"), + `gitdir: ${path.join(primaryOpenClaw, ".git", "worktrees", "codex-harness")}\n`, + ); + createProtocolSchema(codexRepo); + delete process.env.OPENCLAW_CODEX_REPO; + + await expect(resolveCodexAppServerProtocolSource(worktreeRoot)).resolves.toMatchObject({ + codexRepo, + sourceRoot: path.join(codexRepo, "codex-rs/app-server-protocol/schema"), + }); + }); +}); + +function createProtocolSchema(codexRepo: string): void { + fs.mkdirSync(path.join(codexRepo, "codex-rs/app-server-protocol/schema/typescript"), { + recursive: true, + }); +} diff --git a/test/scripts/postinstall-bundled-plugins.test.ts b/test/scripts/postinstall-bundled-plugins.test.ts index 0184e6d0f62..744fea8792c 100644 --- a/test/scripts/postinstall-bundled-plugins.test.ts +++ b/test/scripts/postinstall-bundled-plugins.test.ts @@ -1,4 +1,4 @@ -import { readFileSync as readFileSyncOriginal } from "node:fs"; +import { existsSync as existsSyncOriginal, readFileSync as readFileSyncOriginal } from "node:fs"; import fs from "node:fs/promises"; import { tmpdir } from "node:os"; import path from "node:path"; @@ -51,6 +51,13 @@ async function writePluginPackage( } describe("bundled plugin postinstall", () => { + function existsSyncWithoutGlobalCompileCache(value: string) { + if (path.resolve(value) === path.join(tmpdir(), "node-compile-cache")) { + return false; + } + return existsSyncOriginal(value); + } + it("recognizes direct invocation through symlinked temp prefixes", () => { const realpathSync = vi.fn((value: string) => value.replace(/^\/var\/folders\//u, "/private/var/folders/"), @@ -448,6 +455,7 @@ describe("bundled plugin postinstall", () => { STATE_DIRECTORY: systemState, }, packageRoot, + existsSync: existsSyncWithoutGlobalCompileCache, log, }); diff --git a/test/scripts/run-oxlint.test.ts b/test/scripts/run-oxlint.test.ts index c8fe9daeb42..319dd3d3203 100644 --- a/test/scripts/run-oxlint.test.ts +++ b/test/scripts/run-oxlint.test.ts @@ -49,6 +49,27 @@ describe("run-oxlint", () => { hadExplicitTargets: true, remainingExplicitTargets: 1, skippedTargets: ["ui", "packages"], + skippedConfigs: [], + }); + }); + + it("filters tracked tsconfig files missing from sparse checkouts", () => { + const result = filterSparseMissingOxlintTargets( + ["--tsconfig", "config/tsconfig/oxlint.core.json", "src"], + { + fileExists: (target: string) => target.endsWith("/src"), + isSparseCheckoutEnabled: () => true, + isTrackedPath: ({ target }: { target: string }) => + target === "config/tsconfig/oxlint.core.json", + }, + ); + + expect(result).toEqual({ + args: ["src"], + hadExplicitTargets: true, + remainingExplicitTargets: 1, + skippedTargets: [], + skippedConfigs: ["config/tsconfig/oxlint.core.json"], }); }); @@ -63,6 +84,7 @@ describe("run-oxlint", () => { args: ["src", "typo"], remainingExplicitTargets: 2, skippedTargets: [], + skippedConfigs: [], }); }); }); diff --git a/test/scripts/test-live-codex-harness-docker.test.ts b/test/scripts/test-live-codex-harness-docker.test.ts new file mode 100644 index 00000000000..ff1ab0da32f --- /dev/null +++ b/test/scripts/test-live-codex-harness-docker.test.ts @@ -0,0 +1,40 @@ +import fs from "node:fs"; +import path from "node:path"; +import { describe, expect, it } from "vitest"; + +const SCRIPT_PATH = path.resolve( + import.meta.dirname, + "../../scripts/test-live-codex-harness-docker.sh", +); + +describe("scripts/test-live-codex-harness-docker.sh", () => { + it("mounts cache and npm tool dirs outside the bind-mounted Docker home", () => { + const script = fs.readFileSync(SCRIPT_PATH, "utf8"); + + expect(script).toContain('DOCKER_CACHE_CONTAINER_DIR="/tmp/openclaw-cache"'); + expect(script).toContain('DOCKER_CLI_TOOLS_CONTAINER_DIR="/tmp/openclaw-npm-global"'); + expect(script).toContain("openclaw_live_codex_harness_is_ci()"); + expect(script).toContain('[[ -n "${CI:-}" && "${CI:-}" != "false" ]]'); + expect(script).toContain('-e XDG_CACHE_HOME="$DOCKER_CACHE_CONTAINER_DIR"'); + expect(script).toContain('-e NPM_CONFIG_PREFIX="$DOCKER_CLI_TOOLS_CONTAINER_DIR"'); + expect(script).toContain('chmod 0777 "$CLI_TOOLS_DIR" "$CACHE_HOME_DIR" || true'); + expect(script).toContain('-v "$CACHE_HOME_DIR":"$DOCKER_CACHE_CONTAINER_DIR"'); + expect(script).toContain('-v "$CLI_TOOLS_DIR":"$DOCKER_CLI_TOOLS_CONTAINER_DIR"'); + expect(script).not.toContain('-v "$CACHE_HOME_DIR":/home/node/.cache'); + expect(script).not.toContain('-v "$CLI_TOOLS_DIR":/home/node/.npm-global'); + }); + + it("fails before Docker build when codex-auth has no host auth file", () => { + const script = fs.readFileSync(SCRIPT_PATH, "utf8"); + + expect(script).toContain( + "OPENCLAW_LIVE_CODEX_HARNESS_AUTH=codex-auth requires ~/.codex/auth.json before building the live Docker image", + ); + expect(script).toContain( + "If this is a Testbox/API-key run, set OPENCLAW_LIVE_CODEX_HARNESS_AUTH=api-key and run through openclaw-testbox-env.", + ); + expect(script.indexOf("requires ~/.codex/auth.json before building")).toBeLessThan( + script.indexOf('OPENCLAW_LIVE_DOCKER_REPO_ROOT="$ROOT_DIR"'), + ); + }); +});