fix(cli): verify onboard models with real probes and harden the wizard#102
Open
0xKT wants to merge 9 commits into
Open
fix(cli): verify onboard models with real probes and harden the wizard#1020xKT wants to merge 9 commits into
0xKT wants to merge 9 commits into
Conversation
Onboard's EverOS memory setup reported a false green whenever an endpoint answered GET /models but could not serve the picked model (reusing a DeepSeek chat endpoint for embedding, or a memory-LLM model the endpoint does not offer). It also had inescapable navigation loops and an unstripped API key that could build an illegal Authorization header. - Replace the connectivity-only _probe_everos_endpoint with real capability probes: _probe_everos_chat (POST /chat/completions) for the memory LLM and _probe_everos_embedding (POST /embeddings) for embedding. Guard the JSON shape and catch httpx.InvalidURL so a malformed base_url never crashes the wizard; apply the same InvalidURL guard to _fetch_everos_models. - Embedding no longer fetches/shows the /models list (chat models on a reused chat endpoint mislead as embedding candidates); the id is entered directly, guided by an example and a capability hint, with the real probe as the arbiter. - Strip api_key / base_url / channel fields on input. - Gate _memory_enabled on both required models (llm and embedding). - Navigation: the model picker's empty submit falls back to the default instead of exiting the wizard; a required EverOS role offers a bounded "give up EverOS" exit (falls back to Markdown) instead of looping; reprint switch-provider guidance when the embedding step bounces back. - Render the capability hint with highlight=False so it does not read as an error. Co-authored-by: Claude (claude-opus-4-8) <noreply@anthropic.com>
…lure Custom OpenAI-compatible providers skipped the test message entirely (the highest-typo-risk case: a hand-typed base_url/model passed with no real request). They now get the same one-shot probe as every other provider, built from the stored config so a wrong base_url/model fails at setup, not at first chat. - _run_test_probe: on failure, offer Re-enter key and Switch provider (not just retry/repick/continue), matching the connectivity-failure menu; add allow_repick to drop the model option for custom (Switch re-enters both). - _resolve_model_with_test: run the probe for custom too; handle rekey/switch in both the custom and picked-model loops. Co-authored-by: Claude (claude-opus-4-8) <noreply@anthropic.com>
Onboard let several required gates slip: deleting every provider still advanced past the required Step 1; a required channel credential could be left empty and the channel enabled half-configured; and Ctrl+C on the Step 1/3 menus (and the memory enable/skip prompts) counted as "done"/"skip" rather than the documented "exit at any point". - Step 1 "Done" now requires at least one provider AND a default model. - Channel required fields re-prompt on empty instead of being dropped. - Ctrl+C (questionary None) exits at every menu, per the module contract. - Align the navigation docstring: Steps 3 and 4 are forward-only. Co-authored-by: Claude (claude-opus-4-8) <noreply@anthropic.com>
- A brand-new provider whose key was written but then failed verification is cleared when the user switches away, so a dead provider is not left listed as configured (a pre-existing provider is preserved). - Clarify --reset help: it re-runs the wizard over an existing config and does not erase it. - Recap warnings now point at re-running onboard (covers provider and memory) instead of raven doctor, which has no EverOS memory checks. Co-authored-by: Claude (claude-opus-4-8) <noreply@anthropic.com>
Non-interactive onboard always sent a real (billed) chat test message with no way to opt out, so CI / scripted installs incurred a call. Add --skip-test to skip the one-shot probe; connectivity (GET /models via test_provider) is still checked. Threaded from the CLI option through run_wizard -> _step1_provider -> _configure_one_provider -> _resolve_model_with_test. Co-authored-by: Claude (claude-opus-4-8) <noreply@anthropic.com>
_scancode_login enables the channel before driving the QR login and reverts on every failure/skip path. But KeyboardInterrupt (Ctrl+C mid-scan) is not an Exception subclass, so it skipped the revert and left the channel enabled but unauthenticated. Catch BaseException at the login call to disable it before letting the interrupt propagate. Co-authored-by: Claude (claude-opus-4-8) <noreply@anthropic.com>
Address the full-branch review (2 BLOCKs plus untested paths): - _scancode_login: wrap the whole flow in try/finally + a logged_in flag so ANY non-login exit reverts the enable, including Ctrl+C in the node-missing or retry/skip submenus (raises typer.Exit) which the prior narrow fix missed. - Thread --skip-test into Step 4: interactively enabling memory no longer fires the billed EverOS chat/embeddings probes when --skip-test is set. - A required EverOS role that already has a value returns to its keep/redo menu on back-out instead of being forced into the give-up exit. - Restore an existing provider's prior key when a re-configuration fails and the user switches away, so a failed edit no longer clobbers a working key. Co-authored-by: Claude (claude-opus-4-8) <noreply@anthropic.com>
Follow-up to the key rollback: snapshot and restore api_base alongside api_key, so reconfiguring an existing custom provider with a bad base_url and switching away doesn't leave the stale/broken base_url behind. Co-authored-by: Claude (claude-opus-4-8) <noreply@anthropic.com>
- Thread skip_test into _verify_provider so the pre-check skip note no longer promises a test message when --skip-test has skipped it. - Thread non_interactive through _scancode_login / _handle_missing_node / _add_one_channel instead of hardcoding False, so the node-missing and retry/skip submenus honor non-interactive mode. Co-authored-by: Claude (claude-opus-4-8) <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
1 participant
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.
Summary
Harden the
raven onboardwizard, which reported false "connected" results,could trap or crash mid-flow, and billed silently in CI. One file
(
raven/cli/onboard_commands.py) plus its tests; no schema/config changes.send a real
POST /chat/completions//embeddingsprobe (wasGET /models,which greenlit models the endpoint doesn't serve); custom providers are probed
too. Embedding takes the model id directly instead of showing a misleading
chat-model list. A malformed base_url no longer crashes (catches
InvalidURL).required EverOS role can give up EverOS (keep Markdown) instead of looping;
the main-model empty submit uses the default instead of exiting; Ctrl+C exits
every menu; Step 1 requires at least one provider and a default model;
required channel fields re-prompt on empty; scancode reverts the enable on any
non-login exit.
_memory_enabledrequires both llm and embedding; key / URL / channel inputsare stripped.
--skip-testskips the billed test call (connectivity is still checked); thecapability hint no longer renders like an error;
--resethelp and setupwarnings clarified.
Type
Verification
uv run pytest tests/test_cli_onboard_commands.py -q-> 88 passed; ruffcheck/format clean.
Real DeepSeek: probes reject wrong / chat-only models (HTTP 400 / 404) and
pass a valid one;
send_probereturns a real reply and raises on a bad model;a malformed base_url returns no list without crashing;
--skip-testskips thechat call while a control run sends one.
Relevant tests pass locally
Relevant lint / type checks pass locally
User-facing docs or screenshots are updated when needed
Risk
--skip-testdefaults off, so non-interactive behavior is unchanged; noschema/config changes; rollback is reverting the branch.
Related Issues
Fixes #81