fix(onboard): require explicit save in interactive wizard

Cherry-pick from d6acf1a with manual merge resolution. Keep onboarding edits in draft state until users choose Done or Save and Exit, so backing out or discarding the wizard no longer persists partial changes. Co-Authored-By: Jason Zhao <144443939+JasonZhaoWW@users.noreply.github.com>
2026-03-19 16:54:23 +08:00
parent a6fb90291d
commit 45e89d917b
4 changed files with 297 additions and 89 deletions
--- a/tests/test_commands.py
+++ b/tests/test_commands.py
@@ -15,7 +15,7 @@ from nanobot.providers.registry import find_by_model
 runner = CliRunner()


-class _StopGateway(RuntimeError):
+class _StopGatewayError(RuntimeError):
    pass


@@ -133,6 +133,24 @@ def test_onboard_help_shows_workspace_and_config_options():
    assert "--dir" not in stripped_output


+def test_onboard_interactive_discard_does_not_save_or_create_workspace(mock_paths, monkeypatch):
+    config_file, workspace_dir, _ = mock_paths
+
+    from nanobot.cli.onboard_wizard import OnboardResult
+
+    monkeypatch.setattr(
+        "nanobot.cli.onboard_wizard.run_onboard",
+        lambda initial_config: OnboardResult(config=initial_config, should_save=False),
+    )
+
+    result = runner.invoke(app, ["onboard"])
+
+    assert result.exit_code == 0
+    assert "No changes were saved" in result.stdout
+    assert not config_file.exists()
+    assert not workspace_dir.exists()
+
+
 def test_onboard_uses_explicit_config_and_workspace_paths(tmp_path, monkeypatch):
    config_path = tmp_path / "instance" / "config.json"
    workspace_path = tmp_path / "workspace"
@@ -438,12 +456,12 @@ def test_gateway_uses_workspace_from_config_by_default(monkeypatch, tmp_path: Pa
    )
    monkeypatch.setattr(
        "nanobot.cli.commands._make_provider",
-        lambda _config: (_ for _ in ()).throw(_StopGateway("stop")),
+        lambda _config: (_ for _ in ()).throw(_StopGatewayError("stop")),
    )

    result = runner.invoke(app, ["gateway", "--config", str(config_file)])

-    assert isinstance(result.exception, _StopGateway)
+    assert isinstance(result.exception, _StopGatewayError)
    assert seen["config_path"] == config_file.resolve()
    assert seen["workspace"] == Path(config.agents.defaults.workspace)

@@ -466,7 +484,7 @@ def test_gateway_workspace_option_overrides_config(monkeypatch, tmp_path: Path)
    )
    monkeypatch.setattr(
        "nanobot.cli.commands._make_provider",
-        lambda _config: (_ for _ in ()).throw(_StopGateway("stop")),
+        lambda _config: (_ for _ in ()).throw(_StopGatewayError("stop")),
    )

    result = runner.invoke(
@@ -474,7 +492,7 @@ def test_gateway_workspace_option_overrides_config(monkeypatch, tmp_path: Path)
        ["gateway", "--config", str(config_file), "--workspace", str(override)],
    )

-    assert isinstance(result.exception, _StopGateway)
+    assert isinstance(result.exception, _StopGatewayError)
    assert seen["workspace"] == override
    assert config.workspace_path == override

