ci-status's person hold on a hold-path PR keeps pr-verify from judging the head before approval #10699

Activity

1. github-actions

   added

   needs-triageNot yet classified. Floor until a type and one priority tier are set.Not yet classified. Floor until a type and one priority tier are set.

   on Oct 28, 2026
2. github-actions commented on Oct 28, 2026

   

   github-actionsBot

   on Oct 28, 2026 – with GitHub Actions

   Contributor

   More actions

   Proposed work class: none

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

   I understood this as: on a hold-path PR, ci-status fails on the person-hold reason alone, so pr-verify never runs. The trusted person then approves a head that pr-verify hasn't judged, and earlier fail-notes stay open. You want pr-verify to judge first and the approval to come last. The hold must not weaken, and pr-verify must still skip heads with real defects. No duplicate found among open issues.

   The issue lists three options and picks none, so I need the owner's call:

   1. Which option should be built: (1) a separate required hold status, (2) pr-verify also triggering on a hold-only ci-status failure, or (3) keep the current order?
      RECOMMENDED: option 2. It keeps ci-status as the single gate and leaves the hold check unchanged. Option 1 adds a new required status and changes the merge rules, which touches the R2 boundary more.

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

   **Decision recorded.** <your answer>
   

   _{Automated triage. A maintainer reviews it.}
3. github-actions

   added

   priority: mediumReal value, no hard deadline; normal backlog flow.Real value, no hard deadline; normal backlog flow.

   status: needs-decisionAwaiting a human or maintainer judgment call.Awaiting a human or maintainer judgment call.

   on Oct 28, 2026
4. github-actions

   added the

   Task

   issue type on Oct 28, 2026
5. melodic-automation-lanes commented on Oct 28, 2026

   

   melodic-automation-lanesBot

   on Oct 28, 2026

   Contributor

   More actions

   The triage verdict left this issue at status: needs-decision:

   • flagged: intent
   • held by status: needs-decision
   • no lanes App verdict comment shows whether an earlier verdict was flagged
6. melodic-automation-lanes

   removed

   needs-triageNot yet classified. Floor until a type and one priority tier are set.Not yet classified. Floor until a type and one priority tier are set.

   on Oct 28, 2026
7. melodic-automation-lanes commented on Oct 28, 2026

   

   melodic-automation-lanesBot

   on Oct 28, 2026

   Contributor

   More actions

   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: pr-verify judges a hold-path PR head before the trusted person approves it, with ci-status still failing without approval and still gating pr-verify on real defects.

   Related work

   • fix(pr-merge): arm a hold-path pull request a trusted person approved at its head #10397 (related): merged-closed work on arming a hold-path PR a trusted person approved at its head; touches the same approval lift
   • fix(pr-merge): carry a hold-path lift over a later base merge #10596 (related): carries a hold-path lift over a later base merge; same hold-path approval logic
   • feat(pr-pipeline): hold lanes App human-merge pull requests with a label only a listed human can remove #9437 (related): person-only hold label for lanes App PRs, the R2 boundary this issue must not weaken
   • fix(pr-pipeline): align every needs-human path with the owner stop rule #10517 (related): aligns needs-human paths with the owner stop rule; overlaps on hold handling
   • fix(source-control): hold pr-verify-failed pull requests in pr-merge #10676 (related): pr-merge holds pr-verify-failed PRs; interacts with when pr-verify runs

   Open questions

   1. Which option should be built: (1) a separate required hold status, (2) pr-verify also triggering on a hold-only ci-status failure, or (3) keep the current order?
      RECOMMENDED: Option 2, with the hold-only reason read from a machine-readable marker in ci-status output or run data, and pr-verify still skipping any other failure. Basis: .github/workflows/pr-verify.yml:74 gates on conclusion == 'success'; option 2 leaves ci-status as the one gate and does not add a required status, so branch-protection rules stay unchanged.
   2. If option 2, how should pr-verify know ci-status failed only for the person-hold reason, given that a workflow_run trigger sees only the conclusion and not the reason?
      RECOMMENDED: Have ci-status upload a small artifact or write a job output listing failure reasons, and have pr-verify's gate job fetch it from the upstream run and proceed only when the sole reason is the hold. It must fail closed if the marker is missing or malformed. Basis: judgment
   3. Should a pr-verify pass on a hold-only head have any effect on the hold, or must the lift still come only from a trusted person's approving review of that head?
      RECOMMENDED: No effect. Only the trusted person's approval lifts the hold, so R2 is unchanged, and pr-verify output must never count as approval. Basis: judgment
   4. Is it acceptable that pr-verify's Claude-driven steps run on a head that has not yet been approved by a person, for hold-path PRs from lanes App authors?
      RECOMMENDED: Yes only if pr-verify runs from the default branch with no write power beyond fail-notes and its current permissions. The owner should confirm this against the R2 threat model. Basis: judgment

   Sources read

   • /home/runner/work/claude-code-plugins/claude-code-plugins/.base/.github/workflows/pr-verify.yml:44: workflow_run trigger runs from the default branch, never a PR's copy
   • /home/runner/work/claude-code-plugins/claude-code-plugins/.base/.github/workflows/pr-verify.yml:74: job runs only when the upstream workflow_run conclusion is success; lines 420 and 776 repeat it for the other jobs

   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.**.}
