diff --git a/docs-site/src/content/docs/guides/subagent-v1-default.md b/docs-site/src/content/docs/guides/subagent-v1-default.md index 6c6c69d55e..5f4fd79b58 100644 --- a/docs-site/src/content/docs/guides/subagent-v1-default.md +++ b/docs-site/src/content/docs/guides/subagent-v1-default.md @@ -105,9 +105,10 @@ Four routes, in the order most people should try them: 3. **Trust a direct key-auth Responses relay.** A provider you explicitly mark with `allowEncryptedV2AgentTasks: true` receives the opaque payload instead of the 400. Only do this for a destination you know can consume it. -4. **Enable `agentTaskRecovery`.** Experimental and off by default. It recovers most fresh spawns - through the ChatGPT backend, at the cost of quota, latency and a dependency on undocumented - behavior, and it still loses message-type follow-ups and multipart envelopes. +4. **Enable `agentTaskRecovery`.** Experimental and off by default. It recovers unreadable encrypted + `NEW_TASK`, `MESSAGE`, `FOLLOWUP_TASK`, and `FINAL_ANSWER` items through the ChatGPT backend, at + the cost of quota, latency and a dependency on undocumented behavior; combo recovery remains + limited to spawned-child turns, and split-token fragments stay unsupported. See [Sub-agent Surface](/guides/sub-agent-surface/) for the full mechanics of each, and [Agent configuration](/reference/configuration/agents/) for the settings themselves. diff --git a/docs-site/src/content/docs/reference/configuration/agents.md b/docs-site/src/content/docs/reference/configuration/agents.md index 0fd021ad65..d546eb07a2 100644 --- a/docs-site/src/content/docs/reference/configuration/agents.md +++ b/docs-site/src/content/docs/reference/configuration/agents.md @@ -225,7 +225,13 @@ explicitly enabled and the final routed task contains an otherwise unreadable Fe opencodex uses a raw Responses passthrough request to the fixed `https://chatgpt.com/backend-api/codex/responses` endpoint with forward-mode authentication. ChatGPT returns the plaintext assignment through a forced function call; opencodex then converts -only that task item to a standard user message before routed-provider dispatch. +only that task item to a standard user message before routed-provider dispatch. Direct routed +recovery, cached history replay, and the unreadable-task detector recognise all four codex-rs +agent-message types: `NEW_TASK`, `MESSAGE`, `FOLLOWUP_TASK`, and `FINAL_ANSWER`. Combo recovery +remains limited to spawned-child turns. A `FINAL_ANSWER` envelope may omit its `Task name` line. +Recovery then has no header address to compare with the item's recipient, so that single cross-check +does not run; the sender comparison and the cache scope, which still binds the structured recipient, +are unchanged. This is not local decryption and does not fix the Codex wire protocol. It depends on undocumented ChatGPT backend behavior and may stop working after a backend change. The recovered assignment is diff --git a/docs-site/src/content/docs/reference/configuration/providers.md b/docs-site/src/content/docs/reference/configuration/providers.md index a62e8ed817..5ee01aa278 100644 --- a/docs-site/src/content/docs/reference/configuration/providers.md +++ b/docs-site/src/content/docs/reference/configuration/providers.md @@ -1093,12 +1093,14 @@ whitespace-only strings remain unchanged, as do incomplete and mixed encrypted/u Encrypted and unknown content is not normalized; native encrypted tasks still require the separate opt-in [task recovery](/reference/configuration/agents/#encrypted-v2-task-recovery). -With task recovery enabled, replayed `NEW_TASK` and `MESSAGE` items reuse a cached assignment only -after validating the caller and matching the parent-thread scope. Replay restoration -does not make a new recovery request or extend cache expiry. Expired or unseen -ciphertext is not replaced. Fresh encrypted `NEW_TASK` and `MESSAGE` items use the same -opt-in recovery path, including native-parent `send_message` delivery. Message type, -sender, recipient, parent scope and caller credentials remain part of validation or cache identity. +With task recovery enabled, replayed `NEW_TASK`, `MESSAGE`, `FOLLOWUP_TASK`, and `FINAL_ANSWER` +items reuse a cached assignment only after validating the caller and matching the parent-thread +scope. Replay restoration does not make a new recovery request or extend cache expiry. Expired or +unseen ciphertext is not replaced. Fresh encrypted `NEW_TASK`, `MESSAGE`, `FOLLOWUP_TASK`, and +`FINAL_ANSWER` items use the same opt-in recovery path, including native-parent `send_message` +delivery. Message type, sender, recipient, parent scope and caller credentials remain part of +validation or cache identity. A `FINAL_ANSWER` without a `Task name` line has no header address to +cross-check, but its recipient still scopes the cache. When a request contains several agent messages, cached replay restoration checks each message independently. The cache separates message type, sender, recipient and ciphertext diff --git a/src/server/responses/agent-task-recovery.ts b/src/server/responses/agent-task-recovery.ts index 8f44661d22..a77ac55c53 100644 --- a/src/server/responses/agent-task-recovery.ts +++ b/src/server/responses/agent-task-recovery.ts @@ -75,15 +75,17 @@ interface AgentEnvelope { encryptedStartIndex: number; inputSnapshot: string; headerText: string; - messageType: "NEW_TASK" | "MESSAGE"; - taskName: string; + messageType: "NEW_TASK" | "MESSAGE" | "FOLLOWUP_TASK" | "FINAL_ANSWER"; + taskName: string | null; sender: string; ciphertexts: readonly string[]; author: string; recipient: string; } -const ROUTING_HEADER = /(?:^|\n)Message Type\s*:\s*(NEW_TASK|MESSAGE)\s*\nTask name\s*:\s*(\S+)\s*\nSender\s*:\s*(\S+)\s*\nPayload\s*:\s*(?:\n|$)/; +const ROUTING_HEADER = /(?:^|\n)Message Type\s*:\s*(NEW_TASK|MESSAGE|FOLLOWUP_TASK)\s*\nTask name\s*:\s*(\S+)\s*\nSender\s*:\s*(\S+)\s*\nPayload\s*:\s*(?:\n|$)/; +// FINAL_ANSWER omits the Task name line when the sender declares no recipient. +const FINAL_ANSWER_HEADER = /(?:^|\n)Message Type\s*:\s*FINAL_ANSWER\s*\n(?:Task name\s*:\s*(\S+)\s*\n)?Sender\s*:\s*(\S+)\s*\nPayload\s*:\s*(?:\n|$)/; function findEnvelope(input: unknown): AgentEnvelope | null { if (!Array.isArray(input)) return null; @@ -104,7 +106,7 @@ function findEnvelope(input: unknown): AgentEnvelope | null { if (!Array.isArray(content)) return null; let headerText: string | null = null; - let messageType: "NEW_TASK" | "MESSAGE" | null = null; + let messageType: "NEW_TASK" | "MESSAGE" | "FOLLOWUP_TASK" | "FINAL_ANSWER" | null = null; let taskName: string | null = null; let sender: string | null = null; let encryptedStartIndex = -1; @@ -119,16 +121,24 @@ function findEnvelope(input: unknown): AgentEnvelope | null { && typeof part.text === "string" ) { const match = ROUTING_HEADER.exec(part.text); - if (match) { + const finalMatch = match ? null : FINAL_ANSWER_HEADER.exec(part.text); + if (match || finalMatch) { if (headerText !== null) return null; + const m = match ?? finalMatch!; if ( - part.text.slice(0, match.index).trim().length > 0 - || part.text.slice(match.index + match[0].length).trim().length > 0 + part.text.slice(0, m.index).trim().length > 0 + || part.text.slice(m.index + m[0].length).trim().length > 0 ) return null; - headerText = match[0].startsWith("\n") ? match[0].slice(1) : match[0]; - messageType = match[1] as "NEW_TASK" | "MESSAGE"; - taskName = match[2]!; - sender = match[3]!; + headerText = m[0].startsWith("\n") ? m[0].slice(1) : m[0]; + if (match) { + messageType = match[1] as "NEW_TASK" | "MESSAGE" | "FOLLOWUP_TASK"; + taskName = match[2]!; + sender = match[3]!; + } else { + messageType = "FINAL_ANSWER"; + taskName = finalMatch![1] ?? null; + sender = finalMatch![2]!; + } } } if (part.type !== "encrypted_content") continue; @@ -145,7 +155,6 @@ function findEnvelope(input: unknown): AgentEnvelope | null { if ( !headerText || !messageType - || !taskName || !sender || encryptedStartIndex < 0 || ciphertexts.length === 0 @@ -153,7 +162,13 @@ function findEnvelope(input: unknown): AgentEnvelope | null { const itemRecord = item as { author?: unknown; recipient?: unknown }; if (typeof itemRecord.author !== "string" || typeof itemRecord.recipient !== "string") return null; - if (itemRecord.author !== sender || itemRecord.recipient !== taskName) return null; + // A FINAL_ANSWER without a Task name line declares no recipient, so the structured + // recipient is only cross-checked when a task name is present; admission is the + // trust boundary either way. + if ( + itemRecord.author !== sender + || (taskName !== null && itemRecord.recipient !== taskName) + ) return null; return { itemIndex, @@ -170,10 +185,18 @@ function findEnvelope(input: unknown): AgentEnvelope | null { } function stripMatchingEnvelope(assignment: string, envelope: AgentEnvelope): string | null { - const match = ROUTING_HEADER.exec(assignment); - if (!match) return assignment; + const header = envelope.messageType === "FINAL_ANSWER" ? FINAL_ANSWER_HEADER : ROUTING_HEADER; + const match = header.exec(assignment); + if (!match) { + // A routing header of the other family is still an echoed envelope: a bare payload does not + // carry one, and a same-family header past the start already fails closed, so reject it too. + const foreign = (header === FINAL_ANSWER_HEADER ? ROUTING_HEADER : FINAL_ANSWER_HEADER).exec(assignment); + return foreign ? null : assignment; + } if (match.index !== 0) return null; - if ( + if (envelope.messageType === "FINAL_ANSWER") { + if ((match[1] ?? null) !== envelope.taskName || match[2] !== envelope.sender) return null; + } else if ( match[1] !== envelope.messageType || match[2] !== envelope.taskName || match[3] !== envelope.sender @@ -295,18 +318,21 @@ function admittedRecovery( if (!envelope) return { admitted: false, reason: "unsupported_envelope" }; const admission = recoveryAdmission(req, config); if (!admission) return { admitted: false, reason: "admission_denied" }; + // A JSON-encoded fixed-order tuple, not a delimiter-joined string: a field that carries the + // delimiter shifts every boundary after it, so two envelopes could hash to one entry and one + // recovery would replay for the other. A FINAL_ANSWER that omits its Task name line carries no + // addressing in the header, which leaves the structured recipient as the only field separating + // two such envelopes and makes the boundary the whole difference. const cacheKey = createHash("sha256") - .update(admission.cacheScope) - .update("\0") - .update(parentThreadId ?? "") - .update("\0") - .update(envelope.messageType) - .update("\0") - .update(envelope.taskName) - .update("\0") - .update(envelope.sender) - .update("\0") - .update(JSON.stringify(envelope.ciphertexts)) + .update(JSON.stringify([ + admission.cacheScope, + parentThreadId ?? "", + envelope.messageType, + envelope.taskName ?? "", + envelope.recipient, + envelope.sender, + envelope.ciphertexts, + ])) .digest("hex"); return { admitted: true, recovery: { envelope, admission, cacheKey } }; } diff --git a/src/server/responses/encrypted-payload.ts b/src/server/responses/encrypted-payload.ts index 0e9efddf99..2df430aecc 100644 --- a/src/server/responses/encrypted-payload.ts +++ b/src/server/responses/encrypted-payload.ts @@ -177,21 +177,21 @@ function textWithoutFernetRuns(payload: string, runs: readonly FernetTokenRun[]) /** * The routing header codex-rs writes above a delegated agent payload. * - * `MESSAGE` is matched as well as `NEW_TASK`, and only for the unreadability CHECK -- - * recovery stays NEW_TASK-only. #3021 reported a subagent `MESSAGE` arriving in the - * parent conversation as raw `gAAAA...` ciphertext after an `adapter_eof`. The detector - * decides "unreadable" by stripping the envelope and asking whether any plaintext - * survives, so an envelope shape it does not recognise counts as surviving text: a - * `MESSAGE` whose entire body is one Fernet token measured as READABLE and was forwarded - * verbatim. + * All four codex-rs message types are recognised: NEW_TASK, MESSAGE, FOLLOWUP_TASK, + * and FINAL_ANSWER, whose Task name line is optional. #3021 reported a subagent + * `MESSAGE` arriving in the parent conversation as raw `gAAAA...` ciphertext after an + * `adapter_eof`. The detector decides "unreadable" by stripping the envelope and asking + * whether any plaintext survives, so an envelope shape it does not recognise counts as + * surviving text: an unrecognised type whose entire body is one Fernet token measured as + * READABLE and would be forwarded verbatim. * - * Widening the strip is not the same as widening recovery. Recovery decrypts, and - * decrypting a `MESSAGE` on the parent's behalf would build a plaintext oracle out of a - * payload the parent's session may have no right to read. This only lets the proxy - * NOTICE that what it is about to forward is unreadable ciphertext, which is what the - * report asks for: fail closed with a structured error rather than paste the token. + * This strip must therefore stay in step with the message types the opt-in recovery + * recognises (see agent-task-recovery.ts). The strip itself is still only detection: it + * lets the proxy notice that what it is about to forward is unreadable ciphertext and + * fail closed with a structured error rather than paste the token. Recovery admission, + * not the strip, is the trust boundary for decryption. */ -export const AGENT_MESSAGE_ROUTING_ENVELOPE = /(?:^|\n)Message Type\s*:\s*(?:NEW_TASK|MESSAGE)[^\n]*\nTask name\s*:[^\n]*\nSender\s*:[^\n]*\nPayload\s*:\s*(?:\n|$)/gi; +export const AGENT_MESSAGE_ROUTING_ENVELOPE = /(?:^|\n)Message Type\s*:\s*(?:NEW_TASK|MESSAGE|FOLLOWUP_TASK)[^\n]*\nTask name\s*:[^\n]*\nSender\s*:[^\n]*\nPayload\s*:\s*(?:\n|$)|(?:^|\n)Message Type\s*:\s*FINAL_ANSWER[^\n]*\n(?:Task name\s*:[^\n]*\n)?Sender\s*:[^\n]*\nPayload\s*:\s*(?:\n|$)/gi; // CXC is the compatibility-hook control namespace. Strip only the tagged paragraph: // later untagged paragraphs may be genuine task text. Repeated CXC paragraphs are @@ -241,7 +241,8 @@ function splitFernetParts(content: unknown[]): Set { export function hasUnreadableEncryptedAgentTask(input: unknown): boolean { if (!Array.isArray(input)) return false; - // codex-rs appends one NEW_TASK agent_message at the current input tail. Historical + // codex-rs appends one agent_message (any of the four codex-rs message types) at the + // current input tail. Historical // agent messages may be adjacent in full-history bodies; they must not poison the // later task. compaction_trigger/additional_tools are trailing metadata rather than // a newer user turn. diff --git a/structure/subagents.md b/structure/subagents.md index 527b2b3752..c9836ad3b1 100644 --- a/structure/subagents.md +++ b/structure/subagents.md @@ -160,10 +160,16 @@ Full derivation with per-line citations: `devlog/_plan/260816_codexrs_multiagent `src/server/responses/agent-task-recovery.ts` admits at most 32 consecutive, individually complete Fernet-shaped parts with a combined 2 MiB ciphertext limit. Every encrypted slot must belong to -that run. The existing credential admission precedes cache access; the cache key includes an -unambiguous ordered sequence. One fixed-endpoint request forwards separate parts, and assignment +that run. The existing credential admission precedes cache access; the cache key is a JSON-encoded +fixed-order tuple of every addressing field (scope, parent thread, message type, task name, +recipient, sender, ciphertexts) rather than a delimiter-joined string, so no field content can shift +a boundary. One fixed-endpoint request forwards separate parts, and assignment replacement compares the complete original item snapshot before splicing the run. Recovery output is model-transcribed plaintext, not cryptographic fidelity proof, and no internal outage retry is added. +Recovery recognises all four codex-rs message types (NEW_TASK, MESSAGE, FOLLOWUP_TASK, +FINAL_ANSWER); a FINAL_ANSWER envelope may omit the Task name line, in which case the +structured recipient is not cross-checked because the envelope names no recipient, and +admission remains the trust boundary. `src/server/responses/encrypted-payload.ts` uses bounded concatenation only to recognize otherwise unreadable split-token shapes. The sanitizer preserves just those fragment objects and continues @@ -223,7 +229,8 @@ target its own `structuredClone` and its own concrete route, so a sibling's repa them and a target resolving to a routed Responses wire would otherwise send what the parent's own dispatch no longer does. -Nothing here decrypts, and the tail NEW_TASK envelope keeps `unreadable_encrypted_agent_task` and +Nothing here decrypts, and the tail agent_message envelope (any of the four codex-rs +message types) keeps `unreadable_encrypted_agent_task` and its opt-in recovery unchanged: an unreadable current task still fails closed rather than reaching a child with a marker where its assignment should be. An `agent_message` carrying unknown parts but no ciphertext still reaches the wire unchanged and still draws the destination's own 422, which is diff --git a/tests/claude-integration/claude-native-passthrough.test.ts b/tests/claude-integration/claude-native-passthrough.test.ts index 44cf9333f4..8966ee7ed9 100644 --- a/tests/claude-integration/claude-native-passthrough.test.ts +++ b/tests/claude-integration/claude-native-passthrough.test.ts @@ -6,6 +6,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { saveConfig } from "../../src/config"; import { buildDesktop3pRegistry } from "../../src/claude/desktop-3p"; +import { isolationBudgetMs } from "../helpers/ci-watchdog"; import { SERVER_BUDGET_MS } from "../helpers/test-budget"; import { startServer } from "../../src/server"; import type { OcxConfig } from "../../src/types"; @@ -62,7 +63,9 @@ function cfg(anthropicBaseUrl: string, extraClaude?: Record): O providers: { mock: { adapter: "openai-chat", baseUrl: "http://127.0.0.1:1/v1", apiKey: "k", allowPrivateNetwork: true, liveModels: false, models: ["test-model"] }, }, - connectTimeoutMs: 250, + // Shortened on purpose so a wedged upstream fails fast; the wrapper's full-suite + // lane needs headroom for a loopback round-trip on a busy machine. + connectTimeoutMs: isolationBudgetMs(250), claudeCode: { anthropicBaseUrl, ...extraClaude }, } as OcxConfig; } diff --git a/tests/cli/cli-headless-parity.test.ts b/tests/cli/cli-headless-parity.test.ts index 9b4bd04ce4..ce52216d16 100644 --- a/tests/cli/cli-headless-parity.test.ts +++ b/tests/cli/cli-headless-parity.test.ts @@ -1249,6 +1249,13 @@ describe("Aside CLI recovery metadata", () => { expect(await handleClientIntegrationCommand([ "enable", "--client", "aside", ...(wantsJson ? ["--json"] : []), ], runtime.deps)).toBe(1); + // Assert the failure channel before stdout. A request that never reached the route leaves + // stdout empty for a reason stderr alone names, and checking stdout first reports the empty + // rendering instead of the transport error that caused it. + expect(error.mock.calls.map(call => String(call[0])).join("\n")).toContain(result.message); + expect(runtime.requests).toEqual([{ + path: "/api/client-integrations/aside/profiles", method: "PUT", body: { enabled: true }, + }]); const stdout = log.mock.calls.map(call => String(call[0])).join("\n"); if (wantsJson) { expect(JSON.parse(stdout)).toEqual(result); @@ -1261,10 +1268,6 @@ describe("Aside CLI recovery metadata", () => { "aside:9 Profile 9 recovery failed Recovery did not finish.", ]); } - expect(error.mock.calls.map(call => String(call[0])).join("\n")).toContain(result.message); - expect(runtime.requests).toEqual([{ - path: "/api/client-integrations/aside/profiles", method: "PUT", body: { enabled: true }, - }]); } finally { log.mockRestore(); error.mockRestore(); diff --git a/tests/clients/remote-workspace-command-runner.test.ts b/tests/clients/remote-workspace-command-runner.test.ts index 376ed9e0c3..cf3ffd5d76 100644 --- a/tests/clients/remote-workspace-command-runner.test.ts +++ b/tests/clients/remote-workspace-command-runner.test.ts @@ -17,6 +17,19 @@ import { removeTreeWithRetry } from "../helpers/remove-tree"; const roots: string[] = []; +// trustedBubblewrap() only stats this path, so POSIX uses the system shell and Windows a minimal +// fixture that is never executed. The host runtime cannot serve: node_modules/bun hard-links +// bin/bunx.exe to bin/bun.exe, and a /tmp checkout fails the ancestor rule Windows skips. +function trustedSandboxBinary(): string { + if (process.platform !== "win32") return realpathSync("/bin/sh"); + const root = mkdtempSync(join(tmpdir(), "ocx-trusted-sandbox-")); + roots.push(root); + const fixture = join(root, "bwrap.exe"); + writeFileSync(fixture, "", { mode: 0o755 }); + chmodSync(fixture, 0o755); + return realpathSync(fixture); +} + afterEach(() => { for (const root of roots.splice(0)) removeTreeWithRetry(root); }); @@ -83,14 +96,15 @@ describe("remote workspace Linux command sandbox", () => { test("builds a minimal bubblewrap argv with one writable workspace", () => { const state = fixture(); + const sandboxBinary = trustedSandboxBinary(); const argv = linuxRemoteWorkspaceCommandArgv({ command: ["/bin/sh", "-lc", "pwd"], root: state.workspace, cwd: join(state.workspace, "project"), timeoutMs: 1_000, maxOutputBytes: 4_096, - }, { bubblewrapPath: process.execPath }); - expect(argv[0]).toBe(process.execPath); + }, { bubblewrapPath: sandboxBinary }); + expect(argv[0]).toBe(sandboxBinary); expect(argv).toContain("--unshare-net"); expect(argv).toContain("--clearenv"); expect(argv).toContain("--bind"); @@ -280,6 +294,7 @@ describe("remote workspace Linux command sandbox", () => { test("revalidates approved toolchain roots and rejects a later symlink substitution", () => { const state = fixture(); + const sandboxBinary = trustedSandboxBinary(); const realToolchain = join(state.root, "real-toolchain"); const substituted = join(state.root, "toolchain"); mkdirSync(realToolchain); @@ -291,7 +306,7 @@ describe("remote workspace Linux command sandbox", () => { timeoutMs: 1_000, maxOutputBytes: 4_096, }, { - bubblewrapPath: process.execPath, + bubblewrapPath: sandboxBinary, toolchainRoots: [substituted], })).toThrow("remain a real directory"); }); diff --git a/tests/helpers/agent-task-recovery.ts b/tests/helpers/agent-task-recovery.ts index 4a6a95c5ae..1d5178bc88 100644 --- a/tests/helpers/agent-task-recovery.ts +++ b/tests/helpers/agent-task-recovery.ts @@ -47,6 +47,19 @@ function routingEnvelope( export const ROUTING_ENVELOPE = routingEnvelope(); +function finalAnswerEnvelope(withTaskName: boolean, taskName = "/root/worker", sender = "/root"): string { + return [ + "Message Type: FINAL_ANSWER", + ...(withTaskName ? [`Task name: ${taskName}`] : []), + `Sender: ${sender}`, + "Payload:", + "", + ].join("\n"); +} + +export const FINAL_ANSWER_ENVELOPE = finalAnswerEnvelope(false); +export const FINAL_ANSWER_TASK_ENVELOPE = finalAnswerEnvelope(true); + export function agentMessage(content: Array>): unknown[] { return [{ type: "agent_message", diff --git a/tests/helpers/responses-state-never-settling-acl-child.ts b/tests/helpers/responses-state-never-settling-acl-child.ts index 0f83f78ff1..b46c64e00d 100644 --- a/tests/helpers/responses-state-never-settling-acl-child.ts +++ b/tests/helpers/responses-state-never-settling-acl-child.ts @@ -11,6 +11,7 @@ import { setResponseStateByteCapForTests, } from "../../src/responses/state"; import { + setAsyncIcaclsBeltSchedulerForTests, setAsyncIcaclsRunnerForTests, setPlatformForTests, } from "../../src/lib/windows-secret-acl"; @@ -46,6 +47,16 @@ if (mode === "principal") { setAsyncIcaclsRunnerForTests(async () => ({ success: true, exitCode: 0, timedOut: false, stdout: "" })); } else { setAsyncIcaclsRunnerForTests(() => new Promise(() => {})); + // The product belt waits out SUBPROCESS_KILL_GRACE_MS plus its margin before releasing a caller + // whose killed child has not reaped, so an in-process runner that never settles would spend + // 2 x 2350 ms reaching the queue's unavoidable retry and tombstone -- longer than this fixture's + // watchdog. There is no child here to reap, so fire the same belt on a short real timer: the + // bounded attempt/retry/tombstone path stays under test, and the real belt duration stays + // covered by tests/lib/stall-subprocess-exit.test.ts. + setAsyncIcaclsBeltSchedulerForTests(callback => { + const timer = setTimeout(callback, 50); + return () => clearTimeout(timer); + }); } rememberLarge(`resp_never_settling_${mode}_first`); diff --git a/tests/responses/responses-context-overflow.test.ts b/tests/responses/responses-context-overflow.test.ts index 24be2316ee..f6023d53ac 100644 --- a/tests/responses/responses-context-overflow.test.ts +++ b/tests/responses/responses-context-overflow.test.ts @@ -1,4 +1,4 @@ -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { afterEach, beforeEach, describe, expect, setDefaultTimeout, test } from "bun:test"; import { mkdtempSync} from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -7,6 +7,11 @@ import { startServer } from "../../src/server"; import { PROVIDER_INPUT_TOO_LARGE_MESSAGE } from "../../src/server/responses/context-overflow"; import type { OcxConfig, OcxProviderConfig } from "../../src/types"; import { removeTreeWithRetry } from "../helpers/remove-tree"; +import { SERVER_BUDGET_MS } from "../helpers/test-budget"; + +// Every case here binds a real listener and drives it over HTTP, so the file-wide budget is +// the server budget; the assertions themselves are unchanged. +setDefaultTimeout(SERVER_BUDGET_MS); let testDir = ""; let previousOcxHome: string | undefined; diff --git a/tests/server/agent-task-recovery.test.ts b/tests/server/agent-task-recovery.test.ts index faf117f1a7..31d01e67ee 100644 --- a/tests/server/agent-task-recovery.test.ts +++ b/tests/server/agent-task-recovery.test.ts @@ -16,6 +16,8 @@ import { codexHeaders, encryptedInput, FERNET_TASK, + FINAL_ANSWER_ENVELOPE, + FINAL_ANSWER_TASK_ENVELOPE, originalFetch, post, providerResponse, @@ -37,7 +39,7 @@ describe("agent task recovery (opt-in, default off)", () => { resetAgentTaskRecoveryState(); }); - for (const messageType of ["NEW_TASK", "MESSAGE"] as const) { + for (const messageType of ["NEW_TASK", "MESSAGE", "FOLLOWUP_TASK"] as const) { test(`typed ${messageType} recovery preserves boolean, replay and discard contracts`, async () => { const req = new Request("http://localhost/v1/responses", { headers: codexHeaders() }); const config = routedConfig(); @@ -991,7 +993,7 @@ describe("bounded multipart encrypted task recovery", () => { ...tokens.map(encrypted_content => ({ type: "encrypted_content", encrypted_content })), ]); - test.each(["NEW_TASK", "MESSAGE"] as const)("recovers ordered %s parts in one request and isolates sequence caches", async messageType => { + test.each(["NEW_TASK", "MESSAGE", "FOLLOWUP_TASK"] as const)("recovers ordered %s parts in one request and isolates sequence caches", async messageType => { let sends = 0; const sent: Array<{ input: Array<{ content: Array<{ encrypted_content?: string }> }> }> = []; globalThis.fetch = (async (_url, init) => { @@ -1115,3 +1117,158 @@ describe("bounded multipart encrypted task recovery", () => { expect(sends).toBe(1); }); }); + +describe("FINAL_ANSWER encrypted task recovery", () => { + beforeEach(() => resetAgentTaskRecoveryState()); + afterEach(() => { globalThis.fetch = originalFetch; resetAgentTaskRecoveryState(); }); + + test("recovers a FINAL_ANSWER without a Task name line and replays it from cache", async () => { + const req = new Request("http://localhost/v1/responses", { headers: codexHeaders() }); + let fetches = 0; + globalThis.fetch = (async () => { + fetches += 1; + return new Response(recoverySse("Recovered final answer.")); + }) as typeof fetch; + const input = () => agentMessage([ + { type: "input_text", text: FINAL_ANSWER_ENVELOPE }, + { type: "encrypted_content", encrypted_content: FERNET_TASK }, + ]); + const typedInput = input(); + expect(await recoverEncryptedAgentTaskWithResult(req, typedInput, {}, routedConfig())).toEqual({ recovered: true }); + expect(typedInput).toEqual([{ + type: "message", role: "user", content: [ + { type: "input_text", text: FINAL_ANSWER_ENVELOPE }, + { type: "input_text", text: "Recovered final answer." }, + ], + }]); + expect(restoreCachedEncryptedAgentTasks(req, input(), routedConfig())).toBe(1); + expect(fetches).toBe(1); + discardEncryptedAgentTaskRecovery(req, input(), routedConfig()); + expect(restoreCachedEncryptedAgentTasks(req, input(), routedConfig())).toBe(0); + }); + + test("does not share a cache entry when a NUL byte moves between recipient and sender", async () => { + // Both envelopes below carry the same admission scope, parent thread, message type, absent + // Task name and ciphertext, and their recipient/sender fields concatenate to the same bytes + // once a separator is placed between them. Moving where the NUL sits must not move the key. + const req = new Request("http://localhost/v1/responses", { headers: codexHeaders() }); + let fetches = 0; + globalThis.fetch = (async () => { + fetches += 1; + return new Response(recoverySse("Recovered final answer.")); + }) as typeof fetch; + const input = (recipient: string, sender: string) => [{ + type: "agent_message", + author: sender, + recipient, + content: [ + { + type: "input_text", + text: ["Message Type: FINAL_ANSWER", `Sender: ${sender}`, "Payload:", ""].join("\n"), + }, + { type: "encrypted_content", encrypted_content: FERNET_TASK }, + ], + }]; + expect(await recoverEncryptedAgentTaskWithResult(req, input("r", "s\0t"), {}, routedConfig())) + .toEqual({ recovered: true }); + expect(fetches).toBe(1); + // A different split of the same concatenation is a different envelope, not a cache hit. + expect(restoreCachedEncryptedAgentTasks(req, input("r\0s", "t"), routedConfig())).toBe(0); + // The envelope the cache was actually filled from still replays, so the line above is not + // passing because nothing was cached at all. + expect(restoreCachedEncryptedAgentTasks(req, input("r", "s\0t"), routedConfig())).toBe(1); + }); + + test("recovers a FINAL_ANSWER whose Task name matches the structured recipient", async () => { + const req = new Request("http://localhost/v1/responses", { headers: codexHeaders() }); + let fetches = 0; + globalThis.fetch = (async () => { + fetches += 1; + return new Response(recoverySse("Recovered final answer.")); + }) as typeof fetch; + const input = agentMessage([ + { type: "input_text", text: FINAL_ANSWER_TASK_ENVELOPE }, + { type: "encrypted_content", encrypted_content: FERNET_TASK }, + ]); + expect(await recoverEncryptedAgentTaskWithResult(req, input, {}, routedConfig())).toEqual({ recovered: true }); + expect(fetches).toBe(1); + }); + + test("accepts a FINAL_ANSWER assignment that echoes its own header", async () => { + const req = new Request("http://localhost/v1/responses", { headers: codexHeaders() }); + let fetches = 0; + globalThis.fetch = (async () => { + fetches += 1; + return new Response(recoverySse(`${FINAL_ANSWER_ENVELOPE}Recovered final answer.`)); + }) as typeof fetch; + const input = agentMessage([ + { type: "input_text", text: FINAL_ANSWER_ENVELOPE }, + { type: "encrypted_content", encrypted_content: FERNET_TASK }, + ]); + expect(await recoverEncryptedAgentTaskWithResult(req, input, {}, routedConfig())).toEqual({ recovered: true }); + expect(input).toEqual([{ + type: "message", role: "user", content: [ + { type: "input_text", text: FINAL_ANSWER_ENVELOPE }, + { type: "input_text", text: "Recovered final answer." }, + ], + }]); + expect(fetches).toBe(1); + }); + + test.each([ + ["sender mismatch", () => [{ + type: "agent_message", + author: "/other", + recipient: "/root/worker", + content: [ + { type: "input_text", text: FINAL_ANSWER_ENVELOPE }, + { type: "encrypted_content", encrypted_content: FERNET_TASK }, + ], + }]], + ["recipient mismatch against the Task name line", () => agentMessage([ + { type: "input_text", text: FINAL_ANSWER_TASK_ENVELOPE.replace("/root/worker", "/root/other-worker") }, + { type: "encrypted_content", encrypted_content: FERNET_TASK }, + ])], + ] as const)("refuses %s FINAL_ANSWER without a recovery dispatch", async (_label, makeInput) => { + let fetches = 0; + globalThis.fetch = (async () => { + fetches += 1; + return new Response(recoverySse("must not run")); + }) as typeof fetch; + const req = new Request("http://localhost/v1/responses", { headers: codexHeaders() }); + const input = makeInput(); + const before = structuredClone(input); + expect(await recoverEncryptedAgentTaskWithResult(req, input, {}, routedConfig())).toEqual({ recovered: false, reason: "unsupported_envelope" }); + expect(input).toEqual(before); + expect(fetches).toBe(0); + }); +}); + +describe("recovery refuses a wrong-family echoed routing header", () => { + beforeEach(() => resetAgentTaskRecoveryState()); + afterEach(() => { globalThis.fetch = originalFetch; resetAgentTaskRecoveryState(); }); + + const echoCases: Array<[string, string, string]> = [ + ["FINAL_ANSWER envelope echoing a NEW_TASK header", FINAL_ANSWER_ENVELOPE, `${ROUTING_ENVELOPE}Recovered final answer.`], + ["FINAL_ANSWER envelope echoing a NEW_TASK header mid-assignment", FINAL_ANSWER_ENVELOPE, `Recovered final answer.\n\n${ROUTING_ENVELOPE}`], + ["NEW_TASK envelope echoing a FINAL_ANSWER header", ROUTING_ENVELOPE, `${FINAL_ANSWER_ENVELOPE}Recovered final answer.`], + ["MESSAGE envelope echoing a FINAL_ANSWER header", ROUTING_ENVELOPE.replace("NEW_TASK", "MESSAGE"), `${FINAL_ANSWER_ENVELOPE}Recovered final answer.`], + ]; + + test.each(echoCases)("refuses %s", async (_label, envelopeText, assignment) => { + let fetches = 0; + globalThis.fetch = (async () => { + fetches += 1; + return new Response(recoverySse(assignment)); + }) as typeof fetch; + const req = new Request("http://localhost/v1/responses", { headers: codexHeaders() }); + const input = agentMessage([ + { type: "input_text", text: envelopeText }, + { type: "encrypted_content", encrypted_content: FERNET_TASK }, + ]); + const before = structuredClone(input); + expect(await recoverEncryptedAgentTaskWithResult(req, input, {}, routedConfig())).toEqual({ recovered: false, reason: "recovery_invalid_output" }); + expect(input).toEqual(before); + expect(fetches).toBe(1); + }); +}); diff --git a/tests/server/server-agent-task-recovery-replay.test.ts b/tests/server/server-agent-task-recovery-replay.test.ts index caaad660aa..014c3a57cd 100644 --- a/tests/server/server-agent-task-recovery-replay.test.ts +++ b/tests/server/server-agent-task-recovery-replay.test.ts @@ -6,7 +6,7 @@ import { bindTurnTerminationScope, rememberDeliveredFinalAnswer } from "../../sr import { conversationIdFromResponsesRequest } from "../../src/server/request-log-conversation"; import type { OcxParsedRequest } from "../../src/types"; import { recoverEncryptedAgentTask, resetAgentTaskRecoveryState, restoreCachedEncryptedAgentTasks } from "../../src/server/responses/agent-task-recovery"; -import { codexHeaders, encryptedInput, fakeChatGptJwt, FERNET_TASK, SECOND_FERNET_TASK, originalFetch, recoverySse, routedConfig } from "../helpers/agent-task-recovery"; +import { codexHeaders, encryptedInput, fakeChatGptJwt, FINAL_ANSWER_ENVELOPE, FERNET_TASK, SECOND_FERNET_TASK, originalFetch, recoverySse, routedConfig } from "../helpers/agent-task-recovery"; afterEach(() => { globalThis.fetch = originalFetch; resetAgentTaskRecoveryState(); }); test("replay reuses admitted recovery after a tool result without another network call", async () => { @@ -264,6 +264,37 @@ test("MESSAGE cache remains isolated by message type, account, parent and sender expect(calls).toBe(1); }); +test("FINAL_ANSWER cache stays isolated by structured recipient when the envelope names no task", async () => { + let calls = 0; + globalThis.fetch = (async () => { + calls++; + return new Response(recoverySse(calls === 1 ? "Worker assignment." : "Other worker assignment.")); + }) as typeof fetch; + const config = routedConfig({ enabled: true }); + const req = new Request("http://localhost/v1/responses", { headers: codexHeaders() }); + const scope = { parentThreadId: "parent" }; + // Same ciphertext, sender, credentials, and Task-name-less header for both; only the + // structured recipient differs, so the header alone cannot separate these envelopes. + const finalAnswer = (recipient: string): unknown[] => [{ + type: "agent_message", + author: "/root", + recipient, + content: [ + { type: "input_text", text: FINAL_ANSWER_ENVELOPE }, + { type: "encrypted_content", encrypted_content: FERNET_TASK }, + ], + }]; + expect(await recoverEncryptedAgentTask(req, finalAnswer("/root/worker"), {}, config, scope)).toBe(true); + expect(calls).toBe(1); + const other = finalAnswer("/root/other-worker"); + expect(restoreCachedEncryptedAgentTasks(req, other, config, scope)).toBe(0); + expect(JSON.stringify(other)).toContain(FERNET_TASK); + expect(JSON.stringify(other)).not.toContain("Worker assignment."); + expect(await recoverEncryptedAgentTask(req, other, {}, config, scope)).toBe(true); + expect(calls).toBe(2); + expect(JSON.stringify(other)).toContain("Other worker assignment."); +}); + test("mixed history restores cached NEW_TASK and MESSAGE separately before recovering only the new tail", async () => { let calls = 0; diff --git a/tests/server/server-kiro-completion-e2e.test.ts b/tests/server/server-kiro-completion-e2e.test.ts index 01327b482c..a81796425c 100644 --- a/tests/server/server-kiro-completion-e2e.test.ts +++ b/tests/server/server-kiro-completion-e2e.test.ts @@ -1,4 +1,4 @@ -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { afterEach, beforeEach, describe, expect, setDefaultTimeout, test } from "bun:test"; import { mkdtempSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -10,6 +10,11 @@ import { clearRequestLogsForTests, getRequestLogEntries } from "../../src/server import type { OcxConfig } from "../../src/types"; import { installIsolatedCodexHome, type IsolatedCodexHome } from "../helpers/isolated-codex-home"; import { removeTreeWithRetry } from "../helpers/remove-tree"; +import { SERVER_BUDGET_MS } from "../helpers/test-budget"; + +// Every case here binds a real listener and drives it over HTTP, so the file-wide budget is +// the server budget; the assertions themselves are unchanged. +setDefaultTimeout(SERVER_BUDGET_MS); const enc = new TextEncoder(); const originalFetch = globalThis.fetch; diff --git a/tests/server/server-management-auth.test.ts b/tests/server/server-management-auth.test.ts index d5dae24041..c8b661e322 100644 --- a/tests/server/server-management-auth.test.ts +++ b/tests/server/server-management-auth.test.ts @@ -462,7 +462,7 @@ describe("management and data-plane credential separation", () => { } finally { await server.stop(true); } - }); + }, SERVER_BUDGET_MS); // eight sequential live requests against a real listener; Bun's 5s default is not a server budget. test("a provider-reload capability is one-shot and exact to its operation", () => { const secret = "A".repeat(43); diff --git a/tests/server/v2-agent-message-failfast.test.ts b/tests/server/v2-agent-message-failfast.test.ts index 0e0dc66a24..d684ffb3d1 100644 --- a/tests/server/v2-agent-message-failfast.test.ts +++ b/tests/server/v2-agent-message-failfast.test.ts @@ -38,6 +38,15 @@ const ROUTING_ENVELOPE = [ // The same envelope a delegated agent uses to REPLY, as opposed to being spawned. // #3021 saw one of these reach the parent conversation as raw `gAAAA...` text. const MESSAGE_ROUTING_ENVELOPE = ROUTING_ENVELOPE.replace("NEW_TASK", "MESSAGE"); +// FOLLOWUP_TASK shares the four-line header; a FINAL_ANSWER completion may omit the +// Task name line entirely, in which case the envelope names no recipient. +const FOLLOWUP_ROUTING_ENVELOPE = ROUTING_ENVELOPE.replace("NEW_TASK", "FOLLOWUP_TASK"); +const FINAL_ANSWER_ENVELOPE = [ + "Message Type: FINAL_ANSWER", + "Sender: /root", + "Payload:", + "", +].join("\n"); afterEach(() => { globalThis.fetch = originalFetch; @@ -151,9 +160,8 @@ describe("V2 routed agent-message ciphertext guard", () => { * as surviving text. The envelope pattern matched only NEW_TASK, so a MESSAGE whose * entire body was one Fernet token measured as READABLE and was forwarded verbatim. * - * This is the detection half only. Recovery stays NEW_TASK-only on purpose: - * decrypting a MESSAGE on the parent's behalf would build a plaintext oracle out of - * a payload the parent's session may not be entitled to read. + * This is the detection half only. The opt-in recovery recognises the same envelope + * types; its admission gate, not the detector, is the trust boundary for decryption. */ test("blocks a MESSAGE reply envelope followed only by a Fernet payload", () => { expect(hasUnreadableEncryptedAgentTask(agentMessage([ @@ -182,6 +190,29 @@ describe("V2 routed agent-message ciphertext guard", () => { ]))).toBe(false); }); + test("blocks a FOLLOWUP_TASK envelope followed only by a Fernet payload", () => { + expect(hasUnreadableEncryptedAgentTask(agentMessage([ + { type: "input_text", text: FOLLOWUP_ROUTING_ENVELOPE }, + { type: "encrypted_content", encrypted_content: FERNET_TASK }, + ]))).toBe(true); + }); + + test("blocks a FINAL_ANSWER envelope without a Task name followed only by a Fernet payload", () => { + expect(hasUnreadableEncryptedAgentTask(agentMessage([ + { type: "input_text", text: FINAL_ANSWER_ENVELOPE }, + { type: "encrypted_content", encrypted_content: FERNET_TASK }, + ]))).toBe(true); + }); + + test("a FINAL_ANSWER reply that carries real text stays readable", () => { + // The control. The widened strip must not turn a FINAL_ANSWER carrying real text + // into a blocked one. + expect(hasUnreadableEncryptedAgentTask(agentMessage([ + { type: "input_text", text: `${FINAL_ANSWER_ENVELOPE}the worker finished the migration` }, + { type: "encrypted_content", encrypted_content: FERNET_TASK }, + ]))).toBe(false); + }); + test("blocks a control preamble mixed into the Fernet slot before sanitization", async () => { const input = agentMessage([ { type: "input_text", text: ROUTING_ENVELOPE },