Skip to content

fix(resilience): fence combo provider probes and local refusals - #15594

Merged
diegosouzapw merged 1 commit into
cheaper-inference:release/v3.8.52from
TheViniAlmeida:fix/provider-breaker-recovery-051026
Oct 7, 2026
Merged

diegosouzapw merged 1 commit into
cheaper-inference:release/v3.8.52from
TheViniAlmeida:fix/provider-breaker-recovery-051026

Conversation

@TheViniAlmeida

@TheViniAlmeida TheViniAlmeida commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Problem and behavior

When a combo consumes a provider HALF_OPEN probe, a successful response cannot pass its own canExecute() guard. The provider stays blocked until the probe lease expires. Alias keys can diverge from the canonical chat key, and locally generated provider_circuit_open responses can be retried and recorded as upstream failures.

This change settles acquired probes inside execute() under the existing generation fence, preserves the settled identity through SSE wrappers, canonicalizes provider/connection accounting, and immediately advances priority and round-robin combos on local circuit refusals. External callbacks without the acquired lease cannot settle HALF_OPEN provider probes. Persisted credential errors do not count as new upstream attempts; genuine upstream 503 responses still count.

Closes #15588. Complements the consumed-probe recovery work in #15555 and #15257 with alias identity, stale-probe fencing, streaming propagation and local-refusal handling. Neither referenced PR is modified or assumed merged.

Validation

  • Six isolated RED runs reproduce assertion failures on base 23a11484 (consumed probe, stale generation, alias identity, priority/round-robin local refusal, classifier, SSE and stored credential 503 paths).

  • 33 focused regression tests pass, including an actual streamed combo response and no-credentials/all-rate-limited with zero provider fetches.

  • Expanded native regression selection: 187 tests pass (breaker, cooldown, combo fallback/retry and streaming).

  • Independent review: 24 regression tests pass in a clean environment.

  • Full MCP/autoCombo/cache Vitest suite: 54 files, 493 tests pass (npm run test:vitest -- --maxWorkers=2, clean environment).

  • Core typecheck, focused ESLint, file-size limits, changelog integrity and git diff --check pass. Commit hooks run without bypass.

  • Production Docker runner-base build passes with the supported builder-only arguments OMNIROUTE_BUILD_MEMORY_MB=12288 and OMNIROUTE_BUILD_WORKERS=1 (the default 6144MB heap exhausted during webpack compilation). This custom artifact also preserves a pre-existing deployment heartbeat patch; that separate patch is outside this PR.

  • Self-hosted canary and previous-image offline control pass with unchanged stable configuration and schema. Authenticated post-rollout validation passes for eight combo streams plus Chat nonstream and Responses stream (short synthetic prompts).

⚠️ base-red inherited: #15306. The full native/integration suites are not claimed green by this PR; native validation uses the expanded regression selection above. The complete configured Vitest suite passed in this contribution. No dependency, schema, security-policy or baseline-budget changes.

@diegosouzapw
diegosouzapw merged commit 3b3b578 into cheaper-inference:release/v3.8.52 Oct 7, 2026
3 checks passed
diegosouzapw added a commit that referenced this pull request Oct 7, 2026
Records the Onda A quality drift on the post-squash tip. Product squashes do not carry the baseline.

File-size ceilings inside `frozen`, measured with `split("\n").length`:

