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
1 change: 1 addition & 0 deletions scripts/test-layout/layout.json
Original file line number Diff line number Diff line change
Expand Up @@ -1270,6 +1270,7 @@
"responses-account-label.test.ts": "responses",
"responses-canonical-only-top-level-fields.test.ts": "responses",
"responses-compact-handoff-admission.test.ts": "responses",
"responses-compaction-policy-identity.test.ts": "responses",
"responses-compaction-override.test.ts": "responses",
"responses-compaction-routing.test.ts": "responses",
"responses-compaction.test.ts": "responses",
Expand Down
31 changes: 28 additions & 3 deletions src/server/responses/compaction-routing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { isDeclaredReasoningEffort } from "../../reasoning-effort";
import { COMPACTION_TRIGGERS } from "../../config/schema/compaction-triggers";
import { routeConcreteModel, type RouteResult } from "../../router";
import { resolveComboId } from "../../combos/identifiers";
import { resolvePolicyProfileId } from "../../routing/profile";
import { parseSyntheticRowId } from "../fast-row";
import { recallComboForLane } from "./combo-session-recall";
import { sessionLaneIdFromRequest } from "../request-log-conversation";

Expand Down Expand Up @@ -83,7 +85,7 @@ export function applyCompactionRoutingOverride(
if (trigger === undefined) return null;

const sourceModel = raw.model;
const sourceCombo = recallComboForLane(config, sessionLaneIdFromRequest(headers), sourceModel);
const sourceCombo = recallComboForLane(config, sessionLaneIdFromRequest(headers), sourceSelectorOf(config, sourceModel));
const targetCombo = resolveComboId(config, override.model.trim()) ?? undefined;
raw.model = override.model.trim();
if (override.reasoningEffort !== undefined) {
Expand All @@ -92,19 +94,42 @@ export function applyCompactionRoutingOverride(
return { sourceModel, ...(sourceCombo ? { sourceCombo } : {}), ...(targetCombo ? { targetCombo } : {}) };
}

/**
* The selector a synthetic-row grammar actually routed on. `--fast`/`--effort` suffixes are
* decoration applied at ingress; identity checks must see the base id or a decorated
* virtual selector (`alias--fast`) slips past them.
*/
function sourceSelectorOf(config: OcxConfig, sourceModel: string): string {
const { fastRow, effortRow } = parseSyntheticRowId(sourceModel, config);
return fastRow?.baseId ?? effortRow?.baseId ?? sourceModel;
}

/** Same provider identity keeps caller auth and may use native compact; its ciphertext replays only there. */
export function compactionRoutingKeepsProviderIdentity(
config: OcxConfig,
override: CompactionRoutingOverride,
route: RouteResult,
): boolean {
if (route.combo || override.sourceCombo || override.targetCombo || resolveComboId(config, override.sourceModel)) return false;
// `sourceModel` is the selector as the client sent it, so a synthetic `--fast` or
// effort suffix can still be attached. The base id is what the conversation routed on,
// and only the base can match the combo/policy guards below.
const sourceSelector = sourceSelectorOf(config, override.sourceModel);
if (route.combo || override.sourceCombo || override.targetCombo || resolveComboId(config, sourceSelector)) return false;
// A policy selector does not identify one stable serving backend: its route depends on
// request evidence and live candidate state that this post-rewrite check no longer has.
// Treat it as crossing identity rather than reconstructing it through concrete routing,
// which deliberately bypasses policy evaluation and may fall through to defaultProvider.
if (resolvePolicyProfileId(config, sourceSelector) !== null) return false;
let source: RouteResult;
try {
source = routeConcreteModel(config, override.sourceModel);
source = routeConcreteModel(config, sourceSelector);
} catch {
return false;
}
// The default-provider branch is where every unrecognized selector lands — including a
// policy/combo alias that was renamed or deleted since the conversation began. Such a
// selector cannot prove which backend served it, so it can never match an identity.
if (source.routeReason === "default-provider") return false;
return source.providerName === route.providerName
&& source.codexAccountMode === route.codexAccountMode
&& source.codexAccountNamespace === route.codexAccountNamespace;
Expand Down
11 changes: 6 additions & 5 deletions structure/transports/responses.md
Original file line number Diff line number Diff line change
Expand Up @@ -1075,11 +1075,12 @@ combo recall, and do not publish replacement combo/handoff recall. They never ch
conversation's configured model or any compaction request outside the configured triggers.

`compactionRoutingKeepsProviderIdentity` compares the source model's concrete route with the
selected route (provider name, Codex account mode and namespace; combos on either side never
match, and a bare source model the lane remembers as a combo target counts as a combo source,
recorded as `sourceCombo` when the override is applied, and a configured combo target is recorded as
`targetCombo` so its concretely routed children stay portable too). A matching identity keeps the caller's credential and may use the native compact
endpoint. A mismatch marks the credential domain as rewritten, exactly like a shadow
selected route (provider name, Codex account mode and namespace; policy selectors and combos on
either side never match, and a bare source model the lane remembers as a combo target counts as a
combo source, recorded as `sourceCombo` when the override is applied, and a configured combo target
is recorded as `targetCombo` so its concretely routed children stay portable too). A matching
identity keeps the caller's credential and may use the native compact endpoint. A mismatch marks
the credential domain as rewritten, exactly like a shadow
intercept, and forces the portable summarizer even for a native-capable target: `compact.ts`
skips `/responses/compact`, and `request-prepare.ts` sets `parsed._portableCompaction`, which
`request-sidecar-auth.ts` (`routedCompaction`) and the passthrough adapter's compaction body
Expand Down
39 changes: 38 additions & 1 deletion tests/ci-workflows/cold-spawn-warmup.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@ import {
COLD_SPAWN_WARMUP_HOOK_BUDGET_MS,
moduleGraphSpecifiers,
resetColdSpawnWarmupForTests,
spawnModuleGraphWarmupChild,
warmColdSpawn,
warmModuleGraph,
} from "../helpers/cold-spawn-warmup";
import { repoPath, repoRoot } from "../helpers/repo-root";
import { SPAWN_BUDGET_MS } from "../helpers/test-budget";
import { INTERNAL_DEADLINE_MS, SPAWN_BUDGET_MS } from "../helpers/test-budget";
import {
analyzeWarmupRegistration,
dispositionComplaints,
Expand Down Expand Up @@ -264,6 +265,42 @@ describe("warm-up failure policy", () => {
.rejects.toThrow("needs either an entry or a source");
});

test("a warm-up child that never exits is killed at the deadline, not awaited forever", async () => {
resetColdSpawnWarmupForTests();
// Run 35511743422's macos 2/2 leg held this shape for eighteen silent minutes: a child
// that could not be observed to exit, waited on through a synchronous spawn whose own
// timeout rode the dead event loop. The bound has to live on the parent's live loop —
// SIGKILL at the deadline, then settle.
const startedAt = performance.now();
const result = await spawnModuleGraphWarmupChild(
"setInterval(() => undefined, 60_000)",
repoRoot(),
undefined,
1_000,
);
expect(performance.now() - startedAt).toBeLessThan(INTERNAL_DEADLINE_MS);
expect(result.timedOut).toBe(true);
expect(result.exitCode).not.toBe(0);
});

test("a descendant holding the child's pipes does not turn exit into a wait for EOF", async () => {
resetColdSpawnWarmupForTests();
// `close` is what a clean exit earns. A grandchild that keeps the write end open must not
// convert it into an unbounded wait, so exit starts a reap grace instead.
const script = [
'const { spawn } = require("node:child_process");',
'spawn(process.execPath, ["--eval", "setTimeout(() => process.exit(0), 8_000)"], { detached: true, stdio: "inherit" }).unref();',
'process.stdout.write("ok\\n");',
"process.exit(0);",
].join("\n");
const startedAt = performance.now();
const result = await spawnModuleGraphWarmupChild(script, repoRoot(), undefined, INTERNAL_DEADLINE_MS);
expect(performance.now() - startedAt).toBeLessThan(INTERNAL_DEADLINE_MS);
expect(result.exitCode).toBe(0);
expect(result.timedOut).toBe(false);
expect(result.stdout).toContain("ok");
});

test("a real module graph loads, and reports what it loaded", async () => {
resetColdSpawnWarmupForTests();
// The end-to-end path: scan a child source, spawn one Bun child, import what it named, exit.
Expand Down
1 change: 1 addition & 0 deletions tests/fixtures/test-layout-expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -1097,6 +1097,7 @@
"responses-bare-echo-helper-fence.test.ts": "responses",
"responses-canonical-only-top-level-fields.test.ts": "responses",
"responses-compact-handoff-admission.test.ts": "responses",
"responses-compaction-policy-identity.test.ts": "responses",
"responses-compaction-override.test.ts": "responses",
"responses-compaction-routing.test.ts": "responses",
"responses-compaction.test.ts": "responses",
Expand Down
130 changes: 120 additions & 10 deletions tests/helpers/cold-spawn-warmup.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { spawn } from "node:child_process";
import { readFileSync } from "node:fs";
import { dirname, isAbsolute, resolve } from "node:path";
import { repoRoot } from "./repo-root";
Expand Down Expand Up @@ -201,7 +202,111 @@ export async function warmModuleGraph(options: ColdSpawnWarmup): Promise<void> {
return warmColdSpawn(options.graph, deadlineMs => runModuleGraphWarmup(options, deadlineMs));
}

function runModuleGraphWarmup(options: ColdSpawnWarmup, deadlineMs: number): void {
export interface ModuleGraphWarmupResult {
stdout: string;
stderr: string;
exitCode: number | null;
signal: NodeJS.Signals | null;
timedOut: boolean;
}

/**
* Spawn the warm-up child asynchronously and bound it on a live event loop.
*
* A blocking `Bun.spawnSync` made its own `timeout` the only bound it could honour, and that
* turned out to be no bound at all: while the synchronous wait runs, the event loop is dead, so
* the calling hook's budget and the suite's per-test timeout freeze inside the same wait and
* nothing can report anything. Run 35511743422's macos 2/2 leg held that shape for eighteen
* silent minutes inside tests/clients/client-connect.test.ts before the job ceiling cut it and
* reported `cancelled` — a result the `ci` gate reads as failure rather than evidence. Whether
* the child or the spawn primitive wedged is not observable from the outside, so the bound here
* does not depend on either: SIGKILL at the deadline, a short reap grace, and the call settles
* with or without the child's exit or EOF. A child that outlives its kill — or a descendant
* holding its pipes — cannot turn a warm-up into an unbounded wait.
*/
export function spawnModuleGraphWarmupChild(
script: string,
cwd: string,
env: Record<string, string | undefined> | undefined,
deadlineMs: number,
): Promise<ModuleGraphWarmupResult> {
const maxCaptureBytes = 1024 * 1024;
return new Promise((resolve, reject) => {
let child: ReturnType<typeof spawn>;
try {
child = spawn(process.execPath, ["--eval", script], {
cwd,
env: { ...process.env, ...env },
stdio: ["ignore", "pipe", "pipe"],
});
} catch {
reject(new Error("[cold-spawn-warmup] the warm-up child could not be spawned"));
return;
}
const stdoutChunks: Buffer[] = [];
const stderrChunks: Buffer[] = [];
let bytes = 0;
let settled = false;
let timedOut = false;
let exitCode: number | null = null;
let signal: NodeJS.Signals | null = null;
let deadline: ReturnType<typeof setTimeout> | undefined;
let reap: ReturnType<typeof setTimeout> | undefined;
const finish = () => {
if (settled) return;
settled = true;
clearTimeout(deadline);
clearTimeout(reap);
child.stdout?.destroy();
child.stderr?.destroy();
child.unref();
resolve({
stdout: Buffer.concat(stdoutChunks).toString("utf8"),
stderr: Buffer.concat(stderrChunks).toString("utf8"),
exitCode,
signal,
timedOut,
});
};
const beginReapGrace = () => {
if (settled) return;
reap ??= setTimeout(finish, WARMUP_REAP_RESERVE_MS);
};
const stop = () => {
if (settled || timedOut) return;
timedOut = true;
clearTimeout(deadline);
beginReapGrace();
try { child.kill("SIGKILL"); } catch { /* The kill's own failure must not extend the wait. */ }
};
const capture = (chunk: Buffer, into: Buffer[]) => {
if (settled || timedOut) return;
bytes += chunk.length;
if (bytes > maxCaptureBytes) { stop(); return; }
into.push(chunk);
};
child.stdout?.on("data", (chunk: Buffer) => capture(chunk, stdoutChunks));
child.stderr?.on("data", (chunk: Buffer) => capture(chunk, stderrChunks));
child.stdout?.on("error", stop);
child.stderr?.on("error", stop);
// The child was never started or died at launch; there is nothing to reap.
child.on("error", finish);
child.once("exit", (code, exitSignal) => {
exitCode = code;
signal = exitSignal;
// A descendant retaining a pipe must not turn a clean exit into a wait for EOF.
beginReapGrace();
});
child.once("close", (code, exitSignal) => {
exitCode = code;
signal = exitSignal;
finish();
});
deadline = setTimeout(stop, deadlineMs);
});
}

async function runModuleGraphWarmup(options: ColdSpawnWarmup, deadlineMs: number): Promise<void> {
const cwd = options.cwd ?? repoRoot();
const source = options.source ?? readFileSync(requireEntry(options), "utf8");
const resolveDir = options.entry === undefined ? cwd : dirname(options.entry);
Expand All @@ -214,21 +319,26 @@ function runModuleGraphWarmup(options: ColdSpawnWarmup, deadlineMs: number): voi
}

const startedAt = performance.now();
const result = Bun.spawnSync([process.execPath, "--eval", warmupScript(specifiers, deadlineMs)], {
const result = await spawnModuleGraphWarmupChild(
warmupScript(specifiers, deadlineMs),
cwd,
env: { ...process.env, ...options.env },
stdout: "pipe",
stderr: "pipe",
timeout: deadlineMs,
});
options.env,
deadlineMs,
);
const elapsedMs = (performance.now() - startedAt).toFixed(0);
const stdout = result.stdout.toString();
const report = parseWarmupReport(stdout);
const report = parseWarmupReport(result.stdout);
if (result.timedOut) {
throw new Error(
`[cold-spawn-warmup] graph=${options.graph} warm-up child did not exit within ${deadlineMs}ms `
+ `and was killed (specifiers=${specifiers.length}). `
+ `stderr: ${result.stderr.trim().slice(0, 600)}`,
);
}
if (result.exitCode !== 0 || report === undefined || report.loaded === 0) {
throw new Error(
`[cold-spawn-warmup] graph=${options.graph} loaded nothing in ${elapsedMs}ms `
+ `(exitCode=${String(result.exitCode)}, specifiers=${specifiers.length}). `
+ `stderr: ${result.stderr.toString().trim().slice(0, 600)}`,
+ `stderr: ${result.stderr.trim().slice(0, 600)}`,
);
}
console.log(
Expand Down
88 changes: 88 additions & 0 deletions tests/responses/responses-compaction-policy-identity.test.ts

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔍 Required full-suite validation remains incomplete

Review readiness requires bun run test before approval. The PR reports that run was interrupted, so exact-head readiness remains unverified.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import { describe, expect, test } from "bun:test";
import { getDefaultConfig } from "../../src/config";
import { routeConcreteModel } from "../../src/router";
import { compactionRoutingKeepsProviderIdentity } from "../../src/server/responses/compaction-routing";
import type { OcxConfig } from "../../src/types";

function policyConfig(): OcxConfig {
return {
...getDefaultConfig(),
defaultProvider: "openai-apikey",
providers: {
openai: {
adapter: "openai-responses", authMode: "forward",
baseUrl: "https://chatgpt.com/backend-api/codex",
},
"openai-apikey": {
adapter: "openai-responses", authMode: "key", apiKey: "fixture-key",
baseUrl: "https://api.openai.com/v1",
},
},
routingProfiles: {
primary: {
alias: "ocx/primary",
candidates: [{ provider: "openai", model: "gpt-5.6-luna" }],
},
},
};
}

describe("compaction routing policy identity", () => {
test.each(["policy/primary", "ocx/primary"])("treats policy source %s as cross-identity", sourceModel => {
const config = policyConfig();
const target = routeConcreteModel(config, "openai-apikey/gpt-5.6-luna");

expect(compactionRoutingKeepsProviderIdentity(config, { sourceModel }, target)).toBe(false);
});

test.each(["policy/primary--fast", "ocx/primary--fast"])(
"treats synthetic policy selector %s as cross-identity",
sourceModel => {
const config = policyConfig();
const target = routeConcreteModel(config, "openai-apikey/gpt-5.6-luna");

expect(compactionRoutingKeepsProviderIdentity(config, { sourceModel }, target)).toBe(false);
},
);

test("treats a stale policy alias as cross-identity after the profile is deleted", () => {
const config = policyConfig();
delete config.routingProfiles;
const target = routeConcreteModel(config, "openai-apikey/gpt-5.6-luna");

expect(compactionRoutingKeepsProviderIdentity(config, { sourceModel: "ocx/primary" }, target)).toBe(false);
});

test("fails closed for a selector that only resolves through the default provider", () => {
const config = policyConfig();
const target = routeConcreteModel(config, "openai-apikey/gpt-5.6-luna");

expect(compactionRoutingKeepsProviderIdentity(
config,
{ sourceModel: "unconfigured-model" },
target,
)).toBe(false);
});

test("retains identity for a concrete source on the target provider", () => {
const config = policyConfig();
const target = routeConcreteModel(config, "openai-apikey/gpt-5.6-luna");

expect(compactionRoutingKeepsProviderIdentity(
config,
{ sourceModel: "openai-apikey/gpt-6-astra" },
target,
)).toBe(true);
});

test("retains identity for a concrete fast selector on the target provider", () => {
const config = policyConfig();
const target = routeConcreteModel(config, "openai-apikey/gpt-5.6-luna");

expect(compactionRoutingKeepsProviderIdentity(
config,
{ sourceModel: "openai-apikey/gpt-6-astra--fast" },
target,
)).toBe(true);
});
});
Loading