starred/orca
1
0
Fork 0
mirror of https://github.com/stablyai/orca.git synced 2026-10-22 16:03:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(ssh): harden PTY reattach reliability

Browse Source
This commit is contained in:
Neil
2026-08-12 20:23:51 -07:00
parent 43166a96f5
commit 0ba2bed7ee
29 changed files with 1508 additions and 246 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/reference/terminal-session-correctness-handoff-2026-08-07/design-brief-for-review.md
+28 -29
View File
@@ -10,12 +10,12 @@ Review artifact. Repo: Orca (Electron + renderer + relay). Everything below mark
Orca runs terminals on four "routes":
| Route | Provider | Survives app restart |
| --- | --- | --- |
| local in-process | `LocalPtyProvider` | no |
| local daemon | `DaemonPtyAdapter` (swapped into the same slot) | yes |
| direct SSH | `SshPtyProvider` → remote relay | yes, within a grace window |
| paired remote runtime | not an `IPtyProvider`; runtime RPC | yes |
| Route | Provider | Survives app restart |
| --------------------- | ----------------------------------------------- | -------------------------- |
| local in-process | `LocalPtyProvider` | no |
| local daemon | `DaemonPtyAdapter` (swapped into the same slot) | yes |
| direct SSH | `SshPtyProvider` → remote relay | yes, within a grace window |
| paired remote runtime | not an `IPtyProvider`; runtime RPC | yes |
Reported failures:
@@ -30,12 +30,14 @@ Reported failures:
**[V] Dispatch is a substitution for daemon, a parallel registry for SSH.**
`src/main/ipc/pty.ts:932-941`:
```ts
function getProvider(connectionId) {
if (!connectionId) return localProvider // local AND daemon — one slot
const provider = sshProviders.get(connectionId) // SSH — separate map
if (!connectionId) return localProvider // local AND daemon — one slot
const provider = sshProviders.get(connectionId) // SSH — separate map
}
```
The daemon is installed by `setLocalPtyProvider(routedAdapter)`
(`src/main/daemon/daemon-init.ts:1031`) and adds **zero** branches to the shared
spine. The paired runtime is encapsulated behind a `PtyTransport`. SSH is neither —
@@ -47,9 +49,11 @@ The `remote:` predicate is re-declared 5 times in the renderer.
**[V] The transport dropped the half that makes it persistent.**
`src/main/ssh/relay-protocol.ts:34`:
```ts
export const MessageType = { Regular: 1, KeepAlive: 9 } as const
```
The 13-byte frame header was copied from a well-known persistent protocol, but
only 2 of its 9 message types were kept. Dropped: `Control`, `Ack`, `Disconnect`,
`ReplayRequest`, `Pause`, `Resume`, `None`. The surviving ACK field feeds only
@@ -64,23 +68,23 @@ ledger, daemon batcher — the last deliberately lossy).
**[V] Three durable records exist, plus a fourth copy on the host.**
| | key | pane identity is |
| --- | --- | --- |
| PTY binding (`WorkspaceSessionState`) | `(hostId, tabId, leafId)` | **the key** |
| `SshRemotePtyLease` | `(targetId, relayPtyId)` | **optional payload** |
| `SshPtyConsumerRecovery` | `targetId` | n/a (per-target resume token) |
| relay `attachIdentity` | per PTY, in-memory | compare-only |
| | key | pane identity is |
| ------------------------------------- | ------------------------- | ----------------------------- |
| PTY binding (`WorkspaceSessionState`) | `(hostId, tabId, leafId)` | **the key** |
| `SshRemotePtyLease` | `(targetId, relayPtyId)` | **optional payload** |
| `SshPtyConsumerRecovery` | `targetId` | n/a (per-target resume token) |
| relay `attachIdentity` | per PTY, in-memory | compare-only |
**[R] ~374 lines of pure lease↔binding reconciliation in `persistence.ts`**, and 9
enumerated states where the records can disagree. Example of an independent latent
bug: absence of a lease means "restorable" in `isRestorablePtyBinding` but "not
restorable" in `hasRestorableSshRemotePtyLease` — so whether a binding survives
depends on whether a *sibling leaf* had a binding.
depends on whether a _sibling leaf_ had a binding.
**[V] The relay holds nothing durably.** Its PTY table is
`private ptys = new Map<string, ManagedPty>()` (`src/relay/pty-handler.ts:356`).
No relay-side persistence of PTY state exists. `pty.serialize`/`revive` hand state
to the *client* and take it back; `revive` requires the original PIDs to still be
to the _client_ and take it back; `revive` requires the original PIDs to still be
alive. On PTY exit the entry is deleted with no tombstone.
**[V] Pane identity is write-only over the wire.** `listProcesses` and `attach`
@@ -120,7 +124,7 @@ implementation parameterised by unit and limit; one chunk-and-ack primitive; one
lane vocabulary; binary PTY payloads reusing the binary terminal frame Orca already
ships to mobile; prebuilt native binaries so first connect stops needing a compiler;
then teach the relay to listen on a loopback WebSocket, forward that port, and let
the *same* client serve the SSH route — after which SSH is "launch + access" only.
the _same_ client serve the SSH route — after which SSH is "launch + access" only.
### Plane 2 — control plane (proposed here; the reviewed contribution)
@@ -131,7 +135,7 @@ The lease answers two different questions with one record keyed for neither:
Proposal: split by question.
- **Ownership record**, keyed `(host, pane)` — the *same* key local already uses.
- **Ownership record**, keyed `(host, pane)` — the _same_ key local already uses.
Transport-specific payload (lease state + timestamps for SSH; nothing extra for
local). Makes "one pane, one live shell" structural rather than enforced.
- **Orphan inventory**, keyed `(host, ptyId)` — live shells with no owner. Feeds the
@@ -140,19 +144,14 @@ Proposal: split by question.
- Offline consumers keep reading local disk synchronously, unchanged.
Claimed payoff: the ~209 lines of supersession/ranking/rollback/dual-matchers exist
*only* to bolt a pane-keyed constraint onto a pty-keyed record, and become
_only_ to bolt a pane-keyed constraint onto a pty-keyed record, and become
unnecessary.
## 5. Known open defect in the shipped work
## 5. Historical defect, now resolved
A "superseded PTY" fence was added to `pty:write`/`writeAccepted`/`resize`/`signal`.
**[V]** Its maps (`ptyPaneKey`, `paneKeyPtyId`) have exactly two writers, both spawn
paths (`pty.ts:5222`, `pty.ts:6668`). The SSH relay reattach path never populates
them (it calls `runtime.registerPty` instead), so `isSupersededPtyId` returns
`false` ("unknown, not stale") for every relay-reattached PTY after restart. The
fence is **inert on the path it was built for**. Its test pins the *consumer* call
sites and mirrors the predicate structurally, so it cannot detect that the
*producer* never runs on that route.
This defect was resolved by the shared `bindPaneShell` producer used by relay reattach and both
spawn handlers. Its mutation proof is recorded in
[S5](./new-design-goalposts.md#s5--the-superseded-pane-fence-is-live-on-the-reattach-path--proven).
## 6. Hard constraints
@@ -175,7 +174,7 @@ sites and mirrors the predicate structurally, so it cannot detect that the
disconnected; relay restart; host reboot; PTY exit during disconnect; two panes
racing for one shell; migration from existing on-disk state; a pane deleted while
its shell lives; the same worktree open in two windows.
2. **Simplicity.** Is there a *simpler* design with the same correctness? Simpler
2. **Simplicity.** Is there a _simpler_ design with the same correctness? Simpler
means fewer code paths and fewer concepts, not merely fewer lines. Is the
two-record split actually necessary, or is there a single-record formulation that
survives the section-3 refutations?
docs/reference/terminal-session-correctness-handoff-2026-08-07/design-final-detail.md
+53 -33
View File
@@ -7,34 +7,38 @@ Problem (from the brief): SSH reconnect grows pane cardinality (2 -> 19 -> 20, S
One rule, applied without exception:
Ownership ("which shell does this pane own?") is DURABLE, CLIENT-LOCAL, and read
synchronously from disk while offline. Evidence about a shell's FATE is
NON-DURABLE, HOST-LOCAL, and must be REPORTED by the host, never INFERRED by the
client from timing, from the absence of an entry, or from the shape of an error
string.
Ownership ("which shell does this pane own?") is DURABLE, CLIENT-LOCAL, and read
synchronously from disk while offline. Evidence about a shell's FATE is
NON-DURABLE, HOST-LOCAL, and must be REPORTED by the host, never INFERRED by the
client from timing, from the absence of an entry, or from the shape of an error
string.
Every defect found across three review rounds is one violation of the second clause. The design's job is to remove all of them, not to add an authority layer.
SCOPE DECLARATION (grok, low): the record is host-agnostic (`hostId` is a field). The *death rule*'s row 2 is relay-specific. Local and daemon panes reach `terminated` only through row 1 (an exit observed in-process, which is how they already work) and never through row 2. That is stated rather than left implicit.
SCOPE DECLARATION (grok, low): the record is host-agnostic (`hostId` is a field). The _death rule_'s row 2 is relay-specific. Local and daemon panes reach `terminated` only through row 1 (an exit observed in-process, which is how they already work) and never through row 2. That is stated rather than left implicit.
=====================================================================
D1. THE DURABLE RECORD
=====================================================================
```ts
/** Main-owned. TOP-LEVEL in PersistedState — never inside WorkspaceSessionState. */
type PaneShellOwnership = {
leafId: string // PRIMARY KEY. Terminal-layout leaf UUID.
leafId: string // PRIMARY KEY. Terminal-layout leaf UUID.
hostId: ExecutionHostId // FIELD: 'local' | 'ssh:<target>' | 'runtime:<id>'
worktreeId: string // FIELD. Opaque FULL string incl. any `::workspace:<uuid>` suffix.
ptyId: string // RELAY-NATIVE form for SSH (toRelaySshPtyId). One namespace, always.
incarnationId: string // REQUIRED. Relay mints one per pty (relay/pty-handler.ts:1525).
relayInstanceId?: string// The relay PROCESS that owned it, as reported at bind time.
worktreeId: string // FIELD. Opaque FULL string incl. any `::workspace:<uuid>` suffix.
ptyId: string // RELAY-NATIVE form for SSH (toRelaySshPtyId). One namespace, always.
incarnationId: string // REQUIRED. Relay mints one per pty (relay/pty-handler.ts:1525).
relayInstanceId?: string // The relay PROCESS that owned it, as reported at bind time.
state: 'attached' | 'detached' | 'terminated'
createdAt: number; updatedAt: number
lastAttachedAt?: number; lastDetachedAt?: number
createdAt: number
updatedAt: number
lastAttachedAt?: number
lastDetachedAt?: number
}
// Storage: PersistedState.paneShellOwnershipByLeafId: Record<string, PaneShellOwnership>
```
Note what is NOT here: no `tabId`, no `paneKey`, no `hostEpoch`, no `resumed`, no second (pty-keyed) record.
WHY `leafId` AND NOT `paneKey`. `makePaneKey(tabId, leafId)` returns `${tabId}:${leafId}` and the repo states at src/shared/stable-pane-id.ts:47-49 that "only the leaf UUID is remint-stable pane identity (the tab half changes on pane break-out)" — which is why `isEquivalentPaneKey` exists at :50-57. The mutation is a shipping gesture: `detachTerminalPaneToTab` (src/renderer/src/components/terminal-pane/terminal-pane-tab-detach.ts:245-310) moves a LIVE pane and its ptyId into a new tab via `createTab(..., initialPtyId)` while explicitly not killing the PTY (:275-276). `leafId` is a UUID and is never remapped once it is one (src/main/persistence.ts:1858-1872 skips ids already passing `isTerminalLeafId`). Keying by the leaf deletes the transfer problem instead of managing it.
@@ -46,11 +50,12 @@ WHY TOP-LEVEL. `session:set` / `session:set-sync` (src/main/ipc/session.ts:13, :
UNIQUENESS. At most one non-terminal record may name a given `(hostId, ptyId, incarnationId)`, enforced on write through a reverse index. Not ptyId alone: leases store the relay-native id (`getRelayPtyIdForSshLeaseStorage` -> `toRelaySshPtyId` strips the connection scope, src/main/persistence.ts:6641-6643, src/shared/ssh-pty-id.ts:58-67) and relay ids are `pty-${this.nextId++}` with `nextId = 1` per relay PROCESS (src/relay/pty-handler.ts:356-357, :1440-1443), so `pty-1` recurs on every host after every relay restart. Round 1's rejection ("a cross-target collision on ptyId is impossible") is withdrawn; it is guaranteed, not impossible.
=====================================================================
D2. DELETE PANE IDENTITY FROM THE ATTACH PATH (the largest round-3 change)
D2. DELETE PANE IDENTITY FROM THE ATTACH PATH (the largest round-3 change)
=====================================================================
Both reviewers independently attacked the attach-time identity comparison. Verified, and it is worse than either stated: the apparatus is simultaneously over-strict and under-strict, and it is a live duplicate-agent bug today.
VERIFIED FACTS
- The relay compares frozen strings: `attachIdentityMismatches` (src/relay/pty-handler.ts:334-339) does full-string `expected.paneKey !== managed.paneKey || expected.tabId !== managed.tabId`; `attachIdentity` is captured once at spawn from `ORCA_PANE_KEY`/`ORCA_TAB_ID` (:1512-1517, :1446). On mismatch it throws `PTY "<id>" not found (identity mismatch)` (:1616-1626).
- `isSshPtyNotFoundError` is a message regex `/PTY ".+" not found/i` (src/main/providers/ssh-pty-errors.ts:12-15), so it is TRUE for the mismatch string. `reattachSshPtySession` re-emits it as `SSH_SESSION_EXPIRED: <id> SSH_PTY_IDENTITY_MISMATCH` (src/main/providers/ssh-pty-session-reattach.ts:218-226).
- The renderer's `isProvenSshSessionGoneError` returns TRUE for anything containing `SSH_SESSION_EXPIRED` and for the raw not-found regex, and IGNORES the mismatch marker (src/renderer/src/components/terminal-pane/reattach-failure-classification.ts:24-30). Its two call sites then clear the binding and call `startFreshColdRestoreAgentResume` (pty-connection.ts:8839-8847, :9086-9097) — a second shell and a SECOND AGENT RESUME while the original shell keeps running. That is RC2, on a live process, reachable from a shipped gesture.
@@ -58,6 +63,7 @@ VERIFIED FACTS
- The apparatus does not even do its own stated job ("generation resets can reuse PTY IDs; reject conflicting identities", pty-handler.ts:1615): after a relay restart that recycles `pty-3` for the SAME pane, paneKey and tabId both match and a different shell is accepted.
RESOLUTION — delete the comparison; do not replace it with another comparison.
1. Relay: delete `attachIdentity`, `attachIdentityMismatches`, the `params.expectedPaneKey`/`expectedTabId` handling and the `(identity mismatch)` throw. `ManagedPty.paneKey`/`tabId` STAY — they are documented as separate from attach identity (pty-handler.ts:139-143) and are used by the exit listener (:765) and, newly, by listProcesses reporting (D4). paneKey stops being an authorization INPUT and becomes a reported HINT.
2. Main: delete `ExpectedPtyIdentity`, `expectedIdentityForLease`, `expectedIdentityByPtyId`, the `expectedIdentity` parameter threaded through `attachPtyWithRetry`, `isSshPtyIdentityMismatchError`, `SSH_PTY_IDENTITY_MISMATCH_ERROR`, the marker append, and the mismatch branch at ssh-relay-session.ts:2628-2634.
3. Renderer: `isProvenSshSessionGoneError` keeps only proofs. (After step 1/2 the mismatch string ceases to exist; the renderer fix ships FIRST and independently, because it must protect users running against relays that have not been redeployed.)
@@ -73,18 +79,19 @@ ONE IDENTITY RULE, EVERYWHERE: the leaf is identity; the tab is derived location
D3. THE DEATH RULE — two rows, one new field
=====================================================================
`resumed` and `hostEpoch` are deleted and must not be built.
- `resumed: replaces !== null` (src/shared/pty-consumer-session.ts:275); `expireOwner()` nulls the incumbent after `PTY_CONSUMER_OWNER_GRACE_MS = 30_000` (:281-287, contract :2), after which admission takes `if (!current) return this.newOwner(...)` (:215-216) and `resumed` is false regardless of proof. Unreachable for quit, sleep, overnight, reboot — the normal case. Its own contract comment says it means "the client's checkpoints for the previous claim no longer apply" (pty-consumer-session-contract.ts:49-52): a delivery-checkpoint flag, not liveness.
- `hostEpoch = Date.now() - os.uptime()*1000` under exact equality is a reconstructed estimate; second-granularity uptime, NTP steps and suspend/resume make it differ on an unrebooted host, and "differs" was the grant-YES row — it fails OPEN toward respawn. Repo precedent settles it: `readCurrentDaemonReadyIdentity` reads `/proc/sys/kernel/random/boot_id` on Linux only and returns bare `{ startedAtMs }` elsewhere (src/main/daemon/daemon-ready-identity.ts:11-13).
THE ONE NEW GRANT FIELD: `relayInstanceId?: string` on `PtyConsumerSessionGrant` (src/shared/pty-consumer-session-contract.ts:43-59) — a `randomUUID()` minted once at relay process start (`randomUUID` is already imported at relay/pty-handler.ts:6). It is genuinely new information: the grant's existing `serverBuildId` is `launchVersion` (src/relay/relay.ts:694-696), a build string that is identical across restarts of the same build.
| Condition | Transition | Respawn grant |
| --- | --- | --- |
| PTY exit observed on a live attached stream | `terminated` | yes |
| Condition | Transition | Respawn grant |
| ---------------------------------------------------------------------------------- | ------------ | -------------------- |
| PTY exit observed on a live attached stream | `terminated` | yes |
| attach returns not-found AND the grant's `relayInstanceId` EQUALS the recorded one | `terminated` | yes (subject to E-1) |
| attach returns an incarnation different from the recorded one | `detached` | no |
| `relayInstanceId` absent on either side, or differing | `detached` | no |
| any other error (today's `restoreRequired` path, ssh-relay-session.ts:2612-2620) | `detached` | no |
| attach returns an incarnation different from the recorded one | `detached` | no |
| `relayInstanceId` absent on either side, or differing | `detached` | no |
| any other error (today's `restoreRequired` path, ssh-relay-session.ts:2612-2620) | `detached` | no |
Row 2 is sound and UNBOUNDED IN TIME: `this.ptys` is the relay process's only store (pty-handler.ts:356) and entries are deleted on exit, so "the same process that minted this pty no longer has it" means it exited. It is sound on Windows too, because the relay OBSERVED the exit — unlike any rule that reasons from a relay restart. A restarted relay reports a different id and we correctly claim no knowledge.
@@ -95,10 +102,12 @@ STOP FABRICATING THE EXIT. `handlePtyReattachFailure` currently sends `pty:exit
=====================================================================
D4. ORPHANS — a connect-time projection, never an authority
=====================================================================
```
orphans(host) = successful listProcesses(host)
MINUS { (ptyId, incarnationId) named by a non-terminal record on this host }
```
Computed at connect/reattach, held in memory for the connection's life, discarded on disconnect. No durability, no migration, no rollback.
WHAT `listProcesses` IS: it iterates only `this.ptys` (pty-handler.ts:1894-1911) — the current relay process's Map. No durability, no OS scan. It is therefore (i) the cleanup-UI source and (ii) a SUPPLEMENT to the reattach work list, never the sole discovery authority. The work list stays `getPtyIdsForConnection(target) UNION` the non-terminal records (today's `leasedPtyIds` shape, ssh-relay-session.ts:2211-2219).
@@ -116,6 +125,7 @@ OLD-RELAY DEGRADATION: a relay that omits `paneKey` yields unclassified orphans
=====================================================================
D5. RECORD LIFETIME — three exits, none of them "absent from a list"
=====================================================================
1. RETIRE (delete). The record reached `terminated` (row 1 or row 2 of D3, or explicit user close via `retireExitedPty`, ssh-relay-session.ts:2153-2172). Terminal records are garbage-collected after a bounded retention (long enough to serve any surviving grant window) so they cannot mask a recycled `pty-1`. There is NO retire-on-absence clause: the case it was written for (shell exited during a disconnect, relay still up) is already covered by row 2 — the attach itself proves the exit — so deleting the clause costs nothing and removes a direct contradiction with #8459.
2. RELEASE (keep the shell, drop the claim). Fired ONLY from the explicit pane-close and tab-delete code paths — never from "this publish did not contain the leaf". Detach is not atomic across layouts: `createTab` installs an empty layout, then `setTabLayout(sourceTabId, ...)` removes the leaf, then `setTabLayout(tab.id, ...)` adds it (terminal-pane-tab-detach.ts:298-300), so between the last two no layout contains the leafId, and any `session:set` flush in that window would look like a durable removal. Released records are cleared to a non-claiming state: the shell becomes orphan-eligible (visible, adoptable) and is deliberately NOT `terminated`, so it can never authorize a respawn.
3. EXPLICIT USER CLOSE -> `terminated`, and the record dies with the shell.
@@ -125,15 +135,21 @@ RESIDUAL, STATED HONESTLY: a shell that died while the relay ALSO restarted leav
PRODUCT AFFORDANCE, REQUIRED IN THE SAME PR. A `detached` pane renders as disconnected with two explicit actions — "reattach (retry)" and "start a new shell" (the latter retires the record). Nothing infers death, nothing auto-spawns. Per AGENTS.md this must follow docs/STYLEGUIDE.md. COPY CONSTRAINT: the UI must never assert that a shell is dead. The claim "after a relay crash the shells are almost certainly dead" is REMOVED from this design: it rests on POSIX master-fd close SIGHUPing the foreground group, and on Windows the relay is deliberately launched through WMI because "Windows sshd kills the exec channel's process tree on close" (src/main/ssh/ssh-relay-deploy.ts:1805), with ConPTY and no SIGHUP. No rule in this design concludes anything from a relay restart, so the design is correct either way; only the copy had to change.
=====================================================================
D6. PRECONDITION P — one partition per (target, pane). BLOCKS everything.
D6. HISTORICAL — one partition per (target, pane). SUPERSEDED.
=====================================================================
This migration proposal was rejected. The shipped containment keeps the existing persistence
planes and gives the pane binding one local home; see [S4](./new-design-goalposts.md#s4--one-partition-per-target-pane--proven).
SSH pane bindings live in two partitions today. Readers and writers that consult ONLY `ssh:<target>`: `resolvePersistedStablePaneOwner` (src/main/ipc/pty.ts:676-677), `retirePersistedStablePaneOwner` (:760-761, :783), the CAS write `persistPtyBinding(..., expectedBinding)` (:833), and the two spawn upserts (:5108, :6493). Against that, the relay reattach write has no hostId (ssh-relay-session.ts:2500-2511 -> `resolveHostId(undefined)` -> `LOCAL_EXECUTION_HOST_ID`, persistence.ts:6243-6246) and the renderer keeps SSH worktrees in `local` deliberately (workspace-session-host-persistence.ts:167-173). `durablyBoundPtyIdForPane` hedges ssh-first-then-local (persistence.ts:7269-7277), so `supersedeSiblingLeasesForPane` early-returns when the bound pty differs (:7229-7232) — supersession silently no-ops and both leases stay live. That is the STA-3077 mechanism itself.
FIX: one accessor `sessionForSshPaneBindings(connectionId): WorkspaceSessionState` returning the `local` partition, routed through by EVERY reader and EVERY writer above, flipped in a single atomic PR — not five write-site edits. `local` wins because the only publisher of pane membership writes there and `mayCreate:false` is evaluated there. A one-time fold moves `ssh:<target>.terminalLayoutsByTabId[*].ptyIdsByLeafId` and `tabsByWorktree[*].ptyId` into `local`, preferring `local` on conflict; the loser is NOT demoted to orphan-candidate by ptyId alone (recycled ids collide) — it carries `incarnationId` or is dropped.
=====================================================================
D7. PRECONDITION F — one bind producer, so the shipped fence has one
D7. HISTORICAL — one bind producer. RESOLVED.
=====================================================================
The shared `bindPaneShell` producer now serves relay reattach and both spawn handlers; see
[S5](./new-design-goalposts.md#s5--the-superseded-pane-fence-is-live-on-the-reattach-path--proven).
`rememberPaneKeyForPty` (ipc/pty.ts:517-525) has two callers, both spawn (:5222, :6668). `restoreReattachedPtyRuntime` calls `runtime.registerPty` instead (ssh-relay-session.ts:2523), and `isSupersededPtyId` returns `false` for an unrecorded id by design (:284-296) — so the shipped superseded-PTY fence is INERT on the path it was built for. Collapse to one `bindPaneShell({ hostId, worktreeId, leafId, ptyId, incarnationId, relayInstanceId })` that, in one call, (a) resolves the current tab containing the leaf (refusing if none), (b) writes the durable record, (c) writes the in-memory `ptyPaneKey`/`paneKeyPtyId` fence maps using the paneKey composed from the CURRENT tab, (d) calls `registerPty`. All three paths call it and nothing else.
=====================================================================
@@ -148,6 +164,7 @@ OFFLINE / SYNCHRONOUS ACCESS PATH (refutation B, honored exactly). `setLocalWork
=====================================================================
D9. THE REATTACH ALGORITHM (SSH, on connect)
=====================================================================
1. Obtain the consumer owner grant. Read `grant.relayInstanceId` (may be absent — old relay).
2. Work list = `getPtyIdsForConnection(target)` UNION `{ r.ptyId | r.hostId === 'ssh:'+target and r.state !== 'terminated' }`, every id normalized through `toRelaySshPtyId`.
3. PROCESS GATE. For each record: if both `grant.relayInstanceId` and `r.relayInstanceId` are present and differ, do not attach by that id. Set `detached`. Its shell, if any, is rediscoverable only through step 5.
@@ -162,26 +179,28 @@ D10. MIGRATION, SEQUENCING, ROLLBACK
=====================================================================
ORDER — each its own PR, each independently revertible:
| | Step | Contents |
| --- | --- | --- |
| A | Identity-mismatch is not death | Renderer: `isProvenSshSessionGoneError` stops treating the mismatch marker as proof. Relay+main: delete the whole attach-identity apparatus (D2). Pure bug fix, no design dependency, ships FIRST; the renderer half must ship even before relays are redeployed. |
| E-0 | Stop lying about exits | Collapse the plain-not-found branch of `handlePtyReattachFailure` into the existing non-destructive `restoreRequired` branch (D3). Ships with, or immediately followed by, the D5 disconnected-pane affordance. |
| P | Single partition | D6: one accessor for every reader and writer, plus the fold migration, atomically. |
| F | Single bind producer | D7: `bindPaneShell`; makes the shipped fence live on the reattach path; introduces tab-from-layout resolution. |
| E-1 | Prove the grant executes | Production-id-shape oracle for `recoverTerminalPane`; record the actual behavior. |
| E-2 | Death rule | `relayInstanceId` on the grant + the D3 table. Conditional on E-1. |
| W | Projection | `paneKey` through relay -> `PtyProcessInfo` -> admission allowlist; incarnation-scoped subtraction; explicit-only kill (D4). |
| K | Re-key | The leaf-keyed record + D8 + D5 lifetime + `reassignSshTargetId` field rewrite + porting all lease readers. |
| | Step | Contents |
| --- | ------------------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| A | Identity-mismatch is not death | Renderer: `isProvenSshSessionGoneError` stops treating the mismatch marker as proof. Relay+main: delete the whole attach-identity apparatus (D2). Pure bug fix, no design dependency, ships FIRST; the renderer half must ship even before relays are redeployed. |
| E-0 | Stop lying about exits | Collapse the plain-not-found branch of `handlePtyReattachFailure` into the existing non-destructive `restoreRequired` branch (D3). Ships with, or immediately followed by, the D5 disconnected-pane affordance. |
| P | Single partition | D6: one accessor for every reader and writer, plus the fold migration, atomically. |
| F | Single bind producer | D7: `bindPaneShell`; makes the shipped fence live on the reattach path; introduces tab-from-layout resolution. |
| E-1 | Prove the grant executes | Production-id-shape oracle for `recoverTerminalPane`; record the actual behavior. |
| E-2 | Death rule | `relayInstanceId` on the grant + the D3 table. Conditional on E-1. |
| W | Projection | `paneKey` through relay -> `PtyProcessInfo` -> admission allowlist; incarnation-scoped subtraction; explicit-only kill (D4). |
| K | Re-key | The leaf-keyed record + D8 + D5 lifetime + `reassignSshTargetId` field rewrite + porting all lease readers. |
E-1 CONTEXT (why E-2 is conditional). The chain `not-found -> expired -> 30s grant -> createTerminal` has one spawn-authorizing caller, `recoverTerminalPane` (src/main/runtime/orca-runtime.ts:16449-16490), gated on `getRecentExpiredSshLease(...)`, whose comparison is raw `lease.ptyId === ptyId` with no normalization (:6310). `lease.ptyId` is relay-native (persistence.ts:7184) while the graph's `pty.ptyId` is app-form `ssh:<conn>@@pty-N` (ssh-relay-session.ts:2523), so for a real SSH pane they can never be equal, and the branch also cannot be reached by a local pane (it requires an SSH lease). The covering test seeds both sides as `'pty-expired'` and registers with a NULL connectionId (orca-runtime.test.ts:1350-1370, :2932-2940) — a shape production cannot produce. Do not build grant-arbitration machinery before E-1 shows the branch executing.
FORWARD MIGRATION AT K, per `SshRemotePtyLease`:
- has `leafId` -> one record. `state`, `createdAt`, `updatedAt` copied VERBATIM (any surviving grace window reads `updatedAt`). `worktreeId` copied as an OPAQUE FULL STRING — the `::workspace:<uuid>` folder-workspace suffix must survive, since matching is full-string equality (persistence.ts:7239, orca-runtime.ts:6308) and issue #12474 was a comparator that stripped exactly that. `ptyId` stays relay-native. `tabId` is DROPPED (D1). `relayInstanceId` absent -> the record falls to `detached` on first reconnect with no ambiguity to resolve. `incarnationId` absent on a legacy row -> treat as adoption-eligible rather than as a subtracting claim, so it cannot mask a recycled id.
- no `leafId` -> no record; recovered at first reconnect by the leaf-half `paneKey` match, or listed as an orphan.
- ALSO synthesize a record from any live layout binding (`ptyIdsByLeafId`) whose ptyId has no lease, so a bound-but-unleased pane is not silently demoted.
- Sequence AFTER the legacy numeric-leaf remap (persistence.ts:1858-1872) so every record keys on a UUID.
PORT EVERY LEASE READER — the "write-only legacy projection" claim was FALSE and is corrected. Verified production readers of `sshRemotePtyLeases` outside persistence.ts:
- `getRecentExpiredSshLease` (orca-runtime.ts:6295-6316) with `ptyId` UNDEFINED, feeding `hasRecentExpiredSshLeasePane` (:6318-6323) -> the headless-mobile terminal-tab filter (:5290-5299 region, the `onlyRuntimeOwnedTerminals` filter) and `hasLiveOrPersistedServeOrSshOwnedPtyBinding` (:6355-6360). These are a live VISIBILITY signal for paired mobile/HUB clients and are NOT killed by the namespace bug, because they pass `ptyId` undefined. Port: "a non-terminal record names this leaf" replaces "an `expired` lease within 30s". Visibility becomes bounded by the D5 lifetime rule instead of a timer; that behavior change is deliberate and must be covered by an oracle.
- `ssh:terminateSessions` (src/main/ipc/ssh.ts:1282-1300) enumerates leases and distinguishes owned from expired (#2626). This is the EXISTING kill affordance; the D4 orphan UI FEEDS it rather than duplicating it. It is re-expressed over records: non-terminal records plus projection entries, with kill always explicit (D4).
- `ssh:resetRelay` (ipc/ssh.ts:1370-1385) marks every live lease `expired` after force-killing the relay. Ported to: mark records `detached` (never `terminated` — the client killed the relay, which proves nothing about the shells). BEHAVIOR CHANGE, flagged for the owner: a user-initiated relay reset stops silently re-showing panes for 30s and instead shows the disconnected affordance.
@@ -196,6 +215,7 @@ PARTIAL WRITE / ROLLBACK. Reuse the existing discipline (persistence.ts:7346-736
=====================================================================
D11. ORACLES — each must redden under a stated mutation
=====================================================================
1. DETACH-TO-TAB OVER SSH, THROUGH THE REAL RELAY COMPARISON. Bind an SSH pane in tab A leaf L, run `detachTerminalPaneToTab` to tab B, disconnect, reconnect. Assert: exactly one shell on the host, no second agent resume, the pane reattaches and the ptyId lands in tab B's layout. THIS MUST BE RED BEFORE STEP A. If it is green today, the harness stubbed the relay comparison or omitted `ORCA_PANE_KEY`/`ORCA_TAB_ID` from the spawn env, and the whole detach oracle family is measuring nothing. Gate the program on it going red first.
2. Identity mismatch is not proof of death. Assert `isProvenSshSessionGoneError` is false for the mismatch string, no record reaches `terminated`, no grant. Mutation: restore `.includes(SSH_SESSION_EXPIRED)` as the sole predicate (reattach-failure-classification.ts:29).
3. Wrong-incarnation attach is abandoned, not adopted and not fatal. Relay returns a different `incarnationId` than recorded: assert no bind, no stream, `detached`, no grant, activation lease rolled back. Mutation: skip the response check.
@@ -233,4 +253,4 @@ REFUTATION B — "make the host authoritative." SURVIVES, and is honored more ch
REFUTATION C — a second authority architecture. Untouched, and this revision deletes more than the last: no decision table, no `hostEpoch`, no `os.uptime` math, no per-platform boot semantics, no paneKey transfer/alias layer, no attach-time identity apparatus, no orphan-ownership classifier, no second durable record.
WHERE THE CUT IS. Plane 1 (delivery identity, credit ledger, chunk-and-ack, binary payloads, loopback WebSocket) proceeds in parallel. The cut is DURABILITY SCOPE vs EVIDENCE SOURCE, and this revision now applies that rule to all four inferences the prior round left standing: an error string means dead (deleted, D2), absence from a list means dead (deleted, D4/D5), a delivery-checkpoint flag means alive (deleted, D3), an arithmetic clock means rebooted (never built, D3). What remains on the wire is two optional fields: `relayInstanceId` on the grant and `paneKey` on the process summary. RC3 is entirely Plane 1's.
WHERE THE CUT IS. Plane 1 (delivery identity, credit ledger, chunk-and-ack, binary payloads, loopback WebSocket) proceeds in parallel. The cut is DURABILITY SCOPE vs EVIDENCE SOURCE, and this revision now applies that rule to all four inferences the prior round left standing: an error string means dead (deleted, D2), absence from a list means dead (deleted, D4/D5), a delivery-checkpoint flag means alive (deleted, D3), an arithmetic clock means rebooted (never built, D3). What remains on the wire is two optional fields: `relayInstanceId` on the grant and `paneKey` on the process summary. RC3 is entirely Plane 1's.
docs/reference/terminal-session-correctness-handoff-2026-08-07/goalposts.md
+5 -4
View File
@@ -395,12 +395,13 @@ remains correct.
Mocking `process.platform`, running Linux inside Docker, or passing in-process
wire unit tests does not prove the corresponding native or live-skew row.
## G6 — Simpler, strictly smaller production code
## G6 — Simpler, minimised production code
**Current status: partial; final size condition currently fails.**
**Current status: partial; structural clauses remain open.**
The user has tightened this gate: the final integrated program must contain
strictly less production source than its frozen pre-program baseline.
Decision D1 replaces the original strict inequality: the final integrated
program must minimise production source and justify every net addition against
the correctness it buys.
The default baseline is
`5ed45739e94bdf6460364e033bfcec9b32c0b42a`, the base recorded by GitHub for
docs/reference/terminal-session-correctness-handoff-2026-08-07/resume-plan.md
+10 -8
View File
@@ -119,7 +119,7 @@ The implementation design must explicitly settle:
- mixed-version capability and isolated legacy behavior;
- lost-device consumer retirement and compaction liveness;
- one-way migration/cutover; and
- the four-role deletion oracle and strictly net-negative production target.
- the four-role deletion oracle and minimised, justified production-code target.
Reuse existing pane, PTY, incarnation, CAS, and three-valued liveness primitives
where their semantics match. Do not equate client routing IDs with host identity
@@ -142,8 +142,9 @@ A sensible dependency shape is:
3. local/daemon/WSL/SSH/relay/paired/remote adapters using those primitives;
4. durable outcome, projection, reset, and recovery lifecycle;
5. one-way migration and capability cutover;
6. deletion of superseded writers, ledgers, quarantine, repair scans, timers
used as verdicts, and duplicate state machines; and
6. deletion of superseded writers, ledgers, repair scans, timers used as verdicts,
and duplicate state machines; retain `terminal-input-quarantine.ts` until a
replacement passes its destructive-input oracle; and
7. full platform, skew, performance, packaging, and independent review proof.
Each stack layer must be safe and tested, but the comprehensive status remains
@@ -180,9 +181,9 @@ are required. At minimum the final matrix must include:
5. Rerun all journeys and static/package checks on the rebased SHA.
6. Report production, tests, docs, CI/runner, generated, and vendored LOC
separately.
7. Require strictly negative aggregate production source net LOC against the
frozen pre-program baseline, including all program-attributable carry-in and
stack layers.
7. Minimise aggregate production source net LOC against the frozen pre-program
baseline, including all program-attributable carry-in and stack layers, and
justify every net addition as required by D1.
8. Run an independent repository review and the release-readiness checklist.
9. Resolve every P0–P2 finding.
10. Only then prepare the comprehensive PR and ask the user to approve any
@@ -269,5 +270,6 @@ Stop and return to the user before:
> [`goalposts.md`](./goalposts.md) passes on one rebased candidate. Reconcile the
> conflicting open work in [`related-open-work.md`](./related-open-work.md),
> build discriminating real SSH/daemon/paired/remote/cross-platform journeys,
> remove root causes and superseded reconciliation, and finish with strictly
> net-negative production LOC and no correctness or performance regression.
> remove root causes and superseded reconciliation, minimise and justify every
> production-code addition per D1, and finish with no correctness or performance
> regression.
src/main/ipc/pty.test.ts
+162 -6
View File
@@ -253,6 +253,7 @@ import {
getLocalPtyProvider,
isCurrentPtyExit,
restorePtyIncarnation,
bindPaneShell,
type PrepareCodexSessionResume
} from './pty'
import { __resetPersistedWindowsPathCacheForTests } from '../pty/windows-environment-path'
@@ -7880,6 +7881,7 @@ describe('registerPtyHandlers', () => {
id: opts.sessionId ?? 'daemon-pty'
})),
write,
hasPty: vi.fn(() => true),
resize: vi.fn(args.resize ?? (() => {})),
getAppliedSize: vi.fn(args.getAppliedSize ?? (async () => args.applied)),
kill: vi.fn(),
@@ -8104,6 +8106,107 @@ describe('registerPtyHandlers', () => {
expect(write).not.toHaveBeenCalled()
})
it('does not forward queued host input after the pane rebinds', async () => {
const write = setupProviderWithAppliedSize({ applied: { cols: 80, rows: 24 } })
let resolveClaim: (claimed: boolean) => void = () => {}
const claimRemoteDesktopHost = vi.fn(
() =>
new Promise<boolean>((resolve) => {
resolveClaim = resolve
})
)
const runtime = {
setPtyController: vi.fn(),
createPreAllocatedTerminalHandle: vi.fn(() => null),
registerPty: vi.fn(),
getDriver: vi.fn(() => ({ kind: 'idle' })),
claimRemoteDesktopHost,
onPtySpawned: vi.fn(),
onPtyExit: vi.fn(),
onPtyData: vi.fn()
}
handlers.clear()
registerPtyHandlers(mainWindow as never, runtime as never)
const spawn = await handlers.get('pty:spawn')!(null, { cols: 80, rows: 24, env: {} })
const id = (spawn as { id: string }).id
const leafId = '3f1c9a2e-7b4d-4e1a-9c8f-2d5e6a7b8c90'
const bindingStore = {
persistPtyBinding: vi.fn(() => true),
getWorkspaceSession: vi.fn(() => getDefaultWorkspaceSession())
}
bindPaneShell({
store: bindingStore,
worktreeId: 'repo-1:wt-1',
tabId: 'tab-1',
leafId,
ptyId: id
})
const claim = onMock.mock.calls.find((entry: unknown[]) => entry[0] === 'pty:claimViewport')
const writeEvent = onMock.mock.calls.find((entry: unknown[]) => entry[0] === 'pty:write')
claim?.[1](mainWindowIpcEvent, { id, cols: 125, rows: 48 })
writeEvent?.[1](mainWindowIpcEvent, { id, data: 'queued' })
const accepted = handlers.get('pty:writeAccepted')!(mainWindowIpcEvent, {
id,
data: 'accepted'
})
bindPaneShell({
store: bindingStore,
worktreeId: 'repo-1:wt-1',
tabId: 'tab-1',
leafId,
ptyId: 'pty-successor'
})
resolveClaim(true)
await expect(accepted).resolves.toBe(false)
await Promise.resolve()
expect(write).not.toHaveBeenCalled()
clearProviderPtyState(id)
clearProviderPtyState('pty-successor')
})
it('does not forward queued host input after the PTY incarnation changes', async () => {
const write = setupProviderWithAppliedSize({ applied: { cols: 80, rows: 24 } })
let resolveClaim: (claimed: boolean) => void = () => {}
const runtime = {
setPtyController: vi.fn(),
createPreAllocatedTerminalHandle: vi.fn(() => null),
registerPty: vi.fn(),
getDriver: vi.fn(() => ({ kind: 'idle' })),
claimRemoteDesktopHost: vi.fn(
() =>
new Promise<boolean>((resolve) => {
resolveClaim = resolve
})
),
onPtySpawned: vi.fn(),
onPtyExit: vi.fn(),
onPtyData: vi.fn()
}
handlers.clear()
registerPtyHandlers(mainWindow as never, runtime as never)
const spawn = await handlers.get('pty:spawn')!(null, { cols: 80, rows: 24, env: {} })
const id = (spawn as { id: string }).id
restorePtyIncarnation(id, 'incarnation-old')
const claim = onMock.mock.calls.find((entry: unknown[]) => entry[0] === 'pty:claimViewport')
const writeEvent = onMock.mock.calls.find((entry: unknown[]) => entry[0] === 'pty:write')
claim?.[1](mainWindowIpcEvent, { id, cols: 125, rows: 48 })
writeEvent?.[1](mainWindowIpcEvent, { id, data: 'queued' })
const accepted = handlers.get('pty:writeAccepted')!(mainWindowIpcEvent, {
id,
data: 'accepted'
})
restorePtyIncarnation(id, 'incarnation-new')
resolveClaim(true)
await expect(accepted).resolves.toBe(false)
await Promise.resolve()
expect(write).not.toHaveBeenCalled()
clearProviderPtyState(id)
})
it('does not populate the remote reclaim cache when only a phone drives', async () => {
const resizeSpy = vi.fn()
setupProviderWithAppliedSize({ applied: { cols: 80, rows: 24 }, resize: resizeSpy })
@@ -8707,12 +8810,17 @@ describe('registerPtyHandlers', () => {
}
}): Promise<{ id: string }>
}
let emitExit: ((event: { exitCode: number; signal: number }) => void) | undefined
const kill = vi.fn(() => emitExit?.({ exitCode: 0, signal: 0 }))
const proc = {
onData: vi.fn(),
onExit: vi.fn(),
onExit: vi.fn((callback) => {
emitExit = callback
return makeDisposable()
}),
write: vi.fn(),
resize: vi.fn(),
kill: vi.fn(),
kill,
process: 'zsh',
pid: 12345
}
@@ -8767,7 +8875,7 @@ describe('registerPtyHandlers', () => {
expect(store.persistPtyBinding).toHaveBeenCalledWith(
expect.objectContaining({ expectedSourceBinding })
)
expect(proc.kill).toHaveBeenCalledOnce()
expect(kill).toHaveBeenCalledOnce()
})
it('reports lower-owner commit before rejecting an early-exited runtime incarnation', async () => {
@@ -9698,6 +9806,7 @@ describe('registerPtyHandlers', () => {
})
)
expect(store.persistPtyBinding.mock.calls[0]!).toHaveLength(1)
expect(getPtyIdForPaneKey(paneKey)).toBe('pty-persisted-owner')
expect(
mainWindow.webContents.send.mock.calls.filter(([channel]) => channel === 'pty:spawned')
).toHaveLength(1)
@@ -10537,9 +10646,7 @@ describe('registerPtyHandlers', () => {
const freshPtyId = `ssh:${connectionId}@@fresh-relay-pty`
const remoteSpawn = vi.fn(async (options: { attachOnly?: boolean; command?: string }) => {
if (options.attachOnly) {
// The reattach mints this only after verifying the relay's observed exit names this shell.
// A bare not-found reaches here too and must NOT retire the owner — pinned by the sibling
// clause below, because retirement authorizes a replacement carrying the resume payload.
// Proven exit: only this error authorizes retiring the owner (bare not-found must not).
throw new Error('SSH_SESSION_EXPIRED: dead-relay-pty')
}
return { id: freshPtyId, incarnationId: 'inc-fresh-ssh-owner' }
@@ -17851,6 +17958,31 @@ describe('registerPtyHandlers', () => {
expect(mockProc.proc.write).toHaveBeenNthCalledWith(2, 'tail')
})
it('stops a chunked write when the PTY incarnation changes between chunks', async () => {
vi.useFakeTimers()
const mockProc = createMockProc()
spawnMock.mockReturnValue(mockProc.proc)
registerPtyHandlers(mainWindow as never)
const result = (await handlers.get('pty:spawn')!(null, {
cols: 80,
rows: 24
})) as { id: string }
restorePtyIncarnation(result.id, 'incarnation-old')
const text = ['x'.repeat(TERMINAL_INPUT_CHUNK_MAX_BYTES), 'tail'].join('')
const writeResult = handlers.get('pty:writeAccepted')!(mainWindowIpcEvent, {
id: result.id,
data: text
})
expect(mockProc.proc.write).toHaveBeenCalledOnce()
restorePtyIncarnation(result.id, 'incarnation-new')
await vi.runAllTimersAsync()
await expect(writeResult).resolves.toBe(false)
expect(mockProc.proc.write).toHaveBeenCalledOnce()
clearProviderPtyState(result.id)
})
it('yields while validating accepted large acknowledged pty writes before provider writes', async () => {
const mockProc = createMockProc()
spawnMock.mockReturnValue(mockProc.proc)
@@ -17875,6 +18007,30 @@ describe('registerPtyHandlers', () => {
expect(mockProc.proc.write.mock.calls.map(([chunk]) => chunk).join('')).toBe(text)
})
it('drops a deferred write when the PTY incarnation changes during validation', async () => {
vi.useFakeTimers()
const mockProc = createMockProc()
spawnMock.mockReturnValue(mockProc.proc)
registerPtyHandlers(mainWindow as never)
const result = (await handlers.get('pty:spawn')!(null, {
cols: 80,
rows: 24
})) as { id: string }
restorePtyIncarnation(result.id, 'incarnation-old')
const text = 'é'.repeat(CLIPBOARD_TEXT_MEASURE_YIELD_CODE_UNITS + 1)
const writeResult = handlers.get('pty:writeAccepted')!(mainWindowIpcEvent, {
id: result.id,
data: text
})
restorePtyIncarnation(result.id, 'incarnation-new')
await vi.runAllTimersAsync()
await expect(writeResult).resolves.toBe(false)
expect(mockProc.proc.write).not.toHaveBeenCalled()
clearProviderPtyState(result.id)
})
it('rejects oversized acknowledged pty writes before provider writes', async () => {
const mockProc = createMockProc()
spawnMock.mockReturnValue(mockProc.proc)
src/main/ipc/pty.ts
+51 -18
View File
@@ -575,6 +575,7 @@ export function bindPaneShell(args: {
incarnationId?: string
startupCwd?: string
mayCreate?: boolean
expectedBinding?: { ptyId: string; incarnationId?: string }
expectedSourceBinding?: PtyBindingSourceExpectation
}): { bound: boolean; tabId: string } {
const tabId = args.tabId
@@ -586,6 +587,7 @@ export function bindPaneShell(args: {
...(args.incarnationId ? { incarnationId: args.incarnationId } : {}),
...(args.startupCwd ? { startupCwd: args.startupCwd } : {}),
...(args.mayCreate === false ? { mayCreate: false } : {}),
...(args.expectedBinding ? { expectedBinding: args.expectedBinding } : {}),
...(args.expectedSourceBinding ? { expectedSourceBinding: args.expectedSourceBinding } : {})
})
if (bound === false) {
@@ -888,7 +890,8 @@ function persistAdmittedStablePaneBinding(args: {
if (!args.store || !args.owner || !args.worktreeId || !expectedBinding) {
return false
}
const persisted = args.store.persistPtyBinding({
const persisted = bindPaneShell({
store: args.store,
worktreeId: args.worktreeId,
tabId: args.owner.tabId,
leafId: args.owner.leafId,
@@ -897,7 +900,7 @@ function persistAdmittedStablePaneBinding(args: {
...(args.startupCwd ? { startupCwd: args.startupCwd } : {}),
expectedBinding
})
if (persisted === false) {
if (persisted.bound === false) {
throw new Error('terminal_pane_owner_changed')
}
return true
@@ -7115,11 +7118,20 @@ export function registerPtyHandlers(
mainWindow.webContents.send('pty:writeUnavailable', { id })
}
type PtyWriteFence = { incarnationId: string | undefined }
const isCurrentPtyWrite = (id: string, fence: PtyWriteFence): boolean =>
!isSupersededPtyId(id) && ptyIncarnationById.get(id) === fence.incarnationId
const writePtyProviderInputWithinLimit = (
provider: IPtyProvider,
id: string,
data: string
data: string,
fence: PtyWriteFence
): boolean | Promise<boolean> => {
if (!isCurrentPtyWrite(id, fence)) {
return false
}
const chunks = iterateTerminalInputChunks(data)
const first = chunks.next()
if (first.done) {
@@ -7131,21 +7143,24 @@ export function registerPtyHandlers(
provider.write(id, first.value)
return true
}
return writePtyProviderInputChunks(provider, id, chunks, first.value, second.value)
return writePtyProviderInputChunks(provider, id, chunks, first.value, second.value, fence)
}
const writePtyProviderInput = (
provider: IPtyProvider,
id: string,
data: string
data: string,
fence: PtyWriteFence
): boolean | Promise<boolean> => {
try {
const tooLarge = isTerminalInputTooLargeWithDeferredMeasurement(data)
if (typeof tooLarge === 'boolean') {
return tooLarge ? false : writePtyProviderInputWithinLimit(provider, id, data)
return tooLarge ? false : writePtyProviderInputWithinLimit(provider, id, data, fence)
}
return tooLarge
.then((result) => (result ? false : writePtyProviderInputWithinLimit(provider, id, data)))
.then((result) =>
result ? false : writePtyProviderInputWithinLimit(provider, id, data, fence)
)
.catch((error) => {
reportUnavailablePtyWrite(id, error)
return false
@@ -7161,12 +7176,16 @@ export function registerPtyHandlers(
id: string,
chunks: Iterator<string>,
firstChunk: string,
secondChunk: string
secondChunk: string,
fence: PtyWriteFence
): Promise<boolean> => {
try {
let chunk: IteratorResult<string> = { done: false, value: firstChunk }
let nextChunk: IteratorResult<string> = { done: false, value: secondChunk }
while (!chunk.done) {
if (!isCurrentPtyWrite(id, fence)) {
return false
}
provider.write(id, chunk.value)
if (!nextChunk.done) {
await new Promise((resolve) => setTimeout(resolve, 0))
@@ -7211,11 +7230,17 @@ export function registerPtyHandlers(
!mainWindow.isDestroyed() &&
!(typeof mainWebContents.isDestroyed === 'function' && mainWebContents.isDestroyed())
const writePtyInput = (args: PtyWritePayload): boolean | Promise<boolean> => {
const writePtyInput = (
args: PtyWritePayload,
fence: PtyWriteFence
): boolean | Promise<boolean> => {
// Why: mobile-presence-lock defense-in-depth — the renderer's onData guard can let one keystroke slip during the state-flip lag, so catch it server-side. See docs/mobile-presence-lock.md.
if (runtime?.getDriver(args.id).kind === 'mobile') {
return false
}
if (!isCurrentPtyWrite(args.id, fence)) {
return false
}
const provider = ptyOwnership.has(args.id) ? tryGetProviderForPty(args.id) : undefined
if (!provider) {
return false
@@ -7227,16 +7252,22 @@ export function registerPtyHandlers(
if (visibleRendererPtys.has(args.id)) {
clearHiddenRendererResizeOutput(args.id)
}
return writePtyProviderInput(provider, args.id, args.data)
return writePtyProviderInput(provider, args.id, args.data, fence)
} catch {
return false
}
}
const writePtyInputAccepted = (args: PtyWritePayload): boolean | Promise<boolean> => {
const writePtyInputAccepted = (
args: PtyWritePayload,
fence: PtyWriteFence
): boolean | Promise<boolean> => {
if (runtime?.getDriver(args.id).kind === 'mobile') {
return false
}
if (!isCurrentPtyWrite(args.id, fence)) {
return false
}
// Why: the ack infers Ctrl+C/Escape reached the local PTY; SSH providers are fire-and-forget relay notifications and can't truthfully acknowledge yet.
if (ptyOwnership.get(args.id) !== null) {
return false
@@ -7252,7 +7283,7 @@ export function registerPtyHandlers(
if (visibleRendererPtys.has(args.id)) {
clearHiddenRendererResizeOutput(args.id)
}
return writePtyProviderInput(provider, args.id, args.data)
return writePtyProviderInput(provider, args.id, args.data, fence)
} catch {
return false
}
@@ -7266,27 +7297,29 @@ export function registerPtyHandlers(
}
// Why here and not in the renderer: input queued before a reattach would
// otherwise land on whatever PTY now holds the pane.
if (isSupersededPtyId(args.id)) {
const fence: PtyWriteFence = { incarnationId: ptyIncarnationById.get(args.id) }
if (!isCurrentPtyWrite(args.id, fence)) {
return
}
const claimTail = hostViewportClaimTails.get(args.id)
if (claimTail) {
void claimTail.then((claimed) => (claimed ? writePtyInput(args) : false))
void claimTail.then((claimed) => (claimed ? writePtyInput(args, fence) : false))
return
}
writePtyInput(args)
writePtyInput(args, fence)
})
ipcMain.handle('pty:writeAccepted', (event, args: unknown): boolean | Promise<boolean> => {
if (!isPtyWriteEventFromMainWindow(event, mainWindow.webContents) || !isPtyWritePayload(args)) {
return false
}
if (isSupersededPtyId(args.id)) {
const fence: PtyWriteFence = { incarnationId: ptyIncarnationById.get(args.id) }
if (!isCurrentPtyWrite(args.id, fence)) {
return false
}
const claimTail = hostViewportClaimTails.get(args.id)
return claimTail
? claimTail.then((claimed) => (claimed ? writePtyInputAccepted(args) : false))
: writePtyInputAccepted(args)
? claimTail.then((claimed) => (claimed ? writePtyInputAccepted(args, fence) : false))
: writePtyInputAccepted(args, fence)
})
ipcMain.removeAllListeners('pty:claimViewport')
src/main/persistence-ssh-lease-termination.test.ts
+152
View File
@@ -0,0 +1,152 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import { getDefaultPersistedState } from '../shared/constants'
const testState = { dir: '' }
vi.mock('electron', () => ({
app: { getPath: () => testState.dir },
safeStorage: { isEncryptionAvailable: () => false }
}))
vi.mock('./telemetry/client', () => ({ track: vi.fn() }))
vi.mock('./telemetry/cohort-classifier', () => ({ getCohortAtEmit: vi.fn() }))
const TARGET = 'ssh-target-1'
type DurableFlushStore = {
flushDurableStateOrThrowAsync: (drainToStableGeneration?: boolean) => Promise<void>
}
function lease(ptyId: string, state: 'attached' | 'detached') {
return {
targetId: TARGET,
ptyId,
state,
createdAt: 1,
updatedAt: 1
}
}
async function createStore() {
mkdirSync(testState.dir, { recursive: true })
writeFileSync(
join(testState.dir, 'orca-data.json'),
JSON.stringify({
...getDefaultPersistedState(testState.dir),
sshRemotePtyLeases: [
lease('pty-1', 'detached'),
lease('pty-2', 'attached'),
lease('pty-3', 'detached')
]
}),
'utf-8'
)
vi.resetModules()
const { Store, initDataPath } = await import('./persistence')
initDataPath()
return new Store()
}
beforeEach(() => {
testState.dir = mkdtempSync(join(tmpdir(), 'orca-ssh-lease-termination-'))
})
afterEach(() => {
rmSync(testState.dir, { force: true, recursive: true })
})
describe('SSH lease termination persistence', () => {
it('updates only selected leases and awaits one asynchronous durability barrier', async () => {
const store = await createStore()
const asyncFlush = vi
.spyOn(store as unknown as DurableFlushStore, 'flushDurableStateOrThrowAsync')
.mockResolvedValue()
const syncFlush = vi.spyOn(store, 'flush')
const syncFlushOrThrow = vi.spyOn(store, 'flushOrThrow')
await store.markSshRemotePtyLeasesTerminatedAsync(TARGET, ['pty-1', 'pty-3'])
expect(asyncFlush).toHaveBeenCalledOnce()
expect(asyncFlush).toHaveBeenCalledWith(false)
expect(syncFlush).not.toHaveBeenCalled()
expect(syncFlushOrThrow).not.toHaveBeenCalled()
expect(store.getSshRemotePtyLeases(TARGET)).toEqual(
expect.arrayContaining([
expect.objectContaining({ ptyId: 'pty-1', state: 'terminated' }),
expect.objectContaining({ ptyId: 'pty-2', state: 'attached' }),
expect.objectContaining({ ptyId: 'pty-3', state: 'terminated' })
])
)
})
it('does not overwrite concurrent lease state when durability fails', async () => {
const store = await createStore()
const writeError = new Error('disk full')
const asyncFlush = vi
.spyOn(store as unknown as DurableFlushStore, 'flushDurableStateOrThrowAsync')
.mockRejectedValue(writeError)
const scheduleSave = vi.spyOn(store as unknown as { scheduleSave: () => void }, 'scheduleSave')
scheduleSave.mockClear()
const retirement = store.markSshRemotePtyLeasesTerminatedAsync(TARGET, ['pty-1'])
store.markSshRemotePtyLeasesForShutdown(TARGET, 'detached')
await expect(retirement).rejects.toBe(writeError)
expect(scheduleSave).toHaveBeenCalledTimes(2)
expect(store.getSshRemotePtyLeases(TARGET)).toEqual(
expect.arrayContaining([
expect.objectContaining({ ptyId: 'pty-1', state: 'terminated' }),
expect.objectContaining({ ptyId: 'pty-2', state: 'detached' })
])
)
asyncFlush.mockRestore()
await store.flushPendingOrThrowAsync({ drainToStableGeneration: false })
const persisted = JSON.parse(readFileSync(join(testState.dir, 'orca-data.json'), 'utf-8')) as {
sshRemotePtyLeases: { ptyId: string; state: string }[]
}
expect(persisted.sshRemotePtyLeases).toContainEqual(
expect.objectContaining({ ptyId: 'pty-1', state: 'terminated' })
)
})
it('does not write when no requested lease exists', async () => {
const store = await createStore()
const asyncFlush = vi.spyOn(
store as unknown as DurableFlushStore,
'flushDurableStateOrThrowAsync'
)
await store.markSshRemotePtyLeasesTerminatedAsync(TARGET, ['missing'])
expect(asyncFlush).not.toHaveBeenCalled()
})
it('does not chase mutations made after the retirement generation is durable', async () => {
const store = await createStore()
const internal = store as unknown as { enqueueWrite: () => Promise<void> }
const enqueueWrite = internal.enqueueWrite.bind(store)
let injectedMutation = false
const enqueueSpy = vi.spyOn(internal, 'enqueueWrite').mockImplementation(async () => {
await enqueueWrite()
if (!injectedMutation) {
injectedMutation = true
store.updateUI({ sidebarWidth: 777 })
}
})
await store.markSshRemotePtyLeasesTerminatedAsync(TARGET, ['pty-1'])
expect(enqueueSpy).toHaveBeenCalledOnce()
const persisted = JSON.parse(readFileSync(join(testState.dir, 'orca-data.json'), 'utf-8')) as {
sshRemotePtyLeases: { ptyId: string; state: string }[]
}
expect(persisted.sshRemotePtyLeases).toContainEqual(
expect.objectContaining({ ptyId: 'pty-1', state: 'terminated' })
)
enqueueSpy.mockRestore()
await store.flushPendingOrThrowAsync({ drainToStableGeneration: false })
})
})
src/main/persistence.ts
+145 -26
View File
@@ -97,6 +97,7 @@ import type { MigrationUnsupportedPtyEntry } from '../shared/agent-status-types'
import { MOBILE_PAIRING_USERDATA_FILES } from './runtime/mobile-pairing-files'
import { normalizePersistedMobileClientTabSelections } from './runtime/client-session-tab-selection-persistence'
import { sanitizeWorkspaceSessionTerminalRetirements } from './runtime/mobile-session-terminal-persistence-retirement'
import { findTerminalTabIdForLeaf } from './runtime/workspace-session-terminal-membership-authority'
import {
removeRepoFromHostWorkspaceSessions,
removeRepoFromWorkspaceSession
@@ -2836,6 +2837,7 @@ export class Store {
private pendingGithubCacheWrite: Promise<void> | null = null
private readonly staleGithubCacheTempCleanup: Promise<void>
private gitUsernameCache = new Map<string, string>()
private readonly sshRemotePtyLeaseMutationVersions = new WeakMap<SshRemotePtyLease, number>()
private readonly protectedSecrets = new ProtectedSecretPersistence()
private loadNeedsSave = false
private settingsChangeListeners = new Set<
@@ -7272,6 +7274,12 @@ export class Store {
return leases.filter((lease) => targetId === undefined || lease.targetId === targetId)
}
private advanceSshRemotePtyLeaseMutationVersion(lease: SshRemotePtyLease): number {
const version = (this.sshRemotePtyLeaseMutationVersions.get(lease) ?? 0) + 1
this.sshRemotePtyLeaseMutationVersions.set(lease, version)
return version
}
upsertSshRemotePtyLease(
lease: Omit<SshRemotePtyLease, 'createdAt' | 'updatedAt'> &
Partial<Pick<SshRemotePtyLease, 'createdAt' | 'updatedAt'>>
@@ -7346,6 +7354,7 @@ export class Store {
) {
lease.state = 'expired'
lease.updatedAt = now
this.advanceSshRemotePtyLeaseMutationVersion(lease)
superseded.push(lease)
}
}
@@ -7452,61 +7461,141 @@ export class Store {
continue
}
const { state, updatedAt } = lease
restore.push(() => {
lease.state = state
lease.updatedAt = updatedAt
})
lease.state = 'expired'
lease.updatedAt = now
const rollbackVersion = this.advanceSshRemotePtyLeaseMutationVersion(lease)
restore.push(() => {
if (
this.state.sshRemotePtyLeases?.includes(lease) &&
this.sshRemotePtyLeaseMutationVersions.get(lease) === rollbackVersion &&
lease.state === 'expired' &&
lease.updatedAt === now
) {
lease.state = state
lease.updatedAt = updatedAt
this.advanceSshRemotePtyLeaseMutationVersion(lease)
}
})
superseded.push(lease)
}
if (superseded.length === 0) {
return 0
}
const sessionsBefore = this.cloneSshLeaseBindingSessions(targetId)
const bindingsBefore = this.snapshotSshLeaseBindings(targetId, superseded)
this.clearSshRemotePtyBindingsForLeases(targetId, superseded, 'local')
this.scheduleSave()
try {
// Why the ASYNC twin: the sync flush fsyncs a multi-MB file from the Electron main thread,
// and this runs on every reconnect. On a stalled network profile mount that syscall is
// uninterruptible and nothing can bound it — see flushAsync. Why "OrThrow" and not plain
// flush(): flush() swallows write errors, which would leave these leases retired in memory
// but attached on disk for the rest of the session.
await this.flushDurableStateOrThrowAsync()
await this.flushDurableStateOrThrowAsync(false)
} catch (err) {
for (const undo of restore) {
undo()
}
this.restoreSshLeaseBindingSessions(targetId, sessionsBefore)
this.restoreSshLeaseBindings(targetId, superseded, bindingsBefore)
this.scheduleSave()
console.error('[persistence] Failed to retire duplicate pane leases:', err)
return 0
}
return superseded.length
}
private cloneSshLeaseBindingSessions(targetId: string): {
local?: WorkspaceSessionState
host?: WorkspaceSessionState
private snapshotSshLeaseBindings(
targetId: string,
leases: SshRemotePtyLease[]
): {
tabs: { leafId: string; ptyId: string }[]
leaves: { leafId: string; ptyId: string }[]
} {
const host = this.state.workspaceSessionsByHostId?.[toSshExecutionHostId(targetId)]
return {
...(this.state.workspaceSession
? { local: cloneWorkspaceSessionState(this.state.workspaceSession) }
: {}),
...(host ? { host: cloneWorkspaceSessionState(host) } : {})
const tabSnapshots: { leafId: string; ptyId: string }[] = []
const leaves: { leafId: string; ptyId: string }[] = []
const session = this.state.workspaceSession
for (const [worktreeId, tabs] of Object.entries(session?.tabsByWorktree ?? {})) {
for (const tab of tabs) {
if (tab.ptyId) {
const lease = leases.find((candidate) =>
this.sshRemotePtyLeaseMayReferenceBinding(candidate, {
ptyId: tab.ptyId!,
worktreeId,
targetId,
tabId: tab.id
})
)
if (lease?.leafId) {
tabSnapshots.push({ leafId: lease.leafId, ptyId: tab.ptyId })
}
}
}
}
for (const [tabId, layout] of Object.entries(session?.terminalLayoutsByTabId ?? {})) {
const worktreeId = Object.entries(session?.tabsByWorktree ?? {}).find(([, tabs]) =>
tabs.some((tab) => tab.id === tabId)
)?.[0]
for (const [leafId, ptyId] of Object.entries(layout.ptyIdsByLeafId ?? {})) {
if (
leases.some(
(lease) =>
lease.leafId === leafId &&
this.sshRemotePtyLeaseMayReferenceBinding(lease, {
ptyId,
targetId,
...(worktreeId ? { worktreeId } : {}),
tabId,
leafId
})
)
) {
leaves.push({ leafId, ptyId })
}
}
}
return { tabs: tabSnapshots, leaves }
}
private restoreSshLeaseBindingSessions(
private restoreSshLeaseBindings(
targetId: string,
sessions: { local?: WorkspaceSessionState; host?: WorkspaceSessionState }
): void {
if (sessions.local) {
this.state.workspaceSession = sessions.local
leases: SshRemotePtyLease[],
snapshots: {
tabs: { leafId: string; ptyId: string }[]
leaves: { leafId: string; ptyId: string }[]
}
if (sessions.host) {
this.state.workspaceSessionsByHostId = {
...this.state.workspaceSessionsByHostId,
[toSshExecutionHostId(targetId)]: sessions.host
): void {
const currentLeases = new Set(this.state.sshRemotePtyLeases ?? [])
const session = this.state.workspaceSession
const hasLiveLease = (snapshot: { leafId: string; ptyId: string }): boolean =>
leases.some(
(lease) =>
currentLeases.has(lease) &&
lease.state !== 'terminated' &&
lease.state !== 'expired' &&
lease.targetId === targetId &&
lease.leafId === snapshot.leafId &&
lease.ptyId === this.getRelayPtyIdForSshLeaseComparison(targetId, snapshot.ptyId)
)
for (const snapshot of snapshots.tabs) {
const tabId = findTerminalTabIdForLeaf(session, snapshot.leafId)
const tab = Object.values(session?.tabsByWorktree ?? {})
.flat()
.find((candidate) => candidate.id === tabId)
if (hasLiveLease(snapshot) && tab?.ptyId === null) {
tab.ptyId = snapshot.ptyId
}
}
for (const snapshot of snapshots.leaves) {
const tabId = findTerminalTabIdForLeaf(session, snapshot.leafId)
const layout = tabId ? session?.terminalLayoutsByTabId?.[tabId] : undefined
if (
hasLiveLease(snapshot) &&
layout &&
layout.ptyIdsByLeafId?.[snapshot.leafId] === undefined
) {
layout.ptyIdsByLeafId = {
...layout.ptyIdsByLeafId,
[snapshot.leafId]: snapshot.ptyId
}
}
}
}
@@ -7529,6 +7618,7 @@ export class Store {
state: SshRemotePtyLease['state']
): Promise<void> {
if (this.updateSshRemotePtyLeaseStates(targetId, state)) {
this.scheduleSave()
await this.flushDurableStateOrThrowAsync()
}
}
@@ -7541,10 +7631,30 @@ export class Store {
ptyIds.map((ptyId) => this.getRelayPtyIdForSshLeaseStorage(targetId, ptyId))
)
if (this.updateSshRemotePtyLeaseStates(targetId, 'attached', relayPtyIds)) {
this.scheduleSave()
await this.flushDurableStateOrThrowAsync()
}
}
async markSshRemotePtyLeasesTerminatedAsync(
targetId: string,
ptyIds: readonly string[]
): Promise<void> {
const relayPtyIds = new Set(
ptyIds.map((ptyId) => this.getRelayPtyIdForSshLeaseStorage(targetId, ptyId))
)
if (this.updateSshRemotePtyLeaseStates(targetId, 'terminated', relayPtyIds)) {
this.scheduleSave()
try {
await this.flushDurableStateOrThrowAsync(false)
} catch (error) {
// Keep the proven exit retryable after a transient persistence failure.
this.scheduleSave()
throw error
}
}
}
private updateSshRemotePtyLeaseStates(
targetId: string,
state: SshRemotePtyLease['state'],
@@ -7565,6 +7675,7 @@ export class Store {
if (state === 'detached' && lease.state !== 'attached') {
continue
}
this.advanceSshRemotePtyLeaseMutationVersion(lease)
if (lease.state !== state) {
lease.state = state
lease.updatedAt = now
@@ -7593,6 +7704,7 @@ export class Store {
if (!lease) {
return
}
this.advanceSshRemotePtyLeaseMutationVersion(lease)
const shouldClearBindings = state === 'terminated' || state === 'expired'
if (lease.state === state) {
if (shouldClearBindings && this.clearSshRemotePtyBindingsForLeases(targetId, [lease])) {
@@ -7775,10 +7887,11 @@ export class Store {
// Async twin of flushOrThrow: durable state only. Active-view and GitHub sidecars are
// quit/startup work and must not be snapshotted on the live SSH establish/reconnect path.
private async flushDurableStateOrThrowAsync(): Promise<void> {
private async flushDurableStateOrThrowAsync(drainToStableGeneration = true): Promise<void> {
if (this.writesFrozen || this.quitFlushStarted) {
throw new Error('Cannot flush while persistence is finalized')
}
const requiredDurableGeneration = this.writeGeneration
for (;;) {
if (this.writeTimer) {
clearTimeout(this.writeTimer)
@@ -7787,6 +7900,12 @@ export class Store {
this.firstPendingSaveAt = null
const generation = this.writeGeneration
await this.enqueueWrite()
if (
!drainToStableGeneration &&
this.lastDurableWriteGeneration >= requiredDurableGeneration
) {
break
}
if (generation === this.writeGeneration) {
break
}
src/main/providers/ssh-pty-identity-mismatch-is-not-death.test.ts
+20 -15
View File
@@ -122,6 +122,16 @@ describe('only an exit the relay watched is a death', () => {
expect(transportWouldReportSessionExpired(error.message)).toBe(false)
})
it('does not publish expiry for an exit that names a different PTY id', async () => {
const error = await reattachError(
formatPtyExitedError('pty-some-other-shell', 0, 'inc-host-7'),
'inc-host-7'
)
expect(error.message).not.toContain(SSH_SESSION_EXPIRED_ERROR)
expect(transportWouldReportSessionExpired(error.message)).toBe(false)
})
it('does not publish expiry for an exit when the pane knows no incarnation', async () => {
const error = await reattachError(formatPtyExitedError(RELAY_PTY_ID, 0, 'inc-host-7'))
@@ -143,16 +153,9 @@ describe('only an exit the relay watched is a death', () => {
})
})
// The relay only compares identity fields present on BOTH sides ("absent
// identity stays permissive", relay/pty-handler.ts). So not sending them stops
// every relay version from rejecting a moved pane — including relays already
// deployed on people's hosts, with no wire change and no redeploy.
//
// Nothing is lost: the check existed to catch a relay restart recycling pty-N
// for a new shell, and in exactly that case the pane and tab both still match,
// so it accepted the wrong shell anyway. What IS sent is the shell's own
// incarnation, which catches that case and follows the pane between tabs.
describe('reattach does not ask the relay to police pane identity', () => {
// New relays prioritize incarnation, so the pane fields do not reject a moved pane. Old relays
// ignore the additive incarnation field and still need their weaker pane fence.
describe('reattach preserves the old-relay pane fence', () => {
async function attachParams(
extraOptions: Record<string, unknown> = {}
): Promise<Record<string, unknown>> {
@@ -178,12 +181,12 @@ describe('reattach does not ask the relay to police pane identity', () => {
return (call?.[1] ?? call?.[0] ?? {}) as Record<string, unknown>
}
it('sends no expected pane key', async () => {
expect(await attachParams()).not.toHaveProperty('expectedPaneKey')
it('sends the current pane key for relays that do not understand incarnation', async () => {
expect(await attachParams()).toMatchObject({ expectedPaneKey: 'tab-new:leaf-1' })
})
it('sends no expected tab id', async () => {
expect(await attachParams()).not.toHaveProperty('expectedTabId')
it('sends the current tab id for relays that do not understand incarnation', async () => {
expect(await attachParams()).toMatchObject({ expectedTabId: 'tab-new' })
})
// The producer pin for the relay's incarnation guard. The guard is only worth having if the
@@ -191,7 +194,9 @@ describe('reattach does not ask the relay to police pane identity', () => {
// the relay stays permissive on an absent field, so a silent regression reads as "all green".
it('sends the expected incarnation when the host attested one', async () => {
expect(await attachParams({ expectedIncarnationId: 'inc-host-7' })).toMatchObject({
expectedIncarnationId: 'inc-host-7'
expectedIncarnationId: 'inc-host-7',
expectedPaneKey: 'tab-new:leaf-1',
expectedTabId: 'tab-new'
})
})
src/main/providers/ssh-pty-provider-exit-race.test.ts
+27
View File
@@ -170,3 +170,30 @@ it('returns a provisional source activation lease to reconnect authority', async
deliveryToken: 'token-reconnect'
})
})
it('fences a mismatched attach response before installing its source activation', async () => {
const response = {
incarnationId: 'incarnation-other',
sourceActivation: sourceActivation('incarnation-other')
}
const mux = {
request: vi.fn(async (method: string, _params, options) => {
if (method !== 'pty.attach') {
return undefined
}
options?.beforeResolve?.(response)
return response
}),
notify: vi.fn(),
onNotification: vi.fn(),
dispose: vi.fn(),
isDisposed: vi.fn().mockReturnValue(false)
}
const provider = new SshPtyProvider('conn-1', mux as never)
await expect(
provider.attachForReconnect('ssh:conn-1@@pty-1', undefined, 'incarnation-expected')
).rejects.toThrow('SSH_PTY_IDENTITY_MISMATCH')
expect(mux.request).toHaveBeenCalledTimes(1)
})
src/main/providers/ssh-pty-provider-reattach-incarnation.test.ts
+21 -1
View File
@@ -1,8 +1,28 @@
import { describe, expect, it, vi } from 'vitest'
import { SSH_SESSION_EXPIRED_ERROR, SSH_SOURCE_RESTORE_REQUIRED_ERROR } from './ssh-pty-errors'
import {
SSH_PTY_IDENTITY_MISMATCH_ERROR,
SSH_SESSION_EXPIRED_ERROR,
SSH_SOURCE_RESTORE_REQUIRED_ERROR
} from './ssh-pty-errors'
import { SshPtyProvider } from './ssh-pty-provider'
describe('SSH PTY provider session reattach incarnation', () => {
it.each([
['an old-relay response without an incarnation', {}],
['a response for a different incarnation', { incarnationId: 'incarnation-other' }]
])('rejects %s', async (_label, response) => {
const mux = {
request: vi.fn().mockResolvedValue(response),
notify: vi.fn(),
onNotification: vi.fn().mockReturnValue(vi.fn())
}
const provider = new SshPtyProvider('conn-1', mux as never)
await expect(
provider.attachForReconnect('ssh:conn-1@@pty-old', undefined, 'incarnation-expected')
).rejects.toThrow(SSH_PTY_IDENTITY_MISMATCH_ERROR)
})
it('remembers the authoritative incarnation before a legacy exit arrives', async () => {
let notify: ((method: string, params: Record<string, unknown>) => void) | undefined
const mux = {
src/main/providers/ssh-pty-provider.test.ts
+5 -4
View File
@@ -642,9 +642,8 @@ describe('SshPtyProvider', () => {
})
})
// Pane identity is deliberately no longer sent: the relay's copy is frozen
// at spawn, so moving a pane to another tab made it refuse a live shell.
it('reattaches without asking the relay to police pane identity', async () => {
// New relays prioritize incarnation; old relays ignore that field and retain this weaker fence.
it('preserves pane identity for relays that predate incarnation fencing', async () => {
mux.request.mockResolvedValue({ replay: 'buffered-output' })
await provider.spawn({
@@ -660,7 +659,9 @@ describe('SshPtyProvider', () => {
cols: 80,
rows: 24,
suppressReplayNotification: true,
exitProofSupported: true
exitProofSupported: true,
expectedPaneKey: 'tab-a:leaf-a',
expectedTabId: 'tab-a'
})
})
src/main/providers/ssh-pty-provider.ts
+8 -12
View File
@@ -2,7 +2,6 @@ import type { SshChannelMultiplexer } from '../ssh/ssh-channel-multiplexer'
import type { IPtyProvider, PtyProcessInfo, PtySpawnOptions, PtySpawnResult } from './types'
import { toAppSshPtyId, toRelaySshPtyId } from './ssh-pty-id'
import { createSshPtyAppliedSizeReader } from './ssh-pty-applied-size'
import { isRelayAttestedPtyIncarnationId } from '../../shared/pty-incarnation'
import type {
RemoteCliBridgeEnv,
SshPtyDataCallback,
@@ -14,6 +13,7 @@ import { SshPtyProviderOutputState } from './ssh-pty-provider-output-state'
import { spawnFreshSshPty } from './ssh-agent-session-create-operation'
import { mapSshPtyProcessList } from './ssh-agent-session-process-list'
import {
buildSshPtyReconnectAttachParams,
requestSshPtyAttach,
reattachSshPtySessionWithExitFence,
type PtySourceRecoveryRequest,
@@ -184,19 +184,15 @@ export class SshPtyProvider implements IPtyProvider {
async attachForReconnect(
id: string,
sourceRecovery?: PtySourceRecoveryRequest,
expectedIncarnationId?: string
expectedIncarnationId?: string,
legacyExpectedIdentity?: { paneKey?: string; tabId?: string }
): Promise<SshPtyAttachResult> {
// Why: reconnect owns replay delivery so stale/duplicate attach results can
// be filtered before they reach the renderer. Pane identity stays unsent (the relay froze it at
// spawn, so it rejected moved panes); the shell's own identity is sent instead. This path
// reattaches every pty after a relay returns — exactly when ids have been recycled.
const params = {
const params = buildSshPtyReconnectAttachParams({
id: this.toRelayPtyId(id),
suppressReplayNotification: true,
exitProofSupported: true,
...(isRelayAttestedPtyIncarnationId(expectedIncarnationId) ? { expectedIncarnationId } : {}),
...(sourceRecovery ? { sourceRecovery } : {})
}
...(sourceRecovery ? { sourceRecovery } : {}),
...(expectedIncarnationId ? { expectedIncarnationId } : {}),
...(legacyExpectedIdentity ? { legacyExpectedIdentity } : {})
})
const relayPtyId = this.toRelayPtyId(id)
return await requestSshPtyAttach({
mux: this.mux,
src/main/providers/ssh-pty-session-reattach.ts
+50 -13
View File
@@ -10,7 +10,7 @@ import {
isSshPtyExitedError,
isSshPtyIdentityMismatchError
} from './ssh-pty-errors'
import { parsePtyExitedError } from '../../shared/ssh-pty-failure-tokens'
import { parseMatchingPtyExitedError } from '../../shared/ssh-pty-failure-tokens'
import { toAppSshPtyId, toRelaySshPtyId } from './ssh-pty-id'
import type { PtySpawnOptions, PtySpawnResult } from './types'
import type { SshPtySpawnExitRaceTracker } from './ssh-pty-spawn-exit-race'
@@ -32,6 +32,29 @@ export type SshPtyAttachResult = {
sourceActivationLease?: SshPtyReceivingActivationLease
}
export function buildSshPtyReconnectAttachParams(args: {
id: string
sourceRecovery?: PtySourceRecoveryRequest
expectedIncarnationId?: string
legacyExpectedIdentity?: { paneKey?: string; tabId?: string }
}): Record<string, unknown> {
return {
id: args.id,
suppressReplayNotification: true,
exitProofSupported: true,
...(isRelayAttestedPtyIncarnationId(args.expectedIncarnationId)
? { expectedIncarnationId: args.expectedIncarnationId }
: {}),
...(args.legacyExpectedIdentity?.paneKey
? { expectedPaneKey: args.legacyExpectedIdentity.paneKey }
: {}),
...(args.legacyExpectedIdentity?.tabId
? { expectedTabId: args.legacyExpectedIdentity.tabId }
: {}),
...(args.sourceRecovery ? { sourceRecovery: args.sourceRecovery } : {})
}
}
type SshPtyReattachResult = PtySpawnResult & {
sourceRecovery?: PtySourceRecoveryResult
sourceActivationLease?: SshPtyReceivingActivationLease
@@ -90,6 +113,18 @@ export async function requestSshPtyAttach(args: {
rememberPtyIncarnation?: (relayPtyId: string, incarnationId: unknown) => void
}): Promise<SshPtyAttachResult> {
let activationLease: SshPtyReceivingActivationLease | undefined
const parseResult = (value: unknown): SshPtyAttachResult => {
const result = parseSshPtyAttachResult(value)
const expectedIncarnationId = args.params.expectedIncarnationId
// Old relays ignore the request fence, so their response must prove the same shell.
if (
isRelayAttestedPtyIncarnationId(expectedIncarnationId) &&
result.incarnationId !== expectedIncarnationId
) {
throw new Error(`${SSH_PTY_IDENTITY_MISMATCH_ERROR}: ${args.relayPtyId}`)
}
return result
}
const installFromResult = (result: SshPtyAttachResult): void => {
if (!activationLease && result.sourceActivation && args.installSourceActivation) {
activationLease = args.installSourceActivation(args.relayPtyId, result.sourceActivation)
@@ -98,9 +133,9 @@ export async function requestSshPtyAttach(args: {
try {
const rawResult = await args.mux.request('pty.attach', args.params, {
...(args.timeoutMs === undefined ? {} : { timeoutMs: args.timeoutMs }),
beforeResolve: (value) => installFromResult(parseSshPtyAttachResult(value))
beforeResolve: (value) => installFromResult(parseResult(value))
})
const result = parseSshPtyAttachResult(rawResult)
const result = parseResult(rawResult)
installFromResult(result)
args.rememberPtyIncarnation?.(args.relayPtyId, result.incarnationId)
if (args.commitSourceActivation) {
@@ -190,11 +225,8 @@ export async function reattachSshPtySession(args: {
const relaySessionId = toRelaySshPtyId(args.connectionId, args.sessionId)
console.warn(`[ssh-pty] spawn() called with sessionId=${args.sessionId}, attempting pty.attach`)
try {
// Why no expected pane identity: the relay froze it at spawn, so moving a
// pane to another tab made it refuse a live shell — and refuse by saying
// "not found", which read as death. It never caught what it was for either:
// a relay restart recycling this id for a new shell leaves pane and tab
// matching. Recycling is caught by the incarnation the attach returns.
const expectedPaneKey = args.options.paneKey ?? args.options.env?.ORCA_PANE_KEY
const expectedTabId = args.options.tabId ?? args.options.env?.ORCA_TAB_ID
const attachResult = await requestSshPtyAttach({
mux: args.mux,
relayPtyId: relaySessionId,
@@ -210,10 +242,12 @@ export async function reattachSshPtySession(args: {
// The shell's own identity, so it survives a pane moving between tabs — unlike the pane
// identity this replaced. Sent only when the host attested it: a locally synthesized
// stand-in is not stable across reconnects and would refuse the pane its own shell. An
// older relay ignores the field, which leaves today's permissive behaviour.
// older relay ignores the field, so the legacy pane fence below remains its fallback.
...(isRelayAttestedPtyIncarnationId(args.options.expectedIncarnationId)
? { expectedIncarnationId: args.options.expectedIncarnationId }
: {})
: {}),
...(expectedPaneKey ? { expectedPaneKey } : {}),
...(expectedTabId ? { expectedTabId } : {})
},
installSourceActivation: args.installSourceActivation,
rememberPtyIncarnation: args.rememberPtyIncarnation
@@ -247,9 +281,12 @@ export async function reattachSshPtySession(args: {
// versions differ, so a proof we cannot tie to the incarnation we asked about is not proof and
// falls through to the disconnected pane instead of replacing a shell that may be running.
if (isSshPtyExitedError(error)) {
const proof = parsePtyExitedError(error instanceof Error ? error.message : String(error))
const expected = args.options.expectedIncarnationId
if (proof && isRelayAttestedPtyIncarnationId(expected) && proof.incarnationId === expected) {
const proof = parseMatchingPtyExitedError(
error instanceof Error ? error.message : String(error),
relaySessionId,
args.options.expectedIncarnationId
)
if (proof) {
throw new Error(`${SSH_SESSION_EXPIRED_ERROR}: ${relaySessionId}`)
}
throw error
src/main/runtime/orca-runtime.ts
+8 -1
View File
@@ -29953,13 +29953,20 @@ export class OrcaRuntimeService {
// The sync hasPty rescue closes the spawn/list race: a just-spawned PTY can
// register after the inventory snapshot, and federation reads one
// connected:false as exited.
let directLiveness: boolean | null | undefined
try {
directLiveness = leaf.ptyId ? this.ptyController?.hasPty?.(leaf.ptyId) : undefined
} catch {
directLiveness = null
}
const provenAbsent =
provenLivePtyIds !== null &&
leaf.ptyId !== null &&
!provenLivePtyIds.has(leaf.ptyId) &&
!leaf.ptyId.startsWith('remote:') &&
parseAppSshPtyId(leaf.ptyId) === null &&
this.ptyController?.hasPty?.(leaf.ptyId) !== true
directLiveness !== true &&
directLiveness !== null
return {
handle: this.issueHandle(leaf),
ptyId: leaf.ptyId,
src/main/runtime/terminal-list-stale-leaf-liveness.test.ts
+17
View File
@@ -148,6 +148,23 @@ describe('listTerminals liveness truth for restored leaves', () => {
})
})
it('keeps a leaf connected when the snapshot missed it but provider liveness is unknown', async () => {
const runtime = makeRuntimeWithLeaf({
leafPtyId: 'pty-provider-disconnected',
controllerSessions: [],
hasPty: () => null
})
const { terminals } = await runtime.listTerminals(`id:${WORKTREE_ID}`)
expect(terminals).toHaveLength(1)
expect(terminals[0]).toMatchObject({
ptyId: 'pty-provider-disconnected',
connected: true,
writable: true
})
})
it('does not demote remote-runtime-scoped leaves the local inventory never covers', async () => {
const runtime = makeRuntimeWithLeaf({
leafPtyId: 'remote:env-1@@term_abc',
src/main/ssh-reattach-pane-cardinality.test.ts
+325
View File
@@ -209,6 +209,291 @@ describe('STA-3077: existing duplicate leases are healed, not revived', () => {
expect(liveLeasesForPane(store)).toHaveLength(2)
})
it('does not chase store mutations after the retirement generation is durable', async () => {
const store = await createStore({
sshRemotePtyLeases: [
{ ...leaseFor('relay-pty-a', 1), createdAt: 1 },
{ ...leaseFor('relay-pty-b', 2), createdAt: 2 }
]
})
const internal = store as unknown as { enqueueWrite: () => Promise<void> }
const enqueueWrite = internal.enqueueWrite.bind(store)
let injectedMutation = false
const enqueueSpy = vi.spyOn(internal, 'enqueueWrite').mockImplementation(async () => {
await enqueueWrite()
if (!injectedMutation) {
injectedMutation = true
store.updateUI({ sidebarWidth: 777 })
}
})
expect(await store.supersedeDuplicatePaneLeases(TARGET)).toBe(1)
expect(enqueueSpy).toHaveBeenCalledOnce()
enqueueSpy.mockRestore()
await store.flushPendingOrThrowAsync({ drainToStableGeneration: false })
})
it('retries when a mutation invalidates the retirement write before rename', async () => {
const store = await createStore({
sshRemotePtyLeases: [
{ ...leaseFor('relay-pty-a', 1), createdAt: 1 },
{ ...leaseFor('relay-pty-b', 2), createdAt: 2 }
]
})
const internal = store as unknown as { writeToDiskAsync: () => Promise<void> }
const writeToDiskAsync = internal.writeToDiskAsync.bind(store)
let injectedMutation = false
const writeSpy = vi.spyOn(internal, 'writeToDiskAsync').mockImplementation(async () => {
const write = writeToDiskAsync()
if (!injectedMutation) {
injectedMutation = true
store.updateUI({ sidebarWidth: 778 })
}
await write
})
expect(await store.supersedeDuplicatePaneLeases(TARGET)).toBe(1)
expect(writeSpy).toHaveBeenCalledTimes(2)
})
it('does not undo a concurrent confirmation of the provisional expired state', async () => {
const store = await createStore({
sshRemotePtyLeases: [
{ ...leaseFor('relay-pty-a', 1), createdAt: 1 },
{ ...leaseFor('relay-pty-b', 2), createdAt: 2 }
]
})
let rejectWrite: (error: Error) => void = () => {}
vi.spyOn(
store as unknown as { flushDurableStateOrThrowAsync: () => Promise<void> },
'flushDurableStateOrThrowAsync'
).mockImplementation(
() =>
new Promise((_, reject) => {
rejectWrite = reject
})
)
const retirement = store.supersedeDuplicatePaneLeases(TARGET)
store.markSshRemotePtyLease(TARGET, 'relay-pty-a', 'expired')
rejectWrite(new Error('disk full'))
expect(await retirement).toBe(0)
expect(
store.getSshRemotePtyLeases(TARGET).find((lease) => lease.ptyId === 'relay-pty-a')?.state
).toBe('expired')
await store.flushPendingOrThrowAsync({ drainToStableGeneration: false })
})
it('rolls back after a rejected concurrent attach request', async () => {
const store = await createStore({
sshRemotePtyLeases: [
{ ...leaseFor('relay-pty-a', 1), createdAt: 1 },
{ ...leaseFor('relay-pty-b', 2), createdAt: 2 }
]
})
let rejectWrite: (error: Error) => void = () => {}
vi.spyOn(
store as unknown as { flushDurableStateOrThrowAsync: () => Promise<void> },
'flushDurableStateOrThrowAsync'
).mockImplementation(
() =>
new Promise((_, reject) => {
rejectWrite = reject
})
)
const retirement = store.supersedeDuplicatePaneLeases(TARGET)
store.markSshRemotePtyLease(TARGET, 'relay-pty-a', 'attached')
rejectWrite(new Error('disk full'))
expect(await retirement).toBe(0)
expect(
store.getSshRemotePtyLeases(TARGET).find((lease) => lease.ptyId === 'relay-pty-a')?.state
).toBe('attached')
})
it('does not roll back concurrent session or lease changes after a failed write', async () => {
const store = await createStore({
sshRemotePtyLeases: [
{ ...leaseFor('relay-pty-a', 1), createdAt: 1 },
{ ...leaseFor('relay-pty-b', 2), createdAt: 2 }
]
})
let rejectWrite: (error: Error) => void = () => {}
vi.spyOn(
store as unknown as { flushDurableStateOrThrowAsync: () => Promise<void> },
'flushDurableStateOrThrowAsync'
).mockImplementation(
() =>
new Promise((_, reject) => {
rejectWrite = reject
})
)
const retirement = store.supersedeDuplicatePaneLeases(TARGET)
store.markSshRemotePtyLease(TARGET, 'relay-pty-a', 'terminated')
const concurrentTab = 'tab-concurrent'
const concurrentLeaf = '8a2b4c6d-1e3f-4a5b-8c7d-9e0f1a2b3c4d'
store.persistPtyBinding({
worktreeId: WORKTREE,
tabId: concurrentTab,
leafId: concurrentLeaf,
ptyId: 'relay-pty-concurrent',
incarnationId: 'inc-concurrent'
})
rejectWrite(new Error('disk full'))
expect(await retirement).toBe(0)
expect(
store.getSshRemotePtyLeases(TARGET).find((lease) => lease.ptyId === 'relay-pty-a')?.state
).toBe('terminated')
expect(
store.getWorkspaceSession().terminalLayoutsByTabId?.[concurrentTab]?.ptyIdsByLeafId?.[
concurrentLeaf
]
).toBe('relay-pty-concurrent')
})
it('does not restore a binding for a lease removed during a failed write', async () => {
const store = await createStore({
sshRemotePtyLeases: [
{ ...leaseFor('relay-pty-a', 1), createdAt: 1 },
{ ...leaseFor('relay-pty-b', 2), createdAt: 2 }
]
})
store.persistPtyBinding({
worktreeId: WORKTREE,
tabId: TAB,
leafId: LEAF,
ptyId: 'relay-pty-a',
incarnationId: 'inc-a'
})
delete store.getWorkspaceSession().terminalLayoutsByTabId?.[TAB]?.ptyIdsByLeafId?.[LEAF]
let rejectWrite: (error: Error) => void = () => {}
vi.spyOn(
store as unknown as { flushDurableStateOrThrowAsync: () => Promise<void> },
'flushDurableStateOrThrowAsync'
).mockImplementation(
() =>
new Promise((_, reject) => {
rejectWrite = reject
})
)
const retirement = store.supersedeDuplicatePaneLeases(TARGET)
store.removeSshRemotePtyLease(TARGET, 'relay-pty-a')
rejectWrite(new Error('disk full'))
expect(await retirement).toBe(0)
expect(store.getSshRemotePtyLeases(TARGET).some((lease) => lease.ptyId === 'relay-pty-a')).toBe(
false
)
expect(
store.getWorkspaceSession().tabsByWorktree?.[WORKTREE]?.find((tab) => tab.id === TAB)?.ptyId
).toBeNull()
})
it('restores a tab binding into the moved leaf after a concurrent session replacement', async () => {
const store = await createStore({
sshRemotePtyLeases: [
{ ...leaseFor('relay-pty-a', 1), createdAt: 1 },
{ ...leaseFor('relay-pty-b', 2), createdAt: 2 }
]
})
store.persistPtyBinding({
worktreeId: WORKTREE,
tabId: TAB,
leafId: LEAF,
ptyId: 'relay-pty-a',
incarnationId: 'inc-a'
})
delete store.getWorkspaceSession().terminalLayoutsByTabId?.[TAB]?.ptyIdsByLeafId?.[LEAF]
let rejectWrite: (error: Error) => void = () => {}
vi.spyOn(
store as unknown as { flushDurableStateOrThrowAsync: () => Promise<void> },
'flushDurableStateOrThrowAsync'
).mockImplementation(
() =>
new Promise((_, reject) => {
rejectWrite = reject
})
)
const retirement = store.supersedeDuplicatePaneLeases(TARGET)
expect(
store.getWorkspaceSession().tabsByWorktree?.[WORKTREE]?.find((tab) => tab.id === TAB)?.ptyId
).toBeNull()
const movedTabId = 'tab-moved'
const replacement = structuredClone(store.getWorkspaceSession())
const movedTab = replacement.tabsByWorktree?.[WORKTREE]?.find((tab) => tab.id === TAB)
if (!movedTab || !replacement.terminalLayoutsByTabId?.[TAB]) {
throw new Error('expected source tab and layout')
}
movedTab.id = movedTabId
replacement.terminalLayoutsByTabId[movedTabId] = replacement.terminalLayoutsByTabId[TAB]
delete replacement.terminalLayoutsByTabId[TAB]
store.setWorkspaceSession(replacement)
rejectWrite(new Error('disk full'))
expect(await retirement).toBe(0)
expect(
store.getWorkspaceSession().tabsByWorktree?.[WORKTREE]?.find((tab) => tab.id === movedTabId)
?.ptyId
).toBe('relay-pty-a')
await store.flushPendingOrThrowAsync({ drainToStableGeneration: false })
})
it('does not restore a retired binding after the leaf is deleted', async () => {
const otherTab = 'tab-other'
const store = await createStore({
sshRemotePtyLeases: [
{ ...leaseFor('relay-pty-a', 1), createdAt: 1 },
{ ...leaseFor('relay-pty-b', 2), createdAt: 2, tabId: otherTab }
]
})
store.persistPtyBinding({
worktreeId: WORKTREE,
tabId: TAB,
leafId: LEAF,
ptyId: 'relay-pty-a',
incarnationId: 'inc-a'
})
store.persistPtyBinding({
worktreeId: WORKTREE,
tabId: otherTab,
leafId: LEAF,
ptyId: 'relay-pty-b',
incarnationId: 'inc-b'
})
let rejectWrite: (error: Error) => void = () => {}
vi.spyOn(
store as unknown as { flushDurableStateOrThrowAsync: () => Promise<void> },
'flushDurableStateOrThrowAsync'
).mockImplementation(
() =>
new Promise((_, reject) => {
rejectWrite = reject
})
)
const retirement = store.supersedeDuplicatePaneLeases(TARGET)
expect(store.getWorkspaceSession().terminalLayoutsByTabId?.[TAB]?.ptyIdsByLeafId?.[LEAF]).toBe(
undefined
)
const replacement = structuredClone(store.getWorkspaceSession())
replacement.tabsByWorktree![WORKTREE] = []
replacement.terminalLayoutsByTabId = {}
store.setWorkspaceSession(replacement)
rejectWrite(new Error('disk full'))
expect(await retirement).toBe(0)
expect(store.getWorkspaceSession().tabsByWorktree?.[WORKTREE]).toEqual([])
expect(store.getWorkspaceSession().terminalLayoutsByTabId).toEqual({})
})
it('leaves distinct panes alone', async () => {
const otherLeaf = '8a2b4c6d-1e3f-4a5b-8c7d-9e0f1a2b3c4d'
const store = await createStore({
@@ -243,6 +528,46 @@ describe('STA-3077: existing duplicate leases are healed, not revived', () => {
})
})
describe('SSH reconnect lease retirement batching', () => {
it('keeps concurrent terminal state when the asynchronous write fails', async () => {
const store = await createStore({
sshRemotePtyLeases: [
{ ...leaseFor('relay-pty-a', 1), createdAt: 1 },
{ ...leaseFor('relay-pty-b', 2), createdAt: 2 }
]
})
let rejectWrite: (error: Error) => void = () => {}
vi.spyOn(
store as unknown as { flushDurableStateOrThrowAsync: () => Promise<void> },
'flushDurableStateOrThrowAsync'
).mockImplementation(
() =>
new Promise((_, reject) => {
rejectWrite = reject
})
)
const retirement = store.markSshRemotePtyLeasesTerminatedAsync(TARGET, ['relay-pty-a'])
const concurrentTab = 'tab-concurrent'
const concurrentLeaf = '8a2b4c6d-1e3f-4a5b-8c7d-9e0f1a2b3c4d'
store.persistPtyBinding({
worktreeId: WORKTREE,
tabId: concurrentTab,
leafId: concurrentLeaf,
ptyId: 'relay-pty-concurrent',
incarnationId: 'inc-concurrent'
})
rejectWrite(new Error('disk full'))
await expect(retirement).rejects.toThrow('disk full')
expect(
store.getWorkspaceSession().terminalLayoutsByTabId?.[concurrentTab]?.ptyIdsByLeafId?.[
concurrentLeaf
]
).toBe('relay-pty-concurrent')
})
})
describe('STA-3077: reattach binds panes, it never creates them', () => {
// RC3: persistPtyBinding has four creating branches (mint tab, mint root leaf,
// split root and graft leaf, mint layout). They are load-bearing for spawn and
src/main/ssh/ssh-relay-reattach-exit-proof.test.ts
+59 -9
View File
@@ -120,7 +120,8 @@ function detachedLease() {
state: 'detached' as const,
worktreeId: 'worktree-1',
tabId: 'tab-1',
leafId: LEAF_ID
leafId: LEAF_ID,
incarnationId: 'inc-proven-exit'
}
}
@@ -227,21 +228,70 @@ describe('SshRelaySession reattach failures never fabricate an exit', () => {
formatPtyExitedError(RELAY_PTY_ID, 0, 'inc-proven-exit')
)
expect(deps.mockStore.markSshRemotePtyLease).toHaveBeenCalledWith(
expect(deps.mockStore.markSshRemotePtyLeasesTerminatedAsync).toHaveBeenCalledWith(TARGET_ID, [
RELAY_PTY_ID
])
})
it('persists a reconnect batch of proven exits in one async write', async () => {
const deps = createMockDeps()
const secondPtyId = 'pty-second'
const secondIncarnationId = 'inc-proven-second-exit'
const attachForReconnect = vi.fn().mockImplementation((ptyId: string) => {
const incarnationId = ptyId === RELAY_PTY_ID ? 'inc-proven-exit' : secondIncarnationId
return Promise.reject(new Error(formatPtyExitedError(ptyId, 0, incarnationId)))
})
vi.mocked(getSshPtyProvider).mockReturnValue({
attachForReconnect,
shutdown: vi.fn().mockResolvedValue(undefined),
dispose: vi.fn()
} as unknown as ReturnType<typeof getSshPtyProvider>)
vi.mocked(deps.mockStore.getSshRemotePtyLeases).mockReturnValue([
detachedLease(),
{
...detachedLease(),
ptyId: secondPtyId,
leafId: '22222222-2222-4222-8222-222222222222',
incarnationId: secondIncarnationId
}
] as ReturnType<typeof deps.mockStore.getSshRemotePtyLeases>)
const session = new SshRelaySession(
TARGET_ID,
RELAY_PTY_ID,
'terminated'
deps.getMainWindow,
deps.mockStore,
deps.mockPortForward
)
await session.establish(deps.mockConn)
expect(deps.mockStore.markSshRemotePtyLeasesTerminatedAsync).toHaveBeenCalledOnce()
expect(deps.mockStore.markSshRemotePtyLeasesTerminatedAsync).toHaveBeenCalledWith(TARGET_ID, [
RELAY_PTY_ID,
secondPtyId
])
expect(deps.mockStore.markSshRemotePtyLease).not.toHaveBeenCalled()
})
it('does not retire the lease for proof naming another PTY', async () => {
const { deps } = await reattachFailsWith(
formatPtyExitedError('pty-some-other-shell', 0, 'inc-proven-exit')
)
expect(deps.mockStore.markSshRemotePtyLeasesTerminatedAsync).not.toHaveBeenCalled()
})
it('does not retire the lease for proof naming another incarnation', async () => {
const { deps } = await reattachFailsWith(
formatPtyExitedError(RELAY_PTY_ID, 0, 'inc-some-other-shell')
)
expect(deps.mockStore.markSshRemotePtyLeasesTerminatedAsync).not.toHaveBeenCalled()
})
it('does not retire the lease when the failure proves nothing', async () => {
const { deps } = await reattachFailsWith(UNPROVEN_NOT_FOUND)
expect(deps.mockStore.markSshRemotePtyLease).not.toHaveBeenCalledWith(
TARGET_ID,
RELAY_PTY_ID,
'terminated'
)
expect(deps.mockStore.markSshRemotePtyLeasesTerminatedAsync).not.toHaveBeenCalled()
})
it('routes an unproven not-found into the non-destructive detached branch', async () => {
src/main/ssh/ssh-relay-session-reconnect-incarnation.test.ts
+20
View File
@@ -456,6 +456,26 @@ describe('SshRelaySession reconnect incarnation ordering', () => {
)
})
it('sends pane identity beside incarnation for relays that only understand the legacy fence', async () => {
const { mockConn, mockStore, mockPortForward, getMainWindow } = createMockDeps()
const attachForReconnect = vi.fn().mockResolvedValue({ incarnationId: 'incarnation-lease' })
vi.mocked(getSshPtyProvider).mockReturnValue({
attachForReconnect,
dispose: vi.fn()
} as unknown as ReturnType<typeof getSshPtyProvider>)
vi.mocked(mockStore.getSshRemotePtyLeases).mockReturnValue([
{ ...detachedLease(), incarnationId: 'incarnation-lease' }
] as ReturnType<typeof mockStore.getSshRemotePtyLeases>)
const session = new SshRelaySession('target-1', getMainWindow, mockStore, mockPortForward)
await session.establish(mockConn)
expect(attachForReconnect).toHaveBeenCalledWith('pty-live', undefined, 'incarnation-lease', {
paneKey: `tab-1:${INCARNATION_LEAF_ID}`,
tabId: 'tab-1'
})
})
it('does not restore a PTY whose matching exit shares the attach reply batch', async () => {
const { mockConn, mockStore, mockPortForward, getMainWindow, mockWindow } = createMockDeps()
const incarnationId = 'incarnation-exited-during-attach'
src/main/ssh/ssh-relay-session-test-fixtures.ts
+1
View File
@@ -27,6 +27,7 @@ export function createMockDeps(): SshRelaySessionTestDeps {
markSshRemotePtyLeasesAsync: vi.fn(),
markSshRemotePtyLeasesForShutdown: vi.fn(),
markSshRemotePtyLeasesAttachedAsync: vi.fn(),
markSshRemotePtyLeasesTerminatedAsync: vi.fn(),
persistPtyBinding: vi.fn().mockReturnValue(true)
} as unknown as Store
const mockPortForward = {
src/main/ssh/ssh-relay-session.test.ts
+12 -8
View File
@@ -531,10 +531,8 @@ describe('SshRelaySession', () => {
)
})
// The relay froze pane identity at spawn, so sending it made a moved pane
// unreachable — and it never caught the reused-id case it was written for,
// because after a relay restart the pane and tab both still match.
it('does not forward pane identity to reattach', async () => {
// New relays prioritize incarnation; old relays ignore that additive field and need this fence.
it('preserves pane identity for relays that predate incarnation fencing', async () => {
const { mockConn, mockStore, mockPortForward, getMainWindow } = createMockDeps()
const { getSshPtyProvider } = await import('../ipc/pty')
const mockAttach = vi.fn().mockResolvedValue(undefined)
@@ -551,13 +549,15 @@ describe('SshRelaySession', () => {
const session = new SshRelaySession('target-1', getMainWindow, mockStore, mockPortForward)
await session.establish(mockConn)
expect(mockAttach).toHaveBeenCalledWith('pty-1')
expect(mockAttach).toHaveBeenCalledWith('pty-1', undefined, undefined, {
paneKey: `tab-a:${leafId}`,
tabId: 'tab-a'
})
})
// This is the path that reattaches every known pty when a relay comes back — precisely when ids
// have been reissued from pty-1 — so it is the one that most needs to say which shell it means.
// Pane identity still is not sent; the shell's own identity is, and only when the host attested
// it. Without this the main reconnect attaches by id alone and can land on somebody else's shell.
// The shell's own identity is authoritative; pane identity remains the fallback for old relays.
it('forwards the shell identity its lease recorded when reattaching', async () => {
const { mockConn, mockStore, mockPortForward, getMainWindow } = createMockDeps()
const { getSshPtyProvider } = await import('../ipc/pty')
@@ -582,7 +582,10 @@ describe('SshRelaySession', () => {
const session = new SshRelaySession('target-1', getMainWindow, mockStore, mockPortForward)
await session.establish(mockConn)
expect(mockAttach).toHaveBeenCalledWith('pty-1', undefined, 'inc-host-1')
expect(mockAttach).toHaveBeenCalledWith('pty-1', undefined, 'inc-host-1', {
paneKey: `tab-a:${leafId}`,
tabId: 'tab-a'
})
})
it('does not expire a live reused relay id when attach rejects identity mismatch', async () => {
@@ -617,6 +620,7 @@ describe('SshRelaySession', () => {
expect(clearProviderPtyState).not.toHaveBeenCalledWith('ssh:target-1@@pty-1')
expect(deletePtyOwnership).not.toHaveBeenCalledWith('ssh:target-1@@pty-1')
expect(mockStore.markSshRemotePtyLease).not.toHaveBeenCalledWith('target-1', 'pty-1', 'expired')
expect(mockAttach).toHaveBeenCalledOnce()
expect(mockWindow.webContents.send).not.toHaveBeenCalledWith('pty:exit', {
id: 'ssh:target-1@@pty-1',
code: -1
src/main/ssh/ssh-relay-session.ts
+81 -17
View File
@@ -11,7 +11,8 @@ import { SshPtyProvider } from '../providers/ssh-pty-provider'
import type { SshPtyAttachResult } from '../providers/ssh-pty-session-reattach'
import type { SshPtyDataCallback, SshPtyExitCallback } from '../providers/ssh-pty-provider-contract'
import type { SshPtyRecoveryActivationLease } from '../providers/ssh-pty-notification-routing'
import { isSshPtyExitedError, isSshPtyNotFoundError } from '../providers/ssh-pty-errors'
import { isSshPtyIdentityMismatchError, isSshPtyNotFoundError } from '../providers/ssh-pty-errors'
import { parseMatchingPtyExitedError } from '../../shared/ssh-pty-failure-tokens'
import { toAppSshPtyId, toRelaySshPtyId } from '../providers/ssh-pty-id'
import { SshFilesystemProvider } from '../providers/ssh-filesystem-provider'
import { isMethodNotFoundError } from './ssh-filesystem-stream-reader'
@@ -97,6 +98,8 @@ import {
parseRemoteOrcaCliPostOutput
} from './ssh-remote-orchestration-post-output'
import { toSshExecutionHostId, type ExecutionHostId } from '../../shared/execution-host'
import { isTerminalLeafId, makePaneKey } from '../../shared/stable-pane-id'
import { isValidTerminalTabId } from '../../shared/terminal-tab-id'
import {
SSH_AI_VAULT_LIST_SESSIONS_METHOD,
SSH_AI_VAULT_LIST_SESSIONS_TIMEOUT_MS,
@@ -195,8 +198,25 @@ type RemoteCliBridgeEnv = {
pathDelimiter?: ':' | ';'
}
type LegacyExpectedPtyIdentity = { paneKey?: string; tabId?: string }
type TargetedDeliveryRecovery = 'confirm-existing' | 'fresh-activation'
function legacyExpectedIdentityForLease(lease: {
tabId?: string
leafId?: string
}): LegacyExpectedPtyIdentity | undefined {
if (typeof lease.tabId !== 'string' || lease.tabId.length === 0) {
return undefined
}
const paneKey =
isValidTerminalTabId(lease.tabId) &&
typeof lease.leafId === 'string' &&
isTerminalLeafId(lease.leafId)
? makePaneKey(lease.tabId, lease.leafId)
: undefined
return { ...(paneKey ? { paneKey } : {}), tabId: lease.tabId }
}
function parseRecoveryComplete(params: Record<string, unknown>): PtySourceRecoveryComplete | null {
if (
typeof params.id !== 'string' ||
@@ -1986,16 +2006,24 @@ export class SshRelaySession {
? new Map<string, SshPtyLease>([[relayPtyId, activeLease]])
: new Map<string, SshPtyLease>()
const attachedLeaseIds = new Set<string>()
const terminatedLeaseIds = new Set<string>()
await this.reattachKnownPty({
ptyProvider,
ptyId: relayPtyId,
activeLeaseByPtyId,
attachedLeaseIds,
terminatedLeaseIds,
mux,
providerGeneration,
shouldContinue,
targetedDeliveryRecovery
})
if (terminatedLeaseIds.size > 0) {
await this.store.markSshRemotePtyLeasesTerminatedAsync(
this.targetId,
Array.from(terminatedLeaseIds)
)
}
if (attachedLeaseIds.size > 0 && shouldContinue()) {
await this.store.markSshRemotePtyLeasesAttachedAsync(
this.targetId,
@@ -2261,6 +2289,7 @@ export class SshRelaySession {
const activeLeaseByPtyId = new Map(activeLeases.map((lease) => [lease.ptyId, lease]))
const leasedPtyIds = activeLeases.map((lease) => lease.ptyId)
const attachedLeaseIds = new Set<string>()
const terminatedLeaseIds = new Set<string>()
// Why: after app restart ptyOwnership is empty, but durable SSH leases still describe grace-window survivors.
const ptyIds = Array.from(
new Set([
@@ -2288,6 +2317,7 @@ export class SshRelaySession {
ptyId,
activeLeaseByPtyId,
attachedLeaseIds,
terminatedLeaseIds,
mux,
providerGeneration,
shouldContinue
@@ -2307,6 +2337,12 @@ export class SshRelaySession {
await Promise.all(
Array.from({ length: Math.min(SSH_PTY_REATTACH_MAX_CONCURRENCY, ptyIds.length) }, worker)
)
if (terminatedLeaseIds.size > 0) {
await this.store.markSshRemotePtyLeasesTerminatedAsync(
this.targetId,
Array.from(terminatedLeaseIds)
)
}
if (attachedLeaseIds.size > 0 && shouldContinue()) {
await this.store.markSshRemotePtyLeasesAttachedAsync(
this.targetId,
@@ -2320,6 +2356,7 @@ export class SshRelaySession {
ptyId: string
activeLeaseByPtyId: Map<string, SshPtyLease>
attachedLeaseIds: Set<string>
terminatedLeaseIds: Set<string>
mux: SshChannelMultiplexer
providerGeneration: number
shouldContinue: () => boolean
@@ -2330,6 +2367,7 @@ export class SshRelaySession {
ptyId,
activeLeaseByPtyId,
attachedLeaseIds,
terminatedLeaseIds,
mux,
providerGeneration,
shouldContinue,
@@ -2363,7 +2401,10 @@ export class SshRelaySession {
ptyId,
recoveryRequest,
shouldContinue,
activeLeaseByPtyId.get(ptyId)?.incarnationId
activeLeaseByPtyId.get(ptyId)?.incarnationId,
activeLeaseByPtyId.get(ptyId)
? legacyExpectedIdentityForLease(activeLeaseByPtyId.get(ptyId)!)
: undefined
)
sourceActivationLease = attachResult.sourceActivationLease
if (!shouldContinue()) {
@@ -2509,7 +2550,13 @@ export class SshRelaySession {
if (!shouldContinue()) {
return
}
this.handlePtyReattachFailure(ptyId, pendingReattach, error)
this.handlePtyReattachFailure(
ptyId,
pendingReattach,
error,
activeLeaseByPtyId.get(ptyId)?.incarnationId,
terminatedLeaseIds
)
} finally {
recoveryActivationLease?.retire()
sourceActivationLease?.rollback()
@@ -2594,7 +2641,8 @@ export class SshRelaySession {
ptyId: string,
recoveryRequest: PtySourceRecoveryRequest | undefined,
shouldContinue: () => boolean,
expectedIncarnationId: string | undefined
expectedIncarnationId: string | undefined,
legacyExpectedIdentity: LegacyExpectedPtyIdentity | undefined
): Promise<SshPtyAttachResult> {
let lastError: unknown
for (let attempt = 0; attempt < 2; attempt++) {
@@ -2606,7 +2654,8 @@ export class SshRelaySession {
ptyProvider,
ptyId,
recoveryRequest,
expectedIncarnationId
expectedIncarnationId,
legacyExpectedIdentity
)
} catch (error) {
lastError = error
@@ -2616,7 +2665,12 @@ export class SshRelaySession {
if (
!shouldContinue() ||
isSshPtyNotFoundError(error) ||
isSshPtyExitedError(error) ||
isSshPtyIdentityMismatchError(error) ||
parseMatchingPtyExitedError(
error instanceof Error ? error.message : String(error),
ptyId,
expectedIncarnationId
) !== null ||
attempt === 1
) {
throw error
@@ -2631,7 +2685,8 @@ export class SshRelaySession {
ptyProvider: SshPtyProvider,
ptyId: string,
recoveryRequest: PtySourceRecoveryRequest | undefined,
expectedIncarnationId: string | undefined
expectedIncarnationId: string | undefined,
legacyExpectedIdentity: LegacyExpectedPtyIdentity | undefined
): Promise<SshPtyAttachResult> {
let timer: ReturnType<typeof setTimeout> | undefined
let timedOut = false
@@ -2645,10 +2700,14 @@ export class SshRelaySession {
timer.unref?.()
})
try {
// Only pass what we actually have: a caller with nothing to add keeps the older call shape.
const attach =
expectedIncarnationId !== undefined
? ptyProvider.attachForReconnect(ptyId, recoveryRequest, expectedIncarnationId)
expectedIncarnationId !== undefined || legacyExpectedIdentity !== undefined
? ptyProvider.attachForReconnect(
ptyId,
recoveryRequest,
expectedIncarnationId,
legacyExpectedIdentity
)
: recoveryRequest !== undefined
? ptyProvider.attachForReconnect(ptyId, recoveryRequest)
: ptyProvider.attachForReconnect(ptyId)
@@ -2679,15 +2738,20 @@ export class SshRelaySession {
private handlePtyReattachFailure(
ptyId: string,
pending: PendingPtyReattach,
error: unknown
error: unknown,
expectedIncarnationId: string | undefined,
terminatedLeaseIds: Set<string>
): void {
// An exit the relay watched is the one answer that settles it, so retire the record rather than
// re-attaching a shell known dead on every future reconnect. `markSshRemotePtyLease` normalizes
// the id itself, and terminated is not the state the recovery grant reads, so this retires the
// record without authorizing a replacement.
// An exact exit proof settles the lease; queue it for one post-worker durable retirement batch.
// `terminated` is not the recovery-grant state, so this cannot authorize a replacement.
if (isSshPtyExitedError(error)) {
this.store.markSshRemotePtyLease(this.targetId, ptyId, 'terminated')
const exitProof = parseMatchingPtyExitedError(
error instanceof Error ? error.message : String(error),
ptyId,
expectedIncarnationId
)
if (exitProof) {
terminatedLeaseIds.add(ptyId)
}
// Nothing else proves it — not even a not-found, which the relay also returns when it merely
// cannot hand this id back — so every other failure leaves the pane detached and recoverable.
src/renderer/src/components/terminal-pane/TerminalPane.tsx
+1
View File
@@ -3210,6 +3210,7 @@ function TerminalPane(
paneTransportsRef.current.get(pane.id)?.retryRecovery?.()
}}
onStartNewTerminal={recoveryState.unreachablePane?.onStartNewTerminal}
onRestoreTerminalFocus={() => pane.terminal.focus()}
/>,
pane.container,
`remote-runtime-reconnect-${pane.id}`
src/renderer/src/components/terminal-pane/TerminalPaneDisconnectedBanner.test.tsx
+75 -8
View File
@@ -4,6 +4,7 @@ import '@testing-library/jest-dom/vitest'
import { cleanup, render, screen } from '@testing-library/react'
import userEvent from '@testing-library/user-event'
import { afterEach, describe, expect, it, vi } from 'vitest'
import { TooltipProvider } from '@/components/ui/tooltip'
import { TerminalPaneDisconnectedBanner } from './TerminalPaneDisconnectedBanner'
vi.mock('@/i18n/i18n', () => ({
@@ -12,9 +13,19 @@ vi.mock('@/i18n/i18n', () => ({
afterEach(cleanup)
function renderBanner(banner: React.ReactNode): ReturnType<typeof render> {
return render(<TooltipProvider>{banner}</TooltipProvider>)
}
describe('TerminalPaneDisconnectedBanner', () => {
it('shows quiet bounded automatic recovery without a manual action', () => {
render(<TerminalPaneDisconnectedBanner phase="backoff" onReconnect={vi.fn()} />)
renderBanner(
<TerminalPaneDisconnectedBanner
phase="backoff"
onReconnect={vi.fn()}
onRestoreTerminalFocus={vi.fn()}
/>
)
expect(screen.getByText('Reconnecting to remote runtime')).toBeInTheDocument()
expect(screen.getByText(/retry for up to one minute/)).toBeInTheDocument()
@@ -24,7 +35,13 @@ describe('TerminalPaneDisconnectedBanner', () => {
it('offers one explicit reconnect action after automatic recovery stops', async () => {
const onReconnect = vi.fn()
const user = userEvent.setup()
render(<TerminalPaneDisconnectedBanner phase="disconnected" onReconnect={onReconnect} />)
renderBanner(
<TerminalPaneDisconnectedBanner
phase="disconnected"
onReconnect={onReconnect}
onRestoreTerminalFocus={vi.fn()}
/>
)
expect(screen.getByText('Remote runtime disconnected')).toBeInTheDocument()
await user.click(screen.getByRole('button', { name: 'Reconnect' }))
@@ -38,18 +55,20 @@ describe('TerminalPaneDisconnectedBanner', () => {
* frozen behind a raw error toast. It now renders here, with the two actions the design approved.
*/
describe('TerminalPaneDisconnectedBanner, unreachable SSH pane', () => {
function renderUnreachable(overrides: { onReconnect?: () => void; onStartNew?: () => void } = {}) {
const onReconnect = overrides.onReconnect ?? vi.fn()
const onStartNewTerminal = overrides.onStartNew ?? vi.fn()
render(
function renderUnreachable() {
const onReconnect = vi.fn()
const onStartNewTerminal = vi.fn()
const onRestoreTerminalFocus = vi.fn()
renderBanner(
<TerminalPaneDisconnectedBanner
phase="disconnected"
variant="ssh-pane"
onReconnect={onReconnect}
onStartNewTerminal={onStartNewTerminal}
onRestoreTerminalFocus={onRestoreTerminalFocus}
/>
)
return { onReconnect, onStartNewTerminal }
return { onReconnect, onStartNewTerminal, onRestoreTerminalFocus }
}
it('offers exactly the two approved actions', async () => {
@@ -68,6 +87,48 @@ describe('TerminalPaneDisconnectedBanner, unreachable SSH pane', () => {
expect(onStartNewTerminal).toHaveBeenCalledOnce()
})
it('returns keyboard focus to this terminal after either action', async () => {
const user = userEvent.setup()
const onReconnect = vi.fn()
const onStartNewTerminal = vi.fn()
renderBanner(
<>
<textarea aria-label="Terminal input" />
<TerminalPaneDisconnectedBanner
phase="disconnected"
variant="ssh-pane"
onReconnect={onReconnect}
onStartNewTerminal={onStartNewTerminal}
onRestoreTerminalFocus={() =>
screen.getByRole('textbox', { name: 'Terminal input' }).focus()
}
/>
</>
)
await user.click(screen.getByRole('button', { name: 'Try again' }))
expect(onReconnect).toHaveBeenCalledOnce()
expect(screen.getByRole('textbox', { name: 'Terminal input' })).toHaveFocus()
await user.click(screen.getByRole('button', { name: 'Start a new terminal' }))
expect(onStartNewTerminal).toHaveBeenCalledOnce()
expect(screen.getByRole('textbox', { name: 'Terminal input' })).toHaveFocus()
})
it('keeps both actions inside the clipped pane at its supported narrow width', () => {
renderUnreachable()
const card = screen.getByRole('status')
const banner = card.parentElement
const buttons = screen.getAllByRole('button')
expect(banner).toHaveAttribute('data-terminal-pane-disconnected-variant', 'ssh-pane')
expect(banner).toHaveClass('@container/disconnected-banner', 'inset-x-0')
expect(card).toHaveClass('max-h-full', 'overflow-y-auto')
for (const button of buttons) {
expect(button).toHaveAttribute('data-size', 'icon-xs')
}
})
// The copy constraint as an oracle. Nothing here observed the process, so the UI may not claim
// it died — and STYLEGUIDE.md forbids result verbs without result data either way.
it('never asserts the shell is dead', () => {
@@ -89,7 +150,13 @@ describe('TerminalPaneDisconnectedBanner, unreachable SSH pane', () => {
// Guards the two-action clause from a banner that always renders the second button: the
// remote-runtime variant has no record to retire, so it must keep exactly one action.
it('keeps the remote-runtime variant single-action', () => {
render(<TerminalPaneDisconnectedBanner phase="disconnected" onReconnect={vi.fn()} />)
renderBanner(
<TerminalPaneDisconnectedBanner
phase="disconnected"
onReconnect={vi.fn()}
onRestoreTerminalFocus={vi.fn()}
/>
)
expect(screen.getAllByRole('button')).toHaveLength(1)
})
src/renderer/src/components/terminal-pane/TerminalPaneDisconnectedBanner.tsx
+124 -26
View File
@@ -1,5 +1,7 @@
import { Loader2, ServerOff } from 'lucide-react'
import { useLayoutEffect, useRef, useState } from 'react'
import { Loader2, RefreshCw, ServerOff, SquareTerminal } from 'lucide-react'
import { Button } from '@/components/ui/button'
import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'
import { translate } from '@/i18n/i18n'
import type { PtyTransportRecoveryState } from './pty-transport-types'
@@ -11,19 +13,52 @@ type VisibleRecoveryPhase = Extract<
/** `ssh-pane` copy may never say the shell exited: a failed attach does not prove it did. */
type DisconnectedBannerVariant = 'remote-runtime' | 'ssh-pane'
const COMPACT_ACTIONS_MAX_WIDTH_PX = 240
function useCompactDisconnectedBannerActions(): {
bannerRef: React.RefObject<HTMLDivElement | null>
compactActions: boolean
} {
const bannerRef = useRef<HTMLDivElement>(null)
const [compactActions, setCompactActions] = useState(false)
useLayoutEffect(() => {
const pane = bannerRef.current?.parentElement
if (!pane) {
return
}
const sync = (): void => {
setCompactActions(pane.getBoundingClientRect().width < COMPACT_ACTIONS_MAX_WIDTH_PX)
}
sync()
if (typeof ResizeObserver === 'undefined') {
window.addEventListener('resize', sync)
return () => window.removeEventListener('resize', sync)
}
const observer = new ResizeObserver(sync)
observer.observe(pane)
return () => observer.disconnect()
}, [])
return { bannerRef, compactActions }
}
export function TerminalPaneDisconnectedBanner({
phase,
variant = 'remote-runtime',
onReconnect,
onStartNewTerminal
onStartNewTerminal,
onRestoreTerminalFocus
}: {
phase: VisibleRecoveryPhase
variant?: DisconnectedBannerVariant
onReconnect: () => void
onStartNewTerminal?: () => void
onRestoreTerminalFocus: () => void
}): React.JSX.Element {
const retrying = phase !== 'disconnected'
const sshPane = variant === 'ssh-pane'
const { bannerRef, compactActions } = useCompactDisconnectedBannerActions()
const title = sshPane
? translate(
@@ -55,51 +90,114 @@ export function TerminalPaneDisconnectedBanner({
'Automatic retries stopped. Reconnect to resume this terminal session.'
)
const runRecoveryAction = (action: () => void): void => {
try {
action()
} finally {
onRestoreTerminalFocus()
}
}
const reconnectLabel = sshPane
? translate(
'auto.components.terminal.pane.TerminalPaneDisconnectedBanner.tryAgainButton',
'Try again'
)
: translate(
'auto.components.terminal.pane.TerminalPaneDisconnectedBanner.reconnectButton',
'Reconnect'
)
const startNewTerminalLabel = translate(
'auto.components.terminal.pane.TerminalPaneDisconnectedBanner.startNewTerminalButton',
'Start a new terminal'
)
const reconnectButton = (
<Button
size={compactActions ? 'icon-xs' : 'sm'}
aria-label={reconnectLabel}
onClick={() => runRecoveryAction(onReconnect)}
>
{compactActions ? (
<>
<RefreshCw />
<span className="sr-only">{reconnectLabel}</span>
</>
) : (
reconnectLabel
)}
</Button>
)
const startNewTerminalButton = onStartNewTerminal ? (
<Button
size={compactActions ? 'icon-xs' : 'sm'}
variant="outline"
aria-label={startNewTerminalLabel}
onClick={() => runRecoveryAction(onStartNewTerminal)}
>
{compactActions ? (
<>
<SquareTerminal />
<span className="sr-only">{startNewTerminalLabel}</span>
</>
) : (
startNewTerminalLabel
)}
</Button>
) : null
return (
<div
className="pointer-events-none absolute inset-x-3 bottom-3 z-30 flex justify-center"
ref={bannerRef}
className="@container/disconnected-banner pointer-events-none absolute inset-x-0 top-3 bottom-3 z-30 flex items-end justify-center"
data-terminal-remote-runtime-reconnect-banner={phase}
data-terminal-pane-disconnected-variant={variant}
>
<div
className="pointer-events-auto flex w-full max-w-xl items-center gap-3 rounded-md border border-border bg-card/95 px-3 py-3 text-card-foreground shadow-xs backdrop-blur-[1px]"
className="scrollbar-sleek pointer-events-auto flex max-h-full w-[calc(100%-1.5rem)] max-w-xl items-center gap-3 overflow-y-auto rounded-md border border-border bg-card/95 px-3 py-3 text-card-foreground shadow-xs backdrop-blur-[1px] @max-[360px]/disconnected-banner:flex-col @max-[360px]/disconnected-banner:items-stretch @max-[240px]/disconnected-banner:w-auto @max-[240px]/disconnected-banner:gap-0 @max-[240px]/disconnected-banner:p-0"
role="status"
aria-live="polite"
>
<div className="flex size-8 shrink-0 items-center justify-center rounded-md border border-border bg-muted text-muted-foreground">
<div
className={`flex size-8 shrink-0 items-center justify-center rounded-md border border-border bg-muted text-muted-foreground ${
retrying
? '@max-[240px]/disconnected-banner:size-6'
: '@max-[240px]/disconnected-banner:hidden'
}`}
>
{retrying ? (
<Loader2 className="size-4 animate-spin" />
) : (
<ServerOff className="size-4" />
)}
</div>
<div className="min-w-0 flex-1">
<div className="min-w-0 flex-1 @max-[240px]/disconnected-banner:sr-only">
<div className="text-sm font-semibold">{title}</div>
<div className="mt-0.5 text-xs leading-5 text-muted-foreground">{body}</div>
</div>
{!retrying ? (
<div className="flex shrink-0 items-center gap-2">
<Button size="sm" onClick={onReconnect}>
{sshPane
? translate(
'auto.components.terminal.pane.TerminalPaneDisconnectedBanner.tryAgainButton',
'Try again'
)
: translate(
'auto.components.terminal.pane.TerminalPaneDisconnectedBanner.reconnectButton',
'Reconnect'
)}
</Button>
<div className="flex shrink-0 items-center gap-2 @max-[360px]/disconnected-banner:w-full @max-[360px]/disconnected-banner:flex-wrap @max-[360px]/disconnected-banner:justify-end @max-[240px]/disconnected-banner:gap-0">
{compactActions ? (
<Tooltip>
<TooltipTrigger asChild>{reconnectButton}</TooltipTrigger>
<TooltipContent side="top" sideOffset={4}>
{reconnectLabel}
</TooltipContent>
</Tooltip>
) : (
reconnectButton
)}
{/* Not `destructive`: the remote shell keeps running, so styling this as a loss would
itself overclaim. */}
{onStartNewTerminal ? (
<Button size="sm" variant="outline" onClick={onStartNewTerminal}>
{translate(
'auto.components.terminal.pane.TerminalPaneDisconnectedBanner.startNewTerminalButton',
'Start a new terminal'
)}
</Button>
) : null}
{compactActions && startNewTerminalButton ? (
<Tooltip>
<TooltipTrigger asChild>{startNewTerminalButton}</TooltipTrigger>
<TooltipContent side="top" sideOffset={4}>
{startNewTerminalLabel}
</TooltipContent>
</Tooltip>
) : (
startNewTerminalButton
)}
</div>
) : null}
</div>
src/renderer/src/components/terminal-pane/pty-transport.test.ts
+23
View File
@@ -243,6 +243,29 @@ describe('createIpcPtyTransport', () => {
)
})
it('drops a retired launch token from a suppressed fresh spawn', async () => {
const { createIpcPtyTransport } = await import('./pty-transport')
const spawn = window.api.pty.spawn as unknown as ReturnType<typeof vi.fn>
const transport = createIpcPtyTransport({
env: {
ORCA_PANE_KEY: 'tab-1:11111111-1111-4111-8111-111111111111',
ORCA_AGENT_LAUNCH_TOKEN: 'retired-token'
},
launchToken: 'retired-token'
})
await transport.connect({ url: '', callbacks: {}, suppressSavedStartup: true })
expect(spawn).toHaveBeenCalledWith(
expect.objectContaining({
env: { ORCA_PANE_KEY: 'tab-1:11111111-1111-4111-8111-111111111111' }
})
)
expect(spawn).toHaveBeenCalledWith(
expect.not.objectContaining({ launchToken: 'retired-token' })
)
})
it('forwards automatic resume provenance to the PTY spawn', async () => {
const { createIpcPtyTransport } = await import('./pty-transport')
const spawn = window.api.pty.spawn as unknown as ReturnType<typeof vi.fn>
src/renderer/src/components/terminal-pane/pty-transport.ts
+7 -6
View File
@@ -809,11 +809,7 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra
// Why: cwd fallback is only for fresh local spawns — reattach keeps the session's cwd and SSH transports resolve cwd on the remote host.
const shouldSendLocalCwdFallback =
cwdFallback === 'worktree' && !connectionId && !admittedSessionId
// Every constructor value that can LAUNCH OR RESUME an agent. `suppressSavedStartup` drops
// the set at once: a "fresh" shell inheriting any one of them can resume the very session
// it was meant to leave alone, and suppressing them field by field is how two were missed.
// `env`/`envToDelete` stay deliberately outside — they cannot start or resume anything on
// their own, and the spawn still needs the pane's identity environment.
// Every constructor value that can launch or resume an agent is suppressed together.
const saved = options.suppressSavedStartup
? ({} as Partial<{
command: typeof command
@@ -831,12 +827,17 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra
launchAgent,
startupCommandDelivery
}
let spawnEnv = options.env ?? env
if (options.suppressSavedStartup && spawnEnv?.ORCA_AGENT_LAUNCH_TOKEN !== undefined) {
spawnEnv = { ...spawnEnv }
delete spawnEnv.ORCA_AGENT_LAUNCH_TOKEN
}
const result = await window.api.pty.spawn({
cols: options.cols ?? 80,
rows: options.rows ?? 24,
cwd,
...(shouldSendLocalCwdFallback ? { cwdFallback } : {}),
env: options.env ?? env,
env: spawnEnv,
...((options.envToDelete ?? envToDelete)
? { envToDelete: options.envToDelete ?? envToDelete }
: {}),
src/shared/ssh-pty-failure-tokens.ts
+16
View File
@@ -3,6 +3,8 @@
// what an identity mismatch meant — main marked it, the renderer ignored the
// mark and respawned a live shell. One definition, so a change reaches both.
import { isRelayAttestedPtyIncarnationId } from './pty-incarnation'
/** The host proved the session is gone. Callers may respawn. */
export const SSH_SESSION_EXPIRED_ERROR = 'SSH_SESSION_EXPIRED'
@@ -55,6 +57,20 @@ export function parsePtyExitedError(
}
}
export function parseMatchingPtyExitedError(
message: string,
expectedId: string,
expectedIncarnationId: unknown
): { id: string; code: number; incarnationId: string } | null {
const proof = parsePtyExitedError(message)
return proof &&
proof.id === expectedId &&
isRelayAttestedPtyIncarnationId(expectedIncarnationId) &&
proof.incarnationId === expectedIncarnationId
? proof
: null
}
export function isSshPtyIdentityMismatchMessage(message: string): boolean {
return message.includes(SSH_PTY_IDENTITY_MISMATCH_ERROR) || /identity mismatch/i.test(message)
}
tests/e2e/ssh-lazy-discovery-skipped-host-restart.spec.ts
+2 -2
View File
@@ -159,7 +159,7 @@ async function reportShellPid(
return execDockerSshRelayTargetCommand(
target,
`cat ${shellQuote(stamp)} 2>/dev/null || printf ''`
)
).trim()
},
{
timeout: 90_000,
@@ -168,7 +168,7 @@ async function reportShellPid(
}
)
.toMatch(/^\d+$/)
return Number(execDockerSshRelayTargetCommand(target, `cat ${shellQuote(stamp)}`))
return Number(execDockerSshRelayTargetCommand(target, `cat ${shellQuote(stamp)}`).trim())
}
async function readWorktreeTabIds(page: Page, worktreeId: string): Promise<string[]> {