Skip to content

Commit fd3c405

Browse files
authored
feat: enable safe reopening of prs (#2429)
Signed-off-by: exploreriii <133720349+exploreriii@users.noreply.github.com>
1 parent 92e7bcd commit fd3c405

2 files changed

Lines changed: 98 additions & 2 deletions

File tree

.github/scripts/__tests__/jest/bot-contributor-lifecycle.test.js

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ function makeEnv(spec) {
2929
failUnassignFor = [], // logins whose removeAssignees call should throw
3030
failCommentOn = [], // issue/PR numbers whose createComment call should throw
3131
failCloseFor = [], // PR numbers whose pulls.update (close) call should throw
32+
reopenedAtByPr = {}, // PR number -> ISO date of its latest `reopened` event
3233
} = spec;
3334

3435
const github = {
@@ -50,6 +51,8 @@ function makeEnv(spec) {
5051
}
5152
mut.unassigned.push({ issue_number, assignees });
5253
},
54+
listEvents: ({ issue_number }) =>
55+
reopenedAtByPr[issue_number] ? [{ event: "reopened", created_at: reopenedAtByPr[issue_number] }] : [],
5356
},
5457
pulls: {
5558
get: async ({ pull_number }) => {
@@ -610,6 +613,53 @@ describe("bot-contributor-lifecycle", () => {
610613
expect(m.unassigned).toHaveLength(0);
611614
});
612615

616+
test("a recently reopened stale PR is NOT re-closed — the clock restarts from the reopen", async () => {
617+
// The maintainer-recovery flow: PR closed for staleness weeks ago, contributor is
618+
// re-assigned and the PR reopened 2 days ago with no new commits yet. Must survive.
619+
const spec = {
620+
...specWithPR(143, 271, "alice", { author: "alice", reviews: [humanReview(70)], commitDate: daysAgo(80) }),
621+
reopenedAtByPr: { 271: daysAgo(2) },
622+
};
623+
const m = await run(spec, { DRY_RUN: "false" });
624+
expect(m.closed).toHaveLength(0);
625+
expect(m.comments).toHaveLength(0);
626+
expect(m.unassigned).toHaveLength(0);
627+
});
628+
629+
test("a reopen also prevents a premature reminder (remind-window case)", async () => {
630+
// Last review is inside the remind window (15d) but the PR was manually closed
631+
// and reopened 2 days ago — the reopen is the newest activity, so no action.
632+
const spec = {
633+
...specWithPR(146, 274, "alice", { author: "alice", reviews: [humanReview(15)], commitDate: daysAgo(20) }),
634+
reopenedAtByPr: { 274: daysAgo(2) },
635+
};
636+
const m = await run(spec, { DRY_RUN: "false" });
637+
expect(m.closed).toHaveLength(0);
638+
expect(m.comments).toHaveLength(0);
639+
expect(m.unassigned).toHaveLength(0);
640+
});
641+
642+
test("a reopened PR idle past the remind threshold gets a reminder, not a close", async () => {
643+
const spec = {
644+
...specWithPR(144, 272, "alice", { author: "alice", reviews: [humanReview(70)], commitDate: daysAgo(80) }),
645+
reopenedAtByPr: { 272: daysAgo(15) },
646+
};
647+
const m = await run(spec, { DRY_RUN: "false" });
648+
expect(m.closed).toHaveLength(0);
649+
expect(commentedOn(m, 272, "<!-- pr-inactivity-bot-marker -->")).toBe(true);
650+
expect(m.unassigned).toHaveLength(0);
651+
});
652+
653+
test("a reopened PR that idles past the close threshold again IS closed", async () => {
654+
const spec = {
655+
...specWithPR(145, 273, "alice", { author: "alice", reviews: [humanReview(90)], commitDate: daysAgo(95) }),
656+
reopenedAtByPr: { 273: daysAgo(70) },
657+
};
658+
const m = await run(spec, { DRY_RUN: "false" });
659+
expect(m.closed.some((c) => c.pull_number === 273)).toBe(true);
660+
expect(unassignedFrom(m, 145, "alice")).toBe(true);
661+
});
662+
613663
test("writes a markdown stats table to GITHUB_STEP_SUMMARY when set", async () => {
614664
const dir = fs.mkdtempSync(path.join(os.tmpdir(), "lifecycle-summary-"));
615665
const summaryFile = path.join(dir, "summary.md");

.github/scripts/bot-contributor-lifecycle.js

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ assignees (not PRs). For each issue it applies a staged escalation:
2424
"age" is measured from the most recent activity, which always includes a recent
2525
`/working` comment from an assignee — so `/working` resets every timer. PR age
2626
also includes the last commit and the last human (non-bot, non-author) review.
27+
Reopening a closed PR also counts as activity: the reopen timestamp restarts the
28+
clock, so a deliberately revived PR gets a full fresh remind/close cycle instead
29+
of being re-closed on the next run (works for PRs closed by the pre-consolidation
30+
bots too, whose close comments carried no marker).
2731
2832
Markers are cycle-scoped: a bot marker comment only suppresses a repeat action if it
2933
was posted AFTER the current cycle began (the assignee's latest assignment for issue
@@ -283,6 +287,33 @@ async function lastCommitDate(github, pr) {
283287
}
284288
}
285289

290+
// Most recent `reopened` event on the PR (Date or null). A reopen is a deliberate
291+
// human signal — server-timestamped and works no matter who/what closed the PR
292+
// (including the pre-consolidation bots, whose close comments carried no marker) —
293+
// so it resets the inactivity clock and grants a full fresh remind/close cycle.
294+
// Only called when a PR is old enough that the bot is about to act on it
295+
// (remind or close), so the extra API call stays rare.
296+
async function lastReopenedDate(github, owner, repo, prNumber) {
297+
try {
298+
const events = await github.paginate(github.rest.issues.listEvents, {
299+
owner,
300+
repo,
301+
issue_number: prNumber,
302+
per_page: 100,
303+
});
304+
let latest = null;
305+
for (const e of events) {
306+
if (e?.event !== "reopened" || !e.created_at) continue;
307+
const d = new Date(e.created_at);
308+
if (!latest || d > latest) latest = d;
309+
}
310+
return latest;
311+
} catch (err) {
312+
console.log(` [WARN] Could not fetch events for PR #${prNumber}:`, err.message || err);
313+
return null;
314+
}
315+
}
316+
286317
// ---- message builders ----
287318

288319
function issueReminderBody(assignees) {
@@ -496,10 +527,25 @@ async function handlePRStage(github, owner, repo, issue, graph, prNumbers, comme
496527
continue;
497528
}
498529
const commitDate = await lastCommitDate(github, pr);
499-
const activityAt = maxDate([reviewDate, commitDate, workingAt]);
500-
const ageDays = daysSince(activityAt, nowMs);
530+
let activityAt = maxDate([reviewDate, commitDate, workingAt]);
531+
let ageDays = daysSince(activityAt, nowMs);
501532
console.log(` [INFO] PR #${prNum} reviewed; last activity ~${ageDays}d ago`);
502533

534+
// Before reminding/closing, honour a deliberate reopen: if the PR was reopened
535+
// after its last review/commit activity, the reopen timestamp becomes the
536+
// activity baseline, so a revived PR gets a full new remind/close cycle instead
537+
// of a premature reminder or an immediate re-close (a silent close/reopen war
538+
// the human can't win). Checked whenever the bot is about to take ANY action.
539+
const actionThreshold = Math.min(cfg.prRemindDays, cfg.prCloseDays);
540+
if (ageDays >= actionThreshold) {
541+
const reopenedAt = await lastReopenedDate(github, owner, repo, prNum);
542+
if (reopenedAt && (!activityAt || reopenedAt > activityAt)) {
543+
activityAt = maxDate([activityAt, reopenedAt]);
544+
ageDays = daysSince(activityAt, nowMs);
545+
console.log(` [INFO] PR #${prNum} was reopened ~${ageDays}d ago; clock restarted from the reopen`);
546+
}
547+
}
548+
503549
// Close takes priority over remind so an (unusual) close < remind config still closes.
504550
if (ageDays >= cfg.prCloseDays) {
505551
toClose.push({ prNum, ageDays, activityAt });

0 commit comments

Comments
 (0)