fix(runtime): a handle-gap verdict answers for its pane, not for the tab id

Folds adv2-skew's e8cac056d7 into the reconciled union. Closes the fourth orphan
class, the only one that was not conservative: a retracted tab id republished as a
different pane inherited the old pane's verdict and skipped its own wait — the
#19735 direction rather than a longer hold.

It needs no fourth trigger, which is why it composes with the three drains rather
than competing with them. Every trigger those rules own fires downstream of the
moment this hazard needs. The verdict instead carries the environment-minted PTY
binding its pane held AT PARK TIME, and only answers for a pane that still holds
it: a republished pane binds a newly minted PTY and serves its own wait, while a
genuine reattach to the same PTY inherits, which is correct — the verdict follows
the PTY, not the id. A transient rowless frame touches neither, so the read-time
check is safe where a retraction-triggered prune would not have been.

TWO MEASUREMENTS, both requested rather than assumed.

1. The record-then-release ordering is load-bearing and IS pinned. `recordExpiredWait`
reads the waiter's park-time binding, so it must run before `releaseWaiter` deletes
the entry. Swapping the two statements fails three cases, so the capture is not
correct merely by accident of statement order.

2. The `''` fallback is a MATCH VALUE, not a null: two panes that both hold no
environment-minted PTY compare equal and inherit, which is the same hazard in a
narrower window. Measured unreachable through the production park path rather than
assumed — the only route in is `kind: 'handle'`, which `findUnhydratedHostMirrorForPane`
reports only when `tabHoldsEnvironmentPtyBinding` finds a binding, reading the SAME
map through the SAME predicate as `paneBindingFor`. It now refuses to answer anyway.
That coupling is two functions in two files with nothing enforcing it, refusing costs
only a re-park, and the direction is conservative.

THE REFUSAL IS WHAT FOUND THE REAL BUG. With `''` matching, any fixture that omits
`terminalLayoutsByTabId` records `''`, compares `'' === ''`, and passes while the
pane-identity check is entirely inert. Making it refuse turned that silence into
four failures across host-mirror-handle-gap-drain and -teardown, whose fixtures seed
no layout binding at all. Both now bind per environment — one shared environment id
filters every other environment's pane back to `''` and restores the no-op.

Mutation-tested on the merged tree: ignoring the binding fails case D and the
mid-wait case; re-reading at expiry fails the mid-wait case and nothing else;
letting `''` match fails the empty-binding case; widening the tab-death rule across
environments still fails the live-verdict case, so pane identity does not weaken the
scoping the sweep was reconciled around.

