Skip to content

Dashboard: eliminate REST fleet refresh with SSE-pushed snapshots#388

Merged
m-aebrer merged 8 commits into
masterfrom
feature/issue-387-sse-fleet-snapshots
Jul 23, 2026
Merged

Dashboard: eliminate REST fleet refresh with SSE-pushed snapshots#388
m-aebrer merged 8 commits into
masterfrom
feature/issue-387-sse-fleet-snapshots

Conversation

@aebrer

@aebrer aebrer commented Jul 20, 2026

Copy link
Copy Markdown
Owner

Closes #387

Measure the dashboard's mobile fleet-refresh bottleneck, then—if the baseline confirms the diagnosis—replace lifecycle-triggered full fleet reads with debounced SSE snapshots and consolidate session hydration into one request/RPC call.

Implementation plan posted as a comment below.

@aebrer

aebrer commented Jul 20, 2026

Copy link
Copy Markdown
Owner Author

Implementation Plan

Problem analysis

The current turn-boundary path is confirmed: runtime events pass through RuntimePool.handleEvent() and EventHub, then store.applyEnvelope() starts an unawaited GET /api/fleet. Each fleet request performs three sequential RPC calls per live runtime and a full cross-project disk-session scan. Multiple lifecycle events can overlap, so an older REST result can also overwrite newer fleet state (the concern tracked separately as issue 380).

The proposed event mapping needs two corrections from the current types:

  • agent_start has a model but no session ID.
  • message_end has no top-level context-usage field; authoritative context usage remains available through runtime state/session stats.

The implementation should therefore seed each runtime's complete baseline state from the existing initial/create/resync snapshot, update only event-derivable fields in memory, and refresh stats/context usage on the slower stats cadence. Initial and recovery fleet reads remain authoritative but become exceptional rather than turn-boundary traffic.

The issue explicitly requires evidence before committing to the architecture. Implementation is therefore split by an empirical checkpoint: land privacy-safe measurement support first, post aggregate baseline results on the PR, and proceed with the SSE replacement only if the measurements confirm that fleet REST work materially contributes to the weak-mobile path.

Deliverables

Stage 1 — Measurement and go/no-go checkpoint

  1. Add structured, payload-free profiling for GET /api/fleet: elapsed server time, encoded response bytes, runtime count, and disk-session count. Do not log cwd, session names, prompts, or response contents.
  2. Reuse the existing per-connection SSE write diagnostics (type, frameBytes, live/replay/resync kind) rather than adding a second EventHub accounting mechanism.
  3. Add an opt-in mobile profiling harness that:
    • captures one or more fleet response timings and transfer sizes;
    • captures a 60-second SSE trace summarized by event type, bytes, bursts, and replay/live classification;
    • can run against a realistic workload of at least five runtimes; and
    • emits aggregate JSON suitable for a PR comment without retaining event payloads.
  4. Add a documented Chromium/Playwright 3G profile (100 ms RTT, 1.5 Mbps, with loss when supported) for fleet load, a lifecycle update, and session drill-in.
  5. Post sanitized baseline measurements to the PR. If REST fleet refresh is not a material contributor, stop before stage 2 and revise the architecture/acceptance criteria on GitHub.

Stage 2 — SSE-pushed fleet state

  1. Define a lightweight fleet-runtime snapshot protocol that excludes stats and the fetched assistant preview while retaining stable identity/order fields, runtime state, attention/error state, and background agents.
  2. Extend RuntimePool to maintain the fleet-card state it owns:
    • preserve stable/non-event fields from the last authoritative state;
    • update streaming, compaction, tasks, session name, model, message count, attention/error, background-agent state, last activity, and assistant-message-derived preview inputs from events where available;
    • build lightweight snapshots directly from handles without child RPC calls; and
    • emit one global, 200 ms debounced snapshot for rapid changes, excluding removed runtimes and clearing the timer during stopAll().
  3. Publish synthesized snapshots through the existing EventHub with normal sequencing/replay behavior and a global key. Keep EventHub transport-agnostic.
  4. In the client store, merge snapshot runtimes into the fleet while preserving:
    • diskSessions from the latest authoritative/disk refresh;
    • polled stats/context values; and
    • the initial/resync assistant preview until client transcript entries provide a newer preview.
  5. Remove lifecycle-triggered refreshFleet() calls. A lifecycle envelope must never initiate GET /api/fleet; queued/replayed snapshots must continue to obey the existing resync barrier ordering.
  6. Add a lightweight GET /api/sessions path and refresh only disk inventory after create, resume, stop, and delete actions. Other user actions should use their direct response or SSE event rather than a full fleet read. Guard any remaining full-fleet request so a stale response cannot overwrite a newer snapshot.
  7. Poll the existing per-runtime stats endpoint for live cards only while the fleet screen is mounted, at most once every 30 seconds and without overlapping poll rounds. Merge cost/token totals, authoritative message count, and context usage without clearing the last good values on a failed poll; surface failures explicitly.
  8. Derive the fleet activity preview from the latest assistant text block in the client session entries, with the initial/resync preview as a reload fallback. This removes repeated getLastAssistantText() calls without blanking unseen-runtime cards.
  9. Preserve deterministic card ordering by project path and stable creation time; never sort by mutable activity.

Stage 3 — Single-call session hydration

  1. Add GET /api/runtimes/:key/hydrate, backed by the existing getDashboardSnapshot() RPC and snapshot barrier helper, returning state, messages, background agents, and its ordering cursor from one child IPC call.
  2. Replace the client's three-request Promise.allSettled hydration with the single hydrate call while retaining abort, runtime-removal, resync-epoch, task-revision, and live-SSE race protections.
  3. Keep the existing narrow runtime/messages/background-agent routes for compatibility, but do not use them for normal session drill-in.

Stage 4 — Documentation and validation

  1. Document the SSE fleet snapshot, exceptional authoritative reads, disk-inventory refresh, 30-second stats cadence, hydration request, replay behavior, and mobile profiling procedure.
  2. Record the measured before/after aggregates on the PR and verify the weak-mobile thresholds against the same workload/profile used for the baseline.