8. kyle-sexton

   closed this as completedin #10736on Oct 28, 2026
9. kyle-sexton commented on Oct 29, 2026

   

   kyle-sexton

   on Oct 29, 2026

   MemberAuthor

   More actions

   Re-triage. No owner stop applies. The person hold stays exactly as it is. This changes only when pr-verify judges a head, not what lifts the hold. The open questions are decided below on origin/main 1ac46f037a.

   1. Which option?
      Decided: option 2. pr-verify also runs when ci-status failed for the person-hold reason alone. Any other ci-status failure still skips it.

      • Option 1 adds a required status and changes merge rules on the R2 boundary.
      • Option 3 keeps the defect, where a person approves a head pr-verify has not judged.

      Basis: .github/workflows/pr-verify.yml:74 gates every job on conclusion == 'success'.

   2. How does pr-verify know the failure was hold-only?
      Decided: ci-status records its failure reasons in machine-readable form (a job output or a small artifact on its run). pr-verify's gate reads it from the upstream run and proceeds only when the sole reason is the hold path. It fails closed, skipping pr-verify, when the record is missing, unreadable or lists any other reason. The hold reason's text is pinned at scripts/ci-lanes-pr-hold.test.sh:458. The build finds the step that emits it and changes it in that step's own repository.

   3. Does a pr-verify pass touch the hold?
      Decided: no. Only an approving review of the head by a person on the trusted-actor list lifts it, and pr-verify output never counts as approval.

   4. Is it safe for pr-verify to run on an unapproved hold-path head?
      Decided: yes. pr-verify runs from the default branch, never the PR's copy (pr-verify.yml:44), checks out refs/heads/main (:91), and each of its three jobs holds only contents: read, pull-requests: read and actions: read (:78-81, :427-430, :782-785). The review lanes already run Claude on every hold-path head before approval (pr-review-hosted.yml:31, :198-200). So pr-verify adds no new class of exposure.

   Builder: the Automation Lanes session builds this from the owner's machine, because the fix edits .github/workflows/pr-verify.yml, which the lanes App cannot push (.github/actions/resolve-config/patch-rules.json:3). agent-ready stays off.

   Acceptance criteria:

   • On a hold-path PR whose only ci-status failure is the hold-path reason, pr-verify runs on that head and posts its verdict before the approval.
   • On a head where ci-status fails for any other reason, alone or with the hold, pr-verify does not run. A test covers that case and the missing or malformed reason record.
   • ci-status still fails on a hold-path PR until a trusted person approves the head, and a pr-verify pass does not change that. A test covers it.
   • The person-hold path in docs/conventions/pr-pipeline/README.md states the new order: pr-verify judges first, then the person approves.
   • actionlint passes on the changed workflows, and ci-status is green on the PR head.
   • A security-review pass ran at the PR head.
10. kyle-sexton

    added

    status: confirmedA maintainer confirmed the item is real and in scope; ready to be worked.A maintainer confirmed the item is real and in scope; ready to be worked.

    and removed

    status: needs-decisionAwaiting a human or maintainer judgment call.Awaiting a human or maintainer judgment call.

    on Oct 29, 2026