Skip to content

pr-pipeline: stop ci-status starting red before both review lanes finish #12447

Open

Description

@kyle-sexton

Refs: #12284

Owner: Automation Lanes.

Owner gate. The security review lane is a control. If a design would let a PR merge before both review lanes complete on its head, it comes to the owner before building.

Summary

ci-status requires both review lanes on the head (.github/workflows/pr-require-checks.yml:6755 at cb8429f). Reviews usually finish after ci-status first reads them, so ci-status starts red. pr-feedback-dispatch.yml:575 (rerun-ci-status) then re-runs it.

Measured over 7 days: 790 re-run attempts, 14% of 5,515 PR runs of pr-require-checks. On 2026-11-01: 192 re-runs. Source: audit comment, gate #9.

Both lanes stay. The code-review lane also raises the cost hold (pr-review-hosted.yml:242-250), which is a billing control.

Options:

  • Gate pr-verify or pr-merge's verdict on the reviews, rather than ci-status's first pass.
  • Start ci-status's review read only once both lanes have finished.

Each option must still keep a PR from merging until both lanes complete on its head.

Done when

  • A design that keeps "both lanes reviewed this head" as a merge precondition is posted here. If it would not, the owner decided it first.
  • ci-status re-run attempts per day are measured before and after.
  • docs/conventions/pr-pipeline/README.md "Review-lane holds" describes the new order.

