Skip to content

Build first-class dreb web dashboard (foundation) - #321

Merged
m-aebrer merged 30 commits into
masterfrom
feature/issue-307-dashboard-foundation
Jul 9, 2026
Merged

m-aebrer merged 30 commits into
masterfrom
feature/issue-307-dashboard-foundation

Conversation

@m-aebrer

@m-aebrer m-aebrer commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

Closes #307

First-party dreb web dashboard — foundation PR. Implements the accepted UX/UI design from the design phase (issue 311): dashboard server (loopback default, Tailscale remote opt-in, fail-closed auth, SSE pipeline, host-wide file API), fleet overview, full-parity session view, live subagent observability (including the RPC registry exposure + event relay in coding-agent), files tab, pairing + settings, on the tokens.css visual language.

Design artifacts land on this branch by merging the design PR (320) here as the first act; they iterate on this branch from that point (SPEC.md section 7 is the scope contract, section 9 the review contract).

Implementation plan posted as a comment below.

m-aebrer and others added 6 commits July 7, 2026 10:04
…klist, spec

Design deliverables for the dreb web dashboard (design issue, PR 320):

- tokens.css: design language forked from the approved gallery template
  (IBM Plex Mono, pure black/white + auto dark mode, hairline borders,
  status chips as the only accent color)
- mockups/: six static screens (fleet-overview, session-view, pairing,
  files, settings, tree) sharing tokens.css
- capture.mjs + screenshots/: Playwright captures at desktop/mobile/dark,
  all vision-reviewed
- PARITY.md: TUI feature parity checklist, ground-truth re-verified
  against source (21 built-in commands, ~74 keybindings, 19 event types)
- SPEC.md: IA, flows, responsive plan, two-mode security UX, foundation
  scope, SolidJS recommendation, acceptance criteria
Maintainer feedback on PR 320:

1. Files screen reworked from project-scoped to host-wide browsing:
   places shortcuts (~, /tmp, project roots), breadcrumbs to /, new
   folder, 'new session here' on any directory. Trusted-operator model
   documented in SPEC.md: a paired device already equals terminal
   access, so a project jail would be security theater; paths are still
   canonicalized and operations logged.

2. Subagent observability designed as three levels: fleet card
   counts/lines, session subagent strip (clickable chips), and a new
   read-only subagent-view mockup showing a live drill-in transcript.
   SPEC.md gains a section on the RPC plumbing gap: background_agent
   events carry no session path and child JSONL events are consumed
   privately by the parent, so the implementation PR must add registry
   exposure over RPC plus an agentId-namespaced event relay — file
   tailing explicitly rejected. Relay addressing is designed to be
   reused by future subagent steering (not MVP; requires child stdin
   control channel).
…sign

Dashboard UX/UI design (design-only, merges into future implementation branch)
@m-aebrer

m-aebrer commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator Author

Implementation Plan

Analysis

The design phase (issue 311, PR 320) produced an accepted, review-hardened spec: design/dashboard/SPEC.md (ground truth for scope, architecture, and acceptance) and design/dashboard/PARITY.md (TUI-parity coverage authority), plus tokens.css and seven mockups. All decisions are made — IA (fleet-centric), stack (SolidJS + Vite client, Express + RpcClient server), security (exactly two modes: loopback-only default, Tailscale+allowlist+PIN remote), files (host-wide, canonicalized, logged), theming (fixed light/dark). This plan converts SPEC §7 (foundation scope) into a buildable sequence, reviewed against SPEC §9 (eleven acceptance criteria).

