diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 41daa0cd488..60c0b8f6716 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -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", diff --git a/src/server/responses/compaction-routing.ts b/src/server/responses/compaction-routing.ts index a7d50962db5..4660d636a51 100644 --- a/src/server/responses/compaction-routing.ts +++ b/src/server/responses/compaction-routing.ts @@ -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"; @@ -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) { @@ -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; diff --git a/structure/transports/responses.md b/structure/transports/responses.md index b1e28958fa1..0f31381699d 100644 --- a/structure/transports/responses.md +++ b/structure/transports/responses.md @@ -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 diff --git a/tests/ci-workflows/cold-spawn-warmup.test.ts b/tests/ci-workflows/cold-spawn-warmup.test.ts index 29e86fdc944..7df6eb92181 100644 --- a/tests/ci-workflows/cold-spawn-warmup.test.ts +++ b/tests/ci-workflows/cold-spawn-warmup.test.ts @@ -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, @@ -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. diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index a7a7fcee72f..5d5228dcff2 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -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", diff --git a/tests/helpers/cold-spawn-warmup.ts b/tests/helpers/cold-spawn-warmup.ts index cc8b5289c78..aada38df630 100644 --- a/tests/helpers/cold-spawn-warmup.ts +++ b/tests/helpers/cold-spawn-warmup.ts @@ -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"; @@ -201,7 +202,111 @@ export async function warmModuleGraph(options: ColdSpawnWarmup): Promise { 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 | undefined, + deadlineMs: number, +): Promise { + const maxCaptureBytes = 1024 * 1024; + return new Promise((resolve, reject) => { + let child: ReturnType; + 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 | undefined; + let reap: ReturnType | 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 { const cwd = options.cwd ?? repoRoot(); const source = options.source ?? readFileSync(requireEntry(options), "utf8"); const resolveDir = options.entry === undefined ? cwd : dirname(options.entry); @@ -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( diff --git a/tests/responses/responses-compaction-policy-identity.test.ts b/tests/responses/responses-compaction-policy-identity.test.ts new file mode 100644 index 00000000000..36f1a029e2d --- /dev/null +++ b/tests/responses/responses-compaction-policy-identity.test.ts @@ -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); + }); +});