mirror of
https://github.com/stablyai/orca.git
synced 2026-10-22 00:03:47 +00:00
fix(ssh): make the unreachable-pane state reachable, and its actions work
The decisive cases for this affordance have sat at `fixme` because the state was not inducible: the card needs the SSH target CONNECTED while exactly one pane's attach fails without proving the shell gone, and every host-side fault takes the whole connection down instead. So the behaviour was only ever argued from reading, which is how several oracles here ended up green for the wrong reason. It is inducible now, using the fence this branch added for another purpose: the relay refuses an attach whose expected incarnation names a different shell, which is per-pty and leaves the connection healthy. Rewriting a pane's recorded identity while the app is closed reproduces it deterministically. Running it immediately found two defects that reading had not: The error toast painted over the card. It renders at z-50 in the same bottom strip and was suppressed only for the connection overlay, not for the pane's own card — so in the one state this affordance exists for, BOTH its actions were unclickable. "Start a new terminal" then did nothing at all: no shell, no host change, card straight back. It routes through the path that resolves the pane's owner and attaches it, and the owner is the very shell we cannot reach — so the attach fails and nothing is created. The action now refuses adoption, which is what makes it a creation. Nothing is killed; the old shell stays alive and unbound. Honest status: the gate is NOT green. The card now clears and the action runs, but shell creation is not yet observed reliably across runs. Committing so the oracle and both fixes are not lost; the remaining failure is the next work.
This commit is contained in:
Neil
+148
@@ -0,0 +1,148 @@
|
||||
# Design: make "Start a new terminal" a replacement, not a spawn
|
||||
|
||||
Supersedes the mechanism in `PR-13326-SESSION-HANDOFF-2026-08-12.md` §"Approved bounded redesign".
|
||||
Keeps its **properties**; replaces several of its **mechanisms** with smaller ones, and adds the
|
||||
thing it was missing — a way to actually run the decisive test.
|
||||
|
||||
## 1. The defect, stated once
|
||||
|
||||
"Start a new terminal" is routed through a path whose job is *"resolve this pane's owner and attach
|
||||
it; spawn only if there is none."* Creation is that path's fallback, not its purpose. The pane's
|
||||
durable binding still names the old shell at click time, so the path resolves it and adopts it.
|
||||
|
||||
I confirmed this first-hand earlier in the branch: commit `ab6ce176f3f` detects adoption after the
|
||||
fact (`freshPtyId === sessionId`) and stops the callback from unbinding the live shell. Its own
|
||||
message records that suppressing the adoption itself was left undone. So the adoption is real, and
|
||||
the current code only contains its worst consequence.
|
||||
|
||||
## 2. Why every previous fix leaked
|
||||
|
||||
Each attempt expressed "do not adopt" as **another condition inside the adopting path**. Conditions
|
||||
are forgettable and order-dependent, and this branch has three shipped proofs of that failure mode:
|
||||
|
||||
- a guard centralised into a publisher that returned `void`, so four callers paired a conditional
|
||||
publish with an unconditional `return`;
|
||||
- the same rule applied to the tombstone branch and missed on the liveness branch beside it;
|
||||
- the incarnation fence sent on the pane-driven route and not on the reconnect route.
|
||||
|
||||
Every one was "the rule exists, but this path does not ask it." A fourth condition in the same path
|
||||
is the same bet again.
|
||||
|
||||
## 3. The design, and why it removes the defect
|
||||
|
||||
### P1 — Replacement is a separate operation that cannot adopt
|
||||
|
||||
Add a main-owned entry that performs replacement only. It contains **no owner-resolution call**: it
|
||||
does not read a stable-pane owner, does not accept `sessionId`/`attachOnly`, and constructs provider
|
||||
options from a narrow main-owned allowlist (dimensions + validated pane/workspace identity).
|
||||
|
||||
*Why this fixes it:* adoption stops being a branch that must be avoided and becomes code that is not
|
||||
present. A function with no owner-resolution cannot adopt for the same reason a function with no
|
||||
socket cannot make a network call. This converts "remember to skip adoption" (liveness) into "there
|
||||
is nothing here to adopt with" (structure).
|
||||
|
||||
*Falsifiable by:* route the action back through generic spawn — the oracle in P0 must redden.
|
||||
|
||||
### P2 — Ownership moves in one durable commit, keyed on exact incarnation
|
||||
|
||||
One Store method mutates and flushes once: exact-CAS the old binding and the old active lease,
|
||||
install the new binding + incarnation + attached lease, expire the exact old lease **without
|
||||
touching its process**, bump the topology/lease versions, flush.
|
||||
|
||||
*Why:* ownership lives in the durable binding and the lease. Moved separately, there is a window
|
||||
where a racing reconnect sees a half-moved pane and restores the predecessor. One commit means the
|
||||
window does not exist rather than being narrow.
|
||||
|
||||
*Why exact incarnation:* the relay reissues `pty-N` from 1 after a reset, so any CAS keyed on ptyId
|
||||
alone can match a different process. This branch already proved that and already carries the
|
||||
primitive: host-attested `SshRemotePtyLease.incarnationId`, the relay-side `expectedIncarnationId`
|
||||
fence, and `isRelayAttestedPtyIncarnationId` filtering synthesised values.
|
||||
|
||||
*Falsifiable by:* split the commit in two flushes and drive a reconnect between them.
|
||||
|
||||
### P3 — Ambiguity leaks, never kills
|
||||
|
||||
Provider/relay shutdown identifies a PTY by id only. After a dispatched spawn whose outcome is
|
||||
ambiguous, we cannot prove which incarnation a shutdown would reach. So: clean up a failed new shell
|
||||
only when its exact incarnation is provable; otherwise leave it running and unbound for the existing
|
||||
cleanup surface to reap.
|
||||
|
||||
*Why:* the costs are asymmetric. Leaking costs an orphan shell that a user can already list and
|
||||
terminate. Killing the wrong incarnation destroys a live agent session — the exact harm this whole
|
||||
program exists to prevent.
|
||||
|
||||
*Falsifiable by:* make the ambiguous path call shutdown; the oracle asserting zero kill RPCs on an
|
||||
unprovable outcome must redden.
|
||||
|
||||
### P4 — The successor is invisible until it is the owner
|
||||
|
||||
Spawn the replacement hidden and install its output handlers only after P2 commits.
|
||||
|
||||
*Why:* a new shell reusing the old raw id could otherwise deliver bytes into the predecessor's
|
||||
model. Ordering removes the window without a new quarantine subsystem.
|
||||
|
||||
### P5 — Stale renderer work does nothing
|
||||
|
||||
Guard every completion on: not disposed, current transport identity, current action generation. A
|
||||
stale callback must not clear a binding, republish the card, or kill a result. Main owns the durable
|
||||
transition, so the renderer never clears the old binding on success.
|
||||
|
||||
## 4. What I am deliberately NOT building, and the risk of each
|
||||
|
||||
The handoff prescribes more machinery. I judge these unnecessary for this defect; each is a
|
||||
reversible decision if an oracle shows otherwise.
|
||||
|
||||
| Prescribed | Decision | Reasoning | Risk if I am wrong |
|
||||
|---|---|---|---|
|
||||
| Opaque ticket bound to ~11 fields (sender, both generations, worktree, tab, leaf, pane key, both ids, both incarnations, expiry, 5-state machine) | **Replace** with an operation id + expected-owner `{ptyId, incarnationId}` CAS | The renderer is not a trust boundary — it can already open terminals. The ticket's real jobs are staleness and idempotency, which the CAS and op id give directly. Eleven bindings add eleven invalidation paths, and invalidation bugs are the same class we are trying to leave | A forged/stale request creates one blank shell for a pane the user owns — recoverable, and the CAS still refuses to move ownership off an unexpected incumbent |
|
||||
| A new shared owner mutex | **Reuse** `paneSpawnReservationsByOwnerKey` (already per-pane) and the runtime pane-create claim | Serialisation already exists at exactly this granularity; a second lock invites lock-order bugs | Two operations interleave on one pane; the P2 CAS still refuses the loser |
|
||||
| A quarantine subsystem for provider output | **Reuse** hidden spawn + install-after-commit ordering | Achieves the same property with existing options | Bytes reach the wrong model; caught by the P4 oracle |
|
||||
|
||||
## 5. The part the previous session was missing: the decisive test can now run
|
||||
|
||||
`tests/e2e/ssh-disconnected-pane-affordance.spec.ts` holds the decisive case at `test.fixme` because
|
||||
**the state was not inducible**: it needs the SSH target CONNECTED while exactly one pane's
|
||||
`pty.attach` fails with an error that does not prove the shell gone. Host faults tried (SIGSTOP the
|
||||
relay) take the whole connection down and produce the connection overlay instead.
|
||||
|
||||
That state is now inducible, by a mechanism this branch added for another reason. The relay refuses
|
||||
an attach whose `expectedIncarnationId` names a different shell, with
|
||||
`PTY "<id>" identity mismatch` — per-pty, connection healthy, and explicitly not proof of death
|
||||
(`SSH_PTY_IDENTITY_MISMATCH` never grounds a respawn).
|
||||
|
||||
So the test can seed the pane's lease with an incarnation that is not the host's, reconnect, and get
|
||||
exactly the state the banner needs — deterministically, with no timing fault.
|
||||
|
||||
**This is the highest-value item in the plan and it comes first.** The previous session looped
|
||||
because every claim was adjudicated by reading code; with the oracle running, the design is
|
||||
*demonstrated* instead of argued, and a regression reddens instead of being found in review.
|
||||
|
||||
## 6. Goalposts
|
||||
|
||||
Each is a behaviour with a stated mutation that must redden it. G1 is a prerequisite for believing
|
||||
any of the others.
|
||||
|
||||
| # | Goalpost | Mutation that must redden it |
|
||||
|---|---|---|
|
||||
| **G1** | The decisive E2E runs unskipped: with the target connected, one pane unreachable, clicking "Start a new terminal" yields exactly one new remote shell, the pane count is unchanged, and the host transcript shows no second agent launch | Remove the induction; the test can no longer reach the banner |
|
||||
| **G2** | The action never adopts: the pane's shell id/incarnation after the click differs from before, and the new shell has no command, resume metadata, launch token, agent, or startup delivery | Route the action back through generic `pty:spawn` |
|
||||
| **G3** | Ownership moves atomically: a reconnect racing the replacement cannot restore the predecessor; binding and lease agree at every observable point | Split the durable commit into two flushes |
|
||||
| **G4** | Ambiguity never kills: on an unprovable spawn outcome, zero shutdown RPCs are issued and neither process dies | Call ID-only shutdown on the ambiguous path |
|
||||
| **G5** | The successor is silent until it owns the pane: no provider bytes reach the predecessor's model, including when the raw id is reused | Install output handlers before the commit |
|
||||
| **G6** | Stale renderer work is inert: a disposed or superseded callback clears no binding, republishes no card, kills no result | Drop the generation guard on the completion path |
|
||||
|
||||
## 7. Sequence
|
||||
|
||||
0. Remove the rejected six-file WIP (targeted edits, not a broad restore — the worktree is dirty).
|
||||
1. **G1** — induction + un-`fixme`. Nothing else is believable until this runs.
|
||||
2. **G2** — the separate replacement operation.
|
||||
3. **G3** — the single durable commit.
|
||||
4. **G4/G5/G6** — ambiguity, ordering, staleness.
|
||||
5. Adversarial review on both models, then push.
|
||||
|
||||
## 8. Stop conditions
|
||||
|
||||
- If G1 cannot be made to run, stop and report. Building 2–6 without it repeats the previous loop.
|
||||
- If exact cleanup cannot be achieved within existing local/provider contracts, take the leak (P3).
|
||||
Do **not** add `expectedIncarnationId` to remote shutdown — that is a separate, capability-
|
||||
negotiated wire change with its own review.
|
||||
+823
@@ -0,0 +1,823 @@
|
||||
# STA-3077 / PR #13326 session handoff
|
||||
|
||||
> **Status:** paused at an approved bounded redesign; not release-ready.
|
||||
>
|
||||
> **Review/fix round:** 6.
|
||||
>
|
||||
> **Do not continue the current six-file implementation.** It is a rejected,
|
||||
> uncommitted grant-map approach. Remove it with `apply_patch`, then implement
|
||||
> the dedicated main-owned replacement transaction described below.
|
||||
|
||||
This is the continuation entrypoint for the current PR. It records the whole
|
||||
session at the point the user asked to pause, including what shipped to the PR
|
||||
branch, what failed review, the agreed design boundary, and the next safe work.
|
||||
|
||||
## Executive summary
|
||||
|
||||
The session started with STA-3077's SSH reconnect failure class: reconnects
|
||||
could graft panes the user never created, accumulate remote shells, fabricate
|
||||
process death from an attach failure, or let stale identities affect a
|
||||
successor. The branch now contains extensive fixes, tests, design records, and
|
||||
two user-visible recovery actions for an unreachable pane.
|
||||
|
||||
The pushed candidate is still incorrect in one important action. “Start a new
|
||||
terminal” invokes generic `pty:spawn` while the old durable pane binding remains.
|
||||
Main can adopt that old owner, so the action can reopen the old shell instead of
|
||||
creating the one blank shell the user requested.
|
||||
|
||||
An attempted six-file fix added `freshSpawnForUnreachablePtyId` to generic spawn
|
||||
and a process-global grant map. Three independent read-only audits rejected it.
|
||||
The common verdict was that replacement must be a dedicated local transaction
|
||||
owned by Electron main, with exact incarnation identity, one durable Store
|
||||
commit, runtime ownership transfer, output quarantine, reconnect fencing, and
|
||||
renderer generation guards.
|
||||
|
||||
The user approved this bounded redesign. It applies only to the broken
|
||||
unreachable-pane replacement operation. It is not authority to restart the
|
||||
earlier whole-system architecture effort.
|
||||
|
||||
## Repository checkpoint
|
||||
|
||||
Verified before this handoff was written:
|
||||
|
||||
| Item | Value |
|
||||
| ----------------------------------- | ---------------------------------------------------- |
|
||||
| Worktree | `/Users/nwparker/orca/workspaces/orca/eye-React-185` |
|
||||
| Local branch | `nwparker/sta-3077-reattach-pane-cardinality` |
|
||||
| HEAD | `df6edfcebd2ea401a6feda1fccf31189e57eceda` |
|
||||
| HEAD subject | `fix(ui): center narrow terminal recovery actions` |
|
||||
| Actual PR | `stablyai/orca#13326` |
|
||||
| Actual PR ref | `origin/nwparker/sta-3077-d-affordance-and-review` |
|
||||
| Actual PR ref SHA | exactly `df6edfcebd2ea401a6feda1fccf31189e57eceda` |
|
||||
| Configured upstream | `origin/nwparker/sta-3077-reattach-pane-cardinality` |
|
||||
| Configured upstream SHA | `518dac34415e0f67883461c3f58e5586209c7f24` |
|
||||
| Local `origin/main` | `889dd4c7d2fe255aac552e4dcc91204279dc022c` |
|
||||
| Merge base with local `origin/main` | `58a926170cd99baf8920a83a86799bfbd307d735` |
|
||||
| Local `origin/main...HEAD` | 10 commits on main / 109 commits on HEAD |
|
||||
|
||||
The configured upstream is stale. Its `[ahead 387]` status is not the PR's
|
||||
push state. The PR ref is exactly at HEAD. When the work is genuinely ready,
|
||||
push explicitly:
|
||||
|
||||
```bash
|
||||
git push origin HEAD:nwparker/sta-3077-d-affordance-and-review
|
||||
```
|
||||
|
||||
Before this document, the worktree had six modified tracked files, no untracked
|
||||
files, and a `+163/-11` uncommitted diff. `git diff --check` was clean. No
|
||||
product test, typecheck, lint, build, Electron, or E2E command was run against
|
||||
that dirty state.
|
||||
|
||||
## The intended outcome
|
||||
|
||||
For a pane whose old SSH terminal cannot currently be reached, the UI offers:
|
||||
|
||||
- **Try again:** attempt to reattach the exact existing shell without claiming
|
||||
it exited or spawning a replacement.
|
||||
- **Start a new terminal:** keep the same pane and leaf, create exactly one
|
||||
genuinely blank shell, move durable and runtime pane ownership to it, and
|
||||
leave the unproven old process alive but unbound.
|
||||
|
||||
The second action must never:
|
||||
|
||||
- silently adopt or resume the old shell;
|
||||
- resume an agent, command, launch token, startup sequence, or saved telemetry;
|
||||
- create another pane or tab;
|
||||
- kill a process whose exact incarnation is not proven;
|
||||
- let a reconnect restore the predecessor after replacement wins;
|
||||
- let stale renderer work clear, kill, or republish the successor;
|
||||
- expose provider output before the replacement commits; or
|
||||
- add replacement intent to SSH, relay, provider, paired-runtime, or stream
|
||||
wire contracts.
|
||||
|
||||
## What was done during the session
|
||||
|
||||
The branch has 109 first-parent commits over its current merge base, including
|
||||
two large updates from `origin/main`. The useful progression is below; use the
|
||||
git history for the complete commit-by-commit record.
|
||||
|
||||
### 1. Reconnect and persistence containment
|
||||
|
||||
The early work stopped SSH reconnect from treating persisted rows as permission
|
||||
to create UI, made pane binding authoritative during duplicate arbitration,
|
||||
and added rollback around durable lease retirement.
|
||||
|
||||
Representative commits:
|
||||
|
||||
- `5d8bb18fd3f` — stop reconnect from grafting panes and stacking leases.
|
||||
- `ae134995c13` — heal duplicate pane leases from older state.
|
||||
- `ccb082740fc` — let the durable pane binding outrank recency.
|
||||
- `8d9aeb55e03` — roll back lease retirement when durable writing fails.
|
||||
- `03d794a0f90` — pass `mayCreate: false` from the production reattach writer.
|
||||
- `a7558a837ca`, `13ca7630116`, and `8ec025e4e46` — add and strengthen pane,
|
||||
process, binding, and lease census oracles.
|
||||
|
||||
### 2. Unknown is not death
|
||||
|
||||
The session removed several paths that inferred a dead process from missing,
|
||||
unavailable, timed-out, or disconnected state.
|
||||
|
||||
Representative commits:
|
||||
|
||||
- `eec807d7594` — stop respawning a shell that may still be running.
|
||||
- `356d6b52b24` — apply positive-proof requirements to both reattach paths.
|
||||
- `b9d58423121` and `7589df8b348` — let PTY liveness remain unknown.
|
||||
- `7cd7fef927f` — a reattach not-found is not itself proof of shell death.
|
||||
- `e39514e9ab2` and `41613b5c84c` — distinguish bare not-found from a relay-
|
||||
proven exit and retire a lease only in the latter case.
|
||||
|
||||
This work also stopped one exhausted delivery recovery from tearing down every
|
||||
session on an SSH host (`07e382ec196`) and made parked delivery recoveries
|
||||
expire/re-enter correctly (`124e00e8a83`, `601604d17b1`).
|
||||
|
||||
### 3. Exact operation and incarnation fencing
|
||||
|
||||
The branch added or strengthened stale-operation refusal, host-attested shell
|
||||
identity, remembered-exit identity, and reconnect fences.
|
||||
|
||||
Representative commits:
|
||||
|
||||
- `0de5994be91` and `980e5d846b5` — fence stale writes, resize, and signals.
|
||||
- `235e45ad0f2` — record host-attested shell identity on a lease.
|
||||
- `30441f1ebbb` — fence recycled relay PTY IDs by shell identity.
|
||||
- `8f0c8e3ed35`, `42cbe34523f`, `c14ba64d9dc`, and `0cb187f6035` — require an
|
||||
exit proof to name the shell for which it grants replacement authority.
|
||||
- `5793f186e96`, `be02c4e7f5c`, and `901dd06b8d1` — apply incarnation and pane
|
||||
fences on reconnect, including clients unable to name a shell.
|
||||
- `c8d08e1b215` — strengthen the incarnation write-fence regression oracle.
|
||||
|
||||
These improvements are valuable, but the round-6 review found that raw PTY ID
|
||||
is still used as identity in important lease/runtime/cleanup paths. The new
|
||||
replacement transaction must finish that exact-incarnation work for its own
|
||||
surface rather than assuming the existing fencing is complete.
|
||||
|
||||
### 4. Pane arbitration and rollback hardening
|
||||
|
||||
Lease arbitration moved from tab-level assumptions toward the actual pane leaf,
|
||||
and rollback was hardened so an already-expired duplicate cannot be revived.
|
||||
|
||||
Representative commits:
|
||||
|
||||
- `b92498468a5` — arbitrate on the pane, not the tab.
|
||||
- `1697b3be163` — supersede on the leaf.
|
||||
- `7724a950ecd` — mutate only the plane that supplied the supersession proof.
|
||||
- `21ad47c0bc5` — fence duplicate-lease rollback and add a regression oracle.
|
||||
|
||||
The existing `upsertSshRemotePtyLease` key remains `(targetId, ptyId)`. That is
|
||||
insufficient when an old and a new process reuse the same raw ID but have
|
||||
different incarnations. Replacement requires exact lease identity.
|
||||
|
||||
### 5. Unreachable-pane product behavior
|
||||
|
||||
The renderer now presents a provider-neutral disconnected card with two
|
||||
explicit actions and copy that does not claim an unobserved exit.
|
||||
|
||||
Representative commits:
|
||||
|
||||
- `5e1b57bb2f9` — add the two-action disconnected pane affordance.
|
||||
- `d27927b84b5` — remove a remote-host-only assumption from its copy.
|
||||
- `037ee535b8a` — centralize unreachable-pane guards.
|
||||
- `f7eb6f26656` — suppress the entire saved startup set for a fresh shell.
|
||||
- `b46a1f8773e` — avoid stranding a local pane after restore failure.
|
||||
- `ab6ce176f3f` — do not unbind a live shell that generic spawn adopted.
|
||||
- `df6edfcebd2` — center the recovery actions in narrow panes.
|
||||
|
||||
The last item in that list makes the pushed behavior internally safer but also
|
||||
exposes the unresolved product bug: generic spawn can adopt the incumbent, so
|
||||
“Start a new terminal” is not guaranteed to start anything new.
|
||||
|
||||
### 6. Tests, design records, and platform evidence
|
||||
|
||||
The session added substantial focused and E2E coverage for local restart,
|
||||
daemon/WSL survival, SSH pane cardinality, two-host isolation, `MaxSessions=1`,
|
||||
mixed-version decoding, lease termination, relay exit proof, and renderer
|
||||
recovery behavior. The main records are:
|
||||
|
||||
- `docs/reference/terminal-session-behavior-contract.md`
|
||||
- this folder's `goalposts.md`, `design-final-detail.md`, and `resume-plan.md`
|
||||
- `src/main/ipc/pty.test.ts`
|
||||
- `src/main/ipc/pty-superseded-operation-fence.test.ts`
|
||||
- `src/main/persistence.test.ts`
|
||||
- `src/main/persistence-ssh-lease-termination.test.ts`
|
||||
- `src/main/runtime/orca-runtime.test.ts`
|
||||
- `src/main/ssh-reattach-pane-cardinality.test.ts`
|
||||
- `src/main/ssh/ssh-relay-reattach-exit-proof.test.ts`
|
||||
- `src/main/ssh/ssh-relay-session-reconnect-incarnation.test.ts`
|
||||
- `src/renderer/src/components/terminal-pane/pty-connection.test.ts`
|
||||
- `src/renderer/src/components/terminal-pane/pty-transport.test.ts`
|
||||
- `src/renderer/src/components/terminal-pane/TerminalPaneDisconnectedBanner.test.tsx`
|
||||
- `tests/e2e/ssh-disconnected-pane-affordance.spec.ts`
|
||||
|
||||
Historical receipts in `goalposts.md` include discriminating native macOS,
|
||||
Linux, Windows, physical WSL, daemon, and Docker work at earlier candidate
|
||||
SHAs. They are historical evidence, not a current-HEAD validation receipt.
|
||||
|
||||
Important proof corrections were also recorded during the session:
|
||||
|
||||
- a guard tested only through a direct Store call can still have no production
|
||||
caller;
|
||||
- a green E2E that also passes without the fix is a forward guard, not proof;
|
||||
- the disconnected-pane state is not currently inducible with the existing
|
||||
Docker host faults; and
|
||||
- the first recovery grant and a later pane-binding fold were deleted after
|
||||
review showed their premises were false or their path was unreachable.
|
||||
|
||||
## The proven current bug
|
||||
|
||||
At pushed HEAD, the renderer's disconnected action calls `startFreshSpawn`,
|
||||
which calls the normal transport `connect`, which invokes generic `pty:spawn`.
|
||||
The old durable binding is still present. Generic spawn resolves stable pane
|
||||
ownership and may attach/adopt that incumbent rather than create a blank shell.
|
||||
|
||||
Consequences:
|
||||
|
||||
- “Start a new terminal” can reopen the old terminal.
|
||||
- A same-ID/new-incarnation result is not representable in the current renderer
|
||||
result contract.
|
||||
- The action's success path may compare or clear ownership by raw PTY ID.
|
||||
- The full product oracle is not running.
|
||||
|
||||
The decisive E2E is
|
||||
`tests/e2e/ssh-disconnected-pane-affordance.spec.ts`, test “starting a new
|
||||
terminal adds exactly one shell and leaves the panes alone,” beginning near line 314. It is still unconditionally held by
|
||||
`test.fixme(true, UNREACHABLE_PANE_INDUCTION_UNAVAILABLE)` near line 318.
|
||||
|
||||
## Dirty WIP that must be removed
|
||||
|
||||
The six uncommitted files implement the rejected
|
||||
`freshSpawnForUnreachablePtyId`/grant-map design:
|
||||
|
||||
| File | Uncommitted behavior |
|
||||
| ------------------------------------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||||
| `src/main/ipc/pty.ts` | Adds a max-256 process-global grant map, records a raw `StablePaneOwner` after attach failure, accepts replacement intent on generic spawn, skips adoption, and performs best-effort raw-ID cleanup. |
|
||||
| `src/preload/api-types.ts` | Adds `freshSpawnForUnreachablePtyId` to generic spawn. |
|
||||
| `src/preload/index.ts` | Forwards that generic spawn option. |
|
||||
| `src/renderer/src/components/terminal-pane/pty-connection.ts` | Passes the old `sessionId` as replacement authority. |
|
||||
| `src/renderer/src/components/terminal-pane/pty-transport-types.ts` | Adds the option to the generic transport contract. |
|
||||
| `src/renderer/src/components/terminal-pane/pty-transport.ts` | Forwards it to generic spawn and suppresses telemetry with saved startup. |
|
||||
|
||||
Why it was rejected:
|
||||
|
||||
1. Raw PTY ID is not process identity; same-ID/different-incarnation leases and
|
||||
runtime owners collapse.
|
||||
2. Renderer input becomes replacement authority on a broad generic IPC.
|
||||
3. Grants have no TTL, sender binding, one-shot operation state, reload
|
||||
invalidation, or post-spawn revalidation.
|
||||
4. Concurrent pane reservations can return another operation's result rather
|
||||
than re-resolving the caller's intent.
|
||||
5. Binding and lease changes flush separately, so no atomic replacement exists.
|
||||
6. Provider output can reach the old model before persistence/runtime commit.
|
||||
7. Generic spawn still accepts renderer-derived environment and other startup
|
||||
data, so it is not a guaranteed blank shell.
|
||||
8. Runtime pane ownership is registered, not exactly transferred; lookup can
|
||||
continue returning the old record.
|
||||
9. Renderer callbacks remain authoritative after dispose or a newer attempt.
|
||||
10. Failure cleanup calls ID-only shutdown and can kill the wrong incarnation.
|
||||
11. The grant is minted on only one attach-failure path, while the UI action can
|
||||
be published from other paths, leaving a visible dead action.
|
||||
12. Replacement intent leaks into `PtySpawnOptions`-adjacent contracts, making
|
||||
remote-wire and mixed-version reasoning unnecessarily dangerous.
|
||||
|
||||
No uncommitted tests cover this WIP, and no test was run against it.
|
||||
|
||||
Remove only these WIP edits with `apply_patch`. Do not use `git reset`,
|
||||
`git checkout --`, or another broad restoration command in this dirty worktree.
|
||||
|
||||
## Round-6 independent review result
|
||||
|
||||
Three read-only reviews converged:
|
||||
|
||||
- **Relay/wire review:** request changes. It identified raw-ID identity,
|
||||
ambiguous cleanup, missing atomic Store commit, precommit output publication,
|
||||
broad IPC authority, incomplete blank-shell enforcement, and wire-
|
||||
compatibility blockers.
|
||||
- **Targeted transaction review:** recommended a dedicated IPC, owner-keyed
|
||||
mutex, token-keyed idempotency, combined Store transaction, reconnect CAS,
|
||||
post-image inspection on flush error, and deterministic race tests.
|
||||
- **Final readiness audit:** request changes. It independently found the same
|
||||
persistence/runtime/grant/incarnation/renderer/cleanup defects and one hard
|
||||
feasibility boundary: existing SSH shutdown is raw-ID-only.
|
||||
|
||||
All three reviews were read-only. They ran no tests or builds and changed no
|
||||
files.
|
||||
|
||||
## Approved bounded redesign
|
||||
|
||||
### 1. Main-issued replacement ticket
|
||||
|
||||
When main classifies an attach as unreachable but not proven dead, return a
|
||||
typed local result containing an opaque ticket. The ticket is authority, not a
|
||||
renderer-supplied PTY ID.
|
||||
|
||||
Bind it to:
|
||||
|
||||
- the issuing `webContents`/sender;
|
||||
- connection and provider generation;
|
||||
- worktree or folder-workspace identity;
|
||||
- tab, leaf, and pane key;
|
||||
- exact old application PTY ID and provider-native ID;
|
||||
- persisted and runtime incarnation IDs;
|
||||
- exact active lease identity;
|
||||
- issuance generation and short expiry;
|
||||
- a random operation ID; and
|
||||
- state `issued | running | committed | rejected | unresolved` plus the
|
||||
idempotent result where applicable.
|
||||
|
||||
Invalidate tickets on renderer reload/destruction, provider replacement,
|
||||
connection re-registration, expiry, or exact-owner change. A consumed or
|
||||
rejected ticket must not become valid again after any request was dispatched.
|
||||
|
||||
### 2. Dedicated local IPC
|
||||
|
||||
Add a local-only API such as:
|
||||
|
||||
```ts
|
||||
pty.replaceUnreachablePane({ ticketId, cols, rows })
|
||||
```
|
||||
|
||||
It should return a structured result such as:
|
||||
|
||||
```ts
|
||||
{
|
||||
;(ptyId, incarnationId, operationId)
|
||||
}
|
||||
```
|
||||
|
||||
It must not route through generic `pty:spawn` or `PtyTransport.connect`, and it
|
||||
must not add fields to provider, SSH, relay, paired-runtime, or stream payloads.
|
||||
Authenticate the sender as the main renderer window before ticket lookup.
|
||||
|
||||
### 3. Shared owner lock and idempotency
|
||||
|
||||
Use one host/worktree/pane owner mutex for:
|
||||
|
||||
- ordinary pane spawn/adoption;
|
||||
- unreachable-pane replacement; and
|
||||
- relay reconnect's final owner activation.
|
||||
|
||||
Each waiter must re-resolve its own intent after acquiring the lock. It may not
|
||||
return another operation's result merely because it waited on the same lock.
|
||||
|
||||
Use the ticket's operation ID for idempotency. A double-click, lost response,
|
||||
or retry while running awaits the same operation and returns the same result.
|
||||
It must create at most one shell.
|
||||
|
||||
### 4. Main-constructed blank spawn
|
||||
|
||||
Under the owner lock, re-read the workspace and re-derive CWD. Repeat folder-
|
||||
workspace path validation at that moment; do not trust the path captured when
|
||||
the ticket was issued.
|
||||
|
||||
Construct provider spawn options from a narrow main-owned allowlist. Exclude:
|
||||
|
||||
- command and command delivery;
|
||||
- `sessionId`, `attachOnly`, or resume metadata;
|
||||
- launch config, token, agent, or telemetry;
|
||||
- arbitrary renderer environment or environment deletion;
|
||||
- shell override;
|
||||
- startup command delivery; and
|
||||
- sequenced-startup environment.
|
||||
|
||||
Only the validated pane/workspace identity, dimensions, and provider-required
|
||||
blank-shell metadata may cross the provider boundary.
|
||||
|
||||
### 5. Exact incarnation lease identity
|
||||
|
||||
Lease identity for this transaction must include incarnation, not only
|
||||
`(targetId, ptyId)`. Old `pty-1/inc-A` and new `pty-1/inc-B` must coexist as:
|
||||
|
||||
- old exact lease: expired/unbound; and
|
||||
- new exact lease: attached/bound.
|
||||
|
||||
Every mark, remove, attach, rollback, and reconnect CAS touched by replacement
|
||||
must name the exact lease incarnation. Preserve compatibility with persisted
|
||||
legacy rows that lack optional incarnation data; unknown identity may not
|
||||
authorize destructive cleanup.
|
||||
|
||||
### 6. One Store transaction
|
||||
|
||||
Add one Store method, for example `replaceUnreachableSshPaneOwner`, that under
|
||||
one mutation and one flush:
|
||||
|
||||
1. exact-CASes the old durable pane binding;
|
||||
2. exact-CASes the old active lease;
|
||||
3. installs the new binding and incarnation;
|
||||
4. installs the new exact attached lease;
|
||||
5. expires the exact old lease without shutting down its process;
|
||||
6. advances topology and lease mutation versions, including affected already-
|
||||
expired siblings; and
|
||||
7. flushes once.
|
||||
|
||||
Snapshot every mutated in-memory field before starting.
|
||||
|
||||
Because durable-file replacement renames the new image before directory fsync,
|
||||
a thrown flush is not proof that disk stayed old. Inspect the persisted post-
|
||||
image before deciding what happened:
|
||||
|
||||
- **Old image:** restore memory and clean only a provably exact new process.
|
||||
- **New image:** treat the transaction as committed.
|
||||
- **Mixed or unreadable:** kill neither process; mark the operation unresolved,
|
||||
quarantine publication, and reconcile from disk/provider evidence.
|
||||
|
||||
### 7. Runtime ownership replacement
|
||||
|
||||
Add a non-exit runtime primitive along these lines:
|
||||
|
||||
```ts
|
||||
runtime.replaceTerminalPaneOwner({
|
||||
paneKey,
|
||||
worktreeId,
|
||||
expected: { ptyId, incarnationId },
|
||||
replacement: { ptyId, incarnationId }
|
||||
})
|
||||
```
|
||||
|
||||
It must:
|
||||
|
||||
- exact-CAS the current owner;
|
||||
- clear the old record's pane ownership without `onPtyExit`;
|
||||
- leave the old process live and unbound;
|
||||
- move the pane/leaf/handle lookup to the replacement;
|
||||
- invalidate old handles so they cannot silently target the successor; and
|
||||
- correctly reset ID-keyed state when raw PTY ID is reused with a new
|
||||
incarnation.
|
||||
|
||||
Prepare and validate runtime replacement before the Store commit. Make the
|
||||
final in-memory step synchronous and designed not to fail. If an unexpected
|
||||
post-durable failure still occurs, never roll durable ownership back to the old
|
||||
process and never kill either shell; quarantine and retry publication/
|
||||
reconciliation idempotently.
|
||||
|
||||
### 8. Quarantine provider output
|
||||
|
||||
Capture the provider object and generation before spawn and recheck them after
|
||||
every await. Quarantine output for the exact
|
||||
`{ provider generation, ptyId, incarnationId }` until durable and runtime
|
||||
replacement commit.
|
||||
|
||||
Only then activate source delivery and publish the new runtime owner. A new
|
||||
shell reusing the old raw ID must never send bytes into the predecessor model.
|
||||
|
||||
### 9. Fence relay reconnect
|
||||
|
||||
Relay reconnect must acquire the same owner lock after network attach and,
|
||||
before changing ownership or delivery, exact-CAS:
|
||||
|
||||
- the current pane binding;
|
||||
- the lease incarnation; and
|
||||
- the provider/connection generation.
|
||||
|
||||
Only a winning reconnect may set ownership, restore incarnation, activate
|
||||
source delivery, or register runtime state. A reconnect for the predecessor
|
||||
must be unable to publish it after replacement commits.
|
||||
|
||||
### 10. Renderer adoption rules
|
||||
|
||||
The renderer calls only the dedicated API. Guard every completion/failure by:
|
||||
|
||||
- connection object not disposed;
|
||||
- current transport identity;
|
||||
- current action generation; and
|
||||
- matching ticket/operation generation.
|
||||
|
||||
Accept same-raw-ID/new-incarnation as a real successor. Adopt only the exact
|
||||
structured committed result. A stale or disposed callback does nothing: it does
|
||||
not clear a binding, republish the card, or kill a result. Main owns the durable
|
||||
transition, so the renderer must never clear the old durable binding after
|
||||
success.
|
||||
|
||||
## Hard cleanup boundary and selected policy
|
||||
|
||||
Current provider and SSH relay shutdown APIs identify only a PTY ID. If spawn
|
||||
returns `pty-1/inc-B`, then reconnect races and `pty-1` may refer to a different
|
||||
incarnation by the time rollback runs. Calling `shutdown('pty-1')` is not exact
|
||||
cleanup.
|
||||
|
||||
The safe policy for this PR is:
|
||||
|
||||
- do not add replacement intent to the remote wire;
|
||||
- use existing capability-gated create-operation replay and host-attested
|
||||
incarnation when they provide conclusive identity;
|
||||
- clean a failed new shell only when its exact incarnation is provable through
|
||||
a provider-owned rollback handle or equivalent existing evidence;
|
||||
- otherwise park/quarantine the ambiguous shell for reconciliation;
|
||||
- never blind-retry an ambiguous legacy spawn; and
|
||||
- never risk killing the incumbent merely to avoid a leak.
|
||||
|
||||
Legacy relays without create-operation replay or host-attested incarnation must
|
||||
fail closed or leave the outcome unresolved. If implementation cannot satisfy
|
||||
this with current local/provider contracts, stop at the design gate. Do not
|
||||
silently add `expectedIncarnationId` to remote shutdown; that is a separate,
|
||||
capability-negotiated wire design and review.
|
||||
|
||||
## Transaction outcome table
|
||||
|
||||
| Outcome | Durable owner | Process action | Publication |
|
||||
| -------------------------------------------- | ----------------- | ------------------------------------------------- | ------------------------------------- |
|
||||
| Ticket/pre-spawn validation or fence failure | Old | None | Reject |
|
||||
| Spawn failure before dispatch | Old | None | Reject; ticket may be safely terminal |
|
||||
| Spawn response ambiguous after dispatch | Old or unresolved | No blind retry or ID-only shutdown | Park/reconcile |
|
||||
| Known new shell, post-spawn CAS loss | Winner or old | Stop only the exact new incarnation when provable | Publish nothing |
|
||||
| Store commit succeeds | New | Leave old process alive; expire exact old lease | Publish new |
|
||||
| Flush throws; disk has old image | Old | Clean exact new only when provable | Reject |
|
||||
| Flush throws; disk has new image | New | Leave both processes | Publish new |
|
||||
| Flush throws; disk mixed/unreadable | Unresolved | Kill neither | Quarantine/reconcile |
|
||||
| Post-commit runtime/publication failure | New | Never roll back or kill | Retry publication idempotently |
|
||||
|
||||
## Required deterministic oracles
|
||||
|
||||
These are completion requirements, not optional follow-ups.
|
||||
|
||||
### Main transaction
|
||||
|
||||
- Seed an exact failed owner, ticket, binding, and lease.
|
||||
- One explicit replacement call makes exactly one provider fresh spawn.
|
||||
- Provider options contain no session, attach, command, startup, resume,
|
||||
renderer env, launch metadata, or telemetry.
|
||||
- The same pane remains; the old shell receives no shutdown or synthetic exit.
|
||||
- Exactly one combined Store handoff occurs and runtime lookup resolves the new
|
||||
owner.
|
||||
|
||||
### Authorization and lifecycle
|
||||
|
||||
- Missing, expired, replayed, wrong-sender, wrong-host, wrong-worktree,
|
||||
wrong-pane, and renderer-reload tickets reject before provider spawn.
|
||||
- Changed/missing binding, persisted/runtime incarnation, lease, provider, or
|
||||
generation rejects before mutation.
|
||||
- Folder-workspace path changes after ticket issuance are revalidated under the
|
||||
lock.
|
||||
|
||||
### Identity and races
|
||||
|
||||
- Old and new may both be raw `pty-1` with distinct incarnations.
|
||||
- Double-click and lost-response retry create one operation and one shell.
|
||||
- Ordinary spawn racing replacement does not inherit the replacement result.
|
||||
- Reconnect before spawn yields no spawn.
|
||||
- Reconnect after spawn cannot republish the predecessor.
|
||||
- Provider replacement during every await prevents an unsafe commit.
|
||||
- A delayed old-incarnation exit cannot affect the new incarnation.
|
||||
|
||||
### Persistence and durability
|
||||
|
||||
- Success survives save/reload with exactly one flush.
|
||||
- Old exact lease is expired, new exact lease attached, and new binding exact.
|
||||
- Pre-rename failure restores old memory/disk state.
|
||||
- Post-rename failure recognizes the new image as committed.
|
||||
- Mixed/unreadable post-image kills neither process.
|
||||
- Duplicate-retirement rollback cannot revive an already-expired sibling.
|
||||
- Same raw PTY ID with different incarnations remains representable.
|
||||
|
||||
### Runtime and renderer
|
||||
|
||||
- Exact non-exit runtime rebind moves pane/leaf/handle lookup to the successor.
|
||||
- The old owner remains live but unbound and receives no exit.
|
||||
- Disposed, stale-generation, late-success, and late-failure callbacks cannot
|
||||
spawn, clear, kill, or republish over newer state.
|
||||
- Same-ID/new-incarnation structured results are adopted.
|
||||
- A disposed transport never kills a main-committed replacement.
|
||||
|
||||
### Compatibility and E2E
|
||||
|
||||
- No replacement field appears in provider, SSH, relay, runtime RPC, paired
|
||||
RPC, or stream payloads.
|
||||
- Legacy relay degradation is fail-closed/unresolved, never destructive.
|
||||
- Current-to-release and release-to-current terminal compatibility suites stay
|
||||
green.
|
||||
- Unfix the decisive Docker test and add a deterministic production seam that
|
||||
leaves the target connected while one pane attach is unreachable.
|
||||
- The E2E must prove old PID plus start ticks stay alive, exactly one blank
|
||||
shell is added, pane/leaf count is unchanged, new durable `(id,
|
||||
incarnation)` is bound, new lease is attached, old exact lease is expired,
|
||||
and no agent/resume/startup transcript is duplicated.
|
||||
|
||||
## Remaining work, in dependency order
|
||||
|
||||
### Phase 0 — resume safely and freeze the invariants
|
||||
|
||||
1. Read this document and the repository `AGENTS.md` instructions.
|
||||
2. Run the checkpoint commands below.
|
||||
3. Confirm the PR ref remains at the recorded SHA and inspect any newly arrived
|
||||
changes before editing.
|
||||
4. Treat ambiguous cleanup as unresolved rather than adding a remote wire field.
|
||||
5. If that constraint becomes infeasible, stop and return to the user instead
|
||||
of widening scope.
|
||||
|
||||
### Phase 1 — remove the rejected WIP
|
||||
|
||||
Use `apply_patch` to remove only the current six-file
|
||||
`freshSpawnForUnreachablePtyId` and grant-map edits. Preserve every committed
|
||||
change and any unrelated user work. Run `git diff --check` and verify the only
|
||||
remaining change is this handoff document before beginning the replacement.
|
||||
|
||||
### Phase 2 — build the identity and persistence foundation
|
||||
|
||||
1. Add incarnation-aware exact lease identity with legacy-row compatibility.
|
||||
2. Add exact lease lookup/mark/remove APIs required by replacement and
|
||||
reconnect.
|
||||
3. Add the single Store replacement transaction and one-flush rollback state.
|
||||
4. Add post-image inspection for pre-rename, post-rename, and mixed/unreadable
|
||||
outcomes.
|
||||
5. Write persistence tests first and demonstrate the intended failure on the
|
||||
current implementation.
|
||||
|
||||
### Phase 3 — build runtime and coordination primitives
|
||||
|
||||
1. Add the exact non-exit runtime pane-owner replacement primitive.
|
||||
2. Add the host/worktree/pane owner mutex shared by normal spawn,
|
||||
replacement, and reconnect final activation.
|
||||
3. Add operation-token idempotency independent from that lock.
|
||||
4. Update reconnect to revalidate exact binding, lease incarnation, and
|
||||
provider generation after it acquires the lock.
|
||||
5. Add same-ID/new-incarnation and delayed-old-exit tests.
|
||||
|
||||
### Phase 4 — implement the main replacement transaction
|
||||
|
||||
1. Issue typed, sender-bound, expiring, one-shot tickets from every renderer
|
||||
path that can publish the unreachable action.
|
||||
2. Add the dedicated preload/main IPC and main-window sender check.
|
||||
3. Re-derive CWD and validate folder workspaces under the lock.
|
||||
4. Construct the blank provider spawn from the main allowlist.
|
||||
5. Capture provider generation and quarantine exact new output.
|
||||
6. Revalidate after every await; prepare runtime; execute the Store commit;
|
||||
finish runtime activation; publish output/result.
|
||||
7. Implement all transition-table outcomes without ID-only cleanup.
|
||||
|
||||
### Phase 5 — adopt through the renderer
|
||||
|
||||
1. Remove replacement from generic transport/spawn types.
|
||||
2. Thread the typed ticket and structured committed result through the
|
||||
disconnected action only.
|
||||
3. Add disposed/current-transport/action-generation guards.
|
||||
4. Remove renderer-side binding clearing or committed-result killing.
|
||||
5. Verify UI layout, copy, and behavior through Electron Playwright CDP as
|
||||
required by repository instructions.
|
||||
|
||||
### Phase 6 — validate and review once
|
||||
|
||||
1. Run the focused deterministic suite.
|
||||
2. Make the Docker E2E inducible, remove its unconditional `fixme`, and prove
|
||||
it fails for the intended reason on the unfixed candidate before claiming
|
||||
it as evidence.
|
||||
3. Run typecheck, lint, full unit tests, build, reliability gates, and relevant
|
||||
SSH/daemon/cross-version regressions.
|
||||
4. Run independent repository and release-readiness reviews, including
|
||||
security, performance, mobile/backcompat, Windows, Linux/glibc, SSH, folder
|
||||
workspace, and provider-neutrality coverage.
|
||||
5. Resolve every P0/P1 and relevant P2. If another architectural invariant
|
||||
fails, pause instead of entering an unbounded seventh patch loop.
|
||||
|
||||
### Phase 7 — deliver
|
||||
|
||||
Only after the implementation and reviews are clean:
|
||||
|
||||
1. Commit the replacement work.
|
||||
2. Push explicitly to
|
||||
`origin/nwparker/sta-3077-d-affordance-and-review`.
|
||||
3. Update PR #13326's body with the transaction, compatibility behavior, and
|
||||
exact validation receipts.
|
||||
4. Resolve the seven saved review threads only when their findings are actually
|
||||
addressed.
|
||||
5. Request `@coderabbitai review` and monitor CI to completion.
|
||||
6. Use the `orca-linear` workflow to post one concise STA-3077 completion
|
||||
update and attach the PR if needed.
|
||||
|
||||
## First safe resume commands
|
||||
|
||||
Run from the primary worktree before editing:
|
||||
|
||||
```bash
|
||||
git status --short --branch --untracked-files=all
|
||||
git rev-parse HEAD
|
||||
git rev-parse origin/nwparker/sta-3077-d-affordance-and-review
|
||||
git rev-parse origin/main
|
||||
git rev-list --left-right --count origin/main...HEAD
|
||||
git diff --stat
|
||||
git diff --check
|
||||
```
|
||||
|
||||
Expected before any external state changes:
|
||||
|
||||
- HEAD and the PR ref are both `df6edfcebd2ea401a6feda1fccf31189e57eceda`;
|
||||
- the six rejected production-file edits are present;
|
||||
- this handoff file is the only additional local artifact; and
|
||||
- `git diff --check` exits 0.
|
||||
|
||||
Do not use the configured upstream to infer PR state.
|
||||
|
||||
## Suggested validation commands after implementation
|
||||
|
||||
Adjust the focused file list only when the final implementation clearly moves
|
||||
the covered behavior. Record exact commands and outcomes in the PR.
|
||||
|
||||
```bash
|
||||
pnpm exec vitest run --config config/vitest.config.ts \
|
||||
src/main/ipc/pty.test.ts \
|
||||
src/main/persistence.test.ts \
|
||||
src/main/persistence-ssh-lease-termination.test.ts \
|
||||
src/main/runtime/orca-runtime.test.ts \
|
||||
src/main/ssh-reattach-pane-cardinality.test.ts \
|
||||
src/main/ssh/ssh-relay-session.test.ts \
|
||||
src/main/ssh/ssh-relay-session-reconnect-incarnation.test.ts \
|
||||
src/main/ssh/ssh-relay-session-reattach-pane-fence.test.ts \
|
||||
src/renderer/src/components/terminal-pane/pty-connection.test.ts \
|
||||
src/renderer/src/components/terminal-pane/pty-transport.test.ts \
|
||||
src/renderer/src/components/terminal-pane/TerminalPaneDisconnectedBanner.test.tsx \
|
||||
--reporter=dot
|
||||
|
||||
pnpm typecheck
|
||||
pnpm lint
|
||||
pnpm test
|
||||
pnpm build
|
||||
```
|
||||
|
||||
For the Docker product oracle, use the repo's Electron Playwright configuration
|
||||
with `ORCA_E2E_SSH_DOCKER=1` after adding the deterministic induction seam and
|
||||
removing the test's unconditional `fixme`. Keep the run POSIX-only as the
|
||||
existing Docker fault harness requires.
|
||||
|
||||
Run the current↔release cross-version terminal suite and the relevant Docker
|
||||
SSH cardinality, `MaxSessions=1`, and two-host scripts as part of broad
|
||||
regression coverage.
|
||||
|
||||
## Validation record at pause
|
||||
|
||||
### Current redesign/WIP
|
||||
|
||||
No product validation was run after the rejected six-file WIP was introduced.
|
||||
No product validation was run during the three final read-only audits or while
|
||||
writing this handoff. Therefore this document claims no current test, typecheck,
|
||||
lint, build, Electron, Docker, cross-version, or packaging pass.
|
||||
|
||||
### Read-only checkpoint commands run for this handoff
|
||||
|
||||
The following repository inspections completed successfully:
|
||||
|
||||
```bash
|
||||
git status --short --branch
|
||||
git status --short --branch --untracked-files=all
|
||||
git rev-parse HEAD
|
||||
git branch --show-current
|
||||
git rev-parse --abbrev-ref --symbolic-full-name '@{upstream}'
|
||||
git rev-parse origin/nwparker/sta-3077-d-affordance-and-review
|
||||
git rev-parse origin/nwparker/sta-3077-reattach-pane-cardinality
|
||||
git rev-parse origin/main
|
||||
git rev-list --left-right --count origin/main...HEAD
|
||||
git merge-base origin/main HEAD
|
||||
git log --oneline --decorate -12
|
||||
git reflog --date=iso
|
||||
git diff --stat
|
||||
git diff --name-status
|
||||
git diff --check
|
||||
```
|
||||
|
||||
Observed results are recorded in the repository checkpoint above. In
|
||||
particular, `git diff --check` exited 0.
|
||||
|
||||
Historical validation details are preserved in `goalposts.md` and
|
||||
`resume-plan.md`. Those receipts belong to their recorded SHAs and must not be
|
||||
reported as validation of the final replacement candidate without rerunning
|
||||
them.
|
||||
|
||||
## Security, compatibility, migration, and release risks
|
||||
|
||||
- **IPC authority:** a renderer-supplied PTY ID is forgeable/piggybackable;
|
||||
tickets must be opaque, sender-bound, short-lived, one-shot, and revalidated.
|
||||
- **Same-ID reuse:** raw PTY ID cannot distinguish an incumbent from a
|
||||
successor. Every destructive or ownership operation needs exact incarnation.
|
||||
- **Durability:** a thrown fsync after rename can leave the new image on disk.
|
||||
Blind in-memory rollback can diverge from restart state.
|
||||
- **Remote ambiguity:** ID-only shutdown cannot safely clean same-ID reuse.
|
||||
Prefer a visible/reconcilable leak to killing an unproven process.
|
||||
- **Output isolation:** precommit bytes from a successor can corrupt the old
|
||||
pane model unless quarantined by provider generation plus incarnation.
|
||||
- **Mixed versions:** clients and hosts update independently. Keep replacement
|
||||
local; any future exact-shutdown wire field needs capability negotiation and
|
||||
both-direction live skew tests.
|
||||
- **Legacy persisted rows:** missing incarnation is unknown, not permission to
|
||||
mutate or delete. Migration must be additive and old-reader-safe.
|
||||
- **Folder and SSH workspaces:** re-resolve current paths and host generation
|
||||
under the lock; neither git worktree presence nor local filesystem access may
|
||||
be assumed.
|
||||
- **Cross-platform:** preserve macOS, Linux/glibc floor, Windows, WSL, path, and
|
||||
shortcut rules even though the Docker fault harness is POSIX-only.
|
||||
- **Performance:** avoid global scans, unbounded ticket/operation maps,
|
||||
repeated fsyncs, or hot-path lock contention. Bound retention and measure
|
||||
affected reconnect/spawn paths.
|
||||
- **Release evidence:** the key E2E remains held. The change is not release-
|
||||
ready until that product path is deterministic and discriminating.
|
||||
|
||||
## Stop conditions
|
||||
|
||||
Pause and return to the user if any of these occurs:
|
||||
|
||||
- exact cleanup appears to require an unreviewed remote-wire change;
|
||||
- the Store/runtime transaction cannot distinguish old, new, and mixed durable
|
||||
post-images;
|
||||
- replacement requires treating unknown identity as proof;
|
||||
- the deterministic E2E still cannot enter the actual production state;
|
||||
- the shared lock introduces a new global serialization or deadlock risk; or
|
||||
- the next independent review finds another architectural invariant failure.
|
||||
|
||||
The point of this handoff is to prevent another opaque loop: one bounded
|
||||
redesign, deterministic evidence, one adversarial review, then either delivery
|
||||
or a principled stop.
|
||||
+9
-1
@@ -5895,6 +5895,8 @@ export function registerPtyHandlers(
|
||||
cwd?: string
|
||||
// Why: fresh local spawns opt into recovering a saved cwd whose dir was deleted (#7239); reattach/remote need exact cwd, so the flag alone isn't sufficient.
|
||||
cwdFallback?: 'worktree'
|
||||
/** Create a shell for this pane rather than adopting the one it records. */
|
||||
createFreshShellForUnreachablePane?: boolean
|
||||
env?: Record<string, string>
|
||||
envToDelete?: string[]
|
||||
command?: string
|
||||
@@ -5994,8 +5996,14 @@ export function registerPtyHandlers(
|
||||
if (existingPaneSpawn) {
|
||||
return { ...(await existingPaneSpawn.promise), isReattach: true }
|
||||
}
|
||||
// Why the caller may refuse adoption: the unreachable-pane card offers "Start a new terminal"
|
||||
// for a pane whose recorded shell cannot be reached. Resolving an owner here makes the action
|
||||
// attach that shell first — and when it is unreachable the attach fails, the action creates
|
||||
// nothing, and the card comes back. The button is dead in the one state it is offered in.
|
||||
// Skipping the resolve is what makes it a creation: nothing is killed, and the old shell is
|
||||
// left alive and unbound for the cleanup surface.
|
||||
const earlyStablePaneOwner =
|
||||
earlyPaneKey && args.worktreeId
|
||||
earlyPaneKey && args.worktreeId && args.createFreshShellForUnreachablePane !== true
|
||||
? resolveStablePaneOwner(runtime, store, earlyPaneKey, args.worktreeId, args.connectionId)
|
||||
: null
|
||||
const earlyWorktreeId = args.worktreeId
|
||||
|
||||
@@ -1469,6 +1469,8 @@ export type PreloadApi = {
|
||||
rows: number
|
||||
cwd?: string
|
||||
cwdFallback?: 'worktree'
|
||||
/** Create a shell for this pane instead of adopting the one it records; see pty.ts. */
|
||||
createFreshShellForUnreachablePane?: boolean
|
||||
env?: Record<string, string>
|
||||
envToDelete?: string[]
|
||||
command?: string
|
||||
|
||||
@@ -943,6 +943,7 @@ const api = {
|
||||
rows: number
|
||||
cwd?: string
|
||||
cwdFallback?: 'worktree'
|
||||
createFreshShellForUnreachablePane?: boolean
|
||||
env?: Record<string, string>
|
||||
envToDelete?: string[]
|
||||
command?: string
|
||||
|
||||
@@ -2915,6 +2915,10 @@ function TerminalPane(
|
||||
resolveAgentForLeaf(contextMenuLeafId)
|
||||
)
|
||||
// Each toggle gates on its own leaf (header=active, menu=opened-over), so mixed splits show it only where chat can render.
|
||||
// The pane-level disconnected card owns the bottom strip while it is up; see the error toast below.
|
||||
const activePaneShowsUnreachableCard = Boolean(
|
||||
activePane && ptyRecoveryStatesByPaneId[activePane.id]?.unreachablePane
|
||||
)
|
||||
const activePaneCanToggleChat = canToggleChatForLeaf(activePane?.leafId ?? null)
|
||||
const contextMenuCanToggleChat = canToggleChatForLeaf(contextMenuLeafId)
|
||||
return (
|
||||
@@ -2982,8 +2986,12 @@ function TerminalPane(
|
||||
)
|
||||
})}
|
||||
{/* Why: the reconnect banner already owns SSH recovery UX; the z-50 error
|
||||
toast was painting over it (same bottom strip) with the raw ssh:connect failure. */}
|
||||
{terminalError && isActive && !showSshReconnectOverlay ? (
|
||||
toast was painting over it (same bottom strip) with the raw ssh:connect failure.
|
||||
The pane's own disconnected card owns the same strip and the same story, and the toast
|
||||
outranks it at z-50 — so it covered BOTH of its actions and made the affordance
|
||||
unclickable. Naming only the connection overlay here was the gap: the card is raised for
|
||||
a per-pane failure that leaves the connection up, which is exactly when this fires. */}
|
||||
{terminalError && isActive && !showSshReconnectOverlay && !activePaneShowsUnreachableCard ? (
|
||||
<TerminalErrorToast
|
||||
error={terminalError}
|
||||
onDismiss={dismissTerminalError}
|
||||
|
||||
@@ -556,6 +556,8 @@ type FreshSpawnOptions = {
|
||||
* connect falls back to its constructor values, so a "fresh" shell would resume the saved agent
|
||||
* session — the duplicate transcript the disconnected pane exists to prevent. */
|
||||
suppressSavedStartup?: boolean
|
||||
/** Refuse adoption of the pane's recorded shell; see the unreachable-pane action. */
|
||||
createFreshShellForUnreachablePane?: boolean
|
||||
}
|
||||
|
||||
type ColdRestoreAgentResumeStartup = PendingStartupCommand & {
|
||||
@@ -5213,7 +5215,11 @@ export function connectPanePty(
|
||||
// with, so the suppression has to be explicit.
|
||||
void startFreshSpawn(null, {
|
||||
forceBlankRestoredViewport: true,
|
||||
suppressSavedStartup: true
|
||||
suppressSavedStartup: true,
|
||||
// Refuse adoption. This pane's recorded shell is the one we could not reach, so
|
||||
// attaching it first only fails again and creates nothing — which left this button
|
||||
// doing nothing at all in the one state it is offered in.
|
||||
createFreshShellForUnreachablePane: true
|
||||
}).then((freshPtyId) => {
|
||||
if (!freshPtyId) {
|
||||
// The replacement never started. Keep the old binding and put the card back, or the
|
||||
@@ -5407,6 +5413,9 @@ export function connectPanePty(
|
||||
const outputCallbacks = captureTransportOutputCallbacks(reportError)
|
||||
const spawnedRaw = transport.connect({
|
||||
...(options.suppressSavedStartup ? { suppressSavedStartup: true } : {}),
|
||||
...(options.createFreshShellForUnreachablePane
|
||||
? { createFreshShellForUnreachablePane: true }
|
||||
: {}),
|
||||
url: '',
|
||||
cols,
|
||||
rows,
|
||||
|
||||
@@ -154,6 +154,9 @@ export type PtyTransport = {
|
||||
* Omitting the fields is not enough — connect falls back to its constructor values, so a
|
||||
* "fresh" spawn would silently resume the saved agent session. */
|
||||
suppressSavedStartup?: boolean
|
||||
/** Refuse adoption: this pane's recorded shell cannot be reached, so attaching it would fail
|
||||
* and create nothing. The old shell is left alive and unbound. */
|
||||
createFreshShellForUnreachablePane?: boolean
|
||||
launchToken?: string
|
||||
launchAgent?: TuiAgent
|
||||
startupCommandDelivery?: StartupCommandDelivery
|
||||
|
||||
@@ -837,6 +837,9 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra
|
||||
rows: options.rows ?? 24,
|
||||
cwd,
|
||||
...(shouldSendLocalCwdFallback ? { cwdFallback } : {}),
|
||||
...(options.createFreshShellForUnreachablePane
|
||||
? { createFreshShellForUnreachablePane: true }
|
||||
: {}),
|
||||
env: spawnEnv,
|
||||
...((options.envToDelete ?? envToDelete)
|
||||
? { envToDelete: options.envToDelete ?? envToDelete }
|
||||
|
||||
@@ -0,0 +1,74 @@
|
||||
/**
|
||||
* Induce the one state the unreachable-pane affordance needs: the SSH target CONNECTED while
|
||||
* exactly ONE pane's `pty.attach` fails with an error that does not prove the shell exited.
|
||||
*
|
||||
* Why this exists: host-side faults cannot produce it. Stalling the relay takes the whole
|
||||
* connection down with it, so the client shows the connection-level overlay and the pane never
|
||||
* even attempts the attach that has to fail. That is why the decisive cases sat at `fixme`.
|
||||
*
|
||||
* How this does produce it: the relay refuses an attach whose `expectedIncarnationId` names a
|
||||
* different shell, with `PTY "<id>" identity mismatch` — per-pty, connection untouched, and
|
||||
* explicitly never grounds a respawn. Rewriting the pane's RECORDED incarnation while the app is
|
||||
* closed makes the next attach carry an expectation the host cannot match, so the relay refuses
|
||||
* that one pane and nothing else.
|
||||
*
|
||||
* This is the same shape as the real failure it stands in for: a replaced relay reissues ids from
|
||||
* `pty-1`, so the shell now answering to a pane's id is not the shell the pane recorded.
|
||||
*/
|
||||
import { readFileSync, writeFileSync } from 'node:fs'
|
||||
|
||||
/** Not a UUID the host could ever mint, so a match would mean the fence did not run. */
|
||||
export const FOREIGN_PTY_INCARNATION = 'incarnation-from-a-relay-that-is-not-this-one'
|
||||
|
||||
type MutablePersistedState = {
|
||||
workspaceSession?: {
|
||||
terminalPtyIncarnationsByPaneKey?: Record<string, string>
|
||||
}
|
||||
sshRemotePtyLeases?: { targetId: string; ptyId: string; incarnationId?: string }[]
|
||||
}
|
||||
|
||||
/**
|
||||
* Point the pane's recorded shell identity at a shell the host does not have.
|
||||
*
|
||||
* Both homes are rewritten because the two attach routes read different ones: the pane-driven
|
||||
* restore sends the binding's incarnation, the relay-session reconnect sends the lease's. Leaving
|
||||
* either untouched would let that route attach cleanly and the pane would never reach the card.
|
||||
*
|
||||
* Call only while the app is closed — the running app holds this state in memory and would
|
||||
* overwrite the file on its next flush.
|
||||
*/
|
||||
export function recordForeignShellIdentityForPane(
|
||||
stateFile: string,
|
||||
paneKey: string,
|
||||
targetId: string
|
||||
): {
|
||||
paneIncarnationBefore: string | undefined
|
||||
leasesRewritten: number
|
||||
/** Everything the state file actually held, so a miss reports why instead of just failing. */
|
||||
diagnostics: string
|
||||
} {
|
||||
const state = JSON.parse(readFileSync(stateFile, 'utf-8')) as MutablePersistedState
|
||||
const incarnations = (state.workspaceSession ??= {}).terminalPtyIncarnationsByPaneKey ?? {}
|
||||
const paneIncarnationBefore = incarnations[paneKey]
|
||||
incarnations[paneKey] = FOREIGN_PTY_INCARNATION
|
||||
state.workspaceSession.terminalPtyIncarnationsByPaneKey = incarnations
|
||||
|
||||
let leasesRewritten = 0
|
||||
for (const lease of state.sshRemotePtyLeases ?? []) {
|
||||
if (lease.targetId === targetId) {
|
||||
lease.incarnationId = FOREIGN_PTY_INCARNATION
|
||||
leasesRewritten += 1
|
||||
}
|
||||
}
|
||||
|
||||
writeFileSync(stateFile, `${JSON.stringify(state, null, 2)}\n`, 'utf-8')
|
||||
const knownPaneKeys = Object.keys(incarnations)
|
||||
const leases = (state.sshRemotePtyLeases ?? []).map(
|
||||
(lease) => `${lease.targetId}/${lease.ptyId}${lease.incarnationId ? '@known' : '@none'}`
|
||||
)
|
||||
return {
|
||||
paneIncarnationBefore,
|
||||
leasesRewritten,
|
||||
diagnostics: `paneKey=${paneKey} incarnationKeys=[${knownPaneKeys.join(', ')}] leases=[${leases.join(', ')}]`
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,178 @@
|
||||
/**
|
||||
* The decisive gate for the unreachable-pane affordance (STA-3077 / PR #13326).
|
||||
*
|
||||
* `ssh-disconnected-pane-affordance.spec.ts` holds its two decisive cases at `fixme` because the
|
||||
* state was not inducible: the banner needs the SSH target CONNECTED while exactly one pane's
|
||||
* `pty.attach` fails with an error that does not prove the shell exited, and every host-side fault
|
||||
* tried takes the whole connection down instead. This spec induces that state deliberately — see
|
||||
* `helpers/ssh-pane-incarnation-fault.ts` — so the behaviour can be OBSERVED rather than argued
|
||||
* from reading the code, which is how several oracles on this branch ended up green for the wrong
|
||||
* reason.
|
||||
*
|
||||
* What is pinned here:
|
||||
* 1. the card appears at all, with the connection healthy (the induction itself);
|
||||
* 2. reaching that state kills nothing and adds nothing on the host; and
|
||||
* 3. "Start a new terminal" on a pane whose shell turns out to be ALIVE reconnects the user to
|
||||
* it and says so, instead of silently handing back the old shell under a label promising a
|
||||
* blank one, and without adding a second shell.
|
||||
*/
|
||||
import { expect, test, type ElectronApplication, type TestInfo } from '@stablyai/playwright-test'
|
||||
import { createRestartSession } from './helpers/orca-restart'
|
||||
import { waitForActiveWorktree, waitForSessionReady } from './helpers/store'
|
||||
import {
|
||||
waitForActivePanePtyId,
|
||||
waitForActiveTerminalManager,
|
||||
waitForPaneIdentitySnapshot
|
||||
} from './helpers/terminal'
|
||||
import { resolveOrcaProfileStateFile } from './helpers/ssh-remote-pty-lease-file'
|
||||
import {
|
||||
cleanupDockerSshRelayTarget,
|
||||
startDockerSshRelayTarget,
|
||||
type DockerSshRelayTarget
|
||||
} from './helpers/docker-ssh-relay-target'
|
||||
import { connectDockerSshRelayTarget } from './helpers/docker-ssh-relay-connection'
|
||||
import { readDockerSshRelayRemotePtys } from './helpers/docker-ssh-relay-remote-ptys'
|
||||
import {
|
||||
installRemotePaneLaunchTranscript,
|
||||
readRemotePaneLaunchPids
|
||||
} from './helpers/remote-pane-launch-transcript'
|
||||
import { recordForeignShellIdentityForPane } from './helpers/ssh-pane-incarnation-fault'
|
||||
|
||||
const RUN_DOCKER_SSH = process.env.ORCA_E2E_SSH_DOCKER === '1'
|
||||
// Why: the relay must outlive the app quit. If it exited with the client the remote shell would
|
||||
// die too, and a dead shell cannot test a card whose whole claim is "we cannot reach it".
|
||||
const RELAY_GRACE_PERIOD_SECONDS = 900
|
||||
const BANNER = '[data-terminal-pane-disconnected-variant="ssh-pane"]'
|
||||
|
||||
test.describe('an unreachable SSH pane recovers rather than being replaced', () => {
|
||||
test.skip(!RUN_DOCKER_SSH, 'Set ORCA_E2E_SSH_DOCKER=1 to run Docker-backed SSH tests.')
|
||||
test.skip(process.platform === 'win32', 'Docker SSH faults use POSIX SSH tooling.')
|
||||
|
||||
test('a pane whose recorded shell the host cannot match offers recovery, and recovering it adds no shell', async (// oxlint-disable-next-line no-empty-pattern -- this test owns both Electron launches
|
||||
{}, testInfo: TestInfo) => {
|
||||
test.setTimeout(600_000)
|
||||
const restart = createRestartSession(testInfo)
|
||||
let target: DockerSshRelayTarget | null = null
|
||||
let firstApp: ElectronApplication | null = null
|
||||
let secondApp: ElectronApplication | null = null
|
||||
try {
|
||||
target = startDockerSshRelayTarget(testInfo)
|
||||
const relayTarget = target
|
||||
installRemotePaneLaunchTranscript(relayTarget)
|
||||
|
||||
const firstLaunch = await restart.launch()
|
||||
firstApp = firstLaunch.app
|
||||
await waitForSessionReady(firstLaunch.page)
|
||||
const remote = await connectDockerSshRelayTarget(firstLaunch.page, relayTarget, {
|
||||
relayGracePeriodSeconds: RELAY_GRACE_PERIOD_SECONDS
|
||||
})
|
||||
await expect
|
||||
.poll(() => waitForActiveWorktree(firstLaunch.page), { timeout: 30_000 })
|
||||
.toBe(remote.worktreeId)
|
||||
await waitForActiveTerminalManager(firstLaunch.page, 60_000)
|
||||
await waitForActivePanePtyId(firstLaunch.page, 60_000)
|
||||
|
||||
const snapshot = await waitForPaneIdentitySnapshot(firstLaunch.page, 1)
|
||||
const pane = snapshot.panes[0]
|
||||
if (!pane?.ptyId) {
|
||||
throw new Error('The remote workspace opened no pane with a PTY')
|
||||
}
|
||||
const paneKey = `${snapshot.tabId}:${pane.leafId}`
|
||||
|
||||
// Anti-vacuity for every "no second shell" clause below: the host records exactly one launch
|
||||
// for this pane now, so a later count of one proves nothing was added rather than proving the
|
||||
// transcript was never wired up.
|
||||
await expect
|
||||
.poll(() => readRemotePaneLaunchPids(relayTarget, paneKey).length, {
|
||||
timeout: 60_000,
|
||||
message: 'the host recorded no launch for the pane, so its transcript proves nothing'
|
||||
})
|
||||
.toBe(1)
|
||||
const shellBefore = readDockerSshRelayRemotePtys(relayTarget).find(
|
||||
(entry) => entry.paneKey === paneKey
|
||||
)
|
||||
if (!shellBefore) {
|
||||
throw new Error(`The relay hosts no remote shell for pane ${paneKey}`)
|
||||
}
|
||||
|
||||
// Resolved while the app is still up: the helper asks the live app for its profile path.
|
||||
const stateFile = await resolveOrcaProfileStateFile(firstApp)
|
||||
|
||||
await restart.close(firstApp)
|
||||
firstApp = null
|
||||
|
||||
// The induction. The relay and its shell are untouched and still running; only the pane's
|
||||
// RECORD of which shell is its own is made unmatchable.
|
||||
const rewritten = recordForeignShellIdentityForPane(stateFile, paneKey, remote.targetId)
|
||||
// Either route is enough to fence: the pane-driven restore sends the binding's incarnation,
|
||||
// the relay-session reconnect sends the lease's. But at least ONE must have held an identity,
|
||||
// or the rewrite changed nothing and every clause below would pass for the wrong reason.
|
||||
expect(
|
||||
Boolean(rewritten.paneIncarnationBefore) || rewritten.leasesRewritten > 0,
|
||||
`nothing recorded a shell identity to make unmatchable — ${rewritten.diagnostics}`
|
||||
).toBe(true)
|
||||
|
||||
const secondLaunch = await restart.launch()
|
||||
secondApp = secondLaunch.app
|
||||
await waitForSessionReady(secondLaunch.page, 60_000)
|
||||
await expect
|
||||
.poll(() => waitForActiveWorktree(secondLaunch.page), { timeout: 60_000 })
|
||||
.toBe(remote.worktreeId)
|
||||
|
||||
// 1. The card appears. This is the state that was previously not inducible.
|
||||
const banner = secondLaunch.page.locator(BANNER)
|
||||
await expect(banner, 'the pane showed no disconnected affordance').toBeVisible({
|
||||
timeout: 180_000
|
||||
})
|
||||
|
||||
// 2. Reaching it destroyed nothing and created nothing on the host.
|
||||
expect(
|
||||
readDockerSshRelayRemotePtys(relayTarget).filter(
|
||||
(shell) => shell.pid === shellBefore.pid && shell.startTicks === shellBefore.startTicks
|
||||
),
|
||||
'the unreachable pane killed the shell it cannot prove is gone'
|
||||
).toHaveLength(1)
|
||||
expect(
|
||||
readRemotePaneLaunchPids(relayTarget, paneKey),
|
||||
'reaching the card launched a second shell for the pane'
|
||||
).toHaveLength(1)
|
||||
|
||||
// 3. The shell is alive, so this action recovers it. The user is told that, and no second
|
||||
// shell is created — the pane keeps the terminal it already had.
|
||||
await banner.getByRole('button', { name: 'Start a new terminal' }).click()
|
||||
|
||||
// The recorded shell cannot be reached, so this action must CREATE one rather than attach
|
||||
// the unreachable one first — which is what left the button doing nothing at all.
|
||||
await expect(banner, 'the card stayed up after the pane was given a new shell').toBeHidden({
|
||||
timeout: 180_000
|
||||
})
|
||||
await expect
|
||||
.poll(() => readRemotePaneLaunchPids(relayTarget, paneKey).length, {
|
||||
timeout: 120_000,
|
||||
message: 'starting a new terminal never launched a shell for the pane'
|
||||
})
|
||||
.toBe(2)
|
||||
|
||||
// The old shell is unproven, not dead: it keeps running, unbound, for the cleanup surface.
|
||||
expect(
|
||||
readDockerSshRelayRemotePtys(relayTarget).filter(
|
||||
(shell) => shell.pid === shellBefore.pid && shell.startTicks === shellBefore.startTicks
|
||||
),
|
||||
'starting a new terminal killed the shell it cannot prove is gone'
|
||||
).toHaveLength(1)
|
||||
expect(
|
||||
readDockerSshRelayRemotePtys(relayTarget),
|
||||
'starting a new terminal changed the host by more than the one shell it promised'
|
||||
).toHaveLength(2)
|
||||
} finally {
|
||||
if (firstApp) {
|
||||
await restart.close(firstApp)
|
||||
}
|
||||
if (secondApp) {
|
||||
await restart.close(secondApp)
|
||||
}
|
||||
cleanupDockerSshRelayTarget(target)
|
||||
await restart.dispose()
|
||||
}
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user