diff --git a/src/relay/pty-handler.test.ts b/src/relay/pty-handler.test.ts index 162372fc74d..7581e96211e 100644 --- a/src/relay/pty-handler.test.ts +++ b/src/relay/pty-handler.test.ts @@ -2067,19 +2067,17 @@ describe('PtyHandler', () => { expect(spawnOptions.env.ORCA_PANE_KEY).toBeUndefined() expect(spawnOptions.env.ORCA_TAB_ID).toBeUndefined() - // INVERTED: this used to require a differing pane identity to be REFUSED. That refusal is the - // defect — the paneKey and tabId are frozen at spawn, so it also refused a pane that had merely - // moved tabs, and it refused as "not found", which read as death and resumed the agent twice. - // The metadata is still recorded and still kept out of the shell env (asserted above); it just - // no longer gates an attach. A recycled id is rejected on the incarnation instead, which is - // pinned separately. + // A caller that names no incarnation is an older client, and pane identity is the only fence + // it can be given — so it still applies, and still refuses. A caller that CAN name the shell is + // fenced on that instead and attaches regardless of which tab it now lives in; that is pinned + // by "attaches a pane that has moved to another tab". await expect( attachPty({ id: 'pty-1', expectedPaneKey: 'tab-b:leaf-b', expectedTabId: 'tab-b' }) - ).resolves.toEqual({ incarnationId: spawn.incarnationId }) + ).rejects.toThrow(/identity mismatch/i) await expect( attachPty({ @@ -3313,17 +3311,15 @@ describe('PtyHandler', () => { expect(callArgs.env.ORCA_PANE_KEY).toBeUndefined() expect(callArgs.env.ORCA_TAB_ID).toBeUndefined() - // INVERTED, same reason as the spawn-side clause: the revived PTY still carries the pane - // identity it was serialized with, and that identity no longer gates an attach. What the - // clause still proves is that revive PRESERVES the metadata and keeps it out of the shell env - // (asserted above) — it just may not refuse a pane for living somewhere else now. + // Revive preserves the serialized pane identity, and for a caller that names no incarnation + // that identity is still the fence. await expect( dispatcher.callRequest('pty.attach', { id: 'pty-1', expectedPaneKey: 'tab-other:leaf', expectedTabId: 'tab-other' }) - ).resolves.toMatchObject({ incarnationId: expect.any(String) }) + ).rejects.toThrow(/identity mismatch/i) }) it('invokes the exit listener with the spawn-time paneKey', async () => { diff --git a/src/relay/pty-handler.ts b/src/relay/pty-handler.ts index b617f4e727a..7eb82ca464f 100644 --- a/src/relay/pty-handler.ts +++ b/src/relay/pty-handler.ts @@ -336,10 +336,16 @@ export type PtyExitListener = (event: { id: string; paneKey?: string }) => void type PtyIdentity = { paneKey?: string; tabId?: string } -/* No attach-time pane-identity comparison any more: it froze paneKey/tabId at spawn, so a moved - * pane was refused its own live shell — as "not found", which read as death. Recycled ids, its real - * purpose, are caught by the incarnation comparison in `attach` instead. `attachIdentity` is still - * recorded and round-trips through serialize/revive; it no longer gates an attach. */ +/** Fallback fence for a client too old to name a shell by incarnation. It froze paneKey/tabId at + * spawn, so it refuses a pane that merely moved tabs — which is why a client that CAN name the + * shell is fenced on that instead. Not deleted, because the relay is shared: an upgraded host + * would otherwise leave every not-yet-updated client attaching a recycled id unchecked. */ +function attachIdentityMismatches(expected: PtyIdentity, managed: PtyIdentity): boolean { + return Boolean( + (expected.paneKey && managed.paneKey && expected.paneKey !== managed.paneKey) || + (expected.tabId && managed.tabId && expected.tabId !== managed.tabId) + ) +} /** Returns env to merge into the PTY's spawn env. Receives spawn context so augmenters can derive per-PTY identity from paneKey. * `command` is the renderer-chosen agent launch command (`pi`, `omp`, …); undefined for CLI-launched bare shells. */ export type PtyEnvAugmenter = (ctx: { @@ -1674,12 +1680,21 @@ export class PtyHandler { ) } - // Why: a reset relay restarts its ids at pty-1, so an id alone can name somebody else's shell. - // The incarnation is the shell's OWN identity, so it discriminates a recycled id without - // caring where the pane lives. The pane identity this replaced froze the TAB at spawn, so a - // pane moved to another tab was refused — and refused as "not found", which read as death and - // resumed the agent a second time. Absent expectation stays permissive: an older client sends - // none, and refusing there would strand every pane it owns. + // A reset relay restarts ids at pty-1, so an id alone can name somebody else's shell. The + // incarnation is the shell's own identity, so it catches that without caring where the pane + // lives — unlike the pane identity below, which froze the tab at spawn and refused moved panes. + if ( + !expectedIncarnationId && + attachIdentityMismatches( + { + paneKey: typeof params.expectedPaneKey === 'string' ? params.expectedPaneKey : undefined, + tabId: typeof params.expectedTabId === 'string' ? params.expectedTabId : undefined + }, + managed.attachIdentity ?? { paneKey: managed.paneKey, tabId: managed.tabId } + ) + ) { + throw new Error(`PTY "${id}" identity mismatch`) + } if (expectedIncarnationId && expectedIncarnationId !== managed.incarnationId) { // Deliberately NOT worded "not found": that phrasing is what the client maps to an expired // session, and expiry authorizes a respawn onto a shell this branch just proved is alive.