starred/orca
1
0
Fork 0
mirror of https://github.com/stablyai/orca.git synced 2026-09-22 08:02:28 +00:00
Code Issues Packages Projects Releases Wiki Activity
10,591 Commits 7,634 Branches 1,105 Tags
eedaf2bdfc2e10ef1e4937cfd86a6233e49ecf07
Commit Graph
13 Commits
Author SHA1 Message Date
Neil e182930670 test: cover input in five simultaneously flooding SSH panes (#19071) 
* test: cover keyboard input in five simultaneously flooding SSH panes

* test: capture pane focus and buffers on flood input failure

* test: capture pane focus and buffers on flood input failure

* test: capture pane focus and buffers on flood input failure

* test: record replay input loss and application fix dependency

* test: record merged replay-input fix in the five-pane flood gate
 2026-09-07 19:51:44 -07:00
Jinwoo Hong 0c33f58e8a fix(ssh-relay): daemon owns the endpoint credential; a losing start never rotates it (#19052) 
<!-- orca-pr-loc -->
<!-- Programmatic LoC summary. Do not edit by hand; rewritten on every commit. -->

| | Files | Added | Deleted | Net |
| :--- | ---: | ---: | ---: | ---: |
| Test | 19 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$​962 | $\color{#cf222e}{\Huge{\mathbf{−}}}$​136 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$​826 |
| Prod | 18 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$​295 | $\color{#cf222e}{\Huge{\mathbf{−}}}$​116 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$​179 |

<!-- /orca-pr-loc -->

## Symptom

Live 2026-09-05 (Orca 1.4.198 client, Ubuntu host): both relay processes `kill -STOP`ped for 20 s, then `-CONT`. The client redeployed while the host was frozen. Its fresh daemon lost the socket bind (`Socket path already in use`) but had **already rewritten** `relay-<id>.sock.credential`. The surviving daemon kept its in-memory credential, so every later `--connect` got `Endpoint credential mismatch; closing socket`, then `Grace started … timeoutMs=0 … ptys=1, clients=0` every ~20 s, forever. Only a manual `kill -TERM` cleared it. Receipts: `review-archive/orchestration-v3-pr16904/smoke-receipts-t012b/E16,E17,E18,E24`.

Three independent defects kept the wedge alive; each is fixed at its own seam.

## Fix

**1. The relay daemon owns credential publication (race-free under two concurrent starters).**
`relay-daemon.ts` binds the socket first, then publishes via the new `src/relay/relay-endpoint-credential-publication.ts`: adopt a valid pre-existing file (older clients still pre-write), else mint 32 random bytes and write temp+rename at 0600. A start that loses the bind exits inside `listen()` and never reaches the file. Why this option and not restore-on-loss or a client-side write: the only process that can *prove* ownership is the one whose `listen()` succeeded, and that proof is atomic with the bind. The client-side pre-write (`ssh-relay-endpoint-credential.ts`) and the launch-command `chmod 600`/`icacls` are removed on POSIX and Windows. The racing test also exposed that macOS reports a mid-bind collision as `EEXIST` rather than `EADDRINUSE`; `relay-socket-ownership.ts` now treats both as "held or stale".

**2. The client distinguishes "no daemon" from "daemon present but not answering", and never rewrites.**
A credential refusal is now typed on the wire: the daemon replies `orca-relay-handshake-credential-mismatch` (same frame type, no new opcode) and the bridge exits **43**; `waitForSentinel` maps it to `RelayCredentialMismatchError`, which the takeover treats as handshake-refusal evidence exactly like exit 42. A relay that holds the endpoint but **never refused** (the stalled-host shape: kernel backlog accepts the probe, handshake gets no answer) is now `RelayEndpointUnresponsiveError`, routed to the relay-lost backoff instead of the terminal Reset Relay path. Silence is not a decision (`docs/reference/ssh-execution-boundary.md`).

**2b. Deploy honours the verdict.** The 40 s live run exposed that the `--connect` catch block in `deployAndLaunchRelay` predates the incumbent probe and swallowed both verdicts as "probe failed, launch fresh", so a fresh daemon was still launched over the live one (it lost the bind by luck, which is exactly the collision in the incident). Held and Unresponsive now propagate; the session backs off on Unresponsive and surfaces Reset Relay on Held. Red-first in `ssh-relay-deploy-incumbent-verdict.test.ts`.

**3. The daemon cannot be wedged by a rotated file, because nothing can rotate it.**
The credential lives in the content-hashed relay dir, and after (1) the only writer is the daemon that owns the socket, so the "file changed under a live daemon" state the incident depended on is no longer reachable in-product. The credential is therefore fixed for the daemon's lifetime, as a plain secret should be. A hand-edited file is refused with the typed reply until restored (tested). Startup adoption of a pre-written file applies an owner-only + same-uid rule (review finding): anything else is replaced by a fresh mint. An earlier revision of this PR also re-read the file on mismatch and adopted it; that was removed as unreachable machinery that turned the credential into a per-handshake file-ownership check.

**3b. Fail closed between bind and publication.** A client that arrives after `listen()` resolves but before the credential is set is refused, not admitted as `unproved`. Nothing can be delivered in that window today; the guard makes the boundary structural instead of an event-loop ordering fact. Red-first in `relay-reconnect-listener-credential-gate.test.ts`.

**Wire compat.** New optional handshake reply only; an old `--connect` hits `Unknown handshake type` and exits 1 pre-sentinel, which it already treated as a generic failure. New daemon adopts an old client's pre-written file; new client still passes `--credential-file` so an old daemon reads it as before. Absence of exit 43 is never used as evidence.

**Also.** `terminal create` on a reconnecting SSH host now says what to do instead of a bare `No PTY provider for connection "<id>"` (prefix preserved; the renderer matches it).

## Tests (red first)

- `src/relay/subprocess.test.ts`: two `--detached` starts race one socket + credential file → exactly one reaches the sentinel, loser exits 1 with `Socket path already in use`, file valid + 0600, a `--connect` reading it reaches `relay.status` and reports the winner's pid. Red before (both starters died: daemon required a pre-existing file), green 6/6 after.
- `src/relay/relay-endpoint-credential-publication.test.ts`: mints after bind; adopts a pre-written 0600 file; replaces a pre-written 0644 file with a fresh mint; refuses a stale credential with exit 43 while still serving the real one, and keeps refusing a rewritten file until it is restored.
- `src/relay/relay-reconnect-listener-credential-gate.test.ts`: a client in the bind-to-publish window is refused and never attached; after publication the right credential is accepted and a wrong one refused; a daemon launched without a credential file is not gated. Red without the guard.
- `ssh-relay-deploy-incumbent-verdict.test.ts`: live-but-silent incumbent → `RelayEndpointUnresponsiveError`, refused → `RelayEndpointHeldError`, and in neither case is `--detached` launched; a failed `test -S` probe still launches fresh. Red 2/3 without the deploy change.
- `ssh-relay-deploy-helpers.test.ts` (exit 43), `ssh-relay-endpoint-takeover.test.ts` (refused → Held even with no `lsof`; silent → Unresponsive, nothing unlinked or signalled), `ssh-relay-session-terminal-error.test.ts` (Unresponsive → `onRelayLost`, not terminal). Deploy/namespace/native-deps tests updated to assert the client writes **no** credential.

## Live proof

New `tests/e2e/ssh-docker-relay-stall-credential.spec.ts` (claimed in `run-ssh-docker-e2e.mjs` and PR source routing), two cases: `kill -STOP` every relay pid in the container, send input during the freeze, hold **20 s** (the incident's duration, which races the mux liveness timeout) or **40 s** (past it for sure), `kill -CONT`; assert status back to `connected`, same pty, same daemon pid, same credential inode and content, relay.log did not shrink (a relaunch truncates it) and has zero `Endpoint credential mismatch` / `Socket path already in use` lines, in-stall input delivered at most once.

Run output (local, fixture image `orca-e2e-ssh-relay:3a864c665ba2cefd`, `ORCA_E2E_SSH_DOCKER=1 SKIP_BUILD=1 ORCA_E2E_FORWARD_APP_LOGS=1 … --project electron-headless --workers=1`, head `c2c20fd994`; re-run identically on the final head after the credential-lifetime change, 2 passed (1.7m), same annotations, and the bind-to-publish refusal never fired):

```
✓ keeps the same daemon and credential across a 20s relay freeze (38.3s)
    relay-processes-stopped: 2          relay-processes-continued: 2
    bridge-pids-before-after: 480 -> 480
    socket-clients-accepted-before-after: 1 -> 1
    in-stall-input-delivered: 1
✓ backs off and reattaches, never relaunching, across a 40s relay freeze (57.5s)
    relay-processes-stopped: 2          relay-processes-continued: 4
    bridge-pids-before-after: 480 -> 1202
    socket-clients-accepted-before-after: 1 -> 3
    in-stall-input-delivered: 1
2 passed (1.6m)
```

Client log in the 40 s case shows the new path end to end: `Relay channel lost … reconnect attempt 1/6` → `Socket probe result: "ALIVE"` → `Socket reconnect failed … Relay failed to start within 10s` → `Relay endpoint incumbent: … verdict=live evidence=accepted-connection holders=unenumerable` → `Failed to re-establish relay … A relay still owns … but did not answer the handshake … Orca will retry` → `reconnect attempt 2/6` → `Reconnected to existing relay via socket`. The 20 s case never left the frozen bridge (same bridge pid, one accept), so it exercises the "silence is not death" side of the same race. The 20 s case passed 6/6 across the session; the 40 s case was red on the prior head (`Socket path already in use` + `Startup failed: listen EADDRINUSE` in relay.log from the swallowed verdict) and is green after 2b. Before the fix the same injection produced a fresh daemon that rewrote the credential and a survivor refusing every client.

The `relay-processes-continued` count exceeds `stopped` in the 40 s case because the timed-out client's `--connect` bridge and the loser-side processes are parked behind the frozen listener when `CONT` runs; they exit on their own once it resumes.

## Gates

`pnpm test src/relay src/main/ssh` 332 files / 3884 tests pass · `pnpm typecheck:tsc:node` clean · `check:code-quality:changed` 0 findings · `check:react-doctor:changed` 0 findings · `pr-e2e-gate-contract.test.mjs` 42 pass · no lint disables or max-lines bumps added.

## Noted, not fixed here

- `terminal list` `orphaned:false` / `terminal close` `ptyKilled:true` for a pane whose relay is gone (`orca-runtime-stop-explicitly-closed-tab-ptys.ts`): different seam, `@ts-nocheck` characterization-covered file.
- On a host with no `lsof`, a stalled relay still cannot be enumerated as the holder; it is now retried rather than declared held, but a relay frozen past the backoff budget still ends in the existing "reconnect manually" banner.
 2026-09-06 14:39:25 -04:00
Neil 3be526c5e6 test: cover SSH reattach replay and enable deterministic Codex CI (#19106) 
* test: cover SSH replay replies and run deterministic Codex restore scenarios

* test: register replay probe unit command in reliability gate
 2026-09-06 11:25:58 -07:00
Neil 4d9e963ffd test: enable localhost SSH terminal and hook journey in CI (#19097) 
* test: run localhost SSH terminal and hooks in CI

* test: isolate localhost SSH session fixtures across repetitions

* test: route remote agent hook source changes to localhost journey

* test: record localhost SSH reliability evidence and remaining gaps

* test: route the real SSH session hook authority
 2026-09-06 10:05:26 -07:00
Neil f5960cec00 test: enable Docker SSH browser network route coverage in CI (#19095) 
* test: enable Docker SSH browser network route journeys in CI

* test: register Docker browser job in token permissions contract

* test: declare SSH client dependency and narrow browser fixture routing
 2026-09-06 09:29:56 -07:00
Neil 3631f886a7 test: enable direct and client-hosted SSH browser coverage (#19090) 
* test: enable direct and client-hosted SSH browser coverage

* test: record twelve passing SSH browser journey repetitions

* test: distinguish SSH journey evidence from unit runtime budget
 2026-09-06 08:25:13 -07:00
Neil 1d2e00819f test: restore SSH bulk-open freeze coverage in headed CI (#19081) 
* test: restore SSH bulk-open freeze coverage in headed CI

* test: record ten passing headed SSH freeze repetitions

* test: record ten passing headed SSH freeze repetitions

* test: route changed SSH freeze spec only to its dedicated lane
 2026-09-06 07:42:50 -07:00
Neil 6cd477a2f1 test(e2e): un-rot the SSH freeze repro and probe two failure modes nothing covered (#17940) 
Test-only. No production code.

## The freeze repro was rotted in three ways, not one

#16764 tracks four stale call sites. There were three separate problems:

1. **Stale call sites** — `execInTerminal` gained a `ptyId` and
   `splitActiveTerminalPane` gained a direction. (`startDockerSshRelayTarget`'s
   missing `testInfo` was the third; #18257 has since landed it on main.)
2. **It connected before session restore settled**, so the seeded tab never bound
   to a remote PTY and the terminal sat on "Connecting…" forever.
3. **It could never have passed, even once.** It waited for a one-shot `READY:`
   line through a 4000-char terminal window while its own 2 KB-every-8 ms flood
   buries that line within ~16 ms. Readiness is now keyed on the repeating `BG:`
   flood marker, which is strictly stronger — it proves the pane is streaming
   rather than merely started.

It now runs end to end and prints a measurement instead of dying on a call site:

```
[freeze-repro R2] hiddenFloodMaxLagMs 2.1  bulkOpenMaxLagMs 41.5
                  interactionProbeMs 53.6  softFreeze false  hardFreeze false
```

**It is still not CI-gateable, and the exclusion comment now says so.** The same
spec on the same commit measured `bulkOpen 2575.6ms / interaction 3464.2ms` on a
GitHub ubuntu runner against a 2500 ms soft budget — a ~60x spread on the number
the budget reads, with the relay still streaming. That is the budget failing, not
the product. The earlier draft of this comment claimed "repaired and passing",
which was true only of the host it was measured on; gating this needs a
host-relative oracle, not a bigger constant.

## New: a half-open link is judged, not wedged

The fixture image has no `iptables` and the container has no `NET_ADMIN`, so
`docker pause` is used instead — a harder case, because the container's TCP stack
keeps ACKing: no FIN, no RST, and the socket looks perfectly healthy. Only an
application-level probe can detect it.

```
[half-open] {"verdict":"reconnecting","verdictMs":25135,"budgetMs":90000}
```

Nothing in the suite covered the failure mode behind the "SSH hangs until I
restart Orca" reports.

## New: resource accumulation measured on the remote host

6 terminals, then 5 reconnect cycles, counted on the container itself:

```
open:       pts 1->6 (exactly 1/terminal), relay fds 25->30 (exactly 1/terminal)
reconnect:  pts flat at 6, relay procs flat at 1, node procs flat at 3
```

`leakedMasterFdCount` is now **asserted**, not merely recorded. It counts PTY
master fds held by non-relay processes: without `FD_CLOEXEC` a master is inherited
by every later child, so terminal k adds k of them — the triangular signature
measured as 15 across 5 terminals before the fix. #17914 patched the app and
daemon and #17920 shipped the same patch to the relay host, and both are now on
main, so the correct value is 0 and the probe holds it there:

```
baseline    leakedMasterFdCount 0
6 terminals leakedMasterFdCount 0    (holders: only relay.js, n=6)
reconnects  leakedMasterFdCount 0 across all 5 cycles
```

Any growth here means the relay's node-pty rebuild did not take on that host,
which is exactly what a remote-host probe exists to catch — and it is the half of
#17914's claim that no unit test can reach.

## Routing

Both new probes are claimed by `run-ssh-docker-e2e.mjs` (a Docker-gated spec no
runner names self-skips everywhere and still reports green) **and** by the
`ssh-terminal-source` route in `pr-e2e-source-routing.mjs`, so they run when the
relay and SSH code they guard changes rather than only on a scheduled lane.
 2026-09-02 15:58:42 -07:00
Neil f737f3499f fix(relay): stream an oversized fs.listFiles reply instead of refusing it (#17954) 
Opening Orca's own checkout over SSH cannot list its files in one response frame.
22,617 tracked paths average 58 characters, so the 20,001-row page the client asks
for serializes to 1,223,415 bytes — past `DISPATCHER_CONTROL_QUEUE_MAX_BYTES`, so
`sendResponse` demotes it to the `legacy-response` lane, where an unrelated
producer backlog can refuse it as an opaque `ResponseOverCapacity`. Break-even is
around 49 characters of average path; any `packages/<name>/src/...` monorepo is
over the line.

Picking a ceiling to refuse at does not fix that, it just moves where it shows up
and refuses listings that would have been delivered. `__streamResponse` already
exists for exactly this on the git methods, and it is its own negotiation in both
directions: an old client never sends it and gets the plain array on the
legacy-response lane as before, and an old relay ignores it and answers plainly,
which the client detects by the sentinel marker being absent. So fs.listFiles opts
into it — no new method, no new opcode, nothing to advertise — and the size of a
listing stops being a correctness question.

The response-stream registry becomes one per relay, shared by FsHandler and
GitHandler. A second registry is not an option and the header of
git-response-stream.ts says why: a client keys reassembly on `streamId` alone, so
two would hand out the same id and cross-feed chunks, and only the handler that
registers `git.responseAck` can credit the window a pump parks on.

Also declares `maxResults` on the runtime-RPC `files.listAll` and forwards it.
The mechanism "the client names its cap, so a full page reads as truncation" was
wired only on the Electron IPC hop; web and mobile were saved incidentally by
`remoteFileContentBudget` defaulting the cap inside `listRuntimeFiles`. A new
optional field is additive in both directions (wire rule 1).

The new Docker-gated spec is claimed by run-ssh-docker-e2e.mjs. The sharded e2e
lanes set no ORCA_E2E_SSH_DOCKER, so a Docker-gated spec that no runner names
self-skips everywhere and still reports green — pr-e2e-gate-contract enforces that.

Closes #12547
 2026-09-02 05:36:54 -07:00
Neil 0dbe9d0504 test(ssh): dockerized relay fault injection with verdict assertions (#18017) 
* test(ssh): add a dockerized SSH fault-injection lane with four fault shapes

The existing SSH reconnect specs all reconnect by calling ssh.disconnect() then
ssh.connect() - a clean cycle the client knows is coming. Nothing covered the
faults the reconnect machinery exists for.

Four shapes, each documented with why it is not the others: killing sshd's
per-connection forks (transport dies, relay survives), `docker pause` (silence
with TCP still established), SIGKILLing every relay.js (the only fault where
`exited` is the correct verdict), and a 48MB flood with nobody attached.

The relay-kill case is the one that makes the rest meaningful: every other case
asserts the session survived, which only means something if a genuinely dead
session is distinguishable. It is the only case where replacing the pane is
correct, so it pins the boundary in
docs/reference/ssh-execution-boundary.md rather than just testing reconnection.

The `docker pause` case pins the other side of that boundary: after 30s of
silence from a healthy host the pane keeps its PTY and its scrollback, because
loss of contact is never evidence of death.

No network-blackhole fault: reconnecting the fixture does not restore its
published port mapping, so that fault is not reversible on this container and
would strand the worker it ran on.

* test(ssh): fixme the flood case pending #18018

It fails in CI on its first real run: the pane keeps its PTY and repaints,
but a command run after the flood produces no output within the poll budget.
Same shape as #18018 and not caused by this spec. The three verdict
assertions around it stay enforced.
 2026-09-01 23:35:47 -07:00
Neil 2b391652b1 fix(terminal): a close the host never heard must survive the reconnect (#16752) 
An enterprise user: "Every day I open orca and it opens more tabs daily at a
linear scale." Three reports over a week, told on 08-19 that a PR had fixed it,
reported twice more after. STA-4658 (P0), GH #12447, #15136, #10342, #9585. One
install held 39 zombie tab records. The revived tab's sleeping-agent record still
holds the pre-close session id, so it boots `claude --resume <old id>` -- two
agents on one transcript.

## The chain, measured

Reproduced deterministically in `ssh-lost-kill-tab-resurrection.spec.ts`: close
an SSH tab, kill the relay daemon in the container so `pty.kill` rejects with a
transport-class error, reconnect.

    drop 2 resurrected the closed tab <id>:
      baseline=1  drop1=1  drop2=2 (closed tab returned)  drop3=1

The trigger is narrow and had to be measured rather than assumed: killed relay
daemon reproduces **6 of 6 runs**; an orderly `ssh.disconnect` **passes**. Only
an ungraceful loss -- network partition, host reboot, relay crash, a laptop
sleeping mid-session -- strands the close with the RPC rejecting on a
transport-class error. Both variants live in the spec behind one
`runResurrectionCycles` parameterized solely by the disruption, so the difference
is attributable to that single variable.

What actually carries the tab back, from the pull path
(`workspace.get` -> `getRemoteSnapshot`, `remote-workspace-relay-sync.ts:29`):

    pullSnapshot rev=3 tabs={repo:["16c4a3e1","06aba6b6"]}
    pullSnapshot rev=4 tabs={repo:["16c4a3e1","ff72768e"]}   <- ff72768e IS the resurrected tab
    pullSnapshot rev=5 tabs={repo:["16c4a3e1","ff72768e","da21b76c"]}

The client uploaded the session containing the tab; the user closed it; the kill
RPC rejected so the close never reached the host; the host's snapshot still lists
it; the client pulls it back and the merge restores it -- **correctly, by its own
rule that the host is authoritative for what it knows.** A pane then mounts,
respawns, and takes the recycled pty id.

Client-side correlation from the same run, two controls and one positive in one
run differing in exactly one variable:

| Tab | Close events observed | Resurrected? |
|---|---|---|
| `6305cc07` | `user` + `pty-exit` | No |
| `ed56f66c` | `user` + `pty-exit` | No |
| `2036e760` | `user` only | **YES** |

## The fix

`src/shared/closed-terminal-tab-tombstones.ts` (99 lines). A client-recorded
close is first-party intent and must survive until the host acknowledges it. Per
`docs/reference/ssh-execution-boundary.md` the remote verdict is `unverifiable`
-- which may not authorise declaring the process dead, but equally must not
authorise resurrecting the tab. This is SSH-v3 principle P2, "durable tombstones
with a monotonic per-scope revision", reusing the existing
`RemoteWorkspaceSnapshot.revision` rather than adding a twelfth per-tab identity
field (the codebase carries eleven, 784 refs, that SSH-v3 Phase 3 deletes).

- **Recorded** only on `closeReason === 'user'` (`terminal-tab-close.ts:69`).
- **Suppresses** a host-sourced tab only when
  `tabId in tombstones && !currentTabsById.has(tabId)` -- a live local tab always
  wins, because deleting a live pane is the one outcome the merge exists to
  avoid.
- **Retires** on positive acknowledgement:
  `!hostKnownTabIds.has(tabId) && hostRevision > observed`. Strictly newer, so a
  pull already in flight at close time cannot ack a close it predates.
- Three never-retire guards: no revision retires nothing; a worktree the snapshot
  has no row for retires nothing; the first omitting snapshot only stamps the
  watermark.
- TTL (30d) + cap (500) are **backstops** for a target the user never returns to,
  not the mechanism.
- **Client-local only** -- never crosses the wire, so there is no mixed-version
  exposure.
- Suppression is scoped to `replaceWorktreeIds`, which is what makes the
  live-tab check meaningful. A final whole-map sweep over the assembled
  `tabsByWorktree` would break that (a live tab is absent from `currentTabsById`
  outside the scope and would look suppressible); it is deliberately not there,
  and the comment at the top of the function says so.

## Evidence

The load-bearing evidence is an A/B control on one tree, not the oracle's
assertion. Flipping `isSuppressedByClose` to `false` -- one character --
reproduces the resurrection on demand:

    --repeat-each=2:
      1) drop 2 resurrected the closed tab ab0e305d-…: baseline=1 drop1=1 drop2=2
      2) drop 2 resurrected the closed tab 51533e34-…: baseline=1 drop1=1 drop2=2
      2 failed

With suppression on: **0 occurrences of "resurrected the closed tab" across five
runs plus one independent run by a second agent.** Provenance verified
positively, not by mtime: `closedTerminalTabTombstonesByTabId` appears 13x across
3 renderer chunks including `store-Do3KBvRE.js`; for every red control run
`mayCreate` appeared 0 times in `out/main/index.js`.

At the unit layer, disabling the same predicate: 3 failed | 39 passed. Restored:
42 passed; 287 across the workspace-session, terminal-store, remote-workspace,
shared-tombstone and profile suites; 24 in the four tombstone suites.

## The oracle spec: GREEN in the full lane

`ssh-lost-kill-tab-resurrection.spec.ts` passes both tests at this commit. Full
Docker-SSH lane, clean tree:

    BUILD_SHA=49bb96e0b4c   DIRTY=0
    PROVENANCE  tombstone=13  hasLocalTabsRow=2  hostAuthority=4  mayCreate=3
    14 specs / 20 tests -> 17 passed, 2 failed, 1 skipped (10.7m)

    [12/20] :178 does not resurrect tabs whose kill was lost to a killed relay
            daemon                                                      PASSED
    [13/20] :190 does not resurrect tabs closed while the host is
            disconnected                                                PASSED

    grep -c "resurrected the closed tab"  (whole lane)  -> 0

It passes WITHOUT PR 7 in the build (`mayCreate` present,
`SshPtyAbsentFromRelayError` absent), so the bug-2 fix below is not required for
it.

Test 1 fails intermittently in ISOLATED single-spec runs, where a third defect
blocks its cycle-2 setup. The resurrection assertion itself has never failed with
this fix in place -- the intermittent failure is always a setup failure, never a
resurrected tab. A reviewer running the spec alone may see it red; that is not
this fix regressing.

Three defects sit under STA-3374 and should not be conflated:

- Bug 1 -- the closed tab resurrects. Fixed here.
- Bug 2 -- `ssh-pty-session-reattach.ts:227-231` rewrites the relay's
  `PTY "pty-1" not found` into a bare `SSH_SESSION_EXPIRED`, so
  `isPtyAlreadyGoneError`'s `/PTY ".+" not found/` cannot match and
  `attachStablePaneOwner:242`'s already-correct fallback never runs. Owned by
  PR 7 (`nwparker/ssh-07-absent-from-relay`). Not required for the oracle above.
- Bug 3 -- after the daemon is killed and the client launches a replacement, the
  client's OWN SSH transport drops and does not reconnect within 60s: no
  "delay step 2/9", no handshake failure, nothing. `ssh-connection.ts:1533` only
  logs on an SSH-level close. Unfixed, its own ticket. This is what makes test 1
  intermittent in isolation.

Discriminator for bug 3, measured in the isolated runs (the lane above ran
without `ORCA_E2E_FORWARD_APP_LOGS=1`, so it was not re-confirmed there):
`[ssh-relay] Socket probe result:` reads "DEAD" on every cycle of test 1 (daemon
killed, a NEW relay must be launched) and "ALIVE" on every cycle of test 2
(daemon survived). Whenever a new daemon must be launched, the SSH transport
drops afterwards and does not recover.

An earlier reading blamed `kill.ts:82-84` for skipping `finishPtyShutdown` on a
non-already-gone error. That was eliminated by direct test: the implied fix,
`markSshRemotePtyLease(…, 'expired')` in that branch, was implemented, changed
nothing, and was reverted rather than shipped unproven. Recorded so the path is
not re-walked. The `SSH_SESSION_EXPIRED` rejection is real but fires during cycle
1 for the baseline pane, after which cycle 1 completes; the 60s silence begins
only after `Relay channel lost ..., triggering reconnect`.

The spec is claimed by the Docker-SSH lane, and that lane does not gate merges
today.

## Persistence: the tombstone must survive a relaunch

`closedTerminalTabTombstonesByTabId` is declared on `WorkspaceSessionState` but was missing from
`workspaceSessionStateSchema` (`src/shared/workspace-session-schema.ts`), which is the load boundary
for BOTH partitions -- `normalize-loaded-state-collections.ts` for `local` and
`workspace-session-partitions.ts` for `ssh:<target>`. Zod strips unknown keys and the write side does
not validate, so the map reached disk and was discarded on the next launch. Measured with the repo's
own parser:

    input : closedTerminalTabTombstonesByTabId: { 'tab-1': {...} }
    ok    = true
    tombstones after parse = undefined

That made the fix ineffective in the exact reported scenario: close an SSH tab with the transport
down, QUIT, relaunch, reconnect -- the merge runs with an empty map, the host still lists the tab,
and it resurrects. "Every day I open orca and it opens more tabs" is a claim about restarts.

Neither the green oracle nor the A/B control could see it: both run entirely inside one app process.
It also made the 30-day TTL and the 500 cap unreachable.

Fixed by adding the field with a `salvagingRecord` matching its sibling
`terminalSurfaceTombstonesByPaneKey`, so one malformed entry drops that entry rather than the map.

`workspace-session-schema.ts` was one line under its 300-line max-lines limit, so adding the field
required room rather than a suppression (the project forbids max-lines disables and per-file bumps).
Two value schemas were extracted to modules named after what they contain:
`terminal-tab-id-schema.ts` and `terminal-surface-tombstone-schema.ts`. The closed-tab tombstone's
own schema is colocated with its type in `closed-terminal-tab-tombstones.ts`, which is where it
belongs -- omitting it from the session schema is exactly the drift that caused this bug.

`workspace-session-schema-field-coverage.test.ts` is the ratchet. Two sibling tables already pin
themselves with `satisfies Record<keyof WorkspaceSessionState, ...>`; this schema had no such guard
and is the one that fell behind. The new file adds both halves -- a `satisfies` list that makes a
forgotten field a compile error, and a runtime assertion that names it -- plus a
`parseWorkspaceSession` round-trip. Without the schema entry: 3 failed. With it: 3 passed.

## A host tab the user never closed could be deleted

`tabId in closedTerminalTabTombstonesByTabId` answers true for every `Object.prototype` key even on
an EMPTY map, because the map is a plain object from `Object.fromEntries`. A host tab whose id is
`toString` was filtered from the reconciled list, blocked from the host-unknown branch, and stripped
of its layout and session id. Tab ids are validated only as non-empty and colon-free, and `createTab`
honours caller-supplied id hints, so the id is reachable rather than theoretical. This was the only
path in either direction that could delete a tab the user never closed.

Now `Object.hasOwn`, as the same file already uses elsewhere.

Suppression is also scoped structurally: `isSuppressedByClose` compares the tombstone's stored
`worktreeId`, which it already carried, so it cannot reach another workspace's tab. The two sweeps
that have no worktree in scope (`terminalLayoutsByTabId`, `remoteSessionIdsByTabId`) now consult the
set of ids this merge actually suppressed rather than re-deriving a verdict without that scope.

The scope comment at the top of the function was also wrong and is corrected. It claimed every use of
suppression sits inside `replaceWorktreeIds`; it does not -- the tabs pass walks all of
`orderedWorktreeIds` and the two sweeps cover the whole remote maps. What actually makes it safe is
that `closeTab` strips the id from every worktree row before recording the tombstone, plus the
worktree match above, plus `closeReason === 'user'` being the only writer. Real guarantee, different
from the documented one.

## Divergences from open PR #16571

#16571 implements the same concept. Three deliberate changes:

1. It never retires on acknowledgement -- TTL+cap only, so it never converges.
   Ack retirement added.
2. It crosses the wire and lets a HOST-sourced tombstone delete a LOCAL tab in a
   final whole-map sweep. After #14361 that is the wrong risk; dropped. This also
   removes the mixed-version regression its own body flags.
3. Its hydration unions rather than replaces the map -- a union resurrects every
   tombstone the merge just retired, so it never converges.

Its `activeTabId` nulling is also dropped as redundant:
`workspace-terminal-hydration.ts:99-105,126-138` already revalidates both
pointers against the tab rows it just built, and nulling twice would add a second
rule that has to stay in step with the first.

## Can a tab the user did NOT close disappear?

No, but the guarantee needs stating precisely. The only writer is
`recordClosedTerminalTabTombstone` (`terminal-tab-close.ts:69`), reachable only
on `closeReason === 'user'`; suppression additionally requires the tab not be live
locally. Reopen (`recently-closed-tabs.ts:122-166`) calls `createTab` and restores
cwd/shell/title/color/position, never the old id.

**Caveat, stated because the slogan is not literally true:** `createTab` honours a
caller-supplied id hint (`terminal-tab-creation.ts:53-65`, used by `useIpcEvents`
for host-admitted tabs), so "tab ids are uuids that never recur" does not hold in
this codebase. The guarantee rests on the `closeReason === 'user'` writer plus the
live-local-tab check, not on id uniqueness.

## Risk

Renderer-side, client-local, no wire change. The blast radius is
`mergeDirectSshRemoteWorkspaceSession` and the persisted session field. Worst case
if the ack logic were wrong in the retiring direction: a tombstone outlives its
usefulness and suppresses a host tab whose id the host re-issues -- bounded by the
live-local-tab check, the 30d TTL and the 500 cap. Worst case in the other
direction is today's behaviour. `profile-project-session-field-disposition.ts`
records the new field as `notRepoScoped` / `notTransferred` residue, bounded by
the same TTL and cap.

## Verify

    pnpm test src/shared/closed-terminal-tab-tombstones.test.ts \
      src/renderer/src/lib/workspace-session-closed-tab-tombstones.test.ts \
      src/renderer/src/store/terminals/terminal-tab-close-tombstone.test.ts \
      src/renderer/src/hooks/remote-workspace-session-merge-close-tombstones.test.ts

To reproduce the bug this fixes, set `isSuppressedByClose` to `() => false` in
`remote-workspace-session-merge.ts` and run
`pnpm test:e2e:ssh-docker -- tests/e2e/ssh-lost-kill-tab-resurrection.spec.ts --repeat-each=2`.
 2026-08-27 19:47:15 -07:00
Neil e06a8667a9 fix(terminal): do not seed or resume while the execution host has not answered (#16750) 
Two client behaviours read local tab rows as the verdict on what the execution
host is running. Before the host answers, "I hold no pane for this" is
`unverifiable`, not `exited` -- the collapse
`docs/reference/ssh-execution-boundary.md` forbids.

Symptom 1, seeding. `worktree-initial-terminal-seeding.ts:47,128` seeds a
terminal when `renderableTabCount === 0`. Its only bail-out (`:72-77`) covered
the paired-web-runtime flavor -- "while that session is live the host owns
terminal creation" -- with no equivalent for direct SSH. So a client that has
never held the workspace runs the predicate during the hydration gap and creates
a tab from nothing. The snapshot then arrives, the merge rightly keeps the tab it
was never told about, and the union uploads as the new host truth. Measured on a
fresh client against a host owning 3 tabs: **1 tab created from nothing, 0 of the
host's 3 adopted.** (A restart never reaches the predicate -- local state
restores the row first -- which is why restart-only repros came back flat.)

That guard was also the wrong question. It asked "am I a client of a live paired
session?", which a host desktop window answers "no" and a paired client answers
"yes", so both seeded -- #15556.

Symptom 2, sleeping-agent resume, and the data-corrupting half.
`Terminal.tsx:1554` calls `resumeSleepingAgentSessionsForWorktree` twenty lines
after the seeding call at `:1529-1534` -- same startup path, same pre-hydration
window, and not SSH-gated at all. Seeding produces a spare empty tab; the sweep
launches `claude --resume <id>` for a session still running on the remote and
still owned by a live pane. Two agent processes writing one transcript; STA-3498
observed five. STA-3500 files exactly this race. Failure is asymmetric: declining
to resume is user-recoverable, a duplicate resume corrupts a transcript
irreversibly.

`workspace-terminal-host-authority.ts` answers the one ownership question both
paths ask, in the three-verdict vocabulary the renderer already uses for host
terminal inventory (`HostLiveTerminalProbeVerdict`, aliased rather than restated
so the two cannot drift): `live` (a remote host owns creation here),
`unverifiable` (there is a remote host and it has not answered), `none` (local,
or the host answered and holds nothing). Seeding requires `none`; the sweep
declines on `unverifiable` without consuming its one-shot, so the agents are not
stranded for the session once the verdict lands.

Shape notes:
- An ownership question, not a client-liveness one -- that is what fixes #15556.
- Folder workspaces resolve to `none`: the snapshot replaces exactly
  `DirectSshTargetScope.gitWorktreeIds`, so a folder's rows are never replaced by
  the host and waiting for an answer that will never name them would leave it
  terminal-less for good.
- A `conflict` sync phase is `unverifiable`, matching the pair
  `use-app-session-persistence.ts` already gates uploads on.
- Explicit launch work (setup/issue commands) stays ungated -- that is a request
  to create a terminal now.
- `Terminal.tsx` subscribes through a retained selector rather than reading in
  the effect: the verdict flipping to `none` is what must re-run the passes, and
  resolution walks the owner catalogs, so recomputing per store write would be
  the STA-3363 render-path multiplier again.

The `unverifiable` verdict is BOUNDED, and must be. `remoteWorkspaceHydratedTargetIds` is add-only
in practice -- `markRemoteWorkspaceHydrated` has two production call sites, both on success paths,
and `clearRemoteWorkspaceHydrated` has NONE. Four paths return without marking: local-hydration
timeout (`remote-workspace-target-sync.ts:136-145`), a null `remoteWorkspace.get` (`:160-169`), a
falsy apply token (`:172-185`), and never connecting at all. Without a floor, any of them would
leave every git worktree on that target `unverifiable` for the rest of the app session: no initial
terminal, no sleeping-agent resume, escapable only by creating a tab by hand. That is strictly worse
than the behaviour it replaces -- on main the user got a terminal. So a sync that terminates in
`offline` or `error` without ever hydrating resolves `none`: declining to seed is meant to be a
wait, not a permanent refusal. `pulling` still declines, and a target that HAS hydrated stays `none`
even if a later sync errors.

Scope, stated because the doc comment previously overstated it: this gate is
first-hydration-per-target, not per-connection-generation. Since nothing clears the flag, a
disconnected target that hydrated once reads `none`. It does not cover mid-session reconnect or
sleep/resume.

The memo's input list is checked for COMPLETENESS, not just membership. `satisfies readonly
(keyof State)[]` only proves each listed key exists; a field added to the state and forgotten from
the list would type-check while making the memo return a stale verdict -- silent, and it looks like
"the gate did not fire". A conditional type now names the missing key at compile time. Deliberately
not `const x: Missing[] = []`, which passes regardless because an empty array literal is assignable
to every array type.

Known limitation, stated rather than hidden: the SEEDING half of this change has no measurable
end-to-end effect today, and the branch's own e2e spec says so.
`applyDirectSshRemoteWorkspaceSnapshot` calls `markRemoteWorkspaceHydrated` unconditionally AFTER
the hydrate calls -- including when they wrote nothing. So in the same tick adoption yields zero,
the verdict flips `unverifiable` -> `none`, `Terminal.tsx` re-runs the effect, and it seeds. The
gate cannot outlive the failure it guards against, because the same function that fails to adopt is
the one that lifts it.

`ssh-cold-hydration-gap-tab-seeding.spec.ts:218` is named for what it asserts -- one tab, adopted
none -- rather than for the behaviour we want. The fixme at `:293` pins the intended behaviour.

Making the seeding half effective needs hydration resolved PER WORKTREE (or a refusal to say `none`
when the completed apply's `replaceWorkspaceKeys` did not name this worktree) rather than a
per-target "some apply finished" flag. That is deliberately not in this commit.

The RESUME half is the valuable half and is unit-proven: it declines while the host is unanswered
and wakes the same session once the verdict lands, without consuming its one-shot. Preventing one
duplicate `claude --resume` on a live transcript is worth more than preventing one spare tab --
declining to resume is user-recoverable, a duplicate resume corrupts a transcript irreversibly.

Before: 7 failed | 3 passed. After: 10 passed; 103 across the seeding, resume,
authority and remote-workspace suites.
 2026-08-27 19:44:30 -07:00
Neil 971d987c4b ci(e2e): trigger the Docker-SSH lane from SSH source and claim every gated spec (#16746) 
The Docker-SSH e2e lane only ran when a PR's changed specs happened to include
`ssh-startup-exec-readiness.spec.ts` or `paired-startup-exec-readiness.spec.ts`.
Editing SSH source itself did not trigger it, and pruning either spec from a
route's list would have silently retired the whole lane. Meanwhile the sharded
lanes set no `ORCA_E2E_SSH_DOCKER`, so every Docker-gated spec skipped itself
while the shard still reported green -- the exact silent-skip shape
`docs/reference/ssh-reconnect-source-recovery.md` blames for four regressions
that reached users.

Separately, the modules that actually own direct-SSH workspace and tab restore
carry no "ssh" in their names, so the `ssh-terminal-source` route never reached
them. Measured on the real script before this change:

    printf '%s\n' src/renderer/src/hooks/remote-workspace-session-merge.ts \
      src/main/ipc/remote-workspace-snapshot-normalization.ts \
      src/renderer/src/lib/worktree-initial-terminal-seeding.ts \
      src/shared/remote-workspace-session-projection.ts \
      | node config/scripts/pr-e2e-source-routing.mjs
    => []

Three changes, all pinned by the executable gate contract:

- `hasSshSourceChange` derives an `ssh_source_changed` signal from the SSH
  routes themselves, plumbed pr.yml -> e2e.yml, so the lane triggers on source
  rather than on a spec name surviving in a list. One list, so the two cannot
  drift.
- A sibling `ssh-workspace-session-restore` route names the restore seams
  (`remote-workspace-*`, `worktree-initial-terminal-seeding`,
  `worktree-default-terminal-tabs`, `initial-terminal`) and routes them to the
  two restore specs -- a sibling rather than more paths on `ssh-terminal-source`
  so a tab-tombstone edit does not run the whole SSH terminal list.
- A new `test:e2e:ssh-docker` runner claims the remaining Docker-gated specs on
  the one VM that sets the flag, and the contract now fails by name when any
  Docker-gated spec is claimed by no runner. `ssh-docker-relay-perf` and
  `ssh-codex-display-artifacts-repro` are recorded exemptions (wall-clock
  budgets; needs a real remote codex binary) and the contract asserts each
  exemption still corresponds to a real gated spec, so a stale one cannot
  quietly excuse a gap. Lane timeout raised 35 -> 60 minutes for the added
  serial specs.

The lane's first act was to surface four latent bugs in a spec that had been
silently skipping. `ssh-docker-bulk-open-freeze-repro.spec.ts` is four call sites
out of date against `tests/e2e/helpers/terminal.ts`: `startDockerSshRelayTarget()`
is called with no argument though the helper dereferences `testInfo.workerIndex`
(a 100% failure, not a flake), `execInTerminal` gained a `ptyId` parameter, and
`splitActiveTerminalPane` gained a direction. It was invisible because it ran
nowhere and `typecheck:e2e` is red on main with 240 pre-existing errors, so four
more could not be seen.

The `testInfo` bug is fixed here -- correct on its own, and it removes one real
error from `typecheck:e2e` (240 -> 239). The other three are not, because they
are not argument plumbing: repairing them requires choosing which ptyId to
capture and which split direction to use, and both change what the repro
measures.

The spec is therefore added to the exemption list rather than repaired, for two
independent reasons recorded in the runner: it is a perf oracle, not a
correctness one (`SOFT_FREEZE_LAG_MS=2500` / `HARD_FREEZE_LAG_MS=5000` measured
under a deliberate 5-pane flood on a 420s budget -- the same rule already applied
to `ssh-docker-relay-perf.spec.ts`), and it is known-rotted. Repair is tracked in
stablyai/orca#16764. Applying an existing written rule to a sibling that plainly
meets it is consistency; inventing a new exemption to dodge a red would not be.

Three hardening fixes to the contract itself:

- Runner text is comment-stripped before the claimed-by-a-lane scan. A substring
  scan over raw text lets a spec merely *discussed* in a runner comment count as
  claimed -- the silent skip this assertion exists to catch, re-entering through
  the documentation. Not live today only because the existing comments write the
  spec names without their `tests/e2e/` prefix.
- An exempt spec must not be invoked by any runner. `unreachableSpecs`
  short-circuits the unclaimed check, so a spec could be documented as exempt
  while a runner still ran it -- an exemption that reads as coverage removal but
  changes nothing, leaving the lane red for a reason the file says it excluded.
  This is not hypothetical: adding the bulk-open exemption without removing it
  from the runner's spec list produced exactly that state, and this assertion is
  what caught it.

- The Docker-gate detector is now `/ORCA_E2E_SSH_DOCKER\s*[!=]==\s*['"]1['"]/`
  rather than one fixed string, so a double-quoted or `!==` spelling can no
  longer escape the contract.

`ssh-restart-tab-accumulation.spec.ts` is a new three-cycle restart fence
asserting tab-id set identity, not just the active pane's reclaimed ptyId as
`ssh-cold-activation-restore.spec.ts:241` did. It passes today; it was validated
by a negative control that injected one tab after cycle 1 and correctly failed.
 2026-08-27 19:40:38 -07:00