fix(sweep): paginate issue events and honor unlabeled when closing expired issues#75541
Open
fcarvajalbrown wants to merge 1 commit into
Open
Conversation
…pired issues closeExpired() fetched only page 1 of the issue events endpoint (oldest-first, 100 per page) and used .pop() on "labeled" events. Two failures resulted: - On busy issues the lifecycle label is applied late, so it lands on a later events page that was never fetched. labeledAt was undefined and the issue was skipped forever, so the highest-traffic issues never auto-closed. - The code ignored "unlabeled" events. A label removed and later re-applied was closed against its original application time, a premature close even though the grace period had restarted. Fetch every event page and reconstruct the label's current state from the full labeled/unlabeled stream, sorted by created_at rather than trusting the API's undocumented order. The reasoning lives in a pure helper (label-events.ts) with unit tests covering the page-2, re-apply, and removed cases. Also fix auto-close-duplicates: add the "duplicate" label through the labels endpoint instead of the issue PATCH, whose labels field replaces the entire set and wiped any pre-existing labels.
|
oh wow you are so brown |
This was referenced Jul 9, 2026
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
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.
Problem
closeExpired()inscripts/sweep.tsdecides whether to auto-close an issue by finding when its lifecycle label (invalid/needs-repro/needs-info/stale/autoclose) was applied:Two correctness bugs, both on high-visibility issues:
labeledAtisundefined, the issue is skipped, and its event count only grows, so it is skipped forever. The busiest issues never auto-close.unlabeled. The code only inspectslabeledevents. If a label was applied early, removed, then re-applied recently, it closes the issue against the old application time even though the grace period restarted, a premature close with a "Closing for now" comment andstate_reason: "not_planned"on an active thread.The other issue-list fetches in this file already loop pages; only the per-issue events fetch did not.
Fix
fetchLabelEvents()walks every event page (natural end, plus a 10-page cap consistent with the existing loops).scripts/label-events.tsreconstructs the label's current state from the full labeled/unlabeled stream, sorted bycreated_atrather than trusting the endpoint's undocumented order, and returns the most recent still-active application time (ornullif the label is no longer applied).closeExpired()usesshouldClose(); downstream behavior (comments-since window, dry-run age) is unchanged.Tests
scripts/label-events.test.ts(Bun), 5 passing:The old logic was confirmed failing on the same inputs (page-2 case returned "don't close"; removed-label case returned "close"), and passing after.
Also
scripts/auto-close-duplicates.tsset the duplicate label through the issuePATCH, whoselabelsfield replaces the entire set and therefore wiped any pre-existing labels (bug,area:*,priority, ...). It now adds the label through the labels endpoint, which merges.