From 63de499e1add6bd05e3baaeecd16402fa65d0095 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sat, 31 Oct 2026 10:37:36 +0900 Subject: [PATCH 01/13] chore(release): open dev at 2.96.0 before releasing 2.95.0 (#8033) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- desktop/src-tauri/Cargo.lock | 2 +- desktop/src-tauri/Cargo.toml | 2 +- desktop/src-tauri/tauri.conf.json | 2 +- package.json | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/desktop/src-tauri/Cargo.lock b/desktop/src-tauri/Cargo.lock index 92ee4e1fbd9..8f416f48dc5 100644 --- a/desktop/src-tauri/Cargo.lock +++ b/desktop/src-tauri/Cargo.lock @@ -2645,7 +2645,7 @@ dependencies = [ [[package]] name = "opencodex-desktop" -version = "2.95.0" +version = "2.96.0" dependencies = [ "base64 0.22.1", "dbus", diff --git a/desktop/src-tauri/Cargo.toml b/desktop/src-tauri/Cargo.toml index 9407ec769d7..385e89c5464 100644 --- a/desktop/src-tauri/Cargo.toml +++ b/desktop/src-tauri/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "opencodex-desktop" -version = "2.95.0" +version = "2.96.0" description = "OpenCodex desktop shell" authors = ["OpenCodex contributors"] license = "MIT" diff --git a/desktop/src-tauri/tauri.conf.json b/desktop/src-tauri/tauri.conf.json index d5c8dfbe104..38ac6967560 100644 --- a/desktop/src-tauri/tauri.conf.json +++ b/desktop/src-tauri/tauri.conf.json @@ -1,7 +1,7 @@ { "$schema": "https://schema.tauri.app/config/2", "productName": "OpenCodex", - "version": "2.95.0", + "version": "2.96.0", "identifier": "com.opencodex.desktop", "build": { "frontendDist": "../ui", diff --git a/package.json b/package.json index 587d24507bc..1c5ea6f0687 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@bitkyc08/opencodex", - "version": "2.95.0", + "version": "2.96.0", "description": "Universal provider proxy for OpenAI Codex & Claude Code — use any LLM with Codex CLI/App/SDK and Claude Code", "type": "module", "main": "./bin/package-main.mjs", From eb6f1e52c1bbe9229499459b4dc357793d82f0c1 Mon Sep 17 00:00:00 2001 From: Hojoon Kim <99799197+HojooonKim@users.noreply.github.com> Date: Sat, 31 Oct 2026 22:16:36 +0900 Subject: [PATCH 02/13] feat(messaging): add opt-in MCP tools for stored Codex queues (#7999) * feat(messaging): add opt-in MCP tools for stored Codex queues * fix(cli): include MCP syntax in messaging usage --- .../content/docs/reference/cli/messaging.md | 67 +++++++++++- .../docs/tr/reference/cli/messaging.md | 8 +- scripts/test-layout/layout.json | 2 +- .../ocx/references/01_management_surface.md | 10 +- .../references/01_surface_agents-routing.md | 20 +++- src/cli/capabilities-agents-routing.ts | 9 ++ src/cli/message-command.ts | 3 +- src/cli/message-mcp-command.ts | 29 +++++ src/cli/registry.ts | 2 +- src/messaging/mcp-queue-rpc.ts | 102 ++++++++++++++++++ src/messaging/mcp-queue-server.ts | 42 ++++++++ src/messaging/mcp-queue-tools.ts | 39 +++++++ structure/INDEX.md | 2 +- structure/cli-management.md | 2 +- structure/local-messaging.md | 48 ++++++++- structure/manifest.json | 2 +- tests/cli/cli-message-mcp.test.ts | 69 ++++++++++++ .../messaging-local-lifecycle.test.ts | 4 +- .../messaging-mcp-queue.test.ts | 98 +++++++++++++++++ tests/fixtures/test-layout-expected.json | 2 + tests/helpers/messaging-mcp-queue.ts | 49 +++++++++ 21 files changed, 593 insertions(+), 16 deletions(-) create mode 100644 src/cli/message-mcp-command.ts create mode 100644 src/messaging/mcp-queue-rpc.ts create mode 100644 src/messaging/mcp-queue-server.ts create mode 100644 src/messaging/mcp-queue-tools.ts create mode 100644 tests/cli/cli-message-mcp.test.ts create mode 100644 tests/codex-integration/messaging-mcp-queue.test.ts create mode 100644 tests/helpers/messaging-mcp-queue.ts diff --git a/docs-site/src/content/docs/reference/cli/messaging.md b/docs-site/src/content/docs/reference/cli/messaging.md index ef90c846e13..45bb05ce008 100644 --- a/docs-site/src/content/docs/reference/cli/messaging.md +++ b/docs-site/src/content/docs/reference/cli/messaging.md @@ -3,12 +3,77 @@ title: Codex Messaging description: Queue correlated messages to loaded Codex sessions locally or through explicit SSH peers. --- -`ocx message` is an opt-in surface for existing Codex sessions. +`ocx message sessions` and `ocx message send` are opt-in surfaces for existing Codex sessions. It starts no proxy or app-server, resumes no thread and installs no skill. Linux/macOS Unix sockets are supported; Windows fails explicitly. The existing Codex daemon must support experimental `thread/queue/add`. No CLI version pin or `CODEX_CLI_PATH` selection is required. +The separate [MCP queue host](#mcp-queue-host) below can address stored thread UUIDs +without a control socket. It does not change the loaded-only commands above. + +## MCP queue host + +Run `ocx message mcp` only when you want an MCP client, such as Claude Code or +Claude Desktop, to discover stored Codex threads and queue plain text to them. +The host runs over stdio; it exposes no network listener and registers itself in +no client. Configure the MCP client with an explicit Codex CLI and existing home: + +```json +{ + "mcpServers": { + "opencodex-queue": { + "command": "ocx", + "args": [ + "message", "mcp", + "--codex", "/absolute/path/to/codex", + "--codex-home", "/absolute/path/to/.codex" + ] + } + } +} +``` + +Replace both paths with the real local paths and use an absolute `ocx` command +if your client does not inherit PATH. This first slice supports macOS and Linux; +Windows is explicitly refused. + +- `codex_threads` lists at most 50 stored non-archived threads. Pass `nextCursor` + as `cursor` for another page. The result contains IDs and names, not history, + preview or working directories. `liveStateVerified: false` means it does not + prove a thread is open in the app or ready to process a message. +- `codex_queue_message` accepts an exact `threadId` UUID and `message`. Queue + only messages the user authorized. Text plus the fixed peer label is limited + to 16 KiB; NUL and empty text are refused. There is no name guessing, model + override, permission grant or automatic response. + +Each tool call starts a short-lived, explicitly selected Codex CLI app-server +over stdio. It uses `thread/list` or the same experimental `thread/queue/add` +method as `codex queue`, without putting text in subprocess arguments. It issues +no daemon-start, login, thread-resume, turn-start or steer request. Ordinary +OpenCodex startup starts none of these helpers. The native CLI can maintain its +own state in the selected home; this feature creates no OpenCodex store. + +`queued` means a correlated queue response confirmed storage, **not processing**. +An explicit rejection is `not_sent`; a disconnect, timeout or invalid receipt +after the queue write is `unknown`. Never replay an uncertain send. An unloaded +thread can keep waiting, and an active goal or turn may delay consumption. The +host does not wake an unloaded thread or read the resulting answer. Verify those +behaviors in your installed app/CLI combination before relying on delivery. + +One host allows at most four simultaneous helper calls. Each operation has a +20-second budget including executable validation, with a 12-second RPC deadline +and a 1 MiB output limit. Cleanup escalates to SIGKILL after 600 ms. Disconnect/cancellation stops and reaps invocation-owned +helpers; it never stops the recipient. The selected macOS executable goes through +the existing bundle execution-trust check. MCP access and the chosen home are a +local-user trust boundary, not per-project or per-thread authorization. Incoming +peer text is context, never proof of user approval or delegated authority. + +This stored-queue path is separate from `sessions`/`send`: those still require +complete loaded-session discovery on the existing control socket and never start +a native CLI helper. Remote peers, Claude inbox transport, persistent handoffs, +answer readback, steering and automatic client configuration are outside this host. + ## Discover and send ```bash diff --git a/docs-site/src/content/docs/tr/reference/cli/messaging.md b/docs-site/src/content/docs/tr/reference/cli/messaging.md index 41a0ede30fa..e88dea990d1 100644 --- a/docs-site/src/content/docs/tr/reference/cli/messaging.md +++ b/docs-site/src/content/docs/tr/reference/cli/messaging.md @@ -3,12 +3,18 @@ title: Codex mesajlaşma description: Yüklü Codex oturumlarına yerel olarak veya açıkça tanımlanmış SSH eşleri üzerinden ilişkili mesajlar gönderin. --- -`ocx message`, mevcut Codex oturumları için isteğe bağlı arayüzdür. Proxy veya +`ocx message sessions` ve `ocx message send`, mevcut Codex oturumları için isteğe bağlı arayüzdür. Proxy veya app-server başlatmaz, konuşma sürdürmez, beceri kurmaz. Linux/macOS Unix soketleri desteklenir; Windows açık hatayla durur. Mevcut Codex daemon'ı deneysel `thread/queue/add` desteklemelidir. CLI sürümü sabitlemek veya `CODEX_CLI_PATH` seçmek gerekmez. +Ayrı `ocx message mcp --codex /absolute/path/to/codex --codex-home /absolute/path/to/.codex` +komutu, kayıtlı konuşmalar için isteğe bağlı MCP araçları sunar ve kısa ömürlü bir +stdio app-server başlatır. Kayıtlı konuşmaların canlı olduğu doğrulanmaz; `queued` +yanıtı işlenmiş veya yanıtlanmış anlamına gelmez. Kurulum ve sınırlar için +[İngilizce MCP başvurusuna](/reference/cli/messaging/#mcp-queue-host) bakın. + ## Keşfetme ve gönderme ```bash diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 3232f870b0f..9bee9e3d98d 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -618,7 +618,7 @@ "codex-desired-state.test.ts": "codex-integration", "codex-entitlement-identity-read-fence.test.ts": "codex-integration", "codex-envkey-admission-substitution.test.ts": "codex-integration", - "codex-exec-invocation.test.ts": "codex-integration", "messaging-local-discovery.test.ts": "codex-integration", "messaging-local-cli.test.ts": "codex-integration", "messaging-local-envelope.test.ts": "codex-integration", "messaging-remote-contract.test.ts": "codex-integration", "messaging-remote-store.test.ts": "codex-integration", "messaging-remote-send.test.ts": "codex-integration", + "codex-exec-invocation.test.ts": "codex-integration", "messaging-local-discovery.test.ts": "codex-integration", "messaging-local-cli.test.ts": "codex-integration", "messaging-local-envelope.test.ts": "codex-integration", "messaging-remote-contract.test.ts": "codex-integration", "messaging-remote-store.test.ts": "codex-integration", "messaging-remote-send.test.ts": "codex-integration", "messaging-mcp-queue.test.ts": "codex-integration", "cli-message-mcp.test.ts": "cli", "messaging-local-send.test.ts": "codex-integration", "messaging-local-lifecycle.test.ts": "codex-integration", "messaging-local-socket.test.ts": "codex-integration", "codex-features-cache.test.ts": "codex-integration", "messaging-remote-enrollment.test.ts": "codex-integration", "messaging-remote-lifecycle.test.ts": "codex-integration", "messaging-remote-interop.test.ts": "codex-integration", "messaging-remote-native.test.ts": "codex-integration", "messaging-remote-process.test.ts": "codex-integration", "messaging-remote-admission.test.ts": "codex-integration", "messaging-remote-replies.test.ts": "codex-integration", "messaging-remote-scratch.test.ts": "codex-integration", "messaging-remote-gaps.test.ts": "codex-integration", "messaging-remote-enrollment-gaps.test.ts": "codex-integration", diff --git a/skills/ocx/references/01_management_surface.md b/skills/ocx/references/01_management_surface.md index 4a1e28959bd..45326e7a834 100644 --- a/skills/ocx/references/01_management_surface.md +++ b/skills/ocx/references/01_management_surface.md @@ -28,7 +28,7 @@ These answer in the CLI head and never reach the proxy, so they work with nothin | [lifecycle](01_surface_lifecycle.md) | 14 | | [providers-models](01_surface_providers-models.md) | 50 | | [accounts](01_surface_accounts.md) | 42 | -| [agents-routing](01_surface_agents-routing.md) | 67 | +| [agents-routing](01_surface_agents-routing.md) | 68 | | [integrations](01_surface_integrations.md) | 41 | | [observe-system](01_surface_observe-system.md) | 98 | | [access-remote](01_surface_access-remote.md) | 33 | @@ -671,6 +671,10 @@ Original invocation order. These headings preserve links to the previous single- [State-changing task](01_surface_agents-routing.md#ocx-message-send) +### `ocx message mcp` + +[State-changing task](01_surface_agents-routing.md#ocx-message-mcp) + ### `ocx advisor` [State-changing task](01_surface_agents-routing.md#ocx-advisor) @@ -1513,6 +1517,6 @@ Original invocation order. These headings preserve links to the previous single- ## Counts -- declared capabilities: 366 -- of those, state-changing: 224 +- declared capabilities: 367 +- of those, state-changing: 225 - head-resolved invocations: 2 diff --git a/skills/ocx/references/01_surface_agents-routing.md b/skills/ocx/references/01_surface_agents-routing.md index 27f730badc9..63866fbb7a3 100644 --- a/skills/ocx/references/01_surface_agents-routing.md +++ b/skills/ocx/references/01_surface_agents-routing.md @@ -8,7 +8,7 @@ Use these declarations to choose a task, then check its flags and authority before execution. Non-mutating probes may still contact providers, consume quota or refresh caches. -Declared capabilities: 67. +Declared capabilities: 68. ### `ocx agent subagents force` @@ -180,6 +180,24 @@ Drives no management route. - queued means submitted, not processed. unknown must not be replayed; no automatic retry, daemon start or thread resume. - Exit 0: queued; 1: not sent; 3: unknown; 64: invalid usage. No Claude transport, native lifecycle mutation or skill installation. +### `ocx message mcp` + +Usage: `ocx message mcp --codex --codex-home ` + +Serve opt-in MCP tools for stored Codex metadata and queued text delivery over stdio. + +State-changing: yes. JSON mode: `none`. + +Drives no management route. + +| Flag | Value | Meaning | +|---|---|---| +| `--codex` | string | Explicit local Codex CLI executable. | +| `--codex-home` | string | Explicit existing Codex home; no creation or login. | + +- macOS/Linux only. No proxy/control socket required. Starts bounded stdio helpers only when a tool is called; existing sessions/send behavior stays unchanged. +- Stored threads are not verified live sessions. queued is not processing; unknown is never replayed. No daemon start, thread resume, steer, automatic replies or registration. + ### `ocx advisor` Inspect and configure the advisor sidecar (expert consultation for routed workers). diff --git a/src/cli/capabilities-agents-routing.ts b/src/cli/capabilities-agents-routing.ts index a49ba0a3470..b8e04c348b1 100644 --- a/src/cli/capabilities-agents-routing.ts +++ b/src/cli/capabilities-agents-routing.ts @@ -26,6 +26,15 @@ export const AGENT_ROUTING_CAPABILITIES: readonly Capability[] = [ "queued means submitted, not processed. unknown must not be replayed; no automatic retry, daemon start or thread resume.", "Exit 0: queued; 1: not sent; 3: unknown; 64: invalid usage. No Claude transport, native lifecycle mutation or skill installation."], }, + { + command: ["message", "mcp"], usage: "ocx message mcp --codex --codex-home ", + summary: "Serve opt-in MCP tools for stored Codex metadata and queued text delivery over stdio.", + routes: [], mutates: true, json: "none", + flags: [{ name: "--codex", value: "string", required: true, summary: "Explicit local Codex CLI executable." }, + { name: "--codex-home", value: "string", required: true, summary: "Explicit existing Codex home; no creation or login." }], + details: ["macOS/Linux only. No proxy/control socket required. Starts bounded stdio helpers only when a tool is called; existing sessions/send behavior stays unchanged.", + "Stored threads are not verified live sessions. queued is not processing; unknown is never replayed. No daemon start, thread resume, steer, automatic replies or registration."], + }, { command: ["advisor"], summary: "Inspect and configure the advisor sidecar (expert consultation for routed workers).", diff --git a/src/cli/message-command.ts b/src/cli/message-command.ts index a4e7e88eb98..37a45993595 100644 --- a/src/cli/message-command.ts +++ b/src/cli/message-command.ts @@ -7,11 +7,12 @@ import { terminalSafeText } from "./runtime-api"; /** Run one local messaging operation after pure syntax validation, then release owned resources. */ export async function runMessageCommand(argv: readonly string[], env: NodeJS.ProcessEnv = process.env): Promise { + if (argv[0] === "mcp") return (await import("./message-mcp-command")).runMessageMcpCommand(argv.slice(1)); const remote = parseRemoteMessageArgs(argv); if (remote) return (await import("./message-remote-command")).runRemoteMessageCommand(remote, env); const args = parseMessageArgs(argv); if (!args) { - console.error("Usage: ocx message sessions [--host ] [--json] | send (--thread | --name ) --stdin [--host ] [--kind request|response|notification] [--in-reply-to ] [--json] | enable [--port ] | disable | status | hosts | serve [--host ]..."); + console.error("Usage: ocx message sessions [--host ] [--json] | send (--thread | --name ) --stdin [--host ] [--kind request|response|notification] [--in-reply-to ] [--json] | mcp --codex --codex-home | enable [--port ] | disable | status | hosts | serve [--host ]..."); return 64; } const controller = new AbortController(); diff --git a/src/cli/message-mcp-command.ts b/src/cli/message-mcp-command.ts new file mode 100644 index 00000000000..9186b44e18e --- /dev/null +++ b/src/cli/message-mcp-command.ts @@ -0,0 +1,29 @@ +import { isAbsolute } from "node:path"; +import type { QueueMcpOptions } from "../messaging/mcp-queue-rpc"; + +/** Validate all syntax before importing the MCP host or resolving any executable/home. */ +export function parseQueueMcpArgs(argv: readonly string[]): QueueMcpOptions | null { + if (argv.length !== 4) return null; + const values: Record = {}; + for (let i = 0; i < argv.length; i += 2) { + const flag = argv[i]!, value = argv[i + 1]!; + if (!["--codex", "--codex-home"].includes(flag) || values[flag] !== undefined + || !value || !isAbsolute(value) || /[\x00-\x1f]/.test(value)) return null; + values[flag] = value; + } + return values["--codex"] && values["--codex-home"] ? { codex: values["--codex"], codexHome: values["--codex-home"] } : null; +} + +export async function runMessageMcpCommand(argv: readonly string[]): Promise { + const options = parseQueueMcpArgs(argv); + if (!options) { + console.error("Usage: ocx message mcp --codex --codex-home "); + return 64; + } + if (process.platform === "win32") { + console.error("Queue MCP currently supports macOS and Linux only."); + return 1; + } + try { await (await import("../messaging/mcp-queue-server")).serveQueueMcp(options); return 0; } + catch { console.error("Queue MCP could not serve the stdio connection."); return 1; } +} diff --git a/src/cli/registry.ts b/src/cli/registry.ts index 2a73eec650c..bfab1a29715 100644 --- a/src/cli/registry.ts +++ b/src/cli/registry.ts @@ -18,7 +18,7 @@ export const CLI_COMMANDS: CliCommandEntry[] = [ }, { name: "message", - usage: "ocx message [options]", + usage: "ocx message [options]", summary: "Discover loaded Codex sessions and queue messages locally or through explicit SSH peers.", details: ["Remote listeners/tunnels require explicit foreground serve; no daemon start. queued is submission, not processing; never replay an unknown send."], }, diff --git a/src/messaging/mcp-queue-rpc.ts b/src/messaging/mcp-queue-rpc.ts new file mode 100644 index 00000000000..d526da25e09 --- /dev/null +++ b/src/messaging/mcp-queue-rpc.ts @@ -0,0 +1,102 @@ +import { spawn } from "node:child_process"; +import { statSync } from "node:fs"; +import { isAbsolute } from "node:path"; +import { validatedCodexExecInvocationAsync } from "../codex/exec-boundary"; +import { MessageBudget } from "./budget"; +import { isRecord } from "./types"; + +export interface QueueMcpOptions { codex: string; codexHome: string } +export type QueueRpcResult = { status: "response"; result: Record } + | { status: "not_sent" | "unknown"; error: { code: string } }; + +/** Execute only the stored-thread queue protocol; bodies never enter argv or diagnostics. */ +export async function queueMcpRpc(options: QueueMcpOptions, method: "thread/list" | "thread/queue/add", + params: Record, signal?: AbortSignal): Promise { + const budget = new MessageBudget(20_000, signal); + try { + budget.throwIfEnded(); + if (process.platform === "win32" || !isAbsolute(options.codex) || !isAbsolute(options.codexHome) + || !statSync(options.codexHome).isDirectory()) { + return { status: "not_sent", error: { code: "unsupported_queue_mcp_environment" } }; + } + const env: NodeJS.ProcessEnv = { ...process.env, CODEX_HOME: options.codexHome }; + delete env.CODEX_THREAD_ID; + const invocation = await validatedCodexExecInvocationAsync(options.codex, ["app-server", "--listen", "stdio://"], + { env, cwd: options.codexHome, signal: budget.signal, verifyAnyAppBundle: true }); + budget.throwIfEnded(); + return await new Promise(resolve => { + const child = spawn(invocation.file, invocation.args, { + ...invocation.options, env, cwd: options.codexHome, detached: true, stdio: ["pipe", "pipe", "ignore"], + }); + const decoder = new TextDecoder("utf-8", { fatal: true }); + let pending = "", bytes = 0, submitted = false, stage = "initialize"; + let outcome: QueueRpcResult | undefined; + let termTimer: ReturnType | undefined, killTimer: ReturnType | undefined; + const stopGroup = (kind: NodeJS.Signals): void => { + if (!child.pid) return; + try { process.kill(-child.pid, kind); } + catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ESRCH") child.kill(kind); + } + }; + const finish = (value: QueueRpcResult): void => { + if (outcome) return; + outcome = value; + clearTimeout(deadline); + child.stdin.end(); + termTimer = setTimeout(() => stopGroup("SIGTERM"), 300); + killTimer = setTimeout(() => stopGroup("SIGKILL"), 600); + }; + const fail = (code: string): void => finish({ status: submitted ? "unknown" : "not_sent", error: { code } }); + const cancel = (): void => fail("queue_mcp_cancelled"); + const write = (frame: unknown): void => { child.stdin.write(JSON.stringify(frame) + "\n"); }; + const deadline = setTimeout(() => fail("queue_mcp_timeout"), budget.remainingMs(12_000)); + budget.signal.addEventListener("abort", cancel, { once: true }); + child.stdin.on("error", () => fail("queue_mcp_stdin_failed")); + child.on("error", () => fail("queue_mcp_launch_failed")); + child.stdout.on("data", (chunk: Buffer) => { + if (outcome) return; + bytes += chunk.byteLength; + if (bytes > 1024 * 1024) { fail("queue_mcp_output_limit"); return; } + try { pending += decoder.decode(chunk, { stream: true }); } + catch { fail("queue_mcp_invalid_utf8"); return; } + let newline: number; + while (!outcome && (newline = pending.indexOf("\n")) >= 0) { + const line = pending.slice(0, newline); pending = pending.slice(newline + 1); + if (!line.trim()) continue; + let frame: unknown; + try { frame = JSON.parse(line); } catch { fail("queue_mcp_invalid_json"); break; } + if (!isRecord(frame)) { fail("queue_mcp_invalid_response"); break; } + if (frame.id === undefined) continue; + if ((stage === "initialize" && frame.id !== 1) || (stage === "request" && frame.id !== 2)) { + fail("queue_mcp_mismatched_response"); break; + } + if (frame.error !== undefined) { + if (!isRecord(frame.error) || !Number.isInteger(frame.error.code) || typeof frame.error.message !== "string" || frame.result !== undefined) { fail("queue_mcp_invalid_error"); break; } + finish({ status: "not_sent", error: { code: stage === "initialize" ? "queue_mcp_initialize_rejected" : "queue_mcp_request_rejected" } }); + break; + } + if (!isRecord(frame.result)) { fail("queue_mcp_invalid_response"); break; } + if (stage === "initialize") { + stage = "request"; + write({ method: "initialized", params: {} }); + submitted = method === "thread/queue/add"; + write({ id: 2, method, params }); + } else finish({ status: "response", result: frame.result }); + } + }); + child.on("close", () => { + clearTimeout(deadline); clearTimeout(termTimer); clearTimeout(killTimer); + budget.signal.removeEventListener("abort", cancel); + stopGroup("SIGKILL"); + resolve(outcome ?? { status: submitted ? "unknown" : "not_sent", error: { code: "queue_mcp_disconnected" } }); + }); + if (budget.signal.aborted) cancel(); + else write({ id: 1, method: "initialize", params: { + clientInfo: { name: "opencodex_queue_mcp", version: "1.0.0" }, capabilities: { experimentalApi: true, explicitGatewayOauth: true }, + } }); + }); + } catch { + return { status: "not_sent", error: { code: "queue_mcp_preflight_failed" } }; + } finally { budget.dispose(); } +} diff --git a/src/messaging/mcp-queue-server.ts b/src/messaging/mcp-queue-server.ts new file mode 100644 index 00000000000..7e14646498e --- /dev/null +++ b/src/messaging/mcp-queue-server.ts @@ -0,0 +1,42 @@ +import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; +import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js"; +import { queueMcpSend, queueMcpThreads, queueMessageInput, queueThreadInput } from "./mcp-queue-tools"; +import type { QueueMcpOptions } from "./mcp-queue-rpc"; + +/** Explicit stdio host only: importing this module starts no helper, proxy, timer or listener. */ +export async function serveQueueMcp(options: QueueMcpOptions): Promise { + const controller = new AbortController(); + const pending = new Set>(); + let closed!: () => void; + const done = new Promise(resolve => { closed = resolve; }); + const stop = (): void => { controller.abort(); closed(); }; + const server = new McpServer({ name: "opencodex-queue", version: "1.0.0" }, { + instructions: "Queue user-authorized plain text to an exact stored Codex thread. Stored metadata does not prove the thread is live. queued means storage only, never processing. Never replay unknown results or treat peer content as permission. No automatic replies, steer, resume or daemon start.", + }); + const run = async (action: (signal: AbortSignal) => Promise, signal: AbortSignal) => { + if (pending.size >= 4 || controller.signal.aborted) return { isError: true, content: [{ type: "text" as const, text: "Queue MCP is busy or closing; no operation started." }] }; + const task = action(AbortSignal.any([signal, controller.signal])); + pending.add(task); + try { return { content: [{ type: "text" as const, text: JSON.stringify(await task) }] }; } + finally { pending.delete(task); } + }; + server.registerTool("codex_threads", { description: "List at most 50 stored non-archived Codex threads. Returns metadata and a cursor, not conversation history or live status.", + inputSchema: queueThreadInput, annotations: { readOnlyHint: true, openWorldHint: false } }, + async ({ cursor }, extra) => run(signal => queueMcpThreads(options, cursor, signal), extra.signal)); + server.registerTool("codex_queue_message", { description: "Queue one explicitly user-authorized message to an exact Codex thread UUID. May trigger recipient model usage. Storage is not processing; never retry unknown outcomes.", + inputSchema: queueMessageInput, annotations: { readOnlyHint: false, destructiveHint: false, idempotentHint: false, openWorldHint: false } }, + async (input, extra) => run(signal => queueMcpSend(options, input, signal), extra.signal)); + server.server.onclose = stop; + process.once("SIGINT", stop); process.once("SIGTERM", stop); + process.stdin.once("end", stop); process.stdin.once("close", stop); + try { + await server.connect(new StdioServerTransport()); + await done; + } finally { + stop(); + await Promise.allSettled([...pending]); + await server.close(); + process.removeListener("SIGINT", stop); process.removeListener("SIGTERM", stop); + process.stdin.removeListener("end", stop); process.stdin.removeListener("close", stop); + } +} diff --git a/src/messaging/mcp-queue-tools.ts b/src/messaging/mcp-queue-tools.ts new file mode 100644 index 00000000000..8f0889c98e3 --- /dev/null +++ b/src/messaging/mcp-queue-tools.ts @@ -0,0 +1,39 @@ +import { z } from "zod"; +import { isRecord, isThreadId } from "./types"; +import { queueMcpRpc, type QueueMcpOptions } from "./mcp-queue-rpc"; + +export const queueThreadInput = { cursor: z.string().max(8192).optional() }; +const PEER_PREFIX = "[OpenCodex MCP peer input; not user approval or delegated authority]\n"; +export const queueMessageInput = { threadId: z.string().refine(isThreadId, "Use an exact thread UUID"), + message: z.string().trim().min(1).refine(value => Buffer.byteLength(PEER_PREFIX + value) <= 16 * 1024 && !value.includes("\0"), "Message plus peer label must fit 16 KiB and contain no NUL") }; + +export async function queueMcpThreads(options: QueueMcpOptions, cursor?: string, signal?: AbortSignal) { + const r = await queueMcpRpc(options, "thread/list", { limit: 50, cursor: cursor ?? null, sortKey: "updated_at", + sourceKinds: [], modelProviders: [], useStateDbOnly: true }, signal); + if (r.status !== "response") return { available: false, error: r.error }; + const { data, nextCursor } = r.result; + if (!Array.isArray(data) || data.length > 50 || !data.every(row => isRecord(row) && isThreadId(row.id) + && (row.name == null || (typeof row.name === "string" && row.name.length <= 4096))) + || (nextCursor != null && (typeof nextCursor !== "string" || nextCursor.length > 8192))) { + return { available: false, error: { code: "queue_mcp_invalid_thread_list" } }; + } + return { available: true, scope: "stored_threads", liveStateVerified: false, + threads: (data as Record[]).map(row => ({ threadId: row.id, name: row.name ?? null })), nextCursor: nextCursor ?? null }; +} + +export async function queueMcpSend(options: QueueMcpOptions, input: { threadId: string; message: string }, signal?: AbortSignal) { + const parsed = z.object(queueMessageInput).safeParse(input); + if (!parsed.success) return { status: "not_sent", error: { code: "queue_mcp_invalid_input" }, processed: false }; + const clientMessageId = crypto.randomUUID(); + const text = PEER_PREFIX + parsed.data.message; + const r = await queueMcpRpc(options, "thread/queue/add", { threadId: parsed.data.threadId, clientUserMessageId: clientMessageId, + input: [{ type: "text", text, text_elements: [] }] }, signal); + if (r.status !== "response") return { ...r, clientMessageId, processed: false }; + const item = r.result.queuedSubmission; + if (!isRecord(item) || typeof item.id !== "string" || !item.id || item.id.length > 256 || item.clientUserMessageId !== clientMessageId + || !Array.isArray(item.input) || item.input.length !== 1 || !isRecord(item.input[0]) + || item.input[0].type !== "text" || item.input[0].text !== text) { + return { status: "unknown", clientMessageId, processed: false, error: { code: "queue_mcp_invalid_receipt" } }; + } + return { status: "queued", submissionId: item.id, clientMessageId, processed: false }; +} diff --git a/structure/INDEX.md b/structure/INDEX.md index ebef961aade..05e82c619bc 100644 --- a/structure/INDEX.md +++ b/structure/INDEX.md @@ -47,7 +47,7 @@ The wire surfaces a client actually talks to. | --- | --- | | [`transports/timeouts.md`](transports/timeouts.md) | Local header and JSON body deadlines, cancellation and optional OpenCode timeout export. | | [`remote-messaging.md`](remote-messaging.md) | Explicit peer enrollment, connection-bound capabilities, bounded foreground SSH ownership and queued remote replies. | -| [`local-messaging.md`](local-messaging.md) | Command-local metadata discovery, Unix RPC queued submission, peer envelopes, receipts and owned lifecycle bounds. | +| [`local-messaging.md`](local-messaging.md) | Loaded-session Unix messaging and separate opt-in stored-queue MCP tools, receipts and owned helper bounds. | | [`transports/byte-accounting.md`](transports/byte-accounting.md) | Request-copy and stream-buffer byte accounting shared by parsing, SSE rewriting, the adapters, and the translator budget, plus durable spill admission. | | [`transports/anthropic-thinking-recovery.md`](transports/anthropic-thinking-recovery.md) | One-shot exact signature-binding recovery before output, and successful-retry memo boundaries. | | [`transports/response-turn-membership.md`](transports/response-turn-membership.md) | Embedding-owned signing material, dedicated authenticated caller/own-thread scope and pre-parser structural normalization. | diff --git a/structure/cli-management.md b/structure/cli-management.md index cd1294ef63c..79f43c0710b 100644 --- a/structure/cli-management.md +++ b/structure/cli-management.md @@ -55,7 +55,7 @@ A declaration may carry `usage`, the exact synopsis for a verified leaf. `capabi `ocx models auto-review status` and `ocx models auto-review set` read and update the global approval reviewer through the same management route as the Models panel. A refresh that can be retried is reported as pending and points to `ocx sync`; a failed refresh preserves the saved-settings receipt, exits 1, and directs operators to resolve the issue before syncing. -The agents/routing leaf declares local `message sessions` and `message send`, plus explicit remote enable/disable/status, hosts (including exact-transaction abandonment, guarded removal and local inbound consent) and serve commands. Session details distinguish local Unix discovery from authenticated loopback peer routes. Local transport belongs to [local messaging](local-messaging.md#command-local-cli); enrolled peers and foreground lifetime belong to [remote messaging](remote-messaging.md). Declarations import no messaging handlers and preserve baseline capability order. The operating-reference generator places these verbs in the agents/routing chapter. +The agents/routing leaf declares local `message sessions` and `message send`, the separate opt-in `message mcp` stdio host for stored-thread queue tools, plus explicit remote enable/disable/status, hosts (including exact-transaction abandonment, guarded removal and local inbound consent) and serve commands. Session details distinguish local Unix discovery from authenticated loopback peer routes and stored metadata from live state. Local transport belongs to [local messaging](local-messaging.md#command-local-cli); the new host belongs to [stored-queue MCP](local-messaging.md#opt-in-stored-queue-mcp-host); enrolled peers and foreground lifetime belong to [remote messaging](remote-messaging.md). Declarations import no messaging handlers and preserve existing capability order. The operating-reference generator places these verbs in the agents/routing chapter. ## Generated operating reference diff --git a/structure/local-messaging.md b/structure/local-messaging.md index 556d0f2a6fe..24aaf4c1898 100644 --- a/structure/local-messaging.md +++ b/structure/local-messaging.md @@ -2,9 +2,11 @@ `src/messaging/` contains command-owned local Codex discovery and queued peer submission. `src/cli/message-command.ts` activates it only for `ocx message`. -The local path has no management API, startup hook, enabled setting, listener, +The existing sessions/send path has no management API, startup hook, enabled setting, listener, persistent store or skill installation. Ordinary proxy startup does not activate messaging. Explicit enrolled peers belong to [remote messaging](remote-messaging.md). +The separate opt-in stored-queue MCP host is described below; it does not replace +the loaded-session transport or change its discovery/admission rules. > Decision record: [ADR-6478](decisions/ADR-6478-local-messaging-foundation.md) @@ -58,7 +60,7 @@ guarantee that a session stays loaded after discovery. parent cancellation. Callers create and dispose that budget around input, home selection, connection, discovery, revalidation and submission. Individual RPCs are also capped at 10 seconds. Modules allocate no resources at import time. -Local messaging launches no subprocess. SIGINT/SIGTERM cancel only command-owned work; +The loaded-session path launches no subprocess. SIGINT/SIGTERM cancel only command-owned work; no recipient, daemon or proxy is stopped. ## Command-local CLI @@ -150,3 +152,45 @@ obedience or a real daemon's atomic loaded-target enforcement. Remote authentica and enrollment have a [separate owner](remote-messaging.md); Claude, isolation and idle notices are absent. Independent review and architecture acceptance remain separate from this contribution implementation. + +## Opt-in stored-queue MCP host + +`src/cli/message-mcp-command.ts` parses `message mcp --codex --codex-home` before +loading the SDK. Both paths are explicit and absolute; no setup, registration, +proxy ensure or fallback into the existing sessions/send command occurs. Windows +is refused. `src/messaging/mcp-queue-server.ts` is activated only by this command, +exposes stdio tools, and allocates no helper at import or initialization time. + +`src/messaging/mcp-queue-tools.ts` exposes only `codex_threads` and +`codex_queue_message`. The first returns one bounded page of stored metadata and +a cursor, excluding preview, history and cwd. It explicitly reports that live +state is unverified. The second validates an exact UUID and at most 16 KiB of +text including a fixed peer-context label; NUL/empty text is refused. Sender +identity, model overrides and permission escalation are not part of this API. +These tools do not share the loaded-only admission contract above. + +`src/messaging/mcp-queue-rpc.ts` owns each explicitly selected CLI helper. The +existing `src/codex/exec-boundary.ts` validates actual execution, including +macOS bundle trust. Helpers use only app-server stdio and an explicit existing +Codex home; inherited CODEX_THREAD_ID is removed. After initialize/initialized, +the only allowed method is thread/list or thread/queue/add. No lifecycle or +login RPC is issued. The queue request carries a generated clientUserMessageId; +only a matching correlated receipt becomes queued. Rejection before/at a valid +error is not_sent. Uncertain outcomes after write are unknown without replay. +Queued is storage, not recipient processing. Message bodies and native output +never appear in argv, receipts or diagnostics. + +At most four operations run per host. Each has a 20-second budget including +execution validation, a 12-second RPC deadline, incremental strict UTF-8 decode +and a 1 MiB output cap. Cleanup escalates to SIGKILL after 600 ms. SDK cancellation, host disconnect and process signals +cancel owned work. EOF is followed by bounded TERM/KILL escalation and joined +child exit. Cleanup targets invocation-owned groups, never the recipient. +The native CLI may maintain state in its selected home; OpenCodex adds no store. +Local-user MCP access is not per-project/thread isolation. + +`tests/codex-integration/messaging-mcp-queue.test.ts` covers offline protocol, +limits, privacy, failure receipts and cancellation. `tests/cli/cli-message-mcp.test.ts` +exercises the actual CLI/MCP handshake, inert initialization, the four-helper +bound and disconnect cleanup. The fixtures use disposable native helpers, not +real accounts. They prove queue contracts rather than app consumption, goal +behavior, model obedience or cross-platform native interoperability. diff --git a/structure/manifest.json b/structure/manifest.json index 2857b699d95..417f9e712c4 100644 --- a/structure/manifest.json +++ b/structure/manifest.json @@ -188,7 +188,7 @@ "path": "local-messaging.md", "tier": 3, "title": "Local Codex Messaging", - "scope": "Command-local metadata discovery, Unix RPC queued submission, peer envelopes, receipts and owned lifecycle bounds.", + "scope": "Loaded-session Unix messaging and separate opt-in stored-queue MCP tools, receipts and owned helper bounds.", "documents": [ "src/messaging/", "src/cli/" diff --git a/tests/cli/cli-message-mcp.test.ts b/tests/cli/cli-message-mcp.test.ts new file mode 100644 index 00000000000..0e6646397c1 --- /dev/null +++ b/tests/cli/cli-message-mcp.test.ts @@ -0,0 +1,69 @@ +import { describe, expect, test } from "bun:test"; +import { Client } from "@modelcontextprotocol/sdk/client/index.js"; +import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"; +import { parseQueueMcpArgs } from "../../src/cli/message-mcp-command"; +import { QUEUE_MCP_TARGET, queueMcpFixture } from "../helpers/messaging-mcp-queue"; +import { repoPath } from "../helpers/repo-root"; + +test("message mcp requires exactly one explicit absolute CLI and home", () => { + expect(parseQueueMcpArgs(["--codex", "/fixture/codex", "--codex-home", "/fixture/home"])) + .toEqual({ codex: "/fixture/codex", codexHome: "/fixture/home" }); + for (const args of [[], ["--codex", "relative", "--codex-home", "/home"], ["--codex", "/bin/codex"], + ["--codex", "/a", "--codex", "/b"], ["--codex", "/a", "--codex-home", "/b", "--json"], + ["--remote", "/a", "--codex-home", "/b"], ["--codex", "/a\0b", "--codex-home", "/b"]]) { + expect(parseQueueMcpArgs(args)).toBeNull(); + } +}); + +describe.skipIf(process.platform === "win32")("message MCP CLI", () => { + test("malformed invocation writes no protocol bytes and creates no home", async () => { + const child = Bun.spawn([process.execPath, repoPath("src/cli/index.ts"), "message", "mcp"], { stdout: "pipe", stderr: "pipe" }); + const [code, out, err] = await Promise.all([child.exited, new Response(child.stdout).text(), new Response(child.stderr).text()]); + expect(code).toBe(64); expect(out).toBe(""); expect(err).toContain("Usage: ocx message mcp"); + }); + + test("real MCP handshake advertises only explicit queue tools, starts no helper until called", async () => { + const f = queueMcpFixture(); + const client = new Client({ name: "fixture", version: "1" }); + const transport = new StdioClientTransport({ command: process.execPath, + args: [repoPath("src/cli/index.ts"), "message", "mcp", "--codex", f.options.codex, "--codex-home", f.options.codexHome], stderr: "pipe" }); + try { + await client.connect(transport); + const tools = await client.listTools(); + expect(tools.tools.map(t => t.name).sort()).toEqual(["codex_queue_message", "codex_threads"]); + expect(f.calls()).toHaveLength(0); + const listed = await client.callTool({ name: "codex_threads", arguments: {} }); + const payload = JSON.parse((listed.content as { text: string }[])[0]!.text); + expect(payload.scope).toBe("stored_threads"); expect(payload.liveStateVerified).toBe(false); + const sent = await client.callTool({ name: "codex_queue_message", arguments: { threadId: QUEUE_MCP_TARGET, message: "PRIVATE_MCP_BODY" } }); + const receipt = JSON.parse((sent.content as { text: string }[])[0]!.text); + expect(receipt.status).toBe("queued"); expect(receipt.processed).toBe(false); + expect(JSON.stringify(sent)).not.toContain("PRIVATE_MCP_BODY"); + } finally { await client.close(); await f.close(); } + }, 15000); + + test("four-helper bound and MCP disconnect cancel and reap pending work", async () => { + const f = queueMcpFixture("hang"); + const client = new Client({ name: "fixture", version: "1" }); + const transport = new StdioClientTransport({ command: process.execPath, + args: [repoPath("src/cli/index.ts"), "message", "mcp", "--codex", f.options.codex, "--codex-home", f.options.codexHome], stderr: "pipe" }); + const pending: Promise[] = []; + try { + await client.connect(transport); + for (let i = 0; i < 4; i++) pending.push(client.callTool({ name: "codex_queue_message", arguments: { threadId: QUEUE_MCP_TARGET, message: "fixture" } }).catch(error => error)); + const until = performance.now() + 5000; + while (f.calls().filter(c => c.frame.method === "thread/queue/add").length < 4 && performance.now() < until) await Bun.sleep(10); + expect(f.calls().filter(c => c.frame.method === "thread/queue/add")).toHaveLength(4); + const refused = await client.callTool({ name: "codex_threads", arguments: {} }); + expect(refused.isError).toBe(true); + const pids = [...new Set(f.calls().map(c => c.pid))]; + expect(pids).toHaveLength(4); + await client.close(); await Promise.allSettled(pending); + const stoppedBy = performance.now() + 2500; + const alive = (pid: number) => { try { process.kill(pid, 0); return true; } catch { return false; } }; + while (pids.some(alive) && performance.now() < stoppedBy) await Bun.sleep(10); + expect(pids.some(alive)).toBe(false); + expect(f.calls().filter(c => c.frame.method === "thread/queue/add")).toHaveLength(4); + } finally { await client.close(); await Promise.allSettled(pending); await f.close(); } + }, 20000); +}); diff --git a/tests/codex-integration/messaging-local-lifecycle.test.ts b/tests/codex-integration/messaging-local-lifecycle.test.ts index 4dd0cfc1ea1..7358960c701 100644 --- a/tests/codex-integration/messaging-local-lifecycle.test.ts +++ b/tests/codex-integration/messaging-local-lifecycle.test.ts @@ -19,7 +19,7 @@ test("transport rejects remote URLs and unsupported platforms without opening a }); test("importing the unactivated subsystem allocates no listener, timer, child or socket", async () => { - const modules = ["types", "budget", "socket", "rpc", "discovery", "envelope", "input", "send"].map(name => repoPath("src", "messaging", `${name}.ts`)); + const modules = ["types", "budget", "socket", "rpc", "discovery", "envelope", "input", "send", "mcp-queue-rpc", "mcp-queue-tools", "mcp-queue-server"].map(name => repoPath("src", "messaging", `${name}.ts`)); const script = ` const forbidden = () => { throw new Error("unexpected messaging resource allocation"); }; globalThis.setTimeout = forbidden; @@ -35,7 +35,7 @@ test("importing the unactivated subsystem allocates no listener, timer, child or test("messaging activation is command-local, never on an ordinary proxy startup path", async () => { // Native parser inventory, not graph completeness: catches static/dynamic literal imports. - const allowed = new Set(["src/cli/message-args.ts", "src/cli/message-command.ts", "src/cli/message-remote-args.ts", "src/cli/message-remote-command.ts"]); + const allowed = new Set(["src/cli/message-args.ts", "src/cli/message-command.ts", "src/cli/message-remote-args.ts", "src/cli/message-remote-command.ts", "src/cli/message-mcp-command.ts"]); const transpiler = new Bun.Transpiler({ loader: "ts" }); for await (const scanned of new Bun.Glob("src/**/*.{ts,mts}").scan({ cwd: repoPath() })) { const path = scanned.replaceAll("\\", "/"); // Bun.Glob yields native separators on Windows. diff --git a/tests/codex-integration/messaging-mcp-queue.test.ts b/tests/codex-integration/messaging-mcp-queue.test.ts new file mode 100644 index 00000000000..18f8f7fc5b8 --- /dev/null +++ b/tests/codex-integration/messaging-mcp-queue.test.ts @@ -0,0 +1,98 @@ +import { describe, expect, test } from "bun:test"; +import { z } from "zod"; +import { queueMcpRpc } from "../../src/messaging/mcp-queue-rpc"; +import { queueMcpSend, queueMcpThreads, queueMessageInput } from "../../src/messaging/mcp-queue-tools"; +import { QUEUE_MCP_TARGET, queueMcpFixture } from "../helpers/messaging-mcp-queue"; + +describe.skipIf(process.platform === "win32")("opt-in stored-thread MCP queue", () => { + test("correlated queue request uses stdin, explicit home and no lifecycle RPCs", async () => { + const f = queueMcpFixture(); + try { + const receipt = await queueMcpSend(f.options, { threadId: QUEUE_MCP_TARGET, message: "PRIVATE_INPUT" }); + expect(receipt.status).toBe("queued"); expect(receipt.processed).toBe(false); + const calls = f.calls(); + expect(calls.map(c => c.frame.method)).toEqual(["initialize", "initialized", "thread/queue/add"]); + expect(calls[2]!.frame.params!.threadId).toBe(QUEUE_MCP_TARGET); + expect(calls[2]!.frame.params!.clientUserMessageId).toBe(receipt.clientMessageId); + expect(JSON.stringify(calls[2]!.frame.params!.input)).toContain("PRIVATE_INPUT"); + for (const c of calls) { + expect(c.argv).toEqual(["app-server", "--listen", "stdio://"]); + expect(c.home).toBe(f.options.codexHome); expect(c.sender).toBeUndefined(); + expect(JSON.stringify(c.argv)).not.toContain("PRIVATE_INPUT"); + } + expect(JSON.stringify(receipt)).not.toContain("PRIVATE_INPUT"); + } finally { await f.close(); } + }); + + test("one metadata-only page carries a cursor and does not claim live state", async () => { + const f = queueMcpFixture(); + try { + const result = await queueMcpThreads(f.options, "requested-cursor"); + expect(result).toEqual({ available: true, scope: "stored_threads", liveStateVerified: false, + threads: [{ threadId: QUEUE_MCP_TARGET, name: "fixture" }], nextCursor: "next-page" }); + expect(f.calls().at(-1)!.frame.params).toMatchObject({ limit: 50, cursor: "requested-cursor", useStateDbOnly: true }); + expect(JSON.stringify(result)).not.toContain("PRIVATE_"); + } finally { await f.close(); } + }); + + for (const [mode, status] of [["reject", "not_sent"], ["init-reject", "not_sent"], ["wrong-id", "unknown"], + ["wrong-correlation", "unknown"], ["disconnect", "unknown"], ["oversize", "unknown"], ["bad-json", "unknown"], + ["bad-utf8", "unknown"], ["bad-error", "unknown"], ["missing-error-message", "unknown"], + ["mixed-error-result", "unknown"], ["missing-receipt-input", "unknown"]] as const) { + test(`${mode}: ${status}, no retry or private diagnostics`, async () => { + const f = queueMcpFixture(mode); + try { + const r = await queueMcpSend(f.options, { threadId: QUEUE_MCP_TARGET, message: "PRIVATE_INPUT" }); + expect(r.status).toBe(status); expect(r.processed).toBe(false); + expect(JSON.stringify(r)).not.toContain("PRIVATE_"); + expect(f.calls().filter(c => c.frame.method === "thread/queue/add")).toHaveLength(mode === "init-reject" ? 0 : 1); + } finally { await f.close(); } + }); + } + + for (const mode of ["bad-list", "bad-cursor"]) { + test(`${mode}: invalid discovery is not an empty success`, async () => { + const f = queueMcpFixture(mode); + try { expect((await queueMcpThreads(f.options)).available).toBe(false); } + finally { await f.close(); } + }); + } + + test("cancellation after queue write is unknown and the helper is reaped", async () => { + const f = queueMcpFixture("hang"); + const abort = new AbortController(); + try { + const pending = queueMcpSend(f.options, { threadId: QUEUE_MCP_TARGET, message: "fixture" }, abort.signal); + const deadline = performance.now() + 2000; + while (!f.calls().some(c => c.frame.method === "thread/queue/add") && performance.now() < deadline) await Bun.sleep(10); + const pid = f.calls().at(-1)!.pid; + expect(f.calls().at(-1)!.frame.method).toBe("thread/queue/add"); + abort.abort(); + expect((await pending).status).toBe("unknown"); + expect(() => process.kill(pid, 0)).toThrow(); + expect(f.calls().filter(c => c.frame.method === "thread/queue/add")).toHaveLength(1); + } finally { abort.abort(); await f.close(); } + }); + + test("deadline after queue write is unknown without resubmission", async () => { + const f = queueMcpFixture("hang"); + try { + expect((await queueMcpSend(f.options, { threadId: QUEUE_MCP_TARGET, message: "fixture" })).status).toBe("unknown"); + expect(f.calls().filter(c => c.frame.method === "thread/queue/add")).toHaveLength(1); + } finally { await f.close(); } + }, 15000); + + test("invalid inputs and pre-cancelled calls start no helper", async () => { + const f = queueMcpFixture(); + const abort = new AbortController(); abort.abort(); + try { + for (const input of [{ threadId: "name", message: "text" }, { threadId: QUEUE_MCP_TARGET, message: " " }, + { threadId: QUEUE_MCP_TARGET, message: "x\0y" }, { threadId: QUEUE_MCP_TARGET, message: "x".repeat(16385) }]) { + expect(z.object(queueMessageInput).safeParse(input).success).toBe(false); + expect((await queueMcpSend(f.options, input)).status).toBe("not_sent"); + } + expect((await queueMcpRpc(f.options, "thread/list", {}, abort.signal)).status).toBe("not_sent"); + expect(f.calls()).toHaveLength(0); + } finally { await f.close(); } + }); +}); diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index b97dd7e3e9c..4425ab3e9e4 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -626,6 +626,8 @@ "messaging-local-discovery.test.ts": "codex-integration", "messaging-local-cli.test.ts": "codex-integration", "messaging-local-envelope.test.ts": "codex-integration", + "messaging-mcp-queue.test.ts": "codex-integration", + "cli-message-mcp.test.ts": "cli", "messaging-local-send.test.ts": "codex-integration", "messaging-local-lifecycle.test.ts": "codex-integration", "messaging-local-socket.test.ts": "codex-integration", diff --git a/tests/helpers/messaging-mcp-queue.ts b/tests/helpers/messaging-mcp-queue.ts new file mode 100644 index 00000000000..ce3fc759b56 --- /dev/null +++ b/tests/helpers/messaging-mcp-queue.ts @@ -0,0 +1,49 @@ +import { chmodSync, mkdirSync, mkdtempSync, readFileSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { writeTestTempOwner } from "../../scripts/test-temp"; +import { removeTreeWithRetry } from "./remove-tree"; + +export const QUEUE_MCP_TARGET = "00000000-0000-4000-8000-000000000002"; +export interface QueueMcpCall { frame: { method: string; params?: Record }; argv: string[]; sender?: string; home: string; pid: number } + +/** Offline native CLI with one fixed failure mode, private capture and no user credentials. */ +export function queueMcpFixture(mode = "success") { + const root = mkdtempSync(join(tmpdir(), "opencodex-test-queue-mcp-")); + writeTestTempOwner(root); + const home = join(root, "home"), capture = join(root, "capture.jsonl"), codex = join(root, "codex"); + mkdirSync(home, { mode: 0o700 }); + writeFileSync(capture, "", { mode: 0o600 }); + writeFileSync(codex, `#!${process.execPath} +const fs = require("node:fs"); +const rl = require("node:readline").createInterface({input:process.stdin}); +const mode = ${JSON.stringify(mode)}; +const emit = value => process.stdout.write(JSON.stringify(value)+"\\n"); +rl.on("line", line => { + const f = JSON.parse(line); + fs.appendFileSync(${JSON.stringify(capture)}, JSON.stringify({frame:f,argv:process.argv.slice(2),sender:process.env.CODEX_THREAD_ID,home:process.env.CODEX_HOME,pid:process.pid})+"\\n"); + if(f.method === "initialize") return emit(mode === "init-reject" ? {id:1,error:{code:-32600,message:"PRIVATE_NATIVE_ERROR"}} : {id:1,result:{userAgent:"fixture"}}); + if(f.id !== 2) return; + if(mode === "hang") return; + if(mode === "disconnect") return process.exit(0); + if(mode === "oversize") return process.stdout.write("x".repeat(1048577)); + if(mode === "bad-json") return process.stdout.write("bad\\n"); + if(mode === "bad-utf8") return process.stdout.write(Buffer.from([255,10])); + if(mode === "bad-error") return emit({id:2,error:{message:"PRIVATE_NATIVE_ERROR"}}); + if(mode === "missing-error-message") return emit({id:2,error:{code:-32602}}); + if(mode === "mixed-error-result") return emit({id:2,error:{code:-32602,message:"private"},result:{}}); + if(mode === "reject") return emit({id:2,error:{code:-32602,message:"PRIVATE_NATIVE_ERROR"}}); + if(f.method === "thread/list") { + const result = {data:[{id:${JSON.stringify(QUEUE_MCP_TARGET)},name:"fixture",preview:"PRIVATE_PREVIEW",cwd:"PRIVATE_CWD",turns:["PRIVATE_TURN"]}],nextCursor:"next-page"}; + if(mode === "bad-list") result.data[0].id="not-a-uuid"; + if(mode === "bad-cursor") result.nextCursor="x".repeat(8193); + return emit({id:2,result}); + } + if(f.method !== "thread/queue/add") return emit({id:2,error:{code:-32601,message:"unexpected lifecycle RPC"}}); + emit({id:mode === "wrong-id" ? 99 : 2,result:{queuedSubmission:{id:"fixture-submission",clientUserMessageId:mode === "wrong-correlation" ? "wrong" : f.params.clientUserMessageId,input:mode === "missing-receipt-input" ? undefined : f.params.input}}}); +}); +`, { mode: 0o700 }); + chmodSync(codex, 0o700); + return { root, options: { codex, codexHome: home }, calls: (): QueueMcpCall[] => readFileSync(capture, "utf8").trim().split("\n").filter(Boolean).map(line => JSON.parse(line)), + close: () => removeTreeWithRetry(root) }; +} From cc23d5d3ca4089b01c44947ea0e9d7e54a61a983 Mon Sep 17 00:00:00 2001 From: Epinephrine Date: Sat, 31 Oct 2026 13:17:02 +0000 Subject: [PATCH 03/13] fix(combo): exclude Meta for direct hosted tool choices (#8010) * fix(combo): exclude Meta for direct hosted tool choices * fix(responses): accept recognized hosted choices in combo children * fix(combos): preserve mandatory choices across Chat translation * fix(combos): honor auto-only policy for resolved Chat selectors * fix(combos): resolve virtual wire policy before tool eligibility * docs(combos): link localized mandatory-tool routing guidance --- docs-site/src/content/docs/guides/combos.md | 5 +- .../src/content/docs/ja/guides/combos.md | 2 + .../src/content/docs/ko/guides/combos.md | 2 + .../src/content/docs/ru/guides/combos.md | 2 + .../src/content/docs/zh-cn/guides/combos.md | 2 + src/responses/parser-tools.ts | 1 + src/responses/schema.ts | 7 +- .../responses/combo-required-tool-contract.ts | 36 ++-- src/server/responses/core-combo.ts | 20 +- structure/providers-and-adapters.md | 2 +- .../transports/responses-combo-boundaries.md | 8 +- tests/responses/responses-parser.test.ts | 23 +++ ...erver-combo-required-tool-contract.test.ts | 183 ++++++++++++++++++ 13 files changed, 267 insertions(+), 26 deletions(-) diff --git a/docs-site/src/content/docs/guides/combos.md b/docs-site/src/content/docs/guides/combos.md index a0b9de7c718..91ffeb34b0d 100644 --- a/docs-site/src/content/docs/guides/combos.md +++ b/docs-site/src/content/docs/guides/combos.md @@ -154,7 +154,10 @@ native routing and catalog identity on the next sync. For Responses requests that require a tool, Meta Responses targets are skipped before sending because Meta does not support mandatory `tool_choice`. An eligible later target receives the -original choice. If every target is incompatible for this reason, the combo reports the constraint. +original choice, subject to its existing adapter compatibility rules. This includes hosted choices +such as `web_search`, `tool_search`, `computer_use` and `image_gen`, and MCP choices with a +nonempty `server_label` and an omitted, null or nonempty name, including inside required allowed-tools lists. Chat translation preserves a forced `tool_search` as the synthetic function choice. If every target is incompatible for this reason, +the combo reports the constraint. Chat targets that cannot preserve a mandatory selector, including models configured for auto-only tool choice, are also skipped before sending. Registered OpenAI API virtual models use the actual wire model's adapter and tool-choice policy for this decision. Malformed or unknown choices retain the provider's ordinary error path. `failover` selects the first eligible target in configuration order. A target is eligible when its provider exists, is enabled, is not cooling down, and can handle any special request constraint. diff --git a/docs-site/src/content/docs/ja/guides/combos.md b/docs-site/src/content/docs/ja/guides/combos.md index 943ccef2bc0..9823756979f 100644 --- a/docs-site/src/content/docs/ja/guides/combos.md +++ b/docs-site/src/content/docs/ja/guides/combos.md @@ -64,6 +64,8 @@ ocx combo show main ## 戦略を選択してください +必須の `tool_choice` を維持できないターゲットは送信前にスキップされ、すべて非対応の場合はその制約を示すエラーを返します。自動選択のみを許可する Chat モデルと、OpenAI 仮想モデルの実際の送信モデルのポリシーも判定対象です。直接の hosted/MCP 選択、`tool_search`、無効な選択値の扱いは[英語の基準説明](/guides/combos/#failover-ordered-primary-and-backups)を参照してください。 + ### フェイルオーバー: 順序付けされたプライマリとバックアップ `failover` は、構成順序で最初の適格なターゲットを選択します。ターゲットは、プロバイダーが存在し、有効になっていて、クールダウン中でなく、特別なリクエスト制約を処理できる場合に適格となります。重みと `stickyLimit` はこの戦略に影響しません。 diff --git a/docs-site/src/content/docs/ko/guides/combos.md b/docs-site/src/content/docs/ko/guides/combos.md index 40ad3327e05..b81448df36b 100644 --- a/docs-site/src/content/docs/ko/guides/combos.md +++ b/docs-site/src/content/docs/ko/guides/combos.md @@ -70,6 +70,8 @@ alias는 클라이언트가 요청하는 공개 이름만 바꿉니다. 콤보 ## 전략 선택 +필수 `tool_choice`를 보존할 수 없는 대상은 전송 전에 건너뛰며, 모든 대상이 호환되지 않으면 해당 제약을 오류로 반환합니다. 자동 선택만 허용하는 Chat 모델과 OpenAI 가상 모델의 실제 전송 모델 정책도 이 판단에 반영됩니다. 직접 hosted/MCP 선택, `tool_search`, 잘못된 선택값의 처리 범위는 [영문 기준 설명](/guides/combos/#failover-ordered-primary-and-backups)을 참고하세요. + ### 페일오버: 순서가 있는 기본값과 예비값 `failover`는 설정 순서에서 가장 먼저 적합한 대상을 선택합니다. 대상이 적합하려면 해당 공급자가 존재하고, 활성화되어 있고, 쿨다운 중이 아니며, 요청에 붙은 특수 조건을 처리할 수 있어야 합니다. 이 전략에서는 가중치와 `stickyLimit`이 영향을 주지 않습니다. diff --git a/docs-site/src/content/docs/ru/guides/combos.md b/docs-site/src/content/docs/ru/guides/combos.md index ccb5444da6f..5991f1a868c 100644 --- a/docs-site/src/content/docs/ru/guides/combos.md +++ b/docs-site/src/content/docs/ru/guides/combos.md @@ -80,6 +80,8 @@ Alias меняет публичное имя, которое запрашива ## Выбор стратегии +Цель, которая не может сохранить обязательный `tool_choice`, пропускается до отправки; если совместимых целей нет, возвращается ошибка с этим ограничением. Проверка учитывает модели Chat с выбором только `auto` и политику фактически отправляемой модели для виртуальных моделей OpenAI. Правила для прямого выбора hosted/MCP, `tool_search` и некорректных значений приведены в [каноническом руководстве на английском](/guides/combos/#failover-ordered-primary-and-backups). + ### Failover: основной вариант и резерв в заданном порядке `failover` выбирает первую подходящую цель в порядке конфигурации. Цель считается подходящей, diff --git a/docs-site/src/content/docs/zh-cn/guides/combos.md b/docs-site/src/content/docs/zh-cn/guides/combos.md index 5244fe9b646..7262139cb71 100644 --- a/docs-site/src/content/docs/zh-cn/guides/combos.md +++ b/docs-site/src/content/docs/zh-cn/guides/combos.md @@ -90,6 +90,8 @@ modality 和 reasoning metadata；目标显式声明的限制仍然优先，因 ## 选择策略 +无法保留强制 `tool_choice` 的目标会在发送前跳过；如果所有目标都不兼容，则返回说明该限制的错误。判断会考虑仅支持 `auto` 选择的 Chat 模型，以及 OpenAI 虚拟模型实际发送到上游的模型策略。直接 hosted/MCP 选择、`tool_search` 和无效选择值的处理范围请参阅[英文基准说明](/guides/combos/#failover-ordered-primary-and-backups)。 + ### 故障切换：按顺序的主目标和备用目标 `failover` 会按配置顺序选择第一个合格目标。当 provider 存在、已启用、未处于冷却中，并且能够满足任何特殊请求约束时，该目标就是合格的。权重和 `stickyLimit` 不影响这种策略。 diff --git a/src/responses/parser-tools.ts b/src/responses/parser-tools.ts index 763b061c8f6..ee0268d524c 100644 --- a/src/responses/parser-tools.ts +++ b/src/responses/parser-tools.ts @@ -16,6 +16,7 @@ export function mapToolChoice(value: unknown): OcxRequestOptions["toolChoice"] { if (t === "image_generation" || t === "image_gen") { return { name: IMAGE_GEN_TOOL_NAME }; } + if (t === "tool_search") return { name: "tool_search" }; if (t === "allowed_tools" && Array.isArray(value.tools)) { const names = value.tools .map(allowedToolName) diff --git a/src/responses/schema.ts b/src/responses/schema.ts index d3fac2f8632..213e443d2d4 100644 --- a/src/responses/schema.ts +++ b/src/responses/schema.ts @@ -162,12 +162,15 @@ const builtinToolSchema = z.object({ type: z.string() }).loose(); */ export const HOSTED_TOOL_TYPES = [ "web_search", "web_search_preview", "file_search", "computer_use_preview", - "code_interpreter", "image_generation", "mcp", + "code_interpreter", "image_generation", "mcp", "tool_search", "computer_use", "image_gen", ] as const; const hostedToolType = z.enum(HOSTED_TOOL_TYPES); -const allowedToolEntrySchema = z.object({ type: z.string(), name: z.string().optional() }); +const allowedToolEntrySchema = z.union([ + z.object({ type: z.literal("mcp"), server_label: z.string().min(1), name: z.string().nullable().optional() }), + z.object({ type: z.string(), name: z.string().optional() }), +]); export const toolChoiceSchema = z.union([ z.literal("auto"), diff --git a/src/server/responses/combo-required-tool-contract.ts b/src/server/responses/combo-required-tool-contract.ts index 2f5e16a22d5..a5177b364d8 100644 --- a/src/server/responses/combo-required-tool-contract.ts +++ b/src/server/responses/combo-required-tool-contract.ts @@ -1,4 +1,6 @@ import { REQUIRED_TOOL_CONTRACT_CODE } from "../../lib/errors"; +import { mapToolChoice } from "../../responses/parser-tools"; +import { modelInList, type OcxProviderConfig } from "../../types"; import { hasResponseMediaType } from "../../lib/http-response-semantics"; import { applyReplayRefusalClientHeaders, isNonReplayableResponse, markResponseNonReplayable } from "../../lib/upstream-retry"; import { clientWireOf } from "../inference/client-wire"; @@ -15,6 +17,16 @@ const allowedHostedSelectors = new Set([ "code_interpreter", "image_generation", "image_gen", "tool_search", ]); +function knownToolSelector(value: unknown): boolean { + if (!value || typeof value !== "object" || Array.isArray(value)) return false; + const entry = value as Record; + if (entry.type === "mcp") return typeof entry.server_label === "string" && !!entry.server_label.trim() + && (entry.name === undefined || entry.name === null || (typeof entry.name === "string" && !!entry.name.trim())); + if (entry.name !== undefined && (typeof entry.name !== "string" || !entry.name.trim())) return false; + if (entry.type === "function" || entry.type === "custom") return typeof entry.name === "string"; + return typeof entry.type === "string" && allowedHostedSelectors.has(entry.type); +} + /** Only definite, well-formed mandatory selectors justify excluding a destination. */ export function hasMandatoryToolChoice(rawBody: unknown): boolean { if (!rawBody || typeof rawBody !== "object" || Array.isArray(rawBody)) return false; @@ -22,19 +34,19 @@ export function hasMandatoryToolChoice(rawBody: unknown): boolean { if (choice === "required") return true; if (!choice || typeof choice !== "object" || Array.isArray(choice)) return false; const selector = choice as Record; - if (selector.type === "function" || selector.type === "custom") { - return typeof selector.name === "string" && selector.name.trim().length > 0; - } - return selector.type === "allowed_tools" && selector.mode === "required" + // Eligibility is broader than terminal inspection: hosted selectors retain their own terminal owner. + if (selector.type !== "allowed_tools") return knownToolSelector(selector); + return selector.mode === "required" && Array.isArray(selector.tools) && selector.tools.length > 0 - && selector.tools.every((tool: unknown) => { - if (!tool || typeof tool !== "object" || Array.isArray(tool)) return false; - const entry = tool as Record; - if (entry.name !== undefined && (typeof entry.name !== "string" || !entry.name.trim())) return false; - if (entry.type === "function" || entry.type === "custom") return typeof entry.name === "string"; - if (entry.type === "mcp") return typeof entry.server_label === "string" && !!entry.server_label.trim(); - return typeof entry.type === "string" && allowedHostedSelectors.has(entry.type); - }); + && selector.tools.every(knownToolSelector); +} + +/** A translated Chat target must not turn a mandatory selector into auto or none. */ +export function chatCanPreserveMandatoryToolChoice(rawBody: unknown, provider: OcxProviderConfig, modelId: string): boolean { + if (!hasMandatoryToolChoice(rawBody)) return true; + if (modelInList(provider.autoToolChoiceOnlyModels, modelId)) return false; + const mapped = mapToolChoice((rawBody as Record).tool_choice); + return mapped === "required" || (!!mapped && typeof mapped === "object"); } const hostedCalls = new Map([ diff --git a/src/server/responses/core-combo.ts b/src/server/responses/core-combo.ts index f2de582343d..20dae280c0a 100644 --- a/src/server/responses/core-combo.ts +++ b/src/server/responses/core-combo.ts @@ -1,9 +1,11 @@ import { effectiveAzureOutputBudget } from "../../adapters/azure"; import { buildResponsesTransport } from "../../adapters/openai-responses/transport"; import { isMetaAiResponsesDestination } from "../../responses/muse-tool-name-alias"; -import { hasMandatoryToolChoice } from "./combo-required-tool-contract"; +import { chatCanPreserveMandatoryToolChoice, hasMandatoryToolChoice } from "./combo-required-tool-contract"; import { isCodexAccountCooldownResponse } from "../../codex/account-cooldown-response"; import { resolveWireProtocolOverride } from "../adapter-resolve"; +import { parseSyntheticRowId } from "../fast-row"; +import { resolveOpenAiVirtualModel } from "../../providers/openai-virtual-models"; import { comboCooldownRetryAfterSeconds, isDefiniteContextOverflow, type CodexAccountModelRefusal } from "../../combos/failover"; import { fallbackTriggerForStatus, guardFallbackHop } from "../../routing/fallback-policy"; import { @@ -539,13 +541,15 @@ export async function executeComboResponses( const museToolEligible = (target: (typeof combo.targets)[number]): boolean => { if (!hasMandatoryToolChoice(body)) return true; try { - const route = routeConcreteModel(config, `${target.provider}/${target.model}`); + const selector = `${target.provider}/${target.model}`, { fastRow, effortRow } = parseSyntheticRowId(selector, config); + const route = routeConcreteModel(config, fastRow?.baseId ?? effortRow?.baseId ?? selector); + const modelId = resolveOpenAiVirtualModel(route.providerName, route.modelId)?.wireModelId ?? route.modelId; const inboundWire = options.inboundWire ?? "responses"; - const policy = captureRouteStaticPolicy(route.providerName, route.modelId, route.provider, + const policy = captureRouteStaticPolicy(route.providerName, modelId, route.provider, route.staticPolicy.effectiveAlias, inboundWire); - const provider = resolveWireProtocolOverride(route.providerName, route.modelId, route.provider, inboundWire, policy); - return provider.adapter !== "openai-responses" - || !isMetaAiResponsesDestination(buildResponsesTransport(provider, { headers: req.headers }).url); + const provider = resolveWireProtocolOverride(route.providerName, modelId, route.provider, inboundWire, policy); + return provider.adapter === "openai-chat" ? chatCanPreserveMandatoryToolChoice(body, provider, modelId) + : provider.adapter !== "openai-responses" || !isMetaAiResponsesDestination(buildResponsesTransport(provider, { headers: req.headers }).url); } catch { return true; } @@ -674,7 +678,7 @@ export async function executeComboResponses( { parentThreadId: inboundClientThreadId }, ); if (!options.abortSignal?.aborted && onlyMuseToolIncompatible()) { - return comboUnavailableResponse(`No available targets for combo: ${comboId}; Meta Responses does not support mandatory tool_choice`); + return comboUnavailableResponse(`No available targets for combo: ${comboId}; targets cannot preserve mandatory tool_choice`); } return options.abortSignal?.aborted ? clientCancelledResponse() @@ -694,7 +698,7 @@ export async function executeComboResponses( if (protocolRefusal) return protocolRefusal; if (onlyReplayIncompatibleTargetsRemain()) return targetIncompatibleResponse(); if (!options.abortSignal?.aborted && onlyMuseToolIncompatible()) { - return comboUnavailableResponse(`No available targets for combo: ${comboId}; Meta Responses does not support mandatory tool_choice`); + return comboUnavailableResponse(`No available targets for combo: ${comboId}; targets cannot preserve mandatory tool_choice`); } return options.abortSignal?.aborted ? clientCancelledResponse() diff --git a/structure/providers-and-adapters.md b/structure/providers-and-adapters.md index 240a397f29b..a90cac5a79d 100644 --- a/structure/providers-and-adapters.md +++ b/structure/providers-and-adapters.md @@ -555,7 +555,7 @@ uses `src/adapters/openai-responses/muse-tool-choice.ts` to normalize `tool_choi `additional_tools` items from `input`, and omits `tool_choice` and `parallel_tool_calls` from the outgoing body. Forced, named, and `allowed_tools` selections fail with HTTP 400 before the upstream send because Muse supports only `auto`. Filtering a required tool never changes the caller's obligation into `none` or `auto`. This rule applies only to the Meta Responses -destination. The input body, historical tool calls and results, and non-Meta requests keep their existing meaning. +destination. Combo eligibility excludes it before sending for known mandatory selectors, including direct hosted types and MCP selectors with a nonempty server label; malformed and unknown selectors retain the adapter error path. The input body, historical tool calls and results, and non-Meta requests keep their existing meaning. ## Shared type declarations diff --git a/structure/transports/responses-combo-boundaries.md b/structure/transports/responses-combo-boundaries.md index 1c057789d0a..4ffdf66af74 100644 --- a/structure/transports/responses-combo-boundaries.md +++ b/structure/transports/responses-combo-boundaries.md @@ -21,11 +21,15 @@ and `tests/responses/responses-combo-exhausted-error.test.ts`. ## Buffered required-tool contract Before selection, `src/server/responses/core-combo.ts` excludes resolved Meta Responses destinations -for well-formed mandatory `tool_choice` selectors, including required allowed-tools lists. Normal and +for well-formed mandatory `tool_choice` selectors, including direct hosted/MCP choices and required allowed-tools lists. MCP's optional name may be null; an invalid name or server label still abstains. Normal and recovery picks share the check, resolving the inbound wire's static model policy and actual Responses transport URL; malformed and unknown selectors retain their ordinary error path. The next target receives the original selector, and an all-Meta exhaustion names the constraint before any send. -Coverage: `tests/server/server-combo-required-tool-contract.test.ts`. +The child parser accepts `tool_search`, `computer_use` and `image_gen` as recognized hosted choices. +Its raw body preserves the selector; existing destination compatibility, including routed private +tool-search lowering, still applies after parsing. Chat translation maps a direct `tool_search` +choice to its existing synthetic function name. MCP null names also parse inside required allowed-tools lists; function/custom null names remain invalid. A Chat destination is skipped before send when translation would degrade a mandatory choice to auto/none, including the resolved model's auto-only tool-choice policy; exhaustion reports the tool-choice constraint. Eligibility resolves synthetic Fast/effort selectors before model policy, matching child request preparation while preserving exact configured IDs. +Registered OpenAI API virtual models resolve to their wire model before adapter and auto-only policy checks, using the same registry resolver as child normalization. Other providers and unregistered IDs remain literal. Coverage: `tests/server/server-combo-required-tool-contract.test.ts`. Before combo success bookkeeping, native delivery in `src/server/responses/passthrough-delivery.ts` uses `src/server/responses/combo-required-tool-contract.ts` to check successful native diff --git a/tests/responses/responses-parser.test.ts b/tests/responses/responses-parser.test.ts index 93698c8b5d3..6387308433d 100644 --- a/tests/responses/responses-parser.test.ts +++ b/tests/responses/responses-parser.test.ts @@ -10,6 +10,29 @@ import { repoPath } from "../helpers/repo-root"; import { normalizeThinkingOptions, resolveThinkingEnabled } from "../../src/clients/reasoning-controls"; import type { OcxProviderConfig } from "../../src/types"; +test.each(["tool_search", "computer_use", "image_gen", "mcp"])("preserves direct hosted selector %s in the raw Responses body", type => { + const choice = type === "mcp" ? { type, server_label: "docs", name: null } : { type }; + const raw = { model: "fixture", input: "hello", tool_choice: choice }; + expect(parseRequest(raw)._rawBody?.tool_choice).toEqual(choice); +}); + +test("translated tool_search keeps the mandatory synthetic function choice", () => { + const parsed = parseRequest({ model: "fixture", input: "hello", tools: [{ type: "tool_search" }], + tool_choice: { type: "tool_search" } }); + expect(parsed.options.toolChoice).toEqual({ name: "tool_search" }); + expect(parsed.context.tools?.map(tool => tool.name)).toContain("tool_search"); +}); + +test("MCP allowed-tools null name preserves its raw selector without accepting function null names", () => { + const choice = { type: "allowed_tools", mode: "required", tools: [{ type: "mcp", server_label: "docs", name: null }] }; + expect(parseRequest({ model: "fixture", input: "hello", tool_choice: choice })._rawBody?.tool_choice).toEqual(choice); + for (const type of ["function", "custom"]) { + expect(() => parseRequest({ model: "fixture", input: "hello", tool_choice: { + ...choice, tools: [{ type, name: null }], + } })).toThrow(); + } +}); + describe("generic effort-only client compatibility", () => { const provider: OcxProviderConfig = { adapter: "openai-chat", baseUrl: "https://fixture.invalid/v1", diff --git a/tests/server/server-combo-required-tool-contract.test.ts b/tests/server/server-combo-required-tool-contract.test.ts index f25491239a2..35d57202d0d 100644 --- a/tests/server/server-combo-required-tool-contract.test.ts +++ b/tests/server/server-combo-required-tool-contract.test.ts @@ -165,6 +165,189 @@ function requiredRequest(extra: Record = {}): Request { } describe("combo required-tool contract on buffered children (#7456)", () => { + test.each(["chat", "chat-fast", "chat-effort", "chat-only", "wire-chat", "virtual-chat", "other-provider", "unknown-virtual"])( + "virtual Chat eligibility follows the actual wire policy: %s", async mode => { + const virtual = mode === "unknown-virtual" ? "gpt-5.6-sol-pro-unknown" : "gpt-5.6-sol-pro"; + const wire = "gpt-5.6-sol", name = mode === "other-provider" ? "fixture" : "openai-apikey"; + const sent: Array<{ path: string; model: string; tool_choice: unknown }> = []; + const fetchWire = (async (input, init) => { + const body = JSON.parse(String(init?.body ?? "{}")) as { model: string; tool_choice: unknown }; + const path = new URL(input instanceof globalThis.Request ? input.url : String(input)).pathname; sent.push({ path, ...body }); + return path.endsWith("/responses") ? Response.json(completedNoCall(body.model, "virtual wire response")) + : Response.json({ id: "chat-virtual", object: "chat.completion", choices: [{ index: 0, + message: { role: "assistant", content: null, tool_calls: [{ id: "call_search", type: "function", + function: { name: "tool_search", arguments: '{"query":"lookup"}' } }] }, finish_reason: "tool_calls" }] }); + }) as typeof fetch; + const primary = provider(mode === "wire-chat" || mode === "virtual-chat" ? "openai-responses" : "openai-chat", "https://api.openai.com/v1", "key-a", { + fetch: fetchWire, + models: [virtual], autoToolChoiceOnlyModels: [mode === "virtual-chat" ? virtual : wire], + // The registry defaults this provider to Responses; explicit per-model overrides + // distinguish virtual and wire policy, rather than relying on the provider fallback. + ...(mode.startsWith("chat") ? { modelAdapters: { [virtual]: "openai-chat", [wire]: "openai-chat" } } + : mode === "wire-chat" ? { modelAdapters: { [wire]: "openai-chat" } } + : mode === "virtual-chat" || mode === "unknown-virtual" ? { modelAdapters: { [virtual]: "openai-chat" } } : {}), + }); + const cfg = comboConfig({ [name]: primary, ...(mode === "chat-only" ? {} : { backup: provider("openai-chat", "https://backup.fixture.invalid/v1", "key-b", { fetch: fetchWire }) }) }); + cfg.combos!.free!.targets[0]!.model = virtual + (mode === "chat-fast" ? "--fast" : mode === "chat-effort" ? "--high" : ""); + cfg.cursorEffortRows = mode === "chat-effort"; + const response = await handleResponses(requiredRequest({ tools: [{ type: "tool_search" }], tool_choice: { type: "tool_search" } }), cfg, {} as RequestLogContext); + if (mode === "chat-only") { + expect(response.status).toBe(503); expect(await response.text()).toContain("mandatory tool_choice"); expect(sent).toEqual([]); return; + } + expect(response.status).toBe(200); await response.text(); + expect(sent).toHaveLength(1); + if (mode.startsWith("chat") || mode === "wire-chat") { + expect(sent[0]).toMatchObject({ model: "m2", path: "/v1/chat/completions", tool_choice: { type: "function", function: { name: "tool_search" } } }); + } else if (mode === "virtual-chat") { + expect(sent[0]).toMatchObject({ model: wire, path: "/v1/responses", tool_choice: { type: "function", name: "tool_search" } }); + } else expect(sent[0]).toMatchObject({ model: virtual, path: "/v1/chat/completions", tool_choice: { type: "function", function: { name: "tool_search" } } }); + }); + + for (const choice of ["required", { type: "function", name: "tool_search" }, { type: "tool_search" }, + { type: "allowed_tools", mode: "required", tools: [{ type: "tool_search" }] }]) { + for (const [backup, suffix] of [[false, ""], [true, ""], [false, "--fast"], [true, "--fast"], [false, "--high"], [true, "--high"]] as const) { + test(`auto-only Chat cannot consume a mandatory choice (${JSON.stringify(choice)}, backup=${backup}, suffix=${suffix})`, async () => { + const sent: Array<{ model: string; tool_choice: unknown }> = []; + const upstream = serve(async request => { + const body = await request.json() as { model: string; tool_choice: unknown }; sent.push(body); + return Response.json({ id: "chat-auto-policy", object: "chat.completion", + choices: [{ index: 0, message: { role: "assistant", content: null, tool_calls: [{ + id: "call_search", type: "function", function: { name: "tool_search", arguments: '{"query":"lookup"}' }, + }] }, finish_reason: "tool_calls" }], usage: { prompt_tokens: 1, completion_tokens: 1, total_tokens: 2 } }); + }); + const providers = { a: provider("openai-chat", baseUrl(upstream), "key-a", { autoToolChoiceOnlyModels: ["m1"] }), + ...(backup ? { b: provider("openai-chat", baseUrl(upstream), "key-b") } : {}) }; + const cfg = comboConfig(providers); + cfg.cursorEffortRows = suffix === "--high"; + cfg.combos!.free!.targets[0]!.model += suffix; + const response = await handleResponses(requiredRequest({ tools: [{ type: "tool_search" }], tool_choice: choice }), + cfg, {} as RequestLogContext); + expect(response.status).toBe(backup ? 200 : 503); + const text = await response.text(); + if (!backup) { expect(text).toContain("mandatory tool_choice"); expect(sent).toEqual([]); } + else { + expect(sent).toHaveLength(1); expect(sent[0]?.model).toBe("m2"); + expect(sent[0]?.tool_choice).toEqual(typeof choice === "string" || choice.type === "allowed_tools" + ? "required" : { type: "function", function: { name: "tool_search" } }); + } + }); + } + } + + for (const choice of [{ type: "computer_use" }, + { type: "allowed_tools", mode: "required", tools: [{ type: "mcp", server_label: "docs", name: null }] }]) { + test(`untranslatable mandatory Chat choice skips before send: ${JSON.stringify(choice)}`, async () => { + let chatSends = 0; let backupSends = 0; + const chat = provider("openai-chat", "https://chat.fixture.invalid/v1", "key-a", { + fetch: (async () => { chatSends++; throw new Error("unexpected Chat send"); }) as typeof fetch }); + const backup = serve(() => { backupSends++; return Response.json(completedNoCall("m2", "eligible Responses target")); }); + const response = await handleResponses(requiredRequest({ tool_choice: choice }), + comboConfig({ a: chat, b: provider("openai-responses", baseUrl(backup), "key-b") }), {} as RequestLogContext); + expect(response.status).toBe(200); expect(await response.text()).toContain("eligible Responses target"); + expect(chatSends).toBe(0); expect(backupSends).toBe(1); + }); + test(`untranslatable mandatory Chat choice fails closed without a backup: ${JSON.stringify(choice)}`, async () => { + let sends = 0; + const chat = provider("openai-chat", "https://chat.fixture.invalid/v1", "key-a", { + fetch: (async () => { sends++; throw new Error("unexpected Chat send"); }) as typeof fetch }); + const response = await handleResponses(requiredRequest({ tool_choice: choice }), comboConfig({ a: chat }), {} as RequestLogContext); + expect(response.status).toBe(503); expect(await response.text()).toContain("mandatory tool_choice"); + expect(sends).toBe(0); + }); + } + + test.each(["ordinary", "exact-suffix", "disabled-fast"])("translated Chat child preserves forced tool_search on its actual wire: %s", async mode => { + const bodies: unknown[] = []; + const upstream = serve(async request => { + bodies.push(await request.json()); + return Response.json({ id: "chat-forced-search", object: "chat.completion", + choices: [{ index: 0, message: { role: "assistant", content: null, tool_calls: [{ + id: "call_search", type: "function", function: { name: "tool_search", arguments: '{"query":"lookup"}' }, + }] }, finish_reason: "tool_calls" }], usage: { prompt_tokens: 1, completion_tokens: 1, total_tokens: 2 } }); + }); + const model = mode === "ordinary" ? "m1" : "m1--fast"; + const cfg = comboConfig({ a: provider("openai-chat", baseUrl(upstream), "key-a", { + autoToolChoiceOnlyModels: [mode === "ordinary" ? "another-model" : "m1"], + ...(mode === "exact-suffix" ? { models: [model] } : {}), + }) }); + cfg.combos!.free!.targets[0]!.model = model; + if (mode === "disabled-fast") cfg.fastRows = false; + const response = await handleResponses(requiredRequest({ tools: [{ type: "tool_search" }], + tool_choice: { type: "tool_search" } }), cfg, {} as RequestLogContext); + expect(response.status).toBe(200); + expect(await response.text()).toContain("tool_search"); + expect(bodies).toHaveLength(1); + expect(bodies[0]).toMatchObject({ model, tool_choice: { type: "function", function: { name: "tool_search" } } }); + }); + + test.each(["auto", "none"])("auto-only Chat retains optional %s behavior on its wire", async choice => { + const bodies: unknown[] = []; + const upstream = serve(async request => { bodies.push(await request.json()); + return Response.json({ id: "chat-optional", object: "chat.completion", + choices: [{ index: 0, message: { role: "assistant", content: "optional done" }, finish_reason: "stop" }] }); }); + const response = await handleResponses(requiredRequest({ tool_choice: choice }), comboConfig({ + a: provider("openai-chat", baseUrl(upstream), "key-a", { autoToolChoiceOnlyModels: ["m1"] }), + }), {} as RequestLogContext); + expect(response.status).toBe(200); expect(await response.text()).toContain("optional done"); + expect(bodies).toHaveLength(1); expect(bodies[0]).toMatchObject({ model: "m1" }); + if (choice === "auto") expect(bodies[0]).toMatchObject({ tool_choice: "auto" }); + else { expect(bodies[0]).not.toHaveProperty("tools"); expect(bodies[0]).not.toHaveProperty("tool_choice"); } + }); + + for (const choice of [{ type: "web_search" }, { type: "mcp", server_label: "docs" }, + { type: "tool_search" }, { type: "computer_use" }, { type: "image_gen" }, + { type: "mcp", server_label: "docs", name: null }, + { type: "allowed_tools", mode: "required", tools: [{ type: "mcp", server_label: "docs", name: null }] }]) { + for (const stream of [false, true]) { + test(`direct hosted choice skips Meta before send (${JSON.stringify(choice)}, stream=${stream})`, async () => { + const bodies: unknown[] = []; let metaSends = 0; + const upstream = serve(async request => { + bodies.push(await request.json()); + const response = completedNoCall("m2", "hosted terminal owner"); + return stream ? new Response(`data: ${JSON.stringify({ type: "response.completed", response })}\n\n`, + { headers: { "content-type": "text/event-stream" } }) : Response.json(response); + }); + const declaration = choice.type === "mcp" ? { ...choice, server_url: "https://docs.example.invalid/mcp" } : choice; + const response = await handleResponsesWith(comboConfig({ + a: provider("openai-responses", "https://api.meta.ai/v1", "key-a", { + fetch: (async () => { metaSends++; throw new Error("unexpected Meta send"); }) as typeof fetch }), + b: provider("openai-responses", baseUrl(upstream), "key-b"), + }), requiredRequest({ tools: [declaration], tool_choice: choice, stream })); + expect(response.status).toBe(200); + expect(await response.text()).toContain("hosted terminal owner"); + expect(metaSends).toBe(0); expect(bodies).toHaveLength(1); + // Noncanonical Responses targets retain the existing private tool-search lowering. + const upstreamChoice = choice.type === "tool_search" ? { type: "function", name: "tool_search" } : choice; + expect(bodies[0]).toMatchObject({ tool_choice: upstreamChoice }); + }); + } + test(`direct hosted choice reports all-Meta exhaustion (${JSON.stringify(choice)})`, async () => { + let sends = 0; + const meta = provider("openai-responses", "https://api.meta.ai/v1", "key-a", { + fetch: (async () => { sends++; throw new Error("unexpected Meta send"); }) as typeof fetch }); + const response = await handleResponsesWith(comboConfig({ a: meta, b: meta }), requiredRequest({ tool_choice: choice })); + expect(response.status).toBe(503); + expect(await response.text()).toContain("mandatory tool_choice"); + expect(sends).toBe(0); + }); + } + test("direct hosted selectors keep their terminal owner and reject malformed selector shapes", async () => { + const { hasMandatoryToolChoice, requiredToolContractOf } = await import("../../src/server/responses/combo-required-tool-contract"); + const { createComboRequiredToolStreamGuard } = await import("../../src/server/responses/combo-required-tool-stream"); + for (const choice of [{ type: "web_search" }, { type: "mcp", server_label: "docs" }, { type: "tool_search" }, + { type: "mcp", server_label: "docs", name: null }, { type: "computer_use" }, { type: "image_gen" }]) { + const body = { tools: [choice], tool_choice: choice }; + expect(hasMandatoryToolChoice(body)).toBe(true); + expect(requiredToolContractOf(body)).toBeUndefined(); + expect(createComboRequiredToolStreamGuard(body, new Set())).toBeUndefined(); + } + for (const choice of [{ type: "web_search", name: 5 }, { type: "web_search", name: " " }, + { type: "mcp" }, { type: "mcp", server_label: 5 }, { type: "mcp", server_label: " " }, + { type: "mcp", server_label: "docs", name: " " }, { type: "mcp", server_label: "docs", name: 123 }, + { type: "function", name: null }, { type: "custom", name: null }, { type: "unknown_hosted" }]) { + expect(hasMandatoryToolChoice({ tool_choice: choice })).toBe(false); + } + }); for (const choice of ["required", { type: "function", name: "shell" }, { type: "allowed_tools", mode: "required", tools: [{ type: "function", name: "shell" }] }]) { test(`Meta mandatory tool choice selects the next target before send (#7964): ${JSON.stringify(choice)}`, async () => { From b2e47bd26d52d12935e2319f74d3749931ccb93f Mon Sep 17 00:00:00 2001 From: ZeroDi Date: Sat, 31 Oct 2026 22:17:18 +0900 Subject: [PATCH 04/13] perf(ollama): count fragmented NDJSON bytes incrementally (#8015) * perf(ollama): count fragmented NDJSON bytes incrementally * test(ollama): assert fragmented UTF-8 budget boundaries --- scripts/test-layout/layout.json | 2 +- src/adapters/ollama-native.ts | 16 ++- structure/transports/byte-accounting.md | 8 ++ tests/fixtures/test-layout-expected.json | 1 + .../ollama/ollama-native-scaling.test.ts | 131 ++++++++++++++++++ 5 files changed, 151 insertions(+), 7 deletions(-) create mode 100644 tests/providers/ollama/ollama-native-scaling.test.ts diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 9bee9e3d98d..95b9ca3dbf1 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -1331,7 +1331,7 @@ "ocx-launcher-source.test.ts": "cli", "ocx-run.test.ts": "cli", "ollama-native-parser.test.ts": "providers/ollama", "ollama-native-reasoning-wire.test.ts": "providers/ollama", "ollama-native-structured-output.test.ts": "providers/ollama", "ollama-native-tool-continuations.test.ts": "providers/ollama", - "ollama-native-v4.test.ts": "providers/ollama", "ollama-native.test.ts": "providers/ollama", "ollama-show-enrichment-v7.test.ts": "providers/ollama", "ollama-show-enrichment.test.ts": "providers/ollama", + "ollama-native-v4.test.ts": "providers/ollama", "ollama-native-scaling.test.ts": "providers/ollama", "ollama-native.test.ts": "providers/ollama", "ollama-show-enrichment-v7.test.ts": "providers/ollama", "ollama-show-enrichment.test.ts": "providers/ollama", "ollama-show-ignore-abort.test.ts": "providers/ollama", "opaque-blob-wrapped-rejection.test.ts": "responses", "openai-api-virtual-models.test.ts": "adapters/openai", "openai-chat-bounded-tool-names.test.ts": "adapters/openai", "openai-chat-dangling-toolcalls.test.ts": "adapters/openai", "openai-chat-developer-position.test.ts": "adapters/openai", "openai-chat-eof.test.ts": "adapters/openai", "openai-chat-hardening.test.ts": "adapters/openai", "openai-empty-assistant-history.test.ts": "adapters/openai", "openai-chat-image-normalization.test.ts": "adapters/openai", diff --git a/src/adapters/ollama-native.ts b/src/adapters/ollama-native.ts index 61ceeb6d827..e612011347d 100644 --- a/src/adapters/ollama-native.ts +++ b/src/adapters/ollama-native.ts @@ -1051,7 +1051,6 @@ async function* parseOllamaNativeStream( } const reader = response.body.getReader(); const decoder = new TextDecoder("utf-8", { fatal: true }); - const encoder = new TextEncoder(); const state = streamState(allowParallelToolCalls); const issuedIds = issuedToolCallIds ?? new Set(); let buffer = ""; @@ -1060,15 +1059,20 @@ async function* parseOllamaNativeStream( const ingest = function* (text: string): Generator { if (!text) return; + const hasNewline = text.includes("\n"); buffer += text; - const lines = buffer.split("\n"); - buffer = lines.pop() ?? ""; + // After draining, the carry contains no newline. Only new text can complete a record. + const lines = hasNewline ? buffer.split("\n") : []; + if (hasNewline) buffer = lines.pop() ?? ""; // The safety bound applies to the genuinely retained residual — the incomplete NDJSON record // still being assembled — and to each complete record below. It must never depend on the // transport read size (one read may carry many valid records) nor transiently charge // old + replacement together. On a ceiling violation the old reservation is left in place so // the generator's finally releases exactly what is held. - const residualBytes = encoder.encode(buffer).byteLength; + // TextDecoder emits complete code points; fragment byte counts add without a surrogate fixup. + const residualBytes = hasNewline + ? Buffer.byteLength(buffer, "utf8") + : bufferBytes + Buffer.byteLength(text, "utf8"); if (residualBytes > TRANSLATOR_MAX_SSE_EVENT_BYTES) { throw new TranslatorBudgetExceededError("live_transient", TRANSLATOR_MAX_SSE_EVENT_BYTES); } @@ -1077,7 +1081,7 @@ async function* parseOllamaNativeStream( for (const rawLine of lines) { const line = rawLine.endsWith("\r") ? rawLine.slice(0, -1) : rawLine; - const lineBytes = encoder.encode(line).byteLength; + const lineBytes = Buffer.byteLength(line, "utf8"); if (lineBytes > TRANSLATOR_MAX_SSE_EVENT_BYTES) { throw new TranslatorBudgetExceededError("live_transient", TRANSLATOR_MAX_SSE_EVENT_BYTES); } @@ -1112,7 +1116,7 @@ async function* parseOllamaNativeStream( yield* ingest(decoder.decode()); if (!state.terminal && buffer.length > 0) { const rawLine = buffer.endsWith("\r") ? buffer.slice(0, -1) : buffer; - const lineBytes = encoder.encode(rawLine).byteLength; + const lineBytes = Buffer.byteLength(rawLine, "utf8"); if (lineBytes > TRANSLATOR_MAX_SSE_EVENT_BYTES) throw new TranslatorBudgetExceededError("live_transient", TRANSLATOR_MAX_SSE_EVENT_BYTES); try { // The EOF record keeps its residual charge while it is parsed and consumed — releasing it diff --git a/structure/transports/byte-accounting.md b/structure/transports/byte-accounting.md index ff55305299f..3eefc7360a8 100644 --- a/structure/transports/byte-accounting.md +++ b/structure/transports/byte-accounting.md @@ -140,6 +140,14 @@ envelopes, and real HTTP streams; `tests/server/inbound-body-budget.test.ts` cov ## Stream-buffer accounting +Native Ollama NDJSON in `src/adapters/ollama-native.ts` measures new decoded fragments without +allocating UTF-8 arrays. A fragment without a newline extends the cached residual byte count; +only a fragment containing a newline triggers splitting of the accumulated record. Complete +records and the remaining residual retain their independent size checks and existing reservation +order, including CRLF and EOF handling. The fatal streaming decoder emits complete code points, +so fragment counts remain exact across split UTF-8 reads. `tests/providers/ollama/ollama-native-scaling.test.ts` +checks linear measurement and splitting work, framing equivalence, invalid UTF-8 and cancellation. + `src/lib/pinned-http.ts` pauses its socket response when the receiving Web Stream queue fills and resumes when the reader asks for more. For gzip and deflate, the final decoded queue also gates subsequent socket input even when the decoder would accept more coded bytes. diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 4425ab3e9e4..7a4919ad18e 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -1075,6 +1075,7 @@ "ocx-launcher-desktop-handoff.test.ts": "cli", "cli-path-diagnostics.test.ts": "cli", "ocx-run.test.ts": "cli", "ollama-native-parser.test.ts": "providers/ollama", "ollama-native-reasoning-wire.test.ts": "providers/ollama", + "ollama-native-scaling.test.ts": "providers/ollama", "ollama-native-structured-output.test.ts": "providers/ollama", "ollama-native-v4.test.ts": "providers/ollama", "ollama-native.test.ts": "providers/ollama", "ollama-show-enrichment-v7.test.ts": "providers/ollama", "ollama-show-enrichment.test.ts": "providers/ollama", "ollama-show-ignore-abort.test.ts": "providers/ollama", diff --git a/tests/providers/ollama/ollama-native-scaling.test.ts b/tests/providers/ollama/ollama-native-scaling.test.ts new file mode 100644 index 00000000000..a9052bbfdbd --- /dev/null +++ b/tests/providers/ollama/ollama-native-scaling.test.ts @@ -0,0 +1,131 @@ +import { expect, spyOn, test } from "bun:test"; +import { createOllamaNativeAdapter } from "../../../src/adapters/ollama-native"; +import type { AdapterEvent } from "../../../src/types"; +import { createTestTranslatorBudget } from "../../helpers/translator-budget"; + +const encoder = new TextEncoder(); +const adapter = () => createOllamaNativeAdapter({ adapter: "ollama-native", baseUrl: "http://127.0.0.1:11434", models: ["fixture"] }); +const record = (content: string, done = true) => JSON.stringify({ model: "fixture", message: { role: "assistant", content }, done, ...(done ? { done_reason: "stop", eval_count: 7 } : {}) }); + +function response(bytes: Uint8Array, chunkSize: number, emptyChunks = false, keepOpen = false) { + let offset = 0; + let empty = emptyChunks; + let cancelled = false; + const body = new ReadableStream({ + pull(controller) { + if (empty) { empty = false; controller.enqueue(new Uint8Array(0)); return; } + if (offset === bytes.length) { if (!keepOpen) controller.close(); return; } + const end = Math.min(bytes.length, offset + chunkSize); + controller.enqueue(bytes.subarray(offset, end)); + offset = end; + empty = emptyChunks; + }, + cancel() { cancelled = true; }, + }); + return { response: new Response(body), cancelled: () => cancelled }; +} + +async function collect(input: Response) { + const budget = createTestTranslatorBudget(); + const events: AdapterEvent[] = []; + for await (const event of adapter().parseStream(input, budget)) events.push(event); + expect(budget.snapshot().currentBytes).toBe(0); + return events; +} + +for (const metric of ["measurement", "splitting"] as const) { + test(`fragmented Ollama records have linear ${metric} work`, async () => { + const content = "x".repeat(256 * 1024); + const bytes = encoder.encode(record(content) + "\n"); + const input = response(bytes, 256, false, true); + const encode = TextEncoder.prototype.encode; + const byteLength = Buffer.byteLength; + const split = String.prototype.split; + let measuredChars = 0; + let splitChars = 0; + const encoding = spyOn(TextEncoder.prototype, "encode").mockImplementation(function (value = "") { + measuredChars += value.length; + return Reflect.apply(encode, this, [value]); + }); + const measuring = spyOn(Buffer, "byteLength").mockImplementation((value, encoding) => { + if (typeof value === "string") measuredChars += value.length; + return byteLength(value, encoding); + }); + const splitting = spyOn(String.prototype, "split").mockImplementation(function (...args: Parameters) { + if (args[0] === "\n") splitChars += this.length; + return Reflect.apply(split, this, args); + }); + let events: AdapterEvent[]; + try { events = await collect(input.response); } + finally { encoding.mockRestore(); measuring.mockRestore(); splitting.mockRestore(); } + expect(events.filter(event => event.type === "text_delta").map(event => event.text).join("")).toBe(content); + expect(events.at(-1)).toMatchObject({ type: "done", usage: { outputTokens: 7 } }); + expect(input.cancelled()).toBe(true); + // Bound actual work, independent of CPU speed. Both UTF-8 counting APIs are observed. + if (metric === "measurement") expect(measuredChars).toBeLessThanOrEqual(bytes.length * 8); + else expect(splitChars).toBeLessThanOrEqual(bytes.length * 2); + }); +} + +for (const ending of ["\n", "\r\n", ""] as const) { + test(`Ollama fragmented UTF-8 preserves BOM, blank lines and ${JSON.stringify(ending)} termination`, async () => { + const bytes = encoder.encode("\ufeff\n" + record("한글 😀", false) + "\r\n\n" + record("끝") + ending); + const whole = await collect(response(bytes, bytes.length).response); + const fragmented = await collect(response(bytes, 1, true).response); + expect(fragmented).toEqual(whole); + expect(fragmented.filter(event => event.type === "text_delta").map(event => event.text).join("")).toBe("한글 😀끝"); + expect(fragmented.at(-1)?.type).toBe("done"); + }); +} + +for (const deficit of [0, 1]) { + test(`fragmented Unicode residual ${deficit ? "rejects one byte below" : "admits at"} its UTF-8 byte limit`, async () => { + const content = "é한😀"; + const text = record(content); + const bytes = encoder.encode(text); + expect(bytes.length).toBeGreaterThan(text.length); + const budget = createTestTranslatorBudget({ maxTurnBytes: bytes.length - deficit }); + let offset = 0; + let cancelled = false; + let beforeEof: ReturnType | undefined; + const body = new ReadableStream({ + pull(controller) { + if (offset < bytes.length) { + controller.enqueue(bytes.subarray(offset, ++offset)); + return; + } + // With no read-ahead, every fragment has been ingested; no delimiter or EOF yet. + beforeEof = budget.snapshot(); + controller.close(); + }, + cancel() { cancelled = true; }, + }, { highWaterMark: 0 }); + const events: AdapterEvent[] = []; + for await (const event of adapter().parseStream(new Response(body), budget)) events.push(event); + if (deficit === 0) { + expect(beforeEof).toMatchObject({ currentBytes: bytes.length, highWaterBytes: bytes.length, overflows: 0 }); + expect(events.filter(event => event.type === "text_delta").map(event => event.text).join("")).toBe(content); + expect(events.at(-1)?.type).toBe("done"); + expect(budget.snapshot().highWaterBytes).toBe(bytes.length); + } else { + expect(beforeEof).toBeUndefined(); // Refusal must happen while the record is still buffered. + expect(events).toHaveLength(1); + expect(events[0]).toMatchObject({ type: "error", code: "translation_buffer_limit" }); + expect(budget.snapshot().overflows).toBe(1); + expect(cancelled).toBe(true); + } + expect(budget.snapshot().currentBytes).toBe(0); + }); +} + +test("invalid UTF-8 after a retained partial record releases its budget and cancels", async () => { + const prefix = encoder.encode('{"message":{"content":"partial '); + const bytes = new Uint8Array(prefix.length + 1); + bytes.set(prefix); + bytes[prefix.length] = 0xff; + const input = response(bytes, prefix.length, false, true); + const events = await collect(input.response); + expect(events).toHaveLength(1); + expect(events[0]).toMatchObject({ type: "error" }); + expect(input.cancelled()).toBe(true); +}); From 468e635abd1d88f5fcb8ae3b274b9f3ac764b448 Mon Sep 17 00:00:00 2001 From: Epinephrine Date: Sat, 31 Oct 2026 13:17:35 +0000 Subject: [PATCH 05/13] fix(grok): honor disable and shutdown during startup sync (#8017) * fix(grok): recheck admission after startup catalog discovery * test(grok): cover real startup admission after discovery --- src/cli/ensure-desired-integrations.ts | 7 +- src/cli/index.ts | 5 +- src/grok/sync.ts | 11 ++- structure/runtime.md | 2 +- .../ensure-desired-integrations-race.test.ts | 19 ++++ .../codex-composed-acceptance.test.ts | 89 ++++++++++++++++++- .../grok-startup-discovery-preload.ts | 38 ++++++++ tests/providers/xai/grok-sync.test.ts | 46 ++++++++++ 8 files changed, 208 insertions(+), 9 deletions(-) create mode 100644 tests/fixtures/grok-startup-discovery-preload.ts diff --git a/src/cli/ensure-desired-integrations.ts b/src/cli/ensure-desired-integrations.ts index d3804992fe4..9b160f0cf31 100644 --- a/src/cli/ensure-desired-integrations.ts +++ b/src/cli/ensure-desired-integrations.ts @@ -48,7 +48,7 @@ export interface EnsureDesiredIntegrationsDeps { syncGrokConfig: ( port: number, config: OcxConfig, - opts?: { hostname?: string; probeHostname?: string }, + opts?: { hostname?: string; probeHostname?: string; admit?: () => boolean }, ) => Promise; removeDesktop3pStandardPivot: typeof removeDesktop3pStandardPivot; removeDesktopFirstParty?: typeof removeDesktopFirstParty; @@ -67,7 +67,7 @@ export interface EnsureDesiredIntegrationsDeps { async function defaultSyncGrokConfig( port: number, config: OcxConfig, - opts: { hostname?: string; probeHostname?: string } = {}, + opts: { hostname?: string; probeHostname?: string; admit?: () => boolean } = {}, ): Promise { const { syncGrokConfig } = await import("../grok/sync"); return syncGrokConfig(port, config, opts); @@ -133,7 +133,8 @@ export async function ensureGrokFenceMatchesDesired( port, config, { ...(hostname !== undefined ? { hostname } : {}), - ...(opts.probeHostname !== undefined ? { probeHostname: opts.probeHostname } : {}) }, + ...(opts.probeHostname !== undefined ? { probeHostname: opts.probeHostname } : {}), + admit: () => shouldSyncGrokOnStart(deps.loadConfig()) }, ); if (g.changed) log(" + Grok Build config updated (~/.grok/config.toml)"); else if (!g.ok) error(`⚠️ ${g.message}`); diff --git a/src/cli/index.ts b/src/cli/index.ts index 52cc993a5fe..038b5275275 100755 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -781,7 +781,10 @@ async function handleStart(options: { block?: boolean } = {}) { // one restart, because the toggle removed the fence and start wrote it back. if (shouldSyncGrokOnStart(config)) try { const { syncGrokConfig } = await import("../grok/sync"); - const r = await syncGrokConfig(port, config, config.hostname ? { hostname: config.hostname } : {}); + const r = await syncGrokConfig(port, config, { + ...(config.hostname ? { hostname: config.hostname } : {}), + admit: () => !shuttingDown && !cleaned && shouldSyncGrokOnStart(loadConfig()), + }); if (r.changed) console.log(" + Grok Build config updated (~/.grok/config.toml)"); else if (!r.ok) console.error(`⚠️ ${r.message}`); } catch (err) { diff --git a/src/grok/sync.ts b/src/grok/sync.ts index 46571b030d4..fbd76ef7f3e 100644 --- a/src/grok/sync.ts +++ b/src/grok/sync.ts @@ -32,9 +32,15 @@ async function defaultFetchAllModels(config: OcxConfig): Promise export async function syncGrokConfig( port: number, config: OcxConfig, - opts: { hostname?: string; probeHostname?: string; grokHome?: string; refreshOnly?: { admit: () => boolean } } = {}, + opts: { hostname?: string; probeHostname?: string; grokHome?: string; refreshOnly?: { admit: () => boolean }; + /** Recheck caller-owned desired state/lifecycle after asynchronous catalog work. */ + admit?: () => boolean } = {}, deps: GrokSyncDeps = { fetchAllModels: defaultFetchAllModels, injectGrokConfig }, ): Promise { + const skipped = (): GrokInjectResult => ({ + ok: true, changed: false, message: "Grok config sync skipped: caller admission is no longer current.", + }); + if (opts.admit && !opts.admit()) return skipped(); let projection: ReturnType; try { const allRouted = await deps.fetchAllModels(config); @@ -54,6 +60,9 @@ export async function syncGrokConfig( unauthenticatedLoopbackListener: config.unauthenticatedLoopbackListener, }); const targetUrl = new URL(target.baseUrl); + // Admission and the synchronous writer share one event-loop turn. A toggle or + // shutdown that landed while catalog discovery awaited must win over this snapshot. + if (opts.admit && !opts.admit()) return skipped(); return deps.injectGrokConfig(Number(targetUrl.port), projection.models, { hostname: target.requiresAdmissionToken ? (opts.hostname ?? config.hostname) diff --git a/structure/runtime.md b/structure/runtime.md index 24fd3ad5035..52a3a83c061 100644 --- a/structure/runtime.md +++ b/structure/runtime.md @@ -181,7 +181,7 @@ uninstall still restore. `src/service/cli.ts` removes the service token on unins `src/cli/tray-proxy.ts` coordinates `ocx restart` and tray restart. A second start requires proof the prior attempt never launched or its child exited, plus confirmed absence under a fresh bounded discovery window; a missing health response alone is insufficient. After an accepted restart, polling continues through uncertain observations until its deadline and requires a new runtime PID on the same port. If the old proxy then disappears, explicit recovery starts it regardless of the Codex autostart preference; a skipped recovery fails rather than reporting success. The Windows tray startup-health probe in `src/tray/windows-tray.ps1` requests termination at its 30-second deadline without waiting on the UI thread. It retains the process through later ticks until actual exit and both redirected readers settle; replacements cannot start while that reference is held. Launch setup failures follow the same cleanup. Tray shutdown alone waits at most 500 ms before disposal. `ocx restart` also restarts a connected client runtime, which serves the same bound `POST /api/system/restart` from its client listener; the CLI sends that request only while the client's 90-second restart budget still fits before replacement observation ends (`CLIENT_RESTART_REQUEST_MIN_REMAINING_MS`), and reports a supervised client's `409 restart_supervised` as "not performed, runtime kept". See [Remote Link](remote-link.md) for the rollback. -The package-tree integrity fence for live package replacement follows the [update transaction contract](ops/docs-and-release.md#package-tree-integrity-fence). After client journal reconciliation, `ocx ensure` in `src/cli/dispatch.ts` treats validated connected-client state as a successful no-op without local startup or a remote health probe; invalid or mismatched state still fails, and disconnected state keeps local ensure. `tests/cli/cli-ensure-client-state.test.ts` pins this dispatch contract so the Codex shim does not warn merely because its proxy is delegated to a hub. Startup journal reconciliation, including replacement after restart/drain, follows the [Codex-home recovery contract](codex-home.md#codex-home): already-original recovery is silent, incomplete recovery retains its journal and warns without claiming restoration, and only complete recovery that rewrites or removes an artifact reports a restore; restart admission, drain deadlines and replacement ownership are unchanged. `src/cli/start-owner-fence.ts` holds the ownership mutation lease through recovery for ordinary and supervised starts. After an early negative probe, `start` and `ensure` recheck with `START_OWNERSHIP_LIVENESS` under that lease through `observeProxyLiveness` in `src/server/proxy-liveness.ts`, probing both available loopback families concurrently for every loopback or wildcard spelling classified after the same startup normalization (including `localhost.` and case variants) within the existing per-family retry ceilings and shared deadline. Only definitive absence permits PID deletion and journal reconciliation: no alive recorded PID unless fresh readable command-line evidence confirms it is foreign, no answering runtime owner, and only unanimous aggregate connection refusals or clean foreign identities at probed endpoints. Port zero names no existing listener. Timeouts, resets, unreadable/non-200 responses, PID mismatches and exhausted deadlines remain unknown; startup refuses with a fixed message while preserving PID, journal and shared config. Unreadable/empty PID command lines and any OpenCodex invocation remain unknown; a confirmed foreign PID is never signalled. A discovered live owner preserves both records and its runtime/config bind scope in `LiveProxy.hostname`; omitted or blank hostnames become explicit `127.0.0.1` in both early and recovery discovery, so a later config change cannot replace a default loopback bind. Explicit wildcard scope remains intact, and the concrete responding address is separate `probeHostname` metadata. Integration policy uses the bind scope, so loopback discovery of a wildcard listener does not authorize tokenless Grok registration. Ensure carries `probeHostname` separately through `src/cli/ensure-desired-integrations.ts` and `src/grok/sync.ts` to `src/grok/inject.ts` for the written destination; an explicit secondary loopback listener keeps its own address and port; after lease release, start applies `decideStartWithLiveOwner` and ensure follows its existing live-owner path. Cross-home sibling marking still precedes journal recovery. The bind/publication fence remains separate, and supervised recovery checks durable service ownership before liveness or mutation. `tests/cli/cli-start-journal-order.test.ts` covers an early miss; `tests/cli/cli-start-owner-absence.test.ts` covers consecutive misses and dead-owner recovery; `tests/server/proxy-liveness-recovery.test.ts` binds classification and existing budgets; `tests/server/proxy-liveness-bind-scope.test.ts` follows a missed early probe through desired-state reconciliation and the real Grok writer for runtime, orphan and configured-port discovery; `tests/server/proxy-liveness-owner-evidence.test.ts` and `tests/cli/cli-start-owner-evidence.test.ts` cover aggregate children, both listener families, shared deadlines and foreign-PID recovery. +The package-tree integrity fence for live package replacement follows the [update transaction contract](ops/docs-and-release.md#package-tree-integrity-fence). After client journal reconciliation, `ocx ensure` in `src/cli/dispatch.ts` treats validated connected-client state as a successful no-op without local startup or a remote health probe; invalid or mismatched state still fails, and disconnected state keeps local ensure. `tests/cli/cli-ensure-client-state.test.ts` pins this dispatch contract so the Codex shim does not warn merely because its proxy is delegated to a hub. Startup journal reconciliation, including replacement after restart/drain, follows the [Codex-home recovery contract](codex-home.md#codex-home): already-original recovery is silent, incomplete recovery retains its journal and warns without claiming restoration, and only complete recovery that rewrites or removes an artifact reports a restore; restart admission, drain deadlines and replacement ownership are unchanged. `src/cli/start-owner-fence.ts` holds the ownership mutation lease through recovery for ordinary and supervised starts. After an early negative probe, `start` and `ensure` recheck with `START_OWNERSHIP_LIVENESS` under that lease through `observeProxyLiveness` in `src/server/proxy-liveness.ts`, probing both available loopback families concurrently for every loopback or wildcard spelling classified after the same startup normalization (including `localhost.` and case variants) within the existing per-family retry ceilings and shared deadline. Only definitive absence permits PID deletion and journal reconciliation: no alive recorded PID unless fresh readable command-line evidence confirms it is foreign, no answering runtime owner, and only unanimous aggregate connection refusals or clean foreign identities at probed endpoints. Port zero names no existing listener. Timeouts, resets, unreadable/non-200 responses, PID mismatches and exhausted deadlines remain unknown; startup refuses with a fixed message while preserving PID, journal and shared config. Unreadable/empty PID command lines and any OpenCodex invocation remain unknown; a confirmed foreign PID is never signalled. A discovered live owner preserves both records and its runtime/config bind scope in `LiveProxy.hostname`; omitted or blank hostnames become explicit `127.0.0.1` in both early and recovery discovery, so a later config change cannot replace a default loopback bind. Explicit wildcard scope remains intact, and the concrete responding address is separate `probeHostname` metadata. Integration policy uses the bind scope, so loopback discovery of a wildcard listener does not authorize tokenless Grok registration. Grok start and ensure/restart synchronization also recheck current desired-state admission after catalog discovery, immediately before the synchronous writer in `src/grok/sync.ts`; start additionally rejects work after shutdown begins or cleanup runs. This closes the same-process await gap without turning the desired-state and Grok files into a cross-process transaction. Ensure carries `probeHostname` separately through `src/cli/ensure-desired-integrations.ts` and `src/grok/sync.ts` to `src/grok/inject.ts` for the written destination; an explicit secondary loopback listener keeps its own address and port; after lease release, start applies `decideStartWithLiveOwner` and ensure follows its existing live-owner path. Cross-home sibling marking still precedes journal recovery. The bind/publication fence remains separate, and supervised recovery checks durable service ownership before liveness or mutation. `tests/cli/cli-start-journal-order.test.ts` covers an early miss; `tests/cli/cli-start-owner-absence.test.ts` covers consecutive misses and dead-owner recovery; `tests/server/proxy-liveness-recovery.test.ts` binds classification and existing budgets; `tests/server/proxy-liveness-bind-scope.test.ts` follows a missed early probe through desired-state reconciliation and the real Grok writer for runtime, orphan and configured-port discovery; `tests/server/proxy-liveness-owner-evidence.test.ts` and `tests/cli/cli-start-owner-evidence.test.ts` cover aggregate children, both listener families, shared deadlines and foreign-PID recovery. A busy preferred port is never resolved by starting somewhere else. Both questions a start asks about an existing proxy — the pre-bind owner check and the port-is-busy check in `src/cli/index.ts` diff --git a/tests/cli/ensure-desired-integrations-race.test.ts b/tests/cli/ensure-desired-integrations-race.test.ts index e4e858cecca..f099c708717 100644 --- a/tests/cli/ensure-desired-integrations-race.test.ts +++ b/tests/cli/ensure-desired-integrations-race.test.ts @@ -1,11 +1,14 @@ import { describe, expect, test } from "bun:test"; import { + ensureGrokFenceMatchesDesired, reconcileEnsureDesiredIntegrations, type EnsureDesiredIntegrationsDeps, } from "../../src/cli/ensure-desired-integrations"; import type { OcxConfig } from "../../src/types"; import type { GrokInjectResult } from "../../src/grok/inject"; import type { Desktop3pRemovalResult } from "../../src/claude/desktop-3p"; +import { syncGrokConfig } from "../../src/grok/sync"; +import type { CatalogModel } from "../../src/codex/catalog"; function config(overrides: { grok?: boolean; @@ -159,6 +162,22 @@ test("CLI intent with disabled intercept retains the env through Desktop-off cle }); describe("ensure desired-state races", () => { + test("Grok disabled during catalog fetch cannot be reinjected by ensure", async () => { + const h = harness(config()); + let finishCatalog!: (models: CatalogModel[]) => void; + const catalog = new Promise(resolve => { finishCatalog = resolve; }); + let injectCalls = 0; + h.deps.syncGrokConfig = (port, current, opts) => syncGrokConfig(port, current, opts, { + fetchAllModels: () => catalog, + injectGrokConfig: () => { injectCalls++; return okGrok(true); }, + }); + const pending = ensureGrokFenceMatchesDesired(10100, {}, h.deps); + h.flip(config({ grok: false })); + finishCatalog([]); + await pending; + expect(injectCalls).toBe(0); + }); + test("live-proxy OFF→ON uses the current ON snapshot instead of stripping", async () => { const staleOff = config({ grok: false, desktop: false, hostname: "stale-host", fingerprint: "fp-off" }); const currentOn = config({ hostname: "fresh-host", fingerprint: "fp-on" }); diff --git a/tests/codex-integration/codex-composed-acceptance.test.ts b/tests/codex-integration/codex-composed-acceptance.test.ts index b383c7a35c4..eb309836dc2 100644 --- a/tests/codex-integration/codex-composed-acceptance.test.ts +++ b/tests/codex-integration/codex-composed-acceptance.test.ts @@ -94,6 +94,7 @@ type StartedServer = { /** Captured during start(): the child's streams can only be read once. */ stdout: Promise; stderr: Promise; + stdoutSnapshot: () => string; }; type CapturedChildStream = { @@ -334,8 +335,8 @@ class Fixture { return { exitCode, stdout, stderr }; } - async start(): Promise { - const child = this.spawnCli(["start"]); + async start(options: { readonly preloadPaths?: readonly string[]; readonly env?: Record } = {}): Promise { + const child = this.spawnCli(["start"], this.homeA, this.userprofileA, options); const pidPath = join(this.ocx, "ocx.pid"); const runtimePath = join(this.ocx, "runtime-port.json"); // Run 35093667426 waited the full 45 s Windows watchdog with the child alive, but @@ -387,7 +388,7 @@ class Fixture { } }, "child /healthz").catch(() => diagnose("timed out waiting for child /healthz")); expect(health).toMatchObject({ pid: child.pid, port: runtime.port }); - return { process: child, runtime, stdout: stdout.completed, stderr: stderr.completed }; + return { process: child, runtime, stdout: stdout.completed, stderr: stderr.completed, stdoutSnapshot: stdout.snapshot }; } async stop(server: StartedServer): Promise { @@ -821,6 +822,88 @@ describe("WP13 composed toggle acceptance", () => { expect(await holder.exited).toBe(0); }, CASE_TIMEOUT_MS); + /** RED: remove the corresponding handleStart admission term; discovery recreates the fence. */ + test.each(["persisted-off", "shutdown"] as const)("Grok E2E: held startup discovery respects %s", async boundary => { + const fx = fixture(); + let turnEntered = false; + const enteredFile = join(fx.root, "grok-catalog-entered"); + const releaseFile = join(fx.root, "grok-catalog-release"); + const releaseCatalog = () => writeFileSync(releaseFile, "release\n"); + let releaseTurn!: () => void; + const turnGate = new Promise(resolve => { releaseTurn = resolve; }); + const provider = Bun.serve({ + port: 0, + idleTimeout: 255, // The gates below, not an idle socket timeout, bound these deliberate holds. + fetch: async request => { + const path = new URL(request.url).pathname; + if (path.endsWith("/models")) { + return Response.json({ data: [{ id: "held-model" }] }); + } + if (path.endsWith("/chat/completions")) { + turnEntered = true; + await turnGate; + return Response.json({ id: "held-turn", choices: [{ index: 0, + message: { role: "assistant", content: "done" }, finish_reason: "stop" }] }); + } + return new Response("not found", { status: 404 }); + }, + }); + const providers = { fixture: { adapter: "openai-chat", baseUrl: `http://127.0.0.1:${provider.port}/v1`, + apiKey: "fixture-key", allowPrivateNetwork: true, liveModels: true, models: ["held-model"] } }; + const grokHome = join(fx.homeA, ".grok"); + mkdirSync(grokHome); + const original = "# user grok config\n"; + writeFileSync(join(grokHome, "config.toml"), original); + fx.writeConfig({ providers, clientIntegrations: { codex: false }, shutdownTimeoutMs: HELD_REQUEST_BUDGET_MS }); + const signalFile = join(fx.root, "signal-startup-stop"); + let server: StartedServer | undefined; + let heldTurn: Promise | undefined; + const turnAbort = new AbortController(); + try { + server = await fx.start({ + preloadPaths: [resolve(repoRoot, "tests/fixtures/grok-startup-discovery-preload.ts")], + env: { OCX_TEST_GROK_ENTERED_FILE: enteredFile, OCX_TEST_GROK_RELEASE_FILE: releaseFile, + OCX_TEST_GROK_HOLD_MS: String(HELD_REQUEST_BUDGET_MS), + ...(boundary === "shutdown" ? { OCX_TEST_STARTUP_SIGNAL_FILE: signalFile } : {}) }, + }); + await waitFor(() => existsSync(enteredFile) || null, "Grok startup catalog hold"); + expect(server.stdoutSnapshot()).not.toContain("Client startup work complete."); + if (boundary === "shutdown") { + // Keep real admission occupied so shutdown cannot exit before discovery resumes. + heldTurn = fetch(`http://127.0.0.1:${server.runtime.port}/v1/chat/completions`, { + method: "POST", headers: { "content-type": "application/json", "x-opencodex-api-key": fx.dataToken }, + body: JSON.stringify({ model: "fixture/held-model", messages: [{ role: "user", content: "fixture" }] }), + signal: AbortSignal.any([turnAbort.signal, AbortSignal.timeout(HELD_REQUEST_BUDGET_MS)]), + }).then(async response => { await response.text(); return response.status; }); + void heldTurn.catch(() => {}); // Teardown joins it even when an earlier assertion fails. + await waitFor(() => turnEntered || null, "admitted provider turn"); + writeFileSync(signalFile, "SIGINT\n"); + await waitFor(() => server!.stdoutSnapshot().includes("Shutting down opencodex proxy") || null, "startup shutdown handler"); + } else { + fx.writeConfig({ providers, clientIntegrations: { codex: false, grok: false } }); + } + releaseCatalog(); + await waitFor(() => server!.stdoutSnapshot().includes("Client startup work complete.") || null, "resumed Grok startup"); + // Read before shutdown cleanup: a transient write followed by strip must fail too. + expect(server.process.exitCode).toBeNull(); + expect(readFileSync(join(grokHome, "config.toml"), "utf8")).toBe(original); + expect(existsSync(join(grokHome, "config.toml.bak-opencodex"))).toBe(false); + expect(server.stdoutSnapshot()).not.toContain("Grok Build config updated"); + releaseTurn(); + if (heldTurn) { + expect(await heldTurn).toBe(200); + expect(await server.process.exited).toBe(0); + } + } finally { + releaseCatalog(); + releaseTurn(); + turnAbort.abort(); + await heldTurn?.catch(() => {}); + try { if (server) await fx.stop(server); } + finally { provider.stop(true); } + } + }, CASE_TIMEOUT_MS); + /** RED: delete the durable Grok intent or bypass `shouldSyncGrokOnStart`; startup recreates the fence. */ test("Grok E2E: route-disabled Grok stays absent across a real startup", async () => { const fx = fixture(); diff --git a/tests/fixtures/grok-startup-discovery-preload.ts b/tests/fixtures/grok-startup-discovery-preload.ts new file mode 100644 index 00000000000..d27d3cfe2a8 --- /dev/null +++ b/tests/fixtures/grok-startup-discovery-preload.ts @@ -0,0 +1,38 @@ +// Hold only the real Grok startup caller's catalog result. Earlier Desktop/Claude +// catalog consumers must complete normally, or the test misses the post-await check. +import { mock } from "bun:test"; +import { existsSync, writeFileSync } from "node:fs"; +import { syncGrokConfig } from "../../src/grok/sync"; +import { injectGrokConfig } from "../../src/grok/inject"; +import { fetchAllModels } from "../../src/server/management/shared"; + +const entered = process.env.OCX_TEST_GROK_ENTERED_FILE; +const release = process.env.OCX_TEST_GROK_RELEASE_FILE; +if (!entered || !release) throw new Error("Grok discovery fixture requires its owned markers"); +const actualSync = syncGrokConfig; +mock.module("../../src/grok/sync", () => ({ + syncGrokConfig: (...args: Parameters) => actualSync(args[0], args[1], args[2], { + injectGrokConfig, + fetchAllModels: async config => { + const models = await fetchAllModels(config); + writeFileSync(entered, "grok-catalog-ready\n"); + const deadline = Date.now() + Number(process.env.OCX_TEST_GROK_HOLD_MS ?? 60_000); + while (!existsSync(release)) { + if (Date.now() >= deadline) throw new Error("Grok discovery fixture release timeout"); + await Bun.sleep(10); + } + return models; + }, + }), +})); + +// Invoke the child's installed handler instead of Windows TerminateProcess. +const signal = process.env.OCX_TEST_STARTUP_SIGNAL_FILE; +if (signal) { + const poller = setInterval(() => { + if (!existsSync(signal)) return; + clearInterval(poller); + process.emit("SIGINT", "SIGINT"); + }, 10); + poller.unref(); +} diff --git a/tests/providers/xai/grok-sync.test.ts b/tests/providers/xai/grok-sync.test.ts index a045aa515cb..5b7392659c5 100644 --- a/tests/providers/xai/grok-sync.test.ts +++ b/tests/providers/xai/grok-sync.test.ts @@ -25,6 +25,52 @@ function tempGrokHome(): { root: string; grokHome: string } { } describe("syncGrokConfig", () => { + test("declined caller admission performs no catalog work or file writes", async () => { + let fetchCalls = 0, injectCalls = 0; + const result = await syncGrokConfig(10190, baseConfig, { admit: () => false }, { + fetchAllModels: async () => { fetchCalls++; return []; }, + injectGrokConfig: () => { injectCalls++; return { ok: true, changed: true, message: "unexpected" }; }, + }); + expect(result).toMatchObject({ ok: true, changed: false }); + expect(fetchCalls).toBe(0); + expect(injectCalls).toBe(0); + }); + + for (const change of ["disabled", "stopping", "unchanged"] as const) { + test(`rechecks caller admission after a delayed catalog: ${change}`, async () => { + const { root, grokHome } = tempGrokHome(); + const original = "# user Grok config\n"; + writeFileSync(join(grokHome, "config.toml"), original); + let finishCatalog!: (models: CatalogModel[]) => void; + const catalog = new Promise(resolve => { finishCatalog = resolve; }); + let enabled = true, running = true, injectCalls = 0; + const options = { grokHome, admit: () => enabled && running }; + try { + const pending = syncGrokConfig(10190, baseConfig, options, { + fetchAllModels: () => catalog, + injectGrokConfig: (...args) => { injectCalls++; return injectGrokConfig(...args); }, + }); + if (change === "disabled") enabled = false; + if (change === "stopping") running = false; + finishCatalog([]); + const result = await pending; + if (change === "unchanged") { + expect(result).toMatchObject({ ok: true, changed: true }); + expect(injectCalls).toBe(1); + expect(readFileSync(join(grokHome, "config.toml"), "utf8")).toContain("opencodex managed block"); + } else { + expect(result).toMatchObject({ ok: true, changed: false }); + expect(injectCalls).toBe(0); + expect(readFileSync(join(grokHome, "config.toml"), "utf8")).toBe(original); + expect(existsSync(join(grokHome, "config.toml.bak-opencodex"))).toBe(false); + } + } finally { + finishCatalog([]); + removeTreeWithRetry(root); + } + }); + } + test("injects natives plus routed models with catalog context windows", async () => { const { root, grokHome } = tempGrokHome(); try { From 2ee32ab25f40a8eef1e99311abd4011ca9e5bbb5 Mon Sep 17 00:00:00 2001 From: Epinephrine Date: Sat, 31 Oct 2026 13:17:54 +0000 Subject: [PATCH 06/13] fix(qoder): bound CN PNG raster dimensions (#8021) * fix(qoder): bound CN PNG raster dimensions * test(qoder): encode complete PNG raster boundary fixtures * fix(qoder): explain PNG size and raster limits on rejection --- .../src/content/docs/guides/providers.md | 2 +- src/adapters/qoder/adapter.ts | 2 +- src/adapters/qoder/image-input.ts | 7 ++- structure/providers/chat-compat.md | 2 +- .../qoder-cn-image-validation.test.ts | 56 ++++++++++++++++--- 5 files changed, 56 insertions(+), 13 deletions(-) diff --git a/docs-site/src/content/docs/guides/providers.md b/docs-site/src/content/docs/guides/providers.md index 5a43f759fe2..ebd12465265 100644 --- a/docs-site/src/content/docs/guides/providers.md +++ b/docs-site/src/content/docs/guides/providers.md @@ -1450,7 +1450,7 @@ OpenCodex provides official adapter support for Qoder through the `qoder` (Globa - **Token usage:** Qoder CLI reports zero token counts unless its usage switch is on, so each preset turns it on in its scoped CLI child environment: `QODER_EXPOSE_TOKEN_USAGE=true` for Global and `QODERCN_EXPOSE_TOKEN_USAGE=true` for CN, which reads only its own prefix. - **Tool Ownership:** The CLI runs single-turn `stream-json` with `--tools ""`, `--strict-mcp-config`, setting sources disabled, and session persistence disabled, so Codex keeps exclusive tool ownership. Client-declared tools are exposed through an isolated request-scoped MCP server. - **Output guard:** If the CLI echoes OpenCodex tool-call/result replay envelopes into assistant text, the turn fails with `vendor_scaffold_detected` (502) instead of returning the envelope as a successful answer, and neither reasoning nor tool calls following the envelope are forwarded. Reasoning starts a fresh replay-text block, so earlier prose or code fences cannot hide an envelope in later text. If the CLI's turn already ended with its own error, that error is reported instead, still without the envelope. The proxy does not retry that failure automatically. Ordinary prose and closed fenced examples follow the shared envelope recognizer; an unverified overflowing fenced example is refused. Assistant text preserves Unicode characters split across CLI frames; unpaired surrogates are replaced with U+FFFD. -- **Image Input:** Global continues forwarding supported images through the shared projection without a per-model vision allowlist. CN retains its vision-sidecar defaults. Its raw-image route admits only one canonical base64 PNG of at most 1 MiB decoded in a single user turn with `DeepSeek-Flash` or `Qwen3.8-Flash`. Use `qoderclicn 1.1.70`: [issue #7516](https://github.com/lidge-jun/opencodex/issues/7516) reports Windows 11 x64 live success for those models on both `--attachment` and inline `--input-format stream-json --output-format stream-json` stdin; OpenCodex uses only stdin. The adapter validates base64 length/padding, decoded PNG signature and basic IHDR fields before CLI discovery; it does not decompress pixels or validate every PNG chunk/CRC. The 1 MiB ceiling is an OpenCodex resource policy, not a verified vendor maximum: the reported live evidence covers a 96×96 PNG, whose RGBA raster is only 36 KiB. Larger inputs require separate verification. Other CLI versions have not been verified, and the adapter does not probe the installed version. CN refuses raw URL images, other image formats/models, multiple images, tool-result images and image-bearing history/multi-turn replay before starting the CLI with `unsupported_input_modality`; use the vision sidecar for those inputs. Injected CLI tests cover serialization and cancellation without claiming live upstream support. Qoder's supported projection keeps image blocks in conversation order and marks unsupported references as `[image omitted: unsupported reference]`. Global stateless replay can still associate historical images unreliably with their original text; CN does not enable that unverified replay surface. +- **Image Input:** Global continues forwarding supported images through the shared projection without a per-model vision allowlist. CN retains its vision-sidecar defaults. Its raw-image route admits only one canonical base64 PNG of at most 1 MiB decoded, 8192 pixels per edge, and 16 million total pixels in a single user turn with `DeepSeek-Flash` or `Qwen3.8-Flash`. Use `qoderclicn 1.1.70`: [issue #7516](https://github.com/lidge-jun/opencodex/issues/7516) reports Windows 11 x64 live success for those models on both `--attachment` and inline `--input-format stream-json --output-format stream-json` stdin; OpenCodex uses only stdin. The adapter validates base64 length/padding, decoded PNG signature and basic IHDR fields before CLI discovery; it does not decompress pixels or validate every PNG chunk/CRC. These ceilings are OpenCodex resource policy, not verified vendor maxima: the reported live evidence covers a 96×96 PNG, whose RGBA raster is only 36 KiB. Larger inputs require separate verification. Other CLI versions have not been verified, and the adapter does not probe the installed version. CN refuses raw URL images, other image formats/models, multiple images, tool-result images and image-bearing history/multi-turn replay before starting the CLI with `unsupported_input_modality`; use the vision sidecar for those inputs. Injected CLI tests cover serialization and cancellation without claiming live upstream support. Qoder's supported projection keeps image blocks in conversation order and marks unsupported references as `[image omitted: unsupported reference]`. Global stateless replay can still associate historical images unreliably with their original text; CN does not enable that unverified replay surface. - **PAT recovery:** For both Qoder presets, a multi-key API-key pool can switch to another PAT on a supported authentication rejection or account-level quota exhaustion, but only before response content or an event that makes replay unsafe. Recovery requires Qoder-specific classification and respects credential-hop and send budgets; an HTTP 401/429 or quota message alone is not permission to replay. Single-key configurations do not rotate. Sharing the CLI runner does not enable this API-key recovery for CodeBuddy or self-authenticated Claude CLI. - **Quota:** No public quota API is used, so totals and reset times are unavailable. Codes 110 (account daily limit) and 118 (personal Credits exhaustion) report `insufficient_quota` and permit consideration of PAT recovery. Other supported quota codes also report `insufficient_quota` but do not authorize it. Numeric error codes take precedence: an unrecognized numeric code does not fall back to quota or authentication text. Without a numeric code, only the supported account-daily message beginning “You've reached your daily usage limit for Chat”, with no team/model/shared qualifier, grants quota recovery; generic credit-limit text does not. Only quota failures eligible for PAT recovery enter key cooldown handling. When such a failure has no reset time, the failed key uses the existing default cooldown; nonrecoverable quota classifications do not create a cooldown through this path, and no next-day reset is inferred. - **Operators:** Qoder Global is operated by BRIGHT ZENITH PRIVATE LIMITED under the [product service terms](https://qoder.com/product-service); Qoder CN by 通义云启（杭州）信息技术有限公司 with Alibaba Cloud. Verify `ocx provider test qoder` (or `qoder-cn`) after configuring. diff --git a/src/adapters/qoder/adapter.ts b/src/adapters/qoder/adapter.ts index 3da6e0407cd..7bf29ddbe39 100644 --- a/src/adapters/qoder/adapter.ts +++ b/src/adapters/qoder/adapter.ts @@ -280,7 +280,7 @@ export function createQoderAdapter(provider: OcxProviderConfig, deps: QoderAdapt emit({ type: "error", status: 400, errorType: "invalid_request_error", code: "unsupported_input_modality", retryable: false, - message: "Qoder CN raw-image routing is verified only with qoderclicn 1.1.70, DeepSeek-Flash or Qwen3.8-Flash, and one base64 PNG in a single user turn on stream-json stdin. URL, multiple-image, tool-result and history replay surfaces remain unverified; use the vision sidecar for those inputs.", + message: "Qoder CN raw-image routing is verified only with qoderclicn 1.1.70, DeepSeek-Flash or Qwen3.8-Flash, and one base64 PNG in a single user turn on stream-json stdin. The PNG must be at most 1 MiB decoded, with each edge at most 8192 pixels and at most 16,000,000 total pixels. URL, multiple-image, tool-result and history replay surfaces remain unverified; use the vision sidecar for those inputs.", }); return; } diff --git a/src/adapters/qoder/image-input.ts b/src/adapters/qoder/image-input.ts index ea67a032e28..2e9a72008bc 100644 --- a/src/adapters/qoder/image-input.ts +++ b/src/adapters/qoder/image-input.ts @@ -4,6 +4,8 @@ import { buildConversationInput } from "../coding-agent/protocol"; // Local resource policy: #7516 verifies a 96x96 PNG, not a vendor size maximum. // 1 MiB leaves ample room over its 36 KiB RGBA raster while bounding stdin copies. const MAX_CN_PNG_BYTES = 1024 * 1024; +const MAX_CN_PNG_EDGE = 8192; +const MAX_CN_PNG_PIXELS = 16_000_000; const PNG_DATA_PREFIX = "data:image/png;base64,"; const PNG_SIGNATURE = Buffer.from([137, 80, 78, 71, 13, 10, 26, 10]); const PNG_DEPTHS: Readonly> = { @@ -25,8 +27,9 @@ function boundedPngDataUrl(imageUrl: string): boolean { || bytes.readUInt32BE(8) !== 13 || bytes.readUInt32BE(12) !== 0x49484452) return false; const width = bytes.readUInt32BE(16); const height = bytes.readUInt32BE(20); - // Minimal IHDR validation only; this does not decompress or validate all chunks. - return width > 0 && width <= 0x7fffffff && height > 0 && height <= 0x7fffffff + // Bound declared raster work before the child can decode compressed pixels. + return width > 0 && width <= MAX_CN_PNG_EDGE && height > 0 && height <= MAX_CN_PNG_EDGE + && width * height <= MAX_CN_PNG_PIXELS && (PNG_DEPTHS[bytes[25]!] ?? []).includes(bytes[24]!) && bytes[26] === 0 && bytes[27] === 0 && (bytes[28] === 0 || bytes[28] === 1); } diff --git a/structure/providers/chat-compat.md b/structure/providers/chat-compat.md index c9c33cb8278..d9d648da1c4 100644 --- a/structure/providers/chat-compat.md +++ b/structure/providers/chat-compat.md @@ -581,7 +581,7 @@ transport or URL fetch is added, and no client filename, payload, URL or metadat charges supported audio by decoded size at the conservative 256-bytes-per-token proxy, not an upstream token formula. The shared coding-agent projection in `src/adapters/coding-agent/` carries supported tool-result images as real image blocks in conversation order, history before current. Current tool-result prose retains `[image attached below]` and `[image omitted: unsupported reference]`. Qoder alone opts into omission markers for single-user input and historical user/tool-result prose through `src/adapters/qoder/image-input.ts`; CodeBuddy retains its previous bytes (historical `[image]` and single-user unsupported-reference omission), and Claude keeps its stable replay builder. Vendor tool execution stays disabled. -CodeBuddy refuses an unquoted, line-oriented full-width-bar DSML `calls` container followed by a named bare or namespaced invoke control line in either output channel; it preserves preceding answer text, never promotes vendor prose into execution authority, and leaves discussed or quoted literals and code examples untouched. Qoder Global continues forwarding supported images without a per-model vision allowlist. CN retains its sidecar classification. Its raw-image route admits only one canonical base64 PNG of at most 1 MiB decoded in one non-developer user message, on `DeepSeek-Flash` or `Qwen3.8-Flash`; developer images, URL references, other media types/models, multiple images, tool-result images and image-bearing multi-turn replay return a fixed non-retryable 400 `unsupported_input_modality` before CLI discovery/spawn. Before CLI discovery, admission bounds encoded length, validates base64 length/padding and round-trip decoding, then checks the PNG signature and basic IHDR fields (13-byte first chunk, positive PNG-range dimensions, legal depth/colour pair, compression/filter/interlace). It does not decompress pixels or validate all chunks/CRCs. Invalid or oversized images receive the same fixed error event; text-only CN turns remain unrestricted by this gate. +CodeBuddy refuses an unquoted, line-oriented full-width-bar DSML `calls` container followed by a named bare or namespaced invoke control line in either output channel; it preserves preceding answer text, never promotes vendor prose into execution authority, and leaves discussed or quoted literals and code examples untouched. Qoder Global continues forwarding supported images without a per-model vision allowlist. CN retains its sidecar classification. Its raw-image route admits only one canonical base64 PNG of at most 1 MiB decoded in one non-developer user message, on `DeepSeek-Flash` or `Qwen3.8-Flash`; developer images, URL references, other media types/models, multiple images, tool-result images and image-bearing multi-turn replay return a fixed non-retryable 400 `unsupported_input_modality` before CLI discovery/spawn. Before CLI discovery, admission bounds encoded length, validates base64 length/padding and round-trip decoding, then checks the PNG signature and basic IHDR fields (13-byte first chunk, positive dimensions no larger than 8192 per edge or 16 million pixels, legal depth/colour pair, compression/filter/interlace). It does not decompress pixels or validate all chunks/CRCs. Invalid or oversized images receive the same fixed error event; text-only CN turns remain unrestricted by this gate. Issue #7516 reports live verification on Windows 11 x64 with `qoderclicn 1.1.70` for those two models and a single PNG, through both `--attachment` and inline `--input-format stream-json --output-format stream-json` stdin. OpenCodex uses only stdin; attachment flags are not added. The 1 MiB decoded ceiling is local resource policy, not a vendor maximum: the reported 96×96 PNG has a 36 KiB RGBA raster; larger upstream inputs remain unverified. This is reported evidence, not a fresh live test or a guarantee for other CLI versions; operators must use the verified version (no runtime version probe is added). Injected CLI tests prove serialization, omission markers, refusals and cancellation, not upstream image understanding. Global still replays historical images with `--no-session-persistence`, whose text/image association remains unverified across turns. The scaffold/replay output guards ([ADR-7517](../clients/qoder.md#qoder-assistant-text-replay-guard)) and selector ownership ([ADR-7512](../clients/qoder.md#qoder-settings-ownership)) remain independent. Canonical Responses identity sanitation and narrowly scoped pre-output combo recovery follow [request-local target compatibility](../runtime.md#request-local-target-compatibility); other adapter contracts remain unchanged. diff --git a/tests/providers/qoder-cn-image-validation.test.ts b/tests/providers/qoder-cn-image-validation.test.ts index 57d12a9d126..edf3291aeca 100644 --- a/tests/providers/qoder-cn-image-validation.test.ts +++ b/tests/providers/qoder-cn-image-validation.test.ts @@ -1,4 +1,5 @@ import { beforeEach, describe, expect, test } from "bun:test"; +import { deflateSync } from "node:zlib"; import { createQoderAdapter } from "../../src/adapters/qoder/adapter"; import { qoderCnImageInputSupported } from "../../src/adapters/qoder/image-input"; import { clearQoderBinaryCache } from "../../src/adapters/qoder/profiles"; @@ -20,19 +21,45 @@ function changedHeader(offset: number, value: number): Buffer { return bytes; } -// Insert a valid ancillary tEXt chunk before IEND to exercise exact decoded sizes. -function sizedPng(size: number): Buffer { - const chunk = Buffer.alloc(size - png.length, 0x20); - chunk.writeUInt32BE(chunk.length - 12, 0); - chunk.write("tEXt", 4, "ascii"); - chunk.write("Note\0", 8, "ascii"); +function changedDimensions(width: number, height: number): Buffer { + const bytes = Buffer.from(png); + bytes.writeUInt32BE(width, 16); + bytes.writeUInt32BE(height, 20); + return bytes; +} + +function pngChunk(type: string, data: Buffer): Buffer { + const chunk = Buffer.alloc(data.length + 12); + chunk.writeUInt32BE(data.length, 0); + chunk.write(type, 4, "ascii"); + data.copy(chunk, 8); let crc = 0xffffffff; for (const byte of chunk.subarray(4, -4)) { crc ^= byte; for (let bit = 0; bit < 8; bit++) crc = (crc >>> 1) ^ ((crc & 1) ? 0xedb88320 : 0); } chunk.writeUInt32BE((crc ^ 0xffffffff) >>> 0, chunk.length - 4); - return Buffer.concat([png.subarray(0, -12), chunk, png.subarray(-12)]); + return chunk; +} + +// Insert a valid ancillary tEXt chunk before IEND to exercise exact decoded sizes. +function sizedPng(size: number): Buffer { + const text = Buffer.alloc(size - png.length - 12, 0x20); + text.write("Note\0", 0, "ascii"); + return Buffer.concat([png.subarray(0, -12), pngChunk("tEXt", text), png.subarray(-12)]); +} + +function rasterPng(width: number, height: number): Buffer { + const header = Buffer.alloc(13); + header.writeUInt32BE(width, 0); + header.writeUInt32BE(height, 4); + header[8] = 8; // 8-bit grayscale; no interlace. + // Every scanline includes filter 0 followed by width black pixels. + const raster = Buffer.alloc((width + 1) * height); + return Buffer.concat([ + png.subarray(0, 8), pngChunk("IHDR", header), + pngChunk("IDAT", deflateSync(raster)), pngChunk("IEND", Buffer.alloc(0)), + ]); } beforeEach(() => clearQoderBinaryCache()); @@ -57,6 +84,9 @@ describe("Qoder CN decoded PNG admission", () => { ["zero height", dataUrl(changedHeader(23, 0))], ["width beyond PNG range", dataUrl(changedHeader(16, 0x80))], ["height beyond PNG range", dataUrl(changedHeader(20, 0x80))], + ["width above edge ceiling", dataUrl(changedDimensions(8193, 1))], + ["height above edge ceiling", dataUrl(changedDimensions(1, 8193))], + ["pixel count above ceiling", dataUrl(changedDimensions(4001, 4000))], ["invalid bit depth", dataUrl(changedHeader(24, 3))], ["invalid colour type", dataUrl(changedHeader(25, 1))], ["invalid compression", dataUrl(changedHeader(26, 1))], @@ -77,7 +107,7 @@ describe("Qoder CN decoded PNG admission", () => { expect(spawns).toBe(0); expect(events).toEqual([{ type: "error", status: 400, errorType: "invalid_request_error", code: "unsupported_input_modality", retryable: false, - message: "Qoder CN raw-image routing is verified only with qoderclicn 1.1.70, DeepSeek-Flash or Qwen3.8-Flash, and one base64 PNG in a single user turn on stream-json stdin. URL, multiple-image, tool-result and history replay surfaces remain unverified; use the vision sidecar for those inputs.", + message: "Qoder CN raw-image routing is verified only with qoderclicn 1.1.70, DeepSeek-Flash or Qwen3.8-Flash, and one base64 PNG in a single user turn on stream-json stdin. The PNG must be at most 1 MiB decoded, with each edge at most 8192 pixels and at most 16,000,000 total pixels. URL, multiple-image, tool-result and history replay surfaces remain unverified; use the vision sidecar for those inputs.", }]); }); @@ -86,4 +116,14 @@ describe("Qoder CN decoded PNG admission", () => { expect(qoderCnImageInputSupported(request(dataUrl(sizedPng(ceiling))))).toBe(true); expect(qoderCnImageInputSupported(request(dataUrl(sizedPng(ceiling - 1))))).toBe(true); }); + + test.each([ + ["pixel ceiling", 8000, 2000], + ["width ceiling", 8192, 1], + ["height ceiling", 1, 8192], + ] as const)("admits a complete PNG at the %s raster boundary", (_name, width, height) => { + const bytes = rasterPng(width, height); + expect(bytes.byteLength).toBeLessThanOrEqual(ceiling); + expect(qoderCnImageInputSupported(request(dataUrl(bytes)))).toBe(true); + }); }); From afafe6d360120f315aeba48122274400942cdc89 Mon Sep 17 00:00:00 2001 From: Epinephrine Date: Sat, 31 Oct 2026 13:18:11 +0000 Subject: [PATCH 07/13] fix(cli): preserve server-issued batch for role suggestion --apply (#8022) * fix(cli): batch role suggestion history writes * fix(cli): bound role suggestion apply and report partial results Refuse more than ROLE_HISTORY_LIMIT changed suggestions before any role PUT so a recorded operation remains one listable history batch. Preserve server-issued batch reuse for accepted operations. Report confirmed writes, an unconfirmed role, unattempted roles and the known batch before propagating the original failure. Do not replay or roll back a request whose response may have been lost. Add real-handler coverage for the 50/51 boundary, whole-batch restore, no-op exclusions, partial refusal and lost acknowledgements. Document the apply limit and recovery contract. * docs(cli): clarify localized role apply limits and validate durable batches --- .../src/content/docs/guides/integrations.md | 5 + .../content/docs/ja/guides/integrations.md | 2 + .../content/docs/ko/guides/integrations.md | 2 + .../src/content/docs/reference/cli/agents.md | 17 ++ .../content/docs/ru/guides/integrations.md | 2 + .../content/docs/zh-cn/guides/integrations.md | 2 + scripts/test-layout/layout.json | 2 +- src/cli/agent.ts | 59 +++- structure/cli-management.md | 2 + tests/cli/cli-headless-parity.test.ts | 4 +- tests/cli/cli-role-suggestion-history.test.ts | 256 ++++++++++++++++++ tests/fixtures/test-layout-expected.json | 2 +- 12 files changed, 336 insertions(+), 19 deletions(-) create mode 100644 tests/cli/cli-role-suggestion-history.test.ts diff --git a/docs-site/src/content/docs/guides/integrations.md b/docs-site/src/content/docs/guides/integrations.md index ae6a4775100..3d427ac444a 100644 --- a/docs-site/src/content/docs/guides/integrations.md +++ b/docs-site/src/content/docs/guides/integrations.md @@ -1036,6 +1036,11 @@ ocx agent roles suggest ocx agent roles suggest --model xai/grok-4.5 --apply ``` +The CLI's `--apply` accepts up to 50 changed roles in one history batch. A larger operation is +refused before any role update; preview it and apply individual choices with `ocx agent roles set`. +An interrupted operation reports confirmed writes and the unconfirmed request, and leaves earlier +writes applied. See the [CLI apply and recovery contract](/reference/cli/agents/) before retrying. + Under the table, **Change history** lists the newest changes a save, an auto-assign apply or `ocx agent roles set` wrote, newest first, in whole batches and never more than 50, so an Apply all is listed with every role it wrote or not at all, with each role's model and effort before and after. One Apply all is one entry. **Restore** writes the earlier values back through the same save, so a restore is listed too. A save never removes a model pin, so a role that had none keeps its current model and gets only its effort back. The history lives in `integrations/codex-agent-roles.jsonl` under the opencodex config directory, which retains at most 256 KiB and 1000 rows by dropping its oldest whole batches. Compaction targets 128 KiB and preserves a Restore together with its complete source batch when they fit both limits, making room by dropping unrelated older batches; history can still expire as later writes arrive. The writer captures the bounded history before appending so a Restore does not evict its own source at the retention boundary; the omo card's Restore center above tracks only `~/.omo/agent/models.json`. Each change records the Codex home it was made in, and only the current home's changes are listed or restored, so after pointing `CODEX_HOME` elsewhere the list starts empty and a Restore from another home's history is refused. If the history file cannot be read, the section says so in place of the list, and the role table keeps working. ## Kilo diff --git a/docs-site/src/content/docs/ja/guides/integrations.md b/docs-site/src/content/docs/ja/guides/integrations.md index 78a542c29b8..e34ff7db555 100644 --- a/docs-site/src/content/docs/ja/guides/integrations.md +++ b/docs-site/src/content/docs/ja/guides/integrations.md @@ -245,6 +245,8 @@ LazyCodex が検出されると、omo タブの **omo (Codex / LazyCodex)** セ **自動割り当て** は、必要な能力と推論負荷を評価したロールのうち、選んだモデルの推論負荷の段階リストが空でないすべてのロールに推論負荷を提案します。**適用**、**すべて適用**、または `ocx agent roles suggest --apply` は、そのロールの `model_reasoning_effort` を追加または置き換えます。推論負荷は選んだモデル自身の段階から、最も低い段階、既定の段階、既定より 1 つ上の段階、最も高い段階のいずれかになります。段階リストが空なら推論負荷を提案せず（`proposedEffort: null`）、適用時は `effort` を送らないため既存の固定値はそのまま残ります。`null` の提案が固定値を消すことはありません。 +CLI の `ocx agent roles suggest --apply` は、1 つの履歴バッチで最大 **50 件の変更対象ロール**を適用します。設定が既に一致するロール、評価されていないロール、割当先のないロールは数えません。50 件を超える場合は、ロールの更新を一件も送信せず終了コード 5 で拒否します。`--apply` なしでプレビューし、個別の変更には `ocx agent roles set [--effort ]` を使ってください。途中で失敗した場合の確認済み・未確認の更新と再試行の注意点は、[CLI の適用と復旧の契約](/reference/cli/agents/)を参照してください。 + ## Kilo Kilo CLI、VS Code、JetBrains は同じグローバル設定を共有します。この統合は `~/.config/kilo` 内の `kilo.jsonc`、`kilo.json`、`opencode.jsonc`、`opencode.json`、`config.json` のうち最初に存在するファイルに `provider.opencodex` を書き込みます。`XDG_CONFIG_HOME` でこのディレクトリを変更できます。候補がなければ `kilo.jsonc` を作成します。プロジェクト設定には書き込みません。 diff --git a/docs-site/src/content/docs/ko/guides/integrations.md b/docs-site/src/content/docs/ko/guides/integrations.md index ba1a77a7ea9..db7efc81b07 100644 --- a/docs-site/src/content/docs/ko/guides/integrations.md +++ b/docs-site/src/content/docs/ko/guides/integrations.md @@ -255,6 +255,8 @@ LazyCodex가 감지되면 omo 탭의 **omo (Codex / LazyCodex)** 섹션에서 `$ **자동 배정**은 필요한 능력과 추론 강도를 판정한 역할 가운데 선택된 모델의 강도 단계 목록이 비어 있지 않은 모든 역할에 추론 강도를 제안하며, **적용**, **모두 적용** 또는 `ocx agent roles suggest --apply`는 그 역할의 `model_reasoning_effort` 값을 추가하거나 바꿉니다. 강도는 선택된 모델 자체의 단계에서 정하며, 가장 낮은 단계, 기본 단계, 기본보다 한 단계 높은 단계, 가장 높은 단계 중 하나입니다. 단계 목록이 비어 있으면 강도를 제안하지 않고(`proposedEffort: null`), 적용할 때 `effort`를 보내지 않아 기존 강도 고정을 그대로 둡니다. `null` 제안은 고정값을 지우지 않습니다. +CLI의 `ocx agent roles suggest --apply`는 한 이력 묶음에 **변경할 역할을 최대 50개** 적용합니다. 이미 같은 값으로 설정됐거나 평가·배정되지 않은 역할은 세지 않습니다. 50개를 넘으면 역할 업데이트를 하나도 보내지 않고 종료 코드 5로 거부합니다. `--apply` 없이 미리 확인한 뒤, 개별 변경에는 `ocx agent roles set [--effort ]`을 사용하세요. 도중에 실패했을 때 확인된 쓰기와 미확인 요청을 구분하고 재시도하는 방법은 [CLI 적용·복구 규칙](/reference/cli/agents/)을 참고하세요. + ## Kilo Kilo CLI, VS Code, JetBrains는 전역 설정을 공유합니다. 이 연동은 `~/.config/kilo` 아래의 `kilo.jsonc`, `kilo.json`, `opencode.jsonc`, `opencode.json`, `config.json` 중 먼저 존재하는 파일에 `provider.opencodex`를 씁니다. `XDG_CONFIG_HOME`로 이 디렉터리를 옮길 수 있습니다. 후보 파일이 없으면 `kilo.jsonc`를 만듭니다. 프로젝트 설정은 수정하지 않습니다. diff --git a/docs-site/src/content/docs/reference/cli/agents.md b/docs-site/src/content/docs/reference/cli/agents.md index bce042afced..11b7f0269cf 100644 --- a/docs-site/src/content/docs/reference/cli/agents.md +++ b/docs-site/src/content/docs/reference/cli/agents.md @@ -34,6 +34,23 @@ prints a proposed model and effort per role without writing anything. `--apply` through the same write as `set`, skipping and naming the roles whose model and effort already match. See [Auto-assign](/guides/integrations/#auto-assign). +One `--apply` accepts at most **50 changed roles**, so its recorded changes fit one complete +history batch and can be restored together in the dashboard's **Change history**. Already-set, +unsized and unassigned roles do not count toward this limit. If more than 50 roles would change, +the command exits 5 before sending any role update. Preview without `--apply`, then use +`ocx agent roles set [--effort ]` for individual changes. + +Updates run sequentially. If a request fails, earlier confirmed writes remain applied, later +roles are not attempted, and the command keeps the original nonzero error exit. The receipt names +confirmed writes, the role whose write was not confirmed, roles not attempted, and the server-issued +history batch when known. A lost response can follow a saved write, including the first write before +its batch id is received; inspect `ocx agent roles --json` and the dashboard history before retrying. +No automatic retry or rollback occurs. History remains subject to its existing availability and +retention limits; a batch id does not guarantee that every write was recorded or is still restorable. +With `--json`, an interrupted apply adds `complete: false`, `unconfirmedRole` and `unattemptedRoles` +beside the existing `applied`, `skipped` and `omoJsoncNotWritten` arrays. A preflight refusal instead +adds `refusal` with code `too_many_role_changes`, the limit and the proposed change count. + Human-readable suggestion output displays terminal control characters as visible escapes. Use `--json` when you need the original suggestion values without presentation escaping. diff --git a/docs-site/src/content/docs/ru/guides/integrations.md b/docs-site/src/content/docs/ru/guides/integrations.md index 47299f9e003..6afaa08e162 100644 --- a/docs-site/src/content/docs/ru/guides/integrations.md +++ b/docs-site/src/content/docs/ru/guides/integrations.md @@ -579,6 +579,8 @@ OpenCodex требует явного `--overwrite-conflict`. Отключени **Автоназначение** предлагает уровень для каждой оценённой роли, у выбранной модели которой непустой список уровней, а **Применить**, **Применить все** или `ocx agent roles suggest --apply` добавляют или заменяют её значение `model_reasoning_effort`. Уровень выбирается из собственных уровней модели: самый низкий, уровень по умолчанию, на один выше уровня по умолчанию или самый высокий. Для пустого списка уровень не предлагается (`proposedEffort: null`); **Применить** тогда не отправляет `effort` и оставляет закреплённый уровень как есть. Предложение `null` никогда не сбрасывает закреплённый уровень. +Команда CLI `ocx agent roles suggest --apply` применяет не более **50 изменяемых ролей** в одном пакете истории. Роли с уже совпадающими настройками, без оценки или без назначения не учитываются. При превышении лимита команда завершается с кодом 5 до отправки любого обновления роли. Просмотрите предложения без `--apply`, затем применяйте отдельные изменения через `ocx agent roles set [--effort ]`. Подтверждённые записи, запросы с неподтверждённым результатом и порядок повторной попытки описаны в [контракте применения и восстановления CLI](/reference/cli/agents/). + ## Kilo Kilo CLI, VS Code и JetBrains используют общую глобальную конфигурацию. Интеграция записывает `provider.opencodex` в первый существующий файл среди `kilo.jsonc`, `kilo.json`, `opencode.jsonc`, `opencode.json` и `config.json` в `~/.config/kilo`. Переменная `XDG_CONFIG_HOME` переносит этот каталог. Если файлов нет, создаётся `kilo.jsonc`. Конфигурация проекта не изменяется. diff --git a/docs-site/src/content/docs/zh-cn/guides/integrations.md b/docs-site/src/content/docs/zh-cn/guides/integrations.md index 41c8042427e..6b8c37a9f81 100644 --- a/docs-site/src/content/docs/zh-cn/guides/integrations.md +++ b/docs-site/src/content/docs/zh-cn/guides/integrations.md @@ -243,6 +243,8 @@ Undo 会恢复**两个原始字节串**，包括原本不存在的文件。操 **自动分配** 会为每个已评估、且所选模型的推理强度级别列表不为空的角色提出推理强度；**应用**、**全部应用** 或 `ocx agent roles suggest --apply` 会插入或替换该角色的 `model_reasoning_effort`。推理强度取自所选模型自身的级别：最低级别、默认级别、比默认高一级，或最高级别。级别列表为空时不提出推理强度（`proposedEffort: null`）；此时 **应用** 会省略 `effort`，保留已有的固定强度。`null` 提案永远不会清除固定的推理强度。 +CLI 命令 `ocx agent roles suggest --apply` 在一个历史批次中最多应用 **50 个需要更改的角色**。设置已一致、未完成评估或未分配的角色不计入限额。超过 50 个时，命令会在发送任何角色更新之前以退出码 5 拒绝操作。请先不带 `--apply` 预览，再用 `ocx agent roles set [--effort ]` 逐个应用更改。中途中断后如何区分已确认的写入、结果未确认的请求以及安全重试，请参阅 [CLI 应用与恢复约定](/reference/cli/agents/)。 + ## Kilo Kilo CLI、VS Code 和 JetBrains 共用一份全局配置。此集成只在 `~/.config/kilo` 下最先存在的 `kilo.jsonc`、`kilo.json`、`opencode.jsonc`、`opencode.json` 或 `config.json` 中写入 `provider.opencodex`。`XDG_CONFIG_HOME` 可以迁移该目录；如果候选文件均不存在，则创建 `kilo.jsonc`。不会修改项目配置。 diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 95b9ca3dbf1..f4e44633c98 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -499,7 +499,7 @@ "cli-dto-fidelity.test.ts": "cli", "cli-export-command.test.ts": "cli", "cli-head.test.ts": "cli", - "cli-headless-parity.test.ts": "cli", + "cli-headless-parity.test.ts": "cli", "cli-role-suggestion-history.test.ts": "cli", "cli-help.test.ts": "cli", "cli-help-paths.test.ts": "cli", "cli-help-navigation.test.ts": "cli", diff --git a/src/cli/agent.ts b/src/cli/agent.ts index 8f0f7eecca3..48928cacd8a 100644 --- a/src/cli/agent.ts +++ b/src/cli/agent.ts @@ -1,5 +1,6 @@ import { CliUsageError, + RuntimeApiError, csv, desktopSwitchApplyReason, printData, @@ -345,27 +346,48 @@ async function suggestRoles(args: string[], wantsJson: boolean, deps: RuntimeApi ); const proposals = result.proposals ?? []; const applied: Array<{ role: string; model: string; effort?: string }> = []; - const skipped: string[] = []; + const skipped = apply ? proposals.filter(p => p.status === "proposed" && p.proposedModel && proposalAlreadySet(p)).map(p => p.role) : []; + const changes = proposals.filter((p): p is CodexRoleProposal & { proposedModel: string } => + p.status === "proposed" && !!p.proposedModel && !proposalAlreadySet(p)); + const { ROLE_HISTORY_LIMIT } = await import("../codex/role-model-history"); const omoJsoncNotWritten: Array<{ role: string; status: string }> = []; + let batch: string | undefined; + let failure: { cause: unknown; unconfirmedRole: string; unattemptedRoles: string[] } | undefined; if (apply) { - for (const proposal of proposals) { - if (proposal.status !== "proposed" || !proposal.proposedModel) continue; - if (proposalAlreadySet(proposal)) { - skipped.push(proposal.role); - continue; + // Splitting into several batches would hide earlier changes from the bounded Restore list. + if (changes.length > ROLE_HISTORY_LIMIT) { + const message = `${changes.length} changed proposals exceed the ${ROLE_HISTORY_LIMIT}-role batch limit. No role files were written. Preview without --apply and use 'ocx agent roles set' for individual changes.`; + printData({ ...result, applied, skipped, omoJsoncNotWritten, complete: false, + refusal: { code: "too_many_role_changes", limit: ROLE_HISTORY_LIMIT, count: changes.length }, + unattemptedRoles: changes.map(p => p.role) }, wantsJson, [message]); + throw new RuntimeApiError(message, 409, null); + } + for (const [index, proposal] of changes.entries()) { + const body = { + model: proposal.proposedModel, + ...(proposal.proposedEffort ? { effort: proposal.proposedEffort } : {}), + ...(batch ? { batch } : {}), + }; + let written: { batch?: string; omoJsonc?: { status?: string } } | null; + try { + written = await runtimeRequest( + `/api/codex-agent-roles/${encodeURIComponent(proposal.role)}`, + { method: "PUT", body: JSON.stringify(body) }, + deps, + ); + } catch (cause) { + // A lost response can follow a persisted write: report uncertainty and do not replay it. + failure = { cause, unconfirmedRole: proposal.role, unattemptedRoles: changes.slice(index + 1).map(p => p.role) }; + break; } - const body = { model: proposal.proposedModel, ...(proposal.proposedEffort ? { effort: proposal.proposedEffort } : {}) }; - const written = await runtimeRequest<{ omoJsonc?: { status?: string } } | null>( - `/api/codex-agent-roles/${encodeURIComponent(proposal.role)}`, - { method: "PUT", body: JSON.stringify(body) }, - deps, - ); - applied.push({ role: proposal.role, ...body }); + batch = written?.batch ?? batch; + applied.push({ role: proposal.role, model: body.model, ...(proposal.proposedEffort ? { effort: proposal.proposedEffort } : {}) }); const omoStatus = written?.omoJsonc?.status; if (omoStatus && OMO_JSONC_NOT_WRITTEN.has(omoStatus)) omoJsoncNotWritten.push({ role: proposal.role, status: omoStatus }); } } - printData(apply ? { ...result, applied, skipped, omoJsoncNotWritten } : result, wantsJson, [ + printData(apply ? { ...result, applied, skipped, omoJsoncNotWritten, ...(batch ? { batch } : {}), + ...(failure ? { complete: false, unconfirmedRole: failure.unconfirmedRole, unattemptedRoles: failure.unattemptedRoles } : {}) } : result, wantsJson, [ `Sized with ${result.sizingModel ?? "unknown"}.`, ...proposals.map(p => p.status === "unsized" ? `${p.role}: not sized (${p.reason ?? "unknown"})` @@ -374,9 +396,16 @@ async function suggestRoles(args: string[], wantsJson: boolean, deps: RuntimeApi : `${p.role}: ${p.model ?? "(no pin)"} -> ${p.proposedModel}${p.proposedEffort ? ` (${p.proposedEffort})` : ""} [${p.tier}/${p.effortIntent}] ${p.rationale ?? ""}`), apply ? `Applied ${applied.length} of ${proposals.length} roles.${skipped.length > 0 ? ` Skipped ${skipped.length} already set: ${skipped.join(", ")}.` : ""}` - : "Nothing was written; rerun with --apply to write every proposal that differs from the role's current pin.", + : `Nothing was written; --apply accepts up to ${ROLE_HISTORY_LIMIT} changed roles in one batch. Use 'ocx agent roles set' for individual changes.`, ...omoJsoncNotWritten.map(entry => `${entry.role}: omo.jsonc not written (${entry.status}); the role TOML was updated.`), + ...(batch ? [`Recorded history batch: ${batch}. Restore uses the retained changes in the dashboard's Change history.`] : []), + ...(failure ? [ + `Confirmed writes: ${applied.map(p => p.role).join(", ") || "none"}.`, + `Write not confirmed for ${failure.unconfirmedRole}; it may have been saved. Not attempted: ${failure.unattemptedRoles.join(", ") || "none"}.`, + "Earlier writes remain applied. Inspect 'ocx agent roles --json' before retrying.", + ] : []), ]); + if (failure) throw failure.cause; } async function roles(argv: string[], deps: RuntimeApiDeps): Promise { diff --git a/structure/cli-management.md b/structure/cli-management.md index 79f43c0710b..27f2c65acee 100644 --- a/structure/cli-management.md +++ b/structure/cli-management.md @@ -107,6 +107,8 @@ live and latest durable rows are captured under the same mutation lock as deleti `src/cli/agent-settings.ts` writes strict full memory/compaction override blocks via their canonical schemas. Empty memory blocks and null clears restore existing/default routing rather than disabling pipelines. `src/cli/system-settings-parity.ts` projects new settings observations; ultraFastTier is read back because its mutation receipt omits it. `src/cli/agent-runtime-settings.ts` keeps supported injection/guidance and sidecar field ownership, including web reasoning persistence despite reply omission. The actual window spellings and sidecar limits come from their owners, not similar flag names. +`src/cli/agent.ts` preflights `roles suggest --apply` against `ROLE_HISTORY_LIMIT` in `src/codex/role-model-history.ts`, counting only proposed changes that differ from the current pin. More than 50 changes refuse with exit 5 before any role PUT; accepted operations reuse the server-issued batch so recorded changes fit the existing whole-batch Restore listing. An interrupted apply prints confirmed writes, the unconfirmed role, never-attempted roles and the known batch id before preserving the original error exit. A lost response can follow a persisted write, so the CLI never retries or rolls back automatically. History availability, retention and server-side restore validation remain authoritative. `tests/cli/cli-role-suggestion-history.test.ts` exercises the real handlers and files at the 50/51 boundary, complete restoration, partial refusal and lost acknowledgements. + `src/cli/settings-result.ts` handles the settings pending boolean and shared native apply report without private detail; it never treats that flag as a full catalog disposition. Missing confirmation stays unverified/nonzero, while the accepted state remains visible. Existing no-new-option settings output remains compatible. Shared static usage text retains authored line breaks while each displayed line is terminal-safe. `src/cli/v2-input.ts` parses target/output/acknowledgment before effects and protects literal hint text after the terminator. `src/cli/v2-runtime.ts` has no parent-local-writer import and validates the management state/advisory/catalog contract. `src/cli/v2-local-output.ts` reads actual local state and validates unknown injected sync results before projection. Local mode/keep and changed feature toggles retain their sync call even without a discovered port; other verbs do not gain new synchronization. Live failure never invokes local writers, and partial native state never becomes an implicit rollback claim. diff --git a/tests/cli/cli-headless-parity.test.ts b/tests/cli/cli-headless-parity.test.ts index c304144e182..a7c6a5faa6a 100644 --- a/tests/cli/cli-headless-parity.test.ts +++ b/tests/cli/cli-headless-parity.test.ts @@ -1078,7 +1078,7 @@ describe("headless GUI parity CLI", () => { { role: "vague", model: null, status: "unsized", reason: "no JSON" }, ], }; - const runtime = fakeRuntime(req => req.method === "POST" ? proposals : { ok: true }); + const runtime = fakeRuntime(req => req.method === "POST" ? proposals : { ok: true, batch: "suggest-batch" }); const logSpy = spyOn(console, "log").mockImplementation(() => {}); let output = ""; try { @@ -1094,7 +1094,7 @@ describe("headless GUI parity CLI", () => { { path: "/api/codex-agent-roles/auto-assign", method: "POST", body: { model: "a/sizer" } }, { path: "/api/codex-agent-roles/auto-assign", method: "POST", body: {} }, { path: "/api/codex-agent-roles/explorer", method: "PUT", body: { model: "a/small", effort: "low" } }, - { path: "/api/codex-agent-roles/worker", method: "PUT", body: { model: "a/mid" } }, + { path: "/api/codex-agent-roles/worker", method: "PUT", body: { model: "a/mid", batch: "suggest-batch" } }, ]); expect(output).not.toMatch(/[\x07\x1b]/); expect(output).toContain("Use \\x1b]52;c;cG9pc29uZWQ=\\x07 carefully."); diff --git a/tests/cli/cli-role-suggestion-history.test.ts b/tests/cli/cli-role-suggestion-history.test.ts new file mode 100644 index 00000000000..10e76e8153a --- /dev/null +++ b/tests/cli/cli-role-suggestion-history.test.ts @@ -0,0 +1,256 @@ +import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, unlinkSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { handleAgentCommand } from "../../src/cli/agent"; +import type { RuntimeApiDeps } from "../../src/cli/runtime-api"; +import type { RoleModelChange } from "../../src/codex/role-model-history"; +import { handleManagementAPI } from "../../src/server/management-api"; +import type { OcxConfig } from "../../src/types"; +import { ManagementRequest } from "../helpers/management-auth"; +import { STORE_BUDGET_MS } from "../helpers/test-budget"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; + +const saved = { CODEX_HOME: process.env.CODEX_HOME, OPENCODEX_HOME: process.env.OPENCODEX_HOME, + HOME: process.env.HOME, USERPROFILE: process.env.USERPROFILE }; +const config = { port: 10100, providers: {}, defaultProvider: "openai" } as OcxConfig; +let root: string; +let codexHome: string; +let mirrorPath: string; +let historyPath: string; + +interface Proposal { + role: string; + model: string | null; + effort: string | null; + status: "proposed" | "unsized" | "unassigned"; + proposedModel: string | null; + proposedEffort: string | null; +} + +beforeEach(() => { + root = realpathSync.native(mkdtempSync(join(tmpdir(), "ocx-cli-role-history-"))); + codexHome = join(root, "codex"); + const userHome = join(root, "home"); + process.env.CODEX_HOME = codexHome; + process.env.OPENCODEX_HOME = join(root, "ocx"); + process.env.HOME = process.env.USERPROFILE = userHome; + mkdirSync(join(codexHome, "agents"), { recursive: true }); + mkdirSync(join(userHome, ".omo"), { recursive: true }); + const plugin = join(codexHome, "plugins", "cache", "sisyphuslabs", "omo", "5.1.1"); + mkdirSync(plugin, { recursive: true }); + writeFileSync(join(plugin, "lazycodex-install.json"), "{}"); + writeFileSync(join(codexHome, "config.toml"), '[plugins."omo@sisyphuslabs"]\nenabled = true\n'); + mirrorPath = join(userHome, ".omo", "omo.jsonc"); + historyPath = join(root, "ocx", "integrations", "codex-agent-roles.jsonl"); +}); + +afterEach(() => { + for (const key of Object.keys(saved) as Array) { + if (saved[key] === undefined) delete process.env[key]; + else process.env[key] = saved[key]; + } + removeTreeWithRetry(root); +}); + +const rolePath = (role: string) => join(codexHome, "agents", `${role}.toml`); +const roleText = (role: string) => readFileSync(rolePath(role), "utf8"); +const original = '# Keep the role instructions.\nmodel = "a/before"\nmodel_reasoning_effort = "high"\n'; + +function seed(count: number): Proposal[] { + const proposals = Array.from({ length: count }, (_, i): Proposal => ({ + role: `role-${i}`, model: "a/before", effort: "high", status: "proposed", + proposedModel: "a/after", proposedEffort: "low", + })); + for (const proposal of proposals) writeFileSync(rolePath(proposal.role), original); + writeFileSync(mirrorPath, JSON.stringify({ "[codex]": { agents: Object.fromEntries( + proposals.map(p => [p.role, { model: "a/before", reasoning: "high" }]), + ) } })); + return proposals; +} + +async function management(path: string, init?: RequestInit): Promise { + const request = new ManagementRequest(`http://127.0.0.1:10100${path}`, init); + const response = await handleManagementAPI(request, new URL(request.url), config); + if (!response) throw new Error(`fixture route not handled: ${path}`); + return response; +} + +async function listedHistory(): Promise { + const response = await management("/api/codex-agent-roles"); + expect(response.status).toBe(200); + return (await response.json() as { history: RoleModelChange[] }).history; +} + +interface WriteCall { role: string; body: Record; status?: number; result?: Record } +function runtime(proposals: Proposal[], hooks: { before?: (role: string) => void; after?: (role: string) => void } = {}) { + const writes: WriteCall[] = []; + const deps: RuntimeApiDeps = { + baseUrl: "http://127.0.0.1:10100", + fetchImpl: (async (input, init) => { + const path = new URL(String(input)).pathname; + if (path === "/api/codex-agent-roles/auto-assign") return Response.json({ sizingModel: "fixture", proposals }); + const role = decodeURIComponent(path.split("/").at(-1)!); + const call: WriteCall = { role, body: JSON.parse(String(init?.body)) }; + writes.push(call); + hooks.before?.(role); + const response = await management(path, init); + call.status = response.status; + call.result = await response.clone().json() as Record; + hooks.after?.(role); + return response; + }) as typeof fetch, + }; + return { deps, writes }; +} + +async function invoke(deps: RuntimeApiDeps, json = true, apply = true) { + const out = spyOn(console, "log").mockImplementation(() => {}); + const err = spyOn(console, "error").mockImplementation(() => {}); + try { + const exit = await handleAgentCommand(["roles", "suggest", ...(apply ? ["--apply"] : []), ...(json ? ["--json"] : [])], deps); + const stdout = out.mock.calls.flat().join("\n"); + return { exit, stdout, stderr: err.mock.calls.flat().join("\n"), receipt: json && stdout ? JSON.parse(stdout) : undefined }; + } finally { + out.mockRestore(); + err.mockRestore(); + } +} + +async function restoreBatch(changes: RoleModelChange[]): Promise { + let batch: string | undefined; + for (const change of changes) { + const response = await management(`/api/codex-agent-roles/${encodeURIComponent(change.role)}`, { + method: "PUT", + body: JSON.stringify({ model: change.before.model ?? undefined, effort: change.before.effort, + restores: change.batch, ...(batch ? { batch } : {}) }), + }); + expect(response.status).toBe(200); + const written = await response.json() as { batch: string }; + if (batch) expect(written.batch).toBe(batch); + batch = written.batch; + } + return batch!; +} + +describe("CLI role suggestions preserve bounded whole-batch history", () => { + test("50 real writes share one listed batch and every role restores after the source leaves the listing", async () => { + const proposals = seed(50); + const mirrorBefore = JSON.parse(readFileSync(mirrorPath, "utf8")); + const transport = runtime(proposals); + const { exit, receipt } = await invoke(transport.deps); + expect(exit).toBe(0); + expect(receipt.applied).toHaveLength(50); + expect(receipt.applied[0]).toEqual({ role: "role-0", model: "a/after", effort: "low" }); + expect(transport.writes).toHaveLength(50); + expect(transport.writes[0]!.body).not.toHaveProperty("batch"); + expect(new Set(transport.writes.slice(1).map(w => w.body.batch))).toEqual(new Set([receipt.batch])); + const changes = await listedHistory(); + expect(changes).toHaveLength(50); + expect(new Set(changes.map(c => c.batch))).toEqual(new Set([receipt.batch])); + const stored = readFileSync(historyPath, "utf8").trim().split("\n").map(line => JSON.parse(line)); + expect(stored.map(row => row.n)).toEqual(Array.from({ length: 50 }, (_, i) => i + 1)); + expect(proposals.every(p => roleText(p.role).includes('model = "a/after"'))).toBe(true); + const restored = await restoreBatch(changes); + expect(proposals.map(p => roleText(p.role))).toEqual(Array(50).fill(original)); + expect(JSON.parse(readFileSync(mirrorPath, "utf8"))).toEqual(mirrorBefore); + const latest = await listedHistory(); + expect(latest).toHaveLength(50); + expect(new Set(latest.map(c => c.batch))).toEqual(new Set([restored])); + }, { timeout: STORE_BUDGET_MS }); + + test.each([false, true])("51 changed roles are refused before any PUT or file change (json=%s)", async json => { + const proposals = seed(51); + const mirrorBefore = readFileSync(mirrorPath, "utf8"); + const transport = runtime(proposals); + const result = await invoke(transport.deps, json); + expect(result.exit).toBe(5); + expect(transport.writes).toEqual([]); + expect(proposals.map(p => roleText(p.role))).toEqual(Array(51).fill(original)); + expect(readFileSync(mirrorPath, "utf8")).toBe(mirrorBefore); + expect(existsSync(historyPath)).toBe(false); + expect(result.stderr).toContain("51 changed proposals exceed the 50-role batch limit. No role files were written."); + if (json) { + expect(result.receipt).toMatchObject({ applied: [], complete: false, + refusal: { code: "too_many_role_changes", limit: 50, count: 51 }, unattemptedRoles: proposals.map(p => p.role) }); + } else expect(result.stdout).toContain("Preview without --apply"); + }); + + test("already-set, unsized and unassigned roles do not consume the 50-change budget", async () => { + const proposals = seed(53); + Object.assign(proposals[50]!, { proposedModel: "a/before", proposedEffort: "high" }); + Object.assign(proposals[51]!, { status: "unsized", proposedModel: null, proposedEffort: null }); + Object.assign(proposals[52]!, { status: "unassigned", proposedModel: null, proposedEffort: null }); + const transport = runtime(proposals); + const { exit, receipt } = await invoke(transport.deps); + expect(exit).toBe(0); + expect(receipt.skipped).toEqual(["role-50"]); + expect(receipt.applied).toHaveLength(50); + expect(transport.writes).toHaveLength(50); + expect(proposals.slice(50).map(p => roleText(p.role))).toEqual(Array(3).fill(original)); + expect(await listedHistory()).toHaveLength(50); + }, { timeout: STORE_BUDGET_MS }); + + test("preview remains available above the apply limit", async () => { + const proposals = seed(51); + const transport = runtime(proposals); + const { exit, receipt } = await invoke(transport.deps, true, false); + expect(exit).toBe(0); + expect(receipt).toEqual({ sizingModel: "fixture", proposals }); + const preview = await invoke(transport.deps, false, false); + expect(preview.exit).toBe(0); + expect(preview.stdout).toContain("--apply accepts up to 50 changed roles in one batch"); + expect(transport.writes).toEqual([]); + expect(existsSync(historyPath)).toBe(false); + }); + + test.each([false, true])("a real role refusal reports and preserves the restorable prefix (json=%s)", async json => { + const proposals = seed(3); + writeFileSync(mirrorPath, '{\n // keep this comment\n "[codex]": {"agents": {}}\n}\n'); + const transport = runtime(proposals, { before: role => { if (role === "role-1") unlinkSync(rolePath(role)); } }); + const result = await invoke(transport.deps, json); + expect(result.exit).toBe(4); + expect(transport.writes.map(w => w.status)).toEqual([200, 404]); + expect(result.stderr).toBe(`Error: ${transport.writes[1]!.result!.error}`); + expect(roleText("role-0")).toContain('model = "a/after"'); + expect(roleText("role-2")).toBe(original); + const changes = await listedHistory(); + expect(changes).toHaveLength(1); + if (json) { + expect(result.receipt).toMatchObject({ complete: false, batch: changes[0]!.batch, + applied: [{ role: "role-0", model: "a/after", effort: "low" }], unconfirmedRole: "role-1", + unattemptedRoles: ["role-2"], omoJsoncNotWritten: [{ role: "role-0", status: "skipped_comments" }] }); + } else { + expect(result.stdout).toContain("Confirmed writes: role-0."); + expect(result.stdout).toContain("Write not confirmed for role-1; it may have been saved. Not attempted: role-2."); + expect(result.stdout).toContain(`Recorded history batch: ${changes[0]!.batch}`); + expect(result.stdout).toContain("role-0: omo.jsonc not written (skipped_comments)"); + } + await restoreBatch(changes); + expect(roleText("role-0")).toBe(original); + }); + + test.each([0, 1])("a response lost after real write %s is unconfirmed, never retried or rolled back", async lostIndex => { + const proposals = seed(3); + const lostRole = `role-${lostIndex}`; + const transport = runtime(proposals, { after: role => { if (role === lostRole) throw new TypeError("fixture response lost"); } }); + const { exit, receipt, stderr } = await invoke(transport.deps); + expect(exit).toBe(1); + expect(stderr).toBe("Error: Management API is unreachable: fixture response lost"); + expect(transport.writes).toHaveLength(lostIndex + 1); + expect(receipt.applied.map((p: { role: string }) => p.role)).toEqual(proposals.slice(0, lostIndex).map(p => p.role)); + expect(receipt.unconfirmedRole).toBe(lostRole); + expect(receipt.unattemptedRoles).toEqual(proposals.slice(lostIndex + 1).map(p => p.role)); + expect(receipt.complete).toBe(false); + expect(roleText(lostRole)).toContain('model = "a/after"'); + expect(proposals.slice(lostIndex + 1).map(p => roleText(p.role))).toEqual(Array(2 - lostIndex).fill(original)); + const changes = await listedHistory(); + expect(changes).toHaveLength(lostIndex + 1); + expect(new Set(changes.map(c => c.batch)).size).toBe(1); + if (lostIndex === 0) expect(receipt).not.toHaveProperty("batch"); + else expect(receipt.batch).toBe(changes[0]!.batch); + // Reading the real history recovers even the unacknowledged write's server-issued batch. + await restoreBatch(changes); + expect(proposals.map(p => roleText(p.role))).toEqual(Array(3).fill(original)); + }); +}); diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 7a4919ad18e..80d15abbd48 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -523,7 +523,7 @@ "cli-dispatch.test.ts": "cli", "cli-client-start-fence.test.ts": "cli", "cli-ensure-client-state.test.ts": "cli", "cli-client-start-owner-e2e.test.ts": "cli", "cli-dto-fidelity.test.ts": "cli", "cli-export-command.test.ts": "cli", - "cli-head.test.ts": "cli", "cli-headless-parity.test.ts": "cli", + "cli-head.test.ts": "cli", "cli-headless-parity.test.ts": "cli", "cli-role-suggestion-history.test.ts": "cli", "cli-help.test.ts": "cli", "cli-help-paths.test.ts": "cli", "cli-help-navigation.test.ts": "cli", "cli-help-recovery.test.ts": "cli", "cli-integer-options.test.ts": "cli", "cli-init-port.test.ts": "cli", From 2f7f1d113ec061e5e08e1a46228240a0ac3064e1 Mon Sep 17 00:00:00 2001 From: Epinephrine Date: Sat, 31 Oct 2026 13:18:26 +0000 Subject: [PATCH 08/13] fix(responses): strip rejected reasoning toggle from native passthrough (#8025) * fix(responses): strip rejected reasoning toggle * docs(responses): qualify native reasoning cleanup --- src/server/responses/core-normalize.ts | 8 +++++++- structure/transports/responses.md | 1 + .../reasoning-effort-summary-default.test.ts | 20 +++++++++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/server/responses/core-normalize.ts b/src/server/responses/core-normalize.ts index 880f3a4d099..744ed28bdec 100644 --- a/src/server/responses/core-normalize.ts +++ b/src/server/responses/core-normalize.ts @@ -346,11 +346,17 @@ export async function applyFinalRouteRequestNormalization(args: { // Native passthrough keeps its snapshot-owned effort contract, including saved exclusions. if (!shouldApplyNativeEffortClamp(route.providerName, route.provider, finalSelectedModelId)) { const previousEffort = parsed.options.reasoning; + const previousThinkingEnabled = parsed.options.thinkingEnabled; normalizeThinkingOptions(route.provider, route.modelId, parsed.options); + const raw = parsed._rawBody as { reasoning?: unknown } | undefined; if (previousEffort !== undefined && parsed.options.reasoning === undefined) { - const raw = parsed._rawBody as { reasoning?: unknown } | undefined; if (raw) raw.reasoning = stripEmptyLadderEffort(raw.reasoning, []); } + if (previousThinkingEnabled !== undefined && parsed.options.thinkingEnabled === undefined + && raw?.reasoning && typeof raw.reasoning === "object" && !Array.isArray(raw.reasoning)) { + delete (raw.reasoning as { enabled?: boolean }).enabled; + if (Object.keys(raw.reasoning).length === 0) delete raw.reasoning; + } } const { applyEffortCap, effortCapAppliesTo, supportedLadderFor } = await import("../effort-policy"); const surface = collabSurface(parsed); diff --git a/structure/transports/responses.md b/structure/transports/responses.md index d67380d7ec7..9b05c5b4097 100644 --- a/structure/transports/responses.md +++ b/structure/transports/responses.md @@ -17,6 +17,7 @@ provider, lets the selected adapter speak the upstream protocol, then bridges ad Responses-compatible streaming output. Every Responses serializer projects unreferenced hosted-search history when the final body declares no hosted search tool; with search declared, the canonical Codex ordinary serializer retains structural hosted history by default, and only selected-provider `projectResponsesLiteHostedSearchHistory === true` plus exact canonical `/responses` and explicit Lite intent enables model-independent projection under the [hosted-search history boundary](../providers-and-adapters.md#hosted-search-continuation-binding), shared by HTTP/SSE and WS. The routed provider name follows every adapter build, including retries and continuations; only `github-copilot` injects a configured `modelContextTiers` value as upstream `contextTier`, while unconfigured passthrough retains the caller field. For an opted-in key-auth provider, a hosted-search continuation stays bound to the API-key selection that served the first leg; the contract is the [hosted-search continuation binding](../providers-and-adapters.md#hosted-search-continuation-binding). The native passthrough undeclared-tool guard fails closed on the Responses inbound wire in SSE and buffered JSON before continuation state is stored. Chat and Anthropic inbounds defer to the caller's own registry except an explicit empty catalog; the [declared-tool contract](responses-wire-shapes.md#declared-tool-membership-by-inbound-wire) also covers managed native, translated, directly encoded, and final media output while preserving internal hosted operations. GitHub Copilot Auto admission rechecks the negotiated provider/model against the key scope captured at request admission (a key edited or removed mid-request cannot widen it) before route mutation or dispatch, on initial and renewal negotiation; denial returns the standard `model_not_allowed_for_key` 403 naming the caller's inbound selector without disclosing the destination. The web-search and image sidecars answer a denial during their credential rotation with that 403 instead of the original 429, and a combo child's denial keeps the same envelope. Local model-scope refusals are non-replayable. A denial during Copilot dispatch renewal propagates as an admission exception to the request boundary, never as a provider fetch response; account recovery and combo/policy fallback cannot continue after that denial. A committed continuation, including OAuth account rotation after a 429, emits the same refusal type in-stream without dispatching another inference; ordinary negotiation failures still preserve the original provider refusal. The `openai-responses` adapter preserves caller `User-Agent` unless provider headers own it, in either auth mode. +Final route capability normalization removes rejected reasoning effort and toggle fields from the native passthrough body when those fields were parsed into options, before custom gateway-object serialization. `forwardClientHeaders` additionally opts into `originator`, `x-client-request-id`, `x-codex-app-version`, or `user-agent`. Provider headers win case-insensitively; arbitrary names are rejected on load/write and ignored at runtime. Canonical forward auth retains its separate fixed credential/metadata allowlist; absent caller identity is not invented. Upstream WebSocket reasoning-inclusion headers follow the [native prelude and connection-signal contract](responses-wire-shapes.md#passthrough-sse-stream-shapes-314). diff --git a/tests/responses/reasoning-effort-summary-default.test.ts b/tests/responses/reasoning-effort-summary-default.test.ts index 564af8427ef..155e41e5d85 100644 --- a/tests/responses/reasoning-effort-summary-default.test.ts +++ b/tests/responses/reasoning-effort-summary-default.test.ts @@ -111,6 +111,26 @@ describe("reasoning effort preserves visible thinking when summary is omitted", expect(child.reasoning).toEqual({ effort: "high", summary: "auto" }); }); + test("final route removes a rejected thinking toggle from native passthrough", async () => { + const config: OcxConfig = { + port: 0, + defaultProvider: "native", + providers: { native: { + adapter: "openai-responses", baseUrl: "https://example.test/v1", reasoningWireFormat: "gateway-object", + models: ["test"], + modelCapabilities: { test: { supportsReasoning: false, supportsThinkingToggle: false, supportsReasoningEffort: false } }, + } }, + }; + const parsed = parseRequest({ model: "native/test", input: [], reasoning: { enabled: true, effort: "high", summary: "auto" } }); + const route = routeModel(config, parsed.modelId); + await applyFinalRouteRequestNormalization({ + parsed, route, config, req: new Request("http://localhost/v1/responses"), + logCtx: { model: parsed.modelId, provider: route.providerName }, inboundWire: "responses", + }); + expect(parsed.options.thinkingEnabled).toBeUndefined(); + expect((parsed._rawBody as { reasoning?: unknown }).reasoning).toEqual({ summary: "auto" }); + }); + test("Chat final-route normalization strips both shapes without losing summary or later-route effort", async () => { const config: OcxConfig = { port: 0, From 67bdeb03a3a3e6d3b59435527d4bf5b6a232b731 Mon Sep 17 00:00:00 2001 From: Harsha Vardhan <80092815+harshav167@users.noreply.github.com> Date: Sun, 1 Nov 2026 00:18:40 +1100 Subject: [PATCH 09/13] fix(adapters): strip safety_identifier at the canonical ChatGPT forward destination (#8030) * fix(adapters): strip safety_identifier at the canonical ChatGPT forward destination droid's BYOK path attaches safety_identifier to any custom model whose name fuzzy-matches its built-in OpenAI registry (gpt-6.1-sol etc.), with no isCustomModel guard, unlike prompt_cache_retention/prompt_cache_key. The canonical ChatGPT Codex backend runs a strict parameter allowlist and rejects it with {"detail":"Unsupported parameter: safety_identifier"}, surfacing as a 400 BYOK error through every registry-matched model pointed at ocx. Add it to the canonical-only drop list in stripUnsupportedForwardParams, beside metadata and max_output_tokens. The official OpenAI API-key surface and noncanonical gateways keep the field (it is a real platform parameter). Reproduced live: POST /v1/responses with safety_identifier -> 400 before the fix; regression tests pin strip-on-canonical and keep-on-everywhere-else. * docs(runtime): the canonical safety_identifier strip is source-agnostic CodeRabbit finding on b0e571964: the doc claimed the parameter reaches the canonical surface only from third-party clients (droid's registry-matched BYOK). Any Responses client may send the public OpenAI-platform parameter explicitly; the strip removes it regardless of which client supplied it. The other-destination retention sentences are unchanged. --- .../openai-responses/canonical-forward.ts | 63 +++++++++++--- structure/runtime.md | 2 +- .../responses-forward-prompt-envelope.test.ts | 86 ++++++++++++------- 3 files changed, 107 insertions(+), 44 deletions(-) diff --git a/src/adapters/openai-responses/canonical-forward.ts b/src/adapters/openai-responses/canonical-forward.ts index 2935eb434ff..5d1fc392f77 100644 --- a/src/adapters/openai-responses/canonical-forward.ts +++ b/src/adapters/openai-responses/canonical-forward.ts @@ -1,4 +1,11 @@ -import { namespacedToolName, type AdapterEvent, type OcxParsedRequest, type OcxProviderConfig, type OcxUsage, type TierDecision } from "../../types"; +import { + namespacedToolName, + type AdapterEvent, + type OcxParsedRequest, + type OcxProviderConfig, + type OcxUsage, + type TierDecision, +} from "../../types"; import { stripItemIdsWhenUnstored } from "./request-strips"; import { isPlainObject } from "./internal"; import { stripPromptCacheBreakpoints } from "./prompt-cache"; @@ -15,13 +22,21 @@ import { stripPromptCacheBreakpoints } from "./prompt-cache"; * unexpanded requests: the platform `/v1/responses` supports real server-side storage. */ export function stripPreviousResponseId(body: unknown, strip: boolean): unknown { - if (!strip || !isPlainObject(body) || !Object.prototype.hasOwnProperty.call(body, "previous_response_id")) return body; + if ( + !strip || + !isPlainObject(body) || + !Object.prototype.hasOwnProperty.call(body, "previous_response_id") + ) + return body; const { previous_response_id: _previousResponseId, ...rest } = body; return rest; } /** Apply the settled tier only to a fresh outbound object; `_rawBody` remains caller-owned. */ -export function applyTierDecisionToResponsesBody(body: unknown, decision: TierDecision | undefined): unknown { +export function applyTierDecisionToResponsesBody( + body: unknown, + decision: TierDecision | undefined, +): unknown { if (!decision || decision.kind === "forward-caller" || !isPlainObject(body)) return body; const next: Record = { ...body }; if (decision.kind === "set") next.service_tier = decision.value; @@ -54,8 +69,14 @@ export function applyTierDecisionToResponsesBody(body: unknown, decision: TierDe */ export function stripStatefulResponsesParams(body: unknown): unknown { if (!isPlainObject(body)) return body; - const drop = ["previous_response_id", "conversation", "background", "metadata", "prompt"] as const; - const present = drop.some(key => Object.prototype.hasOwnProperty.call(body, key)); + const drop = [ + "previous_response_id", + "conversation", + "background", + "metadata", + "prompt", + ] as const; + const present = drop.some((key) => Object.prototype.hasOwnProperty.call(body, key)); if (!present && body.store === false) return body; const next: Record = { ...body }; for (const key of drop) delete next[key]; @@ -77,11 +98,19 @@ export function stripStatefulResponsesParams(body: unknown): unknown { * self-hosted or third-party gateway may honour it — dropping it there silently * removes the cap. Only the canonical ChatGPT backend rejects it outright, so only * `canonical` callers strip it. + * + * `safety_identifier` is the third canonical-only drop: a public OpenAI-platform + * parameter some BYOK clients (droid's registry-matched models) attach from the + * model name alone. The official API-key surface accepts it, but the canonical + * ChatGPT backend rejects it with the same strict-allowlist envelope, so only + * `canonical` callers strip it. */ export function stripUnsupportedForwardParams(body: unknown, canonical: boolean): unknown { if (!isPlainObject(body)) return body; - const drop = canonical ? (["metadata", "max_output_tokens"] as const) : (["metadata"] as const); - if (!drop.some(key => Object.hasOwn(body, key))) return body; + const drop = canonical + ? (["metadata", "max_output_tokens", "safety_identifier"] as const) + : (["metadata"] as const); + if (!drop.some((key) => Object.hasOwn(body, key))) return body; const next: Record = { ...body }; for (const key of drop) delete next[key]; return next; @@ -104,7 +133,11 @@ const CANONICAL_FORWARD_UNSUPPORTED_SAMPLING = ["temperature", "top_p", "stop", */ export function stripCanonicalForwardSamplingParams(body: unknown): unknown { if (!isPlainObject(body)) return body; - if (!CANONICAL_FORWARD_UNSUPPORTED_SAMPLING.some(key => Object.prototype.hasOwnProperty.call(body, key))) { + if ( + !CANONICAL_FORWARD_UNSUPPORTED_SAMPLING.some((key) => + Object.prototype.hasOwnProperty.call(body, key), + ) + ) { return body; } const next: Record = { ...body }; @@ -130,9 +163,11 @@ function canonicalForwardSystemText(item: Record): string | nul /** Only message items may carry privileged system instructions. */ function isCanonicalForwardSystemMessage(item: unknown): item is Record { - return isPlainObject(item) - && (item.type === undefined || item.type === "message") - && item.role === "system"; + return ( + isPlainObject(item) && + (item.type === undefined || item.type === "message") && + item.role === "system" + ); } /** @@ -172,7 +207,7 @@ export function normalizeCanonicalForwardPromptEnvelope(body: unknown): unknown const next: Record = { ...body }; if (stripTruncation) delete next.truncation; if (sawSystemMessage && canFoldAllSystemMessages) { - next.input = input.filter(item => !isCanonicalForwardSystemMessage(item)); + next.input = input.filter((item) => !isCanonicalForwardSystemMessage(item)); const folded = foldedText.join("\n\n"); if (folded !== "") { const existing = typeof body.instructions === "string" ? body.instructions : ""; @@ -193,7 +228,9 @@ export function normalizeCanonicalForwardContinuationEnvelope(body: unknown): un let input: unknown[] = body.input; let changed = false; if (body.store === false) { - const withoutReferences = input.filter(item => !isPlainObject(item) || item.type !== "item_reference"); + const withoutReferences = input.filter( + (item) => !isPlainObject(item) || item.type !== "item_reference", + ); if (withoutReferences.length !== input.length) { input = withoutReferences; changed = true; diff --git a/structure/runtime.md b/structure/runtime.md index 52a3a83c061..b61361243b3 100644 --- a/structure/runtime.md +++ b/structure/runtime.md @@ -490,7 +490,7 @@ Translated audio/file admission follows the [final-adapter input contract](adapt `src/server/admission-model-scope.ts` grants metered combo eligibility to a matching configured key with literal `allowMeteredComboTargets: true`, or to local/missing admission when the selected combo has literal `allowMeteredTargets: true` and the full business config satisfies `!isApiAuthRequired(config)`. Configured and environment admissions never inherit combo consent; linked keys retain their key consent requirement even on a loopback Home. The full hostname also prevents secondary loopback listeners on public proxies from granting operator consent. `src/server/responses/core-combo.ts` filters marked targets on initial, failure and recovery selection for every strategy. A route the router collapsed to one combo pick outside that loop (a subagent fallback naming a combo alias) is re-picked among usable unmetered targets in `captureInboundRoutePolicy` of `src/server/responses/request-prepare.ts` when its pick is metered without consent, or refused if none remain; the replacement retains combo metadata and passes destination policy/scope checks. Request headers never grant consent; absent/false consent denies marked targets; provider/model scopes remain independent. `src/cli/combo-input.ts` accepts and validates the optional target boolean. Unmarked targets retain existing behavior; metering is operator metadata, not automatic billing detection. Coverage: `tests/server/combo-metered-consent.test.ts`, `tests/server/combo-loopback-metered-consent.test.ts` and `tests/adapters/anthropic/anthropic-subscription-chain.test.ts`. The exact Anthropic credit-balance refusal on HTTP 400/402 is a provider-state failure under the [Anthropic API-key billing contract](providers/anthropic-key-billing.md). `src/combos/failover.ts` recognizes complete bounded JSON or exact formatted prose with an accepted structured code before the generic invalid-request stop, granting a provider-scoped hop; malformed, truncated and unrelated 400s keep their existing decision. Google's final adapter compiler may emit an opt-in, content-free [tool-schema loss diagnostic](providers/google.md#google-tool-schema-loss-reporting). It observes adapter-local narrowing only and changes neither provider routing nor the serialized request body. -`src/adapters/openai-responses.ts` omits only top-level `user` at the canonical ChatGPT Codex forward destination. Claude translation retains its original identity and prompt-cache key; public API and noncanonical gateways retain their `user` field. Input roles, tool-schema properties, safety identifiers and original replay bodies are not changed. +`src/adapters/openai-responses.ts` omits top-level `user` and `safety_identifier` at the canonical ChatGPT Codex forward destination. The backend rejects both with its strict-allowlist `{"detail":"Unsupported parameter: …"}` envelope, and canonical forwarding removes `safety_identifier` regardless of which client supplied it — third-party clients (droid's registry-matched BYOK) attach it from the model name alone, but any Responses client may send the public OpenAI-platform parameter explicitly. Claude translation retains its original identity and prompt-cache key; public API and noncanonical gateways retain their `user` and `safety_identifier` fields. Input roles, tool-schema properties and original replay bodies are not changed. `src/combos/failover.ts` treats four intact HTTP 400 invalid-request envelopes as request-local incompatibilities: exactly `Unsupported parameter: user`; `unsupported_value` naming `reasoning.effort` or `reasoning_effort` with an explicit unsupported-value message; `param: input` with a bounded model-scoped `does not support image inputs` message; and the exact null-code `gpt-6-astra` function-tool routing mismatch that tells an existing Responses request to use `/v1/responses`, allowing only the bare model name or its strict `YYYY-MM-DD` deployment suffix. A null provider code is accepted only for the two observed envelopes that require it. Only the exact proxy wrapper is unwrapped, within three envelopes and 16,384 characters; conflicting codes, malformed/truncated envelopes and reflected JSON do not gain hop permission. diff --git a/tests/responses/responses-forward-prompt-envelope.test.ts b/tests/responses/responses-forward-prompt-envelope.test.ts index 9ca3578971e..c612d8154a2 100644 --- a/tests/responses/responses-forward-prompt-envelope.test.ts +++ b/tests/responses/responses-forward-prompt-envelope.test.ts @@ -13,15 +13,21 @@ const canonicalForward: OcxProviderConfig = { authMode: "forward", }; -function outboundBody(provider: OcxProviderConfig, rawBody: Record): Record { +function outboundBody( + provider: OcxProviderConfig, + rawBody: Record, +): Record { const adapter = createAdapter(provider); - const request = adapter.buildRequest({ - modelId: String(rawBody.model ?? "gpt-5.6-luna"), - context: { messages: [] }, - stream: rawBody.stream === true, - options: {}, - _rawBody: rawBody, - }, { headers: new Headers({ authorization: "Bearer test-token" }) }); + const request = adapter.buildRequest( + { + modelId: String(rawBody.model ?? "gpt-5.6-luna"), + context: { messages: [] }, + stream: rawBody.stream === true, + options: {}, + _rawBody: rawBody, + }, + { headers: new Headers({ authorization: "Bearer test-token" }) }, + ); try { return JSON.parse(request.body) as Record; } finally { @@ -142,29 +148,43 @@ describe("canonical ChatGPT forward prompt envelope", () => { }); }); - describe("canonical forward user metadata boundary", () => { - test.each(["gpt-5.3-codex-spark", "gpt-5.6-luna"])("strips only top-level user for %s", model => { - const raw = { - model, user: "synthetic-client", prompt_cache_key: "synthetic-cache", - safety_identifier: "synthetic-safety", stream: true, - input: [{ type: "message", role: "user", content: "hello" }], - tools: [{ type: "function", name: "lookup", parameters: { - type: "object", properties: { user: { type: "string" } }, required: ["user"], - } }], - }; - const snapshot = structuredClone(raw); - const body = outboundBody(canonicalForward, raw); - expect(Object.hasOwn(body, "user")).toBe(false); - expect(body.prompt_cache_key).toBe(raw.prompt_cache_key); - expect(body.safety_identifier).toBe(raw.safety_identifier); - expect(body.input).toEqual(raw.input); - expect(body.tools).toEqual(raw.tools); - expect(raw).toEqual(snapshot); - }); + test.each(["gpt-5.3-codex-spark", "gpt-5.6-luna"])( + "strips top-level user and safety_identifier for %s", + (model) => { + const raw = { + model, + user: "synthetic-client", + prompt_cache_key: "synthetic-cache", + safety_identifier: "synthetic-safety", + stream: true, + input: [{ type: "message", role: "user", content: "hello" }], + tools: [ + { + type: "function", + name: "lookup", + parameters: { + type: "object", + properties: { user: { type: "string" } }, + required: ["user"], + }, + }, + ], + }; + const snapshot = structuredClone(raw); + const body = outboundBody(canonicalForward, raw); + expect(Object.hasOwn(body, "user")).toBe(false); + expect(body.prompt_cache_key).toBe(raw.prompt_cache_key); + expect(Object.hasOwn(body, "safety_identifier")).toBe(false); + expect(body.input).toEqual(raw.input); + expect(body.tools).toEqual(raw.tools); + expect(raw).toEqual(snapshot); + }, + ); test("Claude translation retains session metadata locally but omits user on the native wire", () => { const raw = anthropicToResponsesBody({ - model: "gpt-5.3-codex-spark", max_tokens: 32, + model: "gpt-5.3-codex-spark", + max_tokens: 32, metadata: { user_id: "synthetic-claude-session" }, messages: [{ role: "user", content: "hello" }], }); @@ -181,9 +201,15 @@ describe("canonical forward user metadata boundary", () => { { ...canonicalForward, baseUrl: "https://gateway.example/v1" }, { ...canonicalForward, baseUrl: "https://chatgpt.com.example/backend-api/codex" }, { ...canonicalForward, authMode: "key" as const }, - ])("does not strip user from a different destination: %j", provider => { - const body = outboundBody(provider, { model: "model", user: "synthetic-client", input: "hello" }); + ])("does not strip user or safety_identifier from a different destination: %j", (provider) => { + const body = outboundBody(provider, { + model: "model", + user: "synthetic-client", + safety_identifier: "synthetic-safety", + input: "hello", + }); expect(body.user).toBe("synthetic-client"); + expect(body.safety_identifier).toBe("synthetic-safety"); }); test("normalizes the canonical trailing slash without changing field-absent requests", () => { const provider = { ...canonicalForward, baseUrl: canonicalForward.baseUrl + "/" }; From e972776eced1b266692b780263828635f47b2754 Mon Sep 17 00:00:00 2001 From: JUN Date: Sun, 1 Nov 2026 00:22:10 +0900 Subject: [PATCH 10/13] fix(anthropic): support Ultra quota windows and show subscription names (#8055) * fix(anthropic): honor active subscription usage windows Prefer authoritative limits rows for shared and model-scoped quota, suppress inactive legacy duplicates, and share source selection with raw decisions. Accept Ultra and claude-prefixed profile tiers without granting entitlements. Cover null session windows, active/inactive Fable admission, pool selection, legacy quotas, unknown tiers and conservative cooldown recovery. Update the owning structure and public docs. Independent Sol security/correctness review reported no findings for the complete diff. * feat(anthropic): show subscription names beside Claude account quotas * fix(gui): keep Claude account identities out of account dialogs * fix(gui): suppress Claude emails in catalog account status --- .../src/content/docs/guides/claude-code.md | 8 + .../docs/reference/configuration/providers.md | 14 ++ .../provider-workspace/ProviderAuthPanel.tsx | 13 +- .../ProviderCurrentQuota.tsx | 1 + .../provider-workspace/ProviderOverview.tsx | 8 +- .../components/provider-workspace/types.ts | 2 + gui/src/hooks/useProviderAccountPools.ts | 16 +- gui/src/pages/providers-page-utils.ts | 7 + gui/src/pages/providers-shared.ts | 1 + gui/src/provider-workspace/auth.ts | 5 +- .../anthropic-subscription-label.test.tsx | 60 ++++++++ .../anthropic2-provider-surfaces.test.tsx | 55 ++++++- scripts/test-layout/layout-adapters.json | 1 + scripts/test-layout/layout-providers.json | 1 + src/oauth/anthropic-identity.ts | 8 +- src/oauth/index.ts | 5 + src/providers/anthropic-subscription.ts | 25 ++++ src/providers/claude-cli-usage.ts | 82 ++--------- .../quota/anthropic-decision-windows.ts | 24 +-- .../quota/anthropic-usage-windows.ts | 55 +++++++ structure/gui-and-management-api.md | 2 +- structure/providers-and-adapters.md | 4 +- structure/providers/anthropic-account-pool.md | 38 ++++- .../anthropic-active-limit-routing.test.ts | 76 ++++++++++ tests/fixtures/anthropic-usage-shapes.ts | 31 ++++ tests/fixtures/test-layout-expected.json | 2 + tests/oauth/oauth-account-plans.test.ts | 12 ++ .../provider-anthropic-usage-windows.test.ts | 139 ++++++++++++++++++ tests/providers/provider-quota.test.ts | 4 +- 29 files changed, 578 insertions(+), 121 deletions(-) create mode 100644 gui/tests/anthropic-subscription-label.test.tsx create mode 100644 src/providers/anthropic-subscription.ts create mode 100644 src/providers/quota/anthropic-usage-windows.ts create mode 100644 tests/adapters/anthropic/anthropic-active-limit-routing.test.ts create mode 100644 tests/fixtures/anthropic-usage-shapes.ts create mode 100644 tests/providers/provider-anthropic-usage-windows.test.ts diff --git a/docs-site/src/content/docs/guides/claude-code.md b/docs-site/src/content/docs/guides/claude-code.md index c0b83efa0ce..46687ed3468 100644 --- a/docs-site/src/content/docs/guides/claude-code.md +++ b/docs-site/src/content/docs/guides/claude-code.md @@ -69,6 +69,14 @@ response headers on streaming, JSON and upstream-error responses, so compatible statusLine consumers can read the provider's quota observations. Missing observations are not filled with invented values. This relay does not change account selection or retry behavior. +Usage reads support Ultra (formerly Max 20x). Pro and Max 5x retain their five-hour and weekly meters. A null five-hour limit leaves that meter +absent and still allows weekly usage to display. Current `limits` counters take precedence +over legacy duplicates; only active model-specific weekly limits affect that model family. +The dashboard shows Pro, Max 5x, or Ultra beside Claude account quotas, using subscription +metadata from the authenticated profile. Both `ultra` and `claude_max_20x` display as Ultra +(the existing `ultra` and `max_20x` tier aliases are retained). Unfamiliar validated tiers retain their original name without +failing the usage read. Claude account rows use ordinal labels and omit account IDs and emails. + You can log in multiple Claude accounts via the Providers dashboard (`ocx login anthropic` / add-account). By default every request uses the **active** account only. diff --git a/docs-site/src/content/docs/reference/configuration/providers.md b/docs-site/src/content/docs/reference/configuration/providers.md index 7ed86852041..16853705892 100644 --- a/docs-site/src/content/docs/reference/configuration/providers.md +++ b/docs-site/src/content/docs/reference/configuration/providers.md @@ -1087,6 +1087,20 @@ eligible on the same account. Passive Fable evidence ages out after thirty minut known reset and is revalidated by one serving request at a time. Usage thresholds remain soft preferences with an all-drained fallback; these controls are not hard usage or billing caps. Active usage probes preserve absent family windows unless the response authoritatively enumerates limits. +Usage reads prefer the active `limits` rows over legacy quota buckets. An inactive +session or model window does not constrain admission, and a missing/null five-hour +bucket does not create a zero-percent five-hour bar. A model-scoped weekly limit +constrains only that model family while active; the all-model weekly limit applies +to every model. Legacy Pro/Max usage buckets remain supported. + +[Ultra replaces Max 20x](https://support.claude.com/en/articles/17728952-what-is-the-ultra-plan); +upstream may still identify it as `claude_max_20x` (shown as `max_20x`). Ultra accounts +with no active five-hour limit show weekly usage only. Tier names are display +metadata, including unknown future tiers, and never imply extra quota or API credit. +Reset grants remain optional, explicit actions with their own eligibility and expiry; +a usage read never consumes one. A quota read refused with `access_denied` can also +mean the account's access token expired; a plan rename does not repair credentials. + Affinity is process-local and size-bounded. Token-refresh credential failures retain the existing reauthentication policy. Classified pre-output account-entitlement/billing 403s clear affinity and cool the account for `Retry-After`, or ten minutes by default, before trying an eligible replacement. Generic or request-level 403s remain terminal; see [Claude account recovery](/guides/claude-code/). If all eligible accounts are cooling, clients receive 429 with `Retry-After` when known, not an authentication error. diff --git a/gui/src/components/provider-workspace/ProviderAuthPanel.tsx b/gui/src/components/provider-workspace/ProviderAuthPanel.tsx index d6f6b0c833c..a76d3c60746 100644 --- a/gui/src/components/provider-workspace/ProviderAuthPanel.tsx +++ b/gui/src/components/provider-workspace/ProviderAuthPanel.tsx @@ -1,3 +1,4 @@ +import { anthropicSubscriptionLabel } from "../../../../src/providers/anthropic-subscription"; import { anthropicInstanceRowShapeMatches, isAnthropicInstanceId } from "../../../../src/providers/anthropic-instance-id"; /** * ProviderAuthPanel — OAuth accounts, API-key pool, and forward-auth @@ -486,7 +487,7 @@ export default function ProviderAuthPanel({