Acceptance criteria

  • Sanitized baseline and after-change measurements are posted to the PR, including fleet response server time/bytes and 60-second SSE bytes by event type.
  • Under the documented throttled-3G profile, a fleet card reflects a lifecycle change within 2 seconds.
  • After initial page load, lifecycle events (agent_start, agent_end, background-agent lifecycle, and runtime removal) produce no browser GET /api/fleet request.
  • Rapid relevant events coalesce into one fleet snapshot after the 200 ms debounce window.
  • Building and publishing a fleet snapshot performs zero child RPC calls and no disk-session scan.
  • Initial load and /api/resync remain authoritative; replayed snapshots cannot regress post-barrier state or re-add a removed runtime.
  • User actions keep disk sessions accurate through the lightweight endpoint, and no stale full-fleet response can overwrite newer state.
  • Fleet stats poll no more frequently than every 30 seconds while mounted, is single-flight, stops on unmount, and preserves last-good card values on error.
  • Fleet cards retain all current visible information: background agent names/types, status and attention, tasks, model, context percentage, cost, message count, assistant activity preview, and stable ordering.
  • Session drill-in makes one HTTP request and one getDashboardSnapshot() IPC call; abort and concurrent-SSE races do not create stale or phantom state.
  • All focused dashboard tests, browser tests, the full workspace suite, build, type-check, lint, and workspace-link verification pass.

Files to create or modify

Measurement and harness

  • packages/dashboard/src/server/server.ts — payload-free fleet request metrics; shared authoritative fleet/session/hydrate helpers and new routes.
  • packages/dashboard/scripts/profile-mobile.mjs (new) — aggregate fleet/SSE profiler with no payload retention.
  • packages/dashboard/package.json — opt-in profiling command.
  • packages/dashboard/test/client/fleet-mobile.browser.test.ts (new) — throttled browser acceptance and request-count coverage.

Server and protocol

  • packages/dashboard/src/shared/protocol.ts — lightweight fleet snapshot and hydrate response DTOs.
  • packages/dashboard/src/server/runtime-pool.ts — event-derived runtime state, zero-RPC snapshot builder, debounce lifecycle, and snapshot-barrier reuse.
  • packages/dashboard/src/server/event-hub.ts — no domain logic expected; change only if typed global-event plumbing requires it.

Client

  • packages/dashboard/src/client/api.ts — sessions and hydrate request wrappers.
  • packages/dashboard/src/client/state/store.ts — snapshot merge, removal of lifecycle REST refreshes, disk-only refresh, remaining-request ordering guard, stats merge, and single-call hydration.
  • packages/dashboard/src/client/screens/fleet.tsx — mounted stats polling and client-side assistant preview selection.
  • packages/dashboard/src/client/screens/session.tsx — remove full-fleet refreshes that are replaced by direct/SSE updates.
  • packages/dashboard/src/client/screens/files.tsx — use disk-only refresh after creating a session.

Tests

  • packages/dashboard/test/runtime-pool.test.ts — tracked fields, baseline preservation, zero-RPC snapshot creation, debounce/coalescing, removal ordering, and timer cleanup.
  • packages/dashboard/test/event-hub.test.ts — global fleet snapshot sequencing/replay/byte-budget behavior.
  • packages/dashboard/test/server.test.ts — metrics privacy/shape, sessions endpoint, global snapshot publication, no-RPC snapshots, and hydrate one-call/error behavior.
  • packages/dashboard/test/client/store.test.ts — snapshot merging, no lifecycle fleet fetch, initial/resync/replay races, disk refreshes, stats merge/errors, stale-request protection, and one-call hydration guards.
  • packages/dashboard/test/client/screens.test.tsx — card field parity, assistant preview derivation, stats poll cadence/single-flight/unmount behavior, and user-action refresh behavior.
  • packages/dashboard/test/client/hard-refresh.integration.test.ts — real HTTP one-request/one-RPC hydration and startup-SSE race coverage; update all RPC mocks for the snapshot interface.

Documentation

  • README.md — describe the mobile fleet as SSE-updated rather than lifecycle-refetched.
  • packages/dashboard/README.md — architecture and mobile behavior.
  • packages/coding-agent/docs/dashboard.md — detailed fleet transport, recovery, stats, hydration, and profiling contract.

Testing approach

  • Use fake timers around the 200 ms snapshot debounce and 30-second stats cadence; verify one emission/poll round, unref/cleanup behavior, and no overlap.
  • Spy on all fake RPC methods to prove snapshots make no calls and hydration calls only getDashboardSnapshot() once. Update every dashboard RpcClient mock when the interface usage changes.
  • Exercise happy paths plus RPC failure, stats failure, missing runtime, aborted hydration, runtime removal during hydration, resync during hydration, replayed stale snapshots, queue overflow, and snapshot oversize fallback.
  • In the browser acceptance test, capture requests and SSE application timestamps under CDP network emulation. Assert one initial fleet request, no lifecycle fleet request, one hydrate request, and the 2-second card-update threshold.
  • Keep machine-dependent profiling thresholds out of ordinary unit tests; the opt-in harness records empirical numbers, while CI asserts deterministic request counts, byte accounting, ordering, and functional latency under controlled emulation.
  • Validation order after implementation: format/lint changed files, npm run build, focused dashboard tests including browser coverage, npx tsgo --noEmit, npm test/bash test.sh, and npm run verify-workspace-links. Build before any test against the real compiled dashboard binary.

Risks and open questions

  • Measurement gate: the proposed architecture remains conditional until the baseline confirms the bottleneck. A contrary result requires a revised plan rather than forcing the SSE design through.
  • Incomplete event fields: session ID/session file require an authoritative baseline; context usage requires stats/state; message counts need reconciliation with authoritative stats. Snapshot merging must not replace these with fabricated defaults.
  • Replay pressure: whole-fleet snapshots consume replay bytes. The baseline and after trace must confirm that 200 ms coalescing does not materially evict transcript history; otherwise snapshots need lower frequency or a different recovery policy.
  • Creation/removal races: a delayed snapshot must not resurrect a stopped runtime, and a newly created runtime must have a usable baseline before its first lightweight snapshot.
  • Direct user changes: model changes may not emit an immediate dedicated event. Their API responses must update the client or trigger a narrow refresh without bringing back full-fleet churn.
  • Related issue 380: this work removes the primary source of overlapping fleet requests, but acceptance still requires explicit stale-response protection for any full-fleet read that remains.
  • Stats fan-out: a 30-second per-runtime poll is intentionally much cheaper than turn-boundary full refreshes but is still O(N). Measurements should confirm it remains acceptable at the tested fleet size.
  • Public compatibility: the old narrow hydrate component routes should remain available unless a separate compatibility decision is made.

Plan created by mach6

@github-actions

github-actions Bot commented Jul 20, 2026

Copy link
Copy Markdown

Vitest coverage

Metric Covered Total Coverage
Statements 25009 42119 59.37%
Branches 13383 25733 52%
Functions 4727 7643 61.84%
Lines 21442 36437 58.84%

View full coverage run

@aebrer

aebrer commented Jul 20, 2026

Copy link
Copy Markdown
Owner Author

Baseline measurement checkpoint

The measurement gate confirms the fleet REST path is the dominant weak-mobile bottleneck, so implementation can proceed to the SSE snapshot stages.

Sanitized local workload:

  • 5 real dashboard RPC child runtimes
  • 687 on-disk sessions returned by the current inventory scan
  • 3 fleet samples
  • 60-second SSE capture with 55 synthetic session-name lifecycle updates across the runtimes

Results:

Metric Result
Fleet server processing 882–932 ms (mean 904 ms from server diagnostics)
Fleet client round trip 907–968 ms (mean 933 ms on loopback)
Fleet JSON body 15,644,934 bytes per request
SSE wire bytes over 60 seconds 6,087 bytes
SSE application-event bytes 5,947 bytes across 55 events
SSE bursts 12, mean 507 bytes

At 1.5 Mbps, one uncompressed fleet response has a transfer-time floor of roughly 83 seconds before RTT or contention. The current Express response had no content encoding, and the server/client byte measurements matched exactly. Even allowing for a compressing proxy, the roughly 15.6 MB repeated inventory response and roughly 0.9 seconds of local server work dominate the measured SSE traffic by several orders of magnitude.

The trace did not exercise model text deltas, so it is not a complete upper bound on active-stream SSE bandwidth. That does not change the go decision: removing lifecycle-triggered fleet reads is strongly supported by the measured payload and processing cost.

@aebrer

aebrer commented Jul 21, 2026

Copy link
Copy Markdown
Owner Author

After-change measurement

Repeated the sanitized five-runtime, 687-disk-session, 60-second local profile after building the implementation. The workload produced 55 session-name events in 12 bursts, causing 12 coalesced fleet snapshots.

Metric Baseline After
Authoritative fleet body 15,644,934 bytes 15,675,327 bytes
SSE wire bytes over 60 seconds 6,087 bytes 142,757 bytes
Coalesced fleet snapshots none 12 / 136,827 bytes total
Mean fleet snapshot frame n/a 11,402 bytes
Browser lifecycle fleet reads after initial load present in old store path 0
Session hydration 3 HTTP / 3 IPC 1 HTTP / 1 IPC

The authoritative fleet endpoint remains intentionally expensive because it is still used for initial load and exceptional recovery. The improvement is removing it from the lifecycle path: one five-runtime snapshot is roughly 1,370 times smaller than the measured full fleet body, and the entire minute of after-change SSE traffic was roughly 110 times smaller than one fleet response.

The real Chromium acceptance test passed with 100 ms latency and 1.5 Mbps throughput: exactly one initial GET /api/fleet, no additional fleet read after agent_start, card transition within the 2-second threshold, and exactly one hydrate request / snapshot RPC on drill-in. HTTP packet loss was not claimed because Chromium's exposed packet-loss control applies to WebRTC rather than this HTTP/SSE path.

@aebrer

aebrer commented Jul 21, 2026

Copy link
Copy Markdown
Owner Author

Progress Update

Full implementation pushed — all four stages:

  • Stage 1 — measurement: structured payload-free fleet diagnostics, opt-in profile:mobile profiler, and sanitized baseline confirming REST fleet reads dominate the weak-mobile path (15.6 MB response at ~900 ms server time vs 6 KB SSE over 60 seconds).

  • Stage 2 — fleet snapshots: RuntimePool tracks event-derived runtime state, builds zero-RPC FleetRuntimeSnapshotDto snapshots, debounces at 200 ms (trailing, skipping ineligible streaming deltas), publishes through EventHub with global key. Client merges snapshots preserving enriched card fields, stats, and context usage; removes lifecycle-triggered refreshFleet(). Disk inventory refreshes narrowly via GET /api/sessions; fleet screen polls refreshFleetStats() at 30-second cadence, single-flight, preserving last-good values; client-side assistant preview from transcript entries with initial/resync fallback.

  • Stage 3 — single-call hydration: GET /api/runtimes/:key/hydrate backed by one getDashboardSnapshot() RPC + barrier, replacing the old three-request hydration. Retains abort, removal, resync, and live-SSE race guards; uses barrier-ordered replay for live envelopes arriving during the hydrate flight.

  • Stage 4 — docs and validation: updated root README, dashboard README, and dashboard.md with fleet transport, hydration, profiling, and mobile acceptance details. After-measurements posted — 12 coalesced snapshots totaling 137 KB SSE over the same 60-second workload (vs 15.6 MB per fleet call).

Review findings from code-reviewer, error-auditor, test-reviewer, and completeness-checker were addressed: monotonic message count now allows fork/rewind decreases, hydration uses barrier-ordered replay, disk inventory converges across tabs on membership changes and disk_sessions_changed events, and stop() cleans up pending hydration transactions.

Commit: 94a97b1


Progress tracked by mach6

@aebrer
aebrer marked this pull request as ready for review July 21, 2026 01:20
@aebrer

aebrer commented Jul 21, 2026

Copy link
Copy Markdown
Owner Author

Code Review

Critical

No critical findings.

Important

Finding 1 — Hydration queue overflow path is untested

  • File: packages/dashboard/src/client/state/store.ts
  • Severity: high
  • Confidence: 95
  • Type: missing-test
  • Detail: When a hydration is in-flight and the SSE stream sends enough envelopes to exceed MAX_PENDING_HYDRATION_ENVELOPES (2,000) or MAX_PENDING_HYDRATION_BYTES (3 MB), the code clears the hydration transaction and throws. No test exercises this threshold or verifies that the thrown error propagates to the SSE reconnect/resync path.

Finding 2 — Superseding hydration for the same key is untested

  • File: packages/dashboard/src/client/state/store.ts
  • Severity: high
  • Confidence: 90
  • Type: missing-test
  • Detail: hydrateSession calls clearHydrationTransaction(key) at the top, replacing any prior pending hydration. When the first hydration resolves, pendingHydrations.get(key) !== pending causes it to return early. No test verifies that the first snapshot is silently discarded and the second is applied atomically, or that queued envelopes from the first are not replayed by the second.

Finding 3 — Stale hydrate overwrites newer fleet snapshot state

  • File: packages/dashboard/src/client/state/store.ts
  • Severity: medium
  • Confidence: 90
  • Type: runtime-risk
  • Detail: setHydratedRuntimeState replaces the fleet card’s runtime state unconditionally, without checking whether a newer fleet_snapshot SSE has already mutated the fleet. During hydration, a fleet_snapshot (key="") is not queued in pendingHydrations, so it is applied immediately. When the hydrate snapshot arrives later, setHydratedRuntimeState rewinds card fields to the older barrier values.

Finding 4 — fleet_snapshot events during hydration are not queued and can be lost to stale hydrate overwrite

  • File: packages/dashboard/src/client/state/store.ts
  • Severity: medium
  • Confidence: 85
  • Type: runtime-risk
  • Detail: handleEnvelope only queues envelopes in pendingHydrations when envelope.key is truthy. Global fleet_snapshot events have key="", so they bypass the hydration queue and are applied immediately. If a hydration for the same runtime is in flight, the fleet snapshot updates the card state, but the hydrate’s later setHydratedRuntimeState overwrites it with older snapshot data. Because the fleet snapshot was never queued, it is not replayed after the hydrate baseline.

Finding 5 — agent_end with errorMessage records error and triggers fleet snapshot — untested

  • File: packages/dashboard/src/server/runtime-pool.ts
  • Severity: medium
  • Confidence: 90
  • Type: missing-test
  • Detail: When an agent_end event carries an errorMessage, the pool records a runtime error and schedules a fleet snapshot emission. Existing tests exercise agent_start, agent_end (without error), auto_compaction_start/end, tasks_update, session_name_changed, and message_start, but never an agent_end with errorMessage.

Finding 6 — describe() fallback merge for missing authoritative fields is untested

  • File: packages/dashboard/src/server/runtime-pool.ts
  • Severity: medium
  • Confidence: 85
  • Type: missing-test
  • Detail: The describe() method merges fallbackState(handle) with the authoritative getState() result, preserving session file path, session ID, and tasks when the RPC response omits them. No test validates this fallback behavior.

Finding 7 — Browser test has a CI-flaky wall-clock timing assertion

  • File: packages/dashboard/test/client/fleet-mobile.browser.test.ts
  • Severity: medium
  • Confidence: 85
  • Type: flaky-pattern
  • Detail: The test asserts expect(Date.now() - lifecycleStartedAt).toBeLessThanOrEqual(2_000). Under CI load, the full round-trip (SSE frame → event hub → debounce timer → SSE write → browser EventSource → Solid re-render → DOM mutation) can exceed 2 seconds. The waitFor timeout is the same value as the assertion ceiling, so the test self-defeats.

Suggestions

Finding 8 — Unhandled promise rejection from the stats interval

  • File: packages/dashboard/src/client/screens/fleet.tsx
  • Severity: low
  • Confidence: 85
  • Type: runtime-risk
  • Detail: setInterval(() => void props.store.refreshFleetStats(), 30_000) discards the returned promise. refreshFleetStats() normally catches errors internally, but if the .then callback throws synchronously, the returned promise rejects and becomes an unhandled promise rejection. Suggested fix: props.store.refreshFleetStats().catch(() => {}).

Strengths

  1. Race-condition discipline. Every concurrent path has a guard: generation counters for stale REST fleet/disk responses, hydrationEpoch + per-key hydrationGeneration + revision for stale hydrations, removedRuntimeKeys preventing resurrection, and liveStateRaced in mergeRuntimeStats.
  2. Barrier ordering correctness. Server captures hub.currentSequence synchronously at the RPC snapshot marker before the async snapshot returns. Client replays only seq > barrierSeq. Subagent drill-in has a second boundary so relays between disk read and parent snapshot are preserved.
  3. Bounded resource usage. Resync queue: 2,000 envelopes / 3 MiB. Hydration queue: same caps. SSE write buffer: 4 MiB backpressure disconnect.
  4. Deterministic UI ordering. Fleet cards sort by cwd then createdAt — stable, not by mutable lastActivity.
  5. Comprehensive test coverage. The browser acceptance test runs under throttled 3G and asserts exact HTTP call inventory.

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


Reviewed by mach6

@aebrer

aebrer commented Jul 21, 2026

Copy link
Copy Markdown
Owner Author

Review Assessment

Review comment: #388 (comment)

Classifications

Finding Classification Reasoning
1 — Stale hydrate overwrites newer fleet snapshot state genuine setHydratedRuntimeState replaces card state unconditionally. A mid-hydration fleet_snapshot applies immediately, then gets rewound by the older hydrate baseline. Card can be stuck in wrong state until next event. Fix: capture fleetMutationGeneration at hydrate start and skip if changed (mirrors existing refreshFleet guard).
2 — Unhandled rejection from stats interval nitpick refreshFleetStats uses Promise.allSettled (never rejects) and .then only does Solid setters. Synchronous throw is pathological-only.
3 — fleet_snapshot not queued during hydration genuine Same root defect as finding 1: handleEnvelope queues only when envelope.key is truthy, so global snapshots bypass hydration queue, apply immediately, then get rewound. Treat as one fix with finding 1.
4 — Hydration queue overflow untested genuine PendingHydration queue/overflow throw is brand-new code in this PR. The parallel resync overflow is tested, but the hydration variant has zero coverage.
5 — Superseding hydration untested genuine clearHydrationTransaction(key) + pendingHydrations.get(key) !== pending discard is new machinery this PR. No test verifies first snapshot/queued envelopes are discarded and not cross-replayed.
6 — agent_end with errorMessage untested genuine New line in this PR's diff. Cheap to test and covers a real path. Low severity.
7 — describe() fallback merge untested nitpick Partially covered: default mock omits sessionFile, so the ?? fallback branch is exercised. Only sessionId/tasks ?? are unexercised; merge is simple spread.
8 — Browser test 2s timing assertion nitpick waitFor({timeout: 2_000}) already enforces the same ceiling. The Date.now() assertion is redundant and adds a narrow flake window.
9 — Extract patchRuntimeState helper nitpick Pure style — three 6-line functions with distinct payloads.
10 — Unreachable try/catch in refreshFleetStats nitpick Correct observation: api.stats is async and never throws synchronously. Harmless dead code.
11 — Repeated error-to-string pattern nitpick Accurate, but extracting a one-line helper is stylistic.
12 — Unnecessary optional chaining in diskGroups sort nitpick Confirmed unneeded, but harmless.
User observation — slow mobile PWA reconnection deferred Pre-existing connectEvents retry/backoff/validation behavior, essentially untouched by this PR. Plausibly explains the observation, but out of scope here. No tracking issue exists — one should be created.

Action Plan

  1. Fix the hydrate vs. fleet_snapshot race (findings 1 + 3) — In hydrateSession, capture fleetMutationGeneration when the transaction starts and skip setHydratedRuntimeState if the fleet was mutated while the request was in flight. Add a regression test: fleet_snapshot arrives mid-hydrate with newer state → card keeps the newer state after hydrate resolves.
  2. Add superseding-hydration test (finding 5) — Two concurrent hydrateSession calls for the same key: first resolves last and must be discarded; second applies; queued envelopes from the first are not cross-replayed.
  3. Add hydration queue overflow test (finding 4) — Exercise MAX_PENDING_HYDRATION_ENVELOPES/bytes overflow: transaction cleared, error thrown, recovery triggered, no partial replay applied.
  4. Add agent_end with errorMessage test (finding 6) — Direct emit({ type: "agent_end", errorMessage })handle.error and attention recorded, fleet snapshot scheduled.

Deferred: Mobile PWA SSE reconnection latency — pre-existing connectEvents backoff/validation behavior. Recommend filing a separate issue covering: backoff reset on foreground, and whether the visibility-change path can skip the full auth-validate + resync when the stream is still healthy.

Everything else (findings 2, 7–12) requires no action before merge.


Assessment by mach6

…g tests

- capture fleetMutationGeneration at hydrate start and skip
  setHydratedRuntimeState if fleet mutated mid-flight (findings 1+3)
- add hydration queue overflow tests (count and bytes) (finding 4)
- add superseding-hydration test (finding 5)
- add fleet_snapshot race regression test (finding 1+3)
- add agent_end with errorMessage test (finding 6)
- pre-commit hook now runs dashboard tests only (fast, no live APIs)
@aebrer

aebrer commented Jul 21, 2026

Copy link
Copy Markdown
Owner Author

Review Fixes Applied

Fixed all 5 genuine findings from the review:

  1. Findings 1+3 — Stale hydrate vs fleet_snapshot race: hydrateSession now captures fleetMutationGeneration at transaction start and setHydratedRuntimeState skips applying older snapshot state if the fleet was mutated mid-flight. Added regression test verifying a mid-hydration fleet_snapshot wins over the older hydrate baseline.

  2. Finding 4 — Hydration queue overflow untested: Added count-overflow (2,000 envelopes) and byte-overflow (3 MB) tests verifying the throw-and-clear behavior and that the incomplete snapshot is discarded.

  3. Finding 5 — Superseding hydration untested: Added test verifying a superseded hydration is silently discarded (its snapshot messages are not applied) while the latest hydration applies normally.

  4. Finding 6 — agent_end with errorMessage untested: Added test verifying error recording, attention flag setting, and fleet snapshot scheduling.

Verification: All 708 dashboard tests pass, biome clean, tsgo --noEmit clean, full workspace build passes.

Also: The pre-commit hook now runs npx vitest --run packages/dashboard instead of the full bash test.sh, avoiding live-API tests in the hook. Manual full tests still run via bash test.sh.

Commit: 2f2da68

Every describe.skipIf/it.skipIf block gated on provider credentials now
also checks process.env.DREB_SKIP_LIVE_API === "1" so live-API tests can
be skipped en masse without unseting env vars or maintaining a key list.

test.sh accepts --no-live-api which exports DREB_SKIP_LIVE_API=1.
.husky/pre-commit passes --no-live-api so the hook runs the full suite
minus live-API tests. Manual full runs (bash test.sh) still hit APIs.

340 skipIf conditions across 29 files guarded. Commented-out code left
untouched.
@aebrer

aebrer commented Jul 21, 2026

Copy link
Copy Markdown
Owner Author

24h testing on mobile network, and I can honestly say this fix has already made a huge impact. Stability and speed are both much improved, usability is way up in a lower data environment. Solid 5G signal now behaves about as well as localhost.

@m-aebrer

Copy link
Copy Markdown
Collaborator

Code Review

Critical

No critical findings.

Important

Finding 1 — --no-live-api still runs opt-in Bedrock API suites

  • Files: test.sh, packages/agent/test/bedrock-models.test.ts, packages/ai/test/bedrock-models.test.ts
  • Severity: high
  • Confidence: 100
  • Type: runtime-risk / missing-test
  • Detail: The new pre-commit path claims bash test.sh --no-live-api skips live API tests, but both Bedrock suites dynamically register real calls from hasBedrockCredentials() && BEDROCK_EXTENSIVE_MODEL_TEST without checking DREB_SKIP_LIVE_API. With AWS credentials and the opt-in flag set, the no-live run can execute hundreds of external tests. Add the skip guard to both registration predicates and cover the no-live contract deterministically.

Suggestions

Finding 2 — --no-live-api can still resolve and refresh OAuth tokens

  • File: packages/ai/test/stream.test.ts and shared test credential helpers
  • Severity: medium
  • Confidence: 90
  • Type: runtime-risk
  • Detail: Several modules call resolveApiKey() at top level before Vitest evaluates the new skipIf guards. That helper can refresh expired OAuth credentials and write ~/.dreb/agent/auth.json, so a supposedly no-live pre-commit can still contact auth endpoints and mutate credentials. Short-circuit credential resolution when DREB_SKIP_LIVE_API === "1".

Finding 3 — One stalled stats request freezes every future fleet stats poll

  • Files: packages/dashboard/src/client/state/store.ts, packages/dashboard/src/client/api.ts, packages/dashboard/src/client/screens/fleet.tsx
  • Severity: medium
  • Confidence: 85
  • Type: runtime-risk
  • Detail: refreshFleetStats() uses one Promise.allSettled round as a store-scoped single-flight guard, but individual fetch calls have no timeout or abort signal. If one request hangs on a weak mobile connection, the round never settles, later 30-second ticks issue no requests, and no error is surfaced. Bound each stats request so the round settles, clears the guard, and exposes the failure.

Finding 4 — Unrelated fleet mutations suppress hydrated state for the target runtime

  • File: packages/dashboard/src/client/state/store.ts
  • Severity: medium
  • Confidence: 94
  • Type: runtime-risk / missing-test
  • Detail: The hydrate race fix compares one global fleetMutationGeneration. A disk refresh, stats merge, or snapshot changing only runtime B while runtime A hydrates causes A's hydrated card state to be discarded, even though A was not superseded. Add a two-runtime race test and use target-card-specific freshness if A should still reconcile.

Finding 5 — Files-screen session creation lacks behavior-level regression coverage

  • Files: packages/dashboard/src/client/screens/files.tsx, packages/dashboard/test/client/screens.test.tsx
  • Severity: medium
  • Confidence: 98
  • Type: missing-test
  • Detail: newSessionHere() changed from a full fleet refresh to upsertRuntime(), refreshDiskSessions(), and navigation, but its existing test only checks that the control renders. Add a click-path test verifying the selected cwd, returned runtime upsert, disk refresh, navigation, and absence of api.fleet().

Finding 6 — Read-only hydration advances the fleet card's activity timestamp

  • File: packages/dashboard/src/server/runtime-pool.ts
  • Severity: low
  • Confidence: 85
  • Type: correctness
  • Detail: getDashboardSnapshot() emits an internal dashboard_snapshot_barrier, while handleEvent() updates lastActivity before inspecting event type. Opening a session can therefore make an idle runtime appear active “now.” Deliver the marker for barrier ordering without treating it as user/agent activity.

Finding 7 — Filter surviving fleet snapshots once

  • File: packages/dashboard/src/client/state/store.ts
  • Severity: low
  • Confidence: 92
  • Type: simplification
  • Detail: applyFleetSnapshot() filters removedRuntimeKeys twice in one synchronous call. Compute a surviving array once and reuse it for membership and mapping.

Finding 8 — Remove unreachable synchronous error wrapper around api.stats()

  • File: packages/dashboard/src/client/state/store.ts
  • Severity: low
  • Confidence: 95
  • Type: simplification
  • Detail: api.stats() calls an async request helper and always returns a promise, so the surrounding synchronous try/catch cannot run. Map directly to api.stats(key) and let Promise.allSettled handle rejections.

Finding 9 — Consolidate duplicate runtime-state patch functions

  • File: packages/dashboard/src/client/state/store.ts
  • Severity: low
  • Confidence: 82
  • Type: simplification
  • Detail: setRuntimeModel() and setRuntimeThinkingLevel() have identical map-and-spread structure. A small typed patchRuntimeState(key, patch) helper removes the duplication without changing behavior.

Strengths

  • The core architecture satisfies the linked issue: snapshots are debounced, replayable, and built without child RPC or disk scans; lifecycle fleet reads are removed.
  • Hydration has strong barrier, generation, abort, removal, overflow, and supersession protections, including the previously requested race tests.
  • Disk inventory, stats, previews, deterministic ordering, profiling, browser acceptance, and documentation are all implemented comprehensively.
  • Server route errors, stale full-fleet responses, queue bounds, timer cleanup, and last-good stats retention are handled carefully.

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


Reviewed by mach6

@m-aebrer

Copy link
Copy Markdown
Collaborator

Review Assessment

Review comment: #388 (comment)

Classifications

Finding Classification Reasoning
Finding 1 — Bedrock suites bypass --no-live-api genuine test.sh only exports DREB_SKIP_LIVE_API, while both extensive Bedrock registration predicates ignore it. With AWS credentials and the opt-in flag, they still register real API tests.
Finding 2 — OAuth resolution still has live side effects genuine stream.test.ts resolves OAuth credentials at module evaluation, before skipIf; the resolver can refresh externally and rewrite auth.json.
Finding 3 — Stalled stats request freezes polling genuine Stats use an unbounded fetch, and the shared single-flight promise is retained until every request settles. A hung request blocks later rounds and prevents error reporting.
Finding 4 — Global mutation generation suppresses unrelated hydrate state genuine Every fleet mutation, including disk-only changes, increments the global generation checked by hydration. Runtime A can therefore discard authoritative hydrated state because runtime B or disk inventory changed.
Finding 5 — Files-screen creation coverage genuine The changed action path is not exercised: the existing Files test only renders the control and never verifies create, upsert, disk refresh, navigation, or absence of a fleet read.
Finding 6 — Snapshot barrier updates lastActivity genuine The RPC marker reaches RuntimePool.handleEvent(), which updates activity before checking type; server interception happens afterward. Read-only session drill-in can therefore make an idle runtime look active.
Finding 7 — Repeated snapshot filter nitpick The filter is redundant but local, synchronous, and inexpensive; this is optional cleanup rather than a merge blocker.
Finding 8 — Unreachable synchronous stats catch genuine api.stats() always returns a promise, so the synchronous catch cannot handle fetch rejection. Promise.allSettled already handles the real asynchronous error path; the block is dead code.
Finding 9 — Generic runtime-state patch helper nitpick The functions are similar but small, live, and type-specific. A generic helper is an optional abstraction.

Action Plan

  1. Enforce DREB_SKIP_LIVE_API in both Bedrock suite registration predicates and add deterministic no-live coverage.
  2. Short-circuit shared test credential resolution during no-live runs so OAuth refresh and credential writes cannot occur; test those side effects stay absent.
  3. Add a bounded timeout/abort path for each fleet stats request, surface timeout errors, allow the next polling round, and add a stalled-request regression test.
  4. Replace the over-broad hydration freshness check with target-runtime-specific invalidation while preserving same-runtime snapshot precedence; test unrelated runtime and disk-only races.
  5. Add a Files-screen “new session here” behavior test covering cwd, upsert, disk refresh, navigation, and zero fleet requests.
  6. Keep dashboard_snapshot_barrier ordering-neutral for runtime activity and test that hydration does not change lastActivity.
  7. Remove the unreachable synchronous wrapper around api.stats() and rely on Promise.allSettled for rejected promises.

Counts: 7 genuine, 2 nitpicks, 0 false positives, 0 deferred.


Assessment by mach6

@m-aebrer

Copy link
Copy Markdown
Collaborator

Remaining Review Findings Fixed

Addressed all seven genuine findings from the latest assessment:

  • Both extensive Bedrock suites now honor DREB_SKIP_LIVE_API, with deterministic guard tests.
  • Shared AI and coding-agent test credential resolvers now short-circuit before reading, refreshing, or writing OAuth credentials during no-live runs.
  • Fleet stats requests have a per-runtime 10-second timeout and abort path, preserving last-good values while allowing later poll rounds to recover.
  • Hydration freshness is tracked per runtime, so unrelated runtime or disk-inventory changes no longer suppress authoritative hydrate state while same-runtime updates still win.
  • Files-screen session creation now has behavior coverage for cwd selection, runtime upsert, disk refresh, navigation, and zero fleet reads.
  • Internal dashboard snapshot barriers no longer advance runtime activity timestamps.
  • The unreachable synchronous stats error wrapper was removed.

Validation passed: Biome, build, tsgo --noEmit, throttled-browser acceptance, full no-live suite with 4,923 passed and 706 skipped, and workspace-link verification.

Commit: 4782de0


Progress tracked by mach6

@m-aebrer

Copy link
Copy Markdown
Collaborator

Code Review

Critical

No critical findings.

Important

No high-severity findings.

Suggestions

Finding 1 — Successful stop actions rely entirely on SSE to remove the runtime locally

  • Files: packages/dashboard/src/client/screens/fleet.tsx, packages/dashboard/src/client/screens/session.tsx
  • Severity: medium
  • Confidence: 88
  • Type: runtime-risk
  • Detail: Both stop handlers await api.stopRuntime() and refresh disk inventory, but do not apply the successful removal to the local fleet/session store. If SSE is reconnecting or has a replay gap, the initiating tab can retain a dead runtime card and navigate to a runtime that no longer exists. The direct API result should drive the same narrow local removal transition as runtime_removed, without restoring a full fleet read.

Finding 2 — The first snapshot for a created/resumed runtime can remain fallback-only

  • File: packages/dashboard/src/server/runtime-pool.ts
  • Severity: medium
  • Confidence: 84
  • Type: runtime-risk
  • Detail: startSessionRuntime() registers the handle and schedules a snapshot before describe() seeds handle.lastState. If the 200 ms timer fires before getState() finishes, other clients receive fallback fields. describe() updates the authoritative baseline but does not schedule a follow-up snapshot, so the placeholder can remain until another lifecycle event.

Finding 3 — Unrelated fleet mutations can suppress authoritative lower stats counts

  • File: packages/dashboard/src/client/state/store.ts
  • Severity: low
  • Confidence: 90
  • Type: correctness
  • Detail: A stats round captures the global fleetMutationGeneration and uses one liveStateRaced value for every runtime. A disk refresh or a change to runtime B therefore makes runtime A preserve Math.max(current, stats.totalMessages), even when A legitimately forked/rewound to a lower authoritative count. Capture per-runtime state generations for stats, as hydration already does.

Finding 4 — A pre-barrier fleet snapshot can suppress a newer hydrate card baseline

  • File: packages/dashboard/src/client/state/store.ts
  • Severity: low
  • Confidence: 82
  • Type: ordering-risk
  • Detail: Any same-runtime global snapshot arriving during hydrate increments the target generation and prevents applying hydrated fleet-card state, even when that snapshot's sequence is older than the hydrate response's barrierSeq. The session baseline is still applied and a later snapshot should self-heal the card, but sequence-aware freshness would avoid this transient inversion.

Finding 5 — Runtime-stop controls lack behavior-level regression tests

  • Files: packages/dashboard/src/client/screens/fleet.tsx, packages/dashboard/src/client/screens/session.tsx, packages/dashboard/test/client/screens.test.tsx
  • Severity: medium
  • Confidence: 98
  • Type: missing-test
  • Detail: Neither stop click path verifies api.stopRuntime(key), one narrow disk refresh, no api.fleet() request, and session-screen navigation back to fleet. These paths changed from full fleet refreshes and are part of the user-action inventory contract.

Finding 6 — The fork test still mocks the removed full-fleet behavior

  • Files: packages/dashboard/src/client/screens/session.tsx, packages/dashboard/test/client/screens.test.tsx
  • Severity: medium
  • Confidence: 99
  • Type: stale-test
  • Detail: The fork path now hydrates and calls refreshDiskSessions(), but the test supplies an unused refreshFleet mock and asserts neither the new inventory refresh nor the absence of a fleet request. Removing the new refresh would not fail the test.

Finding 7 — The error snapshot test does not assert the projected error state

  • File: packages/dashboard/test/runtime-pool.test.ts
  • Severity: medium
  • Confidence: 96
  • Type: weak-test
  • Detail: The agent_end error test checks the internal handle and only that some snapshot was emitted. It would pass if snapshot projection dropped error, needsAttention, or the stopped streaming state. Assert those fields in the emitted runtime payload.

