diff --git a/scripts/auto-close-duplicates.ts b/scripts/auto-close-duplicates.ts index 2ad3bd3112..ae3d58bd6d 100644 --- a/scripts/auto-close-duplicates.ts +++ b/scripts/auto-close-duplicates.ts @@ -70,14 +70,23 @@ async function closeIssueAsDuplicate( duplicateOfNumber: number, token: string ): Promise { + // Add the "duplicate" label via the labels endpoint, which merges it in. The + // issue PATCH endpoint replaces the entire label set, so passing labels there + // would wipe any existing labels (bug, area:*, priority, and so on). + await githubRequest( + `/repos/${owner}/${repo}/issues/${issueNumber}/labels`, + token, + 'POST', + { labels: ['duplicate'] } + ); + await githubRequest( `/repos/${owner}/${repo}/issues/${issueNumber}`, token, 'PATCH', { state: 'closed', - state_reason: 'duplicate', - labels: ['duplicate'] + state_reason: 'duplicate' } ); diff --git a/scripts/label-events.test.ts b/scripts/label-events.test.ts new file mode 100644 index 0000000000..8f434222de --- /dev/null +++ b/scripts/label-events.test.ts @@ -0,0 +1,89 @@ +import { test, expect } from "bun:test"; +import { effectiveLabelAppliedAt, shouldClose, type LabelEvent } from "./label-events.ts"; + +// Helper: build a "labeled"/"unlabeled" event for `label` at a given time. +const ev = (event: string, label: string, created_at: string): LabelEvent => ({ + event, + label: { name: label }, + created_at, +}); + +// The bug this replaces: the old code did +// events.filter(e => e.event === "labeled" && e.label?.name === label) +// .map(e => new Date(e.created_at)).pop() +// on only page 1 of the events endpoint (oldest-first, 100/page). Reproduce that +// old logic here so the tests document exactly what used to go wrong. +const oldLogic = (events: LabelEvent[], label: string): Date | undefined => + events + .filter((e) => e.event === "labeled" && e.label?.name === label) + .map((e) => new Date(e.created_at)) + .pop(); + +const cutoff = new Date("2026-01-10T00:00:00Z"); // "close if applied on/before this" + +test("scenario A: label applied on page 2 (>100 events) is still detected", () => { + // 100 unrelated older events (page 1), then the lifecycle label applied late + // (page 2). The old single-page logic never sees it. + const page1: LabelEvent[] = Array.from({ length: 100 }, (_, i) => + ev("referenced", "stale", `2025-01-01T00:00:${String(i % 60).padStart(2, "0")}Z`) + ).map((e) => ({ ...e, event: "referenced", label: undefined })); + const page2 = [ev("labeled", "stale", "2025-12-20T00:00:00Z")]; + const all = [...page1, ...page2]; + + // Old logic (page 1 only): finds nothing -> issue skipped forever. + expect(oldLogic(page1, "stale")).toBeUndefined(); + + // New logic (all pages): detects the application and it is before cutoff. + const applied = effectiveLabelAppliedAt(all, "stale"); + expect(applied?.toISOString()).toBe("2025-12-20T00:00:00.000Z"); + expect(shouldClose(all, "stale", cutoff).close).toBe(true); +}); + +test("scenario B: labeled early, unlabeled, re-labeled recently -> uses recent timestamp", () => { + const events = [ + ev("labeled", "stale", "2025-11-01T00:00:00Z"), // old application (before cutoff) + ev("unlabeled", "stale", "2025-11-05T00:00:00Z"), // removed + ev("labeled", "stale", "2026-01-15T00:00:00Z"), // re-applied recently (grace restarted) + ]; + + // Old logic keyed off an early "labeled" (before the cutoff) and would close the + // issue even though its grace period restarted on 2026-01-15. Sanity-check that + // an early-application view really is before the cutoff (would have closed): + expect(events[0].created_at < cutoff.toISOString()).toBe(true); + + // New logic uses the most recent application, and is correct regardless of the + // order events arrive in across pages: + const shuffled = [events[2], events[0], events[1]]; // out of order on purpose + const applied = effectiveLabelAppliedAt(shuffled, "stale"); + expect(applied?.toISOString()).toBe("2026-01-15T00:00:00.000Z"); + + // Grace period restarted 2026-01-15, which is AFTER the cutoff -> must NOT close. + expect(shouldClose(shuffled, "stale", cutoff).close).toBe(false); +}); + +test("scenario C: labeled then unlabeled, not re-applied -> treated as not labeled", () => { + const events = [ + ev("labeled", "stale", "2025-11-01T00:00:00Z"), + ev("unlabeled", "stale", "2025-11-05T00:00:00Z"), + ]; + expect(effectiveLabelAppliedAt(events, "stale")).toBeNull(); + expect(shouldClose(events, "stale", cutoff).close).toBe(false); +}); + +test("only considers the matching label, ignores other labels' events", () => { + const events = [ + ev("labeled", "needs-repro", "2025-12-01T00:00:00Z"), + ev("labeled", "stale", "2025-12-15T00:00:00Z"), + ev("unlabeled", "needs-repro", "2025-12-20T00:00:00Z"), + ]; + expect(effectiveLabelAppliedAt(events, "stale")?.toISOString()).toBe( + "2025-12-15T00:00:00.000Z" + ); +}); + +test("no events for the label -> null (not currently applied)", () => { + expect(effectiveLabelAppliedAt([], "stale")).toBeNull(); + expect( + effectiveLabelAppliedAt([ev("labeled", "invalid", "2025-12-01T00:00:00Z")], "stale") + ).toBeNull(); +}); diff --git a/scripts/label-events.ts b/scripts/label-events.ts new file mode 100644 index 0000000000..4c777e2ad6 --- /dev/null +++ b/scripts/label-events.ts @@ -0,0 +1,49 @@ +// Helpers for working out a label's current state from an issue's +// labeled/unlabeled events. No I/O here so it's easy to unit test +// (see label-events.test.ts). + +export interface LabelEvent { + event: string; + label?: { name: string }; + created_at: string; +} + +// Given all of an issue's events, find when `label` was last applied and is +// still on the issue: the newest "labeled" event with no "unlabeled" for the +// same label after it. Returns null if the label isn't currently applied. +// +// We sort by created_at here instead of trusting the API's order (GitHub doesn't +// document how the events endpoint orders results), so callers can pass events +// from any page in any order. +export function effectiveLabelAppliedAt( + events: LabelEvent[], + label: string +): Date | null { + const stream = events + .filter( + (e) => + (e.event === "labeled" || e.event === "unlabeled") && + e.label?.name === label + ) + .sort( + (a, b) => + new Date(a.created_at).getTime() - new Date(b.created_at).getTime() + ); + + let appliedAt: Date | null = null; + for (const e of stream) { + appliedAt = e.event === "labeled" ? new Date(e.created_at) : null; + } + return appliedAt; +} + +// Close an issue only if `label` is still applied and was last applied on or +// before `cutoff`. +export function shouldClose( + events: LabelEvent[], + label: string, + cutoff: Date +): { close: boolean; appliedAt: Date | null } { + const appliedAt = effectiveLabelAppliedAt(events, label); + return { close: appliedAt !== null && appliedAt <= cutoff, appliedAt }; +} diff --git a/scripts/sweep.ts b/scripts/sweep.ts index 4709290be3..b21053d38a 100644 --- a/scripts/sweep.ts +++ b/scripts/sweep.ts @@ -1,6 +1,7 @@ #!/usr/bin/env bun import { lifecycle, STALE_UPVOTE_THRESHOLD } from "./issue-lifecycle.ts"; +import { shouldClose, type LabelEvent } from "./label-events.ts"; // -- @@ -40,6 +41,25 @@ async function githubRequest( return response.json(); } +// Grab every labeled/unlabeled event for an issue. The events endpoint pages at +// 100 per request and returns them oldest-first, so on a busy issue the lifecycle +// label (applied late by triage) sits on a later page. The old code read only +// page 1 and missed it, so those issues never closed. Walk the pages to the end. +// The 10-page cap matches the issue-list loops above and is well past any real +// issue's event count. +async function fetchLabelEvents(base: string): Promise { + const events: LabelEvent[] = []; + for (let page = 1; page <= 10; page++) { + const pageEvents = await githubRequest( + `${base}/events?per_page=100&page=${page}` + ); + if (!Array.isArray(pageEvents) || pageEvents.length === 0) break; + events.push(...pageEvents); + if (pageEvents.length < 100) break; + } + return events; +} + // -- async function markStale(owner: string, repo: string) { @@ -112,14 +132,15 @@ async function closeExpired(owner: string, repo: string) { const base = `/repos/${owner}/${repo}/issues/${issue.number}`; - const events = await githubRequest(`${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; + // Work out the label's current state from its full event history. Only + // close if the label is still on the issue and was last applied before + // the cutoff. This covers labels applied on a later events page, and + // labels that were removed and re-applied (which restarts the grace + // period). + const events = await fetchLabelEvents(base); + const { close, appliedAt } = shouldClose(events, label, cutoff); + if (!close || !appliedAt) continue; + const labeledAt = appliedAt; // Skip if a non-bot user commented after the label was applied. // The triage workflow should remove lifecycle labels on human