fix(wrap): isolate proxy stdio from proxy.log on Windows#1191
Open
rodboev wants to merge 1 commit into
Open
Conversation
Contributor
PR governanceThis PR follows the template and is marked ready for human review. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fix the Windows
proxy.logrollover storm by separating wrap-managed subprocess stdio from the proxy's rotating runtime log.headroom/cli/wrap.pycurrently opens~/.headroom/logs/proxy.logand hands that file handle to the proxy subprocess, whileheadroom/proxy/helpers.pyalso rotates that same path at 10 MB with five backups. On Windows, the inherited stdio handle prevents the rename inRotatingFileHandler.doRollover(), which matches the repeatedWinError 32traceback loop documented in#1184. This change keepsproxy.logas the canonical rotating runtime log and moves wrap-managed stdio into a dedicated sibling file so rollover can succeed without losing startup diagnostics. Closes #1184The reproduction and split-fix sketch in #1184 materially shaped the chosen scope; this PR follows that root-cause split rather than changing the proxy's rotation policy.
Type of Change
Changes Made
stdoutandstderrinto a dedicated sibling log instead ofproxy.logproxy.logas the success-pathLogs:target and the sole rotating runtime log owned by the proxy_start_proxy()and document the behavior change inCHANGELOG.mdTesting
uv run pytest tests/test_cli_proxy_env.py)uv run ruff check headroom/cli/wrap.py tests/test_cli_proxy_env.py)uv run ruff format headroom/cli/wrap.py tests/test_cli_proxy_env.py --check)uv run mypy headroom)Test Output
Real Behavior Proof
uv run pytest tests/test_cli_proxy_env.py -k "start_proxy_redirects_subprocess_stdio_to_standalone_log or start_proxy_tail_reads_standalone_stdio_log_on_process_exit or start_proxy_passes_resolved_copilot_api_url_to_proxy" -q3 passed, 43 deselected in 0.37s; the regression slice proves_start_proxy()now routes subprocessstdoutandstderrtoproxy-stdio.log, still reportsLogs: .../proxy.logto the user, reads startup-failure tails fromproxy-stdio.log, and preserves Copilot target URL/token env wiring.proxy.log;uv run mypy headroom; the repo-wide suite beyond the focused regression and lint checks.Review Readiness
Checklist
Screenshots (if applicable)
Not applicable, the proof is command and log behavior rather than a visual change.
Additional Notes
The intended scope stayed narrow: isolate wrap-managed stdio from
proxy.log, keep runtime logging semantics unchanged, and avoid widening into proxy-side logging policy changes unless the wrap-only fix proves insufficient during implementation.