perf/fix: PR#48 observability followup — readdir cache, bootstrap spawns, morphdom id, localStorage GC

public/sidebar.js

@@ -18,7 +18,25 @@ function folderId(projectPath) {
 }
 
 // --- Subagent localStorage helpers ---
+
+// One-time GC: prune sessionIds that no longer exist in sessionMap.
+// Runs once per page load (lazily on first read) to keep the stored set
+// from growing indefinitely across long-lived Switchboard instances.
+let _expandedSubagentsGCDone = false;
+function _gcExpandedSubagentsOnce() {
+  if (_expandedSubagentsGCDone) return;
+  _expandedSubagentsGCDone = true;
+  try {
+    const raw = new Set(JSON.parse(localStorage.getItem('expandedSubagents') || '[]'));
+    const pruned = new Set([...raw].filter(id => sessionMap.has(id)));
+    if (pruned.size !== raw.size) {
+      localStorage.setItem('expandedSubagents', JSON.stringify([...pruned]));
+    }
+  } catch {} // eslint: allowEmptyCatch
+}
+
 function getExpandedSubagents() {
+  _gcExpandedSubagentsOnce();
   try {
     return new Set(JSON.parse(localStorage.getItem('expandedSubagents') || '[]'));
   } catch (e) { return new Set(); }
@@ -421,6 +439,9 @@ function renderProjects(projects, resort) {
 
         const orphanGroup = document.createElement('div');
         orphanGroup.className = 'sidebar-orphan-subagents' + (expanded ? '' : ' collapsed');
+        // Stable id so morphdom reconciles the element instead of rebuilding
+        // it from scratch on every render, preventing minor flicker.
+        orphanGroup.id = 'orphan-' + fId;
 
         const orphanLabel = document.createElement('div');
         orphanLabel.className = 'sidebar-orphan-label';

session-transitions.js

@@ -23,19 +23,55 @@ function init(ctx) {
  *  Emits IPC 'subagent-spawned' and 'subagent-completed' via mainWindow. */
 function detectSubagentTransitions(sessionId, session, folderPath) {
   const subagentsDir = path.join(folderPath, sessionId, 'subagents');
-  let files;
+
+  // --- Hot-path cache: avoid readdirSync + N×statSync when dir is quiet ---
+  // With 1000+ subagents, the full scan blocks the main thread ~70 ms per
+  // flush. We cache the dir's mtime; when it hasn't changed, no new files
+  // could have appeared, so we skip readdirSync AND statSync for unknown
+  // files. Known-active entries still get statSync for the stability timer.
+  //
+  // Session-local state:
+  //   _prevDirMtime  — subagentsDir mtime at the last readdirSync
+  //   _subFileList   — .jsonl file list from that scan
+  let dirMtime;
   try {
-    files = fs.readdirSync(subagentsDir).filter(f => f.endsWith('.jsonl'));
+    dirMtime = fs.statSync(subagentsDir).mtimeMs;
   } catch {
     return; // directory doesn't exist yet — normal
   }
 
+  const isBootstrap = !session.knownSubagents;
+
+  // dirChanged: true when dir mtime moved or we have no prior scan yet.
+  // Also true when the prior scan returned 0 files: the dir mtime may not
+  // advance within the same filesystem-clock tick on fast writes, so we
+  // must rescan until we see at least one file to avoid missing arrivals.
+  const prevFileList = session._subFileList;
+  const dirChanged = isBootstrap
+    || session._prevDirMtime !== dirMtime
+    || !prevFileList
+    || prevFileList.length === 0;
+
+  let files;
+  if (dirChanged) {
+    try {
+      files = fs.readdirSync(subagentsDir).filter(f => f.endsWith('.jsonl'));
+    } catch {
+      return;
+    }
+    session._prevDirMtime = dirMtime;
+    session._subFileList = files;
+  } else {
+    // Dir mtime unchanged and we saw files before — reuse cached list; no
+    // new files can have appeared.
+    files = prevFileList;
+  }
+
   // First walk for this session: pre-populate knownSubagents with every
   // existing file silently so we don't flood the renderer with spawn/complete
   // events for agents that already finished before Switchboard started watching.
   // Files modified in the last 60s get a normal lifecycle; older ones are
   // recorded as already-completed without IPC.
-  const isBootstrap = !session.knownSubagents;
   if (isBootstrap) {
     session.knownSubagents = new Map();
   }
@@ -52,12 +88,18 @@ function detectSubagentTransitions(sessionId, session, folderPath) {
     const agentId = m[1];
     const filePath = path.join(subagentsDir, file);
 
+    const known = session.knownSubagents.get(agentId);
+
+    // Already completed — nothing more to do, skip statSync.
+    if (known && known.completed) continue;
+
+    // Dir unchanged → no new entries can exist; skip statSync for unknown files.
+    if (!known && !dirChanged) continue;
+
     let stat;
     try { stat = fs.statSync(filePath); } catch { continue; }
     const mtimeMs = stat.mtimeMs;
 
-    const known = session.knownSubagents.get(agentId);
-
     if (!known) {
       if (isBootstrap) {
         // Cold-start initialization — record silently without firing IPC.
@@ -69,6 +111,21 @@ function detectSubagentTransitions(sessionId, session, folderPath) {
           completed: !looksAlive,
           _completedAt: looksAlive ? null : now,
         });
+        // Fix 2: emit a synthetic spawn for live bootstrap files so the
+        // renderer's liveSubagents / activeSubagents Maps have an entry and
+        // can correctly handle the subsequent subagent-completed event.
+        // The _bootstrap flag lets the renderer dedupe if it already has state.
+        if (looksAlive && mainWindow && !mainWindow.isDestroyed()) {
+          const meta = readSubagentMeta(filePath) || {};
+          log.info(`[subagent-spawn-bootstrap] parent=${sessionId} agentId=${agentId}`);
+          mainWindow.webContents.send('subagent-spawned', {
+            parentSessionId: sessionId,
+            agentId,
+            subagentType: meta.agentType || null,
+            description: meta.description || null,
+            _bootstrap: true,
+          });
+        }
         continue;
       }
       // First sighting post-bootstrap — real spawn event

test/dom-sidebar.test.js

@@ -125,6 +125,53 @@ test('renderProjects: empty project does not crash, sidebar stays empty', () =>
   }
 });
 
+test('renderProjects: orphan group has stable id for morphdom reconciliation', () => {
+  // Fix 3: orphanGroup.id = 'orphan-' + fId prevents morphdom from rebuilding
+  // the element on every render (which caused minor flicker).
+  const ctx = setupSidebarDom();
+  const projectPath = '/home/dev/myproj';
+  try {
+    ctx.sidebar.renderProjects([makeSampleProject({ projectPath })], true);
+
+    const orphanGroup = ctx.document.querySelector('.sidebar-orphan-subagents');
+    assert.ok(orphanGroup, 'orphan group must exist');
+    assert.ok(orphanGroup.id, 'orphan group must have an id for morphdom keying');
+    assert.match(orphanGroup.id, /^orphan-/, 'id must start with orphan-');
+
+    // Re-render should reuse the same DOM element (morphdom won't recreate it).
+    const idBefore = orphanGroup.id;
+    ctx.sidebar.renderProjects([makeSampleProject({ projectPath })], false);
+    const orphanGroupAfter = ctx.document.querySelector('.sidebar-orphan-subagents');
+    assert.equal(orphanGroupAfter.id, idBefore, 'orphan group id must be stable across re-renders');
+  } finally {
+    ctx.destroy();
+  }
+});
+
+test('getExpandedSubagents: one-time GC prunes stale session ids from localStorage', () => {
+  // Fix 4: on first call to getExpandedSubagents(), stale session ids (not in
+  // sessionMap) are removed from the stored set. This prevents unbounded growth
+  // of the key across long-lived instances.
+  const ctx = setupSidebarDom();
+  try {
+    // Populate sessionMap with one known session.
+    ctx.window.sessionMap.set('live-session', {});
+
+    // Seed localStorage with two entries: one stale, one live.
+    ctx.window.localStorage.setItem('expandedSubagents', JSON.stringify(['stale-id', 'live-session']));
+
+    // Trigger GC by calling getExpandedSubagents (via a render).
+    ctx.sidebar.renderProjects([makeSampleProject()], true);
+
+    // After GC, stale-id must have been removed.
+    const stored = JSON.parse(ctx.window.localStorage.getItem('expandedSubagents') || '[]');
+    assert.ok(!stored.includes('stale-id'), 'stale-id must be pruned by GC');
+    assert.ok(stored.includes('live-session'), 'live-session must survive GC');
+  } finally {
+    ctx.destroy();
+  }
+});
+
 test('renderProjects: result destructure is complete — re-render works (no stale closures)', () => {
   // This test would have caught bug #1 directly: if renderProjects's
   // destructure of processProjectSessions() result is incomplete and the

test/session-transitions.test.js

@@ -56,19 +56,31 @@ function seedAgents(folder, sessionId, agents) {
   return subDir;
 }
 
-test('bootstrap call with 5 pre-existing subagents emits zero events and populates the map', () => {
+test('bootstrap call with 5 pre-existing subagents: old agents silent, fresh agents get synthetic spawn', () => {
+  // Updated for Fix 2: looksAlive bootstrap files now emit subagent-spawned
+  // with _bootstrap:true so the renderer can track their lifecycle. Old
+  // agents (>60s) stay silent to avoid flooding on startup.
   const events = setupModule();
   const tmp = mkTmp();
   try {
     const sessionId = 'parent-session';
+    // 3 fresh (looksAlive) + 2 old (completed-at-boot)
     seedAgents(tmp, sessionId, [
-      { id: 'a1' }, { id: 'a2' }, { id: 'a3' }, { id: 'a4' }, { id: 'a5' },
+      { id: 'a1', ageMs: 120_000 }, // 2 min old — silent
+      { id: 'a2', ageMs: 120_000 }, // 2 min old — silent
+      { id: 'a3', ageMs: 5_000 },   // fresh — gets synthetic spawn
+      { id: 'a4', ageMs: 5_000 },   // fresh — gets synthetic spawn
+      { id: 'a5', ageMs: 5_000 },   // fresh — gets synthetic spawn
     ]);
 
     const session = {}; // knownSubagents undefined → bootstrap
     detectSubagentTransitions(sessionId, session, tmp);
 
-    assert.equal(events.length, 0, 'bootstrap must not emit IPC');
+    assert.equal(events.length, 3, 'exactly 3 synthetic spawns for fresh bootstrap agents');
+    for (const ev of events) {
+      assert.equal(ev.channel, 'subagent-spawned');
+      assert.equal(ev.payload._bootstrap, true, 'bootstrap spawn must carry _bootstrap flag');
+    }
     assert.ok(session.knownSubagents instanceof Map);
     assert.equal(session.knownSubagents.size, 5);
   } finally {
@@ -96,7 +108,8 @@ test('bootstrap marks an old-mtime agent (>60s) as completed: true', () => {
   }
 });
 
-test('bootstrap marks a fresh-mtime agent as completed: false (lifecycle continues)', () => {
+test('bootstrap marks a fresh-mtime agent as completed: false and emits synthetic spawn', () => {
+  // Fix 2: fresh bootstrap files now emit subagent-spawned with _bootstrap:true.
   const events = setupModule();
   const tmp = mkTmp();
   try {
@@ -106,7 +119,10 @@ test('bootstrap marks a fresh-mtime agent as completed: false (lifecycle continu
     const session = {};
     detectSubagentTransitions(sessionId, session, tmp);
 
-    assert.equal(events.length, 0, 'bootstrap must still be silent for fresh agents');
+    assert.equal(events.length, 1, 'bootstrap emits exactly 1 synthetic spawn for fresh agent');
+    assert.equal(events[0].channel, 'subagent-spawned');
+    assert.equal(events[0].payload._bootstrap, true);
+    assert.equal(events[0].payload.agentId, 'fresh');
     const entry = session.knownSubagents.get('fresh');
     assert.ok(entry);
     assert.equal(entry.completed, false);
@@ -142,17 +158,26 @@ test('post-bootstrap: a brand-new agent file emits exactly one subagent-spawned
   }
 });
 
-test('post-bootstrap with no new agents emits zero events (IPC-flood regression)', () => {
+test('post-bootstrap with no new agents emits no additional events (IPC-flood regression)', () => {
+  // Fix 2: bootstrap for fresh (ageMs:0) agents now emits synthetic spawns.
+  // The regression guard is that *subsequent* flushes with no new files must
+  // not re-emit — the event count must not increase after the first call.
   const events = setupModule();
   const tmp = mkTmp();
   try {
     const sessionId = 'parent';
-    seedAgents(tmp, sessionId, [{ id: 'a' }, { id: 'b' }, { id: 'c' }]);
+    // Use old agents (ageMs > 60s) so bootstrap stays silent — keeps the
+    // test focused purely on the "no subsequent events" regression.
+    seedAgents(tmp, sessionId, [
+      { id: 'a', ageMs: 120_000 },
+      { id: 'b', ageMs: 120_000 },
+      { id: 'c', ageMs: 120_000 },
+    ]);
 
     const session = {};
-    // Bootstrap absorbs all three silently
+    // Bootstrap absorbs all three silently (all old → no synthetic spawns)
     detectSubagentTransitions(sessionId, session, tmp);
-    assert.equal(events.length, 0);
+    assert.equal(events.length, 0, 'old-agent bootstrap must be silent');
 
     // Subsequent flushes with no new files must stay silent
     detectSubagentTransitions(sessionId, session, tmp);