From 6b2d32d3f6b2dca1ad78e2d599b1523d714faaf8 Mon Sep 17 00:00:00 2001 From: unsupportedpastels Date: Tue, 1 Sep 2026 14:13:37 +0000 Subject: [PATCH] fix(picker): keep signed-in copilot-acp visible in explicit-only desktop pickers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two follow-up gaps found by actually running 'copilot login' end-to-end: 1. The CLI (without an OS keychain) stores its token in ~/.copilot/config.json under copilotTokens — a JSONC file with //-comment header lines. Add it as an auth-evidence source in _external_process_auth_evidence(), parsed comment-tolerantly and counting only a non-empty copilotTokens map (config.json exists after first launch even when logged out). 2. The desktop chat picker requests explicit_only rows, and _filter_explicit_provider_rows() dropped copilot-acp because a CLI login leaves no trace in active_provider, model.provider, or env vars — exactly the Anthropic-OAuth carve-out case. Keep external_process rows when their CLI credentials are verified (auth_verified), while still dropping ambient executable-on-PATH-only rows so the filter's narrower contract holds. Net effect: after 'copilot login', copilot-acp appears in the desktop picker and the Accounts card reads signed in; a machine with only the binary installed keeps today's hidden-until-configured behavior. --- hermes_cli/auth.py | 21 ++++- hermes_cli/inventory.py | 24 ++++++ .../test_external_process_auth_status.py | 80 +++++++++++++++++++ 3 files changed, 124 insertions(+), 1 deletion(-) diff --git a/hermes_cli/auth.py b/hermes_cli/auth.py index 6ff3881786..1f056199a7 100644 --- a/hermes_cli/auth.py +++ b/hermes_cli/auth.py @@ -7866,7 +7866,26 @@ def _external_process_auth_evidence(provider_id: str) -> tuple[bool, Optional[st return True, f"env: {env_var}" except Exception as exc: logger.debug("copilot-acp env token evidence check failed: %s", exc) - # 2. Known on-disk GitHub Copilot credential stores (the same locations + # 2. The Copilot CLI's own plaintext token store (~/.copilot/config.json, + # written by `copilot login` when no OS keychain is available). The file + # is JSONC — strip //-comment lines before parsing. + try: + cli_config = os.path.expanduser("~/.copilot/config.json") + if os.path.isfile(cli_config): + with open(cli_config, "r", encoding="utf-8", errors="ignore") as fh: + raw = "\n".join( + line for line in fh.read().splitlines() + if not line.lstrip().startswith("//") + ) + data = json.loads(raw) if raw.strip() else {} + tokens = data.get("copilotTokens") + if isinstance(tokens, dict) and any( + isinstance(v, str) and v.strip() for v in tokens.values() + ): + return True, "~/.copilot/config.json" + except Exception as exc: + logger.debug("copilot-acp CLI config evidence check failed: %s", exc) + # 3. Known on-disk GitHub Copilot credential stores (the same locations # models.py already fingerprints as external credential files). for cred_path in ( "~/.config/github-copilot/hosts.json", diff --git a/hermes_cli/inventory.py b/hermes_cli/inventory.py index 320bbc0310..f9b0de4d90 100644 --- a/hermes_cli/inventory.py +++ b/hermes_cli/inventory.py @@ -798,11 +798,35 @@ def _filter_explicit_provider_rows(rows: list[dict], ctx: ConfigContext) -> list # just accepted those same credentials when building it. kept.append(row) continue + if _external_process_signed_in(slug): + # External-process providers (copilot-acp) authenticate through + # their own CLI (`copilot login`), which — like the Anthropic + # OAuth case above — leaves no trace in active_provider, + # model.provider, or env vars. Verified CLI credentials are a + # deliberate sign-in; without this the desktop picker drops the + # row the picker-discovery side just accepted. + kept.append(row) + continue if is_provider_explicitly_configured(slug): kept.append(row) return kept +def _external_process_signed_in(slug: str) -> bool: + """True when an external-process provider has verified CLI credentials.""" + try: + from hermes_cli.auth import ( + PROVIDER_REGISTRY, + get_external_process_provider_status, + ) + pconfig = PROVIDER_REGISTRY.get(slug) + if not pconfig or pconfig.auth_type != "external_process": + return False + return bool(get_external_process_provider_status(slug).get("auth_verified")) + except Exception: + return False + + def _provider_is_keyless(slug: str) -> bool: """True when the provider's Hermes overlay declares it keyless.""" try: diff --git a/tests/hermes_cli/test_external_process_auth_status.py b/tests/hermes_cli/test_external_process_auth_status.py index ac6f0ce178..b8546d2e9c 100644 --- a/tests/hermes_cli/test_external_process_auth_status.py +++ b/tests/hermes_cli/test_external_process_auth_status.py @@ -120,6 +120,86 @@ def test_empty_credential_store_is_not_evidence(tmp_path, monkeypatch, _clean_co assert status["auth_verified"] is False +def test_auth_verified_from_copilot_cli_plaintext_store(tmp_path, monkeypatch, _clean_copilot_env): + # `copilot login` without an OS keychain writes the token into + # ~/.copilot/config.json (JSONC, with //-comment header lines). + monkeypatch.setenv("HOME", str(tmp_path)) + cfg_dir = tmp_path / ".copilot" + cfg_dir.mkdir() + (cfg_dir / "config.json").write_text( + "// User settings belong in settings.json.\n" + "// This file is managed automatically.\n" + "{\n" + ' "copilotTokens": {"https://github.com:someuser": "gho_test"},\n' + ' "lastLoggedInUser": {"host": "https://github.com", "login": "someuser"}\n' + "}\n", + encoding="utf-8", + ) + + status = get_external_process_provider_status("copilot-acp") + + assert status["auth_verified"] is True + assert status["auth_source"] == "~/.copilot/config.json" + + +def test_copilot_cli_store_without_tokens_is_not_evidence(tmp_path, monkeypatch, _clean_copilot_env): + # A config.json exists after first launch even before any login — + # its presence alone must not read as signed-in. + monkeypatch.setenv("HOME", str(tmp_path)) + cfg_dir = tmp_path / ".copilot" + cfg_dir.mkdir() + (cfg_dir / "config.json").write_text( + '// managed\n{"firstLaunchAt": "2026-01-01T00:00:00Z", "copilotTokens": {}}\n', + encoding="utf-8", + ) + + status = get_external_process_provider_status("copilot-acp") + + assert status["auth_verified"] is False + + +# --- desktop picker explicit-only filter ------------------------------------ + + +def test_explicit_filter_keeps_signed_in_external_process_row(tmp_path, monkeypatch, _clean_copilot_env): + # A verified CLI login leaves no trace in active_provider/config/env — + # the explicit-only desktop filter must treat it like the Anthropic OAuth + # carve-out and keep the row. + from hermes_cli.inventory import _filter_explicit_provider_rows + + monkeypatch.setenv("HOME", str(tmp_path)) + cfg_dir = tmp_path / ".copilot" + cfg_dir.mkdir() + (cfg_dir / "config.json").write_text( + '{"copilotTokens": {"https://github.com:u": "gho_test"}}', encoding="utf-8" + ) + + class _Ctx: + current_provider = "nous" + + rows = [{"slug": "copilot-acp", "models": ["gpt-5.4"]}] + kept = _filter_explicit_provider_rows(rows, _Ctx()) + + assert any(r["slug"] == "copilot-acp" for r in kept), \ + "signed-in copilot-acp must survive the explicit-only picker filter" + + +def test_explicit_filter_drops_unverified_external_process_row(tmp_path, monkeypatch, _clean_copilot_env): + # Merely having the executable on PATH is ambient discovery, not an + # explicit configuration — the desktop filter keeps its narrower contract. + from hermes_cli.inventory import _filter_explicit_provider_rows + + monkeypatch.setenv("HOME", str(tmp_path)) # no credential stores + + class _Ctx: + current_provider = "nous" + + rows = [{"slug": "copilot-acp", "models": ["gpt-5.4"]}] + kept = _filter_explicit_provider_rows(rows, _Ctx()) + + assert all(r["slug"] != "copilot-acp" for r in kept) + + # --- Accounts-tab cli_command ------------------------------------------------