mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
feat: add Copilot BYOK provider wrapper utilities and CLI support (#1041)
## Description Fix `--model auto` causing `400 The requested model is not supported` errors when using Copilot BYOK mode. `auto` is a Copilot-internal virtual routing token that external providers (Anthropic, OpenAI) do not recognise as a valid model name. In subscription/OAuth mode the wrapper now strips `--model auto` before launching Copilot so its own native auto-selection takes effect. In BYOK mode `auto` is treated as unconfigured and a clear, actionable error message is shown. Closes #972 ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [ ] New feature (non-breaking change that adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update - [ ] Performance improvement - [ ] Code refactoring (no functional changes) ## Changes Made - `headroom/providers/copilot/wrap.py`: added `is_auto_model()` and `strip_auto_model_args()` helpers; updated `model_configured()` to treat `auto` as unconfigured for BYOK - `headroom/providers/copilot/__init__.py`: exported both new helpers via `__all__` - `headroom/cli/wrap.py`: strips `--model auto` in subscription mode before launch; shows specific actionable error in BYOK mode - `tests/test_provider_copilot_wrap.py`: 17 new parametrized test cases for `is_auto_model`, `strip_auto_model_args`, and updated `model_configured` ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [ ] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [ ] Manual testing performed ### Test Output ```text $ uv run pytest tests/test_provider_copilot_wrap.py -v platform win32 -- Python 3.14.6, pytest-9.0.3, pluggy-1.6.0 collected 34 items tests/test_provider_copilot_wrap.py::test_is_auto_model[auto-True] PASSED tests/test_provider_copilot_wrap.py::test_is_auto_model[Auto-True] PASSED tests/test_provider_copilot_wrap.py::test_is_auto_model[AUTO-True] PASSED tests/test_provider_copilot_wrap.py::test_strip_auto_model_args[args0-expected0] PASSED tests/test_provider_copilot_wrap.py::test_strip_auto_model_args[args1-expected1] PASSED tests/test_provider_copilot_wrap.py::test_strip_auto_model_args[args2-expected2] PASSED tests/test_provider_copilot_wrap.py::test_model_configured_detects_env_and_cli_variants PASSED ============================= 34 passed in 0.46s ============================== $ ruff check headroom/providers/copilot/wrap.py headroom/cli/wrap.py All checks passed! ``` ## Real Behavior Proof - Environment: Windows 11, Python 3.14.6, headroom-ai 0.25.0 editable install from branch fix-automode-issue - Exact command / steps: ran uv run pytest tests/test_provider_copilot_wrap.py -v and ruff check on all four changed files; reviewed CLI code path for both subscription and BYOK modes - Observed result: 34 passed, ruff All checks passed; --model auto is stripped silently in subscription mode and rejected with a specific actionable error in BYOK mode - Not tested: live end-to-end Copilot CLI session, macOS/Linux keychain auth, Docker/CI token-injection paths ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review ## Checklist - [x] My code follows the project's style guidelines - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] I have updated the CHANGELOG.md if applicable ## Additional Notes mypy is not installed in the local venv so type checking was skipped; the code uses standard type hints and passes ruff checks cleanly. --------- Co-authored-by: JerrettDavis <mxjerrett@gmail.com>
This commit is contained in:
parent
74dff94fb8
commit
e67ee2af65
5 changed files with 203 additions and 20 deletions
|
|
@ -63,6 +63,9 @@ from headroom.providers.copilot import (
|
|||
from headroom.providers.copilot import (
|
||||
detect_running_proxy_backend as _copilot_detect_running_proxy_backend,
|
||||
)
|
||||
from headroom.providers.copilot import (
|
||||
is_auto_model as _is_auto_model,
|
||||
)
|
||||
from headroom.providers.copilot import (
|
||||
model_configured as _copilot_model_configured_impl,
|
||||
)
|
||||
|
|
@ -75,6 +78,9 @@ from headroom.providers.copilot import (
|
|||
from headroom.providers.copilot import (
|
||||
resolve_provider_type as _copilot_resolve_provider_type,
|
||||
)
|
||||
from headroom.providers.copilot import (
|
||||
strip_auto_model_args as _strip_auto_model_args,
|
||||
)
|
||||
from headroom.providers.copilot import (
|
||||
validate_configuration as _validate_copilot_configuration,
|
||||
)
|
||||
|
|
@ -2294,9 +2300,9 @@ def _proc_identity(pid: int) -> tuple[str, float] | None:
|
|||
different units; we only compare like-for-like.
|
||||
"""
|
||||
try:
|
||||
import psutil # optional dependency; portable when present
|
||||
import psutil # type: ignore[import-untyped] # optional dependency; portable when present
|
||||
|
||||
return ("psutil", float(psutil.Process(pid).create_time()))
|
||||
return ("psutil", psutil.Process(pid).create_time())
|
||||
except Exception:
|
||||
pass
|
||||
# Linux fallback: field 22 of /proc/<pid>/stat is starttime in clock ticks
|
||||
|
|
@ -3202,6 +3208,22 @@ def copilot(
|
|||
)
|
||||
|
||||
selected_model = _copilot_model_from_args(copilot_args, env)
|
||||
|
||||
# ``--model auto`` is a Copilot-internal routing token that the BYOK
|
||||
# API rejects with ``400 The requested model is not supported``. In
|
||||
# subscription/OAuth mode we route to the real Copilot hosted API, so
|
||||
# Copilot's own native auto-selection works fine — we just need to
|
||||
# strip the ``--model auto`` flag before launch so Copilot doesn't
|
||||
# forward it to the provider endpoint.
|
||||
if _is_auto_model(selected_model):
|
||||
copilot_args = _strip_auto_model_args(copilot_args)
|
||||
selected_model = None
|
||||
click.echo(
|
||||
" Note: '--model auto' is not forwarded to the Copilot API "
|
||||
"(it would cause a 400). Removed it; Copilot will use its own "
|
||||
"automatic model selection."
|
||||
)
|
||||
|
||||
effective_wire_api = wire_api or (
|
||||
_copilot_default_wire_api_for_model(selected_model) if subscription else "completions"
|
||||
)
|
||||
|
|
@ -3270,10 +3292,26 @@ def copilot(
|
|||
raise SystemExit(1)
|
||||
|
||||
if not subscription and not _copilot_model_configured(copilot_args, env):
|
||||
click.echo(
|
||||
" Note: Copilot BYOK requires a model. Pass `--model <name>` "
|
||||
"or set `COPILOT_MODEL` / `COPILOT_PROVIDER_MODEL_ID`."
|
||||
)
|
||||
# Distinguish between "--model auto" (wrong model for BYOK) and
|
||||
# genuinely missing model (no --model flag at all).
|
||||
raw_model = _copilot_model_from_args(copilot_args, env)
|
||||
if _is_auto_model(raw_model):
|
||||
click.echo(
|
||||
" Error: '--model auto' is not supported in Copilot BYOK mode.\n"
|
||||
" BYOK routes to an external provider (Anthropic/OpenAI) which\n"
|
||||
" does not recognise 'auto' as a model name — the request will\n"
|
||||
" fail with a 400 error.\n"
|
||||
" Options:\n"
|
||||
" • Use a concrete model: --model gpt-4o\n"
|
||||
" • Use subscription mode for native auto-routing:\n"
|
||||
" headroom wrap copilot --subscription -- --model auto"
|
||||
)
|
||||
raise SystemExit(1)
|
||||
else:
|
||||
click.echo(
|
||||
" Note: Copilot BYOK requires a model. Pass `--model <name>` "
|
||||
"or set `COPILOT_MODEL` / `COPILOT_PROVIDER_MODEL_ID`."
|
||||
)
|
||||
|
||||
_launch_tool(
|
||||
binary=copilot_bin,
|
||||
|
|
|
|||
|
|
@ -5,11 +5,13 @@ from .wrap import (
|
|||
copilot_model_from_args,
|
||||
default_wire_api_for_model,
|
||||
detect_running_proxy_backend,
|
||||
is_auto_model,
|
||||
model_configured,
|
||||
model_prefers_responses_api,
|
||||
provider_key_source,
|
||||
query_proxy_config,
|
||||
resolve_provider_type,
|
||||
strip_auto_model_args,
|
||||
validate_configuration,
|
||||
)
|
||||
|
||||
|
|
@ -18,10 +20,12 @@ __all__ = [
|
|||
"copilot_model_from_args",
|
||||
"default_wire_api_for_model",
|
||||
"detect_running_proxy_backend",
|
||||
"is_auto_model",
|
||||
"model_prefers_responses_api",
|
||||
"model_configured",
|
||||
"provider_key_source",
|
||||
"query_proxy_config",
|
||||
"resolve_provider_type",
|
||||
"strip_auto_model_args",
|
||||
"validate_configuration",
|
||||
]
|
||||
|
|
|
|||
|
|
@ -67,6 +67,49 @@ def validate_configuration(
|
|||
)
|
||||
|
||||
|
||||
#: Copilot virtual model names that map to native auto-routing.
|
||||
#: Forwarding these to BYOK endpoints causes a 400; they must be stripped.
|
||||
_AUTO_MODEL_ALIASES: frozenset[str] = frozenset({"auto"})
|
||||
|
||||
|
||||
def is_auto_model(model: str | None) -> bool:
|
||||
"""Return True when the model name is a Copilot auto-routing alias.
|
||||
|
||||
``model auto`` is a virtual model ID that Copilot resolves internally.
|
||||
It is **not** a valid model string for BYOK providers (Anthropic, OpenAI)
|
||||
and causes a ``400 The requested model is not supported`` error if forwarded
|
||||
verbatim. This helper centralises the detection so both the CLI and the
|
||||
proxy layer can guard against it.
|
||||
"""
|
||||
if not model:
|
||||
return False
|
||||
return model.strip().lower() in _AUTO_MODEL_ALIASES
|
||||
|
||||
|
||||
def strip_auto_model_args(copilot_args: tuple[str, ...]) -> tuple[str, ...]:
|
||||
"""Remove ``--model auto`` (and ``--model=auto``) from Copilot CLI args.
|
||||
|
||||
Used in the subscription/OAuth path: when the user passes ``--model auto``
|
||||
to ``headroom wrap copilot --subscription``, we strip it before launching
|
||||
Copilot so the CLI falls back to its own native automatic model selection
|
||||
instead of sending the unsupported ``auto`` string to the BYOK API.
|
||||
"""
|
||||
result: list[str] = []
|
||||
i = 0
|
||||
while i < len(copilot_args):
|
||||
arg = copilot_args[i]
|
||||
if arg == "--model" and i + 1 < len(copilot_args):
|
||||
if is_auto_model(copilot_args[i + 1]):
|
||||
i += 2 # skip both --model and auto
|
||||
continue
|
||||
elif arg.startswith("--model=") and is_auto_model(arg.split("=", 1)[1]):
|
||||
i += 1 # skip --model=auto
|
||||
continue
|
||||
result.append(arg)
|
||||
i += 1
|
||||
return tuple(result)
|
||||
|
||||
|
||||
def _normalized_model_name(model: str | None) -> str:
|
||||
"""Return a lowercase model name without provider/path prefixes."""
|
||||
if not model:
|
||||
|
|
@ -156,14 +199,15 @@ def build_launch_env(
|
|||
|
||||
|
||||
def model_configured(copilot_args: tuple[str, ...], env: Mapping[str, str]) -> bool:
|
||||
"""Return True when Copilot BYOK model selection is configured."""
|
||||
if env.get("COPILOT_MODEL") or env.get("COPILOT_PROVIDER_MODEL_ID"):
|
||||
return True
|
||||
"""Return True when Copilot BYOK model selection is configured (non-auto).
|
||||
|
||||
for idx, arg in enumerate(copilot_args):
|
||||
if arg == "--model" and idx + 1 < len(copilot_args):
|
||||
return True
|
||||
if arg.startswith("--model="):
|
||||
return True
|
||||
|
||||
return False
|
||||
``--model auto`` is **not** considered configured for BYOK purposes: it is
|
||||
a virtual Copilot routing token that has no meaning to external providers
|
||||
such as Anthropic or OpenAI, and forwarding it causes a 400. Returning
|
||||
``False`` here ensures the BYOK "model required" warning is still shown
|
||||
when the user mistakenly passes ``--model auto`` in BYOK mode.
|
||||
"""
|
||||
model = copilot_model_from_args(copilot_args, env)
|
||||
if model is None or is_auto_model(model):
|
||||
return False
|
||||
return True
|
||||
|
|
|
|||
|
|
@ -171,10 +171,41 @@ def test_wrap_copilot_openai_backend_sets_completions_env(
|
|||
f"http://127.0.0.1:8787{_expected_project_prefix()}/v1"
|
||||
)
|
||||
assert env["COPILOT_PROVIDER_WIRE_API"] == "completions"
|
||||
assert captured["backend"] == "anyllm"
|
||||
assert captured["anyllm_provider"] == "groq"
|
||||
assert captured["region"] == "us-central1"
|
||||
assert captured["args"] == ("--model", "gpt-4o")
|
||||
|
||||
|
||||
def test_wrap_copilot_byok_rejects_auto_model_before_launch(
|
||||
runner: CliRunner,
|
||||
wrap_modules: tuple[types.ModuleType, click.Group],
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
_wrap_cli, main = wrap_modules
|
||||
monkeypatch.setenv("OPENAI_API_KEY", "sk-test-dummy")
|
||||
|
||||
def fail_launch_tool(**_kwargs: object) -> None:
|
||||
raise AssertionError("_launch_tool must not run with --model auto in BYOK mode")
|
||||
|
||||
with (
|
||||
patch("headroom.cli.wrap.shutil.which", return_value="copilot"),
|
||||
patch("headroom.cli.wrap.has_oauth_auth", return_value=False),
|
||||
patch("headroom.cli.wrap._launch_tool", side_effect=fail_launch_tool),
|
||||
):
|
||||
result = runner.invoke(
|
||||
main,
|
||||
[
|
||||
"wrap",
|
||||
"copilot",
|
||||
"--provider-type",
|
||||
"openai",
|
||||
"--no-context-tool",
|
||||
"--",
|
||||
"--model",
|
||||
"auto",
|
||||
],
|
||||
)
|
||||
|
||||
assert result.exit_code == 1
|
||||
assert "'--model auto' is not supported in Copilot BYOK mode" in result.output
|
||||
assert "Use a concrete model" in result.output
|
||||
|
||||
|
||||
def test_wrap_copilot_auto_detects_running_proxy_backend(
|
||||
|
|
|
|||
|
|
@ -13,11 +13,13 @@ from headroom.providers.copilot.wrap import (
|
|||
copilot_model_from_args,
|
||||
default_wire_api_for_model,
|
||||
detect_running_proxy_backend,
|
||||
is_auto_model,
|
||||
model_configured,
|
||||
model_prefers_responses_api,
|
||||
provider_key_source,
|
||||
query_proxy_config,
|
||||
resolve_provider_type,
|
||||
strip_auto_model_args,
|
||||
validate_configuration,
|
||||
)
|
||||
|
||||
|
|
@ -164,6 +166,70 @@ def test_model_configured_detects_env_and_cli_variants() -> None:
|
|||
assert model_configured(("--model", "gpt-4o"), {}) is True
|
||||
assert model_configured(("--model=gpt-4o",), {}) is True
|
||||
assert model_configured(("--other", "value"), {}) is False
|
||||
# ``auto`` is not a valid BYOK model — must be treated as unconfigured.
|
||||
assert model_configured(("--model", "auto"), {}) is False
|
||||
assert model_configured(("--model=auto",), {}) is False
|
||||
assert model_configured((), {"COPILOT_MODEL": "auto"}) is False
|
||||
assert model_configured((), {"COPILOT_PROVIDER_MODEL_ID": "auto"}) is False
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("model", "expected"),
|
||||
[
|
||||
("auto", True),
|
||||
("Auto", True),
|
||||
("AUTO", True),
|
||||
(" auto ", True),
|
||||
("gpt-4o", False),
|
||||
("gpt-5", False),
|
||||
("claude-sonnet-4.6", False),
|
||||
(None, False),
|
||||
("", False),
|
||||
],
|
||||
)
|
||||
def test_is_auto_model(model: str | None, expected: bool) -> None:
|
||||
assert is_auto_model(model) is expected
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("args", "expected"),
|
||||
[
|
||||
# Strips --model auto (space-separated)
|
||||
(
|
||||
("--model", "auto", "--port", "8788"),
|
||||
("--port", "8788"),
|
||||
),
|
||||
# Strips --model=auto (equals form)
|
||||
(
|
||||
("--model=auto", "-p", "hello"),
|
||||
("-p", "hello"),
|
||||
),
|
||||
# Case-insensitive stripping
|
||||
(
|
||||
("--model", "AUTO", "--allow-all-tools"),
|
||||
("--allow-all-tools",),
|
||||
),
|
||||
# Leaves concrete models untouched
|
||||
(
|
||||
("--model", "gpt-4o", "--port", "8788"),
|
||||
("--model", "gpt-4o", "--port", "8788"),
|
||||
),
|
||||
# Leaves --model=gpt-4o untouched
|
||||
(
|
||||
("--model=gpt-4o",),
|
||||
("--model=gpt-4o",),
|
||||
),
|
||||
# Empty args unchanged
|
||||
((), ()),
|
||||
# --model at end with no value (malformed) — leave as-is, don't crash
|
||||
(("--model",), ("--model",)),
|
||||
],
|
||||
)
|
||||
def test_strip_auto_model_args(
|
||||
args: tuple[str, ...],
|
||||
expected: tuple[str, ...],
|
||||
) -> None:
|
||||
assert strip_auto_model_args(args) == expected
|
||||
|
||||
|
||||
def test_build_launch_env_applies_project_path_prefix() -> None:
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue