From c203d493d67f0ade27adee1ba4f05d245fc6877d Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 18:38:18 -0700 Subject: [PATCH] docs(runtime): the reused-tab-id class is closed at read time, not still open MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `ExpiredHandleGapVerdict` docstring told the next reader that a retracted tab id republished under the same id still inherits its predecessor's verdict, and that closing it "needs a fourth trigger, on row retraction". The test it names as its own pin says the opposite: class D in host-mirror-handle-gap-verdict-union.test.ts asserts the verdict does not answer, and explains it is closed at READ time rather than by any prune. Provenance, since two sources disagreeing is what made this expensive: the paragraph was last written in 46ad377ceb9 and the read-time pane-identity check landed one commit later in cdafc90d8f9 (adv2-skew). The prose predates its own fix by a single commit and was never updated. Confirmed by mutation rather than by reading: dropping `verdict.paneBinding === paneBindingFor(...)` fails exactly "handles all four orphan classes simultaneously", which is the class-D assertion, so the read-time check is what closes it. Rewritten to say what the code does, keeping the part that was always true — why no trigger could have reached that class — and keeping the distinction the new PUBLISHED HANDLE drain needs: read-time identity separates two panes behind one tab id, the drain separates two gaps on one pane. The drain does not close class D and must not be read as closing it. Also records why this block specifically keeps going stale: several agents change this map in parallel, the invariants move faster than the prose, and when the two disagree the test file is the one that ran. --- .../src/lib/host-mirror-handle-gap-wait.ts | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) 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 9ef857ec312..2e884fc43f0 100644 --- a/src/renderer/src/lib/host-mirror-handle-gap-wait.ts +++ b/src/renderer/src/lib/host-mirror-handle-gap-wait.ts @@ -67,12 +67,26 @@ const waitersByPane = new Map() * subscription to outlive the waiters, which is why `stopStoreSubscriptionIfIdle` counts * verdicts too. * - * 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. + * A FIFTH class is covered but NOT by any of those drains: a retracted tab id republished as a + * different pane, which would inherit the old pane's verdict and skip its own wait — the #19735 + * direction rather than a longer hold. No trigger can reach it, and the reason is worth keeping: + * 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 is there to observe the retraction. It is + * closed at READ time instead, by `hasHostMirrorHandleWaitExpired` comparing the verdict's + * park-time `paneBinding` — a pane that binds a newly minted PTY does not answer to a verdict + * about its predecessor. Pinned as class D in host-mirror-handle-gap-verdict-union.test.ts; do not + * delete that case. + * + * The PUBLISHED HANDLE drain does not close that class and must not be read as closing it: it + * needs the row to stay published throughout, and that class needs the row to go away. Read-time + * identity separates two panes behind one tab id; the drain separates two gaps on one pane. They + * look adjacent and are orthogonal — mutation kills them with disjoint tests. + * + * Why this comment block is worth re-reading against the code rather than trusting: the paragraph + * above it spent one commit asserting this class was still open and demanding a trigger that had + * just been replaced by the read-time check, while the test it named as its pin said the opposite. + * Several agents change this map in parallel and the invariants move faster than the prose, so + * when the two disagree the test file is the one that ran. */ type ExpiredHandleGapVerdict = { generation: number