Repository navigation
pr-pipeline: review-lane hold follow-ups from the #9361 security review #9369
Copy link
Copy link
Open
Labels
needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.Human-in-the-loop required; autonomous sessions must not resolve items carrying this.status: needs-decisionAwaiting a human or maintainer judgment call.Awaiting a human or maintainer judgment call.
Description
Refs: #9276
Follow-ups from the security review of #9361 (review-lane holds in ci-status). Each was rated MEDIUM or LOW and left out of that PR on purpose.
Medium
- Review runs are matched to a PR by head SHA only. ci-status's "Require both review lanes on this head" step, and
pr-feedback-dispatch.yml'srerun-ci-status, take the newestpull_requestrun of each caller on the head SHA. A second PR on the same SHA, with a different base, has its own review, and that review can count for this PR. Fix: give both callers arun-namecarrying the PR number, and filter ondisplay_title. Alternatively, also require this PR's ownclaude-lane-review-completemarker for the head. - The kill switch's pass can fail open and outlive the switch. A lane passes when its
Report the kill switchstep ran, and that step also runs whencheck-kill-switchfails closed on an unreadable variable. A head that passed while the switch was on stays green after it clears.pr-merge.ymldoes not readCLAUDE_REVIEW_DISABLED, so a review-only stop leaves agent PRs merging with no AI review. OD-8 chose no merge freeze. Options: pass a stopped lane only on PRs a trusted person authored, or make pr-merge refuse lane merges while a review switch is on, and re-run ci-status on open heads when the switch clears. - The gate runs from the PR's own copy of
pr-require-checks.yml.hold-pathsdoes not list the gate's own files:pr-require-checks.yml, both review callers,pr-feedback-dispatch.ymlandpr-merge.yml.- The merge-group run re-checks labels only, not the review-lane requirement or who lifted a hold.
- Options: add the workflow files to
hold-paths(owner decision Q48 set the current list), and move the lift and review checks into default-branch code, such as the pr-merge prefilter or aworkflow_runjob posting a status.
Low
- An exempt author's PR skips the review wait after a pushed fix. fix-ci can push to a release-App PR, which is exempt from the review requirement by author id. Agent-written commits can then merge before their review finishes. Fix: exempt only when every commit is the exempt App's own, or keep exempt-author PRs out of fix-ci.
- The hold label name can diverge. ci-status reads the human-gated label from the base
.work-item-tracker.json, while both callers hardcodehold-label: needs-human. Renaming the tracker label would let review holds stop holding. Fix: fail ci-status on the literalneeds-humanas well, or feed the callers the tracker value. - The rerun listener is unproven live.
rerun-ci-statusruns from main, so feat(pr-pipeline): turn on review-lane holds and require both review lanes in ci-status #9361's own CI could not exercise it. Confirm it on the first PR after merge.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.Human-in-the-loop required; autonomous sessions must not resolve items carrying this.status: needs-decisionAwaiting a human or maintainer judgment call.Awaiting a human or maintainer judgment call.
Type
Fields
No fields configured for Task.
Activity
github-actions commentedon Oct 23, 2026
Proposed work class: none
Thanks for the report. Automated triage handed this issue to a maintainer.
Unconfirmed model suggestion: work-class: structural
Basis: automated intake triage; a maintainer confirms or changes it.
Automated triage. A maintainer reviews it.