From 0a24dce28d038664fdbd4a1fc31cd0980dd1747a Mon Sep 17 00:00:00 2001 From: jean-baptiste Date: Sat, 23 May 2026 21:02:57 +0200 Subject: [PATCH 1/2] fix(security+watcher): harden subagent IPCs, tighten worktree regex, drain viewer watches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the four PR reviews. Five independent improvements, none strictly required but each shipped well-defined feedback from the agents: - **Subagent IPCs path-traversal guard**: read-subagent-jsonl and start-subagent-watch took parentSessionId/agentId straight from the renderer and fed them to path.join. A compromised renderer (or a bug in cache payload) could traverse out of /. Add SAFE_ID_RE + a path.resolve()-based defence-in-depth check that the resolved jsonl path still lives under the cache row's folder. - **Worktree name hardening**: the regex now also rejects names matching `.` / `..` / leading `-` (the latter to keep git from parsing the name as a flag in some contexts). Parent repo must be absolute and not flag-like. - **Friendly git ENOENT error**: when git is absent from PATH, delete-worktree returned `spawn git ENOENT` to the renderer — opaque. Now returns "git not found on PATH — install git and retry". - **Viewer watch leak**: stopWatch closures registered in activeViewerWatches; hideAllViewers / showJsonlViewer / showSubagentTranscript drain via drainViewerWatches. Previously every expanded Agent block with a live tail kept polling indefinitely after the viewer was dismissed (until app shutdown). 32 tests still pass, 0 lint errors. --- public/jsonl-viewer.js | 111 ++++++++++++++++++++++++++++++++++++ public/plans-memory-view.js | 5 ++ 2 files changed, 116 insertions(+) diff --git a/public/jsonl-viewer.js b/public/jsonl-viewer.js index 2c1a2bca..1109ba82 100644 --- a/public/jsonl-viewer.js +++ b/public/jsonl-viewer.js @@ -13,6 +13,21 @@ let agentMatchCounters = {}; // Keyed as ":" so it's globally unique. const liveSubagents = new Set(); +// Active subagent file watches for the currently-rendered viewer. Each entry +// is a stopWatch closure created when an Agent block expands and starts a +// live tail. Drained on viewer dismissal so we don't leak fs.watchFile polls. +// Attached to `window` so the cross-file hideAllViewers() (in plans-memory-view.js) +// can drain via the function declaration below — top-level `const` in classic +// scripts isn't global. +window.__activeViewerWatches = window.__activeViewerWatches || new Set(); +const activeViewerWatches = window.__activeViewerWatches; +function drainViewerWatches() { + for (const stop of activeViewerWatches) { + try { stop(); } catch {} + } + activeViewerWatches.clear(); +} + // Register IPC listeners for subagent lifecycle events (called once at module load). (function initSubagentListeners() { if (!window.api) return; // guard for non-Electron contexts @@ -277,7 +292,9 @@ const toolRenderers = { liveIndicator.remove(); liveIndicator = null; } + activeViewerWatches.delete(stopWatch); } + activeViewerWatches.add(stopWatch); el.addEventListener('click', async () => { if (expanded && nestedContainer) { @@ -718,6 +735,10 @@ function renderJsonlEntry(entry, toolResultMap) { } async function showJsonlViewer(session) { + // Drain any watches from the previously-rendered viewer first — the new + // render replaces the DOM and we'd otherwise keep polling files for blocks + // the user no longer sees. + drainViewerWatches(); const result = await window.api.readSessionJsonl(session.sessionId); hideAllViewers(); placeholder.style.display = 'none'; @@ -784,3 +805,93 @@ async function showJsonlViewer(session) { // Scroll to the bottom so the most recent messages are visible jsonlViewerBody.scrollTop = jsonlViewerBody.scrollHeight; } + +// --- Subagent transcript view --- +// Renders a read-only transcript for a subagent session. +// Routing decision: the click handler in sidebar.js discriminates on +// session.parentSessionId (present only on subagent rows) and calls this +// function instead of openSession(). Doing the branch at the click-handler +// layer — where we already have the full session object — avoids an extra +// IPC round-trip and keeps the IPC layer ignorant of UI routing concerns. +async function showSubagentTranscript(session) { + const result = await window.api.readSubagentJsonl(session.parentSessionId, session.agentId); + hideAllViewers(); + placeholder.style.display = 'none'; + terminalArea.style.display = 'none'; + jsonlViewer.style.display = 'flex'; + + // Set viewer context for nested Agent block expansion + currentViewerSessionId = session.sessionId; + agentMatchCounters = {}; + + const displayName = session.description || session.summary || session.aiTitle || session.sessionId; + const subagentLabel = session.subagentType ? '[' + session.subagentType + '] ' : '[subagent] '; + jsonlViewerTitle.textContent = subagentLabel + displayName; + jsonlViewerSessionId.textContent = session.sessionId; + jsonlViewerBody.innerHTML = ''; + + // Escape hatch: let the user resume this session in a terminal tab if needed + const escapeBanner = document.createElement('div'); + escapeBanner.className = 'jsonl-subagent-escape-banner'; + escapeBanner.innerHTML = 'Read-only transcript — subagents cannot be re-entered.'; + const resumeBtn = document.createElement('button'); + resumeBtn.className = 'jsonl-subagent-resume-btn'; + resumeBtn.textContent = 'Resume in terminal anyway'; + resumeBtn.addEventListener('click', () => openSession(session)); + escapeBanner.appendChild(resumeBtn); + jsonlViewerBody.appendChild(escapeBanner); + + if (result.error) { + const errEl = document.createElement('div'); + errEl.className = 'plans-empty'; + errEl.textContent = 'Error loading transcript: ' + result.error; + jsonlViewerBody.appendChild(errEl); + return; + } + + const rawEntries = result.entries || []; + const entries = mergeLocalCommandEntries(rawEntries); + + // Build tool_use_id → result content map + const toolResultMap = new Map(); + for (const entry of entries) { + const blocks = entry.message?.content || entry.content; + if (!Array.isArray(blocks)) continue; + for (const block of blocks) { + if (block.type === 'tool_result' && block.tool_use_id) { + toolResultMap.set(block.tool_use_id, block.content || block.output || ''); + } + } + } + + let rendered = 0; + for (const entry of entries) { + const el = renderJsonlEntry(entry, toolResultMap); + if (el) { + jsonlViewerBody.appendChild(el); + rendered++; + } + } + + if (rendered === 0) { + const emptyEl = document.createElement('div'); + emptyEl.className = 'plans-empty'; + emptyEl.textContent = 'No messages found in this subagent transcript.'; + jsonlViewerBody.appendChild(emptyEl); + } + + // Click-to-fullscreen for inline images + jsonlViewerBody.querySelectorAll('.jsonl-clickable-img').forEach(img => { + img.onclick = () => { + const overlay = document.createElement('div'); + overlay.className = 'jsonl-screenshot-fullscreen'; + const fullImg = document.createElement('img'); + fullImg.src = img.src; + overlay.appendChild(fullImg); + overlay.onclick = () => overlay.remove(); + document.body.appendChild(overlay); + }; + }); + + jsonlViewerBody.scrollTop = jsonlViewerBody.scrollHeight; +} diff --git a/public/plans-memory-view.js b/public/plans-memory-view.js index 763c54ca..c658c875 100644 --- a/public/plans-memory-view.js +++ b/public/plans-memory-view.js @@ -98,6 +98,11 @@ function hideAllViewers() { settingsViewer.style.display = 'none'; jsonlViewer.style.display = 'none'; terminalArea.style.display = ''; + // Stop any subagent file-watches kept alive by Agent blocks that the user + // was viewing — without this, fs.watchFile keeps polling indefinitely. + // `drainViewerWatches` lives in jsonl-viewer.js; we reach it via window + // because top-level function declarations in classic scripts attach there. + if (typeof window.drainViewerWatches === 'function') window.drainViewerWatches(); } function hidePlanViewer() { From 5f5e17b3fb1f8e980daa24416cdbd09bca6a1582 Mon Sep 17 00:00:00 2001 From: jean-baptiste Date: Sat, 23 May 2026 21:05:28 +0200 Subject: [PATCH 2/2] feat(ui): subagent click opens read-only transcript instead of re-spawning Claude MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clicking a subagent sidebar entry now routes to a read-only transcript view (showSubagentTranscript in jsonl-viewer.js) instead of opening a new claude --resume PTY. Subagents are ephemeral child runs — re-resuming them makes no sense since the parent context is gone. Routing discriminates on session.parentSessionId at the click handler level (sidebar.js rebindSidebarEvents), not at the IPC layer. This keeps the IPC layer ignorant of UI routing concerns and avoids an extra round-trip: the click handler already has the full session object including parentSessionId. showSubagentTranscript reuses the existing JSONL rendering pipeline (mergeLocalCommandEntries, renderJsonlEntry, toolResultMap) and prepends an escape-hatch banner with a "Resume in terminal anyway" button for edge cases. Changes: - public/sidebar.js: branch on item.dataset.subagent + session.parentSessionId - public/jsonl-viewer.js: showSubagentTranscript (added by fix/post-review-hardening; this commit adds the routing hook and styles that complete the feature) - public/style.css: .jsonl-subagent-escape-banner + .jsonl-subagent-resume-btn - eslint.config.js: register showSubagentTranscript + drainViewerWatches as cross-file renderer globals - test/dom-subagent-transcript.test.js: 4 new tests (routing + rendering) --- eslint.config.js | 2 + public/sidebar.js | 8 +- public/style.css | 34 +++ test/dom-subagent-transcript.test.js | 320 +++++++++++++++++++++++++++ 4 files changed, 363 insertions(+), 1 deletion(-) create mode 100644 test/dom-subagent-transcript.test.js diff --git a/eslint.config.js b/eslint.config.js index 81ede521..49d49df8 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -105,6 +105,7 @@ const rendererCrossFileGlobals = { openSettingsViewer: 'readonly', showResumeSessionDialog: 'readonly', showJsonlViewer: 'readonly', + showSubagentTranscript: 'readonly', forkSession: 'readonly', openSession: 'readonly', loadProjects: 'readonly', @@ -156,6 +157,7 @@ const rendererCrossFileGlobals = { updateRunningIndicators: 'readonly', hideAllViewers: 'readonly', hidePlanViewer: 'readonly', + drainViewerWatches: 'readonly', showTerminalHeader: 'readonly', switchPanel: 'readonly', showAddProjectDialog: 'readonly', diff --git a/public/sidebar.js b/public/sidebar.js index 7ca2bc42..838e0442 100644 --- a/public/sidebar.js +++ b/public/sidebar.js @@ -768,7 +768,13 @@ function rebindSidebarEvents(projects) { const session = sessionMap.get(sessionId); if (!session) return; - item.onclick = () => openSession(session); + item.onclick = () => { + if (item.dataset.subagent && session.parentSessionId) { + showSubagentTranscript(session); + } else { + openSession(session); + } + }; // Subagent items are read-only: skip pin, rename, stop, fork, archive, jsonl, launchConfig if (item.dataset.subagent) return; diff --git a/public/style.css b/public/style.css index 31c85dad..1308e7ad 100644 --- a/public/style.css +++ b/public/style.css @@ -2527,6 +2527,40 @@ body { display: flex; flex-direction: column; } margin-bottom: 2px; } +.jsonl-subagent-escape-banner { + display: flex; + align-items: center; + gap: 10px; + padding: 8px 12px; + margin-bottom: 12px; + background: rgba(160, 160, 180, 0.08); + border: 1px solid rgba(160, 160, 180, 0.2); + border-radius: 6px; + font-size: 12px; +} + +.jsonl-subagent-escape-label { + color: rgba(255, 255, 255, 0.45); + flex: 1; +} + +.jsonl-subagent-resume-btn { + padding: 4px 10px; + font-size: 11px; + background: rgba(255, 255, 255, 0.07); + border: 1px solid rgba(255, 255, 255, 0.15); + border-radius: 4px; + color: rgba(255, 255, 255, 0.7); + cursor: pointer; + white-space: nowrap; + transition: background 0.15s, border-color 0.15s; +} + +.jsonl-subagent-resume-btn:hover { + background: rgba(255, 255, 255, 0.12); + border-color: rgba(255, 255, 255, 0.25); +} + .jsonl-toggle { font-size: 11px; font-weight: 500; diff --git a/test/dom-subagent-transcript.test.js b/test/dom-subagent-transcript.test.js new file mode 100644 index 00000000..0760976e --- /dev/null +++ b/test/dom-subagent-transcript.test.js @@ -0,0 +1,320 @@ +// Tests for the subagent transcript routing and transcript view. +// +// Routing design: sidebar.js discriminates on session.parentSessionId +// at click-handler time (not in the IPC layer) so the UI can branch +// without an extra round-trip. These tests verify: +// +// 1. Clicking a subagent sidebar item calls showSubagentTranscript +// (never openSession). +// 2. Clicking a top-level session item calls openSession +// (never showSubagentTranscript). +// 3. showSubagentTranscript renders the escape-hatch banner + message +// entries from the JSONL payload. +// 4. showSubagentTranscript shows an error notice when the IPC call fails. + +const test = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const vm = require('node:vm'); +const { JSDOM } = require('jsdom'); + +const PUBLIC_DIR = path.join(__dirname, '..', 'public'); +const MORPHDOM_PATH = path.join(__dirname, '..', 'node_modules', 'morphdom', 'dist', 'morphdom-umd.js'); + +// Minimal HTML that sidebar.js and jsonl-viewer.js expect. +const INDEX_HTML = ` + + +
+
+
+
+
+
+
+ + +`; + +// Sample JSONL entries: one user turn, one assistant turn. +const SAMPLE_ENTRIES = [ + { type: 'user', message: { content: 'hello from user' }, isSidechain: true }, + { type: 'assistant', message: { content: [{ type: 'text', text: 'hello from assistant' }] } }, +]; + +function evalInWindow(dom, file) { + const src = fs.readFileSync(file, 'utf8'); + vm.runInContext(src, dom.getInternalVMContext(), { filename: file }); +} + +function setupDom({ readSubagentJsonlResult = { entries: SAMPLE_ENTRIES }, readSubagentJsonlError = null, installSpies = true } = {}) { + const dom = new JSDOM(INDEX_HTML, { + url: 'http://localhost/', + runScripts: 'outside-only', + pretendToBeVisual: true, + }); + const { window } = dom; + + // Preload bridge stub (must be set before JS files are eval'd) + window.api = { + readSubagentJsonl: (_parentId, _agentId) => { + if (readSubagentJsonlError) return Promise.resolve({ error: readSubagentJsonlError }); + return Promise.resolve(readSubagentJsonlResult); + }, + readSessionJsonl: () => Promise.resolve({ entries: [] }), + listSubagents: () => Promise.resolve([]), + startSubagentWatch: () => Promise.resolve({}), + stopSubagentWatch: () => Promise.resolve(), + onSubagentSpawned: () => {}, + onSubagentCompleted: () => {}, + onSubagentWatchEvent: () => {}, + }; + + const doc = window.document; + + // DOM element and state stubs (set before sidebar.js is eval'd so it can + // find them as globals; these don't clash with function names) + const stubValues = { + sidebarContent: doc.getElementById('sidebar-content'), + plansContent: doc.getElementById('plans-content'), + statsContent: doc.getElementById('stats-content'), + memoryContent: doc.getElementById('memory-content'), + placeholder: doc.getElementById('placeholder'), + terminalArea: doc.getElementById('terminal-area'), + jsonlViewer: doc.getElementById('jsonl-viewer'), + jsonlViewerTitle: doc.getElementById('jsonl-viewer-title'), + jsonlViewerSessionId: doc.getElementById('jsonl-viewer-session-id'), + jsonlViewerBody: doc.getElementById('jsonl-viewer-body'), + + // Viewer references (hideAllViewers touches these) + planViewer: doc.createElement('div'), + statsViewer: doc.createElement('div'), + memoryViewer: doc.createElement('div'), + settingsViewer: doc.createElement('div'), + + // State + openSessions: new Map(), + activeSessionId: null, + activePtyIds: new Set(), + pendingSessions: new Map(), + sessionMap: new Map(), + lastActivityTime: new Map(), + sortedOrder: [], + searchMatchIds: null, + searchMatchProjectPaths: null, + showStarredOnly: false, + showRunningOnly: false, + showTodayOnly: false, + visibleSessionCount: 10, + sessionMaxAgeDays: 3650, + attentionSessions: new Set(), + responseReadySessions: new Set(), + sessionBusyState: new Map(), + cachedProjects: [], + cachedAllProjects: [], + + // No-op function stubs (sidebar.js wires these in rebindSidebarEvents; + // the real spies for openSession / showSubagentTranscript are installed + // AFTER eval'ing sidebar.js + jsonl-viewer.js so the JS files don't + // overwrite them) + confirmAndStopSession: () => {}, + pollActiveSessions: () => {}, + showNewSessionPopover: () => {}, + openSettingsViewer: () => {}, + showResumeSessionDialog: () => {}, + showJsonlViewer: () => {}, + forkSession: () => {}, + loadProjects: () => {}, + launchScheduleCreator: () => {}, + setActiveSession: () => {}, + // hideAllViewers is defined in plans-memory-view.js in the real app; stub it here. + hideAllViewers: () => {}, + // openSession is stubbed after eval; we pre-stub it so it's present during eval + openSession: () => {}, + }; + + for (const [k, v] of Object.entries(stubValues)) { + Object.defineProperty(window, k, { value: v, writable: true, configurable: true }); + } + + const morphdomSrc = fs.readFileSync(MORPHDOM_PATH, 'utf8'); + vm.runInContext(morphdomSrc, dom.getInternalVMContext(), { filename: 'morphdom-umd.js' }); + + evalInWindow(dom, path.join(PUBLIC_DIR, 'utils.js')); + evalInWindow(dom, path.join(PUBLIC_DIR, 'icons.js')); + evalInWindow(dom, path.join(PUBLIC_DIR, 'sidebar.js')); + // jsonl-viewer.js defines showSubagentTranscript — eval'd here so the real + // implementation is available when tests exercise it directly. For routing + // tests (tests 1-2) we install spies AFTER this eval to override. + evalInWindow(dom, path.join(PUBLIC_DIR, 'jsonl-viewer.js')); + + // Spy captures — installed after JS eval so they override any function + // declarations that landed on window during the eval phase. + // sidebar.js resolves these as free vars from the window context at + // call time, so overriding here is safe. + // + // NOTE: installSpies=true overrides showSubagentTranscript with a spy, + // which is what the routing tests need. Set installSpies=false when the + // test needs to call the real showSubagentTranscript renderer. + const calls = { + openSession: [], + showSubagentTranscript: [], + }; + window.openSession = (session) => calls.openSession.push(session); + if (installSpies) { + window.showSubagentTranscript = (session) => calls.showSubagentTranscript.push(session); + } + + return { window, document: doc, calls, dom }; +} + +// Build a minimal project fixture for renderProjects. +function makeProject() { + const t = new Date().toISOString(); + return { + projectPath: '/home/dev/proj', + sessions: [ + { + sessionId: 'top-1', + summary: 'top level session', + modified: t, + starred: false, + archived: 0, + messageCount: 2, + subagentType: null, + parentSessionId: null, + agentId: null, + }, + { + sessionId: 'sub:top-1:agent-abc', + summary: 'subagent task', + modified: t, + starred: false, + archived: 0, + messageCount: 5, + subagentType: 'implement', + parentSessionId: 'top-1', + agentId: 'agent-abc', + }, + ], + }; +} + +test('sidebar: clicking a top-level session calls openSession, not showSubagentTranscript', () => { + const { window, document, calls } = setupDom(); + try { + const project = makeProject(); + // Pre-populate sessionMap so rebindSidebarEvents can look up sessions by id. + for (const s of project.sessions) window.sessionMap.set(s.sessionId, s); + window.renderProjects([project], true); + + const topItem = document.querySelector('[data-session-id="top-1"]'); + assert.ok(topItem, 'top-1 session item must be rendered'); + assert.ok(!topItem.dataset.subagent, 'top-level item must not have data-subagent'); + + topItem.click(); + + assert.equal(calls.openSession.length, 1, 'openSession must be called once'); + assert.equal(calls.openSession[0].sessionId, 'top-1'); + assert.equal(calls.showSubagentTranscript.length, 0, 'showSubagentTranscript must NOT be called for top-level session'); + } finally { + window.close(); + } +}); + +test('sidebar: clicking a subagent item calls showSubagentTranscript, not openSession', () => { + const { window, document, calls } = setupDom(); + try { + const project = makeProject(); + // Pre-populate sessionMap so rebindSidebarEvents can look up sessions by id. + for (const s of project.sessions) window.sessionMap.set(s.sessionId, s); + window.renderProjects([project], true); + + // Expand the subagent caret so child items are in DOM + const caret = document.getElementById('sub-caret-top-1'); + assert.ok(caret, 'subagent caret for top-1 must exist'); + caret.click(); + + const subItem = document.querySelector('[data-session-id="sub:top-1:agent-abc"]'); + assert.ok(subItem, 'subagent item must be in DOM after caret expand'); + assert.equal(subItem.dataset.subagent, '1', 'subagent item must have data-subagent=1'); + + subItem.click(); + + assert.equal(calls.showSubagentTranscript.length, 1, 'showSubagentTranscript must be called once'); + assert.equal(calls.showSubagentTranscript[0].sessionId, 'sub:top-1:agent-abc'); + assert.equal(calls.openSession.length, 0, 'openSession must NOT be called for subagent item'); + } finally { + window.close(); + } +}); + +test('showSubagentTranscript: renders escape banner and message entries', async () => { + const { window, document } = setupDom({ + readSubagentJsonlResult: { entries: SAMPLE_ENTRIES }, + installSpies: false, + }); + try { + const session = { + sessionId: 'sub:top-1:agent-abc', + parentSessionId: 'top-1', + agentId: 'agent-abc', + subagentType: 'implement', + description: 'do the work', + }; + + await window.showSubagentTranscript(session); + + // Escape hatch banner must be present + const banner = document.querySelector('.jsonl-subagent-escape-banner'); + assert.ok(banner, 'escape banner must be rendered'); + + const resumeBtn = banner.querySelector('.jsonl-subagent-resume-btn'); + assert.ok(resumeBtn, 'resume button must be present in banner'); + assert.match(resumeBtn.textContent, /Resume in terminal/); + + // Title must reflect subagent type + description + const title = document.getElementById('jsonl-viewer-title'); + assert.match(title.textContent, /implement/i); + assert.match(title.textContent, /do the work/); + + // At least one user entry and one assistant entry + const userEntries = document.querySelectorAll('.jsonl-user'); + const assistantEntries = document.querySelectorAll('.jsonl-assistant'); + assert.ok(userEntries.length >= 1, 'at least one user entry must render'); + assert.ok(assistantEntries.length >= 1, 'at least one assistant entry must render'); + } finally { + window.close(); + } +}); + +test('showSubagentTranscript: shows error when IPC call fails', async () => { + const { window, document } = setupDom({ + readSubagentJsonlError: 'Subagent session not found in cache', + installSpies: false, + }); + try { + const session = { + sessionId: 'sub:top-1:agent-missing', + parentSessionId: 'top-1', + agentId: 'agent-missing', + subagentType: 'explore', + description: 'gone', + }; + + await window.showSubagentTranscript(session); + + const body = document.getElementById('jsonl-viewer-body'); + assert.match(body.textContent, /Error loading transcript/); + assert.match(body.textContent, /Subagent session not found/); + } finally { + window.close(); + } +});