Skip to content

Commit 84319e6

Browse files
committed
harden effort-cap gate: strict thread-spawn markers, surface-agnostic child admission, compaction bypass
Audit blockers (sol reviewer, GO-WITH-FIXES): - isSubagentRequest -> isThreadSpawnRequest: exact x-openai-subagent=collab_spawn or subagent_kind=thread_spawn; review/compact/memory_consolidation and other maintenance markers no longer trip subagentEffortCap - effortCapAppliesTo admits header-marked children regardless of tool surface (depth-limited leaves vs shallower children were capped inconsistently) - compaction turns bypass caps so routed compaction matches native /v1/responses/compact which never enters handleResponses
1 parent a971442 commit 84319e6

3 files changed

Lines changed: 94 additions & 27 deletions

File tree

src/server/effort-policy.ts

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,20 +18,25 @@ import { codexEffortRank, configuredReasoningEfforts, isCodexReasoningEffort, mo
1818
import { catalogModelEfforts } from "../codex/catalog";
1919

2020
/**
21-
* True when the request carries codex-rs's spawned-child markers. Source of truth
22-
* (openai/codex @ 6138909d): every collab-spawned child turn sends
21+
* True when the request carries codex-rs's spawned-child markers, matched EXACTLY.
22+
* Source of truth (openai/codex @ 6138909d): every collab-spawned child turn sends
2323
* `x-openai-subagent: collab_spawn` (core/src/responses_metadata.rs) and embeds
2424
* `"subagent_kind":"thread_spawn"` in the JSON `x-codex-turn-metadata` compatibility
2525
* header. Both are checked: the WS bridge rebuilds internal requests from the
2626
* FORWARD_HEADERS allowlist, so either header alone is sufficient evidence.
27+
*
28+
* Exact matching matters: upstream emits `x-openai-subagent` for OTHER internal
29+
* turn categories too (review, compact, memory_consolidation, arbitrary "other"
30+
* sources — responses_metadata.rs subagent_source). Those are maintenance turns,
31+
* not spawned children, and must never trip subagentEffortCap.
2732
*/
28-
export function isSubagentRequest(headers: Headers): boolean {
29-
if (headers.get("x-openai-subagent")) return true;
33+
export function isThreadSpawnRequest(headers: Headers): boolean {
34+
if (headers.get("x-openai-subagent") === "collab_spawn") return true;
3035
const turnMeta = headers.get("x-codex-turn-metadata");
3136
if (!turnMeta) return false;
3237
try {
3338
const parsed = JSON.parse(turnMeta) as { subagent_kind?: unknown };
34-
return typeof parsed.subagent_kind === "string" && parsed.subagent_kind.length > 0;
39+
return parsed.subagent_kind === "thread_spawn";
3540
} catch {
3641
return false;
3742
}
@@ -52,20 +57,26 @@ export function effortCapFor(config: OcxConfig, subagent: boolean): string | und
5257
* Whether the effort caps apply to this turn at all. Caps are a V2-surface feature
5358
* (v1 sub-agents are pinned via explicit spawn args + injectionEffort prompting, so
5459
* the ultra-default leak this module intercepts is v2-specific):
60+
* - compaction turns are maintenance, not agent turns: they bypass caps entirely so
61+
* native /v1/responses/compact (forwarded, never enters handleResponses) and routed
62+
* compaction (synthesized internal request) get identical cap semantics.
5563
* - multiAgentMode "v1" disables caps entirely (mirrors the GUI hiding the panel).
5664
* - a main turn qualifies when its own tool list carries the v2 collab surface.
57-
* - a CHILD turn carries no collab tools (surface null) — its discriminator is the
58-
* spawned-child header pair (isSubagentRequest), so the subagent cap still lands
59-
* on exactly the turns it targets.
60-
* - a v1-surface turn never qualifies.
65+
* - a CHILD turn is admitted by its spawned-child markers (isThreadSpawnRequest)
66+
* REGARDLESS of tool surface: depth-limited leaves carry no collab tools (surface
67+
* null) while children below the spawn-depth limit retain collab tools (spec_plan.rs
68+
* leaf guard), so tool sniffing alone would cap siblings inconsistently.
69+
* - a v1-surface MAIN turn (no child markers) never qualifies.
6170
*/
6271
export function effortCapAppliesTo(
6372
surface: "v1" | "v2" | null,
6473
headers: Headers,
6574
config: OcxConfig,
75+
compaction = false,
6676
): boolean {
77+
if (compaction) return false;
6778
if (config.multiAgentMode === "v1") return false;
68-
return surface === "v2" || (surface === null && isSubagentRequest(headers));
79+
return surface === "v2" || isThreadSpawnRequest(headers);
6980
}
7081

7182
/**
@@ -141,7 +152,7 @@ export function applyEffortCap(
141152
config: OcxConfig,
142153
supported?: readonly string[] | undefined,
143154
): { from: string; to: string; subagent: boolean } | null {
144-
const subagent = isSubagentRequest(headers);
155+
const subagent = isThreadSpawnRequest(headers);
145156
const cap = effortCapFor(config, subagent);
146157
if (!cap) return null;
147158
const resolved = resolveCappedEffort(cap, supported);

src/server/responses.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -476,12 +476,15 @@ export async function handleResponses(
476476
// mock-max clamp so a capped effort is what nativeness clamping then validates; rewrites
477477
// both request shapes (same dual-write contract as the clamp below).
478478
// GATE: v2 feature only (effortCapAppliesTo) — v2-surface main turns plus header-marked
479-
// child turns (children carry no collab tools, so tool sniffing alone would skip the very
480-
// turns subagentEffortCap targets); multiAgentMode "v1" disables caps entirely.
479+
// child turns admitted regardless of tool surface (depth-limited leaves carry no collab
480+
// tools while shallower children do, so tool sniffing alone would cap siblings
481+
// inconsistently); multiAgentMode "v1" disables caps entirely; compaction turns bypass
482+
// caps so routed compaction matches native /v1/responses/compact (which never enters
483+
// handleResponses).
481484
{
482485
const { applyEffortCap, effortCapAppliesTo, supportedLadderFor } = await import("./effort-policy");
483486
const surface = collabSurface(parsed);
484-
if (effortCapAppliesTo(surface, req.headers, config)) {
487+
if (effortCapAppliesTo(surface, req.headers, config, parsed._compactionRequest === true)) {
485488
const capped = applyEffortCap(parsed, req.headers, config, supportedLadderFor(route));
486489
if (capped) {
487490
logCtx.requestedEffort = `${capped.from}->${capped.to}`;

tests/effort-policy.test.ts

Lines changed: 66 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { afterEach, describe, expect, test } from "bun:test";
77
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
88
import { tmpdir } from "node:os";
99
import { join } from "node:path";
10-
import { applyEffortCap, effortCapAppliesTo, effortCapFor, isSubagentRequest, resolveCappedEffort, supportedLadderFor } from "../src/server/effort-policy";
10+
import { applyEffortCap, effortCapAppliesTo, effortCapFor, isThreadSpawnRequest, resolveCappedEffort, supportedLadderFor } from "../src/server/effort-policy";
1111
import { collabSurface } from "../src/server/responses";
1212
import { handleManagementAPI } from "../src/server/management-api";
1313
import { routeModel } from "../src/router";
@@ -51,22 +51,33 @@ const MAIN_HEADERS = new Headers({
5151
"x-codex-turn-metadata": JSON.stringify({ turn_id: "t1" }),
5252
});
5353

54-
describe("isSubagentRequest", () => {
55-
test("x-openai-subagent marker classifies as sub-agent", () => {
56-
expect(isSubagentRequest(SUBAGENT_HEADERS)).toBe(true);
54+
describe("isThreadSpawnRequest", () => {
55+
test("x-openai-subagent: collab_spawn classifies as spawned child", () => {
56+
expect(isThreadSpawnRequest(SUBAGENT_HEADERS)).toBe(true);
5757
});
5858

59-
test("subagent_kind inside x-codex-turn-metadata classifies as sub-agent", () => {
60-
expect(isSubagentRequest(TURN_META_HEADERS)).toBe(true);
59+
test("subagent_kind: thread_spawn inside x-codex-turn-metadata classifies as spawned child", () => {
60+
expect(isThreadSpawnRequest(TURN_META_HEADERS)).toBe(true);
6161
});
6262

6363
test("main-agent turn metadata and bare headers stay main", () => {
64-
expect(isSubagentRequest(MAIN_HEADERS)).toBe(false);
65-
expect(isSubagentRequest(new Headers())).toBe(false);
64+
expect(isThreadSpawnRequest(MAIN_HEADERS)).toBe(false);
65+
expect(isThreadSpawnRequest(new Headers())).toBe(false);
6666
});
6767

68-
test("malformed turn metadata never classifies as sub-agent", () => {
69-
expect(isSubagentRequest(new Headers({ "x-codex-turn-metadata": "{not json" }))).toBe(false);
68+
test("non-spawn subagent categories never classify as spawned children", () => {
69+
// Upstream emits x-openai-subagent for review/compact/memory-consolidation/"other"
70+
// maintenance turns too (responses_metadata.rs) — only collab_spawn is a child.
71+
for (const kind of ["review", "compact", "memory_consolidation", "other", "anything"]) {
72+
expect(isThreadSpawnRequest(new Headers({ "x-openai-subagent": kind }))).toBe(false);
73+
const meta = new Headers({ "x-codex-turn-metadata": JSON.stringify({ subagent_kind: kind }) });
74+
expect(isThreadSpawnRequest(meta)).toBe(false);
75+
}
76+
});
77+
78+
test("malformed turn metadata never classifies as spawned child", () => {
79+
expect(isThreadSpawnRequest(new Headers({ "x-codex-turn-metadata": "{not json" }))).toBe(false);
80+
expect(isThreadSpawnRequest(new Headers({ "x-codex-turn-metadata": JSON.stringify({ subagent_kind: 42 }) }))).toBe(false);
7081
});
7182
});
7283

@@ -320,9 +331,10 @@ describe("effortCapAppliesTo (caps are a v2-feature gate)", () => {
320331
expect(effortCapAppliesTo(null, new Headers(), makeConfig({ effortCap: "high", subagentEffortCap: "medium" }))).toBe(false);
321332
});
322333

323-
test("child turn carries no collab tools — the spawned-child header still admits the cap", () => {
324-
// Regression guard: children never carry spawn_agent, so a surface-only gate would
325-
// skip exactly the turns subagentEffortCap exists for.
334+
test("depth-limited child turn carries no collab tools — the spawned-child header still admits the cap", () => {
335+
// Regression guard: depth-limited leaf children carry no collab tools (surface
336+
// null), so a surface-only gate would skip exactly the turns subagentEffortCap
337+
// exists for.
326338
const parsed = parsedWithTools([{ name: "shell" }], "max");
327339
expect(collabSurface(parsed)).toBeNull();
328340
const config = makeConfig({ subagentEffortCap: "medium" });
@@ -331,6 +343,29 @@ describe("effortCapAppliesTo (caps are a v2-feature gate)", () => {
331343
expect(applyEffortCap(parsed, childHeaders, config)).toEqual({ from: "max", to: "medium", subagent: true });
332344
});
333345

346+
test("header-marked child with a v1 tool surface is still admitted (surface-agnostic child gate)", () => {
347+
// Children below the spawn-depth limit retain collab tools (spec_plan.rs leaf
348+
// guard) — two siblings must not get different cap treatment based on depth.
349+
const childHeaders = new Headers({ "x-openai-subagent": "collab_spawn" });
350+
const config = makeConfig({ subagentEffortCap: "medium" });
351+
expect(effortCapAppliesTo("v1", childHeaders, config)).toBe(true);
352+
expect(effortCapAppliesTo("v2", childHeaders, config)).toBe(true);
353+
});
354+
355+
test("non-spawn subagent markers do not admit the cap", () => {
356+
const config = makeConfig({ effortCap: "high", subagentEffortCap: "medium" });
357+
expect(effortCapAppliesTo(null, new Headers({ "x-openai-subagent": "review" }), config)).toBe(false);
358+
expect(effortCapAppliesTo(null, new Headers({ "x-openai-subagent": "compact" }), config)).toBe(false);
359+
expect(effortCapAppliesTo(null, new Headers({ "x-openai-subagent": "memory_consolidation" }), config)).toBe(false);
360+
const otherMeta = new Headers({ "x-codex-turn-metadata": JSON.stringify({ subagent_kind: "other" }) });
361+
expect(effortCapAppliesTo(null, otherMeta, config)).toBe(false);
362+
});
363+
364+
test("malformed turn metadata never admits the cap", () => {
365+
const config = makeConfig({ effortCap: "high", subagentEffortCap: "medium" });
366+
expect(effortCapAppliesTo(null, new Headers({ "x-codex-turn-metadata": "{not json" }), config)).toBe(false);
367+
});
368+
334369
test("turn-metadata subagent_kind alone admits the cap for a child turn", () => {
335370
const headers = new Headers({ "x-codex-turn-metadata": JSON.stringify({ subagent_kind: "thread_spawn" }) });
336371
expect(effortCapAppliesTo(null, headers, makeConfig({ subagentEffortCap: "medium" }))).toBe(true);
@@ -341,6 +376,24 @@ describe("effortCapAppliesTo (caps are a v2-feature gate)", () => {
341376
expect(effortCapAppliesTo("v2", new Headers(), config)).toBe(false);
342377
expect(effortCapAppliesTo(null, new Headers({ "x-openai-subagent": "collab_spawn" }), config)).toBe(false);
343378
});
379+
380+
test("explicit default and forced-v2 modes keep the gate open for v2 surfaces and marked children", () => {
381+
for (const mode of ["default", "v2"] as const) {
382+
const config = makeConfig({ effortCap: "high", multiAgentMode: mode });
383+
expect(effortCapAppliesTo("v2", new Headers(), config)).toBe(true);
384+
expect(effortCapAppliesTo(null, new Headers({ "x-openai-subagent": "collab_spawn" }), config)).toBe(true);
385+
// A plain main turn stays out even under forced v2 — no collab tools, no markers.
386+
expect(effortCapAppliesTo(null, new Headers(), config)).toBe(false);
387+
}
388+
});
389+
390+
test("compaction turns bypass the cap regardless of surface or markers", () => {
391+
// Native /v1/responses/compact never enters handleResponses; routed compaction
392+
// must not diverge from it, so the gate refuses when the compaction flag is set.
393+
const config = makeConfig({ effortCap: "high", subagentEffortCap: "medium" });
394+
expect(effortCapAppliesTo("v2", new Headers(), config, true)).toBe(false);
395+
expect(effortCapAppliesTo(null, new Headers({ "x-openai-subagent": "collab_spawn" }), config, true)).toBe(false);
396+
});
344397
});
345398

346399
describe("cap composition with downstream clamps", () => {

0 commit comments

Comments
 (0)