Skip to content

fix(streaming): fail fast when an upstream stream produces only lifecycle/heartbeat events, never real content - #12741

Merged
diegosouzapw merged 1 commit into
diegosouzapw:release/v3.8.51from
hartmark:fix-content-stall-watchdog
Sep 10, 2026
Merged

diegosouzapw merged 1 commit into
diegosouzapw:release/v3.8.51from
hartmark:fix-content-stall-watchdog

Conversation

@hartmark

@hartmark hartmark commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Live incident (2026-09-04): a free-tier OpenRouter model
    (minimax/minimax-m3:free, under load) streamed nothing but OpenAI
    Responses response.in_progress heartbeats for ~90 seconds before the
    calling client's own idle timeout (120s for that internal task, 900s for
    the main agent) finally gave up with a generic timeout message. See
    pipeWithDisconnect's own doc comment for the full mechanism.
  • ensureStreamReadiness's pre-handoff gate correctly treats a bare
    response.in_progress as "ready" (deliberately — kiro.ts's early
    role-only start chunk relies on the exact same behavior) and hands the
    connection to the client. From that point nothing was watching whether
    the model ever actually said anything. The existing byte-level stall
    watchdog (ported from decolua/9router#1243) doesn't catch this either —
    it deliberately tracks raw upstream byte activity, not transform output,
    to avoid false-positiving on reasoning models, and the heartbeat bytes
    keep it satisfied indefinitely.
  • Adds a second, independent watchdog to pipeWithDisconnect: armed once
    at stream start, cleared permanently the first time real model output is
    observed (reusing createStreamContentWatcher, the exact classifier
    createDisconnectAwareStream already trusts for its own end-of-stream
    fix(resilience): auto/* combos swallow upstream auth failure — empty finish:stop with no error when tools attached #8649 empty-content check), firing only if the deadline elapses with
    lifecycle/ping frames only. Off by default — no arbitrary constant picked
    at this layer; chatCore wires it to the same adaptive
    streamReadinessPolicy.timeoutMs already computed for the pre-handoff
    readiness gate, so slow-first-content reasoning models keep the same
    generous, request-specific budget in both phases.
  • A genuine invisible mid-stream provider swap is architecturally
    impossible over HTTP SSE passthrough once headers have reached the
    client — this cannot make combo silently retry with a different model.
    What it does: turn an indefinite silent hang into a fast, explicit,
    well-formed SSE error the calling agent's own existing retry/fallback
    logic reacts to immediately, instead of waiting out whichever client-side
    timeout happened to apply to that specific call.

Related Issues

  • Closes #
  • Related to #

Validation

  • Change type: routing / other (streaming reliability)
  • Focused tests and category gates from the golden path
  • npm run lint (targeted: touched files clean; unrelated pre-existing
    chatCore.ts unused-import errors verified via stash/pop)
  • 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

Tests Added Or Updated

  • tests/unit/stream-handler.test.ts (+4 tests):
    • pipeWithDisconnect flags a stream that only ever produces lifecycle/heartbeat frames, never real content — reproduces the
      incident shape (a bare response.in_progress frame, nothing else) with
      a tight contentStallTimeoutMs. Verified RED on pre-fix code (falls
      back to the byte-stall watchdog's much longer budget, missing the
      incident's actual failure signature) and GREEN after.
    • ... does NOT flag a stream that starts with lifecycle frames but then produces real content — no false positive once real output arrives.
    • ... content stall watchdog is off by default (no contentStallTimeoutMs) — legacy behavior preserved when the option
      isn't threaded.
    • Existing byte-level stall watchdog tests (3) still pass unmodified,
      proving the two watchdogs are independent and don't interfere.

Coverage Notes

  • Touches open-sse/utils/streamHandler.ts (the watchdog itself),
    open-sse/handlers/chatCore/streamingPipeline.ts (threading), and
    open-sse/handlers/chatCore.ts (one call site, one new field). The new
    tests cover pipeWithDisconnect directly at the same level as its
    existing byte-stall coverage; chatcore-streaming-pipeline.test.ts's
    existing type-contract test (loose variadic mock) still passes unchanged
    since the new option is additive/optional.

Reviewer Notes

  • No migration, no protocol change, no new config surface — purely additive
    and default-off unless explicitly wired.
  • git diff --numstat: production +119/-15 across 3 files (chatCore.ts
    +5, streamingPipeline.ts +8/-1, streamHandler.ts +91/-14), tests
    +130. The production growth is the watchdog itself plus its wiring; it
    reuses createStreamContentWatcher and mirrors the existing byte-stall
    watchdog's exact arm/clear/wrappedController pattern rather than
    inventing a new mechanism.
  • Full whole-project tsc --noEmit run against this branch produces zero
    errors in any touched file (pre-existing project-wide noise elsewhere is
    unaffected, verified via stash/pop).

…ycle/heartbeat events, never real content

Live incident (2026-09-04): a free-tier OpenRouter model
(minimax-m3:free, under load) streamed nothing but OpenAI Responses
response.in_progress heartbeats for ~90 seconds before OpenClaw's own
client-side idle timeout (120s for that particular internal task,
900s for the main agent) finally gave up. OmniRoute's own
ensureStreamReadiness pre-handoff gate correctly treats a bare
response.in_progress as "ready" (deliberately -- kiro.ts's early
role-only start chunk relies on the exact same behavior for UX, so
tightening readiness itself would regress that), hands the connection
to the client, and from that point on nothing was watching whether the
model ever actually produced real output. The existing byte-level
stall watchdog in pipeWithDisconnect (ported from decolua/9router#1243)
doesn't catch this either -- it deliberately tracks raw upstream byte
activity (not transform output) to avoid false-positiving on reasoning
models, and the heartbeat bytes keep it happy indefinitely.

Adds a second, independent watchdog to pipeWithDisconnect: armed once
at stream start, cleared permanently the first time real model output
is observed (reusing createStreamContentWatcher, the exact classifier
createDisconnectAwareStream already trusts for its own end-of-stream
diegosouzapw#8649 empty-content check), and firing only if the deadline elapses
with lifecycle/ping frames only. Off by default (no arbitrary constant
picked at this layer); chatCore wires it to the same adaptive
streamReadinessPolicy.timeoutMs already computed for the pre-handoff
readiness gate, so slow-first-content reasoning models keep the same
generous, request-specific budget in both phases.

A genuine invisible mid-stream provider swap is architecturally
impossible over HTTP SSE passthrough once headers have been sent to
the client -- this cannot make combo retry with a different model. What
it does do: turn an indefinite silent hang into a fast, explicit,
well-formed SSE error the calling agent's own existing retry/fallback
logic can react to immediately, instead of waiting out whichever
client-side timeout happened to apply to that specific call.
@hartmark
hartmark marked this pull request as ready for review September 4, 2026 15:49
hartmark added a commit to hartmark/OmniRoute that referenced this pull request Sep 4, 2026
…am stream produces only lifecycle/heartbeat events, never real content) into dev/omniroute-dev-combined
hartmark added a commit to hartmark/OmniRoute that referenced this pull request Sep 5, 2026
…am stream produces only lifecycle/heartbeat events, never real content) into dev/omniroute-dev-combined
hartmark added a commit to hartmark/OmniRoute that referenced this pull request Sep 6, 2026
…am stream produces only lifecycle/heartbeat events, never real content) into dev/omniroute-dev-combined
hartmark added a commit to hartmark/OmniRoute that referenced this pull request Sep 7, 2026
…am stream produces only lifecycle/heartbeat events, never real content) into dev/omniroute-dev-combined
hartmark added a commit to hartmark/OmniRoute that referenced this pull request Sep 7, 2026
…am stream produces only lifecycle/heartbeat events, never real content) into dev/omniroute-dev-combined
hartmark added a commit to hartmark/OmniRoute that referenced this pull request Sep 8, 2026
…am stream produces only lifecycle/heartbeat events, never real content) into dev/omniroute-dev-combined
hartmark added a commit to hartmark/OmniRoute that referenced this pull request Sep 8, 2026
…am stream produces only lifecycle/heartbeat events, never real content) into dev/omniroute-dev-combined
hartmark added a commit to hartmark/OmniRoute that referenced this pull request Sep 9, 2026
…am stream produces only lifecycle/heartbeat events, never real content) into dev/omniroute-dev-combined
@diegosouzapw
diegosouzapw merged commit 1216cff into diegosouzapw:release/v3.8.51 Sep 10, 2026
10 of 25 checks passed
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…ycle/heartbeat events, never real content (diegosouzapw#12741)

Validado numa worktree combinada com a onda de streaming desta leva sobre `release/v3.8.51`: typecheck:core limpo, check-api-typecheck OK (289), check-file-size OK após rebaseline, 88/88 nos testes focados.

Um stream que só emite eventos de ciclo de vida e heartbeat, sem conteúdo nenhum, é falha disfarçada de sucesso: o cliente espera até o timeout dele. Falhar rápido devolve o controle.
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