diff --git a/docs/superpowers/plans/2026-09-05-pane-runtime-isolation.md b/docs/superpowers/plans/2026-09-05-pane-runtime-isolation.md new file mode 100644 index 00000000..7d5b639d --- /dev/null +++ b/docs/superpowers/plans/2026-09-05-pane-runtime-isolation.md @@ -0,0 +1,105 @@ +# Per-pane runtime subscriptions + +Status: implemented; final checks/PR review in progress. Refs #763. Base: main 5d641845. + +## Invariants and scope + +Runtime-only updates and composer draft changes must not execute App or walk +unrelated tile trees. Layout/controller state remains React-owned; do not freeze +action closures. Commands and IPC must read the current store synchronously. +Session observers retain layout-effect ordering for lifecycle observations, +picker invalidation and autosave. Reader, headers and inspection surfaces must +continue subscribing to the state they paint. No lifecycle/readiness policy, +terminal mounting/resize ownership, screen transport or diagnostics policy change. + +## Steps + +1. Remove runtime-map and invisible draft-version invalidations from the root + controller. Keep an immediately current imperative runtime read boundary. +2. Mount per-session runtime observers below the controller; keep autosave + invalidation separate from pane painting. Preserve provider observation + chronology, timers and teardown behavior. +3. Add per-session subscribed leaf boundaries and layout-only context access; + migrate Reader, headers and other runtime consumers explicitly. Keep broad + inspection surfaces reactive until narrowed individually, rather than + silently rendering stale state. +4. Add deterministic render-count/freshness regressions, run relevant renderer + suites, full typecheck and current CI. Record before/after counts rather than + infer a CPU percentage. Open a complete PR; never merge without approval. + +## Coordination + +#808 independently fixes worktree projection/replay waste. #762 and #767 remain +subsequent independent increments; A6 owns #802–805. Older mixed experimental +worktrees are not edited or committed wholesale. Preserve root package-lock.json. + +## Evidence and review boundaries + +`runtimeIsolation.renderer.test.tsx` mounts the real workspace controller, +store, helpers, draft/autosave path and subscribed tile boundaries. It stubs +provider paint and external boot/event ingress so it cannot start live agents. +The control arm restores only the old root runtime-map subscription. For 100 +committed single-session updates: controller and unrelated-pane renders are +100/100 in the control and 0/0 in the optimized arm; the affected pane paints +100 times in both. This is a deterministic invalidation measurement, not an +app-wide CPU or typing-latency claim. + +Additional regressions cover synchronous draft reads before React commit, +debounced persistence and clear/undo, session replacement subscription routing, +fresh focus actions, late rendered-lease cleanup in Terminal mode, and exactly +once lifecycle publication before passive visibility with the retired run ID. + +Review caught two cases beyond the earlier experiment: fresh inline arrays +would defeat a memo boundary, so leaf props are explicit; removing root runtime +renders also removes incidental lease cleanup, so hygiene now subscribes only +to picker/lease signals. Tab counts and related headers select painted status +values, while Reader subscribes to its chosen runtime. + +Broad inspection/context consumers intentionally remain reactive; narrowing +individual debug/modal subscriptions further is not silently approximated. +No transport or backend processing changed. Local Node 25 exposes an incomplete +native localStorage without a backing file; renderer checks run with +NODE_OPTIONS=--no-experimental-webstorage so happy-dom owns storage, matching the +Node 24 CI environment. The initial run exposed this environment issue and four +outdated store/context mocks; production guards were not weakened to hide them. + +Verification: full renderer run passed 115 files / 499 tests before adding the +last three regression cases; all six targeted isolation cases now pass. Final +full run: 501 passed, one timeout in the unchanged lazy-prose dynamic-import +test (existing #700), under substantially increased machine load (146.97 s +suite versus 24.52 s earlier). No retry policy, timeout, or test assertion was +weakened. Full typecheck, test contract and checked-in fixture privacy gates +passed during implementation; final typecheck and public CI tracked in the PR. + +## Independent review resolution (heads cccae6e1 / a96f00a2) + +Both reviewers approved; one latent coupling fixed in this increment. PaneHeader +shares the phone bundle, whose stub store (appStateHooks.ts) has NO +`workspaceRuntimes` key; the new related-status store read was safe only because +SessionView passes an empty chip list. The read is now optional-chained with a +WHY comment, and a dedicated renderer regression mounts the two phone shapes +(empty chips; keyless store with chips) plus the desktop store path. Also +adopted from review: `getRuntime`'s shared fallback runtime documents its +never-mutate invariant; `useFeedDebugPersist` no longer takes a render-time +snapshot it ignored; the draft-version signal types honestly as `() => void` +instead of a React state setter whose argument was discarded. The one remaining +runtime-derived root invalidation is the picker/lease shallow subscription in +useRenderedLeaseHygiene — it fires only on user commands, is no-op guarded, and +is the previous universal behavior; kept deliberately. + +## Post-review main merge and merged-tree CI (head c8126f48 + origin/main) + +Reviewer confirmation landed for heads cccae6e1/a96f00a2 and 29e2dc81/c8126f48, +then pushing the fix head exposed what the stale worktree base hid: origin/main +had advanced past merge-base 5d641845 (external operator toolkit #812, MCP tool +policy #818) while this branch was in review, and those PRs added new control +tests that call the pre-isolation hook signatures. The merged-tree quality-gate +caught exactly two TS errors (control.renderer.test.tsx's third argument to +useDraftActions; preferences.renderer.test.tsx's 3-arg useWorkspaceHelpers). +origin/main was merged into this branch (no production conflicts), and the two +tests were adapted: the draft harness now passes a no-op bumpDraftChanges (it +reads drafts imperatively via inspectAgentDraft), and the preferences harness +passes only (setRuntimes, refs) since useWorkspaceHelpers now reads runtimes +through refs and toggles via the store updater. No production code changed in +this step. Merged-tree verification: forced typecheck clean, full renderer run +123 files / 523 tests green (Node 24, two workers), worktree suites unaffected. diff --git a/src/renderer/src/app/App.tsx b/src/renderer/src/app/App.tsx index 8f490b49..aa594b8c 100644 --- a/src/renderer/src/app/App.tsx +++ b/src/renderer/src/app/App.tsx @@ -105,6 +105,7 @@ export default function App() { return ( + {workspace.runtimeServices}
diff --git a/src/renderer/src/app/shell/MainSurface.tsx b/src/renderer/src/app/shell/MainSurface.tsx index c15ebbf7..ee74b182 100644 --- a/src/renderer/src/app/shell/MainSurface.tsx +++ b/src/renderer/src/app/shell/MainSurface.tsx @@ -1,5 +1,5 @@ import { useAppStore } from '@renderer/app-state/hooks' -import { useWorkspaceContext } from '@renderer/workspace/WorkspaceContext' +import { useWorkspaceLayoutContext } from '@renderer/workspace/WorkspaceContext' import { SettingsPage } from '@renderer/features/settings/ui/SettingsPage' import { ReaderView } from '@renderer/features/reader/ui/ReaderView' import { SpotlightView } from '@renderer/features/spotlight/ui/SpotlightView' @@ -21,7 +21,7 @@ import { WelcomeEmpty } from './WelcomeEmpty' // clickable escape hatch. Otherwise the main area renders null // and the app looks bricked. export function MainSurface({ onNewTabRequest }: { onNewTabRequest: () => void }) { - const workspace = useWorkspaceContext() + const workspace = useWorkspaceLayoutContext() const settings = useAppStore(state => state.settings) const setSettings = useAppStore(state => state.setSettings) const resetSettings = useAppStore(state => state.resetSettings) diff --git a/src/renderer/src/app/shell/RestoreBanner.tsx b/src/renderer/src/app/shell/RestoreBanner.tsx index 28d080c3..e8002511 100644 --- a/src/renderer/src/app/shell/RestoreBanner.tsx +++ b/src/renderer/src/app/shell/RestoreBanner.tsx @@ -1,6 +1,6 @@ import { useEffect, useState } from 'react' -import { useWorkspaceContext } from '@renderer/workspace/WorkspaceContext' +import { useWorkspaceLayoutContext } from '@renderer/workspace/WorkspaceContext' // WHY render this above TabBar instead of as a toast: // @@ -23,7 +23,7 @@ import { useWorkspaceContext } from '@renderer/workspace/WorkspaceContext' const COLLAPSE_AFTER_MS = 60_000 export function RestoreBanner() { - const workspace = useWorkspaceContext() + const workspace = useWorkspaceLayoutContext() const message: string | null = workspace.restoreStatus === 'partial-restore' ? 'Workspace partially restored — autosave is disabled to protect your saved state. Restart Agent Code after fixing the underlying spawn or proxy issue.' diff --git a/src/renderer/src/app/shell/SettingsBar.tsx b/src/renderer/src/app/shell/SettingsBar.tsx index a693f7c8..39cd8b0b 100644 --- a/src/renderer/src/app/shell/SettingsBar.tsx +++ b/src/renderer/src/app/shell/SettingsBar.tsx @@ -4,12 +4,12 @@ import { SystemPerfHeader } from '@renderer/features/system-perf/ui/SystemPerfHe import { UsageHeaderIndicator } from '@renderer/features/usage/ui/UsageHeaderIndicator' import { useAppStore } from '@renderer/app-state/hooks' import { useCaffeinateStore } from '@renderer/features/caffeinate/store' -import { useWorkspaceContext } from '@renderer/workspace/WorkspaceContext' +import { useWorkspaceLayoutContext } from '@renderer/workspace/WorkspaceContext' // Settings bar — compact row under tabs holding app chrome. // (Extracted verbatim from App.tsx by #494.) export function SettingsBar() { - const workspace = useWorkspaceContext() + const workspace = useWorkspaceLayoutContext() const settings = useAppStore(state => state.settings) const setSettings = useAppStore(state => state.setSettings) const performancePanelOpen = useAppStore(state => state.performancePanelOpen) diff --git a/src/renderer/src/app/shell/TerminalDimensionOwnership.renderer.test.tsx b/src/renderer/src/app/shell/TerminalDimensionOwnership.renderer.test.tsx index a51ba372..d9b7b431 100644 --- a/src/renderer/src/app/shell/TerminalDimensionOwnership.renderer.test.tsx +++ b/src/renderer/src/app/shell/TerminalDimensionOwnership.renderer.test.tsx @@ -24,6 +24,7 @@ vi.mock('@renderer/app-state/hooks', () => ({ vi.mock('@renderer/workspace/WorkspaceContext', () => ({ useWorkspaceContext: () => harness.workspace, + useWorkspaceLayoutContext: () => harness.workspace, })) vi.mock('@renderer/features/workspace/surfaces/usePlacementOverlay', () => ({ @@ -95,6 +96,7 @@ describe('terminal dimension ownership across main-surface takeovers', () => { root: { type: 'leaf', sessionId: 'session-1' }, } harness.appState = { + workspaceRuntimes: {}, debugPanelOpen: true, feedDebugPanelOpen: false, proxyDebugPanelOpen: false, diff --git a/src/renderer/src/features/prompt-templates/control.renderer.test.tsx b/src/renderer/src/features/prompt-templates/control.renderer.test.tsx index 444a64f9..7033432a 100644 --- a/src/renderer/src/features/prompt-templates/control.renderer.test.tsx +++ b/src/renderer/src/features/prompt-templates/control.renderer.test.tsx @@ -1,4 +1,3 @@ -import { useState } from 'react' import { act, cleanup, renderHook } from '@testing-library/react' import { afterEach, expect, it, vi } from 'vitest' import { useAppStore } from '@renderer/app-state/store' @@ -17,8 +16,12 @@ it('inserts dynamic project context into the named agent without following focus sessions: { target: { kind: 'claude', cwd: '/target', providerSessionId: 'native-target' }, other: { kind: 'codex', cwd: '/other', providerSessionId: 'native-other' } }, detachedSessions: {}, buried: [], }, workspaceRuntimes: { target: emptyRuntime(), other: { ...emptyRuntime(), draftInput: 'Other human draft' } } }) const mounted = renderHook(() => { - const [, setVersion] = useState(0), setRuntimes = useAppStore.getState().setWorkspaceRuntimes - return { ...useDraftActions(setRuntimes, (id, patch) => setRuntimes(prev => ({ ...prev, [id]: { ...prev[id], ...patch } })), setVersion), restoreStatus: 'fresh' } + const setRuntimes = useAppStore.getState().setWorkspaceRuntimes + // The production wiring passes draftChanges.bump, which forces draft + // surfaces to re-read through a React version state. This harness reads + // drafts imperatively via inspectAgentDraft, so a stable no-op bump is + // behaviorally faithful to the old setVersion argument. + return { ...useDraftActions(setRuntimes, (id, patch) => setRuntimes(prev => ({ ...prev, [id]: { ...prev[id], ...patch } })), () => {}), restoreStatus: 'fresh' } }) const resolveTranscriptPaths = vi.fn(async requests => requests.map((request: object) => ({ ...request, transcriptPath: '/recorded/source.jsonl', exists: true }))) window.api = { ...originalApi, resolveTranscriptPaths } diff --git a/src/renderer/src/features/reader/ui/ReaderView.tsx b/src/renderer/src/features/reader/ui/ReaderView.tsx index 17c69fb6..c58fe4c3 100644 --- a/src/renderer/src/features/reader/ui/ReaderView.tsx +++ b/src/renderer/src/features/reader/ui/ReaderView.tsx @@ -12,6 +12,7 @@ import { extractAssistantInProgress } from '@shared/parsers/extractAssistant' import { DEFAULT_PROVIDER, isAgentProviderKind } from '@shared/types/providerKind' import { assistantUuidsWithText, extractAssistantByUuid } from '@renderer/lib/copyAssistant' import { resolveTabSessions } from '@renderer/workspace/queries' +import { useSessionRuntime } from '@renderer/workspace/useSessionRuntime' import { dispatchSessionIdsForTab } from '@renderer/workspace/dispatch/dispatchSelectors' import type { SessionId, Workspace } from '@renderer/workspace/workspaceStore' import { PaneToast } from '@renderer/workspace/tile-tree/TileLeaf/PaneToast' @@ -135,7 +136,7 @@ function ReaderBody({ sessionId: SessionId sessionIds: SessionId[] }) { - const runtime = workspace.getRuntime(sessionId) + const runtime = useSessionRuntime(workspace, sessionId) const meta = workspace.state.sessions[sessionId] // Use the pane's actual provider for the screen extractor rather than // the old `=== 'codex' ? 'codex' : 'claude'` negation, which collapsed diff --git a/src/renderer/src/workspace/WorkspaceContext.tsx b/src/renderer/src/workspace/WorkspaceContext.tsx index 489dcd4f..d78fc620 100644 --- a/src/renderer/src/workspace/WorkspaceContext.tsx +++ b/src/renderer/src/workspace/WorkspaceContext.tsx @@ -1,6 +1,7 @@ import { createContext, useContext } from 'react' import type { ReactNode } from 'react' import type { Workspace } from '@renderer/workspace/workspaceStore' +import { useAppStore } from '@renderer/app-state/hooks' // WHY this exists (issue #494): App.tsx was the only owner of the // useWorkspace() hook value and therefore had to mount every feature @@ -8,13 +9,11 @@ import type { Workspace } from '@renderer/workspace/workspaceStore' // context lets surface wrappers (app/surfaces/registry.tsx) be // self-contained files that App never has to know about. // -// Re-render semantics — deliberately unchanged from prop drilling: the -// context value is the object useWorkspace() returns, which has a fresh -// identity on every App render, so every consumer re-renders whenever -// App does. That is exactly what the prop-drilled components already -// did. Do NOT try to memoize the workspace object here to "optimize" — -// its methods close over current state and a stale snapshot is a -// correctness bug, not a perf win. +// Layout/controller changes still publish a new context because actions may +// close over that state. Runtime traffic no longer renders the controller: +// panes subscribe by session, while useWorkspaceContext below explicitly keeps +// broad inspection surfaces reactive. Do not freeze action closures or assume +// an imperative getRuntime read constitutes a React subscription. // // NOTE for the remote client: this file must stay Electron-free (it is — // pure React). The phone bundle never mounts WorkspaceProvider, so @@ -32,7 +31,7 @@ export function WorkspaceProvider({ return {children} } -export function useWorkspaceContext(): Workspace { +export function useWorkspaceLayoutContext(): Workspace { const workspace = useContext(WorkspaceContext) if (!workspace) { // Loud failure beats a silent null: a surface rendered outside the @@ -41,3 +40,13 @@ export function useWorkspaceContext(): Workspace { } return workspace } + +// Broad inspection surfaces (debug, palette, multi-agent modals) explicitly +// retain their reactive runtime view. Layout shells instead use the layout +// hook above; individual panes subscribe by session id. Do not freeze action +// closures: the provider still updates whenever controller/layout state changes. +export function useWorkspaceContext(): Workspace { + const workspace = useWorkspaceLayoutContext() + const runtimes = useAppStore(state => state.workspaceRuntimes) + return { ...workspace, runtimes } +} diff --git a/src/renderer/src/workspace/control/drafts.renderer.test.tsx b/src/renderer/src/workspace/control/drafts.renderer.test.tsx index 61db9055..64c243b4 100644 --- a/src/renderer/src/workspace/control/drafts.renderer.test.tsx +++ b/src/renderer/src/workspace/control/drafts.renderer.test.tsx @@ -17,7 +17,7 @@ it('reads actual composer edits, protects concurrent text, and uses the existing const mounted = renderHook(() => { const [version, setVersion] = useState(0) const setRuntimes = useAppStore.getState().setWorkspaceRuntimes - const actions = useDraftActions(setRuntimes, (id, patch) => setRuntimes(prev => ({ ...prev, [id]: { ...prev[id], ...patch } })), setVersion) + const actions = useDraftActions(setRuntimes, (id, patch) => setRuntimes(prev => ({ ...prev, [id]: { ...prev[id], ...patch } })), () => setVersion(v => v + 1)) return { ...actions, version, restoreStatus: 'fresh' } }) const capabilities = draftControlCapabilities(() => mounted.result.current as unknown as Workspace) diff --git a/src/renderer/src/workspace/control/preferences.renderer.test.tsx b/src/renderer/src/workspace/control/preferences.renderer.test.tsx index 4bdd5b6a..30c059e4 100644 --- a/src/renderer/src/workspace/control/preferences.renderer.test.tsx +++ b/src/renderer/src/workspace/control/preferences.renderer.test.tsx @@ -13,7 +13,10 @@ it('uses real follow owners, preserves lanes and other agents, and reports Tail useAppStore.setState({ tailAllMode: true, workspaceState: { ...original.workspaceState, sessions: { first: { kind: 'claude', cwd: '/trial' }, second: { kind: 'codex', cwd: '/trial' } } }, workspaceRuntimes: { first: { ...emptyRuntime(), tailMode: true }, second: { ...emptyRuntime(), tailMode: true } } }) const layout = useAppStore.getState().workspaceState const refs = makeRefs(layout) - const mounted = renderHook(() => useWorkspaceHelpers(useAppStore.getState().workspaceRuntimes, useAppStore.getState().setWorkspaceRuntimes, refs)) + // useWorkspaceHelpers reads runtimes through refs and toggles via the + // setRuntimes updater, so the harness only needs the setter after the + // runtime-isolation refactor (dropped the render-time runtimes parameter). + const mounted = renderHook(() => useWorkspaceHelpers(useAppStore.getState().setWorkspaceRuntimes, refs)) const caps = preferenceControlCapabilities(() => ({ ...mounted.result.current, restoreStatus: 'fresh' }) as unknown as Workspace) const invoke = (id: string, input: unknown) => caps.find(cap => cap.descriptor.id === id)!.execute(input, context) const before = await invoke('views.preferencesRead', { sessionId: 'first' }) diff --git a/src/renderer/src/workspace/dispatch/DispatchPaneLabel.recorded.renderer.test.tsx b/src/renderer/src/workspace/dispatch/DispatchPaneLabel.recorded.renderer.test.tsx index a66bb05d..feef9412 100644 --- a/src/renderer/src/workspace/dispatch/DispatchPaneLabel.recorded.renderer.test.tsx +++ b/src/renderer/src/workspace/dispatch/DispatchPaneLabel.recorded.renderer.test.tsx @@ -12,6 +12,7 @@ import type { WorkspaceState } from '@renderer/workspace/types' import { asRecord } from '@shared/lib/asRecord' const appState = vi.hoisted(() => ({ + workspaceRuntimes: {}, dispatchListRatio: 0.25, openNewAgentForProject: vi.fn(), setDispatchListRatio: vi.fn(), diff --git a/src/renderer/src/workspace/dispatch/gridDispatchLayout.renderer.test.tsx b/src/renderer/src/workspace/dispatch/gridDispatchLayout.renderer.test.tsx index 742698e1..983e6a14 100644 --- a/src/renderer/src/workspace/dispatch/gridDispatchLayout.renderer.test.tsx +++ b/src/renderer/src/workspace/dispatch/gridDispatchLayout.renderer.test.tsx @@ -16,6 +16,7 @@ import type { Workspace } from '@renderer/workspace/workspaceStore' // unit test shipped as a no-op because the component undid it on render. const appState = vi.hoisted(() => ({ + workspaceRuntimes: {}, dispatchListRatio: 0.25, openNewAgentForProject: vi.fn(), setDispatchListRatio: vi.fn(), diff --git a/src/renderer/src/workspace/dispatch/tiledLaneResolution.renderer.test.tsx b/src/renderer/src/workspace/dispatch/tiledLaneResolution.renderer.test.tsx index 5a07ced6..ef02c74a 100644 --- a/src/renderer/src/workspace/dispatch/tiledLaneResolution.renderer.test.tsx +++ b/src/renderer/src/workspace/dispatch/tiledLaneResolution.renderer.test.tsx @@ -26,6 +26,7 @@ import type { Workspace } from '@renderer/workspace/workspaceStore' // lane on the next render. A unit test cannot see that; only mounting can. const appState = vi.hoisted(() => ({ + workspaceRuntimes: {}, dispatchListRatio: 0.25, openNewAgentForProject: vi.fn(), setDispatchListRatio: vi.fn(), diff --git a/src/renderer/src/workspace/hook/actions/draft.ts b/src/renderer/src/workspace/hook/actions/draft.ts index cead2136..0e652cea 100644 --- a/src/renderer/src/workspace/hook/actions/draft.ts +++ b/src/renderer/src/workspace/hook/actions/draft.ts @@ -1,5 +1,4 @@ import { useCallback } from 'react' -import type { Dispatch, SetStateAction } from 'react' import { emptyRuntime } from '@renderer/session-runtime/state' import type { SessionRuntime } from '@renderer/session-runtime/state' @@ -37,7 +36,7 @@ const clearedDrafts = new Map() export function useDraftActions( setRuntimes: WorkspaceSetRuntimes, updateRuntime: (sessionId: SessionId, patch: Partial) => void, - setDraftVersion: Dispatch>, + bumpDraftChanges: () => void, ): { setDraftInput: (sessionId: SessionId, text: string) => void setDraftImages: ( @@ -52,9 +51,9 @@ export function useDraftActions( const setDraftInput = useCallback( (sessionId: SessionId, text: string) => { updateRuntime(sessionId, { draftInput: text }) - setDraftVersion(v => v + 1) + bumpDraftChanges() }, - [setDraftVersion, updateRuntime], + [bumpDraftChanges, updateRuntime], ) const setDraftImages = useCallback( @@ -78,9 +77,9 @@ export function useDraftActions( }, } }) - setDraftVersion(v => v + 1) + bumpDraftChanges() }, - [setDraftVersion, setRuntimes], + [bumpDraftChanges, setRuntimes], ) /** @@ -113,10 +112,10 @@ export function useDraftActions( [sessionId]: { ...current, draftInput: '', draftImages: [] }, } }) - if (cleared) setDraftVersion(v => v + 1) + if (cleared) bumpDraftChanges() return cleared }, - [setDraftVersion, setRuntimes], + [bumpDraftChanges, setRuntimes], ) /** @@ -149,10 +148,10 @@ export function useDraftActions( }) if (previous.length > 0) clearedDrafts.set(sessionId, previous) else clearedDrafts.delete(sessionId) - setDraftVersion(v => v + 1) + bumpDraftChanges() return true }, - [setDraftVersion, setRuntimes], + [bumpDraftChanges, setRuntimes], ) return { setDraftInput, setDraftImages, clearDraft, undoClearDraft } diff --git a/src/renderer/src/workspace/hook/effects/useRenderedLeaseHygiene.ts b/src/renderer/src/workspace/hook/effects/useRenderedLeaseHygiene.ts index 8472568e..ff85a737 100644 --- a/src/renderer/src/workspace/hook/effects/useRenderedLeaseHygiene.ts +++ b/src/renderer/src/workspace/hook/effects/useRenderedLeaseHygiene.ts @@ -1,4 +1,5 @@ import { useEffect } from 'react' +import { useShallow } from 'zustand/react/shallow' import { useAppStore } from '@renderer/app-state/hooks' import type { Workspace } from '@renderer/workspace/workspaceStore' @@ -14,6 +15,11 @@ import type { Workspace } from '@renderer/workspace/workspaceStore' export function useRenderedLeaseHygiene(workspace: Workspace): void { const agentViewMode = useAppStore(state => state.settings.agentViewMode) const settingsPageOpen = useAppStore(state => state.settingsPageOpen) + // The controller no longer renders on every runtime replacement. A picker + // acquired while its surface is hidden must still be cleared, but ordinary + // terminal/text/debug changes cannot invalidate this cross-cutting guard. + const leaseSignals = useAppStore(useShallow(state => Object.entries(state.workspaceRuntimes) + .flatMap(([id, runtime]) => [id, runtime.assistantPicker, runtime.codeBlockPicker, runtime.renderedViewLeases]))) useEffect(() => { if (agentViewMode !== 'terminal') return @@ -31,7 +37,7 @@ export function useRenderedLeaseHygiene(workspace: Workspace): void { if (runtime?.codeBlockPicker) workspace.setCodeBlockPicker(sessionId, null) workspace.releaseAllRenderedViewLeases(sessionId) } - }, [agentViewMode, workspace]) + }, [agentViewMode, workspace, leaseSignals]) useEffect(() => { // NOTE the field is `readerMode`, not `reader` (cross-app audit V1): the @@ -58,5 +64,5 @@ export function useRenderedLeaseHygiene(workspace: Workspace): void { workspace.releaseRenderedViewLease(sessionId, 'copy-assistant-message') workspace.releaseRenderedViewLease(sessionId, 'copy-code-block') } - }, [settingsPageOpen, workspace]) + }, [settingsPageOpen, workspace, leaseSignals]) } diff --git a/src/renderer/src/workspace/hook/helpers.ts b/src/renderer/src/workspace/hook/helpers.ts index 991ff0fb..bc73a38b 100644 --- a/src/renderer/src/workspace/hook/helpers.ts +++ b/src/renderer/src/workspace/hook/helpers.ts @@ -14,6 +14,17 @@ import type { import type { WorkspaceRefs } from '@renderer/workspace/hook/refs' import { commandTargetSessionIdForState } from '@renderer/workspace/hook/selectors/commandTargetSessionId' +// Missing sessions need a stable read-only fallback so merely asking for their +// state cannot invalidate a memo boundary. Reducers still allocate their own. +// +// INVARIANT: this object is SHARED across every missing session and must never +// be mutated in place. Update paths always build fresh reducer-owned runtimes +// (spread + patch), so a `getRuntime()` result is only ever read; an in-place +// edit on it would silently contaminate every session that has no stored +// runtime yet — until one session is created, then exactly one shared object +// is shared by all of them at once (the worst possible time to corrupt it). +const EMPTY_RUNTIME = emptyRuntime() + // ----------------------------------------------------------------------------- // Cross-cutting runtime helpers // @@ -24,7 +35,6 @@ import { commandTargetSessionIdForState } from '@renderer/workspace/hook/selecto // ----------------------------------------------------------------------------- export function useWorkspaceHelpers( - runtimes: Record, setRuntimes: WorkspaceSetRuntimes, refs: WorkspaceRefs, ): { @@ -86,9 +96,9 @@ export function useWorkspaceHelpers( const getRuntime = useCallback( (sessionId: SessionId): SessionRuntime => { - return runtimes[sessionId] ?? emptyRuntime() + return refs.latestRuntimesRef.current[sessionId] ?? EMPTY_RUNTIME }, - [runtimes], + [refs.latestRuntimesRef], ) const toggleTailMode = useCallback( diff --git a/src/renderer/src/workspace/hook/index.ts b/src/renderer/src/workspace/hook/index.ts index c1c1ce42..75cd22b2 100644 --- a/src/renderer/src/workspace/hook/index.ts +++ b/src/renderer/src/workspace/hook/index.ts @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useMemo, useRef, useState } from 'react' +import { createElement, useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react' import { useAppStore } from '@renderer/app-state/hooks' import { useGlobalToast } from '@renderer/ui/GlobalToast' @@ -25,13 +25,11 @@ import { useHistoryActions } from '@renderer/workspace/hook/actions/history' import { useUndoCloseAction } from '@renderer/workspace/hook/actions/undoClose' import { useDispatchActions } from '@renderer/workspace/hook/actions/dispatch' import { useAgentIndexNavigationActions } from '@renderer/workspace/hook/actions/agentIndexNavigation' -import { useAutoSave } from '@renderer/workspace/hook/persistence/useAutoSave' +import { createDraftChanges, WorkspaceRuntimeServices } from './persistence/WorkspaceRuntimeServices' import { useBootstrap } from '@renderer/workspace/hook/persistence/useBootstrap' import type { WorkspaceRestoreStatus } from '@renderer/workspace/hook/persistence/useBootstrap' import { useFeedDebugPersist } from '@renderer/workspace/hook/persistence/useFeedDebugPersist' -import { useCodexTranscriptObservationOutbox } from '@renderer/lifecycle/codexTranscriptObservationOutbox' import { - usePickerSanity, usePinnedSessionIdsSanity, useReaderModeSanity, useSpotlightSanity, @@ -97,7 +95,10 @@ export function useWorkspace( const state = useAppStore(store => store.workspaceState) const setState = useAppStore(store => store.setWorkspaceState) - const runtimes = useAppStore(store => store.workspaceRuntimes) + // Runtime rendering is owned by session subscribers, not the composition + // root. The snapshot seeds refs only; an imperative subscription below keeps + // callbacks current even when no layout render is scheduled. + const runtimes = useAppStore.getState().workspaceRuntimes const setRuntimes = useAppStore(store => store.setWorkspaceRuntimes) const spotlight = useAppStore(store => store.workspaceSpotlight) const setSpotlight = useAppStore(store => store.setWorkspaceSpotlight) @@ -120,14 +121,20 @@ export function useWorkspace( refs.stateRef.current = state refs.latestStateRef.current = state refs.latestRuntimesRef.current = runtimes + useLayoutEffect(() => { + refs.latestRuntimesRef.current = useAppStore.getState().workspaceRuntimes + return useAppStore.subscribe(store => store.workspaceRuntimes, next => { + refs.latestRuntimesRef.current = next + }) + }, [refs]) refs.latestTileTabsRef.current = tileTabs refs.dangerousAgentsRef.current = dangerousAgentsEnabled refs.useProxyStreamingRef.current = useProxyStreaming refs.defaultBuiltInMcpDomainsRef.current = defaultBuiltInMcpDomains - // ---- Draft version counter (React state because the save effect - // reads it as a dep) ---- - const [draftVersion, setDraftVersion] = useState(0) + // Draft changes invalidate the autosave service, not the whole controller. + // The service observes this stable signal even when App does not rerender. + const draftChanges = useMemo(createDraftChanges, []) const [bootstrapComplete, setBootstrapComplete] = useState(false) // Surfaces the bootstrap outcome to the UI so it can render a banner // when the workspace is in a partial-restore / persisted-fallback @@ -247,7 +254,7 @@ export function useWorkspace( releaseAllRenderedViewLeases, scrollFocusedToLatest, } = - useWorkspaceHelpers(runtimes, setRuntimes, refs) + useWorkspaceHelpers(setRuntimes, refs) // ---- Pane toast (needs updateRuntime, so after helpers) ---- const showPaneToast = usePaneToast(refs.paneToastTimers, updateRuntime) @@ -260,7 +267,7 @@ export function useWorkspace( const { setDraftInput, setDraftImages, clearDraft, undoClearDraft } = useDraftActions( setRuntimes, updateRuntime, - setDraftVersion, + draftChanges.bump, ) const { setStreamingBaseline, @@ -862,9 +869,7 @@ export function useWorkspace( // see the WHY on useIpcSubscriptions. const sessionFeed = useSessionFeed() useIpcSubscriptions(sessionFeed, refs, setState, setRuntimes, updateRuntime, appendFeedDebug) - useCodexTranscriptObservationOutbox(runtimes) useWorkspaceAdoption(refs, setState, setRuntimes, bootstrapComplete) - useAutoSave(state, draftVersion, refs, bootstrapComplete) useBootstrap( refs, setState, @@ -876,10 +881,12 @@ export function useWorkspace( defaultWorkspaceMode, dispatchActions.enterDispatchMode, ) - useFeedDebugPersist(runtimes, refs) + // The persist effect reads current refs on its own timer, so it needs no + // render-time snapshot — passing `runtimes` here would suggest a reactivity + // dependency that deliberately does not exist. + useFeedDebugPersist(refs) useSpotlightSanity(spotlight, state, setSpotlight) useReaderModeSanity(readerMode, state, setReaderMode) - usePickerSanity(runtimes, pickerCancel) useTileTabsSanity(tileTabs, state.tabs, setTileTabs) usePinnedSessionIdsSanity(state, setState) @@ -896,7 +903,12 @@ export function useWorkspace( // ---- Return the stable Workspace shape ---- return { state, - runtimes, + // Imperative commands/debug capture read the latest committed store. + // Renderers must subscribe through useSessionRuntime/useWorkspaceContext. + get runtimes() { return refs.latestRuntimesRef.current }, + runtimeServices: createElement(WorkspaceRuntimeServices, { + state, refs, bootstrapComplete, draftChanges, pickerCancel, + }), activeTab, spotlight, tileTabs, diff --git a/src/renderer/src/workspace/hook/persistence/WorkspaceRuntimeServices.tsx b/src/renderer/src/workspace/hook/persistence/WorkspaceRuntimeServices.tsx new file mode 100644 index 00000000..d59ada5b --- /dev/null +++ b/src/renderer/src/workspace/hook/persistence/WorkspaceRuntimeServices.tsx @@ -0,0 +1,55 @@ +import { memo, useMemo, useSyncExternalStore } from 'react' +import { useShallow } from 'zustand/react/shallow' +import { useAppStore } from '@renderer/app-state/hooks' +import { useCodexTranscriptObservationOutbox } from '@renderer/lifecycle/codexTranscriptObservationOutbox' +import { usePickerSanity } from '@renderer/workspace/hook/invalidation/effects' +import type { WorkspaceRefs } from '@renderer/workspace/hook/refs' +import type { WorkspaceState } from '@renderer/workspace/types' +import { useAutoSave } from './useAutoSave' + +/** Autosave invalidation belongs to the persistence service, not App's React + * state. Typing should update one composer and its save deadline, not execute + * the entire workspace controller just to increment an invisible counter. */ +export function createDraftChanges() { + let version = 0 + const listeners = new Set<() => void>() + return { + getSnapshot: () => version, + subscribe: (listener: () => void) => { + listeners.add(listener) + return () => { listeners.delete(listener) } + }, + bump: () => { + version++ + for (const listener of listeners) listener() + }, + } +} + +const SessionRuntimeObserver = memo(function SessionRuntimeObserver({ sessionId, pickerCancel }: { + sessionId: string + pickerCancel: (sessionId: string) => void +}) { + const runtime = useAppStore(state => state.workspaceRuntimes[sessionId]) + const snapshot = useMemo(() => runtime ? { [sessionId]: runtime } : {}, [runtime, sessionId]) + // Keep mutation-before-visible-surface chronology: the outbox still flushes + // in a layout effect. A one-second timer would change forensic ordering. + useCodexTranscriptObservationOutbox(snapshot) + usePickerSanity(snapshot, pickerCancel) + return null +}) + +export function WorkspaceRuntimeServices({ state, refs, bootstrapComplete, draftChanges, pickerCancel }: { + state: WorkspaceState + refs: WorkspaceRefs + bootstrapComplete: boolean + draftChanges: ReturnType + pickerCancel: (sessionId: string) => void +}) { + const ids = useAppStore(useShallow(store => Object.keys(store.workspaceRuntimes))) + const draftVersion = useSyncExternalStore(draftChanges.subscribe, draftChanges.getSnapshot) + useAutoSave(state, draftVersion, refs, bootstrapComplete) + return <>{ids.map(sessionId => + , + )} +} diff --git a/src/renderer/src/workspace/hook/persistence/useFeedDebugPersist.renderer.test.tsx b/src/renderer/src/workspace/hook/persistence/useFeedDebugPersist.renderer.test.tsx index 675f7e2b..b4b13aa6 100644 --- a/src/renderer/src/workspace/hook/persistence/useFeedDebugPersist.renderer.test.tsx +++ b/src/renderer/src/workspace/hook/persistence/useFeedDebugPersist.renderer.test.tsx @@ -53,15 +53,15 @@ describe('feed debug persistence cadence and durability', () => { it('coalesces continuously replaced runtimes into one ordered batch on the fixed tick', async () => { const refs = makeRefs({ a: emptyRuntime() }) const { rerender } = renderHook( - ({ runtimes }) => useFeedDebugPersist(runtimes, refs), - { initialProps: { runtimes: refs.latestRuntimesRef.current } }, + () => useFeedDebugPersist(refs), + { initialProps: {} }, ) // Continuous provider traffic must neither flush on every React effect nor // postpone the timer indefinitely as a trailing debounce would. for (let index = 0; index < 20; index += 1) { refs.latestRuntimesRef.current = { a: add(refs.latestRuntimesRef.current.a!, `row ${index}`) } - rerender({ runtimes: refs.latestRuntimesRef.current }) + rerender() await advance(40) } await advance(199) @@ -77,7 +77,7 @@ describe('feed debug persistence cadence and durability', () => { const pending = deferred() append.mockImplementation(({ sessionId }) => sessionId === 'a' ? pending.promise : Promise.resolve()) const refs = makeRefs({ a: add(emptyRuntime(), 'a1'), b: add(emptyRuntime(), 'b1') }) - renderHook(() => useFeedDebugPersist(refs.latestRuntimesRef.current, refs)) + renderHook(() => useFeedDebugPersist(refs)) await advance(1000) expect(append).toHaveBeenCalledTimes(2) refs.latestRuntimesRef.current = { @@ -109,7 +109,7 @@ describe('feed debug persistence cadence and durability', () => { append.mockReturnValueOnce(pending.promise) const refs = makeRefs({ a: add(add(emptyRuntime(), 'already durable'), 'pending') }) refs.persistedFeedDebugIdRef.current.a = 1 - renderHook(() => useFeedDebugPersist(refs.latestRuntimesRef.current, refs)) + renderHook(() => useFeedDebugPersist(refs)) await advance(1000) expect(refs.inFlightFeedDebugIdRef.current.a).toBe(2) await act(async () => { pending.reject(new Error('disk unavailable')); await Promise.resolve() }) @@ -128,7 +128,7 @@ describe('feed debug persistence cadence and durability', () => { it('leaves empty and already durable sessions quiet', async () => { const refs = makeRefs({ empty: emptyRuntime(), durable: add(emptyRuntime(), 'saved') }) refs.persistedFeedDebugIdRef.current.durable = 1 - const { unmount } = renderHook(() => useFeedDebugPersist(refs.latestRuntimesRef.current, refs)) + const { unmount } = renderHook(() => useFeedDebugPersist(refs)) await advance(3000) unmount() expect(append).not.toHaveBeenCalled() @@ -136,7 +136,7 @@ describe('feed debug persistence cadence and durability', () => { it('flushes the latest refs once on unmount and removes the interval', async () => { const refs = makeRefs({ a: emptyRuntime() }) - const { unmount } = renderHook(() => useFeedDebugPersist(refs.latestRuntimesRef.current, refs)) + const { unmount } = renderHook(() => useFeedDebugPersist(refs)) refs.latestRuntimesRef.current = { a: add(emptyRuntime(), 'last record') } unmount() expect(append).toHaveBeenCalledExactlyOnceWith({ @@ -153,7 +153,7 @@ describe('feed debug persistence cadence and durability', () => { const pending = deferred() append.mockReturnValueOnce(pending.promise) const refs = makeRefs({ a: add(emptyRuntime(), 'first') }) - const { unmount } = renderHook(() => useFeedDebugPersist(refs.latestRuntimesRef.current, refs)) + const { unmount } = renderHook(() => useFeedDebugPersist(refs)) await advance(1000) refs.latestRuntimesRef.current = { a: add(refs.latestRuntimesRef.current.a!, 'later') } unmount() diff --git a/src/renderer/src/workspace/hook/persistence/useFeedDebugPersist.ts b/src/renderer/src/workspace/hook/persistence/useFeedDebugPersist.ts index d035158b..fa4496c6 100644 --- a/src/renderer/src/workspace/hook/persistence/useFeedDebugPersist.ts +++ b/src/renderer/src/workspace/hook/persistence/useFeedDebugPersist.ts @@ -42,10 +42,7 @@ export function selectFeedDebugAppendBatch( } } -export function useFeedDebugPersist( - _runtimes: Record, - refs: WorkspaceRefs, -): void { +export function useFeedDebugPersist(refs: WorkspaceRefs): void { useEffect(() => { const flushSession = (sessionId: SessionId, runtime: SessionRuntime): void => { if (runtime.feedDebugLog.length === 0) return diff --git a/src/renderer/src/workspace/hook/refs.ts b/src/renderer/src/workspace/hook/refs.ts index e5aa806e..a782dff5 100644 --- a/src/renderer/src/workspace/hook/refs.ts +++ b/src/renderer/src/workspace/hook/refs.ts @@ -19,10 +19,11 @@ import type { ConfigurableBuiltInMcpDomain } from '@mcp/shared/types' // need 15 separate useRef lines. Ref identity is stable across renders // (useRef contract), so putting them together doesn't cost anything. // -// NOTE: this hook updates `stateRef.current = state` style mirrors in -// the caller's render body (not here) — the mirrors are per-render -// writes, not per-action. That's what lets IPC callbacks close over -// stale React state and still read the live value via .current. +// Layout/settings mirrors are refreshed in the caller's render body. Runtime +// state additionally has a synchronous store subscription there: runtime-only +// updates no longer render the controller, but IPC/actions must see them before +// React commits any subscribed pane. Keep that subscription and its cleanup +// coupled to this identity-stable ref bundle. // ----------------------------------------------------------------------------- export type WorkspaceRefs = { diff --git a/src/renderer/src/workspace/hook/runtimeIsolation.renderer.test.tsx b/src/renderer/src/workspace/hook/runtimeIsolation.renderer.test.tsx new file mode 100644 index 00000000..1ba70ae6 --- /dev/null +++ b/src/renderer/src/workspace/hook/runtimeIsolation.renderer.test.tsx @@ -0,0 +1,164 @@ +import { act, cleanup, fireEvent, render } from '@testing-library/react' +import { useEffect } from 'react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { useAppStore } from '@renderer/app-state/hooks' +import { emptyRuntime, type SessionRuntime } from '@renderer/session-runtime/state' +import { TileTree } from '@renderer/workspace/tile-tree/TileTree' +import type { TileNode, WorkspaceState } from '@renderer/workspace/types' +import { useWorkspace } from './index' +import { useRenderedLeaseHygiene } from './effects/useRenderedLeaseHygiene' +import { appendCodexTranscriptObservation } from '@renderer/lifecycle/codexTranscriptObservationOutbox' + +const counts = vi.hoisted(() => ({ controller: 0, panes: {} as Record, chronology: [] as string[] })) +vi.mock('@providers/registry.renderer', () => ({ + getRendererProvider: () => ({ TileLeaf: ({ sessionId, runtime, onFocusRequest }: { + sessionId: string; runtime: SessionRuntime; onFocusRequest: () => void + }) => { + counts.panes[sessionId] = (counts.panes[sessionId] ?? 0) + 1 + useEffect(() => { counts.chronology.push(`visible:${sessionId}`) }, [sessionId, runtime]) + return + } }), +})) +// No provider/IPC process is started by this test. The controller, store, +// helpers, draft actions, autosave and real subscribed tile boundaries remain +// mounted; only boot/event ingress and the expensive provider paint are faked. +vi.mock('./ipc/useIpcSubscriptions', () => ({ useIpcSubscriptions: () => undefined })) +vi.mock('./ipc/useWorkspaceAdoption', () => ({ useWorkspaceAdoption: () => undefined })) +vi.mock('@renderer/features/sessionFeed/SessionFeedContext', () => ({ useSessionFeed: () => ({}) })) +vi.mock('./persistence/useBootstrap', async () => { + const { useEffect } = await import('react') + return { useBootstrap: (...args: Parameters) => { + useEffect(() => args[5](true), [args[5]]) + } } +}) + +const original = useAppStore.getState() +const originalApi = window.api +let current!: ReturnType +const saveWorkspace = vi.fn(async (_json: string) => undefined) +const reportSessionLifecycle = vi.fn(() => { counts.chronology.push('mutation') }) +function Controller({ legacy = false }: { legacy?: boolean }) { + // Control arm recreates the old root subscription while leaving everything + // else identical. Counts compare real committed renders, not a synthetic + // selector microbenchmark or guessed CPU savings. + useAppStore(state => legacy ? state.workspaceRuntimes : null) + current = useWorkspace(false) + useRenderedLeaseHygiene(current) + counts.controller += 1 + return <>{current.runtimeServices} +} + +beforeEach(() => { + vi.useFakeTimers() + counts.controller = 0 + counts.panes = {} + counts.chronology = [] + saveWorkspace.mockClear() + reportSessionLifecycle.mockClear() + const root: TileNode = { type: 'split', direction: 'vertical', ratio: 0.5, + a: { type: 'leaf', sessionId: 'one' }, b: { type: 'leaf', sessionId: 'two' } } + const state: WorkspaceState = { ...original.workspaceState, + tabs: [{ id: 'tab', title: 'Test', focusedSessionId: 'one', root }], activeTabId: 'tab', + sessions: { one: { kind: 'claude', cwd: '/repo' }, two: { kind: 'claude', cwd: '/repo' } }, + } + useAppStore.setState({ workspaceState: state, workspaceRuntimes: { one: emptyRuntime(), two: emptyRuntime() } }) + Object.defineProperty(window, 'api', { configurable: true, value: { + onOrchestrationRequest: () => () => undefined, + onAgentManagementRequest: () => () => undefined, + saveWorkspace, + reportSessionLifecycle, + appendFeedDebugLog: async () => undefined, + } }) +}) +afterEach(() => { + cleanup() + vi.useRealTimers() + useAppStore.setState(original, true) + Object.defineProperty(window, 'api', { configurable: true, value: originalApi }) +}) + +describe('runtime updates below the workspace controller', () => { + it.each([false, true])('isolates unrelated panes; legacy control=%s', legacy => { + const view = render() + const before = { controller: counts.controller, one: counts.panes.one, two: counts.panes.two } + for (let index = 0; index < 100; index += 1) { + act(() => current.updateRuntime('one', { draftInput: `frame-${index}` })) + } + expect(counts.controller - before.controller).toBe(legacy ? 100 : 0) + expect(counts.panes.two - before.two).toBe(legacy ? 100 : 0) + expect(counts.panes.one - before.one).toBe(100) + expect(view.getByTestId('one')).toHaveTextContent('frame-99') + expect(current.getRuntime('one').draftInput).toBe('frame-99') + }) + + it('reads fresh drafts synchronously and saves them without rerendering the controller', async () => { + render() + const before = counts.controller + const getRuntime = current.getRuntime + act(() => { + current.setDraftInput('one', 'unsent text') + // Must work before a React commit, not just after act settles. + expect(getRuntime('one').draftInput).toBe('unsent text') + }) + expect(counts.controller).toBe(before) + await act(async () => { await vi.advanceTimersByTimeAsync(401) }) + const saved = JSON.parse(saveWorkspace.mock.calls.at(-1)![0]) + expect(saved.workspace.drafts.one).toBe('unsent text') + act(() => current.clearDraft('one')) + expect(getRuntime('one').draftInput).toBe('') + act(() => current.undoClearDraft('one')) + expect(getRuntime('one').draftInput).toBe('unsent text') + expect(counts.controller).toBe(before) + }) + + it('keeps layout actions fresh and moves subscriptions when a leaf changes session', () => { + const view = render() + fireEvent.click(view.getByTestId('two')) + expect(current.activeTab?.focusedSessionId).toBe('two') + act(() => { + const store = useAppStore.getState() + store.setWorkspaceRuntimes(prev => ({ ...prev, three: emptyRuntime() })) + store.setWorkspaceState(prev => ({ ...prev, + sessions: { ...prev.sessions, three: { kind: 'claude', cwd: '/repo' } }, + tabs: prev.tabs.map(tab => ({ ...tab, root: { type: 'leaf', sessionId: 'three' } })), + })) + }) + expect(view.queryByTestId('one')).toBeNull() + const before = counts.panes.three + act(() => current.updateRuntime('one', { draftInput: 'old pane output' })) + expect(counts.panes.three).toBe(before) + act(() => current.setDraftInput('three', 'new pane draft')) + expect(view.getByTestId('three')).toHaveTextContent('new pane draft') + fireEvent.click(view.getByTestId('three')) + expect(current.activeTab?.focusedSessionId).toBe('three') + }) + + it('still clears a rendered-view lease acquired after terminal mode hid the feed', () => { + useAppStore.setState({ settings: { ...original.settings, agentViewMode: 'terminal' } }) + render() + act(() => current.acquireRenderedViewLease('one', 'copy-assistant-message')) + expect(current.getRuntime('one').renderedViewLeases).toEqual({}) + }) + + it('flushes committed lifecycle observations before passive visibility without duplicating them', () => { + render() + const retiredRun = '11111111-1111-4111-8111-111111111111' + const successorRun = '22222222-2222-4222-8222-222222222222' + act(() => current.updateRuntime('one', { sessionRunId: retiredRun })) + counts.chronology = [] + act(() => { + useAppStore.getState().setWorkspaceRuntimes(prev => ({ ...prev, one: { + ...appendCodexTranscriptObservation(prev.one, 'submit.release', { cause: 'session-exit' }), + sessionRunId: successorRun, + } })) + }) + expect(counts.chronology).toEqual(['mutation', 'visible:one']) + expect(reportSessionLifecycle).toHaveBeenCalledWith(expect.objectContaining({ + correlationIds: expect.objectContaining({ sessionRunId: retiredRun }), + })) + act(() => current.updateRuntime('two', { draftInput: 'unrelated' })) + act(() => current.updateRuntime('one', { draftInput: 'later output' })) + expect(reportSessionLifecycle).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/renderer/src/workspace/tile-tree/TabBar.tsx b/src/renderer/src/workspace/tile-tree/TabBar.tsx index 45c96efc..a9d8ab0c 100644 --- a/src/renderer/src/workspace/tile-tree/TabBar.tsx +++ b/src/renderer/src/workspace/tile-tree/TabBar.tsx @@ -1,4 +1,6 @@ -import { useEffect, useState } from 'react' +import { useEffect, useMemo, useState } from 'react' +import { useAppStore } from '@renderer/app-state/hooks' +import { useShallow } from 'zustand/react/shallow' import { resolveTabSessions } from '@renderer/workspace/queries' import type { Workspace } from '@renderer/workspace/workspaceStore' @@ -21,7 +23,12 @@ type Props = { } export function TabBar({ workspace, onNewTabRequest }: Props) { - const { state, runtimes, activateTab, closeTab } = workspace + const { state, activateTab, closeTab } = workspace + // Only the running flags affect tab counts. Text, spinner, draft and debug + // mutations must not render all tab buttons merely because their map changed. + const runningIds = useAppStore(useShallow(store => Object.keys(store.workspaceRuntimes) + .filter(id => store.workspaceRuntimes[id]?.sessionStatus === 'running'))) + const running = useMemo(() => new Set(runningIds), [runningIds]) // Dynamic traffic light inset from main process. Updated on // resize / zoom / display change. 70 is the fallback for the @@ -64,10 +71,7 @@ export function TabBar({ workspace, onNewTabRequest }: Props) { // runtimes. Pure derivation — no extra state needed. const sessionIds = resolveTabSessions(state, tab.id) const total = sessionIds.length - const alive = sessionIds.filter(id => { - const rt = runtimes[id] - return rt?.sessionStatus === 'running' - }).length + const alive = sessionIds.filter(id => running.has(id)).length const allDone = alive === 0 return ( diff --git a/src/renderer/src/workspace/tile-tree/TileLeaf.tsx b/src/renderer/src/workspace/tile-tree/TileLeaf.tsx index 7176c6b4..c2a57cde 100644 --- a/src/renderer/src/workspace/tile-tree/TileLeaf.tsx +++ b/src/renderer/src/workspace/tile-tree/TileLeaf.tsx @@ -810,7 +810,6 @@ export function TileLeaf({ isSessionLive={isSessionLive} relatedAgentTabs={relatedAgentTabs} selectedRelatedSessionId={selectedRelatedSessionId ?? sessionId} - runtimes={workspace.runtimes} ownerSessionId={ownerSessionId ?? sessionId} onSelectRelatedSession={onSelectRelatedSession} /> diff --git a/src/renderer/src/workspace/tile-tree/TileLeaf/PaneHeader.phoneCoupling.renderer.test.tsx b/src/renderer/src/workspace/tile-tree/TileLeaf/PaneHeader.phoneCoupling.renderer.test.tsx new file mode 100644 index 00000000..52f3b185 --- /dev/null +++ b/src/renderer/src/workspace/tile-tree/TileLeaf/PaneHeader.phoneCoupling.renderer.test.tsx @@ -0,0 +1,79 @@ +import { cleanup, render } from '@testing-library/react' +import { afterEach, describe, expect, it } from 'vitest' + +import { useAppStore } from '@renderer/app-state/store' +import { emptyRuntime } from '@renderer/session-runtime/state' +import type { GridRelatedAgentTab } from '@renderer/workspace/gridRelatedAgents' + +import { PaneHeader } from './PaneHeader' + +const original = useAppStore.getState() +afterEach(() => { cleanup(); useAppStore.setState(original, true) }) + +const related: GridRelatedAgentTab[] = [{ + sessionId: 'related-session', + relation: 'orchestration', + label: 'assistant', + title: 'assistant', + kind: 'claude', + placement: 'grid', +}] + +const running = (): ReturnType => + ({ ...emptyRuntime(), sessionStatus: 'running' }) + +describe('PaneHeader related-status store coupling', () => { + it('renders with no related chips without touching workspaceRuntimes (today\'s phone call shape)', () => { + const { container } = render( + , + ) + // The phone (src/remote-client) shares this component but stubs the app + // store to `{ settings }` only and always passes an empty chip list. The + // related row must not mount — and this must never throw on the missing + // `workspaceRuntimes` key. + expect(container.querySelector('button')).toBeNull() + }) + + it('survives a store with no workspaceRuntimes key when chips are non-empty (phone stub shape)', () => { + // The real renderer store always has the key, so reproducing the phone + // stub requires temporarily removing it. This pins the fallback contract: + // a keyless store must degrade to the `runtimes` prop instead of throwing. + useAppStore.setState({ workspaceRuntimes: undefined as never }) + const { container } = render( + undefined} + />, + ) + const chip = container.querySelector('button') + expect(chip).not.toBeNull() + expect(chip!.querySelector('.bg-accent')).not.toBeNull() + }) + + it('derives related status from the store when the key exists (desktop path)', () => { + useAppStore.setState({ workspaceRuntimes: { 'related-session': running() } }) + const { container } = render( + , + ) + const chip = container.querySelector('button') + expect(chip).not.toBeNull() + expect(chip!.querySelector('.bg-accent')).not.toBeNull() + }) +}) \ No newline at end of file diff --git a/src/renderer/src/workspace/tile-tree/TileLeaf/PaneHeader.tsx b/src/renderer/src/workspace/tile-tree/TileLeaf/PaneHeader.tsx index 42b56b45..8ddf3eae 100644 --- a/src/renderer/src/workspace/tile-tree/TileLeaf/PaneHeader.tsx +++ b/src/renderer/src/workspace/tile-tree/TileLeaf/PaneHeader.tsx @@ -1,4 +1,6 @@ import { shortenCwd } from '@renderer/workspace/tile-tree/TileLeaf/labels' +import { useAppStore } from '@renderer/app-state/hooks' +import { useShallow } from 'zustand/react/shallow' import { PaneHeaderColorFlag } from '@renderer/workspace/tile-tree/TileLeaf/PaneHeaderColorFlag' import type { GridRelatedAgentTab } from '@renderer/workspace/gridRelatedAgents' import { dispatchAttentionLabelFromConditions } from '@renderer/workspace/conditions/selectors' @@ -54,6 +56,25 @@ export function PaneHeader({ ownerSessionId?: string onSelectRelatedSession?: (sessionId: string) => void }) { + // Related agents can change without rerendering this session. Only the two + // painted status values are dependencies; subscribing to their entire + // runtimes would couple every related transcript delta back to this header. + // + // WHY the store read is optional-chained instead of a bare index: the phone + // shares this header, and the phone bundle stubs @renderer/app-state/hooks + // to a `{ settings }`-only store + // (src/remote-client/src/stubs/appStateHooks.ts) that has NO + // `workspaceRuntimes` key. SessionView passes `relatedAgentTabs={[]}`, so + // today the flatMap body never runs and the key is never touched; the `?.` + // keeps a hypothetical future phone caller that passes chips from throwing + // on the missing key, degrading to the `runtimes` prop and then to + // "unknown" instead. Un-optional-chained, this whole header is sound on the + // phone only by the empty-array accident of one call site. + const relatedStatus = useAppStore(useShallow(state => relatedAgentTabs.flatMap(tab => { + const runtime = state.workspaceRuntimes?.[tab.sessionId] ?? runtimes?.[tab.sessionId] + return [runtime?.sessionStatus === 'running', + dispatchAttentionLabelFromConditions(runtime?.conditions ?? null) ?? (runtime?.processError ? 'ERROR' : null)] + }))) return (
{relatedAgentTabs.length > 0 && (
- {relatedAgentTabs.map(tab => { + {relatedAgentTabs.map((tab, index) => { const active = tab.sessionId === selectedRelatedSessionId - const runtime = runtimes?.[tab.sessionId] - const running = runtime?.sessionStatus === 'running' - const attention = dispatchAttentionLabelFromConditions(runtime?.conditions ?? null) - ?? (runtime?.processError ? 'ERROR' : null) + const running = relatedStatus[index * 2] + const attention = relatedStatus[index * 2 + 1] const title = `${tab.relation}: ${tab.title}${tab.placement === 'detached' ? ' (detached)' : ''}` return (