mirror of
https://github.com/stablyai/orca.git
synced 2026-10-25 16:03:53 +00:00
fix(ssh): keep the pane fence for clients that cannot name a shell
Deleting the relay's pane-identity comparison disarmed the recycled-id guard for every client that has not upgraded. The relay is shared and host-side: one person updating a host would leave their colleagues attaching by id alone, with nothing checking it in either direction. It is back as a FALLBACK, used only when the caller sends no incarnation. A client that can name the shell is still fenced on that and still attaches after moving tabs; a client that cannot gets the older, coarser check it was already living with rather than none at all. The two clauses inverted when it was deleted are restored, because they send pane identity with no incarnation — the old-client shape, which should be refused. The moved-pane property they used to contradict is pinned separately by the clause that sends an incarnation, and reverting the override reddens it.
This commit is contained in:
@@ -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 () => {
|
||||
|
||||
+25
-10
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user