diff --git a/public/sidebar.js b/public/sidebar.js index 7ca2bc42..9f68a63a 100644 --- a/public/sidebar.js +++ b/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'; diff --git a/session-transitions.js b/session-transitions.js index 6da11bc6..e47aea36 100644 --- a/session-transitions.js +++ b/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 diff --git a/test/dom-sidebar.test.js b/test/dom-sidebar.test.js index d7e4026c..ec16f478 100644 --- a/test/dom-sidebar.test.js +++ b/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 diff --git a/test/session-transitions.test.js b/test/session-transitions.test.js index a0366188..7bb5b444 100644 --- a/test/session-transitions.test.js +++ b/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);