Repair lost closes in repos outside config.repos

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();
+}

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,17 +406,54 @@ 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;
    try {
       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) };
 }

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: [] });
+});