fix(desktop): derive terminal agent status from host bindings (single source of truth)

Merged
KI
4mo ago
+999-902

Links

  • ExecPlan: apps/desktop/plans/done/20260703-1507-derive-terminal-agent-status-from-host-binding.md
  • Supersedes #5443 (close it when this merges)

Summary

  • Terminal agent status (working/permission/idle/review) is now derived from the host's TerminalAgentBinding instead of being replayed into a localStorage-persisted renderer status map. Stale "working" dots after an app restart become structurally impossible — there is no stored copy left to go stale.
  • Host-side, binding visibility is derived from session liveness: listByWorkspace/findActive join terminal_sessions on status = 'active', so agents in dead terminals (kill -9, crash, host downtime) disappear from reads with zero prune bookkeeping. This is the root fix for the leak #5443 patched with compensating layers.
  • Net −378 lines in tracked files; the renderer notification store shrinks to two maps (manualUnread, terminalSeenAt).

Why / Context

The binding store and the renderer status map were both projections of the same agent-hook stream, coupled only at the source (notifications.hook broadcasts the WS event and records the binding in one mutation). The renderer projection only updated while a window was open and connected, and was persisted with no reconciliation — any missed Stop/Detached became permanent drift: phantom "working" dots, workspace indicators lit for terminals that no longer exist.

How It Works

  • working ⇔ binding.lastEventType === "Start", permission ⇔ "PermissionRequest", idle ⇔ "Stop"/"Attached" — a pure function (deriveTerminalAgentStatus) over the binding the renderer already fetches.
  • review = the host recorded a Stop newer than a renderer-persisted per-terminal lastSeenAt. Seen marks are written when a pane is focused, when an event lands while its target is visible, on interrupt/close, and by the sidebar mark-read action. Because it's computed from the host's lastEventAt, review survives the missed-event case (agent finishes while the app is closed → correct green dot on relaunch).
  • Interrupt (Esc/Ctrl+C) POSTs a synthetic Stop to the existing notifications.hook endpoint (agents fire no Stop hook on interrupt), gated on the binding actually being working/permission so vim-style Escape spam doesn't spam the host.
  • The Electron fallback for adopted shells (stale hook URLs) forwards events to the host so the binding — now the source of truth — converges instead of being bypassed.
  • The zustand store's persist version bumps to 2; the migration keeps only manual mark-unread entries and drops all legacy per-source statuses.

Manual QA Checklist

  • Run claude in a v2 workspace terminal, submit a prompt → amber "working" dot on pane tab, sidebar chip, workspace row
  • Trigger a permission prompt → red "permission" dot until answered
  • Switch workspaces before the turn ends → green "review" dot + chime on finish; focusing the pane clears it
  • Stale-state regression: quit the app (Cmd+Q) mid-turn, let the agent finish, relaunch → dot is green "review", NOT amber "working"
  • Interrupt: Esc during a turn → dot clears within ~1s in this window and in a second window
  • Binding leak: kill -9 a bound agent's terminal shell → agent chip disappears from the sidebar without any hook firing
  • localStorage v2-notifications-v1 shows manualUnread/terminalSeenAt, no legacy sources statuses

Testing

  • cd apps/desktop && bun test — 2045 pass (note: must run with cwd apps/desktop so bunfig.toml preloads test-setup.ts)
  • cd packages/host-service && bun test — 750 pass, incl. new persistence.test.ts (in-memory sqlite + migrations) covering the liveness join and deleteDefunct
  • bun run typecheck, bun run lint — clean
  • Derivation tests were mutation-checked (breaking the Stop branch fails 2 tests)
  • Manual QA above not yet performed — needs a human driving the app; checklist mirrors the ExecPlan acceptance

Design Decisions

  • Join, not pruning (host): terminal_sessions.status is the already-maintained liveness source (pty onExit, dispose routes, reaper healing). Reading through it makes dead bindings unrepresentable; every review finding on #5443 was a failure mode of its patch machinery (listener leaks, startup crash, blocking broadcasts).
  • Derived review via lastSeenAt, not stored review: a stored review requires witnessing the event; a derived one is correct even when the app was closed when the agent finished.
  • Deleted rather than reconciled: no sync protocol between two stores — the second projection is simply gone. The store now holds facts about the user (seen timestamps, manual unread), never about the agents.

Known Limitations

  • review no longer survives agent Detached (binding is deleted when the agent process exits). Accepted: the chip disappears with the dot, and the failure is benign.
  • The sidebar "clear status" action can no longer wipe a working/permission dot — those are live host state now, which is the point.
  • Chat pane statuses (previously dead plumbing with zero callers) were deleted; rebuild on the derived model when chat statuses actually ship.

Rollback

Revert the two commits; the persist migration is forward-only but harmless on rollback (old code treats the v2 blob as empty state and rebuilds from live events).

Open in Stage

Summary by cubic

Derives terminal agent status from the host binding as the single source of truth, removing stale “working” dots and auto-hiding agents from dead terminals via session liveness. Adds a 30s cache window so focus/remount refetch self-heals WS outages; dock badge and workspace indicators now use the derived model.

  • Bug Fixes

    • Bindings visibility reads through a session‑liveness join; best‑effort prune runs at startup, and session dispose now broadcasts an exit.
    • Status is derived from bindings: working/permission/idle from lastEventType, review when Stop > seen; seen marks use host time only and are written on focus/visible events; exit prunes seen.
    • Interrupt posts a synthetic Stop only when an agent is active; adopted‑shell events are forwarded to the host so bindings converge.
    • Bindings query uses staleTime: 30s; lifecycle events still invalidate for instant updates.

  • Refactors

    • Replaced the renderer status map with useTerminalAgentStatuses and useV2NotificationStatus; the store now keeps only manualUnread and terminalSeenAt with a v2 persist migration.
    • Deleted status‑transition code and the unused terminalAgents.onWorkspaceChange stream; dock badge/workspace indicators aggregate derived statuses from the react‑query cache.
    • Host persistence adds live reads (listLiveByWorkspace/findLiveActive) and deleteDefunct; derivation and persistence are covered by new tests.

Summary by CodeRabbit

  • New Features
    • Desktop terminal agent indicators (working/permission/review/idle) are now derived from host-side lifecycle signals, and workspace attention/unread is computed from derived terminal state plus manual unread, including an OS dock badge count.
  • Bug Fixes
    • Ctrl+C / Escape now records an interruption Stop when an agent turn is active, improving interrupt handling.
    • Visibility and terminal lifecycle now mark items as “seen” consistently and prune stale “seen” data.
    • Lifecycle events are forwarded to the active host so host state remains the source of truth.
  • Tests
    • Added tests for status derivation and terminal-agent persistence “live” reads/pruning, and expanded v2 notification store/migration coverage.