Also fixes a real-clock race this branch introduced: the revoke-window test read
`Date.now()` separately from `enqueue`'s own stamp, and under load the drift ate
into the window. It now anchors the injected clock to the item's `createdAt`.
This commit is contained in:
Neil
2026-09-10 18:13:21 -07:00
parent 46ad377ceb
commit cdafc90d8f
5 changed files with 183 additions and 35 deletions
@@ -56,9 +56,11 @@ function serviceOver(revokeOutbox: RelayRevokeOutbox, ledger: RelayDemandLedger)
describe('a revoke that can never succeed', () => {
it('stops pinning relay demand once its window passes, but is never abandoned', async () => {
let now = Date.now()
let now = 0
const { revokeOutbox, ledger } = fixture(() => now)
revokeOutbox.enqueue(binding('device-1'))
// Anchor the injected clock to the item's own `createdAt`. Reading Date.now() separately races
// enqueue's real-clock stamp, and under load the drift silently eats into the window.
now = revokeOutbox.enqueue(binding('device-1')).createdAt
expect(ledger.hasDemand(ownerIdentityKey)).toBe(true)
const revokeDevice = vi.fn().mockRejectedValue(new Error('device_not_found'))
@@ -83,9 +85,11 @@ describe('a revoke that can never succeed', () => {
})
it('still lets a revoke that lands remove the item and release demand', async () => {
let now = Date.now()
let now = 0
const { revokeOutbox, ledger } = fixture(() => now)
revokeOutbox.enqueue(binding('device-1'))
// Anchor the injected clock to the item's own `createdAt`. Reading Date.now() separately races
// enqueue's real-clock stamp, and under load the drift silently eats into the window.
now = revokeOutbox.enqueue(binding('device-1')).createdAt
const revokeDevice = vi.fn().mockResolvedValue(undefined)
const service = serviceOver(revokeOutbox, ledger)
@@ -24,6 +24,9 @@ const SECOND_TAB_ID = 'web-terminal-host-tab-2'
const initialAppStoreState = useAppStore.getState()
function seedRows(): void {
// Layout bindings are seeded because a verdict names the PANE by the environment-minted PTY it
// held at park time. A pane with no binding never reaches the park path in production, and its
// verdict deliberately refuses to answer, so a fixture without one models nothing real.
useAppStore.setState({
ptyIdsByTabId: {},
tabsByWorktree: {
@@ -31,6 +34,20 @@ function seedRows(): void {
{ id: FIRST_TAB_ID, title: 'one' },
{ id: SECOND_TAB_ID, title: 'two' }
]
},
terminalLayoutsByTabId: {
[FIRST_TAB_ID]: {
root: { type: 'leaf', leafId: 'leaf-1' },
activeLeafId: 'leaf-1',
expandedLeafId: null,
ptyIdsByLeafId: { 'leaf-1': `remote:${ENVIRONMENT_ID}@@term_1` }
},
[SECOND_TAB_ID]: {
root: { type: 'leaf', leafId: 'leaf-2' },
activeLeafId: 'leaf-2',
expandedLeafId: null,
ptyIdsByLeafId: { 'leaf-2': `remote:${ENVIRONMENT_ID}@@term_2` }
}
}
} as never)
}
@@ -29,11 +29,24 @@ function parkAndExpire(environmentId: string, tabId: string): void {
// Rows ACCUMULATE. Replacing them would unpublish the panes parked earlier, and the tab-death
// rule would then legitimately sweep their verdicts before teardown was ever reached — this
// suite is about a class no recording-driven prune can reach, so every pane here stays live.
const published = useAppStore.getState().tabsByWorktree[WORKTREE_ID] ?? []
const state = useAppStore.getState()
const published = state.tabsByWorktree[WORKTREE_ID] ?? []
useAppStore.setState({
ptyIdsByTabId: {},
tabsByWorktree: {
[WORKTREE_ID]: [...published.filter((tab) => tab.id !== tabId), { id: tabId, title: tabId }]
},
// A verdict names its PANE by the environment-minted PTY held at park time, so a fixture with
// no layout binding records '' and the verdict refuses to answer. Bind per environment: one
// shared environment id would filter to '' for every other environment's pane.
terminalLayoutsByTabId: {
...state.terminalLayoutsByTabId,
[tabId]: {
root: { type: 'leaf', leafId: `leaf-${tabId}` },
activeLeafId: `leaf-${tabId}`,
expandedLeafId: null,
ptyIdsByLeafId: { [`leaf-${tabId}`]: `remote:${environmentId}@@term_${tabId}` }
}
}
} as never)
parkUntilHostMirrorHandleLands(environmentId, WORKTREE_ID, tabId, () => {})
@@ -24,10 +24,15 @@ import {
* A tab churn on a LIVE environment tab-death rule, recording environment only
* B REMOVED environment clearHostMirrorHandleGapVerdictsForEnvironment
* C cross-environment QUIESCENCE generation rule, per key, every environment
* D REUSED tab id NOTHING. Pinned below as a live hazard.
* D REUSED tab id read-time pane identity, NOT a prune
*
* Plus the two properties no rule may break: the verdict stays sticky enough to break the
* park/expire/replay loop, and no rule evicts a verdict a live pane still needs.
* D is the one that needed no new trigger: every trigger the other three own fires downstream of
* the moment it needs. The verdict instead carries the PTY binding its pane held AT PARK TIME and
* only answers for a pane that still holds it.
*
* Plus the properties no rule may break: the verdict stays sticky enough to break the
* park/expire/replay loop, a genuine reattach still inherits, and no rule evicts a verdict a live
* pane still needs.
*/
const ENV_A = 'env-union-a'
@@ -36,9 +41,30 @@ const ENV_C = 'env-union-c'
const WORKTREE = 'repo-1::wt-union'
const initialAppStoreState = useAppStore.getState()
function setLiveTabs(tabIds: string[]): void {
/** Which environment minted each pane's PTY; the binding only counts for its own environment. */
const ENV_OF_TAB: Record<string, string> = {
a1: ENV_A,
a2: ENV_A,
reused: ENV_A,
b1: ENV_B,
c1: ENV_C
}
/** Publishes rows AND the layout PTY binding each pane holds — the binding is the pane's identity. */
function setLiveTabs(tabIds: string[], ptyByTabId: Record<string, string> = {}): void {
const layouts: Record<string, unknown> = {}
for (const id of tabIds) {
const ptyId = ptyByTabId[id] ?? `remote:${ENV_OF_TAB[id] ?? ENV_A}@@term_${id}`
layouts[id] = {
root: { type: 'leaf', leafId: `leaf-${id}` },
activeLeafId: `leaf-${id}`,
expandedLeafId: null,
ptyIdsByLeafId: { [`leaf-${id}`]: ptyId }
}
}
useAppStore.setState({
tabsByWorktree: { [WORKTREE]: tabIds.map((id) => ({ id, title: id, ptyId: null })) },
terminalLayoutsByTabId: layouts,
ptyIdsByTabId: {}
} as unknown as AppState)
}
@@ -82,8 +108,9 @@ describe('handle-gap verdict map, all rules on one tree', () => {
setRuntimeEnvironmentConnectionGenerationForTests(ENV_C, 2)
clearHostMirrorHandleGapVerdictsForEnvironment(ENV_B)
// A's tab closes; the reused id is retracted and republished as a DIFFERENT pane.
setLiveTabs(['a2', 'reused'])
// A's tab closes; the reused id is retracted and republished as a DIFFERENT pane, which binds
// a PTY the host newly minted. That new binding is what makes it a different pane, not the id.
setLiveTabs(['a2', 'reused'], { reused: `remote:${ENV_A}@@term_freshly_minted` })
parkAndExpire(ENV_A, 'a2')
// A drained: a1's row is gone and env-a recorded again, so the tab-death rule swept it.
@@ -93,27 +120,61 @@ describe('handle-gap verdict map, all rules on one tree', () => {
// C drained: env-a's expiry retired env-c's superseded row, though env-c never expired again.
expect(hasHostMirrorHandleWaitExpired(ENV_C, 'c1')).toBe(false)
// D IS NOT DRAINED, and this assertion pins a LIVE HAZARD rather than a desired behaviour.
// The republished pane inherits the retracted pane's verdict and skips its own wait.
//
// Why this one is different from every other gap argued over on this map: the others DROP a
// verdict, so the pane re-parks and only ever holds longer. This one RETAINS a verdict and
// lets a fresh pane resume on a handle that has not landed — the #19735 direction itself.
// It is therefore the one gap here that is not conservative.
//
// No rule reaches it, and each for its own reason: the tab-death rule's predicate stops
// matching the moment the id is republished, so it is not even eventually consistent; the
// teardown drain fires on environment teardown, not on tab retraction inside a live one; and
// no waiter exists to observe the retraction, because a pane holding a verdict never parks
// (`findUnhydratedHostMirrorForPane` returns null on it). Closing it needs a fourth trigger,
// on row retraction. DO NOT delete this case when a prune for dead tabs lands — "a prune for
// dead tabs shipped" is exactly the plausible assumption that would delete it.
expect(hasHostMirrorHandleWaitExpired(ENV_A, 'reused')).toBe(true)
// D is closed, and NOT by a prune. No trigger any rule above owns fires at the right moment:
// the tab-death predicate stops matching once the id is live again, teardown is the wrong
// event, and no waiter observes the retraction because a pane holding a verdict never parks.
// It is closed at READ time instead — the verdict names the pane it was about, so a pane that
// binds a newly minted PTY does not answer to it and serves its own wait.
expect(hasHostMirrorHandleWaitExpired(ENV_A, 'reused')).toBe(false)
// Only the two live verdicts survive: a2's and the stranded reused-id row.
expect(countHostMirrorHandleGapVerdictsForTests()).toBe(2)
})
it('answers for a genuine reattach that still holds the same PTY', () => {
// The verdict follows the PTY, not the tab id. A pane that reattaches to the SAME environment
// PTY is the same pane, so it must inherit — otherwise the identity check would have quietly
// removed the loop-breaker for every reattach.
setRuntimeEnvironmentConnectionGenerationForTests(ENV_A, 1)
setLiveTabs(['a1'])
parkAndExpire(ENV_A, 'a1')
setLiveTabs([])
setLiveTabs(['a1'])
expect(hasHostMirrorHandleWaitExpired(ENV_A, 'a1')).toBe(true)
})
it('records the binding the pane held at PARK time, not at expiry', () => {
// The mutation this kills: reading the binding inside `recordExpiredWait` from the store
// instead of from the waiter. A pane replaced mid-wait leaves the original waiter running to
// term, and an expiry-time read would attribute the verdict to whoever holds the id by then —
// handing the new pane a wait it never served. Three earlier cases all survived that bug;
// only rebinding BETWEEN park and expire distinguishes the two implementations.
setRuntimeEnvironmentConnectionGenerationForTests(ENV_A, 1)
setLiveTabs(['a1'])
parkUntilHostMirrorHandleLands(ENV_A, WORKTREE, 'a1', () => {})
setLiveTabs(['a1'], { a1: `remote:${ENV_A}@@term_replacement` })
vi.advanceTimersByTime(HOST_MIRROR_HANDLE_GAP_DEADLINE_MS + 1)
// The replacement pane never served this wait, so it must not inherit its verdict.
expect(hasHostMirrorHandleWaitExpired(ENV_A, 'a1')).toBe(false)
})
it('refuses to answer on an empty binding, which is a match value and not a null', () => {
// '' is what `paneBindingFor` returns when no leaf holds an environment-minted PTY. Two
// different panes both reading '' would compare EQUAL and inherit, which is the reused-tab-id
// shape again. Measured unreachable through the production park path rather than assumed: the
// only route into `parkUntilHostMirrorHandleLands` is `kind: 'handle'`, which
// `findUnhydratedHostMirrorForPane` reports only when `tabHoldsEnvironmentPtyBinding`
// (host-mirrored-pane-liveness.ts:28-31) finds a match — the SAME `terminalLayoutsByTabId`
// map through the SAME `parseRemoteRuntimePtyId` predicate `paneBindingFor` uses, so a pane
// that would bind '' never parks. It must still refuse rather than match, because that
// coupling is two functions in two files and nothing enforces it.
setRuntimeEnvironmentConnectionGenerationForTests(ENV_A, 1)
setLiveTabs(['a1'], { a1: 'remote:some-other-env@@term_1' })
parkAndExpire(ENV_A, 'a1')
expect(hasHostMirrorHandleWaitExpired(ENV_A, 'a1')).toBe(false)
})
it('keeps a verdict sticky enough to break the park/expire/replay loop', () => {
// The verdict exists to stop a pane re-parking forever. If any rule evicted it while the pane
// is live and its connection current, the wait would rearm on a fresh budget every replay.
@@ -1,6 +1,7 @@
import { useAppStore } from '@/store'
import { getRuntimeEnvironmentConnectionGeneration } from '@/store/slices/runtime-status'
import { WEB_SESSION_TAB_RPC_TIMEOUT_MS } from '@/runtime/web-session-tab-rpc-timeout'
import { parseRemoteRuntimePtyId } from '../../../shared/remote-runtime-pty-id'
/**
* Per-pane park for the frame between a host's tab rows and its PTY handles.
@@ -34,6 +35,8 @@ type HandleGapWaiter = {
tabId: string
/** Connection generation the wait was armed on; its verdict is void on any other. */
generation: number
/** Which PANE this wait is about, captured at park time; see ExpiredHandleGapVerdict. */
paneBinding: string
deadline: ReturnType<typeof setTimeout>
run: () => void
}
@@ -63,18 +66,54 @@ const waitersByPane = new Map<string, HandleGapWaiter>()
* never parks, so no waiter observes the retraction. Closing it needs a fourth trigger, on row
* retraction. Pinned in host-mirror-handle-gap-verdict-union.test.ts; do not delete that case.
*/
const expiredGenerationByPane = new Map<string, number>()
type ExpiredHandleGapVerdict = {
generation: number
/** Sorted environment-minted PTY ids the tab's leaves held AT PARK TIME; '' when none. */
paneBinding: string
}
const expiredGenerationByPane = new Map<string, ExpiredHandleGapVerdict>()
let unsubscribeStore: (() => void) | null = null
function paneWaitKey(environmentId: string, tabId: string): string {
return `${environmentId}\0${tabId}`
}
/** True once the deadline fired for this pane on the current connection. */
/**
* The environment-minted PTY ids this tab's leaves are bound to, as one comparable string.
*
* Read from the layout, not `ptyIdsByTabId`: during the handle gap the published-handle map is
* empty by definition — that is the gap — while the layout binding is what
* `tabHoldsEnvironmentPtyBinding` already uses to call the pane unverifiable rather than dead.
*/
function paneBindingFor(tabId: string, environmentId: string): string {
const bindings = useAppStore.getState().terminalLayoutsByTabId[tabId]?.ptyIdsByLeafId ?? {}
return Object.values(bindings)
.filter(
(ptyId): ptyId is string =>
typeof ptyId === 'string' && parseRemoteRuntimePtyId(ptyId)?.environmentId === environmentId
)
.sort()
.join('')
}
/** True once the deadline fired for THIS pane on the current connection. */
export function hasHostMirrorHandleWaitExpired(environmentId: string, tabId: string): boolean {
const verdict = expiredGenerationByPane.get(paneWaitKey(environmentId, tabId))
if (verdict === undefined || verdict.paneBinding === '') {
// Why '' never answers: it is a MATCH VALUE, not a null. Two different panes that both hold no
// environment-minted PTY compare equal, which is the reused-tab-id inheritance this check
// exists to stop, in a narrower window. Unreachable through the production park path —
// `findUnhydratedHostMirrorForPane` only reports `kind: 'handle'` when
// `tabHoldsEnvironmentPtyBinding` finds a binding, reading the same map through the same
// predicate as `paneBindingFor` — and pinned by the coupling test in
// host-mirror-handle-gap-verdict-union.test.ts. Refusing costs a re-park, which is the
// conservative direction, so the pair stays safe even if those two reads ever drift apart.
return false
}
return (
expiredGenerationByPane.get(paneWaitKey(environmentId, tabId)) ===
getRuntimeEnvironmentConnectionGeneration(environmentId)
verdict.generation === getRuntimeEnvironmentConnectionGeneration(environmentId) &&
// Why this and not the key alone: the key is a tab id, and the pane behind it can be replaced.
verdict.paneBinding === paneBindingFor(tabId, environmentId)
)
}
@@ -94,14 +133,14 @@ function recordExpiredWait(environmentId: string, key: string): void {
// way round, and both wrong shapes were independently written before this was reconciled.
const prefix = `${environmentId}\0`
const liveTabs = liveTabIds()
for (const [staleKey, staleGeneration] of expiredGenerationByPane) {
for (const [staleKey, stale] of expiredGenerationByPane) {
// GENERATION, judged per key across EVERY environment. `hasHostMirrorHandleWaitExpired`
// compares a row against its own environment's CURRENT generation, so a row whose generation
// has moved can never return true for anyone. Retiring it cannot cost a reader a verdict,
// whoever owns it. Scoped to the recording environment, an environment that reconnects and
// then goes quiet strands its rows forever.
const staleEnvironmentId = staleKey.slice(0, staleKey.indexOf('\0'))
if (staleGeneration !== getRuntimeEnvironmentConnectionGeneration(staleEnvironmentId)) {
if (stale.generation !== getRuntimeEnvironmentConnectionGeneration(staleEnvironmentId)) {
expiredGenerationByPane.delete(staleKey)
continue
}
@@ -116,7 +155,14 @@ function recordExpiredWait(environmentId: string, key: string): void {
expiredGenerationByPane.delete(staleKey)
}
}
expiredGenerationByPane.set(key, generation)
// Why the waiter's park-time binding and not a fresh read: this verdict is about the pane whose
// wait just ran out. Re-reading here would attribute it to whatever holds the id NOW, handing a
// pane that replaced it mid-wait a verdict it never served. The caller must therefore record
// BEFORE `releaseWaiter` deletes the entry; the union suite pins that ordering.
expiredGenerationByPane.set(key, {
generation,
paneBinding: waitersByPane.get(key)?.paneBinding ?? ''
})
}
function stopStoreSubscriptionIfIdle(): void {
@@ -218,7 +264,14 @@ export function parkUntilHostMirrorHandleLands(
}
releaseWaiter(key)
}, HOST_MIRROR_HANDLE_GAP_DEADLINE_MS)
waitersByPane.set(key, { worktreeId, tabId, generation, deadline, run })
waitersByPane.set(key, {
worktreeId,
tabId,
generation,
paneBinding: paneBindingFor(tabId, environmentId),
deadline,
run
})
startStoreSubscription()
}