Skip to content

fix(sse): price the default keepalive threshold below the client watchdog and stop leaking Anthropic finish reasons - #14247

Merged
diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.51from
ishan-parihar:fix/combo-streaming-watchdog
Sep 22, 2026
Merged

diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.51from
ishan-parihar:fix/combo-streaming-watchdog

Conversation

@ishan-parihar

Copy link
Copy Markdown
Contributor

Summary

Two independent faults made combo streaming unusable from a strict OpenAI-compatible client — oh-my-pi (omp), which had previously worked. Both are observable from the client side as the same symptom (a combo that looks like it "does not stream"), so they are fixed together.

1. The default keepalive threshold raced the client's first-byte watchdog

resolveKeepaliveThreshold() returns the delay after which withEarlyStreamKeepalive stops waiting for the handler and commits to a 200 text/event-stream response — whose first byte is the synthetic keepalive frame. That first byte is the client's time-to-first-byte, so for the default tier the threshold is a hard latency budget, not a tuning knob.

Every combo reaches the resolver as a bare name with no provider prefix (small-stack, paper-stack, free-stack), so a combo always takes the default tier — and a combo handler essentially never resolves inside 2 s, because it probes candidate legs first. The default threshold therefore was the combo time-to-first-byte, and it equalled the tightest watchdog observed in practice.

Measured on this deployment (call-log artifacts, api_key_name=hermes-agents):

request status duration
free-stack (combo) 499 2011 / 2013 / 2016 ms
nvidia/z-ai/glm-5.3 (direct) 499 1945–2292 ms ×32

has_response_body: 0 on those records — zero bytes reached the client. The keepalive byte landed at 2.003 s and the client aborted at 2.011–2.016 s, i.e. the two 2 s budgets raced and the abort won by a few milliseconds of scheduling jitter.

Fix: price the default tier at half the observed watchdog (1 000 ms) and tighten the keepalive cadence from 2 500 ms to 1 500 ms so the inter-byte gap stays inside the same budget. Fast validation errors are unaffected (they resolve in single-digit milliseconds), and stream: true callers already expect SSE.

The slow tier keeps its deliberately long 15 000 ms value on purpose: for browser-session and anonymous-fallback providers, committing early only adds SSE framing to a request the caller already expects to wait on. This PR does not change that policy — the invariant is pinned per tier in the test so a future re-tuning cannot silently reintroduce the race on the default tier.

2. An Anthropic stop_reason leaked onto the OpenAI wire and failed the whole turn

normalizeOpenAICompatibleFinishReason() maps the OpenAI vocabulary, max_tokens→length, and safety reasons→content_filter, then passes everything else through raw. That raw fallthrough is deliberate for genuinely unknown values and for the abort reasons (malformed_function_call, …) whose entire purpose is to not present as a clean stop — but it also let cross-vendor synonyms through.

omniroute/paper-stack resolves to zenmux-free/openai/gpt-5.6-luna, which returns Anthropic-style stop reasons. Reproduced end-to-end against the live gateway with omp -p --mode=json:

{"type":"message_end","message":{
  "role":"assistant","content":[{"type":"text","text":"PONG"}],
  "provider":"omniroute","model":"paper-stack",
  "stopReason":"error","errorMessage":"Provider finish_reason: end_turn"}}

The text streamed completely and was then discarded — the client reads an unrecognized finish_reason as a provider fault and fails the turn. Raw SSE confirms the leak:

data: {"delta":{"content":"pong"},...}
data: {"delta":{},"finish_reason":"end_turn"}   ← Anthropic vocabulary on an OpenAI stream

claude-to-openai.ts's own convertStopReason() switch already maps these; this normalizer is the other door into an OpenAI-format response (responseSanitizer.ts streaming + non-streaming, jsonToSse.ts) and was missing the same mapping.

Fix: map the Claude stop_reason vocabulary onto its exact OpenAI equivalent (end_turn/stop_sequence/pause_turn→stop, tool_use→tool_calls, refusal→content_filter, model_context_window_exceeded→length), checked before the raw fallthrough.

3. The combo 403 did not say what to do

Combo "small-stack" is not allowed for this API key is a key-policy decision that reads like a routing bug, so callers retry the same doomed model — or fall through an entire compaction cascade (Auto-compaction failed → Auto-shake → …) on every attempt. The message now names the combo and the two real remedies.

Related Issues

  • No upstream issue filed; reported from a local omp combo-streaming failure (403 Combo "small-stack" is not allowed for this API key, then Provider finish_reason: end_turn).

Validation

  • Change type: routing / SSE streaming
  • Focused tests and category gates from the golden path
  • npm run typecheck:core
  • Reconciled with the current active release base (release/v3.8.51); focused checks rerun afterward
  • Production-code changes include a new or updated automated test in this PR
  • SonarQube is temporarily opt-in while the private project has no quota; it is not a PR gate.

