Merge aaa9546890 into 554a3d1f6a

cli/test/test_config_generator.py

@@ -3,8 +3,11 @@ import pytest
 import yaml
 from unittest import mock
 from planoai.config_generator import (
-    validate_and_render_schema,
+    CLAUDE_CLI_DEFAULT_BASE_URL,
+    _apply_claude_cli_autofill,
+    _is_claude_cli_provider,
     migrate_inline_routing_preferences,
+    validate_and_render_schema,
 )
 
 
@@ -795,3 +798,64 @@ model_providers:
     migrate_inline_routing_preferences(config_yaml)
 
     assert config_yaml["version"] == "v0.5.0"
+
+
+def test_claude_cli_autofill_wildcard_provider():
+    provider = {"model": "claude-cli/*"}
+    assert _is_claude_cli_provider(provider) is True
+    assert _apply_claude_cli_autofill(provider) is True
+    assert provider["name"] == "claude-cli/*"
+    assert provider["provider_interface"] == "claude-cli"
+    assert provider["base_url"] == CLAUDE_CLI_DEFAULT_BASE_URL
+    assert provider["access_key"] == "claude-cli-local"
+    # `model` itself must not be rewritten — the wildcard expansion happens
+    # downstream and we want to preserve the user's intent.
+    assert provider["model"] == "claude-cli/*"
+
+
+def test_claude_cli_autofill_specific_model():
+    provider = {"model": "claude-cli/sonnet", "default": True}
+    assert _apply_claude_cli_autofill(provider) is True
+    assert provider["name"] == "claude-cli/sonnet"
+    assert provider["provider_interface"] == "claude-cli"
+    assert provider["base_url"] == CLAUDE_CLI_DEFAULT_BASE_URL
+    # Existing fields like `default` survive.
+    assert provider["default"] is True
+
+
+def test_claude_cli_autofill_does_not_override_user_fields():
+    provider = {
+        "model": "claude-cli/*",
+        "name": "custom-name",
+        "base_url": "http://192.0.2.10:9000",
+        "access_key": "do-not-touch",
+    }
+    assert _apply_claude_cli_autofill(provider) is True
+    assert provider["name"] == "custom-name"
+    assert provider["base_url"] == "http://192.0.2.10:9000"
+    assert provider["access_key"] == "do-not-touch"
+    # provider_interface still gets injected because it was missing.
+    assert provider["provider_interface"] == "claude-cli"
+
+
+def test_claude_cli_autofill_skips_non_matching_providers():
+    provider = {"model": "openai/gpt-4o"}
+    assert _is_claude_cli_provider(provider) is False
+    assert _apply_claude_cli_autofill(provider) is False
+    assert "provider_interface" not in provider
+
+
+def test_claude_cli_autofill_passthrough_auth_skips_access_key():
+    provider = {"model": "claude-cli/*", "passthrough_auth": True}
+    assert _apply_claude_cli_autofill(provider) is True
+    # Honor passthrough_auth: do not inject a placeholder access_key.
+    assert "access_key" not in provider
+    assert provider["passthrough_auth"] is True
+
+
+def test_claude_cli_autofill_detects_via_provider_interface_only():
+    provider = {"model": "sonnet", "provider_interface": "claude-cli"}
+    assert _is_claude_cli_provider(provider) is True
+    assert _apply_claude_cli_autofill(provider) is True
+    assert provider["base_url"] == CLAUDE_CLI_DEFAULT_BASE_URL
+    assert provider["name"] == "sonnet"

cli/test/test_local_agent_warning.py

