fix(repo-refresh): cancel manager-owned timers + run tests in CI

.github/workflows/ci.yml

@@ -24,3 +24,5 @@ jobs:
           node bin/cli.js help
       - name: Syntax check
         run: node -c bin/cli.js && node -c src/data.js && node -c src/server.js && node -c src/terminals.js && node -c src/html.js
+      - name: Tests
+        run: node --test test/*.test.js

src/repo-refresh.js

@@ -52,6 +52,22 @@ function createRepoRefreshManager(opts = {}) {
   const waiters = new Map();     // gitRoot -> Set<(state)=>void> for waitForRefreshOrTimeout
   let settings = { ...DEFAULT_SETTINGS };
   let saveTimer = null;
+  // Track every manager-owned setTimeout so shutdown() can cancel them.
+  // We deliberately do NOT .unref() these timers — node:test treats unref'd
+  // timers as if they don't keep the event loop alive, so awaiting a Promise
+  // whose only "alive" handle is an unref'd timer reports as
+  // "Promise resolution is still pending but the event loop has already
+  // resolved" (observed on macOS/ubuntu CI runners). Hosts should call
+  // shutdown() at process exit instead to release timers gracefully.
+  const pendingTimers = new Set();
+  // Track spawned children so shutdown() can SIGTERM them on host teardown.
+  const inflightChildren = new Set();
+  // Track each in-flight runFetch finalizer so shutdown() can resolve the
+  // promise. Without this, an unresolved fetch leaves a pending promise that
+  // node:test flags as "Promise resolution is still pending" on stricter
+  // hosts (e.g. macOS CI runners), even after timers are cancelled.
+  const activeFinalizers = new Set();
+  let isShutdown = false;
 
   // ── Semaphore ────────────────────────────────────────────
   // Synchronous when capacity is available so triggerRefresh can spawn the
@@ -105,16 +121,25 @@ function createRepoRefreshManager(opts = {}) {
       const finalize = (next) => {
         if (settled) return;
         settled = true;
+        activeFinalizers.delete(finalize);
         if (timeoutTimer) clearTimeout(timeoutTimer);
         if (killTimer) clearTimeout(killTimer);
         resolve(next);
       };
+      activeFinalizers.add(finalize);
+
+      const cleanupTimers = () => {
+        if (timeoutTimer) { pendingTimers.delete(timeoutTimer); clearTimeout(timeoutTimer); timeoutTimer = null; }
+        if (killTimer) { pendingTimers.delete(killTimer); clearTimeout(killTimer); killTimer = null; }
+      };
 
       const child = execFile('git', ['-C', gitRoot, 'fetch', '--all', '--prune'], {
         timeout: 0, // we manage timeout manually for SIGTERM → SIGKILL escalation
         windowsHide: true,
       }, (err, stdout, stderr) => {
+        inflightChildren.delete(child);
         if (timeoutFired) {
+          cleanupTimers();
           finalize(setState(gitRoot, {
             status: 'error',
             startedAt: null,
@@ -124,6 +149,7 @@ function createRepoRefreshManager(opts = {}) {
           return;
         }
         if (err) {
+          cleanupTimers();
           const errStderr = err.stderr || stderr || '';
           const msg = errStderr || err.message || 'git fetch failed';
           finalize(setState(gitRoot, {
@@ -134,6 +160,7 @@ function createRepoRefreshManager(opts = {}) {
           }));
           return;
         }
+        cleanupTimers();
         finalize(setState(gitRoot, {
           status: 'idle',
           startedAt: null,
@@ -142,16 +169,19 @@ function createRepoRefreshManager(opts = {}) {
           lastErrorAt: null,
         }));
       });
+      inflightChildren.add(child);
 
       timeoutTimer = setTimeout(() => {
+        pendingTimers.delete(timeoutTimer);
         timeoutFired = true;
         try { child.kill('SIGTERM'); } catch {}
         killTimer = setTimeout(() => {
+          pendingTimers.delete(killTimer);
           try { child.kill('SIGKILL'); } catch {}
         }, sigkillGraceMs);
-        if (killTimer && killTimer.unref) killTimer.unref();
+        pendingTimers.add(killTimer);
       }, fetchTimeoutMs);
-      if (timeoutTimer && timeoutTimer.unref) timeoutTimer.unref();
+      pendingTimers.add(timeoutTimer);
     });
   }
 
@@ -223,13 +253,14 @@ function createRepoRefreshManager(opts = {}) {
       bucket.add(onDone);
 
       to = setTimeout(() => {
+        pendingTimers.delete(to);
         if (settled) return;
         settled = true;
         bucket.delete(onDone);
         if (bucket.size === 0) waiters.delete(gitRoot);
         resolve({ state: state.get(gitRoot) || defaultRepoState(), timedOut: true });
       }, timeoutMs);
-      if (to && to.unref) to.unref();
+      pendingTimers.add(to);
     });
   }
 
@@ -270,8 +301,9 @@ function createRepoRefreshManager(opts = {}) {
   }
 
   function scheduleSave() {
-    if (saveTimer) clearTimeout(saveTimer);
+    if (saveTimer) { clearTimeout(saveTimer); pendingTimers.delete(saveTimer); }
     saveTimer = setTimeout(() => {
+      pendingTimers.delete(saveTimer);
       saveTimer = null;
       try {
         // 0o600 — settings include the user's project list; keep them
@@ -281,7 +313,7 @@ function createRepoRefreshManager(opts = {}) {
         logger.error('Failed to save refresh settings: ' + err.message);
       }
     }, debounceMs);
-    if (saveTimer && saveTimer.unref) saveTimer.unref();
+    pendingTimers.add(saveTimer);
   }
 
   function updateSettings(partial) {
@@ -350,6 +382,37 @@ function createRepoRefreshManager(opts = {}) {
     }
   }
 
+  // Cancel every manager-owned timer and signal SIGTERM to any in-flight git
+  // fetch children. Safe to call multiple times. Intended for test teardown
+  // and host process shutdown — once called, the manager should not be used
+  // for new operations (no internal flag enforces this; callers decide).
+  function shutdown() {
+    if (isShutdown) return;
+    isShutdown = true;
+    if (saveTimer) { clearTimeout(saveTimer); saveTimer = null; }
+    for (const t of pendingTimers) clearTimeout(t);
+    pendingTimers.clear();
+    for (const child of inflightChildren) {
+      try { child.kill('SIGTERM'); } catch {}
+    }
+    inflightChildren.clear();
+    // Resolve every in-flight fetch with a synthetic "cancelled" error state
+    // so callers awaiting the promise don't hang and node:test does not flag
+    // pending promises at teardown. Snapshot first — finalize mutates the Set.
+    const pending = [...activeFinalizers];
+    activeFinalizers.clear();
+    for (const fin of pending) {
+      try {
+        fin({
+          status: 'error',
+          startedAt: null,
+          lastError: 'cancelled (manager shutdown)',
+          lastErrorAt: Date.now(),
+        });
+      } catch {}
+    }
+  }
+
   // Load settings synchronously on construction.
   loadSettings();
 
@@ -360,6 +423,7 @@ function createRepoRefreshManager(opts = {}) {
     updateSettings,
     initOnStartup,
     setKnownGitRootsProvider,
+    shutdown,
     // Exposed for tests only — production callers should never invoke these
     // directly (loadSettings can drop in-flight debounced changes;
     // triggerAllEnabled duplicates initOnStartup minus the known-roots gate).

test/repo-refresh-api.test.js

@@ -73,7 +73,7 @@ function request(port, method, urlPath, body) {
   });
 }
 
-function makeDeps(extraOverrides = {}) {
+function makeDeps(t, extraOverrides = {}) {
   const settingsPath = path.join(mkTmp('codbash-api-'), 'settings.json');
   const exec = makeMockExecFile();
   const manager = createRepoRefreshManager({
@@ -87,6 +87,11 @@ function makeDeps(extraOverrides = {}) {
     existsSync: () => true,
     ...extraOverrides,
   });
+  // Cancel manager-owned timers and resolve any in-flight fetch promise on
+  // test teardown so node:test's resource tracker doesn't see them as leaks.
+  if (t && typeof t.after === 'function') {
+    t.after(() => { try { manager.shutdown(); } catch {} });
+  }
   // Fixed list of "known" gitRoots used for /trigger and /settings validation.
   const knownGitRoots = new Set(['/repos/known-a', '/repos/known-b']);
   const getKnownGitRoots = () => knownGitRoots;
@@ -95,8 +100,8 @@ function makeDeps(extraOverrides = {}) {
 
 // ── Tests ─────────────────────────────────────────────────────
 
-test('GET /api/repo-refresh/state returns repos + settings on a fresh manager', async () => {
-  const deps = makeDeps();
+test('GET /api/repo-refresh/state returns repos + settings on a fresh manager', async (t) => {
+  const deps = makeDeps(t);
   const { server, port } = await startTestServer(deps);
   try {
     const res = await request(port, 'GET', '/api/repo-refresh/state');
@@ -109,8 +114,8 @@ test('GET /api/repo-refresh/state returns repos + settings on a fresh manager',
   }
 });
 
-test('POST /api/repo-refresh/trigger spawns a fetch for a known gitRoot', async () => {
-  const deps = makeDeps();
+test('POST /api/repo-refresh/trigger spawns a fetch for a known gitRoot', async (t) => {
+  const deps = makeDeps(t);
   const { server, port } = await startTestServer(deps);
   try {
     const res = await request(port, 'POST', '/api/repo-refresh/trigger', { gitRoot: '/repos/known-a' });
@@ -124,8 +129,8 @@ test('POST /api/repo-refresh/trigger spawns a fetch for a known gitRoot', async
   }
 });
 
-test('POST /api/repo-refresh/trigger returns 404 with code=not_found for an unknown gitRoot', async () => {
-  const deps = makeDeps();
+test('POST /api/repo-refresh/trigger returns 404 with code=not_found for an unknown gitRoot', async (t) => {
+  const deps = makeDeps(t);
   const { server, port } = await startTestServer(deps);
   try {
     const res = await request(port, 'POST', '/api/repo-refresh/trigger', { gitRoot: '/repos/wat' });
@@ -137,8 +142,8 @@ test('POST /api/repo-refresh/trigger returns 404 with code=not_found for an unkn
   }
 });
 
-test('POST /api/repo-refresh/trigger returns 400 with code=invalid_payload when gitRoot is missing', async () => {
-  const deps = makeDeps();
+test('POST /api/repo-refresh/trigger returns 400 with code=invalid_payload when gitRoot is missing', async (t) => {
+  const deps = makeDeps(t);
   const { server, port } = await startTestServer(deps);
   try {
     const res = await request(port, 'POST', '/api/repo-refresh/trigger', {});
@@ -149,8 +154,8 @@ test('POST /api/repo-refresh/trigger returns 400 with code=invalid_payload when
   }
 });
 
-test('POST /api/repo-refresh/trigger returns 400 for malformed JSON body', async () => {
-  const deps = makeDeps();
+test('POST /api/repo-refresh/trigger returns 400 for malformed JSON body', async (t) => {
+  const deps = makeDeps(t);
   const { server, port } = await startTestServer(deps);
   try {
     const res = await request(port, 'POST', '/api/repo-refresh/trigger', '{ not json');
@@ -161,8 +166,8 @@ test('POST /api/repo-refresh/trigger returns 400 for malformed JSON body', async
   }
 });
 
-test('POST /api/repo-refresh/wait returns timedOut=true when fetch outruns the wait', async () => {
-  const deps = makeDeps();
+test('POST /api/repo-refresh/wait returns timedOut=true when fetch outruns the wait', async (t) => {
+  const deps = makeDeps(t);
   const { server, port } = await startTestServer(deps);
   try {
     // Start a fetch that never resolves.
@@ -177,8 +182,8 @@ test('POST /api/repo-refresh/wait returns timedOut=true when fetch outruns the w
   }
 });
 
-test('POST /api/repo-refresh/wait returns 404 with code=not_found for unknown gitRoot', async () => {
-  const deps = makeDeps();
+test('POST /api/repo-refresh/wait returns 404 with code=not_found for unknown gitRoot', async (t) => {
+  const deps = makeDeps(t);
   const { server, port } = await startTestServer(deps);
   try {
     const res = await request(port, 'POST', '/api/repo-refresh/wait', { gitRoot: '/repos/not-mine' });
@@ -189,8 +194,8 @@ test('POST /api/repo-refresh/wait returns 404 with code=not_found for unknown gi
   }
 });
 
-test('POST /api/repo-refresh/wait clamps timeoutMs to a sane maximum', async () => {
-  const deps = makeDeps();
+test('POST /api/repo-refresh/wait clamps timeoutMs to a sane maximum', async (t) => {
+  const deps = makeDeps(t);
   const { server, port } = await startTestServer(deps);
   try {
     // No fetch in flight — wait should return immediately regardless of timeoutMs.
@@ -203,8 +208,8 @@ test('POST /api/repo-refresh/wait clamps timeoutMs to a sane maximum', async ()
   }
 });
 
-test('GET /api/repo-refresh/settings returns the current settings', async () => {
-  const deps = makeDeps();
+test('GET /api/repo-refresh/settings returns the current settings', async (t) => {
+  const deps = makeDeps(t);
   const { server, port } = await startTestServer(deps);
   try {
     const res = await request(port, 'GET', '/api/repo-refresh/settings');
@@ -216,8 +221,8 @@ test('GET /api/repo-refresh/settings returns the current settings', async () =>
   }
 });
 
-test('POST /api/repo-refresh/settings merges and persists valid input', async () => {
-  const deps = makeDeps();
+test('POST /api/repo-refresh/settings merges and persists valid input', async (t) => {
+  const deps = makeDeps(t);
   const { server, port } = await startTestServer(deps);
   try {
     const res = await request(port, 'POST', '/api/repo-refresh/settings', {
@@ -238,8 +243,8 @@ test('POST /api/repo-refresh/settings merges and persists valid input', async ()
   }
 });
 
-test('POST /api/repo-refresh/settings returns 400 with code=invalid_payload for unknown gitRoot in perProject', async () => {
-  const deps = makeDeps();
+test('POST /api/repo-refresh/settings returns 400 with code=invalid_payload for unknown gitRoot in perProject', async (t) => {
+  const deps = makeDeps(t);
   const { server, port } = await startTestServer(deps);
   try {
     const res = await request(port, 'POST', '/api/repo-refresh/settings', {
@@ -252,8 +257,8 @@ test('POST /api/repo-refresh/settings returns 400 with code=invalid_payload for
   }
 });
 
-test('GET /api/repo-refresh/settings returns the value just POSTed', async () => {
-  const deps = makeDeps();
+test('GET /api/repo-refresh/settings returns the value just POSTed', async (t) => {
+  const deps = makeDeps(t);
   const { server, port } = await startTestServer(deps);
   try {
     await request(port, 'POST', '/api/repo-refresh/settings', {
@@ -267,8 +272,8 @@ test('GET /api/repo-refresh/settings returns the value just POSTed', async () =>
   }
 });
 
-test('Unknown route under /api/repo-refresh/ returns 404', async () => {
-  const deps = makeDeps();
+test('Unknown route under /api/repo-refresh/ returns 404', async (t) => {
+  const deps = makeDeps(t);
   const { server, port } = await startTestServer(deps);
   try {
     const res = await request(port, 'GET', '/api/repo-refresh/nonsense');