Focused runs on release/v3.8.51:

  • tests/unit/finishReason.test.ts + tests/unit/keepalive-threshold.test.ts + tests/unit/early-stream-keepalive.test.ts — 47 pass / 0 fail
  • combo-streaming-*, combo-round-robin-streaming-lock-3811, combo-stream-readiness-fallback, apikeypolicy-*, combo-account-allowlist-3266, combo-pin-implicit-allowlist — 55 pass / 0 fail
  • npm run typecheck:core — clean

Live end-to-end reproduction before/after (omp -p, non-interactive):

combo before after
omniroute/small-stack turn failed PONG
omniroute/paper-stack Provider finish_reason: end_turn (text discarded) PONG

Tests Added Or Updated

  • tests/unit/finishReason.test.ts — Claude→OpenAI stop-reason mapping, case-insensitivity, and a guard that abort reasons still pass through raw.
  • tests/unit/keepalive-threshold.test.ts — default tier recalibrated; bare combo names asserted to take the default tier; a per-tier invariant test pinning the default tier strictly below the observed watchdog.

Coverage Notes

  • open-sse/utils/finishReason.ts, open-sse/utils/keepaliveThreshold.ts and open-sse/utils/earlyStreamKeepalive.ts are fully exercised by the two updated unit files above (pure resolvers plus the wrapper's existing behavioural tests).
  • src/shared/utils/apiKeyPolicy.ts change is message-only on two existing rejection paths; apikeypolicy-* and the combo-allowlist suites cover both branches.

Reviewer Notes

  • The slow tier is intentionally unchanged. If you want the watchdog budget to apply to web-session/anonymous-fallback providers too, that is a separate behaviour change and should be argued on its own — it adds SSE framing to requests that currently return a verbatim body.
  • withEarlyStreamKeepalive's internal options.thresholdMs ?? 2_000 fallback is left as-is: every route passes resolveKeepaliveThreshold(...), so the resolver remains the single source of truth.
  • Related but deliberately out of scope for this PR (found while reproducing, will file separately): the synthetic keepalive chunk is client-visible and becomes the response id (chatcmpl-keepalive) for clients that key off the first chunk.

…ropic finish reasons

Two independent faults made combo streaming unusable from a strict OpenAI client
(oh-my-pi/omp) that had previously worked.

1. The early-stream keepalive threshold equalled the tightest client first-byte
   watchdog. Every combo request reached the resolver as a bare name with no
   provider prefix, so it took the 2 000 ms default — and a combo handler
   essentially never resolves inside 2 s, because it probes candidate legs first.
   The default threshold therefore *was* the combo time-to-first-byte: the
   synthetic keepalive byte landed at 2 003 ms while omp aborted at 2 011–2 016 ms
   having received nothing, so the request died as a 499 with no diagnosable error.
   Price the default tier at half the observed watchdog (1 000 ms) and tighten the
   keepalive cadence from 2 500 ms to 1 500 ms so the inter-byte gap stays inside
   the same budget. The slow tier keeps its deliberately long value: for
   browser-session and anonymous-fallback providers, committing early only adds SSE
   framing to a request the caller already expects to wait on.

2. normalizeOpenAICompatibleFinishReason passed cross-vendor synonyms through raw,
   so an Anthropic-style `end_turn` returned by an OpenAI-format leg landed on the
   OpenAI wire. omp reads an unrecognized finish_reason as a provider fault and
   fails the whole turn with `Provider finish_reason: end_turn` — after the text had
   already streamed, discarding a completed assistant message. Map the Claude
   stop_reason vocabulary onto its exact OpenAI equivalent, keeping the deliberate
   raw passthrough for genuinely unknown values and for the abort reasons whose
   whole purpose is to not present as a clean `stop`.

Also make the combo 403 actionable: the bare "Combo X is not allowed for this API
key" reads like a routing bug, so callers retry the same doomed model or fall
through a whole compaction cascade instead of adding the combo to the key.
@diegosouzapw

Copy link
Copy Markdown
Owner

Thanks @ishan-parihar — merging via the release merge-train. Validated in local merge-train (mt-train10c) on the devbox @ train tip 4d841aa1c740bbaa03868dc0a403c62099a99a42 with the 72 sibling PRs of this batch: typecheck:core, file-size, complexity, cognitive-complexity, changelog-integrity green; changed-area node:test 831/831 (0 failing) and vitest 480/482 — the two reds are autoCombo/provider-family-combos.test.ts timing out at 20s, which reproduces on the PURE release tip under the full vitest suite (and is already tracked by the Release-Green issue #13866), so it is inherited, not this batch's. Merged --admin per merge-gates §3/§4/§7.

@diegosouzapw
diegosouzapw merged commit cd396b0 into diegosouzapw:release/v3.8.51 Sep 22, 2026
3 checks passed
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.

2 participants