From ddc55f217b162f2fdc7e4ba6911127f0a4bdd2f4 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Sun, 20 Sep 2026 12:24:04 +0530 Subject: [PATCH] fix(agent): Responses-upgrade predicate declines every external-process profile, not three names ``_provider_model_requires_responses_api`` is the one predicate both primary routing (``agent_init._finalize_routing``) and GPT-5 fallback activation (``chat_completion_helpers._fallback_api_mode_resolved``) consult; the fallback path had no ACP exclusion at all, so activating ``copilot-acp / gpt-5.x`` as a fallback selected codex_responses and the next request died with ``'CopilotACPClient' object has no attribute 'responses'`` (#65842). Key the exclusion on the profile's external_process auth_type so the bundled facade and out-of-tree ACP plugins are covered by the same line. --- run_agent.py | 15 ++++---- .../test_external_process_provider_init.py | 34 +++++++++++++++++++ 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/run_agent.py b/run_agent.py index 12249ae028..9fe1c58e88 100644 --- a/run_agent.py +++ b/run_agent.py @@ -670,15 +670,12 @@ class AIAgent( # may relay GPT-5 without full Responses semantics — only direct OpenAI/xAI URLs auto-upgrade. if normalized_provider in ("nous", "custom") or is_actual_route(provider): return False - # The Copilot ACP facade deliberately exposes the OpenAI-compatible - # chat.completions shape regardless of model family. It has no - # ``responses`` attribute, so GPT-5 fallback routing must not upgrade - # it to codex_responses. - if normalized_provider in { - "copilot-acp", - "github-copilot-acp", - "copilot-acp-agent", - }: + # ACP facades expose the OpenAI-compatible chat.completions shape regardless of model + # family and have no ``responses`` attribute, so neither primary routing nor GPT-5 + # fallback activation may upgrade them. Keyed on the profile's auth_type: every + # external-process provider, not one vendor's names. + from hermes_cli.runtime_provider_backends import _is_external_process_provider + if _is_external_process_provider(normalized_provider): return False if normalized_provider == "copilot": try: diff --git a/tests/agent/test_external_process_provider_init.py b/tests/agent/test_external_process_provider_init.py index 5660fb598a..99e793437b 100644 --- a/tests/agent/test_external_process_provider_init.py +++ b/tests/agent/test_external_process_provider_init.py @@ -63,3 +63,37 @@ def test_responses_upgrade_is_skipped_for_external_process_profile_on_any_base_u plain._provider_model_requires_responses_api = lambda model, provider=None: True _finalize_routing(plain, None, None) assert plain.api_mode == "codex_responses" + + +def test_fallback_activation_keeps_external_process_provider_on_chat_completions(monkeypatch): + """GPT-5 fallback activation re-derives api_mode through ``_provider_model_requires_responses_api``; + an external-process (ACP) facade has no ``responses`` surface, so the predicate must decline for + any such profile — the bundled copilot-acp and an out-of-tree one alike (#65842, #107754).""" + from agent.chat_completion_helpers import _fallback_api_mode_resolved + from providers.base import ProviderProfile + from run_agent import AIAgent + + profiles = {name: ProviderProfile(name=name, auth_type="external_process") for name in ("copilot-acp", "acme-acp")} + monkeypatch.setattr("providers.get_provider_profile", profiles.get) + agent = SimpleNamespace( + _is_azure_openai_url=lambda url: False, _is_direct_openai_url=lambda url: False, + _provider_model_requires_responses_api=AIAgent._provider_model_requires_responses_api) + + for provider in profiles: + assert _fallback_api_mode_resolved(agent, provider, "gpt-5.6", "https://proxy.example.invalid/v1") == "chat_completions", provider + assert _fallback_api_mode_resolved(agent, "acme-http", "gpt-5.6", "https://proxy.example.invalid/v1") == "codex_responses" + + +def test_should_stream_is_off_for_any_external_process_profile(monkeypatch): + """Streaming is disabled for every ACP provider, keyed on the profile — not on the ``acp://`` + marker alone and not on one vendor's slug.""" + from agent.turn_api_call import _should_stream + from providers.base import ProviderProfile + + profile = ProviderProfile(name="acme-acp", auth_type="external_process") + monkeypatch.setattr("providers.get_provider_profile", lambda name: profile if name == "acme-acp" else None) + make = lambda provider: SimpleNamespace( # noqa: E731 + provider=provider, base_url="https://proxy.example.invalid/v1", _has_stream_consumers=lambda: True) + + assert _should_stream(make("acme-acp")) is False + assert _should_stream(make("acme-http")) is True