headroom/tests/test_proxy_settings_endpoints.py

Ignoring revisions in .git-blame-ignore-revs. Click here to bypass and see the normal blame view.

288 lines
11 KiB
Python
Raw Permalink Normal View History

fix(proxy): dedupe Codex WS request logging for accurate mixed-provider dashboards (#2189) ## Description Running Claude Code (Anthropic) and Codex (OpenAI) against the **same** Headroom proxy instance on one port produced incorrect, unstable dashboard data. The proxy core is provider-isolated and multi-provider-safe by design; the defect was in the observability layer. The Codex `/v1/responses` **WebSocket** handler was the only path in the proxy that wrote to the request logger by hand instead of through the unified `emit_request_outcome` funnel, and it did so twice per session close: the per-turn funnel record plus an unconditional cumulative session-summary `RequestLog`. This PR removes the duplicate summary log so Codex WS emits exactly one request log per turn, matching the HTTP provider paths. ## 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 - Dropped the duplicate cumulative session-summary `RequestLog` in the Codex WS handler while preserving the per-turn `emit_request_outcome` path. - Preserved gated `request_messages` and `turn_id` on residual outcomes so dashboard telemetry keeps the useful attribution without double-counting tokens. - Ensured explicit `--anyllm-provider` wins over a leaked `HEADROOM_ANYLLM_PROVIDER` environment variable. - Registered retry delay settings that had drifted out of the settings registry. - Hardened tests against developer-shell `HEADROOM_*` / `ANTHROPIC_CUSTOM_HEADERS` leakage and stabilized several focused proxy/wrap test fixtures. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [ ] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [x] Manual testing performed ### Test Output ```text $ .venv/bin/pytest tests/ -q -p no:cacheprovider 8529 passed, 537 skipped, 5831 warnings in 276.25s (0:04:36) $ .venv/bin/ruff check <touched files> All checks passed! ``` ## Real Behavior Proof - Environment: macOS (Darwin 25.4.0), Python 3.13.14, pytest 9.0.3, ruff via project venv, branch `fix/multi-provider-runtime`. - Exact command / steps: Ran the full test suite without pytest cache provider and Ruff on all touched files; used `git stash` to confirm the stale fake-config failures pre-existed this change. - Observed result: Full suite passed with no failures; Ruff passed; Codex WS now routes end-of-session logging through `emit_request_outcome`, emitting one request log per turn with the same accounting model as Anthropic HTTP turns. - Not tested: Live simultaneous Claude + Codex dashboard run. `mypy headroom` was not run to completion; a scoped run reported one pre-existing `settings_store.py:470` coercion error outside this diff. ## 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 ## Screenshots (if applicable) N/A - server-side observability fix; no UI markup changed. ## Additional Notes - The proxy's multi-provider routing, header/auth isolation, and per-model cache keying are already correct and unchanged here; only the WS observability write path was double-counting. - Architectural assessment: `plans/reports/research-260714-0004-multi-provider-upstream-compression-report.md`; root-cause + resolution trail: `plans/reports/debug-assessment-260714-0011-dashboard-instability-mixed-claude-codex-report.md`. - No live simultaneous Claude + Codex dashboard run was performed; validation is from test coverage and code review of the WS logging path. --------- Co-authored-by: JerrettDavis <mxjerrett@gmail.com>
2026-07-16 01:18:34 +07:00
"""Tests for the dashboard settings API endpoints (Phases 2-4).
Covers GET /settings/schema, GET /settings, POST /settings, POST /settings/apply
and the GET /dashboard/settings route: loopback gating on all read/write settings
endpoints, registry validation (400 unknown key / 422 bad value), secret masking,
and the deployment-aware apply/restart dispatch (service 202 + background restart,
docker host command, foreground instruction).
"""
from __future__ import annotations
from dataclasses import replace
import pytest
pytest.importorskip("fastapi")
from fastapi.testclient import TestClient # noqa: E402
from headroom import settings_store # noqa: E402
from headroom.proxy.server import ProxyConfig, create_app # noqa: E402
def _make_app():
return create_app(
ProxyConfig(
optimize=False,
cache_enabled=False,
rate_limit_enabled=False,
cost_tracking_enabled=False,
log_requests=False,
ccr_inject_tool=False,
ccr_handle_responses=False,
ccr_context_tracking=False,
image_optimize=False,
)
)
@pytest.fixture
def workspace(tmp_path, monkeypatch):
"""Isolate settings.json under a tmp workspace for each test."""
monkeypatch.setenv("HEADROOM_WORKSPACE_DIR", str(tmp_path))
monkeypatch.delenv("HEADROOM_SETTINGS_PATH", raising=False)
# Make deployment detection deterministic: no supervisor env -> foreground.
monkeypatch.delenv("HEADROOM_DEPLOYMENT_PROFILE", raising=False)
monkeypatch.delenv("HEADROOM_DEPLOYMENT_PRESET", raising=False)
return tmp_path
@pytest.fixture
def client(workspace):
"""Loopback client — passes both the client-IP and Host-header gates."""
return TestClient(_make_app(), base_url="http://127.0.0.1", client=("127.0.0.1", 12345))
@pytest.fixture
def network_client(workspace):
"""Default TestClient presents client.host='testclient' — treated as non-loopback."""
return TestClient(_make_app())
class TestSchemaAndRead:
def test_schema_returns_grouped_fields(self, client):
resp = client.get("/settings/schema")
assert resp.status_code == 200, resp.text
body = resp.json()
assert body["groups"]
assert body["fields"]
assert body["needs_restart_keys"]
assert "supervised" in body # server-added deployment flag
assert body["supervised"] is False # foreground in tests
def test_get_settings_reflects_saved_values(self, client, workspace):
settings_store.save({"target_ratio": 0.5, "savings_profile": "balanced"})
resp = client.get("/settings")
assert resp.status_code == 200, resp.text
assert resp.json() == {"target_ratio": 0.5, "savings_profile": "balanced"}
class TestEndpointsGroup:
def test_schema_includes_endpoints_group_and_secret_flags(self, client):
resp = client.get("/settings/schema")
assert resp.status_code == 200, resp.text
body = resp.json()
assert "Endpoints" in body["groups"]
by_key = {f["key"]: f for f in body["fields"]}
for key in (
"anthropic_base_url",
"openai_base_url",
"anthropic_extra_headers",
"openai_extra_headers",
):
assert key in by_key, f"missing field {key}"
assert by_key["anthropic_extra_headers"]["secret"] is True
assert by_key["openai_extra_headers"]["secret"] is True
assert by_key["anthropic_base_url"]["secret"] is False
def test_get_settings_masks_extra_headers(self, client, workspace):
settings_store.save({"anthropic_extra_headers": '{"Api-Key": "super-secret"}'})
resp = client.get("/settings")
assert resp.status_code == 200, resp.text
assert resp.json()["anthropic_extra_headers"] == settings_store._MASK
def test_schema_values_mask_extra_headers(self, client, workspace):
settings_store.save({"openai_extra_headers": '{"Api-Key": "super-secret"}'})
resp = client.get("/settings/schema")
assert resp.status_code == 200, resp.text
body = resp.json()
assert body["values"]["openai_extra_headers"] == settings_store._MASK
class TestWriteValidation:
def test_valid_write_persists(self, client, workspace):
resp = client.post("/settings", json={"values": {"target_ratio": 0.4, "rpm": 30}})
assert resp.status_code == 200, resp.text
body = resp.json()
assert body["ok"] is True
assert body["needs_restart"] is True
assert set(body["changed_keys"]) == {"target_ratio", "rpm"}
assert settings_store.load() == {"target_ratio": 0.4, "rpm": 30}
def test_unknown_key_400(self, client):
resp = client.post("/settings", json={"values": {"nope": 1}})
assert resp.status_code == 400, resp.text
assert "nope" in resp.json()["unknown_keys"]
def test_bad_value_422(self, client):
resp = client.post("/settings", json={"values": {"target_ratio": 5}})
assert resp.status_code == 422, resp.text
assert "target_ratio" in resp.json()["field_errors"]
def test_missing_values_object_400(self, client):
resp = client.post("/settings", json={"foo": 1})
assert resp.status_code == 400, resp.text
class TestLoopbackGating:
def test_reads_and_writes_rejected_off_loopback(self, network_client):
assert network_client.get("/settings/schema").status_code == 404
assert network_client.get("/settings").status_code == 404
assert network_client.post("/settings", json={"values": {}}).status_code == 404
assert network_client.post("/settings/apply", json={}).status_code == 404
def test_settings_page_rejected_off_loopback(self, network_client):
assert network_client.get("/dashboard/settings").status_code == 404
class TestSameOriginGuard:
"""CSRF: a same-machine (loopback) attacker page can still send a
non-preflighted 'simple' request straight to a known 127.0.0.1 URL --
the Host header reads the real (loopback) destination either way, but
a real browser's Origin header reflects the page's actual origin.
require_loopback's Host-header check alone does not catch this.
"""
def test_foreign_origin_rejected_on_settings_post(self, client, workspace):
resp = client.post(
"/settings",
json={"values": {"target_ratio": 0.4}},
headers={"Origin": "https://evil.example"},
)
assert resp.status_code == 403, resp.text
def test_null_origin_rejected(self, client, workspace):
resp = client.post(
"/settings",
json={"values": {"target_ratio": 0.4}},
headers={"Origin": "null"},
)
assert resp.status_code == 403, resp.text
def test_foreign_origin_rejected_on_settings_apply(self, client, monkeypatch):
from headroom.install import runtime as rt
monkeypatch.setattr(
rt, "restart_current_deployment", lambda: {"restarted": False, "mode": "foreground"}
)
resp = client.post(
"/settings/apply",
json={},
headers={"Origin": "https://evil.example"},
)
assert resp.status_code == 403, resp.text
def test_loopback_origin_allowed(self, client, workspace):
resp = client.post(
"/settings",
json={"values": {"target_ratio": 0.4}},
headers={"Origin": "http://127.0.0.1:8787"},
)
assert resp.status_code == 200, resp.text
def test_absent_origin_allowed(self, client, workspace):
# No Origin header at all (CLI tools, curl, TestClient's default) must pass.
resp = client.post("/settings", json={"values": {"target_ratio": 0.4}})
assert resp.status_code == 200, resp.text
class TestSecretMasking:
def test_secret_never_returned_unmasked(self, client, workspace, monkeypatch):
# No curated knob is secret; flag one to exercise the masking path end-to-end.
base_field = next(f for f in settings_store.SETTINGS if f.key == "log_file")
secret_field = replace(base_field, secret=True)
registry = tuple(
secret_field if f.key == "log_file" else f for f in settings_store.SETTINGS
)
monkeypatch.setattr(settings_store, "SETTINGS", registry)
monkeypatch.setattr(settings_store, "_BY_KEY", {f.key: f for f in registry})
fix(settings): accept documented HEADROOM_* env names as settings keys (#2833) ## Description Settings validation only accepted short JSON/API keys, so documented HEADROOM_* env names were rejected as unknown. Users following the docs (for example HEADROOM_LOSSLESS) hit SettingsValidationError / PUT /settings 400 even though those names are already on each registry field. This normalizes known env aliases to their short keys before validate/save, keeps existing short-key behavior, and rejects conflicting env+key pairs for the same field. Closes #2812 ## 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 - Added _BY_ENV and _normalize_values() in settings_store to map documented env names to short keys - Call normalization at the start of validate() and save() so clear/retain paths also accept env aliases - Reject payloads that supply both an env alias and its short key with different values - Add unit coverage for accept/clear/conflict/same-value paths and update registry monkeypatches to rebuild _BY_ENV ## 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 python -m pytest tests/test_proxy/test_settings_store.py -q -k "env_alias or validate_accepts or save_rejects or same_env or conflicting or save_accepts or env_alias_clear or anthropic_extra_headers_retain or TestValidation" 23 passed, 11 deselected ruff check headroom/settings_store.py tests/test_proxy/test_settings_store.py tests/test_proxy_settings_endpoints.py All checks passed! ruff format --check headroom/settings_store.py tests/test_proxy/test_settings_store.py tests/test_proxy_settings_endpoints.py 3 files already formatted ``` ## Real Behavior Proof - Environment: Linux x86_64, Python 3.14.5 via contributor venv, worktree of headroom main at 7940c05e plus commit e4c87edf - Exact command / steps: pytest tests/test_proxy/test_settings_store.py focused selection; ruff check and ruff format --check on the three touched files; settings_store.validate({"HEADROOM_LOSSLESS": True}) returns {"lossless": True} - Observed result: Env aliases coerce and persist under short keys; unknown short keys still error; conflicting env+key pairs raise SettingsValidationError; ruff clean on touched files - Not tested: Live dashboard PUT /settings through a running proxy (HTTP suite needs native headroom._core); mypy; full monorepo CI ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review ## Checklist - [x] My code follows the project style guidelines - [x] I have performed a self-review of my code - [ ] 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 - [x] I did not edit CHANGELOG.md - it is generated by release-please from my Conventional Commit PR title (a CI guard enforces this) ## Screenshots (if applicable) N/A ## Additional Notes - Scoped to settings key normalization only - Registry drift Click test was not exercised here because this environment lacks tomlkit for an unrelated import path
2026-08-12 05:22:27 +07:00
monkeypatch.setattr(settings_store, "_BY_ENV", {f.env: f for f in registry})
fix(proxy): dedupe Codex WS request logging for accurate mixed-provider dashboards (#2189) ## Description Running Claude Code (Anthropic) and Codex (OpenAI) against the **same** Headroom proxy instance on one port produced incorrect, unstable dashboard data. The proxy core is provider-isolated and multi-provider-safe by design; the defect was in the observability layer. The Codex `/v1/responses` **WebSocket** handler was the only path in the proxy that wrote to the request logger by hand instead of through the unified `emit_request_outcome` funnel, and it did so twice per session close: the per-turn funnel record plus an unconditional cumulative session-summary `RequestLog`. This PR removes the duplicate summary log so Codex WS emits exactly one request log per turn, matching the HTTP provider paths. ## 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 - Dropped the duplicate cumulative session-summary `RequestLog` in the Codex WS handler while preserving the per-turn `emit_request_outcome` path. - Preserved gated `request_messages` and `turn_id` on residual outcomes so dashboard telemetry keeps the useful attribution without double-counting tokens. - Ensured explicit `--anyllm-provider` wins over a leaked `HEADROOM_ANYLLM_PROVIDER` environment variable. - Registered retry delay settings that had drifted out of the settings registry. - Hardened tests against developer-shell `HEADROOM_*` / `ANTHROPIC_CUSTOM_HEADERS` leakage and stabilized several focused proxy/wrap test fixtures. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [ ] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [x] Manual testing performed ### Test Output ```text $ .venv/bin/pytest tests/ -q -p no:cacheprovider 8529 passed, 537 skipped, 5831 warnings in 276.25s (0:04:36) $ .venv/bin/ruff check <touched files> All checks passed! ``` ## Real Behavior Proof - Environment: macOS (Darwin 25.4.0), Python 3.13.14, pytest 9.0.3, ruff via project venv, branch `fix/multi-provider-runtime`. - Exact command / steps: Ran the full test suite without pytest cache provider and Ruff on all touched files; used `git stash` to confirm the stale fake-config failures pre-existed this change. - Observed result: Full suite passed with no failures; Ruff passed; Codex WS now routes end-of-session logging through `emit_request_outcome`, emitting one request log per turn with the same accounting model as Anthropic HTTP turns. - Not tested: Live simultaneous Claude + Codex dashboard run. `mypy headroom` was not run to completion; a scoped run reported one pre-existing `settings_store.py:470` coercion error outside this diff. ## 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 ## Screenshots (if applicable) N/A - server-side observability fix; no UI markup changed. ## Additional Notes - The proxy's multi-provider routing, header/auth isolation, and per-model cache keying are already correct and unchanged here; only the WS observability write path was double-counting. - Architectural assessment: `plans/reports/research-260714-0004-multi-provider-upstream-compression-report.md`; root-cause + resolution trail: `plans/reports/debug-assessment-260714-0011-dashboard-instability-mixed-claude-codex-report.md`. - No live simultaneous Claude + Codex dashboard run was performed; validation is from test coverage and code review of the WS logging path. --------- Co-authored-by: JerrettDavis <mxjerrett@gmail.com>
2026-07-16 01:18:34 +07:00
settings_store.save({"log_file": "/tmp/secret.log"})
assert client.get("/settings").json()["log_file"] == settings_store._MASK
schema = client.get("/settings/schema").json()
log_field = next(f for f in schema["fields"] if f["key"] == "log_file")
assert log_field["value"] == settings_store._MASK
assert log_field["stored"] == settings_store._MASK
class TestApplyRestart:
def _patch_mode(self, monkeypatch, mode, restart_result):
from headroom.install import runtime as rt
monkeypatch.setattr(rt, "detect_current_deployment", lambda: (None, mode))
calls = []
def fake_restart():
calls.append(True)
return restart_result
monkeypatch.setattr(rt, "restart_current_deployment", fake_restart)
return calls
def test_service_returns_202_and_runs_background_restart(self, client, monkeypatch):
calls = self._patch_mode(monkeypatch, "service", {"restarted": True, "mode": "service"})
resp = client.post("/settings/apply", json={})
assert resp.status_code == 202, resp.text
assert resp.json()["restarted"] is True
# TestClient runs the BackgroundTask after sending the response.
assert calls == [True]
def test_docker_returns_host_command(self, client, monkeypatch):
self._patch_mode(
monkeypatch,
"docker",
{
"restarted": False,
"mode": "docker",
"command": "headroom install restart --profile default",
},
)
resp = client.post("/settings/apply", json={})
assert resp.status_code == 200, resp.text
body = resp.json()
assert body["mode"] == "docker"
assert "headroom install restart" in body["command"]
def test_foreground_returns_instruction(self, client, monkeypatch):
self._patch_mode(
monkeypatch,
"foreground",
{
"restarted": False,
"mode": "foreground",
"instruction": "Restart the proxy to apply the new settings.",
},
)
resp = client.post("/settings/apply", json={})
assert resp.status_code == 200, resp.text
assert "instruction" in resp.json()
def test_apply_persists_provided_values(self, client, workspace, monkeypatch):
self._patch_mode(
monkeypatch,
"foreground",
{"restarted": False, "mode": "foreground", "instruction": "x"},
)
resp = client.post("/settings/apply", json={"values": {"rpm": 42}})
assert resp.status_code == 200, resp.text
assert settings_store.load() == {"rpm": 42}
class TestSettingsPageRoute:
def test_dashboard_settings_serves_html(self, client):
resp = client.get("/dashboard/settings")
assert resp.status_code == 200, resp.text
assert "<" in resp.text # rendered HTML page