Merge remote-tracking branch 'origin/main' into HEAD

This commit is contained in:
Brennan Benson
2026-08-27 11:58:44 -07:00
59 changed files with 3142 additions and 504 deletions
+1
View File
@@ -703,6 +703,7 @@ jobs:
src/main/providers/windows-conpty-wide-char-duplication.node-pty.test.ts
src/main/providers/pty-repaint-wide-char-buffer.node-pty.test.ts
src/shared/child-process/windows-command-line.win32.test.ts
src/main/agent-hooks/windows-hook-payload-delivery.test.ts
src/main/windows/windows-pty-job.win32.test.ts
src/main/windows/windows-host-job.win32.test.ts
src/main/wsl/wsl-runner.test.ts
@@ -46,6 +46,40 @@ risk — but the headroom is roughly 2x on time and 1.7x on bytes, not the ~4x t
parse, and the read rejects, so a busy host loses the table rather than
receiving a wrong one.
## When a read wedges
The vendored reader pushes every callback onto a module-global queue and drains
that queue only when the request holding its `requestInProgress` latch
completes. If a Toolhelp32 snapshot never comes back — an EDR hook, a restricted
token, a worker that dies — the latch is stuck for the life of the process and
every later call parks another closure in that queue.
Two guards, and they work together:
- a **3 s deadline** on each read, so a caller gets a rejection instead of a
promise that never settles;
- a **sticky wedge**: once a read misses its deadline and has not called back,
the module refuses every further read until that read's callback fires.
The wedge used to be a 30 s cooldown that let one probe through per window. That
bounded the *rate* of new callbacks but not the total: a permanently wedged
reader retained one more closure every 30 s for as long as the app ran, and each
probe also blocked its caller for the full 3 s deadline first. Gating on the
outstanding read instead bounds retention at exactly one callback, and gives up
nothing on recovery — a probe queued behind the latch could never have observed
recovery anyway, whereas the stuck callback firing is the drain itself. On the
relay's bare addon, which has no queue of its own, it is also what keeps Orca
from re-entering `CreateToolhelp32Snapshot` while a call is still running.
That last part is not just tidiness. The addon runs each read as a
`Napi::AsyncWorker`, so a wedged read holds a libuv threadpool slot for good. On
the relay the JS queue is not there to absorb the retries, so one probe per
window would have pinned all four default threads inside ~2 minutes — hanging
every async `fs` and DNS call in that process, not only the process table.
A wedge does **not** engage the PowerShell fallback; see the next section for
why only absence does.
## The relay has no binding, and falls back
Relay deployment installs only `node-pty` and `@parcel/watcher` on the remote
+2 -2
View File
@@ -604,7 +604,7 @@ describe('wrapPosixHookCommand', () => {
})
const qualifiedWindowsPowerShellCommand =
/^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -WindowStyle Hidden -EncodedCommand \S+$/
/^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -EncodedCommand \S+$/
function decodeWindowsHookCommand(command: string): string {
const encodedCommand = command.match(/ -EncodedCommand (\S+)$/)?.[1]
@@ -762,7 +762,7 @@ describe('wrapRuntimeHomeHookCommand', () => {
// applies to the exact same string (#16003).
const command = wrapRuntimeHomeHookCommand('claude-hook')
expect(command).toContain('powershell.exe" -NoProfile -WindowStyle Hidden -EncodedCommand ')
expect(command).toContain('powershell.exe" -NoProfile -EncodedCommand ')
expect(command).not.toMatch(/-ExecutionPolicy/i)
})
@@ -1,6 +1,7 @@
import { describe, expect, it } from 'vitest'
import {
encodeWindowsPowerShellHookCommand,
getWindowsPowerShellExecutablePath,
WINDOWS_POWERSHELL_HOOK_SWITCHES,
wrapWindowsPowerShellEncodedCommand
} from './windows-powershell-hook-launcher'
@@ -12,38 +13,41 @@ function decodePayload(command: string): string {
}
/*
* Two reproduced Windows failures constrain this one string, in opposite
* directions:
* #16003 — endpoint security (Kaspersky, Windows 11) denies process creation
* for the encoded launcher shape whatever the payload decodes to, and no
* exclusion re-enabled it. Measured on the reporting host: the denied shape
* exits 126 with or without the policy switch and with or without `-NoProfile`,
* while `-NoProfile -EncodedCommand` alone runs 5/5. So the command line spells
* neither the policy bypass (moved into the payload by #16576) nor the denied
* window switch.
*
* #16003 — endpoint security (Kaspersky Premium, Windows 11) denies process
* creation for `-ExecutionPolicy Bypass -WindowStyle Hidden -EncodedCommand`
* whatever the payload decodes to, and no exclusion re-enabled it. The triple
* must stop being spelled.
*
* #14815 (+ #14828, #15117, #15447, #15767) — without `-WindowStyle Hidden`
* every hook event allocates a console that takes foreground and eats the
* user's keystrokes, and strands a visible window outright when the hook blocks
* on stdin. Window suppression must stay.
*
* Both hold only if the flag that leaves the command line is the policy bypass,
* which is the one with an exact in-payload equivalent.
* The counter-pressure is real and is NOT resolved by these assertions:
* #14815 (+ #14828, #15117, #15447, #15767) reported consoles taking
* foreground. The previous suppression switch was that fix, but its behavior
* was never measured — see the launcher's comment. These tests pin the shape
* so it cannot be restored silently; whether a console appears is a question
* for a live window measurement, which no unit test here can answer.
*/
describe('windows PowerShell hook launcher', () => {
it('never spells the denied flag triple on the command line', () => {
it('never spells a denied flag on the command line', () => {
const command = wrapWindowsPowerShellEncodedCommand('exit 0')
const switches = command.replace(/ -EncodedCommand \S+$/, '')
expect(WINDOWS_POWERSHELL_HOOK_SWITCHES).not.toMatch(/-ExecutionPolicy/i)
expect(command.replace(/ -EncodedCommand \S+$/, '')).not.toMatch(/-ExecutionPolicy/i)
expect(switches).not.toMatch(/-ExecutionPolicy/i)
// Why: measured exit 126 for the denied switch paired with -EncodedCommand.
expect(WINDOWS_POWERSHELL_HOOK_SWITCHES).toBe('-NoProfile')
expect(switches).toBe(`${getWindowsPowerShellExecutablePath()} -NoProfile`)
})
it('keeps hiding the console window on every hook event (#14815)', () => {
// Dropping this flag is not a cosmetic flash: the console takes foreground
// and swallows what the user is typing into Orca (#14828), and never closes
// at all when the hook blocks reading stdin (#14815).
it('pins the exact launcher shape so a flag cannot come back unnoticed', () => {
// Why this is worth a test: #16576 round 3 dropped a required launcher
// invariant by accident and updated the tests to match, so nothing caught
// it. Restoring either denied switch re-breaks every hook on an AV host.
const command = wrapWindowsPowerShellEncodedCommand('exit 0')
expect(WINDOWS_POWERSHELL_HOOK_SWITCHES).toBe('-NoProfile -WindowStyle Hidden')
expect(command).toMatch(/ -NoProfile -WindowStyle Hidden -EncodedCommand [A-Za-z0-9+/=]+$/)
expect(WINDOWS_POWERSHELL_HOOK_SWITCHES).toBe('-NoProfile')
expect(command).toMatch(/ -NoProfile -EncodedCommand [A-Za-z0-9+/=]+$/)
})
it('keeps the execution-policy bypass, in the payload where AV cannot read it', () => {
@@ -14,26 +14,33 @@ export function getWindowsPowerShellExecutablePath(): string {
* Switches for the PowerShell that relays hook output and exit status
* (#14818 — conhost does neither).
*
* `-WindowStyle Hidden` stays. It is the shipped fix for #14815 and its four
* duplicates (#14828, #15117, #15447, #15767): without it Windows allocates a
* console per hook event, which steals foreground from whatever the user is
* typing into, and strands a permanently visible window whenever a hook blocks
* reading stdin (see hook-stdin-contract.ts). Those are reported, reproduced
* user-facing failures on every hook event of every managed agent.
* The command line spells no flag beyond `-NoProfile`, because AV denies the
* combinations. #16003 measured, on the reporting Kaspersky host:
*
* `-ExecutionPolicy Bypass` is the flag that leaves the command line, because
* it is the only one of the three with an exact in-payload equivalent (below):
* it costs nothing to move. #16003 measured `-NoProfile -ExecutionPolicy Bypass
* -WindowStyle Hidden -EncodedCommand` as denied at CreateProcess (exit 126,
* Kaspersky Premium on Windows 11) no matter what the payload decodes to, so
* the triple has to stop being spelled; dropping the policy flag breaks it.
* -NoProfile -ExecutionPolicy Bypass -WindowStyle Hidden -EncodedCommand 126
* -NoProfile -WindowStyle Hidden -EncodedCommand 126
* -WindowStyle Hidden -EncodedCommand 126
* -NoProfile -EncodedCommand 0 (5/5)
* -NoProfile -ExecutionPolicy Bypass -Command 0 (5/5)
*
* What is not established: that the remaining pair clears that AV signature —
* the reporter measured no two-flag encoded shape. If it turns out not to, the
* answer is another launcher shape that still suppresses the window, not
* trading a reproduced regression for an unmeasured hope.
* The denial is at CreateProcess and is independent of the payload: `exit 0` is
* denied too, and bash reports it as `Permission denied`. So `-WindowStyle
* Hidden` + `-EncodedCommand` is the pair that has to stop being spelled.
* #16576 removed `-ExecutionPolicy Bypass` (kept in-payload below), which was
* the one flag of the three NOT in the signature — hooks kept failing.
*
* `-WindowStyle Hidden` was the shipped fix for #14815 (+#14828, #15117,
* #15447, #15767). Removing it is a real tradeoff and is recorded as such: its
* suppression was never measured — #14825 confirmed it *visually*, #16576's
* author stated it "remains unverified on a real box", and #15506's author
* argued it cannot help a `.cmd` child that has no console to inherit. The
* console is allocated by the parent chain, not by this command line.
*
* Do not restore the flag to fix a console report. That trades every hook on an
* AV host for a flicker. The answer is to shorten the interpreter chain — the
* shipped doctrine of #15520 and #15595 — or a launcher that owns no console.
*/
export const WINDOWS_POWERSHELL_HOOK_SWITCHES = '-NoProfile -WindowStyle Hidden'
export const WINDOWS_POWERSHELL_HOOK_SWITCHES = '-NoProfile'
// Why: redirected PowerShell progress becomes CLIXML that can corrupt merged JSON
// output. It must be the FIRST statement: Set-ExecutionPolicy autoloads
@@ -64,7 +64,7 @@ describe('browser clicked-link routing', () => {
vi.restoreAllMocks()
})
it('routes plain target=_blank links back into the current Orca tab', () => {
it('routes plain target=_blank links into a new Orca tab', () => {
const link = document.createElement('a')
link.href = 'https://docs.example.com/guide'
link.target = '_blank'
@@ -73,9 +73,8 @@ describe('browser clicked-link routing', () => {
const { event, open } = clickLink(link)
expect(event.defaultPrevented).toBe(false)
expect(open).not.toHaveBeenCalled()
expect(link.getAttribute('target')).toBe('_self')
expect(event.defaultPrevented).toBe(true)
expect(open).toHaveBeenCalledWith('https://docs.example.com/guide', FOREGROUND_FRAME_NAME)
})
it('routes the host-platform modifier without trusting an emulated guest user agent', () => {
@@ -127,9 +126,11 @@ describe('browser clicked-link routing', () => {
expect(clickLink(cancelled).open).not.toHaveBeenCalled()
expect(cancelled.getAttribute('target')).toBe('_blank')
expect(clickLink(rewritten).open).not.toHaveBeenCalled()
expect(rewritten.href).toBe('https://example.com/rewritten')
expect(rewritten.getAttribute('target')).toBe('_self')
// The page's own handler ran first, so routing must follow the rewritten href.
expect(clickLink(rewritten).open).toHaveBeenCalledWith(
'https://example.com/rewritten',
FOREGROUND_FRAME_NAME
)
})
it('routes SVG links but leaves download links and links without href alone', () => {
@@ -145,8 +146,10 @@ describe('browser clicked-link routing', () => {
document.body.append(svgLink, areaDownload, noHref)
installRouting()
expect(clickLink(svgLink).open).not.toHaveBeenCalled()
expect(svgLink.getAttribute('target')).toBe('_self')
expect(clickLink(svgLink).open).toHaveBeenCalledWith(
'https://example.com/svg',
FOREGROUND_FRAME_NAME
)
expect(clickLink(areaDownload).open).not.toHaveBeenCalled()
expect(clickLink(noHref).open).not.toHaveBeenCalled()
})
@@ -195,7 +198,7 @@ describe('browser clicked-link routing', () => {
expect(script).not.toContain('BrowserClickedLinkRoutingState')
})
it('routes plain iframe target=_blank links into the top-level guest', () => {
it('routes plain iframe target=_blank links into a new Orca tab', () => {
const link = document.createElement('a')
link.href = 'https://example.com/from-frame'
link.target = '_blank'
@@ -204,9 +207,8 @@ describe('browser clicked-link routing', () => {
const { event, open } = clickLink(link)
expect(event.defaultPrevented).toBe(false)
expect(open).not.toHaveBeenCalled()
expect(link.getAttribute('target')).toBe('_top')
expect(event.defaultPrevented).toBe(true)
expect(open).toHaveBeenCalledWith('https://example.com/from-frame', FOREGROUND_FRAME_NAME)
})
it('routes explicit iframe new-tab gestures through one-use frame names', () => {
@@ -75,8 +75,9 @@ export function installBrowserClickedLinkRouting(
const baseTarget = document.querySelector('base[target]')?.getAttribute('target') ?? ''
const ownTarget = link.getAttribute('target')
const effectiveTarget = (ownTarget === null ? baseTarget : ownTarget).trim().toLowerCase()
const opensNewContext = middleClick || modifierClick
if (!opensNewContext && effectiveTarget !== '_blank') {
// target=_blank is a new-tab request, exactly as it is in every other
// browser; the modifiers are the other two ways to ask for one.
if (!(middleClick || modifierClick || effectiveTarget === '_blank')) {
return
}
@@ -96,13 +97,6 @@ export function installBrowserClickedLinkRouting(
return
}
if (!opensNewContext) {
// Why: changing only the browsing context keeps Chromium's native anchor
// navigation, including referrer policy, attribution, and history.
link.setAttribute('target', '_self')
return
}
// Why: Electron reports direct link clicks and featureless window.open()
// with the same disposition. The private frame name preserves that one
// distinction without weakening OAuth popups that need window.opener.
@@ -118,8 +112,8 @@ export function installBrowserClickedLinkRouting(
}
/**
* Keeps plain target=_blank clicks inside the top-level guest when Electron's
* isolated-world API cannot target a child frame.
* Same routing for links inside child frames, which Electron's isolated-world
* API cannot reach, so this runs in the page world against a one-use token.
*/
export function installBrowserIframeClickedLinkRouting(
frameName: string,
@@ -161,8 +155,7 @@ export function installBrowserIframeClickedLinkRouting(
const baseTarget = document.querySelector('base[target]')?.getAttribute('target') ?? ''
const ownTarget = link.getAttribute('target')
const effectiveTarget = (ownTarget === null ? baseTarget : ownTarget).trim().toLowerCase()
const opensNewContext = middleClick || modifierClick
if (!opensNewContext && effectiveTarget !== '_blank') {
if (!(middleClick || modifierClick || effectiveTarget === '_blank')) {
return
}
@@ -182,13 +175,6 @@ export function installBrowserIframeClickedLinkRouting(
return
}
if (!opensNewContext) {
// Why: WebContents isolated worlds only cover the main frame. Rewriting
// to `_top` preserves native anchor semantics without opening a popup.
link.setAttribute('target', '_top')
return
}
// Why: child-frame code runs in the page world, so each token is one-use.
// A page that observes a real click cannot replay it to create more tabs.
event.preventDefault()
@@ -0,0 +1,253 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
const browserMocks = vi.hoisted(() => ({
appGetPathMock: vi.fn(() => '/downloads'),
menuBuildFromTemplateMock: vi.fn(),
guestOffMock: vi.fn(),
guestOnMock: vi.fn(),
guestSetBackgroundThrottlingMock: vi.fn(),
guestSetWindowOpenHandlerMock: vi.fn(),
guestOpenDevToolsMock: vi.fn(),
webContentsFromIdMock: vi.fn(),
browserWindowFromWebContentsMock: vi.fn(),
screenGetCursorScreenPointMock: vi.fn(() => ({ x: 0, y: 0 })),
shellOpenExternalMock: vi.fn(),
openPopupWithOriginBarMock: vi.fn()
}))
vi.mock('electron', () => ({
app: { getPath: browserMocks.appGetPathMock },
BrowserWindow: { fromWebContents: browserMocks.browserWindowFromWebContentsMock },
clipboard: { writeText: vi.fn() },
shell: { openExternal: browserMocks.shellOpenExternalMock },
Menu: { buildFromTemplate: browserMocks.menuBuildFromTemplateMock },
screen: { getCursorScreenPoint: browserMocks.screenGetCursorScreenPointMock },
webContents: { fromId: browserMocks.webContentsFromIdMock }
}))
vi.mock('./popup-origin-bar-window', () => ({
openPopupWithOriginBar: browserMocks.openPopupWithOriginBarMock
}))
import { browserManager } from './browser-manager'
import {
createDownloadItem,
getDownloadItemEventHandler,
rendererWebContentsId,
resetBrowserManagerMocks,
resetBrowserManagerState
} from './browser-manager-test-harness'
const {
guestOffMock,
guestOnMock,
guestSetBackgroundThrottlingMock,
guestSetWindowOpenHandlerMock,
guestOpenDevToolsMock,
webContentsFromIdMock,
shellOpenExternalMock
} = browserMocks
describe('browserManager popup child policies', () => {
beforeEach(() => {
resetBrowserManagerMocks(browserMocks)
resetBrowserManagerState()
})
afterEach(() => {
vi.useRealTimers()
})
it('attaches guest policies to created popup child windows', () => {
const rendererSendMock = vi.fn()
const childSetBackgroundThrottlingMock = vi.fn()
const childSetWindowOpenHandlerMock = vi.fn()
const childOnMock = vi.fn()
const childOffMock = vi.fn()
const childOpenDevToolsMock = vi.fn()
const childGuest = {
id: 4040,
isDestroyed: vi.fn(() => false),
getType: vi.fn(() => 'webview'),
setBackgroundThrottling: childSetBackgroundThrottlingMock,
setWindowOpenHandler: childSetWindowOpenHandlerMock,
on: childOnMock,
off: childOffMock,
openDevTools: childOpenDevToolsMock
}
const guest = {
id: 404,
isDestroyed: vi.fn(() => false),
getType: vi.fn(() => 'webview'),
setBackgroundThrottling: guestSetBackgroundThrottlingMock,
setWindowOpenHandler: guestSetWindowOpenHandlerMock,
on: guestOnMock,
off: guestOffMock,
openDevTools: guestOpenDevToolsMock
}
webContentsFromIdMock.mockImplementation((id: number) => {
if (id === guest.id) {
return guest
}
if (id === rendererWebContentsId) {
return { isDestroyed: vi.fn(() => false), send: rendererSendMock }
}
return null
})
browserManager.attachGuestPolicies(guest as never)
browserManager.registerGuest({
browserPageId: 'browser-1',
webContentsId: guest.id,
rendererWebContentsId
})
const didCreateWindowHandler = guestOnMock.mock.calls.find(
([event]) => event === 'did-create-window'
)?.[1] as ((window: { webContents: typeof childGuest }) => void) | undefined
expect(didCreateWindowHandler).toBeTypeOf('function')
didCreateWindowHandler?.({ webContents: childGuest })
expect(childSetBackgroundThrottlingMock).toHaveBeenCalledWith(false)
expect(childSetWindowOpenHandlerMock).toHaveBeenCalledTimes(1)
expect(childOnMock.mock.calls.filter(([event]) => event === 'did-create-window')).toHaveLength(
1
)
expect(childOnMock.mock.calls.filter(([event]) => event === 'will-navigate')).toHaveLength(1)
expect(childOnMock.mock.calls.filter(([event]) => event === 'will-redirect')).toHaveLength(1)
const childWindowOpenHandler = childSetWindowOpenHandlerMock.mock.calls[0][0] as (details: {
url: string
}) => { action: 'allow' | 'deny' }
expect(childWindowOpenHandler({ url: 'https://identity.example.com/login' })).toMatchObject({
action: 'allow'
})
expect(childWindowOpenHandler({ url: 'file:///etc/passwd' })).toEqual({ action: 'deny' })
expect(rendererSendMock).toHaveBeenCalledWith('browser:popup', {
browserPageId: 'browser-1',
origin: 'null',
action: 'blocked'
})
browserManager.notifyPermissionDenied({
guestWebContentsId: childGuest.id,
permission: 'notifications',
rawUrl: 'https://identity.example.com/login'
})
expect(rendererSendMock).toHaveBeenCalledWith('browser:permission-denied', {
browserPageId: 'browser-1',
permission: 'notifications',
origin: 'https://identity.example.com'
})
const childDidFailLoadHandler = childOnMock.mock.calls.find(
([event]) => event === 'did-fail-load'
)?.[1] as
| ((
event: Electron.Event,
errorCode: number,
errorDescription: string,
validatedURL: string,
isMainFrame: boolean
) => void)
| undefined
childDidFailLoadHandler?.(
{} as Electron.Event,
-105,
'Name not resolved',
'https://identity.example.com/unavailable',
true
)
expect(rendererSendMock).not.toHaveBeenCalledWith(
'browser:guest-load-failed',
expect.anything()
)
const childDownloadItem = createDownloadItem()
browserManager.handleGuestWillDownload({
guestWebContentsId: childGuest.id,
item: childDownloadItem
})
expect(rendererSendMock).toHaveBeenCalledWith(
'browser:download-requested',
expect.objectContaining({ browserPageId: 'browser-1' })
)
const childDownloadDoneHandler = getDownloadItemEventHandler(childDownloadItem, 'once', 'done')
childDownloadDoneHandler?.({} as Electron.Event, 'completed')
const managerState = browserManager as unknown as {
popupOwnerContextByGuestId: Map<number, unknown>
}
expect(managerState.popupOwnerContextByGuestId.has(childGuest.id)).toBe(true)
const cleanupChildOnMock = vi.fn()
const cleanupChildGuest = {
...childGuest,
id: 4041,
on: cleanupChildOnMock,
off: vi.fn(),
setBackgroundThrottling: vi.fn(),
setWindowOpenHandler: vi.fn()
}
const childDidCreateWindowHandler = childOnMock.mock.calls.find(
([event]) => event === 'did-create-window'
)?.[1] as ((window: { webContents: typeof cleanupChildGuest }) => void) | undefined
childDidCreateWindowHandler?.({ webContents: cleanupChildGuest })
expect(managerState.popupOwnerContextByGuestId.has(cleanupChildGuest.id)).toBe(true)
const cleanupChildWindowOpenHandler = cleanupChildGuest.setWindowOpenHandler.mock
.calls[0][0] as (details: { url: string }) => { action: 'allow' | 'deny' }
expect(
cleanupChildWindowOpenHandler({ url: 'https://identity.example.com/continue' })
).toMatchObject({ action: 'allow' })
const cleanupChildDestroyedHandler = cleanupChildOnMock.mock.calls.find(
([event]) => event === 'destroyed'
)?.[1] as (() => void) | undefined
cleanupChildDestroyedHandler?.()
expect(managerState.popupOwnerContextByGuestId.has(cleanupChildGuest.id)).toBe(false)
const replacementGuest = {
...guest,
id: 405,
on: vi.fn(),
off: vi.fn(),
setBackgroundThrottling: vi.fn(),
setWindowOpenHandler: vi.fn()
}
webContentsFromIdMock.mockImplementation((id: number) => {
if (id === guest.id) {
return guest
}
if (id === replacementGuest.id) {
return replacementGuest
}
if (id === rendererWebContentsId) {
return { isDestroyed: vi.fn(() => false), send: rendererSendMock }
}
return null
})
browserManager.attachGuestPolicies(replacementGuest as never)
browserManager.registerGuest({
browserPageId: 'browser-1',
webContentsId: replacementGuest.id,
rendererWebContentsId
})
expect(childWindowOpenHandler({ url: 'https://identity.example.com/next' })).toEqual({
action: 'deny'
})
expect(shellOpenExternalMock).toHaveBeenCalledWith('https://identity.example.com/next')
expect(managerState.popupOwnerContextByGuestId.has(childGuest.id)).toBe(false)
const childDestroyedHandler = childOnMock.mock.calls.find(
([event]) => event === 'destroyed'
)?.[1] as (() => void) | undefined
childDestroyedHandler?.()
expect(managerState.popupOwnerContextByGuestId.has(childGuest.id)).toBe(false)
browserManager.unregisterAll()
expect(childOffMock).toHaveBeenCalledWith('did-create-window', expect.any(Function))
expect(childOffMock).toHaveBeenCalledWith('will-navigate', expect.any(Function))
expect(childOffMock).toHaveBeenCalledWith('will-redirect', expect.any(Function))
})
})
@@ -40,9 +40,8 @@ vi.mock('./popup-origin-bar-window', () => ({
}))
import { browserManager } from './browser-manager'
import { MAX_PAGE_INITIATED_TABS_PER_WINDOW } from './browser-page-initiated-tab-budget'
import {
createDownloadItem,
getDownloadItemEventHandler,
rendererWebContentsId,
resetBrowserManagerMocks,
resetBrowserManagerState
@@ -169,11 +168,12 @@ describe('browserManager', () => {
expect(rendererSendMock).not.toHaveBeenCalled()
})
it('keeps featureless window.open popups in-app for every disposition', () => {
it('keeps opener-dependent window.open popups in-app for every disposition', () => {
// Regression guard for the reverted #8332: gating the allow on
// disposition === 'new-window' silently broke featureless window.open()
// OAuth flows (disposition 'foreground-tab'), whose returned handle must
// stay live. Disposition is a UX hint, not a trust signal.
// OAuth flows, whose returned handle must stay live. A named target, a
// features string, and a blank URL each mark such a flow, so all three keep
// a real child window no matter which disposition Chromium reports.
const guest = {
id: 140,
isDestroyed: vi.fn(() => false),
@@ -199,14 +199,186 @@ describe('browserManager', () => {
features: string
disposition: string
}) => { action: 'allow' | 'deny' }
const openerDependentOpens = [
{ url: 'https://sso.example.com/auth', frameName: 'ssoWindow', features: '' },
{ url: 'https://sso.example.com/auth', frameName: '', features: 'width=500,height=600' },
{ url: 'about:blank', frameName: '', features: '' }
]
for (const disposition of ['foreground-tab', 'background-tab', 'new-window']) {
expect(
handler({ url: 'https://sso.example.com/auth', frameName: '', features: '', disposition })
).toMatchObject({ action: 'allow' })
for (const open of openerDependentOpens) {
expect(handler({ ...open, disposition })).toMatchObject({ action: 'allow' })
}
}
expect(shellOpenExternalMock).not.toHaveBeenCalled()
})
it('routes unnamed featureless window.open safely, including after renderer destruction', () => {
const rendererSendMock = vi.fn()
let rendererDestroyed = false
const guest = {
id: 142,
isDestroyed: vi.fn(() => false),
getType: vi.fn(() => 'webview'),
setBackgroundThrottling: guestSetBackgroundThrottlingMock,
setWindowOpenHandler: guestSetWindowOpenHandlerMock,
on: guestOnMock,
off: guestOffMock,
openDevTools: guestOpenDevToolsMock
}
webContentsFromIdMock.mockImplementation((id: number) => {
if (id === guest.id) {
return guest
}
if (id === rendererWebContentsId) {
return { isDestroyed: vi.fn(() => rendererDestroyed), send: rendererSendMock }
}
return null
})
browserManager.attachGuestPolicies(guest as never)
browserManager.registerGuest({
browserPageId: 'browser-1',
webContentsId: guest.id,
rendererWebContentsId
})
const handler = guestSetWindowOpenHandlerMock.mock.calls[0][0] as (details: {
url: string
frameName: string
features: string
disposition: string
}) => { action: 'allow' | 'deny' }
for (const disposition of ['foreground-tab', 'background-tab']) {
expect(
handler({
url: 'https://docs.example.com/guide',
frameName: '',
features: '',
disposition
})
).toEqual({ action: 'deny' })
}
expect(rendererSendMock).toHaveBeenCalledWith('browser:open-link-in-orca-tab', {
browserPageId: 'browser-1',
url: 'https://docs.example.com/guide'
})
expect(rendererSendMock).toHaveBeenCalledWith('browser:popup', {
browserPageId: 'browser-1',
origin: 'https://docs.example.com',
action: 'opened-in-orca'
})
expect(openPopupWithOriginBarMock).not.toHaveBeenCalled()
expect(shellOpenExternalMock).not.toHaveBeenCalled()
rendererDestroyed = true
expect(
handler({
url: 'https://docs.example.com/guide',
frameName: '',
features: '',
disposition: 'foreground-tab'
})
).toEqual({ action: 'deny' })
expect(openPopupWithOriginBarMock).not.toHaveBeenCalled()
expect(shellOpenExternalMock).not.toHaveBeenCalled()
})
it('shares the page-initiated tab budget across the whole opener popup tree', () => {
const rendererSendMock = vi.fn()
const guest = {
id: 150,
isDestroyed: vi.fn(() => false),
getType: vi.fn(() => 'webview'),
setBackgroundThrottling: guestSetBackgroundThrottlingMock,
setWindowOpenHandler: guestSetWindowOpenHandlerMock,
on: guestOnMock,
once: vi.fn(),
off: guestOffMock,
openDevTools: guestOpenDevToolsMock
}
const guestsById = new Map<number, unknown>([[guest.id, guest]])
webContentsFromIdMock.mockImplementation((id: number) => {
if (id === rendererWebContentsId) {
return { isDestroyed: vi.fn(() => false), send: rendererSendMock }
}
return guestsById.get(id) ?? null
})
browserManager.attachGuestPolicies(guest as never)
browserManager.registerGuest({
browserPageId: 'browser-1',
webContentsId: guest.id,
rendererWebContentsId
})
type WindowOpenHandler = (details: {
url: string
frameName: string
features: string
disposition: string
}) => {
action: 'allow' | 'deny'
createWindow?: (options: Record<string, never>) => unknown
}
const handler = guestSetWindowOpenHandlerMock.mock.calls[0][0] as WindowOpenHandler
// A named child popup keeps a real child window; each one used to start with a fresh budget.
const openNamedChild = (index: number): WindowOpenHandler => {
const childSetWindowOpenHandlerMock = vi.fn()
const child = {
id: 1500 + index,
isDestroyed: vi.fn(() => false),
getType: vi.fn(() => 'webview'),
setBackgroundThrottling: vi.fn(),
setWindowOpenHandler: childSetWindowOpenHandlerMock,
on: vi.fn(),
once: vi.fn(),
off: vi.fn(),
openDevTools: vi.fn()
}
guestsById.set(child.id, child)
openPopupWithOriginBarMock.mockReturnValueOnce({
contentWebContents: child,
close: vi.fn(),
onClosed: vi.fn()
})
const result = handler({
url: `https://sso.example.com/child-${index}`,
frameName: `child-${index}`,
features: '',
disposition: 'new-window'
})
expect(result).toMatchObject({ action: 'allow' })
result.createWindow?.({})
return childSetWindowOpenHandlerMock.mock.calls[0][0] as WindowOpenHandler
}
const childHandlers = [openNamedChild(0), openNamedChild(1), openNamedChild(2)]
for (const [childIndex, childHandler] of childHandlers.entries()) {
for (let openIndex = 0; openIndex < MAX_PAGE_INITIATED_TABS_PER_WINDOW; openIndex++) {
expect(
childHandler({
url: `https://docs.example.com/${childIndex}-${openIndex}`,
frameName: '',
features: '',
disposition: 'foreground-tab'
})
).toEqual({ action: 'deny' })
}
}
const routedTabs = rendererSendMock.mock.calls.filter(
([channel]) => channel === 'browser:open-link-in-orca-tab'
)
expect(routedTabs).toHaveLength(MAX_PAGE_INITIATED_TABS_PER_WINDOW)
expect(rendererSendMock).toHaveBeenCalledWith('browser:popup', {
browserPageId: 'browser-1',
origin: 'https://docs.example.com',
action: 'blocked'
})
expect(openPopupWithOriginBarMock).toHaveBeenCalledTimes(childHandlers.length)
})
it('keeps plain links current and routes explicit new-tab gestures to Orca tabs', async () => {
const rendererSendMock = vi.fn()
const executeJavaScriptInIsolatedWorldMock = vi.fn().mockResolvedValue(undefined)
@@ -589,197 +761,4 @@ describe('browserManager', () => {
})
)
})
it('attaches guest policies to created popup child windows', () => {
const rendererSendMock = vi.fn()
const childSetBackgroundThrottlingMock = vi.fn()
const childSetWindowOpenHandlerMock = vi.fn()
const childOnMock = vi.fn()
const childOffMock = vi.fn()
const childOpenDevToolsMock = vi.fn()
const childGuest = {
id: 4040,
isDestroyed: vi.fn(() => false),
getType: vi.fn(() => 'webview'),
setBackgroundThrottling: childSetBackgroundThrottlingMock,
setWindowOpenHandler: childSetWindowOpenHandlerMock,
on: childOnMock,
off: childOffMock,
openDevTools: childOpenDevToolsMock
}
const guest = {
id: 404,
isDestroyed: vi.fn(() => false),
getType: vi.fn(() => 'webview'),
setBackgroundThrottling: guestSetBackgroundThrottlingMock,
setWindowOpenHandler: guestSetWindowOpenHandlerMock,
on: guestOnMock,
off: guestOffMock,
openDevTools: guestOpenDevToolsMock
}
webContentsFromIdMock.mockImplementation((id: number) => {
if (id === guest.id) {
return guest
}
if (id === rendererWebContentsId) {
return { isDestroyed: vi.fn(() => false), send: rendererSendMock }
}
return null
})
browserManager.attachGuestPolicies(guest as never)
browserManager.registerGuest({
browserPageId: 'browser-1',
webContentsId: guest.id,
rendererWebContentsId
})
const didCreateWindowHandler = guestOnMock.mock.calls.find(
([event]) => event === 'did-create-window'
)?.[1] as ((window: { webContents: typeof childGuest }) => void) | undefined
expect(didCreateWindowHandler).toBeTypeOf('function')
didCreateWindowHandler?.({ webContents: childGuest })
expect(childSetBackgroundThrottlingMock).toHaveBeenCalledWith(false)
expect(childSetWindowOpenHandlerMock).toHaveBeenCalledTimes(1)
expect(childOnMock.mock.calls.filter(([event]) => event === 'did-create-window')).toHaveLength(
1
)
expect(childOnMock.mock.calls.filter(([event]) => event === 'will-navigate')).toHaveLength(1)
expect(childOnMock.mock.calls.filter(([event]) => event === 'will-redirect')).toHaveLength(1)
const childWindowOpenHandler = childSetWindowOpenHandlerMock.mock.calls[0][0] as (details: {
url: string
}) => { action: 'allow' | 'deny' }
expect(childWindowOpenHandler({ url: 'https://identity.example.com/login' })).toMatchObject({
action: 'allow'
})
expect(childWindowOpenHandler({ url: 'file:///etc/passwd' })).toEqual({ action: 'deny' })
expect(rendererSendMock).toHaveBeenCalledWith('browser:popup', {
browserPageId: 'browser-1',
origin: 'null',
action: 'blocked'
})
browserManager.notifyPermissionDenied({
guestWebContentsId: childGuest.id,
permission: 'notifications',
rawUrl: 'https://identity.example.com/login'
})
expect(rendererSendMock).toHaveBeenCalledWith('browser:permission-denied', {
browserPageId: 'browser-1',
permission: 'notifications',
origin: 'https://identity.example.com'
})
const childDidFailLoadHandler = childOnMock.mock.calls.find(
([event]) => event === 'did-fail-load'
)?.[1] as
| ((
event: Electron.Event,
errorCode: number,
errorDescription: string,
validatedURL: string,
isMainFrame: boolean
) => void)
| undefined
childDidFailLoadHandler?.(
{} as Electron.Event,
-105,
'Name not resolved',
'https://identity.example.com/unavailable',
true
)
expect(rendererSendMock).not.toHaveBeenCalledWith(
'browser:guest-load-failed',
expect.anything()
)
const childDownloadItem = createDownloadItem()
browserManager.handleGuestWillDownload({
guestWebContentsId: childGuest.id,
item: childDownloadItem
})
expect(rendererSendMock).toHaveBeenCalledWith(
'browser:download-requested',
expect.objectContaining({ browserPageId: 'browser-1' })
)
const childDownloadDoneHandler = getDownloadItemEventHandler(childDownloadItem, 'once', 'done')
childDownloadDoneHandler?.({} as Electron.Event, 'completed')
const managerState = browserManager as unknown as {
popupOwnerContextByGuestId: Map<number, unknown>
}
expect(managerState.popupOwnerContextByGuestId.has(childGuest.id)).toBe(true)
const cleanupChildOnMock = vi.fn()
const cleanupChildGuest = {
...childGuest,
id: 4041,
on: cleanupChildOnMock,
off: vi.fn(),
setBackgroundThrottling: vi.fn(),
setWindowOpenHandler: vi.fn()
}
const childDidCreateWindowHandler = childOnMock.mock.calls.find(
([event]) => event === 'did-create-window'
)?.[1] as ((window: { webContents: typeof cleanupChildGuest }) => void) | undefined
childDidCreateWindowHandler?.({ webContents: cleanupChildGuest })
expect(managerState.popupOwnerContextByGuestId.has(cleanupChildGuest.id)).toBe(true)
const cleanupChildWindowOpenHandler = cleanupChildGuest.setWindowOpenHandler.mock
.calls[0][0] as (details: { url: string }) => { action: 'allow' | 'deny' }
expect(
cleanupChildWindowOpenHandler({ url: 'https://identity.example.com/continue' })
).toMatchObject({ action: 'allow' })
const cleanupChildDestroyedHandler = cleanupChildOnMock.mock.calls.find(
([event]) => event === 'destroyed'
)?.[1] as (() => void) | undefined
cleanupChildDestroyedHandler?.()
expect(managerState.popupOwnerContextByGuestId.has(cleanupChildGuest.id)).toBe(false)
const replacementGuest = {
...guest,
id: 405,
on: vi.fn(),
off: vi.fn(),
setBackgroundThrottling: vi.fn(),
setWindowOpenHandler: vi.fn()
}
webContentsFromIdMock.mockImplementation((id: number) => {
if (id === guest.id) {
return guest
}
if (id === replacementGuest.id) {
return replacementGuest
}
if (id === rendererWebContentsId) {
return { isDestroyed: vi.fn(() => false), send: rendererSendMock }
}
return null
})
browserManager.attachGuestPolicies(replacementGuest as never)
browserManager.registerGuest({
browserPageId: 'browser-1',
webContentsId: replacementGuest.id,
rendererWebContentsId
})
expect(childWindowOpenHandler({ url: 'https://identity.example.com/next' })).toEqual({
action: 'deny'
})
expect(shellOpenExternalMock).toHaveBeenCalledWith('https://identity.example.com/next')
expect(managerState.popupOwnerContextByGuestId.has(childGuest.id)).toBe(false)
const childDestroyedHandler = childOnMock.mock.calls.find(
([event]) => event === 'destroyed'
)?.[1] as (() => void) | undefined
childDestroyedHandler?.()
expect(managerState.popupOwnerContextByGuestId.has(childGuest.id)).toBe(false)
browserManager.unregisterAll()
expect(childOffMock).toHaveBeenCalledWith('did-create-window', expect.any(Function))
expect(childOffMock).toHaveBeenCalledWith('will-navigate', expect.any(Function))
expect(childOffMock).toHaveBeenCalledWith('will-redirect', expect.any(Function))
})
})
+49 -2
View File
@@ -43,6 +43,11 @@ import {
buildBrowserClickedLinkRoutingScript,
buildBrowserIframeClickedLinkRoutingScript
} from './browser-clicked-link-routing'
import {
createPageInitiatedTabBudget,
type PageInitiatedTabBudget
} from './browser-page-initiated-tab-budget'
import { isNewBrowserTabPopupIntent } from './browser-popup-new-tab-intent'
import { cleanElectronUserAgent } from './browser-session-ua'
import { getBrowserSessionUserAgentMode } from './browser-session-user-agent-mode'
import { googleAuthUserAgent, isGoogleAuthUrl } from './browser-google-auth-ua'
@@ -233,6 +238,8 @@ export class BrowserManager {
// Why: reverse map gives O(1) guest→tab lookups on every mouse/load/permission/popup event.
private readonly tabIdByWebContentsId = new Map<number, string>()
private readonly popupOwnerContextByGuestId = new Map<number, PopupOwnerContext>()
// Why: keyed by the opener tree's root so named child popups can't each mint a fresh tab quota.
private readonly pageInitiatedTabBudgetByRootGuestId = new Map<number, PageInitiatedTabBudget>()
// Why: guests are keyed by page id but renderer visibility by workspace id; bridge the mismatch to activate the right tab before capture.
private readonly workspaceIdByPageId = new Map<string, string>()
private readonly sessionProfileIdByPageId = new Map<string, string | null>()
@@ -394,6 +401,16 @@ export class BrowserManager {
return null
}
/** Shared across the whole opener tree, so a chain of popups draws from one budget. */
private tryConsumePageInitiatedTab(rootGuestWebContentsId: number): boolean {
let budget = this.pageInitiatedTabBudgetByRootGuestId.get(rootGuestWebContentsId)
if (!budget) {
budget = createPageInitiatedTabBudget()
this.pageInitiatedTabBudgetByRootGuestId.set(rootGuestWebContentsId, budget)
}
return budget.tryConsume(Date.now())
}
private resolveRendererForBrowserTab(browserTabId: string): Electron.WebContents | null {
const rendererWebContentsId = this.rendererWebContentsIdByTabId.get(browserTabId)
if (!rendererWebContentsId) {
@@ -758,8 +775,9 @@ export class BrowserManager {
this.attachGuestPolicies(window.webContents, this.resolvePopupOwnerContext(guest.id))
}
guest.on('did-create-window', handleDidCreateWindow)
guest.setWindowOpenHandler(({ url, frameName }) => {
const browserTabId = this.resolveBrowserTabIdForGuestWebContentsId(guest.id)
guest.setWindowOpenHandler(({ url, frameName, disposition, features }) => {
const ownerContext = this.resolvePopupOwnerContext(guest.id)
const browserTabId = ownerContext?.browserTabId ?? null
const browserUrl = normalizeBrowserNavigationUrl(url)
const externalUrl = normalizeExternalBrowserUrl(url)
const expectedClickedLinkFrameName = this.clickedLinkFrameNameByGuestId.get(guest.id)
@@ -784,6 +802,33 @@ export class BrowserManager {
return { action: 'deny' }
}
// Why: an unnamed, featureless window.open() is Chromium's own new-tab shape, so an Orca tab is
// the honest presentation; a floating origin-bar window is not. Opener-dependent shapes are
// excluded by isNewBrowserTabPopupIntent and still get a real child window below.
if (
ownerContext &&
externalUrl &&
isNewBrowserTabPopupIntent({ frameName, disposition, features })
) {
// Why: one activation lets a page loop window.open, and each routed tab persists into
// workspace session state, so it survives the quit that used to clear popup windows.
if (!this.tryConsumePageInitiatedTab(ownerContext.rootGuestWebContentsId)) {
this.forwardOrQueuePopupEvent(guest.id, {
origin: safeOrigin(externalUrl),
action: 'blocked'
})
return { action: 'deny' }
}
if (this.openLinkInOrcaTab(ownerContext.browserTabId, externalUrl)) {
this.forwardOrQueuePopupEvent(guest.id, {
origin: safeOrigin(externalUrl),
action: 'opened-in-orca'
})
}
// Why: a recognized new-tab intent must never fall through to a native popup if its renderer vanished mid-open.
return { action: 'deny' }
}
// Why: file URLs are fine for in-pane previews, but must not spawn native child windows targeting local paths.
const canOpenAsChild = Boolean(externalUrl || browserUrl === ORCA_BROWSER_BLANK_URL)
if (browserTabId && canOpenAsChild) {
@@ -1243,6 +1288,7 @@ export class BrowserManager {
this.clickedLinkFrameNameByGuestId.delete(guestWebContentsId)
this.offscreenGuestIds.delete(guestWebContentsId)
this.popupOwnerContextByGuestId.delete(guestWebContentsId)
this.pageInitiatedTabBudgetByRootGuestId.delete(guestWebContentsId)
this.authUserAgentOverrideStateByGuestId.delete(guestWebContentsId)
this.pendingNavigationByGuestId.delete(guestWebContentsId)
// Why: a popup must stop inheriting authorization the moment its owner retires, before Chromium destroys the child.
@@ -1444,6 +1490,7 @@ export class BrowserManager {
this.clickedLinkFrameNameByGuestId.clear()
this.tabIdByWebContentsId.clear()
this.popupOwnerContextByGuestId.clear()
this.pageInitiatedTabBudgetByRootGuestId.clear()
this.worktreeIdByTabId.clear()
this.sessionProfileIdByPageId.clear()
this.userAgentModeByPageId.clear()
@@ -0,0 +1,34 @@
import { describe, expect, it } from 'vitest'
import {
createPageInitiatedTabBudget,
MAX_PAGE_INITIATED_TABS_PER_WINDOW,
PAGE_INITIATED_TAB_WINDOW_MS
} from './browser-page-initiated-tab-budget'
describe('page-initiated tab budget', () => {
it('absorbs a window.open loop fired from a single activation', () => {
const budget = createPageInitiatedTabBudget()
const granted = Array.from({ length: 12 }, () => budget.tryConsume(1_000)).filter(Boolean)
expect(granted).toHaveLength(MAX_PAGE_INITIATED_TABS_PER_WINDOW)
})
it('refills once the rolling window has passed, so real browsing is unaffected', () => {
const budget = createPageInitiatedTabBudget()
for (let i = 0; i < MAX_PAGE_INITIATED_TABS_PER_WINDOW; i++) {
expect(budget.tryConsume(1_000)).toBe(true)
}
expect(budget.tryConsume(1_000 + PAGE_INITIATED_TAB_WINDOW_MS - 1)).toBe(false)
expect(budget.tryConsume(1_000 + PAGE_INITIATED_TAB_WINDOW_MS)).toBe(true)
})
it('slides rather than resetting, so a paced flood cannot outrun the cap', () => {
const budget = createPageInitiatedTabBudget(2, 1_000)
expect(budget.tryConsume(0)).toBe(true)
expect(budget.tryConsume(900)).toBe(true)
// The first grant has aged out by 1_000 but the second has not.
expect(budget.tryConsume(1_000)).toBe(true)
expect(budget.tryConsume(1_100)).toBe(false)
})
})
@@ -0,0 +1,26 @@
/** Bounded so one hostile click cannot plant an unbounded number of tabs. */
export const MAX_PAGE_INITIATED_TABS_PER_WINDOW = 4
export const PAGE_INITIATED_TAB_WINDOW_MS = 2_000
export type PageInitiatedTabBudget = {
/** Records the grant when it returns true; call only when the tab is actually being opened. */
tryConsume: (now: number) => boolean
}
/** Rolling window, so it absorbs a same-tick `window.open` loop without capping real browsing. */
export function createPageInitiatedTabBudget(
maxPerWindow = MAX_PAGE_INITIATED_TABS_PER_WINDOW,
windowMs = PAGE_INITIATED_TAB_WINDOW_MS
): PageInitiatedTabBudget {
let grants: number[] = []
return {
tryConsume: (now) => {
grants = grants.filter((grantedAt) => now - grantedAt < windowMs)
if (grants.length >= maxPerWindow) {
return false
}
grants.push(now)
return true
}
}
}
@@ -0,0 +1,17 @@
/**
* Whether a `window.open()` asks for a new tab rather than a popup window.
*
* Orca answers a tab by denying, which hands the page `null`, so named and featured opens — whose
* flow may use that handle — stay popups.
*/
export function isNewBrowserTabPopupIntent(details: {
frameName: string
disposition: string
features: string
}): boolean {
return (
details.frameName === '' &&
details.features.trim() === '' &&
(details.disposition === 'foreground-tab' || details.disposition === 'background-tab')
)
}
+2 -6
View File
@@ -40,9 +40,7 @@ describe('getWindowsManagedLifecycleHook', () => {
const hook = getWindowsManagedLifecycleHook(scriptPath)
expect(hook.args).toBeUndefined()
expect(hook.command).toMatch(
/\/powershell\.exe -NoProfile -WindowStyle Hidden -EncodedCommand /
)
expect(hook.command).toMatch(/\/powershell\.exe -NoProfile -EncodedCommand /)
expect(hook.command).not.toContain(scriptPath)
// Why: Git Bash/MSYS mangles backslash paths and slash-prefixed switches.
expect(hook.command.replace(/-EncodedCommand \S+$/, '')).not.toMatch(/\\| \/[a-zA-Z]+( |$)/)
@@ -384,9 +382,7 @@ describe('ClaudeHookService.install', () => {
for (const eventName of ['UserPromptSubmit', 'Stop', 'StopFailure']) {
const hook = settings.hooks[eventName]?.[0]?.hooks?.[0]
expect(hook?.args).toBeUndefined()
expect(hook?.command).toMatch(
/\/powershell\.exe -NoProfile -WindowStyle Hidden -EncodedCommand /
)
expect(hook?.command).toMatch(/\/powershell\.exe -NoProfile -EncodedCommand /)
expect(hook?.command).not.toContain(scriptPath)
const encoded = hook?.command.match(/-EncodedCommand (\S+)$/)?.[1]
@@ -31,7 +31,7 @@ import { CodexHookService } from './hook-service'
import { runExclusivelyForCodexTrustConfig } from './codex-trust-config-mutation-queue'
const WINDOWS_POWERSHELL_LAUNCHER =
/^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -WindowStyle Hidden -EncodedCommand \S+$/
/^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -EncodedCommand \S+$/
const homes = setupCodexHookHomes(homedirMock, getPathMock)
+1 -1
View File
@@ -21,7 +21,7 @@ vi.mock('os', async () => {
import { CommandCodeHookService } from './hook-service'
const WINDOWS_POWERSHELL_LAUNCHER =
/^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -WindowStyle Hidden -EncodedCommand \S+$/
/^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -EncodedCommand \S+$/
describe('CommandCodeHookService', () => {
let homeDir: string
+1 -1
View File
@@ -23,7 +23,7 @@ import { CURSOR_EVENTS, type CursorEvent } from './hook-events'
const CURSOR_SCRIPT_FILE_NAME = process.platform === 'win32' ? 'cursor-hook.cmd' : 'cursor-hook.sh'
const WINDOWS_POWERSHELL_LAUNCHER =
/^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -WindowStyle Hidden -EncodedCommand \S+$/
/^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -EncodedCommand \S+$/
type InstalledCursorHooks = {
hooks: Record<string, { command?: string }[]>
@@ -1,4 +1,5 @@
import { DAEMON_ENDPOINT_LOST_MESSAGE } from './daemon-endpoint-ownership'
import { DaemonConnectionLostError } from './daemon-errors'
import { DAEMON_UNAVAILABLE_RECONNECT_MESSAGE } from './types'
/**
@@ -17,6 +18,9 @@ export function isDaemonGoneError(err: unknown): boolean {
if (!(err instanceof Error)) {
return false
}
if (err instanceof DaemonConnectionLostError) {
return true
}
const errno = err as NodeJS.ErrnoException
if ((errno.code === 'ENOENT' || errno.code === 'ECONNREFUSED') && errno.syscall === 'connect') {
return true
@@ -0,0 +1,78 @@
import { expect, it, vi } from 'vitest'
import { mkdtempSync, rmSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import { DaemonPtyAdapter } from './daemon-pty-adapter'
import { DaemonServer } from './daemon-server'
import { createMockSubprocess } from './daemon-pty-adapter-test-harness'
import { getDaemonSocketPath } from './daemon-spawner'
it('recovers concurrent spawns rejected by an adapter-initiated disconnect', async () => {
const concurrentSpawnCount = 70
const dir = mkdtempSync(join(tmpdir(), 'daemon-concurrent-recovery-test-'))
const socketPath = getDaemonSocketPath(dir)
const tokenPath = join(dir, 'test.token')
const allPreparationsEntered = Promise.withResolvers<void>()
const releaseHeldPreparations = Promise.withResolvers<void>()
const spawnedSessionIds: string[] = []
let preparationCount = 0
const spawnSubprocess: ConstructorParameters<typeof DaemonServer>[0]['spawnSubprocess'] = (
options
) => {
spawnedSessionIds.push(options.sessionId)
return createMockSubprocess()
}
let server = new DaemonServer({
socketPath,
tokenPath,
spawnSubprocess,
// Hold siblings until one retriable reply makes the adapter disconnect their shared socket.
preparePtySpawn: async () => {
const preparationNumber = ++preparationCount
if (preparationCount === concurrentSpawnCount) {
allPreparationsEntered.resolve()
}
await allPreparationsEntered.promise
if (preparationNumber === 1) {
throw new Error('Connection lost')
}
await releaseHeldPreparations.promise
}
})
await server.start()
const respawn = vi.fn(async () => {
await server.shutdown()
releaseHeldPreparations.resolve()
server = new DaemonServer({ socketPath, tokenPath, spawnSubprocess })
await server.start()
})
const adapter = new DaemonPtyAdapter({ socketPath, tokenPath, respawn })
try {
const sessionIds = Array.from(
{ length: concurrentSpawnCount },
(_, index) => `high-volume-${index}`
)
const results = await Promise.all(
sessionIds.map((sessionId) => adapter.spawn({ sessionId, cols: 80, rows: 24 }))
)
expect(preparationCount).toBe(concurrentSpawnCount)
expect(respawn).toHaveBeenCalledTimes(1)
expect(respawn).toHaveBeenCalledWith('daemon_died')
expect(new Set(results.map(({ id }) => id))).toEqual(new Set(sessionIds))
expect(new Set(spawnedSessionIds)).toEqual(new Set(sessionIds))
const later = await adapter.spawn({ sessionId: 'after-recovery', cols: 80, rows: 24 })
expect(later.id).toBe('after-recovery')
expect(respawn).toHaveBeenCalledTimes(1)
expect(spawnedSessionIds).toHaveLength(concurrentSpawnCount + 1)
} finally {
adapter.dispose()
releaseHeldPreparations.resolve()
await server.shutdown()
rmSync(dir, { recursive: true, force: true })
}
})
+1 -1
View File
@@ -25,7 +25,7 @@ vi.mock('os', async () => {
import { DroidHookService } from './hook-service'
const WINDOWS_POWERSHELL_LAUNCHER =
/^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -WindowStyle Hidden -EncodedCommand \S+$/
/^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -EncodedCommand \S+$/
describe('DroidHookService', () => {
let homeDir: string
+1 -1
View File
@@ -26,7 +26,7 @@ vi.mock('os', async (importOriginal) => {
import { GeminiHookService } from './hook-service'
const WINDOWS_POWERSHELL_LAUNCHER =
/^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -WindowStyle Hidden -EncodedCommand \S+$/
/^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -EncodedCommand \S+$/
describe('GeminiHookService', () => {
let homeDir: string
+1 -1
View File
@@ -33,7 +33,7 @@ import { POSIX_HOOK_STDIN_READER } from '../agent-hooks/hook-stdin-contract'
const GROK_SCRIPT_FILE_NAME = process.platform === 'win32' ? 'grok-hook.cmd' : 'grok-hook.sh'
const WINDOWS_POWERSHELL_LAUNCHER =
/^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -WindowStyle Hidden -EncodedCommand \S+$/
/^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -EncodedCommand \S+$/
// Why (#14828): Windows registers the bare script path when it is cmd-safe and only falls back
// to the encoded launcher for a profile path that is not (#6078). windows-hook-launcher-chain
@@ -393,6 +393,81 @@ describe('renderer breadcrumb IPC routing', () => {
])
})
// Why: a #185 crash loop emits a cascade notice per cascade, back to back,
// and uncoalesced they would flush the 30-entry ring holding the trail.
it('coalesces react commit cascade notices from one driver into one slot', () => {
emitRendererBreadcrumb({
name: 'react_commit_cascade',
data: { commits: 40, rendererSurface: 'main', driverFrame: 'tabs.ts:12:3 bump' }
})
emitRendererBreadcrumb({
name: 'react_commit_cascade',
data: { commits: 41, rendererSurface: 'main', driverFrame: 'tabs.ts:12:3 bump' }
})
expect(recordCrashBreadcrumbMock).not.toHaveBeenCalled()
expect(
recordCoalescedCrashBreadcrumbMock.mock.calls.map(
(call) => (call[0] as { coalesceKey: string }).coalesceKey
)
).toEqual([
'react_commit_cascade:main:tabs.ts:12:3 bump',
'react_commit_cascade:main:tabs.ts:12:3 bump'
])
})
// Why keyed and not name-only: coalescing keeps only the newest payload, so
// one surface or driver would erase the other's evidence.
it('keeps different cascade surfaces and drivers in separate slots', () => {
emitRendererBreadcrumb({
name: 'react_commit_cascade',
data: { commits: 40, rendererSurface: 'main', driverFrame: 'tabs.ts:12:3 bump' }
})
emitRendererBreadcrumb({
name: 'react_commit_cascade',
data: { commits: 40, rendererSurface: 'dashboard-popout', driverFrame: 'tabs.ts:12:3 bump' }
})
emitRendererBreadcrumb({
name: 'react_commit_cascade',
data: { commits: 40, rendererSurface: 'main', storeWrites: 0 }
})
expect(
new Set(
recordCoalescedCrashBreadcrumbMock.mock.calls.map(
(call) => (call[0] as { coalesceKey: string }).coalesceKey
)
).size
).toBe(3)
})
// Why asserted here: the key ends in `stack`, which is what buys the driver
// frames the 4000-char budget instead of the 240-char one details get.
it('keeps the full driver stack on a react commit cascade notice', () => {
const driverStack = `tabs.ts:12:3 bump\n${'tabs.ts:99:1 other\n'.repeat(60)}`
emitRendererBreadcrumb({
name: 'react_commit_cascade',
data: { commits: 40, rendererSurface: 'main', driverFrame: 'tabs.ts:12:3 bump', driverStack }
})
const recorded = recordCoalescedCrashBreadcrumbMock.mock.calls[0]?.[0] as
| { data: { driverStack: string } }
| undefined
expect(recorded?.data.driverStack.length).toBeGreaterThan(240)
})
// Why uncoalesced: it fires at most once per renderer, and it is the signal
// that the diagnostic itself never wired up.
it('records the uninstalled self-check notice directly', () => {
emitRendererBreadcrumb({ name: 'react_commit_cascade_uninstalled' })
expect(recordCrashBreadcrumbMock).toHaveBeenCalledWith(
'react_commit_cascade_uninstalled',
undefined
)
expect(recordCoalescedCrashBreadcrumbMock).not.toHaveBeenCalled()
})
it('records non-error renderer breadcrumbs without coalescing', () => {
emitRendererBreadcrumb({ name: 'renderer_bootstrap_started', data: { dev: true } })
@@ -48,12 +48,14 @@ function recordRendererBreadcrumbTrace(
// suppressed count instead.
const DUPLICATE_TAB_OWNER_BREADCRUMB = 'terminal_tab_id_owned_by_multiple_worktrees'
const PARK_VERDICT_CHURN_BREADCRUMB = 'terminal_park_verdict_churn'
const REACT_COMMIT_CASCADE_BREADCRUMB = 'react_commit_cascade'
const COALESCED_RENDERER_BREADCRUMB_NAMES = new Set([
'renderer_error',
'renderer_unhandled_rejection',
'terminal_safe_fit_retry_exhausted',
DUPLICATE_TAB_OWNER_BREADCRUMB,
PARK_VERDICT_CHURN_BREADCRUMB,
REACT_COMMIT_CASCADE_BREADCRUMB,
TERMINAL_WEBGL_DIAGNOSTIC_BREADCRUMB
])
const RENDERER_BREADCRUMB_COALESCE_MS = 30_000
@@ -82,6 +84,13 @@ function rendererBreadcrumbCoalesceKey(
if (name === PARK_VERDICT_CHURN_BREADCRUMB) {
return `${name}:${String(data?.trigger ?? '')}`
}
// Why keyed on surface and driver frame: the popout and the main window can
// cascade independently, and two different driving writes are two different
// bugs that last-write coalescing would collapse into one. A driverless crumb
// (storeWrites 0, a useState loop) keys separately for the same reason.
if (name === REACT_COMMIT_CASCADE_BREADCRUMB) {
return `${name}:${String(data?.rendererSurface ?? '')}:${String(data?.driverFrame ?? '')}`
}
// Preserve distinct GPU failures and atlas-reset triggers while coalescing each storm.
if (name === TERMINAL_WEBGL_DIAGNOSTIC_BREADCRUMB) {
const kind = String(data?.kind ?? '')
+73 -17
View File
@@ -184,7 +184,7 @@ describe('PowerShell fallback when the native binding is absent', () => {
})
})
describe('wedge cooldown', () => {
describe('sticky wedge', () => {
let platform: PropertyDescriptor | undefined
beforeEach(() => {
@@ -204,7 +204,7 @@ describe('wedge cooldown', () => {
// The vendored reader latches a global while a request is in flight and
// drains its queue only when that request completes. In the wedge this
// guards against it never does, so every retry would add a closure that is
// never called. One probe per cooldown bounds that.
// never called.
vi.useFakeTimers()
const getAllProcesses = vi.fn(() => {})
__setWindowsProcessTreeLoaderForTests(() => ({
@@ -218,15 +218,15 @@ describe('wedge cooldown', () => {
await firstAssertion
expect(getAllProcesses).toHaveBeenCalledTimes(1)
await expect(readWindowsProcessTableFresh()).rejects.toThrow(/cooling down/)
await expect(readWindowsProcessTableFresh()).rejects.toThrow(/wedged/)
expect(getAllProcesses).toHaveBeenCalledTimes(1)
})
it('lets exactly one caller probe when the cooldown expires', async () => {
// Without claiming the recovery slot before probing, every concurrent
// caller passes the cooldown check at expiry and each enqueues a callback
// into the still-latched native queue -- so each cycle leaks another batch
// rather than bounding it to one probe.
it('never re-enters the reader while the timed-out read is still out', async () => {
// A cooldown bounds the RATE of new callbacks, not the total: one probe per
// window still retains one more closure in the still-latched native queue
// every window, for the life of the app. Only the outstanding read can
// reopen the gate, so a permanent wedge retains exactly one callback.
vi.useFakeTimers()
const getAllProcesses = vi.fn(() => {})
__setWindowsProcessTreeLoaderForTests(() => ({
@@ -240,16 +240,72 @@ describe('wedge cooldown', () => {
await wedgeAssertion
expect(getAllProcesses).toHaveBeenCalledTimes(1)
await vi.advanceTimersByTimeAsync(30_000)
const attempts = [
readWindowsProcessTableFresh().catch(() => 'rejected'),
readWindowsProcessTableFresh().catch(() => 'rejected'),
readWindowsProcessTableFresh().catch(() => 'rejected')
]
await vi.advanceTimersByTimeAsync(3_000)
await Promise.all(attempts)
// Four windows of the cooldown this replaced, each with concurrent callers.
// Rejection reasons are deliberately not asserted here: the leak this test
// pins is the call count, and it must fail on that alone.
for (let window = 0; window < 4; window += 1) {
await vi.advanceTimersByTimeAsync(30_000)
const attempts = [
readWindowsProcessTableFresh().catch(() => 'rejected'),
readWindowsProcessTableFresh().catch(() => 'rejected'),
readWindowsProcessTableFresh().catch(() => 'rejected')
]
// Long enough for a probe's own deadline, had one been let through.
await vi.advanceTimersByTimeAsync(3_000)
expect(await Promise.all(attempts)).toEqual(['rejected', 'rejected', 'rejected'])
}
// One recovery probe, not three.
// Still the one callback the wedge is holding -- not one more per window.
expect(getAllProcesses).toHaveBeenCalledTimes(1)
})
it('resumes as soon as the timed-out read finally calls back', async () => {
// The stuck request completing is what drains the vendored queue, so it is
// the only honest evidence the reader recovered. Waiting out a wall clock
// afterwards would strand a reader that is already answering.
vi.useFakeTimers()
let stuck: ((rows: typeof NATIVE | undefined) => void) | undefined
const getAllProcesses = vi.fn((cb: (rows: typeof NATIVE | undefined) => void) => {
if (stuck) {
cb(NATIVE)
return
}
stuck = cb
})
__setWindowsProcessTreeLoaderForTests(() => ({
ProcessDataFlag: { None: 0, Memory: 1, CommandLine: 2 },
getAllProcesses
}))
const wedge = readWindowsProcessTableFresh()
const wedgeAssertion = expect(wedge).rejects.toThrow(/timed out/)
await vi.advanceTimersByTimeAsync(3_000)
await wedgeAssertion
await expect(readWindowsProcessTableFresh()).rejects.toThrow(/wedged/)
stuck?.(NATIVE)
await expect(readWindowsProcessTableFresh()).resolves.toHaveLength(NATIVE.length)
expect(getAllProcesses).toHaveBeenCalledTimes(2)
})
it('ignores a pending timeout from before a test reset', async () => {
vi.useFakeTimers()
const getAllProcesses = vi.fn((_cb: (rows: typeof NATIVE | undefined) => void) => {})
__setWindowsProcessTreeLoaderForTests(() => ({
ProcessDataFlag: { None: 0, Memory: 1, CommandLine: 2 },
getAllProcesses
}))
const staleRead = readWindowsProcessTableFresh()
const staleAssertion = expect(staleRead).rejects.toThrow(/timed out/)
await vi.advanceTimersByTimeAsync(0)
expect(getAllProcesses).toHaveBeenCalledTimes(1)
resetWindowsProcessTableForTests()
await vi.advanceTimersByTimeAsync(3_000)
await staleAssertion
getAllProcesses.mockImplementation((cb) => cb(NATIVE))
await expect(readWindowsProcessTableFresh()).resolves.toHaveLength(NATIVE.length)
expect(getAllProcesses).toHaveBeenCalledTimes(2)
})
+26 -33
View File
@@ -146,20 +146,18 @@ function loadWindowsProcessTree(): WindowsProcessTreeModule | null {
const WINDOWS_PROCESS_QUERY_TIMEOUT_MS = 3_000
/**
* How long to stop calling the reader after it misses its deadline.
*
* Why a cooldown and not just the deadline: a timed-out call leaves its
* callback in the vendored module's queue, and that queue only drains when the
* latched request finally completes -- which, in the wedge this guards against,
* never happens. Retrying at the caller's poll rate would then add a closure
* per tick forever. One probe per cooldown bounds it.
* Reads that missed their deadline and have not called back yet.
* Refusing re-entry bounds both vendored callbacks and relay addon workers to
* one; read ids keep a late callback from clearing a newer wedge.
*/
const WINDOWS_PROCESS_QUERY_COOLDOWN_MS = 30_000
const unreturnedReads = new Set<number>()
let readSequence = 0
let nativeReaderEpoch = 0
let wedgedUntilMs = 0
// Why a generation: a request that already lost its deadline must not later
// clear or re-arm the wedge on behalf of the request that replaced it.
let readGeneration = 0
function resetNativeReaderState(): void {
nativeReaderEpoch += 1
unreturnedReads.clear()
}
function readNativeRows(): Promise<WindowsProcessRow[]> {
const native = moduleLoader()
@@ -177,19 +175,13 @@ function readNativeRows(): Promise<WindowsProcessRow[]> {
// tree dead. "Unavailable" has to stay distinguishable from "empty".
return Promise.reject(new Error('windows process table unavailable'))
}
const startedAt = Date.now()
if (startedAt < wedgedUntilMs) {
return Promise.reject(new Error('windows process table is cooling down after a timeout'))
if (unreturnedReads.size > 0) {
return Promise.reject(
new Error('windows process table is wedged: an earlier read has not returned')
)
}
if (wedgedUntilMs > 0) {
// Coming out of a wedge: re-arm the cooldown BEFORE probing, so exactly one
// caller gets through. Without this every concurrent caller passes the
// check above at expiry, each enqueues a callback into the still-latched
// native queue, and each cooldown cycle leaks another batch rather than
// bounding it to one probe.
wedgedUntilMs = startedAt + WINDOWS_PROCESS_QUERY_COOLDOWN_MS
}
const generation = ++readGeneration
const readId = ++readSequence
const readerEpoch = nativeReaderEpoch
// Why always both flags: each adds an OpenProcess per process (Memory a
// GetProcessMemoryInfo, CommandLine a PEB read), so asking for less would be
// cheaper -- 15.9ms p50 versus 30.6ms at 1050 processes. But every read shares
@@ -204,18 +196,19 @@ function readNativeRows(): Promise<WindowsProcessRow[]> {
let deadline: ReturnType<typeof setTimeout> | undefined
try {
deadline = setTimeout(() => {
if (generation === readGeneration) {
wedgedUntilMs = Date.now() + WINDOWS_PROCESS_QUERY_COOLDOWN_MS
// Test resets invalidate deadlines owned by the prior injected reader.
if (readerEpoch === nativeReaderEpoch) {
unreturnedReads.add(readId)
}
reject(new Error('windows process table timed out'))
}, WINDOWS_PROCESS_QUERY_TIMEOUT_MS)
deadline.unref?.()
native.getAllProcesses((processes) => {
clearTimeout(deadline)
// A callback proves the reader is answering, so stop refusing.
if (generation === readGeneration) {
wedgedUntilMs = 0
}
// A callback proves this read drained, so stop refusing. Unconditional:
// dropping an id that was never added is a no-op, and only the read
// that actually wedged can be holding the gate shut.
unreturnedReads.delete(readId)
if (!processes) {
reject(new Error('windows process table returned no snapshot'))
return
@@ -303,7 +296,7 @@ export function __setWindowsProcessTreeLoaderForTests(
): void {
moduleLoader = loader ?? loadWindowsProcessTree
cachedModule = undefined
wedgedUntilMs = 0
resetNativeReaderState()
snapshotReader.reset()
}
@@ -314,7 +307,7 @@ export function __setWindowsProcessTreeRequireForTests(
requireNative = resolve ?? requireFromMain
moduleLoader = loadWindowsProcessTree
cachedModule = undefined
wedgedUntilMs = 0
resetNativeReaderState()
snapshotReader.reset()
}
@@ -330,5 +323,5 @@ export function __setWindowsProcessTableCimScanForTests(
export function resetWindowsProcessTableForTests(): void {
snapshotReader.reset()
cachedModule = undefined
wedgedUntilMs = 0
resetNativeReaderState()
}
@@ -1,26 +1,14 @@
import React from 'react'
import { cn } from '@/lib/utils'
import {
getWorkItemStateTone,
WorkItemStateBadge as SharedWorkItemStateBadge
} from '@/components/github/work-item-state-badge'
import type { GitHubWorkItem } from '../../../../../shared/github/work-item-types'
import { getStateLabel } from '@/components/github/work-item-state-presentation'
// Why: closed issues stay neutral here (may be completed/resolved); red is reserved
// for PR closed-without-merge.
export function getStateTone(item: GitHubWorkItem): string {
if (item.type === 'pr') {
if (item.state === 'merged') {
return 'border-purple-500/30 bg-purple-500/10 text-purple-600 dark:text-purple-300'
}
if (item.state === 'draft') {
return 'border-slate-500/30 bg-slate-500/10 text-slate-600 dark:text-slate-300'
}
if (item.state === 'closed') {
return 'border-rose-500/30 bg-rose-500/10 text-rose-600 dark:text-rose-300'
}
return 'border-emerald-500/30 bg-emerald-500/10 text-emerald-600 dark:text-emerald-300'
}
if (item.state === 'closed') {
// Why: keep closed issues neutral (may be completed/resolved); red is reserved for PR closed-without-merge.
return 'border-ring/50 bg-primary/10 text-foreground'
}
return 'border-emerald-500/30 bg-emerald-500/10 text-emerald-600 dark:text-emerald-300'
return getWorkItemStateTone(item, 'neutral')
}
export function WorkItemStateBadge({
@@ -30,15 +18,5 @@ export function WorkItemStateBadge({
item: GitHubWorkItem
className?: string
}): React.JSX.Element {
return (
<span
className={cn(
'inline-flex h-5 items-center rounded-full border px-2 text-[11px] font-medium',
getStateTone(item),
className
)}
>
{getStateLabel(item)}
</span>
)
return <SharedWorkItemStateBadge item={item} className={className} closedIssue="neutral" />
}
@@ -0,0 +1,41 @@
import { GitMerge, GitPullRequestClosed, GitPullRequestDraft, type LucideIcon } from 'lucide-react'
import type { HostedReviewState } from '../../../../shared/hosted-review'
/** Every review surface renders state from here; `unknown` means we have a number but no fetched state. */
export type ReviewStateForDisplay = HostedReviewState | 'unknown' | null | undefined
/** Also the tone for an open issue, which reads the same as an open review. */
export const OPEN_REVIEW_STATE_TONE =
'border-emerald-500/30 bg-emerald-500/10 text-emerald-600 dark:text-emerald-300'
/** Outline-pill tone: purple merged, rose closed, slate draft, emerald open, neutral unknown. */
export function getReviewStateTone(state: ReviewStateForDisplay): string {
if (state === 'merged') {
return 'border-purple-500/30 bg-purple-500/10 text-purple-600 dark:text-purple-300'
}
if (state === 'draft') {
return 'border-slate-500/30 bg-slate-500/10 text-slate-600 dark:text-slate-300'
}
if (state === 'closed') {
return 'border-rose-500/30 bg-rose-500/10 text-rose-600 dark:text-rose-300'
}
if (state === 'open') {
return OPEN_REVIEW_STATE_TONE
}
return 'border-border bg-background text-muted-foreground'
}
// Why: the glyph must carry review state — tone alone made a draft with failing
// checks render as a red PR icon, which reads as closed.
export function getReviewStateIcon(state: ReviewStateForDisplay): LucideIcon | null {
if (state === 'merged') {
return GitMerge
}
if (state === 'closed') {
return GitPullRequestClosed
}
if (state === 'draft') {
return GitPullRequestDraft
}
return null
}
@@ -0,0 +1,48 @@
import React from 'react'
import { cn } from '@/lib/utils'
import { getStateLabel } from '@/components/github/work-item-state-presentation'
import {
getReviewStateTone,
OPEN_REVIEW_STATE_TONE
} from '@/components/github/review-state-presentation'
import type { GitHubWorkItem } from '../../../../shared/github/work-item-types'
/**
* Whether a closed ISSUE reads as failure or as neutral completion. PR state tone
* never varies — only surfaces disagree on what a closed issue means.
*/
export type ClosedIssueTone = 'destructive' | 'neutral'
export function getWorkItemStateTone(item: GitHubWorkItem, closedIssue: ClosedIssueTone): string {
if (item.type === 'pr') {
return getReviewStateTone(item.state)
}
if (item.state === 'closed') {
return closedIssue === 'neutral'
? 'border-ring/50 bg-primary/10 text-foreground'
: 'border-rose-500/30 bg-rose-500/10 text-rose-600 dark:text-rose-300'
}
return OPEN_REVIEW_STATE_TONE
}
export function WorkItemStateBadge({
item,
className,
closedIssue
}: {
item: GitHubWorkItem
className?: string
closedIssue: ClosedIssueTone
}): React.JSX.Element {
return (
<span
className={cn(
'inline-flex h-5 items-center rounded-full border px-2 text-[11px] font-medium',
getWorkItemStateTone(item, closedIssue),
className
)}
>
{getStateLabel(item)}
</span>
)
}
@@ -1,25 +1,12 @@
import React from 'react'
import { cn } from '@/lib/utils'
import { getStateLabel } from '@/components/github/work-item-state-presentation'
import {
getWorkItemStateTone,
WorkItemStateBadge as SharedWorkItemStateBadge
} from '@/components/github/work-item-state-badge'
import type { GitHubWorkItem } from '../../../../../shared/github/work-item-types'
export function getStateTone(item: GitHubWorkItem): string {
if (item.type === 'pr') {
if (item.state === 'merged') {
return 'border-purple-500/30 bg-purple-500/10 text-purple-600 dark:text-purple-300'
}
if (item.state === 'draft') {
return 'border-slate-500/30 bg-slate-500/10 text-slate-600 dark:text-slate-300'
}
if (item.state === 'closed') {
return 'border-rose-500/30 bg-rose-500/10 text-rose-600 dark:text-rose-300'
}
return 'border-emerald-500/30 bg-emerald-500/10 text-emerald-600 dark:text-emerald-300'
}
if (item.state === 'closed') {
return 'border-rose-500/30 bg-rose-500/10 text-rose-600 dark:text-rose-300'
}
return 'border-emerald-500/30 bg-emerald-500/10 text-emerald-600 dark:text-emerald-300'
return getWorkItemStateTone(item, 'destructive')
}
/** Filled counterpart of `getStateTone` for high-emphasis surfaces like the PR page header. */
@@ -42,15 +29,5 @@ export function WorkItemStateBadge({
item: GitHubWorkItem
className?: string
}): React.JSX.Element {
return (
<span
className={cn(
'inline-flex h-5 items-center rounded-full border px-2 text-[11px] font-medium',
getStateTone(item),
className
)}
>
{getStateLabel(item)}
</span>
)
return <SharedWorkItemStateBadge item={item} className={className} closedIssue="destructive" />
}
@@ -1,5 +1,6 @@
import { GitMerge, GitPullRequestClosed, GitPullRequestDraft } from 'lucide-react'
import { GitMerge } from 'lucide-react'
import { cn } from '@/lib/utils'
import { getReviewStateIcon } from '@/components/github/review-state-presentation'
import { PullRequestIcon } from './WorktreeCardHelpers'
import type { WorktreeCardPrDisplay } from './worktree-card-pr-display'
@@ -23,23 +24,6 @@ export function getProviderName(review: WorktreeCardPrDisplay): string {
return 'GitHub'
}
// Why: the glyph must carry review state — tone alone made a draft with failing
// checks render as a red PR icon, which reads as closed.
function getStateIcon(
state: WorktreeCardPrDisplay['state']
): React.ComponentType<{ className?: string }> | null {
if (state === 'merged') {
return GitMerge
}
if (state === 'closed') {
return GitPullRequestClosed
}
if (state === 'draft') {
return GitPullRequestDraft
}
return null
}
// Why: checks only gate a review that is actually open; draft/closed/merged keep
// their state tone so the glyph agrees with its tooltip. A stateless row (folder
// cards render one while a linked review is loading or its details failed) has no
@@ -88,6 +72,6 @@ export function ReviewIcon({
}): React.JSX.Element {
const providerIcon =
variant === 'provider' && review.provider === 'gitlab' ? GitMerge : PullRequestIcon
const Icon = getStateIcon(review.state) ?? providerIcon
const Icon = getReviewStateIcon(review.state) ?? providerIcon
return <Icon className={cn(className, getCheckTone(review) ?? getStateTone(review.state))} />
}
@@ -10,13 +10,12 @@
* one tab as "damping did not reach the oscillating input".
*/
import { recordRendererCrashBreadcrumb } from '@/lib/crash-breadcrumb-recorder'
import { REACT_NESTED_UPDATE_LIMIT } from '../../../../shared/react-update-depth-attribution'
export const TERMINAL_TAB_PARK_FLIP_WINDOW_MS = 60_000
/** Flips per window that no sane park policy should reach. Breadcrumb only. */
export const TERMINAL_TAB_PARK_FLIP_NOTICE_LIMIT = 12
/** react-dom 19.2.x nested commit limit. */
const REACT_NESTED_UPDATE_LIMIT = 50
/** Measured upper bound after the passive-effect pin engages. */
const PARK_PIN_SETTLE_COMMITS = 6
/** Worst-case commits from pane, watcher, and store work per verdict flip. */
@@ -1,9 +1,6 @@
import { translate } from '@/i18n/i18n'
import type { WorkspaceCleanupCandidate } from '../../../../shared/workspace-cleanup'
import {
getWorkspaceCleanupGitLabel,
type WorkspaceCleanupReviewInfo
} from './workspace-cleanup-presentation'
import { getWorkspaceCleanupGitLabel } from './workspace-cleanup-presentation'
import {
formatUnpushedCommitCount,
formatWorkspaceCleanupContextDetail,
@@ -172,13 +169,6 @@ function hasGitStatusPill(candidate: WorkspaceCleanupCandidate): boolean {
return (candidate.git.upstreamAhead ?? 0) > 0 || candidate.git.clean === false
}
export function getReviewPillTone(reviewInfo: WorkspaceCleanupReviewInfo): StatusPillTone {
if (reviewInfo.state === 'open' || reviewInfo.state === 'draft') {
return 'review'
}
return 'neutral'
}
export function getContextCount(candidate: WorkspaceCleanupCandidate): number {
return (
candidate.localContext.terminalTabCount +
@@ -247,4 +247,78 @@ describe('CandidateRow', () => {
expect(container?.querySelector(`[aria-label="Select ${remoteName}"]`)).not.toBeNull()
expect(container?.querySelector(`[aria-label="Host: builder"]`)).not.toBeNull()
})
it('colors the review pill by state the way the PR page and item dialog do', () => {
const states = [
{ state: 'merged' as const, label: 'PR #1', hue: 'purple' },
{ state: 'closed' as const, label: 'PR #2', hue: 'rose' },
{ state: 'open' as const, label: 'PR #3', hue: 'emerald' },
{ state: 'draft' as const, label: 'PR #4', hue: 'slate' }
]
for (const { state, label, hue } of states) {
const candidate = makeCandidate()
act(() => {
root?.render(
<CandidateRow
identity={getWorkspaceCleanupCandidateIdentity(candidate)}
candidate={candidate}
expanded={false}
last
lastActivityLabel="1d ago"
reviewInfo={{
hasReview: true,
label,
provider: 'github',
state,
title: 'Some change'
}}
selected={false}
onIgnore={vi.fn()}
onRemove={vi.fn()}
onToggleExpanded={vi.fn()}
onToggleSelected={vi.fn()}
onView={vi.fn()}
/>
)
})
const pill = container?.querySelector(`[aria-label*="${label}"]`)
expect(pill?.className).toContain(`text-${hue}-`)
expect(pill?.className).not.toContain('text-muted-foreground')
}
})
it('leaves a review with an unfetched state neutral', () => {
const candidate = makeCandidate()
act(() => {
root?.render(
<CandidateRow
identity={getWorkspaceCleanupCandidateIdentity(candidate)}
candidate={candidate}
expanded={false}
last
lastActivityLabel="1d ago"
reviewInfo={{
hasReview: true,
label: 'PR #9',
provider: 'github',
state: 'unknown',
title: null
}}
selected={false}
onIgnore={vi.fn()}
onRemove={vi.fn()}
onToggleExpanded={vi.fn()}
onToggleSelected={vi.fn()}
onView={vi.fn()}
/>
)
})
const pill = container?.querySelector('[aria-label*="PR #9"]')
expect(pill?.className).toContain('text-muted-foreground')
})
})
@@ -36,10 +36,13 @@ import {
getCandidateFactStatuses,
getContextCount,
getDirtyGitLabel,
getReviewPillTone,
getWorkspaceCleanupBlockerLabels,
shouldShowGitMetadataChip
} from './workspace-cleanup-candidate-row-data'
import {
getReviewStateIcon,
getReviewStateTone
} from '@/components/github/review-state-presentation'
import { StatusPill } from './workspace-cleanup-status-pill'
import { WorkspaceCleanupMetadataChip } from './workspace-cleanup-metadata-chip'
import { WorkspaceCleanupForgetLocallyButton } from './workspace-cleanup-forget-locally-button'
@@ -255,10 +258,10 @@ export const CandidateRow = React.memo(function CandidateRow({
) : null}
{reviewInfo.label ? (
<WorkspaceCleanupMetadataChip
icon={GitPullRequest}
icon={getReviewStateIcon(reviewInfo.state) ?? GitPullRequest}
label={getReviewTooltip(reviewInfo)}
value={reviewInfo.label}
tone={getReviewPillTone(reviewInfo)}
toneClassName={getReviewStateTone(reviewInfo.state)}
/>
) : null}
</div>
@@ -218,4 +218,43 @@ describe('WorkspaceCleanupConfirmRemove', () => {
'1 workspace currently shows risk and may need a force delete'
)
})
it('gives the review pill a text equivalent for its state color without repeating the number', () => {
const candidate = makeFacetCandidate({
worktreeId: 'repo-1::/repo/merged-review',
displayName: 'Merged review',
path: '/repo/merged-review'
})
act(() => {
root.render(
<WorkspaceCleanupConfirmRemove
candidates={[candidate]}
now={Date.now()}
reviewInfoByWorktreeId={
new Map([
[
candidate.worktreeId,
{
hasReview: true,
label: 'PR #15716',
provider: 'github' as const,
state: 'merged' as const,
title: 'keep editor focus'
}
]
])
}
progress={null}
onBack={vi.fn()}
onCancel={vi.fn()}
onConfirm={vi.fn()}
/>
)
})
const srText = container.querySelector('span.sr-only')?.textContent ?? ''
expect(srText).toContain('Merged')
expect(srText).not.toContain('PR #15716')
})
})
@@ -15,11 +15,12 @@ import {
getCandidateFactStatuses,
formatContextDetailLabels,
getDirtyGitLabel,
getReviewPillTone,
shouldShowGitMetadataChip
} from './workspace-cleanup-candidate-row-data'
import type { WorkspaceCleanupReviewInfo } from './workspace-cleanup-presentation'
import { formatWorkspaceCleanupRelativeTime } from './workspace-cleanup-relative-time'
import { getReviewStateTone } from '@/components/github/review-state-presentation'
import { getReviewStateSrText } from './workspace-cleanup-row-labels'
import { StatusPill } from './workspace-cleanup-status-pill'
import { WorkspaceCleanupCandidateList } from './workspace-cleanup-candidate-list'
import {
@@ -255,7 +256,10 @@ function ConfirmRemoveRow({
</StatusPill>
))}
{reviewInfo.label ? (
<StatusPill tone={getReviewPillTone(reviewInfo)}>{reviewInfo.label}</StatusPill>
<StatusPill toneClassName={getReviewStateTone(reviewInfo.state)}>
{reviewInfo.label}
<span className="sr-only">{getReviewStateSrText(reviewInfo)}</span>
</StatusPill>
) : null}
{contextPillLabels.map((label) => (
<StatusPill key={label}>{label}</StatusPill>
@@ -8,12 +8,15 @@ export function WorkspaceCleanupMetadataChip({
icon: Icon,
label,
value,
tone = 'neutral'
tone = 'neutral',
toneClassName
}: {
icon: LucideIcon
label: string
value?: string
tone?: StatusPillTone
/** Overrides `tone` for chips whose color comes from a domain state (e.g. review state). */
toneClassName?: string
}): React.JSX.Element {
return (
<Tooltip>
@@ -25,7 +28,8 @@ export function WorkspaceCleanupMetadataChip({
tone === 'ready' &&
'border-[color:color-mix(in_srgb,var(--git-decoration-added)_45%,transparent)] bg-[color:color-mix(in_srgb,var(--git-decoration-added)_10%,transparent)] text-[var(--git-decoration-added)]',
tone === 'review' && 'bg-muted text-foreground',
tone === 'destructive' && 'border-destructive/30 text-destructive'
tone === 'destructive' && 'border-destructive/30 text-destructive',
toneClassName
)}
aria-label={label}
>
@@ -1,9 +1,20 @@
import type { WorkspaceCleanupReviewInfo } from './workspace-cleanup-presentation'
import { getWorkspaceCleanupReviewStateLabel } from './workspace-cleanup-facet-labels'
export function formatCompactActivityLabel(label: string): string {
return label === 'Just now' ? 'now' : label.replace(/ ago$/, '')
}
export function getReviewTooltip(reviewInfo: WorkspaceCleanupReviewInfo): string {
return [reviewInfo.label, reviewInfo.state, reviewInfo.title].filter(Boolean).join(' · ')
return [reviewInfo.label, getReviewStateSrText(reviewInfo)].filter(Boolean).join(' · ')
}
/** Text equivalent for the state color on pills that render the number visibly. */
export function getReviewStateSrText(reviewInfo: WorkspaceCleanupReviewInfo): string {
if (!reviewInfo.state) {
return ''
}
return [getWorkspaceCleanupReviewStateLabel(reviewInfo.state), reviewInfo.title]
.filter(Boolean)
.join(' · ')
}
@@ -4,10 +4,13 @@ import type { StatusPillTone } from './workspace-cleanup-candidate-row-data'
export function StatusPill({
children,
tone = 'neutral'
tone = 'neutral',
toneClassName
}: {
children: React.ReactNode
tone?: StatusPillTone
/** Overrides `tone` for pills whose color comes from a domain state (e.g. review state). */
toneClassName?: string
}): React.JSX.Element {
return (
<span
@@ -17,7 +20,8 @@ export function StatusPill({
tone === 'ready' &&
'border-status-success-border bg-status-success-background text-status-success',
tone === 'review' && 'border-border bg-muted text-foreground',
tone === 'destructive' && 'border-destructive/30 text-destructive'
tone === 'destructive' && 'border-destructive/30 text-destructive',
toneClassName
)}
>
{children}
@@ -89,7 +89,20 @@ export function registerBrowserStateIpcBridge(
if (!sourcePage || getRuntimeEnvironmentIdForWorktree(store, sourcePage.worktreeId)) {
return
}
store.createBrowserTab(sourcePage.worktreeId, url, { title: url })
// Why: the link inherits the opener's cookie jar. Falling back to the default profile would let
// a page in an isolated session hand its links to the default one, silently crossing profiles.
const sourceTab = (store.browserTabsByWorktree[sourcePage.worktreeId] ?? []).find(
(tab) => tab.id === sourcePage.workspaceId
)
store.createBrowserTab(sourcePage.worktreeId, url, {
title: url,
...(sourceTab
? {
sessionProfileId: sourceTab.sessionProfileId,
sessionPartition: sourceTab.sessionPartition
}
: {})
})
})
)
}
@@ -0,0 +1,104 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
const { createBrowserTabMock, storeState } = vi.hoisted(() => ({
createBrowserTabMock: vi.fn(),
storeState: {
value: {} as Record<string, unknown>
}
}))
vi.mock('../../store', () => ({
useAppStore: { getState: () => storeState.value }
}))
vi.mock('@/lib/worktree-runtime-owner', () => ({
getRuntimeEnvironmentIdForWorktree: () => null
}))
vi.mock('@/components/browser-pane/describe-page/live-browser-url-registry', () => ({
rememberLiveBrowserUrl: vi.fn()
}))
vi.mock('./browser-automation-bootstrap-lease', () => ({
acquireBrowserAutomationBootstrapLease: vi.fn()
}))
import { registerBrowserStateIpcBridge } from './browser-state-ipc-bridge'
const noopUnsubscribe = (): void => {}
function captureOpenLinkHandler(): (event: { browserPageId: string; url: string }) => void {
let handler: ((event: { browserPageId: string; url: string }) => void) | null = null
const browserApi = new Proxy(
{
onOpenLinkInOrcaTab: (callback: (event: { browserPageId: string; url: string }) => void) => {
handler = callback
return noopUnsubscribe
}
} as Record<string, unknown>,
{
get: (target, property) =>
property in target ? target[property as string] : () => noopUnsubscribe
}
)
const api = new Proxy({ browser: browserApi } as Record<string, unknown>, {
get: (target, property) =>
property in target
? target[property as string]
: new Proxy({}, { get: () => () => noopUnsubscribe })
})
;(globalThis as { window?: unknown }).window = { api }
registerBrowserStateIpcBridge([], () => false)
if (!handler) {
throw new Error('Expected the bridge to subscribe to browser:open-link-in-orca-tab')
}
return handler
}
describe('link-opened Orca tabs', () => {
beforeEach(() => {
createBrowserTabMock.mockReset()
})
it('inherits the opener tab session so an isolated profile cannot leak into the default one', () => {
storeState.value = {
browserPagesByWorkspace: {
'workspace-1': [{ id: 'page-1', workspaceId: 'workspace-1', worktreeId: 'worktree-1' }]
},
browserTabsByWorktree: {
'worktree-1': [
{
id: 'workspace-1',
sessionProfileId: 'profile-client-a',
sessionPartition: 'persist:orca-browser-session-client-a'
}
]
},
createBrowserTab: createBrowserTabMock
}
captureOpenLinkHandler()({ browserPageId: 'page-1', url: 'https://docs.example.com/guide' })
expect(createBrowserTabMock).toHaveBeenCalledWith(
'worktree-1',
'https://docs.example.com/guide',
expect.objectContaining({
sessionProfileId: 'profile-client-a',
sessionPartition: 'persist:orca-browser-session-client-a'
})
)
})
it('leaves the profile unset when the opener tab is gone, so the user default still applies', () => {
storeState.value = {
browserPagesByWorkspace: {
'workspace-1': [{ id: 'page-1', workspaceId: 'missing', worktreeId: 'worktree-1' }]
},
browserTabsByWorktree: {},
createBrowserTab: createBrowserTabMock
}
captureOpenLinkHandler()({ browserPageId: 'page-1', url: 'https://docs.example.com/guide' })
const options = createBrowserTabMock.mock.calls[0][2] as Record<string, unknown>
expect('sessionProfileId' in options).toBe(false)
})
})
@@ -0,0 +1,137 @@
import { existsSync, readFileSync } from 'node:fs'
import { join, resolve, dirname } from 'node:path'
import { describe, expect, it } from 'vitest'
/**
* react-dom reads __REACT_DEVTOOLS_GLOBAL_HOOK__ once, at its own module
* evaluation, and enters the graph through several component modules rather
* than just the entry's import of react-renderer-root. ES evaluation is
* depth-first in source order, so only the FIRST import statement is guaranteed
* to beat it — and only if that module's own graph cannot reach react-dom
* first, which is why the shim has no imports at all.
*
* Nothing else fails when either property stops holding: the diagnostic simply
* records nothing. These tests are the failure.
*/
const RENDERER_ROOT = resolve(__dirname, '..')
const SHIM_MODULE = 'react-devtools-commit-hook-shim'
const SHIM_IMPORT = `import './lib/${SHIM_MODULE}'`
const OBSERVER_IMPORT = "import './lib/react-commit-cascade-observer'"
/** Entries whose crash reports reach the breadcrumb pipe. */
const INSTRUMENTED_ENTRIES = ['main.tsx', 'popout.tsx']
/**
* The web preload stubs crashReports.recordBreadcrumb to a no-op
* (src/renderer/src/web/preload-api/web-diagnostics-api.ts), so instrumenting
* the web entry would cost commits and record nothing.
*/
const UNINSTRUMENTED_ENTRIES = [join('web', 'main.tsx')]
function firstImportStatement(source: string): string | undefined {
return source
.split('\n')
.map((line) => line.trim())
.find((line) => line.startsWith('import '))
}
function importedSpecifiers(source: string): string[] {
return Array.from(source.matchAll(/(?:^|\n)\s*(?:import|export)[^\n]*?from\s*'([^']+)'/g))
.map((match) => match[1] as string)
.concat(
Array.from(source.matchAll(/(?:^|\n)\s*import\s*'([^']+)'/g)).map(
(match) => match[1] as string
)
)
}
/** Mirrors the alias in config/vitest.config.ts and electron.vite.config.ts. */
function resolveLocalModule(specifier: string, fromFile: string): string | undefined {
const base = specifier.startsWith('@/')
? join(RENDERER_ROOT, specifier.slice(2))
: specifier.startsWith('.')
? join(dirname(fromFile), specifier)
: undefined
if (base === undefined) {
return undefined
}
// Only TS: a .css or asset import cannot pull react-dom into the graph.
return [`${base}.ts`, `${base}.tsx`, join(base, 'index.ts')].find((candidate) =>
existsSync(candidate)
)
}
/** Bare specifiers stay as-is; local ones are followed so a hop cannot hide react-dom. */
function transitiveSpecifiers(entryFile: string): Set<string> {
const seen = new Set<string>()
const specifiers = new Set<string>()
const queue = [entryFile]
while (queue.length > 0) {
const file = queue.pop() as string
if (seen.has(file)) {
continue
}
seen.add(file)
for (const specifier of importedSpecifiers(readFileSync(file, 'utf8'))) {
specifiers.add(specifier)
const local = resolveLocalModule(specifier, file)
if (local) {
queue.push(local)
}
}
}
return specifiers
}
describe('react commit cascade shim install order', () => {
for (const entry of INSTRUMENTED_ENTRIES) {
it(`keeps the import-free shim as the first import in ${entry}`, () => {
const source = readFileSync(join(RENDERER_ROOT, entry), 'utf8')
expect(source).toContain(SHIM_IMPORT)
expect(firstImportStatement(source)).toBe(SHIM_IMPORT)
})
// Why separate: the observer wraps a property react-dom re-reads per commit,
// so it only has to be in the graph, not first in it.
it(`still installs the observer in ${entry}`, () => {
expect(readFileSync(join(RENDERER_ROOT, entry), 'utf8')).toContain(OBSERVER_IMPORT)
})
}
for (const entry of UNINSTRUMENTED_ENTRIES) {
it(`leaves ${entry} uninstrumented, where breadcrumbs are a no-op`, () => {
const source = readFileSync(join(RENDERER_ROOT, entry), 'utf8')
expect(source).not.toContain('react-commit-cascade-observer')
})
}
// Why this is the real ratchet: one import here that transitively reaches
// react-dom would let react-dom evaluate first and read no hook, killing the
// diagnostic in production with every other test still green.
it('keeps the shim free of imports, react-dom above all', () => {
const shimFile = join(RENDERER_ROOT, 'lib', `${SHIM_MODULE}.ts`)
const graph = transitiveSpecifiers(shimFile)
expect(Array.from(graph).filter((specifier) => specifier.includes('react-dom'))).toEqual([])
expect(Array.from(graph)).toEqual([])
// Why a source scan too: the specifier regex only sees an import whose `from`
// shares a line with the keyword, and a multi-line import is the common form
// here — so the graph check alone would miss the one edit that matters.
// Why `from` must be followed by a quote: bare `\bfrom\b` also matched the
// word in a comment, and a ratchet that fails on prose gets deleted.
// Why the `import(` arm: a top-level `await import(...)` makes this module
// async, so react-dom evaluates before the hook is installed.
expect(readFileSync(shimFile, 'utf8')).not.toMatch(
/^\s*(?:import|export)\b[\s\S]*?\bfrom\s*(?:\/\*[\s\S]*?\*\/\s*)*['"]|^\s*import\s*['"]|\bimport\s*\(/m
)
})
it('follows local hops when checking a graph for react-dom', () => {
const observerFile = join(RENDERER_ROOT, 'lib', 'react-commit-cascade-observer.ts')
// The observer's own graph is allowed to be wide; this only proves the walker
// above sees past the first hop, so an empty shim graph means something.
expect(transitiveSpecifiers(observerFile).size).toBeGreaterThan(3)
})
})
@@ -0,0 +1,169 @@
/** @vitest-environment happy-dom */
// Why first: react-dom below reads __REACT_DEVTOOLS_GLOBAL_HOOK__ at module
// evaluation. The shim has no imports of its own, so unlike the observer it
// cannot drag the real breadcrumb recorder in ahead of the vi.mock below.
import './react-devtools-commit-hook-shim'
import { act, useLayoutEffect } from 'react'
import { createRoot, type Root } from 'react-dom/client'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { create } from 'zustand'
import { withReactCommitCascadeWriteProbe } from '../store/react-commit-cascade-write-probe'
import {
REACT_CASCADING_LANES,
REACT_COMMIT_CASCADE_BREADCRUMB,
REACT_COMMIT_CASCADE_NOTICE_LIMIT,
resetReactCommitCascadeTelemetryForTests
} from './react-commit-cascade-telemetry'
import {
installReactCommitCascadeObserver,
resetReactCommitCascadeObserverForTests
} from './react-commit-cascade-observer'
import type { ReactDevtoolsCommitHook } from './react-devtools-commit-hook-shim'
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
const recordBreadcrumb = vi.fn()
vi.mock('@/lib/crash-breadcrumb-recorder', () => ({
recordRendererCrashBreadcrumb: (name: string, data?: unknown) => recordBreadcrumb(name, data)
}))
/**
* A single-driver cascade measures 438 bytes here, where every frame carries this
* file's long basename; a production bundle name is shorter. Wide enough to sit
* still, narrow enough that a raw stack or an uncapped key list breaks it.
*/
const MAX_CASCADE_CRUMB_BYTES = 768
/** Enough to pass the notice limit while staying under React's own 50-commit bail. */
const CASCADE_TICKS = REACT_COMMIT_CASCADE_NOTICE_LIMIT + 5
type CascadeState = { ticks: number; bump: () => void }
const useCascadeStore = create<CascadeState>()(
withReactCommitCascadeWriteProbe((set) => ({
ticks: 0,
bump: () => {
set({ ticks: useCascadeStore.getState().ticks + 1 })
}
}))
)
/**
* Layout effect, not passive: only the synchronous counter throws #185. A
* useEffect loop leaves `pendingLanes: 0` at commit time (measured) because
* passive effects flush after the callback, and React only console.errors it.
*/
function RunawayLayoutEffectPane(): React.JSX.Element {
const ticks = useCascadeStore((state) => state.ticks)
useLayoutEffect(() => {
if (ticks < CASCADE_TICKS) {
useCascadeStore.getState().bump()
}
})
return <div>{ticks}</div>
}
let host: HTMLDivElement
let root: Root
const commitHook = (globalThis as { __REACT_DEVTOOLS_GLOBAL_HOOK__?: ReactDevtoolsCommitHook })
.__REACT_DEVTOOLS_GLOBAL_HOOK__
beforeEach(() => {
recordBreadcrumb.mockReset()
resetReactCommitCascadeTelemetryForTests()
resetReactCommitCascadeObserverForTests()
useCascadeStore.setState({ ticks: 0 })
// Why cleared rather than deleting the global: react-dom captured this hook
// OBJECT at its own module evaluation, so a replacement object would never be
// called — and reinstalling over the last test's wrapper double-counts commits.
if (commitHook) {
commitHook.onCommitFiberRoot = undefined
}
installReactCommitCascadeObserver()
host = document.createElement('div')
document.body.appendChild(host)
root = createRoot(host)
})
afterEach(() => {
act(() => {
root.unmount()
})
host.remove()
})
describe('react commit cascade observer', () => {
it('counts a genuine synchronous cascade and names the store write driving it', () => {
act(() => {
root.render(<RunawayLayoutEffectPane />)
})
const cascadeCalls = recordBreadcrumb.mock.calls.filter(
([name]) => name === REACT_COMMIT_CASCADE_BREADCRUMB
)
expect(cascadeCalls).toHaveLength(1)
const payload = (cascadeCalls[0]?.[1] ?? {}) as Record<string, unknown>
expect(payload.commits).toBe(REACT_COMMIT_CASCADE_NOTICE_LIMIT)
expect(payload.pendingLanes).toBeGreaterThan(0)
expect(payload.storeWrites).toBeGreaterThan(0)
// The middleware boundary is elided, so this is the code that called `set`.
expect(String(payload.driverFrame)).toContain('bump')
expect(payload.changedKeys).toBe('ticks')
expect(payload.rendererSurface).toBe('main')
})
// Why counted from outside our own wrapper: a bundler change that installs the
// observer twice counts every commit twice and fires the crumb at half the real
// depth, which no other assertion here can tell from a correct run.
it('counts exactly one commit per hook invocation', () => {
let cascadingInvocations = 0
const observed = commitHook?.onCommitFiberRoot
if (commitHook) {
// The mount commit holds no cascading lanes, so only these are countable.
commitHook.onCommitFiberRoot = (rendererId, fiberRoot, priorityLevel, didError) => {
const lanes = (fiberRoot as { pendingLanes?: number } | null)?.pendingLanes ?? 0
if ((lanes & REACT_CASCADING_LANES) !== 0) {
cascadingInvocations += 1
}
observed?.call(commitHook, rendererId, fiberRoot, priorityLevel, didError)
}
}
let invocationsAtReport: number | undefined
recordBreadcrumb.mockImplementation((name: string) => {
if (name === REACT_COMMIT_CASCADE_BREADCRUMB && invocationsAtReport === undefined) {
invocationsAtReport = cascadingInvocations
}
})
act(() => {
root.render(<RunawayLayoutEffectPane />)
})
expect(invocationsAtReport).toBe(REACT_COMMIT_CASCADE_NOTICE_LIMIT)
})
// Why a byte budget: driverStack's key ends in `stack`, which buys 4000 chars,
// so a raw stack or an uncapped key list could be added here unnoticed.
it('keeps the crumb inside a flat byte budget', () => {
act(() => {
root.render(<RunawayLayoutEffectPane />)
})
const payload = recordBreadcrumb.mock.calls.find(
([name]) => name === REACT_COMMIT_CASCADE_BREADCRUMB
)?.[1]
expect(JSON.stringify(payload).length).toBeLessThan(MAX_CASCADE_CRUMB_BYTES)
})
it('stays silent for a render that settles well below the notice limit', () => {
useCascadeStore.setState({ ticks: CASCADE_TICKS })
act(() => {
root.render(<RunawayLayoutEffectPane />)
})
expect(recordBreadcrumb).not.toHaveBeenCalled()
})
})
@@ -0,0 +1,307 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import {
REACT_COMMIT_CASCADE_BREADCRUMB,
REACT_COMMIT_CASCADE_NOTICE_LIMIT,
resetReactCommitCascadeTelemetryForTests
} from './react-commit-cascade-telemetry'
import {
REACT_COMMIT_CASCADE_INSTALL_CHECK_MS,
REACT_COMMIT_CASCADE_UNINSTALLED_BREADCRUMB,
installReactCommitCascadeObserver,
resetReactCommitCascadeObserverForTests
} from './react-commit-cascade-observer'
import {
ensureReactDevtoolsCommitHook,
type ReactDevtoolsCommitHook
} from './react-devtools-commit-hook-shim'
const recordBreadcrumb = vi.fn()
vi.mock('@/lib/crash-breadcrumb-recorder', () => ({
recordRendererCrashBreadcrumb: (name: string, data?: unknown) => recordBreadcrumb(name, data)
}))
const CASCADING_ROOT = { pendingLanes: 2 }
function readHook(): ReactDevtoolsCommitHook {
return (globalThis as { __REACT_DEVTOOLS_GLOBAL_HOOK__?: ReactDevtoolsCommitHook })
.__REACT_DEVTOOLS_GLOBAL_HOOK__ as ReactDevtoolsCommitHook
}
beforeEach(() => {
recordBreadcrumb.mockClear()
resetReactCommitCascadeTelemetryForTests()
resetReactCommitCascadeObserverForTests()
// Why the delete: install wraps whatever is there, so a hook left over from
// the previous test would stack a second counter onto the same callback.
delete (globalThis as { __REACT_DEVTOOLS_GLOBAL_HOOK__?: unknown }).__REACT_DEVTOOLS_GLOBAL_HOOK__
installReactCommitCascadeObserver()
})
afterEach(() => {
vi.useRealTimers()
})
describe('react devtools commit hook shim', () => {
// Why asserted: without a pre-existing hook react-dom injects nothing, so a
// packaged build only gets commit callbacks if the shim installs its own.
it('exposes what react-dom requires to inject', () => {
const hook = readHook()
expect(hook.supportsFiber).toBe(true)
expect(hook.isDisabled).toBe(false)
expect(hook.inject?.({})).toBeGreaterThan(0)
expect(typeof hook.onCommitFiberRoot).toBe('function')
})
// Why asserted: react-dom calls this once per DELETED FIBER, guarded only by
// a typeof check. Defining it would put our shim on every unmount path.
it('leaves the per-deleted-fiber callbacks undefined', () => {
const hook = readHook() as Record<string, unknown>
expect(hook.onCommitFiberUnmount).toBeUndefined()
expect(hook.onPostCommitFiberRoot).toBeUndefined()
expect(hook.setStrictMode).toBeUndefined()
})
// Why asserted: react-refresh captures this property and calls
// oldOnCommitFiberRoot.apply(this, arguments) with no typeof guard, unlike the
// fallback it gives onScheduleFiberRoot.
it('ships a callable onCommitFiberRoot before anything wraps it', () => {
delete (globalThis as { __REACT_DEVTOOLS_GLOBAL_HOOK__?: unknown })
.__REACT_DEVTOOLS_GLOBAL_HOOK__
const hook = ensureReactDevtoolsCommitHook()
expect(typeof hook?.onCommitFiberRoot).toBe('function')
expect(() => hook?.onCommitFiberRoot?.(1, null, undefined, false)).not.toThrow()
})
})
describe('installReactCommitCascadeObserver', () => {
it('breadcrumbs once the commit callback reaches the notice limit', () => {
const hook = readHook()
for (let commit = 0; commit < REACT_COMMIT_CASCADE_NOTICE_LIMIT; commit += 1) {
hook.onCommitFiberRoot?.(1, CASCADING_ROOT, undefined, false)
}
expect(recordBreadcrumb).toHaveBeenCalledTimes(1)
expect(recordBreadcrumb.mock.calls[0]?.[0]).toBe(REACT_COMMIT_CASCADE_BREADCRUMB)
})
it('ignores a commit whose root reports no cascading lanes', () => {
const hook = readHook()
for (let commit = 0; commit < REACT_COMMIT_CASCADE_NOTICE_LIMIT * 2; commit += 1) {
hook.onCommitFiberRoot?.(1, { pendingLanes: 0 }, undefined, false)
}
expect(recordBreadcrumb).not.toHaveBeenCalled()
})
// Why asserted: a future React that stops exposing pendingLanes must make the
// diagnostic go quiet, not fire on a depth it can no longer evaluate.
it('ends the cascade when pendingLanes is not readable', () => {
const hook = readHook()
for (let commit = 0; commit < REACT_COMMIT_CASCADE_NOTICE_LIMIT - 1; commit += 1) {
hook.onCommitFiberRoot?.(1, CASCADING_ROOT, undefined, false)
}
hook.onCommitFiberRoot?.(1, { renamedLanes: 2 }, undefined, false)
for (let commit = 0; commit < REACT_COMMIT_CASCADE_NOTICE_LIMIT - 1; commit += 1) {
hook.onCommitFiberRoot?.(1, CASCADING_ROOT, undefined, false)
}
expect(recordBreadcrumb).not.toHaveBeenCalled()
})
// Why a string and not undefined: '2' & 42 is 2, so a bitmask-only guard would
// keep the cascade alive on a value React never meant as a lane.
it('ends the cascade when pendingLanes is not a number', () => {
const hook = readHook()
for (let commit = 0; commit < REACT_COMMIT_CASCADE_NOTICE_LIMIT - 1; commit += 1) {
hook.onCommitFiberRoot?.(1, CASCADING_ROOT, undefined, false)
}
hook.onCommitFiberRoot?.(1, { pendingLanes: '2' }, undefined, false)
for (let commit = 0; commit < REACT_COMMIT_CASCADE_NOTICE_LIMIT - 1; commit += 1) {
hook.onCommitFiberRoot?.(1, CASCADING_ROOT, undefined, false)
}
expect(recordBreadcrumb).not.toHaveBeenCalled()
})
it('chains onto an existing callback instead of replacing it', () => {
const previous = vi.fn()
const hook = readHook()
hook.onCommitFiberRoot = previous
resetReactCommitCascadeObserverForTests()
installReactCommitCascadeObserver()
hook.onCommitFiberRoot?.(7, CASCADING_ROOT, undefined, true)
expect(previous).toHaveBeenCalledWith(7, CASCADING_ROOT, undefined, true)
})
// Why asserted: our throw would otherwise land in react-dom's own catch and
// silently unhook DevTools and Fast Refresh from the rest of the chain.
it('still calls the chained hook when our own work throws', () => {
const previous = vi.fn()
const hook = readHook()
hook.onCommitFiberRoot = previous
resetReactCommitCascadeObserverForTests()
installReactCommitCascadeObserver()
const hostile = {
get pendingLanes(): number {
throw new Error('hostile root')
}
}
expect(() => hook.onCommitFiberRoot?.(1, hostile, undefined, false)).not.toThrow()
expect(previous).toHaveBeenCalledTimes(1)
})
it('is idempotent, so a second install cannot double-count commits', () => {
installReactCommitCascadeObserver()
const hook = readHook()
for (let commit = 0; commit < REACT_COMMIT_CASCADE_NOTICE_LIMIT - 1; commit += 1) {
hook.onCommitFiberRoot?.(1, CASCADING_ROOT, undefined, false)
}
expect(recordBreadcrumb).not.toHaveBeenCalled()
})
})
// Why: a reshuffled import or a bundler hoist disables the diagnostic with no
// test failure. This is what makes that visible in the field instead.
describe('install self-check', () => {
it('breadcrumbs when no commit ever reached the hook', () => {
vi.useFakeTimers()
resetReactCommitCascadeObserverForTests()
installReactCommitCascadeObserver()
vi.advanceTimersByTime(REACT_COMMIT_CASCADE_INSTALL_CHECK_MS)
expect(recordBreadcrumb).toHaveBeenCalledWith(
REACT_COMMIT_CASCADE_UNINSTALLED_BREADCRUMB,
undefined
)
})
it('stays silent once any commit has been seen', () => {
vi.useFakeTimers()
resetReactCommitCascadeObserverForTests()
installReactCommitCascadeObserver()
readHook().onCommitFiberRoot?.(1, { pendingLanes: 0 }, undefined, false)
vi.advanceTimersByTime(REACT_COMMIT_CASCADE_INSTALL_CHECK_MS)
expect(recordBreadcrumb).not.toHaveBeenCalled()
})
// Why asserted: a hook that refuses the assignment is exactly the case the
// self-check exists for, so it must arm on the failure path too.
it('breadcrumbs when the hook refuses the callback assignment', () => {
vi.useFakeTimers()
resetReactCommitCascadeObserverForTests()
const frozenHook = Object.freeze({ isDisabled: false, supportsFiber: true })
;(globalThis as { __REACT_DEVTOOLS_GLOBAL_HOOK__?: unknown }).__REACT_DEVTOOLS_GLOBAL_HOOK__ =
frozenHook
expect(() => installReactCommitCascadeObserver()).not.toThrow()
vi.advanceTimersByTime(REACT_COMMIT_CASCADE_INSTALL_CHECK_MS)
expect((frozenHook as { onCommitFiberRoot?: unknown }).onCommitFiberRoot).toBeUndefined()
expect(recordBreadcrumb).toHaveBeenCalledWith(
REACT_COMMIT_CASCADE_UNINSTALLED_BREADCRUMB,
undefined
)
})
it('breadcrumbs when no hook can be reached at all', () => {
vi.useFakeTimers()
resetReactCommitCascadeObserverForTests()
Object.defineProperty(globalThis, '__REACT_DEVTOOLS_GLOBAL_HOOK__', {
configurable: true,
get() {
throw new Error('no devtools global')
}
})
expect(() => installReactCommitCascadeObserver()).not.toThrow()
vi.advanceTimersByTime(REACT_COMMIT_CASCADE_INSTALL_CHECK_MS)
expect(recordBreadcrumb).toHaveBeenCalledWith(
REACT_COMMIT_CASCADE_UNINSTALLED_BREADCRUMB,
undefined
)
})
})
/**
* Why an allocation assertion and not a timing one: this callback runs on every
* commit the app makes, so the cost that matters is garbage. Measured on this
* loop, an escaping object literal, a template string and a per-call closure
* each cost more than 3.8 MB where the current code costs tens of KB.
*/
describe('commit hook cost', () => {
const HOT_COMMITS = 1_000_000
/** ~30x the measured zero-allocation baseline, ~4x under the cheapest regression. */
const MAX_HEAP_GROWTH_BYTES = 1_000_000
function measureHeapGrowth(commit: () => void): number {
const collectGarbage = (globalThis as { gc?: () => void }).gc
// --expose-gc is pinned in config/vitest.config.ts execArgv.
expect(typeof collectGarbage).toBe('function')
for (let warmup = 0; warmup < 50_000; warmup += 1) {
commit()
}
collectGarbage?.()
const before = process.memoryUsage().heapUsed
for (let iteration = 0; iteration < HOT_COMMITS; iteration += 1) {
commit()
}
return process.memoryUsage().heapUsed - before
}
it('allocates nothing on an ordinary commit', () => {
const hook = readHook()
const quietRoot = { pendingLanes: 0 }
const growth = measureHeapGrowth(() => hook.onCommitFiberRoot?.(1, quietRoot, undefined, false))
expect(growth).toBeLessThan(MAX_HEAP_GROWTH_BYTES)
})
// Why separately: the cascading branch runs the counter, the arm check and the
// report guard, none of which may start allocating once a cascade is deep.
it('allocates nothing while a cascade is being counted', () => {
const hook = readHook()
const growth = measureHeapGrowth(() =>
hook.onCommitFiberRoot?.(1, CASCADING_ROOT, undefined, false)
)
expect(growth).toBeLessThan(MAX_HEAP_GROWTH_BYTES)
})
})
// Why asserted: main.tsx imports this module for its side effect alone, so
// deleting the bare install call would disable the diagnostic in production.
describe('module self-install', () => {
it('counts commits after nothing but an import', async () => {
vi.useFakeTimers()
delete (globalThis as { __REACT_DEVTOOLS_GLOBAL_HOOK__?: unknown })
.__REACT_DEVTOOLS_GLOBAL_HOOK__
vi.resetModules()
await import('./react-commit-cascade-observer')
const hook = readHook()
for (let commit = 0; commit < REACT_COMMIT_CASCADE_NOTICE_LIMIT; commit += 1) {
hook.onCommitFiberRoot?.(1, CASCADING_ROOT, undefined, false)
}
expect(recordBreadcrumb).toHaveBeenCalledWith(
REACT_COMMIT_CASCADE_BREADCRUMB,
expect.objectContaining({ commits: REACT_COMMIT_CASCADE_NOTICE_LIMIT })
)
})
})
@@ -0,0 +1,101 @@
/**
* Installs the per-commit hook that feeds the cascade telemetry.
*
* Why the devtools callback and not <Profiler> or a root effect: Profiler is
* stripped from production react-dom, and a dependency-less useLayoutEffect
* fires per render of its OWN component, not per commit — measured, a root
* effect saw 1 of 11 commits a leaf drove. react-dom calls onCommitFiberRoot
* once per commit per root in both bundles, inside its own try/catch, and hands
* us the FiberRoot whose `pendingLanes` drive the reset rule.
*
* Ordering: this module only WRAPS `onCommitFiberRoot`, which react-dom re-reads
* at every commit, so it may be imported anywhere. The deadline belongs to
* react-devtools-commit-hook-shim, which must merely exist before react-dom's
* module evaluation and is therefore the entries' first import.
*/
import { observeReactCommit } from '@/lib/react-commit-cascade-telemetry'
import { recordRendererCrashBreadcrumb } from '@/lib/crash-breadcrumb-recorder'
import {
ensureReactDevtoolsCommitHook,
type ReactDevtoolsCommitHook
} from '@/lib/react-devtools-commit-hook-shim'
export const REACT_COMMIT_CASCADE_UNINSTALLED_BREADCRUMB = 'react_commit_cascade_uninstalled'
/** Long enough that any live renderer has committed; short enough to reach a crash report. */
export const REACT_COMMIT_CASCADE_INSTALL_CHECK_MS = 10_000
type FiberRootLike = { pendingLanes?: unknown }
let installed = false
let commitsSeen = 0
let installCheckTimer: ReturnType<typeof setTimeout> | undefined
/**
* A reshuffled import or a bundler hoist would disable the diagnostic with no
* test failure. Make that visible in the field instead of silent.
*/
function scheduleInstallSelfCheck(): void {
if (installCheckTimer !== undefined || typeof setTimeout !== 'function') {
return
}
installCheckTimer = setTimeout(() => {
if (commitsSeen === 0) {
recordRendererCrashBreadcrumb(REACT_COMMIT_CASCADE_UNINSTALLED_BREADCRUMB)
}
}, REACT_COMMIT_CASCADE_INSTALL_CHECK_MS)
// Why unref where available: the check must never hold a renderer teardown open.
;(installCheckTimer as { unref?: () => void })?.unref?.()
}
export function installReactCommitCascadeObserver(): void {
if (installed) {
return
}
try {
const hook = ensureReactDevtoolsCommitHook()
if (hook) {
installObserverOnHook(hook)
// Why only after the assignment: a hook that refuses it stays uninstalled,
// so a later call can retry rather than being gated out.
installed = true
}
} catch {
// A renderer without a patchable global still has to boot.
}
// Why outside the try: a failed install is exactly when the crumb matters.
scheduleInstallSelfCheck()
}
function installObserverOnHook(hook: ReactDevtoolsCommitHook): void {
const previous = hook.onCommitFiberRoot
// Fixed arity, not rest args: this runs on every commit and must not allocate.
hook.onCommitFiberRoot = (rendererId, root, priorityLevel, didError) => {
// Why our own try/catch when react-dom already has one: theirs would
// swallow the rest of this callback, silently unhooking DevTools and
// Fast Refresh from the chain below.
try {
commitsSeen += 1
const pendingLanes = (root as FiberRootLike | null)?.pendingLanes
// Why 0 and not a skip: a React shape change that hides pendingLanes must
// end the cascade, not freeze a stale depth that later commits push over
// the limit. 0 is the lane value that means "nothing pending", so it
// takes the same reset path and this fails safe to no crumb.
observeReactCommit(root, typeof pendingLanes === 'number' ? pendingLanes : 0)
} catch {
// Best-effort crash evidence only.
}
previous?.call(hook, rendererId, root, priorityLevel, didError)
}
}
export function resetReactCommitCascadeObserverForTests(): void {
installed = false
commitsSeen = 0
if (installCheckTimer !== undefined) {
clearTimeout(installCheckTimer)
installCheckTimer = undefined
}
}
installReactCommitCascadeObserver()
@@ -0,0 +1,144 @@
import { afterEach, beforeEach, describe, expect, it } from 'vitest'
import { create } from 'zustand'
import { withReactCommitCascadeWriteProbe } from '../store/react-commit-cascade-write-probe'
import {
MAX_REPORTED_CHANGED_KEYS,
MAX_SAMPLED_WRITES,
armReactCommitCascadeWriteSampling,
noteReactCommitCascadeStoreWrite,
readReactCommitCascadeWriteSummary,
resetReactCommitCascadeWriteSamples
} from './react-commit-cascade-store-write-samples'
// Not an intersection with ErrorConstructor: both fields have to stay optional
// so the absent-captureStackTrace platform can be simulated.
type ErrorWithCapture = {
captureStackTrace?: (target: object, constructorOpt?: unknown) => void
stackTraceLimit?: number
}
const errorWithCapture = Error as unknown as ErrorWithCapture
const originalCapture = errorWithCapture.captureStackTrace
const originalLimit = errorWithCapture.stackTraceLimit
/** Stands in for the zustand slice action that calls `set`. */
function storeAction(): void {
noteReactCommitCascadeStoreWrite(storeAction, { ticks: 1 })
}
function runawayEffect(): void {
storeAction()
}
function paneRender(): void {
runawayEffect()
}
beforeEach(() => {
resetReactCommitCascadeWriteSamples()
})
afterEach(() => {
resetReactCommitCascadeWriteSamples()
errorWithCapture.captureStackTrace = originalCapture
errorWithCapture.stackTraceLimit = originalLimit
})
describe('driver stack frames', () => {
// Why more than one frame: frame 0 is the slice action every cascade shares.
// The caller above it is the loop we are actually hunting.
it('names callers above the frame the write came from', () => {
armReactCommitCascadeWriteSampling()
paneRender()
const summary = readReactCommitCascadeWriteSummary()
const frames = summary.driverStack?.split('\n') ?? []
expect(summary.driverFrame).toContain('runawayEffect')
expect(frames[0]).toBe(summary.driverFrame)
expect(frames.length).toBeGreaterThan(1)
expect(summary.driverStack).toContain('paneRender')
expect(summary.driverFrame).not.toContain('paneRender')
})
// Why: the redaction that strips paths is keyed on the detail NAME, so a path
// inside this value would ship a developer's home directory.
it('keeps every reported frame path-free', () => {
armReactCommitCascadeWriteSampling()
paneRender()
expect(readReactCommitCascadeWriteSummary().driverStack).not.toContain('/')
})
})
describe('capture failures', () => {
// Why asserted: the limit is process-global, so leaking a raised value would
// make every unrelated throw in the app capture eight frames forever.
it('restores Error.stackTraceLimit when the capture throws', () => {
errorWithCapture.stackTraceLimit = 3
errorWithCapture.captureStackTrace = () => {
throw new Error('capture refused')
}
armReactCommitCascadeWriteSampling()
expect(() => storeAction()).not.toThrow()
expect(errorWithCapture.stackTraceLimit).toBe(3)
expect(readReactCommitCascadeWriteSummary().storeWrites).toBe(1)
})
// Why asserted: no sample is ever pushed without captureStackTrace, so a cap
// bound to samples.length would leave key collection running for the whole
// cascade on any engine that lacks the API.
it('stops collecting after the sample cap when captureStackTrace is unavailable', () => {
delete errorWithCapture.captureStackTrace
armReactCommitCascadeWriteSampling()
for (let write = 0; write < 100; write += 1) {
noteReactCommitCascadeStoreWrite(storeAction, { [`key${write}`]: write })
}
// Restored before asserting: vitest builds its own failure stacks with it.
errorWithCapture.captureStackTrace = originalCapture
const summary = readReactCommitCascadeWriteSummary()
expect(summary.storeWrites).toBe(100)
expect(summary.driverFrame).toBeUndefined()
expect(summary.storeWriteSites).toBe(0)
expect(summary.changedKeys?.split(',')).toHaveLength(MAX_SAMPLED_WRITES)
expect(summary.changedKeys).not.toContain('key99')
expect(summary.changedKeys?.length).toBeLessThan(240)
})
// Why still asserted: one write can change many keys, so the report cap is
// what keeps the detail inside its 240-char budget.
it('caps reported changed keys from a single wide write', () => {
const wide = Object.fromEntries(
Array.from({ length: 40 }, (_, index) => [`key${index}`, index])
)
armReactCommitCascadeWriteSampling()
noteReactCommitCascadeStoreWrite(storeAction, wide)
const summary = readReactCommitCascadeWriteSummary()
expect(summary.changedKeys?.split(',')).toHaveLength(MAX_REPORTED_CHANGED_KEYS)
expect(summary.changedKeys?.length).toBeLessThan(240)
})
})
// Why measured through the real middleware: the arming condition is what keeps
// an ordinary write down to one boolean read, and nothing else asserts it end to end.
describe('disarmed cost', () => {
const WRITE_FLOOD = 10_000
it('captures no stack across a write flood with no cascade', () => {
const useFloodStore = create<{ ticks: number }>()(
withReactCommitCascadeWriteProbe(() => ({ ticks: 0 }))
)
let captures = 0
errorWithCapture.captureStackTrace = (target, boundary) => {
captures += 1
originalCapture?.(target, boundary)
}
for (let write = 0; write < WRITE_FLOOD; write += 1) {
useFloodStore.setState({ ticks: write })
}
expect(captures).toBe(0)
expect(useFloodStore.getState().ticks).toBe(WRITE_FLOOD - 1)
expect(readReactCommitCascadeWriteSummary().storeWrites).toBe(0)
})
})
@@ -0,0 +1,157 @@
/**
* Samples the store writes made during a suspected React commit cascade.
*
* Why sampling writes and not the throw site: React #185 lands on whichever
* fiber dispatched after a root-global counter tripped, so the reported
* component is a bystander (see shared/react-update-depth-attribution.ts). The
* writes that kept scheduling are the only frames that identify the loop.
*
* Cost discipline: the armed flag is the only thing the store-write path reads
* when no cascade is suspected, capture is capped per cascade, and frames are
* formatted at report time so a sampled write pays for nothing it may not use.
*/
/**
* Read on every store write, so it is a plain field load on a shared object
* rather than a cross-module call.
*/
export const reactCommitCascadeWriteProbe = { armed: false }
/** Frame 0 is the store action; the rest reach up into whatever loops over it. */
const CAPTURE_STACK_FRAME_LIMIT = 8
/**
* Frame 0 alone names a slice action, which every cascade shares. Two frames of
* caller is what distinguishes the effect driving the loop from its neighbours.
*/
const REPORTED_FRAMES_PER_SITE = 3
/** Whole frames only, inside the 4000-char budget a `stack`-suffixed detail gets. */
const MAX_DRIVER_STACK_CHARS = 3_800
/** A loop repeats; six samples name every distinct participant worth naming. */
export const MAX_SAMPLED_WRITES = 6
/** Keeps `changedKeys` inside the 240-char detail cap without truncating mid-key. */
export const MAX_REPORTED_CHANGED_KEYS = 12
type SampledWrite = { stack?: string }
let storeWrites = 0
let samples: SampledWrite[] = []
let changedKeys: Set<string> | null = null
export type ReactCommitCascadeWriteSummary = {
storeWrites: number
storeWriteSites: number
driverFrame: string | undefined
driverStack: string | undefined
changedKeys: string | undefined
}
export function armReactCommitCascadeWriteSampling(): void {
reactCommitCascadeWriteProbe.armed = true
}
export function resetReactCommitCascadeWriteSamples(): void {
reactCommitCascadeWriteProbe.armed = false
storeWrites = 0
samples = []
changedKeys = null
}
/**
* Call only while armed. `boundary` is the wrapping `set` function, so V8 elides
* our own frames and the first captured frame is the real caller. Typed as
* `object` because zustand's `set` is an overload set, not a plain signature.
*/
export function noteReactCommitCascadeStoreWrite(boundary: object, partial: unknown): void {
storeWrites += 1
// Why the write count and not samples.length: samples only grows where
// Error.captureStackTrace exists, so that cap would never engage without it and
// key collection would run for the whole cascade.
if (storeWrites > MAX_SAMPLED_WRITES) {
return
}
// Why object-only: a functional updater's keys are unknowable without running it.
if (partial && typeof partial === 'object') {
changedKeys ??= new Set<string>()
for (const key of Object.keys(partial)) {
changedKeys.add(key)
}
}
const capture = Error as ErrorConstructor & {
captureStackTrace?: (target: object, constructorOpt?: unknown) => void
stackTraceLimit?: number
}
if (typeof capture.captureStackTrace !== 'function') {
return
}
const previousLimit = capture.stackTraceLimit
const sample: SampledWrite = {}
try {
capture.stackTraceLimit = CAPTURE_STACK_FRAME_LIMIT
capture.captureStackTrace(sample, boundary)
samples.push(sample)
} catch {
// Best-effort crash evidence only.
} finally {
capture.stackTraceLimit = previousLimit
}
}
/** `at fn (/Users/me/app/src/x.ts:1:2)` becomes `x.ts:1:2 fn`. */
function reduceFrame(frame: string): string | undefined {
const match = /^\s*at\s+(?:(.+?)\s+\()?(.+?):(\d+):(\d+)\)?\s*$/.exec(frame)
if (!match) {
return undefined
}
const [, fn, location, line, column] = match
// Why basename only: the redaction that strips paths is keyed on the detail
// NAME, so a full path inside this value would ship a developer's home dir.
const basename = location?.split(/[/\\]/).pop() ?? ''
return fn ? `${basename}:${line}:${column} ${fn}` : `${basename}:${line}:${column}`
}
/** One reduced frame group per distinct call site, deepest frame first. */
function sampledSiteFrames(): string[][] {
const sites: string[][] = []
const seen = new Set<string>()
for (const sample of samples) {
const frames = (sample.stack?.split('\n') ?? [])
.map(reduceFrame)
.filter((frame): frame is string => frame !== undefined)
.slice(0, REPORTED_FRAMES_PER_SITE)
const site = frames[0]
if (site === undefined || seen.has(site)) {
continue
}
seen.add(site)
sites.push(frames)
}
return sites
}
/** Drops whole frames, so a truncated stack never ends mid-frame. */
function joinWithinBudget(frames: string[]): string | undefined {
const kept: string[] = []
let remaining = MAX_DRIVER_STACK_CHARS
for (const frame of frames) {
if (frame.length + 1 > remaining) {
break
}
kept.push(frame)
remaining -= frame.length + 1
}
return kept.length > 0 ? kept.join('\n') : undefined
}
/** Formats only here, so an unreported cascade pays nothing for its samples. */
export function readReactCommitCascadeWriteSummary(): ReactCommitCascadeWriteSummary {
const sites = sampledSiteFrames()
const keys = changedKeys ? Array.from(changedKeys).slice(0, MAX_REPORTED_CHANGED_KEYS) : []
return {
storeWrites,
storeWriteSites: sites.length,
// Why frame 0 stays alone: the main-side coalesce key clusters on it.
driverFrame: sites[0]?.[0],
driverStack: joinWithinBudget(sites.flat()),
changedKeys: keys.length > 0 ? keys.join(',') : undefined
}
}
@@ -0,0 +1,339 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { sanitizeCrashReportDetails } from '../../../shared/crash-report-redaction'
import { REACT_NESTED_UPDATE_LIMIT } from '../../../shared/react-update-depth-attribution'
import {
REACT_CASCADING_LANES,
setReactCommitCascadeRendererSurface,
REACT_COMMIT_CASCADE_ARM_COMMITS,
REACT_COMMIT_CASCADE_BREADCRUMB,
REACT_COMMIT_CASCADE_MIN_REPORT_INTERVAL_MS,
REACT_COMMIT_CASCADE_NOTICE_LIMIT,
createReactCommitCascadeState,
recordReactCommit,
resetReactCommitCascadeTelemetryForTests,
type ReactCommitCascadeState
} from './react-commit-cascade-telemetry'
import {
noteReactCommitCascadeStoreWrite,
reactCommitCascadeWriteProbe,
readReactCommitCascadeWriteSummary
} from './react-commit-cascade-store-write-samples'
const recordBreadcrumb = vi.fn()
vi.mock('@/lib/crash-breadcrumb-recorder', () => ({
recordRendererCrashBreadcrumb: (name: string, data?: unknown) => recordBreadcrumb(name, data)
}))
const ROOT = { pendingLanes: 2 }
const OTHER_ROOT = { pendingLanes: 2 }
/** SyncLane; what react-dom leaves pending on a cascading commit. */
const SYNC_LANE = 2
/** Any lane outside React's nested-update mask ends the cascade. */
const IDLE_LANE = 1_073_741_824
function cascadePayload(callIndex = 0): Record<string, unknown> {
const call = recordBreadcrumb.mock.calls[callIndex]
return (call?.[1] ?? {}) as Record<string, unknown>
}
function driveCommits(args: {
state: ReactCommitCascadeState
count: number
root?: unknown
lanes?: number
nowMs?: number
onCommit?: () => void
}): void {
const readNowMs = (): number => args.nowMs ?? 1_000
for (let index = 0; index < args.count; index += 1) {
recordReactCommit({
state: args.state,
root: args.root ?? ROOT,
pendingLanes: args.lanes ?? SYNC_LANE,
readNowMs
})
args.onCommit?.()
}
}
beforeEach(() => {
recordBreadcrumb.mockClear()
resetReactCommitCascadeTelemetryForTests()
})
// Why asserted: both thresholds only mean anything relative to React's own bail.
// If the notice limit ever meets it, the crumb stops beating the throw.
describe('cascade thresholds', () => {
it('derives arm and notice limits from React commit budget', () => {
expect(REACT_COMMIT_CASCADE_NOTICE_LIMIT).toBeLessThan(REACT_NESTED_UPDATE_LIMIT)
expect(REACT_COMMIT_CASCADE_ARM_COMMITS).toBeLessThan(REACT_COMMIT_CASCADE_NOTICE_LIMIT)
expect(REACT_CASCADING_LANES).toBe(42)
})
})
describe('recordReactCommit', () => {
it('stays silent below the notice limit', () => {
const state = createReactCommitCascadeState()
driveCommits({ state, count: REACT_COMMIT_CASCADE_NOTICE_LIMIT - 1 })
expect(recordBreadcrumb).not.toHaveBeenCalled()
expect(state.commits).toBe(REACT_COMMIT_CASCADE_NOTICE_LIMIT - 1)
})
it('fires exactly once at the notice limit and not again in the same cascade', () => {
const state = createReactCommitCascadeState()
driveCommits({ state, count: REACT_COMMIT_CASCADE_NOTICE_LIMIT * 3 })
expect(recordBreadcrumb).toHaveBeenCalledTimes(1)
expect(recordBreadcrumb.mock.calls[0]?.[0]).toBe(REACT_COMMIT_CASCADE_BREADCRUMB)
expect(cascadePayload().commits).toBe(REACT_COMMIT_CASCADE_NOTICE_LIMIT)
})
it('emits flat primitives only', () => {
const state = createReactCommitCascadeState()
driveCommits({ state, count: REACT_COMMIT_CASCADE_NOTICE_LIMIT })
const payload = cascadePayload()
expect(
Object.values(payload).every((value) => value === null || typeof value !== 'object')
).toBe(true)
expect(payload).toMatchObject({
commits: REACT_COMMIT_CASCADE_NOTICE_LIMIT,
commitBudget: REACT_NESTED_UPDATE_LIMIT,
pendingLanes: SYNC_LANE,
storeWrites: 0,
storeWriteSites: 0,
rendererSurface: 'main'
})
})
// Why this is the whole reset rule: React zeroes its counter the moment a
// commit leaves no sync lanes pending, so a lull must not accumulate.
it('ends the cascade when the root stops holding cascading lanes', () => {
const state = createReactCommitCascadeState()
driveCommits({ state, count: REACT_COMMIT_CASCADE_NOTICE_LIMIT - 1 })
driveCommits({ state, count: 1, lanes: 0 })
expect(state.commits).toBe(0)
expect(state.cascadeRoot).toBeNull()
driveCommits({ state, count: REACT_COMMIT_CASCADE_NOTICE_LIMIT - 1 })
expect(recordBreadcrumb).not.toHaveBeenCalled()
})
it('ignores lanes outside React nested-update mask', () => {
const state = createReactCommitCascadeState()
driveCommits({ state, count: REACT_COMMIT_CASCADE_NOTICE_LIMIT * 2, lanes: IDLE_LANE })
expect(state.commits).toBe(0)
expect(recordBreadcrumb).not.toHaveBeenCalled()
})
// Why per root: React's counter is root-global but not cross-root, so two
// roots alternating commits are not one cascade.
it('restarts the count when a different root commits', () => {
const state = createReactCommitCascadeState()
driveCommits({ state, count: REACT_COMMIT_CASCADE_NOTICE_LIMIT - 1 })
driveCommits({ state, count: 1, root: OTHER_ROOT })
expect(state.commits).toBe(1)
expect(state.cascadeRoot).toBe(OTHER_ROOT)
expect(recordBreadcrumb).not.toHaveBeenCalled()
})
it('honours explicit limit overrides', () => {
const state = createReactCommitCascadeState()
for (let index = 0; index < 4; index += 1) {
recordReactCommit({
state,
root: ROOT,
pendingLanes: SYNC_LANE,
readNowMs: () => 1_000,
noticeLimit: 4,
armCommits: 2
})
}
expect(cascadePayload().commits).toBe(4)
})
// Why asserted: a clock read per commit is the one cost this design refuses.
it('reads the clock only at arm and at report', () => {
const state = createReactCommitCascadeState()
const readNowMs = vi.fn(() => 1_000)
for (let index = 0; index < REACT_COMMIT_CASCADE_NOTICE_LIMIT; index += 1) {
recordReactCommit({ state, root: ROOT, pendingLanes: SYNC_LANE, readNowMs })
}
expect(readNowMs).toHaveBeenCalledTimes(2)
})
})
// Why asserted: the popout is a separate BrowserWindow with its own module
// state, so the payload is the only thing that says which window cascaded.
describe('renderer surface', () => {
it('reports the surface the cascade happened on', () => {
setReactCommitCascadeRendererSurface('dashboard-popout')
const state = createReactCommitCascadeState()
driveCommits({ state, count: REACT_COMMIT_CASCADE_NOTICE_LIMIT })
expect(cascadePayload().rendererSurface).toBe('dashboard-popout')
})
})
describe('driver attribution', () => {
it('arms write sampling at the arm floor, not before', () => {
const state = createReactCommitCascadeState()
driveCommits({ state, count: REACT_COMMIT_CASCADE_ARM_COMMITS - 1 })
expect(reactCommitCascadeWriteProbe.armed).toBe(false)
driveCommits({ state, count: 1 })
expect(reactCommitCascadeWriteProbe.armed).toBe(true)
})
it('names the driving write with a path-free frame', () => {
const state = createReactCommitCascadeState()
function drivingWrite(): void {
noteReactCommitCascadeStoreWrite(drivingWrite, { tabs: [] })
}
driveCommits({
state,
count: REACT_COMMIT_CASCADE_NOTICE_LIMIT,
onCommit: () => {
if (reactCommitCascadeWriteProbe.armed) {
drivingWrite()
}
}
})
const payload = cascadePayload()
expect(payload.storeWrites).toBeGreaterThan(0)
// Why the caller and not drivingWrite itself: captureStackTrace elides the
// boundary frame, so in production the first frame is whoever called `set`.
expect(String(payload.driverFrame)).toContain('onCommit')
expect(payload.changedKeys).toBe('tabs')
// Why: the path redaction is keyed on the detail NAME, so a path inside the
// value would ship a developer's home directory.
expect(String(payload.driverStack)).not.toContain('/')
// Why sanitize instead of matching the key name: the 4000-char budget is
// decided by a camel-split rule in crash-report-redaction, so a hand-copied
// regex here can pass while the real rule drops the frames to 240.
// Why resolve the key from the payload: hard-coding it here is the same
// drift this assertion exists to catch.
const stackKey = Object.keys(payload).find(
(key) => key !== 'driverFrame' && String(payload[key]).includes(String(payload.driverFrame))
)
expect(stackKey).toBeDefined()
const longFrames = 'a'.repeat(600)
const sanitized = sanitizeCrashReportDetails({ ...payload, [stackKey as string]: longFrames })
expect(String(sanitized[stackKey as string])).toHaveLength(longFrames.length)
})
it('clears samples when a cascade ends', () => {
const state = createReactCommitCascadeState()
function drivingWrite(): void {
noteReactCommitCascadeStoreWrite(drivingWrite, { tabs: [] })
}
driveCommits({
state,
count: REACT_COMMIT_CASCADE_NOTICE_LIMIT,
onCommit: () => {
if (reactCommitCascadeWriteProbe.armed) {
drivingWrite()
}
}
})
recordBreadcrumb.mockClear()
driveCommits({ state, count: 1, lanes: 0 })
driveCommits({
state,
count: REACT_COMMIT_CASCADE_NOTICE_LIMIT,
nowMs: 1_000 + REACT_COMMIT_CASCADE_MIN_REPORT_INTERVAL_MS + 1
})
expect(cascadePayload().storeWrites).toBe(0)
expect(cascadePayload().driverFrame).toBeUndefined()
})
})
describe('report throttle', () => {
it('suppresses cascades inside the interval and carries the count forward', () => {
const state = createReactCommitCascadeState()
for (let cascade = 0; cascade < 3; cascade += 1) {
driveCommits({ state, count: 1, lanes: 0 })
driveCommits({ state, count: REACT_COMMIT_CASCADE_NOTICE_LIMIT })
}
expect(recordBreadcrumb).toHaveBeenCalledTimes(1)
expect(state.suppressed).toBe(2)
driveCommits({ state, count: 1, lanes: 0 })
driveCommits({
state,
count: REACT_COMMIT_CASCADE_NOTICE_LIMIT,
nowMs: 1_000 + REACT_COMMIT_CASCADE_MIN_REPORT_INTERVAL_MS + 1
})
expect(recordBreadcrumb).toHaveBeenCalledTimes(2)
expect(cascadePayload(1).suppressed).toBe(2)
expect(state.suppressed).toBe(0)
})
})
/**
* Why dimensional and not just functional: a cascade that ends releases its
* samples, but a renderer that oscillates for minutes runs that arm/end cycle
* thousands of times, and anything the reset misses accumulates there.
*/
describe('repeated arm and end cycles', () => {
const CYCLES = 1_000
/** The steady state is a few KB of live objects; a per-cycle leak is megabytes. */
const MAX_HEAP_GROWTH_BYTES = 1_000_000
function drivingWrite(): void {
noteReactCommitCascadeStoreWrite(drivingWrite, { tabs: [], panes: [] })
}
function runCascadeCycle(state: ReactCommitCascadeState): void {
driveCommits({
state,
count: REACT_COMMIT_CASCADE_NOTICE_LIMIT,
onCommit: () => {
if (reactCommitCascadeWriteProbe.armed) {
drivingWrite()
}
}
})
driveCommits({ state, count: 1, lanes: 0 })
}
it('leaves nothing behind across a thousand cascades', () => {
const collectGarbage = (globalThis as { gc?: () => void }).gc
// --expose-gc is pinned in config/vitest.config.ts execArgv.
expect(typeof collectGarbage).toBe('function')
const state = createReactCommitCascadeState()
for (let warmup = 0; warmup < 20; warmup += 1) {
runCascadeCycle(state)
}
collectGarbage?.()
const before = process.memoryUsage().heapUsed
for (let cycle = 0; cycle < CYCLES; cycle += 1) {
runCascadeCycle(state)
}
collectGarbage?.()
expect(process.memoryUsage().heapUsed - before).toBeLessThan(MAX_HEAP_GROWTH_BYTES)
expect(reactCommitCascadeWriteProbe.armed).toBe(false)
expect(readReactCommitCascadeWriteSummary()).toMatchObject({
storeWrites: 0,
storeWriteSites: 0,
driverFrame: undefined,
driverStack: undefined,
changedKeys: undefined
})
expect(state.cascadeRoot).toBeNull()
})
})
@@ -0,0 +1,205 @@
/**
* Counts consecutive same-root commits that keep scheduling synchronous work,
* and breadcrumbs the cascade before React #185 throws.
*
* This mirrors React's own accounting rather than approximating it with a time
* window: react-dom resets its nested-update counter the moment a commit leaves
* no sync lanes pending, and increments it otherwise. Reading `root.pendingLanes`
* at commit time reproduces that reset exactly.
*
* Known over-count: React also requires the committed lanes to intersect its own
* mask, which we cannot read from the devtools callback. Dropping that term can
* only make us count a commit React would have skipped, so we fire early, never
* late — the safe direction for a diagnostic that must beat the throw.
*
* Out of scope by construction: a passive-effect (useEffect) loop reports
* pendingLanes 0 here, because passive effects flush after the commit callback.
* That is correct — React tracks those in nestedPassiveUpdateCount, which only
* console.errors in development and never throws #185.
*/
import { compactBreadcrumbData } from '@/lib/crash-breadcrumb-data'
import { recordRendererCrashBreadcrumb } from '@/lib/crash-breadcrumb-recorder'
import {
armReactCommitCascadeWriteSampling,
readReactCommitCascadeWriteSummary,
resetReactCommitCascadeWriteSamples
} from '@/lib/react-commit-cascade-store-write-samples'
import { readStoreListenerCount } from '@/store/store-listener-census'
import type { RendererSurface } from '@/lib/renderer-memory-sampling'
import { REACT_NESTED_UPDATE_LIMIT } from '../../../shared/react-update-depth-attribution'
export const REACT_COMMIT_CASCADE_BREADCRUMB = 'react_commit_cascade'
/**
* SyncLane | InputContinuousLane | DefaultLane. A root still holding any of
* these after a commit is what React counts as a nested update.
*/
export const REACT_CASCADING_LANES = 42
/** Headroom for the crumb to be built and sent before React throws above 50. */
const REACT_COMMIT_CASCADE_NOTICE_HEADROOM = 10
export const REACT_COMMIT_CASCADE_NOTICE_LIMIT =
REACT_NESTED_UPDATE_LIMIT - REACT_COMMIT_CASCADE_NOTICE_HEADROOM
/**
* Nothing in the app legitimately reaches 20 consecutive same-root cascading
* commits — the two deliberate oscillation loops are capped at 8 and 3 — and it
* leaves 20 commits for the sampled writes to land inside the loop.
*/
export const REACT_COMMIT_CASCADE_ARM_COMMITS = 20
/** Matches RENDERER_BREADCRUMB_COALESCE_MS; main drops anything faster anyway. */
export const REACT_COMMIT_CASCADE_MIN_REPORT_INTERVAL_MS = 30_000
export type ReactCommitCascadeState = {
/**
* Held strongly, and never dereferenced. The next commit that leaves no
* cascading lanes clears the slot — milliseconds away in a live renderer —
* so the only tree this pins is a React root unmounted mid-cascade, such as
* the per-decoration roots in useDiffCommentDecorator.
*/
cascadeRoot: unknown
commits: number
reported: boolean
armedAtMs: number | null
lastReportedAtMs: number | null
/** Cascades that reached the notice limit while the report interval was live. */
suppressed: number
}
export function createReactCommitCascadeState(): ReactCommitCascadeState {
return {
cascadeRoot: null,
commits: 0,
reported: false,
armedAtMs: null,
lastReportedAtMs: null,
suppressed: 0
}
}
const sharedState = createReactCommitCascadeState()
let rendererSurface: RendererSurface = 'main'
export function setReactCommitCascadeRendererSurface(surface: RendererSurface): void {
rendererSurface = surface
}
export function resetReactCommitCascadeTelemetryForTests(): void {
Object.assign(sharedState, createReactCommitCascadeState())
rendererSurface = 'main'
resetReactCommitCascadeWriteSamples()
}
function endCascade(state: ReactCommitCascadeState): void {
state.cascadeRoot = null
state.commits = 0
state.reported = false
state.armedAtMs = null
resetReactCommitCascadeWriteSamples()
}
function reportCascade(state: ReactCommitCascadeState, pendingLanes: number, nowMs: number): void {
const suppressed = state.suppressed
state.suppressed = 0
state.lastReportedAtMs = nowMs
const writes = readReactCommitCascadeWriteSummary()
recordRendererCrashBreadcrumb(
REACT_COMMIT_CASCADE_BREADCRUMB,
compactBreadcrumbData({
commits: state.commits,
commitBudget: REACT_NESTED_UPDATE_LIMIT,
elapsedMs: state.armedAtMs === null ? undefined : nowMs - state.armedAtMs,
pendingLanes,
// Why 0 is worth shipping: it says the loop is useState-driven, not store-driven.
storeWrites: writes.storeWrites,
storeWriteSites: writes.storeWriteSites,
driverFrame: writes.driverFrame,
driverStack: writes.driverStack,
changedKeys: writes.changedKeys,
storeListeners: readStoreListenerCount() ?? undefined,
rendererSurface,
suppressed: suppressed > 0 ? suppressed : undefined
})
)
}
function recordCommit(
state: ReactCommitCascadeState,
root: unknown,
pendingLanes: number,
readNowMs: () => number,
noticeLimit: number,
armCommits: number,
minReportIntervalMs: number
): void {
if ((pendingLanes & REACT_CASCADING_LANES) === 0) {
if (state.cascadeRoot !== null) {
endCascade(state)
}
return
}
if (root !== state.cascadeRoot) {
endCascade(state)
state.cascadeRoot = root
}
state.commits += 1
if (state.commits < armCommits) {
return
}
if (state.commits === armCommits) {
state.armedAtMs = readNowMs()
armReactCommitCascadeWriteSampling()
return
}
if (state.reported || state.commits < noticeLimit) {
return
}
state.reported = true
const nowMs = readNowMs()
// Why the renderer throttles too: nothing rate-limits this pipe, and every
// call is a structured clone plus an ipcRenderer.send.
const sinceReportMs = state.lastReportedAtMs === null ? null : nowMs - state.lastReportedAtMs
if (sinceReportMs !== null && sinceReportMs >= 0 && sinceReportMs < minReportIntervalMs) {
state.suppressed += 1
return
}
reportCascade(state, pendingLanes, nowMs)
}
/** Injectable entry point; production uses observeReactCommit. */
export function recordReactCommit(args: {
state: ReactCommitCascadeState
root: unknown
pendingLanes: number
readNowMs: () => number
noticeLimit?: number
armCommits?: number
minReportIntervalMs?: number
}): void {
recordCommit(
args.state,
args.root,
args.pendingLanes,
args.readNowMs,
args.noticeLimit ?? REACT_COMMIT_CASCADE_NOTICE_LIMIT,
args.armCommits ?? REACT_COMMIT_CASCADE_ARM_COMMITS,
args.minReportIntervalMs ?? REACT_COMMIT_CASCADE_MIN_REPORT_INTERVAL_MS
)
}
/**
* Per-commit hot path: one property read, one mask, two compares, one
* increment. No clock read and no allocation until a cascade arms.
*/
export function observeReactCommit(root: unknown, pendingLanes: number): void {
recordCommit(
sharedState,
root,
pendingLanes,
Date.now,
REACT_COMMIT_CASCADE_NOTICE_LIMIT,
REACT_COMMIT_CASCADE_ARM_COMMITS,
REACT_COMMIT_CASCADE_MIN_REPORT_INTERVAL_MS
)
}
@@ -0,0 +1,70 @@
/**
* Guarantees __REACT_DEVTOOLS_GLOBAL_HOOK__ exists, and nothing else.
*
* Split out from the observer because the two halves have different deadlines:
* react-dom reads this global ONCE, at its own module evaluation, so a fresh
* shim must beat that import — but wrapping `onCommitFiberRoot` can happen at
* any time, since react-dom re-reads the property at every commit. This module
* therefore installs itself on import and has NO imports of its own, so a bare
* side-effect import cannot be outrun by a transitive graph that reaches
* react-dom, and cannot drag the breadcrumb recorder in ahead of any vi.mock.
*/
/**
* Only what react-dom and react-refresh require. onCommitFiberUnmount,
* onPostCommitFiberRoot and setStrictMode are deliberately absent: react-dom
* guards each with a typeof check, and calls onCommitFiberUnmount once per
* DELETED FIBER, which is genuinely hot when terminal panes unmount.
*/
export type ReactDevtoolsCommitHook = {
isDisabled?: boolean
supportsFiber?: boolean
renderers?: Map<number, unknown>
inject?: (renderer: unknown) => number
onCommitFiberRoot?: (
rendererId: number,
root: unknown,
priorityLevel: unknown,
didError: boolean
) => void
}
// Indexed, not a global augmentation: bippy (a react-grab dependency) already
// declares this global with a narrower callback signature.
type GlobalWithReactDevtoolsHook = Record<string, ReactDevtoolsCommitHook | undefined>
function createCommitHookShim(): ReactDevtoolsCommitHook {
const renderers = new Map<number, unknown>()
return {
isDisabled: false,
supportsFiber: true,
renderers,
inject: (renderer) => {
const rendererId = renderers.size + 1
renderers.set(rendererId, renderer)
return rendererId
},
// Why a noop and not undefined: react-refresh captures this property and
// calls oldOnCommitFiberRoot.apply(this, arguments) with no typeof guard.
onCommitFiberRoot: () => {}
}
}
/** Returns the existing hook untouched, or installs a minimal one. */
export function ensureReactDevtoolsCommitHook(): ReactDevtoolsCommitHook | undefined {
try {
const globalWithHook = globalThis as unknown as GlobalWithReactDevtoolsHook
const existing = globalWithHook.__REACT_DEVTOOLS_GLOBAL_HOOK__
if (existing) {
return existing
}
const hook = createCommitHookShim()
globalWithHook.__REACT_DEVTOOLS_GLOBAL_HOOK__ = hook
return hook
} catch {
// A renderer without a patchable global still has to boot.
return undefined
}
}
ensureReactDevtoolsCommitHook()
+6
View File
@@ -1,3 +1,9 @@
// Why first, and why the import-free shim: react-dom reads
// __REACT_DEVTOOLS_GLOBAL_HOOK__ once at module evaluation, so the global has to
// exist before it. The observer below only wraps a property react-dom re-reads
// per commit, so its own import graph can evaluate whenever it likes.
import './lib/react-devtools-commit-hook-shim'
import './lib/react-commit-cascade-observer'
import './assets/main.css'
import { StrictMode } from 'react'
+8
View File
@@ -1,3 +1,9 @@
// Why first, and why the import-free shim: react-dom reads
// __REACT_DEVTOOLS_GLOBAL_HOOK__ once at module evaluation, so the global has to
// exist before it. The observer below only wraps a property react-dom re-reads
// per commit, so its own import graph can evaluate whenever it likes.
import './lib/react-devtools-commit-hook-shim'
import './lib/react-commit-cascade-observer'
import './assets/main.css'
import { StrictMode, useEffect } from 'react'
@@ -15,6 +21,7 @@ import { translate } from './i18n/i18n'
import { useAppStore } from './store'
import type { GlobalSettings } from '../../shared/global-settings-types'
import { getOrCreateRendererRoot } from './lib/react-renderer-root'
import { setReactCommitCascadeRendererSurface } from './lib/react-commit-cascade-telemetry'
// Why: the pop-out window is a separate BrowserWindow with its own React root,
// so it must run the same renderer bootstrap as main.tsx (crash diagnostics,
@@ -22,6 +29,7 @@ import { getOrCreateRendererRoot } from './lib/react-renderer-root'
// window. It shares the preload/window.api but not the DOM or JS context.
recordRendererCrashBreadcrumb('popout_bootstrap_started', { dev: import.meta.env.DEV })
installRendererCrashDiagnostics('dashboard-popout')
setReactCommitCascadeRendererSurface('dashboard-popout')
function applyPopoutAppearance(settings: GlobalSettings | null): void {
applyDocumentTheme(settings?.theme ?? 'system', { disableTransitions: false })
+52 -49
View File
@@ -52,61 +52,64 @@ import {
registerRuntimeHttpLinkBrowserOpener
} from '@/lib/http-link-routing'
import { installStoreListenerCensus } from './store-listener-census'
import { withReactCommitCascadeWriteProbe } from './react-commit-cascade-write-probe'
import {
registerRendererMemoryProfileContributor,
summarizeStateCollectionSizes
} from '@/lib/renderer-memory-profile'
import { estimateStateCollectionKB } from '@/lib/state-collection-byte-estimate'
export const useAppStore = create<AppState>()((...a) => {
// Why: the inner api is only reachable here, before create() copies subscribe onto the hook.
installStoreListenerCensus(a[2])
return {
...createRepoSlice(...a),
...createSparsePresetsSlice(...a),
...createWorktreeSlice(...a),
...createTerminalSlice(...a),
...createTabsSlice(...a),
...createUISlice(...a),
...createSettingsSlice(...a),
...createKeybindingsSlice(...a),
...createGitHubSlice(...a),
...createHostedReviewSlice(...a),
...createLinearSlice(...a),
...createPreflightSlice(...a),
...createJiraSlice(...a),
...createEditorSlice(...a),
...createStatsSlice(...a),
...createMemorySlice(...a),
...createWorkspaceSpaceSlice(...a),
...createClaudeUsageSlice(...a),
...createCodexUsageSlice(...a),
...createOpenCodeUsageSlice(...a),
...createBrowserSlice(...a),
...createRateLimitSlice(...a),
...createSshSlice(...a),
...createRuntimeEnvironmentSshSlice(...a),
...createAgentStatusSlice(...a),
...createPaneForegroundAgentSlice(...a),
...createDiffCommentsSlice(...a),
...createDetectedAgentsSlice(...a),
...createRuntimeDetectedAgentsSlice(...a),
...createWorktreeNavHistorySlice(...a),
...createDictationSlice(...a),
...createWorkspaceCleanupSlice(...a),
...createWorkspaceCleanupBrowseSlice(...a),
...createRuntimeStatusSlice(...a),
...createPullRequestGenerationSlice(...a),
...createCommitMessageGenerationSlice(...a),
...createPinnedTabCloseConfirmSlice(...a),
...createRecentlyClosedTabsSlice(...a),
...createOrcaProfilesSlice(...a),
...createNewIssueDraftSlice(...a),
...createTaskCreationDraftsSlice(...a),
...createRemoteServerUpdatesSlice(...a),
...createTerminalQuickCommandHostsSlice(...a)
}
})
export const useAppStore = create<AppState>()(
withReactCommitCascadeWriteProbe((...a) => {
// Why: the inner api is only reachable here, before create() copies subscribe onto the hook.
installStoreListenerCensus(a[2])
return {
...createRepoSlice(...a),
...createSparsePresetsSlice(...a),
...createWorktreeSlice(...a),
...createTerminalSlice(...a),
...createTabsSlice(...a),
...createUISlice(...a),
...createSettingsSlice(...a),
...createKeybindingsSlice(...a),
...createGitHubSlice(...a),
...createHostedReviewSlice(...a),
...createLinearSlice(...a),
...createPreflightSlice(...a),
...createJiraSlice(...a),
...createEditorSlice(...a),
...createStatsSlice(...a),
...createMemorySlice(...a),
...createWorkspaceSpaceSlice(...a),
...createClaudeUsageSlice(...a),
...createCodexUsageSlice(...a),
...createOpenCodeUsageSlice(...a),
...createBrowserSlice(...a),
...createRateLimitSlice(...a),
...createSshSlice(...a),
...createRuntimeEnvironmentSshSlice(...a),
...createAgentStatusSlice(...a),
...createPaneForegroundAgentSlice(...a),
...createDiffCommentsSlice(...a),
...createDetectedAgentsSlice(...a),
...createRuntimeDetectedAgentsSlice(...a),
...createWorktreeNavHistorySlice(...a),
...createDictationSlice(...a),
...createWorkspaceCleanupSlice(...a),
...createWorkspaceCleanupBrowseSlice(...a),
...createRuntimeStatusSlice(...a),
...createPullRequestGenerationSlice(...a),
...createCommitMessageGenerationSlice(...a),
...createPinnedTabCloseConfirmSlice(...a),
...createRecentlyClosedTabsSlice(...a),
...createOrcaProfilesSlice(...a),
...createNewIssueDraftSlice(...a),
...createTaskCreationDraftsSlice(...a),
...createRemoteServerUpdatesSlice(...a),
...createTerminalQuickCommandHostsSlice(...a)
}
})
)
registerHttpLinkStoreAccessor(() => useAppStore.getState())
registerRuntimeHttpLinkBrowserOpener(async (request) => {
@@ -0,0 +1,73 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { create, type StoreApi, type UseBoundStore } from 'zustand'
import { withReactCommitCascadeWriteProbe } from './react-commit-cascade-write-probe'
const { probe, noteWrite } = vi.hoisted(() => ({
probe: { armed: false },
noteWrite: vi.fn()
}))
vi.mock('@/lib/react-commit-cascade-store-write-samples', () => ({
reactCommitCascadeWriteProbe: probe,
noteReactCommitCascadeStoreWrite: (boundary: object, partial: unknown) =>
noteWrite(boundary, partial)
}))
type ProbedState = { ticks: number; label?: string }
function createProbedStore(): UseBoundStore<StoreApi<ProbedState>> {
return create<ProbedState>()(
withReactCommitCascadeWriteProbe<ProbedState>(() => ({ ticks: 0, label: 'initial' }))
)
}
beforeEach(() => {
probe.armed = false
noteWrite.mockReset()
})
describe('withReactCommitCascadeWriteProbe', () => {
// Why this matters most: this wraps every write the app makes. A diagnostic
// that throws here would drop the write and take app state with it.
it('lands the write even when the sampler throws', () => {
noteWrite.mockImplementation(() => {
throw new Error('sampler exploded')
})
const useProbedStore = createProbedStore()
probe.armed = true
expect(() => useProbedStore.setState({ ticks: 3 })).not.toThrow()
expect(useProbedStore.getState().ticks).toBe(3)
})
// Why asserted: create() copies api.setState onto the hook AFTER the creator
// runs, so a middleware that forgets this line leaves store.setState unprobed.
it('routes the api setState through the probe', () => {
const useProbedStore = createProbedStore()
probe.armed = true
useProbedStore.setState({ ticks: 1 })
expect(noteWrite).toHaveBeenCalledTimes(1)
// The boundary must be the wrapper itself, so V8 elides it from the sample.
expect(noteWrite.mock.calls[0]?.[0]).toBe(useProbedStore.setState)
expect(noteWrite.mock.calls[0]?.[1]).toEqual({ ticks: 1 })
})
it('passes the replace flag through', () => {
const useProbedStore = createProbedStore()
probe.armed = true
useProbedStore.setState({ ticks: 2 }, true)
expect(useProbedStore.getState()).toEqual({ ticks: 2 })
})
it('costs nothing but a flag read while unarmed', () => {
const useProbedStore = createProbedStore()
useProbedStore.setState({ ticks: 4 })
expect(noteWrite).not.toHaveBeenCalled()
expect(useProbedStore.getState().ticks).toBe(4)
})
})
@@ -0,0 +1,37 @@
/**
* zustand middleware that samples store writes during a suspected React commit
* cascade, so the crash report names the code driving it.
*
* Why middleware and not a setState patch: create() hands slices the `set`
* closure built before `api` exists (zustand/esm/vanilla.mjs), so patching
* useAppStore.setState afterwards misses every slice-internal set() — and the
* slices are where the writes are. Wrapping the creator catches both, the same
* way store-listener-census has to reach the inner api to see hook subscribers.
*
* Cost when no cascade is suspected: one boolean field load per write.
*/
import {
noteReactCommitCascadeStoreWrite,
reactCommitCascadeWriteProbe
} from '@/lib/react-commit-cascade-store-write-samples'
import type { StateCreator } from 'zustand'
export function withReactCommitCascadeWriteProbe<TState>(
createState: StateCreator<TState, [], []>
): StateCreator<TState, [], []> {
return (set, get, api) => {
const wrapped = ((partial: unknown, replace?: unknown): void => {
if (reactCommitCascadeWriteProbe.armed) {
try {
noteReactCommitCascadeStoreWrite(wrapped, partial)
} catch {
// This is the app's universal write path: a diagnostic that throws here
// would drop the write itself.
}
}
;(set as (nextPartial: unknown, nextReplace?: unknown) => void)(partial, replace)
}) as typeof set
api.setState = wrapped as typeof api.setState
return createState(wrapped, get, api)
}
}
@@ -4,6 +4,13 @@
* next after the counter trips. The catching boundary is a bystander picked by commit order.
*/
/**
* react-dom 19.2.x NESTED_UPDATE_LIMIT. getRootForUpdatedFiber throws once the
* root-global nested-update counter passes it. Every diagnostic that budgets
* commits against React's bail must derive its threshold from this one number.
*/
export const REACT_NESTED_UPDATE_LIMIT = 50
export const UNRELIABLE_BOUNDARY_ATTRIBUTION = 'unreliable'
export type CrashReportAttribution = typeof UNRELIABLE_BOUNDARY_ATTRIBUTION
@@ -158,6 +158,7 @@ type RuntimeTypeInventory = [
Runtime.RuntimeTerminalSplit,
Runtime.RuntimeTerminalState,
Runtime.RuntimeTerminalSummary,
Runtime.RuntimeTerminalUnavailableReason,
Runtime.RuntimeTerminalVisualGroupNode,
Runtime.RuntimeTerminalVisualLayout,
Runtime.RuntimeTerminalVisualLayoutNode,
@@ -225,8 +226,11 @@ describe('runtime client public export parity', () => {
'BROWSER_UNAVAILABLE_ERROR_CODE',
'COMPUTER_ERROR_CODES',
'HEADLESS_RUNTIME_WINDOW_ID',
'TERMINAL_PTY_DEGRADATION_CAPABILITY',
'TERMINAL_UNAVAILABLE_ERROR_CODE',
'UNPUBLISHED_WORKTREE_PUBLICATION_EPOCH',
'browserUnavailableMessage'
'browserUnavailableMessage',
'terminalUnavailableMessage'
])
expect(Object.keys(RemoteClient).sort()).toEqual([
'RemoteRuntimeClientError',
+10 -13
View File
@@ -680,7 +680,7 @@ test.describe('Browser Tab', () => {
}
})
test('plain links stay current while explicit new-tab gestures activate Orca tabs', async ({
test('every new-tab link gesture activates an Orca tab and never a native window', async ({
electronApp,
orcaPage
}) => {
@@ -698,23 +698,20 @@ test.describe('Browser Tab', () => {
const baseWindowCount = await electronApp.evaluate(
({ BaseWindow }) => BaseWindow.getAllWindows().length
)
const baseTabCount = await orcaPage.locator('[data-tab-id]').count()
await clickBrowserLink(orcaPage, sourceTab!.id, '#external-link')
// A plain target=_blank click is a new-tab request, in the main frame and in an iframe;
// the source tab must stay put rather than navigate away under it.
const sourceTabLocator = orcaPage.locator(`[data-tab-id="${sourceTab!.id}"]`)
await expect(sourceTabLocator).toContainText('Linked destination', { timeout: 10_000 })
await expect(orcaPage.locator('[data-tab-id]')).toHaveCount(baseTabCount)
await clickBrowserLink(orcaPage, sourceTab!.id, '#external-link')
await expectBrowserTabActive(orcaPage, 'Linked destination')
await expect(sourceTabLocator).toContainText('Source page')
await switchToBrowserTab(orcaPage, worktreeId, sourceTab!.id)
await clickBrowserLink(orcaPage, sourceTab!.id, '#return-link')
await expect(sourceTabLocator).toContainText('Source page', { timeout: 10_000 })
await clickBrowserLink(orcaPage, sourceTab!.id, '#frame-link', {
frameSelector: '#link-frame'
})
await expect(sourceTabLocator).toContainText('Frame destination', { timeout: 10_000 })
await expect(orcaPage.locator('[data-tab-id]')).toHaveCount(baseTabCount)
await clickBrowserLink(orcaPage, sourceTab!.id, '#return-link')
await expect(sourceTabLocator).toContainText('Source page', { timeout: 10_000 })
await expectBrowserTabActive(orcaPage, 'Frame destination')
await expect(sourceTabLocator).toContainText('Source page')
await switchToBrowserTab(orcaPage, worktreeId, sourceTab!.id)
await clickBrowserLink(orcaPage, sourceTab!.id, '#frame-modifier-link', {
frameSelector: '#link-frame',