Skip to content

fix(sse): wait out in-flight duplicate turns instead of failing them with 409 - #18717

Closed
hongkongkiwi wants to merge 2 commits into
cheaper-inference:release/v3.8.53from
hongkongkiwi:fix/turn-guard-wait-and-reacquire
Closed

hongkongkiwi wants to merge 2 commits into
cheaper-inference:release/v3.8.53from
hongkongkiwi:fix/turn-guard-wait-and-reacquire

Conversation

@hongkongkiwi

@hongkongkiwi hongkongkiwi commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • acquireTurnExecution now has a wait-and-reacquire wrapper, acquireTurnExecutionWithWait: when the turn slot for an effective idempotency key is held by an in-flight turn, the caller waits (bounded, default 60s, 500ms poll) for the original to release and then acquires. After the bound it returns the same blocked result as before, so the existing turn_in_progress 409 path is unchanged as the fallback.
  • handleChatCore uses the wrapper for the turn guard. The duplicate-blocked branch, its log line, and the 409 body are untouched.
  • This turns a stuck-original self-collision from a fatal client-facing stream_error (and the false all accounts rate limited (reset after Ns) cooldown cascade driven by the 409) into a short serialized wait. Rationale and production logs in fix(resilience): Turn guard 409s internal retries against a stuck in-flight turn, producing a fatal client-facing stream error and false account cooldowns #18716.

Related Issues

Target Branch

  • Base is the active release/v3.8.x branch (release/v3.8.53)
  • New feature (this is a routing/SSE reliability fix)

Validation

  • Change type: routing (sse/chatCore)
  • Focused loop: npx tsx --test tests/unit/chatcore/*.test.ts (35/35 pass, including 5 new tests)
  • npx eslint clean on the three touched files
  • Reconciled with the current active release base (branch cut from release/v3.8.53 today; the guard call site is unchanged upstream since 3.8.52)
  • Production-code change includes a new automated test

Tests Added Or Updated

  • tests/unit/chatcore/turn-execution-guard-wait.test.ts (new): immediate acquire, null-key no-op, waits then acquires after release, bounded fallback to the blocked result, abort stops the wait.

Coverage Notes

  • open-sse/handlers/chatCore/turnExecutionGuard.ts and the guard call site in open-sse/handlers/chatCore.ts are covered by the new test file. The deployed instance of this patch has been running since 2026-11-01; the duplicated-request path now waits ("duplicate waiting" log) instead of returning 409.

Reviewer Notes

  • The wait default (60s) covers the stuck-original lifetimes observed in production incidents (10-49s); pathological 100s+ hangs still fall back to today's 409, which is the better outcome for a turn that cannot succeed anyway.
  • clientRawRequest in chatCore carries no abort signal, so the call site passes no signal; a disconnected caller's wait is bounded by the same cap.
  • Repeated acquire attempts while blocked increment the entry's retryCount (it only feeds the X-OmniRoute-Turn-Retry header on the fallback 409).

@hongkongkiwi

Copy link
Copy Markdown
Contributor Author

Bumped the wait bound from 30s to 60s (da9b010): post-deployment monitoring on the instance from #18716 showed stuck originals held 42-49s, which straddled the original 30s bound; 60s covers the observed class while pathological 100s+ hangs still fall back to the 409.

@hongkongkiwi

Copy link
Copy Markdown
Contributor Author

Superseded by #18778.

This PR's wait is kept in #18778 for genuinely concurrent foreign duplicates. What it missed: the blocked caller in the #18716 incident is usually the same client request re-entering chatCore (same-account retry after a pre-output failure, account rotation, combo legs), and waiting does not save it when the original outlives the 60s bound. Production run on 2026-11-02: the original held 118s, the same-account retry spun 116 acquire attempts, and the 409 landed 3s after the original released.

#18778 adds a per-request ownership scope so those re-entries reuse the slot immediately, and keeps this PR's wait-and-reacquire for foreign duplicates. Closing this in favour of that one; the wait tests moved over with two added ownership tests.

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

Labels

None yet

Projects

None yet

1 participant