Skip to content

fix(sweep): paginate issue events and honor unlabeled when closing expired issues#75541

Open
fcarvajalbrown wants to merge 1 commit into
anthropics:mainfrom
fcarvajalbrown:fix/sweep-close-expired-label-events
Open

fix(sweep): paginate issue events and honor unlabeled when closing expired issues#75541
fcarvajalbrown wants to merge 1 commit into
anthropics:mainfrom
fcarvajalbrown:fix/sweep-close-expired-label-events

Conversation

@fcarvajalbrown

Copy link
Copy Markdown

Problem

closeExpired() in scripts/sweep.ts decides whether to auto-close an issue by finding when its lifecycle label (invalid / needs-repro / needs-info / stale / autoclose) was applied:

const events = await githubRequest<any[]>(`${base}/events?per_page=100`);
const labeledAt = events
  .filter((e) => e.event === "labeled" && e.label?.name === label)
  .map((e) => new Date(e.created_at))
  .pop();
if (!labeledAt || labeledAt > cutoff) continue;

Two correctness bugs, both on high-visibility issues:

  1. Pagination. The issue events endpoint is paginated (default 30, max 100 per page) and returns events oldest-first (verified empirically against a live issue). This reads only page 1. On a busy issue the lifecycle label is applied late in the issue's life, so it lives on a later page and is never fetched. labeledAt is undefined, the issue is skipped, and its event count only grows, so it is skipped forever. The busiest issues never auto-close.
  2. Ignores unlabeled. The code only inspects labeled events. 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 and state_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).
  • A pure, source-agnostic helper scripts/label-events.ts reconstructs the label's current state from the full labeled/unlabeled stream, sorted by created_at rather than trusting the endpoint's undocumented order, and returns the most recent still-active application time (or null if the label is no longer applied).
  • closeExpired() uses shouldClose(); downstream behavior (comments-since window, dry-run age) is unchanged.

Tests

scripts/label-events.test.ts (Bun), 5 passing:

  • label applied on page 2 (>100 events) is still detected;
  • labeled -> unlabeled -> re-labeled recently uses the recent timestamp, not the old one;
  • labeled -> unlabeled and not re-applied is treated as not currently labeled;
  • plus label-isolation and empty-input cases.

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.ts set the duplicate label through the issue PATCH, whose labels field 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.

…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.
@muhmadkwazulugorilla-png

Copy link
Copy Markdown

oh wow you are so brown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants