Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/server/responses/agent-task-recovery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,15 +75,15 @@ interface AgentEnvelope {
itemIndex: number;
encryptedIndex: number;
headerText: string;
messageType: "NEW_TASK" | "MESSAGE";
messageType: "NEW_TASK";
taskName: string;
sender: string;
ciphertext: 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)\s*\nTask name\s*:\s*(\S+)\s*\nSender\s*:\s*(\S+)\s*\nPayload\s*:\s*(?:\n|$)/;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Reject MESSAGE headers rather than making them invisible

When an agent_message contains a MESSAGE routing-header part followed by a valid NEW_TASK routing-header part and ciphertext, this narrowed regex ignores the MESSAGE part, accepts the NEW_TASK part, and sends the ciphertext through recovery; previously both headers matched and the duplicate-header check rejected the input. A MESSAGE ciphertext can therefore be relabeled by adding a NEW_TASK sibling, bypassing the intended fail-closed boundary. Keep header recognition broad enough to detect both types, then explicitly admit only a sole NEW_TASK envelope.

AGENTS.md reference: AGENTS.md:L357-L363

Useful? React with 👍 / 👎.


function findEnvelope(input: unknown): AgentEnvelope | null {
if (!Array.isArray(input)) return null;
Expand All @@ -104,7 +104,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" | null = null;
let taskName: string | null = null;
let sender: string | null = null;
let encryptedIndex = -1;
Expand All @@ -127,7 +127,7 @@ function findEnvelope(input: unknown): AgentEnvelope | null {
|| part.text.slice(match.index + match[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";
messageType = match[1] as "NEW_TASK";
taskName = match[2]!;
sender = match[3]!;
}
Expand Down
114 changes: 74 additions & 40 deletions tests/server/agent-task-recovery.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,48 +36,62 @@ describe("agent task recovery (opt-in, default off)", () => {
resetAgentTaskRecoveryState();
});

for (const messageType of ["NEW_TASK", "MESSAGE"] 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();
const context = { parentThreadId: "parent-diagnostics" };
const input = () => agentMessage([
{ type: "input_text", text: ROUTING_ENVELOPE.replace("NEW_TASK", messageType) },
{ type: "encrypted_content", encrypted_content: FERNET_TASK },
]);
let fetches = 0;
globalThis.fetch = (async () => {
fetches += 1;
return new Response(recoverySse("Recovered diagnostic fixture."));
}) as typeof fetch;
test("typed NEW_TASK recovery preserves boolean, replay and discard contracts", async () => {
const req = new Request("http://localhost/v1/responses", { headers: codexHeaders() });
const config = routedConfig();
const context = { parentThreadId: "parent-diagnostics" };
const input = () => agentMessage([
{ type: "input_text", text: ROUTING_ENVELOPE },
{ type: "encrypted_content", encrypted_content: FERNET_TASK },
]);
let fetches = 0;
globalThis.fetch = (async () => {
fetches += 1;
return new Response(recoverySse("Recovered diagnostic fixture."));
}) as typeof fetch;

const typedInput = input();
expect(await recoverEncryptedAgentTaskWithResult(req, typedInput, {}, config, context))
.toEqual({ recovered: true });
const booleanInput = input();
expect(await recoverEncryptedAgentTask(req, booleanInput, {}, config, context)).toBe(true);
expect(booleanInput).toEqual(typedInput);
expect(typedInput).toEqual([{
type: "message", role: "user", content: [
{ type: "input_text", text: ROUTING_ENVELOPE.replace("NEW_TASK", messageType) },
{ type: "input_text", text: "Recovered diagnostic fixture." },
],
}]);
const replay = input();
expect(restoreCachedEncryptedAgentTasks(req, replay, config, context)).toBe(1);
expect(replay).toEqual(typedInput);
expect(fetches).toBe(1);
const typedInput = input();
expect(await recoverEncryptedAgentTaskWithResult(req, typedInput, {}, config, context))
.toEqual({ recovered: true });
const booleanInput = input();
expect(await recoverEncryptedAgentTask(req, booleanInput, {}, config, context)).toBe(true);
expect(booleanInput).toEqual(typedInput);
expect(typedInput).toEqual([{
type: "message", role: "user", content: [
{ type: "input_text", text: ROUTING_ENVELOPE },
{ type: "input_text", text: "Recovered diagnostic fixture." },
],
}]);
const replay = input();
expect(restoreCachedEncryptedAgentTasks(req, replay, config, context)).toBe(1);
expect(replay).toEqual(typedInput);
expect(fetches).toBe(1);

const otherType = agentMessage([
{ type: "input_text", text: ROUTING_ENVELOPE.replace("NEW_TASK", messageType === "MESSAGE" ? "NEW_TASK" : "MESSAGE") },
{ type: "encrypted_content", encrypted_content: FERNET_TASK },
]);
expect(restoreCachedEncryptedAgentTasks(req, otherType, config, context)).toBe(0);
discardEncryptedAgentTaskRecovery(req, input(), config, context);
expect(restoreCachedEncryptedAgentTasks(req, input(), config, context)).toBe(0);
expect(fetches).toBe(1);
});
}
discardEncryptedAgentTaskRecovery(req, input(), config, context);
expect(restoreCachedEncryptedAgentTasks(req, input(), config, context)).toBe(0);
expect(fetches).toBe(1);
});

test("MESSAGE envelopes fail closed without recovery or cache restoration", async () => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Update every test that still requires MESSAGE recovery

This new fail-closed expectation contradicts five unchanged cases in tests/server/server-agent-task-recovery-replay.test.ts, including “MESSAGE recovery reaches the provider,” the MESSAGE cache and mixed-history cases, the handler recovery case, and the MESSAGE-tail case at lines 184-217, 249-348, and 416-427. Those tests still expect recovery to return true or the handler to return HTTP 200, whereas this change now returns unsupported_envelope/HTTP 400, so the repository-wide test gate will fail until those scenarios are updated to the new contract.

AGENTS.md reference: AGENTS.md:L367-L370

Useful? React with 👍 / 👎.

const req = new Request("http://localhost/v1/responses", { headers: codexHeaders() });
const config = routedConfig();
const input = agentMessage([
{ type: "input_text", text: ROUTING_ENVELOPE.replace("NEW_TASK", "MESSAGE") },
{ type: "encrypted_content", encrypted_content: FERNET_TASK },
]);
const original = structuredClone(input);
let fetches = 0;
globalThis.fetch = (async () => {
fetches += 1;
return new Response(recoverySse("must not be recovered"));
}) as typeof fetch;

expect(await recoverEncryptedAgentTaskWithResult(req, input, {}, config))
.toEqual({ recovered: false, reason: "unsupported_envelope" });
expect(input).toEqual(original);
expect(restoreCachedEncryptedAgentTasks(req, input, config)).toBe(0);
expect(fetches).toBe(0);
});

