Session affinity retention on transient errors: port blocked by pinned release-on-failure semantics

[warelik]

Upstream PR router-for-me/CLIProxyAPI#5109 changes session affinity from "purge the binding on any failure and permanently rebind" to "retain the primary binding across transient failures, use a sticky temporary fallback during the primary's cooldown, and purge/rebind only on terminal auth errors (401/402/403/404, revoked OAuth grant, unsupported model)".

An additive port to this tree is not possible: the behavior change contradicts existing tests that pin the current semantics. Concretely, these existing tests assert the old behavior and would fail under the upstream retention rules:

• sdk/cliproxy/auth/session_affinity_fix_test.go:74 — asserts HTTP 429 invalidates the binding from the session cache:
  expected cacheKey %q to be invalidated on 429 failure, but still bound to %q
• the stream-failure variant in the same file — asserts a stream failure invalidates affinity (stream failure should invalidate affinity; still bound=%q)
• the reselect-after-failure test in selector_test.go — asserts Pick() rebinds to a different auth after a rate_limited failure release
• TestManagerSessionAffinityPreservesBindingAcrossHigherPriorityRecovery — asserts the failover binding stays after the higher-priority auth recovers (upstream retention instead returns the session to the recovered original)

Those tests encode deliberate Plus-side behavior (see the sibling-scoping work in #188), so flipping them is a product decision, not a mechanical port.

What upstream adds (worth having here if the semantics flip is accepted)

• isTerminalSessionAffinityError classification: transient errors (429/5xx, overload) retain the binding; only terminal errors release it.
• A dedicated fallbackCache *SessionCache on SessionAffinitySelector: while the primary is cooling down, the session sticks to one temporary fallback auth (shared across session alias keys) instead of re-drawing a random one per request; when the primary recovers and succeeds, the fallback binding is invalidated and the session returns to the primary.
• SessionCache.Aliases(sessionID) accessor.
• Full test suite to port: sdk/cliproxy/auth/session_affinity_retention_test.go (14 cases: transient 503 retention, 429 retry-after retention, terminal 401 unbind, sticky fallback across aliases, model-variant suffix collision safety).

Options

1. Accept the semantic flip: port the mechanism and update the four pinned tests to the retention expectations (upstream commit 5a4f2086 shows the exact test adjustments).
2. Keep current Plus semantics and skip the port.
3. Keep current semantics but add only the sticky temporary fallback (no retention on transient errors) — partially reduces failover flapping without touching the pinned tests.

Cross-link: router-for-me/CLIProxyAPI#5109, related Plus work #188.

[warelik]

Draft comment for kaitranntt on #204

This is a prepared comment for the maintainer to review and post.

---

Below are the design options for upstream PR router-for-me/CLIProxyAPI#5109
("retain session affinity on transient errors"). I have not posted this
anywhere; please review which choice fits Plus's session-affinity policy.

Recommendation: Option 1 — port the upstream retention and sticky-fallback mechanism

Implement the full upstream mechanism:

• isTerminalSessionAffinityError: only 401/402/403/404, invalid_grant, and
  model_not_supported (or equivalent reason strings) release the primary
  session binding; 429/5xx/408 and Cloudflare challenges keep it.
• Temporary sticky fallback: while the primary auth is cooling down, the
  session uses a dedicated fallbackCache with TTL and does not overwrite
  the primary cache key. The fallback selection stays on one auth for the
  duration of the cooldown.
• Return to primary on recovery: once the primary auth is healthy again and a
  request succeeds, drop the fallback binding and return the session to the
  primary.

Why this is our first choice

It best serves the project mission of stability without degradation:

• Stability (P1): a 503/429 does not immediately detach a live auth, so
  the next request can reuse the same credential and warm caches instead of
  hopping through random fallbacks.
• No degradation (P2): keeping the same auth across transient failures
  preserves prompt-cache warmth and reduces the "account carousel" that harms
  output quality and retry latency.
• It aligns with the upstream direction, so future stock merges are less
  likely to conflict.

Trade-off

This contradicts four existing Plus tests that pin the current
"release-on-failure / rebind-on-failure" semantics:

• sdk/cliproxy/auth/session_affinity_fix_test.go — 429 invalidates the
  binding and a stream failure invalidates it.
• sdk/cliproxy/auth/selector_test.go — Pick() rebinds to a different auth
  after a rate_limited failure.
• TestManagerSessionAffinityPreservesBindingAcrossHigherPriorityRecovery —
  the session stays on the low-priority fallback after the high-priority auth
  recovers.

These tests would need to be updated to the upstream expectations (exact
adjustments are in router-for-me/CLIProxyAPI#5109 commit 5a4f2086), so this
is a product decision, not a mechanical port.

Option 2 — keep current Plus semantics and skip the port

Leave the code as-is and close #204 as "not aligned with Plus behavior."

Trade-offs

• Pros: zero code churn, no test rewrites, preserves the deliberate
  choices made in fix(auth): scope model suspension reasons to prevent stale sibling resume #188 and the sibling affinity scoping work.
• Cons: transient 429/5xx continue to purge session affinity and the
  fallback selection keeps overwriting the primary key, so we keep the
  account-carousel and cold-cache problems that #5109 was written to fix.

Option 3 — keep current semantics but add only the sticky temporary fallback

Implement the fallbackCache with TTL so that while the primary is cooling
it sticks to a single temporary auth, but do not retain the primary
binding across transient failures and do not return the session to the
primary on recovery.

Trade-offs

• Pros: reduces flapping among fallbacks and is probably the smallest
  diff; the four pinned tests above can remain unchanged.
• Cons: the primary binding still gets purged on the first 429/5xx, so
  the main benefit of the upstream work — keeping a healthy primary bound
  across transient glitches — is lost. It is a partial fix only.

My take

Option 1 is the right long-term fix for the mission (stability + quality),
but it intentionally reverses the behavior the pinned tests currently encode.
If you are comfortable flipping that product decision and updating the four
named tests, I can prepare the port as a Plus PR and cross-link it to
router-for-me/CLIProxyAPI#5109 and #188.

If the "stay on fallback after recovery" and "purge on any failure" behavior
from #188 is load-bearing, then Option 3 is the conservative compromise; it
still reduces fallback flapping without touching the four pinned test
contracts.

Please let me know which direction you want, and I will open the PR or close
#204 accordingly.

[warelik]

Implemented in #212.