diff --git a/src/renderer/src/components/activity/ActivityPrototypePage.tsx b/src/renderer/src/components/activity/ActivityPrototypePage.tsx index a6a1df6dc9f..623e41d4c89 100644 --- a/src/renderer/src/components/activity/ActivityPrototypePage.tsx +++ b/src/renderer/src/components/activity/ActivityPrototypePage.tsx @@ -1426,6 +1426,8 @@ export default function ActivityPrototypePage(): React.JSX.Element { const [groupBy, setGroupBy] = useState('status') const [query, setQuery] = useState('') const activityFilterInputRef = useRef(null) + // Why: bounds auto mark-read to one acknowledgement per selected thread turn. + const autoAcknowledgedTurnRef = useRef(null) const [compactMode, setCompactMode] = useState(false) const [selectedPaneKey, setSelectedPaneKey] = useState(null) const [displayedPaneKey, setDisplayedPaneKey] = useState(null) @@ -1731,11 +1733,20 @@ export default function ActivityPrototypePage(): React.JSX.Element { ) { return } + // Why (React #185): a turn stamped ahead of this clock (SSH/remote execution host) can never + // have its unread cleared, and each retry lands on a later millisecond, so acknowledgeAgents' + // `prev < now` guard rewrites the ack map every time and re-enters here forever through + // storeData. Auto-read is once per turn, not a retry. + const autoAcknowledgeKey = `${selectedThread.paneKey}:${selectedThread.latestTimestamp}` + if (autoAcknowledgedTurnRef.current === autoAcknowledgeKey) { + return + } const selectedThreadHasDetailOnlyView = !selectedHasLiveTab || selectedThread.migrationUnsupportedPtyId !== undefined const selectedThreadIsVisibleTerminal = visibleThread?.paneKey === effectiveSelectedPaneKey && visiblePortalReady if (selectedThreadHasDetailOnlyView || selectedThreadIsVisibleTerminal) { + autoAcknowledgedTurnRef.current = autoAcknowledgeKey storeData.acknowledgeAgents([selectedThread.paneKey]) } }, [ diff --git a/src/renderer/src/components/activity/activity-auto-mark-read-loop.react185.test.tsx b/src/renderer/src/components/activity/activity-auto-mark-read-loop.react185.test.tsx new file mode 100644 index 00000000000..9fdc4c0ecef --- /dev/null +++ b/src/renderer/src/components/activity/activity-auto-mark-read-loop.react185.test.tsx @@ -0,0 +1,224 @@ +/** @vitest-environment happy-dom */ +import { act } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { TooltipProvider } from '@/components/ui/tooltip' +import { useAppStore } from '@/store' +import { makePaneKey } from '../../../../shared/stable-pane-id' +import type { Repo } from '../../../../shared/repo-types' +import type { TerminalTab } from '../../../../shared/terminal-tab-types' +import type { Worktree } from '../../../../shared/worktree/types' +import ActivityPrototypePage from './ActivityPrototypePage' + +;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + +const LEAF_A = 'aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaa1' +const TAB_A = 'tab-a' +const PANE_A = makePaneKey(TAB_A, LEAF_A) +const PROMPT = 'turn stamped by the execution host' + +const repo: Repo = { + id: 'repo-1', + path: '/repo', + displayName: 'Repo', + badgeColor: '#000', + addedAt: 1 +} + +const worktree: Worktree = { + id: 'wt-1', + repoId: repo.id, + path: '/repo/wt-1', + head: 'abc123', + branch: 'feature', + isBare: false, + isMainWorktree: false, + displayName: 'feature', + comment: '', + linkedIssue: null, + linkedPR: null, + linkedLinearIssue: null, + isArchived: false, + isUnread: false, + isPinned: false, + sortOrder: 0, + lastActivityAt: 1 +} + +const tab: TerminalTab = { + id: TAB_A, + ptyId: 'pty-a', + worktreeId: worktree.id, + title: 'Claude', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 +} + +const initialState = useAppStore.getInitialState() +let root: Root +let seededContainer: HTMLElement + +beforeEach(() => { + useAppStore.setState(initialState, true) +}) + +afterEach(() => { + act(() => root?.unmount()) + useAppStore.setState(initialState, true) + document.body.replaceChildren() +}) + +function seedRetainedThread(stampedAt: number, lastAssistantMessage: string): void { + useAppStore.setState({ + repos: [repo], + worktreesByRepo: { [repo.id]: [worktree] }, + tabsByWorktree: { [worktree.id]: [] }, + retainedAgentsByPaneKey: { + [PANE_A]: { + entry: { + state: 'done', + prompt: PROMPT, + updatedAt: stampedAt, + stateStartedAt: stampedAt, + paneKey: PANE_A, + terminalTitle: 'Claude', + stateHistory: [], + agentType: 'claude', + lastAssistantMessage + }, + worktreeId: worktree.id, + tab, + agentType: 'claude', + startedAt: stampedAt + } + }, + activeRepoId: repo.id, + activeWorktreeId: worktree.id + }) +} + +function seedThreadStampedByLocalClock(): void { + seedRetainedThread(Date.now() - 60_000, 'finished locally') +} + +// Why: an SSH/remote execution host stamps the turn with its own clock, so the renderer can +// acknowledge a thread whose event still sorts as newer than the acknowledgement. +function seedThreadStampedAheadOfLocalClock(): void { + seedRetainedThread(Date.now() + 60_000, 'finished on the execution host') +} + +async function mountActivityPage(): Promise { + const container = document.createElement('div') + document.body.append(container) + root = createRoot(container) + await act(async () => { + root.render( + + + + ) + await new Promise((resolve) => setTimeout(resolve, 30)) + }) + seededContainer = container +} + +async function selectSeededThread(): Promise { + const row = Array.from(seededContainer.querySelectorAll('[role="button"]')).find( + (element) => element.textContent?.includes(PROMPT) + ) + expect(row).toBeDefined() + await act(async () => { + row?.click() + await new Promise((resolve) => setTimeout(resolve, 30)) + }) +} + +function countAcknowledgeWrites(): { readonly count: () => number; stop: () => void } { + let writes = 0 + const unsubscribe = useAppStore.subscribe((next, prev) => { + if (next.acknowledgedAgentsByPaneKey !== prev.acknowledgedAgentsByPaneKey) { + writes += 1 + } + }) + return { count: () => writes, stop: unsubscribe } +} + +describe('Activity auto mark-read loop (React #185)', () => { + it('acknowledges a selected thread once even when its unread flag cannot clear', async () => { + seedThreadStampedAheadOfLocalClock() + await mountActivityPage() + const acknowledgeWrites = countAcknowledgeWrites() + + try { + await selectSeededThread() + } finally { + acknowledgeWrites.stop() + } + + expect(acknowledgeWrites.count()).toBe(1) + }) + + it('re-acknowledges when a new turn lands on the still-selected thread', async () => { + seedThreadStampedAheadOfLocalClock() + await mountActivityPage() + const acknowledgeWrites = countAcknowledgeWrites() + + try { + await selectSeededThread() + expect(acknowledgeWrites.count()).toBe(1) + + // Why: the per-turn guard must not swallow the next turn's auto mark-read. + const nextTurnAt = Date.now() + 120_000 + await act(async () => { + useAppStore.setState((s) => { + const previous = s.retainedAgentsByPaneKey[PANE_A] + return { + retainedAgentsByPaneKey: { + [PANE_A]: { + ...previous, + startedAt: nextTurnAt, + entry: { + ...previous.entry, + updatedAt: nextTurnAt, + stateStartedAt: nextTurnAt, + lastAssistantMessage: 'second turn on the execution host' + } + } + } + } + }) + await new Promise((resolve) => setTimeout(resolve, 50)) + }) + } finally { + acknowledgeWrites.stop() + } + + expect(acknowledgeWrites.count()).toBe(2) + }) + + it('still marks a locally stamped thread read on selection', async () => { + seedThreadStampedByLocalClock() + await mountActivityPage() + await selectSeededThread() + + expect(useAppStore.getState().acknowledgedAgentsByPaneKey[PANE_A]).toBeGreaterThan(0) + }) + + it('leaves the selected thread unread after the user marks it unread', async () => { + seedThreadStampedByLocalClock() + await mountActivityPage() + await selectSeededThread() + + // Why pinned: auto mark-read is once per turn, so an explicit mark-unread on the + // still-selected thread must survive — a guard keyed on unread instead of the turn + // would silently re-acknowledge it and make the menu action a no-op. + await act(async () => { + useAppStore.getState().unacknowledgeAgents([PANE_A]) + await new Promise((resolve) => setTimeout(resolve, 50)) + }) + + expect(useAppStore.getState().acknowledgedAgentsByPaneKey[PANE_A]).toBeUndefined() + }) +}) diff --git a/src/renderer/src/components/sidebar/WorktreeContextMenu.react185-bystander.test.tsx b/src/renderer/src/components/sidebar/WorktreeContextMenu.react185-bystander.test.tsx new file mode 100644 index 00000000000..da48eb26ae0 --- /dev/null +++ b/src/renderer/src/components/sidebar/WorktreeContextMenu.react185-bystander.test.tsx @@ -0,0 +1,234 @@ +/** + * @vitest-environment happy-dom + * + * Reproduces the shape of the production React #185 cluster filed under + * boundary_id `sidebar.worktrees` (component stack: RovingFocusGroupItem inside + * DropdownMenuItem inside the worktree row's context menu). + * + * The point of the test is that the menu is a BYSTANDER: it contains no loop of + * its own (first case), and it only takes the blame because Radix's roving-focus + * item runs `onFocusableItemAdd()` — a setState — from a mount LAYOUT effect, + * which is the next dispatch after an unrelated driver has already pushed + * React's root-global nested-update counter past its limit (second case). + */ +import React, { act, useLayoutEffect, useState } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { Worktree } from '../../../../shared/worktree/types' +import { REACT_NESTED_UPDATE_LIMIT } from '../../../../shared/react-update-depth-attribution' +import { TooltipProvider } from '@/components/ui/tooltip' +import WorktreeContextMenu from './WorktreeContextMenu' + +globalThis.IS_REACT_ACT_ENVIRONMENT = true + +const state = { + updateWorktreeMeta: vi.fn(), + setWorktreesPinnedAndReveal: vi.fn(), + workspaceStatuses: [ + { id: 'todo', label: 'Todo' }, + { id: 'doing', label: 'Doing' } + ], + openModal: vi.fn(), + projectGroups: [{ id: 'g1', name: 'Group 1' }], + createProjectGroup: vi.fn(), + moveProjectToGroup: vi.fn(), + deleteStateByWorktreeId: {}, + worktreeLineageById: {}, + workspaceLineageByChildKey: {}, + updateWorktreeLineage: vi.fn(), + tabsByWorktree: {}, + ptyIdsByTabId: {}, + browserTabsByWorktree: {}, + keybindings: {}, + settings: { activeRuntimeEnvironmentId: null, openInApplications: [] }, + openSettingsPage: vi.fn(), + openSettingsTarget: vi.fn() +} + +vi.mock('@/store', () => ({ + useAppStore: Object.assign((selector: (s: typeof state) => unknown) => selector(state), { + getState: () => state + }) +})) + +vi.mock('@/store/selectors', () => ({ + useAllWorktrees: () => [], + useRepoById: (repoId?: string) => + repoId ? { id: repoId, name: repoId, displayName: repoId, projectGroupId: null } : undefined, + useRepoMap: () => new Map(), + useWorktreeMap: () => new Map() +})) + +vi.mock('@/i18n/i18n', () => ({ + translate: (_key: string, fallback: string) => fallback, + i18n: { language: 'en', on: () => {}, off: () => {} } +})) + +vi.mock('./ProjectGroupNameDialog', () => ({ ProjectGroupNameDialog: () => null })) +vi.mock('./WorktreeParentPickerPopover', () => ({ WorktreeParentPickerPopover: () => null })) + +const mounted: { container: HTMLDivElement; root: Root }[] = [] + +afterEach(() => { + for (const { root, container } of mounted) { + act(() => root.unmount()) + container.remove() + } + mounted.length = 0 +}) + +function worktreeFixture(): Worktree { + return { + id: 'repo::wt-1', + repoId: 'repo', + name: 'wt-1', + displayName: 'wt-1', + path: '/path/to/wt-1', + isMainWorktree: false + } as unknown as Worktree +} + +type Capture = { errors: Error[]; componentStacks: string[] } + +class SidebarBoundary extends React.Component< + { children: React.ReactNode; capture: Capture }, + { failed: boolean } +> { + state = { failed: false } + static getDerivedStateFromError(): { failed: boolean } { + return { failed: true } + } + componentDidCatch(error: Error, info: React.ErrorInfo): void { + this.props.capture.errors.push(error) + this.props.capture.componentStacks.push(info.componentStack ?? '') + } + render(): React.ReactNode { + return this.state.failed ? null : this.props.children + } +} + +function mount(node: React.ReactNode, capture: Capture): HTMLDivElement { + const container = document.createElement('div') + document.body.appendChild(container) + const root = createRoot(container, { + onUncaughtError: (error) => capture.errors.push(error as Error), + onCaughtError: () => undefined + }) + mounted.push({ container, root }) + act(() => { + root.render( + + {node} + + ) + }) + return container +} + +/** + * Stands in for whatever unrelated sidebar/app code kept sync lanes pending + * commit after commit in the field report. It is the driver, not the menu. + */ +function CommitCascadeDriver({ + ticks, + onTick +}: { + ticks: number + onTick: () => void +}): React.JSX.Element { + useLayoutEffect(() => { + if (ticks < REACT_NESTED_UPDATE_LIMIT + 4) { + onTick() + } + }) + return
{ticks}
+} + +describe('WorktreeContextMenu and React #185', () => { + it('opening the row context menu on its own does not trip the nested-update limit', () => { + const capture: Capture = { errors: [], componentStacks: [] } + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => undefined) + const container = mount( + +
Card
+
, + capture + ) + + const scope = container.querySelector('[data-worktree-context-menu-scope]') as HTMLElement + act(() => { + scope.dispatchEvent( + new MouseEvent('contextmenu', { bubbles: true, cancelable: true, clientX: 10, clientY: 10 }) + ) + }) + consoleError.mockRestore() + + expect(document.querySelector('[data-slot="dropdown-menu-content"]')).toBeTruthy() + expect(capture.errors.map((error) => error.message)).toEqual([]) + }) + + // Reproduces the field report: the driver is elsewhere, the menu is blamed. + it('is blamed for a driver-owned cascade when its items mount past the limit', () => { + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => undefined) + const blamedFrames: string[] = [] + + // Which commit the right-click lands on decides which mounting fiber + // dispatches first past the limit; sweep the window around it rather than + // pinning one React-version-specific offset. + for ( + let openAt = REACT_NESTED_UPDATE_LIMIT - 6; + openAt <= REACT_NESTED_UPDATE_LIMIT; + openAt++ + ) { + const capture: Capture = { errors: [], componentStacks: [] } + + function Harness(): React.JSX.Element { + const [ticks, setTicks] = useState(0) + const hostRef = React.useRef(null) + // Right-click lands mid-cascade, so the menu content — and every + // RovingFocusGroupItem inside it — mounts while the counter is already deep. + useLayoutEffect(() => { + if (ticks !== openAt) { + return + } + hostRef.current + ?.querySelector('[data-worktree-context-menu-scope]') + ?.dispatchEvent(new MouseEvent('contextmenu', { bubbles: true, cancelable: true })) + }, [ticks]) + return ( + <> +
+ +
Card
+
+
+ setTicks((value) => value + 1)} /> + + ) + } + + mount(, capture) + + const depthErrors = capture.errors.filter((error) => + error.message.includes('Maximum update depth exceeded') + ) + expect(depthErrors.length).toBeGreaterThan(0) + blamedFrames.push(capture.componentStacks[0]?.split('\n')[1]?.trim() ?? '') + } + consoleError.mockRestore() + + // Carried as the assertion message so a React/Radix version bump reports the + // frames it blamed instead of a bare `false`. + const blamed = blamedFrames.map((frame) => frame.split(' (')[0]).join(', ') + // The production report's innermost component_stack frame. + expect( + blamedFrames.some((frame) => frame.startsWith('at RovingFocusGroupItem')), + blamed + ).toBe(true) + // ...and the loop is never in menu code: only the driver owns a setState loop. + expect( + blamedFrames.some((frame) => frame.startsWith('at CommitCascadeDriver')), + blamed + ).toBe(true) + }) +}) diff --git a/src/renderer/src/components/tab-bar/SortableTab.update-depth-probe.test.tsx b/src/renderer/src/components/tab-bar/SortableTab.update-depth-probe.test.tsx new file mode 100644 index 00000000000..acbd7ce38bc --- /dev/null +++ b/src/renderer/src/components/tab-bar/SortableTab.update-depth-probe.test.tsx @@ -0,0 +1,212 @@ +// @vitest-environment happy-dom + +/** + * Pins the settling guarantee for every setState-in-effect path SortableTab owns + * (rename shortcut, title churn mid-rename, store write storms, StrictMode double + * invoke) under a real React root, where "Maximum update depth exceeded" throws. + * + * Written while triaging the React #185 crash cluster whose component_stack names + * SortableTab: these paths settle, which matches shared/react-update-depth-attribution.ts + * — #185 lands on whichever fiber dispatched next after a root-global counter tripped, + * so that stack names a bystander. Keep this green so the tab stays exonerated. + */ +import type { ReactElement, ReactNode } from 'react' +import { cloneElement, isValidElement, StrictMode, useEffect, useState } from 'react' +import { act, render, cleanup } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { TerminalTab } from '../../../../shared/terminal-tab-types' +import type { TabDragItemData } from '../tab-group/useTabDragSplit' +import { useAppStore } from '../../store' +import SortableTab from './SortableTab' + +type ProbeState = { + unreadTerminalTabs: Record + unreadAgentCompletionPanes: Record + agentStatusByPaneKey: Record + agentStatusEpoch: number + runtimePaneTitlesByTabId: Record> + ptyIdsByTabId: Record + terminalLayoutsByTabId: Record + renamingTabId: string | null + keybindings: Record + setRenamingTabId: (tabId: string | null) => void +} + +type StoreApiWithHook = { + (selector: (state: ProbeState) => unknown): unknown + getState: () => ProbeState + setState: (partial: Partial) => void +} + +// Both specifiers resolve to the same store module; memoize so they share one instance. +async function createProbeStore(): Promise { + const globalKey = '__sortableTabProbeStore' + const globals = globalThis as Record + if (!globals[globalKey]) { + const { create } = await import('zustand') + globals[globalKey] = create((set) => ({ + unreadTerminalTabs: {}, + unreadAgentCompletionPanes: {}, + agentStatusByPaneKey: {}, + agentStatusEpoch: 0, + runtimePaneTitlesByTabId: {}, + ptyIdsByTabId: {}, + terminalLayoutsByTabId: {}, + renamingTabId: null, + keybindings: {}, + setRenamingTabId: (tabId) => set({ renamingTabId: tabId }) + })) + } + return globals[globalKey] as StoreApiWithHook +} + +vi.mock('@/store', async () => ({ useAppStore: await createProbeStore() })) +vi.mock('../../store', async () => ({ useAppStore: await createProbeStore() })) + +vi.mock('@dnd-kit/sortable', () => ({ + useSortable: ({ id }: { id: string }) => ({ + attributes: { role: 'tab', 'data-sortable-id': id }, + listeners: { onPointerDown: vi.fn() }, + setNodeRef: vi.fn() + }) +})) + +vi.mock('@/lib/use-tab-agent', () => ({ useTabAgent: () => null })) + +vi.mock('@/components/ui/tooltip', () => ({ + Tooltip: ({ children }: { children: ReactNode }) =>
{children}
, + TooltipContent: ({ children }: { children: ReactNode }) =>
{children}
, + TooltipTrigger: ({ children, asChild }: { children: ReactNode; asChild?: boolean }) => + asChild && isValidElement(children) ? ( + cloneElement(children as ReactElement>) + ) : ( + {children} + ) +})) + +vi.mock('@/components/ui/dropdown-menu', () => ({ + DropdownMenu: ({ children }: { children: ReactNode }) => <>{children}, + DropdownMenuContent: () => null, + DropdownMenuItem: ({ children }: { children?: ReactNode }) => <>{children}, + DropdownMenuSeparator: () => null, + DropdownMenuShortcut: ({ children }: { children?: ReactNode }) => <>{children}, + DropdownMenuLabel: ({ children }: { children?: ReactNode }) => <>{children}, + DropdownMenuSub: ({ children }: { children?: ReactNode }) => <>{children}, + DropdownMenuSubContent: ({ children }: { children?: ReactNode }) => <>{children}, + DropdownMenuSubTrigger: ({ children }: { children?: ReactNode }) => <>{children}, + DropdownMenuTrigger: ({ children }: { children: ReactNode }) => <>{children} +})) + +vi.mock('@/components/ui/input', () => ({ + Input: (props: Record) => +})) + +vi.mock('./shell-icons', () => ({ ShellIcon: () => })) +vi.mock('@/lib/agent-catalog', () => ({ AgentIcon: () => })) +vi.mock('../sidebar/WorktreeCardHelpers', () => ({ FilledBellIcon: () => })) + +function makeTab(overrides: Partial = {}): TerminalTab { + return { + id: 'terminal-tab-1', + title: 'Terminal 1', + worktreeId: 'wt-1', + ...overrides + } as TerminalTab +} + +const dragData: TabDragItemData = { + kind: 'tab', + worktreeId: 'wt-1', + groupId: 'group-1', + unifiedTabId: 'unified-1', + visibleTabId: 'terminal-tab-1', + tabType: 'terminal', + label: 'Terminal 1' +} + +const probeStore = useAppStore as unknown as StoreApiWithHook + +let renderCount = 0 + +function Harness({ tab }: { tab: TerminalTab }): ReactElement { + renderCount += 1 + return ( + + ) +} + +/** Re-renders the tab with a new tab object on every store write, like the tab strip does. */ +function ChurningHarness({ titles }: { titles: string[] }): ReactElement { + const [index, setIndex] = useState(0) + useEffect(() => { + if (index < titles.length - 1) { + setIndex(index + 1) + } + }, [index, titles.length]) + return +} + +afterEach(() => { + cleanup() + probeStore.setState({ renamingTabId: null, unreadTerminalTabs: {}, agentStatusEpoch: 0 }) + renderCount = 0 +}) + +describe('SortableTab update-depth probe', () => { + it('settles when the rename shortcut arms renamingTabId', () => { + probeStore.setState({ renamingTabId: 'terminal-tab-1' }) + const { container } = render() + expect(container.querySelector('[data-tab-rename-input]')).not.toBeNull() + expect(probeStore.getState().renamingTabId).toBeNull() + expect(renderCount).toBeLessThan(20) + }) + + it('settles under title churn while the rename editor is open', () => { + probeStore.setState({ renamingTabId: 'terminal-tab-1' }) + render() + expect(renderCount).toBeLessThan(40) + }) + + it('settles under a store write storm', () => { + render() + const before = renderCount + act(() => { + for (let i = 0; i < 60; i += 1) { + probeStore.setState({ agentStatusEpoch: i, agentStatusByPaneKey: {} }) + } + }) + expect(renderCount - before).toBeLessThan(200) + }) + + it('settles in StrictMode double-invoked effects', () => { + probeStore.setState({ renamingTabId: 'terminal-tab-1' }) + render( + + + + ) + expect(probeStore.getState().renamingTabId).toBeNull() + }) +}) diff --git a/src/renderer/src/components/terminal-pane/terminal-cold-park-tab-model-identity.react185.test.tsx b/src/renderer/src/components/terminal-pane/terminal-cold-park-tab-model-identity.react185.test.tsx new file mode 100644 index 00000000000..87973f0d47a --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-cold-park-tab-model-identity.react185.test.tsx @@ -0,0 +1,233 @@ +/** @vitest-environment happy-dom */ +/** + * Crash cluster "React #185 in TerminalPaneOverlayLayer" (boundary terminal.workbench). + * + * The overlay's own park verdict does not oscillate (terminal-cold-park-verdict-loop + * covers that). What used to put it in the stack: its cold-park effect re-runs on + * every tab-model write — a runtime title publication, an unread bump re-mints + * tabsByWorktree — and each run dispatched setColdParkedTerminalTabIds even when + * the verdict was unchanged. React only bails on such a dispatch while the fiber + * has no pending lanes, so inside a cascade some other component drives, this hook + * is the one that trips the root-global nested-update counter and gets blamed (see + * src/shared/react-update-depth-attribution.ts). + */ +import { act, useEffect, useState, StrictMode } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { REACT_NESTED_UPDATE_LIMIT } from '../../../../shared/react-update-depth-attribution' +import type { Tab, TabGroup } from '../../../../shared/tab-types' +import type { TerminalTab } from '../../../../shared/terminal-tab-types' + +;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + +const harness = vi.hoisted(() => ({ + worktreeId: 'repo::/cold-park-tab-model-identity', + parkEffectRuns: 0, + renderedParkedSets: [] as string[][] +})) + +vi.mock('../../store', async () => { + const { create } = await import('zustand') + const useAppStore = create(() => ({ + activeGroupIdByWorktree: {} as Record, + groupsByWorktree: {} as Record, + pendingStartupByTabId: {} as Record, + ptyIdsByTabId: {} as Record, + runtimeStatusByEnvironmentId: new Map(), + runtimePaneTitlesByTabId: {} as Record>, + settings: {} as Record, + sleepingAgentSessionsByPaneKey: {} as Record, + tabsByWorktree: {} as Record, + terminalLayoutsByTabId: {} as Record, + unifiedTabsByWorktree: {} as Record, + consumeSuppressedPtyExit: () => false, + focusGroup: () => {}, + reconcileWorktreeTabModel: () => ({ renderableTabCount: 2 }), + setActiveWorktree: () => {} + })) + return { useAppStore } +}) + +vi.mock('../native-chat/use-native-chat-toggle-shortcut', () => ({ + useNativeChatToggleShortcut: () => {} +})) + +vi.mock('./TerminalOverlaySlot', () => ({ TerminalOverlaySlot: () => null })) + +vi.mock('./terminal-parked-tab-watchers', () => ({ + canWatcherCoverParkedTerminalTab: () => true, + disposeParkedTerminalWatchersForWorktree: () => {}, + syncParkedTerminalTabWatchers: (args: { parkedTabIds: ReadonlySet }) => { + harness.renderedParkedSets.push([...args.parkedTabIds].sort()) + } +})) + +// Counts cold-park effect executions: the effect clears and re-arms its recheck +// timers on every run, so this is also the per-commit work the identity dep costs. +vi.mock('./terminal-parking-e2e-overrides', () => ({ + getTerminalParkingPolicyOverrides: () => { + harness.parkEffectRuns += 1 + return { coldParkDelayMs: 0, hotRetainMs: 0 } + } +})) + +vi.mock('@/lib/crash-breadcrumb-recorder', () => ({ recordRendererCrashBreadcrumb: vi.fn() })) + +import { useAppStore } from '../../store' +import TerminalPaneOverlayLayer from './TerminalPaneOverlayLayer' + +const TAB_IDS = ['tab-a', 'tab-b'] as const +const GROUP_ID = 'group-a' +/** Above React's nested-update limit so the cascade actually reaches the bail. */ +const CASCADE_COMMITS = REACT_NESTED_UPDATE_LIMIT * 2 + +type ParkingStoreState = { + groupsByWorktree: Record + tabsByWorktree: Record + unifiedTabsByWorktree: Record +} + +const parkingStore = useAppStore as unknown as { + getState: () => ParkingStoreState + setState: (partial: unknown) => void +} + +function terminalTab(id: string): TerminalTab { + return { + id, + worktreeId: harness.worktreeId, + ptyId: `${harness.worktreeId}@@session-${id}`, + title: id, + generation: 0 + } as TerminalTab +} + +function unifiedTerminalTab(id: string): Tab { + return { + id: `unified-${id}`, + entityId: id, + worktreeId: harness.worktreeId, + groupId: GROUP_ID, + contentType: 'terminal', + label: id, + customLabel: null, + color: null, + sortOrder: 0, + createdAt: 0 + } as Tab +} + +/** An ordinary runtime title publication: re-mints tabsByWorktree, changes no park input. */ +function publishRuntimeTitle(revision: number): void { + parkingStore.setState((state: ParkingStoreState) => ({ + tabsByWorktree: { + ...state.tabsByWorktree, + [harness.worktreeId]: state.tabsByWorktree[harness.worktreeId].map((tab) => ({ + ...tab, + title: `${tab.id}-${revision}` + })) + } + })) +} + +/** + * The cascade driver: a component with its own runaway passive effect, in no way + * related to terminal parking. It is the bug; the overlay is the bystander. + */ +function UnrelatedCascadeDriver(): null { + const [tick, setTick] = useState(0) + useEffect(() => { + if (tick >= CASCADE_COMMITS) { + return + } + publishRuntimeTitle(tick) + setTick((current) => current + 1) + }, [tick]) + return null +} + +function renderTree(root: Root): unknown { + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}) + let error: unknown = null + try { + act(() => { + root.render( + + + + + ) + }) + } catch (thrown) { + error = thrown + } + consoleError.mockRestore() + return error +} + +describe('cold-park effect vs. unrelated commit cascade', () => { + let container: HTMLDivElement + let root: Root | undefined + beforeEach(() => { + harness.parkEffectRuns = 0 + harness.renderedParkedSets.length = 0 + const tabs = TAB_IDS.map(terminalTab) + const unifiedTabs = TAB_IDS.map(unifiedTerminalTab) + parkingStore.setState({ + activeGroupIdByWorktree: { [harness.worktreeId]: GROUP_ID }, + groupsByWorktree: { + [harness.worktreeId]: [ + { + id: GROUP_ID, + worktreeId: harness.worktreeId, + activeTabId: unifiedTabs[0].id, + tabOrder: unifiedTabs.map((tab) => tab.id), + recentTabIds: [unifiedTabs[0].id] + } as TabGroup + ] + }, + tabsByWorktree: { [harness.worktreeId]: tabs }, + unifiedTabsByWorktree: { [harness.worktreeId]: unifiedTabs } + }) + container = document.createElement('div') + document.body.appendChild(container) + }) + + afterEach(() => { + try { + act(() => root?.unmount()) + } catch { + /* a failed commit leaves no mounted tree */ + } + root = undefined + container.remove() + }) + + it('dispatches no park-state update for tab-model writes that change no park verdict', () => { + root = createRoot(container) + const error = renderTree(root) + + // The park verdict itself never churns: one empty -> parked transition, + // matching the field bundle, which carries no terminal_park_verdict_churn crumb. + expect(harness.renderedParkedSets.at(-1)).toEqual(['tab-b']) + expect(new Set(harness.renderedParkedSets.map((ids) => ids.join(',')))).toEqual( + new Set(['', 'tab-b']) + ) + + // React #185 still fires — the driver is the bug — but it must land on the + // driver's own dispatch, not on the overlay's park state. Asserting the throw + // happened keeps the "not blamed" check below from passing on an empty stack. + expect(error instanceof Error ? error.message : '').toContain('Maximum update depth exceeded') + const stack = error instanceof Error ? (error.stack ?? '') : String(error ?? '') + expect(stack).not.toContain('use-terminal-tab-cold-parking') + expect(stack).toContain('terminal-cold-park-tab-model-identity') + // The effect itself still re-runs — coverage is re-derived from store state + // the park key cannot encode — it just stops dispatching an unchanged verdict. + expect(harness.parkEffectRuns).toBeGreaterThan(REACT_NESTED_UPDATE_LIMIT) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/use-terminal-tab-cold-parking.ts b/src/renderer/src/components/terminal-pane/use-terminal-tab-cold-parking.ts index 76434e024c1..0f25b61830e 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-tab-cold-parking.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-tab-cold-parking.ts @@ -138,6 +138,8 @@ export function useTerminalTabColdParking(args: { const [coldParkedTerminalTabIds, setColdParkedTerminalTabIds] = useState>( () => new Set() ) + // Mirrors the committed park set; written only from the post-commit effect below. + const coldParkedTerminalTabIdsRef = useRef(coldParkedTerminalTabIds) useEffect(() => { const timers = terminalTabParkingTimersRef.current @@ -224,9 +226,16 @@ export function useTerminalTabColdParking(args: { parkVerdictRecords: parkVerdictRecordsRef.current, nowMs }) - setColdParkedTerminalTabIds((current) => - haveSameTerminalTabIds(current, parkedTabIds) ? current : parkedTabIds - ) + // Why the ref and not the updater form: returning `current` still dispatches, + // and React only bails eagerly while the fiber has no pending lanes. This + // effect re-runs on every tab-model write (runtime titles, unread bumps), + // so inside any commit cascade the no-op dispatch was what tripped React's + // root-global nested-update counter — naming this hook in a #185 whose real + // driver is elsewhere (see src/shared/react-update-depth-attribution.ts). + if (!haveSameTerminalTabIds(coldParkedTerminalTabIdsRef.current, parkedTabIds)) { + coldParkedTerminalTabIdsRef.current = parkedTabIds + setColdParkedTerminalTabIds(parkedTabIds) + } for (const candidate of candidates) { if ( diff --git a/src/renderer/src/hooks/useAutoAckViewedAgent.clock-skew.test.ts b/src/renderer/src/hooks/useAutoAckViewedAgent.clock-skew.test.ts new file mode 100644 index 00000000000..d81c0c49ffa --- /dev/null +++ b/src/renderer/src/hooks/useAutoAckViewedAgent.clock-skew.test.ts @@ -0,0 +1,112 @@ +// @vitest-environment happy-dom + +import { cleanup, renderHook } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { useAutoAckViewedAgent } from './useAutoAckViewedAgent' +import { useAppStore } from '../store' +import { makeTab } from '../store/slices/store-test-helpers' +import { makePaneKey } from '../../../shared/stable-pane-id' +import type { AgentStatusEntry } from '../../../shared/agent-status-types' + +// Why: the hook re-scans on every store write and advances its diff refs to the PRE-write snapshot, +// so its ref guard never suppresses the rescan an ack triggers. It only terminated because +// acknowledgeAgents returned the same object within one millisecond — a scan costing >=1ms with a +// turn stamped ahead of the local clock (SSH/remote host) re-acked forever (React #185). + +const TAB_ID = 'tab-main' +const LEAF_ID = '11111111-1111-4111-8111-111111111111' +const PANE_KEY = makePaneKey(TAB_ID, LEAF_ID) +const NOW = new Date('2026-06-02T12:00:00Z').getTime() +const SKEW_MS = 90_000 +const ACK_CALL_CEILING = 20 + +function seedFutureStampedTurn(stateStartedAt: number): void { + const entry: AgentStatusEntry = { + state: 'done', + prompt: 'remote turn', + updatedAt: stateStartedAt, + stateStartedAt, + agentType: 'codex', + paneKey: PANE_KEY, + stateHistory: [] + } + useAppStore.setState({ + activeView: 'terminal', + activeTabId: TAB_ID, + activeWorktreeId: 'wt-1', + activeTabIdByWorktree: {}, + tabsByWorktree: { 'wt-1': [makeTab({ id: TAB_ID, worktreeId: 'wt-1' })] }, + terminalLayoutsByTabId: { + [TAB_ID]: { root: null, activeLeafId: LEAF_ID, expandedLeafId: null } + }, + agentStatusByPaneKey: { [PANE_KEY]: entry }, + retainedAgentsByPaneKey: {}, + acknowledgedAgentsByPaneKey: {}, + unreadAgentCompletionPanes: {}, + unreadTerminalTabs: {} + }) +} + +/** Wraps the real action, then circuit-breaks so a regression fails the assertion instead of hanging. */ +function instrumentAcknowledgeAgents(): string[][] { + const real = useAppStore.getState().acknowledgeAgents + const calls: string[][] = [] + useAppStore.setState({ + acknowledgeAgents: (paneKeys: string[]) => { + calls.push(paneKeys) + if (calls.length > ACK_CALL_CEILING) { + return + } + real(paneKeys) + } + }) + return calls +} + +describe('useAutoAckViewedAgent — clock-skewed execution host', () => { + let realAcknowledgeAgents: (paneKeys: string[]) => void + + beforeEach(() => { + realAcknowledgeAgents = useAppStore.getState().acknowledgeAgents + vi.spyOn(document, 'hasFocus').mockReturnValue(true) + // Why a ticking clock: the loop self-terminated only while Date.now() stayed on one millisecond. + let clock = NOW + vi.spyOn(Date, 'now').mockImplementation(() => clock++) + }) + + afterEach(() => { + cleanup() + vi.restoreAllMocks() + useAppStore.setState({ acknowledgeAgents: realAcknowledgeAgents }) + }) + + it('acks a turn stamped ahead of the local clock exactly once', () => { + seedFutureStampedTurn(NOW + SKEW_MS) + const calls = instrumentAcknowledgeAgents() + + renderHook(() => useAutoAckViewedAgent(false)) + + expect(calls).toEqual([[PANE_KEY]]) + const ackAt = useAppStore.getState().acknowledgedAgentsByPaneKey[PANE_KEY] ?? 0 + expect(ackAt).toBeGreaterThanOrEqual(NOW + SKEW_MS) + }) + + it('leaves nothing to re-scan for a future-stamped turn on the next store write', () => { + seedFutureStampedTurn(NOW + SKEW_MS) + renderHook(() => useAutoAckViewedAgent(false)) + + const calls = instrumentAcknowledgeAgents() + useAppStore.getState().markTerminalTabUnread('tab-unrelated') + + expect(calls).toEqual([]) + }) + + it('still acks a normally stamped turn once', () => { + seedFutureStampedTurn(NOW - 5_000) + const calls = instrumentAcknowledgeAgents() + + renderHook(() => useAutoAckViewedAgent(false)) + + expect(calls).toEqual([[PANE_KEY]]) + }) +}) diff --git a/src/renderer/src/lib/use-tab-agent-observed-signal-dispatch.test.tsx b/src/renderer/src/lib/use-tab-agent-observed-signal-dispatch.test.tsx new file mode 100644 index 00000000000..e7e6ec0012d --- /dev/null +++ b/src/renderer/src/lib/use-tab-agent-observed-signal-dispatch.test.tsx @@ -0,0 +1,148 @@ +/** @vitest-environment happy-dom */ +/** + * Crash cluster "React #185 whose component_stack names SortableTab" + * (boundary terminal.workbench, app 1.4.190). + * + * useTabAgent's signal effect re-runs on every tab-model write that touches the + * title — a working agent republishes an OSC spinner frame many times a second, + * re-minting tabsByWorktree — and it used to dispatch setHasObservedAgentSignal(true) + * on every run, including the overwhelming majority where the flag was already true. + * That dispatch costs a second render+commit of SortableTab per title frame, and it + * puts a dispatchSetState on SortableTab's fiber inside commitHookEffectListMount — + * the exact frame the #185 reports blame (see shared/react-update-depth-attribution.ts: + * the throw lands on whoever dispatches next after a root-global counter trips). + * + * Same defect and same remedy as the cold-park fix in + * terminal-cold-park-tab-model-identity.react185.test.tsx. + */ +import { act } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { useAppStore } from '@/store' +import type { AgentStatusEntry } from '../../../shared/agent-status-types' +import { makePaneKey } from '../../../shared/stable-pane-id' +import type { TerminalLayoutSnapshot, TerminalTab } from '../../../shared/terminal-tab-types' +import type { TuiAgent } from '../../../shared/tui-agent' +import { useTabAgent } from './use-tab-agent' + +;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + +const initialAppState = useAppStore.getInitialState() +const WORKTREE_ID = 'repo::/tab-agent-observed-signal' +const TAB_ID = 'terminal-tab-observed-signal' +const LEAF_ID = '11111111-1111-4111-8111-111111111111' +const PANE_KEY = makePaneKey(TAB_ID, LEAF_ID) +const TITLE_PUBLICATIONS = 10 + +function paneLayout(ptyId: string): TerminalLayoutSnapshot { + return { + root: null, + activeLeafId: LEAF_ID, + expandedLeafId: null, + ptyIdsByLeafId: { [LEAF_ID]: ptyId } + } +} + +function workingAgentStatus(): AgentStatusEntry { + return { + state: 'working', + prompt: '', + updatedAt: Date.now(), + stateStartedAt: Date.now(), + agentType: 'codex', + paneKey: PANE_KEY, + stateHistory: [] + } as AgentStatusEntry +} + +function terminalTab(title: string): TerminalTab { + return { + id: TAB_ID, + worktreeId: WORKTREE_ID, + ptyId: 'pty-a', + title, + generation: 0 + } as TerminalTab +} + +/** An ordinary agent spinner frame: re-mints tabsByWorktree, changes no agent signal. */ +function publishRuntimeTitle(revision: number): void { + useAppStore.setState( + (state) => + ({ + tabsByWorktree: { + ...state.tabsByWorktree, + [WORKTREE_ID]: [terminalTab(`⠋ codex ${revision}`)] + } + }) as never + ) +} + +let probeRenders = 0 +let latestAgent: TuiAgent | null = null + +/** Stands in for SortableTab, useTabAgent's only production caller. */ +function TabAgentProbe(): null { + probeRenders += 1 + const tab = useAppStore((state) => state.tabsByWorktree[WORKTREE_ID]?.[0]) as TerminalTab + latestAgent = useTabAgent(tab) + return null +} + +describe('useTabAgent observed-signal dispatch', () => { + let container: HTMLDivElement + let root: Root | undefined + + beforeEach(() => { + useAppStore.setState({ + agentStatusByPaneKey: { [PANE_KEY]: workingAgentStatus() }, + terminalLayoutsByTabId: { [TAB_ID]: paneLayout('pty-a') }, + ptyIdsByTabId: { [TAB_ID]: ['pty-a'] }, + tabsByWorktree: { [WORKTREE_ID]: [terminalTab('⠋ codex 0')] } + } as never) + container = document.createElement('div') + document.body.appendChild(container) + probeRenders = 0 + latestAgent = null + root = createRoot(container) + act(() => root!.render()) + }) + + afterEach(() => { + act(() => root?.unmount()) + root = undefined + container.remove() + useAppStore.setState(initialAppState, true) + }) + + it('schedules no extra commit for a title publication that changes no agent signal', () => { + probeRenders = 0 + for (let revision = 1; revision <= TITLE_PUBLICATIONS; revision += 1) { + act(() => publishRuntimeTitle(revision)) + } + // One commit per publication. Re-dispatching the unchanged observed-signal + // flag doubled this, and put SortableTab's fiber in every #185 stack. + expect(probeRenders).toBe(TITLE_PUBLICATIONS) + expect(latestAgent).toBe('codex') + }) + + it('still re-arms the observed signal after a pty respawn', () => { + act(() => { + useAppStore.setState({ + agentStatusByPaneKey: {}, + terminalLayoutsByTabId: { [TAB_ID]: paneLayout('pty-b') }, + ptyIdsByTabId: { [TAB_ID]: ['pty-b'] }, + tabsByWorktree: { [WORKTREE_ID]: [terminalTab('zsh')] } + } as never) + }) + expect(latestAgent).toBeNull() + + act(() => { + useAppStore.setState({ + agentStatusByPaneKey: { [PANE_KEY]: workingAgentStatus() }, + tabsByWorktree: { [WORKTREE_ID]: [terminalTab('⠋ codex 1')] } + } as never) + }) + expect(latestAgent).toBe('codex') + }) +}) diff --git a/src/renderer/src/lib/use-tab-agent.ts b/src/renderer/src/lib/use-tab-agent.ts index b6c88400632..fef059ad616 100644 --- a/src/renderer/src/lib/use-tab-agent.ts +++ b/src/renderer/src/lib/use-tab-agent.ts @@ -284,7 +284,13 @@ export function useTabAgent(tab: TerminalTab): TuiAgent | null { ? explicitTitleAgent === tab.launchAgent : Boolean(explicitTitleAgent || siblingHookAgent) // Why: a recognized foreground process arms exit clearing even for agents with no hook or title integration. - if (focusedHookAgent || completedHookEvidence || processAgent || fallbackAgentSignal) { + // Why the ref gate: this effect re-runs on every title frame, and re-dispatching an + // already-true flag costs SortableTab a second commit each time — and names its fiber + // in #185 stacks driven elsewhere (see shared/react-update-depth-attribution.ts). + if ( + !hasObservedAgentSignalRef.current && + (focusedHookAgent || completedHookEvidence || processAgent || fallbackAgentSignal) + ) { hasObservedAgentSignalRef.current = true setHasObservedAgentSignal(true) } diff --git a/src/renderer/src/store/slices/ui-acknowledge-agents-clock-skew.test.ts b/src/renderer/src/store/slices/ui-acknowledge-agents-clock-skew.test.ts new file mode 100644 index 00000000000..669ebceb53e --- /dev/null +++ b/src/renderer/src/store/slices/ui-acknowledge-agents-clock-skew.test.ts @@ -0,0 +1,144 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { createUIStore } from './ui-slice-test-harness' +import type { AppState } from '../types' +import type { AgentStatusEntry } from '../../../../shared/agent-status-types' +import type { TerminalTab } from '../../../../shared/terminal-tab-types' +import { makePaneKey } from '../../../../shared/stable-pane-id' + +// Why: an SSH/remote execution host stamps turns with ITS clock. When that clock runs ahead, every +// unread rule (`ackAt < turnTimestamp`) stayed true after an ack, so the row could never be marked +// read and its auto-ack effect re-fired on each new millisecond — the React #185 update loop. + +const NOW = new Date('2026-06-02T12:00:00Z').getTime() +const SKEW_MS = 90_000 +const LEAF_ID = '11111111-1111-4111-8111-111111111111' +const PANE_KEY = makePaneKey('tab-1', LEAF_ID) + +function makeAgentEntry(overrides: Partial = {}): AgentStatusEntry { + return { + state: 'done', + prompt: 'Review complete', + updatedAt: NOW, + stateStartedAt: NOW, + agentType: 'codex', + paneKey: PANE_KEY, + stateHistory: [], + ...overrides + } +} + +function makeTerminalTab(id: string, worktreeId: string): TerminalTab { + return { + id, + worktreeId, + ptyId: null, + title: 'Terminal', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: NOW + } +} + +describe('acknowledgeAgents with a clock-skewed execution host', () => { + beforeEach(() => { + vi.useFakeTimers() + vi.setSystemTime(NOW) + }) + + afterEach(() => { + vi.useRealTimers() + }) + + it('clears unread for a live turn stamped ahead of the local clock in one acknowledge', () => { + const store = createUIStore() + const futureStartedAt = NOW + SKEW_MS + store.setState({ + tabsByWorktree: { 'wt-1': [makeTerminalTab('tab-1', 'wt-1')] }, + agentStatusByPaneKey: { + [PANE_KEY]: makeAgentEntry({ stateStartedAt: futureStartedAt, updatedAt: futureStartedAt }) + } + } as Partial) + + store.getState().acknowledgeAgents([PANE_KEY]) + + const ackAt = store.getState().acknowledgedAgentsByPaneKey[PANE_KEY] + expect(ackAt).toBe(futureStartedAt) + expect(ackAt < futureStartedAt).toBe(false) + }) + + it('stops rewriting the ack map once a future-stamped turn is acknowledged', () => { + const store = createUIStore() + const futureStartedAt = NOW + SKEW_MS + store.setState({ + agentStatusByPaneKey: { + [PANE_KEY]: makeAgentEntry({ stateStartedAt: futureStartedAt, updatedAt: futureStartedAt }) + } + } as Partial) + + store.getState().acknowledgeAgents([PANE_KEY]) + const firstMap = store.getState().acknowledgedAgentsByPaneKey + + // Later millisecond, same turn: the retry that used to allocate a new map (and re-render forever). + vi.setSystemTime(NOW + 1_000) + store.getState().acknowledgeAgents([PANE_KEY]) + + expect(store.getState().acknowledgedAgentsByPaneKey).toBe(firstMap) + }) + + it('covers a retained row and a future history event on the same pane', () => { + const store = createUIStore() + const futureHistoryAt = NOW + SKEW_MS + store.setState({ + retainedAgentsByPaneKey: { + [PANE_KEY]: { + entry: makeAgentEntry({ + stateStartedAt: NOW + 1_000, + stateHistory: [{ state: 'blocked', prompt: 'p', startedAt: futureHistoryAt }] + }), + worktreeId: 'wt-1', + tab: makeTerminalTab('tab-1', 'wt-1'), + agentType: 'codex', + startedAt: NOW + 1_000 + } + } + } as Partial) + + store.getState().acknowledgeAgents([PANE_KEY]) + + expect(store.getState().acknowledgedAgentsByPaneKey[PANE_KEY]).toBe(futureHistoryAt) + }) + + it('covers a future-stamped migration-unsupported row, which Activity renders as a blocked event', () => { + const store = createUIStore() + const futureUpdatedAt = NOW + SKEW_MS + store.setState({ + migrationUnsupportedByPtyId: { + 'pty-1': { + ptyId: 'pty-1', + paneKey: PANE_KEY, + reason: 'legacy-numeric-pane-key', + source: 'ssh', + updatedAt: futureUpdatedAt + } + } + } as Partial) + + store.getState().acknowledgeAgents([PANE_KEY]) + + expect(store.getState().acknowledgedAgentsByPaneKey[PANE_KEY]).toBe(futureUpdatedAt) + }) + + it('still stamps the local clock when the turn is not ahead of it', () => { + const store = createUIStore() + store.setState({ + agentStatusByPaneKey: { + [PANE_KEY]: makeAgentEntry({ stateStartedAt: NOW - 5_000, updatedAt: NOW - 5_000 }) + } + } as Partial) + + store.getState().acknowledgeAgents([PANE_KEY]) + + expect(store.getState().acknowledgedAgentsByPaneKey[PANE_KEY]).toBe(NOW) + }) +}) diff --git a/src/renderer/src/store/slices/ui.ts b/src/renderer/src/store/slices/ui.ts index 466e3db9724..9c4c550194f 100644 --- a/src/renderer/src/store/slices/ui.ts +++ b/src/renderer/src/store/slices/ui.ts @@ -372,6 +372,23 @@ function collectAcknowledgedAgentNotificationId({ } } +function usableTimestamp(value: unknown): number { + return typeof value === 'number' && Number.isFinite(value) && value > 0 ? value : 0 +} + +/** Newest turn timestamp an unread check can compare against for one agent row. */ +function latestAgentTurnTimestamp(entry: { + stateStartedAt?: number + stateHistory?: { startedAt?: number }[] +}): number { + let latest = usableTimestamp(entry.stateStartedAt) + // Why history too: Activity renders one event per stateHistory entry, each with its own unread check. + for (const history of entry.stateHistory ?? []) { + latest = Math.max(latest, usableTimestamp(history.startedAt)) + } + return latest +} + function isPlainPersistedRecord(value: unknown): value is Record { return Boolean(value) && typeof value === 'object' && !Array.isArray(value) } @@ -1215,10 +1232,15 @@ export const createUISlice: StateCreator = (set, get) return s } const now = Date.now() - // Why: only reallocate if an ack advances; compare prev | null = null for (const key of paneKeys) { const prev = s.acknowledgedAgentsByPaneKey[key] ?? 0 + // Why not plain Date.now(): a remote/SSH execution host can stamp a turn ahead of this clock, + // and every unread rule is `ackAt < turnTimestamp`. A behind-the-turn ack can never clear the + // row, so its auto-ack effect re-fires on each new millisecond forever (React #185). + let stamp = now const liveEntry = s.agentStatusByPaneKey?.[key] if (liveEntry) { collectAcknowledgedAgentNotificationId({ @@ -1228,6 +1250,7 @@ export const createUISlice: StateCreator = (set, get) stateStartedAt: liveEntry.stateStartedAt, previousAckAt: prev }) + stamp = Math.max(stamp, latestAgentTurnTimestamp(liveEntry)) } const retained = s.retainedAgentsByPaneKey?.[key] if (retained) { @@ -1238,12 +1261,19 @@ export const createUISlice: StateCreator = (set, get) stateStartedAt: retained.entry.stateStartedAt, previousAckAt: prev }) + stamp = Math.max(stamp, latestAgentTurnTimestamp(retained.entry)) } - if (prev < now) { + for (const unsupported of migrationUnsupported) { + // Why: Activity synthesizes a blocked row from this entry, stamped by the pane's host like any turn. + if (unsupported.paneKey === key) { + stamp = Math.max(stamp, usableTimestamp(unsupported.updatedAt)) + } + } + if (prev < stamp) { if (next === null) { next = { ...s.acknowledgedAgentsByPaneKey } } - next[key] = now + next[key] = stamp } } return next ? { acknowledgedAgentsByPaneKey: next } : s