fix(copilot): accept unprefixed CLAUDE_AGENT_CLI_PATH in config

The new `claude_agent_cli_path` field inherited the `CHAT_` Pydantic
prefix from `ChatConfig`, so the documented `CLAUDE_AGENT_CLI_PATH`
env var was silently ignored — operators following the PR description
or the field docstring would set the unprefixed form and the config
would fall back to the bundled CLI.

Add a `field_validator` that reads `CHAT_CLAUDE_AGENT_CLI_PATH` first
and falls back to the unprefixed `CLAUDE_AGENT_CLI_PATH`, matching the
same pattern already used by `api_key` and `base_url`. The test helper
`_resolve_cli_path` in `cli_openrouter_compat_test.py` mirrors the
same two-name lookup so the reproduction test picks up the override
regardless of which form is set, and a new test covers the prefixed
variant explicitly.

Flagged by sentry review on #12741 (thread IDs 3067725580 and
3067768817) as two instances of the same bug.
This commit is contained in:
majdyz
2026-04-11 10:11:47 +00:00
parent d6f0fcb052
commit a6e306d28a
2 changed files with 60 additions and 9 deletions

View File

@@ -182,7 +182,9 @@ class ChatConfig(BaseSettings):
"features (the bundled CLI version in 0.1.46+ is broken against "
"OpenRouter — see PR #12294 and "
"anthropics/claude-agent-sdk-python#789). Falls back to the "
"bundled binary when unset.",
"bundled binary when unset. Reads from `CHAT_CLAUDE_AGENT_CLI_PATH` "
"or the unprefixed `CLAUDE_AGENT_CLI_PATH` environment variable "
"(same pattern as `api_key` / `base_url`).",
)
use_openrouter: bool = Field(
default=True,
@@ -306,6 +308,26 @@ class ChatConfig(BaseSettings):
v = OPENROUTER_BASE_URL
return v
@field_validator("claude_agent_cli_path", mode="before")
@classmethod
def get_claude_agent_cli_path(cls, v):
"""Resolve the Claude Code CLI override path from environment.
Accepts either the Pydantic-prefixed ``CHAT_CLAUDE_AGENT_CLI_PATH``
or the unprefixed ``CLAUDE_AGENT_CLI_PATH`` (matching the same
fallback pattern used by ``api_key`` / ``base_url``). Keeping the
unprefixed form working is important because the field is
primarily an operator escape hatch set via container/host env,
and the unprefixed name is what the PR description, the field
docstrings, and the reproduction test in
``cli_openrouter_compat_test.py`` refer to.
"""
if not v:
v = os.getenv("CHAT_CLAUDE_AGENT_CLI_PATH")
if not v:
v = os.getenv("CLAUDE_AGENT_CLI_PATH")
return v
# Prompt paths for different contexts
PROMPT_PATHS: dict[str, str] = {
"default": "prompts/chat_system.md",

View File

@@ -240,12 +240,19 @@ async def _start_fake_anthropic_server(
def _resolve_cli_path() -> Path | None:
"""Return the Claude Code CLI binary the SDK would use.
Honours the same override mechanism as ``service.py``: explicit
``CLAUDE_AGENT_CLI_PATH`` env var first (matching the new
``ChatConfig.claude_agent_cli_path`` field), then the bundled
binary that ships with the installed ``claude-agent-sdk`` wheel.
Honours the same override mechanism as ``service.py`` /
``ChatConfig.claude_agent_cli_path``: checks either the Pydantic-
prefixed ``CHAT_CLAUDE_AGENT_CLI_PATH`` or the unprefixed
``CLAUDE_AGENT_CLI_PATH`` env var first, then falls back to the
bundled binary that ships with the installed ``claude-agent-sdk``
wheel. The two env var names are accepted at the config layer via
``ChatConfig.get_claude_agent_cli_path`` and mirrored here so the
reproduction test picks up the same override regardless of which
form an operator sets.
"""
override = os.environ.get("CLAUDE_AGENT_CLI_PATH")
override = os.environ.get("CHAT_CLAUDE_AGENT_CLI_PATH") or os.environ.get(
"CLAUDE_AGENT_CLI_PATH"
)
if override:
candidate = Path(override)
return candidate if candidate.is_file() else None
@@ -362,7 +369,8 @@ async def test_cli_does_not_send_openrouter_incompatible_features(caplog):
if cli_path is None or not cli_path.is_file():
pytest.skip(
"No Claude Code CLI binary available (neither bundled nor "
"overridden via CLAUDE_AGENT_CLI_PATH); cannot reproduce."
"overridden via CLAUDE_AGENT_CLI_PATH / "
"CHAT_CLAUDE_AGENT_CLI_PATH); cannot reproduce."
)
captured: list[_CapturedRequest] = []
@@ -412,8 +420,8 @@ async def test_cli_does_not_send_openrouter_incompatible_features(caplog):
"https://github.com/Significant-Gravitas/AutoGPT/pull/12294 and "
"https://github.com/anthropics/claude-agent-sdk-python/issues/789. "
"If you intended to upgrade, you must use a known-good CLI binary "
"via `claude_agent_cli_path` (env: `CLAUDE_AGENT_CLI_PATH`) "
"instead of the bundled one."
"via `claude_agent_cli_path` (env: `CLAUDE_AGENT_CLI_PATH` or "
"`CHAT_CLAUDE_AGENT_CLI_PATH`) instead of the bundled one."
)
@@ -500,11 +508,31 @@ class TestResolveCliPath:
fake_cli = tmp_path / "fake-claude"
fake_cli.write_text("#!/bin/sh\necho fake\n")
fake_cli.chmod(0o755)
monkeypatch.delenv("CHAT_CLAUDE_AGENT_CLI_PATH", raising=False)
monkeypatch.setenv("CLAUDE_AGENT_CLI_PATH", str(fake_cli))
resolved = _resolve_cli_path()
assert resolved == fake_cli
def test_honours_chat_prefixed_env_var_when_file_exists(
self, tmp_path, monkeypatch
):
"""The Pydantic ``CHAT_`` prefix variant is also honoured.
Mirrors ``ChatConfig.get_claude_agent_cli_path`` which accepts
either ``CHAT_CLAUDE_AGENT_CLI_PATH`` (prefix applied by
``pydantic_settings``) or the unprefixed ``CLAUDE_AGENT_CLI_PATH``
form documented in the PR and field docstring.
"""
fake_cli = tmp_path / "fake-claude-prefixed"
fake_cli.write_text("#!/bin/sh\necho fake\n")
fake_cli.chmod(0o755)
monkeypatch.delenv("CLAUDE_AGENT_CLI_PATH", raising=False)
monkeypatch.setenv("CHAT_CLAUDE_AGENT_CLI_PATH", str(fake_cli))
resolved = _resolve_cli_path()
assert resolved == fake_cli
def test_returns_none_when_env_var_points_to_missing_file(self, monkeypatch):
monkeypatch.delenv("CHAT_CLAUDE_AGENT_CLI_PATH", raising=False)
monkeypatch.setenv("CLAUDE_AGENT_CLI_PATH", "/nonexistent/path/to/claude")
# Should fall through to the bundled binary OR return None,
# but never raise.
@@ -516,6 +544,7 @@ class TestResolveCliPath:
def test_falls_back_to_bundled_when_env_var_unset(self, monkeypatch):
monkeypatch.delenv("CLAUDE_AGENT_CLI_PATH", raising=False)
monkeypatch.delenv("CHAT_CLAUDE_AGENT_CLI_PATH", raising=False)
# Same caveat as above — returns the bundled path or None,
# depending on what's installed in the test env.
resolved = _resolve_cli_path()