2 commits

Author	SHA1	Message	Date
nangsontay	e5b3a634df	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-15 18:18:34 +00:00
chopratejas	2e874c5e3e	fix: A5 — strip x-headroom-* from upstream-bound headers (P5-49) ... Eliminate P5-49: every Python forwarder and the Rust transparent proxy now drop internal `x-headroom-*` request headers (`x-headroom-bypass`, `x-headroom-mode`, `x-headroom-user-id`, `x-headroom-stack`, `x-headroom-base-url`) before the upstream call. Stops fingerprinting of the proxy by subscription-revocation enforcers and prevents leakage of internal user-id / stack / base-url internals to whichever vendor terminates the request. Python: - `_strip_internal_headers(headers)` in `headroom/proxy/helpers.py` returns a NEW dict with `x-headroom-*` keys removed (case-insensitive prefix match, no regex). Pure function. Operator opt-in `HEADROOM_STRIP_INTERNAL_HEADERS=disabled` keeps internal headers in the upstream-bound dict for diagnostic shadow tracing — explicit, not a fallback. - Strip applied at every handler entry capture in `anthropic.py`, `openai.py`, `batch.py`, `gemini.py` (chat completions, responses, WebSocket handshake, Copilot passthrough, batch passthroughs, Gemini generate / stream / countTokens / cloudcode-assist, Anthropic passthrough + batch results). Inbound reads of x-headroom (bypass gating, memory user-id) migrated to `request.headers.get(...)` so they continue working off the original dict. - `log_outbound_headers` emits `event=outbound_headers forwarder=... stripped_count=N request_id=...` per call. Never logs header values. Rust (crates/headroom-proxy): - `strip_internal_headers(&mut HeaderMap)` and `is_internal_header` helpers in `src/headers.rs`. `build_forward_request_headers` accepts a `strip_internal: bool` so the same path serves HTTP and WebSocket. - `Config::strip_internal_headers: StripInternalHeaders` driven by CLI flag `--strip-internal-headers` and env var `HEADROOM_PROXY_STRIP_INTERNAL_HEADERS` (default `enabled`). - `proxy.rs` and `websocket.rs` call `build_forward_request_headers` with the resolved policy; structured `tracing::info!` / `tracing::warn!` line per request describes the strip decision. Tests: 24 Python (`tests/test_header_isolation.py`) + 4 Rust integration (`crates/headroom-proxy/tests/integration_headers.rs`) + 4 Rust unit tests in `headers.rs`. Covers every named header (`bypass`, `mode`, `user-id`, `stack`, `base-url`), case-insensitive prefix matching, legitimate-headers passthrough, the `disabled` operator-opt-in mode, and that the inbound bypass-gating read path is unaffected by the strip. Acceptance: targeted `pytest -x` suite green (87 tests across test_header_isolation, test_proxy_byte_faithful_forwarding, test_proxy_anthropic_cache_stability, test_proxy_system_prompt_immutable, test_proxy_openai_cache_stability, test_proxy_pipeline_lifecycle). `cargo test -p headroom-proxy` green (23 tests across all integrations plus 7 lib unit tests). `cargo clippy -p headroom-proxy -- -D warnings` clean. `cargo fmt --all -- --check` clean. `cargo test --workspace` green (~900 tests total). Per realignment build constraints: configurable (env + CLI), no hardcodes, no regex (pure `.lower().starts_with()` match), no silent fallbacks (`disabled` is loud operator opt-in), structured logs (`event=outbound_headers`). Remaining `x-headroom-` references in `headroom/proxy/handlers/` are inbound-read sites only: `request.headers.get("x-headroom-bypass")` / `x-headroom-mode` for behavior gating, `request.headers.get ("x-headroom-user-id")` for memory user-id resolution, and `ws_headers .get(...)` on the WebSocket inbound path. Response-side `X-Headroom-*` injection (e.g. `x-headroom-tokens-saved`) is unrelated to upstream forwarding and untouched.	2026-05-02 09:35:27 -07:00