- `EditConnectionModal.tsx` 1631 → 1634 (#14907)
- `open-sse/executors/default.ts` 1209 → 1213 (#15576)
- `open-sse/handlers/imageGeneration.ts` 3334 → 3346 (#15670)
- `open-sse/services/claudeCodeCompatible.ts` new freeze 1245 (#15457)
- `open-sse/services/combo/executeTargetAttempt.ts` 1273 → 1280 (#15594)
- `open-sse/services/combo/roundRobinCombo.ts` 1285 → 1288 (#15594)

Narrow i18n adopt, hashes only, for the five docs this wave edited: `docs/DEVIN_CLAUDE_BRIDGE.md`, `docs/architecture/RESILIENCE_GUIDE.md`, `docs/guides/SETUP_GUIDE.md`, `docs/guides/TROUBLESHOOTING.md`, `docs/reference/API_REFERENCE.md`.

`node scripts/check/check-file-size.mjs` passed. `node scripts/i18n/check-translation-drift.mjs` passed after the adopt. The legacy string map was not edited.

⚠️ base-red inherited: #15306
diegosouzapw added a commit that referenced this pull request Oct 8, 2026
… by #15594 (#15818)

* test(resilience): pin the #15690/#15698 provider-breaker fixes landed by #15594

* test(mutation): register the new regression tests in stryker tap.testFiles
diegosouzapw added a commit that referenced this pull request Oct 8, 2026
…vider id (#15594) (#15910)

Aligns the serial health-autopilot test to #15594's canonical provider-breaker keying (alias failures land on the canonical breaker; reset still proven). test:unit:serial 23/23.
diegosouzapw added a commit to mdc2122/OmniRoute that referenced this pull request Oct 8, 2026
…onnection helper

withSelectedConnectionHeader() moved to chat/selectedConnectionHeader.ts
as a copy of the pre-cheaper-inference#15594 version, so taking this branch's side of the
chatHelpers.ts conflict would silently drop the provider-probe
inheritance the release tip added to its clone path. Port it, drop the
now-unused import from chatHelpers.ts and add a guard test.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
diegosouzapw added a commit to kalpakprod/OmniRoute that referenced this pull request Oct 8, 2026
Same review follow-ups as the standalone cheaper-inference#15567, applied to the
identical ownership code carried here:

- withSelectedConnectionHeader() moved to chat/selectedConnectionHeader.ts
  as a pre-cheaper-inference#15594 copy; port the provider-probe inheritance so the
  chatHelpers.ts conflict resolution does not drop it (guard test added).
- Drop responsesUrlForModel(): the tip routes muse-code to /responses in
  buildUrl's `case "muse-code"` (cheaper-inference#15576); remove its fragment, keep the URL
  tests as a guard of the tip behavior.
- Move the key_value SQL into src/lib/db/museSessionOwnership.ts, prune
  sessions idle for 30 days (owner pin, served flag, opaque/reference
  hashes) and keep unpinned routing for a single OAuth account without a
  session ID instead of returning 400.

Co-authored-by: mdc2122 <259970464+mdc2122@users.noreply.github.com>
Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
@lotexdj

lotexdj commented Oct 13, 2026

Copy link
Copy Markdown

Additional production confirmation from an operator-authorized investigation on 2026-10-13. Posting here rather than opening a duplicate because the consumed-probe recovery defect matches this PR. This is evidence from 3.8.51, not a regression claim against the merged fix.

Affected artifact: official diegosouzapw/omniroute:3.8.51, digest sha256:8bd462c9f60d8eda79329cfbb6ea7ea723505fe7721beb944f3d43835409e218.

Observed behavior: the persisted canonical codex breaker remained HALF_OPEN, failure count 4, reset timeout 60000 ms, one probe permit. A two-hour call-log window contained 77 Codex 503 Pipeline gate rejected entries, while fixed-connection combo requests to the same Codex provider continued to return HTTP 200. Logs said Circuit breaker OPEN ... rejecting request even when the persisted state was HALF_OPEN. No OOM/restarts or resource-pressure events were present in the inspected interval.

Shipped-code reproduction: extracted the actual CircuitBreaker class from the installed bundle and stubbed persistence only. Starting with HALF_OPEN/one permit, execute(async () => ({success:true,status:200}), {classifyResult: () => 'ignore'}) consumed the permit but left HALF_OPEN and canExecute() === false. Advancing past the 60-second lease allowed another success that repeated the cycle. Explicit successful outcome accounting closed the breaker in the control. This reproduces the class-level symptom; it is not a replacement for your full combo regression coverage.

A temporary Codex-only local mitigation credits confirmed parsed nonstream success or clean terminal stream completion, with duplicate/shadow/stale-generation exclusions and unchanged failure handling. After deploying it, the persisted HALF_OPEN breaker transitioned to CLOSED with failure count 0 at 12:42:53 UTC. Two authenticated combo chats, completed combo SSE, and two consecutive direct Codex chats passed; the final breaker remained CLOSED/0. Focused negative checks covered abort/error completion and stale-generation exclusion. This local mitigation is not claimed equivalent to your acquired-lease design, nor is it proposed as an additional upstream patch.

We discovered this merged PR during duplicate checking after the local mitigation. We have not yet tested an official release containing #15594 on this instance. Please treat this as independent confirmation of the impact on the older shipped artifact and of the need to credit successful consumed probes. No hostnames, addresses, account identifiers, credentials, prompts from real workloads, or raw logs are included.

diegosouzapw added a commit to kalpakprod/OmniRoute that referenced this pull request Oct 22, 2026
Reconciles the Muse Code consolidation with the v3.8.53 base:
- managedLeaseFence.ts: keep the base's createManagedLeaseFence factory and add the
  MUSE_OWNERSHIP_REJECTED code/result to it (no lease needed for that fence).
- chat.ts: fold the Muse owner pin (allowed connection, no occupancy session key, forced
  connection, no preselected/empty-turn sibling exclusion) into the base's selectCredentials()
  helper.
- selectedConnectionHeader.ts: clone path now goes through inheritResponsePolicies (probe
  marker cheaper-inference#15594 + trusted empty-turn record cheaper-inference#16072), with a regression test.
- providerModels.ts / registeredEffortVariants.ts: keep the base (OpenCode Free removed in
  cheaper-inference#16310, `ultra` tier) plus the PR's ocg/ocz aliases and `minimal` suffix.
- oauth-polltoken test: assert both the bare invalid_response shape and the no-upstream-echo
  guard (cheaper-inference#15869); stryker list and file-size baseline take the base side.
- opencode-muse-spark-1.3-support test: the removed `opencode` alias is replaced by `ocz`.
- museSessionOwnership.ts: split ownContinuationIds() out of continuationIds() so the PR does
  not add a cognitive-complexity violation over the ratchet baseline.
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.

fix(resilience): complete combo breaker identity, probe fencing and local-refusal handling

3 participants