From d4f83451726c4d86de5cbccde5f18e58448bbc3d Mon Sep 17 00:00:00 2001 From: Neil Date: Tue, 15 Sep 2026 22:30:29 -0700 Subject: [PATCH] fix(runtime): stop an unpublished-worktree placeholder retiring the live publisher MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A worktree the host has published nothing for still answers a forced list, with a synthesized `none`/v0 frame that means "ask me later" (host-session-snapshot-authority.ts). Every post-close list and every activation of an emptied worktree gets one. Noting it as a publication retired the renderer generation that is still live, and because that epoch is per-process, the terminal the user created next never reached this client — the same lockout the retraction path was already careful to avoid, through a door it did not cover. `local-structured-session-tabs-sync` already skips the placeholder for this exact reason; the web mirror now does too, on both the receipt ledger and the frame decision. Bound the receipt ledgers by frame age rather than entry count. One bootstrap inventory records a receipt per worktree under a single reserved frame, so evicting by insertion order dropped that batch's own earlier entries, and an absent receipt is what the recovery gate reads as "no evidence for this worktree". Only a receipt no in-flight frame can still be ranked against is droppable. Take the receipt gate off the `web-session-tabs-sync` barrel in the refresh path. Ordering is that path's gate, not an optional collaborator a caller's module mock may leave out, and being reachable only through the barrel is how the path came to have no ordering at all. --- .../runtime/web-runtime-session-snapshot.ts | 10 ++- ...moved-frame-retires-live-publisher.test.ts | 70 +++++++++++++++++++ .../src/runtime/web-session-tabs-sync.ts | 4 -- .../runtime/web-session-tabs-sync/state.ts | 27 ++++--- .../tracking-decisions.ts | 6 +- .../runtime/web-session-tabs-sync/tracking.ts | 47 +++++++++---- 6 files changed, 133 insertions(+), 31 deletions(-) diff --git a/src/renderer/src/runtime/web-runtime-session-snapshot.ts b/src/renderer/src/runtime/web-runtime-session-snapshot.ts index c434f51f65f..86b5399abab 100644 --- a/src/renderer/src/runtime/web-runtime-session-snapshot.ts +++ b/src/renderer/src/runtime/web-runtime-session-snapshot.ts @@ -13,6 +13,12 @@ import { captureRuntimeEnvironmentCall } from './web-runtime-session-environment import { throwIfE2eWebRuntimeBrowserReconciliationFails } from './web-runtime-browser-creation-e2e-fault' import { getSessionTabsRuntimeIdFromResponse } from './web-session-tabs-sync/publisher-identity-fences' import { WEB_SESSION_TABS_FRAME_OUTRANKED } from './web-session-tabs-sync/tracking-decisions' +// Not through the barrel: receipt ordering is this path's gate, not an optional collaborator a +// caller's module mock may leave out — doing so is what left this path unordered to begin with. +import { + recordReceivedWebSessionTabsSnapshot, + shouldApplyRecoveredWebSessionTabsSnapshot +} from './web-session-tabs-sync/tracking' import { recoverWebSessionTerminalOrphansBeforeApply } from './web-session-terminal-orphan-recovery' const pendingRuntimeWorktreeRecoveryRefreshes = new Map() @@ -90,9 +96,7 @@ export async function refreshWebRuntimeSessionTabsSnapshot( const { applyWebSessionTabsSnapshot, applyWebSessionTabsStorePatch, - decideWebSessionTabsSnapshot, - recordReceivedWebSessionTabsSnapshot, - shouldApplyRecoveredWebSessionTabsSnapshot + decideWebSessionTabsSnapshot } = webSessionTabsSync // A list is evidence about a moment, not about now. Record its place in receipt order before // ranking it, or a snapshot the host answered before a close lands after the retraction did. diff --git a/src/renderer/src/runtime/web-session-tabs-removed-frame-retires-live-publisher.test.ts b/src/renderer/src/runtime/web-session-tabs-removed-frame-retires-live-publisher.test.ts index 695619d08f8..abdaa6feb4f 100644 --- a/src/renderer/src/runtime/web-session-tabs-removed-frame-retires-live-publisher.test.ts +++ b/src/renderer/src/runtime/web-session-tabs-removed-frame-retires-live-publisher.test.ts @@ -7,9 +7,11 @@ import { shouldApplyRecoveredWebSessionTabsSnapshot } from './web-session-tabs-sync/tracking' import { + MAX_TRACKED_SESSION_TABS_RECEIPTS, nextReceivedSessionTabsFrame, VISIBILITY_INVENTORY_REMOVAL_EPOCH } from './web-session-tabs-sync/state' +import { UNPUBLISHED_WORKTREE_PUBLICATION_EPOCH } from '../../../shared/runtime-types' import { resetWebSessionTabsSyncTestState } from './web-session-tabs-sync-test-harness' vi.mock('../store', () => ({ useAppStore: { setState: vi.fn() } })) @@ -113,6 +115,74 @@ describe('a removal frame must not retire the publisher that is still live', () expect(admits(delayed, delayedReceived)).toBe(false) }) + /** + * The receipt ledger is bounded, and one bootstrap inventory records a receipt per worktree under + * a single reserved frame. Evicting by insertion count would drop that batch's own earlier + * entries, and an absent receipt is what the recovery gate reads as "no evidence for this + * worktree" — so the bound would silently discard the worktrees it was meant to protect. + */ + it('keeps every receipt an inventory recorded under one frame, past the bound', () => { + const requestReceivedFrame = nextReceivedSessionTabsFrame() + const worktrees = Array.from( + { length: MAX_TRACKED_SESSION_TABS_RECEIPTS + 64 }, + (_value, index) => `repo::/worktree-${index}` + ) + for (const worktree of worktrees) { + recordReceivedWebSessionTabsSnapshot( + ENVIRONMENT_ID, + { ...liveFrame(1), worktree }, + requestReceivedFrame, + undefined, + 'bootstrap' + ) + } + + for (const worktree of [worktrees[0]!, worktrees.at(-1)!]) { + expect( + shouldApplyRecoveredWebSessionTabsSnapshot( + ENVIRONMENT_ID, + { ...liveFrame(1), worktree }, + requestReceivedFrame + ) + ).toBe(true) + } + }) + + /** + * A worktree the host has published nothing for still answers a forced list, with a synthesized + * `none`/v0 frame that means "ask me later" (host-session-snapshot-authority.ts). Every + * post-close list and every activation of an emptied worktree gets one. Noting it as a + * publication retires the renderer generation that is still live, and since that generation's + * epoch is per-process, the terminal the user creates next never reaches this client. + */ + it('does not let an unpublished-worktree placeholder retire the live publisher', () => { + const liveReceived = recordReceivedWebSessionTabsSnapshot(ENVIRONMENT_ID, liveFrame(1)) + expect(admits(liveFrame(1), liveReceived)).toBe(true) + + const removedReceived = recordReceivedWebSessionTabsSnapshot(ENVIRONMENT_ID, removalFrame()) + expect(admits(removalFrame(), removedReceived)).toBe(true) + + const placeholder = { + ...liveFrame(1), + publicationEpoch: UNPUBLISHED_WORKTREE_PUBLICATION_EPOCH, + snapshotVersion: 0, + tabs: [] + } as RuntimeMobileSessionTabsResult + const placeholderReceived = recordReceivedWebSessionTabsSnapshot( + ENVIRONMENT_ID, + placeholder, + undefined, + undefined, + 'bootstrap' + ) + admits(placeholder, placeholderReceived) + + // The user creates a terminal; the same live generation publishes its worktree again. + const republished = liveFrame(2) + const republishedReceived = recordReceivedWebSessionTabsSnapshot(ENVIRONMENT_ID, republished) + expect(admits(republished, republishedReceived)).toBe(true) + }) + /** * The case the version fallback cannot decide. The receipt ledger is one slot, and the live * republication overwrites it, so by the time the pre-close list lands the only record that a diff --git a/src/renderer/src/runtime/web-session-tabs-sync.ts b/src/renderer/src/runtime/web-session-tabs-sync.ts index 85e8094cc66..ffaf111de41 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync.ts @@ -22,10 +22,6 @@ export { _getWebSessionTabsReceiptTrackingCountsForTest, _getWebSessionTabsTrackingCountsForTest } from './web-session-tabs-sync/tracking-lifecycle' -export { - recordReceivedWebSessionTabsSnapshot, - shouldApplyRecoveredWebSessionTabsSnapshot -} from './web-session-tabs-sync/tracking' export { resolveHostSessionTabIdForWebSessionTab } from './web-session-tabs-sync/tracking-mappings' export { decideWebSessionTabsSnapshot, diff --git a/src/renderer/src/runtime/web-session-tabs-sync/state.ts b/src/renderer/src/runtime/web-session-tabs-sync/state.ts index 18220a30f87..da0ddebf779 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync/state.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync/state.ts @@ -92,16 +92,27 @@ export const latestReceivedSessionTabsSnapshotByWorktree = new Map< /** Receipt ledgers outlive the worktrees they order, so their keys need a bound of their own. */ export const MAX_TRACKED_SESSION_TABS_RECEIPTS = 512 -/** Writes evict least-recently-written first, so the surviving keys are the ones still being ordered. */ -export function setBoundedSessionTabsReceipt(map: Map, key: string, value: T): void { - map.delete(key) +/** + * Bounds a receipt ledger by frame age, never by entry count. One inventory records a receipt per + * worktree under a single reserved frame, and evicting by insertion order would drop that batch's + * own earlier entries — which the recovery gate reads as "no evidence for this worktree" and uses + * to reject it. Only a receipt no in-flight frame can still be ranked against is droppable. + */ +export function setBoundedSessionTabsReceipt( + map: Map, + key: string, + value: T, + frameOf: (entry: T) => number +): void { map.set(key, value) - while (map.size > MAX_TRACKED_SESSION_TABS_RECEIPTS) { - const oldest = map.keys().next().value - if (typeof oldest !== 'string') { - break + if (map.size <= MAX_TRACKED_SESSION_TABS_RECEIPTS) { + return + } + const oldestRankableFrame = receivedSessionTabsFrameSequence - MAX_TRACKED_SESSION_TABS_RECEIPTS + for (const [entryKey, entry] of map) { + if (frameOf(entry) < oldestRankableFrame) { + map.delete(entryKey) } - map.delete(oldest) } } diff --git a/src/renderer/src/runtime/web-session-tabs-sync/tracking-decisions.ts b/src/renderer/src/runtime/web-session-tabs-sync/tracking-decisions.ts index eb9fce184c1..b3923df5111 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync/tracking-decisions.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync/tracking-decisions.ts @@ -18,6 +18,7 @@ import { trackWebSessionTabsWorktree, recordAcceptedWebSessionTabsEnvironment } from './tracking' +import { hostSnapshotAffirmsWorktreeContents } from '../host-session-snapshot-authority' import { clearWebSessionTabsTrackingForWorktree } from './tracking-lifecycle' import { queueAcceptedWebSessionTerminalSnapshot } from '../web-session-terminal-handle-events' @@ -113,7 +114,10 @@ export function decideWebSessionTabsSnapshot( } rememberHostTerminalTabCount(environmentId, snapshot) replayableSessionTabsSnapshotByWorktree.delete(key) - noteSessionTabsPublicationEpoch(key, snapshot.publicationEpoch) + // A frame that affirms nothing about the worktree has not taken over publishing it. + if (hostSnapshotAffirmsWorktreeContents(snapshot)) { + noteSessionTabsPublicationEpoch(key, snapshot.publicationEpoch) + } latestSessionTabsSnapshotByWorktree.set(key, { publicationEpoch: snapshot.publicationEpoch, snapshotVersion: snapshot.snapshotVersion diff --git a/src/renderer/src/runtime/web-session-tabs-sync/tracking.ts b/src/renderer/src/runtime/web-session-tabs-sync/tracking.ts index cb7b9f8aa1d..bda6f92389d 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync/tracking.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync/tracking.ts @@ -22,6 +22,7 @@ import { noteSessionTabsPublicationEpoch, recordReceivedWebSessionTabsEnvironmentFrame } from './publisher-identity-fences' +import { hostSnapshotAffirmsWorktreeContents } from '../host-session-snapshot-authority' export function isSessionTabsListAllResult(value: unknown): value is SessionTabsListAllResult { return ( @@ -110,9 +111,14 @@ export function recordReceivedWebSessionTabsSnapshot( if (isRetiredSessionTabsPublicationEpoch(key, publicationEpoch)) { return frame } - // A retraction withdraws the worktree; it does not take over publishing it. Noting it as current - // would retire the generation that is still live and fence its next frame out of its own worktree. - if (!isRetraction && (!history || history.current !== publicationEpoch)) { + // Neither a retraction nor a "nothing published yet" placeholder takes over publishing this + // worktree, so neither may be noted as current: doing so retires the generation that is still + // live and fences its next frame out of its own worktree. + if ( + !isRetraction && + hostSnapshotAffirmsWorktreeContents(snapshot) && + (!history || history.current !== publicationEpoch) + ) { noteSessionTabsPublicationEpoch(key, publicationEpoch) } // Stream delivery order is the freshest evidence even when a host's version @@ -126,12 +132,17 @@ export function recordReceivedWebSessionTabsSnapshot( snapshot.snapshotVersion > current.snapshotVersion || (snapshot.snapshotVersion === current.snapshotVersion && current.receivedFrame <= frame) ) { - setBoundedSessionTabsReceipt(latestReceivedSessionTabsSnapshotByWorktree, key, { - receivedFrame: frame, - publicationEpoch, - snapshotVersion: snapshot.snapshotVersion, - ...(runtimeId ? { runtimeId } : {}) - }) + setBoundedSessionTabsReceipt( + latestReceivedSessionTabsSnapshotByWorktree, + key, + { + receivedFrame: frame, + publicationEpoch, + snapshotVersion: snapshot.snapshotVersion, + ...(runtimeId ? { runtimeId } : {}) + }, + (entry) => entry.receivedFrame + ) if (isRetraction) { recordReceivedWebSessionTabsRemoval(environmentId, snapshot.worktree, frame, publicationEpoch) } @@ -158,15 +169,21 @@ export function recordReceivedWebSessionTabsRemoval( // live publisher's next frame outrank the pre-close one on version; the watermark is what the // slot cannot be, because a later frame overwrites the slot and the boundary has to outlive it. if (!latest || latest.receivedFrame <= receivedFrame) { - setBoundedSessionTabsReceipt(latestReceivedSessionTabsSnapshotByWorktree, key, { - receivedFrame, - publicationEpoch, - snapshotVersion: 0 - }) + setBoundedSessionTabsReceipt( + latestReceivedSessionTabsSnapshotByWorktree, + key, + { receivedFrame, publicationEpoch, snapshotVersion: 0 }, + (entry) => entry.receivedFrame + ) } const watermark = sessionTabsRemovalWatermarkByWorktree.get(key) ?? 0 if (receivedFrame > watermark) { - setBoundedSessionTabsReceipt(sessionTabsRemovalWatermarkByWorktree, key, receivedFrame) + setBoundedSessionTabsReceipt( + sessionTabsRemovalWatermarkByWorktree, + key, + receivedFrame, + (entry) => entry + ) } }