diff --git a/bin/repair-stale-open-pulls b/bin/repair-stale-open-pulls new file mode 100755 index 00000000..6d64d253 --- /dev/null +++ b/bin/repair-stale-open-pulls @@ -0,0 +1,67 @@ +#!/usr/bin/env node +/** + * Pulls whose `closed` webhook never landed stay open in the DB. Until the + * stale-open check listed repos outside config.repos, nothing repaired them + * there: 21 PRs closed 2026-09-26 to 09-30, while prod wrote into dev's DB, + * stayed open on prod (iFixit/ops#1055 in PR jail). The hourly repair now + * catches these; this fixes them without waiting for a deploy. + * + * It lists the open pulls of every repo the DB holds open pulls in and, with + * --apply, refetches each pull GitHub no longer lists, which stores its real + * state: + * + * docker exec pulldasher bin/repair-stale-open-pulls [--apply] + * + * Without --apply it only lists. GitHub calls are paced, since the board + * shares the token's hourly quota. + */ +import db from '../lib/db.js'; +import dbManager from '../lib/db-manager.js'; +import git from '../lib/git-manager.js'; +import { createPacedRefresh, findStaleOpenPulls, findUnlistedRepos } from '../lib/refresh.js'; + +const apply = process.argv.includes('--apply'); +// at most 40 GitHub calls a minute, well under the 5000-an-hour quota +const PACE_MS = 1500; +const pause = () => new Promise(resolve => setTimeout(resolve, PACE_MS)); + +try { + const dbOpen = await dbManager.getOpenPullIds(); + const listedRepos = []; + const listedPulls = []; + for (const repo of findUnlistedRepos(dbOpen, [])) { + await pause(); + try { + listedPulls.push(...(await git.getOpenPulls(repo))); + listedRepos.push(repo); + } catch (err) { + console.error('Skipped %s (could not list): %s', repo, err.message || err); + process.exitCode = 1; + } + } + const stale = findStaleOpenPulls(dbOpen, listedPulls, listedRepos); + console.log( + '%s of %s stored-open pulls are no longer open on GitHub', + stale.length, + dbOpen.length + ); + stale.forEach(({ repo, number }) => console.log(' %s#%s', repo, number)); + + if (apply) { + const refresh = createPacedRefresh(); + for (const { repo, number } of stale) { + await pause(); + await refresh.pull(repo, number).catch(() => {}); + } + // refresh.pull logs a failed save without rejecting, so read the DB back. + const open = new Set((await dbManager.getOpenPullIds()).map(p => p.repo + '#' + p.number)); + const left = stale.filter(({ repo, number }) => open.has(repo + '#' + number)); + console.log('Refreshed %s pulls; %s still open in the DB.', stale.length, left.length); + left.forEach(({ repo, number }) => console.log(' %s#%s', repo, number)); + if (left.length) process.exitCode = 1; + } else if (stale.length) { + console.log('Dry run: rerun with --apply to refresh them.'); + } +} finally { + db.end(); +} diff --git a/lib/refresh.js b/lib/refresh.js index a7135a8d..38dfd446 100644 --- a/lib/refresh.js +++ b/lib/refresh.js @@ -1,6 +1,7 @@ import gitManager, { observeRateLimit } from './git-manager.js'; import dbManager from './db-manager.js'; import utils from './utils.js'; +import config from './config-loader.js'; import NotifyQueue from 'notify-queue'; import debug from './debug.js'; import Promise from 'bluebird'; @@ -147,9 +148,9 @@ export function createRefresh({ pacer = noopPacer } = {}) { .then(result => drainThrough(pullQueue)(result).then(report => refreshStaleOpenPulls(pullQueue, pacer, result.items, listedRepos).then( - staleFailures => ({ - failedRepos: report.failedRepos, - failedItems: report.failedItems.concat(staleFailures), + stale => ({ + failedRepos: report.failedRepos.concat(stale.failedRepos), + failedItems: report.failedItems.concat(stale.failedItems), }) ) ) @@ -187,15 +188,10 @@ export function createRefresh({ pacer = noopPacer } = {}) { items: missing, failedRepos: listing.failedRepos, }); - const staleFailures = await refreshStaleOpenPulls( - pullQueue, - pacer, - listing.items, - listedRepos - ); + const stale = await refreshStaleOpenPulls(pullQueue, pacer, listing.items, listedRepos); return { - failedRepos: report.failedRepos, - failedItems: report.failedItems.concat(staleFailures), + failedRepos: report.failedRepos.concat(stale.failedRepos), + failedItems: report.failedItems.concat(stale.failedItems), }; }, }; @@ -410,9 +406,27 @@ export function findMissingOpenPulls(dbOpenPulls, listedPulls) { return listedPulls.filter(pull => !known.has(key(pull.base.repo.full_name, pull.number))); } +/** + * The repos the DB holds open pulls in that the caller didn't try to list: org + * repos left out of `config.repos`, whose pulls arrive by webhook only. + */ +export function findUnlistedRepos(dbOpenPulls, triedRepos) { + const tried = new Set(triedRepos.map(repo => repo.toLowerCase())); + const unlisted = new Map(); + dbOpenPulls.forEach(({ repo }) => { + if (!tried.has(repo.toLowerCase()) && !unlisted.has(repo.toLowerCase())) { + unlisted.set(repo.toLowerCase(), repo); + } + }); + return [...unlisted.values()]; +} + /** * A closed pull never appears in the open-pulls listing, so one whose close webhook - * was lost stays open in the DB until refetched. Never rejects: app.js ignores it. + * was lost stays open in the DB until refetched. Repos outside `config.repos` get + * listed here too, or a lost close there stays open forever. Resolves to the + * repos it couldn't list and the pulls it couldn't refresh. Never rejects: + * app.js ignores it. */ async function refreshStaleOpenPulls(queue, pacer, listedPulls, listedRepos) { let dbOpenPulls; @@ -420,7 +434,26 @@ async function refreshStaleOpenPulls(queue, pacer, listedPulls, listedRepos) { dbOpenPulls = await dbManager.getOpenPullIds(); } catch (err) { console.error('Failed to load open pulls from the DB: %s', (err && err.message) || err); - return []; + return { failedRepos: [], failedItems: [] }; + } + // Callers only list config.repos, so every other repo gets listed here. + const unlisted = findUnlistedRepos( + dbOpenPulls, + config.repos.map(repo => repo.name) + ); + let failedRepos = []; + if (unlisted.length) { + listedRepos = listedRepos.slice(); + const extra = await utils.forEachRepo( + repo => + gitManager.getOpenPulls(repo, pacer).then(pulls => { + listedRepos.push(repo); + return pulls; + }), + { repos: unlisted.map(name => ({ name })) } + ); + listedPulls = listedPulls.concat(extra.items); + failedRepos = extra.failedRepos; } const stale = findStaleOpenPulls(dbOpenPulls, listedPulls, listedRepos); refreshDebug('refreshing %s pulls open in the DB but not on GitHub', stale.length); @@ -441,8 +474,8 @@ async function refreshStaleOpenPulls(queue, pacer, listedPulls, listedRepos) { } } if (responses.length === 0) { - return failedItems; + return { failedRepos, failedItems }; } const report = await drainThrough(queue)({ items: responses, failedRepos: [] }); - return failedItems.concat(report.failedItems); + return { failedRepos, failedItems: failedItems.concat(report.failedItems) }; } diff --git a/test/stale-open-pulls.test.js b/test/stale-open-pulls.test.js index f3876120..159d69c8 100644 --- a/test/stale-open-pulls.test.js +++ b/test/stale-open-pulls.test.js @@ -84,3 +84,41 @@ test("openPulls still resolves when the DB can't list open pulls", async (t) => assert.equal(getPull.mock.callCount(), 0); assert.deepEqual(result, { failedRepos: [], failedItems: [] }); }); + +// Org repos left out of config.repos only get pulls by webhook, so a lost close +// there stayed open for good (iFixit/ops#1055). The repair lists them too, and a +// repo whose listing fails proves nothing about its pulls, but is reported. +test("reconcileOpenPulls refetches stale open pulls in repos outside config.repos", async (t) => { + const listed = []; + t.mock.method(gitManager, "getOpenPulls", (repo) => { + listed.push(repo); + if (repo === "test/gone") return Promise.reject(new Error("404")); + return Promise.resolve(repo === "Test/Untracked" ? [githubPull(repo, 6)] : []); + }); + t.mock.method(dbManager, "getOpenPullIds", () => + Promise.resolve([ + { repo: "Test/Untracked", number: 5 }, + { repo: "test/untracked", number: 6 }, + { repo: "test/gone", number: 7 }, + ]) + ); + const fetched = []; + t.mock.method(gitManager, "getPull", (repo, number) => { + fetched.push(`${repo}#${number}`); + return Promise.resolve({ ...githubPull(repo, number), state: "closed" }); + }); + t.mock.method(gitManager, "parse", (response) => Promise.resolve(response)); + t.mock.method(dbManager, "updateAllPullData", () => Promise.resolve()); + + const result = await createRefresh().reconcileOpenPulls(); + + assert.deepEqual(listed.sort(), [ + "Test/Untracked", + "test/gone", + "test/repo-a", + "test/repo-b", + "test/repo-c", + ]); + assert.deepEqual(fetched, ["Test/Untracked#5"]); + assert.deepEqual(result, { failedRepos: ["test/gone"], failedItems: [] }); +});