refactor(utils): add TimerRegistry for centralized timer cleanup (#4248)

.github/workflows/ci.yml

@@ -182,7 +182,7 @@ jobs:
             kill "$AEGIS_PID" || true
           fi
       - name: Check bundle size
-        run: "THRESHOLD_KB=2370\nSERVER_SIZE=$(find dist/ -name \"*.js\" ! -path \"\
+        run: "THRESHOLD_KB=2375\nSERVER_SIZE=$(find dist/ -name \"*.js\" ! -path \"\
           */__tests__/*\" ! -path \"*/dashboard/*\" -exec du -ck {} + | tail -1 | awk '{print $1}')\nSERVER_SIZE_KB=$((SERVER_SIZE))\n\
           echo \"## Bundle Size Report\" >> \"$GITHUB_STEP_SUMMARY\"\necho \"\" >> \"\
           $GITHUB_STEP_SUMMARY\"\necho \"| Scope | Size (KB) | Threshold (KB) | Status\
@@ -330,7 +330,7 @@ jobs:
           }
       - name: Check bundle size
         if: runner.os != 'Windows'
-        run: "THRESHOLD_KB=2370\nSERVER_SIZE=$(find dist/ -name \"*.js\" ! -path \"\
+        run: "THRESHOLD_KB=2375\nSERVER_SIZE=$(find dist/ -name \"*.js\" ! -path \"\
           */__tests__/*\" ! -path \"*/dashboard/*\" -exec du -ck {} + | tail -1 | awk '{print $1}')\nSERVER_SIZE_KB=$((SERVER_SIZE))\n\
           echo \"## Bundle Size Report\" >> \"$GITHUB_STEP_SUMMARY\"\necho \"\" >> \"\
           $GITHUB_STEP_SUMMARY\"\necho \"| Scope | Size (KB) | Threshold (KB) | Status\

src/__tests__/timer-registry.test.ts

@@ -0,0 +1,137 @@
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+import { TimerRegistry } from '../utils/timer-registry.js';
+
+describe('TimerRegistry', () => {
+  let registry: TimerRegistry;
+
+  beforeEach(() => {
+    registry = new TimerRegistry();
+    vi.useFakeTimers({ shouldAdvanceTime: true });
+  });
+
+  afterEach(() => {
+    registry.clearAll();
+    vi.useRealTimers();
+  });
+
+  describe('setTimeout', () => {
+    it('wraps setTimeout and fires callback after delay', () => {
+      const fn = vi.fn();
+      registry.setTimeout(fn, 100);
+      vi.advanceTimersByTime(100);
+      expect(fn).toHaveBeenCalledTimes(1);
+    });
+
+    it('tracks active timer and removes after firing', () => {
+      registry.setTimeout(() => {}, 100);
+      expect(registry.activeCount).toBe(1);
+      vi.advanceTimersByTime(100);
+      expect(registry.activeCount).toBe(0);
+    });
+
+    it('passes through extra arguments', () => {
+      const fn = vi.fn();
+      registry.setTimeout(fn, 50, 'a', 'b');
+      vi.advanceTimersByTime(50);
+      expect(fn).toHaveBeenCalledWith('a', 'b');
+    });
+  });
+
+  describe('setInterval', () => {
+    it('wraps setInterval and fires repeatedly', () => {
+      const fn = vi.fn();
+      registry.setInterval(fn, 100);
+      vi.advanceTimersByTime(350);
+      expect(fn).toHaveBeenCalledTimes(3);
+    });
+
+    it('tracks interval and clears when clearInterval is called', () => {
+      const fn = vi.fn();
+      const id = registry.setInterval(fn, 100);
+      expect(registry.activeCount).toBe(1);
+      registry.clearInterval(id);
+      expect(registry.activeCount).toBe(0);
+      vi.advanceTimersByTime(200);
+      expect(fn).toHaveBeenCalledTimes(0);
+    });
+  });
+
+  describe('clearTimeout', () => {
+    it('prevents callback from firing', () => {
+      const fn = vi.fn();
+      const id = registry.setTimeout(fn, 100);
+      registry.clearTimeout(id);
+      vi.advanceTimersByTime(200);
+      expect(fn).toHaveBeenCalledTimes(0);
+    });
+
+    it('untracks the timer', () => {
+      const id = registry.setTimeout(() => {}, 100);
+      expect(registry.activeCount).toBe(1);
+      registry.clearTimeout(id);
+      expect(registry.activeCount).toBe(0);
+    });
+  });
+
+  describe('clearAll', () => {
+    it('clears all tracked timeouts and intervals', () => {
+      registry.setTimeout(() => {}, 1000);
+      registry.setInterval(() => {}, 1000);
+      registry.setTimeout(() => {}, 2000);
+      expect(registry.activeCount).toBe(3);
+      registry.clearAll();
+      expect(registry.activeCount).toBe(0);
+    });
+
+    it('prevents all callbacks from firing', () => {
+      const fn1 = vi.fn();
+      const fn2 = vi.fn();
+      registry.setTimeout(fn1, 100);
+      registry.setInterval(fn2, 50);
+      registry.clearAll();
+      vi.advanceTimersByTime(500);
+      expect(fn1).toHaveBeenCalledTimes(0);
+      expect(fn2).toHaveBeenCalledTimes(0);
+    });
+  });
+
+  describe('activeCount', () => {
+    it('returns 0 for new registry', () => {
+      expect(registry.activeCount).toBe(0);
+    });
+
+    it('increases when timers are added, decreases when cleared or fired', () => {
+      expect(registry.activeCount).toBe(0);
+      registry.setTimeout(() => {}, 100);
+      expect(registry.activeCount).toBe(1);
+      registry.setInterval(() => {}, 100);
+      expect(registry.activeCount).toBe(2);
+      vi.advanceTimersByTime(100);
+      expect(registry.activeCount).toBe(1);
+    });
+  });
+
+  describe('edge cases', () => {
+    it('handles idempotent clearAll calls', () => {
+      registry.clearAll();
+      registry.clearAll();
+      expect(registry.activeCount).toBe(0);
+    });
+
+    it('handles setTimeout with delay 0', () => {
+      const fn = vi.fn();
+      registry.setTimeout(fn, 0);
+      vi.advanceTimersByTime(0);
+      expect(fn).toHaveBeenCalledTimes(1);
+      expect(registry.activeCount).toBe(0);
+    });
+
+    it('still tracks timer after clearInterval even if clearAll is called later', () => {
+      const id = registry.setInterval(() => {}, 100);
+      registry.clearInterval(id);
+      expect(registry.activeCount).toBe(0);
+      registry.clearAll();
+      expect(registry.activeCount).toBe(0);
+    });
+  });
+});

src/server.ts

@@ -75,6 +75,7 @@ import { InMemoryPauseInterventionStore } from './services/acp/in-memory-pause-i
 import { isWindowsShutdownMessage, parseShutdownTimeoutMs } from './shutdown-utils.js';
 import { ServiceContainer } from './container.js';
 import type { AppContext } from './app-context.js';
+import { TimerRegistry } from './utils/timer-registry.js';
 import { AcpBackend } from './services/acp/backend.js';
 import { AcpSessionService } from './services/acp/session-service.js';
 import { AcpTerminalBridge } from './services/acp/terminal-bridge.js';
@@ -145,6 +146,8 @@ declare module 'fastify' {
 // Module-level const values are still initialized here.
 const channels = new ChannelManager();
 const eventBus = new SessionEventBus();
+// Issue #4248: Centralized timer registry for clean shutdown
+const timers = new TimerRegistry();
 // Issue #4116: Debounce Set for session approval callbacks (prevents duplicate notifications from rapid Telegram clicks).
 const recentApprovalActions = new Set<string>();
 
@@ -771,8 +774,8 @@ function setupConfigWatcher(ctx: AppContext): void {
     ctx.configWatcher = watch(configPath, (_eventType) => {
       // Accept all event types — editors emit rename (atomic save), change, or undefined.
       // Debounce: FS events can fire multiple times for one save
-        if (ctx.configReloadTimer) clearTimeout(ctx.configReloadTimer);
-        ctx.configReloadTimer = setTimeout(() => {
+        if (ctx.configReloadTimer) timers.clearTimeout(ctx.configReloadTimer);
+        ctx.configReloadTimer = timers.setTimeout(() => {
           void handleConfigReload('file-change', ctx);
         }, 300);
     });
@@ -1277,24 +1280,25 @@ registerBudgetRoutes(app, { auth: ctx.auth, budgetStore, budgetEvaluator });
   registerOpenApiRoute(app);
 
   // Issue #361: Store interval refs so graceful shutdown can clear them
-  const reaperInterval = setInterval(() => reapStaleSessions(ctx.config.maxSessionAgeMs, ctx), ctx.config.reaperIntervalMs);
-  const zombieReaperInterval = setInterval(() => reapZombieSessions(ctx), ZOMBIE_REAP_INTERVAL_MS);
-const metricsSaveInterval = setInterval(() => { void ctx.metrics.save(); }, 5 * 60 * 1000);
+  timers.setInterval(() => reapStaleSessions(ctx.config.maxSessionAgeMs, ctx), ctx.config.reaperIntervalMs);
+  timers.setInterval(() => reapZombieSessions(ctx), ZOMBIE_REAP_INTERVAL_MS);
+timers.setInterval(() => { void ctx.metrics.save(); }, 5 * 60 * 1000);
   // Issue #3310: Periodically persist metering data.
-  const meteringSaveInterval = setInterval(() => { void metering.save(); }, 5 * 60 * 1000);
+  timers.setInterval(() => { void metering.save(); }, 5 * 60 * 1000);
   // #357: Prune stale IP rate-limit entries every minute
-  const ipPruneInterval = setInterval(pruneIpRateLimits, 60_000);
+  timers.setInterval(pruneIpRateLimits, 60_000);
   // #632: Prune stale auth failure rate-limit buckets every minute
-  const authFailPruneInterval = setInterval(pruneAuthFailLimits, 60_000);
+  timers.setInterval(pruneAuthFailLimits, 60_000);
   // #398: Sweep stale API key rate limit buckets every 5 minutes
-const authSweepInterval = setInterval(() => ctx.auth.sweepStaleRateLimits(), 5 * 60_000);
+timers.setInterval(() => ctx.auth.sweepStaleRateLimits(), 5 * 60_000);
   // #2452: Sweep expired quota usage entries every 5 minutes to prevent unbounded growth
   const quotaSweepInterval = setInterval(() => routeCtx.quotas.sweep(), 5 * 60_000);
   // Issue #4004: Start orphan action sweeper
 ctx.actionSweeper?.start();
   // Issue #4195: Start budget evaluation timer
   budgetTimer.start();
   // #3227: Prune interval from StaticRateLimiter — assigned after registerDashboardStatic()
+  // Issue #4248: staticPruneInterval tracked via timers.track() after registration
   let staticPruneInterval: ReturnType<typeof setInterval> | null = null;
   let pidFilePath = '';
 
@@ -1360,20 +1364,13 @@ ctx.actionSweeper?.start();
       // #1753: Close config file watcher
 ctx.configWatcher?.close();
       ctx.configWatcher = null;
-if (ctx.configReloadTimer) { clearTimeout(ctx.configReloadTimer); ctx.configReloadTimer = null; }
-      clearInterval(reaperInterval);
-      clearInterval(zombieReaperInterval);
-      clearInterval(metricsSaveInterval);
-      clearInterval(meteringSaveInterval);
-      clearInterval(ipPruneInterval);
-      clearInterval(authFailPruneInterval);
-      clearInterval(authSweepInterval);
-      clearInterval(quotaSweepInterval);
+if (ctx.configReloadTimer) { timers.clearTimeout(ctx.configReloadTimer); ctx.configReloadTimer = null; }
+      // Issue #4248: Clear all tracked timers via TimerRegistry
+      timers.clearAll();
       // Issue #4004: Stop orphan action sweeper
 ctx.actionSweeper?.stop();
       // Issue #4195: Stop budget evaluation timer
       budgetTimer.stop();
-      if (staticPruneInterval) clearInterval(staticPruneInterval);
       rateLimiter.dispose();
 
       // 3. Close file watchers, pipelines, and reaper
@@ -1570,6 +1567,7 @@ ctx.auditLogger?.flush() ?? Promise.resolve(),
   // #3154: Dashboard static serving extracted to plugins/dashboard-static.ts
   // #3227: Capture prune interval handle for cleanup on shutdown
 staticPruneInterval = await registerDashboardStatic(app, { enabled: ctx.config.dashboardEnabled !== false });
+  if (staticPruneInterval) timers.track(staticPruneInterval);
   await container.assertHealthy();
 await listenWithRetry(app, ctx.config.port, ctx.config.host, ctx.config.stateDir);
 pidFilePath = await writePidFile(ctx.config.stateDir);

src/utils/timer-registry.ts

@@ -0,0 +1,60 @@
+/**
+ * Centralized registry for setTimeout/setInterval handles.
+ * Enables clean bulk-clear on shutdown and prevents orphaned timers.
+ */
+export class TimerRegistry {
+  private readonly handles = new Set<ReturnType<typeof setTimeout | typeof setInterval>>();
+
+  /** Wrap setTimeout — auto-untracks after firing. */
+  setTimeout<TArgs extends unknown[]>(
+    fn: (...args: TArgs) => void,
+    ms: number,
+    ...args: TArgs
+  ): ReturnType<typeof setTimeout> {
+    const id = setTimeout(() => {
+      this.handles.delete(id as ReturnType<typeof setInterval>);
+      fn(...args);
+    }, ms);
+    this.handles.add(id as ReturnType<typeof setInterval>);
+    return id;
+  }
+
+  /** Wrap setInterval — stays tracked until clearInterval or clearAll. */
+  setInterval<TArgs extends unknown[]>(fn: (...args: TArgs) => void, ms: number, ...args: TArgs): ReturnType<typeof setInterval> {
+    const id = setInterval(fn, ms, ...args) as ReturnType<typeof setInterval>;
+    this.handles.add(id);
+    return id;
+  }
+
+  /** Clear a tracked timeout. */
+  clearTimeout(id: ReturnType<typeof setTimeout>): void {
+    clearTimeout(id);
+    this.handles.delete(id as ReturnType<typeof setInterval>);
+  }
+
+  /** Clear a tracked interval. */
+  clearInterval(id: ReturnType<typeof setInterval>): void {
+    clearInterval(id);
+    this.handles.delete(id);
+  }
+
+
+  /** Track an externally-created handle (e.g. from a plugin that returns its own interval). */
+  track(id: ReturnType<typeof setTimeout | typeof setInterval>): void {
+    this.handles.add(id as ReturnType<typeof setInterval>);
+  }
+
+  /** Clear all tracked timers. */
+  clearAll(): void {
+    // clearTimeout also clears intervals in Node.js — using it for both is safe
+    for (const id of this.handles) {
+      clearTimeout(id);
+    }
+    this.handles.clear();
+  }
+
+  /** Number of currently active (pending) timers. */
+  get activeCount(): number {
+    return this.handles.size;
+  }
+}