Repository navigation
✨ feat: mark lanes unauthorized when the App lacks rights for the next step - #14595
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
…t step A GitHub App API call the contributor lane needs to advance that fails with 403 Resource not accessible by integration now routes through markUnauthorized: label hive:unauthorized, one comment per (head, step) naming the missing permission and the human step, reason unauthorized:<step> parking the PR at 085, one INFO log per head, and clearUnauthorized in the tick the step is no longer needed. First call site: held-run approval without actions: write. Dashboard draws a muted unauthorized pill and reads Awaiting human: approve workflow runs. Fixes #14589 Signed-off-by: Andy Anderson <andy@clubanderson.com>
a7b2643 to
3b9cd69
Compare
|
@clubanderson is auto-promoted for contributor-lane merging on this PR: the last 5 merged PRs by this author in this hive's repositories have no revert or sentinel-alert evidence. CI, reviewer verdict, diff fences, soak, holds and branch protection still apply. |
|
@clubanderson this PR is held at Failing checks:
Next: the hive does not push to your branch. Push a new head (or reply here); CI and the review swarm re-run automatically and the PR returns to the normal path. |
Open PR overlap checkBase branch: Conflicts are checked with every PR merged onto the current tip of
|
|
Another agent tried to open a PR from |
Signed-off-by: Andy Anderson <andy@clubanderson.com>
There was a problem hiding this comment.
Read at 9792784 against the tree. The core mechanism is sound; one small text/behaviour mismatch worth fixing before merge.
Verified correct
- The new label cannot block its own lane:
labelBlockReason(automerge_sweep.go:1935–1953) only reacts to sentinel /needs-rebase/needs-human/do-not-merge*/ hold labels, sohive:unauthorizedon the PR does not makeapproveHeldRunsForRegistryTrustedbail and thenclearUnauthorizedstrip the label on the next tick. - Restart path:
contributorHeldRunsApprovalAttemptedis in-memory, so after a restart the approve is retried and 403s again, butmarkUnauthorized(automerge_unauthorized.go:118–143) skips the label whenpr.Labelsalready carries it andcommentUnauthorizedreturns on the existing<!-- hive:unauthorized head=… step=… -->marker — no duplicate, as the test asserts. - Clear is tied to the step, not the label:
automerge_contributor_lane.go:1259–1267clears whenever the reason stops beingunauthorized:*(checks appeared, head moved, or green), andclearUnauthorized(:150–172) leaves a mark for a different step alone. isResourceNotAccessiblerequires both 403 and the integration body, so rate-limit / archived 403s are not misclassified.
Finding (low) — automerge_unauthorized.go:211: the posted note ends with "this label and note clear themselves", but nothing in the PR deletes or edits the comment (clearUnauthorized only removes the label; no DeleteComment/EditComment anywhere in the diff). The note is also the idempotency marker, so deleting it would be wrong — the fix is the wording: "this label clears itself" (or say the note stays as a record). labels-and-control-signals.md says "pill, label and payload fields clear themselves", which is accurate; only the comment text over-promises.
Not approving (human signature), but with that one line changed I see nothing else blocking.
— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
merged by Hive trusted-author auto-merge: author @clubanderson holds role owner and GitHub permission admin on repo; CI green at 9792784. |
|
Thank you for your contribution! Your PR has been merged. We'd love to hear how your experience was: share feedback |
Fixes #14589
What
When a GitHub App API call that is a prerequisite for the lane's progress fails with
403 Resource not accessible by integration, the hive now says so plainly instead of warning every tick and silently waiting:markUnauthorized(ctx, repo, pr, head, step, permission, humanAction)/clearUnauthorizedinsrc/pkg/github/automerge/automerge_unauthorized.go: addshive:unauthorized, posts one idempotent comment per (head, step) (<!-- hive:unauthorized head=<sha> step=<step> -->) naming the exact missing permission and the exact human step, logs once per head at INFO (msg="lane unauthorized"), returns reasonunauthorized:<step>.approveHeldRunsForRegistryTrusted) — needsactions: write; the comment says to click Approve and run and lists the run ids and the missing required check. Cleared in the same tick the required checks for the head report (no human label edit).unauthorized:*→ 085 with the reason kept; the 085 entry comment (decisionCause) names the refused step.unauthorizedmodifier (label-driven, likestalled/hold/draft), muted pill (dashed,--text-muted), tooltip from payloadunauthorized, counted under Needs attention; the 085 cell and caption read Awaiting human: approve workflow runs (payloadunauthorized_step).hive:unauthorizedjoins the needs-human label preset.automerge_contributor_adopt.goclose-PR,automerge_sweep.goupdate-branch,automerge_merge_forward.go) are ambiguous (fork editability / rate limit / archived) and carryTODO(hivecommons/hive#14589)comments rather than being wired.actions: writeis optional; without it the hive marksunauthorizedand waits). Changelog fragmentchangelog.d/added-unauthorized-label.md.Tests
Table-driven, no
time.Sleep:TestContributorLaneUnauthorizedWhenApproveForbidden: 403 → label + 1 marker comment + reason set once; second tick same head → no duplicate; checks appear → label removed and state cleared in that tick; restart case (label + marker already present) re-posts nothing.TestIsResourceNotAccessible,TestUnauthorizedCommentAndMarker,TestClassifyStageUnauthorizedParksForMaintainer, dashboard modifier/payload/caption tests and a node-run pill/tooltip/gate-word check.