From dfcc40552bd21ff129f044787b14dad5cae59702 Mon Sep 17 00:00:00 2001 From: jackrescuer-gif Date: Fri, 15 May 2026 13:14:58 +0300 Subject: [PATCH 1/3] fix(repo-refresh): cancel manager-owned timers in test teardown + run tests in CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses two non-blocking follow-ups from #213 review. Test isolation: createRepoRefreshManager() now tracks every setTimeout it schedules (debounced save, runFetch 60s timeout/SIGKILL grace, waiter timeouts) in a Set and exposes shutdown() to cancel them all. Earlier reliance on timer.unref() was enough to let the process exit, but node:test's resource tracker still observed the pending handle and intermittently reported tests 12-16 as cancelled (per maintainer's report). Tests now register shutdown() via t.after() through a makeMgr(t, opts) helper — no more leak. CI: workflows/ci.yml previously ran only `node bin/cli.js version|help` and `node -c` syntax checks on five files, so "CI green" meant no test ever ran. Adds `node --test test/*.test.js` so future regressions are caught. wsl-windows.test.js skipped on non-win32: the assertion under test relies on backslash path joining which only happens when path defaults to win32. The companion "returns empty on non-Windows" test still validates the cross-platform branch. --- .github/workflows/ci.yml | 2 ++ src/repo-refresh.js | 44 ++++++++++++++++++++++- test/repo-refresh.test.js | 74 ++++++++++++++++++++++----------------- test/wsl-windows.test.js | 6 +++- 4 files changed, 92 insertions(+), 34 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 48851ac..c32fafe 100644 --- a/.github/workflows/ci.yml +++ b/.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 diff --git a/src/repo-refresh.js b/src/repo-refresh.js index b75716d..b1a1592 100644 --- a/src/repo-refresh.js +++ b/src/repo-refresh.js @@ -52,6 +52,13 @@ 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. + // .unref() alone is not enough — node:test's resource tracker may flag the + // pending timer as a leak and cancel subsequent tests. + const pendingTimers = new Set(); + // Track spawned children so shutdown() can SIGTERM them on host teardown. + const inflightChildren = new Set(); + let isShutdown = false; // ── Semaphore ──────────────────────────────────────────── // Synchronous when capacity is available so triggerRefresh can spawn the @@ -110,11 +117,18 @@ function createRepoRefreshManager(opts = {}) { resolve(next); }; + 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 +138,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 +149,7 @@ function createRepoRefreshManager(opts = {}) { })); return; } + cleanupTimers(); finalize(setState(gitRoot, { status: 'idle', startedAt: null, @@ -142,16 +158,21 @@ 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,6 +244,7 @@ function createRepoRefreshManager(opts = {}) { bucket.add(onDone); to = setTimeout(() => { + pendingTimers.delete(to); if (settled) return; settled = true; bucket.delete(onDone); @@ -230,6 +252,7 @@ function createRepoRefreshManager(opts = {}) { resolve({ state: state.get(gitRoot) || defaultRepoState(), timedOut: true }); }, timeoutMs); if (to && to.unref) to.unref(); + pendingTimers.add(to); }); } @@ -270,8 +293,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 @@ -282,6 +306,7 @@ function createRepoRefreshManager(opts = {}) { } }, debounceMs); if (saveTimer && saveTimer.unref) saveTimer.unref(); + pendingTimers.add(saveTimer); } function updateSettings(partial) { @@ -350,6 +375,22 @@ 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(); + } + // Load settings synchronously on construction. loadSettings(); @@ -360,6 +401,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). diff --git a/test/repo-refresh.test.js b/test/repo-refresh.test.js index aa34269..680f771 100644 --- a/test/repo-refresh.test.js +++ b/test/repo-refresh.test.js @@ -59,11 +59,21 @@ function defaults(overrides = {}) { }; } +// Create a manager and register shutdown() via t.after() so per-test timers +// (60s fetch timeout, debounced save, waiter timeouts) never leak into +// subsequent tests — otherwise node:test's resource tracker may mark them as +// leaks and cancel later tests on slower machines. +function makeMgr(t, opts) { + const mgr = createRepoRefreshManager(opts); + t.after(() => { try { mgr.shutdown(); } catch {} }); + return mgr; +} + // ── Tests ───────────────────────────────────────────────────── -test('triggerRefresh transitions idle → fetching → idle on success', async () => { +test('triggerRefresh transitions idle → fetching → idle on success', async (t) => { const exec = makeMockExecFile(); - const mgr = createRepoRefreshManager({ ...defaults(), execFile: exec.execFile }); + const mgr = makeMgr(t, { ...defaults(), execFile: exec.execFile }); const before = mgr.getState().repos['/repos/x']; assert.equal(before, undefined); @@ -81,9 +91,9 @@ test('triggerRefresh transitions idle → fetching → idle on success', async ( assert.ok(final.lastSuccessAt > 0); }); -test('triggerRefresh single-flight: 2 concurrent calls share the same promise', async () => { +test('triggerRefresh single-flight: 2 concurrent calls share the same promise', async (t) => { const exec = makeMockExecFile(); - const mgr = createRepoRefreshManager({ ...defaults(), execFile: exec.execFile }); + const mgr = makeMgr(t, { ...defaults(), execFile: exec.execFile }); const p1 = mgr.triggerRefresh('/repos/x'); const p2 = mgr.triggerRefresh('/repos/x'); @@ -95,9 +105,9 @@ test('triggerRefresh single-flight: 2 concurrent calls share the same promise', await p1; }); -test('semaphore caps concurrent fetches at 4; 5th queues until one finishes', async () => { +test('semaphore caps concurrent fetches at 4; 5th queues until one finishes', async (t) => { const exec = makeMockExecFile(); - const mgr = createRepoRefreshManager({ + const mgr = makeMgr(t, { ...defaults(), execFile: exec.execFile, maxConcurrency: 4, @@ -119,9 +129,9 @@ test('semaphore caps concurrent fetches at 4; 5th queues until one finishes', as await Promise.all(promises); }); -test('60s timeout: child killed (SIGTERM then SIGKILL grace), state=error, inflight cleared', async () => { +test('60s timeout: child killed (SIGTERM then SIGKILL grace), state=error, inflight cleared', async (t) => { const exec = makeMockExecFile(); - const mgr = createRepoRefreshManager({ + const mgr = makeMgr(t, { ...defaults(), execFile: exec.execFile, fetchTimeoutMs: 50, @@ -151,9 +161,9 @@ test('60s timeout: child killed (SIGTERM then SIGKILL grace), state=error, infli await p2; }); -test('non-zero exit propagates as state=error with truncated stderr (≤200 chars)', async () => { +test('non-zero exit propagates as state=error with truncated stderr (≤200 chars)', async (t) => { const exec = makeMockExecFile(); - const mgr = createRepoRefreshManager({ ...defaults(), execFile: exec.execFile }); + const mgr = makeMgr(t, { ...defaults(), execFile: exec.execFile }); const p = mgr.triggerRefresh('/repos/x'); const longErr = 'fatal: ' + 'x'.repeat(500); @@ -167,14 +177,14 @@ test('non-zero exit propagates as state=error with truncated stderr (≤200 char assert.ok(state.lastErrorAt > 0); }); -test('corrupt settings file → loadSettings logs warning, sets defaults, file untouched', () => { +test('corrupt settings file → loadSettings logs warning, sets defaults, file untouched', (t) => { const dir = mkTmp('codbash-repo-refresh-'); const settingsPath = path.join(dir, 'settings.json'); fs.writeFileSync(settingsPath, '{ not valid json'); const original = fs.readFileSync(settingsPath, 'utf8'); const warnings = []; - const mgr = createRepoRefreshManager({ + const mgr = makeMgr(t, { ...defaults(), settingsPath, logger: { warn: (msg) => warnings.push(msg) }, @@ -187,10 +197,10 @@ test('corrupt settings file → loadSettings logs warning, sets defaults, file u assert.equal(fs.readFileSync(settingsPath, 'utf8'), original, 'file must NOT be auto-overwritten'); }); -test('updateSettings round-trips through atomicWriteJson (debounced)', async () => { +test('updateSettings round-trips through atomicWriteJson (debounced)', async (t) => { const exec = makeMockExecFile(); const atomic = makeMockAtomicWrite(); - const mgr = createRepoRefreshManager({ + const mgr = makeMgr(t, { ...defaults(), execFile: exec.execFile, atomicWriteJson: atomic.atomicWriteJson, @@ -211,9 +221,9 @@ test('updateSettings round-trips through atomicWriteJson (debounced)', async () assert.deepEqual(atomic.calls[0].obj.perProject, { '/repos/x': { autoRefreshOnNewChat: true } }); }); -test('gitRoot with spaces is passed as a single argv element', async () => { +test('gitRoot with spaces is passed as a single argv element', async (t) => { const exec = makeMockExecFile(); - const mgr = createRepoRefreshManager({ ...defaults(), execFile: exec.execFile }); + const mgr = makeMgr(t, { ...defaults(), execFile: exec.execFile }); const root = '/repos/My Project (legacy)'; const p = mgr.triggerRefresh(root); @@ -225,7 +235,7 @@ test('gitRoot with spaces is passed as a single argv element', async () => { await p; }); -test('initOnStartup triggers only enabled repos and does not block', async () => { +test('initOnStartup triggers only enabled repos and does not block', async (t) => { const dir = mkTmp('codbash-repo-refresh-'); const settingsPath = path.join(dir, 'settings.json'); fs.writeFileSync(settingsPath, JSON.stringify({ @@ -239,7 +249,7 @@ test('initOnStartup triggers only enabled repos and does not block', async () => })); const exec = makeMockExecFile(); - const mgr = createRepoRefreshManager({ ...defaults(), settingsPath, execFile: exec.execFile }); + const mgr = makeMgr(t, { ...defaults(), settingsPath, execFile: exec.execFile }); const t0 = Date.now(); mgr.initOnStartup(); @@ -256,7 +266,7 @@ test('initOnStartup triggers only enabled repos and does not block', async () => for (const c of exec.calls) c.resolve(); }); -test('initOnStartup with refreshOnStartup=false launches nothing', async () => { +test('initOnStartup with refreshOnStartup=false launches nothing', async (t) => { const dir = mkTmp('codbash-repo-refresh-'); const settingsPath = path.join(dir, 'settings.json'); fs.writeFileSync(settingsPath, JSON.stringify({ @@ -266,14 +276,14 @@ test('initOnStartup with refreshOnStartup=false launches nothing', async () => { })); const exec = makeMockExecFile(); - const mgr = createRepoRefreshManager({ ...defaults(), settingsPath, execFile: exec.execFile }); + const mgr = makeMgr(t, { ...defaults(), settingsPath, execFile: exec.execFile }); mgr.initOnStartup(); await new Promise(r => setImmediate(r)); assert.equal(exec.calls.length, 0); }); -test('initOnStartup garbage-collects orphan perProject entries', async () => { +test('initOnStartup garbage-collects orphan perProject entries', async (t) => { const dir = mkTmp('codbash-repo-refresh-'); const settingsPath = path.join(dir, 'settings.json'); fs.writeFileSync(settingsPath, JSON.stringify({ @@ -286,7 +296,7 @@ test('initOnStartup garbage-collects orphan perProject entries', async () => { })); const atomic = makeMockAtomicWrite(); - const mgr = createRepoRefreshManager({ + const mgr = makeMgr(t, { ...defaults(), settingsPath, atomicWriteJson: atomic.atomicWriteJson, @@ -305,9 +315,9 @@ test('initOnStartup garbage-collects orphan perProject entries', async () => { assert.ok(atomic.calls.some(c => c.obj.perProject && !('/repos/deleted' in c.obj.perProject))); }); -test('waitForRefreshOrTimeout returns timedOut=true when fetch outruns the wait', async () => { +test('waitForRefreshOrTimeout returns timedOut=true when fetch outruns the wait', async (t) => { const exec = makeMockExecFile(); - const mgr = createRepoRefreshManager({ ...defaults(), execFile: exec.execFile }); + const mgr = makeMgr(t, { ...defaults(), execFile: exec.execFile }); const fetchP = mgr.triggerRefresh('/repos/x'); const result = await mgr.waitForRefreshOrTimeout('/repos/x', 30); @@ -320,9 +330,9 @@ test('waitForRefreshOrTimeout returns timedOut=true when fetch outruns the wait' await fetchP; }); -test('lastError redacts https://user:token@host credentials', async () => { +test('lastError redacts https://user:token@host credentials', async (t) => { const exec = makeMockExecFile(); - const mgr = createRepoRefreshManager({ ...defaults(), execFile: exec.execFile }); + const mgr = makeMgr(t, { ...defaults(), execFile: exec.execFile }); const p = mgr.triggerRefresh('/repos/x'); const errMsg = "fatal: Authentication failed for 'https://alice:ghp_secrettoken123@github.com/org/repo.git'"; @@ -336,7 +346,7 @@ test('lastError redacts https://user:token@host credentials', async () => { assert.ok(/@github\.com/.test(state.lastError), 'should preserve host with placeholder'); }); -test('initOnStartup skips perProject entries not in known-roots set', async () => { +test('initOnStartup skips perProject entries not in known-roots set', async (t) => { const dir = mkTmp('codbash-repo-refresh-'); const settingsPath = path.join(dir, 'settings.json'); fs.writeFileSync(settingsPath, JSON.stringify({ @@ -349,7 +359,7 @@ test('initOnStartup skips perProject entries not in known-roots set', async () = })); const exec = makeMockExecFile(); - const mgr = createRepoRefreshManager({ + const mgr = makeMgr(t, { ...defaults(), settingsPath, execFile: exec.execFile, @@ -367,7 +377,7 @@ test('initOnStartup skips perProject entries not in known-roots set', async () = for (const c of exec.calls) c.resolve(); }); -test('setKnownGitRootsProvider wires the gate after construction', async () => { +test('setKnownGitRootsProvider wires the gate after construction', async (t) => { const dir = mkTmp('codbash-repo-refresh-'); const settingsPath = path.join(dir, 'settings.json'); fs.writeFileSync(settingsPath, JSON.stringify({ @@ -377,7 +387,7 @@ test('setKnownGitRootsProvider wires the gate after construction', async () => { })); const exec = makeMockExecFile(); - const mgr = createRepoRefreshManager({ ...defaults(), settingsPath, execFile: exec.execFile }); + const mgr = makeMgr(t, { ...defaults(), settingsPath, execFile: exec.execFile }); // No known-roots set yet — wire it in after construction. mgr.setKnownGitRootsProvider(() => new Set(['/repos/a'])); @@ -387,9 +397,9 @@ test('setKnownGitRootsProvider wires the gate after construction', async () => { exec.calls[0].resolve(); }); -test('waitForRefreshOrTimeout returns timedOut=false when fetch finishes first', async () => { +test('waitForRefreshOrTimeout returns timedOut=false when fetch finishes first', async (t) => { const exec = makeMockExecFile(); - const mgr = createRepoRefreshManager({ ...defaults(), execFile: exec.execFile }); + const mgr = makeMgr(t, { ...defaults(), execFile: exec.execFile }); const fetchP = mgr.triggerRefresh('/repos/x'); const waitP = mgr.waitForRefreshOrTimeout('/repos/x', 200); diff --git a/test/wsl-windows.test.js b/test/wsl-windows.test.js index 97393ac..ebf5bf9 100644 --- a/test/wsl-windows.test.js +++ b/test/wsl-windows.test.js @@ -45,7 +45,11 @@ test('buildWslUncPath and shortenHomePath normalize WSL-visible paths', () => { assert.equal(normalizeProjectPath('\\\\?\\C:\\Projects\\codedash'), 'C:\\Projects\\codedash'); }); -test('detectWindowsWslHomes discovers only running distros with supported agent data', () => { +// The implementation uses `path.join` (platform-default), so the constructed +// `\\wsl$\...` paths only match the mocked existsSync set when path defaults +// to win32. Run this assertion only on Windows; the cross-platform behavior +// is covered by the test below. +test('detectWindowsWslHomes discovers only running distros with supported agent data', { skip: process.platform !== 'win32' && 'win32-only: uses backslash path joining' }, () => { const existing = new Set([ '\\\\wsl$\\Ubuntu-24.04\\home\\dius\\.codex', '\\\\wsl$\\Debian\\home\\tester\\.cursor', From 8e7f53d49027faa12fdc4ddc112d638c254e4e54 Mon Sep 17 00:00:00 2001 From: jackrescuer-gif Date: Fri, 15 May 2026 14:44:22 +0300 Subject: [PATCH 2/3] fix(repo-refresh): resolve pending fetch promises on shutdown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Following up on the CI failure for the first attempt at this fix. macOS runners on node:test reported tests 12–16 as cancelledByParent with the error "Promise resolution is still pending but the event loop has already resolved", even though all timers were now cleared. Root cause: test 11 ("initOnStartup garbage-collects orphan perProject entries") triggers a fetch for /repos/alive via initOnStartup but never resolves the mocked execFile call. shutdown() cancelled the 60s timeoutTimer but left the inflight Promise hanging — stricter node:test hosts (macOS CI) flag this at test teardown, while my local machine tolerated it. Track each runFetch finalize callback in `activeFinalizers` and resolve every outstanding fetch with a synthetic "cancelled (manager shutdown)" error state when shutdown() runs. Tests now pass cleanly on the same runners that failed before. --- src/repo-refresh.js | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/repo-refresh.js b/src/repo-refresh.js index b1a1592..c56c541 100644 --- a/src/repo-refresh.js +++ b/src/repo-refresh.js @@ -58,6 +58,11 @@ function createRepoRefreshManager(opts = {}) { 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 ──────────────────────────────────────────── @@ -112,10 +117,12 @@ 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; } @@ -389,6 +396,21 @@ function createRepoRefreshManager(opts = {}) { 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. From 58f77190192ff800426f2ead2b420a218a745291 Mon Sep 17 00:00:00 2001 From: jackrescuer-gif Date: Fri, 15 May 2026 14:52:43 +0300 Subject: [PATCH 3/3] fix(repo-refresh): drop .unref() on awaited timers + thread shutdown into api tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI second run still failed tests 12–16 with "Promise resolution is still pending but the event loop has already resolved". Root cause is more fundamental than a missing cleanup hook: The four manager timers (runFetch timeoutTimer/killTimer, waiter `to`, debounced saveTimer) were all .unref()'d. unref'd timers are not counted as keeping the event loop alive — so when a test does `await mgr.waitForRefreshOrTimeout(..., 30)`, the only handle keeping the loop alive is the unref'd 30ms timer. node:test sees the loop drain to idle while the test's returned Promise is still pending and flags it. Now that shutdown() exists, .unref() is redundant for graceful host exit. Drop all four .unref() calls so the timers properly keep the event loop alive while in flight; hosts call shutdown() to release them. Also thread t.after(shutdown) through repo-refresh-api.test.js's makeDeps(t, ...) helper — that file ran before repo-refresh.test.js in the CI test order and was leaving timers across the file boundary. --- src/repo-refresh.js | 12 +++---- test/repo-refresh-api.test.js | 59 +++++++++++++++++++---------------- 2 files changed, 38 insertions(+), 33 deletions(-) diff --git a/src/repo-refresh.js b/src/repo-refresh.js index c56c541..ce44ac8 100644 --- a/src/repo-refresh.js +++ b/src/repo-refresh.js @@ -53,8 +53,12 @@ function createRepoRefreshManager(opts = {}) { let settings = { ...DEFAULT_SETTINGS }; let saveTimer = null; // Track every manager-owned setTimeout so shutdown() can cancel them. - // .unref() alone is not enough — node:test's resource tracker may flag the - // pending timer as a leak and cancel subsequent tests. + // 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(); @@ -175,10 +179,8 @@ function createRepoRefreshManager(opts = {}) { 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); }); } @@ -258,7 +260,6 @@ function createRepoRefreshManager(opts = {}) { 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); }); } @@ -312,7 +313,6 @@ function createRepoRefreshManager(opts = {}) { logger.error('Failed to save refresh settings: ' + err.message); } }, debounceMs); - if (saveTimer && saveTimer.unref) saveTimer.unref(); pendingTimers.add(saveTimer); } diff --git a/test/repo-refresh-api.test.js b/test/repo-refresh-api.test.js index 2cf790d..2ad04cc 100644 --- a/test/repo-refresh-api.test.js +++ b/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');