From 070bbb4a487cd3e51fad0055ab59bb15a135cf19 Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 23:53:48 -0700 Subject: [PATCH] fix(test): repair the teardown suite the union broke Cherry-picked from 46ad377ceb9 with the relay half dropped: that commit also repaired relay-concurrency-policy-flip-mid-mint.test.ts, which does not exist on this PR and belongs with the relay cluster's own branch. The handle-gap half is what this PR needs. Neither break was visible on its own branch -- both only appear once the verdict rules compose. --- .../host-mirror-handle-gap-teardown.test.ts | 8 ++++++- .../src/lib/host-mirror-handle-gap-wait.ts | 22 +++++++++++++------ 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/renderer/src/lib/host-mirror-handle-gap-teardown.test.ts b/src/renderer/src/lib/host-mirror-handle-gap-teardown.test.ts index ce6a141f017..7076af927f3 100644 --- a/src/renderer/src/lib/host-mirror-handle-gap-teardown.test.ts +++ b/src/renderer/src/lib/host-mirror-handle-gap-teardown.test.ts @@ -26,9 +26,15 @@ const WORKTREE_ID = 'repo-1::/workspace/repo' const initialAppStoreState = useAppStore.getState() 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] ?? [] useAppStore.setState({ ptyIdsByTabId: {}, - tabsByWorktree: { [WORKTREE_ID]: [{ id: tabId, title: tabId }] } + tabsByWorktree: { + [WORKTREE_ID]: [...published.filter((tab) => tab.id !== tabId), { id: tabId, title: tabId }] + } } as never) parkUntilHostMirrorHandleLands(environmentId, WORKTREE_ID, tabId, () => {}) vi.advanceTimersByTime(HOST_MIRROR_HANDLE_GAP_DEADLINE_MS) diff --git a/src/renderer/src/lib/host-mirror-handle-gap-wait.ts b/src/renderer/src/lib/host-mirror-handle-gap-wait.ts index 7e9c75d3171..27be4efd91c 100644 --- a/src/renderer/src/lib/host-mirror-handle-gap-wait.ts +++ b/src/renderer/src/lib/host-mirror-handle-gap-wait.ts @@ -47,13 +47,21 @@ const waitersByPane = new Map() /** * Connection generation whose wait already expired for the pane. * - * Two rules drain it, and neither subsumes the other because both are driven by a recording: - * `recordExpiredWait` drops rows from a superseded generation, and a separate rule (8f166411306) - * drops rows whose tab is no longer published, both scoped to the environment doing the recording. - * An environment that is REMOVED records nothing ever again, so neither rule can reach it — hence - * the teardown clear below, which is the only thing that can. A stranded row is inert (removing an - * environment advances its connection generation, so it can never match again); this is a leak - * fix, not a correctness one. + * THREE drains, with three different triggers. Getting the scopes right is the whole design; see + * `recordExpiredWait` for why the first two must NOT share a scope. + * - superseded generation: per key, EVERY environment. Runs on any recording, anywhere. + * - dead tab row: the recording environment ONLY. Runs on a recording in that environment. + * - removed environment: `clearHostMirrorHandleGapVerdictsForEnvironment`, on teardown. The only + * trigger that fires at all for an environment that will never record again. A row stranded + * there is inert — removal advances the generation, so it can never match — so that one is a + * leak fix, not a correctness fix. + * + * ONE CLASS IS STILL UNCOVERED, and unlike the rest it is NOT conservative: a retracted tab id + * that is republished inherits the old pane's verdict and skips its own wait, which is the #19735 + * direction rather than a longer hold. No trigger above reaches it — the dead-row predicate stops + * matching once the id is live again, teardown is the wrong event, and a pane holding a verdict + * 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() let unsubscribeStore: (() => void) | null = null