perf(crash): scan worktree tabs by key, and soften the verdict rationale

Round 6 corrected my own round-5 comment. I had written that `true` is the
repair-loop signature and `false` covers a deliberate background activation. The
repair effect can emit `false` too: its closure holds the worktree from its render
while the guard runs against live state, so a worktree switch landing in between
reattributes the tab. The verdict hints at the caller; it does not prove it, and
neither value should be discarded. Comment-only.

Also take the free scan win the perf review measured: Object.entries allocates a
pair array per worktree on a path that runs per tab activation. Own keys are safe
to index by, so Object.keys plus an indexed read is behaviour-identical
(16.1us -> 5.0us at 170 worktrees x 10 tabs).
This commit is contained in:
Neil
2026-08-01 06:04:41 -07:00
parent 514717e285
commit ca86a4ab2c
3 changed files with 20 additions and 13 deletions
@@ -319,8 +319,8 @@ describe('renderer breadcrumb IPC routing', () => {
}
})
// Why flag-scoped: coalescing keeps only the newest payload, and the two
// verdicts come from different call paths, so either would erase the other.
// Why flag-scoped: coalescing keeps only the newest payload, and the verdict
// flips under a persisting duplicate, so one would erase the other.
it('keeps a non-converging duplicate-tab-owner notice out of the converging one', () => {
emitRendererBreadcrumb({
name: 'terminal_tab_id_owned_by_multiple_worktrees',
+4 -4
View File
@@ -367,10 +367,10 @@ function rendererBreadcrumbCoalesceKey(
// 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: the
// two verdicts come from different call paths — `true` from the active-terminal
// repair effect (the React #185 signature), `false` also from a deliberate
// background activation — and coalescing keeps only the newest payload, so
// either would erase the other. Two keys still bound the storm.
// verdict flips under a persisting duplicate (it tracks whether the tab still
// resolves to the active worktree), and coalescing keeps only the newest
// payload, so one verdict would erase the other. Two keys still bound the
// storm.
if (name === DUPLICATE_TAB_OWNER_BREADCRUMB) {
return `${name}:${String(data?.resolvedToActiveWorktree ?? '')}`
}
@@ -33,7 +33,10 @@ export function resolveActiveTabOwnerWorktreeId(
// 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)) {
// Why keys and not entries: entries allocates a pair array per worktree on a path
// that runs per tab activation. Own keys stay safe to index by.
for (const worktreeId of Object.keys(tabsByWorktree)) {
const tabs = tabsByWorktree[worktreeId]
if (!tabs.some((tab) => tab.id === tabId)) {
continue
}
@@ -48,12 +51,16 @@ export function resolveActiveTabOwnerWorktreeId(
// Why breadcrumb: no production origin for a duplicated tab id is known yet,
// so a crash bundle carrying this is what proves or kills the hypothesis.
// Reading it: `ownerCount > 1` is the load-bearing datum. `true` is the
// repair-loop signature (that effect only ever activates a tab from the active
// worktree's own list), while `false` also covers a deliberate background
// activation such as jump-to-agent. Why the verdict is in the guard key: the
// active worktree changes under a persisting duplicate, and coalescing keeps
// only the newest payload, so either verdict would otherwise erase the other.
// Reading it: `ownerCount > 1` is the load-bearing datum; the verdict only
// hints at the caller. A sustained repair loop shows up as `true`, since that
// effect picks from the active worktree's own list — but it does not prove the
// caller, and the repair effect can emit `false` too: its closure holds the
// worktree from its render while this runs against live state, so a worktree
// switch landing in between (an earlier-flushed effect, or IPC before the
// passive flush) reattributes the tab. So `false` covers that race as well as
// a deliberate background activation such as jump-to-agent — discard neither.
// Why the verdict is in the guard key: it flips under a persisting duplicate,
// and coalescing keeps only the newest payload, so one would erase the other.
// Still at most two crumbs per tab id.
const resolvedToActiveWorktree = activeOwnerId !== null
const verdictKey = `${tabId}:${resolvedToActiveWorktree}`