@@ -0,0 +1,324 @@
+"""Tests for the local-agent provider warning, ack persistence, and the
+detection logic that decides whether to fire it."""
+
+from __future__ import annotations
+
+import io
+import json
+
+import pytest
+from rich.console import Console
+
+from planoai import local_agent_warning as law
+
+
+def _make_console() -> tuple[Console, io.StringIO]:
+    buf = io.StringIO()
+    # ``force_terminal=False`` keeps Rich from emitting ANSI escapes,
+    # which makes substring assertions readable. ``width`` is generous
+    # so the panel border doesn't soft-wrap text mid-keyword.
+    console = Console(file=buf, force_terminal=False, color_system=None, width=140)
+    return console, buf
+
+
+# ---------------------------------------------------------------------------
+# detection
+# ---------------------------------------------------------------------------
+
+
+def test_detects_claude_cli_via_model_prefix():
+    config = {
+        "model_providers": [
+            {"model": "claude-cli/sonnet"},
+            {"model": "openai/gpt-4o"},
+        ]
+    }
+    found = law.detect_local_agent_providers(config)
+    assert [p.interface for p in found] == ["claude-cli"]
+    assert found[0].model == "claude-cli/sonnet"
+
+
+def test_detects_claude_cli_via_explicit_provider_interface():
+    config = {
+        "model_providers": [
+            {"name": "local-claude", "provider_interface": "claude-cli", "model": "x"},
+        ]
+    }
+    found = law.detect_local_agent_providers(config)
+    assert [p.interface for p in found] == ["claude-cli"]
+    assert found[0].name == "local-claude"
+
+
+def test_detects_claude_cli_via_legacy_provider_field():
+    config = {"model_providers": [{"provider": "claude-cli", "model": "x"}]}
+    assert [p.interface for p in law.detect_local_agent_providers(config)] == [
+        "claude-cli"
+    ]
+
+
+def test_detects_via_legacy_llm_providers_key():
+    config = {"llm_providers": [{"model": "claude-cli/opus"}]}
+    assert [p.interface for p in law.detect_local_agent_providers(config)] == [
+        "claude-cli"
+    ]
+
+
+def test_no_false_positive_for_network_providers():
+    config = {
+        "model_providers": [
+            {"model": "openai/gpt-4o"},
+            {"model": "anthropic/claude-3-5-sonnet"},
+            {"model": "gemini/gemini-2.5-pro"},
+            {"model": "chatgpt/gpt-5"},  # network ChatGPT subscription, not a CLI
+            {"model": "vercel/some-model"},
+        ]
+    }
+    assert law.detect_local_agent_providers(config) == []
+
+
+def test_no_false_positive_for_anthropic_claude_models():
+    # ``anthropic/claude-3-5-sonnet`` must not trigger just because the
+    # word "claude" appears — the prefix has to be ``claude-cli/``.
+    config = {"model_providers": [{"model": "anthropic/claude-3-5-sonnet-20241022"}]}
+    assert law.detect_local_agent_providers(config) == []
+
+
+def test_empty_or_malformed_config_is_safe():
+    assert law.detect_local_agent_providers({}) == []
+    assert law.detect_local_agent_providers({"model_providers": None}) == []
+    assert law.detect_local_agent_providers({"model_providers": "not-a-list"}) == []
+    # ``None`` config (e.g. from an empty yaml file) must also be safe.
+    assert law.detect_local_agent_providers(None) == []  # type: ignore[arg-type]
+
+
+def test_multiple_entries_same_interface_collapse_in_warning_set():
+    config = {
+        "model_providers": [
+            {"model": "claude-cli/sonnet", "name": "fast"},
+            {"model": "claude-cli/opus", "name": "slow"},
+        ]
+    }
+    found = law.detect_local_agent_providers(config)
+    assert len(found) == 2
+    assert {p.interface for p in found} == {"claude-cli"}
+
+
+# ---------------------------------------------------------------------------
+# ack file
+# ---------------------------------------------------------------------------
+
+
+def test_load_ack_returns_empty_when_missing(tmp_path):
+    ack = tmp_path / "ack.json"
+    assert law.load_acknowledged_interfaces(str(ack)) == set()
+
+
+@pytest.mark.parametrize(
+    "contents",
+    [
+        "{not valid json",
+        "[]",  # not a dict
+        '{"acknowledged": "claude-cli"}',  # not a list
+        '{"acknowledged": [1, 2, 3]}',  # not strings
+    ],
+)
+def test_load_ack_handles_malformed_files(tmp_path, contents):
+    ack = tmp_path / "ack.json"
+    ack.write_text(contents, encoding="utf-8")
+    # Malformed contents must degrade to "no ack" rather than crashing.
+    assert law.load_acknowledged_interfaces(str(ack)) == set()
+
+
+def test_write_ack_creates_state_dir(tmp_path):
+    ack = tmp_path / "fresh" / "deeper" / "ack.json"
+    merged = law.write_acknowledgement(["claude-cli"], ack_path=str(ack))
+    assert merged == {"claude-cli"}
+    assert ack.exists()
+    payload = json.loads(ack.read_text(encoding="utf-8"))
+    assert payload["acknowledged"] == ["claude-cli"]
+    assert payload["ack_at"]
+
+
+def test_write_ack_merges_with_existing(tmp_path):
+    ack = tmp_path / "ack.json"
+    law.write_acknowledgement(["claude-cli"], ack_path=str(ack))
+    merged = law.write_acknowledgement(["future-cli"], ack_path=str(ack))
+    assert merged == {"claude-cli", "future-cli"}
+    payload = json.loads(ack.read_text(encoding="utf-8"))
+    assert payload["acknowledged"] == ["claude-cli", "future-cli"]
+
+
+# ---------------------------------------------------------------------------
+# maybe_warn_local_agent_providers
+# ---------------------------------------------------------------------------
+
+
+def test_no_panel_when_no_local_agent_providers(tmp_path):
+    console, buf = _make_console()
+    fired = law.maybe_warn_local_agent_providers(
+        {"model_providers": [{"model": "openai/gpt-4o"}]},
+        console,
+        ack_path=str(tmp_path / "ack.json"),
+        env={},
+    )
+    assert fired is False
+    assert buf.getvalue() == ""
+
+
+def test_panel_fires_for_unacked_claude_cli(tmp_path):
+    console, buf = _make_console()
+    fired = law.maybe_warn_local_agent_providers(
+        {"model_providers": [{"model": "claude-cli/sonnet"}]},
+        console,
+        ack_path=str(tmp_path / "ack.json"),
+        env={},
+    )
+    output = buf.getvalue()
+    assert fired is True
+    # Stable substrings — never pin exact wording.
+    assert "claude-cli" in output
+    assert "Local-agent" in output or "local-agent" in output
+    assert "Learn more" in output
+    assert "--ack-local-agents" in output
+    # The panel is intentionally compact: it must NOT leak the ack file
+    # path into the user-visible reminder. The ``rm`` instruction lives
+    # in the docs page that "Learn more" links to.
+    assert "local_agent_ack.json" not in output
+    assert "docs.planoai.dev" in output
+
+
+def test_panel_suppressed_when_ack_covers_interface(tmp_path):
+    ack = tmp_path / "ack.json"
+    law.write_acknowledgement(["claude-cli"], ack_path=str(ack))
+
+    console, buf = _make_console()
+    fired = law.maybe_warn_local_agent_providers(
+        {"model_providers": [{"model": "claude-cli/sonnet"}]},
+        console,
+        ack_path=str(ack),
+        env={},
+    )
+    assert fired is False
+    # The dim INFO line still mentions the ack file so the operator
+    # knows how to undo, but no panel renders.
+    out = buf.getvalue()
+    assert "Panel" not in out  # no panel object
+    assert "claude-cli" in out
+
+
+def test_new_unacked_interface_re_triggers(tmp_path, monkeypatch):
+    # Simulate a future where two local-agent interfaces exist and the
+    # user has only acknowledged one of them.
+    monkeypatch.setattr(
+        law, "LOCAL_AGENT_PROVIDER_INTERFACES", ("claude-cli", "future-cli")
+    )
+
+    ack = tmp_path / "ack.json"
+    law.write_acknowledgement(["claude-cli"], ack_path=str(ack))
+
+    console, buf = _make_console()
+    fired = law.maybe_warn_local_agent_providers(
+        {
+            "model_providers": [
+                {"model": "claude-cli/sonnet"},
+                {"model": "future-cli/whatever"},
+            ]
+        },
+        console,
+        ack_path=str(ack),
+        env={},
+    )
+    output = buf.getvalue()
+    assert fired is True
+    # The panel must list the *unacknowledged* interface only.
+    assert "future-cli" in output
+    # ...and must NOT re-list the already-acknowledged one as unacked
+    # (it can still appear in the suppressed-info line; we check the
+    # title which only contains pending interfaces).
+    assert "future-cli" in output
+
+
+def test_ack_flag_writes_file_and_suppresses_panel(tmp_path):
+    ack = tmp_path / "ack.json"
+    console, buf = _make_console()
+    fired = law.maybe_warn_local_agent_providers(
+        {"model_providers": [{"model": "claude-cli/sonnet"}]},
+        console,
+        ack_flag=True,
+        ack_path=str(ack),
+        env={},
+    )
+    assert fired is False
+    assert ack.exists()
+    payload = json.loads(ack.read_text(encoding="utf-8"))
+    assert "claude-cli" in payload["acknowledged"]
+    out = buf.getvalue()
+    assert "Acknowledged" in out
+    assert "claude-cli" in out
+
+
+@pytest.mark.parametrize("env_value", ["1", "true", "TRUE", "yes", "on"])
+def test_ack_env_var_truthy_values(tmp_path, env_value):
+    ack = tmp_path / "ack.json"
+    console, _ = _make_console()
+    fired = law.maybe_warn_local_agent_providers(
+        {"model_providers": [{"model": "claude-cli/sonnet"}]},
+        console,
+        ack_path=str(ack),
+        env={law.ACK_ENV_VAR: env_value},
+    )
+    assert fired is False
+    assert ack.exists()
+
+
+@pytest.mark.parametrize("env_value", ["", "0", "false", "no", "off", "maybe"])
+def test_ack_env_var_falsy_values_still_warn(tmp_path, env_value):
+    ack = tmp_path / "ack.json"
+    console, buf = _make_console()
+    fired = law.maybe_warn_local_agent_providers(
+        {"model_providers": [{"model": "claude-cli/sonnet"}]},
+        console,
+        ack_path=str(ack),
+        env={law.ACK_ENV_VAR: env_value},
+    )
+    assert fired is True
+    assert not ack.exists()
+    assert "claude-cli" in buf.getvalue()
+
+
+def test_malformed_ack_falls_back_to_warning(tmp_path):
+    ack = tmp_path / "ack.json"
+    ack.write_text("{not json", encoding="utf-8")
+    console, buf = _make_console()
+    fired = law.maybe_warn_local_agent_providers(
+        {"model_providers": [{"model": "claude-cli/sonnet"}]},
+        console,
+        ack_path=str(ack),
+        env={},
+    )
+    assert fired is True
+    assert "claude-cli" in buf.getvalue()
+
+
+def test_single_panel_when_multiple_local_agent_entries(tmp_path):
+    # Two entries with the same interface must produce one panel,
+    # not two — the warning fires once per ``planoai up`` invocation.
+    console, buf = _make_console()
+    fired = law.maybe_warn_local_agent_providers(
+        {
+            "model_providers": [
+                {"model": "claude-cli/sonnet", "name": "fast"},
+                {"model": "claude-cli/opus", "name": "slow"},
+            ]
+        },
+        console,
+        ack_path=str(tmp_path / "ack.json"),
+        env={},
+    )
+    assert fired is True
+    output = buf.getvalue()
+    # Both names appear in the listing, but the warning header
+    # (``Local-agent provider detected``) appears exactly once.
+    assert output.count("Local-agent provider detected") == 1
+    assert "fast" in output
+    assert "slow" in output