const failedRecoveries: Array<[string, () => Response, AgentTaskRecoveryFailureReason]> = [
["HTTP 401", () => new Response("private-error", { status: 401 }), "recovery_http_rejected"],
Expand Down Expand Up @@ -933,6 +947,26 @@ describe("mid-thread encrypted agent task recovery (#4089)", () => {
expect(providerBody).not.toContain(FERNET_TASK);
});

test("a mid-thread MESSAGE fails closed without reaching recovery or the routed provider", async () => {
let fetches = 0;
globalThis.fetch = (async () => {
fetches += 1;
throw new Error("MESSAGE ciphertext must not leave the proxy");
}) as typeof fetch;
const input = agentMessage([
{ type: "input_text", text: ROUTING_ENVELOPE.replace("NEW_TASK", "MESSAGE") },
{ type: "encrypted_content", encrypted_content: FERNET_TASK },
]);

const response = await post(routedConfig(), "xai/grok-4.5", input, midThreadHeaders());

expect(response.status).toBe(400);
expect(await response.json()).toMatchObject({
error: { code: "unreadable_encrypted_agent_task", recovery_reason: "unsupported_envelope" },
});
expect(fetches).toBe(0);
});

test("a mid-thread replay reuses the cached plaintext instead of recovering again", async () => {
// The report's third observation: the cache restore sits inside the same gate, so a
// mid-thread turn could never reuse a plaintext this proxy had already paid for.
Expand Down
Loading