[nikpanaev]

Summary

After a provider's sole HALF_OPEN permit is consumed by execute(), a successful connection-scoped combo attempt leaves the parent provider breaker HALF_OPEN: recordProviderSuccess() checks canExecute(), which is now false. Later requests are skipped despite the successful probe.

Account for the completed success independently of admission for another request. Use getStatus() to preserve lazy state refresh and leave an active OPEN cooldown intact. The shared success path covers both priority/weighted and round-robin combo attempts; no settings or version metadata change.

Related Issues

⚠️ base-red inherited: #15246

Validation

• Change type: routing
• Reconciled with release/v3.8.52 at dbe703a0000b303cd7b1cf5879cb8740e5bfce71; focused checks rerun on this base
• Production-code changes include an updated automated test
• The updated regression fails on the unpatched base with HALF_OPEN instead of CLOSED, then passes with the fix.
• Focused ESLint and Prettier checks pass for both changed files; git diff --check passes.
• node scripts/check/check-file-size.mjs --base-ref dbe703a0000b303cd7b1cf5879cb8740e5bfce71 passes without changing the frozen baselines or growing accountFallback.ts.
• npm run lint exits 2 with "There are suppressions left that do not occur anymore"; the global suppressions file is unchanged.
• Four focused native test files: 19 passed, 0 failed, using Node 24.21.0 and the release branch's exact lockfile.

DISABLE_SQLITE_AUTO_BACKUP=true node --import tsx/esm \
  --import ./open-sse/utils/setupPolyfill.ts \
  --import ./tests/_setup/isolateDataDir.ts \
  --test --test-force-exit \
  tests/unit/provider-breaker-connection-success-closes-provider.test.ts \
  tests/unit/provider-breaker-halfopen-recovery.test.ts \
  tests/unit/circuit-breaker-half-open-probe-lease-13836.test.ts \
  tests/unit/circuit-breaker-resolved-5xx-12254.test.ts

Tests Added Or Updated

tests/unit/provider-breaker-connection-success-closes-provider.test.ts now consumes the HALF_OPEN permit through execute() with classifyResult: () => "ignore", matching combo-owned outcome accounting, before recording the connection success. It also closes its SQLite handle after the test.

Coverage Notes

The regression directly exercises the previously missed consumed-permit condition; existing focused tests cover recovery, active OPEN cooldown preservation, failed probes, probe leases and resolved 5xx responses. Global coverage has not been measured locally.

Reviewer Notes

This is a draft source fix. Full unit/Vitest suites, production build and a compiled-package HTTP regression remain unverified here. Local production builds on an 8 GB Mac failed at a 4 GB heap; a documented 6 GB API-only build was stopped under sustained host memory pressure without completing. No patched runtime has been deployed. The inherited release-green failures above are outside this two-file change.

[Kizuno18]

This matches a local recovery failure we investigated on the 3.8.51 compiled runtime: a completed neutral execute() had consumed the sole HALF_OPEN permit, so a connection-scoped success could not reach the provider recovery path.

One useful distinction for the fix: the combo's classifyResult: () => "ignore" is intentional because the surrounding pipeline owns outcome accounting. An earlier local attempt changed it to the error-kind classifier, which returns transient / rate_limit / quota_exhausted, not the required success / failure / ignore. That was not a valid repair and was reverted.

I independently checked current release/v3.8.52 at 4399e60057aaee564ec9e31054a5f6c76cc92a2d. With the actual breaker source, a resolved ignored probe or an excluded cancellation still leaves canExecute() false immediately after settlement. It becomes true after resetTimeout, so #13849 prevents the old permanent stall but does not make the consumed-success gate correct. Your regression exercises the missing case directly.

Our older-build workaround releases settled neutral/cancelled permits in finally, with a same-transition guard, and retains the original neutral classifier. Its eight local regressions cover in-flight concurrency, success/failure transitions and stale probes. That immediate-release behavior is a separate possible improvement; it should not replace the completed-success accounting fix here or be ported onto the newer generation-based lease logic without corresponding tests.

[diegosouzapw]

Thanks @nikpanaev. The provider breaker was redesigned in #15594: recordProviderSuccess only touches the provider breaker in CLOSED/DEGRADED and a HALF_OPEN provider is closed by its acquired probe, and the tip test provider-breaker-connection-success-closes-provider.test.ts asserts the opposite of this change. Closing as superseded. (/merge-batch 2026-10-17)