Repository navigation
perf: stop the 5s active-sessions poll from blocking the event loop - #289
Closed
vakovalskii wants to merge 1 commit into
Closed
vakovalskii wants to merge 1 commit into
vakovalskii wants to merge 1 commit into
Conversation
getActiveSessions() backs GET /api/active, which the dashboard polls every 5s.
For Qwen agents it called findQwenSessionByPid(), which ran
execSync('lsof -a -p <pid> -Fn') — with a 2000ms timeout, once per pid — inline.
The server, the HTTP API and the browser-terminal WebSocket share one process
and one event loop, so that stalled the pty data pump: typing in the terminal
froze for up to two seconds per running agent, every five seconds. (7.15.0 moved
the other ps/lsof polls off the loop; this one was missed.)
lsof is kept — it is the only signal that can tell two sessions in the same
folder apart — but it now runs through _execFileAsync off the loop and callers
read a 30s per-pid cache, falling back to the existing cwd match until the first
refresh lands. Refreshes are de-duplicated per pid and the cache is bounded.
Also switches to argv form, so the pid is no longer interpolated into a shell
command string.
Note the measurement trap: on a machine with no running Qwen agent the hot loop
never executes, so timing this looks green either way. The added test asserts
the property structurally instead, and was verified to fail when execSync is
reintroduced.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vakovalskii
added a commit
that referenced
this pull request
Oct 2, 2026
- merge perf/unblock-active-poll (#289) and dapsychyoo's desktop node resolve fix (#290) - resolve #288 x #289 lsof conflict in favor of #289's async argv call; retarget the argv-safety test at _refreshQwenOpenFiles - changelog entries for #289 #290 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
vakovalskii
added a commit
that referenced
this pull request
Oct 2, 2026
…, a11y + security fixes, desktop nvm launch (merges #282 #283 #285 #286 #287 #288 #289 #290) (#284) * fix: unbalanced <div> broke Projects launcher card layout The missing-folder-detection refactor added the launcher-card-actions wrapper only for the missing-folder branch, but left the pre-existing closing </div> in the normal (folder-exists) branch without ever opening it. The unbalanced markup made every action button/select render as a direct flex-column child of .launcher-card, stretching each one to full card width on its own row instead of a compact action row. Also brings the <select> terminal-panes control in line with the button sizing in that row, and pushes the rarely-used Remove control to the end of the row with quieter styling so it doesn't compete visually with New/Last. * feat: unify local + external agents in the Running agents sidebar tree The tree only ever showed agents in external native terminals — an agent running inside codbash's own Workspace pane silently disappeared from it, so a project with both an in-app Claude session and an iTerm one only showed half the picture. That asymmetry (plus dimmed "idle" rows reading as ambiguous "maybe gone") is what read as messy/hard-to-trust grouping. - _wsRunningGroups(mode) replaces _wsRunningByProject(): still built from activeSessions (a live ps-scan re-polled every 5s, so an exited process is simply absent next tick — no separate "ghost" filtering needed), but now includes local (in-codbash) agents alongside external ones instead of dropping them. - Grouping is now a per-browser toggle (project → agent, or agent → project), persisted to localStorage['codedash-running-group'], exposed as a compact 2-button segmented control in the tree header. - Rows are color-coded by where they run: blue dot = inside codbash, orange dot = external native terminal (iTerm/Terminal.app/Warp/cmux). Idle still dims to muted gray, on top of either color. - jumpToRunningAgent now dispatches on the `local` tag: local agents jump straight to their Workspace tab/pane (found by matching cwd against the live pane list); external agents keep the existing /api/focus path. Updates the design doc and CLAUDE.md (which documented "external only" as intentional) and the existing test suite for the new function names/shape. * fix: make the Running agents tree an actual 3-level hierarchy The first pass grouped project/agent as a flat 2-level list — one header, then every session underneath as a same-labeled row ("Claude", "Claude", …). That read as alternating noise (session, agent, session, agent) rather than a real hierarchy, especially once a project had 2+ live sessions of the same agent. _wsRunningTree(mode) replaces _wsRunningGroups(mode): a real 3-level tree — outer group -> inner group -> individual sessions — built via a small generic _wsGroupBy() applied twice. mode:'project' nests project -> agent -> sessions; mode:'agent' nests the mirror, agent -> project -> sessions (re-parenting the whole tree, not just relabeling a flat list). A project+agent pair with exactly one live session collapses its leaf row into the subgroup row itself (.ws-run-leaf) — no redundant single-child row. Once it holds 2+ sessions, the subgroup renders as a real subheader and each session gets its own leaf row, labeled via _wsSessionLeafLabel (a live pane's name if local and matched, else the session-id prefix — same convention as the session cards' "Resume last session (id12345)" — else a bare pid) so same-project/same-agent sessions read as genuinely distinct instead of the same label repeated. * feat: make the Running agents tree an accordion, collapsed by default Top-level (L1) groups — project in project-mode, agent kind in agent-mode — now start collapsed. Clicking a header expands it in place to reveal its agent/project subgroups and individual sessions; the same accordion logic applies to both grouping directions. - _wsRunExpanded (in-memory, keyed `mode|groupKey`) tracks open/closed per group so switching the Project/Agent toggle doesn't share expand state across the two different hierarchies. - _wsToggleRunGroup flips a `collapsed` class directly on the group's wrapper DOM node instead of forcing a full tree rebuild — cheap, and a later rebuild (triggered by a real activeSessions change) re-reads _wsRunExpanded so the user's open/closed choices survive it. - The L1 header's click now means "toggle" rather than "jump to the first session" — with a header disclosing multiple children there's no single unambiguous default target, so the jump action lives entirely on leaf rows (.ws-run-l2.ws-run-leaf / .ws-run-l3), unchanged from before. - Basic keyboard support: role="button", aria-expanded, Enter/Space to toggle. * release: 7.18.0 — unified Running agents tree + Projects launcher layout fix Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: accessibility pass — keyboard reachability and dialog semantics Closes the gaps found in a UX audit across Overview, Projects, Sessions, and Workspace: real keyboard focus and screen-reader semantics were inconsistent — some controls had them, most didn't. - Overview: .ov-card gets a visible :focus-visible outline (it was already a real <button>, just missing the ring). - Calendar: day cells are now real <button>s (were unfocusable divs) with aria-pressed reflecting selection; the popup is a proper role="dialog" aria-modal with Escape-to-close and focus returned to the date button on close (toggleCalendar/closeCalendar/onCalendarPopupKeydown). - Add Project modal: addProjectSwitchTab() now updates aria-selected and roving tabindex when switching tabs (previously only toggled the visual .active class, so a screen reader kept announcing the first tab as selected forever); added Left/Right/Home/End arrow-key navigation matching the existing Projects/History tablist pattern. - Session cards (.card / .list-row / .qa-item): were plain divs with an onclick and nested buttons — unreachable by Tab entirely. Added tabindex, a descriptive aria-label, :focus-visible styling, and a shared onCardKeydown handler (Enter/Space triggers the card's own click, guarded so it doesn't also fire for a nested button/checkbox's own key handling). - Detail panel: role="dialog" aria-modal, wired into the existing _installModalFocusTrap/_uninstallModalFocusTrap helper (same one Add Project / Projects Settings use) for Tab-trapping and focus-return on close. Hardened _uninstallModalFocusTrap to skip focus-return when the captured element has since been detached from the DOM (a background poll/re-render can replace it while the modal is open) rather than silently stranding focus on the modal's own now-hidden close button. - Workspace: aria-label on the pane close button, tab close button, and the three launch/layout <select> menus that only had a title before. Split-pane resize handles (.ws-resizer) are now real ARIA separators — tabindex, role="separator", aria-orientation, and arrow-key resize (mirrors the pointer-drag math without triggering a full handle rebuild mid-interaction, which would destroy and unfocus the handle the user is actively adjusting). * chore(release): note the accessibility pass (#285) in 7.18.0 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(security): never vend the LLM API key to the browser GET /api/llm-config returned the stored API key verbatim on every Settings load — it sat unmasked in each network response/devtools log and the frontend dropped it straight into the password input's value. The GitHub endpoints already follow the right rule ("Never vend raw tokens to the browser", /api/github/profile strips token/repoToken); the LLM endpoint was the one exception. - GET /api/llm-config now returns { model, url, hasKey, keyHint } — keyHint is a ••••-masked last-4 so the user can tell which key is stored without the secret ever reaching the DOM. - Because the key no longer round-trips, POST with an empty apiKey field means "keep the stored key" (otherwise every URL/model tweak would silently wipe it); an explicit { clearApiKey: true } removes it. - saveLLMConfig writes ~/.claude/codedash-llm.json with mode 0600 and chmods a pre-existing world-readable file from older versions. - Frontend shows the stored-key state via the input placeholder ("••••1234 (saved — type to replace)"), never pre-fills the value; autocomplete="new-password" keeps browser autofill managers out. saveLLMSettings now also surfaces save failures instead of toasting "saved" unconditionally. - leaderboard.js: rel="noopener noreferrer" on all target=_blank links (reverse-tabnabbing). Verified live: GET carries no apiKey field, hint shows ••••1234, file lands -rw-------, an empty-key save preserves the key, clearApiKey clears it. Source-contract tests added (test/llm-config-redaction.test.js). * perf: stop the search index from freezing the event loop buildSearchIndex re-parsed every session with detail — a findSessionFile lookup plus a full detail load (sync fs read + JSON.parse per line) each — in one synchronous tick, on whichever request happened to miss the 60s cache. Measured on a 900-session history: a 3.6s hard stall of the event loop, which also stalls every other API call and the terminal WebSocket data pump. The analytics job already solved this exact problem with chunk+yield (_scheduleAnalyticsRecompute); search never got the same treatment. Chunking per session alone wasn't enough: real histories have a heavy tail (median session ~0.1MB here, but codex transcripts up to 78MB), and readLines slurps the whole file into a string before splitting — one such session blocks for seconds regardless of the outer chunk size. - buildSearchIndex is async, processes sessions in small chunks (8 — each item is far heavier than the computeSessionCost calls analytics batches 80-at-a-time) and yields via setImmediate between them. - JSONL sessions over SEARCH_STREAM_THRESHOLD (4MB) are read line-by-line off a stream that yields every 2000 lines, so one huge transcript can't block either. Nothing is truncated — this only changes *when* the work happens, not what gets indexed. - getSearchIndex is stale-while-revalidate (mirroring getCostAnalytics): a >60s-old index is still overwhelmingly accurate, so it's served instantly while the refresh runs in the background. Only a genuine cold start awaits. - Concurrent rebuilds dedupe into one in-flight job, so a burst of searches during a rebuild no longer queues N full-history scans. - Collapsed six near-identical per-format if/else branches into a SEARCH_DETAIL_LOADERS lookup table, and the two JSONL readers now share one per-line parser so they can't drift. Snippet cap is a named constant instead of a repeated 500. - searchFullText is now async; /api/search and the `codbash search` CLI command updated accordingly. Measured before/after on the same 900-session history: worst event-loop stall 3622ms -> 60ms cold build 3.6s -> 3.0s warm search ~14ms (unchanged) Search results verified byte-identical to origin/main across 6 queries (496 result rows) via a git-worktree differential run. * fix: accessible GitHub auth dialog + argv-form exec at three sites Clears the last two items from the UX/server audit backlog. GitHub device-code modal (leaderboard): - Was a bare div with no dialog semantics. Now role="dialog" + aria-modal + aria-labelledby, focus moved inside on open, Tab trapped within it, Escape closes, and focus returns to whatever opened it (guarded against the node having been replaced by render()). - The status line is aria-live so "Code expired" / "Connection error" is announced rather than silently repainted. - The poll loop had a bare `catch {}`: every network failure was swallowed and the user watched "Waiting for authorization..." until the tries ran out, with nothing ever indicating a problem. It now counts consecutive failures (3 → surface and stop), reports a server-sent `error` field, and honours `slow_down` per RFC 8628 §3.5 — matching pollRepoScopeOnce() in app.js, which already did all this. - Cancel is a real handler instead of an inline this.parentElement.parentElement DOM walk. Shell-string exec → argv form: - data.js findQwenSessionByPid interpolated a pid into an `lsof …` shell string; the sibling lsof call in getActiveSessions already used execFileSync, so this was the outlier. stderr now suppressed via stdio rather than a `2>/dev/null` that required a shell. - migrate.js built `find "$path" … | wc -l` from a home path; now argv form with the count done in JS (which also drops the pipe). - Removed the adjacent `du -sb … || du -sk …` call entirely: its result (totalSize) was computed and never printed. It was also wrong — BSD du has no -b, so on macOS it always fell through to -sk and added KILOBYTES to a byte total. Dead code hiding a unit bug behind a shell `||`. Verified: modal semantics/Tab-trap/Escape/focus-return driven in a real browser; lsof argv vs shell forms confirmed to find the same session on a process holding a matching file open; find argv form matches the old pipe count on paths containing quotes, $ and backticks; `codbash export` end-to-end reports 934 files against 934 actually in the archive. * chore(release): note #285 #286 #287 #288 in 7.18.0 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * perf: stop the 5s active-sessions poll from blocking the event loop getActiveSessions() backs GET /api/active, which the dashboard polls every 5s. For Qwen agents it called findQwenSessionByPid(), which ran execSync('lsof -a -p <pid> -Fn') — with a 2000ms timeout, once per pid — inline. The server, the HTTP API and the browser-terminal WebSocket share one process and one event loop, so that stalled the pty data pump: typing in the terminal froze for up to two seconds per running agent, every five seconds. (7.15.0 moved the other ps/lsof polls off the loop; this one was missed.) lsof is kept — it is the only signal that can tell two sessions in the same folder apart — but it now runs through _execFileAsync off the loop and callers read a 30s per-pid cache, falling back to the existing cwd match until the first refresh lands. Refreshes are de-duplicated per pid and the cache is bounded. Also switches to argv form, so the pid is no longer interpolated into a shell command string. Note the measurement trap: on a machine with no running Qwen agent the hot loop never executes, so timing this looks green either way. The added test asserts the property structurally instead, and was verified to fail when execSync is reintroduced. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(desktop): rc-noise-proof node resolution + spawn error dialog A Finder launch crashed with 'Uncaught Exception: spawn node ENOENT' on machines where node is installed only via nvm. The login-shell fallback took the LAST line of stdout from `$SHELL -lic 'command -v node'`, but rc files freely print to stdout — iTerm2 shell integration emits OSC 1337 escape sequences without a trailing newline, so the node path came back glued to escape garbage, existsSync() rejected it, and the app fell through to bare 'node', which the stripped GUI PATH cannot find. - Extract resolution into desktop/node-resolve.js (unit-testable; main.js requires electron) and probe the login shell with the same \x01-sentinel technique src/shell-path.js already uses, immune to rc noise. - Scan ~/.nvm/versions/node/*/bin/node (highest first) before spawning a shell at all — fixes nvm-only machines even when the probe fails, and skips the ~1s shell spawn on them. - Handle spawn 'error' in startServer: a clear dialog (install Node 18+ or set CODBASH_NODE) instead of an uncaught exception. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F3q8EdVFkKPTtSGhAU1Wxf * fix(desktop): address review — reuse shell-path probe, honor nvm default, guard dialogs - Drop the duplicated sentinel probe: reuse src/shell-path.js's cached, logged captureLoginShellPath() (dev: ../src, packaged: extraResources app/src) and scan its PATH for node. - listNvmNodes: put ~/.nvm/alias/default (exact or prefix form) first so the scan never silently overrides the user's default with a newer install (prebuilt node-pty ABI may not match); drop the double stat — the candidate loop owns the existence check via the injected seam. - startServer 'error' handler: return early during a quit in progress; in SMOKE mode fail loudly with a nonzero exit instead of a silent green run. - whenReady catch: skip the generic 'failed to start' dialog when the spawn error dialog already told the user what's wrong. - Tests: scope injected existsSync fakes to the fake home so the suite doesn't depend on the host machine's /usr/local/bin/node (CI runners all have one). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F3q8EdVFkKPTtSGhAU1Wxf * fix(search): background index rebuild can't crash the server Move findSessionFile inside _indexSession's try and catch the fire-and-forget stale-while-revalidate rebuild, so a readdirSync error skips a session instead of raising an unhandled rejection. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(a11y): no double-open on Enter, no orphaned focus trap, Escape closes stacked confirm - onCardKeydown stops propagation so the global handler doesn't also run openFocusedCard() - _installModalFocusTrap removes a previous trap before installing a new one (an orphan swallowed every Escape, incl. in Workspace terminals) - Escape while confirmOverlay is up closes the confirm, not the detail - focusable list recomputed per Tab (messages load after open) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(running-agents): jump local agents to their own pane by ptyPid Matching by cwd alone sent every in-app agent in one project to the first pane. _tagLocalAgents now returns the pane shell pid the agent descends from; the tree matches pane.pid first, cwd as fallback. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore(release): fold #289 #290 + review fixes into 7.18.0 - merge perf/unblock-active-poll (#289) and dapsychyoo's desktop node resolve fix (#290) - resolve #288 x #289 lsof conflict in favor of #289's async argv call; retarget the argv-safety test at _refreshQwenOpenFiles - changelog entries for #289 #290 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: NovakPAai <novakpavela@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: John Doe <johndoe@example.com>
Owner
Author
5 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Второй из трёх источников заморозки терминала, которые мы замерили. #287 (в RC #284) снял индекс поиска — этот про опрос активных сессий.
Проблема
getActiveSessions()обслуживаетGET /api/active, который дашборд опрашивает каждые 5 секунд. Для Qwen-агентов он звалfindQwenSessionByPid(), а тот выполнял— синхронно, на каждый pid, прямо на этом пути.
Сервер, HTTP API и WebSocket браузерного терминала живут в одном процессе и одном event loop. Значит ввод в терминале замирал до двух секунд на каждого запущенного агента, каждые пять секунд. В 7.15.0 остальные опросы
ps/lsofвынесли с цикла, а этот пропустили.Решение
lsofсохранён — это единственный сигнал, который отличает две сессии в одной папке (сопоставление поcwdтут бессильно). Убрана именно блокировка:_execFileAsyncвне цикла;cwd— тот же ответ, что дал быlsofдля одиночной сессии в папке;Про измерения — важная оговорка
Замерять это временем бесполезно: на машине без запущенного Qwen-агента горячий цикл не выполняется вообще, и любой замер выходит зелёным независимо от кода. Я сам на этом обжёгся раньше в этой же функции.
Поэтому тест проверяет свойство структурно, и я убедился, что он не пустой: временно вернул
execSync— тест упал (4 passed / 1 failed), откатил — снова 5/5.Проверка
node --test test/*.test.js→ 261 passed / 0 failed / 2 skippedtest/active-poll-nonblocking.test.js— 5 проверок: нет синхронных вызовов ни вfindQwenSessionByPid, ни вgetActiveSessions;lsofасинхронный и в argv-форме; дедупликация обновлений; ограниченность кешаЧто остаётся
Третий источник —
loadSessions()(648 мс с прогретым кешем, 11.9 с на холодную). Его правку намеренно не тащу сюда: она требует поэтапной пересборки в духе того, что #287 сделал для индекса поиска, и опирается на его помощники. Сделаю после того, как RC #284 вольётся, чтобы не разводить конфликты вdata.js.🤖 Generated with Claude Code