fix(picker): keep signed-in copilot-acp visible in explicit-only desktop pickers
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.
This commit is contained in:
committed by
kshitij
parent
15f003e0b9
commit
6b2d32d3f6
@@ -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",
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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 ------------------------------------------------
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user