fix(fallback): restore the primary model/auth route after any picker failure in hermes fallback add - #104209
Merged
Conversation
Contributor
૮ >ﻌ< ა ci reviewran on f39966a — test(fallback): keep the two persisted-config invariants; PT
|
… for the picker error path Trim the salvaged suite to the behaviour contracts: a picker exception or Ctrl+C leaves config.yaml's model exactly as snapshotted (parametrized), and an absent active_provider stays absent through snapshot+restore. The mock-dispatch tests (asserting calls into our own helpers) are dropped. evals/cli_fallback_add_picker_error.py drives the real `hermes fallback add` under a Linux PTY into an ordinary picker OSError and checks the persisted model: stranded on base, restored after.
teknium1
force-pushed
the
fix/devchan-c085-picker-notices-fallback
branch
from
September 6, 2026 10:29
57f2d18 to
f39966a
Compare
alt-glitch
added
type/bug
42 tasks
This was referenced Sep 6, 2026
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
3 participants
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.
hermes fallback addnow restores the primary model/auth route after any picker failure (ordinary exception, Ctrl+C, post-picker config read error) instead of leaving the temporarily selected fallback active as the primary.Root cause
hermes_cli/fallback_cmd.py::cmd_fallback_addreuses the canonicalhermes modelpicker, which persists its selection as the primarymodelin config.yaml (and touchesauth.jsonactive_provider); the command then snapshots/restores around it. On main the restore ran only onSystemExitand on the normal-return paths. An ordinary exception from inside the picker (e.g. anOSErrorfromdeactivate_providerafter the model was already written) propagated straight out, and the user's primary was silently replaced by the fallback they were trying to add. Cleanup was also best-effort (except Exception: pass) in the auth half, and an absentactive_providerkey was written back as an explicitnull.Fix (contributor commit, cherry-picked verbatim)
try/except BaseExceptionboundary around the picker and the post-picker config read; restore both stores, re-raise the original error with the restore outcome attached as a note (add_note) instead of masking it._restore_primary_routeattempts the config half and the auth half independently and reports partial failure (RuntimeError) rather than swallowing it._auth_store_lock(); a missingactive_providerstays missing (sentinel) instead of becomingnull.custom_providers/ credentials are retained; onlymodelandactive_providerare rolled back.Before / after (real CLI under a Linux PTY)
Harness:
evals/cli_fallback_add_picker_error.py --root <checkout> --output <dir> --expect stranded|restored. TempHERMES_HOME, primaryopenrouter / primary/model-a, a saved custom provider withdiscover_models: false(no network). After the provider menu renders (snapshot already taken) the harnesschmod 0sauth.json, then picks the custom provider + its model: the canonical picker writes the temporary primary and then raises a plainPermissionErrorfromdeactivate_provider.3513a3b9227PermissionError, exit 1)config.yamlmodelafterwards{provider: custom, default: lab-model, base_url: http://127.0.0.1:9/v1}— primary replaced{provider: openrouter, default: primary/model-a, …}— primary restoredCould not fully restore the primary route … auth.json(auth half legitimately unreadable in this scenario; config half restored)Raw PTY captures kept locally (not published).
Tests (
tests/hermes_cli/test_fallback_cmd.py, 2 invariants)test_picker_failure_restores_persisted_primary_without_masking_error[exception|ctrl-c]— a picker that rewritesmodelthen raisesLookupError/KeyboardInterrupt: the same exception object propagates and the persistedmodelequals the pre-picker primary. Red on base (assert persisted["model"] == primary_model), green here.test_restore_preserves_absent_active_provider— snapshot+restore of a store withoutactive_providerleaves the key absent. Red on base, green here.The salvaged suite's mock-dispatch tests (asserting calls into our own helpers) were dropped in the follow-up commit; the existing side-effect-retention assertion (
custom_providerssurvives) is kept.scripts/run_tests.sh -j 1 tests/hermes_cli/test_fallback_cmd.py tests/hermes_cli/test_fallback_config.py→ 22 passed. ruff,check-windows-footguns.py --all,check_compat_pointers.py,git diff --checkclean.Limitations
hermes subagentpicker (feat(cli): add subagent model and reasoning picker #76480) shares this route-isolation concern; it is reviewed separately and does not depend on this PR.Credit / originals
Salvages #76497 by @Xipong (authorship preserved via cherry-pick; noreply email auto-maps). Fixes the contract in #85651. The sweeper review's one ask (a persisted-config failure-path test) was already addressed by the author in
4944099; this PR keeps that test and parametrizes it.Campaign tracker: #104154
Infographic