Enable Headroom context compression by default - #54
Conversation
headroom_enabled now defaults to true instead of false, so every review/triage/interactive/generic run gets Headroom's context compression unless a job explicitly opts out. A job using use_bedrock, use_vertex, use_foundry, or anthropic_federation_rule_id -- or running on a self-hosted runner with neither a usable Python nor Docker -- must now set headroom_enabled: false explicitly, or the run fails at validation (the alternative-provider case) or at the proxy's own startup step (the runner case) instead of silently skipping compression. Also exposes headroom_enabled/headroom_install_method/headroom_version/ headroom_image/headroom_port/headroom_show_savings on all three reusable workflows (review.yml, triage.yml, interactive.yml), matching action.yml's own defaults. Previously these inputs existed only on the composite action and the direct-form examples, so a reusable-workflow caller had no way to opt out at all once the default flipped.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
Security findingsAdvisory findings (1)ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
🛡️ Codex Security Review · Automatically triggered
Here are some automated security review suggestions for this pull request.
Reviewed commit: 4588cfbcf6
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
…ed it Stop Headroom proxy ran immediately after the main review/triage/interactive call, but ANTHROPIC_BASE_URL stays exported to GITHUB_ENV for the rest of the job regardless of where the proxy actually stops. Three later steps -- the turn-limit wrap-up, the automatic-fix pass, and the structured review summary -- also call anthropics/claude-code-action and read that same env var, so any of them running after the proxy was already killed pointed at a dead process instead of a live one. Reproduced directly: a turn-limit wrap-up call failed with is_error:true, num_turns:1, total_cost_usd:0 (no request ever reached the model) after roughly three minutes, reproducible across two separate pull requests. Confirmed the mechanism locally -- a request against an already-killed proxy has nothing to connect to, and the SDK's own retry/backoff before giving up accounts for the multi-minute stall. Moves Stop Headroom proxy to run after the structured review summary, the last of the three call sites, instead of right after the main call.
Superseded by re-review on the latest commit.
…the age gate PyPI allows additional files -- a new platform wheel, a replacement build -- to be uploaded under an already-published version well after its initial release; a version's files are not necessarily all published together. min(upload_times) measured only the oldest file's age, so a compromised maintainer account could add a fresh malicious wheel under an already-old pinned version and still pass the 7-day age gate, since pip is free to select whichever file matches the current platform. max(upload_times) measures the newest file under the version instead, so the whole version stays blocked for the 7-day minimum from whenever it was last touched, not just when it was first published. Confirmed against the real pinned 0.37.0 release (all files published together, same result either way) and a synthetic scenario (an old release with one new file added 2 days ago) -- min() would have let it through, max() correctly blocks it.
Superseded by re-review on the latest commit.
There was a problem hiding this comment.
Re-review on 32f10a6, one new commit since the last claude[bot] review (c1cf630). Dismissed and cleared the stale APPROVED review from that prior commit; no stale inline threads of my own existed to resolve or update. Updated the PR description to also cover this new commit, which the existing text didn't mention.
fix(headroom): use the newest, not oldest, PyPI file upload time for the age gate [policy] — Correct fix for a real gap. action.yml:1327-1335 computed the pip install method's age gate from min(upload_times) across every file published under the pinned version, so a compromised maintainer account could add a fresh malicious wheel under an already-aged, trusted version and still clear the 7-day gate, since pip picks whichever uploaded file matches the runner's platform regardless of when that specific file was added. Switching to max(upload_times) re-anchors the gate to the most recently touched file, closing that gap. This is the fix for the Codex security-review finding on this PR (action.yml:332, confirmed addressed by the human reviewer's own reply) — I independently traced the same logic and agree it's correct.
I checked one thing this change depends on that isn't obvious from the diff: max()/min() here operate on the raw ISO-8601 strings, not parsed datetimes, so the fix only works if PyPI's upload_time_iso_8601 values sort lexicographically the same as chronologically (i.e. consistent fractional-second precision across every file). Fetched the real pinned 0.37.0 release's file listing directly — all six entries use the identical YYYY-MM-DDTHH:MM:SS.ffffffZ format with 6-digit microsecond precision, so lexicographic and chronological order agree in practice. Worth knowing if headroom_version is ever pinned to a release with mixed-precision timestamps, but I have no evidence PyPI produces those, so not flagging it as a finding.
The Docker install method's own age check (action.yml:1208-1299) doesn't need the same fix: it reads a single created timestamp off one content-addressed digest's config blob, not a set of files under a mutable version, so there's no oldest/newest ambiguity for it to have.
No Blocker or Should-fix findings. Nothing else in this revision to raise.
|
🗜️ Headroom context compression
|
|
🎉 This PR is included in version 1.10.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
1 participant
headroom_enabled now defaults to true rather than false. This is a real behavioral change for every existing consumer, so two things go with it, not just the flipped default:
Exposes the full headroom_* input set (headroom_enabled, headroom_install_method, headroom_version, headroom_image, headroom_port, headroom_show_savings) on all three reusable workflows. Before this, only action.yml itself and the direct-form examples could set these -- review.yml/triage.yml/interactive.yml never passed headroom_enabled through at all. Flipping the default without this would have forced Headroom on for every reusable-workflow consumer with literally no way to opt out short of forking or switching to the direct form.
Documents the two real conflicts this creates for existing callers. headroom_enabled: true hard-fails validation when combined with use_bedrock/use_vertex/use_foundry/anthropic_federation_rule_id (those bypass the proxy silently otherwise, by design), and the pip install method needs Python (the default install method) or Docker on the runner. Anyone using one of the alternative providers, or running on a self-hosted runner with neither, now needs to set headroom_enabled: false explicitly. Called out in action.yml's own input description, the README's Headroom section, and the reusable-workflow input descriptions.
This ships directly in v1 (no major bump) per explicit instruction -- existing @v1 consumers pick this up on their next run.
Follow-up fix (
c1cf630): theStop Headroom proxystep originally ran immediately after the main review/triage/interactive call, butANTHROPIC_BASE_URLstays exported to$GITHUB_ENVfor the rest of the job. Three later steps -- the turn-limit wrap-up, the automatic-fix pass, and the structured review summary -- also callanthropics/claude-code-actionand read that same env var, so any of them running after the proxy was already killed pointed at a dead process instead of a live one. Reproduced directly: a turn-limit wrap-up call failed withis_error:true,num_turns:1,total_cost_usd:0(no request ever reached the model) after roughly three minutes, across two separate pull requests. The step now runs after the structured review summary, the last of the call sites that can still need the proxy.Follow-up fix (
32f10a6): the pip install method's age gate usedmin(upload_times)across a pinned version's published files, which measures only the oldest file's age. PyPI allows additional files under an already-published version well after its initial release, so a compromised maintainer account could add a fresh malicious wheel under an already-old pinned version and still pass the 7-day age gate, since pip is free to select whichever uploaded file matches the current platform. Switched tomax(upload_times), so the whole version stays blocked for the 7-day minimum from whenever it was last touched, not just when first published. Confirmed against the real pinned 0.37.0 release (all files published together, same result either way) and a synthetic scenario (an old release with one new file added 2 days ago), wheremin()would have let it through andmax()correctly blocks it. Flagged originally by a Codex security review comment on this PR.Test plan
Validate inputsscript that: (1) the new default (headroom_enabled unset, no alt provider) passes validation, (2) the new default combined with use_bedrock fails validation with the existing error message, (3) explicitly setting headroom_enabled: false alongside use_bedrock passesnpm run lint/typecheck/format:checkall pass