From 251a7bdc2a580d6743724109f7fa323038ec922e Mon Sep 17 00:00:00 2001 From: KT Date: Wed, 8 Jul 2026 19:58:38 +0800 Subject: [PATCH 1/9] feat(cli): verify everos memory with real probes, harden onboard nav 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) --- raven/cli/onboard_commands.py | 244 ++++++++++++--- tests/test_cli_onboard_commands.py | 483 ++++++++++++++++++++++++++++- 2 files changed, 673 insertions(+), 54 deletions(-) diff --git a/raven/cli/onboard_commands.py b/raven/cli/onboard_commands.py index 9bae7c6..2aee08a 100644 --- a/raven/cli/onboard_commands.py +++ b/raven/cli/onboard_commands.py @@ -47,6 +47,10 @@ # screen; ``None`` from a picker means Ctrl+C (exit). _BACK = object() +# Sentinel a required EverOS role returns when the user chooses to give up EverOS +# rather than configure it; ``_step4_memory`` then falls back to Markdown memory. +_ABORT_EVEROS = object() + # Unified prompt chrome (display-only): no leading question glyph (drops # questionary's default "?"). A single-space qmark is rendered as one blank, # which — with questionary's own leading space — puts every prompt line on the @@ -311,8 +315,8 @@ def _bootstrap_empty_config() -> None: warning, never a crash), so an enabled-but-modelless install is safe. The wizard's Step 4 — and its skip / non-interactive guard — resolve the backend back to ``None`` when the user opts out or never configures the required - models (``_memory_enabled`` gates on the llm model being present, not just - the backend name). + models (``_memory_enabled`` gates on both required models (llm + embedding) + being present, not just the backend name). Seeding runs on EVERY onboard, not just a brand-new config: the writer is ``setdefault``-based (non-clobbering), so it backfills these blocks into a @@ -471,6 +475,7 @@ def _validate(v: str) -> Any: ).ask() if key is None: raise typer.Exit(1) + key = key.strip() if allow_back and key == "": return _BACK if not key: @@ -507,6 +512,7 @@ def _validate(v: str) -> Any: ).ask() if url is None: raise typer.Exit(1) + url = url.strip() if allow_back and url == "": return _BACK if not url: @@ -767,9 +773,17 @@ def _pick_model( qmark=_QMARK, ).ask() + if chosen is None: + raise typer.Exit(1) # Ctrl+C + chosen = chosen.strip() if not chosen: + # Empty submit (e.g. the prefilled default was cleared) falls back to the + # default rather than tearing down the wizard. The no-default branch + # validates non-empty, so an empty value only reaches here with a default. + if default_value: + return default_value raise typer.Exit(1) - return chosen.strip() + return chosen def _write_provider_fields(provider: str, fields: dict[str, Any]) -> None: @@ -1586,6 +1600,7 @@ def _prompt_channel_fields(channel: str) -> Any: ).ask() if value is None: raise typer.Exit(1) + value = value.strip() if allow_back and value == "": return _BACK if value: @@ -1992,16 +2007,17 @@ def _everos_section(section: str) -> dict[str, Any]: def _memory_enabled() -> bool: """True iff EverOS memory is both selected AND usable on disk. - "Usable" requires an llm model in the EverOS toml: the seed/schema default - sets ``memory.backend="everos"`` before any models exist, so a bare backend - check would mis-report a fresh, modelless install as "enabled" and make - Step 4 offer "keep current" over a non-functional setup. Gating on the llm - model keeps the wizard's enabled-detection aligned with "actually works". + "Usable" requires both required models (llm and embedding) in the EverOS + toml: the seed/schema default sets ``memory.backend="everos"`` before any + models exist, so a bare backend check would mis-report a fresh, modelless + install as "enabled". Both roles are ``optional: False``; gating on either + alone would treat a half-configured setup (e.g. llm written but embedding + skipped) as enabled and offer "keep current" over a non-functional memory. """ data = _load_raw_config() if (data.get("memory") or {}).get("backend") != "everos": return False - return bool(_everos_section("llm").get("model")) + return bool(_everos_section("llm").get("model") and _everos_section("embedding").get("model")) # Providers whose main model can be reused as the EverOS memory LLM: they @@ -2122,32 +2138,64 @@ def _prompt_text(label: str, *, secret: bool = False, default: str = "", allow_b return value -def _probe_everos_endpoint( - label: str, *, model: Optional[str], api_key: Optional[str], base_url: Optional[str] +def _probe_everos_chat(model: Optional[str], *, api_key: Optional[str], base_url: Optional[str]) -> tuple[bool, str]: + """Real capability probe for a memory-LLM endpoint: ``POST + {base_url}/chat/completions`` once and confirm a choice comes back. Unlike a + bare ``GET /models`` connectivity check, this exercises the picked model, so + an endpoint that lists models but doesn't serve the chosen id fails here + instead of reporting a false green. Provider-agnostic; never raises.""" + import httpx + + if not base_url: + return False, "no base_url configured" + url = base_url.rstrip("/") + ("/chat/completions" if "/v1" in base_url else "/v1/chat/completions") + headers = {"Authorization": f"Bearer {api_key}"} if api_key else {} + body = {"model": model, "messages": [{"role": "user", "content": "ping"}], "max_tokens": 1} + try: + with httpx.Client(timeout=15) as client: + resp = client.post(url, headers=headers, json=body) + if resp.status_code != 200: + return False, f"HTTP {resp.status_code}: {resp.text[:200]}" + data = resp.json() + except (httpx.HTTPError, httpx.InvalidURL, ValueError) as exc: + return False, f"probe failed: {exc}" + choices = data.get("choices") if isinstance(data, dict) else None + if isinstance(choices, list) and choices and isinstance(choices[0], dict): + return True, "ok" + return False, "endpoint returned no completion" + + +def _probe_everos_embedding( + model: Optional[str], *, api_key: Optional[str], base_url: Optional[str] ) -> tuple[bool, str]: - """Lightweight ``GET {base_url}/models`` connectivity check for an EverOS - model endpoint. Returns ``(ok, detail)``; never raises.""" + """Real capability probe for an embedding endpoint: ``POST + {base_url}/embeddings`` once and confirm a vector comes back. Catches + endpoints that answer ``GET /models`` but can't actually embed (e.g. a + chat-only provider). Provider-agnostic; never raises.""" import httpx if not base_url: return False, "no base_url configured" - url = base_url.rstrip("/") + "/models" - if "/v1" not in base_url: - url = base_url.rstrip("/") + "/v1/models" + url = base_url.rstrip("/") + ("/embeddings" if "/v1" in base_url else "/v1/embeddings") headers = {"Authorization": f"Bearer {api_key}"} if api_key else {} try: - with httpx.Client(timeout=10) as client: - resp = client.get(url, headers=headers) - except httpx.HTTPError as exc: - return False, f"network error: {exc}" - if resp.status_code == 200: + with httpx.Client(timeout=15) as client: + resp = client.post(url, headers=headers, json={"model": model, "input": "ping"}) + if resp.status_code != 200: + return False, f"HTTP {resp.status_code}: {resp.text[:200]}" + data = resp.json() + except (httpx.HTTPError, httpx.InvalidURL, ValueError) as exc: + return False, f"probe failed: {exc}" + items = data.get("data") if isinstance(data, dict) else None + if isinstance(items, list) and items and isinstance(items[0], dict) and "embedding" in items[0]: return True, "ok" - return False, f"HTTP {resp.status_code}" + return False, "endpoint returned no embedding vector" def _verify_everos_model( label: str, *, + section: str, model: Optional[str], api_key: Optional[str], base_url: Optional[str], @@ -2163,14 +2211,17 @@ def _verify_everos_model( ``continue_hint`` (en, zh) spells out the consequence of continuing anyway. """ console.print(_t(f" [dim]⏳ Verifying {label}…[/dim]", f" [dim]⏳ 正在验证 {label}…[/dim]")) - ok, detail = _probe_everos_endpoint(label, model=model, api_key=api_key, base_url=base_url) + if section == "embedding": + ok, detail = _probe_everos_embedding(model, api_key=api_key, base_url=base_url) + else: + ok, detail = _probe_everos_chat(model, api_key=api_key, base_url=base_url) if ok: console.print(_t(f" [green]✓ {label} connected.[/green]", f" [green]✓ {label} 连接成功。[/green]")) return True console.print( _t( - f" [yellow]✗ Couldn't reach {label}: {detail}[/yellow]", - f" [yellow]✗ 连不上 {label}:{detail}[/yellow]", + f" [yellow]✗ Couldn't verify {label}: {detail}[/yellow]", + f" [yellow]✗ 验证失败 {label}:{detail}[/yellow]", ) ) if continue_hint: @@ -2252,6 +2303,13 @@ def _verify_everos_model( "把文字转成向量,存入记忆库并在检索时按「意思」匹配,而不只是关键词", ), "continue_hint": ("semantic recall will be unavailable", "语义召回将不可用"), + "hint": ( + "Needs an embedding-capable provider — can differ from your memory LLM. " + "Known-good: OpenAI (text-embedding-3-small), SiliconFlow, DashScope. " + "A chat-only endpoint (e.g. DeepSeek) won't serve embeddings.", + "需要支持 embedding 的服务商 —— 可与记忆 LLM 不同。已知可用:OpenAI" + "(text-embedding-3-small)、SiliconFlow、DashScope。纯对话端点(如 DeepSeek)不提供 embedding。", + ), }, "rerank": { "label": ("Memory rerank", "记忆 rerank"), @@ -2301,7 +2359,7 @@ def _fetch_everos_models(base_url: Optional[str], api_key: Optional[str]) -> Opt if resp.status_code != 200: return None data = resp.json() - except (httpx.HTTPError, ValueError): + except (httpx.HTTPError, httpx.InvalidURL, ValueError): return None items = data.get("data") if isinstance(data, dict) else None if not isinstance(items, list): @@ -2310,14 +2368,42 @@ def _fetch_everos_models(base_url: Optional[str], api_key: Optional[str]) -> Opt return sorted(ids) or None -def _everos_pick_model(*, base_url: Optional[str], api_key: Optional[str], example: str, allow_back: bool) -> Any: - """Pick a model id for an EverOS endpoint: fetch ``/models`` for a - fuzzy-searchable list, else fall back to free text. Empty submit = back.""" +def _print_embedding_switch_hint() -> None: + """Nudge shown whenever the embedding step bounces back to the source picker + (empty model submit or a failed ``/embeddings`` verify): the current endpoint + likely can't embed, so switch provider or give up EverOS via Back.""" + console.print( + _t( + " [dim]No embedding model here? This endpoint likely doesn't serve embeddings — " + "pick an embedding-capable provider (OpenAI / SiliconFlow / DashScope), " + "or choose Back to give up EverOS.[/dim]", + " [dim]这里没有可用的 embedding 模型?该端点很可能不提供 embedding —— " + "请改选支持 embedding 的服务商(OpenAI / SiliconFlow / DashScope)," + "或选「返回」放弃 EverOS。[/dim]", + ), + highlight=False, + ) + + +def _everos_pick_model( + *, base_url: Optional[str], api_key: Optional[str], example: str, allow_back: bool, fetch_list: bool = True +) -> Any: + """Pick a model id for an EverOS endpoint. With ``fetch_list`` (the default), + fetch ``/models`` for a fuzzy-searchable list, else free text. Empty submit = + back. + + ``fetch_list=False`` is used for embedding: ``/models`` lists the endpoint's + chat models (on a reused chat endpoint they're all chat), which is misleading + as embedding candidates — so the caller enters the embedding id directly, + guided by the example and the role's capability hint, and the real + ``/embeddings`` verify is the arbiter.""" questionary = _require_questionary() from raven.cli._styles import RAVEN_STYLE - console.print(_t(" [dim]⏳ Loading models…[/dim]", " [dim]⏳ 正在拉取模型列表…[/dim]")) - models = _fetch_everos_models(base_url, api_key) + models = None + if fetch_list: + console.print(_t(" [dim]⏳ Loading models…[/dim]", " [dim]⏳ 正在拉取模型列表…[/dim]")) + models = _fetch_everos_models(base_url, api_key) if models: chosen = questionary.autocomplete( _t( @@ -2332,12 +2418,13 @@ def _everos_pick_model(*, base_url: Optional[str], api_key: Optional[str], examp qmark=_QMARK, ).ask() else: - console.print( - _t( - " [dim]Couldn't list models from this endpoint — type the id manually.[/dim]", - " [dim]该端点拉不到模型列表 — 请手动输入模型 id。[/dim]", + if fetch_list: + console.print( + _t( + " [dim]Couldn't list models from this endpoint — type the id manually.[/dim]", + " [dim]该端点拉不到模型列表 — 请手动输入模型 id。[/dim]", + ) ) - ) chosen = questionary.text( _t(f"Model id (e.g. {example}):", f"模型 id(如 {example}):"), placeholder=_back_placeholder(allow_back), @@ -2475,8 +2562,19 @@ def _everos_pick_creds_and_model( if rerank_provider is _BACK: continue - model = _everos_pick_model(base_url=base_url, api_key=api_key, example=example, allow_back=True) + model = _everos_pick_model( + base_url=base_url, + api_key=api_key, + example=example, + allow_back=True, + fetch_list=section != "embedding", + ) if model is _BACK: + # Back-out lands on the source picker again; for embedding, nudge the + # user to switch provider (Back gives up EverOS) so re-picking the same + # chat-only reuse endpoint isn't a silent dead end. + if section == "embedding": + _print_embedding_switch_hint() continue result: dict[str, Any] = {"model": model, "api_key": api_key, "base_url": base_url} @@ -2485,9 +2583,12 @@ def _everos_pick_creds_and_model( return result -def _config_everos_role(*, section: str, main_model: Optional[str], non_interactive: bool, warnings: list[str]) -> None: +def _config_everos_role(*, section: str, main_model: Optional[str], non_interactive: bool, warnings: list[str]) -> Any: """Configure one EverOS memory role (llm / embedding / rerank / multimodal) - with the unified provider→key→model flow, reuse shortcuts, and a back loop.""" + with the unified provider→key→model flow, reuse shortcuts, and a back loop. + + Returns ``None`` normally; returns ``_ABORT_EVEROS`` when the user gives up a + required role (the caller then disables EverOS and keeps Markdown memory).""" questionary = _require_questionary() from raven.cli._styles import RAVEN_STYLE from raven.config.update_everos import clear_everos_section, set_everos_section @@ -2502,16 +2603,24 @@ def _config_everos_role(*, section: str, main_model: Optional[str], non_interact # Header sits on the 2-space info column (bold accent); the purpose nests # one line under it (dim), matching the layout system used everywhere else. tag = _t("optional", "可选") + hint = role.get("hint") + hint_en = f"\n [dim]{hint[0]}[/dim]" if hint else "" + hint_zh = f"\n [dim]{hint[1]}[/dim]" if hint else "" console.print() + # highlight=False so Rich's default highlighter doesn't tint the dim prose + # (parens/numbers/words) and make an informational hint read like an error. console.print( _t( f" [bold #fbe23f]{label_en}[/bold #fbe23f]" + (f" [dim]({tag})[/dim]" if optional else "") - + f"\n [dim]{purpose_en}[/dim]", + + f"\n [dim]{purpose_en}[/dim]" + + hint_en, f" [bold #fbe23f]{label_zh}[/bold #fbe23f]" + (f" [dim]({tag})[/dim]" if optional else "") - + f"\n [dim]{purpose_zh}[/dim]", - ) + + f"\n [dim]{purpose_zh}[/dim]" + + hint_zh, + ), + highlight=False, ) while True: # role-menu loop — a back-out of the source picker returns here @@ -2560,11 +2669,36 @@ def _config_everos_role(*, section: str, main_model: Optional[str], non_interact non_interactive=non_interactive, ) if result is _BACK: - continue # back to the role menu + if optional: + continue # optional role: re-show the role menu (it offers Skip) + # A required role has no Skip, so backing out of the picker would loop + # forever. Offer a bounded exit: keep trying, or give up EverOS and + # fall back to Markdown memory. + action = questionary.select( + _t( + f"{label_en} is required for EverOS memory. What would you like to do?", + f"{label_zh} 是 EverOS 记忆必需的。想做什么?", + ), + choices=[ + questionary.Choice(_t("Pick a provider / model", "选择服务商 / 模型"), value="retry"), + questionary.Choice( + _t("Give up EverOS (use native Markdown memory)", "放弃 EverOS(改用原生 Markdown 记忆)"), + value="abort", + ), + ], + style=RAVEN_STYLE, + qmark=_QMARK, + ).ask() + if action is None: + raise typer.Exit(1) + if action == "retry": + continue + return _ABORT_EVEROS if role["verify"]: ok = _verify_everos_model( verify_label, + section=section, model=result["model"], api_key=result["api_key"], base_url=result["base_url"], @@ -2575,7 +2709,12 @@ def _config_everos_role(*, section: str, main_model: Optional[str], non_interact else: ok = True if not ok: - continue # "Re-enter" on a failed probe → back to the role menu + # "Re-enter" on a failed probe → back to the role menu. For embedding, + # a failed /embeddings verify usually means the endpoint can't embed; + # reprint the switch-provider nudge so the retry isn't a blind loop. + if section == "embedding": + _print_embedding_switch_hint() + continue set_everos_section(section, result) console.print( @@ -2593,8 +2732,8 @@ def _step4_memory(*, skip: bool, non_interactive: bool, main_model: Optional[str The bootstrap seeds ``memory.backend="everos"`` (schema default), so this step's job is to either confirm it by configuring the required models or resolve it back to ``None`` (native Markdown) on skip / non-interactive / - decline. ``_memory_enabled`` gates on the llm model being present, so a - fresh modelless seed is treated as "not yet enabled" (the user still sees + decline. ``_memory_enabled`` gates on both required models (llm + embedding) + being present, so a fresh modelless seed is treated as "not yet enabled" (the user still sees the enable prompt) and the "keep current" path only triggers once models are actually on disk. """ @@ -2603,7 +2742,7 @@ def _step4_memory(*, skip: bool, non_interactive: bool, main_model: Optional[str if skip or non_interactive: # Never configured the required models here → disable backend-driven # memory so runtime doesn't activate EverOS without an llm/embedding. - # (``_memory_enabled`` already gates on the llm model, so an + # (``_memory_enabled`` already gates on both required models, so an # already-enabled+configured setup is preserved.) if not _memory_enabled(): _set_memory_backend(None) @@ -2675,12 +2814,21 @@ def _step4_memory(*, skip: bool, non_interactive: bool, main_model: Optional[str for _role in ("llm", "embedding", "rerank", "multimodal"): # Each role prints one leading blank before its own header, so no extra # separator here — avoids the double blank line between roles. - _config_everos_role( + outcome = _config_everos_role( section=_role, main_model=main_model, non_interactive=non_interactive, warnings=warnings, ) + if outcome is _ABORT_EVEROS: + _set_memory_backend(None) + console.print( + _t( + " [dim]Gave up EverOS; keeping native Markdown memory.[/dim]", + " [dim]已放弃 EverOS,改用原生 Markdown 记忆。[/dim]", + ) + ) + return None _set_memory_backend("everos") return None diff --git a/tests/test_cli_onboard_commands.py b/tests/test_cli_onboard_commands.py index b3b0852..425b888 100644 --- a/tests/test_cli_onboard_commands.py +++ b/tests/test_cli_onboard_commands.py @@ -197,8 +197,8 @@ def test_onboard_non_interactive_skips_optional_steps( ``everos_isolated`` keeps ``_memory_enabled`` from reading the dev machine's real ``~/.everos/config.toml``: the seeded backend="everos" is - only kept when an llm model is configured, so an empty (isolated) EverOS - config makes the skip-guard deterministically resolve it back to None. + only kept when both required models (llm + embedding) are configured, so an + empty (isolated) EverOS config makes the skip-guard resolve it back to None. """ r = runner.invoke( app, @@ -982,9 +982,10 @@ def ask(self): monkeypatch.setattr(questionary, "select", lambda *a, **kw: _FQ(next(select_answers))) monkeypatch.setattr(questionary, "text", lambda *a, **kw: _FQ(next(text_answers))) monkeypatch.setattr(questionary, "password", lambda *a, **kw: _FQ(next(password_answers))) - # No network: model list can't be fetched → free-text entry; probe succeeds. + # No network: model list can't be fetched → free-text entry; probes succeed. monkeypatch.setattr(onboard_commands, "_fetch_everos_models", lambda *a, **kw: None) - monkeypatch.setattr(onboard_commands, "_probe_everos_endpoint", lambda *a, **kw: (True, "ok")) + monkeypatch.setattr(onboard_commands, "_probe_everos_chat", lambda *a, **kw: (True, "ok")) + monkeypatch.setattr(onboard_commands, "_probe_everos_embedding", lambda *a, **kw: (True, "ok")) onboard_commands._step4_memory( skip=False, @@ -1012,6 +1013,476 @@ def ask(self): assert "multimodal" not in everos +class _FakeResp: + def __init__(self, status_code: int, payload, text: str = ""): + self.status_code = status_code + self._payload = payload + self.text = text + + def json(self): + if isinstance(self._payload, Exception): + raise self._payload + return self._payload + + +class _FakeClient: + def __init__(self, resp): + self._resp = resp + + def __enter__(self): + return self + + def __exit__(self, *a): + return False + + def post(self, *a, **kw): + if isinstance(self._resp, Exception): + raise self._resp + return self._resp + + +def test_probe_everos_chat_parsing(monkeypatch: pytest.MonkeyPatch) -> None: + """The chat probe never raises and only reports success on a real completion: + a non-dict body, missing/empty choices, non-200, non-JSON, and network errors + all read as failure.""" + import httpx + + def _probe(resp): + monkeypatch.setattr(httpx, "Client", lambda *a, **kw: _FakeClient(resp)) + return onboard_commands._probe_everos_chat("m", api_key="k", base_url="https://x/v1") + + assert _probe(_FakeResp(200, {"choices": [{"message": {"content": "hi"}}]})) == (True, "ok") + assert _probe(_FakeResp(200, {"choices": []}))[0] is False + # A non-dict choice item (e.g. an error string) is not a false green. + assert _probe(_FakeResp(200, {"choices": ["error: model not found"]}))[0] is False + assert _probe(_FakeResp(200, []))[0] is False # top-level non-dict, no crash + ok, detail = _probe(_FakeResp(400, None, text="model not found")) + assert ok is False and "400" in detail + assert _probe(_FakeResp(200, ValueError("no json")))[0] is False + assert _probe(httpx.HTTPError("boom"))[0] is False + + +def test_probe_everos_malformed_url_never_raises() -> None: + """A malformed base_url raises ``httpx.InvalidURL``, which is not an + ``HTTPError`` subclass — both probes must catch it and report failure rather + than crash the wizard (honoring the "never raises" contract).""" + assert onboard_commands._probe_everos_chat("m", api_key="k", base_url="http://[::1")[0] is False + assert onboard_commands._probe_everos_embedding("m", api_key="k", base_url="http://[::1")[0] is False + # The model-list fetch (used by the llm/rerank/multimodal pickers) shares the + # contract: a malformed base_url must read as "no list", not crash. + assert onboard_commands._fetch_everos_models("http://[::1", "k") is None + + +def test_probe_everos_embedding_parsing(monkeypatch: pytest.MonkeyPatch) -> None: + """The embedding probe never raises and only reports success on a real vector: + an error string containing "embedding", a non-dict item, a non-dict/non-JSON + body all read as failure, not crash.""" + import httpx + + def _probe(resp): + monkeypatch.setattr(httpx, "Client", lambda *a, **kw: _FakeClient(resp)) + return onboard_commands._probe_everos_embedding("m", api_key="k", base_url="https://x/v1") + + assert _probe(_FakeResp(200, {"data": [{"embedding": [0.1, 0.2]}]})) == (True, "ok") + # A chat endpoint whose error text contains "embedding" is not a false green. + assert _probe(_FakeResp(200, {"data": ["embedding not supported for chat models"]}))[0] is False + assert _probe(_FakeResp(200, {"data": [42]}))[0] is False # non-dict item, no crash + assert _probe(_FakeResp(200, []))[0] is False # top-level non-dict, no crash + ok, detail = _probe(_FakeResp(400, None, text="bad model")) + assert ok is False and "400" in detail + assert _probe(_FakeResp(200, ValueError("no json")))[0] is False + assert _probe(httpx.HTTPError("boom"))[0] is False + + +def test_memory_enabled_requires_both_llm_and_embedding(tmp_env: Path, everos_isolated: Path) -> None: + """A half-configured EverOS (llm only, embedding missing) is not "enabled" — + both required roles must be on disk.""" + from raven.config.update_everos import set_everos_section + + onboard_commands._set_memory_backend("everos") + set_everos_section("llm", {"model": "m-llm", "api_key": "k", "base_url": "https://x/v1"}) + assert onboard_commands._memory_enabled() is False + set_everos_section("embedding", {"model": "m-emb", "api_key": "k", "base_url": "https://x/v1"}) + assert onboard_commands._memory_enabled() is True + + +def test_prompt_api_key_strips_whitespace(monkeypatch: pytest.MonkeyPatch) -> None: + """A pasted key with surrounding whitespace/newline is stripped, so it can't + produce an illegal ``Authorization: Bearer \\nsk-...`` header downstream.""" + import questionary + + class _FQ: + def ask(self): + return " sk-abc12345\n" + + monkeypatch.setattr(questionary, "password", lambda *a, **kw: _FQ()) + assert onboard_commands._prompt_api_key("openai") == "sk-abc12345" + + +def test_prompt_base_url_strips_whitespace(monkeypatch: pytest.MonkeyPatch) -> None: + """Base URL input is stripped of surrounding whitespace/newline.""" + import questionary + + class _FQ: + def ask(self): + return " https://host/v1 \n" + + monkeypatch.setattr(questionary, "text", lambda *a, **kw: _FQ()) + assert onboard_commands._prompt_base_url() == "https://host/v1" + + +def test_memory_llm_probe_failure_reprompts_then_persists( + tmp_env: Path, everos_isolated: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A model the endpoint doesn't serve fails the real chat probe; Re-enter + loops back and only a probe-passing model is persisted (no false green).""" + import tomllib + + from raven.config.update_providers import set_provider_fields + + set_provider_fields("openai", {"api_key": "sk-main", "api_base": "https://api.openai.com/v1"}) + + import questionary + + class _FQ: + def __init__(self, a): + self._a = a + + def ask(self): + return self._a + + # picker -> reuse main model; failure menu -> Re-enter; picker -> reuse again. + select_answers = iter([("reuse_main",), "rekey", ("reuse_main",)]) + probe_results = iter([(False, "model not served by this endpoint"), (True, "ok")]) + monkeypatch.setattr(questionary, "select", lambda *a, **kw: _FQ(next(select_answers))) + monkeypatch.setattr(onboard_commands, "_probe_everos_chat", lambda *a, **kw: next(probe_results)) + + warnings: list[str] = [] + onboard_commands._config_everos_role( + section="llm", main_model="openai/gpt-4o-mini", non_interactive=False, warnings=warnings + ) + with everos_isolated.open("rb") as f: + everos = tomllib.load(f) + assert everos["llm"]["model"] == "gpt-4o-mini" + assert warnings == [] + + +def test_memory_embedding_probe_failure_rejects_and_reprompts( + tmp_env: Path, everos_isolated: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A chat-only endpoint fails the real embedding probe; Re-enter loops back + and only a probe-passing model is persisted (no false green).""" + import tomllib + + from raven.config.update_everos import set_everos_section + + set_everos_section("llm", {"model": "m", "api_key": "k-llm", "base_url": "https://llm/v1"}) + + import questionary + + class _FQ: + def __init__(self, a): + self._a = a + + def ask(self): + return self._a + + select_answers = iter([("reuse_llm",), "rekey", ("reuse_llm",)]) + text_answers = iter(["deepseek-chat", "text-embedding-3-small"]) + probe_results = iter([(False, "model does not support embeddings"), (True, "ok")]) + monkeypatch.setattr(questionary, "select", lambda *a, **kw: _FQ(next(select_answers))) + monkeypatch.setattr(questionary, "text", lambda *a, **kw: _FQ(next(text_answers))) + monkeypatch.setattr(onboard_commands, "_fetch_everos_models", lambda *a, **kw: None) + monkeypatch.setattr(onboard_commands, "_probe_everos_embedding", lambda *a, **kw: next(probe_results)) + + warnings: list[str] = [] + onboard_commands._config_everos_role( + section="embedding", main_model="deepseek/deepseek-chat", non_interactive=False, warnings=warnings + ) + with everos_isolated.open("rb") as f: + everos = tomllib.load(f) + assert everos["embedding"]["model"] == "text-embedding-3-small" + assert warnings == [] + + +def test_memory_embedding_probe_failure_continue_records_warning( + tmp_env: Path, everos_isolated: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Continue-anyway on a failed embedding probe persists but flags the endpoint + in ``warnings`` (the old connectivity check silently reported a false green).""" + import tomllib + + from raven.config.update_everos import set_everos_section + + set_everos_section("llm", {"model": "m", "api_key": "k-llm", "base_url": "https://llm/v1"}) + + import questionary + + class _FQ: + def __init__(self, a): + self._a = a + + def ask(self): + return self._a + + select_answers = iter([("reuse_llm",), "continue"]) + monkeypatch.setattr(questionary, "select", lambda *a, **kw: _FQ(next(select_answers))) + monkeypatch.setattr(questionary, "text", lambda *a, **kw: _FQ("deepseek-chat")) + monkeypatch.setattr(onboard_commands, "_fetch_everos_models", lambda *a, **kw: None) + monkeypatch.setattr( + onboard_commands, "_probe_everos_embedding", lambda *a, **kw: (False, "model does not support embeddings") + ) + + warnings: list[str] = [] + onboard_commands._config_everos_role( + section="embedding", main_model="deepseek/deepseek-chat", non_interactive=False, warnings=warnings + ) + assert warnings == ["Memory embedding"] + with everos_isolated.open("rb") as f: + everos = tomllib.load(f) + assert everos["embedding"]["model"] == "deepseek-chat" + + +def test_memory_embedding_step_shows_capability_hint( + tmp_env: Path, everos_isolated: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The embedding step prints the capability-guidance line so a chat-only-LLM + user is told upfront to pick an embedding-capable provider.""" + from raven.config.update_everos import set_everos_section + + set_everos_section("llm", {"model": "m", "api_key": "k-llm", "base_url": "https://llm/v1"}) + + import questionary + + class _FQ: + def __init__(self, a): + self._a = a + + def ask(self): + return self._a + + monkeypatch.setattr(questionary, "select", lambda *a, **kw: _FQ(("reuse_llm",))) + monkeypatch.setattr(questionary, "text", lambda *a, **kw: _FQ("text-embedding-3-small")) + monkeypatch.setattr(onboard_commands, "_fetch_everos_models", lambda *a, **kw: None) + monkeypatch.setattr(onboard_commands, "_probe_everos_embedding", lambda *a, **kw: (True, "ok")) + + printed: list[str] = [] + monkeypatch.setattr(onboard_commands.console, "print", lambda *a, **kw: printed.append(" ".join(str(x) for x in a))) + onboard_commands._config_everos_role( + section="embedding", main_model="deepseek/deepseek-chat", non_interactive=False, warnings=[] + ) + blob = "\n".join(printed) + assert "embedding-capable" in blob + assert "DashScope" in blob + + +def test_pick_model_empty_submit_uses_default_not_exit(monkeypatch: pytest.MonkeyPatch) -> None: + """Clearing the prefilled default and submitting empty falls back to the + default model rather than killing the whole wizard.""" + import questionary + + from raven.providers.registry import find_by_name + + spec = find_by_name("deepseek") + + class _FQ: + def ask(self): + return "" # user cleared the prefilled default, then hit Enter + + monkeypatch.setattr(questionary, "text", lambda *a, **kw: _FQ()) + result = onboard_commands._pick_model( + spec, current_model=None, model_ids=None, user_provided_model=None, non_interactive=False + ) + assert result == "deepseek/deepseek-chat" + + +def test_pick_model_ctrl_c_exits(monkeypatch: pytest.MonkeyPatch) -> None: + """Ctrl+C (questionary returns None) still exits — only empty submit is + softened to the default.""" + import questionary + import typer + + from raven.providers.registry import find_by_name + + spec = find_by_name("deepseek") + + class _FQ: + def ask(self): + return None + + monkeypatch.setattr(questionary, "text", lambda *a, **kw: _FQ()) + with pytest.raises(typer.Exit): + onboard_commands._pick_model( + spec, current_model=None, model_ids=None, user_provided_model=None, non_interactive=False + ) + + +def test_config_everos_role_required_giveup_returns_abort( + tmp_env: Path, everos_isolated: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A required role (embedding) backing out of the picker is not an + inescapable loop: it offers a bounded 'give up EverOS' exit that signals + ``_ABORT_EVEROS`` up to the caller.""" + from raven.config.update_everos import set_everos_section + + set_everos_section("llm", {"model": "m", "api_key": "k-llm", "base_url": "https://llm/v1"}) + + import questionary + + class _FQ: + def __init__(self, a): + self._a = a + + def ask(self): + return self._a + + # source picker -> Back (_BACK); required-role bounded-exit prompt -> abort. + select_answers = iter([onboard_commands._BACK, "abort"]) + monkeypatch.setattr(questionary, "select", lambda *a, **kw: _FQ(next(select_answers))) + out = onboard_commands._config_everos_role( + section="embedding", main_model="deepseek/deepseek-chat", non_interactive=False, warnings=[] + ) + assert out is onboard_commands._ABORT_EVEROS + + +def test_step4_giveup_embedding_keeps_markdown( + tmp_env: Path, everos_isolated: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Giving up a required role mid-step disables EverOS and keeps Markdown + memory rather than flipping the backend on half-configured.""" + import questionary + + class _FQ: + def __init__(self, a): + self._a = a + + def ask(self): + return self._a + + monkeypatch.setattr(questionary, "select", lambda *a, **kw: _FQ("on")) # enable EverOS + + def _fake_role(*, section, **kw): + return onboard_commands._ABORT_EVEROS if section == "embedding" else None + + monkeypatch.setattr(onboard_commands, "_config_everos_role", _fake_role) + onboard_commands._step4_memory(skip=False, non_interactive=False, main_model="deepseek/deepseek-chat", warnings=[]) + from raven.config.raven import load_raven_config + + assert load_raven_config().memory.backend != "everos" + + +def test_memory_embedding_backout_prints_switch_guidance( + tmp_env: Path, everos_isolated: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Empty-submitting the embedding model picker prints actionable guidance + every round (not a silent bounce), so re-picking the same chat-only reuse + endpoint isn't a dead end.""" + from raven.config.update_everos import set_everos_section + + set_everos_section("llm", {"model": "m", "api_key": "k-llm", "base_url": "https://llm/v1"}) + + import questionary + + class _FQ: + def __init__(self, a): + self._a = a + + def ask(self): + return self._a + + # source picker -> reuse llm; (model empty -> back, guidance printed) -> source + # picker -> Back; required-role bounded exit -> abort. + select_answers = iter([("reuse_llm",), onboard_commands._BACK, "abort"]) + monkeypatch.setattr(questionary, "select", lambda *a, **kw: _FQ(next(select_answers))) + monkeypatch.setattr(questionary, "text", lambda *a, **kw: _FQ("")) # empty model submit -> _BACK + monkeypatch.setattr(onboard_commands, "_fetch_everos_models", lambda *a, **kw: None) + + printed: list[str] = [] + monkeypatch.setattr(onboard_commands.console, "print", lambda *a, **kw: printed.append(" ".join(str(x) for x in a))) + out = onboard_commands._config_everos_role( + section="embedding", main_model="deepseek/deepseek-chat", non_interactive=False, warnings=[] + ) + blob = "\n".join(printed) + assert "embedding-capable" in blob + assert out is onboard_commands._ABORT_EVEROS + + +def test_memory_embedding_verify_fail_reprints_switch_hint( + tmp_env: Path, everos_isolated: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A failed /embeddings verify reprints the switch-provider nudge before + looping back, so a retry isn't a blind loop.""" + from raven.config.update_everos import set_everos_section + + set_everos_section("llm", {"model": "m", "api_key": "k-llm", "base_url": "https://llm/v1"}) + + import questionary + + class _FQ: + def __init__(self, a): + self._a = a + + def ask(self): + return self._a + + # source -> reuse llm; verify fails -> failure menu "rekey"; source -> Back; + # required-role bounded exit -> abort. + select_answers = iter([("reuse_llm",), "rekey", onboard_commands._BACK, "abort"]) + monkeypatch.setattr(questionary, "select", lambda *a, **kw: _FQ(next(select_answers))) + monkeypatch.setattr(questionary, "text", lambda *a, **kw: _FQ("text-embedding-3-small")) + monkeypatch.setattr(onboard_commands, "_probe_everos_embedding", lambda *a, **kw: (False, "HTTP 404: ")) + + printed: list[str] = [] + monkeypatch.setattr(onboard_commands.console, "print", lambda *a, **kw: printed.append(" ".join(str(x) for x in a))) + out = onboard_commands._config_everos_role( + section="embedding", main_model="deepseek/deepseek-chat", non_interactive=False, warnings=[] + ) + blob = "\n".join(printed) + assert "embedding-capable" in blob + assert out is onboard_commands._ABORT_EVEROS + + +def test_memory_embedding_skips_model_list_fetch( + tmp_env: Path, everos_isolated: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Embedding never fetches/shows the ``/models`` list: on a reused chat + endpoint those are chat models, misleading as embedding candidates. The id is + entered directly and the ``/embeddings`` verify is the arbiter.""" + import tomllib + + from raven.config.update_everos import set_everos_section + + set_everos_section("llm", {"model": "m", "api_key": "k-llm", "base_url": "https://llm/v1"}) + + import questionary + + class _FQ: + def __init__(self, a): + self._a = a + + def ask(self): + return self._a + + fetch_calls: list[bool] = [] + + def _spy_fetch(*a, **kw): + fetch_calls.append(True) + return ["deepseek-chat", "deepseek-reasoner"] # even if it would return, must not be called + + monkeypatch.setattr(onboard_commands, "_fetch_everos_models", _spy_fetch) + monkeypatch.setattr(questionary, "select", lambda *a, **kw: _FQ(("reuse_llm",))) + monkeypatch.setattr(questionary, "text", lambda *a, **kw: _FQ("text-embedding-3-small")) + monkeypatch.setattr(onboard_commands, "_probe_everos_embedding", lambda *a, **kw: (True, "ok")) + + onboard_commands._config_everos_role( + section="embedding", main_model="deepseek/deepseek-chat", non_interactive=False, warnings=[] + ) + assert fetch_calls == [] # embedding must not fetch the /models list + with everos_isolated.open("rb") as f: + everos = tomllib.load(f) + assert everos["embedding"]["model"] == "text-embedding-3-small" + + def test_memory_llm_reuse_pulls_provider_creds( tmp_env: Path, everos_isolated: Path, monkeypatch: pytest.MonkeyPatch ) -> None: @@ -1034,7 +1505,7 @@ def ask(self): # openai IS OpenAI-compatible → the source picker offers "reuse main chat # model", which brings the model id + creds along (no further prompts). monkeypatch.setattr(questionary, "select", lambda *a, **kw: _FQ(("reuse_main",))) - monkeypatch.setattr(onboard_commands, "_probe_everos_endpoint", lambda *a, **kw: (True, "ok")) + monkeypatch.setattr(onboard_commands, "_probe_everos_chat", lambda *a, **kw: (True, "ok")) onboard_commands._config_everos_role( section="llm", main_model="openai/gpt-4o-mini", non_interactive=False, warnings=[] @@ -1130,7 +1601,7 @@ def ask(self): return self._a monkeypatch.setattr(questionary, "select", lambda *a, **kw: _FQ(("reuse_main",))) - monkeypatch.setattr(onboard_commands, "_probe_everos_endpoint", lambda *a, **kw: (True, "ok")) + monkeypatch.setattr(onboard_commands, "_probe_everos_chat", lambda *a, **kw: (True, "ok")) onboard_commands._config_everos_role(section="llm", main_model="qwen-max", non_interactive=False, warnings=[]) with everos_isolated.open("rb") as fh: everos = tomllib.load(fh) From 1d873ca20479a25659bbb4a5a2f4ac40badbc311 Mon Sep 17 00:00:00 2001 From: KT Date: Wed, 8 Jul 2026 20:11:34 +0800 Subject: [PATCH 2/9] fix(cli): probe custom providers, add key/switch recovery on test failure 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) --- raven/cli/onboard_commands.py | 77 ++++++++++++++++++++---------- tests/test_cli_onboard_commands.py | 63 ++++++++++++++++++++++++ 2 files changed, 116 insertions(+), 24 deletions(-) diff --git a/raven/cli/onboard_commands.py b/raven/cli/onboard_commands.py index 2aee08a..51841cc 100644 --- a/raven/cli/onboard_commands.py +++ b/raven/cli/onboard_commands.py @@ -842,11 +842,15 @@ def _failure_choice(options: list[tuple[str, str]], *, non_interactive: bool) -> return chosen -def _run_test_probe(provider: str, *, non_interactive: bool, warnings: list[str]) -> str: - """Send a one-shot test message; on failure offer retry/repick/continue. - - Returns one of ``"ok"`` / ``"continue"`` / ``"repick"``. ``"repick"`` asks - the caller to re-run the model picker. +def _run_test_probe(provider: str, *, non_interactive: bool, warnings: list[str], allow_repick: bool = True) -> str: + """Send a one-shot test message; on failure offer recovery options. + + Returns one of ``"ok"`` / ``"continue"`` / ``"repick"`` / ``"rekey"`` / + ``"switch"``. A test-message failure can be a wrong model, a bad key, or an + account/balance issue, so the menu offers all the matching exits (aligning + with the connectivity-failure menu in ``_resolve_model_with_test``); + ``allow_repick=False`` drops the model option for custom providers whose + model was fixed with the base_url upfront (Switch re-enters both). """ console.print( _t( @@ -865,18 +869,21 @@ def _run_test_probe(provider: str, *, non_interactive: bool, warnings: list[str] ) ) print_probe_troubleshooting(provider) - choice = _failure_choice( - [ - (_t("Retry", "重试"), "retry"), - (_t("Re-pick model", "重新选模型"), "repick"), - (_t("Continue anyway", "仍然继续"), "continue"), - ], - non_interactive=non_interactive, - ) + options = [(_t("Retry", "重试"), "retry")] + if allow_repick: + options.append((_t("Re-pick model", "重新选模型"), "repick")) + options += [ + (_t("Re-enter key", "重新填 Key"), "rekey"), + (_t("Switch provider", "更换服务商"), "switch"), + (_t("Continue anyway", "仍然继续"), "continue"), + ] + choice = _failure_choice(options, non_interactive=non_interactive) if choice == "retry": - return _run_test_probe(provider, non_interactive=non_interactive, warnings=warnings) - if choice == "repick": - return "repick" + return _run_test_probe( + provider, non_interactive=non_interactive, warnings=warnings, allow_repick=allow_repick + ) + if choice in ("repick", "rekey", "switch"): + return choice warnings.append("provider test message") return "continue" @@ -1065,9 +1072,11 @@ def _resolve_model_with_test( ) -> Optional[str]: """Verify connectivity → pick the default model → send a test probe. - On a verify failure, offers a numbered submenu (retry / switch / continue). - Only failures stop; success auto-advances. Returns the chosen model, or - ``None`` to signal "switch provider" (the caller rewinds to the picker). + On a verify or test-message failure, offers a recovery submenu (retry / + re-pick model / re-enter key / switch / continue). Custom providers are + probed too (model was fixed upfront). Only failures stop; success + auto-advances. Returns the chosen model, or ``None`` to signal "switch + provider" (the caller rewinds to the picker). """ while True: ok, status, model_ids = _verify_provider(spec.name) @@ -1095,7 +1104,18 @@ def _resolve_model_with_test( if is_custom: assert custom_model is not None, "custom provider must have model set earlier" - return custom_model + # Custom endpoints were previously trusted without a test message — the + # highest-typo-risk case. Send the real probe (it builds from the stored + # config, so a wrong base_url / model id fails here, not at first chat). + _persist_default_model(custom_model) + while True: + result = _run_test_probe(spec.name, non_interactive=non_interactive, warnings=warnings, allow_repick=False) + if result == "switch": + return None + if result == "rekey": + _write_provider_fields(spec.name, {"api_key": _prompt_api_key(spec.name)}) + continue + return custom_model # ok / continue current = _load_current_default_model() while True: @@ -1108,10 +1128,19 @@ def _resolve_model_with_test( ) _persist_default_model(chosen) result = _run_test_probe(spec.name, non_interactive=non_interactive, warnings=warnings) - if result != "repick": - return chosen - current = chosen - user_model_flag = None + if result == "switch": + return None + if result == "rekey": + _write_provider_fields(spec.name, {"api_key": _prompt_api_key(spec.name)}) + # Re-test the same model with the new key (picker defaults to it). + current = chosen + user_model_flag = None + continue + if result == "repick": + current = chosen + user_model_flag = None + continue + return chosen # ok / continue # --------------------------------------------------------------------------- diff --git a/tests/test_cli_onboard_commands.py b/tests/test_cli_onboard_commands.py index 425b888..3a45298 100644 --- a/tests/test_cli_onboard_commands.py +++ b/tests/test_cli_onboard_commands.py @@ -1276,6 +1276,69 @@ def ask(self): assert "DashScope" in blob +def test_custom_provider_sends_test_probe(tmp_env: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """A custom provider now sends the real test message (was previously trusted + without one) — a wrong base_url/model surfaces here, not at first chat.""" + from raven.config.update_providers import set_provider_fields + from raven.providers.registry import find_by_name + + spec = find_by_name("custom") + set_provider_fields("custom", {"api_key": "k", "api_base": "https://x/v1"}) + monkeypatch.setattr(onboard_commands, "_verify_provider", lambda p: (True, "ok", None)) + calls: list[bool] = [] + + def _probe(): + calls.append(True) + return ("hi", 1, 0.1) + + monkeypatch.setattr(onboard_commands, "send_probe", _probe) + out = onboard_commands._resolve_model_with_test( + spec, is_custom=True, custom_model="my/model", user_model_flag=None, non_interactive=False, warnings=[] + ) + assert calls == [True] # the probe ran for the custom provider + assert out == "my/model" + + +def test_custom_provider_probe_failure_switch_rewinds(tmp_env: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """On a custom-provider probe failure, choosing Switch returns None so the + caller rewinds to the provider picker.""" + from raven.config.update_providers import set_provider_fields + from raven.providers.registry import find_by_name + + spec = find_by_name("custom") + set_provider_fields("custom", {"api_key": "k", "api_base": "https://x/v1"}) + monkeypatch.setattr(onboard_commands, "_verify_provider", lambda p: (True, "ok", None)) + + def _boom(): + raise RuntimeError("model not found") + + monkeypatch.setattr(onboard_commands, "send_probe", _boom) + monkeypatch.setattr(onboard_commands, "_failure_choice", lambda opts, **kw: "switch") + out = onboard_commands._resolve_model_with_test( + spec, is_custom=True, custom_model="m", user_model_flag=None, non_interactive=False, warnings=[] + ) + assert out is None + + +def test_test_probe_failure_menu_offers_rekey_and_switch(monkeypatch: pytest.MonkeyPatch) -> None: + """A test-message failure now offers re-enter-key and switch-provider (not + just retry/repick/continue), matching the connectivity-failure menu.""" + captured: dict[str, list[str]] = {} + + def _cap(opts, **kw): + captured["vals"] = [v for _, v in opts] + return "continue" + + monkeypatch.setattr(onboard_commands, "_failure_choice", _cap) + + def _boom(): + raise RuntimeError("x") + + monkeypatch.setattr(onboard_commands, "send_probe", _boom) + onboard_commands._run_test_probe("openai", non_interactive=False, warnings=[]) + assert {"retry", "repick", "rekey", "switch", "continue"} <= set(captured["vals"]) + + def test_pick_model_empty_submit_uses_default_not_exit(monkeypatch: pytest.MonkeyPatch) -> None: """Clearing the prefilled default and submitting empty falls back to the default model rather than killing the whole wizard.""" From d220e73865ff2753d464faf667482fa831785cac Mon Sep 17 00:00:00 2001 From: KT Date: Wed, 8 Jul 2026 20:20:32 +0800 Subject: [PATCH 3/9] fix(cli): enforce step-1 gate, required channel fields, ctrl+c exit 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) --- raven/cli/onboard_commands.py | 82 +++++++++++++++++------------- tests/test_cli_onboard_commands.py | 54 ++++++++++++++++++++ 2 files changed, 101 insertions(+), 35 deletions(-) diff --git a/raven/cli/onboard_commands.py b/raven/cli/onboard_commands.py index 51841cc..b89cc92 100644 --- a/raven/cli/onboard_commands.py +++ b/raven/cli/onboard_commands.py @@ -19,8 +19,10 @@ Navigation: questionary 2.1.1 has no first-class cross-screen "back", so the wizard is a screen state machine and back is expressed as a ``0) back`` -sentinel choice. Ctrl+C exits at any point, keeping whatever was already -written. +sentinel choice on the screens that support it (Step 1 <-> language pick, +Step 2 -> Step 1); Steps 3 and 4 are optional and forward-only (re-run +``onboard`` to change them). Ctrl+C exits at any point, keeping whatever was +already written. """ from __future__ import annotations @@ -1279,13 +1281,16 @@ def _step1_provider( style=RAVEN_STYLE, qmark=_QMARK, ).ask() - if action in (None, "done"): - # Re-pick a default model if the prior one was removed. - if not _load_current_default_model() and _configured_providers(): + if action is None: + raise typer.Exit(1) # Ctrl+C exits; never treat it as "done" + if action == "done": + # Step 1 is required: never advance without at least one provider AND + # a default model, so deleting every provider can't slip through. + if not (_configured_providers() and _load_current_default_model()): console.print( _t( - " [yellow]No default model set — add or re-pick a provider.[/yellow]", - " [yellow]尚未设置默认模型 — 请新增或重新选择一个服务商。[/yellow]", + " [yellow]At least one provider with a default model is required — add or re-pick one.[/yellow]", + " [yellow]至少需要一个带默认模型的服务商 — 请新增或重新选择一个。[/yellow]", ) ) continue @@ -1608,32 +1613,31 @@ def _prompt_channel_fields(channel: str) -> Any: description = spec.get("description", "") opt_tag = "" if required else _t(" (optional)", " (可选)") prompt_label = f"{path}{opt_tag}" + (f" — {description}" if description else "") + ":" - # First field's empty submit rewinds to the channel picker; later - # optional fields' empty submit skips them. Required later fields get - # no skip hint so we don't invite dropping a value the channel needs. + # First field's empty submit rewinds to the channel picker; a later + # optional field's empty submit skips it; a later required field re-prompts + # (empty was previously accepted silently, enabling a half-configured + # channel — the write layer treats "required" as a UX marker only). allow_back = idx == 0 placeholder = _field_placeholder(allow_back, required) - if spec.get("is_secret"): - value = questionary.password( - prompt_label, - placeholder=placeholder, - style=RAVEN_STYLE, - qmark=_QMARK, - ).ask() - else: - value = questionary.text( - prompt_label, - placeholder=placeholder, - style=RAVEN_STYLE, - qmark=_QMARK, - ).ask() - if value is None: - raise typer.Exit(1) - value = value.strip() - if allow_back and value == "": - return _BACK - if value: - fields[path] = value + while True: + if spec.get("is_secret"): + value = questionary.password( + prompt_label, placeholder=placeholder, style=RAVEN_STYLE, qmark=_QMARK + ).ask() + else: + value = questionary.text(prompt_label, placeholder=placeholder, style=RAVEN_STYLE, qmark=_QMARK).ask() + if value is None: + raise typer.Exit(1) + value = value.strip() + if value: + fields[path] = value + break + if allow_back: + return _BACK # first field empty → back to the channel picker + if required: + console.print(_t(f" [yellow]{path} is required.[/yellow]", f" [yellow]{path} 为必填项。[/yellow]")) + continue # re-prompt instead of enabling a channel missing a credential + break # optional field: empty submit skips it return fields @@ -1980,7 +1984,9 @@ def _step3_channel(*, channel: Optional[str], skip: bool, non_interactive: bool) style=RAVEN_STYLE, qmark=_QMARK, ).ask() - if action in (None, "skip"): + if action is None: + raise typer.Exit(1) + if action == "skip": console.print(_t(" [dim]Skipped.[/dim]", " [dim]已跳过。[/dim]")) return None _add_one_channel() @@ -1999,7 +2005,9 @@ def _step3_channel(*, channel: Optional[str], skip: bool, non_interactive: bool) style=RAVEN_STYLE, qmark=_QMARK, ).ask() - if action in (None, "done"): + if action is None: + raise typer.Exit(1) + if action == "done": return None if action == "add": _add_one_channel() @@ -2685,7 +2693,9 @@ def _config_everos_role(*, section: str, main_model: Optional[str], non_interact style=RAVEN_STYLE, qmark=_QMARK, ).ask() - if action in (None, "skip"): + if action is None: + raise typer.Exit(1) + if action == "skip": note_en, note_zh = role.get("skip_note", (f"Skipped {label_en}.", f"已跳过 {label_zh}。")) console.print(_t(f" [dim]{note_en}[/dim]", f" [dim]{note_zh}[/dim]")) return @@ -2827,7 +2837,9 @@ def _step4_memory(*, skip: bool, non_interactive: bool, main_model: Optional[str style=RAVEN_STYLE, qmark=_QMARK, ).ask() - if action in (None, "off"): + if action is None: + raise typer.Exit(1) + if action == "off": _set_memory_backend(None) console.print( _t( diff --git a/tests/test_cli_onboard_commands.py b/tests/test_cli_onboard_commands.py index 3a45298..e15d11e 100644 --- a/tests/test_cli_onboard_commands.py +++ b/tests/test_cli_onboard_commands.py @@ -1276,6 +1276,60 @@ def ask(self): assert "DashScope" in blob +def test_channel_required_field_reprompts_on_empty(monkeypatch: pytest.MonkeyPatch) -> None: + """A required channel field's empty submit re-prompts instead of enabling a + half-configured channel (the write layer treats required as a UX marker).""" + import questionary + + specs = { + "enabled": {"type": "bool", "default": False, "is_secret": False, "required": False, "description": ""}, + "token": {"type": "str", "default": "", "is_secret": True, "required": False, "description": ""}, + "secret": {"type": "str", "default": "", "is_secret": True, "required": True, "description": ""}, + } + monkeypatch.setattr("raven.config.update_channels.channel_field_specs", lambda name: specs) + answers = iter(["tok", "", "sec"]) # token; secret empty (re-prompt) then filled + + class _FQ: + def ask(self): + return next(answers) + + monkeypatch.setattr(questionary, "password", lambda *a, **kw: _FQ()) + printed: list[str] = [] + monkeypatch.setattr(onboard_commands.console, "print", lambda *a, **kw: printed.append(" ".join(str(x) for x in a))) + out = onboard_commands._prompt_channel_fields("feishu") + assert out == {"token": "tok", "secret": "sec"} + assert any("required" in p for p in printed) + + +def test_step1_done_gated_without_default_model(tmp_env: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """Choosing Done in the multi-provider menu with no default model set does + not advance (Step 1 is required); Ctrl+C then exits.""" + import typer + + from raven.config.update_providers import set_provider_fields + + # A configured provider (so the menu shows) but no default model on disk. + set_provider_fields("openai", {"api_key": "sk-x", "api_base": "https://api.openai.com/v1"}) + monkeypatch.setattr(onboard_commands, "_load_current_default_model", lambda: None) + + import questionary + + answers = iter(["done", None]) # done -> gated (warn + loop); then Ctrl+C -> Exit + + class _FQ: + def ask(self): + return next(answers) + + monkeypatch.setattr(questionary, "select", lambda *a, **kw: _FQ()) + printed: list[str] = [] + monkeypatch.setattr(onboard_commands.console, "print", lambda *a, **kw: printed.append(" ".join(str(x) for x in a))) + with pytest.raises(typer.Exit): + onboard_commands._step1_provider( + provider=None, api_key=None, base_url=None, model=None, non_interactive=False, warnings=[] + ) + assert any("required" in p for p in printed) + + def test_custom_provider_sends_test_probe(tmp_env: Path, monkeypatch: pytest.MonkeyPatch) -> None: """A custom provider now sends the real test message (was previously trusted without one) — a wrong base_url/model surfaces here, not at first chat.""" From b1f218320b5e904a8bffe0d41df01ae335883871 Mon Sep 17 00:00:00 2001 From: KT Date: Wed, 8 Jul 2026 20:30:21 +0800 Subject: [PATCH 4/9] fix(cli): roll back failed new provider, clarify --reset and recap - 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) --- raven/cli/onboard_commands.py | 19 ++++++++++++++----- tests/test_cli_onboard_commands.py | 21 +++++++++++++++++++++ 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/raven/cli/onboard_commands.py b/raven/cli/onboard_commands.py index b89cc92..3a9f7be 100644 --- a/raven/cli/onboard_commands.py +++ b/raven/cli/onboard_commands.py @@ -924,6 +924,7 @@ def _configure_one_provider( # A provider passed by flag is used once; switching then requires the # interactive picker (or, in non-interactive mode, is impossible). flag_provider = provider + configured_before = set(_configured_providers()) while True: if flag_provider: provider = _validate_provider_name(flag_provider) @@ -974,8 +975,12 @@ def _configure_one_provider( warnings=warnings, ) if chosen_model is None: - # "Switch provider" — re-run the picker (drop the flag so the - # second pass prompts rather than reusing the failed flag value). + # "Switch provider" — re-run the picker (drop the flag so the second + # pass prompts rather than reusing the failed flag value). If this + # provider was newly written this pass (not pre-existing), clear its + # key so a failed provider isn't left listed as configured. + if provider not in configured_before: + _write_provider_fields(provider, {"api_key": ""}) flag_provider = None continue _persist_default_model(chosen_model) @@ -2898,8 +2903,8 @@ def _print_next_steps(*, warnings: list[str]) -> None: + f"{', '.join(warnings)}\n" + _t( "[dim]Fix them before relying on the related features " - "(run [/dim][#fbe23f]raven doctor[/#fbe23f][dim] to re-check).[/dim]", - "[dim]在依赖相关功能前请先修复(运行 [/dim][#fbe23f]raven doctor[/#fbe23f][dim] 复查)。[/dim]", + "(re-run [/dim][#fbe23f]raven onboard[/#fbe23f][dim] to reconfigure).[/dim]", + "[dim]在依赖相关功能前请先修复(重新运行 [/dim][#fbe23f]raven onboard[/#fbe23f][dim] 重新配置)。[/dim]", ), border_style="yellow", padding=(1, 2), @@ -3150,7 +3155,11 @@ def onboard( help="Run without prompts (requires flags for any missing field)", ), yes: bool = typer.Option(False, "--yes", "-y", help="Skip all confirm prompts"), - reset: bool = typer.Option(False, "--reset", help="Force re-run even if a config already exists"), + reset: bool = typer.Option( + False, + "--reset", + help="Re-run the wizard over an existing config (does not erase it; each step keeps current values as defaults)", + ), ) -> None: """Four-step setup wizard: LLM provider → sandbox → channel → memory.""" run_wizard( diff --git a/tests/test_cli_onboard_commands.py b/tests/test_cli_onboard_commands.py index e15d11e..22c1db4 100644 --- a/tests/test_cli_onboard_commands.py +++ b/tests/test_cli_onboard_commands.py @@ -1276,6 +1276,27 @@ def ask(self): assert "DashScope" in blob +def test_switch_clears_newly_added_failed_provider(tmp_env: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """A brand-new provider that fails and is switched away from has its key + cleared, so a failed provider is not left listed as configured.""" + + picks = iter(["openai", onboard_commands._BACK]) + monkeypatch.setattr(onboard_commands, "_select_provider", lambda: next(picks)) + + def _collect(provider, **kw): + onboard_commands._write_provider_fields(provider, {"api_key": "sk-bad"}) + return None # not a custom provider + + monkeypatch.setattr(onboard_commands, "_collect_credentials", _collect) + monkeypatch.setattr(onboard_commands, "_resolve_model_with_test", lambda *a, **kw: None) # switch + + out = onboard_commands._configure_one_provider( + provider=None, api_key=None, base_url=None, model=None, non_interactive=False, warnings=[] + ) + assert out is None # user backed out of the picker + assert "openai" not in onboard_commands._configured_providers() # failed new provider cleared + + def test_channel_required_field_reprompts_on_empty(monkeypatch: pytest.MonkeyPatch) -> None: """A required channel field's empty submit re-prompts instead of enabling a half-configured channel (the write layer treats required as a UX marker).""" From fa2638c952f8a389677c9d6f7c2e658295de4283 Mon Sep 17 00:00:00 2001 From: KT Date: Wed, 8 Jul 2026 20:38:23 +0800 Subject: [PATCH 5/9] feat(cli): add --skip-test to skip the billed onboard probe 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) --- raven/cli/onboard_commands.py | 20 ++++++++++++++++++++ tests/test_cli_onboard_commands.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/raven/cli/onboard_commands.py b/raven/cli/onboard_commands.py index 3a9f7be..9ff1f11 100644 --- a/raven/cli/onboard_commands.py +++ b/raven/cli/onboard_commands.py @@ -911,6 +911,7 @@ def _configure_one_provider( model: Optional[str], non_interactive: bool, warnings: list[str], + skip_test: bool = False, ) -> Optional[dict[str, Any]]: """Drive one provider through pick → credentials → verify → model → test. @@ -973,6 +974,7 @@ def _configure_one_provider( user_model_flag=model, non_interactive=non_interactive, warnings=warnings, + skip_test=skip_test, ) if chosen_model is None: # "Switch provider" — re-run the picker (drop the flag so the second @@ -1076,6 +1078,7 @@ def _resolve_model_with_test( user_model_flag: Optional[str], non_interactive: bool, warnings: list[str], + skip_test: bool = False, ) -> Optional[str]: """Verify connectivity → pick the default model → send a test probe. @@ -1115,6 +1118,8 @@ def _resolve_model_with_test( # highest-typo-risk case. Send the real probe (it builds from the stored # config, so a wrong base_url / model id fails here, not at first chat). _persist_default_model(custom_model) + if skip_test: + return custom_model while True: result = _run_test_probe(spec.name, non_interactive=non_interactive, warnings=warnings, allow_repick=False) if result == "switch": @@ -1134,6 +1139,8 @@ def _resolve_model_with_test( non_interactive=non_interactive, ) _persist_default_model(chosen) + if skip_test: + return chosen result = _run_test_probe(spec.name, non_interactive=non_interactive, warnings=warnings) if result == "switch": return None @@ -1243,6 +1250,7 @@ def _step1_provider( model: Optional[str], non_interactive: bool, warnings: list[str], + skip_test: bool = False, ) -> object: """Step 1 screen. Returns ``_BACK`` only when the user backs out of the first-run picker on the welcome screen (handled by the runner).""" @@ -1263,6 +1271,7 @@ def _step1_provider( model=model, non_interactive=non_interactive, warnings=warnings, + skip_test=skip_test, ) if result is None: return _BACK @@ -1308,6 +1317,7 @@ def _step1_provider( model=None, non_interactive=False, warnings=warnings, + skip_test=skip_test, ) elif action == "edit": _manage_existing_providers(non_interactive=non_interactive) @@ -2986,6 +2996,7 @@ def run_wizard( non_interactive: bool = False, yes: bool = False, reset: bool = False, + skip_test: bool = False, ) -> None: """Run the 4-step onboarding wizard end-to-end. @@ -3013,6 +3024,7 @@ def run_wizard( non_interactive=non_interactive, yes=yes, reset=reset, + skip_test=skip_test, ) finally: _logger.enable("raven") @@ -3031,6 +3043,7 @@ def _run_wizard_body( non_interactive: bool = False, yes: bool = False, reset: bool = False, + skip_test: bool = False, ) -> None: global _LANG _check_tty_or_die(non_interactive) @@ -3077,6 +3090,7 @@ def _run_wizard_body( model=model, non_interactive=non_interactive, warnings=warnings, + skip_test=skip_test, ), lambda: _step2_sandbox(skip=skip_sandbox, non_interactive=non_interactive), lambda: _step3_channel(channel=channel, skip=skip_channel, non_interactive=non_interactive), @@ -3160,6 +3174,11 @@ def onboard( "--reset", help="Re-run the wizard over an existing config (does not erase it; each step keeps current values as defaults)", ), + skip_test: bool = typer.Option( + False, + "--skip-test", + help="Skip the one-shot test message (avoids a billed call; connectivity is still checked)", + ), ) -> None: """Four-step setup wizard: LLM provider → sandbox → channel → memory.""" run_wizard( @@ -3174,6 +3193,7 @@ def onboard( non_interactive=non_interactive, yes=yes, reset=reset, + skip_test=skip_test, ) diff --git a/tests/test_cli_onboard_commands.py b/tests/test_cli_onboard_commands.py index 22c1db4..68854b5 100644 --- a/tests/test_cli_onboard_commands.py +++ b/tests/test_cli_onboard_commands.py @@ -158,6 +158,7 @@ def test_onboard_help_lists_all_flags() -> None: "--non-interactive", "--yes", "--reset", + "--skip-test", ): assert flag in out, f"missing flag in help: {flag}" @@ -412,6 +413,34 @@ def _fail(name: str, *args, **kwargs) -> dict[str, Any]: assert "didn't pass a connectivity test" in r.stdout +def test_onboard_skip_test_avoids_billed_probe(tmp_env: Path, monkeypatch: pytest.MonkeyPatch, stub_verify) -> None: + """--skip-test skips the one-shot chat probe (no billed call) while still + completing setup; connectivity (test_provider) still runs.""" + calls: list[bool] = [] + monkeypatch.setattr(onboard_commands, "send_probe", lambda: (calls.append(True), ("hi", 1, 0.1))[1]) + r = runner.invoke( + app, + [ + "onboard", + "--non-interactive", + "--provider", + "openai", + "--api-key", + "sk-x", + "--model", + "openai/gpt-4o-mini", + "--skip-sandbox", + "--skip-channel", + "--skip-memory", + "--yes", + "--skip-test", + ], + ) + assert r.exit_code == 0, r.stdout + assert calls == [] # the billed chat probe was never sent + assert "Setup complete" in r.stdout + + def test_onboard_test_probe_failure_shows_warning_footer( tmp_env: Path, monkeypatch: pytest.MonkeyPatch, stub_verify ) -> None: From 0c8a29c2b1c922eda0e97ab92c0937306169ab73 Mon Sep 17 00:00:00 2001 From: KT Date: Wed, 8 Jul 2026 20:41:40 +0800 Subject: [PATCH 6/9] fix(cli): revert scancode channel enable on ctrl+c during login _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) --- raven/cli/onboard_commands.py | 7 +++++++ tests/test_cli_onboard_commands.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/raven/cli/onboard_commands.py b/raven/cli/onboard_commands.py index 9ff1f11..6d9392e 100644 --- a/raven/cli/onboard_commands.py +++ b/raven/cli/onboard_commands.py @@ -1821,6 +1821,13 @@ def _scancode_login(channel: str) -> None: ) ) ok = False + except BaseException: + # Ctrl+C / cancellation mid-scan is not an ``Exception`` subclass, so + # it would skip the revert below and leave the channel enabled but + # unauthenticated. Disable it (the docstring's "any path that doesn't + # complete login") before letting it propagate. + disable_channel(channel) + raise finally: _wiz_logger.disable(_login_log_scope) if ok: diff --git a/tests/test_cli_onboard_commands.py b/tests/test_cli_onboard_commands.py index 68854b5..0d07be0 100644 --- a/tests/test_cli_onboard_commands.py +++ b/tests/test_cli_onboard_commands.py @@ -1326,6 +1326,36 @@ def _collect(provider, **kw): assert "openai" not in onboard_commands._configured_providers() # failed new provider cleared +def test_scancode_login_reverts_on_keyboard_interrupt(monkeypatch: pytest.MonkeyPatch) -> None: + """Ctrl+C mid-scan (KeyboardInterrupt, not an Exception subclass) still + disables the channel so a cancelled login is not left as "connected".""" + import types + + class _Adapter: + async def login(self, force=False): + raise KeyboardInterrupt + + class _Spec: + display_name = "WhatsApp" + + def factory(self, cfg): + return _Adapter() + + monkeypatch.setattr(onboard_commands, "_node_runtime_missing", lambda ch: False) + monkeypatch.setattr(onboard_commands, "_enable_channel", lambda ch, fields: None) + monkeypatch.setattr("raven.channels.registry.discover_specs", lambda: {"whatsapp": _Spec()}) + monkeypatch.setattr( + "raven.config.loader.load_config", + lambda: types.SimpleNamespace(channels=types.SimpleNamespace(whatsapp=object())), + ) + disabled: list[str] = [] + monkeypatch.setattr("raven.config.update_channels.disable_channel", lambda ch: disabled.append(ch)) + + with pytest.raises(KeyboardInterrupt): + onboard_commands._scancode_login("whatsapp") + assert disabled == ["whatsapp"] + + def test_channel_required_field_reprompts_on_empty(monkeypatch: pytest.MonkeyPatch) -> None: """A required channel field's empty submit re-prompts instead of enabling a half-configured channel (the write layer treats required as a UX marker).""" From 0e1bf3fdc614842a0a8566cf14335e48699ec718 Mon Sep 17 00:00:00 2001 From: KT Date: Wed, 8 Jul 2026 21:10:41 +0800 Subject: [PATCH 7/9] fix(cli): finish scancode revert and thread --skip-test to memory 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) --- raven/cli/onboard_commands.py | 224 ++++++++++++++++------------- tests/test_cli_onboard_commands.py | 156 ++++++++++++++++++++ 2 files changed, 281 insertions(+), 99 deletions(-) diff --git a/raven/cli/onboard_commands.py b/raven/cli/onboard_commands.py index 6d9392e..60b7ae3 100644 --- a/raven/cli/onboard_commands.py +++ b/raven/cli/onboard_commands.py @@ -952,6 +952,11 @@ def _configure_one_provider( ) ) + # Snapshot the stored key before _collect_credentials overwrites it, so a + # failed re-configuration of an existing provider can be rolled back to + # its prior working key (rather than left holding the just-typed bad one). + old_key = ((_load_raw_config().get("providers") or {}).get(provider) or {}).get("apiKey") + custom_model = _collect_credentials( provider, is_oauth=is_oauth, @@ -978,11 +983,14 @@ def _configure_one_provider( ) if chosen_model is None: # "Switch provider" — re-run the picker (drop the flag so the second - # pass prompts rather than reusing the failed flag value). If this - # provider was newly written this pass (not pre-existing), clear its - # key so a failed provider isn't left listed as configured. + # pass prompts rather than reusing the failed flag value). Roll back + # the just-written key: clear it if this provider was newly added this + # pass, or restore the prior key if we were reconfiguring an existing + # one (so a failed edit doesn't clobber a working provider). if provider not in configured_before: _write_provider_fields(provider, {"api_key": ""}) + elif old_key: + _write_provider_fields(provider, {"api_key": old_key}) flag_provider = None continue _persist_default_model(chosen_model) @@ -1752,112 +1760,113 @@ def _scancode_login(channel: str) -> None: console.print(_t(f" [red]✗ Unknown channel: {channel}[/red]", f" [red]✗ 未知渠道:{channel}[/red]")) return - while True: - # Node-bridge channels: gate on the runtime up front so a missing - # Node/npm shows a useful install menu, not a "re-show QR" no-op. - if _node_runtime_missing(channel): - if _handle_missing_node(channel) == "retry": - continue - disable_channel(channel) # not logged in → don't leave it "connected" - console.print( - _t( - f" [dim]Skipped {channel}; install Node.js then run raven channels login {channel}.[/dim]", - f" [dim]已跳过 {channel};装好 Node.js 后运行 raven channels login {channel}。[/dim]", + # Enabled above so the factory can read the config section during login. ANY + # path that doesn't finish login must revert the enable — including Ctrl+C in + # a submenu (raises typer.Exit) or mid-scan (KeyboardInterrupt), neither an + # ``Exception`` subclass — so wrap the whole flow and disable in ``finally`` + # unless we actually logged in. + logged_in = False + try: + while True: + # Node-bridge channels: gate on the runtime up front so a missing + # Node/npm shows a useful install menu, not a "re-show QR" no-op. + if _node_runtime_missing(channel): + if _handle_missing_node(channel) == "retry": + continue + console.print( + _t( + f" [dim]Skipped {channel}; install Node.js then run raven channels login {channel}.[/dim]", + f" [dim]已跳过 {channel};装好 Node.js 后运行 raven channels login {channel}。[/dim]", + ) ) - ) - return + return - from raven.config.loader import load_config + from raven.config.loader import load_config - channel_cfg = getattr(load_config().channels, channel, None) - if channel_cfg is None: - disable_channel(channel) + channel_cfg = getattr(load_config().channels, channel, None) + if channel_cfg is None: + console.print( + _t( + f" [red]✗ No config section for channel: {channel}[/red]", + f" [red]✗ 渠道 {channel} 没有配置段。[/red]", + ) + ) + return + adapter = spec.factory(channel_cfg) + if channel == "whatsapp": + console.print( + _t( + " [dim]Building the WhatsApp bridge — the first run can take 30–120s…[/dim]", + " [dim]正在构建 WhatsApp 桥接,首次约需 30–120 秒…[/dim]", + ) + ) console.print( _t( - f" [red]✗ No config section for channel: {channel}[/red]", - f" [red]✗ 渠道 {channel} 没有配置段。[/red]", + f" [dim]Starting {spec.display_name} QR login…[/dim]", + f" [dim]正在启动 {spec.display_name} 扫码登录…[/dim]", ) ) - return - adapter = spec.factory(channel_cfg) - if channel == "whatsapp": console.print( _t( - " [dim]Building the WhatsApp bridge — the first run can take 30–120s…[/dim]", - " [dim]正在构建 WhatsApp 桥接,首次约需 30–120 秒…[/dim]", + f" [dim]A login link / QR code will appear below — scan it with " + f"{spec.display_name} (or open the link on a phone signed in to " + f"{spec.display_name}) to connect. This waits until you finish.[/dim]", + f" [dim]下方会出现登录链接 / 二维码 — 用 {spec.display_name} 扫码" + f"(或在已登录 {spec.display_name} 的手机上打开该链接)即可接入;" + f"这里会一直等到你完成。[/dim]", ) ) - console.print( - _t( - f" [dim]Starting {spec.display_name} QR login…[/dim]", - f" [dim]正在启动 {spec.display_name} 扫码登录…[/dim]", - ) - ) - console.print( - _t( - f" [dim]A login link / QR code will appear below — scan it with " - f"{spec.display_name} (or open the link on a phone signed in to " - f"{spec.display_name}) to connect. This waits until you finish.[/dim]", - f" [dim]下方会出现登录链接 / 二维码 — 用 {spec.display_name} 扫码" - f"(或在已登录 {spec.display_name} 的手机上打开该链接)即可接入;" - f"这里会一直等到你完成。[/dim]", - ) - ) - from loguru import logger as _wiz_logger - - # The wizard silences raven logs for a clean UI, but a scancode login - # emits its QR / link / progress / failure reason through loguru. Re- - # enable ONLY this channel's adapter subtree for the login attempt (not - # all of raven, which would dump unrelated noise), then restore quiet. - _login_log_scope = f"raven.channels.adapters.{channel}" - try: - _wiz_logger.enable(_login_log_scope) - ok = asyncio.run(adapter.login(force=True)) - except Exception as exc: - console.print( - _t( - f" [yellow]✗ Login failed: {exc}[/yellow]", - f" [yellow]✗ 登录失败:{exc}[/yellow]", + from loguru import logger as _wiz_logger + + # The wizard silences raven logs for a clean UI, but a scancode login + # emits its QR / link / progress / failure reason through loguru. Re- + # enable ONLY this channel's adapter subtree for the login attempt (not + # all of raven, which would dump unrelated noise), then restore quiet. + _login_log_scope = f"raven.channels.adapters.{channel}" + try: + _wiz_logger.enable(_login_log_scope) + ok = asyncio.run(adapter.login(force=True)) + except Exception as exc: + console.print( + _t( + f" [yellow]✗ Login failed: {exc}[/yellow]", + f" [yellow]✗ 登录失败:{exc}[/yellow]", + ) ) + ok = False + finally: + _wiz_logger.disable(_login_log_scope) + if ok: + console.print( + _t( + f" [green]✓ Logged in; {channel} connected.[/green]", + f" [green]✓ 已登录;{channel} 已接入。[/green]", + ) + ) + logged_in = True + return + choice = _failure_choice( + [ + (_t("Retry", "重试"), "retry"), + (_t("Skip this channel", "跳过此渠道"), "skip"), + ], + non_interactive=False, ) - ok = False - except BaseException: - # Ctrl+C / cancellation mid-scan is not an ``Exception`` subclass, so - # it would skip the revert below and leave the channel enabled but - # unauthenticated. Disable it (the docstring's "any path that doesn't - # complete login") before letting it propagate. - disable_channel(channel) - raise - finally: - _wiz_logger.disable(_login_log_scope) - if ok: + if choice == "retry": + continue console.print( _t( - f" [green]✓ Logged in; {channel} connected.[/green]", - f" [green]✓ 已登录;{channel} 已接入。[/green]", + f" [dim]{channel} not connected — finish later with raven channels login {channel}.[/dim]", + f" [dim]{channel} 未接入 — 之后用 raven channels login {channel} 完成。[/dim]", ) ) return - choice = _failure_choice( - [ - (_t("Retry", "重试"), "retry"), - (_t("Skip this channel", "跳过此渠道"), "skip"), - ], - non_interactive=False, - ) - if choice == "retry": - continue - # Not completed (skip) → revert the enable so the channel isn't shown as - # connected. The config section is kept, so the user can finish - # out-of-band with `raven channels login `. - disable_channel(channel) - console.print( - _t( - f" [dim]{channel} not connected — finish later with raven channels login {channel}.[/dim]", - f" [dim]{channel} 未接入 — 之后用 raven channels login {channel} 完成。[/dim]", - ) - ) - return + finally: + if not logged_in: + # Any non-login exit (skip, no-config, submenu Ctrl+C, mid-scan + # interrupt) reverts the enable so a cancelled scan never persists as + # "connected". The config section is kept for `raven channels login`. + disable_channel(channel) def _add_one_channel() -> None: @@ -2642,7 +2651,9 @@ def _everos_pick_creds_and_model( return result -def _config_everos_role(*, section: str, main_model: Optional[str], non_interactive: bool, warnings: list[str]) -> Any: +def _config_everos_role( + *, section: str, main_model: Optional[str], non_interactive: bool, warnings: list[str], skip_test: bool = False +) -> Any: """Configure one EverOS memory role (llm / embedding / rerank / multimodal) with the unified provider→key→model flow, reuse shortcuts, and a back loop. @@ -2730,11 +2741,14 @@ def _config_everos_role(*, section: str, main_model: Optional[str], non_interact non_interactive=non_interactive, ) if result is _BACK: - if optional: - continue # optional role: re-show the role menu (it offers Skip) - # A required role has no Skip, so backing out of the picker would loop - # forever. Offer a bounded exit: keep trying, or give up EverOS and - # fall back to Markdown memory. + if optional or _everos_section(section).get("model"): + # Optional roles offer Skip; a required role that already has a + # value on disk falls back to its keep/reconfigure menu. Either + # way, re-show the role menu rather than forcing the give-up exit. + continue + # A required role with nothing configured has no Skip, so backing out + # of the picker would loop forever. Offer a bounded exit: keep trying, + # or give up EverOS and fall back to Markdown memory. action = questionary.select( _t( f"{label_en} is required for EverOS memory. What would you like to do?", @@ -2756,7 +2770,15 @@ def _config_everos_role(*, section: str, main_model: Optional[str], non_interact continue return _ABORT_EVEROS - if role["verify"]: + if role["verify"] and skip_test: + console.print( + _t( + f" [dim]Skipping the {verify_label} test call (--skip-test).[/dim]", + f" [dim]已跳过 {verify_label} 的测试调用(--skip-test)。[/dim]", + ) + ) + ok = True + elif role["verify"]: ok = _verify_everos_model( verify_label, section=section, @@ -2787,7 +2809,9 @@ def _config_everos_role(*, section: str, main_model: Optional[str], non_interact return -def _step4_memory(*, skip: bool, non_interactive: bool, main_model: Optional[str], warnings: list[str]) -> object: +def _step4_memory( + *, skip: bool, non_interactive: bool, main_model: Optional[str], warnings: list[str], skip_test: bool = False +) -> object: """Step 4 — EverOS long-term memory (enable + model sub-screens). The bootstrap seeds ``memory.backend="everos"`` (schema default), so this @@ -2882,6 +2906,7 @@ def _step4_memory(*, skip: bool, non_interactive: bool, main_model: Optional[str main_model=main_model, non_interactive=non_interactive, warnings=warnings, + skip_test=skip_test, ) if outcome is _ABORT_EVEROS: _set_memory_backend(None) @@ -3106,6 +3131,7 @@ def _run_wizard_body( non_interactive=non_interactive, main_model=_load_current_default_model(), warnings=warnings, + skip_test=skip_test, ), ] diff --git a/tests/test_cli_onboard_commands.py b/tests/test_cli_onboard_commands.py index 0d07be0..4fb6ead 100644 --- a/tests/test_cli_onboard_commands.py +++ b/tests/test_cli_onboard_commands.py @@ -1326,6 +1326,162 @@ def _collect(provider, **kw): assert "openai" not in onboard_commands._configured_providers() # failed new provider cleared +def test_test_probe_rekey_completes_write_and_retry(tmp_env: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """Choosing Re-enter key on a test-message failure writes the new key and + retries; a passing retry then completes (the rekey outcome is exercised end + to end, not just offered).""" + from raven.config.update_providers import set_provider_fields + from raven.providers.registry import find_by_name + + spec = find_by_name("custom") + set_provider_fields("custom", {"api_key": "k-old", "api_base": "https://x/v1"}) + monkeypatch.setattr(onboard_commands, "_verify_provider", lambda p: (True, "ok", None)) + outcomes = iter([RuntimeError("bad key"), ("hi", 1, 0.1)]) + + def _probe(): + r = next(outcomes) + if isinstance(r, Exception): + raise r + return r + + monkeypatch.setattr(onboard_commands, "send_probe", _probe) + monkeypatch.setattr(onboard_commands, "_failure_choice", lambda opts, **kw: "rekey") + monkeypatch.setattr(onboard_commands, "_prompt_api_key", lambda p, **kw: "k-new") + + out = onboard_commands._resolve_model_with_test( + spec, is_custom=True, custom_model="m", user_model_flag=None, non_interactive=False, warnings=[] + ) + assert out == "m" + stored = ((onboard_commands._load_raw_config().get("providers") or {}).get("custom") or {}).get("apiKey") + assert stored == "k-new" + + +def test_switch_restores_reconfigured_provider_key(tmp_env: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """Reconfiguring an existing provider that then fails and is switched away + from restores its prior key (a failed edit must not clobber a working one).""" + from raven.config.update_providers import set_provider_fields + + set_provider_fields("openai", {"api_key": "sk-good", "api_base": "https://api.openai.com/v1"}) + picks = iter(["openai", onboard_commands._BACK]) + monkeypatch.setattr(onboard_commands, "_select_provider", lambda: next(picks)) + + def _collect(provider, **kw): + onboard_commands._write_provider_fields(provider, {"api_key": "sk-bad"}) + return None + + monkeypatch.setattr(onboard_commands, "_collect_credentials", _collect) + monkeypatch.setattr(onboard_commands, "_resolve_model_with_test", lambda *a, **kw: None) # switch + + out = onboard_commands._configure_one_provider( + provider=None, api_key=None, base_url=None, model=None, non_interactive=False, warnings=[] + ) + assert out is None + stored = ((onboard_commands._load_raw_config().get("providers") or {}).get("openai") or {}).get("apiKey") + assert stored == "sk-good" # restored, not left as sk-bad + + +def test_config_everos_role_skip_test_skips_probe( + tmp_env: Path, everos_isolated: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """--skip-test (threaded into Step 4) skips the billed EverOS probe while + still persisting the model.""" + import tomllib + + from raven.config.update_everos import set_everos_section + + set_everos_section("llm", {"model": "m", "api_key": "k-llm", "base_url": "https://llm/v1"}) + + import questionary + + class _FQ: + def __init__(self, a): + self._a = a + + def ask(self): + return self._a + + monkeypatch.setattr(questionary, "select", lambda *a, **kw: _FQ(("reuse_llm",))) + monkeypatch.setattr(questionary, "text", lambda *a, **kw: _FQ("text-embedding-3-small")) + monkeypatch.setattr(onboard_commands, "_fetch_everos_models", lambda *a, **kw: None) + calls: list[bool] = [] + monkeypatch.setattr( + onboard_commands, "_probe_everos_embedding", lambda *a, **kw: (calls.append(True), (True, "ok"))[1] + ) + onboard_commands._config_everos_role( + section="embedding", main_model="deepseek/deepseek-chat", non_interactive=False, warnings=[], skip_test=True + ) + assert calls == [] # billed probe skipped + with everos_isolated.open("rb") as f: + everos = tomllib.load(f) + assert everos["embedding"]["model"] == "text-embedding-3-small" + + +def test_reconfig_existing_required_role_backout_keeps_current( + tmp_env: Path, everos_isolated: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Backing out of the picker while reconfiguring an already-configured + required role returns to the keep/reconfigure menu (not the give-up exit).""" + from raven.config.update_everos import set_everos_section + + set_everos_section("llm", {"model": "m", "api_key": "k-llm", "base_url": "https://llm/v1"}) + set_everos_section("embedding", {"model": "existing-embed", "api_key": "k", "base_url": "https://llm/v1"}) + + import questionary + + class _FQ: + def __init__(self, a): + self._a = a + + def ask(self): + return self._a + + # role menu -> Reconfigure; source picker -> Back; role menu -> Keep current. + select_answers = iter(["redo", onboard_commands._BACK, "keep"]) + monkeypatch.setattr(questionary, "select", lambda *a, **kw: _FQ(next(select_answers))) + out = onboard_commands._config_everos_role( + section="embedding", main_model="deepseek/deepseek-chat", non_interactive=False, warnings=[] + ) + assert out is None # kept current, not _ABORT_EVEROS + + +def test_scancode_login_reverts_on_menu_ctrl_c(monkeypatch: pytest.MonkeyPatch) -> None: + """Ctrl+C in the post-login retry/skip menu (typer.Exit, not an Exception) + still disables the channel — the revert covers submenu exits, not just the + login call.""" + import types + + import typer + + class _Adapter: + async def login(self, force=False): + return False # login fails -> drops into the retry/skip menu + + class _Spec: + display_name = "WhatsApp" + + def factory(self, cfg): + return _Adapter() + + monkeypatch.setattr(onboard_commands, "_node_runtime_missing", lambda ch: False) + monkeypatch.setattr(onboard_commands, "_enable_channel", lambda ch, fields: None) + monkeypatch.setattr("raven.channels.registry.discover_specs", lambda: {"whatsapp": _Spec()}) + monkeypatch.setattr( + "raven.config.loader.load_config", + lambda: types.SimpleNamespace(channels=types.SimpleNamespace(whatsapp=object())), + ) + + def _ctrl_c(opts, **kw): + raise typer.Exit(1) + + monkeypatch.setattr(onboard_commands, "_failure_choice", _ctrl_c) + disabled: list[str] = [] + monkeypatch.setattr("raven.config.update_channels.disable_channel", lambda ch: disabled.append(ch)) + + with pytest.raises(typer.Exit): + onboard_commands._scancode_login("whatsapp") + assert disabled == ["whatsapp"] + + def test_scancode_login_reverts_on_keyboard_interrupt(monkeypatch: pytest.MonkeyPatch) -> None: """Ctrl+C mid-scan (KeyboardInterrupt, not an Exception subclass) still disables the channel so a cancelled login is not left as "connected".""" From 645dd526b10e662c3a2987c55145cd23ef47502e Mon Sep 17 00:00:00 2001 From: KT Date: Wed, 8 Jul 2026 21:19:53 +0800 Subject: [PATCH 8/9] fix(cli): also restore provider base_url on failed re-config switch 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) --- raven/cli/onboard_commands.py | 6 ++++-- tests/test_cli_onboard_commands.py | 7 ++++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/raven/cli/onboard_commands.py b/raven/cli/onboard_commands.py index 60b7ae3..204e388 100644 --- a/raven/cli/onboard_commands.py +++ b/raven/cli/onboard_commands.py @@ -955,7 +955,9 @@ def _configure_one_provider( # Snapshot the stored key before _collect_credentials overwrites it, so a # failed re-configuration of an existing provider can be rolled back to # its prior working key (rather than left holding the just-typed bad one). - old_key = ((_load_raw_config().get("providers") or {}).get(provider) or {}).get("apiKey") + _prev = (_load_raw_config().get("providers") or {}).get(provider) or {} + old_key = _prev.get("apiKey") + old_base = _prev.get("apiBase") custom_model = _collect_credentials( provider, @@ -990,7 +992,7 @@ def _configure_one_provider( if provider not in configured_before: _write_provider_fields(provider, {"api_key": ""}) elif old_key: - _write_provider_fields(provider, {"api_key": old_key}) + _write_provider_fields(provider, {"api_key": old_key, "api_base": old_base}) flag_provider = None continue _persist_default_model(chosen_model) diff --git a/tests/test_cli_onboard_commands.py b/tests/test_cli_onboard_commands.py index 4fb6ead..a0571a2 100644 --- a/tests/test_cli_onboard_commands.py +++ b/tests/test_cli_onboard_commands.py @@ -1366,7 +1366,7 @@ def test_switch_restores_reconfigured_provider_key(tmp_env: Path, monkeypatch: p monkeypatch.setattr(onboard_commands, "_select_provider", lambda: next(picks)) def _collect(provider, **kw): - onboard_commands._write_provider_fields(provider, {"api_key": "sk-bad"}) + onboard_commands._write_provider_fields(provider, {"api_key": "sk-bad", "api_base": "https://bad/v1"}) return None monkeypatch.setattr(onboard_commands, "_collect_credentials", _collect) @@ -1376,8 +1376,9 @@ def _collect(provider, **kw): provider=None, api_key=None, base_url=None, model=None, non_interactive=False, warnings=[] ) assert out is None - stored = ((onboard_commands._load_raw_config().get("providers") or {}).get("openai") or {}).get("apiKey") - assert stored == "sk-good" # restored, not left as sk-bad + stored = (onboard_commands._load_raw_config().get("providers") or {}).get("openai") or {} + assert stored.get("apiKey") == "sk-good" # key restored, not left as sk-bad + assert stored.get("apiBase") == "https://api.openai.com/v1" # base_url restored too def test_config_everos_role_skip_test_skips_probe( From 0d2e28cd688ed5a4c53497f951addd9a5045fdff Mon Sep 17 00:00:00 2001 From: KT Date: Wed, 8 Jul 2026 22:24:25 +0800 Subject: [PATCH 9/9] fix(cli): close the two audited onboard follow-ups - 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) --- raven/cli/onboard_commands.py | 42 ++++++++++++++++------------ tests/test_cli_onboard_commands.py | 44 +++++++++++++++++++++++++++--- 2 files changed, 65 insertions(+), 21 deletions(-) diff --git a/raven/cli/onboard_commands.py b/raven/cli/onboard_commands.py index 204e388..5004776 100644 --- a/raven/cli/onboard_commands.py +++ b/raven/cli/onboard_commands.py @@ -611,7 +611,7 @@ def _run_oauth_login(provider: str) -> bool: return True -def _verify_provider(provider: str) -> tuple[bool, str, Optional[list[str]]]: +def _verify_provider(provider: str, *, skip_test: bool = False) -> tuple[bool, str, Optional[list[str]]]: """Hit ``GET /v1/models`` to verify the credentials we just stored. Returns ``(ok, status, model_ids)``. ``status`` is one of the ops-library @@ -636,12 +636,20 @@ def _verify_provider(provider: str) -> tuple[bool, str, Optional[list[str]]]: # check (the test message sent later exercises real connectivity via # litellm) instead of dumping the user into the failure submenu. if status == "not_configured" and "api_base" in (result.get("error") or ""): - console.print( - _t( - " [dim]Skipping the model-list pre-check (this provider has no public /models endpoint); the test message below will confirm connectivity.[/dim]", - " [dim]跳过模型列表预检(该服务商无公开 /models 端点);稍后的测试消息会验证连通。[/dim]", + if skip_test: + console.print( + _t( + " [dim]Skipping the model-list pre-check (this provider has no public /models endpoint); connectivity is not tested (--skip-test).[/dim]", + " [dim]跳过模型列表预检(该服务商无公开 /models 端点);未做连通测试(--skip-test)。[/dim]", + ) + ) + else: + console.print( + _t( + " [dim]Skipping the model-list pre-check (this provider has no public /models endpoint); the test message below will confirm connectivity.[/dim]", + " [dim]跳过模型列表预检(该服务商无公开 /models 端点);稍后的测试消息会验证连通。[/dim]", + ) ) - ) return True, "skipped", None hint_map = { "invalid_key": _t( @@ -1099,7 +1107,7 @@ def _resolve_model_with_test( provider" (the caller rewinds to the picker). """ while True: - ok, status, model_ids = _verify_provider(spec.name) + ok, status, model_ids = _verify_provider(spec.name, skip_test=skip_test) if not ok: options = ( [(_t("Retry", "重试"), "retry"), (_t("Continue anyway", "仍然继续"), "continue")] @@ -1711,7 +1719,7 @@ def _node_runtime_missing(channel: str) -> bool: return shutil.which("npm") is None -def _handle_missing_node(channel: str) -> str: +def _handle_missing_node(channel: str, *, non_interactive: bool) -> str: """Show the Node-missing submenu (install-then-retry / skip). Returns ``"retry"`` (re-check after install) or ``"skip"`` (leave the @@ -1730,12 +1738,12 @@ def _handle_missing_node(channel: str) -> str: (_t("Retry after install", "安装后重试"), "retry"), (_t("Skip", "跳过"), "skip"), ], - non_interactive=False, + non_interactive=non_interactive, ) return choice -def _scancode_login(channel: str) -> None: +def _scancode_login(channel: str, *, non_interactive: bool = False) -> None: """Run a scancode channel's real QR login (reuses ``channel.login``). Mirrors ``raven channels login``: enable the channel so its config section @@ -1773,7 +1781,7 @@ def _scancode_login(channel: str) -> None: # Node-bridge channels: gate on the runtime up front so a missing # Node/npm shows a useful install menu, not a "re-show QR" no-op. if _node_runtime_missing(channel): - if _handle_missing_node(channel) == "retry": + if _handle_missing_node(channel, non_interactive=non_interactive) == "retry": continue console.print( _t( @@ -1852,7 +1860,7 @@ def _scancode_login(channel: str) -> None: (_t("Retry", "重试"), "retry"), (_t("Skip this channel", "跳过此渠道"), "skip"), ], - non_interactive=False, + non_interactive=non_interactive, ) if choice == "retry": continue @@ -1871,14 +1879,14 @@ def _scancode_login(channel: str) -> None: disable_channel(channel) -def _add_one_channel() -> None: +def _add_one_channel(*, non_interactive: bool = False) -> None: """Pick + (scancode login | reflect-prompt) + enable one channel.""" while True: channel = _select_channel() if channel is None or channel is _BACK: return if _channel_uses_interactive_login(channel): - _scancode_login(channel) + _scancode_login(channel, non_interactive=non_interactive) return fields = _prompt_channel_fields(channel) if fields is _BACK: @@ -1984,7 +1992,7 @@ def _step3_channel(*, channel: Optional[str], skip: bool, non_interactive: bool) if channel: if _channel_uses_interactive_login(channel): - _scancode_login(channel) + _scancode_login(channel, non_interactive=non_interactive) else: fields = _prompt_channel_fields(channel) if fields is _BACK: @@ -2022,7 +2030,7 @@ def _step3_channel(*, channel: Optional[str], skip: bool, non_interactive: bool) if action == "skip": console.print(_t(" [dim]Skipped.[/dim]", " [dim]已跳过。[/dim]")) return None - _add_one_channel() + _add_one_channel(non_interactive=non_interactive) continue action = questionary.select( @@ -2043,7 +2051,7 @@ def _step3_channel(*, channel: Optional[str], skip: bool, non_interactive: bool) if action == "done": return None if action == "add": - _add_one_channel() + _add_one_channel(non_interactive=non_interactive) elif action == "edit": _manage_existing_channels() diff --git a/tests/test_cli_onboard_commands.py b/tests/test_cli_onboard_commands.py index a0571a2..a15ff06 100644 --- a/tests/test_cli_onboard_commands.py +++ b/tests/test_cli_onboard_commands.py @@ -1335,7 +1335,7 @@ def test_test_probe_rekey_completes_write_and_retry(tmp_env: Path, monkeypatch: spec = find_by_name("custom") set_provider_fields("custom", {"api_key": "k-old", "api_base": "https://x/v1"}) - monkeypatch.setattr(onboard_commands, "_verify_provider", lambda p: (True, "ok", None)) + monkeypatch.setattr(onboard_commands, "_verify_provider", lambda p, **kw: (True, "ok", None)) outcomes = iter([RuntimeError("bad key"), ("hi", 1, 0.1)]) def _probe(): @@ -1575,7 +1575,7 @@ def test_custom_provider_sends_test_probe(tmp_env: Path, monkeypatch: pytest.Mon spec = find_by_name("custom") set_provider_fields("custom", {"api_key": "k", "api_base": "https://x/v1"}) - monkeypatch.setattr(onboard_commands, "_verify_provider", lambda p: (True, "ok", None)) + monkeypatch.setattr(onboard_commands, "_verify_provider", lambda p, **kw: (True, "ok", None)) calls: list[bool] = [] def _probe(): @@ -1598,7 +1598,7 @@ def test_custom_provider_probe_failure_switch_rewinds(tmp_env: Path, monkeypatch spec = find_by_name("custom") set_provider_fields("custom", {"api_key": "k", "api_base": "https://x/v1"}) - monkeypatch.setattr(onboard_commands, "_verify_provider", lambda p: (True, "ok", None)) + monkeypatch.setattr(onboard_commands, "_verify_provider", lambda p, **kw: (True, "ok", None)) def _boom(): raise RuntimeError("model not found") @@ -2045,12 +2045,48 @@ def test_scancode_login_skip_reverts_enable(tmp_env: Path, monkeypatch: pytest.M assert data["channels"]["weixin"]["enabled"] is False +def test_verify_provider_skip_test_omits_test_message_promise(monkeypatch: pytest.MonkeyPatch) -> None: + """Under --skip-test, the pre-check skip note must not promise a test message + (none runs); without it, the promise stays.""" + monkeypatch.setattr( + "raven.config.update_providers.test_provider", + lambda p: {"ok": False, "status": "not_configured", "error": "api_base is empty"}, + ) + printed: list[str] = [] + monkeypatch.setattr(onboard_commands.console, "print", lambda *a, **kw: printed.append(" ".join(str(x) for x in a))) + onboard_commands._verify_provider("deepseek", skip_test=True) + blob = "\n".join(printed) + assert "not tested" in blob and "test message below" not in blob + printed.clear() + onboard_commands._verify_provider("deepseek", skip_test=False) + assert "test message below" in "\n".join(printed) + + +def test_scancode_login_non_interactive_node_missing_auto_skips(monkeypatch: pytest.MonkeyPatch) -> None: + """With non_interactive threaded in, a missing Node auto-skips (no prompt) + and still reverts the enable.""" + + class _Spec: + display_name = "WhatsApp" + + def factory(self, cfg): + return object() + + monkeypatch.setattr(onboard_commands, "_node_runtime_missing", lambda ch: True) + monkeypatch.setattr(onboard_commands, "_enable_channel", lambda ch, fields: None) + monkeypatch.setattr("raven.channels.registry.discover_specs", lambda: {"whatsapp": _Spec()}) + disabled: list[str] = [] + monkeypatch.setattr("raven.config.update_channels.disable_channel", lambda ch: disabled.append(ch)) + onboard_commands._scancode_login("whatsapp", non_interactive=True) + assert disabled == ["whatsapp"] + + def test_add_one_channel_routes_scancode(tmp_env: Path, monkeypatch: pytest.MonkeyPatch) -> None: """`_add_one_channel` sends a scancode channel to login, NOT schema prompts.""" monkeypatch.setattr(onboard_commands, "_select_provider", lambda: "weixin") monkeypatch.setattr(onboard_commands, "_select_channel", lambda: "weixin") routed: list[str] = [] - monkeypatch.setattr(onboard_commands, "_scancode_login", lambda c: routed.append(c)) + monkeypatch.setattr(onboard_commands, "_scancode_login", lambda c, **kw: routed.append(c)) monkeypatch.setattr(onboard_commands, "_prompt_channel_fields", _must_not_call("_prompt_channel_fields")) onboard_commands._add_one_channel() assert routed == ["weixin"]