Skip to content

pr-pipeline: resolve stale pr-verify fail-note threads when a later head passes #10113

Closed

Description

@kyle-sexton

Problem

pr-verify fail-note review threads (<!-- pr-verify:fail-note -->, .github/scripts/pr-unblock/address-feedback.mjs:60) stay open after a later head gets a pass verdict. The main ruleset sets required_review_thread_resolution: true, so GitHub keeps the PR BLOCKED with auto-merge armed and nothing says why.

Seen on #9890: four fail-note threads from head 0b7df5d (2026-10-24 23:33Z); heads 9750096 and 57dbf61 then got pass-notes; pr-merge armed auto-merge at 19:48Z; the PR sat BLOCKED until the four threads were resolved by hand on 2026-10-25.

Fix

When pr-verify writes a pass or pass-notes verdict, resolve the open fail-note threads it posted for earlier heads of that PR (in the job that already posts fail notes).

Done when

A PR whose newest head passes pr-verify carries no open fail-note thread from an older head.

🤖 Generated with Claude Code

Activity

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

    @github-actions
    Contributor

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

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

    Understood: when pr-verify gives a pass or pass-notes verdict for a PR's current head, the post-verify-notes job should resolve that PR's open fail-note threads from older heads. Today those threads stay open and keep the PR blocked under the thread-resolution ruleset, as on #9890. The refinement comment's acceptance criteria cover the scope: the workflow download step, verify-notes.mjs, the threads query, and tests. Nothing is missing. No likely duplicates among open issues.

    Automated triage, applied by the lanes App.

  3. added
    priority: highSignificant impact, or blocks an imminent release; staff this cycle.
    and removed
    needs-triageNot yet classified. Floor until a type and one priority tier are set.
    on Oct 25, 2026
  4. github-actions commented on Oct 26, 2026

    @github-actions
    Contributor

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

  5. added
    status: readyTriaged, unblocked, and fully specified; eligible to pick up.
    work-class: scopedA briefed fix or small feature; blast radius bounded by the brief, tests exist.
    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 Oct 26, 2026
  6. melodic-automation-lanes commented on Oct 26, 2026

    @melodic-automation-lanes
    Contributor

    Not admitted to the issue lane: no trusted-human acceptance criteria: a listed human must put the confirmed - [ ] criteria in the body or post them as their own comment, or, on an issue a listed human filed, intake's newest refinement comment must hold them unedited. Admission takes the issue once each of those is met, when intake triages it again (.github/actions/resolve-config/admission-rules.json).

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

    @melodic-automation-lanes
    Contributor

    The latest triage verdict holds nothing: the issue is at status: ready.

  8. added
    needs-triageNot yet classified. Floor until a type and one priority tier are set.
    and removed
    status: confirmedA maintainer confirmed the item is real and in scope; ready to be worked.
    on Oct 26, 2026
  9. melodic-automation-lanes commented on Oct 26, 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: When a pr-verify run uploads a pass or pass-notes verdict for a PR's current head, the post-verify-notes job resolves that PR's open fail-note threads from earlier heads.

    Acceptance criteria

    • pr-feedback-dispatch.yml's post-verify-notes job downloads a pass or pass-notes verdict artifact (names pr-verify-<pr>-<sha>-pass or -pass-notes, same optional digest and -i suffix grammar), not only -fail, and still fails the run on a verdict-shaped name no reader takes (.github/workflows/pr-feedback-dispatch.yml:131-141)
    • verify-notes.mjs reads a pass or pass-notes verdict with the same checks as readVerdict (pr, head_sha, head_ref digest and linked issues match the name) and rejects any other verdict value (.github/scripts/pr-unblock/verify-notes.mjs:106-125)
    • For a pass or pass-notes verdict, the script skips unless the PR is open and its current head is the verdict's head, as postNotes does for fail (verify-notes.mjs:130-133)
    • For such a verdict it lists the PR's review threads and resolves each unresolved thread whose first comment is by MARKER_AUTHOR_ID, has first line VERIFY_NOTE_MARKER, and whose commit is not the verdict's head, using api.listThreads and api.resolveThread from address-feedback.mjs:733 and :775
    • It never resolves a thread whose first comment is by another author or lacks the marker, one already resolved, or one posted for the verdict's own head
    • The threads query returns what the filter needs (first comment's author id, body and commit sha), added to address-feedback.mjs THREADS_QUERY (:611) without breaking its existing callers' tests
    • A pass or pass-notes run does not write the pr-number and head-sha outputs, so the dispatch job gate at pr-feedback-dispatch.yml:174 is unchanged for passes
    • Resolving is idempotent: a rerun with no matching open threads resolves none and exits 0, and a failed resolve of one thread does not stop the others from being tried
    • The job's permissions stay contents: read, actions: read, pull-requests: write (.github/workflows/pr-feedback-dispatch.yml:84-87), and the header comment of pr-feedback-dispatch.yml and verify-notes.mjs describe the new behavior
    • New cases in verify-notes.test.mjs cover: older-head fail-note threads resolved on pass, on pass-notes, same-head thread left, non-marker and other-author threads left, stale head skipped; and the node test files under .github/scripts/pr-unblock pass
    • workflow-gates.test.mjs and log-api-budget.test.mjs, which read the job's download step text (log-api-budget.test.mjs:735), are updated and pass

    Related work

    Sources read

    • /home/runner/work/claude-code-plugins/claude-code-plugins/.base/.github/scripts/pr-unblock/verify-notes.mjs:1-180: posts fail-note review comments only; readVerdict rejects any verdict but fail; no pass handling or thread resolving
    • /home/runner/work/claude-code-plugins/claude-code-plugins/.base/.github/workflows/pr-feedback-dispatch.yml:73-153: post-verify-notes job lists and downloads only -fail artifacts, with pull-requests: write
    • /home/runner/work/claude-code-plugins/claude-code-plugins/.base/.github/scripts/pr-unblock/address-feedback.mjs:53-60: MARKER_AUTHOR_ID and VERIFY_NOTE_MARKER
    • /home/runner/work/claude-code-plugins/claude-code-plugins/.base/.github/scripts/pr-unblock/address-feedback.mjs:611-626: THREADS_QUERY returns no author, body or commit of a thread's first comment
    • /home/runner/work/claude-code-plugins/claude-code-plugins/.base/.github/scripts/pr-unblock/address-feedback.mjs:733-776: listThreads and resolveThread already exist

    Automated refinement by the lanes App. Intake triages the issue again with it.

  10. 1 remaining item

  11. added
    agent-readyFully specified and briefed; eligible for autonomous pickup from the frontier.
    needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.
    and removed
    agent-readyFully specified and briefed; eligible for autonomous pickup from the frontier.
    on Oct 27, 2026
  12. melodic-automation-lanes commented on Oct 27, 2026

    @melodic-automation-lanes
    Contributor

    The work-resolve-issue lane stopped partway through this issue and needs a person.

    Why, in the run's own words:

    The run stopped early (patch-protected-path): its change needs a path the issue lane may not push, which a person changes instead. In the run's words: patch-protected-path: .github/workflows/pr-feedback-dispatch.yml. The post-verify-notes job's 'Download the fail verdict' step (lines 112-144) lists and downloads only pr-verify-<pr>-<sha>-fail artifacts and exits early with 'uploaded no fail verdict; nothing to post' otherwise, so a pass or pass-notes verdict never reaches verify-notes.mjs. Resolving stale fail-note threads when a later head passes, in the job that already posts fail notes (as the issue asks), needs this workflow step widened to take -pass and -pass-notes names, along with its header comment and the permissions comment at line 86. The lane cannot push a change under /.github/workflows/**, so no edit was made.

    What the run says it did not do:

    • The whole fix: the pr-feedback-dispatch.yml download step (protected path), verify-notes.mjs pass/pass-notes handling and thread resolving, the extra THREADS_QUERY fields in address-feedback.mjs, and the new verify-notes.test.mjs, workflow-gates.test.mjs and log-api-budget.test.mjs cases
    • Repository checks check-changelog-fragments and check-affected were not run because the working tree has no changes

    Run: https://github.com/melodic-software/claude-code-plugins/actions/runs/40452332749

    To retry, remove needs-human and add agent-ready again.

  13. kyle-sexton commented on Oct 27, 2026

    @kyle-sexton
    MemberAuthor

    Builder routing. The fix widens the post-verify-notes download step in .github/workflows/pr-feedback-dispatch.yml to take the -pass and -pass-notes verdicts. The issue lane cannot push that (the run stopped on patch-protected-path). Automation Lanes builds it. The criteria are complete and nothing waits on the owner, so this moves from needs-human to status: confirmed.

  14. 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.
    needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.
    on Oct 27, 2026
  15. kyle-sexton commented on Oct 27, 2026

    @kyle-sexton
    MemberAuthor

    Decided on the facts: criteria 4, 7 and 9 amended. As written, criterion 4 (post-verify-notes resolves threads on its GITHUB_TOKEN) conflicts with criterion 9 (that job stays contents: read, pull-requests: write). resolveReviewThread returns FORBIDDEN for an installation token without contents: write, and a GitHub collaborator calls that intended: "resolving a thread should align with merge-level permissions (repo write access)" (github/gh-aw#35726). Whether GITHUB_TOKEN behaves the same is unverified. Granting contents: write would widen a workflow permission, so the resolve moves to the token that already resolves threads: the lanes App in pr-unblock's publish-feedback job (.github/workflows/pr-unblock.yml:1354, id-token: write; .github/scripts/pr-unblock/publish-feedback.mjs:429 calls api.resolveThread). No permission widens anywhere.

    Criteria 1-3, 5, 6, 8, 10 and 11 of the refinement stand. Criteria 4, 7 and 9 now read:

    • (4) For a pass or pass-notes verdict on the PR's current head, the unresolved threads whose first comment is by MARKER_AUTHOR_ID, has first line VERIFY_NOTE_MARKER, and whose commit is not the verdict's head are resolved through the lanes App token in pr-unblock's publish-feedback path, using api.listThreads and api.resolveThread from address-feedback.mjs. post-verify-notes does not resolve them itself.
    • (7) A pass or pass-notes verdict reaches that path only when at least one such stale fail-note thread is open, so a pass with none starts no pr-unblock run, and the fail path's dispatch gate (pr-feedback-dispatch.yml:174) keeps its behavior for fails.
    • (9) post-verify-notes keeps contents: read, actions: read, pull-requests: write (pr-feedback-dispatch.yml:84-87), no job in either workflow gains contents: write, and the header comments of pr-feedback-dispatch.yml, verify-notes.mjs and publish-feedback.mjs describe the new behavior.

    Built by Automation Lanes (workflow edit; the issue lane cannot push .github/workflows/**).

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

    priority: highSignificant impact, or blocks an imminent release; staff this cycle.status: confirmedA maintainer confirmed the item is real and in scope; ready to be worked.work-class: scopedA briefed fix or small feature; blast radius bounded by the brief, tests exist.

    Type

    Fields

    No fields configured for Bug.

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions