diff --git a/agent/copilot_acp_client.py b/agent/copilot_acp_client.py index 421438964a..3df2f1477b 100644 --- a/agent/copilot_acp_client.py +++ b/agent/copilot_acp_client.py @@ -371,13 +371,16 @@ class CopilotACPClient: threading.Thread(target=_pump, args=(proc.stdout, lambda line: inbox.put(_decode(line))), daemon=True).start() threading.Thread(target=_pump, args=(proc.stderr, lambda line: stderr_tail.append(line.rstrip("\n"))), daemon=True).start() request_ids = iter(range(1, 1 << 62)) + # One budget for the WHOLE session (initialize + session/new + any prompt), not per + # request: a hung CLI must not get 2x the caller's timeout on the foreground /model path. + session_deadline = time.monotonic() + timeout_seconds def _request(method: str, params: dict[str, Any], *, text_parts: list[str] | None = None, reasoning_parts: list[str] | None = None) -> Any: request_id = next(request_ids) proc.stdin.write(json.dumps({"jsonrpc": "2.0", "id": request_id, "method": method, "params": params}) + "\n") proc.stdin.flush() - deadline = time.monotonic() + timeout_seconds + deadline = session_deadline while time.monotonic() < deadline and proc.poll() is None: try: msg = inbox.get(timeout=0.1) diff --git a/hermes_cli/models.py b/hermes_cli/models.py index 1179d9e783..a21c3be470 100644 --- a/hermes_cli/models.py +++ b/hermes_cli/models.py @@ -1235,20 +1235,23 @@ def _codex_catalog(normalized: str, force_refresh: bool) -> list[str]: return get_codex_model_ids(access_token=access_token) -_COPILOT_ACP_SESSION_MEMO_TTL = 300.0 # 5 min, same as the GitHub catalog memo; SWR disk cache handles the rest -_copilot_acp_session_memo: Optional[tuple[float, Optional[list[str]]]] = None +_COPILOT_ACP_SESSION_MEMO_TTL = 300.0 # 5 min; SWR disk cache handles the rest +_COPILOT_ACP_SESSION_FAIL_TTL = 30.0 # failed probes re-probe quickly so a fresh CLI login is picked up +_copilot_acp_session_memo: Optional[tuple[float, float, Optional[list[str]]]] = None # (at, ttl, models) def _copilot_acp_session_models(force_refresh: bool) -> Optional[list[str]]: """Enabled models from a signed-in ``copilot --acp`` session, memoized for a few minutes — successes AND failures. Model-switch validation (``models_validate._static_catalog``) reads this uncached on every ``/model`` switch, and each miss is a CLI spawn + handshake (up to the - probe timeout), so without the memo every switch paid a subprocess.""" + probe timeout), so without the memo every switch paid a subprocess. A failed probe is + memoized much more briefly so a user who signs in to the CLI right after a miss is picked up + on the next switch (or immediately via ``/model --refresh``, which clears this memo).""" global _copilot_acp_session_memo now = time.monotonic() memo = _copilot_acp_session_memo - if not force_refresh and memo is not None and now - memo[0] < _COPILOT_ACP_SESSION_MEMO_TTL: - return memo[1] + if not force_refresh and memo is not None and now - memo[0] < memo[1]: + return memo[2] from providers import get_provider_profile try: @@ -1256,7 +1259,7 @@ def _copilot_acp_session_models(force_refresh: bool) -> Optional[list[str]]: except Exception: logger.debug("copilot-acp session model discovery failed", exc_info=True) live = None - _copilot_acp_session_memo = (now, live) + _copilot_acp_session_memo = (now, _COPILOT_ACP_SESSION_MEMO_TTL if live else _COPILOT_ACP_SESSION_FAIL_TTL, live) return live @@ -1738,6 +1741,11 @@ def clear_provider_models_cache(provider: Optional[str] = None) -> None: _OLLAMA_LOCAL_MODELS_CACHE.clear() _OLLAMA_LOCAL_PROBE_FAILURE_CACHE.clear() _OLLAMA_LOCAL_PROBE_REACHABLE.clear() + # A fresh copilot-acp CLI login must be visible to the next /model switch (this helper is + # what ``--refresh`` runs): don't let the 5-min session memo (or its failure memo) serve + # a stale signed-out probe past an explicit refresh. + global _copilot_acp_session_memo + _copilot_acp_session_memo = None if provider is None: path = _provider_models_cache_path() if path.exists(): diff --git a/plugins/model-providers/copilot-acp/__init__.py b/plugins/model-providers/copilot-acp/__init__.py index c351ca364f..6027873a84 100644 --- a/plugins/model-providers/copilot-acp/__init__.py +++ b/plugins/model-providers/copilot-acp/__init__.py @@ -33,13 +33,18 @@ class CopilotACPProfile(ProviderProfile): """ from hermes_cli.auth import resolve_external_process_provider_credentials - creds = resolve_external_process_provider_credentials(self.name) - if not str(creds.get("base_url") or "").startswith("acp://"): + try: + creds = resolve_external_process_provider_credentials(self.name) + if not str(creds.get("base_url") or "").startswith("acp://"): + return None + client = self.create_client( + api_key=creds.get("api_key"), base_url=creds.get("base_url"), + command=creds.get("command"), args=creds.get("args")) + return client.list_models(timeout_seconds=timeout) or None + except Exception: + # Missing CLI (AuthError), refused --acp / failed spawn (RuntimeError), probe + # timeout — the base fetch_models contract is "None if the fetch failed". return None - client = self.create_client( - api_key=creds.get("api_key"), base_url=creds.get("base_url"), - command=creds.get("command"), args=creds.get("args")) - return client.list_models(timeout_seconds=timeout) or None copilot_acp = CopilotACPProfile( diff --git a/tests/hermes_cli/test_copilot_in_model_list.py b/tests/hermes_cli/test_copilot_in_model_list.py index 8337840a9a..6ba0d7b12e 100644 --- a/tests/hermes_cli/test_copilot_in_model_list.py +++ b/tests/hermes_cli/test_copilot_in_model_list.py @@ -93,6 +93,9 @@ _ACP_CREDS = {"api_key": "copilot-acp", "base_url": "acp://copilot", "command": @pytest.fixture() def _fresh_acp_memo(monkeypatch): monkeypatch.setattr(models, "_copilot_acp_session_memo", None) + yield + # Don't leak a memoized (possibly failed) probe into other tests in this process. + monkeypatch.setattr(models, "_copilot_acp_session_memo", None) @pytest.mark.parametrize( @@ -131,7 +134,7 @@ def test_copilot_acp_session_probe_is_memoized_across_model_switch_validation(_f assert verdict["accepted"] and verdict["recognized"] assert list_models.call_count == 1 - models._copilot_acp_session_memo = None + models._copilot_acp_session_memo = None # (teardown in _fresh_acp_memo restores it) with patch("hermes_cli.auth.resolve_external_process_provider_credentials", return_value=_ACP_CREDS), \ patch("agent.copilot_acp_client.CopilotACPClient.list_models", side_effect=RuntimeError("not signed in")) as list_models, \ patch("hermes_cli.models._resolve_copilot_catalog_api_key", return_value=""), \