fix(cli): guard expensive startup model overrides - #70324
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for covering the direct cmd_chat startup path and the missing Nous billing phrase. The underlying gap is still present on current main: cmd_chat forwards the raw overrides into _launch_tui/cli_main without invoking expensive_model_warning (hermes_cli/main.py:2656-2701).
Problems
- The fail-closed behavior does not cover
-z/--oneshot. The parser explicitly accepts-m/--modeland--providerfor oneshot (hermes_cli/_parser.py:127-146), but main sends that path directly torun_oneshot(hermes_cli/main.py:12362-12371) rather thancmd_chat. A non-interactive-zcall can therefore still select the route this PR intends to prevent.
Suggested changes
- Share the startup guard with
run_oneshot(stderr only, so its stdout contract remains intact) and add a-zregression test for rejection plus a safe-model control case.
Automated hermes-sweeper review.
| @@ -2592,6 +2653,7 @@ def cmd_chat(args): | |||
| os.environ["HERMES_SESSION_SOURCE"] = args.source | |||
|
|
|||
| _pin_kanban_board_env() | |||
| _confirm_startup_expensive_model_override(args) | |||
There was a problem hiding this comment.
This only protects the cmd_chat route. -z/--oneshot accepts the same model/provider flags but dispatches directly to run_oneshot, so a non-interactive oneshot invocation can still select the expensive route without this guard. Please share the check with the oneshot path and add coverage there.
Run the expensive-model warning for explicit startup `-m` / `--provider` overrides before the chat loop starts, and fail closed for non-interactive invocations that select an expensive or known-confusing model. Also classify Nous paid-model 404s that say credits are required as billing exhaustion so they fail fast with billing guidance. Tested: - scripts/run_tests.sh tests/hermes_cli/test_cli_startup_model_cost_guard.py tests/hermes_cli/test_model_cost_guard.py tests/agent/test_error_classifier.py -- --tb=short -q
5a9dd18 to
31b1568
Compare
|
For the startup-override path: I rechecked the current diff against the earlier oneshot concern, and the guard is already on the top-level The current branch calls |
…over the light oneshot fast-path Follow-ups on top of the salvaged #70324: - _confirm_startup_expensive_model_override evaluates the unified registry (combined_selection_warning) so id-keyed guards like the data-training-tier warning fire at startup too, not just the cost guard. - The Termux-adjacent light oneshot fast-path (added after the PR branched) ran _run_and_exit_oneshot without the guard — same bug class, third sibling site now covered.
|
Merged via PR #85953 (#85953) — thanks @lkz-de! Your commit was cherry-picked onto current main with your authorship preserved in git log. On top of it we (a) routed the startup guard through the new unified selection-guard registry (#85917), so the data-training-tier warning also fires at startup, and (b) covered a third oneshot fast-path that was added to The "requires available credits" billing classification and the gpt-5.5-pro no-pricing suggestion both landed as-is. Nice, complete work. |
…rust and gpt-5.5-pro confusion nudge (#85970) 54cc39a (distrust foreign pricing for custom providers) tested with openai/gpt-5.5-pro fixtures; 83d373a (salvaged #70324) made that exact id warn unconditionally as a known-confusion model. Each was green alone; together the distrust tests fail on every main run (slice 6). Use a neutral fixture id for the distrust tests and add a regression test pinning the composed behavior: the id-keyed nudge survives custom-provider pricing distrust.
Summary
-m/--provideroverrides before the chat loop startsopenai/gpt-5.5-prowarning fire even when no pricing entry is available, so the Codexgpt-5.5suggestion still appearsWhy
Hermes already warns on interactive model switches before moving to very expensive models, including the easy-to-miss
openai/gpt-5.5-provs Codexgpt-5.5distinction. Direct startup flags did not pass through that interactive picker path, so a command-line invocation could choose the aggregator-styleopenai/gpt-5.5-promodel without seeing the same confirmation.That distinction matters because these are different provider routes, not just two spellings of the same model:
provider=openai-codexwith bare modelgpt-5.5uses the Codex OAuth route configured throughhermes auth add openai-codex.openai/gpt-5.5-pro.A user with Codex OAuth configured can therefore still accidentally send a startup invocation to a paid API/aggregator route if the CLI accepts a provider/model override without applying the same warning used by the interactive picker.
This PR closes that gap by checking explicit startup overrides before any provider call is constructed. Non-interactive invocations fail closed; interactive invocations must confirm before continuing. The guard also covers provider-only startup overrides by evaluating the effective configured default model.
The warning fallback and billing classifier keep the same failure path legible if an override does reach a provider: the known
openai/gpt-5.5-prosuggestion should appear even if pricing lookup has no entry, and a Nous “requires available credits” response for the misrouted paid model should fail fast as billing exhaustion rather than spend retries as an unknown 404.Testing
scripts/run_tests.sh tests/hermes_cli/test_cli_startup_model_cost_guard.py tests/hermes_cli/test_model_cost_guard.py tests/agent/test_error_classifier.py -- --tb=short -qCoverage added includes non-interactive startup rejection, interactive confirm/cancel/EOF handling, provider-only startup override evaluation,
openai/gpt-5.5-prowarning without pricing data, and Nous “requires available credits” billing classification.Overlap check
Related open PRs touch adjacent GPT-5.5 or model-picker behaviour, but none cover this startup-override path:
No open PR found for guarding startup
-m/--providerexpensive-model overrides or for classifying the Nous “requires available credits” 404 as billing exhaustion.