Files
orca/tests/e2e/ssh-cold-hydration-gap-tab-seeding.spec.ts
T
Neil b1fe9075db fix(ssh): never claim authority over host tabs this client could not place (#16956)
* fix(ssh): a host tab row this client cannot place is unverifiable, not absent

A degraded listLineage leaves worktreesByRepo empty, so exactTargetWorktreeIds
returns nothing, every host path fails to resolve, and importRemoteWorkspaceSession
silently dropped every tab row. The apply then marked the target hydrated and
'synced', freezing that emptiness in permanently — nothing re-pulls a hydrated
target (STA-3593).

The importer now reports unplaceable rows, the apply claims authority only when
every row landed, and a bounded chain re-pulls the missing input (catalog +
lineage). On exhaustion it settles back to the pre-fix behaviour so a genuinely
unplaceable path is never left worse off than today.

* fix(ssh): keep the re-pull chain bounded, unwedgeable, and announced once

Five defects in the first cut of the chain, all found before merge:

1. the caller owned the attempt counter, so an unsolicited host push re-armed
   it at 0 and the chain never exhausted - an unbounded workspace.get loop;
2. exhaustion never cleared the counter, so after one bad connection every
   later reconnect re-exhausted instantly and the retry was silently dead;
3. the exhaustion check preceded the armed-timer guard, letting a concurrent
   report cancel the still-pending final retry;
4. a rejected host read left no timer armed and nothing rescheduled, stranding
   the target on 'pulling' and un-hydrated forever - and an un-hydrated target
   never uploads again, the exact permanent degradation this design avoids;
5. exhaustion re-announced on every later report, re-marking hydrated and
   rewriting status on each host push.

The module now owns the counter, resetTarget gives each connection a fresh
chain, the armed guard precedes exhaustion, the timer body always reschedules
so any failure walks to exhaustion, and exhaustion is announced once.

* fix(ssh): never authorise uploads from a picture known to be incomplete

Reversal of this branch's own exhaustion fallback, on evidence.

Hydration authorises uploads (use-app-session-persistence.ts), and an upload is
a workspace.patch of kind 'replace-session' (remote-workspace-relay-sync.ts:66)
which wholesale replaces the host snapshot (relay/workspace-session-handler.ts).
So marking a target hydrated on a picture we know is missing rows does not
'settle back to the old behaviour' - it uploads an empty projection that DELETES
the host tabs we failed to adopt. Suppressed uploads are recoverable; a wiped
host snapshot is not. That data loss is reachable on main today, because today
the apply marks hydrated immediately.

Exhaustion therefore reports 'error' and leaves the target un-hydrated, so
terminal authority stays 'unverifiable' and no upload can be built from it.

Also closes two chain-lifecycle gaps found in review:
- the callback dropped its timer guard before awaiting the host, leaving a gap
  in which a concurrent report armed a second overlapping chain and could trip
  exhaustion before the pending apply resolved; an in-flight guard now spans it;
- resetTarget could not cancel a callback already past its await, so a stale one
  rescheduled on top of the new connection's chain; chains are now generation
  stamped and a stale callback exits.

* fix(ssh): scope re-pull in-flight ownership to a generation

Two races found in review of the previous commit:

- schedule(target,'placed') cleared timer/count/exhaustion but did not
  invalidate an apply already in flight. When that apply later resolved
  'unplaced' its generation still matched, so it started a fresh chain from
  attempt 0; a host repeating placed pushes during each in-flight retry could
  reset the budget indefinitely. Retirement now bumps the generation too.

- the in-flight marker was a bare Set, so a superseded callback's finally
  deleted whichever marker was present - including one a newer generation had
  since taken. A later report could then arm an overlapping timer while that
  newer apply was still running. Ownership is now a target -> generation map and
  a callback releases only the marker it still owns.

resetTarget deliberately no longer drops the marker: its owner is the only party
that may release it, and clearing it there would let a new chain arm while the
superseded apply is still running.

* fix(ssh): replay an unplaced report that was blocked by a superseded apply

Regression from the previous commit. Keeping the stale in-flight marker across
resetTarget stops overlap, but it also swallows the new connection's result: the
new apply reports 'unplaced', hits the guard because the superseded apply still
owns the marker, and the superseded callback then exits on its stale generation
without scheduling. Nothing replayed the dropped report, so no chain started -
the retry silently never ran for that connection.

A blocked unplaced report is now recorded, and the marker's owner replays it on
release. Only the stale path reaches the finally still owning the marker, so the
normal path - which released early and scheduled its own outcome - cannot replay
twice.

* refactor(ssh): drop the re-pull retry chain, keep the fix

The chain produced ten defects across review - unbounded retry, dead retry,
cancelled final attempt, wedged chain, repeated exhaustion, overlapping chains,
stale-callback cleanup, a lost report - every one in code that passed the full
suite at the time. It bought only faster recovery *within* one connection:
syncAfterConnect and applyUnsolicitedSnapshot already re-pull on the next
connect or host push, so dropping it costs a retry, never the data.

What remains is the part that was correct from the first commit: the importer
reports rows it could not place, and an apply that could not place them neither
marks the target hydrated nor sets 'synced'. Because hydration is what
authorises uploads, and an upload wholesale replaces the host snapshot, that
single rule is what stops a client from deleting the host tabs it failed to
adopt.

Status is now 'error' rather than 'pulling': with no chain pending, 'pulling'
claimed a request that was not in flight.

* test(ssh): name the upload-suppression case for the chainless design

* fix(ssh): revoke stale hydration and keep authority unverifiable when unplaced

Two holes in the previous commit, both found in review.

The hydrated set is add-only (ssh.ts), so withholding hydration only protects a
target that never synced. A target that synced cleanly and then reconnected with
a degraded lineage kept its flag, and hydration is what authorises uploads - so
it would still send a replace-session patch built from the incomplete picture
and delete the host tabs it had just failed to place. Hydration is now revoked,
not merely withheld.

The status phase was 'error'. workspace-terminal-host-authority.ts treats
'offline'/'error' on an un-hydrated target as its bounded floor and resolves
them to 'none' - which authorises seeding AND sleeping-agent resume, the exact
double-resume this gate exists to prevent. 'conflict' is the phase that actually
describes the situation, is excluded from uploads by use-app-session-persistence,
and is deliberately outside that floor set, so authority stays 'unverifiable'.

Both invariants are pinned by tests verified to fail when either fix is reverted
individually; the pre-existing tests passed with both reverted.

* fix(ssh): drop the mismatched message on the unplaced conflict status

The phase drives the user-visible label ('Workspace sync conflict'); carrying an
'unavailable' message alongside it only risked contradicting that wherever the
message is surfaced.

* docs+refactor(ssh): correct the authority floor's premise, drop a dead wrapper

Two findings from the post-merge correctness sweep.

The bounded floor in workspace-terminal-host-authority.ts justified itself on
'remoteWorkspaceHydratedTargetIds is add-only, clearRemoteWorkspaceHydrated has
no production caller'. This branch adds that caller, so the premise is now false
and a future reader would have been misled by it. The comment records the real
consequence: a target that later lands on offline/error reaches the floor having
demonstrably answered, so seeding is authorised over live host terminals. Not a
regression - before revocation existed the same target was marked hydrated and
synced, reaching 'none' sooner - but the floor should learn to tell a revoked
target from one that never answered. Flagged for the SSH-v3 consolidation, where
one authoritative liveness source replaces this pair.

applyUnsolicitedSnapshot had become a pass-through to applyPreparedSnapshot,
carrying a docstring about a re-pull chain that no longer exists. The two
collapse back into one function.

* refactor(ssh): delete the DirectSshSnapshotPlacement union

Consolidation pass finding. The union was exported and threaded through two
modules, but no production consumer ever read it: remote-workspace-ipc-bridge.ts
discards the promise's value and syncAfterConnect ignored it. 'not-applied' was
not a placement at all, only 'this apply did not happen'.

That is a parallel verdict concept with no consumer - precisely what the SSH-v3
consolidation would have had to unpick. It collapses to a local
hasUnplacedTerminalTabs boolean and a void return.

The one test that asserted the return value now asserts adoption instead, which
is the observable outcome rather than a proxy for it. All five unplaced oracles
still fail when the placement decision is forced, verified individually.

* docs(ssh): compress the tombstone rationale to its load-bearing WHY

Elegance pass. Kept the two non-obvious claims - absence cannot distinguish
'never told' from 'user closed', and uuid tab ids make a tombstoned id safe to
drop - and cut the incident narrative around them. The twice-reverted history in
remote-workspace-session-merge.ts is deliberately left alone: that one is
institutional memory about regressions, not restatement of the code.

* test(ssh): pin the fixed behaviour instead of the defect it replaced

The spec was a characterization test whose own title said 'because hydration is
marked even when adoption wrote nothing', and whose comment described exactly
the defect this branch fixes: markRemoteWorkspaceHydrated ran unconditionally
after the hydrate calls, so in the same tick adoption yielded zero, authority
flipped unverifiable -> none, and Terminal.tsx seeded a phantom tab. It polled
for hydrated === true, so the fix turned it red.

It now asserts hydrated === false, phase === 'conflict', and zero tabs - the
count measured before asserting rather than assumed, confirming the phantom seed
is gone. The phase is re-read after the tabs settle and asserted a second time,
because a conflict verdict a later apply flipped back would silently re-authorise
seeding and a single poll would miss it.

The fixme stays a fixme: this branch stops the client claiming false authority
and overwriting the host, but adoption is still the open gap. Declining to seed
is a safe wait, not the destination.

Three-legged A/B against fork point c72afda498, spec byte-identical across the
first two legs:
  branch   + updated spec  -> PASS
  baseline + updated spec  -> FAIL 'never reported the unplaced snapshot as a conflict'
  baseline + original spec -> PASS (baseline actively exhibits the old behaviour)
2026-08-27 22:14:35 -07:00

317 lines
14 KiB
TypeScript

import { spawnSync } from 'node:child_process'
import type { ElectronApplication, Page } from '@stablyai/playwright-test'
import { test, expect } from './helpers/orca-app'
import { waitForActiveWorktree, waitForSessionReady } from './helpers/store'
import { waitForActivePanePtyId, waitForActiveTerminalManager } from './helpers/terminal'
import { createRemoteTerminalTab } from './helpers/docker-ssh-relay-terminal-tabs'
import {
cleanupDockerSshRelayTarget,
execDockerSshRelayTargetControlCommand,
shellQuote,
startDockerSshRelayTarget,
type DockerSshRelayTarget
} from './helpers/docker-ssh-relay-target'
import { connectDockerSshRelayTarget } from './helpers/docker-ssh-relay-connection'
import { createRestartSession } from './helpers/orca-restart'
const RUN_DOCKER_SSH = process.env.ORCA_E2E_SSH_DOCKER === '1'
const BASELINE_TAB_COUNT = 3
/** Where the relay persists a target's workspace snapshot inside the fixture container. */
const REMOTE_SNAPSHOT_DIR = '/root/.orca/sessions'
test.use({ seedTestRepo: false })
async function readWorktreeTabIds(page: Page, worktreeId: string): Promise<string[]> {
return page.evaluate(
(id) => (window.__store?.getState().tabsByWorktree[id] ?? []).map((tab) => tab.id),
worktreeId
)
}
async function isTargetHydrated(page: Page, targetId: string): Promise<boolean> {
return page.evaluate(
(id) => window.__store?.getState().remoteWorkspaceHydratedTargetIds.has(id) === true,
targetId
)
}
async function readTargetSyncPhase(page: Page, targetId: string): Promise<string | undefined> {
return page.evaluate(
(id) => window.__store?.getState().remoteWorkspaceSyncStatusByTargetId[id]?.phase,
targetId
)
}
/** Poll until the worktree's tabs stop changing, so a late seed cannot slip past the sample. */
async function waitForSettledTabIds(page: Page, worktreeId: string): Promise<string[]> {
let latest: string[] = []
let previousKey = ''
let agreements = 0
await expect
.poll(
async () => {
latest = await readWorktreeTabIds(page, worktreeId)
const key = latest.join()
agreements = key === previousKey ? agreements + 1 : 0
previousKey = key
return agreements
},
{ timeout: 60_000, intervals: [1_000], message: 'the tab set never stopped changing' }
)
.toBeGreaterThanOrEqual(3)
return latest
}
function findRemoteSnapshotPath(target: DockerSshRelayTarget): string | null {
const listing = execDockerSshRelayTargetControlCommand(
target,
`ls -1 ${REMOTE_SNAPSHOT_DIR}/*.json 2>/dev/null || true`
).trim()
return listing.split('\n').find((line) => line.endsWith('.json')) ?? null
}
async function waitForUploadedRemoteSnapshot(target: DockerSshRelayTarget): Promise<string> {
let snapshotPath: string | null = null
await expect
.poll(
() => {
snapshotPath = findRemoteSnapshotPath(target)
return snapshotPath
},
{ timeout: 60_000, message: 'the relay never persisted a workspace snapshot' }
)
.not.toBeNull()
return snapshotPath!
}
/**
* Replace the relay's snapshot file with a FIFO so `workspace.get` blocks on open.
*
* Why a FIFO and not a stall injected into the app: the relay reads this path with `readFileSync`,
* so an unopened FIFO stalls the real RPC exactly where a stalled link would, and writing to it
* later releases that same call with the real bytes. A test-only hook would drift from the
* production path silently, which is how this class of bug survives in the first place.
*/
function blockRemoteWorkspaceGet(target: DockerSshRelayTarget, snapshotPath: string): string {
const saved = execDockerSshRelayTargetControlCommand(target, `cat ${snapshotPath}`)
execDockerSshRelayTargetControlCommand(
target,
`rm -f ${snapshotPath} && mkfifo -m 600 ${snapshotPath}`
)
return saved
}
function unblockRemoteWorkspaceGet(
target: DockerSshRelayTarget,
snapshotPath: string,
saved: string
): void {
// Detached: a FIFO write blocks until the reader drains it, which must not stall the test.
spawnSync('docker', [
'exec',
'-d',
target.containerName,
'bash',
'--noprofile',
'--norc',
'-c',
`printf '%s' ${shellQuote(saved)} > ${snapshotPath} && rm -f ${snapshotPath} && printf '%s' ${shellQuote(saved)} > ${snapshotPath}`
])
}
async function connectAndSeedTabs(
page: Page,
target: DockerSshRelayTarget
): Promise<{ targetId: string; repoId: string; worktreeId: string; tabIds: string[] }> {
const remote = await connectDockerSshRelayTarget(page, target)
await expect.poll(() => waitForActiveWorktree(page), { timeout: 30_000 }).toBe(remote.worktreeId)
await waitForActiveTerminalManager(page, 60_000)
await waitForActivePanePtyId(page, 60_000)
while ((await readWorktreeTabIds(page, remote.worktreeId)).length < BASELINE_TAB_COUNT) {
await createRemoteTerminalTab(page, remote.worktreeId)
}
return { ...remote, tabIds: await waitForSettledTabIds(page, remote.worktreeId) }
}
async function flushSessionBeforeQuit(page: Page, targetId: string): Promise<void> {
await page.evaluate(() => window.dispatchEvent(new Event('beforeunload')))
await expect
.poll(
() =>
page.evaluate(async (id) => {
const persisted = await window.api.session.get()
return persisted.activeConnectionIdsAtShutdown?.includes(id) === true
}, targetId),
{ timeout: 15_000, message: 'the active SSH target was not persisted before quit' }
)
.toBe(true)
}
test.describe('SSH cold hydration gap tab seeding', () => {
test.skip(!RUN_DOCKER_SSH, 'Set ORCA_E2E_SSH_DOCKER=1 to run Docker-backed SSH tests.')
test.skip(process.platform === 'win32', 'Docker SSH restore uses POSIX SSH tooling.')
// Why this shape: worktree activation seeds an initial terminal from a predicate that knows
// nothing about host authority, so a relaunch that activates the worktree before the host
// snapshot lands can add a tab the host never had. Stalling `workspace.get` holds that window
// open for as long as the assertions need instead of racing a local relay.
test('adds no tab when the host workspace snapshot stalls across a relaunch', async (// oxlint-disable-next-line no-empty-pattern -- This restart test owns every Electron launch.
{}, testInfo) => {
test.setTimeout(600_000)
const restart = createRestartSession(testInfo)
let target: DockerSshRelayTarget | null = null
let app: ElectronApplication | null = null
try {
target = startDockerSshRelayTarget(testInfo)
const firstLaunch = await restart.launch()
app = firstLaunch.app
await waitForSessionReady(firstLaunch.page)
const remote = await connectAndSeedTabs(firstLaunch.page, target)
expect(remote.tabIds).toHaveLength(BASELINE_TAB_COUNT)
const snapshotPath = await waitForUploadedRemoteSnapshot(target)
await flushSessionBeforeQuit(firstLaunch.page, remote.targetId)
await restart.close(app)
app = null
const saved = blockRemoteWorkspaceGet(target, snapshotPath)
const relaunch = await restart.launch()
app = relaunch.app
const page = relaunch.page
// No PTY wait here: the stalled read holds the relay's only thread, so nothing else it serves
// can complete either. That is the point — this is the window a resumed laptop sits in.
await waitForSessionReady(page, 60_000)
await expect
.poll(() => waitForActiveWorktree(page), { timeout: 60_000 })
.toBe(remote.worktreeId)
// Proves the intended branch was taken rather than the symptom merely being absent: the
// target must still be unhydrated while the snapshot has not arrived.
expect(
await isTargetHydrated(page, remote.targetId),
'the target hydrated despite a stalled workspace.get, so this never entered the gap'
).toBe(false)
const duringStall = await waitForSettledTabIds(page, remote.worktreeId)
unblockRemoteWorkspaceGet(target, snapshotPath, saved)
await expect
.poll(() => isTargetHydrated(page, remote.targetId), {
timeout: 120_000,
message: 'the target never hydrated after the snapshot was released'
})
.toBe(true)
const afterHydration = await waitForSettledTabIds(page, remote.worktreeId)
const growth = `baseline=${remote.tabIds.length} duringStall=${duringStall.length} afterHydration=${afterHydration.length}`
expect(duringStall.slice().sort(), `tabs changed inside the stall window: ${growth}`).toEqual(
remote.tabIds.slice().sort()
)
expect(
afterHydration.slice().sort(),
`tabs changed once the stalled snapshot landed: ${growth}`
).toEqual(remote.tabIds.slice().sort())
} finally {
if (app) {
await restart.close(app)
}
await restart.dispose()
cleanupDockerSshRelayTarget(target)
}
})
// Why a second profile and not another restart: the seeding predicate is
// `renderableTabCount === 0 && !Object.hasOwn(tabsByWorktree, worktreeId)`, and a restart always
// restores that key from local state, so the second term is never false. A client that has never
// held this workspace — a re-added host, a cleared profile, a second machine — is the ordinary
// way a user reaches a host that already owns tabs with no local row for them.
test('does not mark hydration or seed a tab when it could not place the host tabs', async (// oxlint-disable-next-line no-empty-pattern -- This restart test owns every Electron launch.
{}, testInfo) => {
test.setTimeout(600_000)
const seeding = createRestartSession(testInfo)
const fresh = createRestartSession(testInfo)
let target: DockerSshRelayTarget | null = null
let seedingApp: ElectronApplication | null = null
let freshApp: ElectronApplication | null = null
try {
target = startDockerSshRelayTarget(testInfo)
const firstLaunch = await seeding.launch()
seedingApp = firstLaunch.app
await waitForSessionReady(firstLaunch.page)
const remote = await connectAndSeedTabs(firstLaunch.page, target)
expect(remote.tabIds).toHaveLength(BASELINE_TAB_COUNT)
await waitForUploadedRemoteSnapshot(target)
await flushSessionBeforeQuit(firstLaunch.page, remote.targetId)
await seeding.close(seedingApp)
seedingApp = null
const freshLaunch = await fresh.launch()
freshApp = freshLaunch.app
await waitForSessionReady(freshLaunch.page, 60_000)
// seedInitialTab: false so every tab counted below is one the product produced — the helper's
// own convenience tab would otherwise be indistinguishable from a spurious seed.
const rejoined = await connectDockerSshRelayTarget(freshLaunch.page, target, {
seedInitialTab: false
})
await expect
.poll(() => waitForActiveWorktree(freshLaunch.page), { timeout: 60_000 })
.toBe(rejoined.worktreeId)
// The sync point the old `hydrated === true` poll used to serve. `conflict` is what the apply
// publishes once it finds rows it cannot place, so it marks the same instant without pinning
// the defect: hydration used to be marked here regardless of what adoption wrote.
await expect
.poll(() => readTargetSyncPhase(freshLaunch.page, rejoined.targetId), {
timeout: 120_000,
message: 'the fresh client never reported the unplaced snapshot as a conflict'
})
.toBe('conflict')
const rejoinedTabIds = await waitForSettledTabIds(freshLaunch.page, rejoined.worktreeId)
const hydrated = await isTargetHydrated(freshLaunch.page, rejoined.targetId)
// Re-read after settling: a conflict verdict that a later apply flips back would re-authorise
// seeding, so the phase has to still hold once the tab set has stopped moving.
const settledPhase = await readTargetSyncPhase(freshLaunch.page, rejoined.targetId)
console.log(
`[unplaced-host-tabs] hydrated=${hydrated} phase=${settledPhase} tabs=${rejoinedTabIds.length}`
)
// STA-3593. The host listed three tabs on paths this client cannot place. Adoption still
// writes nothing (the fixme below), but the client must no longer claim the host's workspace
// on the strength of that empty result:
// 1. hydration is not marked — and is revoked if an earlier clean sync had set it — so
// use-app-session-persistence.ts cannot upload a `replace-session` patch built from the
// incomplete picture and delete the very tabs it failed to place;
// 2. the phase is `conflict`, which workspace-terminal-host-authority.ts deliberately keeps
// out of its `offline`/`error` floor, so authority stays `unverifiable` rather than
// resolving to `none`;
// 3. therefore Terminal.tsx does not seed. The old behaviour was exactly one tab conjured
// from nothing, replacing the host's three.
expect(hydrated, 'an unplaced snapshot must not leave the target marked hydrated').toBe(false)
expect(
settledPhase,
'the unplaced verdict has to survive settling, or authority is re-authorised to seed'
).toBe('conflict')
expect(
rejoinedTabIds.length,
`authority stays unverifiable, so no tab may be seeded: got ${rejoinedTabIds.length}`
).toBe(0)
} finally {
if (freshApp) {
await fresh.close(freshApp)
}
if (seedingApp) {
await seeding.close(seedingApp)
}
await fresh.dispose()
await seeding.dispose()
cleanupDockerSshRelayTarget(target)
}
})
// The remaining half of the gap. The hydration half above is fixed: the client no longer marks
// hydration, no longer overwrites the host, and no longer seeds a phantom tab. What it still does
// not do is ADOPT — a client with no local row is exactly the case the host snapshot exists to
// serve, so it should end up holding the host's tabs rather than an empty workspace. Declining to
// seed is a safe wait, not the destination. Kept as a fixme so the gap stays visible without
// putting a knowingly-red spec in the lane. STA-3593 (snapshot tabs dropped when the worktree
// catalog resolves their paths late).
test.fixme('a client with no local row adopts the tabs the host already owns', async (// oxlint-disable-next-line no-empty-pattern -- Placeholder for the fixed behaviour.
{}) => {})
})