Skip to content

pr-pipeline: review-lane hold follow-ups from the #9361 security review #9369

Open

Description

@kyle-sexton

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

  1. 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's rerun-ci-status, take the newest pull_request run 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 a run-name carrying the PR number, and filter on display_title. Alternatively, also require this PR's own claude-lane-review-complete marker for the head.
  2. The kill switch's pass can fail open and outlive the switch. A lane passes when its Report the kill switch step ran, and that step also runs when check-kill-switch fails closed on an unreadable variable. A head that passed while the switch was on stays green after it clears. pr-merge.yml does not read CLAUDE_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.
  3. The gate runs from the PR's own copy of pr-require-checks.yml.
    • hold-paths does not list the gate's own files: pr-require-checks.yml, both review callers, pr-feedback-dispatch.yml and pr-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 a workflow_run job posting a status.

Low

  1. 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.
  2. The hold label name can diverge. ci-status reads the human-gated label from the base .work-item-tracker.json, while both callers hardcode hold-label: needs-human. Renaming the tracker label would let review holds stop holding. Fix: fail ci-status on the literal needs-human as well, or feed the callers the tracker value.
  3. The rerun listener is unproven live. rerun-ci-status runs 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.

Activity

  1. added
    needs-triageNot yet classified. Floor until a type and one priority tier are set.
    on Oct 23, 2026
  2. github-actions commented on Oct 23, 2026

    @github-actions
    Contributor

    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.

  3. added
    needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.
    and removed
    needs-triageNot yet classified. Floor until a type and one priority tier are set.
    on Oct 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.status: needs-decisionAwaiting a human or maintainer judgment call.

    Type

    Fields

    No fields configured for Task.

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions