mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
5d9bbbeea0
## Description `headroom install apply --providers manual --target codex --scope provider` silently failed to route Codex through the proxy whenever `~/.codex/config.toml` already had a `[table]` section (e.g. `[features]`, `[mcp_servers.*]`). `apply_provider_scope` appended the managed `model_provider = "headroom"` block after the last existing table, so TOML scoped the bare key into that table instead of the document root — Codex silently ignored it and kept routing through its default provider. The same code path never overrode a pre-existing top-level `model_provider` assignment either, so a user's `model_provider = "openai"` kept winning even when Headroom's block was appended elsewhere in the file. This mirrors a bug already fixed in the `headroom init` path (`_ensure_codex_provider`, #260) that was never ported to the persistent-install path. Closes: reported via user session (no tracked issue number yet). ## Type of Change - [x] Bug fix (non-breaking change which fixes an issue) ## Changes Made - **`headroom/providers/codex/install.py`**: Added `_insert_block_at_root()`, which walks the document line-by-line and inserts the managed marker block immediately above the first `[table]`/`[[array-of-tables]]` header, falling back to end-of-file append only when no table exists. Mirrors the root-insertion logic already used by `cli/init.py:_ensure_codex_provider`. - Added `_ANY_MODEL_PROVIDER` / `_ANY_OPENAI_BASE_URL` patterns (match any value, not just `"headroom"`) so `apply_provider_scope` strips **any** prior top-level `model_provider` / `openai_base_url` assignment before re-inserting the managed block — the managed keys now override the user's config outright instead of losing to it. - `apply_provider_scope` merge order is now: strip old managed block → strip prior top-level assignments → insert fresh block at document root. ## Testing - [x] **New regression test**: `test_apply_codex_provider_scope_lands_model_provider_at_root` (`tests/test_install/test_providers.py`) — asserts `model_provider = "headroom"` lands before `[features]`, overrides a prior `"openai"` value, and the user's own table content survives. - [x] **Existing tests**: `tests/test_install/test_providers.py` — 42/42 pass (includes prior codex apply/revert/replace/orphan-cleanup coverage). - [x] **Adversarial (ad-hoc, not committed)**: 6-case TOML round-trip proof — parses output with `tomllib` (not substring matching) across: prior provider before a table, no prior provider, empty file, scalars-only (no tables), CRLF line endings, multiple tables. All 6 pass after the fix; first pass caught a false failure from a stale globally pip-installed `headroom` copy shadowing the repo source when tests run outside the project directory — re-verified from inside the repo to confirm the fix itself is correct. - [x] **Lint**: `ruff check` and `ruff format --check` pass on both changed files. ```text $ uv run pytest tests/test_install/test_providers.py -q 42 passed in 0.21s $ uv run --with ruff ruff check headroom/providers/codex/install.py tests/test_install/test_providers.py All checks passed! $ uv run --with ruff ruff format --check headroom/providers/codex/install.py tests/test_install/test_providers.py 2 files already formatted ``` ## Real Behavior Proof - Environment: macOS 26.4.1 (arm64), Python 3.13.14, headroom branch `patch/install-codex` - Exact command / steps: constructed a temp `config.toml` with `[features]\nweb_search = true` (no existing Headroom block), invoked `apply_provider_scope(manifest)` against it with `codex_config_path` patched to the temp file, then parsed the result with `tomllib.loads()`. - Observed result: before the fix, `tomllib.loads(result)["model_provider"]` raised `KeyError` — the key was nested inside `[features]` due to end-of-file append. After the fix, `parsed["model_provider"] == "headroom"` and `parsed["features"]["web_search"] is True` — both the managed key and the user's table are present and correctly scoped. Revert removes `model_provider` and preserves the user's table. - Tested local build and behavior is correct as expected of this patch. ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review |
||
|---|---|---|
| .. | ||
| test_health.py | ||
| test_native_installers.py | ||
| test_paths.py | ||
| test_planner.py | ||
| test_providers.py | ||
| test_runtime.py | ||
| test_state.py | ||
| test_supervisors.py | ||