diff --git a/changelog.d/fixes/10310-codex-quota-header-budget.md b/changelog.d/fixes/10310-codex-quota-header-budget.md new file mode 100644 index 0000000000..c24d6fda3f --- /dev/null +++ b/changelog.d/fixes/10310-codex-quota-header-budget.md @@ -0,0 +1 @@ +- fix(sse): prioritize Codex quota headers (x-codex-*) in the 768-byte forwarded-header budget (#10310) diff --git a/changelog.d/fixes/10315-header-budget-warn-dedup.md b/changelog.d/fixes/10315-header-budget-warn-dedup.md new file mode 100644 index 0000000000..21632cd39c --- /dev/null +++ b/changelog.d/fixes/10315-header-budget-warn-dedup.md @@ -0,0 +1 @@ +- fix(sse): dedupe forwarded-header drop warns by dropped-name fingerprint (warn once, then debug) (#10315) diff --git a/open-sse/handlers/chatCore/responseHeaders.ts b/open-sse/handlers/chatCore/responseHeaders.ts index 43fdc5a88e..b4440e6697 100644 --- a/open-sse/handlers/chatCore/responseHeaders.ts +++ b/open-sse/handlers/chatCore/responseHeaders.ts @@ -54,8 +54,70 @@ export const MAX_FORWARDED_UPSTREAM_RESPONSE_HEADER_BYTES = resolveForwardedHead const MAX_LOGGED_DROPPED_RESPONSE_HEADERS = 20; const responseHeaderEncoder = new TextEncoder(); +// Warn-once-per-dropped-name-set frequency control for the drop-warning path +// (#10315). The module-level set persists for the process lifetime (and across +// test cases in one process), so a budget/config change that flips which names +// drop yields a new fingerprint and warns again — intended. +const warnedDropFingerprints = new Set(); + +/** + * Stable identity for a dropped-header set, using only header NAMES (not values/ + * bytes) so two payloads dropping the SAME names share one warn. Sorted so the + * identification is order-independent. + */ +function droppedHeadersFingerprint(dropped: Array<{ name: string }>): string { + return dropped + .map((h) => h.name) + .sort() + .join("\n"); +} + +/** + * Test-only isolation helper. The fingerprint cache persists in this process; + * tests that reuse a dropped-set fingerprint must clear it to keep cases + * order-independent. Never used in production paths. + */ +export function resetDroppedHeadersWarningCache(): void { + warnedDropFingerprints.clear(); +} + +/** + * Emit the drop-warning path for headers that exceeded the forwarding budget. + * Warns once per process per dropped-name set, then degrades to debug for + * repeats so a chronic over-budget response set cannot become a warn storm + * that buries real errors (see regression guard #10315). + */ +function logDroppedResponseHeaders( + droppedHeaders: Array<{ name: string; bytes: number }>, + forwardedBytes: number, + log: ResponseHeaderLogger +): void { + if (droppedHeaders.length === 0) return; + const fingerprint = droppedHeadersFingerprint(droppedHeaders); + if (!warnedDropFingerprints.has(fingerprint)) { + warnedDropFingerprints.add(fingerprint); + log?.warn?.("HTTP", "Dropped upstream response headers that exceeded forwarding budget", { + budgetBytes: MAX_FORWARDED_UPSTREAM_RESPONSE_HEADER_BYTES, + forwardedBytes, + droppedCount: droppedHeaders.length, + droppedHeaders: droppedHeaders.slice(0, MAX_LOGGED_DROPPED_RESPONSE_HEADERS), + }); + } else { + log?.debug?.( + "HTTP", + "Dropped upstream response headers exceeded forwarding budget (repeated; see first warn for header list)", + { + budgetBytes: MAX_FORWARDED_UPSTREAM_RESPONSE_HEADER_BYTES, + forwardedBytes, + droppedCount: droppedHeaders.length, + } + ); + } +} + type ResponseHeaderLogger = { warn?: (tag: string, message: string, data?: Record) => void; + debug?: (tag: string, message: string, data?: Record) => void; } | null; function responseHeaderWireBytes(name: string, value: string): number { @@ -66,6 +128,36 @@ function isOmniRouteInternalHeader(headerName: string): boolean { return headerName.toLowerCase().startsWith("x-omniroute-"); } +/** + * Codex quota vocabulary (`x-codex-primary/secondary-* used/reset`, + * `x-codex-credits-*`) carries usage/limit/reset data the client needs. Treat + * it as the same priority class as rate-limit headers so a tight forwarding + * budget never silently strips it (#10310). + */ +function isCodexQuotaHeader(normalized: string): boolean { + return ( + normalized.startsWith("x-codex-") && + (normalized.includes("used") || normalized.includes("reset") || normalized.includes("credits")) + ); +} + +/** + * Known bulky, non-quota response headers (Cloudflare edge family, Codex turn + * state, CSP, `date`, etc.) that can be tens-to-hundreds of bytes. They are + * assigned the LAST priority tier so they are the first dropped when the budget + * is tight, rather than evicting more valuable quota/rate-limit data. + */ +function isForcedLastPriorityHeader(normalized: string): boolean { + return ( + normalized.startsWith("cf-") || + normalized === "x-codex-turn-state" || + normalized === "fireworks-sampling-options" || + normalized === "content-security-policy" || + normalized === "date" || + normalized === "x-robots-tag" + ); +} + function getForwardingPriority(headerName: string): number { const normalized = headerName.toLowerCase(); if ( @@ -78,7 +170,14 @@ function getForwardingPriority(headerName: string): number { return 0; } if (normalized === "retry-after") return 1; - if (normalized.includes("ratelimit") || normalized.includes("rate-limit")) return 2; + if ( + normalized.includes("ratelimit") || + normalized.includes("rate-limit") || + isCodexQuotaHeader(normalized) + ) { + return 2; + } + if (isForcedLastPriorityHeader(normalized)) return 4; return 3; } @@ -181,14 +280,7 @@ export function buildStreamingResponseHeaders( } } - if (droppedHeaders.length > 0) { - log?.warn?.("HTTP", "Dropped upstream response headers that exceeded forwarding budget", { - budgetBytes: MAX_FORWARDED_UPSTREAM_RESPONSE_HEADER_BYTES, - forwardedBytes, - droppedCount: droppedHeaders.length, - droppedHeaders: droppedHeaders.slice(0, MAX_LOGGED_DROPPED_RESPONSE_HEADERS), - }); - } + logDroppedResponseHeaders(droppedHeaders, forwardedBytes, log); const responseHeaders: Record = { ...Object.fromEntries(forwardedHeaders), diff --git a/tests/unit/chatcore-header-budget-codex-quota.test.ts b/tests/unit/chatcore-header-budget-codex-quota.test.ts new file mode 100644 index 0000000000..236152ab49 --- /dev/null +++ b/tests/unit/chatcore-header-budget-codex-quota.test.ts @@ -0,0 +1,121 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; + +const { buildStreamingResponseHeaders } = await import( + "@omniroute/open-sse/handlers/chatCore/responseHeaders.ts" +); + +/** + * #10310 regression guard — Codex quota headers must survive the forwarding budget. + * + * Root cause: `getForwardingPriority` only classifies headers containing + * "ratelimit"/"rate-limit" as high-priority. The entire Codex quota vocabulary + * (`x-codex-primary/secondary-* used/reset`, `x-codex-credits-*`) fell to the + * lowest priority tier, tied against bulky CDN/security noise. Because + * `Headers.forEach` iterates in byte-sorted alphabetical order, a realistic + * multi-header Codex+CDN response exhausted the 768-byte budget on alphabetically- + * earlier noise before reaching any `x-codex-*` quota header. + * + * Fix: promote Codex quota headers to the rate-limit priority class and push + * known bulky noise (cf-*, x-codex-turn-state, firewall-sampling-options, ...) + * to a forced-last tier so they never evict quota data. + */ +const CODEX_QUOTA_HEADERS = [ + "x-codex-primary-used-percent", + "x-codex-primary-reset-after-seconds", + "x-codex-secondary-used-percent", + "x-codex-secondary-reset-after-seconds", + "x-codex-credits-used", + "x-codex-credits-remaining", +]; + +const NOISE_HEADERS = [ + "x-codex-turn-state", + "fireworks-sampling-options", + "cf-ray", + "cf-cache-status", + "content-security-policy", +]; + +function buildUpstreamHeaders(): Headers { + return new Headers({ + "x-request-id": "b6f1c2a4-7e3d-4a1b-9c2e-1234567890ab", + "anthropic-ratelimit-unified-requests-limit": "5000", + "anthropic-ratelimit-unified-requests-remaining": "4998", + "anthropic-ratelimit-unified-reset": "2026-08-14T06:00:00Z", + "anthropic-organization-id": "org-abc123def456ghi789", + "alt-svc": 'h3=":443"; ma=86400', + "cf-cache-status": "DYNAMIC", + "cf-ray": "89abcdef1234ffff-EWR", + "content-security-policy": + "default-src 'self'; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; connect-src 'self'", + "cross-origin-embedder-policy": "require-corp", + "cross-origin-opener-policy": "same-origin", + "cross-origin-resource-policy": "same-origin", + date: "Fri, 14 Aug 2026 06:00:00 GMT", + "fireworks-sampling-options": "x".repeat(340), + nel: '{"report_to":"default","max_age":31536000}', + "permissions-policy": "geolocation=(), microphone=(), camera=()", + "referrer-policy": "strict-origin-when-cross-origin", + "report-to": + '{"group":"default","max_age":31536000,"endpoints":[{"url":"https://a.example.com/r"}]}', + "server-timing": "cf-q-config;dur=1.0000002656e-05", + "strict-transport-security": "max-age=31536000; includeSubDomains; preload", + "timing-allow-origin": "*", + vary: "Accept-Encoding, Origin", + "x-codex-turn-state": "y".repeat(300), + "x-codex-primary-used-percent": "42.5", + "x-codex-primary-reset-after-seconds": "1800", + "x-codex-secondary-used-percent": "10.2", + "x-codex-secondary-reset-after-seconds": "86400", + "x-codex-credits-used": "1234", + "x-codex-credits-remaining": "5678", + "x-content-type-options": "nosniff", + "x-frame-options": "DENY", + "x-robots-tag": "noindex", + "x-xss-protection": "0", + }); +} + +function getHeaderValue(headers: Record, name: string): string | undefined { + const entry = Object.entries(headers).find(([key]) => key.toLowerCase() === name.toLowerCase()); + return entry?.[1]; +} + +test("#10310: Codex quota/reset/credits headers survive the forwarding budget", () => { + const result = buildStreamingResponseHeaders( + buildUpstreamHeaders(), + { provider: "codex", model: "gpt-5-codex", cacheHit: false, latencyMs: 0, usage: null, costUsd: 0 }, + null + ); + + const missing = CODEX_QUOTA_HEADERS.filter((name) => !(name in result)); + assert.deepEqual( + missing, + [], + `Codex quota headers were dropped by the forwarding budget: ${missing.join(", ")}` + ); +}); + +test("#10310: bulky non-quota noise is dropped instead of evicting quota headers", () => { + const result = buildStreamingResponseHeaders( + buildUpstreamHeaders(), + { provider: "codex", model: "gpt-5-codex", cacheHit: false, latencyMs: 0, usage: null, costUsd: 0 }, + null + ); + + for (const name of CODEX_QUOTA_HEADERS) { + assert.ok(name in result, `${name} must be forwarded`); + } + // Anthropic rate-limit class must remain intact after reprioritization. + const anthropicReset = getHeaderValue(result, "anthropic-ratelimit-unified-reset"); + assert.ok( + anthropicReset && anthropicReset === "2026-08-14T06:00:00Z", + "anthropic-ratelimit-unified-reset must survive" + ); + // Known bulky noise may be dropped when the budget is tight. + const confinedToNoise = NOISE_HEADERS.every( + (name) => !(Object.keys(result).some((key) => key.toLowerCase() === name.toLowerCase())) + ); + assert.ok(confinedToNoise, "noise headers should be the ones dropped, not quota"); +}); \ No newline at end of file diff --git a/tests/unit/forwarded-header-budget-dedup.test.ts b/tests/unit/forwarded-header-budget-dedup.test.ts new file mode 100644 index 0000000000..c58af4dff6 --- /dev/null +++ b/tests/unit/forwarded-header-budget-dedup.test.ts @@ -0,0 +1,77 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; + +const { + buildStreamingResponseHeaders, + resetDroppedHeadersWarningCache, +} = await import("@omniroute/open-sse/handlers/chatCore/responseHeaders.ts"); + +/** + * #10315 regression guard — warn storm on the forwarded-header drop path. + * + * Root cause: `buildStreamingResponseHeaders` unconditionally emits a structured + * `warn` (up to 20 {name,bytes} entries) on EVERY response that drops any header + * past the forwarding budget. No dedupe/sample. Under multi-stream Desktop flows + * a chronic over-budget response set buries real errors and adds serialize/log + * I/O per response. + * + * Fix: warn once per process per sorted-dropped-name fingerprint, then degrade + * to `debug` for repeats of the same dropped set. Distinct dropped sets still + * each warn once. + */ +function makeLog() { + const warns: unknown[][] = []; + const debugs: unknown[][] = []; + return { + log: { + warn: (...args: unknown[]) => warns.push(args), + debug: (...args: unknown[]) => debugs.push(args), + }, + warns, + debugs, + }; +} + +function oversizedSet(prefix: string): Headers { + const headers = new Headers({ "x-request-id": `req-${prefix}` }); + for (let index = 0; index < 24; index += 1) { + headers.set(`${prefix}-${index.toString().padStart(2, "0")}`, "x".repeat(69)); + } + return headers; +} + +test("#10315: 100 identical oversized responses produce exactly 1 warn then debug", () => { + resetDroppedHeadersWarningCache(); + const { log, warns, debugs } = makeLog(); + const oversized = oversizedSet("x-big-header"); + for (let index = 0; index < 100; index += 1) { + buildStreamingResponseHeaders(oversized, { provider: "codex", model: "gpt-5-codex" }, log); + } + // Only the first occurrence of this dropped-name set may warn. + assert.equal( + warns.length, + 1, + "expected exactly 1 warn across 100 identical drops, got " + warns.length + ); + // Every subsequent identical drop must be a debug (or at least not a warn). + assert.ok( + debugs.length >= 99, + "expected repeats to degrade to debug, got " + debugs.length + " debug entries" + ); +}); + +test("#10315: two distinct dropped sets each warn once even when repeated", () => { + resetDroppedHeadersWarningCache(); + const { log, warns } = makeLog(); + const setA = oversizedSet("x-big-header-a"); + const setB = oversizedSet("x-big-header-b"); + for (let index = 0; index < 2; index += 1) { + buildStreamingResponseHeaders(setA, { provider: "codex", model: "gpt-5-codex" }, log); + buildStreamingResponseHeaders(setB, { provider: "codex", model: "gpt-5-codex" }, log); + } + assert.equal( + warns.length, + 2, + "expected 1 warn per distinct dropped set (A and B), got " + warns.length + ); +}); \ No newline at end of file