fix: make provider fallback policy explicit and visible - #269
Merged
Conversation
OmarB97
marked this pull request as ready for review
July 12, 2026 21:27
OmarB97
added a commit
that referenced
this pull request
Jul 20, 2026
* wip: make fallback policy explicit and local-safe * fix: enforce fallback policy across runtimes --------- Co-authored-by: Omar Baradei <omar@kostudios.io>
13 tasks
OmarB97
added a commit
that referenced
this pull request
Aug 2, 2026
…#291) PRs #269 and #272 added fallback-policy plumbing to the shared turn prologue (`_refresh_fallback_policy`, `_emit_pending_fallback_notice`) and structured status emission to the fallback activation path (`_emit_fallback_status`, which reads `self.status_callback` directly). Three test files build agents that deliberately bypass `__init__` — either `AIAgent.__new__(AIAgent)` or a hand-rolled `_FakeAgent` — and were never updated with the new attributes, so they raise AttributeError as soon as the prologue or fallback path runs. This is test-side only: `agent_init.py` sets `status_callback` unconditionally during real construction, so no shipped code path is affected. In `test_compressor_fallback_update.py` the breakage was masked: the missing `status_callback` raised inside `try_activate_fallback`, whose `except` swallowed it, logged "Failed to activate fallback", and recursed through the rest of the chain before failing on the terminal status emit. Stubs match the convention already used by tests/agent/test_turn_context.py. Co-authored-by: Omar Baradei <omar@kostudios.io> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
13 tasks
OmarB97
added a commit
that referenced
this pull request
Aug 2, 2026
…ck behavior (#295) Both tests encode call shapes that recent changes moved past. Neither is a production defect — the new behavior is the intended one in both cases. test_codex_content_filter_incomplete_routes_to_policy_fallback #269 threads the classified FailoverReason into the fallback switch, so _try_activate_fallback is now called with the reason rather than bare. Asserting the reason is propagated is strictly stronger and matches what the test name claims to cover. test_output_cap_retry_request_pressure_lower_bound The output-cap retry is now anchored to output_tokens_that_fit(), the documented single source of truth for "how many output tokens fit". It reserves the input conservatively so the returned cap is one the provider will accept, which against this test's near-full window (a ~199k-token system prompt in a 200k model window) binds below the plain min(provider_available, context - estimate) - 64 arithmetic the test re-derived. The expectation now applies the same clamp the code does. Co-authored-by: Omar Baradei <omar@kostudios.io> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This was referenced Aug 2, 2026
OmarB97
pushed a commit
that referenced
this pull request
Aug 2, 2026
TestResolveRuntimeWithFallback patches `server._load_fallback_model`, but `_resolve_runtime_with_fallback()` consults `_load_effective_fallback_model()` — the policy-filtered chain added by #269. That wrapper only reaches the raw loader when the ambient policy is not "off": def _load_effective_fallback_model(): cfg = _load_cfg() policy = get_fallback_policy(cfg) if policy == "off": return [] chain = _load_fallback_model() or [] CI has no hermes config, so the policy defaults to "any", the stub is reached, and these tests are green there. On a host whose config sets `fallback_policy: off`, the wrapper short-circuits to [], the stub is never consulted, no fallback is ever attempted, and all five tests fail for a reason that has nothing to do with what they assert. Patch what the code actually calls, so the case under test is the one that runs on any host. `test_auth_error_all_fallbacks_fail_raises` additionally pins `_load_cfg`, because it asserts the policy-specific exhaustion wording ("no usable configured backup route remained" is the "any" branch) and would otherwise assert against whatever policy the host happens to have. This is not a CI-visible failure — it is a latent host-config dependency that makes the suite behave differently for a contributor than it does in CI. TestResolveRuntimeWithFallback: 8 passed against a config with `fallback_policy: off`, and 8 passed with no hermes config at all. Before this change the first environment produced 5 failures. This file's two remaining failures (golden transcript, session activate) are owned by #296; this change is disjoint from it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
OmarB97
added a commit
that referenced
this pull request
Aug 2, 2026
…ig (#299) TestResolveRuntimeWithFallback patches `server._load_fallback_model`, but `_resolve_runtime_with_fallback()` consults `_load_effective_fallback_model()` — the policy-filtered chain added by #269. That wrapper only reaches the raw loader when the ambient policy is not "off": def _load_effective_fallback_model(): cfg = _load_cfg() policy = get_fallback_policy(cfg) if policy == "off": return [] chain = _load_fallback_model() or [] CI has no hermes config, so the policy defaults to "any", the stub is reached, and these tests are green there. On a host whose config sets `fallback_policy: off`, the wrapper short-circuits to [], the stub is never consulted, no fallback is ever attempted, and all five tests fail for a reason that has nothing to do with what they assert. Patch what the code actually calls, so the case under test is the one that runs on any host. `test_auth_error_all_fallbacks_fail_raises` additionally pins `_load_cfg`, because it asserts the policy-specific exhaustion wording ("no usable configured backup route remained" is the "any" branch) and would otherwise assert against whatever policy the host happens to have. This is not a CI-visible failure — it is a latent host-config dependency that makes the suite behave differently for a contributor than it does in CI. TestResolveRuntimeWithFallback: 8 passed against a config with `fallback_policy: off`, and 8 passed with no hermes config at all. Before this change the first environment produced 5 failures. This file's two remaining failures (golden transcript, session activate) are owned by #296; this change is disjoint from it. Co-authored-by: Omar Baradei <omar@kostudios.io> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
fallback_policywithoff,local-only, andanymodes.hermes fallback policymanagement.Why
Fallback routing was assembled and cached independently by several entry points. That let init-time auth recovery bypass the live policy, made stale routes survive config changes, and could switch a turn to another provider without a durable user-facing explanation. The result was especially confusing when a local Hermes deployment silently recovered through a remote/free provider.
This change makes routing eligibility a shared policy boundary and treats the fallback decision as observable runtime state rather than incidental console text.
Safety and compatibility
offfails loudly without attempting a backup route.local-onlyadmits only routes whose endpoint/provider metadata proves they are local; unknown endpoints fail closed.anypreserves the historical configured-chain behavior for existing configs./modelselections remain excluded from automatic fallback for the active session.mainat TTS-state containmente1754474b6(test(gateway): stop TTS state leaking into later tests #268), with browser-test containment28f371dc8b([Hermes][Browser tests][1/n] Prevent real Chrome launch in retry test #266) immediately below it. Their patch IDs exactly match source commits8f58ce2b553dbf1a7b0f500849fa540e11f492b0and398ed14ef, respectively.Validation
scripts/run_tests.shon 18 affected non-TUI Python files: 1,141 passed.git diff --check: passed.tests/test_tui_gateway_server.pywas intentionally not executed because its browser/TTS test isolation was under active containment review. No live application was launched, installed, restarted, or modified.The repository-wide Desktop lint command still reports one pre-existing sort-import error and one hook-dependency warning in unchanged files (
use-prompt-actions/submit.tsandsettings/model-settings.tsx); neither file is in this diff.Known review follow-ups (non-blocking P2)
api_key/key_env/api_mode, so an active fallback client can retain same-route credential changes until the primary cooldown ends.status_callback, so fallback status is server-visible but not yet streamed to that API client surface.This is intentionally a draft for focused review of those boundaries and the cross-runtime behavior.