mirror of
https://github.com/stablyai/orca.git
synced 2026-10-20 00:03:45 +00:00
fix(crash-reporting): key the duplicate-tab-owner crumb on its convergence flag
Name-only coalescing keeps only the newest payload, so a resolvedToActiveWorktree false sample — the one value saying the activation still could not converge — was erased by any later benign true in the same 30s window. Keys on the flag instead, mirroring the WebGL name:kind branch; two keys still bound the burst. Also: the previous coalescing commit had no test at all (removing the name from both sets broke zero of 2701 tests), the resolver's activeWorktreeId truthiness check was a hole rather than a guard for a '' active id, and the resolver test file failed oxfmt --check. Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
@@ -298,6 +298,49 @@ describe('renderer breadcrumb IPC routing', () => {
|
||||
])
|
||||
})
|
||||
|
||||
// Why: the renderer guard is once-per-tab-id, so one stale worktree map emits
|
||||
// a crumb per duplicated tab and would evict the pre-crash trail.
|
||||
it('coalesces duplicate-tab-owner notices across tabs', () => {
|
||||
emitRendererBreadcrumb({
|
||||
name: 'terminal_tab_id_owned_by_multiple_worktrees',
|
||||
data: { ownerCount: 2, resolvedToActiveWorktree: true }
|
||||
})
|
||||
emitRendererBreadcrumb({
|
||||
name: 'terminal_tab_id_owned_by_multiple_worktrees',
|
||||
data: { ownerCount: 3, resolvedToActiveWorktree: true }
|
||||
})
|
||||
|
||||
expect(recordCrashBreadcrumbMock).not.toHaveBeenCalled()
|
||||
expect(recordCoalescedCrashBreadcrumbMock).toHaveBeenCalledTimes(2)
|
||||
for (const call of recordCoalescedCrashBreadcrumbMock.mock.calls) {
|
||||
expect(call[0]).toMatchObject({
|
||||
coalesceKey: 'terminal_tab_id_owned_by_multiple_worktrees:true'
|
||||
})
|
||||
}
|
||||
})
|
||||
|
||||
// Why flag-scoped: coalescing keeps only the newest payload, and `false` is
|
||||
// the only value saying the activation still could not converge activeTabId.
|
||||
it('keeps a non-converging duplicate-tab-owner notice out of the converging one', () => {
|
||||
emitRendererBreadcrumb({
|
||||
name: 'terminal_tab_id_owned_by_multiple_worktrees',
|
||||
data: { ownerCount: 2, resolvedToActiveWorktree: false }
|
||||
})
|
||||
emitRendererBreadcrumb({
|
||||
name: 'terminal_tab_id_owned_by_multiple_worktrees',
|
||||
data: { ownerCount: 2, resolvedToActiveWorktree: true }
|
||||
})
|
||||
|
||||
expect(
|
||||
recordCoalescedCrashBreadcrumbMock.mock.calls.map(
|
||||
(call) => (call[0] as { coalesceKey: string }).coalesceKey
|
||||
)
|
||||
).toEqual([
|
||||
'terminal_tab_id_owned_by_multiple_worktrees:false',
|
||||
'terminal_tab_id_owned_by_multiple_worktrees:true'
|
||||
])
|
||||
})
|
||||
|
||||
it('records non-error renderer breadcrumbs without coalescing', () => {
|
||||
emitRendererBreadcrumb({ name: 'renderer_bootstrap_started', data: { dev: true } })
|
||||
|
||||
|
||||
@@ -326,12 +326,13 @@ function buildUncapturedCrashReportText(
|
||||
// storm, #8260) can flush the whole fixed-size breadcrumb ring in seconds,
|
||||
// erasing the pre-crash trail. Coalesce repeats into one entry that carries a
|
||||
// suppressed count instead.
|
||||
const DUPLICATE_TAB_OWNER_BREADCRUMB = 'terminal_tab_id_owned_by_multiple_worktrees'
|
||||
const COALESCED_RENDERER_BREADCRUMB_NAMES = new Set([
|
||||
'renderer_error',
|
||||
'renderer_unhandled_rejection',
|
||||
'terminal_park_verdict_churn',
|
||||
'terminal_safe_fit_retry_exhausted',
|
||||
'terminal_tab_id_owned_by_multiple_worktrees',
|
||||
DUPLICATE_TAB_OWNER_BREADCRUMB,
|
||||
TERMINAL_WEBGL_DIAGNOSTIC_BREADCRUMB
|
||||
])
|
||||
const RENDERER_BREADCRUMB_COALESCE_MS = 30_000
|
||||
@@ -344,14 +345,9 @@ const RENDERER_BREADCRUMB_COALESCE_MS = 30_000
|
||||
// mounted pane within ~60ms. Windows crash F0BKR84AHEH lost 26-90% of its
|
||||
// 30-entry ring to two such bursts. `suppressedSinceLast` keeps the pane count
|
||||
// — the only signal these carry — in one slot.
|
||||
//
|
||||
// terminal_tab_id_owned_by_multiple_worktrees: its renderer-side guard is
|
||||
// once-per-tab-id, not once-per-session, so one stale worktree map duplicates
|
||||
// every tab id at once and emits a crumb per tab.
|
||||
const NAME_ONLY_COALESCED_BREADCRUMB_NAMES = new Set([
|
||||
'terminal_park_verdict_churn',
|
||||
'terminal_safe_fit_retry_exhausted',
|
||||
'terminal_tab_id_owned_by_multiple_worktrees'
|
||||
'terminal_safe_fit_retry_exhausted'
|
||||
])
|
||||
|
||||
function rendererBreadcrumbCoalesceKey(
|
||||
@@ -368,6 +364,16 @@ function rendererBreadcrumbCoalesceKey(
|
||||
if (name === TERMINAL_WEBGL_DIAGNOSTIC_BREADCRUMB) {
|
||||
return `${name}:${String(data?.kind ?? '')}`
|
||||
}
|
||||
// Why coalesce at all: the renderer guard is once-per-tab-id, not
|
||||
// once-per-session, so one stale worktree map duplicates every tab id at once
|
||||
// and emits a crumb per tab. Why keyed on the flag rather than name alone:
|
||||
// `false` is the only value that says the activation still could not converge
|
||||
// activeTabId, and coalescing keeps just the newest payload — a later benign
|
||||
// `true` would erase the sample the crumb exists to capture. Two keys still
|
||||
// bound the storm.
|
||||
if (name === DUPLICATE_TAB_OWNER_BREADCRUMB) {
|
||||
return `${name}:${String(data?.resolvedToActiveWorktree ?? '')}`
|
||||
}
|
||||
const primaryMessage = name === 'renderer_error' ? data?.message : data?.reasonMessage
|
||||
const fallbackMessage = name === 'renderer_error' ? data?.errorMessage : undefined
|
||||
const message =
|
||||
|
||||
@@ -6,9 +6,8 @@ vi.mock('../../lib/crash-breadcrumb-recorder', () => ({
|
||||
recordRendererCrashBreadcrumb: (...args: unknown[]) => recordRendererCrashBreadcrumb(...args)
|
||||
}))
|
||||
|
||||
const { resolveActiveTabOwnerWorktreeId, _resetDuplicateTabOwnerBreadcrumbsForTests } = await import(
|
||||
'./active-tab-owner-worktree'
|
||||
)
|
||||
const { resolveActiveTabOwnerWorktreeId, _resetDuplicateTabOwnerBreadcrumbsForTests } =
|
||||
await import('./active-tab-owner-worktree')
|
||||
|
||||
function tab(id: string, worktreeId: string): TerminalTab {
|
||||
return { id, worktreeId, title: id, createdAt: 0, sortOrder: 0 } as unknown as TerminalTab
|
||||
@@ -62,6 +61,16 @@ describe('resolveActiveTabOwnerWorktreeId', () => {
|
||||
)
|
||||
})
|
||||
|
||||
// Why: a truthiness guard on the active id would drop this back to first-match.
|
||||
it('prefers a falsy-but-valid active worktree id', () => {
|
||||
const owner = resolveActiveTabOwnerWorktreeId(
|
||||
{ 'wt-other': [tab('t1', 'wt-other')], '': [tab('t1', '')] },
|
||||
'',
|
||||
't1'
|
||||
)
|
||||
expect(owner).toBe('')
|
||||
})
|
||||
|
||||
it('breadcrumbs a given tab id only once so it cannot flood the ring', () => {
|
||||
const maps = { 'wt-a': [tab('t1', 'wt-a')], 'wt-b': [tab('t1', 'wt-b')] }
|
||||
for (let i = 0; i < 5; i += 1) {
|
||||
|
||||
@@ -25,7 +25,10 @@ export function resolveActiveTabOwnerWorktreeId(
|
||||
let ownerCount = 0
|
||||
// Why tracked in-loop rather than re-read by key: `tabsByWorktree[activeWorktreeId]`
|
||||
// resolves inherited members for ids like `toString`, and `?.some` would then throw.
|
||||
let activeWorktreeOwnsTab = false
|
||||
// Why the id and not a boolean: a falsy-but-valid active id ('') would fail a
|
||||
// truthiness guard below and silently fall back to the first match — the very
|
||||
// misattribution this function exists to remove.
|
||||
let activeOwnerId: string | null = null
|
||||
for (const [worktreeId, tabs] of Object.entries(tabsByWorktree)) {
|
||||
if (!tabs.some((tab) => tab.id === tabId)) {
|
||||
continue
|
||||
@@ -35,7 +38,7 @@ export function resolveActiveTabOwnerWorktreeId(
|
||||
firstOwnerId = worktreeId
|
||||
}
|
||||
if (worktreeId === activeWorktreeId) {
|
||||
activeWorktreeOwnsTab = true
|
||||
activeOwnerId = worktreeId
|
||||
}
|
||||
}
|
||||
|
||||
@@ -45,12 +48,12 @@ export function resolveActiveTabOwnerWorktreeId(
|
||||
reportedDuplicateTabIds.add(tabId)
|
||||
recordRendererCrashBreadcrumb('terminal_tab_id_owned_by_multiple_worktrees', {
|
||||
ownerCount,
|
||||
resolvedToActiveWorktree: activeWorktreeOwnsTab
|
||||
resolvedToActiveWorktree: activeOwnerId !== null
|
||||
})
|
||||
}
|
||||
|
||||
if (ownerCount > 1 && activeWorktreeId && activeWorktreeOwnsTab) {
|
||||
return activeWorktreeId
|
||||
if (ownerCount > 1 && activeOwnerId !== null) {
|
||||
return activeOwnerId
|
||||
}
|
||||
return firstOwnerId
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user