cli/test/test_native_runner_claude_cli.py

@@ -0,0 +1,112 @@
+"""Unit tests for the claude-cli env wiring in native_runner.py."""
+
+import os
+import textwrap
+
+from planoai.native_runner import (
+    CLAUDE_CLI_DEFAULT_LISTEN_ADDR,
+    _apply_claude_cli_env,
+    _needs_claude_cli_runtime,
+)
+
+
+def _write(path, body):
+    path.write_text(textwrap.dedent(body).lstrip())
+    return str(path)
+
+
+def test_needs_claude_cli_runtime_detects_provider(tmp_path):
+    rendered = _write(
+        tmp_path / "rendered.yaml",
+        """
+        version: v0.4.0
+        listeners: []
+        model_providers:
+          - name: claude-cli/*
+            model: '*'
+            provider_interface: claude-cli
+            base_url: http://127.0.0.1:14001
+        """,
+    )
+    assert _needs_claude_cli_runtime(rendered) is True
+
+
+def test_needs_claude_cli_runtime_skips_other_providers(tmp_path):
+    rendered = _write(
+        tmp_path / "rendered.yaml",
+        """
+        version: v0.4.0
+        model_providers:
+          - name: openai/gpt-4o
+            model: gpt-4o
+            provider_interface: openai
+        """,
+    )
+    assert _needs_claude_cli_runtime(rendered) is False
+
+
+def test_needs_claude_cli_runtime_handles_missing_file(tmp_path):
+    assert _needs_claude_cli_runtime(str(tmp_path / "does-not-exist.yaml")) is False
+
+
+def test_apply_claude_cli_env_injects_default_addr(tmp_path, monkeypatch):
+    rendered = _write(
+        tmp_path / "rendered.yaml",
+        """
+        model_providers:
+          - provider_interface: claude-cli
+            model: '*'
+        """,
+    )
+    monkeypatch.delenv("CLAUDE_CLI_LISTEN_ADDR", raising=False)
+    monkeypatch.delenv("CLAUDE_CLI_BIN", raising=False)
+    env = {}
+    assert _apply_claude_cli_env(env, rendered) is True
+    assert env["CLAUDE_CLI_LISTEN_ADDR"] == CLAUDE_CLI_DEFAULT_LISTEN_ADDR
+
+
+def test_apply_claude_cli_env_honors_user_override(tmp_path, monkeypatch):
+    rendered = _write(
+        tmp_path / "rendered.yaml",
+        """
+        model_providers:
+          - provider_interface: claude-cli
+            model: '*'
+        """,
+    )
+    monkeypatch.delenv("CLAUDE_CLI_LISTEN_ADDR", raising=False)
+    env = {"CLAUDE_CLI_LISTEN_ADDR": "127.0.0.1:25000"}
+    assert _apply_claude_cli_env(env, rendered) is True
+    assert env["CLAUDE_CLI_LISTEN_ADDR"] == "127.0.0.1:25000"
+
+
+def test_apply_claude_cli_env_passes_through_user_env(tmp_path, monkeypatch):
+    rendered = _write(
+        tmp_path / "rendered.yaml",
+        """
+        model_providers:
+          - provider_interface: claude-cli
+            model: '*'
+        """,
+    )
+    monkeypatch.delenv("CLAUDE_CLI_LISTEN_ADDR", raising=False)
+    monkeypatch.setenv("CLAUDE_CLI_BIN", "/usr/local/bin/claude-test")
+    monkeypatch.setenv("CLAUDE_CLI_PERMISSION_MODE", "default")
+    env = {}
+    assert _apply_claude_cli_env(env, rendered) is True
+    assert env["CLAUDE_CLI_BIN"] == "/usr/local/bin/claude-test"
+    assert env["CLAUDE_CLI_PERMISSION_MODE"] == "default"
+
+
+def test_apply_claude_cli_env_noop_for_other_configs(tmp_path):
+    rendered = _write(
+        tmp_path / "rendered.yaml",
+        """
+        model_providers:
+          - provider_interface: openai
+            model: gpt-4o
+        """,
+    )
+    env = {}
+    assert _apply_claude_cli_env(env, rendered) is False
+    assert "CLAUDE_CLI_LISTEN_ADDR" not in env