Activity

  1. github-actions commented on Nov 2, 2026

    @github-actions
    Contributor

    Proposed work class: work-class: structural
    Basis: automated intake triage; the lanes App applied this class.

    Size: Medium
    Complexity: Medium
    Uncertainty: Knowable
    Flags: none
    Blocked by: none

    You want ci-status to stop starting red and being re-run, while "both review lanes reviewed this head" stays a merge precondition. A recorded decision exists:

    Decision recorded.

    Build option 2: ci-status starts its review read only once both review lanes have finished on the head, and stays pending (not green, not red) until then, so "both lanes reviewed this head" remains a merge precondition held by the one required check. Option 1 is not built: it moves part of the merge gate out of ci-status. The cost hold the code-review lane raises is unchanged. Basis: this issue's body (its gate applies only to a design that lets a PR merge before both lanes complete); AGENTS.md, "None of their checks is required; ci-status is the only one". Written by the issue-triage session.

    • A design that keeps "both lanes reviewed this head" as a merge precondition is posted here. If it would not, the owner decided it first.
    • ci-status re-run attempts per day are measured before and after.
    • docs/conventions/pr-pipeline/README.md "Review-lane holds" describes the new order.

    #12447 (comment)

    This settles the open design question. Nothing further is asked of the owner.

    Automated triage, applied by the lanes App.

  2. added
    status: needs-decisionAwaiting a human or maintainer judgment call.
    needs-triageNot yet classified. Floor until a type and one priority tier are set.
    on Nov 2, 2026
  3. melodic-automation-lanes commented on Nov 2, 2026

    @melodic-automation-lanes
    Contributor

    The latest triage verdict holds nothing: admission moved the issue to status: confirmed; its comment says why.

  4. added
    work-class: structuralRefactors, migrations, contract changes; cross-cutting and hard to reverse.
    and removed
    needs-triageNot yet classified. Floor until a type and one priority tier are set.
    on Nov 2, 2026
  5. melodic-automation-lanes commented on Nov 2, 2026

    @melodic-automation-lanes
    Contributor

    Refinement. A read-only run explored this repository, searched its issues and pull requests, and read the allowed docs to draft what follows. On an issue a listed human filed, it counts as the issue's spec once posted, while unedited; it is still model text, so check it.

    Intended outcome: Remove the red-start and re-run churn of ci-status by deferring its review read until both review lanes finish on the head, while keeping "both lanes reviewed this head" as a merge precondition.

    Related work

    Open questions

    1. Which design should be built: gate the pr-verify or pr-merge verdict on the reviews, or start ci-status's review read only once both lanes have finished?
      RECOMMENDED: Start ci-status's review read only after both lanes finish, keeping ci-status as the single merge gate. Basis: judgment
    2. The issue's owner gate says any design that could let a PR merge before both lanes complete on its head goes to the owner before building. Does the owner approve the chosen design, and is a pending review state (not red, not green) acceptable for ci-status while a lane is unfinished?
      RECOMMENDED: Approve only if ci-status stays a required check that is not green until both lanes have finished on the current head SHA. Basis: judgment
    3. What is the measurable target for re-run attempts per day, and how is it measured before and after (the audit source is issue 12284's comment)?
      RECOMMENDED: Count rerun-ci-status re-run attempts per day from the pr-feedback-dispatch records over 7 days before and after, with a target near zero. Baseline: 790 per 7 days and 192 on 2026-11-01. Basis: judgment
    4. Is the done-when item to measure re-runs before and after something an unattended run can do, or does it need a person after deploy?
      RECOMMENDED: Split it out as a post-deploy check a person runs, since the after-measurement needs days of production data. Basis: judgment

    Sources read

    • /home/runner/work/claude-code-plugins/claude-code-plugins/.base/.github/workflows/pr-feedback-dispatch.yml:575: the rerun-ci-status job exists and re-runs the head's ci-status once the lanes finish
    • /home/runner/work/claude-code-plugins/claude-code-plugins/.base/docs/conventions/pr-pipeline/README.md:2784: the 'Review-lane holds' section that the issue says must describe the new order

    To answer, post a comment that starts with this line:

    **Decision recorded.** <answer>
    

    Automated refinement by the lanes App. Answer with a comment whose first line is **Decision recorded.**.

  6. added
    host-runNeeds an unattended run on a host; the host runner takes it, not a person or the issue lane.
    and removed
    status: needs-decisionAwaiting a human or maintainer judgment call.
    on Nov 3, 2026
  7. kyle-sexton commented on Nov 3, 2026

    @kyle-sexton
    MemberAuthor

    Decision recorded.

    Build option 2: ci-status starts its review read only once both review lanes have finished on the head, and stays pending (not green, not red) until then, so "both lanes reviewed this head" remains a merge precondition held by the one required check. Option 1 is not built: it moves part of the merge gate out of ci-status. The cost hold the code-review lane raises is unchanged. Basis: this issue's body (its gate applies only to a design that lets a PR merge before both lanes complete); AGENTS.md, "None of their checks is required; ci-status is the only one". Written by the issue-triage session.

    • A design that keeps "both lanes reviewed this head" as a merge precondition is posted here. If it would not, the owner decided it first.
    • ci-status re-run attempts per day are measured before and after.
    • docs/conventions/pr-pipeline/README.md "Review-lane holds" describes the new order.
  8. kyle-sexton commented on Nov 3, 2026

    @kyle-sexton
    MemberAuthor

    Agent choice, not an owner answer. The decision record above was written by the issue-triage session under the owner's standing direction §1 (decide from evidence). The owner did not answer this question; the comment appears under the owner's login only because the session posts with that account.

  9. github-actions commented on Nov 3, 2026

    @github-actions
    Contributor

    The triage sweep dispatched intake triage for this issue (attempt 1 of 2).

  10. added
    status: confirmedA maintainer confirmed the item is real and in scope; ready to be worked.
    and removed
    status: readyTriaged, unblocked, and fully specified; eligible to pick up.
    on Nov 3, 2026
  11. melodic-automation-lanes commented on Nov 3, 2026

    @melodic-automation-lanes
    Contributor

    Not admitted to the issue lane: it carries host-run. Admission takes the issue once each of those is met, when intake triages it again (.github/actions/resolve-config/admission-rules.json).

  12. kyle-sexton commented on Nov 3, 2026

    @kyle-sexton
    MemberAuthor

    Measured: #12797's rerun-ci-status works, but lands 2 to 6 minutes after the last review, and its red message invites a manual re-run.

    On two heads on 2026-11-03 the job ran for both lanes and took the right branch each time. The run for the lane that finished last got a hosted runner 2 to 6 minutes after its review completed:

    Both times a session re-ran ci-status by hand about a minute after the reviews finished, because the red message says "ci-status re-runs when the review finishes". The automatic run then correctly exited with "it read them", so each manual re-run cost an attempt and saved nothing. 323 pr-feedback-dispatch runs were created between 16:25 and 17:15Z; runner contention as the cause of the wait is unverified.

    What this adds to this issue's scope:

    • Until the redesign lands, the "has not finished" text in pr-require-checks.yml should say the re-run comes from pr-feedback-dispatch within a few minutes and that no manual re-run is needed.
    • Measure last-review-completion to rerun-ci-status start over a day (median and p90) as the baseline the redesign is judged against.

    Posted by the overnight session. This was generated by AI.

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

    host-runNeeds an unattended run on a host; the host runner takes it, not a person or the issue lane.priority: mediumReal value, no hard deadline; normal backlog flow.status: confirmedA maintainer confirmed the item is real and in scope; ready to be worked.work-class: structuralRefactors, migrations, contract changes; cross-cutting and hard to reverse.

    Type

    Fields

    Work status

    confirmed

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions