Skip to content

fix(dashboard): eliminate layout/terminal/store stuck-states (audit sweep) - #282

Merged
aterrylu merged 1 commit into
mainfrom
terry/dashboard-stuck-states
Jul 18, 2026
Merged

fix(dashboard): eliminate layout/terminal/store stuck-states (audit sweep)#282
aterrylu merged 1 commit into
mainfrom
terry/dashboard-stuck-states

Conversation

@aterrylu

Copy link
Copy Markdown
Owner

Problem

An audit of the dockview / terminal / store layer (fanned out across 4 read-only agents) surfaced a family of unrecoverable UI states — states you could get into and not get out of without a hard refresh or manually clearing localStorage. These are the "weird bugs where it broke and I couldn't get out" that motivated the audit.

The six clusters

flowchart TD
    subgraph A["🔴 A · Permanent blank screen"]
        A1[corrupt persisted activePane / poison workspace] --> A2[throws during restore] --> A3[re-persists] --> A4[blank on EVERY reload]
    end
    subgraph B["🔴 B · Idle terminal self-shrink"]
        B1[idle GPU context-loss] --> B2[re-fit vs unmeasured renderer] --> B3[plausible-but-wrong size to PTY] --> B4[change-cache never corrects → wedged]
    end
    subgraph C["🟠 C · Every click rebuilds group"]
        C1[workspace member killed] --> C2[stale id lingers in paneIds] --> C3[desired ≠ live → full fromJSON teardown every click]
    end
    subgraph D["🟠 D · Dock blanks on exit"]
        D1[watched agent exits] --> D2[activePane set null, no fallback] --> D3[empty screen despite live siblings]
    end
    subgraph E["🟠 E · Touch drag-tracking death"]
        E1[touch tab drag] --> E2[pointer backend fires no dragend] --> E3[internalDragActive latches true] --> E4[active-pane tracking silently dies]
    end
Loading

Solution

Cluster Fix
A Top-level ErrorBoundary with a two-tier recovery — "Reset layout" (drops layout keys, keeps prefs) → loop-detected "Clear all saved data" escalation with honest copy. merge() now validates activePane (isValidActivePane) and the workspace serialized blob, so corrupt state degrades to the empty state instead of crashing. syncToActive has an outer catch → recoverable empty state.
B Centralized applyFit is plausibility-guarded (isPlausibleFit — rejects fits whose implied cell size exceeds a sane monospace max), retries on unsettled frames (scheduleFit), and invalidates the change-cache so the ResizeObserver can self-correct. Context-loss / focus / RO all route through it.
C reconcileDeadWorkspaces drops dead members from bound workspaces (dissolving ≤1-member groups) on kill / exit / fetch, so sameSet can match again and clicks stop rebuilding.
D pickActiveFallback retargets to a live workspace sibling (or any live session) instead of blanking the dock.
E The internal-drag guard now also settles on pointerup / pointercancel (the touch/pointer backend never fires dragend), with a double-schedule guard for the mouse case.
F reorderHierarchy bounds guard, handleExternalDrop dead-session guard, diagnostic console.warn breadcrumbs on the silent recovery paths.

Testing

  • 20 pure-function unit testsisValidActivePane, isPlausibleFit, reconcileDeadWorkspaces, pickActiveFallback (boundary + dissolve cases).
  • 5 ErrorBoundary component tests — both recovery tiers, loop detection, stale-marker handling, localStorage surgery.
  • 3 real-browser integration tests (stuck-states.spec.ts) — drive the real app in Chromium through the full rehydrate → mount → fetch → reconcile pipeline for A/C/D. (Scenario C's first failure caught a real interaction — the fromJSON-drop path firing before reconcile — that unit tests couldn't see.)
  • make check green (615 server/cli + 254 dashboard vitest); Playwright e2e all 14 pass; biome + tsc clean; pre-push ci-gate green.

Polish

Reviewed by the 3-agent pass (code-reviewer / simplifier / silent-failure-hunter). The reviewer found no blocking issues; the silent-failure-hunter surfaced 4 findings all fixed in this PR — the headline being the ErrorBoundary loop-proofing (a global catch with a layout-scoped remedy could otherwise soft-lock the user on a non-layout crash).

Risks

  • B uses a heuristic cell-size bound (30×60px). Verified safe for the hard-coded 14px font (~8×17px cell) and browser zoom (scales container + font together). Only trips near ~50px fonts.
  • C intentionally leaves the stale dockview serialized blob for a killed member — it self-heals via syncToActive's existing dead-panel strip on the next restore (the store can't call toJSON()).
  • All changes are client-side dashboard state; no server/API surface touched.