@@ -492,12 +510,12 @@ def test_gateway_warns_about_deprecated_memory_window(monkeypatch, tmp_path: Pat
    monkeypatch.setattr("nanobot.cli.commands.sync_workspace_templates", lambda _path: None)
    monkeypatch.setattr(
        "nanobot.cli.commands._make_provider",
-        lambda _config: (_ for _ in ()).throw(_StopGateway("stop")),
+        lambda _config: (_ for _ in ()).throw(_StopGatewayError("stop")),
    )

    result = runner.invoke(app, ["gateway", "--config", str(config_file)])

-    assert isinstance(result.exception, _StopGateway)
+    assert isinstance(result.exception, _StopGatewayError)
    assert "memoryWindow" in result.stdout
    assert "contextWindowTokens" in result.stdout

@@ -521,13 +539,13 @@ def test_gateway_uses_config_directory_for_cron_store(monkeypatch, tmp_path: Pat
    class _StopCron:
        def __init__(self, store_path: Path) -> None:
            seen["cron_store"] = store_path
-            raise _StopGateway("stop")
+            raise _StopGatewayError("stop")

    monkeypatch.setattr("nanobot.cron.service.CronService", _StopCron)

    result = runner.invoke(app, ["gateway", "--config", str(config_file)])

-    assert isinstance(result.exception, _StopGateway)
+    assert isinstance(result.exception, _StopGatewayError)
    assert seen["cron_store"] == config_file.parent / "cron" / "jobs.json"


@@ -544,12 +562,12 @@ def test_gateway_uses_configured_port_when_cli_flag_is_missing(monkeypatch, tmp_
    monkeypatch.setattr("nanobot.cli.commands.sync_workspace_templates", lambda _path: None)
    monkeypatch.setattr(
        "nanobot.cli.commands._make_provider",
-        lambda _config: (_ for _ in ()).throw(_StopGateway("stop")),
+        lambda _config: (_ for _ in ()).throw(_StopGatewayError("stop")),
    )

    result = runner.invoke(app, ["gateway", "--config", str(config_file)])

-    assert isinstance(result.exception, _StopGateway)
+    assert isinstance(result.exception, _StopGatewayError)
    assert "port 18791" in result.stdout


@@ -566,10 +584,10 @@ def test_gateway_cli_port_overrides_configured_port(monkeypatch, tmp_path: Path)
    monkeypatch.setattr("nanobot.cli.commands.sync_workspace_templates", lambda _path: None)
    monkeypatch.setattr(
        "nanobot.cli.commands._make_provider",
-        lambda _config: (_ for _ in ()).throw(_StopGateway("stop")),
+        lambda _config: (_ for _ in ()).throw(_StopGatewayError("stop")),
    )

    result = runner.invoke(app, ["gateway", "--config", str(config_file), "--port", "18792"])

-    assert isinstance(result.exception, _StopGateway)
+    assert isinstance(result.exception, _StopGatewayError)
    assert "port 18792" in result.stdout
--- a/tests/test_onboard_logic.py
+++ b/tests/test_onboard_logic.py
@@ -7,18 +7,24 @@ without testing the interactive UI components.
 import json
 from pathlib import Path
 from types import SimpleNamespace
-from typing import Any
+from typing import Any, cast

 import pytest
 from pydantic import BaseModel, Field

+from nanobot.cli import onboard_wizard
+
 # Import functions to test
 from nanobot.cli.commands import _merge_missing_defaults
 from nanobot.cli.onboard_wizard import (
+    _BACK_PRESSED,
+    _configure_pydantic_model,
    _format_value,
    _get_field_display_name,
    _get_field_type_info,
+    run_onboard,
 )
+from nanobot.config.schema import Config
 from nanobot.utils.helpers import sync_workspace_templates


@@ -371,3 +377,116 @@ class TestProviderChannelInfo:
        for provider_name, value in info.items():
            assert isinstance(value, tuple)
            assert len(value) == 4  # (display_name, needs_api_key, needs_api_base, env_var)
+
+
+class _SimpleDraftModel(BaseModel):
+    api_key: str = ""
+
+
+class _NestedDraftModel(BaseModel):
+    api_key: str = ""
+
+
+class _OuterDraftModel(BaseModel):
+    nested: _NestedDraftModel = Field(default_factory=_NestedDraftModel)
+
+
+class TestConfigurePydanticModelDrafts:
+    @staticmethod
+    def _patch_prompt_helpers(monkeypatch, tokens, text_value="secret"):
+        sequence = iter(tokens)
+
+        def fake_select(_prompt, choices, default=None):
+            token = next(sequence)
+            if token == "first":
+                return choices[0]
+            if token == "done":
+                return "✓ Done"
+            if token == "back":
+                return _BACK_PRESSED
+            return token
+
+        monkeypatch.setattr(onboard_wizard, "_select_with_back", fake_select)
+        monkeypatch.setattr(onboard_wizard, "_show_config_panel", lambda *_args, **_kwargs: None)
+        monkeypatch.setattr(
+            onboard_wizard, "_input_with_existing", lambda *_args, **_kwargs: text_value
+        )
+
+    def test_discarding_section_keeps_original_model_unchanged(self, monkeypatch):
+        model = _SimpleDraftModel()
+        self._patch_prompt_helpers(monkeypatch, ["first", "back"])
+
+        result = _configure_pydantic_model(model, "Simple")
+
+        assert result is None
+        assert model.api_key == ""
+
+    def test_completing_section_returns_updated_draft(self, monkeypatch):
+        model = _SimpleDraftModel()
+        self._patch_prompt_helpers(monkeypatch, ["first", "done"])
+
+        result = _configure_pydantic_model(model, "Simple")
+
+        assert result is not None
+        updated = cast(_SimpleDraftModel, result)
+        assert updated.api_key == "secret"
+        assert model.api_key == ""
+
+    def test_nested_section_back_discards_nested_edits(self, monkeypatch):
+        model = _OuterDraftModel()
+        self._patch_prompt_helpers(monkeypatch, ["first", "first", "back", "done"])
+
+        result = _configure_pydantic_model(model, "Outer")
+
+        assert result is not None
+        updated = cast(_OuterDraftModel, result)
+        assert updated.nested.api_key == ""
+        assert model.nested.api_key == ""
+
+    def test_nested_section_done_commits_nested_edits(self, monkeypatch):
+        model = _OuterDraftModel()
+        self._patch_prompt_helpers(monkeypatch, ["first", "first", "done", "done"])
+
+        result = _configure_pydantic_model(model, "Outer")
+
+        assert result is not None
+        updated = cast(_OuterDraftModel, result)
+        assert updated.nested.api_key == "secret"
+        assert model.nested.api_key == ""
+
+
+class TestRunOnboardExitBehavior:
+    def test_main_menu_interrupt_can_discard_unsaved_session_changes(self, monkeypatch):
+        initial_config = Config()
+
+        responses = iter(
+            [
+                "🤖 Configure Agent Settings",
+                KeyboardInterrupt(),
+                "🗑️ Exit Without Saving",
+            ]
+        )
+
+        class FakePrompt:
+            def __init__(self, response):
+                self.response = response
+
+            def ask(self):
+                if isinstance(self.response, BaseException):
+                    raise self.response
+                return self.response
+
+        def fake_select(*_args, **_kwargs):
+            return FakePrompt(next(responses))
+
+        def fake_configure_agents(config):
+            config.agents.defaults.model = "test/provider-model"
+
+        monkeypatch.setattr(onboard_wizard, "_show_main_menu_header", lambda: None)
+        monkeypatch.setattr(onboard_wizard.questionary, "select", fake_select)
+        monkeypatch.setattr(onboard_wizard, "_configure_agents", fake_configure_agents)
+
+        result = run_onboard(initial_config=initial_config)
+
+        assert result.should_save is False
+        assert result.config.model_dump(by_alias=True) == initial_config.model_dump(by_alias=True)