diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a9b3b8aa..cb41d6292 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -117,6 +117,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * **cli/proxy:** honor `HEADROOM_MIN_TOKENS=0` / `HEADROOM_MAX_ITEMS=0`. The Click `proxy` command built these with `_get_env_int_optional(name) or 500`/`or 50`, so an explicit `0` — a legitimate value (`min_tokens_to_crush=0` means "crush every item") — was treated as falsy and silently replaced with the default. The `headroom proxy` argparse path already preserved `0` via `_get_env_int`, so the two entry points disagreed. The Click path now uses the same None-checking helper. * **proxy:** strip the inbound `Content-Encoding`/`Transfer-Encoding` request headers on the Anthropic `/v1/messages` and OpenAI `/v1/chat/completions` paths before forwarding upstream. `read_request_json_with_bytes` already decompresses the inbound body (zstd/gzip/deflate/br), so the bytes forwarded upstream are plain JSON — but these two handlers left the original `content-encoding` header in place, so a client (or an edge proxy like a Cloudflare Worker) that sent a compressed body got its request rejected with upstream HTTP 400 because the provider tried to decompress already-decoded JSON. The `/v1/responses` handler already carried this fix (#1542); it is now applied to the messages and chat paths too. * **models:** fix the model registry's prefix fallback silently returning the wrong context window. `ModelRegistry.get` accepted any registered name as a `str.startswith` prefix and returned the *first* match, so `gpt-4-32k-0613` resolved to `gpt-4` (8192) instead of `gpt-4-32k` (32768), and unregistered ids like `gpt-4.1`/`gpt-4.5` inherited `gpt-4`'s 8192-token window — making the proxy think a nearly-empty context was almost full and compress far too aggressively. The fallback now requires the registered name to end at a version boundary in the query (so `gpt-4.1` no longer matches `gpt-4`) and picks the longest qualifying name (so `gpt-4-32k-0613` → `gpt-4-32k`). +* **install:** stop `resolve_targets` from rejecting valid `--providers all`/`auto` installs under provider scope. The provider-scope "unsupported targets" validation ran before the mode dispatch, so `headroom install apply --scope provider --providers all --target cursor` raised `ClickException` even though `all`/`auto` ignore the requested target list entirely (user scope silently ignores the same input). The check now runs only on the manual path that actually consults the requested list. * **mcp/opencode:** stop the OpenCode MCP registrar from destroying an existing but unparseable `opencode.json`. `_write_entry` read the config via a helper that returns `{}` on `JSONDecodeError`, then rewrote the whole file with only `{"mcp": {...}}` — wiping the user's `theme`/`model`/`provider` and any other MCP servers (OpenCode configs are commonly JSONC / hand-edited). The write path now refuses to overwrite a present-but-invalid config and returns a `FAILED` result; absent/empty files still register fresh and valid files still merge with all other keys preserved. (Same class of fix as the Claude registrar.) * **proxy:** include the system prompt, tools, and the response-shaping request fields in the SemanticCache key. `_compute_key` hashed only `{model, messages}`, so two non-streaming requests with identical messages but a different top-level `system` prompt, tool set, sampling config, or output-shaping field collided on one key and the second caller was served the first's cached response — generated under different request semantics, in the default config (`cache_enabled` defaults on). The key now folds the request fields that shape generation — `temperature`/`top_p`/`top_k`/`max_tokens`/`stop`, plus OpenAI `tool_choice`/`response_format`/`parallel_tool_calls`/`seed`/`presence_penalty`/`frequency_penalty`/`logit_bias`/`n`/`logprobs`/`top_logprobs`/`reasoning_effort`/`verbosity`/`modalities` and Anthropic `thinking`/`tool_choice`/`output_config` — canonicalizing `system`/`tools` so a moved `cache_control` breakpoint does not fragment it, and the handlers snapshot the fields once at the cache read and reuse them at write so a body mutated by the pipeline cannot diverge the key. Non-streaming path only. * **learn (verbosity):** `--verbosity --apply --all` now aggregates the savings baseline across every project instead of overwriting it per project (last-project-wins), which previously left the output shaper with a tiny, unrepresentative baseline. The applied verbosity level comes from the project with the most samples ([#1288](https://github.com/headroomlabs-ai/headroom/pull/1288)). diff --git a/headroom/install/planner.py b/headroom/install/planner.py index d2f6e9cce..df5557f8e 100644 --- a/headroom/install/planner.py +++ b/headroom/install/planner.py @@ -69,15 +69,6 @@ def resolve_targets( valid = {target.value for target in valid_targets} requested = [target.strip().lower() for target in requested_targets] - if scope == ConfigScope.PROVIDER.value: - unsupported = [target for target in requested if target and target not in valid] - if unsupported: - unsupported_list = ", ".join(sorted(set(unsupported))) - raise click.ClickException( - "Provider scope supports only claude, codex, openclaw, and opencode; " - f"unsupported targets: {unsupported_list}" - ) - if provider_mode == ProviderSelectionMode.ALL.value: return [target.value for target in valid_targets] @@ -89,6 +80,20 @@ def resolve_targets( *([] if scope == ConfigScope.PROVIDER.value else [ToolTarget.COPILOT.value]), ] + # Manual selection is the only mode that consults `requested`, so the + # provider-scope validation belongs here. Running it earlier rejected + # unsupported entries that `all`/`auto` ignore entirely — e.g. + # `install apply --scope provider --providers all --target cursor` raised + # instead of returning the provider target set. + if scope == ConfigScope.PROVIDER.value: + unsupported = [target for target in requested if target and target not in valid] + if unsupported: + unsupported_list = ", ".join(sorted(set(unsupported))) + raise click.ClickException( + "Provider scope supports only claude, codex, openclaw, and opencode; " + f"unsupported targets: {unsupported_list}" + ) + normalized = [] seen: set[str] = set() for value in requested: diff --git a/tests/test_install/test_planner.py b/tests/test_install/test_planner.py index 1eac07d5e..37ee39016 100644 --- a/tests/test_install/test_planner.py +++ b/tests/test_install/test_planner.py @@ -1,7 +1,10 @@ from __future__ import annotations +import click +import pytest + from headroom.install.models import ConfigScope, InstallPreset, ProviderSelectionMode, ToolTarget -from headroom.install.planner import build_manifest, resolve_targets +from headroom.install.planner import PROVIDER_SCOPE_TARGETS, build_manifest, resolve_targets def test_resolve_targets_auto_falls_back_when_detection_empty(monkeypatch) -> None: @@ -146,3 +149,41 @@ def test_build_manifest_persists_no_http2_override() -> None: assert manifest.proxy_args.count("--no-http2") == 1 assert "HEADROOM_HTTP2" not in manifest.base_env + + +def test_resolve_targets_provider_scope_all_ignores_unsupported_requested() -> None: + """`all` mode never consults the requested list, so an unsupported entry + like `cursor` must not make it raise — it should return the full provider + target set (regression: this used to raise a ClickException).""" + targets = resolve_targets( + ProviderSelectionMode.ALL.value, + ["cursor"], + scope=ConfigScope.PROVIDER.value, + ) + + assert targets == [t.value for t in PROVIDER_SCOPE_TARGETS] + + +def test_resolve_targets_provider_scope_auto_ignores_unsupported_requested(monkeypatch) -> None: + """`auto` mode also ignores the requested list, so an unsupported entry + must not raise.""" + monkeypatch.setattr("headroom.install.planner.detect_targets", lambda: []) + + targets = resolve_targets( + ProviderSelectionMode.AUTO.value, + ["cursor"], + scope=ConfigScope.PROVIDER.value, + ) + + assert targets == [ToolTarget.CLAUDE.value, ToolTarget.CODEX.value] + + +def test_resolve_targets_provider_scope_manual_rejects_unsupported() -> None: + """The manual path DOES consult the requested list, so an unsupported + target under provider scope must still be rejected.""" + with pytest.raises(click.ClickException, match="cursor"): + resolve_targets( + ProviderSelectionMode.MANUAL.value, + ["cursor"], + scope=ConfigScope.PROVIDER.value, + )