Alternatives considered

  • Widening the ErrorBoundary remedy to always full-wipe — rejected as user-hostile; tiered escalation preserves prefs on the common (layout) case.
  • Using proposeDimensions() for the plausibility check — rejected to avoid widening the deliberately-narrow IFitAddon wrapper; validating the post-fit terminal.cols/rows needs no new xterm API.

@nox-0x nox-0x left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving — a well-scoped audit sweep that turns each identified stuck-state into a small, testable helper with clear rationale in the comments.

Highlights from the review:

  • Cluster AisValidActivePane + serialized object-check in merge() correctly gate the two persisted fields that reach dockview.addPanel({id}) / api.fromJSON() on restore. The tiered ErrorBoundary (RESET_MARKER / resetJustFailed()) neatly avoids the "reset that can't fix a non-layout crash" soft-lock, and the outer syncToActive catch (DockviewLayout.tsx:237-246) means even a runtime throw during solo/setActive degrades to the recoverable empty state rather than the reset screen.
  • Cluster BisPlausibleFit bounds are reasonable for the 14px default (verified: ~8×17 cell) and browser zoom is size-preserving. applyFit correctly zeroes lastFitW/H on failure so the ResizeObserver's change-cache can retry, and scheduleFit retry with terminal "exhausted" warning is a nice touch.
  • Cluster CreconcileDeadWorkspaces correctly returns null for no-op, handles the ≤1 dissolve case, and only unbinds paneWorkspace when it still points at the same workspace (defensive). Test coverage pins the boundary cases.
  • Cluster DpickActiveFallback prefers a live sibling from the same workspace, falling back to any live session. Applied BEFORE the reconcile in fetchSessions (which may dissolve the group) — order matters and it's correct.
  • Cluster E — the settleInternalDrag + settleScheduled guard is right. onWillDragPanel sets internalDragActive=true only inside the browser's dragstart handler, so a stray pointercancel before dragstart is a no-op (the flag is still false). pointerup doesn't fire after a cancelled pointer, so mouse-drag paths still settle exclusively via dragend.
  • killSession now correctly gates the optimistic retarget on HTTP ok (store.ts:1022-1035) instead of racing fetchSessions() — a nice silent-failure fix caught in the audit.

Tests are proportional to the risk: pure-function unit tests for each helper, DOM tests for the ErrorBoundary tiers, and real-browser E2E for A/C/D through the actual rehydrate pipeline. Ship it.

@aterrylu
aterrylu marked this pull request as ready for review July 18, 2026 06:20
…weep)

An audit of the dockview/terminal/store layer surfaced a family of
unrecoverable UI states — states you could get into and not get out of.
This fixes all six clusters, each with regression coverage:

- A (blank screen): a corrupt persisted activePane / poison workspace threw
  during layout restore and re-persisted → blank on every reload. Added a
  top-level ErrorBoundary with tiered "Reset layout" → "Clear all data"
  recovery (loop-detected) + merge() validation (isValidActivePane, serialized
  blob) so bad state degrades to the empty state instead of crashing.
- B (idle terminal self-shrink): an idle GPU context-loss re-fit shipped a
  plausible-but-wrong size to the PTY that the ResizeObserver change-cache never
  corrected. Fitting is now plausibility-guarded (isPlausibleFit), retries on
  unsettled frames, and invalidates the cache to self-correct.
- C (every click rebuilds the group): a killed/exited workspace member lingered
  in the saved arrangement → full teardown+rebuild on every click. Dead members
  are reconciled out of bound workspaces on kill/exit (reconcileDeadWorkspaces).
- D (dock blanks on active-agent exit): watching an agent that exits dropped to
  the empty screen even with other live agents → now retargets to a live sibling
  (pickActiveFallback).
- E (touch drag-tracking death): internalDragActive only reset on native
  dragend (never fired by the pointer/touch backend) → latched on, killing
  active-pane tracking. Now also settles on pointerup/pointercancel.
- F (hardening): reorderHierarchy bounds guard, handleExternalDrop dead-session
  guard, and diagnostic breadcrumbs on silent recovery paths.

Coverage: 20 pure-function unit tests, 5 ErrorBoundary component tests, and 3
real-browser integration tests (stuck-states.spec.ts) driving the full
rehydrate→mount→fetch→reconcile pipeline. make check + e2e green. Polished via
the 3-agent review (reviewer/simplifier/silent-failure-hunter).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: 4493019a-e7b8-4845-902d-6cec70c188e7
@aterrylu
aterrylu force-pushed the terry/dashboard-stuck-states branch from 5746c85 to c4f78ee Compare July 18, 2026 06:43
@aterrylu
aterrylu enabled auto-merge (squash) July 18, 2026 06:43
@aterrylu
aterrylu merged commit d973992 into main Jul 18, 2026
5 checks passed
@aterrylu
aterrylu deleted the terry/dashboard-stuck-states branch July 18, 2026 06:44
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.

2 participants