diff --git a/config/reliability-gates.jsonc b/config/reliability-gates.jsonc index f1a92f59391..02a4e7ad79a 100644 --- a/config/reliability-gates.jsonc +++ b/config/reliability-gates.jsonc @@ -4829,7 +4829,7 @@ "https://github.com/stablyai/orca/pull/10193", "https://github.com/stablyai/orca/issues/16102" ], - "invariant": "For every claim-capable execution route, one provider-session identity has at most one live PTY owner and one canonical host surface across concurrent clients, retries, reconnects, and stale publications. For paired structured fresh and resume requests, the authenticated owning runtime creates in background without a renderer window; activate=true focuses the exact requested leaf only on the requesting viewer, while activate=false changes no viewer focus. A live orphan may be adopted only when the controller proves its exact handle and incarnation, its worktree and host owner match, no competing visual owner exists, and a host topology CAS wins. A viewer may classify completion only from successful host/provider inspection or explicit lifecycle evidence; transport, handle, or provider unavailability remains unknown and breaks any consecutive-idle proof. A visibility resubscription announces each newly accepted remote done/blocked/waiting transition once, while cold inventory and unchanged reconnect replay remain silent. A physical exit retires that exact incarnation durably so stale client state and host restart cannot recreate it. Mixed-version routes select the unchanged legacy request before any authority side effect or execution-owner-local filesystem access.", + "invariant": "For every claim-capable execution route, one provider-session identity has at most one live PTY owner and one canonical host surface across concurrent clients, retries, reconnects, and stale publications. For paired structured fresh and resume requests, the authenticated owning runtime creates in background without a renderer window; activate=true focuses the exact requested leaf only on the requesting viewer, while activate=false changes no viewer focus. A live orphan may be adopted only when the controller proves its exact handle and incarnation, its worktree and host owner match, no competing visual owner exists, and a host topology CAS wins. A viewer may classify completion only from successful host/provider inspection or explicit lifecycle evidence; transport, handle, or provider unavailability remains unknown and breaks any consecutive-idle proof. A visibility resubscription announces each newly accepted remote done/blocked/waiting transition once, including a worktree first published during a warm resume, while cold inventory, unmirrored workspaces, locally closed panes, replacement owners, and unchanged reconnect replay remain silent. Recovered attention remains unread until user interaction even when an older worktree or folder-workspace catalog response arrives after the persistence acknowledgement. A physical exit retires that exact incarnation durably so stale client state and host restart cannot recreate it. Mixed-version routes select the unchanged legacy request before any authority side effect or execution-owner-local filesystem access.", "oracle": "Race independent clients and repeated operation IDs, then assert one physical spawn and one canonical PTY/surface; inject exit-before-reply, provider disconnect, conflicting claim scope, old daemon/relay capabilities, reused handles, stale incarnations, owner mismatch, and topology revision conflict; assert safe adoption or explicit failure without a second spawn or wrong-process attachment. Run fresh/resume with activate true/false against an isolated headed desktop host and a separate paired client, then against isolated headless serve: assert host presentation stays background, only the requesting viewer focuses the exact leaf, inactive calls preserve client/DOM focus, a same-version publication replay cannot lose focus intent, and sibling-first split publication cannot consume exact-leaf intent. Restore legacy split panes and groups beside a newer host-owned tab, preserving exact predecessor/new/successor order, output, input, resize, titles, tab/leaf identity, active group, and multi-client convergence. For completion, drive a known running agent through outer transport loss, authoritative provider rejection, reconnect, explicit stop, real exit status, and successful hook completion; assert unavailable evidence never dispatches completion and two fresh authoritative idle samples are required after the gap. After exact exit, assert terminal and tab listings omit the surface, a stale publication cannot restore it, restart cannot resurrect it, exact tokened fixture PIDs are dead, and unrelated tabs/processes survive until scoped cleanup.", "commands": [ "pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/rpc/methods/agent-session.test.ts src/renderer/src/runtime/web-runtime-session.test.ts src/renderer/src/runtime/web-session-tabs-sync.test.ts src/renderer/src/runtime/web-session-intent-owner.test.ts src/renderer/src/runtime/remote-server-parity.test.ts", @@ -4845,7 +4845,8 @@ "pnpm exec playwright test tests/e2e/paired-remote-agent-visibility-notifications.spec.ts --config tests/playwright.config.ts --project electron-headless --workers=1", "pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/orca-runtime.test.ts src/main/runtime/terminal-orphan-owner.test.ts src/main/runtime/terminal-orphan-topology.test.ts src/renderer/src/runtime/web-session-terminal-orphan-recovery.test.ts src/renderer/src/runtime/web-session-terminal-orphan-mixed-version.test.ts src/renderer/src/runtime/web-session-tabs-sync.test.ts --maxWorkers=1", "pnpm exec vitest run --config config/vitest.config.ts src/main/providers/pty-process-inspection.test.ts src/main/daemon/terminal-host.test.ts src/main/daemon/daemon-pty-router.test.ts src/main/daemon/degraded-daemon-pty-provider.test.ts src/relay/pty-handler-spawn-admission.test.ts src/main/daemon/daemon-pty-adapter.test.ts src/main/runtime/orca-runtime.test.ts tests/e2e/remote-agent-completion-authority.unit.test.ts src/renderer/src/runtime/runtime-terminal-inspection.test.ts src/renderer/src/components/terminal-pane/agent-completion-coordinator-process-cadence.test.ts src/renderer/src/components/terminal-pane/agent-completion-coordinator-pending-title-inspection.test.ts", - "pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/runtime/web-session-tabs-visibility-resume-notifications.test.tsx src/renderer/src/runtime/web-session-tabs-notification-receipt-order.test.tsx src/renderer/src/runtime/web-session-tabs-notification-reconciler.test.ts src/renderer/src/hooks/agent-hook-completion-notifications-inventory.test.ts src/renderer/src/hooks/useAutoAckViewedAgent.attention.test.tsx src/renderer/src/components/terminal-pane/use-notification-dispatch.test.ts src/main/ipc/notifications-delivery-gating.test.ts", + "pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/runtime/web-session-tabs-visibility-resume-notifications.test.tsx src/renderer/src/runtime/web-session-tabs-notification-receipt-order.test.tsx src/renderer/src/runtime/web-session-tabs-notification-reconciler.test.ts src/renderer/src/runtime/web-session-tabs-new-pane-notification.test.ts src/renderer/src/runtime/web-session-tabs-agent-completion-notifications.test.ts src/renderer/src/hooks/agent-hook-completion-notifications-inventory.test.ts src/renderer/src/hooks/useAutoAckViewedAgent.attention.test.tsx src/renderer/src/components/terminal-pane/use-notification-dispatch.test.ts src/main/ipc/notifications-delivery-gating.test.ts", + "pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/store/slices/worktrees/metadata/passive-worktree-meta-mutation.test.ts src/renderer/src/store/slices/worktrees/metadata/worktree-meta-persist.test.ts src/renderer/src/store/slices/worktrees-fetch-listing-merge.test.ts src/renderer/src/store/slices/folder-workspace-owner-routed-mutations.test.ts", "pnpm exec vitest run --config config/vitest.config.ts tests/e2e/remote-agent-completion-authority.unit.test.ts src/main/providers/pty-process-inspection.test.ts src/main/daemon/terminal-host.test.ts src/main/daemon/daemon-pty-router.test.ts src/main/daemon/degraded-daemon-pty-provider.test.ts src/relay/pty-handler-spawn-admission.test.ts src/main/daemon/daemon-pty-adapter.test.ts src/renderer/src/runtime/runtime-terminal-inspection.test.ts src/renderer/src/components/terminal-pane/agent-completion-coordinator-process-cadence.test.ts src/renderer/src/components/terminal-pane/agent-completion-coordinator-pending-title-inspection.test.ts src/renderer/src/components/terminal-pane/pty-connection-task-complete-dispatch.test.ts src/renderer/src/lib/codex-session-restart.test.ts" ], "testFiles": [ @@ -4860,10 +4861,16 @@ "src/renderer/src/runtime/web-session-tabs-visibility-resume-notifications.test.tsx", "src/renderer/src/runtime/web-session-tabs-notification-receipt-order.test.tsx", "src/renderer/src/runtime/web-session-tabs-notification-reconciler.test.ts", + "src/renderer/src/runtime/web-session-tabs-new-pane-notification.test.ts", + "src/renderer/src/runtime/web-session-tabs-agent-completion-notifications.test.ts", "src/renderer/src/hooks/agent-hook-completion-notifications-inventory.test.ts", "src/renderer/src/hooks/useAutoAckViewedAgent.attention.test.tsx", "src/renderer/src/components/terminal-pane/use-notification-dispatch.test.ts", "src/main/ipc/notifications-delivery-gating.test.ts", + "src/renderer/src/store/slices/worktrees/metadata/passive-worktree-meta-mutation.test.ts", + "src/renderer/src/store/slices/worktrees/metadata/worktree-meta-persist.test.ts", + "src/renderer/src/store/slices/worktrees-fetch-listing-merge.test.ts", + "src/renderer/src/store/slices/folder-workspace-owner-routed-mutations.test.ts", "src/renderer/src/runtime/runtime-terminal-inspection.test.ts", "src/renderer/src/components/terminal-pane/agent-completion-coordinator-process-cadence.test.ts", "src/renderer/src/components/terminal-pane/agent-completion-coordinator-pending-title-inspection.test.ts", @@ -4942,7 +4949,21 @@ "file": "src/renderer/src/runtime/web-session-tabs-notification-receipt-order.test.tsx", "assertions": [ "notification order is independent of slow or failed terminal-orphan recovery", - "a failed cold recovery remains silent while a failed live transition still alerts" + "a failed cold recovery remains silent while a failed live transition still alerts", + "a failed global resume cannot clear hidden attention before the active stream reports" + ] + }, + { + "file": "src/renderer/src/runtime/web-session-tabs-new-pane-notification.test.ts", + "assertions": [ + "new panes and worktrees first published during a warm resume alert before store hydration", + "unmirrored floating workspaces and locally closed committed panes cannot dispatch" + ] + }, + { + "file": "src/renderer/src/runtime/web-session-tabs-agent-completion-notifications.test.ts", + "assertions": [ + "replacement pairings seed silently and unmirrored workspaces never enter notification derivation" ] }, { @@ -4957,6 +4978,19 @@ "attention recovered after hidden parking bypasses focus suppression exactly for that dispatch" ] }, + { + "file": "src/renderer/src/store/slices/worktrees-fetch-listing-merge.test.ts", + "assertions": [ + "a listing started during unread persistence cannot clear attention when it resolves after that persistence settles", + "a genuinely post-settlement listing remains authoritative" + ] + }, + { + "file": "src/renderer/src/store/slices/folder-workspace-owner-routed-mutations.test.ts", + "assertions": [ + "an older folder-workspace catalog row cannot overwrite a newer acknowledged unread revision" + ] + }, { "file": "src/renderer/src/hooks/agent-hook-completion-notifications-inventory.test.ts", "assertions": [ diff --git a/src/renderer/src/components/terminal-pane/agent-completion-coordinator-types.ts b/src/renderer/src/components/terminal-pane/agent-completion-coordinator-types.ts index 65d5e7bfa57..db09d68fe7c 100644 --- a/src/renderer/src/components/terminal-pane/agent-completion-coordinator-types.ts +++ b/src/renderer/src/components/terminal-pane/agent-completion-coordinator-types.ts @@ -2,6 +2,7 @@ import type { ParsedAgentStatusPayload } from '../../../../shared/agent-status-t import type { GlobalSettings } from '../../../../shared/global-settings-types' import type { RecognizedAgentProcess } from '../../../../shared/agent-process-recognition' import type { RuntimeTerminalProcessInspection } from '@/runtime/runtime-terminal-inspection' +import type { WebSessionTabsNotificationPaneEvidence } from '@/runtime/web-session-tabs-notification-reconciler' export type AgentCompletionStatusSnapshot = ParsedAgentStatusPayload & { stateStartedAt?: number @@ -9,6 +10,8 @@ export type AgentCompletionStatusSnapshot = ParsedAgentStatusPayload & { localStateStartedAt?: number /** The transition was recovered after hidden subscription parking. */ attentionRequired?: boolean + /** Renderer-local proof that the accepted remote frame still contains this pane. */ + remotePaneEvidence?: WebSessionTabsNotificationPaneEvidence } export type AgentCompletionDispatchMeta = { @@ -40,7 +43,7 @@ export type AgentCompletionCoordinatorOptions = { replacement: RecognizedAgentProcess ) => boolean shouldSuppressConfirmedProcessExitCompletion?: (exited: RecognizedAgentProcess) => boolean - isLive: () => boolean + isLive: (agentStatus?: AgentCompletionStatusSnapshot) => boolean shouldPollProcessCadence?: () => boolean // Why: on hosts where one inspection forks a whole-process-table scan (local // Windows PowerShell/CIM), panes without agent evidence relax to a slow diff --git a/src/renderer/src/components/terminal-pane/agent-completion-coordinator.ts b/src/renderer/src/components/terminal-pane/agent-completion-coordinator.ts index edb2d052b16..28214147420 100644 --- a/src/renderer/src/components/terminal-pane/agent-completion-coordinator.ts +++ b/src/renderer/src/components/terminal-pane/agent-completion-coordinator.ts @@ -425,7 +425,7 @@ export function createAgentCompletionCoordinator( if (requiresFreshWorking || lastCompletedTurn === currentTurn) { return false } - if (!options.isLive() || !hasAgentRunEvidence) { + if (!options.isLive(optionsOverride.agentStatus) || !hasAgentRunEvidence) { return false } const now = Date.now() @@ -475,6 +475,13 @@ export function createAgentCompletionCoordinator( quietedHookDone: false, agentStatus: optionsOverride.agentStatus }) + } else if (optionsOverride.agentStatus?.remotePaneEvidence) { + // Why: a raw remote frame may precede store hydration, so its pane proof must reach the dispatch gate. + options.dispatchCompletion(title, { + source, + quietedHookDone: false, + agentStatus: optionsOverride.agentStatus + }) } else { options.dispatchCompletion(title) } @@ -489,7 +496,7 @@ export function createAgentCompletionCoordinator( } function dispatchAttention(payload: AgentCompletionStatusSnapshot): void { - if (!options.dispatchAttention || !options.isLive() || !hasAgentRunEvidence) { + if (!options.dispatchAttention || !options.isLive(payload) || !hasAgentRunEvidence) { return } const token = hookAttentionToken(payload) @@ -504,7 +511,7 @@ export function createAgentCompletionCoordinator( clearPendingCodexAttention() pendingCodexAttentionTimer = setTimeout(() => { pendingCodexAttentionTimer = null - if (!options.isLive() || !hasAgentRunEvidence) { + if (!options.isLive(payload) || !hasAgentRunEvidence) { return } dispatchAttentionNotification(payload) diff --git a/src/renderer/src/components/terminal-pane/use-notification-dispatch.ts b/src/renderer/src/components/terminal-pane/use-notification-dispatch.ts index efddd556ec5..d69908efb17 100644 --- a/src/renderer/src/components/terminal-pane/use-notification-dispatch.ts +++ b/src/renderer/src/components/terminal-pane/use-notification-dispatch.ts @@ -26,6 +26,7 @@ import { isOrcaWindowForegroundFocused, isVisibleForegroundPaneKey } from './terminal-notification-pane-visibility' +import { getWebSessionTabsNotificationPaneEvidenceState } from '@/runtime/web-session-tabs-notification-reconciler' const AGENT_NOTIFICATION_SNAPSHOT_MAX_AGE_MS = 10_000 @@ -121,6 +122,14 @@ export function dispatchTerminalNotification( ) { return } + const remotePaneEvidence = eventAgentStatusSnapshot?.remotePaneEvidence + const remotePaneEvidenceState = + remotePaneEvidence && event.paneKey + ? getWebSessionTabsNotificationPaneEvidenceState(remotePaneEvidence, event.paneKey) + : 'stale' + if (remotePaneEvidence && remotePaneEvidenceState === 'stale') { + return + } const agentNotificationStateStartedAt = eventAgentStatusSnapshot?.stateStartedAt ?? freshStoredAgentStatus?.stateStartedAt // Why: main-process hook IPC can update inactive/unmounted worktrees before @@ -148,9 +157,11 @@ export function dispatchTerminalNotification( // Why: delayed completion hooks from a closed split pane can arrive while // another pane in the tab is still live; stale leaf completions must not // create unread state or OS notifications. - const isCurrentPane = hasLivePty - ? isCurrentLivePaneKey(state, worktreeId, event.paneKey) - : isCurrentKnownPaneKey(state, worktreeId, event.paneKey) + const isCurrentPane = + remotePaneEvidenceState === 'uncommitted' || + (hasLivePty + ? isCurrentLivePaneKey(state, worktreeId, event.paneKey) + : isCurrentKnownPaneKey(state, worktreeId, event.paneKey)) if (!tabId || !isCurrentPane) { return } diff --git a/src/renderer/src/hooks/agent-hook-completion-notifications.ts b/src/renderer/src/hooks/agent-hook-completion-notifications.ts index fd6c99913e2..4718766c69d 100644 --- a/src/renderer/src/hooks/agent-hook-completion-notifications.ts +++ b/src/renderer/src/hooks/agent-hook-completion-notifications.ts @@ -1,5 +1,4 @@ import { useAppStore } from '@/store' -import { parsePaneKey } from '../../../shared/stable-pane-id' import { createAgentCompletionCoordinator } from '@/components/terminal-pane/agent-completion-coordinator' import type { AgentCompletionCoordinator, @@ -7,7 +6,6 @@ import type { } from '@/components/terminal-pane/agent-completion-coordinator-types' import type { RuntimeTerminalProcessInspection } from '@/runtime/runtime-terminal-inspection' import { dispatchTerminalNotification } from '@/components/terminal-pane/use-notification-dispatch' -import { collectLeafIdsInOrder } from '@/components/terminal-pane/layout-serialization' import { createCodexAutoApprovalHookCompletionSuppressor } from '@/components/terminal-pane/codex-auto-approval-notification-suppression' import { dispatchAgentHookTerminalLifecycle } from '@/components/terminal-pane/agent-hook-terminal-lifecycle' import { @@ -15,18 +13,19 @@ import { shouldSyncAgentHookCompletionForStoreUpdate, type AgentHookCompletionStoreSnapshot } from './agent-hook-completion-store-sync' +import { + buildAgentHookNotificationTabIndex, + getPtyIdForAgentHookPane, + paneCanReceiveAgentHookNotification +} from './agent-hook-notification-pane-liveness' type CoordinatorEntry = { worktreeId: string coordinator: AgentCompletionCoordinator + remotePaneEvidence?: AgentCompletionStatusSnapshot['remotePaneEvidence'] } type StoreSnapshot = ReturnType -type WorktreeTab = NonNullable[string][number] -// Why: a paneKey resolves to a tab by id. Prebuilding this index once per prune -// pass avoids re-flattening tabsByWorktree per coordinator (O(coordinators x -// tabs)) when a liveness or notification-setting update requires a prune. -type TabIndex = ReadonlyMap type PaneCoordinatorLivenessSnapshot = Pick< StoreSnapshot, 'tabsByWorktree' | 'ptyIdsByTabId' | 'terminalLayoutsByTabId' | 'suppressedPtyExitIds' @@ -44,20 +43,6 @@ function disposeCoordinatorForPaneKey(paneKey: string): void { paneKeysRequiringFreshWorking.delete(paneKey) } -function buildTabIndex(tabsByWorktree: StoreSnapshot['tabsByWorktree']): TabIndex { - const index = new Map() - for (const tabs of Object.values(tabsByWorktree ?? {})) { - for (const tab of tabs) { - // Why: first-wins to match the previous Array.flat().find() semantics - // exactly, even in the degenerate case of a tab id shared across worktrees. - if (!index.has(tab.id)) { - index.set(tab.id, tab) - } - } - } - return index -} - function pruneClosedPaneCoordinators(): void { // Why: hook-completion coordinators are module-scoped and may outlive a pane // unless liveness changes from close/sleep paths evict them here. @@ -83,14 +68,20 @@ function pruneClosedPaneCoordinators(): void { lastPrunedLivenessSnapshot = livenessSnapshot // Why: build the paneKey->tab index once for the whole pass instead of // re-flattening tabsByWorktree inside paneCanReceiveHookCompletion per entry. - const tabIndex = buildTabIndex(livenessSnapshot.tabsByWorktree) - for (const paneKey of coordinatorsByPaneKey.keys()) { - if (!paneCanReceiveHookCompletion(paneKey, tabIndex)) { + const tabIndex = buildAgentHookNotificationTabIndex(livenessSnapshot.tabsByWorktree) + for (const [paneKey, entry] of coordinatorsByPaneKey) { + if (!paneCanReceiveAgentHookNotification(paneKey, tabIndex, entry.remotePaneEvidence)) { disposeCoordinatorForPaneKey(paneKey) } } for (const paneKey of paneKeysRequiringFreshWorking) { - if (!paneCanReceiveHookCompletion(paneKey, tabIndex)) { + if ( + !paneCanReceiveAgentHookNotification( + paneKey, + tabIndex, + coordinatorsByPaneKey.get(paneKey)?.remotePaneEvidence + ) + ) { paneKeysRequiringFreshWorking.delete(paneKey) } } @@ -150,102 +141,11 @@ export function syncAgentHookCompletionNotificationsForStoreUpdate( return true } -function getPtyIdForPaneKey(paneKey: string): string | null { - const parsed = parsePaneKey(paneKey) - if (!parsed) { - return null - } - const state = useAppStore.getState() - const tabPtyIds = state.ptyIdsByTabId?.[parsed.tabId] - if (!tabPtyIds || tabPtyIds.length === 0) { - return null - } - // Why: split-pane leaves share one tab-level pty list, so a tab-level lookup - // would return a sibling's pty for an already-closed leaf and let a late - // 'done' hook event fire a spurious notification. Resolve liveness through - // the leaf-keyed binding maintained by syncPanePtyLayoutBinding, which - // deletes the entry when the leaf closes. - const layout = state.terminalLayoutsByTabId?.[parsed.tabId] - const ptyIdsByLeafId = layout?.ptyIdsByLeafId - if (ptyIdsByLeafId) { - const leafPtyId = ptyIdsByLeafId[parsed.leafId] - if (leafPtyId && tabPtyIds.includes(leafPtyId)) { - return leafPtyId - } - if (!layout?.root) { - // Why: inactive worktree switches can temporarily preserve only tab-level - // PTY liveness; do not drop hook completions just because layout metadata - // is at the empty snapshot. - return tabPtyIds[0] ?? null - } - // Why: switching worktrees can unmount the terminal pane and clear the - // leaf binding before the hook completion arrives, while the tab PTY is - // still live. Keep closed leaves suppressed by requiring the leaf in layout. - return collectLeafIdsInOrder(layout.root).includes(parsed.leafId) - ? (tabPtyIds[0] ?? null) - : null - } - return tabPtyIds[0] ?? null -} - -function paneHasLivePty(paneKey: string): boolean { - return getPtyIdForPaneKey(paneKey) !== null -} - -function resolveTabById( - state: StoreSnapshot, - tabId: string, - tabIndex?: TabIndex -): WorktreeTab | undefined { - if (tabIndex) { - return tabIndex.get(tabId) - } - for (const tabs of Object.values(state.tabsByWorktree ?? {})) { - const found = tabs.find((candidate) => candidate.id === tabId) - if (found) { - return found - } - } - return undefined -} - -function paneKeyHasUnsuppressedPtyHint( - state: StoreSnapshot, - paneKey: string, - tabIndex?: TabIndex -): boolean { - const parsed = parsePaneKey(paneKey) - if (!parsed) { - return false - } - const tab = resolveTabById(state, parsed.tabId, tabIndex) - if (!tab) { - return false - } - const layout = state.terminalLayoutsByTabId?.[parsed.tabId] - if (layout?.root && !collectLeafIdsInOrder(layout.root).includes(parsed.leafId)) { - return false - } - const leafPtyId = layout?.ptyIdsByLeafId?.[parsed.leafId] - // Why: sleep/shutdown preserves tab records while marking their PTYs - // suppressed. Missing hints are allowed because inactive-worktree hydration - // can accept hook status before the renderer restores tab PTY metadata. - const ptyHints = [tab.ptyId, leafPtyId].filter((ptyId): ptyId is string => Boolean(ptyId)) - return ptyHints.length === 0 || ptyHints.some((ptyId) => !state.suppressedPtyExitIds?.[ptyId]) -} - -function paneCanReceiveHookCompletion(paneKey: string, tabIndex?: TabIndex): boolean { - const state = useAppStore.getState() - // Why: native hook IPC is itself a live status signal. Inactive worktrees can - // have accepted hook updates before their renderer PTY map catches up. - return paneKeyHasUnsuppressedPtyHint(state, paneKey, tabIndex) || paneHasLivePty(paneKey) -} - function createCoordinator(paneKey: string, worktreeId: string): AgentCompletionCoordinator { return createAgentCompletionCoordinator({ paneKey, statusLane: 'hook', - getPtyId: () => getPtyIdForPaneKey(paneKey), + getPtyId: () => getPtyIdForAgentHookPane(paneKey), getSettings: () => useAppStore.getState().settings, inspectProcess: async (): Promise => ({ foregroundProcess: null, @@ -280,7 +180,8 @@ function createCoordinator(paneKey: string, worktreeId: string): AgentCompletion agentStatusSnapshot: meta.agentStatus }) }, - isLive: () => paneCanReceiveHookCompletion(paneKey), + isLive: (agentStatus) => + paneCanReceiveAgentHookNotification(paneKey, undefined, agentStatus?.remotePaneEvidence), shouldSuppressHookCompletion: createCodexAutoApprovalHookCompletionSuppressor(paneKey) }) } @@ -299,7 +200,7 @@ export function observeAgentHookCompletionForNotification({ // Why: replay seeds already passed indexed snapshot ownership; re-resolving every row makes startup batches quadratic. if (seedOnly !== true) { pruneClosedPaneCoordinators() - if (!paneCanReceiveHookCompletion(paneKey)) { + if (!paneCanReceiveAgentHookNotification(paneKey, undefined, payload.remotePaneEvidence)) { return } } @@ -312,17 +213,24 @@ export function observeAgentHookCompletionForNotification({ } let entry = coordinatorsByPaneKey.get(paneKey) - if (!entry || entry.worktreeId !== worktreeId) { + if ( + !entry || + entry.worktreeId !== worktreeId || + (payload.remotePaneEvidence !== undefined && + entry.remotePaneEvidence?.paneIncarnation !== payload.remotePaneEvidence.paneIncarnation) + ) { entry?.coordinator.dispose() entry = { worktreeId, - coordinator: createCoordinator(paneKey, worktreeId) + coordinator: createCoordinator(paneKey, worktreeId), + ...(payload.remotePaneEvidence ? { remotePaneEvidence: payload.remotePaneEvidence } : {}) } coordinatorsByPaneKey.set(paneKey, entry) if (requireFreshWorkingForNewTrackingCoordinators) { paneKeysRequiringFreshWorking.add(paneKey) } } + entry.remotePaneEvidence = payload.remotePaneEvidence // Why: notification preferences may suppress alerts, but accepted hooks must // still release pane-owned cursor/cache effects after the quiet window. if (payload.state === 'working' && payload.turnCompletedAt === undefined && trackingEnabled) { diff --git a/src/renderer/src/hooks/agent-hook-notification-pane-liveness.ts b/src/renderer/src/hooks/agent-hook-notification-pane-liveness.ts new file mode 100644 index 00000000000..eefcec8882b --- /dev/null +++ b/src/renderer/src/hooks/agent-hook-notification-pane-liveness.ts @@ -0,0 +1,110 @@ +import { collectLeafIdsInOrder } from '@/components/terminal-pane/layout-serialization' +import type { WebSessionTabsNotificationPaneEvidence } from '@/runtime/web-session-tabs-notification-reconciler' +import { getWebSessionTabsNotificationPaneEvidenceState } from '@/runtime/web-session-tabs-notification-reconciler' +import { useAppStore } from '@/store' +import { parsePaneKey } from '../../../shared/stable-pane-id' + +type StoreSnapshot = ReturnType +type WorktreeTab = NonNullable[string][number] +export type AgentHookNotificationTabIndex = ReadonlyMap + +export function buildAgentHookNotificationTabIndex( + tabsByWorktree: StoreSnapshot['tabsByWorktree'] +): AgentHookNotificationTabIndex { + const index = new Map() + for (const tabs of Object.values(tabsByWorktree ?? {})) { + for (const tab of tabs) { + if (!index.has(tab.id)) { + index.set(tab.id, tab) + } + } + } + return index +} + +export function getPtyIdForAgentHookPane(paneKey: string): string | null { + const parsed = parsePaneKey(paneKey) + if (!parsed) { + return null + } + const state = useAppStore.getState() + const tabPtyIds = state.ptyIdsByTabId?.[parsed.tabId] + if (!tabPtyIds || tabPtyIds.length === 0) { + return null + } + const layout = state.terminalLayoutsByTabId?.[parsed.tabId] + const ptyIdsByLeafId = layout?.ptyIdsByLeafId + if (ptyIdsByLeafId) { + const leafPtyId = ptyIdsByLeafId[parsed.leafId] + if (leafPtyId && tabPtyIds.includes(leafPtyId)) { + return leafPtyId + } + if (!layout?.root) { + return tabPtyIds[0] ?? null + } + return collectLeafIdsInOrder(layout.root).includes(parsed.leafId) + ? (tabPtyIds[0] ?? null) + : null + } + return tabPtyIds[0] ?? null +} + +function resolveTabById( + state: StoreSnapshot, + tabId: string, + tabIndex?: AgentHookNotificationTabIndex +): WorktreeTab | undefined { + if (tabIndex) { + return tabIndex.get(tabId) + } + for (const tabs of Object.values(state.tabsByWorktree ?? {})) { + const found = tabs.find((candidate) => candidate.id === tabId) + if (found) { + return found + } + } + return undefined +} + +function paneKeyHasUnsuppressedPtyHint( + state: StoreSnapshot, + paneKey: string, + tabIndex?: AgentHookNotificationTabIndex +): boolean { + const parsed = parsePaneKey(paneKey) + if (!parsed) { + return false + } + const tab = resolveTabById(state, parsed.tabId, tabIndex) + if (!tab) { + return false + } + const layout = state.terminalLayoutsByTabId?.[parsed.tabId] + if (layout?.root && !collectLeafIdsInOrder(layout.root).includes(parsed.leafId)) { + return false + } + const leafPtyId = layout?.ptyIdsByLeafId?.[parsed.leafId] + const ptyHints = [tab.ptyId, leafPtyId].filter((ptyId): ptyId is string => Boolean(ptyId)) + return ptyHints.length === 0 || ptyHints.some((ptyId) => !state.suppressedPtyExitIds?.[ptyId]) +} + +export function paneCanReceiveAgentHookNotification( + paneKey: string, + tabIndex?: AgentHookNotificationTabIndex, + remotePaneEvidence?: WebSessionTabsNotificationPaneEvidence +): boolean { + if (remotePaneEvidence) { + const evidenceState = getWebSessionTabsNotificationPaneEvidenceState( + remotePaneEvidence, + paneKey + ) + if (evidenceState !== 'committed') { + return evidenceState === 'uncommitted' + } + } + const state = useAppStore.getState() + return ( + paneKeyHasUnsuppressedPtyHint(state, paneKey, tabIndex) || + getPtyIdForAgentHookPane(paneKey) !== null + ) +} diff --git a/src/renderer/src/runtime/web-session-tabs-agent-completion-notifications.test.ts b/src/renderer/src/runtime/web-session-tabs-agent-completion-notifications.test.ts index 1b194ac017f..3acc1bfc83c 100644 --- a/src/renderer/src/runtime/web-session-tabs-agent-completion-notifications.test.ts +++ b/src/renderer/src/runtime/web-session-tabs-agent-completion-notifications.test.ts @@ -1,5 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { RuntimeMobileSessionTabsResult } from '../../../shared/runtime-types' +import type { PublicKnownRuntimeEnvironment } from '../../../shared/runtime-environments' +import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../shared/constants' import { makePaneKey } from '../../../shared/stable-pane-id' import { toWebTerminalSurfaceTabId } from '../../../shared/terminal-surface-id' @@ -29,6 +31,7 @@ import { recordReceivedWebSessionTabsSnapshot, resetWebSessionTabsSnapshotFreshnessForTests } from './web-session-tabs-sync' +import { replaceRuntimeEnvironmentRevisions } from './runtime-environment-revision' const ENVIRONMENT_ID = 'web-env-1' const WORKTREE_ID = 'repo::/worktree' @@ -93,12 +96,14 @@ describe('paired session-tab agent completion notifications', () => { resetAgentCompletionCoordinatorIdentitiesForTest() resetWebSessionTabsSnapshotFreshnessForTests() resetRendererOwnedAgentStatusPanesForTests() + replaceRuntimeEnvironmentRevisions([]) resetAgentCompletionCoordinatorIdentitiesForTest() useAppStore.setState(initialState, true) }) afterEach(() => { useAppStore.setState(initialState, true) + replaceRuntimeEnvironmentRevisions([]) resetWebSessionTabsSnapshotFreshnessForTests() resetRendererOwnedAgentStatusPanesForTests() vi.unstubAllGlobals() @@ -159,6 +164,34 @@ describe('paired session-tab agent completion notifications', () => { expect(dispatchCompletion).toHaveBeenCalledTimes(1) }) + it('seeds the first frame from a replacement pairing silently', () => { + replaceRuntimeEnvironmentRevisions([ + { id: ENVIRONMENT_ID, createdAt: 100, pairingRevision: 101 } + ] as PublicKnownRuntimeEnvironment[]) + applySnapshot(makeAgentSnapshot(1, NOW), false) + + replaceRuntimeEnvironmentRevisions([ + { id: ENVIRONMENT_ID, createdAt: 100, pairingRevision: 102 } + ] as PublicKnownRuntimeEnvironment[]) + recordReceivedWebSessionTabsSnapshot(ENVIRONMENT_ID, { + ...makeAgentSnapshot(1, NOW + 1_000, undefined, 'done'), + publicationEpoch: 'replacement-epoch' + }) + + expect(mocks.observeAgentHookCompletionForNotification).toHaveBeenLastCalledWith( + expect.objectContaining({ seedOnly: true }) + ) + }) + + it('does not admit an unmirrored floating workspace to notification derivation', () => { + recordReceivedWebSessionTabsSnapshot(ENVIRONMENT_ID, { + ...makeAgentSnapshot(1, NOW, undefined, 'done'), + worktree: FLOATING_TERMINAL_WORKTREE_ID + }) + + expect(mocks.observeAgentHookCompletionForNotification).not.toHaveBeenCalled() + }) + it('announces a host-stamped turn while client OSC status owns the pane', () => { const paneKey = makePaneKey(toWebTerminalSurfaceTabId(HOST_TAB_ID), LEAF_ID) registerRendererOwnedAgentStatusPane(paneKey, ENVIRONMENT_ID) diff --git a/src/renderer/src/runtime/web-session-tabs-new-pane-notification.test.ts b/src/renderer/src/runtime/web-session-tabs-new-pane-notification.test.ts new file mode 100644 index 00000000000..d79735295c0 --- /dev/null +++ b/src/renderer/src/runtime/web-session-tabs-new-pane-notification.test.ts @@ -0,0 +1,343 @@ +// @vitest-environment happy-dom + +import { act, cleanup, renderHook } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { PublicKnownRuntimeEnvironment } from '../../../shared/runtime-environments' +import type { RuntimeRpcResponse } from '../../../shared/runtime-rpc-envelope' +import type { RuntimeMobileSessionTabsResult } from '../../../shared/runtime-types' +import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../shared/constants' +import { makePaneKey } from '../../../shared/stable-pane-id' +import { getUnreadBadgeCount } from '@/lib/unread-badge-count' +import type * as WorktreeRuntimeOwnerModule from '@/lib/worktree-runtime-owner' +import { useAppStore } from '@/store' +import { makeWorktree } from '@/store/slices/worktrees-slice-test-fixtures' +import type { AppState } from '@/store/types' + +const mocks = vi.hoisted(() => ({ + recoverSnapshot: vi.fn(), + runtimeSessionMirrorEnvironmentKey: vi.fn() +})) + +vi.mock('./use-runtime-session-mirror-environment-key', () => ({ + useRuntimeSessionMirrorEnvironmentKey: mocks.runtimeSessionMirrorEnvironmentKey +})) +vi.mock('./web-session-terminal-orphan-recovery', () => ({ + recoverWebSessionTerminalOrphansBeforeApply: mocks.recoverSnapshot +})) +vi.mock('@/lib/worktree-runtime-owner', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, getExplicitRuntimeEnvironmentIdForWorktree: () => null } +}) + +import { resetAgentCompletionCoordinatorIdentitiesForTest } from '@/components/terminal-pane/agent-completion-coordinator' +import { resetAgentHookCompletionNotificationCoordinators } from '@/hooks/agent-hook-completion-notifications' +import { replaceRuntimeEnvironmentRevisions } from './runtime-environment-revision' +import { + resetWebSessionTabsSnapshotFreshnessForTests, + useWebSessionTabsSync +} from './web-session-tabs-sync' +import { WINDOW_VISIBILITY_SUBSCRIPTION_PARK_DELAY_MS } from './window-visibility-subscription-parking' + +const ENVIRONMENT_ID = 'env-new-pane' +const RUNTIME_ID = 'runtime-new-pane' +const REVISION = 101 +const REPO_ID = 'repo-new-pane' +const WORKTREE_ID = `${REPO_ID}::/worktree` +const OTHER_WORKTREE_ID = `${REPO_ID}::/other` +const NEW_WORKTREE_ID = `${REPO_ID}::/new-worktree` +const HOST_TAB_ID = 'host-tab-new-pane' +const NEW_HOST_TAB_ID = 'host-tab-new-worktree' +const LEAF_ID = '11111111-1111-4111-8111-111111111111' +const NEW_LEAF_ID = '22222222-2222-4222-8222-222222222222' +const NOW = 1_700_000_000_000 +const MIRROR_KEY = `${ENVIRONMENT_ID}\u0001${RUNTIME_ID}\u00011\u0001${REVISION}` +const initialState = useAppStore.getInitialState() + +type RuntimeSubscribe = typeof window.api.runtimeEnvironments.subscribe +type RuntimeSubscription = { + request: Parameters[0] + callbacks: Parameters[1] +} + +const subscriptions: RuntimeSubscription[] = [] +const notificationDispatch = vi.fn(async () => ({ delivered: true as const })) +const runtimeCall = vi.fn( + async (): Promise> => ({ + id: 'list-all', + ok: true, + result: { snapshots: [] }, + _meta: { runtimeId: RUNTIME_ID } + }) +) +const runtimeSubscribe = vi.fn(async (request, callbacks) => { + subscriptions.push({ request, callbacks }) + return { unsubscribe: vi.fn(), sendBinary: vi.fn() } +}) + +function snapshot( + snapshotVersion: number, + state?: 'working' | 'done' | 'blocked', + identity: { worktree?: string; hostTabId?: string; leafId?: string } = {} +): RuntimeMobileSessionTabsResult { + const worktree = identity.worktree ?? WORKTREE_ID + const hostTabId = identity.hostTabId ?? HOST_TAB_ID + const leafId = identity.leafId ?? LEAF_ID + return { + worktree, + publicationEpoch: 'epoch-1', + snapshotVersion, + activeGroupId: state ? 'host-group-1' : null, + activeTabId: state ? hostTabId : null, + activeTabType: state ? 'terminal' : null, + tabs: state + ? [ + { + type: 'terminal', + id: `${hostTabId}::${leafId}`, + title: 'Claude', + parentTabId: hostTabId, + leafId, + isActive: true, + status: 'ready', + terminal: 'terminal-new-pane', + agentStatus: { + state, + prompt: 'finish while hidden', + updatedAt: NOW + snapshotVersion, + stateStartedAt: NOW + snapshotVersion, + agentType: 'claude', + paneKey: makePaneKey(hostTabId, leafId), + tabId: hostTabId, + worktreeId: worktree, + stateHistory: [] + } + } + ] + : [] + } +} + +async function settle(): Promise { + await Promise.resolve() + await Promise.resolve() + await Promise.resolve() +} + +function globalSubscription(occurrence = 0): RuntimeSubscription { + const subscription = subscriptions.filter( + ({ request }) => request.method === 'session.tabs.subscribeAll' + )[occurrence] + if (!subscription) { + throw new Error(`Missing global subscription ${occurrence}`) + } + return subscription +} + +async function publish(subscription: RuntimeSubscription, result: unknown): Promise { + await act(async () => { + subscription.callbacks.onResponse({ + id: 'subscription-event', + ok: true, + result, + _meta: { runtimeId: RUNTIME_ID } + }) + await settle() + }) +} + +function setDocumentVisibility(state: 'visible' | 'hidden'): void { + Object.defineProperty(document, 'visibilityState', { configurable: true, get: () => state }) + document.dispatchEvent(new Event('visibilitychange')) +} + +async function parkAndReveal(): Promise { + act(() => { + setDocumentVisibility('hidden') + vi.advanceTimersByTime(WINDOW_VISIBILITY_SUBSCRIPTION_PARK_DELAY_MS) + setDocumentVisibility('visible') + }) + await act(settle) +} + +describe('remote new-pane notification evidence', () => { + beforeEach(() => { + vi.useFakeTimers() + subscriptions.length = 0 + notificationDispatch.mockClear() + runtimeCall.mockClear() + runtimeSubscribe.mockClear() + mocks.recoverSnapshot.mockReset().mockImplementation(async (_state, value) => value) + mocks.runtimeSessionMirrorEnvironmentKey.mockReset().mockReturnValue(MIRROR_KEY) + resetAgentCompletionCoordinatorIdentitiesForTest() + resetAgentHookCompletionNotificationCoordinators() + resetWebSessionTabsSnapshotFreshnessForTests() + const runtimeEnvironments = [ + { id: ENVIRONMENT_ID, createdAt: 100, pairingRevision: REVISION } + ] as PublicKnownRuntimeEnvironment[] + replaceRuntimeEnvironmentRevisions(runtimeEnvironments) + useAppStore.setState( + { + ...initialState, + activeWorktreeId: OTHER_WORKTREE_ID, + workspaceSessionReady: true, + runtimeEnvironments, + runtimeStatusByEnvironmentId: new Map([ + [ENVIRONMENT_ID, { status: { runtimeId: RUNTIME_ID }, connectionGeneration: 1 }] + ]) as AppState['runtimeStatusByEnvironmentId'], + worktreesByRepo: { + [REPO_ID]: [ + makeWorktree({ + id: WORKTREE_ID, + repoId: REPO_ID, + runtimeOwnerEnvironmentId: ENVIRONMENT_ID + }), + makeWorktree({ id: OTHER_WORKTREE_ID, repoId: REPO_ID }) + ] + } + }, + true + ) + Object.defineProperty(window, 'api', { + configurable: true, + value: { + runtimeEnvironments: { call: runtimeCall, subscribe: runtimeSubscribe }, + notifications: { dispatch: notificationDispatch } + } + }) + setDocumentVisibility('visible') + }) + + afterEach(() => { + cleanup() + resetAgentHookCompletionNotificationCoordinators() + resetAgentCompletionCoordinatorIdentitiesForTest() + resetWebSessionTabsSnapshotFreshnessForTests() + replaceRuntimeEnvironmentRevisions([]) + useAppStore.setState(initialState, true) + setDocumentVisibility('visible') + vi.useRealTimers() + }) + + it.each(['done', 'blocked'] as const)( + 'alerts before store hydration when a new hidden pane first appears as %s', + async (state) => { + const hook = renderHook(() => useWebSessionTabsSync()) + await act(settle) + await publish(globalSubscription(), { type: 'updated', ...snapshot(1) }) + + let finishRecovery = (_value: RuntimeMobileSessionTabsResult): void => {} + mocks.recoverSnapshot.mockImplementationOnce( + () => + new Promise((resolve) => { + finishRecovery = resolve + }) + ) + notificationDispatch.mockClear() + await parkAndReveal() + await publish(globalSubscription(1), { + type: 'snapshots', + snapshots: [snapshot(2, state)] + }) + + expect(notificationDispatch).toHaveBeenCalledTimes(1) + expect(useAppStore.getState().tabsByWorktree[WORKTREE_ID]).toBeUndefined() + expect(useAppStore.getState().worktreesByRepo[REPO_ID][0]?.isUnread).toBe(true) + expect(getUnreadBadgeCount(useAppStore.getState())).toBe(1) + + finishRecovery(snapshot(2, state)) + await act(settle) + hook.unmount() + } + ) + + it('alerts for a new worktree first published during a warm resume', async () => { + const hook = renderHook(() => useWebSessionTabsSync()) + await act(settle) + await publish(globalSubscription(), { + type: 'snapshots', + snapshots: [snapshot(1, 'working')] + }) + useAppStore.setState((state) => ({ + worktreesByRepo: { + ...state.worktreesByRepo, + [REPO_ID]: [ + ...state.worktreesByRepo[REPO_ID], + makeWorktree({ + id: NEW_WORKTREE_ID, + repoId: REPO_ID, + runtimeOwnerEnvironmentId: ENVIRONMENT_ID + }) + ] + } + })) + notificationDispatch.mockClear() + + await parkAndReveal() + await publish(globalSubscription(1), { + type: 'snapshots', + snapshots: [ + snapshot(1, 'working'), + snapshot(1, 'blocked', { + worktree: NEW_WORKTREE_ID, + hostTabId: NEW_HOST_TAB_ID, + leafId: NEW_LEAF_ID + }) + ] + }) + + expect(notificationDispatch).toHaveBeenCalledTimes(1) + expect(notificationDispatch).toHaveBeenCalledWith( + expect.objectContaining({ attentionRequired: true, worktreeId: NEW_WORKTREE_ID }) + ) + expect(getUnreadBadgeCount(useAppStore.getState())).toBe(1) + hook.unmount() + }) + + it('ignores advancing remote floating-workspace agent state', async () => { + useAppStore.setState({ + settings: { + ...useAppStore.getState().settings, + experimentalTerminalAttention: true + } as AppState['settings'] + }) + const hook = renderHook(() => useWebSessionTabsSync()) + await act(settle) + const identity = { worktree: FLOATING_TERMINAL_WORKTREE_ID } + + await publish(globalSubscription(), { + type: 'updated', + ...snapshot(1, 'working', identity) + }) + notificationDispatch.mockClear() + await publish(globalSubscription(), { + type: 'updated', + ...snapshot(2, 'done', identity) + }) + vi.advanceTimersByTime(1_500) + + expect(notificationDispatch).not.toHaveBeenCalled() + expect(getUnreadBadgeCount(useAppStore.getState())).toBe(0) + hook.unmount() + }) + + it('drops a host transition for a viewer-locally closed committed pane', async () => { + const hook = renderHook(() => useWebSessionTabsSync()) + await act(settle) + await publish(globalSubscription(), { type: 'updated', ...snapshot(1, 'working') }) + expect(useAppStore.getState().tabsByWorktree[WORKTREE_ID]).toHaveLength(1) + + useAppStore.setState((state) => ({ + tabsByWorktree: { ...state.tabsByWorktree, [WORKTREE_ID]: [] }, + unifiedTabsByWorktree: { ...state.unifiedTabsByWorktree, [WORKTREE_ID]: [] }, + ptyIdsByTabId: {}, + terminalLayoutsByTabId: {} + })) + notificationDispatch.mockClear() + await publish(globalSubscription(), { type: 'updated', ...snapshot(2, 'done') }) + vi.advanceTimersByTime(1_500) + + expect(notificationDispatch).not.toHaveBeenCalled() + expect(getUnreadBadgeCount(useAppStore.getState())).toBe(0) + hook.unmount() + }) +}) diff --git a/src/renderer/src/runtime/web-session-tabs-notification-receipt-order.test.tsx b/src/renderer/src/runtime/web-session-tabs-notification-receipt-order.test.tsx index d3b1ae84968..4403ef1233e 100644 --- a/src/renderer/src/runtime/web-session-tabs-notification-receipt-order.test.tsx +++ b/src/renderer/src/runtime/web-session-tabs-notification-receipt-order.test.tsx @@ -513,4 +513,110 @@ describe('paired session-tab notification receipt ordering', () => { expect(getUnreadBadgeCount(useAppStore.getState())).toBe(0) hook.unmount() }) + + it('closes recovered attention after unchanged active and global resume frames', async () => { + seedRemoteMirrorState(true) + mocks.getExplicitRuntimeEnvironmentIdForWorktree.mockReturnValue(ENVIRONMENT_ID) + const hook = renderHook(() => useWebSessionTabsSync()) + await act(settle) + await publish(findSubscription('session.tabs.subscribeAll'), { + type: 'updated', + ...snapshot(1, 'working', NOW) + }) + + await parkAndReveal() + await publish(findSubscription('session.tabs.subscribe', 1), { + type: 'snapshot', + ...snapshot(1, 'working', NOW) + }) + await publish(findSubscription('session.tabs.subscribeAll', 1), { + type: 'snapshots', + snapshots: [snapshot(1, 'working', NOW)] + }) + notificationDispatch.mockClear() + mocks.observeAgentHookCompletionForNotification.mockClear() + + await publish(findSubscription('session.tabs.subscribe', 1), { + type: 'updated', + ...snapshot(2, 'done', NOW + 1_000) + }) + vi.advanceTimersByTime(1_500) + + expect(notificationDispatch).toHaveBeenCalledTimes(1) + expect(mocks.observeAgentHookCompletionForNotification).toHaveBeenLastCalledWith( + expect.objectContaining({ + payload: expect.not.objectContaining({ attentionRequired: true }) + }) + ) + hook.unmount() + }) + + it('preserves recovered attention when reconnect listAll wins the resume race', async () => { + const hook = renderHook(() => useWebSessionTabsSync()) + await act(settle) + await publish(findSubscription('session.tabs.subscribeAll'), { + type: 'updated', + ...snapshot(1, 'working', NOW) + }) + notificationDispatch.mockClear() + + act(() => { + setDocumentVisibility('hidden') + vi.advanceTimersByTime(WINDOW_VISIBILITY_SUBSCRIPTION_PARK_DELAY_MS) + }) + runtimeCall.mockResolvedValueOnce({ + id: 'resume-list-all', + ok: true, + result: { snapshots: [snapshot(2, 'done', NOW + 1_000)] }, + _meta: { runtimeId: RUNTIME_ID } + }) + mocks.runtimeSessionMirrorEnvironmentKey.mockReturnValue(RECONNECTED_MIRROR_KEY) + hook.rerender() + act(() => setDocumentVisibility('visible')) + await act(settle) + vi.advanceTimersByTime(1_500) + + expect(notificationDispatch).toHaveBeenCalledTimes(1) + expect(mocks.observeAgentHookCompletionForNotification).toHaveBeenLastCalledWith( + expect.objectContaining({ + payload: expect.objectContaining({ attentionRequired: true }) + }) + ) + hook.unmount() + }) + + it('keeps hidden attention when global resume fails before the active frame', async () => { + seedRemoteMirrorState(true) + mocks.getExplicitRuntimeEnvironmentIdForWorktree.mockReturnValue(ENVIRONMENT_ID) + const hook = renderHook(() => useWebSessionTabsSync()) + await act(settle) + await publish(findSubscription('session.tabs.subscribeAll'), { + type: 'updated', + ...snapshot(1, 'working', NOW) + }) + notificationDispatch.mockClear() + + await parkAndReveal() + await act(async () => { + findSubscription('session.tabs.subscribeAll', 1).callbacks.onResponse({ + id: 'failed-global-resume', + ok: false, + error: { code: 'disconnected', message: 'global resume failed' }, + _meta: { runtimeId: RUNTIME_ID } + }) + await settle() + }) + await publish(findSubscription('session.tabs.subscribe', 1), { + type: 'snapshot', + ...snapshot(2, 'done', NOW + 1_000) + }) + vi.advanceTimersByTime(1_500) + + expect(notificationDispatch).toHaveBeenCalledTimes(1) + expect(mocks.observeAgentHookCompletionForNotification).toHaveBeenLastCalledWith( + expect.objectContaining({ payload: expect.objectContaining({ attentionRequired: true }) }) + ) + expect(getUnreadBadgeCount(useAppStore.getState())).toBe(1) + hook.unmount() + }) }) diff --git a/src/renderer/src/runtime/web-session-tabs-notification-reconciler.test.ts b/src/renderer/src/runtime/web-session-tabs-notification-reconciler.test.ts index f5cbbd00e5d..8627893f208 100644 --- a/src/renderer/src/runtime/web-session-tabs-notification-reconciler.test.ts +++ b/src/renderer/src/runtime/web-session-tabs-notification-reconciler.test.ts @@ -1,6 +1,9 @@ import { describe, expect, it, vi } from 'vitest' import type { RuntimeMobileSessionTabsResult } from '../../../shared/runtime-types' -import { createWebSessionTabsNotificationReconciler } from './web-session-tabs-notification-reconciler' +import { + createWebSessionTabsNotificationReconciler, + getWebSessionTabsNotificationPaneEvidenceState +} from './web-session-tabs-notification-reconciler' function snapshot( worktree: string, @@ -23,6 +26,7 @@ describe('web session-tabs notification reconciler', () => { const observeAcceptedSnapshot = vi.fn() const reconciler = createWebSessionTabsNotificationReconciler({ trackedWorktrees: [], + getPaneKeys: () => [], observeAcceptedSnapshot }) @@ -32,11 +36,13 @@ describe('web session-tabs notification reconciler', () => { expect(observeAcceptedSnapshot).toHaveBeenNthCalledWith(1, expect.anything(), { seedOnly: true, - attentionRequired: false + attentionRequired: false, + paneEvidenceByKey: expect.any(Map) }) expect(observeAcceptedSnapshot).toHaveBeenNthCalledWith(2, expect.anything(), { seedOnly: false, - attentionRequired: false + attentionRequired: false, + paneEvidenceByKey: expect.any(Map) }) expect(observeAcceptedSnapshot).toHaveBeenCalledTimes(2) }) @@ -45,25 +51,23 @@ describe('web session-tabs notification reconciler', () => { const observeAcceptedSnapshot = vi.fn() const reconciler = createWebSessionTabsNotificationReconciler({ trackedWorktrees: [], + getPaneKeys: () => [], observeAcceptedSnapshot }) - reconciler.observeInventory([snapshot('wt-a', 1)], { - armPublished: true, - attentionRequired: true - }) - reconciler.observeInventory([snapshot('wt-a', 2)], { - armPublished: true, - attentionRequired: true - }) + reconciler.observeInventory([snapshot('wt-a', 1)], { armPublished: true }) + reconciler.beginVisibilityResume() + reconciler.observeInventory([snapshot('wt-a', 2)], { armPublished: true }) expect(observeAcceptedSnapshot).toHaveBeenNthCalledWith(1, expect.anything(), { seedOnly: true, - attentionRequired: false + attentionRequired: false, + paneEvidenceByKey: expect.any(Map) }) expect(observeAcceptedSnapshot).toHaveBeenNthCalledWith(2, expect.anything(), { seedOnly: false, - attentionRequired: true + attentionRequired: true, + paneEvidenceByKey: expect.any(Map) }) }) @@ -71,6 +75,7 @@ describe('web session-tabs notification reconciler', () => { const observeAcceptedSnapshot = vi.fn() const reconciler = createWebSessionTabsNotificationReconciler({ trackedWorktrees: [{ worktree: 'wt-a', freshness: snapshot('wt-a', 1) }], + getPaneKeys: () => [], observeAcceptedSnapshot }) @@ -79,7 +84,8 @@ describe('web session-tabs notification reconciler', () => { expect(observeAcceptedSnapshot).toHaveBeenCalledWith(expect.anything(), { seedOnly: true, - attentionRequired: false + attentionRequired: false, + paneEvidenceByKey: expect.any(Map) }) }) @@ -90,6 +96,7 @@ describe('web session-tabs notification reconciler', () => { { worktree: 'wt-a', freshness: snapshot('wt-a', 1) }, { worktree: 'wt-b', freshness: snapshot('wt-b', 1) } ], + getPaneKeys: () => [], observeAcceptedSnapshot: (value) => observed.push(value.worktree) }) @@ -97,4 +104,81 @@ describe('web session-tabs notification reconciler', () => { expect(observed).toEqual(['wt-b']) }) + + it('treats a new worktree during a warm resume as recovered attention', () => { + const observeAcceptedSnapshot = vi.fn() + const reconciler = createWebSessionTabsNotificationReconciler({ + trackedWorktrees: [], + getPaneKeys: () => [], + observeAcceptedSnapshot + }) + + reconciler.beginVisibilityResume() + reconciler.observeInventory([snapshot('wt-new', 1)], { armPublished: true }) + + expect(observeAcceptedSnapshot).toHaveBeenCalledWith(expect.anything(), { + seedOnly: false, + attentionRequired: true, + paneEvidenceByKey: expect.any(Map) + }) + }) + + it('keeps rejected worktrees outside notification ownership', () => { + const observeAcceptedSnapshot = vi.fn() + const reconciler = createWebSessionTabsNotificationReconciler({ + trackedWorktrees: [], + acceptsSnapshot: (value) => value.worktree !== 'unmirrored', + getPaneKeys: () => [], + observeAcceptedSnapshot + }) + + reconciler.observeSnapshot(snapshot('unmirrored', 1)) + reconciler.observeInventory([snapshot('unmirrored', 2)], { armPublished: true }) + + expect(observeAcceptedSnapshot).not.toHaveBeenCalled() + }) + + it('requires store liveness after pane evidence commits', () => { + let evidence: Parameters[0] | undefined + const value = snapshot('wt-a', 1) + const reconciler = createWebSessionTabsNotificationReconciler({ + trackedWorktrees: [], + getPaneKeys: () => ['pane-a'], + observeAcceptedSnapshot: (_snapshot, observation) => { + evidence = observation.paneEvidenceByKey.get('pane-a') + } + }) + + reconciler.observeSnapshot(value) + expect(evidence).toBeDefined() + expect(getWebSessionTabsNotificationPaneEvidenceState(evidence!, 'pane-a')).toBe('uncommitted') + + reconciler.markSnapshotCommitted(value) + expect(getWebSessionTabsNotificationPaneEvidenceState(evidence!, 'pane-a')).toBe('committed') + }) + + it('arms inventory that arrives after subscription failure without notifying its cold state', () => { + const observeAcceptedSnapshot = vi.fn() + const reconciler = createWebSessionTabsNotificationReconciler({ + trackedWorktrees: [], + getPaneKeys: () => [], + observeAcceptedSnapshot + }) + + reconciler.armPresentWorktrees() + reconciler.observeInventory([snapshot('wt-a', 1)], { armPublished: false }) + reconciler.beginVisibilityResume() + reconciler.observeInventory([snapshot('wt-a', 2)], { armPublished: true }) + + expect(observeAcceptedSnapshot).toHaveBeenNthCalledWith(1, expect.anything(), { + seedOnly: true, + attentionRequired: false, + paneEvidenceByKey: expect.any(Map) + }) + expect(observeAcceptedSnapshot).toHaveBeenNthCalledWith(2, expect.anything(), { + seedOnly: false, + attentionRequired: true, + paneEvidenceByKey: expect.any(Map) + }) + }) }) diff --git a/src/renderer/src/runtime/web-session-tabs-notification-reconciler.ts b/src/renderer/src/runtime/web-session-tabs-notification-reconciler.ts index 41e37c13ec2..59b34db65f7 100644 --- a/src/renderer/src/runtime/web-session-tabs-notification-reconciler.ts +++ b/src/renderer/src/runtime/web-session-tabs-notification-reconciler.ts @@ -5,6 +5,12 @@ type SnapshotFreshness = { snapshotVersion: number } +export type WebSessionTabsNotificationPaneEvidence = { + ownerId: number + worktree: string + paneIncarnation: number +} + export type WebSessionTabsNotificationTrackedWorktree = { worktree: string freshness: SnapshotFreshness @@ -12,23 +18,50 @@ export type WebSessionTabsNotificationTrackedWorktree = { type NotificationWorktreeState = SnapshotFreshness & { eligible: boolean + paneIncarnations: ReadonlyMap + resumeAttentionPending: boolean +} + +type NotificationPaneState = { + incarnation: number + storeCommitted: boolean } export type WebSessionTabsNotificationObservation = { seedOnly: boolean attentionRequired: boolean + paneEvidenceByKey: ReadonlyMap } export type WebSessionTabsNotificationReconciler = { - observeSnapshot: ( - snapshot: RuntimeMobileSessionTabsResult, - options?: { attentionRequired?: boolean } - ) => boolean + observeSnapshot: (snapshot: RuntimeMobileSessionTabsResult) => boolean observeInventory: ( snapshots: readonly RuntimeMobileSessionTabsResult[], - options: { armPublished: boolean; attentionRequired?: boolean } + options: { armPublished: boolean } ) => void armPresentWorktrees: () => void + beginVisibilityResume: () => void + endVisibilityResume: () => void + markSnapshotCommitted: (snapshot: RuntimeMobileSessionTabsResult) => void + dispose: () => void +} + +const worktreesByEvidenceOwner = new Map>() +let nextEvidenceOwnerId = 0 +let nextPaneIncarnation = 0 + +export function getWebSessionTabsNotificationPaneEvidenceState( + evidence: WebSessionTabsNotificationPaneEvidence, + paneKey: string +): 'stale' | 'uncommitted' | 'committed' { + const pane = worktreesByEvidenceOwner + .get(evidence.ownerId) + ?.get(evidence.worktree) + ?.paneIncarnations.get(paneKey) + if (!pane || pane.incarnation !== evidence.paneIncarnation) { + return 'stale' + } + return pane.storeCommitted ? 'committed' : 'uncommitted' } function isRemoval(snapshot: RuntimeMobileSessionTabsResult): boolean { @@ -47,23 +80,52 @@ function advancesFreshness( export function createWebSessionTabsNotificationReconciler(args: { trackedWorktrees: readonly WebSessionTabsNotificationTrackedWorktree[] + acceptsSnapshot?: (snapshot: RuntimeMobileSessionTabsResult) => boolean + getPaneKeys: (snapshot: RuntimeMobileSessionTabsResult) => readonly string[] observeAcceptedSnapshot: ( snapshot: RuntimeMobileSessionTabsResult, observation: WebSessionTabsNotificationObservation ) => void }): WebSessionTabsNotificationReconciler { + const ownerId = (nextEvidenceOwnerId += 1) const worktrees = new Map( args.trackedWorktrees.map(({ worktree, freshness }) => [ worktree, - { ...freshness, eligible: true } + { + ...freshness, + eligible: true, + paneIncarnations: new Map(), + resumeAttentionPending: false + } ]) ) + const observedWorktrees = new Set(worktrees.keys()) + let visibilityResumePending = false + let futureWorktreesArmed = false + worktreesByEvidenceOwner.set(ownerId, worktrees) - const observeAcceptedSnapshot = ( + const paneIncarnationsForSnapshot = ( snapshot: RuntimeMobileSessionTabsResult, - attentionRequired: boolean - ): boolean => { + current: NotificationWorktreeState | undefined + ): ReadonlyMap => { + const paneIncarnations = new Map() + for (const paneKey of args.getPaneKeys(snapshot)) { + const existing = current?.paneIncarnations.get(paneKey) + paneIncarnations.set(paneKey, { + incarnation: existing?.incarnation ?? (nextPaneIncarnation += 1), + storeCommitted: existing?.storeCommitted ?? false + }) + } + return paneIncarnations + } + + const observeAcceptedSnapshot = (snapshot: RuntimeMobileSessionTabsResult): boolean => { + if (args.acceptsSnapshot?.(snapshot) === false) { + return false + } const current = worktrees.get(snapshot.worktree) + const wasObserved = observedWorktrees.has(snapshot.worktree) + observedWorktrees.add(snapshot.worktree) if (isRemoval(snapshot)) { worktrees.delete(snapshot.worktree) return current !== undefined @@ -72,21 +134,36 @@ export function createWebSessionTabsNotificationReconciler(args: { return false } const eligible = current?.eligible === true - worktrees.set(snapshot.worktree, { + const newDuringVisibilityResume = !current && visibilityResumePending && !wasObserved + const paneIncarnations = paneIncarnationsForSnapshot(snapshot, current) + const state = { publicationEpoch: snapshot.publicationEpoch, snapshotVersion: snapshot.snapshotVersion, - eligible - }) + eligible, + paneIncarnations, + resumeAttentionPending: false + } + worktrees.set(snapshot.worktree, state) + const paneEvidenceByKey = new Map() + for (const [paneKey, pane] of paneIncarnations) { + paneEvidenceByKey.set(paneKey, { + ownerId, + worktree: snapshot.worktree, + paneIncarnation: pane.incarnation + }) + } args.observeAcceptedSnapshot(snapshot, { - seedOnly: !eligible, - attentionRequired: eligible && attentionRequired + seedOnly: !eligible && !newDuringVisibilityResume, + attentionRequired: + (eligible && current?.resumeAttentionPending === true) || newDuringVisibilityResume, + paneEvidenceByKey }) return true } return { - observeSnapshot: (snapshot, options) => { - const accepted = observeAcceptedSnapshot(snapshot, options?.attentionRequired === true) + observeSnapshot: (snapshot) => { + const accepted = observeAcceptedSnapshot(snapshot) const state = worktrees.get(snapshot.worktree) if (state) { state.eligible = true @@ -96,15 +173,18 @@ export function createWebSessionTabsNotificationReconciler(args: { observeInventory: (snapshots, options) => { const publishedWorktrees = new Set() for (const snapshot of snapshots) { + if (args.acceptsSnapshot?.(snapshot) === false) { + continue + } publishedWorktrees.add(snapshot.worktree) - observeAcceptedSnapshot(snapshot, options.attentionRequired === true) + observeAcceptedSnapshot(snapshot) } for (const worktree of worktrees.keys()) { if (!publishedWorktrees.has(worktree)) { worktrees.delete(worktree) } } - if (options.armPublished) { + if (options.armPublished || futureWorktreesArmed) { for (const worktree of publishedWorktrees) { const state = worktrees.get(worktree) if (state) { @@ -112,11 +192,44 @@ export function createWebSessionTabsNotificationReconciler(args: { } } } + for (const state of worktrees.values()) { + state.resumeAttentionPending = false + } + visibilityResumePending = false }, armPresentWorktrees: () => { + futureWorktreesArmed = true for (const state of worktrees.values()) { state.eligible = true } + }, + beginVisibilityResume: () => { + visibilityResumePending = true + for (const state of worktrees.values()) { + state.resumeAttentionPending = true + } + }, + endVisibilityResume: () => { + visibilityResumePending = false + for (const state of worktrees.values()) { + state.resumeAttentionPending = false + } + }, + markSnapshotCommitted: (snapshot) => { + const state = worktrees.get(snapshot.worktree) + if ( + !state || + state.publicationEpoch !== snapshot.publicationEpoch || + state.snapshotVersion !== snapshot.snapshotVersion + ) { + return + } + for (const pane of state.paneIncarnations.values()) { + pane.storeCommitted = true + } + }, + dispose: () => { + worktreesByEvidenceOwner.delete(ownerId) } } } diff --git a/src/renderer/src/runtime/web-session-tabs-sync.ts b/src/renderer/src/runtime/web-session-tabs-sync.ts index 8192b607362..b9ea9e0533a 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync.ts @@ -363,19 +363,11 @@ export function recordReceivedWebSessionTabsSnapshot( snapshot: RuntimeMobileSessionTabsResult, options: { observeNotification?: boolean - attentionRequired?: boolean - onNotificationAccepted?: () => void } = {} ): number { const receivedFrame = (receivedSessionTabsFrameSequence += 1) if (options.observeNotification !== false) { - const accepted = getWebSessionTabsNotificationLifecycle(environmentId).observeSnapshot( - snapshot, - { attentionRequired: options.attentionRequired } - ) - if (accepted) { - options.onNotificationAccepted?.() - } + getWebSessionTabsNotificationLifecycle(environmentId).observeSnapshot(snapshot) } const key = sessionTabsFreshnessKey(environmentId, snapshot.worktree) latestReceivedSessionTabsSnapshotByWorktree.set(key, { @@ -392,7 +384,7 @@ export function recordReceivedWebSessionTabsSnapshot( function recordReceivedWebSessionTabsInventory( environmentId: string, snapshots: readonly RuntimeMobileSessionTabsResult[], - options: { armPublished: boolean; attentionRequired?: boolean } + options: { armPublished: boolean } ): number { const receivedFrame = (receivedSessionTabsFrameSequence += 1) getWebSessionTabsNotificationLifecycle(environmentId).observeInventory(snapshots, options) @@ -656,7 +648,6 @@ export function decideWebSessionTabsSnapshot( snapshot: RuntimeMobileSessionTabsResult, environmentId: string ): WebSessionTabsSnapshotDecision { - getWebSessionTabsNotificationLifecycle(environmentId) const key = sessionTabsFreshnessKey(environmentId, snapshot.worktree) if ((snapshot as { removed?: unknown }).removed === true) { // Why: removed worktrees can stop publishing, so clean up their tracking now instead of waiting for a replacement snapshot that may never arrive. @@ -774,10 +765,22 @@ const webSessionTabsNotificationLifecyclesByEnvironment = new Map< >() function createWebSessionTabsNotificationLifecycle( - environmentId: string + environmentId: string, + options: { seedTrackedWorktrees: boolean } ): WebSessionTabsNotificationReconciler { return createWebSessionTabsNotificationReconciler({ - trackedWorktrees: getTrackedWebSessionTabsWorktrees(environmentId), + trackedWorktrees: options.seedTrackedWorktrees + ? getTrackedWebSessionTabsWorktrees(environmentId) + : [], + acceptsSnapshot: (snapshot) => isHostMirroredWorktree(snapshot.worktree), + getPaneKeys: (snapshot) => + snapshot.tabs.flatMap((surface) => { + if (surface.type !== 'terminal') { + return [] + } + const paneKey = toMirroredPaneKey(surface) + return paneKey ? [paneKey] : [] + }), observeAcceptedSnapshot: (snapshot, observation) => observeWebSessionTabsNotificationSnapshot(snapshot, observation) }) @@ -791,7 +794,11 @@ function getWebSessionTabsNotificationLifecycle( if (existing && existing.pairingRevision === pairingRevision) { return existing.lifecycle } - const lifecycle = createWebSessionTabsNotificationLifecycle(environmentId) + existing?.lifecycle.dispose() + const lifecycle = createWebSessionTabsNotificationLifecycle(environmentId, { + // Why: replacement pairings cannot inherit notification eligibility during effect cleanup. + seedTrackedWorktrees: existing === undefined + }) webSessionTabsNotificationLifecyclesByEnvironment.set(environmentId, { pairingRevision, lifecycle @@ -816,7 +823,18 @@ function armExistingWebSessionTabsNotificationPresence( } } +function endExistingWebSessionTabsNotificationVisibilityResume( + environmentId: string, + pairingRevision: number | undefined +): void { + const existing = webSessionTabsNotificationLifecyclesByEnvironment.get(environmentId) + if (existing && existing.pairingRevision === pairingRevision) { + existing.lifecycle.endVisibilityResume() + } +} + function clearWebSessionTabsNotificationLifecycle(environmentId: string): void { + webSessionTabsNotificationLifecyclesByEnvironment.get(environmentId)?.lifecycle.dispose() webSessionTabsNotificationLifecyclesByEnvironment.delete(environmentId) } @@ -833,6 +851,9 @@ export function resetWebSessionTabsSnapshotFreshnessForTests(): void { hostSessionTabIdByLocalKey.clear() hostSessionTabMappingKeysByEnvironmentAndWorktree.clear() hostWorkingClientBoundaryByPaneKey.clear() + for (const { lifecycle } of webSessionTabsNotificationLifecyclesByEnvironment.values()) { + lifecycle.dispose() + } webSessionTabsNotificationLifecyclesByEnvironment.clear() resetWebSessionBrowserPlacementsForTests() } @@ -1304,6 +1325,10 @@ function observeWebSessionTabsNotificationSnapshot( if (!remapped) { continue } + const remotePaneEvidence = observation.paneEvidenceByKey.get(remapped.paneKey) + if (!remotePaneEvidence) { + continue + } const turnCompletedAt = normalizeTurnCompletedAtField(surface.turnCompletedAt, remapped.state) if (remapped.state === 'done' && turnCompletedAt === undefined) { hostWorkingClientBoundaryByPaneKey.delete(remapped.paneKey) @@ -1371,6 +1396,7 @@ function observeWebSessionTabsNotificationSnapshot( ...(turnCompletedAt !== undefined ? { turnCompletedAt } : {}) }), stateStartedAt: remapped.stateStartedAt, + remotePaneEvidence, ...(clientOwnedNotification ? { localStateStartedAt } : {}), ...(observation.attentionRequired ? { attentionRequired: true as const } : {}) } @@ -3938,6 +3964,11 @@ export function applyWebSessionTabsStorePatch( } console.warn('[web-session-tabs-sync] a store subscriber failed after the patch landed:', error) } + for (const { environmentId, snapshot, decision } of hostMirrorVerdict.frames) { + if (decision.apply) { + getWebSessionTabsNotificationLifecycle(environmentId).markSnapshotCommitted(snapshot) + } + } // Why: the receipt exists exactly from the commit on, so no caller can settle // evidence the store does not hold, and none can lose one it does. const settleHostMirror = createHostSessionMirrorSettle(hostMirrorVerdict) @@ -4550,10 +4581,12 @@ export function useWebSessionTabsSync(): void { } } const resumedEnvironments = new Map() + const resumedNotificationEnvironmentIds = new Set() const trackedWorktreeIds = new Set() for (const index of restartingSpecIndexes) { const environmentId = environmentIdBySubscriptionSpec[index] if (environmentId) { + resumedNotificationEnvironmentIds.add(environmentId) const trackedWorktrees = getTrackedWebSessionTabsWorktrees(environmentId) if (trackedWorktrees.length === 0) { continue @@ -4569,6 +4602,9 @@ export function useWebSessionTabsSync(): void { }) } } + for (const environmentId of resumedNotificationEnvironmentIds) { + getWebSessionTabsNotificationLifecycle(environmentId).beginVisibilityResume() + } visibilityResumeBatch = resumedEnvironments.size > 0 ? { @@ -4634,7 +4670,6 @@ export function useWebSessionTabsSync(): void { const event = response.result as SessionTabsStreamEvent const replayed = isRuntimeSubscriptionReplayResponse(response) if (event.type === 'snapshots') { - const attentionRequired = awaitingVisibilityResumeInventory const skipUnchangedResumeWork = awaitingVisibilityResumeInventory && !replayed awaitingVisibilityResumeInventory = false // Why: an unchanged epoch/version proves the host published nothing while parked, so there is no missed frame to recover or replay. @@ -4661,7 +4696,7 @@ export function useWebSessionTabsSync(): void { const inventoryReceivedFrame = recordReceivedWebSessionTabsInventory( environmentId, event.snapshots, - { armPublished: true, attentionRequired } + { armPublished: true } ) const missingWorktrees = recordVisibilityResumeInventoryReceipt( environmentId, @@ -4792,9 +4827,7 @@ export function useWebSessionTabsSync(): void { // talking has not reported a single PTY dead. return } - const receivedFrame = recordReceivedWebSessionTabsSnapshot(environmentId, event, { - attentionRequired: awaitingVisibilityResumeInventory - }) + const receivedFrame = recordReceivedWebSessionTabsSnapshot(environmentId, event) recordVisibilityResumeSnapshotReceipt(environmentId, event, receivedFrame) const finishRecovery = beginWebSessionTabsSnapshotRecovery( environmentId, @@ -4893,6 +4926,10 @@ export function useWebSessionTabsSync(): void { recordVisibilityResumeSnapshotReceiptRef.current = () => {} shouldApplyVisibilityResumeSnapshotRef.current = () => true for (const { environmentId, expectedEnvironmentPairingRevision } of environments) { + endExistingWebSessionTabsNotificationVisibilityResume( + environmentId, + expectedEnvironmentPairingRevision + ) armExistingWebSessionTabsNotificationPresence( environmentId, expectedEnvironmentPairingRevision @@ -5021,8 +5058,7 @@ export function useWebSessionTabsSync(): void { } const disposeSubscription = installWindowVisibilitySubscriptionParking([ { - subscribe: (isCurrent, { visibilityGeneration }) => { - let awaitingVisibilityResumeSnapshot = visibilityGeneration > 0 + subscribe: (isCurrent) => { return window.api.runtimeEnvironments.subscribe( { selector: environmentId, @@ -5053,13 +5089,7 @@ export function useWebSessionTabsSync(): void { // this worktree — let alone about the whole environment. return } - const attentionRequired = awaitingVisibilityResumeSnapshot - const receivedFrame = recordReceivedWebSessionTabsSnapshot(environmentId, event, { - attentionRequired, - onNotificationAccepted: () => { - awaitingVisibilityResumeSnapshot = false - } - }) + const receivedFrame = recordReceivedWebSessionTabsSnapshot(environmentId, event) recordVisibilityResumeSnapshotReceiptRef.current( environmentId, event, diff --git a/src/renderer/src/store/slices/folder-workspace-owner-routed-mutations.test.ts b/src/renderer/src/store/slices/folder-workspace-owner-routed-mutations.test.ts index 23e8a860706..e2a2337fc05 100644 --- a/src/renderer/src/store/slices/folder-workspace-owner-routed-mutations.test.ts +++ b/src/renderer/src/store/slices/folder-workspace-owner-routed-mutations.test.ts @@ -7,6 +7,7 @@ import { } from '../../runtime/runtime-compatibility-test-fixture' import { clearRuntimeCompatibilityCacheForTests } from '../../runtime/runtime-rpc-client' import { createTestStore } from './store-test-helpers' +import { folderWorkspaceKey } from '../../../../shared/workspace-scope' const folderWorkspacesUpdate = vi.fn() const folderWorkspacesDelete = vi.fn() @@ -238,6 +239,48 @@ describe('folder workspace owner-routed mutations', () => { expect(store.getState().folderWorkspaces[0]?.updatedAt).toBe(2) }) + it('does not apply a stale catalog response after an unread update settles', async () => { + const folderWorkspace = makeFolderWorkspace() + let resolveUpdate!: (workspace: FolderWorkspace) => void + folderWorkspacesUpdate.mockImplementation( + () => + new Promise((resolve) => { + resolveUpdate = resolve + }) + ) + let resolveListing!: (workspaces: FolderWorkspace[]) => void + folderWorkspacesList.mockImplementation( + () => + new Promise((resolve) => { + resolveListing = resolve + }) + ) + const store = createTestStore() + store.setState({ + projectGroups: [{ ...projectGroup, executionHostId: 'local' }], + folderWorkspaces: [folderWorkspace] + }) + + const now = vi.spyOn(Date, 'now').mockReturnValue(10) + try { + store.getState().markWorktreeUnread(folderWorkspaceKey(folderWorkspace.id)) + } finally { + now.mockRestore() + } + const pendingCatalog = store.getState().fetchFolderWorkspaces() + await vi.waitFor(() => expect(folderWorkspacesList).toHaveBeenCalledTimes(1)) + resolveUpdate({ ...folderWorkspace, isUnread: true, lastActivityAt: 10, updatedAt: 2 }) + await vi.waitFor(() => expect(store.getState().folderWorkspaces[0]?.updatedAt).toBe(2)) + resolveListing([folderWorkspace]) + await pendingCatalog + + expect(store.getState().folderWorkspaces[0]).toMatchObject({ + isUnread: true, + lastActivityAt: 10, + updatedAt: 2 + }) + }) + it('does not fence a runtime update when the local same-ID catalog refreshes', async () => { const localWorkspace = makeFolderWorkspace({ updatedAt: 3 }) const runtimeWorkspace = { diff --git a/src/renderer/src/store/slices/repos.ts b/src/renderer/src/store/slices/repos.ts index f0fd6fcbae8..d66335c422f 100644 --- a/src/renderer/src/store/slices/repos.ts +++ b/src/renderer/src/store/slices/repos.ts @@ -1230,8 +1230,23 @@ function mergeFetchedFolderWorkspacesForHost({ projectGroups: readonly ProjectGroup[] hostId: string }): readonly FolderWorkspace[] { + const previousByIdentity = new Map( + previous.map((workspace) => [ + getFolderWorkspaceHostIdentity(workspace, projectGroups), + workspace + ]) + ) + const newestFetched = fetched.map((workspace) => { + const previousWorkspace = previousByIdentity.get( + getFolderWorkspaceHostIdentity(workspace, projectGroups) + ) + // Why: concurrent one-shot RPCs can return an older list after a newer update acknowledgement. + return previousWorkspace && previousWorkspace.updatedAt > workspace.updatedAt + ? previousWorkspace + : workspace + }) const fetchedIdentities = new Set( - fetched.map((workspace) => getFolderWorkspaceHostIdentity(workspace, projectGroups)) + newestFetched.map((workspace) => getFolderWorkspaceHostIdentity(workspace, projectGroups)) ) const preserved = previous.filter((workspace) => { const existingHostId = getFolderWorkspaceHostId(workspace, projectGroups) @@ -1243,7 +1258,7 @@ function mergeFetchedFolderWorkspacesForHost({ return unchangedMergeSource( previous, preserved, - mergeByIdentity(preserved, fetched, (workspace) => + mergeByIdentity(preserved, newestFetched, (workspace) => getFolderWorkspaceHostIdentity(workspace, projectGroups) ) ) diff --git a/src/renderer/src/store/slices/worktrees-fetch-listing-merge.test.ts b/src/renderer/src/store/slices/worktrees-fetch-listing-merge.test.ts index 3e295c7c73b..4ce7a1edd87 100644 --- a/src/renderer/src/store/slices/worktrees-fetch-listing-merge.test.ts +++ b/src/renderer/src/store/slices/worktrees-fetch-listing-merge.test.ts @@ -2,10 +2,11 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import type { AppState } from '../types' import type { Worktree } from '../../../../shared/worktree/types' import { clearHugeRepoWarningDismissalsForTests } from '@/lib/source-control-huge-repo-warning-dismissals' -import { makeDetectedResult } from './worktrees-detected-listing-fixtures' +import { makeDetectedResult, qualifyDetectedResult } from './worktrees-detected-listing-fixtures' import { makeWorktree } from './worktrees-slice-test-fixtures' import { createTestStore, + listDetectedMock, mockApi, resetRemoteRuntimeMocks, resetWorktreeSliceModuleMemory, @@ -303,6 +304,207 @@ describe('fetchWorktrees', () => { expect(store.getState().worktreesByRepo.repo1[0]?.head).toBe('def456') }) + it('does not merge stale unread activity over a notification received during refresh', async () => { + const store = createTestStore() + const worktree = makeWorktree({ + id: 'repo1::/path/attention', + repoId: 'repo1', + path: '/path/attention', + isUnread: false, + lastActivityAt: 10 + }) + const detected = makeDetectedResult('repo1', [worktree]) + let resolveListing!: (worktrees: Worktree[]) => void + const listing = new Promise((resolve) => { + resolveListing = resolve + }) + worktreeListMock.mockReturnValueOnce(listing) + store.setState({ + worktreesByRepo: { repo1: [worktree] }, + detectedWorktreesByRepo: { repo1: detected } + } as Partial) + + const refresh = store.getState().fetchWorktrees('repo1') + await vi.waitFor(() => expect(worktreeListMock).toHaveBeenCalledTimes(1)) + const now = vi.spyOn(Date, 'now').mockReturnValue(1_000) + try { + store.getState().markWorktreeUnread(worktree.id) + resolveListing([{ ...worktree, head: 'fresh-head' }]) + await refresh + } finally { + now.mockRestore() + } + + expect(store.getState().worktreesByRepo.repo1[0]).toMatchObject({ + head: 'fresh-head', + isUnread: true, + lastActivityAt: 1_000 + }) + expect(store.getState().detectedWorktreesByRepo.repo1.worktrees[0]).toMatchObject({ + head: 'fresh-head', + isUnread: true, + lastActivityAt: 1_000 + }) + }) + + it('fences a listing started while unread persistence is still in flight', async () => { + const store = createTestStore() + const worktree = makeWorktree({ + id: 'repo1::/path/pending-attention', + repoId: 'repo1', + path: '/path/pending-attention', + isUnread: false, + lastActivityAt: 10 + }) + let settlePersistence!: () => void + mockApi.worktrees.updateMeta.mockReturnValueOnce( + new Promise((resolve) => { + settlePersistence = resolve + }) + ) + store.setState({ + worktreesByRepo: { repo1: [worktree] }, + detectedWorktreesByRepo: { repo1: makeDetectedResult('repo1', [worktree]) } + } as Partial) + const now = vi.spyOn(Date, 'now').mockReturnValue(1_000) + try { + store.getState().markWorktreeUnread(worktree.id) + } finally { + now.mockRestore() + } + worktreeListMock.mockResolvedValueOnce([{ ...worktree, head: 'pending-head' }]) + + await store.getState().fetchWorktrees('repo1') + + expect(store.getState().worktreesByRepo.repo1[0]).toMatchObject({ + head: 'pending-head', + isUnread: true, + lastActivityAt: 1_000 + }) + settlePersistence() + await Promise.resolve() + await Promise.resolve() + + worktreeListMock.mockResolvedValueOnce([{ ...worktree, head: 'settled-head' }]) + await store.getState().fetchWorktrees('repo1') + + expect(store.getState().worktreesByRepo.repo1[0]).toMatchObject({ + head: 'settled-head', + isUnread: false, + lastActivityAt: 10 + }) + }) + + it('fences a stale listing that resolves after unread persistence settles', async () => { + const store = createTestStore() + const worktree = makeWorktree({ + id: 'repo1::/path/settled-attention', + repoId: 'repo1', + path: '/path/settled-attention', + isUnread: false, + lastActivityAt: 10 + }) + let settlePersistence!: () => void + mockApi.worktrees.updateMeta.mockReturnValueOnce( + new Promise((resolve) => { + settlePersistence = resolve + }) + ) + let resolveListing!: (worktrees: Worktree[]) => void + worktreeListMock.mockReturnValueOnce( + new Promise((resolve) => { + resolveListing = resolve + }) + ) + store.setState({ + worktreesByRepo: { repo1: [worktree] }, + detectedWorktreesByRepo: { repo1: makeDetectedResult('repo1', [worktree]) } + } as Partial) + const now = vi.spyOn(Date, 'now').mockReturnValue(1_000) + try { + store.getState().markWorktreeUnread(worktree.id) + } finally { + now.mockRestore() + } + + const refresh = store.getState().fetchWorktrees('repo1') + await vi.waitFor(() => expect(worktreeListMock).toHaveBeenCalledTimes(1)) + settlePersistence() + await Promise.resolve() + await Promise.resolve() + resolveListing([{ ...worktree, head: 'stale-head' }]) + await refresh + + expect(store.getState().worktreesByRepo.repo1[0]).toMatchObject({ + head: 'stale-head', + isUnread: true, + lastActivityAt: 1_000 + }) + }) + + it('does not merge stale unread activity over a hidden detected row', async () => { + const store = createTestStore() + const worktree = makeWorktree({ + id: 'repo1::/path/hidden-attention', + repoId: 'repo1', + path: '/path/hidden-attention', + isUnread: false, + lastActivityAt: 10 + }) + let releaseListing!: () => void + const listingGate = new Promise((resolve) => { + releaseListing = resolve + }) + listDetectedMock.mockImplementationOnce(async (args) => { + await listingGate + const result = makeDetectedResult('repo1', [worktree]) + result.worktrees = [{ ...result.worktrees[0]!, visible: false, head: 'fresh-head' }] + return qualifyDetectedResult(args, result) + }) + store.setState({ + worktreesByRepo: { repo1: [] }, + detectedWorktreesByRepo: { repo1: makeDetectedResult('repo1', [worktree]) } + } as Partial) + + const refresh = store.getState().fetchWorktrees('repo1') + await vi.waitFor(() => expect(listDetectedMock).toHaveBeenCalledTimes(1)) + const now = vi.spyOn(Date, 'now').mockReturnValue(1_000) + try { + store.getState().markWorktreeUnread(worktree.id) + releaseListing() + await refresh + } finally { + now.mockRestore() + } + + expect(store.getState().worktreesByRepo.repo1).toEqual([]) + expect(store.getState().detectedWorktreesByRepo.repo1.worktrees[0]).toMatchObject({ + head: 'fresh-head', + isUnread: true, + lastActivityAt: 1_000 + }) + }) + + it('accepts unread activity from a refresh when no local activity raced it', async () => { + const store = createTestStore() + const worktree = makeWorktree({ + id: 'repo1::/path/attention', + repoId: 'repo1', + path: '/path/attention', + isUnread: false, + lastActivityAt: 10 + }) + store.setState({ worktreesByRepo: { repo1: [worktree] } } as Partial) + worktreeListMock.mockResolvedValueOnce([{ ...worktree, isUnread: true, lastActivityAt: 20 }]) + + await store.getState().fetchWorktrees('repo1') + + expect(store.getState().worktreesByRepo.repo1[0]).toMatchObject({ + isUnread: true, + lastActivityAt: 20 + }) + }) + it('updates the repo entry when only the persisted base ref changes', async () => { const store = createTestStore() const existing = makeWorktree({ diff --git a/src/renderer/src/store/slices/worktrees-slice-test-harness.ts b/src/renderer/src/store/slices/worktrees-slice-test-harness.ts index f84d55a20e5..0a6660fc555 100644 --- a/src/renderer/src/store/slices/worktrees-slice-test-harness.ts +++ b/src/renderer/src/store/slices/worktrees-slice-test-harness.ts @@ -29,6 +29,7 @@ import { TEST_SSH_AUTHORITY } from './worktrees-detected-listing-fixtures' import { makeWorktree } from './worktrees-slice-test-fixtures' +import { resetPassiveWorktreeMetaMutationsForTests } from './worktrees/metadata/passive-worktree-meta-mutation' /** Bare `vi.fn()` infers @vitest/spy's un-nameable `Procedure`, which breaks declaration emit. */ export type StubMock = Mock<(...args: TArgs) => unknown> @@ -218,6 +219,7 @@ export function createLocalLineageTestStore(lineage: WorktreeLineage) { // Why: both the metadata fallback and removeWorktree write this module-level memory, so a leaked entry from an // earlier describe would silently suppress a row here. Reset for every case, not just the fetch suites. export function resetWorktreeSliceModuleMemory() { + resetPassiveWorktreeMetaMutationsForTests() resetAuthoritativelyRemovedWorktreeMemoryForTests() mockApi.worktrees.create.mockReset() mockApi.worktrees.adoptProvisionedRoot.mockReset() diff --git a/src/renderer/src/store/slices/worktrees/listing/fetch-all-worktrees.ts b/src/renderer/src/store/slices/worktrees/listing/fetch-all-worktrees.ts index 1e033adf052..0cc851caa2a 100644 --- a/src/renderer/src/store/slices/worktrees/listing/fetch-all-worktrees.ts +++ b/src/renderer/src/store/slices/worktrees/listing/fetch-all-worktrees.ts @@ -18,6 +18,7 @@ import { mergeFetchedWorktrees } from './fetched-worktree-merge' import { notifyRuntimeScopeForbiddenIfNeeded } from './runtime-scope-forbidden-toast' import { mapReposForWorktreeRefresh } from './worktree-refresh-pool' import { settingsForKnownRepoOwner } from './worktree-owner-settings' +import { capturePassiveWorktreeMetaRequestFences } from '../metadata/passive-worktree-meta-mutation' export function createFetchAllWorktrees( set: WorktreeSliceSet, @@ -40,7 +41,15 @@ export function createFetchAllWorktrees( try { const requestStartedState = get() const requestStartedWorktrees = requestStartedState.worktreesByRepo[r.id] + const requestStartedDetectedWorktrees = + requestStartedState.detectedWorktreesByRepo[r.id]?.worktrees const hostId = getRepoExecutionHostId(r) + const requestStartedPassiveMetaFences = capturePassiveWorktreeMetaRequestFences( + hostId, + [...(requestStartedDetectedWorktrees ?? []), ...(requestStartedWorktrees ?? [])].map( + (worktree) => worktree.id + ) + ) const setup = getProjectHostSetupForRepoHost(requestStartedState, r.id, hostId) const settings = settingsForKnownRepoOwner(requestStartedState.settings, r) const parsedHost = parseExecutionHostId(hostId) @@ -67,6 +76,8 @@ export function createFetchAllWorktrees( hostId, ownerWasMissingAtStart: false, requestStartedWorktrees, + requestStartedDetectedWorktrees, + requestStartedPassiveMetaFences, setup, refresh }) @@ -93,7 +104,15 @@ export function createFetchAllWorktrees( try { const requestStartedState = get() const requestStartedWorktrees = requestStartedState.worktreesByRepo[r.id] + const requestStartedDetectedWorktrees = + requestStartedState.detectedWorktreesByRepo[r.id]?.worktrees const hostId = getRepoExecutionHostId(r) + const requestStartedPassiveMetaFences = capturePassiveWorktreeMetaRequestFences( + hostId, + [...(requestStartedDetectedWorktrees ?? []), ...(requestStartedWorktrees ?? [])].map( + (worktree) => worktree.id + ) + ) const setup = getProjectHostSetupForRepoHost(requestStartedState, r.id, hostId) const parsedHost = parseExecutionHostId(hostId) const directSshAuthority = @@ -123,6 +142,8 @@ export function createFetchAllWorktrees( hostId, ownerWasMissingAtStart: false, requestStartedWorktrees, + requestStartedDetectedWorktrees, + requestStartedPassiveMetaFences, setup, refresh, purgeRemovedWorktrees: false diff --git a/src/renderer/src/store/slices/worktrees/listing/fetch-worktrees.ts b/src/renderer/src/store/slices/worktrees/listing/fetch-worktrees.ts index b812ea170ac..34c0cff1746 100644 --- a/src/renderer/src/store/slices/worktrees/listing/fetch-worktrees.ts +++ b/src/renderer/src/store/slices/worktrees/listing/fetch-worktrees.ts @@ -23,6 +23,7 @@ import { mergeFetchedWorktrees } from './fetched-worktree-merge' import { notifyRuntimeScopeForbiddenIfNeeded } from './runtime-scope-forbidden-toast' import { refreshRemoteWorktreeLineageBestEffort } from '../metadata/worktree-lineage-refresh' import { settingsForRepoOwner } from './worktree-owner-settings' +import { capturePassiveWorktreeMetaRequestFences } from '../metadata/passive-worktree-meta-mutation' export function createFetchWorktrees( set: WorktreeSliceSet, @@ -37,6 +38,7 @@ export function createFetchWorktrees( try { const ownerState = get() const requestStartedWorktrees = ownerState.worktreesByRepo[repoId] + const requestStartedDetectedWorktrees = ownerState.detectedWorktreesByRepo[repoId]?.worktrees const repoOwners = ownerState.repos.filter((repo) => repo.id === repoId) const ownerWasMissingAtStart = repoOwners.length === 0 const hasLocalOwner = repoOwners.some( @@ -49,6 +51,12 @@ export function createFetchWorktrees( const hostId = useLocalOwner ? LOCAL_EXECUTION_HOST_ID : repoHostId(ownerState, repoId, options?.executionHostId) + const requestStartedPassiveMetaFences = capturePassiveWorktreeMetaRequestFences( + hostId, + [...(requestStartedDetectedWorktrees ?? []), ...(requestStartedWorktrees ?? [])].map( + (worktree) => worktree.id + ) + ) const setup = getProjectHostSetupForRepoHost(ownerState, repoId, hostId) const repoOwner = findRepoForHost(ownerState.repos, repoId, { hostId, @@ -99,6 +107,8 @@ export function createFetchWorktrees( ? ownerState.repos : undefined, requestStartedWorktrees, + requestStartedDetectedWorktrees, + requestStartedPassiveMetaFences, setup, refresh }) diff --git a/src/renderer/src/store/slices/worktrees/listing/fetched-worktree-merge.ts b/src/renderer/src/store/slices/worktrees/listing/fetched-worktree-merge.ts index 2311aaf0294..42bd5652fe3 100644 --- a/src/renderer/src/store/slices/worktrees/listing/fetched-worktree-merge.ts +++ b/src/renderer/src/store/slices/worktrees/listing/fetched-worktree-merge.ts @@ -25,6 +25,11 @@ import { rememberAuthoritativelyRemovedWorktrees } from './authoritative-worktree-removal-memory' import type { FencedWorktreeMergeArgs } from './worktree-slice-types' +import { + forgetPassiveWorktreeMetaMutations, + shouldPreservePassiveWorktreeMetaField, + type PassiveWorktreeMetaRequestFence +} from '../metadata/passive-worktree-meta-mutation' export function preserveConcurrentManualOrder( incoming: readonly T[], @@ -52,6 +57,51 @@ export function preserveConcurrentManualOrder( }) } +export function preserveConcurrentWorktreeActivity( + incoming: readonly T[], + requestStarted: readonly Worktree[] | undefined, + current: readonly Worktree[] | undefined, + executionHostId: FencedWorktreeMergeArgs['hostId'], + requestStartedFences: ReadonlyMap, + matchesRefreshHost: (worktree: Worktree) => boolean +): T[] { + if (!requestStarted || !current) { + return [...incoming] + } + const startedById = new Map( + requestStarted.filter(matchesRefreshHost).map((worktree) => [worktree.id, worktree]) + ) + const currentById = new Map( + current.filter(matchesRefreshHost).map((worktree) => [worktree.id, worktree]) + ) + return incoming.map((worktree) => { + const started = startedById.get(worktree.id) + const latest = currentById.get(worktree.id) + if (!started || !latest) { + return worktree + } + const requestFence = requestStartedFences.get(worktree.id) + const unreadChanged = + started.isUnread !== latest.isUnread || + shouldPreservePassiveWorktreeMetaField(executionHostId, worktree.id, 'isUnread', requestFence) + const activityChanged = + started.lastActivityAt !== latest.lastActivityAt || + shouldPreservePassiveWorktreeMetaField( + executionHostId, + worktree.id, + 'lastActivityAt', + requestFence + ) + return unreadChanged || activityChanged + ? { + ...worktree, + ...(unreadChanged ? { isUnread: latest.isUnread } : {}), + ...(activityChanged ? { lastActivityAt: latest.lastActivityAt } : {}) + } + : worktree + }) +} + export function mergeFetchedWorktrees( set: Parameters>[0], args: FencedWorktreeMergeArgs @@ -75,13 +125,31 @@ export function mergeFetchedWorktrees( admitted = true const matchOptions = worktreeHostMatchOptions(s, args.repoId, args.hostId) const currentWorktrees = s.worktreesByRepo[args.repoId] + const requestStartedKnownWorktrees = [ + ...(args.requestStartedDetectedWorktrees ?? []), + ...(args.requestStartedWorktrees ?? []) + ] + const currentKnownWorktrees = [ + ...(s.detectedWorktreesByRepo[args.repoId]?.worktrees ?? []), + ...(currentWorktrees ?? []) + ] + const matchesRefreshHost = (worktree: Worktree): boolean => + worktreeMatchesHost(worktree, args.hostId, matchOptions) + const orderedWorktrees = preserveConcurrentManualOrder( + args.refresh.result.worktrees, + args.requestStartedWorktrees, + currentWorktrees, + matchesRefreshHost + ) const refreshResult = { ...args.refresh.result, - worktrees: preserveConcurrentManualOrder( - args.refresh.result.worktrees, - args.requestStartedWorktrees, - currentWorktrees, - (worktree) => worktreeMatchesHost(worktree, args.hostId, matchOptions) + worktrees: preserveConcurrentWorktreeActivity( + orderedWorktrees, + requestStartedKnownWorktrees, + currentKnownWorktrees, + args.hostId, + args.requestStartedPassiveMetaFences, + matchesRefreshHost ) } let incoming = toVisibleWorktrees(refreshResult, args.hostId, args.setup) @@ -170,6 +238,7 @@ export function mergeFetchedWorktrees( forgetAuthoritativelyRemovedWorktrees(args.hostId, authoritativelySeenIds) rememberAuthoritativelyRemovedWorktrees(args.hostId, authoritativelyRemovedIds) forgetPersistedWorktreeMetaForRemovals(args.repoId, args.hostId, authoritativelyRemovedIds) + forgetPassiveWorktreeMetaMutations(args.hostId, authoritativelyRemovedIds) } return admitted } diff --git a/src/renderer/src/store/slices/worktrees/listing/known-ssh-worktree-fetch.ts b/src/renderer/src/store/slices/worktrees/listing/known-ssh-worktree-fetch.ts index 74ac8f1f815..aff44a22260 100644 --- a/src/renderer/src/store/slices/worktrees/listing/known-ssh-worktree-fetch.ts +++ b/src/renderer/src/store/slices/worktrees/listing/known-ssh-worktree-fetch.ts @@ -29,6 +29,7 @@ import { import { isDetectedWorktreeListResult } from './detected-worktree-provider-request' import { staleDetectedWorktreeProviderResult } from './detected-worktree-refresh-admission' import { mergeFetchedWorktrees } from './fetched-worktree-merge' +import { capturePassiveWorktreeMetaRequestFences } from '../metadata/passive-worktree-meta-mutation' import { getAuthoritativelyRemovedWorktreeIds } from './authoritative-worktree-removal-memory' import type { AdmittedDetectedWorktreeRefresh, @@ -202,6 +203,14 @@ export function acquireDirectSshDetectedWorktreeRefresh( ): DirectSshDetectedWorktreeRefresh { const requestStartedState = store.getState() const requestStartedWorktrees = requestStartedState.worktreesByRepo[request.repoId] + const requestStartedDetectedWorktrees = + requestStartedState.detectedWorktreesByRepo[request.repoId]?.worktrees + const requestStartedPassiveMetaFences = capturePassiveWorktreeMetaRequestFences( + request.executionHostId, + [...(requestStartedDetectedWorktrees ?? []), ...(requestStartedWorktrees ?? [])].map( + (worktree) => worktree.id + ) + ) const ownerWasMissingAtStart = !requestStartedState.repos.some( (repo) => repo.id === request.repoId ) @@ -265,6 +274,8 @@ export function acquireDirectSshDetectedWorktreeRefresh( hostId: request.executionHostId, ownerWasMissingAtStart, requestStartedWorktrees, + requestStartedDetectedWorktrees, + requestStartedPassiveMetaFences, setup, refresh } diff --git a/src/renderer/src/store/slices/worktrees/listing/worktree-owner-settings.ts b/src/renderer/src/store/slices/worktrees/listing/worktree-owner-settings.ts index ae691e4a424..0e715e73cc6 100644 --- a/src/renderer/src/store/slices/worktrees/listing/worktree-owner-settings.ts +++ b/src/renderer/src/store/slices/worktrees/listing/worktree-owner-settings.ts @@ -15,6 +15,7 @@ import { import { WORKTREE_REMOVAL_AMBIGUOUS_ERROR } from './worktree-slice-constants' import { isRuntimeSelectorNotFoundError } from './runtime-worktree-rpc-errors' import { persistWorktreeMeta } from '../metadata/worktree-meta-persist' +import type { WorktreeMetaPersistenceOwner } from '../metadata/worktree-meta-persist' import type { WorktreeSliceGet } from './worktree-slice-types' export function replaceWorktreeInRepoLists( @@ -99,11 +100,7 @@ export function trySettingsForWorktreeOwner( >, worktreeId: string ): AppState['settings'] | null { - const route = resolveWorktreeOperationRoute(state, worktreeId) - if (!route) { - return null - } - return settingsForWorktreeOperationRoute(state.settings, route) + return tryWorktreeMetaPersistenceOwner(state, worktreeId)?.settings ?? null } export function settingsForWorktreeOwner( @@ -117,6 +114,31 @@ export function settingsForWorktreeOwner( return settings } +export function tryWorktreeMetaPersistenceOwner( + state: Parameters[0], + worktreeId: string +): WorktreeMetaPersistenceOwner | null { + const route = resolveWorktreeOperationRoute(state, worktreeId) + if (!route) { + return null + } + return { + settings: settingsForWorktreeOperationRoute(state.settings, route), + executionHostId: route.executionHostId ?? 'local' + } +} + +export function worktreeMetaPersistenceOwner( + state: Parameters[0], + worktreeId: string +): WorktreeMetaPersistenceOwner { + const owner = tryWorktreeMetaPersistenceOwner(state, worktreeId) + if (!owner) { + throw new Error(WORKTREE_REMOVAL_AMBIGUOUS_ERROR) + } + return owner +} + // Why: activity bumps fire on every PTY event, so an ambiguous workspace would warn continuously. // One line per workspace is enough to diagnose it (#10634). export const ambiguousOwnerWarnedWorktreeIds = new Set() @@ -133,16 +155,19 @@ export function persistPassiveWorktreeMetaForOwner( get: WorktreeSliceGet, worktreeId: string, updates: Partial, - errorLabel: string + errorLabel: string, + options: { refreshOnSelectorMiss?: boolean } = {} ): void { - const ownerSettings = trySettingsForWorktreeOwner(get(), worktreeId) - if (!ownerSettings) { + const owner = tryWorktreeMetaPersistenceOwner(get(), worktreeId) + if (!owner) { warnAmbiguousOwnerOnce(worktreeId, errorLabel) return } - void persistWorktreeMeta(ownerSettings, worktreeId, updates).catch((err) => { + void persistWorktreeMeta(owner, worktreeId, updates).catch((err) => { if (isRuntimeSelectorNotFoundError(err)) { - void get().fetchWorktrees(getRepoIdFromWorktreeId(worktreeId)) + if (options.refreshOnSelectorMiss !== false) { + void get().fetchWorktrees(getRepoIdFromWorktreeId(worktreeId)) + } return } console.error(`Failed to ${errorLabel}:`, err) diff --git a/src/renderer/src/store/slices/worktrees/listing/worktree-slice-types.ts b/src/renderer/src/store/slices/worktrees/listing/worktree-slice-types.ts index a3785cde58d..3e964dfc690 100644 --- a/src/renderer/src/store/slices/worktrees/listing/worktree-slice-types.ts +++ b/src/renderer/src/store/slices/worktrees/listing/worktree-slice-types.ts @@ -13,6 +13,7 @@ import type { HostQualifiedDetectedWorktreeResult } from '../../../../../../shar import type { ProjectHostSetup } from '../../../../../../shared/project-types' import type { WorktreeLineage } from '../../../../../../shared/worktree/lineage-types' import type { getActiveRuntimeTarget } from '../../../../runtime/runtime-rpc-client' +import type { PassiveWorktreeMetaRequestFence } from '../metadata/passive-worktree-meta-mutation' export type WorktreeSliceGet = Parameters>[1] export type WorktreeSliceSet = Parameters>[0] @@ -91,6 +92,8 @@ export type FencedWorktreeMergeArgs = { ownerWasMissingAtStart: boolean missingDirectSshOwnerReposSnapshot?: AppState['repos'] requestStartedWorktrees: readonly Worktree[] | undefined + requestStartedDetectedWorktrees: readonly Worktree[] | undefined + requestStartedPassiveMetaFences: ReadonlyMap setup?: ProjectHostSetup refresh: AdmittedDetectedWorktreeRefresh purgeRemovedWorktrees?: boolean diff --git a/src/renderer/src/store/slices/worktrees/metadata/passive-worktree-meta-mutation.test.ts b/src/renderer/src/store/slices/worktrees/metadata/passive-worktree-meta-mutation.test.ts new file mode 100644 index 00000000000..9d7dba1f8fb --- /dev/null +++ b/src/renderer/src/store/slices/worktrees/metadata/passive-worktree-meta-mutation.test.ts @@ -0,0 +1,80 @@ +import { beforeEach, describe, expect, it } from 'vitest' +import { + beginPassiveWorktreeMetaMutation, + capturePassiveWorktreeMetaRequestFences, + forgetPassiveWorktreeMetaMutations, + resetPassiveWorktreeMetaMutationsForTests, + settlePassiveWorktreeMetaMutation, + shouldPreservePassiveWorktreeMetaField +} from './passive-worktree-meta-mutation' + +const WORKTREE_ID = 'repo::/worktree' +const HOST_ID = 'local' + +beforeEach(resetPassiveWorktreeMetaMutationsForTests) + +describe('passive worktree meta mutation', () => { + it('fences a request that starts while persistence is pending', () => { + const mutation = beginPassiveWorktreeMetaMutation(HOST_ID, WORKTREE_ID, { isUnread: true }) + const fence = capturePassiveWorktreeMetaRequestFences(HOST_ID, [WORKTREE_ID]).get(WORKTREE_ID) + + expect(shouldPreservePassiveWorktreeMetaField(HOST_ID, WORKTREE_ID, 'isUnread', fence)).toBe( + true + ) + settlePassiveWorktreeMetaMutation(mutation) + expect(shouldPreservePassiveWorktreeMetaField(HOST_ID, WORKTREE_ID, 'isUnread', fence)).toBe( + true + ) + const settledFence = capturePassiveWorktreeMetaRequestFences(HOST_ID, [WORKTREE_ID]).get( + WORKTREE_ID + ) + expect( + shouldPreservePassiveWorktreeMetaField(HOST_ID, WORKTREE_ID, 'isUnread', settledFence) + ).toBe(false) + }) + + it('fences a mutation that settles after a request starts', () => { + const fence = capturePassiveWorktreeMetaRequestFences(HOST_ID, [WORKTREE_ID]).get(WORKTREE_ID) + const mutation = beginPassiveWorktreeMetaMutation(HOST_ID, WORKTREE_ID, { + lastActivityAt: 10 + }) + settlePassiveWorktreeMetaMutation(mutation) + + expect( + shouldPreservePassiveWorktreeMetaField(HOST_ID, WORKTREE_ID, 'lastActivityAt', fence) + ).toBe(true) + }) + + it('cannot let an old settlement clear a recreated worktree mutation', () => { + const oldMutation = beginPassiveWorktreeMetaMutation(HOST_ID, WORKTREE_ID, { isUnread: true }) + forgetPassiveWorktreeMetaMutations(HOST_ID, [WORKTREE_ID]) + const recreatedMutation = beginPassiveWorktreeMetaMutation(HOST_ID, WORKTREE_ID, { + isUnread: false + }) + const fence = capturePassiveWorktreeMetaRequestFences(HOST_ID, [WORKTREE_ID]).get(WORKTREE_ID) + + settlePassiveWorktreeMetaMutation(oldMutation) + expect(shouldPreservePassiveWorktreeMetaField(HOST_ID, WORKTREE_ID, 'isUnread', fence)).toBe( + true + ) + settlePassiveWorktreeMetaMutation(recreatedMutation) + const settledFence = capturePassiveWorktreeMetaRequestFences(HOST_ID, [WORKTREE_ID]).get( + WORKTREE_ID + ) + expect( + shouldPreservePassiveWorktreeMetaField(HOST_ID, WORKTREE_ID, 'isUnread', settledFence) + ).toBe(false) + }) + + it('isolates the same worktree id across execution hosts', () => { + const runtimeHost = 'runtime:paired' + beginPassiveWorktreeMetaMutation(HOST_ID, WORKTREE_ID, { isUnread: true }) + const runtimeFence = capturePassiveWorktreeMetaRequestFences(runtimeHost, [WORKTREE_ID]).get( + WORKTREE_ID + ) + + expect( + shouldPreservePassiveWorktreeMetaField(runtimeHost, WORKTREE_ID, 'isUnread', runtimeFence) + ).toBe(false) + }) +}) diff --git a/src/renderer/src/store/slices/worktrees/metadata/passive-worktree-meta-mutation.ts b/src/renderer/src/store/slices/worktrees/metadata/passive-worktree-meta-mutation.ts new file mode 100644 index 00000000000..2d981c6c8bd --- /dev/null +++ b/src/renderer/src/store/slices/worktrees/metadata/passive-worktree-meta-mutation.ts @@ -0,0 +1,130 @@ +import type { WorktreeMeta } from '../../../../../../shared/worktree/meta-types' +import type { ExecutionHostId } from '../../../../../../shared/execution-host' + +export type PassiveWorktreeMetaField = 'isUnread' | 'lastActivityAt' + +type MutationEntry = { + generation: number + fieldGeneration: Map + pendingFieldsByGeneration: Map> +} + +export type PassiveWorktreeMetaMutation = { + executionHostId: ExecutionHostId + worktreeId: string + generation: number +} + +export type PassiveWorktreeMetaRequestFence = { + generation: number + pendingFields: ReadonlySet +} + +const entriesByOwnerAndWorktree = new Map() +let nextGeneration = 0 + +function passiveFields(updates: Partial): Set { + const fields = new Set() + if (updates.isUnread !== undefined) { + fields.add('isUnread') + } + if (updates.lastActivityAt !== undefined) { + fields.add('lastActivityAt') + } + return fields +} + +export function beginPassiveWorktreeMetaMutation( + executionHostId: ExecutionHostId, + worktreeId: string, + updates: Partial +): PassiveWorktreeMetaMutation | null { + const fields = passiveFields(updates) + if (fields.size === 0) { + return null + } + const mutationKey = getMutationKey(executionHostId, worktreeId) + const entry = entriesByOwnerAndWorktree.get(mutationKey) ?? { + generation: 0, + fieldGeneration: new Map(), + pendingFieldsByGeneration: new Map() + } + entry.generation = nextGeneration += 1 + for (const field of fields) { + entry.fieldGeneration.set(field, entry.generation) + } + entry.pendingFieldsByGeneration.set(entry.generation, fields) + entriesByOwnerAndWorktree.set(mutationKey, entry) + return { executionHostId, worktreeId, generation: entry.generation } +} + +export function settlePassiveWorktreeMetaMutation( + mutation: PassiveWorktreeMetaMutation | null +): void { + if (!mutation) { + return + } + entriesByOwnerAndWorktree + .get(getMutationKey(mutation.executionHostId, mutation.worktreeId)) + ?.pendingFieldsByGeneration.delete(mutation.generation) +} + +export function capturePassiveWorktreeMetaRequestFences( + executionHostId: ExecutionHostId, + worktreeIds: Iterable +): ReadonlyMap { + const fences = new Map() + for (const worktreeId of worktreeIds) { + const entry = entriesByOwnerAndWorktree.get(getMutationKey(executionHostId, worktreeId)) + const pendingFields = new Set() + for (const fields of entry?.pendingFieldsByGeneration.values() ?? []) { + for (const field of fields) { + pendingFields.add(field) + } + } + fences.set(worktreeId, { generation: entry?.generation ?? 0, pendingFields }) + } + return fences +} + +export function shouldPreservePassiveWorktreeMetaField( + executionHostId: ExecutionHostId, + worktreeId: string, + field: PassiveWorktreeMetaField, + requestFence: PassiveWorktreeMetaRequestFence | undefined +): boolean { + const entry = entriesByOwnerAndWorktree.get(getMutationKey(executionHostId, worktreeId)) + if (!entry) { + return false + } + if (requestFence?.pendingFields.has(field)) { + return true + } + if ((entry.fieldGeneration.get(field) ?? 0) > (requestFence?.generation ?? 0)) { + return true + } + for (const fields of entry.pendingFieldsByGeneration.values()) { + if (fields.has(field)) { + return true + } + } + return false +} + +export function forgetPassiveWorktreeMetaMutations( + executionHostId: ExecutionHostId, + worktreeIds: Iterable +): void { + for (const worktreeId of worktreeIds) { + entriesByOwnerAndWorktree.delete(getMutationKey(executionHostId, worktreeId)) + } +} + +function getMutationKey(executionHostId: ExecutionHostId, worktreeId: string): string { + return `${executionHostId}\0${worktreeId}` +} + +export function resetPassiveWorktreeMetaMutationsForTests(): void { + entriesByOwnerAndWorktree.clear() + nextGeneration = 0 +} diff --git a/src/renderer/src/store/slices/worktrees/metadata/update-worktree-meta.ts b/src/renderer/src/store/slices/worktrees/metadata/update-worktree-meta.ts index c8f0bae7971..5e21de11237 100644 --- a/src/renderer/src/store/slices/worktrees/metadata/update-worktree-meta.ts +++ b/src/renderer/src/store/slices/worktrees/metadata/update-worktree-meta.ts @@ -24,8 +24,8 @@ import { import { persistWorktreeMeta } from './worktree-meta-persist' import { isRuntimeSelectorNotFoundError } from '../listing/runtime-worktree-rpc-errors' import { - settingsForWorktreeOwner, - trySettingsForWorktreeOwner + trySettingsForWorktreeOwner, + worktreeMetaPersistenceOwner } from '../listing/worktree-owner-settings' export function createUpdateWorktreeMeta( @@ -231,7 +231,11 @@ export function createUpdateWorktreeMeta( } try { - await persistWorktreeMeta(settingsForWorktreeOwner(get(), worktreeId), worktreeId, enriched) + await persistWorktreeMeta( + worktreeMetaPersistenceOwner(get(), worktreeId), + worktreeId, + enriched + ) if ( !options?.suppressHostedReviewRefresh && reviewRepo && diff --git a/src/renderer/src/store/slices/worktrees/metadata/update-worktrees-meta.ts b/src/renderer/src/store/slices/worktrees/metadata/update-worktrees-meta.ts index 33f4a224bdd..e6bdfcf2c69 100644 --- a/src/renderer/src/store/slices/worktrees/metadata/update-worktrees-meta.ts +++ b/src/renderer/src/store/slices/worktrees/metadata/update-worktrees-meta.ts @@ -4,7 +4,7 @@ import { applyWorktreeUpdates, getRepoIdFromWorktreeId } from '../../worktree-he import { applyDetectedWorktreeUpdates } from '../listing/detected-worktree-meta' import { persistWorktreeMeta } from './worktree-meta-persist' import { isRuntimeSelectorNotFoundError } from '../listing/runtime-worktree-rpc-errors' -import { settingsForWorktreeOwner } from '../listing/worktree-owner-settings' +import { worktreeMetaPersistenceOwner } from '../listing/worktree-owner-settings' export function createUpdateWorktreesMeta( set: WorktreeSliceSet, @@ -43,7 +43,7 @@ export function createUpdateWorktreesMeta( Array.from(updatesByWorktreeId, async ([worktreeId, updates]) => { try { await persistWorktreeMeta( - settingsForWorktreeOwner(get(), worktreeId), + worktreeMetaPersistenceOwner(get(), worktreeId), worktreeId, updates ) diff --git a/src/renderer/src/store/slices/worktrees/metadata/worktree-git-identity-update.ts b/src/renderer/src/store/slices/worktrees/metadata/worktree-git-identity-update.ts index b4e80783739..3ee4c6dc14a 100644 --- a/src/renderer/src/store/slices/worktrees/metadata/worktree-git-identity-update.ts +++ b/src/renderer/src/store/slices/worktrees/metadata/worktree-git-identity-update.ts @@ -18,7 +18,7 @@ import { } from './hosted-review-link-mutation' import { persistWorktreeMeta } from './worktree-meta-persist' import { isRuntimeSelectorNotFoundError } from '../listing/runtime-worktree-rpc-errors' -import { settingsForWorktreeOwner } from '../listing/worktree-owner-settings' +import { worktreeMetaPersistenceOwner } from '../listing/worktree-owner-settings' export function createUpdateWorktreeGitIdentity( set: WorktreeSliceSet, @@ -149,7 +149,7 @@ export function createUpdateWorktreeGitIdentity( } } await persistWorktreeMeta( - settingsForWorktreeOwner(get(), currentWorktreeId), + worktreeMetaPersistenceOwner(get(), currentWorktreeId), currentWorktreeId, CLEARED_HOSTED_REVIEW_LINK_UPDATES ) @@ -171,7 +171,7 @@ export function createUpdateWorktreeGitIdentity( if (getHostedReviewLinkMutationGeneration(currentWorktreeId) !== clearGeneration) { // Why: a delayed branch-switch clear must not win over a newer manual relink. await persistWorktreeMeta( - settingsForWorktreeOwner(get(), currentWorktreeId), + worktreeMetaPersistenceOwner(get(), currentWorktreeId), currentWorktreeId, getHostedReviewLinkUpdates(latest as Worktree) ) diff --git a/src/renderer/src/store/slices/worktrees/metadata/worktree-meta-persist.test.ts b/src/renderer/src/store/slices/worktrees/metadata/worktree-meta-persist.test.ts new file mode 100644 index 00000000000..a176a56706c --- /dev/null +++ b/src/renderer/src/store/slices/worktrees/metadata/worktree-meta-persist.test.ts @@ -0,0 +1,53 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { + capturePassiveWorktreeMetaRequestFences, + resetPassiveWorktreeMetaMutationsForTests, + shouldPreservePassiveWorktreeMetaField +} from './passive-worktree-meta-mutation' +import { persistWorktreeMeta } from './worktree-meta-persist' + +const WORKTREE_ID = 'repo::/worktree' +const HOST_ID = 'local' + +beforeEach(() => { + resetPassiveWorktreeMetaMutationsForTests() + vi.unstubAllGlobals() +}) + +describe('worktree metadata persistence fencing', () => { + it('tracks passive fields for every caller of the common persistence boundary', async () => { + let settle!: () => void + const updateMeta = vi.fn( + () => + new Promise((resolve) => { + settle = resolve + }) + ) + vi.stubGlobal('window', { api: { worktrees: { updateMeta } } }) + + const persistence = persistWorktreeMeta( + { settings: null, executionHostId: HOST_ID }, + WORKTREE_ID, + { isUnread: true, lastActivityAt: 10 } + ) + const requestFence = capturePassiveWorktreeMetaRequestFences(HOST_ID, [WORKTREE_ID]).get( + WORKTREE_ID + ) + + expect( + shouldPreservePassiveWorktreeMetaField(HOST_ID, WORKTREE_ID, 'isUnread', requestFence) + ).toBe(true) + expect( + shouldPreservePassiveWorktreeMetaField(HOST_ID, WORKTREE_ID, 'lastActivityAt', requestFence) + ).toBe(true) + + settle() + await persistence + const laterRequestFence = capturePassiveWorktreeMetaRequestFences(HOST_ID, [WORKTREE_ID]).get( + WORKTREE_ID + ) + expect( + shouldPreservePassiveWorktreeMetaField(HOST_ID, WORKTREE_ID, 'isUnread', laterRequestFence) + ).toBe(false) + }) +}) diff --git a/src/renderer/src/store/slices/worktrees/metadata/worktree-meta-persist.ts b/src/renderer/src/store/slices/worktrees/metadata/worktree-meta-persist.ts index 7ebdbcc2ccd..6845c702be9 100644 --- a/src/renderer/src/store/slices/worktrees/metadata/worktree-meta-persist.ts +++ b/src/renderer/src/store/slices/worktrees/metadata/worktree-meta-persist.ts @@ -11,53 +11,68 @@ import { toRuntimeWorktreeSelector } from '../../../../runtime/runtime-worktree- import { translate } from '@/i18n/i18n' import type { AppState } from '../../../types' import type { WorktreeMeta } from '../../../../../../shared/worktree/meta-types' +import type { ExecutionHostId } from '../../../../../../shared/execution-host' import { encodePushTargetClearForRuntimeRpc } from './hosted-review-link-mutation' +import { + beginPassiveWorktreeMetaMutation, + settlePassiveWorktreeMetaMutation +} from './passive-worktree-meta-mutation' + +export type WorktreeMetaPersistenceOwner = { + settings: AppState['settings'] + executionHostId: ExecutionHostId +} export async function persistWorktreeMeta( - settings: AppState['settings'], + owner: WorktreeMetaPersistenceOwner, worktreeId: string, updates: Partial ): Promise { - const target = getActiveRuntimeTarget(settings) - if (target.kind === 'local') { - await window.api.worktrees.updateMeta({ worktreeId, updates }) - return - } - // Why: `worktree.set` parses in strip mode, so an older runtime drops the key - // and applies the rest. Both gates key off presence, not value — a dropped - // *clear* strands a stale link that the Issue row then hides. - if ( - target.kind === 'environment' && - ('linkedWorkItem' in updates || 'linkedTaskSourceContext' in updates) - ) { - await assertRuntimeEnvironmentCapability( - target.environmentId, - WORKTREE_LINKED_WORK_ITEM_CONTEXT_RUNTIME_CAPABILITY, - translate( - 'auto.store.slices.worktrees.metadata.worktree.meta.persist.877e3638d8', - 'Update the remote runtime to change this workspace’s linked issue' + const mutation = beginPassiveWorktreeMetaMutation(owner.executionHostId, worktreeId, updates) + try { + const target = getActiveRuntimeTarget(owner.settings) + if (target.kind === 'local') { + await window.api.worktrees.updateMeta({ worktreeId, updates }) + return + } + // Why: `worktree.set` parses in strip mode, so an older runtime drops the key + // and applies the rest. Both gates key off presence, not value — a dropped + // *clear* strands a stale link that the Issue row then hides. + if ( + target.kind === 'environment' && + ('linkedWorkItem' in updates || 'linkedTaskSourceContext' in updates) + ) { + await assertRuntimeEnvironmentCapability( + target.environmentId, + WORKTREE_LINKED_WORK_ITEM_CONTEXT_RUNTIME_CAPABILITY, + translate( + 'auto.store.slices.worktrees.metadata.worktree.meta.persist.877e3638d8', + 'Update the remote runtime to change this workspace’s linked issue' + ) ) - ) - } - // task-source-context.v1 is a sound proxy for the Linear keys: #5322 added them - // to the schema and is an ancestor of the commit introducing that capability. - if (target.kind === 'environment' && 'linkedLinearIssue' in updates) { - await assertRuntimeEnvironmentCapability( - target.environmentId, - TASK_SOURCE_CONTEXT_RUNTIME_CAPABILITY, - translate( - 'auto.store.slices.worktrees.metadata.worktree.meta.persist.4367540861', - 'Update the remote runtime to link Linear issues' + } + // task-source-context.v1 is a sound proxy for the Linear keys: #5322 added them + // to the schema and is an ancestor of the commit introducing that capability. + if (target.kind === 'environment' && 'linkedLinearIssue' in updates) { + await assertRuntimeEnvironmentCapability( + target.environmentId, + TASK_SOURCE_CONTEXT_RUNTIME_CAPABILITY, + translate( + 'auto.store.slices.worktrees.metadata.worktree.meta.persist.4367540861', + 'Update the remote runtime to link Linear issues' + ) ) + } + await callRuntimeRpc( + target, + 'worktree.set', + { + worktree: toRuntimeWorktreeSelector(worktreeId), + ...encodePushTargetClearForRuntimeRpc(updates) + }, + { timeoutMs: 15_000 } ) + } finally { + settlePassiveWorktreeMetaMutation(mutation) } - await callRuntimeRpc( - target, - 'worktree.set', - { - worktree: toRuntimeWorktreeSelector(worktreeId), - ...encodePushTargetClearForRuntimeRpc(updates) - }, - { timeoutMs: 15_000 } - ) } diff --git a/src/renderer/src/store/slices/worktrees/session/worktree-unread-activity.ts b/src/renderer/src/store/slices/worktrees/session/worktree-unread-activity.ts index fa05be0e2b9..92b8338a92c 100644 --- a/src/renderer/src/store/slices/worktrees/session/worktree-unread-activity.ts +++ b/src/renderer/src/store/slices/worktrees/session/worktree-unread-activity.ts @@ -1,7 +1,7 @@ import type { WorktreeSlice } from '../../worktree-helpers' import type { WorktreeSliceGet, WorktreeSliceSet } from '../listing/worktree-slice-types' import { parseWorkspaceKey } from '../../../../../../shared/workspace-scope' -import { applyWorktreeUpdates, getRepoIdFromWorktreeId } from '../../worktree-helpers' +import { applyWorktreeUpdates } from '../../worktree-helpers' import { branchName } from '@/lib/git-utils' import { refreshHostedReviewCard } from '../../hosted-review' import { @@ -9,13 +9,7 @@ import { findKnownWorktreeById } from '../listing/detected-worktree-meta' import { getFolderWorkspaceActivityPersistence } from './folder-workspace-activity' -import { - persistPassiveWorktreeMetaForOwner, - trySettingsForWorktreeOwner, - warnAmbiguousOwnerOnce -} from '../listing/worktree-owner-settings' -import { persistWorktreeMeta } from '../metadata/worktree-meta-persist' -import { isRuntimeSelectorNotFoundError } from '../listing/runtime-worktree-rpc-errors' +import { persistPassiveWorktreeMetaForOwner } from '../listing/worktree-owner-settings' export function createMarkWorktreeUnread( set: WorktreeSliceSet, @@ -293,19 +287,12 @@ export function createBumpWorktreeActivity( return } - const ownerSettings = trySettingsForWorktreeOwner(get(), worktreeId) - if (!ownerSettings) { - warnAmbiguousOwnerOnce(worktreeId, 'persist worktree activity timestamp') - return - } - void persistWorktreeMeta(ownerSettings, worktreeId, { - lastActivityAt: now - }).catch((err) => { - if (isRuntimeSelectorNotFoundError(err)) { - return - } - console.error('Failed to persist worktree activity timestamp:', err) - void get().fetchWorktrees(getRepoIdFromWorktreeId(worktreeId)) - }) + persistPassiveWorktreeMetaForOwner( + get, + worktreeId, + { lastActivityAt: now }, + 'persist worktree activity timestamp', + { refreshOnSelectorMiss: false } + ) } } diff --git a/tests/e2e/paired-remote-agent-visibility-notifications.spec.ts b/tests/e2e/paired-remote-agent-visibility-notifications.spec.ts index b03d916a107..3bd117f91fd 100644 --- a/tests/e2e/paired-remote-agent-visibility-notifications.spec.ts +++ b/tests/e2e/paired-remote-agent-visibility-notifications.spec.ts @@ -6,8 +6,10 @@ import type { AgentHookEndpoint } from '../../src/shared/agent-hook-endpoint-fil import type { RuntimeMobileSessionTabsResult } from '../../src/shared/runtime-types' import { makePaneKey } from '../../src/shared/stable-pane-id' import { toWebTerminalSurfaceTabId } from '../../src/shared/terminal-surface-id' -import { WINDOW_VISIBILITY_SUBSCRIPTION_PARK_DELAY_MS } from '../../src/renderer/src/runtime/window-visibility-subscription-parking' -import { WINDOW_VISIBILITY_SUBSCRIPTION_PARK_DELAY_BACKOFF_LIMIT } from '../../src/renderer/src/runtime/window-visibility-subscription-parking' +import { + WINDOW_VISIBILITY_SUBSCRIPTION_PARK_DELAY_BACKOFF_LIMIT, + WINDOW_VISIBILITY_SUBSCRIPTION_PARK_DELAY_MS +} from '../../src/renderer/src/runtime/window-visibility-subscription-parking' import { expect, test } from './helpers/orca-app' import { readHookEndpoint } from './helpers/agent-hook-endpoint' import { @@ -185,6 +187,20 @@ async function unreadBadgeState( return { ...renderer, dock } } +async function expectUnreadBadgeRetained( + client: PairedElectronClient, + worktreeId: string +): Promise { + const expected = { + unread: true, + unreadCount: 1, + dock: process.platform === 'darwin' ? '1' : null + } + await expect.poll(() => unreadBadgeState(client, worktreeId)).toMatchObject(expected) + await client.page.waitForTimeout(1_000) + expect(await unreadBadgeState(client, worktreeId)).toMatchObject(expected) +} + async function hideUntilSubscriptionsPark(client: PairedElectronClient): Promise { await client.app.evaluate(({ BrowserWindow }) => BrowserWindow.getAllWindows()[0]?.hide()) await expect @@ -472,13 +488,7 @@ test('recovers hidden remote completion and permission alerts exactly once', asy attentionRequired: true }) ]) - await expect - .poll(() => unreadBadgeState(client!, worktreeId)) - .toMatchObject({ - unread: true, - unreadCount: 1, - dock: process.platform === 'darwin' ? '1' : null - }) + await expectUnreadBadgeRetained(client, worktreeId) await expect( client.page.locator(`[data-worktree-id=${JSON.stringify(worktreeId)}]`).first() ).toContainText('Done') @@ -531,13 +541,7 @@ test('recovers hidden remote completion and permission alerts exactly once', asy expect.objectContaining({ agentState: 'done', worktreeId }), expect.objectContaining({ agentState: 'waiting', worktreeId, attentionRequired: true }) ]) - await expect - .poll(() => unreadBadgeState(client!, worktreeId)) - .toMatchObject({ - unread: true, - unreadCount: 1, - dock: process.platform === 'darwin' ? '1' : null - }) + await expectUnreadBadgeRetained(client, worktreeId) await expect( client.page.locator(`[data-worktree-id=${JSON.stringify(worktreeId)}]`).first() ).toContainText('Needs permission')