Codebase facts confirmed by exploration:

  • RPC client: RpcClient is exported only from the @dreb/coding-agent/rpc subpath (the root-import shown in docs/rpc.md is stale). One session per process; the runtime pool must be N child processes (telegram's AgentBridge is the working precedent, including absolute cliPath resolution via import.meta.resolve).
  • Event wire: RPC mode forwards the entire AgentSessionEvent union (21 types) — the docs/rpc.md events table under-documents it by five types (stream_retry, length_retry, background_agent_*, parent_paused_for_background_agents, tasks_update, suggest_next).
  • Subagent gap (the one foundation-scope RPC gap, SPEC §5a): BackgroundAgentInfo (subagent.ts:1027) lacks session path; background_agent_start (agent-session.ts:2956) carries only agentId/agentType/taskSummary; child JSONL events are parsed in spawnSubagent's stdout reader (subagent.ts:307–338) and everything except agent_start/message_end/tool-progress is dropped. The child's sessionDir is known pre-spawn (subagent.ts:1432). Insertion points for registry exposure and relay are clean.
  • RpcClient gap found during exploration: the RPC server accepts extension_ui_response on stdin (rpc-mode.ts:1165) but RpcClient has no method to send one — the dashboard needs this for extension-UI modals, so a small client addition is required.
  • Workspace mechanics: new package registration touches root package.json (build/dev chains), root tsconfig.json paths, vitest.workspace.ts, and .github/workflows/publish.yml (explicit publish step). sync-version.sh globs automatically. Closed PR 310 provides endorsed server-side prior art (fail-closed auth, path-safe file API, runtime pool, SSE fanout) and the tsconfig.client/copy-assets pattern.

Prior art (beyond what the design phase already researched): the loopback trust path must validate Host/Origin headers — DNS rebinding against localhost dev servers is a live vulnerability class (Vite GHSA-vg6x-rcgg-rjx6, patched January 2025, is the canonical precedent). PR 310's loopback bypass trusted any request arriving on 127.0.0.1; the foundation adds Host allowlisting so a malicious website cannot drive the dashboard API through a rebound DNS name. SSE reconnect uses the standard id:/Last-Event-ID catch-up mechanism with a bounded per-session event buffer.

Branch mechanics (stage 0 — agreed on the issue, non-standard)

  1. Create feature/issue-307-dashboard-foundation from master (not from the design branch), push with an empty commit, open the draft foundation PR against master.
  2. Retarget PR 320's base from master to the new implementation branch (gh pr edit 320 --base ...), mark it ready (gh pr ready 320), and merge it with a merge commit into the implementation branch. This is the "first act" required by the handoff: design artifacts land on the implementation branch with a clean merged-PR record, and iterate there from now on.
  3. Post-acceptance changes to SPEC/PARITY/tokens happen on this branch — including re-dispositioning parity rows in the same PR if scope shifts (SPEC §9.4: no silent scope shrink).
  4. Whether design/dashboard/screenshots/ (regenerable via capture.mjs) ships to master is decided at publish time; default is keep during development for reviewability.

Stage 1 — coding-agent RPC extensions (subagent observability, SPEC §5a)

The enabler for dashboard levels 1–3 of subagent visibility; file-tailing explicitly rejected by the spec.

  • Extend BackgroundAgentInfo with the child's session directory (known pre-spawn) and discovered session file; registry entries updated on spawn/close.
  • Enrich background_agent_start events with the session path.
  • New RPC command list_background_agents returning the registry snapshot (pattern-match the existing list_sessions handler; registry is module-global, callable directly from rpc-mode).
  • Event relay: add an opt-in onChildEvent callback threaded through spawnSubagent → the subagent tool options, wired in agent-session to re-emit child events as a namespaced wrapper event (background_agent_event carrying agentId + the child's event) on the parent's stream. Rides the existing generic RPC forwarding with zero rpc-mode changes. The agentId addressing is what future subagent steering reuses.
  • RpcClient additions: listBackgroundAgents(), sendExtensionUIResponse() (needed for extension-UI modals; server already accepts it), and event-listener typing widened to the full AgentSessionEvent union (currently claims AgentEvent).
  • docs/rpc.md: document the new command and relay/enriched events; fix the events table's five missing existing event types while editing it (same table, in-scope categorical work).
  • Tests (packages/coding-agent): registry exposure fields, relay emission (child event → namespaced parent event), RPC command handler, sendExtensionUIResponse round-trip.

Relay volume note: message_update deltas are high-frequency; the relay forwards them as-is and the dashboard server owns batching/throttling policy (SPEC §8 flags streaming feel as implementation-phase validation).

Stage 2 — packages/dashboard scaffold + launch wiring

  • New workspace package @dreb/dashboard, telegram as the structural template: package.json (bin dreb-dashboard), tsconfig.build.json (server), Vite config + client tsconfig (PR 310's two-config pattern), vitest.config.ts.
  • Dependencies: express (server), solid-js + vite + vite-plugin-solid (client build); test-side DOM environment for render smoke tests. No Tailwind, no component kits (SPEC §8 rejections).
  • Root registration: build/dev chains in root package.json (after coding-agent), tsconfig.json paths, vitest.workspace.ts, publish.yml step for @dreb/dashboard.
  • tokens.css adopted unmodified: the client imports a copy, and a unit test asserts byte-equality with design/dashboard/tokens.css so any drift fails CI (extensions live in separate files; overrides require a spec update per SPEC §9.9).
  • dreb dashboard subcommand in coding-agent's main (alongside the existing config/package subcommands): dynamically resolves @dreb/dashboard and hands over; if not installed, fails loudly with install instructions. No hard dependency from coding-agent to dashboard (avoids the cycle); in the workspace it resolves via symlink. Help text updated.

Stage 3 — dashboard server core

Endorsed PR 310 architecture, hardened:

  • auth — exactly two modes (SPEC §6). Local: loopback bind only, plus Host/Origin validation on every request (DNS-rebinding defense — the addition over PR 310). Remote (explicit opt-in flag): Tailscale identity resolution (tailscale status --json), identity/device allowlist (empty allowlist = deny all), single-use 6-digit PIN with 5-minute expiry printed to the host terminal, per-device token cookie (hashed server-side store, HttpOnly + SameSite=Strict + Secure), devices list + unpair. Fail-closed middleware on every route: any auth-subsystem error denies. Denial page names the rejected identity.
  • runtime-poolRpcClient pool keyed by cwd+session file; spawns dreb --mode rpc --ui dashboard with absolute cliPath; loud spawn-failure surfacing (rpc-client already rejects in-flight requests on exit/error); lifecycle: create (new_session), resume (switch_session on a pooled or fresh runtime), stop (fleet "stop runtime"), prune on idle.
  • events — SSE fanout: one stream per browser client carrying namespaced envelopes {sessionKey, event} for all pooled runtimes (fleet needs cross-session events; session view filters client-side). Bounded per-session ring buffer + Last-Event-ID catch-up on reconnect; relay events (background_agent_event) flow through the same pipe.
  • files — host-wide file API (browse/download/upload/mkdir): canonicalized, symlink-resolved, percent-decode-checked paths (API-confusion defense, not a jail — trusted-operator model per SPEC §6); collision detection for upload (409 + explicit overwrite flag); every operation logged server-side.
  • REST surface — fleet state (live pool + list_all_sessions inventory), session ops (create/resume/delete/fork/stats/export-html download), command pass-through (prompt/steer/follow_up/abort, model/thinking, compact, rename), settings (get/set + devices), pairing endpoints, list_background_agents.
  • Tests: auth fail-closed matrix (loopback spoofing, Host-header rejection, disabled-remote, non-allowlisted identity, expired/reused PIN, bad cookie, resolver failure ⇒ deny), path canonicalization (traversal, symlink escape, percent-decode tricks), runtime-pool lifecycle with mocked RpcClient, SSE framing + catch-up. HTTP tests use PR 310's proven pattern: real server on port 0, driven by native fetch.

Stage 4 — client foundation

  • SSE client with reconnect/catch-up; event reducer maintaining per-session state maps (transcript entries, streaming message assembly, tool-card lifecycle, tasks, subagent registry, needs-attention derivation from extension-UI requests / parent-paused / errors). The reducer is pure TypeScript, unit-tested in node without DOM.
  • App shell: router (fleet ⇄ session ⇄ subagent drill-in; files/settings as global tabs; pairing outside the tab structure), topbar, status chips (glyph+color pairs, never color alone), tab-badge for needs-attention, mode badge (⌂ local / ⇄ remote).
  • Transcript components ported structurally from the export-html renderer (renderEntry()/renderToolCall()): user/assistant/thinking, tool cards with bespoke read/write/edit/bash bodies and generic fallbacks, compaction/branch summaries, custom messages — the PARITY §4 ✅ set, adapted for live streaming (running tool card open, others collapsed).

Stage 5 — screens (mockups are the contract)

  1. Fleet overview — live cards (status chip, activity line via last-assistant-text, live subagent lines, tasks progress, ctx%, model), on-disk inventory grouped by project with resume/delete, "+ new session" modal (recent projects + free path + optional first prompt), needs-attention sort + badge, global counters. Empty state per SPEC §3.
  2. Session view — full-parity transcript; dock (collapsible tasks panel, status line with elapsed/stop, composer); composer two-state steer/follow-up toggle visible only while streaming, ■ abort, queued-message chips with dismiss-to-composer, suggest-next chip, image paste; session bar (back, name, model/thinking switchers as searchable modals, ctx%, ⋯ menu: export HTML, compact, rename, fork-from-message, stop runtime); extension-UI modals (select/confirm/input/editor) + toasts/status/widgets/title/prefill; error surfaces (retry status lines, fallback banner, error status). Mobile reductions per SPEC §4 — composer modes, abort, and attention affordances never reduced away.
  3. Subagent drill-in — read-only live transcript via the relay; parent task as first entry; status/elapsed in bar; fixed "can't be steered yet" note; no composer. If the relay were to slip, the chips lose their click target rather than shipping a broken viewer (honest absence).
  4. Files tab — places chips (home, /tmp, project roots), breadcrumbs to /, list with size/modified, new-folder, download, drop-zone + picker upload with collision prompt, "new session here", fixed warning copy verbatim from mockups.
  5. Pairing screen — identity echo, PIN entry with expiry countdown, the two verbatim security copy blocks, denial page.
  6. Settings tab — defaults via get/set_settings (model+provider, thinking, queue modes, compaction/retry toggles) with verbatim RPC validation errors, "live sessions keep their values" copy, paired-devices list with unpair, version footer.

Each screen ships with a smoke-level render test (six shipped screens; the tree screen is designed but deliberately deferred per SPEC §7 — PARITY already dispositions it 🔜, so §9.11's "seven screens" resolves to the shipped set).

Stage 6 — docs + parity audit + release wiring

  • packages/dashboard/README.md (canonical package doc), packages/coding-agent/docs/dashboard.md (launch, local vs remote, security model, workflows — cross-linked, not duplicated), root README.md + packages/coding-agent/README.md feature mentions.
  • Documentation must cover: local-only use, Tailscale remote setup, PIN pairing, allowlist config, the dangerous-capability model, and supported workflows (issue acceptance criterion).
  • PARITY audit: walk every ✅ row; each is implemented or re-dispositioned in this PR with PARITY.md updated (SPEC §9.4). 🔜/❌ rows verified to have no dead UI.
  • Self-review against SPEC §9's eleven criteria before marking the PR ready.

Deliverables

  1. @dreb/dashboard workspace package: Express server (dreb-dashboard bin) + SolidJS client, wired into build/test/publish.
  2. dreb dashboard launch path in coding-agent (loud failure when package absent).
  3. coding-agent RPC extensions: list_background_agents, enriched background_agent_start, background_agent_event relay, RpcClient.sendExtensionUIResponse()/listBackgroundAgents(), docs.
  4. Six live screens implementing the mockups on tokens.css, light+dark, responsive at the 700px breakpoint.
  5. Test suite per SPEC §9.11: auth fail-closed, path canonicalization, SSE reducer, event relay, composer dispatch units + screen smoke tests.
  6. Documentation set (package README, docs/dashboard.md, root/product README updates, rpc.md updates).

Acceptance criteria

SPEC §9's eleven criteria are the review contract, verbatim. Additionally: CI green (build, tsgo, biome, all workspace tests), npm run verify-workspace-links clean, no dead UI for deferred items, PARITY.md accurate at merge time.

Risks and open questions

  • PR size. The handoff sanctions one foundation PR, but it spans two packages. Mitigation: strictly staged commits (RPC extensions → scaffold → server → client foundation → screens → docs) so review can proceed stage-by-stage.
  • Streaming feel (SPEC §8 flagged): token batching/throttling policy and fleet-grid reorder stability can only be tuned live; budgeted as part of stage 5, validated manually via dreb -p + browser.
  • ctx% source: computed from session stats + model context window (both available over RPC); if precision proves inadequate, surface what get_state/stats provide honestly rather than inventing numbers.
  • Tailscale identity data (issue open question): tailscale status --json self+peer mapping is the endorsed resolver; resolver errors deny (fail-closed). Verified only on Linux initially; documented as such.
  • Express major version: pick the current stable at implementation time; middleware surface used is minimal (static, json body, cookies).
  • Solid render tests need a DOM test environment; if flakiness emerges, smoke tests fall back to render-to-string assertions — still per-screen coverage.

Plan created by mach6

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown

Vitest coverage

Metric Covered Total Coverage
Statements 22529 39594 56.9%
Branches 11727 23890 49.08%
Functions 4184 7073 59.15%
Lines 19395 34365 56.43%

View full coverage run

@m-aebrer

m-aebrer commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator Author

Progress: stages 1–5 complete + docs; gaps and observed bugs for the next session

Foundation implementation is pushed through commit d909df2. This comment is the handoff state for the next session.

What's done (4 commits on top of the design merge)

Stage 1 — coding-agent RPC extensions (9ab06d6)

  • BackgroundAgentInfo extended with sessionDir/sessionFile/cwd; registry records them at spawn/close
  • background_agent_start events enriched with sessionDir; background_agent_end with sessionFile
  • New background_agent_event relay: parent re-emits every child JSONL event namespaced by agentId (opt-in onBackgroundEvent callback threaded through spawnSubagent → tool options → agent-session; wired to _emit). Extracted handleChildJsonlLine() makes the stdout parse loop unit-testable — this also fixed a latent crash on JSON.parse("null") lines
  • New RPC command list_background_agents; RpcClient.listBackgroundAgents() + RpcClient.sendExtensionUIResponse() (server accepted extension_ui_response but the client had no way to send one)
  • RpcEventListener typing corrected to the full AgentSessionEvent union (removed the stale cast workaround in telegram)
  • ctx% correction (maintainer feedback): get_state now returns contextUsage — the exact numbers AgentSession.getContextUsage() computes for the TUI footer. The dashboard renders these; it never estimates client-side
  • docs/rpc.md: new command + events documented; fixed the events table (5 missing existing event types); fixed the stale root-import example (@dreb/coding-agent/rpc is the real subpath)
  • 12 new tests (registry, relay, DTO mapping, client methods)

Stages 2–3 — @dreb/dashboard package + server core (5e1c413)

  • New workspace package wired into build chain, tsconfig paths, vitest workspace, publish.yml, biome
  • Express server: fail-closed auth middleware on every route, REST surface (fleet, runtimes, prompt/steer/follow_up/abort, model/thinking, compact/rename/fork/export, settings, devices, files), SSE fanout with Last-Event-ID catch-up + bounded ring buffer + dashboard_resync on gap
  • Auth: exactly two modes. Local = loopback bind + Host/Origin validation (DNS-rebinding defense — new over PR 310's prior art). Remote = Tailscale identity resolution + allowlist (empty = deny all) + single-use 5-min PIN + HMAC'd device tokens in a 0600 pairing file. Every auth error denies
  • RuntimePool: one dreb --mode rpc --ui dashboard child per session, absolute cliPath, needs-attention tracking (extension UI requests, parent-paused, errors), background-agent tracking from events
  • FileApi: host-wide, canonicalized (percent-decode checks, null bytes, symlink resolution), upload collision → 409 + explicit overwrite, all ops logged
  • dreb dashboard subcommand in coding-agent (dynamic resolve, loud failure with install instructions, no dependency cycle)
  • 74 tests: auth fail-closed matrix, path canonicalization, event hub, runtime pool, HTTP integration (real server, native fetch, raw-http Host spoofing)

Stages 4–5 — client foundation + six screens (5f71676)

  • Pure-TS event reducer (node-testable, no DOM): transcript entries, streaming assembly, tool lifecycle, tasks, subagent registry + relay sub-transcripts, extension UI, status lines, needs-attention derivation
  • SolidJS app: hash router, store with reconcile syncing reducer state into fine-grained signals, SSE client with reconnect/catch-up
  • Six screens implementing the mockups: fleet (cards + disk inventory + new-session modal + attention sort), session view (full transcript, composer with steer/follow-up/abort, tasks panel, subagent strip, suggest-next chip, model/thinking switchers, extension-UI modals, export/compact/rename), subagent drill-in (read-only, fixed note, no composer), files (places, breadcrumbs, upload/download/mkdir, "new session here", verbatim warning copy), settings (defaults + devices + verbatim RPC errors), pairing (PIN flow + both security copy blocks)
  • tokens.css adopted unmodified + byte-equality test enforcing it
  • 101 tests total in the package (reducer, screens smoke via jsdom, server)
  • E2E verified manually: real runtime spawned via API, contextUsage flowing, SSE streaming, Host-spoof rejected

Docs (d909df2): packages/dashboard/README.md + packages/coding-agent/docs/dashboard.md

Observed bugs (from maintainer testing — fix first)

  1. Files tab starts at / instead of ~. The default-path logic races: createResource fetches before places load, falls through to /. (Confirmed still an issue — the "can't see anything at /" symptom reported earlier was a transient side effect of killing the backend mid-session, not a real bug; browsing at / itself works correctly.)
  2. Each /tmp test session becomes its own fleet project group. Proposal: bundle all sessions under /tmp into a single "/tmp" project group as an exception.
  3. Markdown is not rendered in agent responses. Transcript shows raw markdown text. The export-html renderer uses marked; the dashboard needs a markdown renderer for assistant text blocks (and user messages).
  4. No setting to always expand thinking. Thinking blocks are always collapsed <details>; add a persistent preference (dashboard-local is fine) to default them open.

Gaps / UX improvements (also next session)

  1. Composer should auto-grow as the user types (long messages are unreadable in a fixed 60px box). Bare minimum: resizable.
  2. Resuming a session must not clobber its model. The resumed runtime should surface the session's stored model, not the global default. Needs verification of what --session restore does to model selection in RPC mode and what the fleet card/session bar display.
  3. No system to invoke skills from the dashboard. The TUI's /skill:name args expansion (and extension commands, prompt templates) has no composer equivalent yet — PARITY.md §2 calls for a / autocomplete fed by get_commands, but it isn't wired up. This is a TUI parity gap, not just a nice-to-have: skills are a primary workflow for some users.
  4. No indication of what context is loaded for a new session. The TUI shows loaded AGENTS.md/CLAUDE.md/CONTEXT.md files, skills, and extensions at startup; the dashboard's session view gives no equivalent signal, so a user can't tell what's in play. Another TUI parity gap — needs a "loaded context" affordance (session bar ⋯ menu or an info panel) surfacing resourceLoader's discovered context files/skills/extensions.
  5. Nearly all of the TUI footer's information is missing from the dashboard. The footer (footer.ts) shows, per session: cwd + git branch + session name; cumulative token breakdown (↑input ↓output R-cache-read W-cache-write); total cost with an OAuth-subscription indicator and a same-day cross-session cost rollup (getDailyCost()); context % (already wired); model + provider + thinking level; and rolling tok/s with a trend arrow vs. a baseline median (getPerformanceTracker()). The dashboard currently surfaces only model + ctx% — everything else is absent. Server-side gaps to close: get_session_stats (tokens/cost — already called by the server but never exposed to the client) and get_performance_stats (tok/s) need REST wiring; git branch and daily cost have no RPC exposure at all today (FooterDataProvider.getGitBranch()/getDailyCost() are TUI-only) and need either a new RPC command or a dashboard-side implementation (e.g. the server can shell out to git itself for branch, and daily cost can be computed from list_all_sessions + per-session stats). This should land in the session bar / fleet card, not just a stats popover.

Root README

  1. Root README needs a dashboard section: how to launch (dreb dashboard / dreb-dashboard --remote --allow ...), how to set it up as an auto-restarting service (systemd user unit / launchd plist — whichever is simplest to document first, both eventually), and image placeholders for screenshots once the six screens have a final visual pass (fleet, session view, files, settings, pairing at minimum).

Remaining planned work (stage 6 + verify)

  • PARITY.md audit — walk every ✅ row, re-disposition anything not shipped in this PR (SPEC §9.4: no silent scope shrink). Known ✅ rows NOT yet implemented that need either implementation or re-disposition: per-message copy button, fork-from-message UI (API route exists, no UI), image paste in composer, queued-message chips with dequeue, session stats popover, composer history (up/down), / command autocomplete fed by get_commands (gap 7 above), per-tool bespoke bodies beyond edit-diff (read/write/bash presentation), status-line abort variants for compaction/retry, loaded-context indication (gap 8 above), footer parity — tok/s, cost, daily cost, git branch (gap 9 above)
  • Root README + packages/coding-agent/README feature mentions (gap 10 above; per repo docs rule — root README must stay accurate)
  • SPEC §9 self-review against all eleven acceptance criteria before marking the PR ready
  • Screenshot pruning decision (design/dashboard/screenshots/) is deferred to publish time per the branch mechanics

Verification state

  • Full workspace: build green, tsgo green, biome green, all tests pass (one flaky live-API E2E in packages/ai unrelated — passes on rerun)
  • Manual E2E: local mode, fleet API, runtime create/stop, SSE, DNS-rebinding rejection all verified against the real binary

Progress tracked by mach6

m-aebrer added 3 commits July 8, 2026 07:33
…models, resources, git branch, daily cost, pending messages

- Move TabTitleGenerator to core and wire into RPC mode: sessions auto-name
  after a threshold of tool calls in dashboard/telegram, not just the TUI;
  new session_name_changed event notifies all frontends
- Fix --session resume clobbering the stored model when scoped models are
  configured (main.ts skips scoped-first for --session; sdk.ts passes real
  scopedModels to findInitialModel)
- get_state: add scopedModels and usingSubscription
- New commands: get_resources, get_git_branch (shared git-branch helper,
  reused by FooterDataProvider), get_daily_cost, get_pending_messages,
  clear_pending_messages, abort_compaction
…ranscript, command autocomplete, and session-view parity features

Fixes from maintainer testing:
- Model selector grouped by provider with scoped/all tabs — same model id
  under multiple providers is now unmissable; provider/id labels in session
  bar and fleet cards
- Transcript turn ordering fixed (thinking -> text -> tools) with visual
  turn grouping; background-agent completions render as distinct collapsed
  agent-result cards instead of 'you' user messages
- Markdown rendering (marked + dompurify) for assistant text and agent
  results; always-expand-thinking browser preference
- Files tab starts at home (places race fixed); /tmp sessions bundle into
  one fleet group; composer auto-grows

TUI parity:
- Session info bar: cwd + branch + name, token breakdown, cost with (sub)
  and daily rollup, ctx%, median tok/s; stats popover
- Composer: / command autocomplete fed by get_commands, image paste/attach,
  history recall, queued-message chips with restore-all
- Per-message copy, fork-from-message modal, skill badges, bespoke
  bash/read/write tool bodies, compaction/retry abort buttons
- Loaded-context modal (get_resources), live session_name_changed handling,
  browser notifications on needs-attention, fleet card cost + last-activity
  preview
…ness

- Root README: dashboard interface bullet, packages table row, launch +
  security + systemd service section with screenshot placeholders
- PARITY.md: every row re-verified against shipped source; honest
  re-dispositions (restore-all dequeue, median tok/s without trend arrow);
  RPC count updated to 47; closed RPC gaps recorded
- rpc.md: all 47 commands documented (added resolve_model, buddy_hatch,
  buddy_reroll); events table verified complete
- dashboard.md + package READMEs refreshed to shipped reality
@m-aebrer

m-aebrer commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

Progress: all handoff items + session-reported bugs implemented

Three commits pushed (c426c08, 5b5892d, ef6f87c). Every item from the previous progress comment is done, plus four issues reported during maintainer testing this session.

Session-reported fixes (this session's testing)

  • Provider-illegible model selection (critical UX) — the selector was a flat list, so the same model id under two providers (e.g. claude-fable-5 on both anthropic and github-copilot) was easy to mis-pick. Now: grouped by provider with headers + per-row provider badges, scoped/all tabs (defaults to scoped when configured — scoped models are now exposed via get_state.scopedModels), current-model checkmark, and provider/id labels in the session bar and fleet cards.
  • Auto session naming in RPC modeTabTitleGenerator moved to core and wired into RPC mode; dashboard/telegram sessions now auto-name after the same tool-call threshold as the TUI. New session_name_changed event updates fleet cards live.
  • Background-agent results rendered as "you" — completions arriving as <background-agent-complete> user messages now render as distinct collapsed "background agent result" cards (markdown-rendered), in both hydration and live paths.
  • Turn ordering — hydrated transcripts showed [tools → thinking → text]; now content order is preserved ([thinking → text → tools]) with visual turn grouping.

Handoff items (previous progress comment)

  1. Files tab starts at home — places race fixed
  2. /tmp sessions bundle into a single fleet group (real cwd still shown on cards)
  3. Markdown rendering (marked + dompurify, sanitized) for assistant text
  4. Always-expand-thinking preference (browser-local, settings → dashboard section)
  5. Composer auto-grows with content, manually resizable
  6. Resume no longer clobbers the session's stored model (--session was not excluded from scoped-first selection in main.ts; sdk.ts now receives real scopedModels)
  7. / command autocomplete fed by get_commands (skills/extensions/templates; expansion stays server-side)
  8. Loaded-context modal fed by new get_resources (context files, skills, extensions, templates)
  9. Footer parity — session info bar with cwd + git branch + name, token breakdown (↑↓RW), cost with (sub) + daily rollup, ctx%, median tok/s; stats popover with full breakdown. New RPC: get_git_branch, get_daily_cost; usingSubscription in get_state. One honest reduction: no trend arrow (RPC lacks the performance delta; documented in PARITY.md rather than faked)
  10. Root README dashboard section: launch commands, security model, systemd user unit, screenshot placeholders

PARITY.md audit (stage 6)

Every ✅ row re-verified against shipped source. Shipped this round: per-message copy, fork-from-message modal, image paste/attach in composer, queued-message chips (restore-all — per-item dismiss has no RPC and the TUI itself only restores all; row updated honestly), stats popover, composer history, status-line abort variants for compaction/retry (new abort_compaction + existing abort_retry wired), bespoke bash/read/write tool bodies, skill badges, browser notifications on needs-attention transitions, fleet card cost + last-activity preview. RPC command count corrected to 47; rpc.md now documents all of them.

Verification

  • Full workspace: build green, biome green, all tests pass (~4,200 per pre-commit run; dashboard 139, coding-agent 2,584)
  • Live E2E against the real binary: spawned server + runtime; verified scopedModels (15 entries), daily-cost ($260.13 real rollup), performance medians, resources, commands, pending/dequeue, branch endpoints
  • SPEC §9 self-review: all eleven acceptance criteria hold, including §9.3 (queued messages visible + dequeueable) and §9.4 (no silent scope shrink)

Remaining before ready-for-review

  • Screenshot capture for the root README placeholders (needs a final visual pass)
  • Screenshot pruning decision (design/dashboard/screenshots/) deferred to publish time per branch mechanics

Progress tracked by mach6

@m-aebrer

m-aebrer commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

Maintainer Bug Report — Investigation Findings (round 2)

Seven items reported from live testing. Root causes confirmed by code inspection; fixes land in this session. One item is a feature request and was filed as a separate issue.

Critical

1. Subagent sessions unviewable after dashboard reload
Subagent transcripts exist only in the browser-side reducer (SessionViewState.subagents), fed by live background_agent_event SSE relays. Closing the window discards that state; on reopen connectEvents starts without a Last-Event-ID, so nothing is replayed, and hydrateSession only restores the parent transcript (get_messages). There is no hydration path for subagent transcripts — even though their session JSONLs are on disk and list_background_agents reports sessionDir/sessionFile. The session view's subagent strip reads the same lost reducer state, so the entry point to the drill-in vanishes as well (fleet cards survive because they read server-side pool state).
Fix: new server endpoint reading the subagent JSONL from disk (GET /api/runtimes/:key/subagents/:agentId/messages), client-side hydrateSubagent on drill-in, and hydrateSession re-seeding backgroundAgents from the runtime's authoritative registry.

2. Fleet view buries live sessions
Live cards render inside per-project groups interleaved with disk inventory, so with several projects the live sessions scatter down the page; disk lists show 5 rows each with no expander, and uniform spacing makes project boundaries hard to scan.
Fix: restructure to live-first — a single grid of all live session cards at the top (attention-first, project path on each card), then a "past sessions" section grouped by project with 3 compact rows per group and an "all N →" expand toggle.

Important

3. Autoscroll only fires after completion, not during streaming
The scroll effect tracks entries.length only. Streaming deltas mutate blocks[].text on the existing tail entry in place — no length change, no reactive trigger, no scroll until the next entry appends (typically at turn end).
Fix: per-session version counter bumped in syncSession on every envelope; the scroll effect subscribes to it. Also adds stick-to-bottom autoscroll to the subagent view, which had none at all.

4. Tool inputs truncated with no way to see them in full
toolArgSummary slices args to 120 chars for the collapsed summary row, and the expanded card body only renders resultText — full inputs are never shown anywhere (e.g. long subagent task prompts, write content after completion).
Fix: expanded tool cards render the full input args (bespoke per tool; generic key/value fallback).

5. Markdown-worthy tool output rendered as plain preformatted text
suggest_next returns details.summary that is markdown by contract (the TUI renders it via the Markdown component); the dashboard ignores details and shows the raw resultText. Same for subagent results, skill loads, and web_fetch extractions.
Fix: markdown rendering (sanitized, same pipeline as assistant text) for the tools whose output contract is markdown; suggest_next renders summary + → command.

Suggestions

6. Expand-thinking should be opt-out
readBooleanPreference returns getItem(key) === "true" — unset means collapsed. New users get thinking collapsed by default.
Fix: default flips to true when the preference is unset; explicit "false" still honored.

Tracked separately

7. Session-status sidebar in session view — feature request, filed as a standalone issue (linked below) rather than scope-crept into this PR.


Investigation by mach6 (maintainer-reported findings)

…l, live-first fleet, streaming autoscroll, full tool inputs, markdown tool results, expand-thinking default

- Subagent views survive browser reloads: new /subagents/:agentId/messages
  endpoint reads the agent's on-disk session JSONL (registry-located via
  list_background_agents); hydrateSubagent on drill-in mount; hydrateSession
  re-seeds backgroundAgents so the session strip reappears too
- Fleet is live-first: one flat grid of all live cards on top
  (attention-first, project path on each card), past sessions below grouped
  by project with 3 compact rows + all-N expander
- Autoscroll works during streaming: per-session revision counter bumps on
  every envelope (text deltas mutate in place and never changed
  entries.length); subagent view gains stick-to-bottom scrolling
- Expanded tool cards show full inputs (subagent tasks as markdown, generic
  long args labeled) instead of only the 120-char summary
- Markdown-contract tool results render as markdown (subagent/skill/
  web_fetch); suggest_next renders its summary + command from details
- Expand-thinking preference is opt-out: unset defaults to expanded
@m-aebrer

m-aebrer commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

Progress: all six maintainer-reported bugs fixed (round 2)

One commit pushed (0855249). Every bug from this round's investigation comment is fixed and verified against a live server with a real subagent run. The sidebar feature request was filed separately as issue 323.

Fixes

  1. Subagent views survive dashboard reloads — new GET /api/runtimes/:key/subagents/:agentId/messages endpoint: the runtime's registry (list_background_agents) locates the agent's on-disk session JSONL (sessionFile, or newest .jsonl in sessionDir for running agents), and a new subagent-log reader converts its message entries to the transcript shape. The drill-in screen hydrates from it on mount; hydrateSession now also re-seeds backgroundAgents from the registry so the session view's subagent strip reappears after reload. Verified live: fresh browser page → drill-in shows the full transcript (task, thinking, tool calls, results); strip chip present; zero console errors.
  2. Fleet is live-first — all live session cards in one grid at the top (attention-first, then most-recent), each card showing its project path. Past sessions below, grouped by project, 3 compact rows per group with an "all N on disk →" / "show fewer ←" toggle. Verified with 2 live + 712 disk sessions: everything live visible at a glance, disk rows compact.
  3. Autoscroll during streaming — root cause: the scroll effect depended on entries.length, but streaming deltas mutate the tail entry's text in place. Added a per-session revision counter (bumped in syncSession on every applied envelope) that the scroll effect subscribes to. Also added stick-to-bottom scrolling to the subagent view (it had none). Verified live: 16/16 samples during a long streaming response were pinned to the bottom while content grew, and scroll-up still disengages.
  4. Full tool inputs — expanded tool cards render complete inputs: subagent task prompts as markdown sections (single/parallel/chain), generic long string args as labeled full-text sections; write shows its content input after the result lands. Summary rows still truncate for scanability — the expansion is where the full input lives.
  5. Markdown tool results — tools whose output contract is markdown (subagent, skill, web_fetch) render sanitized markdown instead of <pre>; suggest_next now renders its details.summary markdown + → command instead of the raw "Suggestion registered" ack (matching the TUI's Markdown-component treatment).
  6. Expand-thinking is opt-out — unset preference now defaults to expanded; explicit "false" still honored.

Tracked separately

  • Session-view fleet/subagent status sidebar → issue 323

Verification

  • 153 dashboard tests pass (14 new: subagent-log reader, server endpoint incl. unknown-agent 502, hydration + error paths, fleet layout/expander, tool inputs, markdown results, expand-thinking default)
  • biome clean, tsc clean (server + client), full workspace build green, full workspace test run green
  • Live E2E against the built binary: real runtime in /tmp, real Explore subagent spawned via prompt, fresh-page drill-in verified with screenshots; streaming autoscroll measured; fleet layout screenshotted at 712 disk sessions

Progress tracked by mach6

@m-aebrer

m-aebrer commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

Maintainer Bug Report — round 3 (not yet investigated)

Reported from live testing by the maintainer. Investigation and fixes deferred to the next session — root causes below are NOT yet confirmed.

Reported bugs

  1. Settings: default model selector is a free-text field — should be a dropdown list of available models, not provider/model-id free text.

  2. Tool cards lost TUI-style color formattingread, edit, and friends no longer have syntax/color formatting like the TUI. Use the default theme, or adopt a good AMOLED-compatible theme from online.

  3. Edit tool output is just the ack — the card body shows "successfully changed text at …" instead of the actual change.

  4. Code reads/changes no longer easy to follow while the model works — regression in at-a-glance visibility of file reads and edits during a live turn (related to items 2 and 3: the working view used to make reads/diffs legible as they happened).

  5. Undismissable warning toastBlocked pytest run missing NATS_URL="" (an extension hook) fired in session ccb0462899b6; the warning popup in the bottom right cannot be dismissed — pressing the "X" does nothing.

  6. suggest_next still not showing the markdown-formatted summary — the markdown summary from round 2's fix is not appearing (regression or incomplete fix; needs verification against a live suggest_next call).

  7. Suggested-command chip persists too long — the footer "suggested: … (tap)" chip should clear after the next user message is sent; instead it persists until replaced by a new suggestion.

  8. Settings tab is missing most TUI settings. The TUI settings selector exposes the full set below; the dashboard only surfaces a small subset. Agent Models is the key one to get working. Some entries (e.g. double-escape action, hardware cursor, editor padding — TUI-specific) don't need a dashboard equivalent; the next session should triage which belong in the dashboard:

    Auto-compact              true
    Auto-resize images        true
    Block images              false
    Skill commands            true
    Show hardware cursor      false      (TUI-only, likely skip)
    Editor padding            0          (TUI-only, likely skip)
    Autocomplete max items    5
    Auto-load nested context  true
    Steering mode             all
    Follow-up mode            all
    Transport                 sse
    Hide thinking             false
    Collapse changelog        false      (TUI-only, likely skip)
    Quiet startup             false      (TUI-only, likely skip)
    Double-escape action      tree       (TUI-only, skip)
    Tree filter mode          default
    Thinking level            high
    Theme                     dark
    Agent Models              (18/19)    ← priority
    

Maintainer-reported findings; investigation to follow in the next session

@m-aebrer

m-aebrer commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

Round 3 — Investigation Findings & Fixes

All eight reported items root-caused and fixed in this session. Verified by unit tests, a full workspace test run, and a live Playwright E2E against the built server with a real resumed session (26 checks, all passing).

Root causes and fixes

1. Settings default model was free text
The settings tab used <input type="text"> with manual provider/model-id parsing; the models listing only existed per-runtime. Fix: new GET /api/settings/models (runtime-agnostic, via any pooled runtime) and a provider-grouped, searchable model picker modal — same visual language as the session view's switcher. Free-text input removed.

2. Tool cards lost TUI-style color formatting
The dashboard never had syntax highlighting — read/write bodies rendered as plain <pre> (the export-html renderer has hljs + theme mapping; the dashboard didn't adopt it). Fix: highlight.js with the same extension→language map the TUI/export renderer uses; read results and write content highlight by file extension; .hljs-* classes map to new syntax-color variables in app.css (light values from the TUI light theme, dark values from the TUI dark theme — the VS Code palette designed for black backgrounds, AMOLED-friendly). tokens.css untouched (byte-locked design contract).

3. Edit tool output was just the ack
The card body rendered resultText, which for edit is the ack string. The actual diff was already in details.diff on both live and hydration paths — never read. Fix: edit cards render details.diff through the diff body (add/del coloring, line numbers), falling back to resultText only when no diff exists (e.g. errors).

4. Reads/changes illegible while the model works
<details open={status === "running"}> is a reactive prop — every card snapped shut the instant its tool completed, so nothing was followable during a turn and hydrated history was all collapsed. Fix: read, edit, write, and suggest_next cards are open by default permanently (manually collapsible); other tools keep collapse-on-complete. Body height capped (scrollable) so open-by-default doesn't blow up the page.

5. Undismissable warning toast
The dismiss handler spliced the toast out of the Solid store's reconciled clone — no store setter, so no reactive update, and the reducer's authoritative copy still held the toast (any later sync resurrected it). Fix: proper dismissToast reducer helper + store method that mutates reducer state and re-syncs; integration test proves dismissal survives subsequent envelope syncs.

6. suggest_next summary still not showing
Round 2's markdown rendering was correct but invisible: suggest_next ends the turn, so item 4's collapse bug hid the card body instantly — the summary only existed behind a manual expand. Item 4's fix (open by default) surfaces it. Also fixed a duplicate: the generic long-arg input section rendered the summary a second time above the details-driven body; suggest_next is now excluded from input sections (regression test added).

7. Suggested-command chip persisted too long
The reducer only ever set suggestedCommand; nothing cleared it. Fix: agent_start (the next turn beginning, i.e. after the user's next message is sent) clears it.

8. Settings tab missing most TUI settings
get_settings/set_settings only carried 7 keys; the persistence for everything else already existed in SettingsManager. Fix (RPC): snapshot/update extended with imageAutoResize, blockImages, enableSkillCommands, autoLoadNestedContext, transport, hideThinkingBlock, and agentModels (Record<agent, fallback list>) — all validated atomically (any invalid field rejects the whole payload, loudly). agentModels semantics: non-empty list writes the global fallback, empty list removes the override; project-file shadowing produces a warnings array on the response instead of silent no-ops (mirrors the TUI warning). New list_agent_types RPC command exposes discovered agent definitions. Fix (dashboard): new sections — images, behavior (skills / nested context / hide thinking), transport — plus the agent models editor (priority item): each discovered agent shows its ordered fallback chips, with an inline editor (move up/down, remove, add via the model picker); save warnings render in a visible banner. TUI-only settings (theme, cursor, editor padding, etc.) are deliberately skipped with a visible footnote pointing at the terminal /settings menu.

Verification

  • 169 dashboard tests + 37 RPC settings tests pass; full workspace run green (3618 passed)
  • biome clean; full workspace build green; workspace links verified
  • Live E2E (Playwright vs built server, real session with 37 edits/24 reads/2 suggest_next): diffs render with add/del coloring and no ack text; 13k+ hljs spans across read cards; read/edit/write/suggest_next open by default while bash stays collapsed; suggest_next summary + → command visible without interaction; model picker lists 94 models across 7 providers; all parity rows present; agent-models editor round-trip (add → chip renders → remove → back to "default") verified against the real settings file, with the file restored afterwards
  • Dark-mode screenshots confirmed TUI-equivalent diff/read rendering and the full settings tab

Also in this session

  • Replaced vitest.workspace.ts with a root vitest.config.ts (same projects list) so path-filtered root runs like npx vitest --run packages/dashboard load each package's own config — previously those runs compiled Solid TSX as React and failed. The coverage script now uses the default config resolution.

Investigation and fixes by mach6 (round 3)

…it diffs, open-by-default legibility, toast dismissal, suggest_next visibility, chip clearing, model dropdown, settings parity with agent-models editor
@m-aebrer

m-aebrer commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

Progress: all eight round-3 bugs fixed

One commit pushed (a2e460e). Every item from this round's investigation comment (posted above) is implemented, tested, and live-verified. Root causes and fix details are in that comment; this is the shipping record.

Shipped

  • Transcript (items 2, 3, 4, 6): highlight.js syntax coloring for read/write bodies using the TUI theme palettes (AMOLED-safe dark values, tokens.css untouched); edit cards render details.diff with add/del coloring instead of the ack; read/edit/write/suggest_next cards open by default with capped scrollable bodies so live work is legible; suggest_next markdown summary + → command now visible without interaction, rendered exactly once.
  • State layer (items 5, 7): toast dismissal goes through a reducer helper + store re-sync (X works, dismissal survives later envelope syncs); suggested-command chip clears on the next agent_start.
  • RPC backend (items 1, 8): get_settings/set_settings extended with imageAutoResize, blockImages, enableSkillCommands, autoLoadNestedContext, transport, hideThinkingBlock, agentModels — atomic validation, empty-list-removes semantics, project-shadow warnings on the response; new list_agent_types command + RpcClient.listAgentTypes(); rpc.md updated.
  • Settings UI (items 1, 8): provider-grouped searchable default-model picker (free text removed); images/behavior/transport rows; per-agent model fallback editor (ordered chips, move/remove/add via picker, warnings banner); visible footnote for deliberately skipped TUI-only settings; new GET /api/settings/models and GET /api/settings/agent-types endpoints.
  • Tooling: vitest.workspace.ts → root vitest.config.ts so path-filtered root runs (npx vitest --run packages/dashboard) load package configs instead of compiling Solid TSX as React.

Verification

  • 169 dashboard + 37 RPC-settings tests (new coverage across all eight fixes); full workspace run green — 3618 passed; pre-commit hook re-ran everything: 4247 passed, 0 failed
  • biome clean, workspace build green, workspace links verified
  • Live Playwright E2E against the built server with a real resumed session: 26/26 checks — 684 diff add-lines rendered, 13k+ hljs spans, open/collapsed card behavior per tool, suggest_next summary visible, 94 models across 7 providers in the picker, agent-models add→chip→remove round-trip against the real settings file (restored afterwards), zero console errors
  • Dark-mode screenshots confirmed TUI-equivalent diff/read rendering and the full settings tab

Commit: a2e460e


Progress tracked by mach6

@m-aebrer
m-aebrer marked this pull request as ready for review July 8, 2026 17:19
@m-aebrer

m-aebrer commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

Maintainer Bug Report — round 4 (live testing, latest build)

Reported from live testing by the maintainer on the latest build. Treat as ground truth. Wording preserved verbatim; formatting/numbering added for reference. Independent root-cause assessment to follow as a separate comment.

Subagent viewing (item 1 — three related observations)

1a. page refresh on fleet -> go to session with pre-existing completed subagents -> click a subagent -> error:

<!DOCTYPE html> <html lang=en> <head> <meta charset=utf-8> <title>Error</title> </head> <body> <pre>Cannot GET /api/runtimes/8a3190122ea6/subagents/1ee53d4b4e72/messages</pre> </body> </html>

waiting for output from this agent…

1b. ahh okay, so update, I just went into a live session without refresh, and still saw this error:

<!DOCTYPE html> <html lang=en> <head> <meta charset=utf-8> <title>Error</title> </head> <body> <pre>Cannot GET /api/runtimes/cd93ed191659/subagents/2b25be9cca84/messages</pre> </body> </html>

So that top error is all subagent views, and the waiting for output bug is only post-refresh (maybe both completed and running, definitely completed)

1c. UPDATE on the latest build: now I cant even see completed subagents at all, at least on resumed sessions

Other bugs

2. when steering is submitted, it should show the pending steering messages, even when the agent is currently streaming

3. tab title setter subagent still not running in RPC/dashboard/telegram mode, meaning the sessions are not getting useful automatic titles

4. suggest_next as the agents ending action should set the state on the dashboard to needs_attention rather than idle

5. fleet-view session cards should be sorted alphabetically by their project location, then by session start time to break ties. Currently they move around seemingly at random and its bad UX. Consistency is going to be better than dynamic movement. This principle should be documented for future work as well, likely in AGENTS.md.

6. after a page refresh -> go to live running session -> stop button is missing, cannot halt agent anymore (CRITICAL)

7. agent models in settings: No agent definitions found. (wrong)

8. settings page has a red error box up top:

<!DOCTYPE html>
<html lang=en>
<head>
<meta charset=utf-8>
<title>Error</title>
</head>
<body>
<pre>Cannot GET /api/settings/agent-types</pre>
</body>
</html>

9. bash tool should also open by default

10. tested on my home pc to test tailscale auth. First load of the page on mobile device (with the server already informed about that device per readme) leads to an error page:

{error: Device is not paired - PIN pairing required,needsPairing:true}

but no pairing page is accessible

11. new default model selector is empty (even while session is running)

12. agent models in the settings page is also empty, no listed models AND no detected agents

  • (thinking we need a special settings only bg session to be running with the server so settings are always available)

13. draft user input should persist as long as the session exists in browser memory perhaps?

14. stop actually has no impact at all? It should be functioning the same as in TUI, stopping subagents in addition to the parent agent, and any running tools.

15. dashboard needs a restart dashboard service button in the settings (with a warning that itll kill running sessions)


Reported by maintainer; posted via mach6

@m-aebrer

m-aebrer commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

Review Assessment — round 4 root-cause analysis

Assessing the maintainer bug report posted above (#321 (comment)). Every item was verified against actual source on this branch; three items were additionally investigated by exploration agents with live testing against the built RPC binary. Maintainer follow-ups after posting: the settings 404 (item 8), the "No agent definitions found" (item 7), and the empty agent-models/models lists (item 12) are resolved on the latest code — they were artifacts of a stale running dashboard service (routes added in commit a2e460e but the long-running server process predates it; verified live: both routes now return JSON). The subagent-messages 404s (items 1a/1b) are the same class. The remaining subagent issues (item 1c) are real.

Classifications

Finding Classification Reasoning
1a/1b — subagent view "Cannot GET …/subagents/…/messages" false positive (stale server) The route exists in source (server.ts) and in the current build; the HTML Cannot GET is Express's default 404 for an unregistered route — the running service predated commit a2e460e. Maintainer confirmed likely resolved.
1c — completed subagents invisible on resumed sessions genuine (high) Confirmed root cause: the background-agent registry is a module-level in-memory Map in the RPC child (core/tools/subagent.ts). A resumed session is a fresh process → empty registry. list_background_agents returns [], so no chips render and the messages endpoint throws "No background agent" — even though the subagent JSONLs still exist on disk under ~/.dreb/agent/subagent-sessions/, each with a parentSession header naming the parent session file. Nothing ever rehydrates the registry from disk, and lifecycle events are never persisted into the parent's session JSONL.
2 — pending steering messages not shown while streaming genuine refreshPendingMessages() in session.tsx guards on runtime()?.state.pendingMessageCount — a stale fleet snapshot (fleet only refreshes on agent start/end events). Submitting a steer mid-turn doesn't refresh the fleet, so the guard sees 0 and actively clears the pending display instead of fetching. The chips would render fine (queued-message-row is not gated on streaming); the data path is what's broken.
3 — tab-title setter not working in RPC/dashboard/telegram genuine (root cause differs from premise) The RPC wiring exists and fires (rpc-mode.ts constructs TabTitleGenerator, forwards tool/message events; moved to core/tab-title.ts in commit c426c08). Live-tested: the threshold triggers at tool call 9, a model resolves — and then completeSimple() fails (a 404 model-not-available from the provider proxy for the resolved Explore-agent model) and the failure is silently swallowed by design (.catch(() => {})). No title, no log, no event. Loud failure alone is NOT the fix — the feature must actually work in RPC mode as it does in the TUI: root-cause why the title call fails where the TUI succeeds (prime suspect: provider-unaware model resolution picking an id the runtime's provider can't serve), fix resolution to TUI-identical semantics, fall back to the session's current model when the Explore-derived model is unreachable (the session model is guaranteed reachable — the session streams with it), AND report failures loudly. Acceptance: dashboard/telegram sessions get automatic titles, verified live.
4 — suggest_next should set needs_attention genuine (feature gap) suggest_next sets suggestedCommand in the client reducer but neither the server attention map (runtime-pool.ts tracks only extension-UI and paused) nor the reducer's updateAttention() considers it. Agent ends → card shows idle.
5 — fleet cards move around; want stable sort genuine (UX decision) Current sort is attention-first then lastActivity desc — lastActivity bumps on every event, so cards reorder constantly. Implement: alphabetical by project path, session start time as tiebreak (requires adding a createdAt to the runtime handle/DTO — only lastActivity exists today). Document the "deterministic ordering over dynamic reordering" principle in AGENTS.md as requested.
6 — stop button missing after page refresh (CRITICAL) genuine (critical) The stop button renders on session().streaming, which only becomes true via an agent_start SSE event. After a refresh mid-turn, that event is in the past: hydrateSession() seeds entries and background agents but never seeds streaming (or compacting) from runtime.state.isStreaming, and no replay occurs (fresh page has no Last-Event-ID). The session stays "not streaming" until the next turn starts — stop button, working status, and steer/follow-up toggle all missing while the agent is visibly running.
7 — "No agent definitions found" false positive (stale server) Same stale-service class; maintainer confirmed the list is populated on the latest build. Route verified live.
8 — settings page red error box (agent-types 404) false positive (stale server) Same as item 7; maintainer confirmed resolved. Secondary polish worth taking: the client error path dumps raw HTML bodies into the error box — request() should detect non-JSON error bodies and render a clean GET /api/… → 404 message instead.
9 — bash tool open by default genuine (trivial) LEGIBLE_OPEN_TOOLS in transcript.tsx is read/edit/write/suggest_next; add bash.
10 — unpaired device gets raw JSON error, pairing page unreachable genuine (high) Confirmed: the auth middleware runs on every route and only exempts /api/pair and /api/auth for allowed-but-unpaired identities. The SPA shell itself (/, /assets/*) is behind the wall, so an unpaired device receives the JSON deny for GET / and the client-side pairing screen can never load. Fix: for identities that pass the Tailscale allowlist but lack a device pairing, allow GET of static assets/SPA shell (no data exposure — all data APIs stay fail-closed); the app then routes to the pairing screen as designed.
11 — default model selector empty false positive (stale server, retest) The selector fetches /api/settings/models, added in the same commit as the agent-types route the maintainer confirmed now works; verified live that the endpoint returns models. Should be re-verified in the same pass as items 7/8/12, but all evidence points to the stale service.
12 — agent models settings empty false positive (stale server) + genuine enhancement Lists confirmed populated on latest build (maintainer follow-up). The parenthetical suggestion stands as a real gap: all settings endpoints route through withAnyRuntime and return 503 with no live runtime. Rather than a special background session, the cleaner fix is to have the dashboard server read/write the settings store directly (or keep one lazily-spawned utility runtime) so settings work with zero sessions.
13 — draft input should persist genuine (small) composerText is a per-mount signal; navigating fleet→session discards drafts. The module-level composerHistory map is the precedent — add a keyed draft map (or store drafts in the reducer session state).
14 — stop has no impact; should stop subagents and tools like TUI genuine (multi-cause) The RPC abort handler already matches TUI's primitives (abortBackgroundAgents() + session.abort(), parity added in an earlier commit and unchanged). Four real contributing causes: (a) after refresh the button is gone entirely (finding 6); (b) when the parent is paused waiting on background agents, isStreaming is false → no stop button at all, so subagents can't be stopped from the session view (TUI ESC handles this exact state); (c) RPC abort does not clear queued steer/follow-up messages (TUI ESC does) — queued messages survive the abort and restart the agent, which reads as "stop did nothing"; (d) session.abort() awaits waitForIdle(), and signal-deaf tools (web_search/web_fetch never receive the AbortSignal) can hold the turn for up to 30s with zero UI feedback. Also: the plain abort endpoint has zero test coverage (only abort-compaction/abort-retry are tested). Needs a live repro after the fixes to confirm nothing else lurks.
15 — restart-dashboard button in settings genuine (feature) No such endpoint exists. Implement POST /api/server/restart + settings button with a kills-running-sessions warning modal. Note it only works under a supervisor (the maintainer runs systemd with restart-on-failure → exit non-zero); the UI must say so when not supervised. This also directly mitigates the stale-server failure mode behind items 1a/1b/7/8/11/12.

Process note — the stale-server trap

Six of sixteen reported symptoms trace to one operational issue: the dashboard runs as a long-lived service loading dist/ once at start, so rebuilt routes 404 as HTML until restart. Two hardening actions belong in this PR's scope: (a) the restart button (finding 15), and (b) surface the server's build/version in the settings page footer so a stale service is visible at a glance.

Action Plan

Ordered by severity, then dependency (items grouped where one fix serves several findings):

  1. Finding 6 (critical): seed streaming/compacting (and workingSince) from runtime.state.isStreaming/isCompacting during hydrateSession(), and fall back to fleet state in the session screen so a mid-turn refresh still shows stop/working UI.
  2. Finding 14: (a) render the stop button when streaming or any background agent is running/parent paused; (b) on abort, dequeue pending messages and restore them to the composer (TUI ESC parity); (c) immediate "stopping…" button feedback; (d) add endpoint + client tests for plain abort; (e) thread the AbortSignal into web_search/web_fetch (coding-agent, shared benefit).
  3. Finding 1c: rehydrate the background-agent registry on session open/resume by scanning ~/.dreb/agent/subagent-sessions/ and matching each child JSONL's parentSession header to the parent session file (all plumbing — discoverSessionFile, headers — already exists); mark rehydrated entries completed/failed by inspecting the log tail. Follow-up (separate, smaller): persist background_agent_start/end as session entries in the parent JSONL so future resumes are exact rather than inferred.
  4. Finding 10: allow static/SPA GETs through auth for allowlisted-but-unpaired identities so the pairing screen can render; keep every /api/* route fail-closed exactly as now.
  5. Finding 3: make tab-title generation actually work in RPC mode (TUI parity) — root-cause the model-resolution failure (provider-unaware lookup suspected), resolve provider-aware exactly as the TUI does, fall back to the session's current model when the Explore-derived model is unreachable, and add an error hook to TabTitleGenerator that logs loudly in RPC mode (never swallow). Acceptance: a fresh dashboard session receives an automatic title, verified live.
  6. Finding 2: fix refreshPendingMessages() to fetch unconditionally after a send (drop the stale-snapshot guard or refresh fleet first) so queued steer/follow-up chips appear during streaming.
  7. Finding 4: track suggest_next in the server attention map (cleared on next prompt/agent start) and include suggestedCommand in the reducer's attention derivation.
  8. Finding 5: stable fleet sort — project path alphabetical, then runtime createdAt (add to handle + DTO); document the determinism-over-recency UI principle in AGENTS.md.
  9. Finding 9: add bash to LEGIBLE_OPEN_TOOLS.
  10. Finding 13: per-session draft persistence (module-level keyed map alongside composerHistory).
  11. Finding 15 + process note: restart endpoint/button with warning; show server build/version in settings.
  12. Finding 12 enhancement: make settings work with zero live runtimes (server reads/writes the settings store directly, or lazily spawns a utility runtime).
  13. Polish (from finding 8's corpse): sanitize non-JSON error bodies in the client request() helper so raw HTML never renders in error boxes.

Assessment by mach6

… button, TUI-parity abort (subagent + web-tool cancel, queue restore), subagent registry rehydration on resume, provider-aware RPC tab-titles, zero-runtime settings, restart button, unpaired SPA shell, stable fleet sort, suggest_next attention, streaming pending chips, bash open-by-default, draft persistence, clean error bodies
@m-aebrer

m-aebrer commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

Progress Update — round-4 findings implemented

All 13 action-plan items from the round-4 assessment are implemented, plus the finding-8 error-body polish. Build passes; full suite green (4268 passed / 0 failed via the pre-commit hook; dashboard 168, coding-agent 2603).

Critical / high

  • Finding 6 (critical): hydrateSession seeds streaming/compacting/workingSince from the runtime; the session view derives a showStopControls() from reducer + fleet state, so a mid-turn browser refresh keeps the stop button and working line.
  • Finding 14: stop controls now render when streaming or compacting or parent-paused or any live subagent; abort dequeues pending steer/follow-up messages back to the composer (TUI ESC parity) with immediate "stopping…" feedback; web_search/web_fetch now honor the AbortSignal (combined with their fetch timeout) so a stop actually cancels in-flight web calls; added abort-endpoint tests.
  • Finding 1c: new rehydrateBackgroundAgentsFromDisk() scans ~/.dreb/agent/subagent-sessions/, matches each child JSONL's parentSession header to the resumed session, and repopulates the in-memory registry (status inferred from the log tail) so completed subagents reappear after a resume. Idempotent; called once at RPC startup for resumed sessions.
  • Finding 10: auth middleware lets allowlisted-but-unpaired identities GET the SPA shell/assets so the pairing screen can render; every /api/* data route stays fail-closed.
  • Finding 3: root-caused the RPC tab-title failure — resolveModel() discarded the resolved provider and re-picked by id via getAvailable().find, selecting a same-id model on a provider the runtime can't serve (→ 404). Now resolves provider-aware (registry.find), falls back to the session's current model when the Explore-derived model is unreachable, and reports failures loudly via a new onError hook instead of swallowing.

Medium / UX / polish

  • Finding 2: pending steer/follow-up chips fetch unconditionally (dropped the stale fleet-snapshot guard) so they show mid-turn.
  • Finding 4: suggest_next marks needs-attention in both the client reducer and the server attention map (cleared on next agent_start).
  • Finding 5: deterministic fleet sort — project path alphabetical, then a new stable createdAt tiebreak; documented the determinism-over-recency UI principle in AGENTS.md.
  • Finding 9: bash opens by default in the transcript.
  • Finding 13: per-session composer draft persistence (module-level keyed map) survives fleet↔session navigation.
  • Finding 15: POST /api/server/restart + /api/server/info; settings restart button with a supervisor-aware kill-sessions warning, and a server-build version in the footer (stale-service visibility).
  • Finding 12: settings/models/agent-types work with zero live sessions via a lazily-spawned hidden utility runtime (kept out of the fleet); no more 503s.
  • Finding 8 polish: client request() turns Express HTML 404/500 bodies into clean METHOD /path → status messages instead of dumping raw markup into error boxes.

Notes

  • The false-positive items (1a/1b, 7, 8, 11, 12-lists) were confirmed stale-server artifacts; the restart button + server-build footer (finding 15) directly mitigate that failure mode.
  • The finding-3 follow-up (persisting background_agent_start/end into the parent JSONL for exact-vs-inferred resume) was explicitly scoped by the assessment as a separate, smaller task and is not included here — the disk rehydration covers the resume case.

Commit: 1c7fc24


Progress tracked by mach6

@m-aebrer

m-aebrer commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

Review Assessment

Human entrypoint into the mach6 loop — the maintainer's manual QA pass on the running dashboard is the "review" here, and this comment is the independent assessment of it: each item verified against the actual source on feature/issue-307-dashboard-foundation, classified, and turned into an ordered action plan. All items are treated as genuine per the reporter; the assessor's value-add is the code-level root cause, and flagging the two items where the requested capability already partially exists so the implementer doesn't rebuild working plumbing.

⚠️ Corrected 2026-07-08 — read before implementing. Finding 1 (the /tmp resume bug) and action-plan step 3 have been rewritten to reflect the confirmed root cause and the maintainer's explicit direction. The earlier "validate cwd / fall back to a valid cwd" wording was wrong and is forbidden: never substitute a fallback cwd or open a new/blank session behind a "resume" click — that is a loud illusion, strictly worse than a silent failure. The corrected fix is to not display sessions whose cwd is gone, plus fix the test that leaks orphan session logs. This comment is the source of truth for the handoff; implement from step 3 as written below. All other findings/steps are unchanged.

Classifications

Finding Classification Reasoning
1. /tmp sessions can't be resumed — clicking resume does nothing genuine — root cause now confirmed (corrected below) Confirmed by console error: Uncaught (in promise) Error: Working directory does not exist: /tmp/dreb-rpc-test5. Ground truth: the .jsonl session logs persist in ~/.dreb/agent/sessions/--tmp-…--/, but their real cwd (a /tmp subdir like /tmp/dreb-rpc-test5) has been deleted. POST /api/runtimes guards existsSync(cwd) (server/server.ts:197-200) and returns a clean 400; the client resume() (screens/fleet.tsx:283-287) has no try/catch, so the rejection is silently swallowed → "does nothing." fleetGroupKey (display-only grouping) is a red herring — resume already passes each session's real cwd/path. The fix is NOT to fall back to another cwd — opening a different/blank session behind a "resume" click is a loud illusion, strictly worse than a silent fail, and is forbidden. The real fix: a session whose cwd is gone is unresumable and must not be displayed as resumable. Two layers: (a) hide cwd-missing disk sessions; (b) fix the test that leaks the orphan logs. Delete (DELETE /api/sessionsSessionManager.deleteSession(path)) is path-based and works fine, but becomes moot once orphans are hidden.
2. Add a "stop runtime" button to the session/chat view genuine (partially exists) The session view already has a mid-turn "■ stop" (abort current turn) at screens/session.tsx:940, wired to api.abort → RPC abort. What's missing is the whole-runtime kill the reporter is asking for: api.stopRuntime(key)DELETE /api/runtimes/:key (server.ts:211) exists but is only surfaced in the fleet view (fleet.tsx:118-127). Genuine gap: expose stop-runtime in the session view (likely the "⋯" overflow menu at session.tsx:799-816).
3. Model selector truncates long model names; needs wrap/resize genuine No native <select> for models anywhere — all three pickers are custom modal lists. Truncation is deliberate ellipsis with no escape hatch: .model-switcher { max-width: min(340px,45vw) } (app.css:409), .model-id / .model-name / .model-picker-button / .agent-model-chip all overflow:hidden; text-overflow:ellipsis; white-space:nowrap (app.css:1493, 1825, 1832, 1536), modal capped at max-width:480px (tokens.css:450). No wrapping, no resize, and no title= attr carrying the full name on the truncated model rows — so a long id is unrecoverable except by widening the window.
4. Per-tool auto-expand toggles in settings (keep tools open but collapse BASH) genuine No per-tool granularity exists. Tool-card auto-expand is a hardcoded compile-time allowlist LEGIBLE_OPEN_TOOLS = {read, edit, write, suggest_next, bash} (components/transcript.tsx:241, applied :271) — not a preference. The only persisted auto-expand pref is dreb.dashboard.expandThinking (state/preferences.ts:3), which controls thinking blocks, not tools. SettingsDto (shared/protocol.ts:191-208) has no tool-expand field. Delivering "keep tools expanding but collapse bash" requires a new per-tool-type preference map.
5. Stream BASH output live while it runs genuine (verify — plumbing already exists) The full streaming path is already wired end-to-end: bash onUpdate emits rolling-tail chunks (core/tools/bash.ts:296-337) → tool_execution_update event (agent-loop.ts:774-784) → RPC stdout → RpcClient.onEventRuntimePool.handleEvent → SSE hub → reducer mutates entry.resultText in place (state/reducer.ts:411-422) → revisions bump forces reactive re-render (store.ts:63-86) → HighlightedPre in the (open-by-default) bash card. So live bash streaming should already work. If the maintainer isn't seeing it, this is a verification/rendering item on the running build, not net-new plumbing — only escalate to a real fix if it's confirmed broken.
6a. Pairing PIN copy claims it's "displayed on the dreb terminal" — false genuine The PIN is console.log'd to the dashboard server process stdout (src/index.ts:146-148), only in --remote mode, once at server startup with a 5-min TTL — it has nothing to do with the TUI, and as a systemd service that output goes to journald. By the time a device pairs, the PIN is almost always already expired, and pairing a new device requires restarting the whole server. The copy at screens/pairing.tsx:87 ("showing in the dreb terminal on the host right now") is doubly wrong (wrong surface + not "right now").
6b. Tailscale identity shows "unknown" even though device is approved genuine The 401 needsPairing response does include the identity (server/server.ts:110: identity: decision.identity?.loginName) and api.ts:48 attaches the body to the thrown error — but the client's start() catch drops it: setAuth({ mode:"remote", needsPairing: err?.body?.needsPairing ?? false, error: err?.message }) (state/store.ts:116) never reads err.body.identity, so pairing.tsx:60 falls back to the literal "unknown". One-line data-flow bug.

Action Plan

Ordered by priority (auth correctness first, then data safety, then UX):

  1. Fix pairing identity passthrough (6b) — in state/store.ts:116, thread identity: err?.body?.identity into setAuth. Trivial, high-signal.
  2. Rework the pairing PIN lifecycle + copy (6a) — this is a solved problem; adopt a standard rather than the one-shot-at-startup PIN:
    • Generate the pairing code with TOTP (RFC 6238) from a server-side secret + current time (6 digits, 30s step, ±1 window tolerance). No restart to re-pair, never "stale."
    • Surface it live in the Settings tab, gated to the already-trusted loopback session (Mode A grants full local access with no auth, so the local user is authorized to see the current code). The Settings pane displays and auto-refreshes the current TOTP, and the pairing endpoint validates the submitted code against it.
    • Rewrite pairing.tsx:87 copy to point at "the dashboard Settings tab on the host machine," removing the false TUI claim.
    • (Keep printing a fallback to server stdout for headless hosts, but the settings surface becomes the primary path.)
  3. Fix /tmp (cwd-gone) resume — CORRECTED (this supersedes the earlier "fall back to a valid cwd" wording, which is forbidden). The reporter clarified the intent, and a console error pinned the root cause. Do NOT ban /tmp, and do NOT substitute a fallback cwd or open a new/blank session behind a resume click — that is a loud illusion and is worse than a silent failure. There are two layers:
    • 3a — Hide unresumable sessions (primary, dashboard). A disk session whose cwd no longer exists cannot be resumed, so it must not be listed. Filter it out server-side (the browser cannot stat): in GET /api/fleet (server/server.ts ~186-192, where diskSessions = await options.listAllSessions() is assembled), drop entries whose cwd fails existsSync. Keep the filter here (not in core SessionManager.listAll, which the TUI also uses). Add a unit test in test/server.test.ts that a disk session with a missing cwd is absent from the fleet payload while a live-cwd one remains.
    • 3b — Loud resume error (defense-in-depth, dashboard). Wrap resume() in screens/fleet.tsx:283-287 in try/catch and surface the error to the user (toast/inline), never swallow the rejection. This only fires in the race where a cwd vanishes between fleet-fetch and click — it is a bug-catch, not a normal path. Do not add any fallback-cwd behavior.
    • 3c — Layer 2: fix the test that leaks orphan logs (root cause of the orphans). packages/coding-agent/test/utilities.ts createTestSession() (~line 250) creates tempDir = /tmp/dreb-test-<ts>-<rand> and calls SessionManager.create(tempDir) with no explicit sessionDir, so session logs are written to the real ~/.dreb/agent/sessions/--tmp-…--/. Its cleanup() removes the /tmp cwd but not the log dir → the orphan. Fix: pass an explicit isolated sessionDir inside the temp dir, e.g. SessionManager.create(tempDir, join(tempDir, "sessions")), so rmSync(tempDir, …) in cleanup removes the logs too. (This is the "fix the root cause / don't write to real ~/.dreb from tests" approach — preferred over cleaning the real dir after the fact.) Existing orphan dirs in the dev's ~/.dreb are harmless once 3a hides them and can be removed manually.
    • 3d — Comment at fleetGroupKey (screens/fleet.tsx:19-21) noting the /tmp grouping is display-only and that each session still resumes with its own real cwd/path.
    • Delete: works fine (path-based, runtime-independent). No change needed; with 3a in place there is intentionally no UI path to delete a cwd-gone orphan — that is acceptable, since 3c stops new orphans at the source.
  4. Add whole-runtime "stop runtime" to the session view (2) — surface the existing api.stopRuntime(key) in the session overflow menu (session.tsx:799-816); keep the existing "■ stop" mid-turn abort distinct.
  5. Per-tool auto-expand preferences (4) — replace the hardcoded LEGIBLE_OPEN_TOOLS set with a per-tool-type map persisted alongside expandThinking (browser-local is fine, matching the existing pattern), and expose the toggles in Settings so tools can stay open while bash collapses (or vice versa).
  6. Model selector legibility (3) — allow the model name to wrap or the modal/rows to widen (drop the nowrap/ellipsis on the modal list rows, and/or add a title= full-name attr on the truncated switcher and rows as a minimum). Cheapest partial fix: add title= everywhere a model label is clipped.
  7. Verify bash streaming on the running build (5) — confirm the already-wired tool_execution_update path renders live in the bash card; only open a real fix if it's demonstrably not updating.

Counts: 7 genuine · 0 nitpick · 0 false-positive · 0 deferred. Items 2 and 5 are genuine but their plumbing already partly (5: fully) exists — scope them as "expose"/"verify," not "build from scratch."


Assessment by mach6 (human-seeded review)

@aebrer

aebrer commented Jul 9, 2026

Copy link
Copy Markdown
Owner

Review Assessment

Review comment: #321 (comment)

Classifications

Finding Classification Reasoning
Finding 1: Rejected remote identities still never reach denial screen genuine Confirmed. store.start() only navigates to pairing on needsPairing; denied identities with identity but needsPairing: false remain on fleet and still start fleet/SSE work.
Finding 2: Pairing persistence can silently lose/corrupt paired devices genuine Confirmed. Pair/unpair/prune perform unserialized load-modify-save operations, and the file storage writes directly to the final JSON path.
Finding 3: Subagent hide button does not appear on mobile genuine Maintainer-observed bug accepted as real. The toggle is rendered inside the strip, but the mobile layout has no specific handling to keep the affordance visible/usable in the dock.
Finding 4: Pairing success/error flow is not tested genuine Confirmed. Tests cover pairing copy and mobile Enter suppression, but not api.pair, error display, or successful restart/navigation.
Finding 5: Fleet resume/delete actions are not tested genuine Confirmed. The implementation has resume/delete paths, but current fleet tests do not click them or assert the API calls/refresh behavior.
Finding 6: Fleet fetch failures silently render as “No sessions yet” genuine Confirmed. refreshFleet() failures are swallowed and the default empty fleet state falls through to the empty-state UI.
Finding 7: Pairing code brute-forceable online by allowlisted peers genuine Confirmed. /api/pair accepts unlimited attempts against a short numeric code with no throttling, lockout, or backoff.
Finding 8: Aborting queued turn silently drops inline image attachments genuine Confirmed. Queued steer/follow-up can include images, but pending/dequeue storage and RPC DTOs only preserve text.
Finding 9: Agent models settings depends on whichever runtime opened first genuine Confirmed. Agent discovery is cwd-sensitive but settings uses the first runtime or a hidden homedir runtime, making project-local agents unstable/omitted.
Finding 10: Settings remote-access controls only smoke-tested genuine Confirmed. Tests do not exercise pairing-code refresh, unpair, or restart controls.
Finding 11: Duplicate thinking-level toggle handler nitpick Duplication is real, but it is maintainability-only; current behavior is correct.
Finding 12: Duplicate retry-warning reducer branches nitpick Duplication is real, but this is a refactor suggestion rather than a correctness issue.
Finding 13: Duplicate auth-denial response shape nitpick The duplicate response shape could be factored, but both paths currently match and work.

Action Plan

  1. Fix denied-identity client routing so auth failures reach the pairing denial screen and do not start fleet/SSE work.
  2. Make pairing persistence safe by serializing mutations and writing pairings via temp file plus atomic rename.
  3. Add pairing attempt throttling/lockout and logging for repeated failed PIN attempts.
  4. Fix the mobile subagent strip layout so the hide/show toggle is always visible and usable.
  5. Preserve queued image attachments in pending/dequeue RPC DTOs, or disable inline-image queued steer/follow-up until supported.
  6. Track fleet load errors explicitly and render an error state instead of “No sessions yet.”
  7. Make settings agent-type discovery project-context explicit or intentionally aggregate project roots.
  8. Add missing tests for pairing success/error, fleet resume/delete, and settings remote-access controls.

Assessment by mach6

@aebrer

aebrer commented Jul 9, 2026

Copy link
Copy Markdown
Owner

Progress Update

Pushed dashboard review fixes and transcript rendering improvements.

Summary:

  • Fixed pairing/auth hardening, including denied identities routing to pairing, serialized/atomic pairing storage, and PIN lockout handling.
  • Preserved queued image attachments in RPC/dashboard pending and dequeue payloads while keeping text compatibility fields.
  • Made settings agent discovery cwd-aware and tightened dashboard fleet/session behavior around runtime actions.
  • Rendered dashboard thinking blocks through the same Markdown/DOMPurify path as assistant text, and matched that behavior in HTML export.
  • Added defensive Markdown visibility CSS and regression coverage for visible text around HTML comments.
  • Removed the attempted provider-side reasoning.summary: "detailed" change after log/live testing showed the recent OpenAI Codex heading-only summaries are upstream output, not dashboard rendering.

Validation:

  • npx vitest --run packages/dashboard/test/client/screens.test.tsx
  • npx vitest --run packages/ai/test/openai-responses-copilot-provider.test.ts packages/ai/test/openai-codex-stream.test.ts packages/dashboard/test/client/screens.test.tsx
  • npm test
  • npm run check
  • npm run build
  • npm run verify-workspace-links
  • Commit hook reran the test suite successfully before commit.

Commit: 7fa8bcb


Progress tracked by mach6

@m-aebrer

m-aebrer commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

Code Review (round 2 — post-fix state, commit 7fa8bcb)

Fresh multi-agent pass over the current HEAD, after the prior round's 13 findings were fixed. Completeness against issue 307 + SPEC §7/§9 is clean (all criteria met, PARITY dispositions all backed by shipping code). New findings below focus on security correctness, silent-failure paths, lifecycle races, dead code, and test gaps.

Critical

None found.

Important

Finding 1 — RPC child-process death is never surfaced; live sessions freeze silently
Severity: high · Confidence: 88
Files: packages/dashboard/src/server/runtime-pool.ts, packages/coding-agent/src/modes/rpc/rpc-client.ts

When a dreb --mode rpc child dies abnormally mid-turn (OOM, segfault, external kill), the browser keeps showing a frozen "streaming/running" transcript indefinitely with no error, and SSE still reports "connected." Chain of silence: RpcClient's procRef.on("exit") only calls failPendingRequests() and sets _dead = true — it never notifies eventListeners. RuntimePool subscribes to the child only via client.onEvent(...) and registers no exit/death handler, so it publishes nothing to the EventHub. The client's handleEnvelope only refreshes on agent_start/agent_end/background_agent_*, none of which arrive on a crash. The error only becomes visible if the user manually refreshes the fleet or sends a new prompt (502). For an observability tool this is exactly the silent failure the project forbids.
Fix: add an onExit(cb) notification to RpcClient (fired from the existing exit/error handlers with {code, signal}); in RuntimePool.create subscribe to it, call recordRuntimeError(...), and synthesize a terminal event through the hub (e.g. agent_end aborted + a runtime-error event) so SSE clients render the session as failed instead of frozen.

Finding 2 — Remote device cookie is explicitly set secure: false
Severity: high · Confidence: 95
File: packages/dashboard/src/server/server.ts (/api/pair, ~lines 150–154)

The remote pairing cookie is set with secure: false, so the bearer device token is allowed over plain HTTP. SPEC §6 requires device-token cookies to be HttpOnly + SameSite=Strict + Secure. This is remote-mode-only code, so the exemption for local dev doesn't apply here.
Fix: set secure: true for remote device cookies (remote runs over Tailscale, which provides TLS / can issue certs). If an insecure path is needed for tests, gate it to non-remote/test only.

Finding 3 — Pairing PIN is rotating but not single-use; reusable within its window
Severity: high · Confidence: 90
File: packages/dashboard/src/server/auth.ts (isValidPairingCode ~265–270, pair ~411–417)

isValidPairingCode() accepts the code for the previous/current/next TOTP window and pair() mints a new device token every time a code is accepted, with no consumed-code state. The same displayed PIN can therefore pair multiple devices during its accepted window — it is not single-use as the security model requires.
Fix: return the matched window from validation and track/persist consumed windows (or rotate the PIN immediately after a successful pair), rejecting a code once consumed.

Suggestions

Finding 4 — RuntimePool.stopAll() can miss runtimes that are still starting
Severity: medium · Confidence: 90
File: packages/dashboard/src/server/runtime-pool.ts

Both session and utility runtimes are registered in their maps only after client.start() resolves. If shutdown/restart happens while a start() is in flight, that child is in neither map and stopAll() won't stop it; a utility start() that resolves after stopAll() cleared utilityPromises can even re-insert a handle post-shutdown, leaking child processes.
Fix: track pending-startup handles and include them in stopAll(), or register handles in a starting state before awaiting; add a closing flag so a start completing after stopAll() immediately stops its client.

Finding 5 — REST hydration can clobber newer live SSE state
Severity: medium · Confidence: 85
File: packages/dashboard/src/client/state/store.ts (hydrateSession, hydrateSubagent)

SSE is live while hydration REST snapshots are in flight. If a token/message/tool event arrives after the snapshot is taken but before hydration assigns session.entries/sub.entries, the assignment discards the newer live mutations (also affects streaming/workingSince).
Fix: make hydration revision-aware — capture the per-session revision before the REST calls and only replace entries if no SSE event touched that session since; otherwise merge/dedupe or retry.

Finding 6 — Chain subagent logs live under step-* subdirs but disk discovery only checks the chain root
Severity: medium · Confidence: 90
Files: packages/coding-agent/src/core/tools/subagent.ts, packages/dashboard/src/server/subagent-log.ts

Chain mode writes each step's session into chain-.../step-N/, but the background agent's sessionDir is the chain root and discoverSubagentSessionFile() only scans .jsonl files directly in that dir (non-recursive). After a dashboard reload, chain subagent drill-in cannot hydrate from disk. rehydrateBackgroundAgentsFromDisk()/discoverSessionFile() share the same non-recursive assumption.
Fix: write an aggregate chain log at the chain root, or carry step session files in metadata and concatenate them in step order; update the discovery/rehydration paths accordingly.

Finding 7 — Dead server code: FileApi.createDownloadStream and FileApi.renameEntry
Severity: medium · Confidence: 88
File: packages/dashboard/src/server/files.ts

createDownloadStream has zero call sites (the download route uses res.download(real)); removing it also makes the createReadStream import unused. renameEntry is a public mutating method with no /api/files/rename route and no test. Both are orphaned. (Note: this interacts with the test-gap finding below — the right resolution for renameEntry is removal, not adding tests, unless a rename endpoint is intended to ship.)
Fix: remove both (and the now-unused import), or wire renameEntry to a route + test if rename is actually intended.

Finding 8 — /api/pair cookie security attributes are never asserted
Severity: high (test) · Confidence: 85
File: packages/dashboard/test/server.test.ts

auth.pair() and the client pairing flow are tested, but the server endpoint that sets the device cookie is not — nothing asserts the Set-Cookie carries HttpOnly, SameSite=Strict, Secure, expiry, and the correct cookie name. This is exactly where Finding 2 (secure: false) slipped through. A regression dropping HttpOnly or flipping SameSite would pass every existing test.
Fix: spy auth.pair to resolve {token, device}, POST /api/pair, and assert the cookie attributes plus that a rejected pair maps err.status (401/429) onto the HTTP response.

Finding 9 — Runtime-pool lifecycle gaps untested: stopAll, ensureUtilityRuntime retry/dedup
Severity: medium (test) · Confidence: 82
File: packages/dashboard/test/runtime-pool.test.ts

stopAll (stops all session + utility runtimes, clears both maps) has no test — a regression that forgets utility runtimes leaks child processes silently. ensureUtilityRuntime's deliberate retry-on-failure (clears utilityPromises so a transient start() failure isn't cached) and in-flight dedup are also untested.
Fix: test that stopAll() stops ≥2 runtimes + a spawned utility and leaves list() empty; test that a first-call start() failure retries on the next call, and that concurrent same-cwd calls resolve to one handle / one spawn.

Finding 10 — Reducer branches untested: compaction-error, compaction-aborted, length_retry; plus server restart/delete routes
Severity: low (test) · Confidence: 80
Files: packages/dashboard/test/reducer.test.ts, packages/dashboard/test/server.test.ts

auto_compaction_end with errorMessage (should push an error status), with aborted:true (should push no summary), and length_retry (independently duplicates stream_retry's tail-removal, can silently diverge) are untested. Server-side, /api/server/restart (501-without-onRestart vs 200 + deferred call) and DELETE /api/sessions (400-on-empty-path) branches are untested at the HTTP layer.
Fix: add targeted reducer cases and the two HTTP branch tests.

Findings 11–13 — Behavior-preserving simplifications (low)
Severity: low · Confidence: 82–90
File: packages/dashboard/src/client/screens/session.tsx, settings.tsx, components/transcript.tsx

(11) modelMatchesQuery + groupedModels + ModelChoice are duplicated byte-for-byte across session.tsx and settings.tsx — hoist to a shared state/models.ts. (12) pendingMessageItems repeats the steer/follow-up normalize mapping twice — extract a local normalize(list, kind). (13) In transcript.tsx, the <Show when={...bash...}> re-inlines the already-defined isBash() accessor, and adjacent write/edit switch cases with identical bodies can share a fall-through.

Prior-round low findings still present: duplicate thinking-level toggle handler (session.tsx), duplicate retry-warning reducer branches (reducer.ts — now three: auto_retry_start/stream_retry/length_retry), duplicate auth-denial response shape (server.ts middleware vs /api/auth).

Strengths

  • Local Host/Origin DNS-rebinding checks are fail-closed and explicit; Tailscale allowlist defaults deny; empty allowlist denies.
  • Fail-closed auth wrapper converts any thrown error to {allowed:false, status:500}; resolver failures deny.
  • File API canonicalization (null-byte, percent-encoding, symlink realpath) and atomic link+EEXIST no-overwrite publish with temp cleanup are correct.
  • Untrusted JSONL parsing (subagent-log, handleChildJsonlLine, rehydrateBackgroundAgentsFromDisk) fails on partial/garbage lines without masking real bugs.
  • SSE event-hub isolates per-client write failures and emits dashboard_resync on buffer gaps; reducer/store split is clean and testable.
  • Completeness against issue 307 + SPEC §7/§9 is fully met with no dead UI for deferred items.

Agents run: code-reviewer, error-auditor, test-reviewer, completeness-checker, simplifier


Reviewed by mach6

@m-aebrer

m-aebrer commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

Review Assessment

Review comment: #321 (comment)

Each finding was independently verified by reading the actual source at commit 7fa8bcb.

Classifications

Finding Classification Reasoning
1 — RPC child death never surfaced genuine rpc-client.ts procRef.on("exit") only calls failPendingRequests(); no death API on RpcClient, eventListeners never notified on crash. runtime-pool.ts create() subscribes only via onEvent — no exit handler. A mid-turn crash pushes nothing to the hub; live SSE stays frozen until an explicit fleet refresh.
2 — Cookie secure: false false positive server.ts:153 has an explicit comment: Tailscale already encrypts and the dashboard serves plain HTTP on the tailnet. Remote mode only accepts Tailscale-resolved peers; the cookie only travels over the WireGuard tailnet. secure: true would stop the browser sending the cookie over http:// and break remote auth. HttpOnly + SameSite=Strict are set. See follow-up note re: SPEC wording.
3 — PIN not single-use genuine pair() validates via isValidPairingCode() (accepts window −1/0/+1), mints a token, no consumed-code state, no PIN rotation on success. Same PIN pairs multiple devices within its ~90s window. Mitigated (attacker must already be an allowlisted identity) but real.
4 — stopAll() misses starting runtimes genuine Handles are registered after await client.start(); a shutdown during startup won't stop the in-flight child, and a utility start resolving after stopAll() re-inserts post-shutdown → leaked process. Server has a restart route, so this path is reachable.
5 — REST hydration clobbers live SSE genuine (low) hydrateSession() awaits REST then unconditionally overwrites entries/streaming/workingSince; a per-session revisions store exists but isn't consulted. Narrow, self-healing race.
6 — Chain subagent logs undiscoverable genuine Chain root registered as sessionDir but steps write to step-N/; discoverSubagentSessionFile/discoverSessionFile are non-recursive, so chain drill-in can't hydrate after reload (live relay still works).
7 — Dead code createDownloadStream / renameEntry genuine Repo-wide grep: only definitions, zero call sites. Download route uses res.download(real). createReadStream import used only by the dead method.
8 — /api/pair cookie attrs untested genuine (test) No POST /api/pair test asserts the Set-Cookie attributes — the exact surface where a security regression passes silently. Ships with the code, not deferred.
9 — stopAll/ensureUtilityRuntime untested genuine (test) No coverage for the utility-leak or retry/dedup paths — the ones most likely to regress silently.
10 — Reducer/route branches untested genuine (test, low) No compaction-error/aborted, no length_retry, no restart 501-vs-200, no delete empty-path 400 tests.
11 — model helper duplication nitpick Real duplication (session.tsx vs settings.tsx), maintainability only.
12 — pendingMessageItems dup mapping nitpick Real but stylistic.
13 — isBash() re-inline / dup switch cases nitpick Cosmetic.

Tally: 8 genuine (3 of them test-coverage), 1 false positive, 4 nitpicks (incl. prior-round dup findings that persist).

Action Plan

  1. Finding 1 — surface RPC child death. Add onExit(cb) to RpcClient (fire from existing exit/error handlers with {code, signal}); in RuntimePool.create() subscribe, call recordRuntimeError(...), and synthesize a terminal event through the hub so SSE renders the session as failed, not frozen. (highest priority — silent failure in an observability tool)
  2. Finding 3 — make the pairing PIN single-use. isValidPairingCode() returns the matched window; pair() tracks/persists consumed windows (or rotates the secret on success) and rejects a consumed code.
  3. Finding 6 — fix chain subagent disk discovery. Aggregate log at the chain root (or record step files in metadata); update discovery/rehydration to find/concatenate step-N/ logs in order.
  4. Finding 4 — close the stopAll() startup race. Register handles in a starting state (or track pending-startup promises) + a closing flag so late start() resolutions immediately stop their client.
  5. Finding 8 — add the /api/pair cookie-attribute test. Assert HttpOnly/SameSite=Strict/expiry/name and that rejected pairs map err.status (401/429) to the HTTP response.
  6. Finding 5 — make hydration revision-aware. Capture the per-session revision before REST; only replace state if no SSE event touched the session since.
  7. Finding 7 — remove dead code. Delete createDownloadStream + renameEntry and the now-unused createReadStream import.
  8. Finding 9 — test runtime-pool lifecycle (stopAll, utility retry/dedup).
  9. Finding 10 — add reducer/route branch tests (compaction-error/aborted, length_retry, restart 501/200, delete 400).

Skip (nitpicks): Findings 11–13 — optional behavior-preserving cleanup, not merge-blocking.

Follow-up (not merge-blocking): Finding 2 is a false positive as a security hole, but SPEC §6 literally lists "Secure" for device cookies while the code intentionally omits it on the plain-HTTP tailnet. Reconcile the SPEC wording (or add optional tailscale serve HTTPS) so the doc matches the shipped tailnet-trust model.


Assessment by mach6

…gle-use PIN, stopAll race, revision-aware hydration, chain subagent hydration, dead code removal, test gaps
@m-aebrer

m-aebrer commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

Progress Update

Fixed the eight genuine findings from the round-2 review assessment. Skipped finding 2 (false positive — secure:false is the intentional tailnet-trust decision, documented in server.ts) and findings 11–13 (nitpicks).

Fixes

  • Finding 1 — silent RPC child death (high): RpcClient.onExit() added, fired from the existing child exit and error handlers with {code, signal}; RuntimePool subscribes, calls recordRuntimeError, and publishes a synthetic terminal agent_end through the EventHub so SSE clients render a crashed session as failed instead of frozen.
  • Finding 3 — single-use pairing PIN (high): validation now returns the matched TOTP window; a successful pair consumes that window (persisted in pairing storage, survives restart) and any reuse is rejected and counts toward the lockout.
  • Finding 4 — stopAll() startup race (medium): closing/starting flags plus pending-start tracking; a start() that resolves after shutdown begins stops its client immediately instead of leaking a child.
  • Finding 5 — hydration clobbering live SSE (medium): hydrateSession/hydrateSubagent capture the per-session revision before the REST round-trip and skip the snapshot overwrite (entries, streaming, workingSince) if an SSE event advanced the session meanwhile.
  • Finding 6 — chain subagent hydration (medium): one-level recursive discovery into step-N/ subdirs with ordered concatenation, applied to both the dashboard subagent-log reader and the coding-agent rehydrateBackgroundAgentsFromDisk path (chain roots are no longer skipped).
  • Finding 7 — dead code (medium): removed createDownloadStream and renameEntry from files.ts and the now-unused createReadStream import.
  • Findings 8, 9, 10 — test gaps: /api/pair cookie attributes (HttpOnly/SameSite=Strict/expiry, intentional absence of Secure, rejection-status passthrough); runtime-pool lifecycle (stopAll, utility retry/dedup, child-exit publication); reducer compaction-error / aborted / length_retry; and /api/server/restart (501 vs 200) + DELETE /api/sessions (400) route branches.

Verification

  • npm run build — green (tsgo across all packages + Vite client build)
  • npm run check — biome + tsgo --noEmit clean (674 files, no warnings)
  • npm test — 4290 passed, 0 failed (18 files changed, +1204/−181; ~433 lines new tests). One flaky live-API E2E in packages/ai (GitHub Copilot image-input, 30s network timeout — untouched by this PR) blocked the first commit hook run; confirmed passing on rerun and the pre-commit suite passed clean on commit.
  • npm run verify-workspace-links — clean

Follow-up (not merge-blocking)

Finding 2 is a false positive as a security hole, but SPEC §6 literally lists "Secure" for device cookies while the code intentionally omits it on the plain-HTTP tailnet. The SPEC wording should be reconciled (or optional tailscale serve HTTPS added) so the doc matches the shipped tailnet-trust model.

Commit: 081ea3f


Progress tracked by mach6

@m-aebrer

m-aebrer commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

Code Review — targeted pass: in-browser performance & memory leaks

Maintainer-requested focus round on browser performance, memory growth, and resource lifecycle for the dashboard client (plus the server ends of the SSE pipeline that feed the browser). Three agents ran: a rendering-performance reviewer, a browser-memory/lifecycle reviewer, and an error-auditor on the SSE pipeline. Overlapping findings are merged; confidence shown is the highest among reporting agents.

Critical

None found.

Important

Finding 1 — Every SSE envelope deep-clones and reconciles the entire session transcript
Severity: high · Confidence: 95 (reported independently by 2 agents)
File: packages/dashboard/src/client/state/store.ts (syncSession, handleEnvelope)

handleEnvelope calls syncSession(key) for every applied envelope — including token-level message_update deltas — and syncSession does setSessions(key, reconcile(structuredClone(session))). structuredClone copies the whole SessionViewState (all entries, tool outputs, nested subagent transcripts) on every event, and reconcile then diffs it. Cost is O(total transcript size) per single-character delta, not O(change). Two multipliers: the event hub fans every runtime's events to every browser (so the tab pays this for all streaming sessions, not just the viewed one), and entries are append-only so the per-delta cost grows monotonically. After hours of streaming, each delta can clone multiple MB dozens of times per second — CPU spikes and GC churn exactly when the UI should be responsive. Direction: granular store patches for the touched path only, or clone/sync only sessions with mounted subscribers, or coalesce/throttle streaming syncs.

Finding 2 — Client state is never pruned: every session and subagent transcript is retained forever
Severity: high · Confidence: 96 (reported independently by 2 agents)
Files: packages/dashboard/src/client/state/reducer.ts, packages/dashboard/src/client/state/store.ts

applyEnvelope lazily creates SessionViewState for any key seen on the wire and appends every event to it — the SSE stream carries all runtimes, so the browser materializes and grows full transcript state for every session on the host, including ones the user never opens. Nothing ever removes a session: the reducer only clears on dashboard_resync (server-restart recovery), never per-key; a runtime that exits leaves its state (and the mirrored Solid sessions/revisions stores — at least two retained copies) resident forever. Subagent transcripts nest under the parent and are retained the same way. A day-long tab with several active agents grows heap without bound and never plateaus. Direction: evict reducer + store state on runtime stop/delete, don't materialize full transcript state for never-viewed sessions, cap retained entries for off-screen sessions.

Finding 3 — No transcript virtualization; unbounded DOM growth; collapsed tool bodies are eagerly mounted
Severity: high · Confidence: 98
Files: packages/dashboard/src/client/screens/session.tsx, packages/dashboard/src/client/components/transcript.tsx

The session view renders <Transcript entries={session()!.entries} /> with no windowing and no entry cap, and tool result bodies (highlighted read/write/bash output, markdown) are mounted inside <details> even when collapsed. At thousands of entries the browser keeps every row component and its DOM alive; large tool outputs can push the DOM plus retained strings into hundreds of MB, and hydrating a long session must build the entire DOM before the screen is responsive. Collapsing a tool does not release its rendered DOM. Direction: windowing/virtualization for the transcript, lazy-mount tool bodies on expand, truncate very large outputs behind an explicit "open full output" affordance.

Finding 4 — Streaming re-parses markdown / re-highlights the full accumulated text on every delta
Severity: high · Confidence: 94
Files: packages/dashboard/src/client/components/transcript.tsx, packages/dashboard/src/client/state/reducer.ts

Streaming assistant blocks render via innerHTML={renderMarkdown(block.text)} where renderMarkdown runs marked.parse + DOMPurify.sanitize on the full accumulated text; each token delta re-parses the whole message. Running tool output similarly re-highlights the entire accumulated resultText via highlight.js on every tool_execution_update. The reducer also grows streaming text with block.text += delta, which is quadratic for very large single messages. A 100KB streamed response causes many MB of repeated parse/sanitize work; a long-running bash command re-highlights its full output repeatedly. Direction: render plain text during streaming and parse/highlight once on finalization (or debounce), and avoid whole-string reaccumulation for large outputs.

Finding 5 — Transcript list defeats SolidJS keyed reuse: fresh wrapper objects every render
Severity: high · Confidence: 92
File: packages/dashboard/src/client/components/transcript.tsx (transcriptRenderItems, Transcript)

<For each={transcriptRenderItems(props.entries)}> rebuilds a new array of new wrapper objects ({ kind: "assistant-turn", entries } / { kind: "entry", entry }) on every run; <For> keys by item identity, so appending one entry to a 5k-entry transcript can remount/reprocess a large portion of the list instead of just the tail — and rows contain markdown and syntax-highlighted code, so remounts are expensive. Transcript entries themselves also lack stable IDs, which compounds with the reconcile-based cloning in finding 1. Direction: stable render-item identities (stable entry IDs, memoized grouping) so unchanged rows keep their DOM.

Finding 6 — SSE fan-out ignores res.write() backpressure toward slow/backgrounded clients (server)
Severity: high · Confidence: 85
Files: packages/dashboard/src/server/event-hub.ts, packages/dashboard/src/server/server.ts

The hub's publish loop and the keepalive both call res.write(chunk) and discard the return value; try/catch only guards throws (closed sockets), not backpressure. A slow consumer returns false and Node buffers the frame in the socket's userland writable buffer — the producer (RPC children streaming deltas) never slows down, so one throttled background tab or half-open connection grows server heap without bound during heavy streaming. The replay ring is correctly bounded at 2000; this is specifically live-write buffering. Direction: track per-client writableLength / write return, and drop or disconnect clients over a threshold (a dropped SSE client reconnects with Last-Event-ID).

Suggestions

Finding 7 — Autoscroll work scheduled per SSE revision without coalescing
Severity: medium · Confidence: 87
Files: packages/dashboard/src/client/screens/session.tsx, packages/dashboard/src/client/screens/subagent.tsx

The session screen schedules a double requestAnimationFrame scroll-to-bottom on every revision bump (every envelope, including token deltas) with no coalescing — many pending rAF callbacks can stack during fast streams, each reading scrollHeight (not free at thousands of DOM nodes) and writing scrollTop. The subagent screen does the read/write synchronously inside the reactive effect. Direction: coalesce to at most one pending frame; share the deferred strategy between both screens.

Finding 8 — Base64 image attachments held in reactive state and duplicated
Severity: medium · Confidence: 86
File: packages/dashboard/src/client/screens/session.tsx

Pending attachments are stored as base64 strings (10MB raw cap each, +33% expansion, no total/count cap) in a Solid signal, duplicated into a data: URL string for the <img> preview, and mapped into another array on send. A few screenshots can retain tens of MB in reactive state. Direction: preview via URL.createObjectURL(file), keep the File/Blob and encode to base64 only at send time, add a total-attachment cap.

Finding 9 — Toast arrays grow unbounded and old toasts become undismissable
Severity: medium · Confidence: 92
Files: packages/dashboard/src/client/state/reducer.ts, packages/dashboard/src/client/app.tsx

Extension notifications/errors push permanently onto each session's toasts array; the UI renders only the newest five across all sessions, so older toasts are retained but have no visible dismiss affordance. A noisy extension over hours accumulates hidden toast objects across all retained sessions. Direction: cap or auto-expire toasts at the reducer layer.

Finding 10 — Composer drafts and prompt history live in module-scope maps with no eviction
Severity: medium · Confidence: 91
File: packages/dashboard/src/client/screens/session.tsx

composerHistory / composerDrafts are module-scope Maps keyed by session key, written on every edit/send, never deleted on runtime stop, session delete, or navigation. History is capped at 100 entries per session but the number of session keys is unbounded, and user prompt text is retained after the session UI is gone. Direction: evict entries when a session is stopped/deleted, or bound by recency.

Finding 11 — Completed background agents accumulate in RuntimeHandle.backgroundAgents and inflate every fleet payload
Severity: medium · Confidence: 82
File: packages/dashboard/src/server/runtime-pool.ts

background_agent_start adds to the map; background_agent_end only mutates status — no eviction. describe() spreads the full map into every /api/fleet response, and the client refreshes the fleet on every background-agent lifecycle event across all sessions, so a session that fans out hundreds of subagents over a day re-ships its full historical agent list on each event. Direction: keep running + last N completed.

Finding 12 — Route-owned hydration fetches are not abortable; in-flight requests retain disposed screen closures
Severity: low · Confidence: 84
Files: packages/dashboard/src/client/api.ts, packages/dashboard/src/client/screens/session.tsx, packages/dashboard/src/client/screens/subagent.tsx, packages/dashboard/src/client/state/store.ts

The shared request() wrapper never takes an AbortSignal; full-log hydration requests started on mount cannot be cancelled on unmount. Rapid navigation between large sessions (or a stalled server) leaves multiple full-transcript fetches in flight, each retaining screen closures and eventually the large response body. Some paths guard setters with a disposed flag but nothing aborts the network work. Direction: per-screen AbortController wired through the API layer, aborted in onCleanup.

Observation (low, informational) — the files tab renders every directory entry with no pagination; very large directories (dependency caches) create thousands of table rows. Worth windowing eventually, but less central than the transcript findings.

Strengths

  • SSE per-connection lifecycle on the server is correct: req.on("close") clears the keepalive timer and detaches from the hub; the replay ring is bounded (2000) with correct eviction.
  • Client reconnect dedupe is correct: lastEventId tracking + ?lastEventId= replay means no duplicate appends; revision-aware hydration guards the hydrate-vs-SSE race.
  • subagent-log.ts is stateless per-request (no watchers/fds to leak); RpcClient onEvent/onExit are registered exactly once per handle.
  • Component lifecycle hygiene is good: document listeners, intervals, scroll-release timers, and the EventSource + reconnect timer are all cleaned up via onCleanup; no un-disconnected observers found.
  • Fleet rendering is bounded (capped model lists, sliced recents, deterministic ordering); composer history is capped per session; localStorage use is fixed-key.

Agents run: code-reviewer (rendering performance), code-reviewer (memory/lifecycle), error-auditor (SSE pipeline)


Reviewed by mach6

@m-aebrer

m-aebrer commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

Review Assessment — performance/memory round

Review comment: #321 (comment)

Every finding was independently verified against the actual source at commit 081ea3f. Git log and prior PR discussion were checked — none of these were previously fixed or rejected (prior rounds addressed different bugs: toast dismiss setter, stop button, stopAll race, RPC child death). This is the first perf-focused round.

Classifications

Finding Classification Reasoning
1 — Deep-clone + reconcile whole transcript per envelope genuine (high, architectural) Confirmed: syncSession runs setSessions(key, reconcile(structuredClone(session))) for every keyed envelope; both walk the entire append-only session state. O(transcript) per token delta, no batching — SSE onmessage processes one envelope at a time.
2 — Client session state never pruned genuine (moderate leak) Confirmed: only dashboard_resync clears; no per-key delete exists anywhere. Stopped runtimes leave transcripts resident in both the reducer Map and the Solid store mirror (2 copies). Also the enabler for items 10 and 11 client-side — there is no session-removed signal to drive any eviction.
3 — No virtualization; eager collapsed tool bodies genuine (high, architectural) Confirmed: all entries rendered, no cap/windowing; ToolCard mounts highlighted/markdown bodies inside <details> regardless of open state (closed <details> keeps children in DOM).
4 — Full re-parse of accumulated text per delta genuine (high, architectural) Confirmed: renderMarkdown/highlightedHtml are unmemoized render functions; streaming mutates block.text/resultText per token and re-parses the full growing string each time — O(n²) per streamed message.
5 — <For> wrapper objects defeat keyed reuse genuine (high, architectural — worst compounder) Confirmed and NOT dominated by finding 1: reconcile preserves entry identity, but transcriptRenderItems() throws it away with fresh wrapper objects on every structural change. One append in a 5k-entry transcript remounts the entire list and re-parses all rows' markdown/code.
6 — SSE fan-out ignores res.write() backpressure genuine (moderate, independent) Confirmed: publish loop and keepalive discard the write return value; try/catch only catches throws, not false. No writableLength check or destroy-on-overflow anywhere.
7 — Autoscroll per revision, no coalescing genuine (low, quick win) Confirmed: double-rAF scheduled per envelope in session view; subagent view does a synchronous scrollTop = scrollHeight (forced reflow) inside the effect. Trivial pending-flag fix.
8 — Base64 images in reactive state, no total cap genuine (low) Confirmed: 10MB per-image cap but no aggregate cap; base64 held in a signal, duplicated into data: URL per render. Bounded by user action and cleared on send, so low severity.
9 — Toasts grow unbounded / undismissable genuine (low) Confirmed: reducer pushes forever, UI renders .slice(-5); older toasts retained with no dismiss affordance. Distinct from the prior-round toast-dismiss bug that was fixed.
10 — Composer history/drafts never evicted genuine (low leak) Confirmed: module-scope Maps, .set on send/draft, no .delete anywhere. Per-session cap of 100 but unbounded session keys; retains user prompt text after sessions are gone.
11 — backgroundAgents map grows unbounded, ships in every fleet payload genuine (low, scale) Confirmed: start adds, end only mutates status, no eviction; describe() ships the full map in every /api/fleet response and the client refreshes fleet on every bg-agent lifecycle event.
12 — Hydration fetches not abortable nitpick Real but transient: the revision guard drops the parsed body and it's GC'd — in-flight waste, not a persistent leak. "Response body retained" overstates it. Reasonable follow-up, not merge-blocking.

Counts: 11 genuine, 1 nitpick, 0 false positives, 0 deferred.

Root-cause structure

Findings 1, 3, 4, 5 are one rendering-pipeline root cause, not four independent bugs. A single new tool result in a long session currently costs: O(transcript) clone (1) + O(transcript) reconcile (1) + full DOM remount via unstable wrappers (5) + re-parse of ALL rows' markdown/highlight (4), against an unbounded DOM (3). Fixing one without the others leaves the super-linear path intact.

Action Plan

Group A — rendering pipeline (findings 5 → 1 → 4 → 3), architectural, highest priority:

  1. Finding 5 first (highest leverage): stable transcript entry IDs / memoized grouping so <For> preserves row identity — prerequisite for the rest to matter.
  2. Finding 1: replace whole-session structuredClone+reconcile with granular store patches for the touched path (or sync only mounted sessions + coalesce streaming syncs to a frame).
  3. Finding 4: render plain text during streaming and parse/highlight on finalization (or debounce/rAF-coalesce the parse).
  4. Finding 3: lazy-mount tool bodies on <details> open (quick win) + transcript windowing (larger lift, tail of this group).

Group B — server backpressure (finding 6), moderate, independent:
5. Track per-client write return / writableLength; disconnect clients over a threshold (they reconnect via Last-Event-ID).

Group C — bounded-growth leaks, quick wins, do together:
6. Finding 2: session-removed signal on runtime stop; prune reducer + Solid store.
7. Finding 9: cap/auto-expire toasts in the reducer.
8. Finding 11: cap retained completed background agents (server + client).
9. Finding 10: evict composer history/drafts on the session-removed signal from step 6.

Group D — quick wins, low priority:
10. Finding 7: coalesce autoscroll (single pending-frame flag); rAF for the subagent scroll.
11. Finding 8: aggregate image-attachment size cap now; object-URL refactor as follow-up.

Follow-up (not merge-blocking):

  • Finding 12: thread AbortController through api.request(), abort in onCleanup.

Given the PR's stated targets (thousands of entries, day-long tabs, dozens of events/sec), Group A and the Group C leaks will bite normal usage — not just extreme scale.


Assessment by mach6

…c, identity-stable transcript items, throttled streaming markdown/highlight, lazy tool bodies, transcript windowing, large-output truncation
@m-aebrer

m-aebrer commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

Progress Update — perf-review Group A implemented

Implemented the four architectural rendering-pipeline findings (Group A) from the performance/memory review assessment, in the assessor's recommended order (item 5 → 1 → 4 → 3). Pre-commit suite: 4300 passed, 0 failed; full workspace vitest + node:test green; build clean.

Shipped

  • Finding 5 — identity-stable transcript render items: transcriptRenderItems now reuses previous wrapper objects when they represent the same entry references (entry wrappers by reference, assistant-turn wrappers by ordered reference equality), wrapped in createMemo. Appending one entry re-renders only the affected tail item; all earlier rows keep their DOM.
  • Finding 1 — granular store sync: removed the reducerState mirror and the per-envelope structuredClone + reconcile entirely. The Solid store is now the single source of truth; reducer mutations apply through produce, so a token delta touches one leaf instead of cloning the whole session tree. Revision-bump semantics, dashboard_resync clearing, hydration race guards, and the "never clobber richer live state with an empty disk snapshot" rule are all preserved. The now-dead applyEnvelope/createDashboardState/DashboardState reducer layer was deleted (dead code) and its multi-session dispatch tests moved to the store test where that responsibility now lives.
  • Finding 4 — bounded streaming parse cost: new MarkdownBody component memoizes marked+DOMPurify output and throttles re-parses to ~150ms while the owning entry is streaming, with an immediate final flush when streaming ends. HighlightedPre gets the same memoization + running-state throttle. All transcript innerHTML call sites (assistant/thinking blocks, tool input sections, suggest summaries, agent-result cards) route through the memoized paths.
  • Finding 3 — bounded DOM: tool <details> bodies now mount only while open (running tools force open; running→done collapses unless auto-open preference applies; a user toggle afterwards wins; collapsing unmounts and releases the DOM). The transcript windows to the newest 150 render items with a "show earlier" affordance (+150 per click, hidden count shown), resetting per session/subagent. Tool outputs over 200KB render the trailing 200KB with an explicit "show full (size)" opt-in — no silent truncation.

Tests added

  • Store: fine-grained identity preservation through streaming deltas (entries array and untouched entry references stay reference-equal while the block text and revision advance); lazy per-key session creation; resync clearing.
  • Transcript: wrapper stability on append, lazy body mount/unmount incl. running-tool behavior, windowing with show-earlier, throttled streaming markdown with final-flush, oversized-output truncation opt-in.

Commit: 1862621

Remaining from the assessment action plan: Group B (SSE write backpressure), Group C (state eviction: session pruning, toast cap, background-agent cap, composer map eviction), Group D (autoscroll coalescing, image attachment cap), and the follow-up item (abortable hydration fetches).


Progress tracked by mach6

…nect, runtime_removed state eviction, toast cap, background-agent cap with subagent transcript cleanup, composer memory eviction
@m-aebrer

m-aebrer commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

Progress Update — perf-review Groups B+C implemented

Implemented the remaining moderate findings from the performance/memory review assessment: Group B (item 6) and Group C (items 2, 9, 11, 10). Pre-commit suite: 4316 passed, 0 failed; full workspace vitest + node:test green; build clean.

Server

  • Finding 6 — SSE write backpressure: every write on /api/events (handshake, hub fan-out, keepalive) goes through a guarded write that checks the res.write() return value. When a write is rejected and res.writableLength exceeds 4 MiB, the server logs loudly and destroys the connection; the existing close handler detaches the hub client and clears the keepalive, and the browser's EventSource reconnects via Last-Event-ID (replay or dashboard_resync). One slow/backgrounded tab can no longer grow server heap without bound.
  • Finding 11 — bounded background-agent registry: non-running agents are capped at 20 per runtime, evicting oldest-by-start deterministically after background_agent_end and after registry seeding. Running agents are never evicted. Fleet payloads stop growing with subagent history.
  • New eviction signal: RuntimePool.stop(key) (the DELETE runtime path) now publishes a synthetic runtime_removed event under the runtime's key before removal. Deliberately not emitted from stopAll() (server shutdown) or crash handling (crashes keep the handle visible with the error until the user deletes it).

Client

  • Finding 2 — session state eviction: on runtime_removed, the store deletes the session and revision state, evicts composer memory, and refreshes the fleet. If the user is viewing that session, the screen degrades to its loading fallback (no crash — covered by the existing guards).
  • Finding 9 — toast cap: per-session toasts capped at 20 in the reducer (UI shows only the newest few; the backing list no longer grows forever).
  • Finding 10 — composer memory eviction: drafts/history moved from module-scope maps in the session screen to a small composer-memory module with an evictComposerMemory(key) hook wired into runtime_removed. The 100-entry per-session history cap is unchanged.
  • Finding 11 (client mirror) — background-agent + subagent transcript cap: the reducer caps completed agents at 20 and deletes the orphaned subagents transcript state of evicted agents (the actual memory weight); the same cap applies when hydration seeds the registry.

Docs

  • packages/dashboard/README.md and packages/coding-agent/docs/dashboard.md document the runtime_removed event and the slow-client disconnect behavior.

Tests added

  • Server: over-buffered SSE client is destroyed and detached while other clients keep receiving; background-agent cap (lifecycle + seed paths); runtime_removed published on stop() and not on stopAll().
  • Client: runtime_removed deletes session/revision state, evicts composer memory, and triggers fleet refresh; toast cap; completed-agent + subagent transcript eviction; composer-memory round-trip; hydration cap.

Commit: 37f4baf

Remaining from the assessment action plan: Group D (item 7 autoscroll coalescing, item 8 image attachment handling) and the follow-up nitpick (item 12 abortable hydration fetches).


Progress tracked by mach6

… image attachments with aggregate cap and objectURL lifecycle, abortable hydration fetches
@m-aebrer

m-aebrer commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

Progress Update — perf-review Group D + follow-up implemented

Implemented the final items from the performance/memory review assessment: Group D (items 7 and 8) plus the follow-up nitpick (item 12). With this commit, every finding from the perf review's action plan is implemented — Group A (items 5, 1, 4, 3), Group B (item 6), Group C (items 2, 9, 11, 10), and now Group D. Pre-commit suite: 4345 passed, 0 failed; full workspace vitest + node:test green; build clean.

Shipped

  • Item 7 — coalesced autoscroll: new shared createCoalescedBottomScroller (client scrolling.ts): at most one pending double-rAF chain per surface; repeated requests while pending are no-ops; stick-to-bottom conditions are re-read at fire time so a stale scheduled scroll cannot fight the user. Wired into the session screen (replacing per-revision rAF scheduling), the subagent screen (replacing the synchronous scrollTop = scrollHeight forced reflow inside the reactive effect), and HighlightedPre's bash autoscroll.
  • Item 8 — attachment memory model: pending image attachments now hold Blob + previewUrl instead of base64 in reactive state. Previews use URL.createObjectURL with full revoke lifecycle (remove, successful send/clear, component cleanup, and error paths during dequeue-restore). Base64 encoding happens only inside send(). Added a 25MB aggregate cap across all pending images (loud error, same path as the per-image 10MB cap). Dequeued base64 image DTOs are converted to Blobs so restored attachments follow the same model.
  • Item 12 — abortable hydration: api.messages / backgroundAgents / runtime / subagentMessages accept an optional AbortSignal; hydrateSession / hydrateSubagent thread it through; session and subagent screens abort their mount hydration in onCleanup. Aborts are silent (shared isAbortError classifier); all other errors surface exactly as before. Hardened beyond the finding: an aborted hydrateSession is also guarded from creating a stub session and bumping its revision via the all-rejected Promise.allSettled path.

Tests added

  • Coalesced scroller unit tests (single pending chain under rapid triggers, fire-time condition re-check, cancel).
  • Attachment lifecycle: aggregate cap rejection, objectURL revoke on remove and on send, dequeue-restored base64 → Blob + preview.
  • Abortable hydration: unmount-before-resolve aborts the signal without setting error state; genuine failures still set it.

Commit: bd5f175

The review's action plan is fully implemented across four commits (Group A, Groups B+C, Group D). Ready for re-review.


Progress tracked by mach6

@m-aebrer

m-aebrer commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

Code Review (round 3 — perf-fix commits 1862621, 37f4baf, bd5f175)

Fresh multi-agent pass focused on the three unreviewed perf-fix commits (the perf review's 12-item action plan implementation), plus a completeness re-verification of the whole PR at HEAD bd5f175. Prior rounds' findings and intentional decisions (e.g. tailnet secure:false) were excluded from re-reporting.

Completeness is clean: all issue-307 acceptance criteria, SPEC section 7 scope items, and SPEC section 9 review-contract items verified against shipping code at HEAD. The perf commits introduced no scope regressions — windowing/truncation/caps are all expandable or preserve running state, and sequenced-later items (tree screen, shell passthrough, subagent steering) remain correctly absent with no dead UI.

Critical

None found.

Important

Finding 1 — Restoring queued messages can destructively clear them before validation succeeds
Severity: high · Confidence: 90
Files: packages/dashboard/src/client/screens/session.tsx (~547–563), packages/dashboard/src/server/server.ts (~294)

restorePendingToComposer() calls the destructive dequeue endpoint (api.dequeue → server clearPendingMessages()) before decoding queued images or checking the 25MB aggregate cap. If imageAttachmentFromQueuedImage() throws on malformed base64, or assertTotalImageBytes() rejects an over-cap batch, the catch block runs before setComposerText()/setImageAttachments() — but the runtime queue is already cleared. The user's queued messages and images are lost.
Fix: preflight with the non-destructive pending snapshot before dequeuing, and only clear the runtime queue after decode/size validation succeeds (or requeue on client-side validation failure).

Suggestions

Finding 2 — 25MB raw image cap exceeds the server's 25MB JSON body limit
Severity: medium · Confidence: 95
Files: packages/dashboard/src/client/screens/session.tsx (~34–35, 799–804), packages/dashboard/src/server/server.ts (~65)

The client accepts up to 25MiB of raw image blobs, but send() base64-encodes them into the JSON prompt body (~4/3 expansion) against express.json({ limit: "25mb" }). Two 10MiB images pass client checks at 20MiB raw but encode to ~26.7MiB plus JSON overhead — rejected before reaching the prompt route. The client's advertised maximum can exceed the server contract.
Fix: lower the aggregate raw cap to an encoded-safe budget (~18MiB), or raise the server body limit; ideally derive both from one shared constant.

Finding 3 — Stale in-flight hydration can undo runtime_removed/resync eviction
Severity: medium · Confidence: 90
File: packages/dashboard/src/client/state/store.ts (~97–100, 198–209, 249–253)

deleteSessionState() deletes the session's revision entry, but currentRevision() maps missing revisions to 0. A hydrateSession()/hydrateSubagent() that started at revision 0 and resolves after a runtime_removed evicts the state still passes the currentRevision(key) === hydrationRevision guard and recreates the evicted session. dashboard_resync (which resets revisions to {}) has the same generation-reset hole.
Fix: a per-session hydration generation/tombstone bumped on runtime_removed and dashboard_resync that is not reset to 0 by state deletion; compare it in the hydration guard.

Finding 4 — runtime_removed leaves an actively-viewed session on a permanent fake "loading transcript…" state
Severity: medium · Confidence: 84
Files: packages/dashboard/src/client/state/store.ts (~118–119), packages/dashboard/src/client/screens/session.tsx (~1157), packages/dashboard/src/client/screens/subagent.tsx (~82–88)

When a runtime is stopped from elsewhere (second tab, another device — only the self-stop path navigates to fleet), the mounted SessionScreen's session() accessor turns undefined and the <Show> fallback renders "loading transcript…" forever — hydrateSession only runs onMount, and the runtime 404s anyway. The fallback copy asserts a transient loading state when the true state is terminal removal — a graceful fallback masking a real state change.
Fix: distinguish "never hydrated" from "removed": navigate to fleet with a toast when runtime_removed arrives for the currently-routed key, or render an explicit "this session was stopped" terminal state.

Finding 5 — A trailing SSE event after runtime_removed silently resurrects the evicted session as an empty stub
Severity: low · Confidence: 80
Files: packages/dashboard/src/client/state/store.ts (~90–121), packages/dashboard/src/server/runtime-pool.ts (~159–161)

RuntimePool.stop() publishes runtime_removed, then awaits client.stop() — any event the child emits during teardown publishes with a higher seq. Client-side, the late event hits mutateSessionensureSession, which recreates a blank state entry for the evicted key. It's invisible (no fleet entry), undismissable, and only clears on resync/reload. The resurrection mechanism is unconditional; only the trigger is a race.
Fix: short-lived tombstone set consulted by ensureSession/mutateSession (pairs naturally with the finding 3 fix).

Finding 6 — Coalesced scroller cancel() is never exercised by tests
Severity: medium · Confidence: 90
File: packages/dashboard/src/client/scrolling.ts + packages/dashboard/test/client/scrolling.test.ts

The test creates a cancelRaf spy but never calls scroller.cancel() nor asserts on the spy. cancel() is wired into onCleanup in three components; a regression that leaks a pending double-rAF firing after unmount (touching a detached element) would pass the suite. Needs: request → cancel → flush asserts no scroll and cancelAnimationFrame called, plus cancel-while-second-frame-pending.

Finding 7 — ToolCard open-state machine (running-lock, running→done collapse, user-toggle persistence) is untested
Severity: medium · Confidence: 82
File: packages/dashboard/src/client/components/transcript.tsx (~428–478)

The lazy-bodies fix introduced a state machine: users cannot collapse a running tool (onToggle forces reopen), running→done collapses to the autoOpen default, and a manual toggle on a done tool must persist across revision bumps (userToggled guard). Tests only cover initial mount states; all three transition rules are unverified and user-visible.

Finding 8 — Aborted-hydration phantom-session guard is not actually asserted
Severity: medium · Confidence: 84
File: packages/dashboard/src/client/state/store.ts (hydrateSession abort guard) + packages/dashboard/test/client/screens.test.tsx

The code comments that without the !signal?.aborted guard, an all-rejected hydration would create a stub session and bump its revision. The abort tests assert only signal.aborted and absence of rendered error — but the component is unmounted, so nothing would render regardless. store.sessions[key]/store.revisions[key] staying undefined after an aborted hydration is never asserted; deleting the guard would regress silently. Needs a store-level test.

Finding 9 — SSE backpressure threshold guard (slow-but-tolerable client stays connected) untested
Severity: medium · Confidence: 80
File: packages/dashboard/src/server/server.ts (guardedWrite) + packages/dashboard/test/server.test.ts

The over-threshold destroy is tested, but not the tolerance side: write() returning false with writableLength under the cap must NOT disconnect. Simplifying the condition to destroy-on-any-backpressure would pass the current suite while killing every momentarily-slow client.

Finding 10 — MAX_COMPLETED_BACKGROUND_AGENTS duplicated across client and server
Severity: medium · Confidence: 85
Files: packages/dashboard/src/client/state/reducer.ts (~294), packages/dashboard/src/server/runtime-pool.ts (~34)

The same cap (20) is defined independently in both; they are semantically one contract and must stay in lockstep. Both files already import from shared/protocol — move the constant there.

Finding 11 — Aggregate-cap rejection + objectURL rollback on the dequeue-restore path untested
Severity: low · Confidence: 80
File: packages/dashboard/src/client/screens/session.tsx (restorePendingToComposer)

The file-input path's cap is tested, but the restore path's failure branch (error surfaced, no attachments committed, every freshly-created objectURL revoked via the imagesCommitted flag) is not. Note this test would also pin down the finding 1 data-loss fix.

Finding 12 — Duplicated suppress-timer arming block in ToolCard
Severity: low · Confidence: 82
File: packages/dashboard/src/client/components/transcript.tsx (~428–478)

The identical four-line "arm suppress flag + reset timer" dance appears verbatim in setProgrammaticOpen and the running-branch of onToggle. Extract an armSuppressToggle() helper; behavior is preserved (synchronous signal writes, toggle event dispatched async).

Finding 13 — Image-file validation duplicated between imageAttachmentFromFile and addImageFiles
Severity: low · Confidence: 82
File: packages/dashboard/src/client/screens/session.tsx (~582–598)

Identical type/size checks with identical message text in two places. Extract assertValidImageFile(file); the validate-all-before-allocating-objectURLs ordering is preserved.

Strengths

  • The produce-based granular sync preserves stable entry identities during streaming — verified by tests asserting array/entry reference equality across deltas (toBe), not just output equality.
  • Throttled markdown always flushes the trailing edge on stream end; tests pin the 149ms/150ms boundary with fake timers.
  • The abort classifier is correctly narrow: errors are swallowed only when both signal.aborted and isAbortError(err) hold; genuine hydration failures surface loudly and are tested.
  • objectURL lifecycle is complete across remove, send, pre-commit failure, and unmount; a failed send deliberately preserves attachments for retry.
  • Truncation and windowing both carry visible affordances ("show full", "show earlier N") — no data hidden silently.
  • SSE backpressure destroy is bounded (4MB writableLength ceiling), logged with byte count, and localized to the HTTP transport; ring-buffer eviction produces a loud dashboard_resync rather than a silent gap.
  • Completeness at HEAD is fully clean: every issue-307 criterion, SPEC section 7 item, and section 9 contract item is backed by shipping code.

Agents run: code-reviewer, error-auditor, test-reviewer, completeness-checker, simplifier


Reviewed by mach6

@m-aebrer

m-aebrer commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

Review Assessment

Review comment: #321 (comment)

Every finding was independently verified against the actual source at HEAD bd5f175. Prior rounds' decisions (skipped style dedups, intentional tailnet cookie) were taken into account.

Classifications

Finding Classification Reasoning
1 — Destructive dequeue before validation genuine (high) Confirmed: restorePendingToComposer() calls api.dequeue (server clearPendingMessages(), destructive) before decoding images / assertTotalImageBytes. An over-cap throw lands after the queue is cleared, composer never populated — total loss. Realistic: queued messages accumulate across sends, so aggregates over 25MB can exist. A non-destructive api.pending snapshot endpoint already exists for a safe preflight.
2 — 25MB raw cap vs 25mb JSON body genuine (medium) Confirmed byte-for-byte: client raw cap 26,214,400 = express limit 26,214,400, but base64 adds ~4/3. Two 10MB images pass the client and produce a ~26.7MB body → 413. Loud failure, so medium not high.
3 — Stale hydration undoes eviction genuine (medium) Confirmed: deleteSessionState clears the revision; currentRevision maps missing → 0, colliding with the first-hydration generation. Screen stays mounted on runtime_removed (route unchanged), so the abort controller never fires — full network window open. dashboard_resync has the same hole.
4 — Permanent "loading transcript…" on remote stop genuine (medium) Confirmed: hydrateSession is onMount-only; only the self-stop path navigates to fleet. A stop from a second tab/device leaves the mounted screen on a forever-"loading" fallback — a graceful fallback masking a terminal state.
5 — Trailing SSE event resurrects stub false positive Two server guards close the window: runtimes.delete(key) happens before await client.stop() so handleRuntimeExit's isLiveHandle guard suppresses the trailing publish, and RpcClient.stop() synchronously detaches stdout before any await. The client resurrection mechanism exists but cannot be triggered. Finding 3's tombstone incidentally hardens it anyway.
6 — Scroller cancel() untested genuine (test gap) Confirmed: the test creates a cancelRaf spy but never calls scroller.cancel() or asserts on it; cancel() is wired into three onCleanup sites.
7 — ToolCard open-state machine untested genuine (test gap) Confirmed: tests cover initial mount states only; the running-lock reopen, running→done collapse, and userToggled persistence rules are all unverified.
8 — Aborted-hydration store guard not asserted genuine (test gap) Confirmed: abort tests assert on a disposed component where nothing renders regardless; sessions[key]/revisions[key] staying undefined is never asserted. Deleting the guard passes the current suite.
9 — Backpressure tolerance untested genuine (test gap) Confirmed: only the over-threshold destroy is tested. write()===false under the byte ceiling staying connected is uncovered — a destroy-on-any-backpressure regression would pass.
10 — Duplicated MAX_COMPLETED_BACKGROUND_AGENTS genuine (cleanup) Confirmed: independent = 20 in reducer.ts and runtime-pool.ts; one semantic client/server contract defined twice, both files already import from shared/protocol.
11 — Restore-path cap/objectURL-rollback untested genuine (test gap) Confirmed: happy-path restore and file-input cap are tested; the restore failure branch (error surfaced, nothing committed, objectURLs revoked) is not. This test also pins the finding 1 fix.
12 — Duplicated suppress-timer arming nitpick Real duplication, behavior-preserving extraction — but pure style, same class the maintainer explicitly skipped in earlier rounds.
13 — Duplicated image validation nitpick Same class; the duplication is partly intentional (validate-all-before-allocating ordering). Optional.

Tally: 10 genuine (1 high, 3 medium correctness, 1 contract, 4 test gaps, 1 cleanup), 2 nitpicks, 1 false positive, 0 deferred.

Action Plan

Priority 1 — data loss

  1. Finding 1: make restorePendingToComposer() preflight with the non-destructive api.pending snapshot — decode + validate first, api.dequeue only after success.
  2. Finding 11: land the restore-failure test alongside (over-cap batch → error surfaced, no attachments committed, objectURLs revoked) — it pins the fix.

Priority 2 — runtime-removal correctness (shared root, fix together)
3. Finding 3: per-session hydration generation/tombstone bumped on deleteSessionState + dashboard_resync, never reset to 0; compare in both hydration guards.
4. Finding 4: on runtime_removed for the currently-routed key, navigate to fleet with a toast or render an explicit "session was stopped" terminal state.

Priority 3 — contract mismatch
5. Finding 2: derive client raw cap and server body limit from one shared constant; prefer lowering the client aggregate to an encoded-safe budget (~18MiB).

Priority 4 — cleanup
6. Finding 10: move MAX_COMPLETED_BACKGROUND_AGENTS to shared/protocol.ts.

Priority 5 — test gaps (new code in this PR ⇒ ship with it, not deferred)
7. Finding 6: scroller cancel() tests (incl. cancel-while-second-frame-pending).
8. Finding 7: ToolCard transition tests (running-lock, running→done collapse, toggle persistence).
9. Finding 8: store-level aborted-hydration assertion (sessions/revisions stay undefined).
10. Finding 9: backpressure tolerance test (write false under ceiling stays connected).

Skip: finding 5 (false positive), findings 12–13 (nitpicks, consistent with prior skips).


Assessment by mach6

…light, shared image byte budget, hydration generation guard, runtime_removed navigation, shared bg-agent cap, test gaps
@m-aebrer

m-aebrer commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

Progress Update — round-3 review findings implemented

Implemented all 10 genuine findings from the round-3 review assessment (#321 (comment)). Skipped items 12–13 (nitpicks, consistent with prior rounds) and item 5 (false positive). Pre-commit suite: 4385 passed, 0 failed; full workspace vitest + node:test green; build + biome clean.

Fixed

  • Finding 1 — non-destructive restore: restorePendingToComposer now preflights with the read-only pending snapshot — decodes queued images and enforces the aggregate cap (revoking all preflight objectURLs on both paths) before calling the destructive dequeue. After dequeue, text is restored to the composer before image decode, so no failure path can clear the queue and discard its content.
  • Finding 2 — shared image byte budget: new shared constants in shared/protocol.ts: MAX_PROMPT_BODY_BYTES (25MiB) and MAX_TOTAL_IMAGE_BYTES derived as floor(25MiB·3/4) − 768KiB (~18MiB) to account for base64 4/3 expansion plus JSON/prompt overhead. Server express.json limit and client cap + error message all derive from the same constants — the client can no longer advertise a payload the server rejects.
  • Finding 3 — hydration generation guard: non-reactive per-session generation map (bumped on deleteSessionState) plus a global epoch (bumped on dashboard_resync), never reset to 0. Both hydrateSession and hydrateSubagent guards now require revision + generation + epoch to match, closing the 0===0 collision that let stale in-flight hydration recreate evicted sessions.
  • Finding 4 — runtime removal surfaced: runtime_removed for the currently-routed session (or a subagent's parent) now navigates to fleet and pushes a dismissable "session … was stopped" warning notice instead of leaving the screen on a permanent "loading transcript…" fallback. Self-stop dedupes naturally via hash check (it navigates before the event arrives).
  • Finding 10 — shared cap constant: MAX_COMPLETED_BACKGROUND_AGENTS moved to shared/protocol.ts; client reducer and server runtime-pool both import it (re-exported to keep existing import sites stable).

Tests added

  • Finding 6: coalesced scroller cancel() — cancel-pending-first-frame, cancel-while-second-frame-pending, fresh request works after cancel.
  • Finding 7: ToolCard open-state machine — running tools can't be collapsed by the user, running→done collapses to the autoOpen default, a user's manual toggle on a done tool persists across entry updates.
  • Finding 8: store-level aborted-hydration guard — an aborted all-rejected hydration creates no stub session and touches no revisions; genuine rejections still surface; plus three tests pinning the new generation/epoch guard (runtime_removed mid-hydration, resync mid-hydration, stale subagent hydration).
  • Finding 9: SSE backpressure tolerance — write() returning false while under the buffer ceiling stays connected, keeps receiving events, and emits no backpressure log.
  • Finding 11: restore failure path — over-cap queued batch surfaces the error, commits no attachments, revokes every preflight objectURL, and never calls dequeue (pins the finding-1 fix). Happy-path restore test updated for the preflight flow.

Drive-by

tsconfig.client.json had 10 pre-existing type errors (the config isn't wired into build/CI, so they'd gone unnoticed). Fixed per the no-pre-existing-failures rule: Uint8Array<ArrayBuffer> return type for bytesFromBase64, QueuedMessageDto typing on the pending-message fallback mappers, and ModelPickerModal typed against ModelChoice with the picker <Show> callback restructured so narrowing holds.

Commit: 176cb3b


Progress tracked by mach6

…forward

- Delete design/ (SPEC.md, PARITY.md, mockups, mockup screenshots, capture
  script) — the shipped code is now the authority
- Delete tokens byte-equality contract test; clean all SPEC/PARITY/mockup
  references from source comments and docs
- Root README: lead with the dashboard as the visual showcase — synchronized
  desktop/mobile sessions, steering from anywhere, subagent observability,
  Tailscale-gated remote with pairing; screenshot placeholders for real-build
  captures
@m-aebrer

m-aebrer commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

Progress Update — design artifacts removed, README made dashboard-forward

Pre-publish cleanup per maintainer direction.

Removed

  • design/dashboard/ in its entirety — SPEC.md, PARITY.md, tokens.css source, mockup HTML, mockup screenshots, and the mockup capture script. The shipped code is now the authority; the design phase's job is done and mock-based screenshots must not represent the product in master.
  • tokens byte-equality contract test (tokens-contract.test.ts) — it existed only to enforce the design/ source; with the spec gone, src/client/styles/tokens.css is the design system itself.
  • All dangling SPEC/PARITY/mockup references in source comments (auth.ts, server.ts, files.ts, index.ts, runtime-pool.ts, four screens, both stylesheets, screens test header) and docs (docs/dashboard.md, dashboard package README) — reworded to be self-contained.

Root README

Rewritten to lead with the dashboard as dreb's visual showcase: tagline and "Why choose dreb" now open with synchronized desktop/mobile sessions; the dashboard section covers the desk-to-couch workflow (same sessions as the TUI, one synchronized state over SSE), fleet overview, full-parity session view with steering, live subagent observability, host files, and the Tailscale-only remote model with rotating-code pairing. Screenshot placeholders (<!-- screenshot: … -->) mark where real-build captures will go — the maintainer will add screenshots of the current build separately.

A Docker-isolated demo/screenshot harness was prototyped this session and scrapped by maintainer decision (npm-inside-Docker proved too slow to iterate on); screenshots will be added manually later.

Verification

Full workspace suite green (dashboard 272 — one fewer, the deleted contract test), build clean, biome clean.

Commit: 60cfc36


Progress tracked by mach6

@m-aebrer
m-aebrer merged commit 693a773 into master Jul 9, 2026
3 checks passed
@m-aebrer
m-aebrer deleted the feature/issue-307-dashboard-foundation branch July 9, 2026 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Build first-class dreb web dashboard

2 participants