From 877e85136f688af219ec2f0019deb95e2ec29941 Mon Sep 17 00:00:00 2001 From: Jacob Suelyn Date: Sat, 15 Aug 2026 18:44:35 -0400 Subject: [PATCH] fix(acp): probe CLI for --acp support before spawning subprocess MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CopilotACPClient unconditionally passes [self._acp_command] + self._acp_args (default ['--acp', '--stdio']) to subprocess.Popen. When the resolved CLI doesn't accept --acp (e.g. Claude Code v2.1.233, where 'claude --acp --stdio' exits 1 with 'error: unknown option') the subprocess dies in ~250ms with the error on stderr, but the parent ACP loop has no fast-fail for this shape and waits the full child_timeout_seconds (default 600s, observed 109s+ before user interruption) for stdout that never arrives. Add _acp_supported() that probes the CLI's --help output for the --acp flag in ~50ms, then call it at the top of _run_prompt before any spawn happens. When the probe fails, raise a RuntimeError that names the unsupported flag, lists the expected fix (install @github/copilot late 2025+, or set HERMES_COPILOT_ACP_*), and returns control to the caller in ~280ms instead of hanging the delegate_task parent for hundreds of seconds. Measured locally against Claude Code v2.1.233: - Before: delegate_task acp_command=claude hangs 109s+ then returns tokens={input:0, output:0}. - After: delegate_task acp_command=claude raises RuntimeError in 280ms with a clear actionable message. This does NOT change behavior for supported CLIs (the new @github/copilot ships with --acp) — the probe returns True and the spawn proceeds unchanged. Refs the bundled claude-review-delegate skill which already documents this class of transport-mismatch pitfall for users who call 'claude -p' directly; this fix closes the same gap for the delegate_task MCP path. --- agent/copilot_acp_client.py | 48 +++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/agent/copilot_acp_client.py b/agent/copilot_acp_client.py index 021326c47d..759ebaceb7 100644 --- a/agent/copilot_acp_client.py +++ b/agent/copilot_acp_client.py @@ -74,6 +74,35 @@ def _resolve_args() -> list[str]: return shlex.split(raw) +def _acp_supported(command: str, args: list[str]) -> bool: + """Return True iff ``command`` accepts the ACP args we'd pass. + + Different CLI versions support different transports. The GitHub + Copilot CLI (`@github/copilot`, late 2025+) ships with ``--acp``; + older releases (and Claude Code v2.x as of Aug 2026) do not. + Spawning a CLI that doesn't recognize the flag silently exits + with code 1 and ``error: unknown option '--acp'`` on stderr, + after which every delegate_task call hangs the parent for + ``child_timeout_seconds`` (default 600s) waiting for stdout + that never arrives. + + This probe fires once per ACP prompt, runs in ~50ms, and + short-circuits to a clear error before any spawn happens. + """ + try: + probe = subprocess.run( + [command, "--help"], + capture_output=True, text=True, timeout=5, + ) + except (FileNotFoundError, subprocess.TimeoutExpired, OSError): + return False + if probe.returncode != 0: + return False + # Match ``--acp`` as a flag in the help text; tolerate spacing and + # variants like ``[--acp]``. + return bool(re.search(r"(?:^|\s)--acp(?:\s|=|\]|\b)", probe.stdout)) + + def _resolve_home_dir() -> str: """Return a stable HOME for child ACP processes.""" home = os.environ.get("HOME", "").strip() @@ -502,6 +531,25 @@ class CopilotACPClient: return completion def _run_prompt(self, prompt_text: str, *, timeout_seconds: float) -> tuple[str, str]: + # Fast-fail when the CLI doesn't support the ACP args we'd pass. + # Without this guard, a CLI like Claude Code v2.x exits with + # ``error: unknown option '--acp'`` immediately, then the parent + # ACP loop waits the full ``child_timeout_seconds`` (default 600s) + # for stdout that never arrives. The probe costs ~50ms and turns + # a 600s silent hang into a 280ms clear error. + if not _acp_supported(self._acp_command, self._acp_args): + preview = " ".join(self._acp_args[:3]) if self._acp_args else "(none)" + raise RuntimeError( + f"ACP transport not supported by '{self._acp_command}': " + f"`{preview}` is rejected as an unknown option. " + f"This usually means the CLI is an older release (e.g. " + f"Claude Code v2.x) or a different tool than expected. " + f"Either install a CLI that ships with --acp support " + f"(e.g. `@github/copilot` late 2025+), or set " + f"HERMES_COPILOT_ACP_COMMAND / HERMES_COPILOT_ACP_ARGS " + f"to a working pair." + ) + try: # Hide the console the CLI child would otherwise flash on Windows # (#56747). Hide-only — stdio pipes stay intact for the ACP wire.