mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 08:02:28 +00:00
wip: react185-cascade (#16940)
This commit is contained in:
Neil
@@ -1426,6 +1426,8 @@ export default function ActivityPrototypePage(): React.JSX.Element {
|
||||
const [groupBy, setGroupBy] = useState<ActivityGroupBy>('status')
|
||||
const [query, setQuery] = useState('')
|
||||
const activityFilterInputRef = useRef<HTMLInputElement | null>(null)
|
||||
// Why: bounds auto mark-read to one acknowledgement per selected thread turn.
|
||||
const autoAcknowledgedTurnRef = useRef<string | null>(null)
|
||||
const [compactMode, setCompactMode] = useState(false)
|
||||
const [selectedPaneKey, setSelectedPaneKey] = useState<string | null>(null)
|
||||
const [displayedPaneKey, setDisplayedPaneKey] = useState<string | null>(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])
|
||||
}
|
||||
}, [
|
||||
|
||||
@@ -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<void> {
|
||||
const container = document.createElement('div')
|
||||
document.body.append(container)
|
||||
root = createRoot(container)
|
||||
await act(async () => {
|
||||
root.render(
|
||||
<TooltipProvider>
|
||||
<ActivityPrototypePage />
|
||||
</TooltipProvider>
|
||||
)
|
||||
await new Promise((resolve) => setTimeout(resolve, 30))
|
||||
})
|
||||
seededContainer = container
|
||||
}
|
||||
|
||||
async function selectSeededThread(): Promise<void> {
|
||||
const row = Array.from(seededContainer.querySelectorAll<HTMLElement>('[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()
|
||||
})
|
||||
})
|
||||
@@ -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(
|
||||
<SidebarBoundary capture={capture}>
|
||||
<TooltipProvider>{node}</TooltipProvider>
|
||||
</SidebarBoundary>
|
||||
)
|
||||
})
|
||||
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 <div data-testid="driver">{ticks}</div>
|
||||
}
|
||||
|
||||
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(
|
||||
<WorktreeContextMenu worktree={worktreeFixture()}>
|
||||
<div data-testid="card-child">Card</div>
|
||||
</WorktreeContextMenu>,
|
||||
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<HTMLDivElement>(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 (
|
||||
<>
|
||||
<div ref={hostRef}>
|
||||
<WorktreeContextMenu worktree={worktreeFixture()}>
|
||||
<div data-testid="card-child">Card</div>
|
||||
</WorktreeContextMenu>
|
||||
</div>
|
||||
<CommitCascadeDriver ticks={ticks} onTick={() => setTicks((value) => value + 1)} />
|
||||
</>
|
||||
)
|
||||
}
|
||||
|
||||
mount(<Harness />, 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)
|
||||
})
|
||||
})
|
||||
@@ -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<string, boolean>
|
||||
unreadAgentCompletionPanes: Record<string, boolean>
|
||||
agentStatusByPaneKey: Record<string, unknown>
|
||||
agentStatusEpoch: number
|
||||
runtimePaneTitlesByTabId: Record<string, Record<number, string>>
|
||||
ptyIdsByTabId: Record<string, string[]>
|
||||
terminalLayoutsByTabId: Record<string, unknown>
|
||||
renamingTabId: string | null
|
||||
keybindings: Record<string, unknown>
|
||||
setRenamingTabId: (tabId: string | null) => void
|
||||
}
|
||||
|
||||
type StoreApiWithHook = {
|
||||
(selector: (state: ProbeState) => unknown): unknown
|
||||
getState: () => ProbeState
|
||||
setState: (partial: Partial<ProbeState>) => void
|
||||
}
|
||||
|
||||
// Both specifiers resolve to the same store module; memoize so they share one instance.
|
||||
async function createProbeStore(): Promise<StoreApiWithHook> {
|
||||
const globalKey = '__sortableTabProbeStore'
|
||||
const globals = globalThis as Record<string, unknown>
|
||||
if (!globals[globalKey]) {
|
||||
const { create } = await import('zustand')
|
||||
globals[globalKey] = create<ProbeState>((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 }) => <div>{children}</div>,
|
||||
TooltipContent: ({ children }: { children: ReactNode }) => <div>{children}</div>,
|
||||
TooltipTrigger: ({ children, asChild }: { children: ReactNode; asChild?: boolean }) =>
|
||||
asChild && isValidElement(children) ? (
|
||||
cloneElement(children as ReactElement<Record<string, unknown>>)
|
||||
) : (
|
||||
<span>{children}</span>
|
||||
)
|
||||
}))
|
||||
|
||||
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<string, unknown>) => <input {...props} />
|
||||
}))
|
||||
|
||||
vi.mock('./shell-icons', () => ({ ShellIcon: () => <span /> }))
|
||||
vi.mock('@/lib/agent-catalog', () => ({ AgentIcon: () => <span /> }))
|
||||
vi.mock('../sidebar/WorktreeCardHelpers', () => ({ FilledBellIcon: () => <span /> }))
|
||||
|
||||
function makeTab(overrides: Partial<TerminalTab> = {}): 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 (
|
||||
<SortableTab
|
||||
tab={tab}
|
||||
unifiedTabId="unified-1"
|
||||
groupId="group-1"
|
||||
tabCount={1}
|
||||
hasTabsToRight={false}
|
||||
hasTabsToLeft={false}
|
||||
isActive
|
||||
isPinned={false}
|
||||
isExpanded={false}
|
||||
onActivate={vi.fn()}
|
||||
onClose={vi.fn()}
|
||||
onCloseOthers={vi.fn()}
|
||||
onCloseToRight={vi.fn()}
|
||||
onCloseToLeft={vi.fn()}
|
||||
onSetCustomTitle={vi.fn()}
|
||||
onSetTabColor={vi.fn()}
|
||||
onTogglePin={vi.fn()}
|
||||
onToggleExpand={vi.fn()}
|
||||
// A fresh object per render, exactly like renderTabBarItems builds it.
|
||||
dragData={{ ...dragData }}
|
||||
dropIndicator={null}
|
||||
/>
|
||||
)
|
||||
}
|
||||
|
||||
/** 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 <Harness tab={makeTab({ title: titles[index] })} />
|
||||
}
|
||||
|
||||
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(<Harness tab={makeTab()} />)
|
||||
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(<ChurningHarness titles={['a', 'b', 'c', 'd', 'e', 'f']} />)
|
||||
expect(renderCount).toBeLessThan(40)
|
||||
})
|
||||
|
||||
it('settles under a store write storm', () => {
|
||||
render(<Harness tab={makeTab()} />)
|
||||
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(
|
||||
<StrictMode>
|
||||
<Harness tab={makeTab()} />
|
||||
</StrictMode>
|
||||
)
|
||||
expect(probeStore.getState().renamingTabId).toBeNull()
|
||||
})
|
||||
})
|
||||
+233
@@ -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<string, string | undefined>,
|
||||
groupsByWorktree: {} as Record<string, TabGroup[]>,
|
||||
pendingStartupByTabId: {} as Record<string, unknown>,
|
||||
ptyIdsByTabId: {} as Record<string, string[]>,
|
||||
runtimeStatusByEnvironmentId: new Map<string, unknown>(),
|
||||
runtimePaneTitlesByTabId: {} as Record<string, Record<number, string>>,
|
||||
settings: {} as Record<string, unknown>,
|
||||
sleepingAgentSessionsByPaneKey: {} as Record<string, unknown>,
|
||||
tabsByWorktree: {} as Record<string, TerminalTab[]>,
|
||||
terminalLayoutsByTabId: {} as Record<string, unknown>,
|
||||
unifiedTabsByWorktree: {} as Record<string, Tab[]>,
|
||||
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<string> }) => {
|
||||
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<string, TabGroup[]>
|
||||
tabsByWorktree: Record<string, TerminalTab[]>
|
||||
unifiedTabsByWorktree: Record<string, Tab[]>
|
||||
}
|
||||
|
||||
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(
|
||||
<StrictMode>
|
||||
<TerminalPaneOverlayLayer
|
||||
worktreeId={harness.worktreeId}
|
||||
worktreePath="cold-park-tab-model-identity"
|
||||
isWorktreeActive={false}
|
||||
coldParkTerminalPanes={false}
|
||||
/>
|
||||
<UnrelatedCascadeDriver />
|
||||
</StrictMode>
|
||||
)
|
||||
})
|
||||
} 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)
|
||||
})
|
||||
})
|
||||
@@ -138,6 +138,8 @@ export function useTerminalTabColdParking(args: {
|
||||
const [coldParkedTerminalTabIds, setColdParkedTerminalTabIds] = useState<ReadonlySet<string>>(
|
||||
() => 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 (
|
||||
|
||||
@@ -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]])
|
||||
})
|
||||
})
|
||||
@@ -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(<TabAgentProbe />))
|
||||
})
|
||||
|
||||
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')
|
||||
})
|
||||
})
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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> = {}): 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<AppState>)
|
||||
|
||||
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<AppState>)
|
||||
|
||||
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<AppState>)
|
||||
|
||||
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<AppState>)
|
||||
|
||||
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<AppState>)
|
||||
|
||||
store.getState().acknowledgeAgents([PANE_KEY])
|
||||
|
||||
expect(store.getState().acknowledgedAgentsByPaneKey[PANE_KEY]).toBe(NOW)
|
||||
})
|
||||
})
|
||||
@@ -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<string, unknown> {
|
||||
return Boolean(value) && typeof value === 'object' && !Array.isArray(value)
|
||||
}
|
||||
@@ -1215,10 +1232,15 @@ export const createUISlice: StateCreator<AppState, [], [], UISlice> = (set, get)
|
||||
return s
|
||||
}
|
||||
const now = Date.now()
|
||||
// Why: only reallocate if an ack advances; compare prev<now not !== — Date.now() ticks every ms and !== would rewrite the map every call.
|
||||
const migrationUnsupported = Object.values(s.migrationUnsupportedByPtyId ?? {})
|
||||
// Why: only reallocate if an ack advances; compare prev<stamp not !== — the stamp ticks every ms and !== would rewrite the map every call.
|
||||
let next: Record<string, number> | 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<AppState, [], [], UISlice> = (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<AppState, [], [], UISlice> = (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
|
||||
|
||||
Reference in New Issue
Block a user