feat(queue): wake linked PRs promptly on an issue-side label/assignment change - #2371
Merged
Conversation
…nt change processGitHubWebhook had no eventName === "issues" branch, so labeling/unlabeling maintainer-only (or another point-bearing label) on a linked ISSUE, or assigning/unassigning the repo owner on it, never re-triggered the linked-issue hard-rule re-evaluation for PRs that link it. That check only ran when the PR itself received a webhook, or via the staleness-ordered sweep, which caps at 3 PRs per repo per ~2-minute tick with no priority signal for "a linked issue just changed" — on a busy repo this can lag for many cycles, letting a should-now-be-closeable PR auto-merge, or a should-now-be-mergeable PR stay wrongly held. Add maybeReReviewOnLinkedIssueChange, mirroring the existing CI-completion re-review handler (maybeReReviewOnCiCompletion): on a labeled/unlabeled/assigned/unassigned issues event, find every OPEN PR that links the issue and re-review it promptly. Reuses the same per-PR coalesce window CI-completion re-review already uses, and the same GITTENSORY_REVIEW_REPOS convergence allowlist gate, so this activates on exactly the same repo footprint as the analogous existing mechanism.
Contributor
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ Gittensory review result - manual review recommendedReview updated: 2026-07-01 22:41:51 UTC
⏸️ Suggested Action - Manual Review
Review summary Nits — 7 non-blocking
Review context
Contributor next steps
Signal definitions
🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2371 +/- ##
=======================================
Coverage 95.85% 95.86%
=======================================
Files 224 224
Lines 25104 25134 +30
Branches 9129 9137 +8
=======================================
+ Hits 24064 24094 +30
Misses 427 427
Partials 613 613
🚀 New features to boost your workflow:
|
loopover-orb
Bot
added
gittensor
gittensor:feature
maybeReReviewOnLinkedIssueChange reused ciReReviewCoalesced's shared
`ci-coalesce:{repo}#{pr}` key, but a CI-completion webhook and an
issue-side label/assignment change answer different questions -- unlike
concurrent CI-completion events (interchangeable: whichever wins the
coalesce race re-fetches the same already-settled CI state), a completely
unrelated CI re-review claiming the shared window could silently suppress
a genuine issue-side signal, leaving the linked-issue verdict stale until
the window expired or the sweep eventually reached the PR.
Add a dedicated issueLinkedPrReReviewCoalesced using its own
`issue-link-coalesce:` key namespace. Within that namespace the window
still bounds frequency for a burst of same-PR issue-side churn -- its
legitimate purpose, matching the CI window's own philosophy -- but it can
no longer be stolen by (or steal from) an unrelated CI-completion event.
Rewrite the test that asserted the CI window suppressing the issue-side
wake as correct behavior into a regression test asserting the opposite,
and add a dedicated test proving same-domain (issue-side) bursts still
coalesce correctly.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
gittensory-ui | 15c00a5 | Commit Preview URL Branch Preview URL |
Jul 01 2026, 09:06 PM |
…ents The prior fix gave issue-side wakes their own coalesce window (separate from CI-completion's), but same-PR issue-side events are ALSO not interchangeable with each other: an add-then-remove label or assign-then-unassign sequence within the 60s window carries genuinely different states. The plain throttle (ciCompletionCoalesced) silently drops every event after the first, so the second event's state was lost entirely -- the PR stayed on the FIRST (now-stale) state until another webhook or the sweep eventually reached it, defeating the "wake promptly" purpose this whole trigger exists for. Add scheduleTrailingIssueLinkedReReview: when an issue-side event is coalesced, schedule exactly one deduped trailing agent-regate-pr job (delaySeconds: 60, the window's length) so the LATEST state is always eventually captured shortly after the window closes. Reuses the existing rate-limit-aware, retried sweep-unit job rather than inventing a new job type; deduped via its own claim key so a burst of N coalesced events schedules ONE trailing job, not N. A failed enqueue is swallowed (best-effort -- the sweep remains the ultimate backstop). Add regression tests: an add-then-remove sequence schedules exactly one correctly-shaped trailing job, a third coalesced event does not schedule a second, and a failed enqueue never propagates into the webhook handler.
…e succeeds scheduleTrailingIssueLinkedReReview used ciCompletionCoalesced, which writes its dedup marker unconditionally as soon as it is called -- BEFORE the subsequent env.JOBS.send even runs. A transient queue failure would therefore leave the "trailing re-review scheduled" marker held with nothing actually queued: every later coalesced issue-side event for the same PR within the window would see the marker already claimed and skip retrying, permanently forfeiting the guarantee this function exists to provide (capturing the latest linked-issue state after a burst). Reorder to check-then-send-then-claim: read the marker first (read-only, via getTransientKey), attempt the enqueue, and write the marker (via putTransientKey) ONLY when the send actually succeeds. A failed attempt leaves the marker unclaimed, so the next coalesced event in the same window retries the enqueue instead of silently giving up. Add a regression test simulating a transient failure (first send throws, second succeeds) that proves the retry works and that a fourth coalesced event correctly dedupes against the now-successful claim.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
processGitHubWebhookhad noeventName === "issues"branch (confirmed absent despite the event being subscribed and delivered), so labeling/unlabelingmaintainer-only(or another point-bearing label) on a linked ISSUE, or assigning/unassigning the repo owner on it, never re-triggered the linked-issue hard-rule re-evaluation for PRs that link it. That check only ran when the PR itself received a webhook, or via the periodic sweep — which caps atSWEEP_MAX_PRS = 3per repo per ~2-minute tick with no priority signal for "a linked issue's label just changed."Failure scenario: a maintainer adds
maintainer-onlyto an issue after a contributor already opened a linking PR (or removes it to unblock a wrongly-flagged PR). The PR keeps whatever verdict it had until either a new push happens on it or the staleness-ordered sweep eventually reaches it — on a repo with more than a few open PRs, this can lag for many sweep cycles. A should-now-be-closeable PR can still auto-merge in the meantime, or a should-now-be-mergeable PR stays wrongly held/closed.Fix
Add
maybeReReviewOnLinkedIssueChange, mirroring the existing CI-completion re-review handler (maybeReReviewOnCiCompletion) that already solves the structurally identical problem for a different trigger: on alabeled/unlabeled/assigned/unassignedissuesevent, find every OPEN PR that links the issue (pr.linkedIssues.includes(issueNumber)) and re-review it promptly via the existingreReviewStoredPullRequest.ciReReviewCoalesced) — its purpose applies identically here: bound re-review frequency, never correctness, since the re-review always re-fetches live state regardless of trigger.GITTENSORY_REVIEW_REPOSconvergence allowlist the CI-completion handler uses, so this activates on exactly the same repo footprint as the analogous existing mechanism — no new rollout surface.processGitHubWebhookalongside the other non-PR wake triggers (CI-completion, deployment_status), since anissuesevent carries nopayload.pull_requesteither.Tests
Full unsharded
test:coveragegreen (5605 passed);typecheckgreen;npm auditclean.Advances #1936. Closes #2259.