Skip to content

fix(sse): cap streaming headers-wait timeout to a client-realistic ceiling (#11526) - #11668

Merged
diegosouzapw merged 1 commit into
release/v3.8.51from
fix/11526-responses-header-timeout
Aug 26, 2026
Merged

diegosouzapw merged 1 commit into
release/v3.8.51from
fix/11526-responses-header-timeout

Conversation

@diegosouzapw

Copy link
Copy Markdown
Collaborator

Closes #11526

⚠️ base-red inherited: #11449 — ESLint errors (5), docs-sync migrations count (both pre-existing on release/v3.8.51, not introduced by this diff)

Root cause

Two watchdogs govern a stalled upstream on the Responses->Chat translated path, and they were asymmetric:

  1. Body-phase (resolveStreamReadinessTimeout, open-sse/utils/streamReadinessPolicy.ts) — fires after headers arrive, tool-heavy/large-history adaptive. For the reported payload shape it computes well under Codex's own ~120s client-abort patience.
  2. Headers-phase (fetchStartTimeoutMs in BaseExecutor.execute(), backed by FETCH_TIMEOUT_MS, default 600_000ms) — governs the wait before any response arrives at all, flat and non-adaptive.

The issue's own log field hasResponseBody: false shows watchdog #1 never engaged — the upstream (an NVIDIA target) never returned a response at all. The only ceiling left was the 10-minute default, 5x longer than a real client's patience, so OmniRoute kept the connection alive on keepalives only, guaranteeing the client aborted first with an opaque 499 instead of OmniRoute detecting and failing the stall fast.

Fix

Adds resolveFetchStartTimeout() (open-sse/utils/fetchStartTimeoutPolicy.ts) that caps the headers-wait timeout to 110s (under Codex's ~120s abort window) for streaming requests only — non-streaming requests keep the existing flat FETCH_TIMEOUT_MS default unchanged, per the plan-file's risk note (some providers are legitimately slow to accept a connection on non-streaming paths). Wired into BaseExecutor.execute() at the one call site that computes fetchStartTimeoutMs.

The existing TimeoutError classification path (chatCore.ts) already maps a fetch-start timeout to HTTP 504 (GATEWAY_TIMEOUT), which is in the circuit-breaker-eligible {408,500,502,503,504} set per AGENTS.md — confirmed by reading the code, no change needed there.

Scope note

This PR addresses the confirmed, in-our-code, fixable gap (the headers-phase timeout asymmetry). The plan-file's secondary hardening item — truncating the oversized (14KB) flattened tool_search tool description for non-Codex-native chat-completions upstreams — is explicitly deferred pending reporter/VPS confirmation that it's the actual NVIDIA-side trigger, so Closes is used because this PR is the full fix for the reported stall-then-499 symptom (the headers-phase ceiling), while that separate follow-up hardening remains open as a distinct, unconfirmed item.

Testing (TDD, Hard Rule #18)

tests/unit/issue-11526-repro.test.ts — new regression test:

  • confirms the body-phase watchdog is fine (stays under Codex's ~120s patience) for the reported payload shape
  • confirms the headers-phase watchdog for streaming requests is now bounded under 120s (would fail on unfixed code — resolveFetchStartTimeout didn't exist)
  • scope guards: non-streaming requests keep the flat FETCH_TIMEOUT_MS default; a base timeout already under the cap is left untouched
✔ issue #11526: body-phase readiness watchdog stays comfortably under Codex's ~120s patience for the reported tool-heavy payload shape
✔ issue #11526 (fixed): headers-phase watchdog for STREAMING requests is bounded under Codex's ~120s patience
✔ issue #11526 scope guard: non-streaming requests keep the flat FETCH_TIMEOUT_MS default
✔ issue #11526 scope guard: a base timeout already under the cap is left untouched for streaming requests
ℹ tests 4, pass 4, fail 0

Also re-ran the existing suites touching the changed symbols:

  • tests/unit/stream-readiness-policy.test.ts + tests/unit/runtime-timeouts.test.ts — 33/33 pass
  • tests/unit/executor-default-base.test.ts (BaseExecutor.execute / DefaultExecutor, incl. "BaseExecutor.execute clears the startup timeout after headers arrive") — 51/51 pass

Gates run

  • node scripts/check/check-file-size.mjs — OK
  • node scripts/check/check-complexity.mjs — OK (2684 violations, baseline 2774)
  • node scripts/check/check-cognitive-complexity.mjs — OK (1208 violations, baseline 1223)
  • node scripts/check/check-changelog-integrity.mjs — OK
  • npm run typecheck:core — clean
  • npx eslint --suppressions-location config/quality/eslint-suppressions.json <changed files> — clean, 0 errors/warnings

Changelog

changelog.d/fixes/11526-responses-headers-phase-timeout-cap.md

…iling (#11526)

The fetch-start (headers-wait) phase inherited the flat, non-adaptive
FETCH_TIMEOUT_MS (default 600_000ms) with no ceiling comparable to a real
client's patience, unlike the body-phase readiness watchdog which already
adapts per payload shape. Codex's own hard client-abort window for a
stalled turn is ~120s, 5x shorter than the old default — so when an
upstream never returned any response at all (not even headers, e.g. a
stalled NVIDIA target behind a tool-heavy Responses->Chat translation),
OmniRoute kept the connection open on keepalives only, guaranteeing the
client gave up first with an opaque 499 instead of OmniRoute detecting and
failing the stall fast.

Adds resolveFetchStartTimeout() (open-sse/utils/fetchStartTimeoutPolicy.ts)
that caps the headers-wait timeout to 110s for STREAMING requests only,
leaving non-streaming requests on the existing flat default. Wired into
BaseExecutor.execute(). The existing TimeoutError classification path
(chatCore.ts) already maps this to a 504, so no change was needed there.

Regression test: tests/unit/issue-11526-repro.test.ts.
@diegosouzapw

Copy link
Copy Markdown
Collaborator Author

Validated in local merge-train (train tip a3a96af7308584900f0fab3ab4afd8499391230a, boarded #11663 #11664 #11665 #11667 #11668 #11669 #11670 #11678 on .113 /srv/omniroute-train) — typecheck:core, file-size, complexity, cognitive-complexity, changelog-integrity all GREEN. npm run test:unit showed 21 failures across 15 test files, none touching this PR's diff; reproduced identically (same test names, same asserts) on origin/release/v3.8.51 alone with zero PRs boarded (targeted re-run, 21/94 fail, same failure set) — confirmed base-red/environment-load drift per merge-gates.md §3, not introduced by any of the 8 boarded PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Responses->Chat: turn stalls with 120s keepalive then client 499 abort after tool_search is flattened (NVIDIA upstreams)

2 participants