From 46ad377ceb916e7ea6e30dff646cadc0c0de41a8 Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 17:58:46 -0700 Subject: [PATCH] fix(test): repair two suites the union broke, neither visible on its own branch Both were green on their own branch and failed only once the branches were combined. This is the interaction class the union exists to find. 1. relay-concurrency-policy-flip-mid-mint.test.ts stubbed `getRelayRevokeOutbox` with `pendingFor` alone. `hasDemand` now reads `demandingFor` (the revoke-demand expiry), so the stub threw INSIDE reconcile, the mint never settled, and the only symptom was a 30s test timeout with the real cause buried in stderr. A stub that implements part of an interface fails this way whenever the production caller moves to another method. 2. host-mirror-handle-gap-teardown.test.ts REPLACED `tabsByWorktree` on every park, so each pane unpublished the one before it. With the tab-death rule in the same loop those earlier verdicts were legitimately swept before teardown was reached, and the suite counted 2 where it expected 3. The fixture, not the rule, was wrong: this suite is about the class no recording-driven prune can reach, so its panes must all stay published. Rows now accumulate. --- ...y-concurrency-policy-flip-mid-mint.test.ts | 8 ++++++- .../host-mirror-handle-gap-teardown.test.ts | 8 ++++++- .../src/lib/host-mirror-handle-gap-wait.ts | 22 +++++++++++++------ 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/main/runtime/relay/relay-concurrency-policy-flip-mid-mint.test.ts b/src/main/runtime/relay/relay-concurrency-policy-flip-mid-mint.test.ts index 17760c23ecb..101381b55ae 100644 --- a/src/main/runtime/relay/relay-concurrency-policy-flip-mid-mint.test.ts +++ b/src/main/runtime/relay/relay-concurrency-policy-flip-mid-mint.test.ts @@ -73,7 +73,13 @@ function service(mode: { current: MobilePairingConnectionMode }): DesktopRelaySe publicKeyB64: 'x' }), getMobileSocketWiring: () => ({ attachTransport: () => () => {} }), - getRelayRevokeOutbox: () => ({ pendingFor: () => [], remove: vi.fn() }), + // `demandingFor` is what hasDemand reads; a stub missing it throws inside reconcile and the + // mint never settles, which surfaces only as a test timeout. + getRelayRevokeOutbox: () => ({ + pendingFor: () => [], + demandingFor: () => [], + remove: vi.fn() + }), getDeviceRegistry: () => ({ listDevices: () => [], getDevice: () => ({ deviceId: 'device-1', scope: 'mobile' }), 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