diff --git a/CHANGELOG.md b/CHANGELOG.md index cba7486..9f2ec11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Changelog +## 0.7.2 · `agent create` KB→index picker cascade; hidden-param OptionInfo leak + +### Fixed + +- **Interactive `pais agent create` now shows a live KB picker followed by an index picker, instead of a raw text prompt.** Previously, because `agent create` has no `kb_ref` parameter, `pick_index` tripped its "kb_ref not yet chosen" fallback and dropped the user into `? type the index alias or UUID:`. The picker now cascades — when no KB is in scope it calls `pick_kb` first, stashes the pick in the shared `PickerContext.answers`, and then lists indexes under it. `pick_or_create_index` does the same against `pick_or_create_kb` and propagates `← back` / `+ create new` cleanly. +- The fix is applied *inside* the pickers, so any future command that binds an index picker without a preceding KB parameter (e.g. a hypothetical `agent update --index-id`) inherits the cascade automatically. +- **Hidden params no longer leak `typer.OptionInfo` into callbacks.** The interactive dispatcher skipped `hidden=True` params entirely, so they weren't added to the `answers` dict — and `spec.callback(**answers)` fell back to the function's declared default, which is the raw `typer.Option(None, …)` `OptionInfo` wrapper. In `agent create`, the legacy `--kb-search-tool` (hidden) tripped `if kb_search_tool:` (always truthy on an `OptionInfo`) and crashed with `ToolLink.tool_id: Input should be a valid string [input_type=OptionInfo]`. The dispatcher now injects the hidden param's declared default (here `None`) into `answers` before calling the callback — matching the pattern already used for destructive `yes`. + ## 0.7.1 · `agent create` doc-aligned; survive undocumented MCP endpoint ### Fixed diff --git a/pyproject.toml b/pyproject.toml index 10cc47a..363e7b7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "pais-sdk-cli" -version = "0.7.1" +version = "0.7.2" description = "Contract-first Python SDK + CLI for VMware Private AI Service (PAIS), with mock server for offline development" readme = "README.md" requires-python = ">=3.10" diff --git a/src/pais/__init__.py b/src/pais/__init__.py index 98a4ea0..8bc294e 100644 --- a/src/pais/__init__.py +++ b/src/pais/__init__.py @@ -26,4 +26,4 @@ "Settings", ] -__version__ = "0.7.1" +__version__ = "0.7.2" diff --git a/src/pais/cli/_pickers.py b/src/pais/cli/_pickers.py index a453263..7e51e02 100644 --- a/src/pais/cli/_pickers.py +++ b/src/pais/cli/_pickers.py @@ -97,10 +97,19 @@ def pick_kb(ctx: PickerContext) -> Any: def pick_index(ctx: PickerContext) -> Any: - """Choose an index under the previously-picked KB.""" + """Choose an index under the previously-picked KB. + + When no KB is in scope yet (e.g. `agent create`, which has no `kb_ref` + parameter), cascade into `pick_kb` first so the user sees a list of + existing KBs instead of a free-text fallback. + """ kb_ref = ctx.answers.get("kb_ref") or ctx.answers.get("kb_id") if not kb_ref: - return _manual_fallback("kb_ref not yet chosen; type the index alias or UUID:") + picked = pick_kb(ctx) + if picked is CANCEL: + return CANCEL + ctx.answers["kb_ref"] = picked + kb_ref = picked cfg, _, _ = load_profile_config() try: kb_uuid = _alias.resolve_kb(ctx.client, ctx.profile, str(kb_ref), cfg=cfg) @@ -416,11 +425,20 @@ def pick_or_create_kb(ctx: PickerContext) -> Any: def pick_or_create_index(ctx: PickerContext) -> Any: - """Like `pick_index` but with recents + `+ create new`. Requires - `ctx.answers['kb_ref']` (or 'kb_id') to scope the index list.""" + """Like `pick_index` but with recents + `+ create new`. + + When no KB is in scope yet, cascade into `pick_or_create_kb` so the + `+ create new` affordance is preserved. If the user chooses to create + a new KB, bubble `CREATE_NEW` back up so the caller's KB-create branch + runs — listing indexes under a non-existent KB would be meaningless. + """ kb_ref = ctx.answers.get("kb_ref") or ctx.answers.get("kb_id") if not kb_ref: - return _manual_fallback("kb_ref not yet chosen; type the index alias or UUID:") + picked = pick_or_create_kb(ctx) + if picked is CANCEL or picked == CREATE_NEW: + return picked + ctx.answers["kb_ref"] = picked + kb_ref = picked cfg, _, _ = load_profile_config() try: kb_uuid = _alias.resolve_kb(ctx.client, ctx.profile, str(kb_ref), cfg=cfg) diff --git a/src/pais/cli/interactive.py b/src/pais/cli/interactive.py index 2f1c056..d4447a4 100644 --- a/src/pais/cli/interactive.py +++ b/src/pais/cli/interactive.py @@ -174,7 +174,11 @@ def _dispatch(spec: CommandSpec, settings: Settings, console: Console) -> None: for param in spec.params: # Hidden params are for scripted use only — keep them callable via # flags but skip the interactive prompt so the shell stays quiet. + # Inject the declared default so the callback receives the real + # value (e.g. None), not typer's raw OptionInfo wrapper — otherwise + # `if param:` branches see a truthy OptionInfo and explode downstream. if param.hidden: + answers[param.name] = param.default continue # The destructive confirm below auto-injects `yes=True`; don't # prompt the user about it separately. diff --git a/tests/test_interactive_dispatch.py b/tests/test_interactive_dispatch.py index cab3048..4ec9b42 100644 --- a/tests/test_interactive_dispatch.py +++ b/tests/test_interactive_dispatch.py @@ -8,7 +8,7 @@ import pytest -from pais.cli import _alias, _landing, _pickers, interactive +from pais.cli import _alias, _landing, _pickers, _prompts, interactive from pais.cli._introspect import walk from pais.cli.app import app from pais.client import PaisClient @@ -56,6 +56,7 @@ def fake_q(monkeypatch: pytest.MonkeyPatch) -> _FakeQuestionary: monkeypatch.setattr(interactive, "questionary", fq) monkeypatch.setattr(_pickers, "questionary", fq) monkeypatch.setattr(_landing, "questionary", fq) + monkeypatch.setattr(_prompts, "questionary", fq) return fq @@ -128,6 +129,80 @@ def test_quit_exits_immediately(fake_q: _FakeQuestionary, isolated_cache: None) assert len([c for c in fake_q.calls if c["kind"] == "select"]) == 2 +def test_agent_create_flow_picks_kb_then_index( + fake_q: _FakeQuestionary, + isolated_cache: None, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """`agent create` has no `kb_ref` parameter. The index_id picker must + cascade into a KB pick first, then an index pick under that KB — not + fall through to a free-text prompt. The resolved index UUID must reach + the callback (not the raw menu title).""" + from rich.console import Console + + from pais.cli._workflows import _base as _workflows_base + from pais.cli.interactive import _dispatch + + # Seed a KB + index in a shared store. + store = Store() + seed_client = PaisClient(FakeTransport(store)) + kb = seed_client.knowledge_bases.create(KnowledgeBaseCreate(name="kb-agent")) + ix = seed_client.indexes.create( + kb.id, + IndexCreate(name="ix-agent", embeddings_model_endpoint="BAAI/bge-small-en-v1.5"), + ) + + def _build(_self: Any) -> PaisClient: + return PaisClient(FakeTransport(store)) + + monkeypatch.setattr(Settings, "build_client", _build) + # The optional-review screen lives in _workflows._base; patch its questionary too. + monkeypatch.setattr(_workflows_base, "questionary", fake_q) + + # Locate the agent-create spec and intercept its callback. + specs = walk(app) + spec = next(s for s in specs if s.path == ("agent", "create")) + captured: dict[str, Any] = {} + + def _recorder(**kwargs: Any) -> None: + captured.update(kwargs) + + spec.callback = _recorder # type: ignore[misc] + + # Script the dispatch flow in order: + # 1. text — name + # 2. select — chat model picker + # 3. select — cascaded KB picker (kb_ref absent → pick_kb fires) + # 4. select — index picker under picked KB + # 5. select — optional-review screen → ✅ Go + fake_q.script( + "my-agent", + "openai/gpt-oss-120b-4x · VLLM", + f"— kb-agent ({kb.id})", + f"— ix-agent (status=AVAILABLE, docs=—, id={ix.id})", + "✅ Go (commit)", + ) + + _dispatch(spec, Settings(), Console()) + + assert captured.get("index_id") == ix.id, "resolved index UUID must reach the callback" + assert captured.get("name") == "my-agent" + + selects = [c for c in fake_q.calls if c["kind"] == "select"] + assert "chat model" in selects[0]["message"].lower() + assert "Pick a KB" in selects[1]["message"] + assert "under" in selects[2]["message"], "index picker must be scoped by the picked KB" + # And explicitly: no free-text prompt for index_id. + texts = [c for c in fake_q.calls if c["kind"] == "text"] + assert not any("index alias or UUID" in t["message"] for t in texts) + + # Hidden `kb_search_tool` must reach the callback as its declared default + # (None), NOT as typer's raw OptionInfo wrapper — otherwise the `if + # kb_search_tool:` branch in agent_create trips and pydantic blows up on + # ToolLink(tool_id=). + assert captured.get("kb_search_tool") is None + + def test_picker_status_label_lookup(fake_q: _FakeQuestionary, isolated_cache: None) -> None: """Make sure the index picker uses ix.status correctly even when the model returns the StatusEnum (not a raw string).""" diff --git a/tests/test_interactive_pickers.py b/tests/test_interactive_pickers.py index d8141c5..7420c3e 100644 --- a/tests/test_interactive_pickers.py +++ b/tests/test_interactive_pickers.py @@ -8,6 +8,7 @@ from pais.cli import _alias, _pickers from pais.cli._pickers import PickerContext, pick_index, pick_kb, pick_splitter_kind +from pais.cli._prompts import CANCEL from pais.client import PaisClient from pais.errors import PaisServerError from pais.models import IndexCreate, KnowledgeBaseCreate @@ -274,3 +275,82 @@ def list(self) -> Any: client.models = _BoomModels() # type: ignore[assignment] ctx = PickerContext(client=client, answers={}, profile="default") assert first_model_id(ctx, kind="EMBEDDINGS") is None + + +# ----- KB→index cascade (v0.7.2) --------------------------------------------- + + +def test_pick_index_cascades_to_kb_pick_when_missing( + fake_q: _FakeQuestionary, isolated_cache: None +) -> None: + """No KB in scope (e.g. `agent create`) → cascade into pick_kb, then list + indexes under the picked KB. User sees two select lists, not a text prompt.""" + store = Store() + client = PaisClient(FakeTransport(store)) + kb = client.knowledge_bases.create(KnowledgeBaseCreate(name="kb-cascade")) + ix = client.indexes.create( + kb.id, + IndexCreate(name="ix-cascade", embeddings_model_endpoint="BAAI/bge-small-en-v1.5"), + ) + ctx = PickerContext(client=client, answers={}, profile="default") + + calls: list[tuple[str, list[Any]]] = [] + + def _select(message: str, *, choices: list[Any], **_: Any) -> _FakeAsk: + calls.append((message, list(choices))) + return _FakeAsk(choices[0]) + + fake_q.select = _select # type: ignore[method-assign] + result = pick_index(ctx) + + assert len(calls) == 2, "expected KB picker then index picker" + assert "Pick a KB" in calls[0][0] + assert "under" in calls[1][0] + assert ctx.answers["kb_ref"] == kb.id + assert result == ix.id + + +def test_pick_index_cascade_cancel_propagates( + fake_q: _FakeQuestionary, isolated_cache: None +) -> None: + """If the user hits `← back` on the cascaded KB picker, the outer + pick_index returns CANCEL and leaves ctx.answers untouched.""" + store = Store() + client = PaisClient(FakeTransport(store)) + client.knowledge_bases.create(KnowledgeBaseCreate(name="kb-cancel")) + ctx = PickerContext(client=client, answers={}, profile="default") + + def _select(message: str, *, choices: list[Any], **_: Any) -> _FakeAsk: + return _FakeAsk(_pickers._BACK) + + fake_q.select = _select # type: ignore[method-assign] + result = pick_index(ctx) + + assert result is CANCEL + assert "kb_ref" not in ctx.answers + + +def test_pick_or_create_index_cascades_create_new( + fake_q: _FakeQuestionary, isolated_cache: None +) -> None: + """pick_or_create_index → user picks `+ create new` from the cascaded KB + picker → surface CREATE_NEW upward without attempting to list indexes + under a non-existent KB.""" + store = Store() + client = PaisClient(FakeTransport(store)) + client.knowledge_bases.create(KnowledgeBaseCreate(name="kb-create-cascade")) + ctx = PickerContext(client=client, answers={}, profile="default") + + calls: list[str] = [] + + def _select(message: str, *, choices: list[Any], **_: Any) -> _FakeAsk: + calls.append(message) + return _FakeAsk(_pickers._CREATE) + + fake_q.select = _select # type: ignore[method-assign] + result = _pickers.pick_or_create_index(ctx) + + assert result == _pickers.CREATE_NEW + assert len(calls) == 1, "index list should never be attempted" + assert "KB" in calls[0] + assert "kb_ref" not in ctx.answers diff --git a/uv.lock b/uv.lock index 91c4289..92f94ee 100644 --- a/uv.lock +++ b/uv.lock @@ -899,7 +899,7 @@ wheels = [ [[package]] name = "pais-sdk-cli" -version = "0.7.1" +version = "0.7.2" source = { editable = "." } dependencies = [ { name = "fastapi" },