docs(ssh): record the three root-cause routes that do not work

I went after the cause and failed three times. Each attempt looks correct until
it runs, so the dead ends are worth more written down than the time they cost:

RETIRING THE DELIVERY ON onClientDetached — the obvious fix, and the one I
argued for, since fs-handler, git-handler and the watch registry all release
their per-client state exactly there. It breaks checkpoint recovery: 10 tests
across relay-pty-source-recovery-interleavings and restore-retry. A delivery
outliving its client is DELIBERATE; that is what lets a reconnecting client
resume from a checkpoint instead of re-receiving everything. This class omits
the subscription on purpose, and that omission is not the bug.

RETIRING WITHOUT session.cancelDelivery() — the credit ledger keeps one upstream
owner per pty, so dropping the record without releasing it leaves the slot
taken and the next open throws 'PTY source delivery already has an upstream
owner'. I saw that live as an error toast over a blank pane.

COMPARING clientGeneration — the delivery identity carries one, but it is
client-supplied through pty.openClient and RequestContext has none to compare
against, so the relay cannot tell the generations apart on its own.

Which points where I would start next, unverified: the SSH client presents the
SAME clientGeneration across a reconnect, so the relay cannot distinguish the new
connection and reuses its delivery. reattachSshPtySession never sends
sourceRecovery at all — the recovery protocol exists and the SSH reattach path
simply does not participate in it. That is likely the real fix, and it is on the
client, not in the relay.

The symptom fix stays because it is verified and the tree is green; the cause
stays open with a map instead of a guess.
This commit is contained in:
Neil
2026-08-15 02:22:15 -07:00
parent 506807c87a
commit 09e1d8c189
@@ -33,16 +33,27 @@ const RUN_DOCKER_SSH = process.env.ORCA_E2E_SSH_DOCKER === '1'
* Reading the pane's own text is the point. Asserting a pty id, a status, or a spy call is what let
* both of these through: every one of those was correct while the screen was wrong.
*
* KNOWN: (1) is fixed at the symptom, not the cause. The cause is that a PTY source delivery is the
* only per-client relay state that survives its client detaching — fs-handler, git-handler and
* relay-filesystem-watch-registry all subscribe to dispatcher.onClientDetached and release theirs;
* relay-pty-source-publication never does. The primary client keeps its id across a transport
* replacement (detachClient refuses to detach it, setWrite revives that id), so its delivery
* outlives the dead transport and activate() answers 'existing' for a client that can no longer
* receive anything. requireReplay makes the client ask for what the relay wrongly decided it did
* not need. Retiring the delivery on detach is the real fix and is deliberately NOT done here: it
* is the flow-control and credit path, and it was not worth shipping unverified. This test pins the
* user-visible contract either way, so it should keep passing when that lands.
* KNOWN: (1) is fixed at the symptom. requireReplay makes the client ask for what the relay wrongly
* decided it did not need. Three attempts at the cause failed, and the reasons are worth keeping
* because each looks correct until you run it:
*
* - RETIRING THE DELIVERY ON dispatcher.onClientDetached, the way fs-handler, git-handler and
* relay-filesystem-watch-registry release their per-client state, BREAKS CHECKPOINT RECOVERY (10
* tests in relay-pty-source-recovery-interleavings / restore-retry). A delivery outliving its
* client is deliberate here: it is what lets a reconnecting client resume from a checkpoint
* instead of re-receiving everything. This class omits that subscription on purpose.
* - RETIRING WITHOUT session.cancelDelivery() orphans the credit ledger's one-upstream-owner-per-pty
* slot, and the next open throws "PTY source delivery already has an upstream owner". Seen live as
* an error toast and a blank pane.
* - COMPARING record.identity.clientGeneration TO THE REQUEST is not available: that value is
* client-supplied through pty.openClient (dispatcher.ts:1219) and RequestContext carries no
* generation of its own.
*
* Which points at the likeliest real cause, unverified: the SSH client presents the SAME
* clientGeneration across a reconnect, so the relay cannot tell a new connection from the old one
* and reuses its delivery. The fix is probably that reattachSshPtySession participates in the
* recovery protocol at all — it never sends sourceRecovery — rather than anything in the relay.
* Start there, not at onClientDetached.
*/
async function openTerminalTab(page: Page): Promise<void> {
await page.evaluate(async () => {