Finding 8 — The mobile profiler's production byte-stream path is untested

  • Files: packages/dashboard/scripts/profile-mobile.mjs, packages/dashboard/test/profile-mobile.test.mjs
  • Severity: low
  • Confidence: 94
  • Type: missing-test
  • Detail: Production feeds fragmented Uint8Array chunks to ingest(), while tests only call ingestText() with complete ASCII frames. A split SSE frame or split multi-byte UTF-8 sequence could break reconstruction or byte accounting without test detection.

Finding 9 — applyFleetSnapshot() filters surviving runtimes twice

  • File: packages/dashboard/src/client/state/store.ts
  • Severity: low
  • Confidence: 92
  • Type: simplification
  • Detail: The same removedRuntimeKeys predicate is evaluated once for membership and again for mapping. Compute one surviving array and reuse it to make the shared input explicit.

Finding 10 — Runtime model and thinking-level setters duplicate the same patch mechanics

  • File: packages/dashboard/src/client/state/store.ts
  • Severity: low
  • Confidence: 80
  • Type: simplification
  • Detail: setRuntimeModel() and setRuntimeThinkingLevel() contain identical map-and-spread logic. A typed patchRuntimeState(key, patch) helper would retain the public methods while centralizing the mutation.

Strengths

  • The linked issue's architecture and acceptance criteria are comprehensively implemented: debounced zero-RPC snapshots, one-call hydration, narrow disk refreshes, bounded stats polling, deterministic card ordering, and documented mobile profiling.
  • Hydration has strong abort, removal, resync, overflow, supersession, and barrier-ordering protections.
  • Error paths are generally loud and preserve last-good values; debounce and stats timers are bounded and cleaned up.
  • Prior review findings around live-API guards, OAuth side effects, stalled stats requests, per-runtime hydrate freshness, and read-only activity timestamps are present and correctly fixed in the final code.

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


Reviewed by mach6

@m-aebrer

Copy link
Copy Markdown
Collaborator

Review Assessment

Review comment: #388 (comment)

Classifications

Finding Classification Reasoning
Finding 1 — Stop actions rely on SSE for local removal genuine Both handlers only stop the server runtime and refresh disk inventory; neither applies the store's existing removal transition. A successful request can leave dead local card/session state until SSE or recovery.
Finding 2 — First create/resume snapshot may stay fallback-only genuine Runtime registration schedules a snapshot before the create route's describe() seeds handle.lastState, and describe() does not schedule a follow-up. A slow state RPC can therefore leave other clients with fallback fields indefinitely.
Finding 3 — Global stats mutation generation suppresses legitimate lower counts genuine One global race flag is shared across all runtime stats results. An unrelated disk or runtime mutation can force Math.max(...) for runtime A and hide A's authoritative fork/rewind count.
Finding 4 — Pre-barrier snapshot can suppress a newer hydrate baseline genuine Same-runtime generation changes reject hydrated card state without comparing the snapshot sequence to barrierSeq. A snapshot at or before the hydrate barrier is older and should not win.
Finding 5 — Stop controls lack regression tests genuine Neither client stop control has behavior coverage for stop, local removal, narrow inventory refresh, zero fleet reads, and session navigation.
Finding 6 — Fork test retains stale refresh behavior genuine The test still supplies an unused refreshFleet mock and does not assert the new refreshDiskSessions() call or absence of api.fleet().
Finding 7 — Error snapshot assertion is too weak genuine The test checks internal handle state and only that an emission exists. It would pass if the snapshot omitted error, needsAttention, or stopped streaming state.
Finding 8 — Fragmented profiler byte-stream path is untested genuine Production uses streaming Uint8Array ingestion, while tests only use complete text frames. Split frame and split UTF-8 reconstruction are meaningful new paths without coverage.
Finding 9 — Duplicate removed-runtime filtering nitpick The two local synchronous filters are redundant but correct; reusing one array is optional cleanup.
Finding 10 — Duplicate runtime-state patch setters nitpick The small type-specific setters are clear and correct. A generic patch helper is stylistic.

Action Plan

  1. Apply successful stops directly to local fleet/session state; test both stop controls for removal, one disk refresh, zero fleet reads, and session navigation.
  2. Emit an authoritative follow-up snapshot after describe() seeds/enriches runtime state; add a delayed-state create/resume race test.
  3. Track stats freshness per runtime and test that unrelated mutations do not suppress a legitimate lower count.
  4. Make hydration card freshness sequence-aware so only same-runtime snapshots newer than the hydrate barrier win; test pre- and post-barrier cases.
  5. Update the fork test to assert disk refresh and no fleet request, removing the stale mock.
  6. Assert projected error, needsAttention, and stopped streaming state in the emitted error snapshot.
  7. Test profiler Uint8Array ingestion with fragmented frames and split multi-byte UTF-8.

Counts: 8 genuine, 2 nitpicks, 0 false positives, 0 deferred.


Assessment by mach6

@m-aebrer

Copy link
Copy Markdown
Collaborator

Final Review Findings Fixed

Addressed all eight genuine findings from the latest assessment:

  • Successful fleet/session stops now apply an idempotent local removal immediately, clear stale session state, refresh disk inventory once, and tolerate the later SSE echo without restoring a full fleet read.
  • Runtime creation/resume now publishes a follow-up snapshot when describe() enriches the initial fallback state, including context-only enrichment, without redundant emissions or activity changes.
  • Stats freshness is tracked per runtime, so unrelated mutations no longer suppress authoritative rewinds while stale same-runtime stats cannot overwrite a newer hydrated count.
  • Hydrate fleet-card freshness is barrier-sequence-aware and also retains intervening non-SSE state mutations.
  • Added behavior coverage for both stop controls, fork inventory refresh, projected agent errors, delayed authoritative snapshots, context-only enrichment, stats/hydrate races, and fragmented SSE profiler byte streams.

Validation passed: build, TypeScript, Biome, workspace-link verification, all 734 dashboard tests, and the full no-live suite with 4,942 passed and 706 skipped.

Commit: c90c38a


Progress tracked by mach6

@m-aebrer
m-aebrer merged commit 39ea93a into master Jul 23, 2026
3 checks passed
@m-aebrer
m-aebrer deleted the feature/issue-387-sse-fleet-snapshots branch July 23, 2026 16:06
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.

Dashboard: eliminate REST fleet refresh with SSE-pushed snapshots for mobile performance

2 participants