From a7c18c31191ba51cd4b7be4eeafab9c59cb8256a Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Thu, 27 Aug 2026 01:56:33 -0700 Subject: [PATCH] fix(terminal): do not seed or resume while the execution host has not answered 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 ` 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. --- config/scripts/run-ssh-docker-e2e.mjs | 1 + src/renderer/src/components/Terminal.tsx | 23 +- ...-snapshot-fresh-client-tab-seeding.test.ts | 179 +++++++++++ ...t-session-direct-ssh-hydration-gap.test.ts | 135 ++++++++ .../src/lib/resume-sleeping-agent-session.ts | 9 + .../workspace-terminal-host-authority.test.ts | 228 ++++++++++++++ .../lib/workspace-terminal-host-authority.ts | 165 ++++++++++ .../lib/worktree-initial-terminal-seeding.ts | 21 +- ...ssh-cold-hydration-gap-tab-seeding.spec.ts | 295 ++++++++++++++++++ 9 files changed, 1042 insertions(+), 14 deletions(-) create mode 100644 src/renderer/src/hooks/remote-workspace-snapshot-fresh-client-tab-seeding.test.ts create mode 100644 src/renderer/src/lib/resume-sleeping-agent-session-direct-ssh-hydration-gap.test.ts create mode 100644 src/renderer/src/lib/workspace-terminal-host-authority.test.ts create mode 100644 src/renderer/src/lib/workspace-terminal-host-authority.ts create mode 100644 tests/e2e/ssh-cold-hydration-gap-tab-seeding.spec.ts diff --git a/config/scripts/run-ssh-docker-e2e.mjs b/config/scripts/run-ssh-docker-e2e.mjs index 8000c8a0c26..d5bab9aca0a 100644 --- a/config/scripts/run-ssh-docker-e2e.mjs +++ b/config/scripts/run-ssh-docker-e2e.mjs @@ -70,6 +70,7 @@ const result = spawnSync( 'tests/e2e/pty-input-write-queue-ssh.spec.ts', 'tests/e2e/ssh-ai-vault-session-history.spec.ts', 'tests/e2e/ssh-cold-activation-restore.spec.ts', + 'tests/e2e/ssh-cold-hydration-gap-tab-seeding.spec.ts', 'tests/e2e/ssh-docker-reconnect-pane-restore.spec.ts', 'tests/e2e/ssh-external-image-preview.spec.ts', 'tests/e2e/ssh-pi-compatible-agent-title.spec.ts', diff --git a/src/renderer/src/components/Terminal.tsx b/src/renderer/src/components/Terminal.tsx index 089f799bbbf..1889c1b7bbd 100644 --- a/src/renderer/src/components/Terminal.tsx +++ b/src/renderer/src/components/Terminal.tsx @@ -79,6 +79,7 @@ import { import TabGroupSplitLayout from './tab-group/TabGroupSplitLayout' import AiVaultSessionDropLayer from './tab-group/AiVaultSessionDropLayer' import { shouldAutoCreateInitialTerminal } from './terminal/initial-terminal' +import { createWorkspaceTerminalHostAuthoritySelector } from '@/lib/workspace-terminal-host-authority' import { useActiveTerminalRepair } from './terminal/use-active-terminal-repair' import { scheduleBackgroundTerminalWorktreeMeasure } from './terminal/background-terminal-worktree-visibility' import { @@ -1514,6 +1515,15 @@ function Terminal(): React.JSX.Element | null { const activeWorktreeHasTerminalState = activeWorktreeId ? Object.hasOwn(tabsByWorktree, activeWorktreeId) : false + // Why a store subscription rather than a read inside the effects: the verdict flips to `none` the + // moment the execution host answers, and that transition is what re-runs the passes below. + // Why the retained selector: resolution walks the owner catalogs, so recomputing it on every store + // write would be the STA-3363 render-path multiplier again. + const hostAuthoritySelector = useMemo( + () => createWorkspaceTerminalHostAuthoritySelector(activeWorktreeId), + [activeWorktreeId] + ) + const activeWorktreeHostAuthority = useAppStore(hostAuthoritySelector) useEffect(() => { if (!workspaceSessionReady) { return @@ -1521,8 +1531,9 @@ function Terminal(): React.JSX.Element | null { if (!activeWorktreeId) { return } - // Why: host session-tabs are authoritative in the paired web client; a local fallback races the host's initial terminal and duplicates tabs. - if (isWebRuntimeSessionActive(getActiveWorktreeRuntimeEnvironmentId(activeWorktreeId))) { + // Why: the execution host owns terminal creation, and a host that has not answered is not a host + // with no terminals — seeding into that gap duplicates its tabs on every launch (STA-4658). + if (activeWorktreeHostAuthority !== 'none') { return } @@ -1537,6 +1548,7 @@ function Terminal(): React.JSX.Element | null { workspaceSessionReady, activeWorktreeId, activeWorktreeHasTerminalState, + activeWorktreeHostAuthority, createTab, reconcileWorktreeTabModel ]) @@ -1549,10 +1561,15 @@ function Terminal(): React.JSX.Element | null { if (startupResumeWorktreeIdsRef.current.has(activeWorktreeId)) { return } + // Why not consume the one-shot here: the sweep declines outright while the host is unanswered, + // so marking it done would strand every sleeping agent on the workspace for the session. + if (activeWorktreeHostAuthority === 'unverifiable') { + return + } startupResumeWorktreeIdsRef.current.add(activeWorktreeId) // Why: startup hydration restores the worktree without activateAndRevealWorktree, so orphaned live/quit records need a terminal-surface pass after cold restore. resumeSleepingAgentSessionsForWorktree(activeWorktreeId) - }, [activeWorktreeId, hydrationSucceeded, workspaceSessionReady]) + }, [activeWorktreeId, activeWorktreeHostAuthority, hydrationSucceeded, workspaceSessionReady]) const handleNewTab = useCallback( (shellOverride?: string) => { diff --git a/src/renderer/src/hooks/remote-workspace-snapshot-fresh-client-tab-seeding.test.ts b/src/renderer/src/hooks/remote-workspace-snapshot-fresh-client-tab-seeding.test.ts new file mode 100644 index 00000000000..0400f860d31 --- /dev/null +++ b/src/renderer/src/hooks/remote-workspace-snapshot-fresh-client-tab-seeding.test.ts @@ -0,0 +1,179 @@ +/** + * A client that has never held a direct-SSH workspace — a re-added host, a cleared profile, a second + * machine — activates it before the host snapshot lands and seeds an initial terminal into that gap. + * The snapshot then arrives, the merge correctly keeps the locally-created tab it was never told + * about, and the union uploads as the new host truth. + * + * Scope, because it moved twice under investigation: an ordinary RESTART does not reach this, since + * local session state restores the workspace's tab row first. The trigger is a client with no local + * row for a workspace the host already owns (#15556 / STA-4908). + * + * Two oracles. Cardinality: the client must not add a tab the host did not name. Adoption: declining + * to seed must not cost the user the host's real tabs — an empty workspace is not an improvement on + * a wrong one. + */ +import { describe, expect, it, vi } from 'vitest' +import type * as AgentStatusModule from '@/lib/agent-status' +import type { RemoteWorkspaceSnapshot } from '../../../shared/remote-workspace-types' +import type { DirectSshAuthority, SshProviderEpoch } from '../../../shared/ssh-types' +import { createTestStore, makeWorktree } from '../store/slices/store-test-helpers' +import { ensureWorktreeHasInitialTerminal } from '@/lib/worktree-initial-terminal-seeding' +import { applyDirectSshRemoteWorkspaceSnapshot } from './remote-workspace-snapshot-apply' +import type { DirectSshSnapshotApplyToken } from './direct-ssh-reconnect-coordinator-types' + +vi.mock('sonner', () => ({ toast: { info: vi.fn(), success: vi.fn(), error: vi.fn() } })) +vi.mock('@/lib/agent-status', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, detectAgentStatusFromTitle: vi.fn().mockReturnValue(null) } +}) + +const TARGET_ID = 'ssh-target-1' +const PATH = '/srv/proj/bug-cats' +const WORKTREE_ID = `repoA::${PATH}` + +const authority: DirectSshAuthority = { + targetId: TARGET_ID, + providerEpoch: 'provider-epoch-1' as SshProviderEpoch, + connectionGeneration: 1 +} + +function token(snapshotRevision: number): DirectSshSnapshotApplyToken { + return { + authority, + catalogRevision: 0, + repoFingerprint: 'fp', + authorityRequirement: 'required', + snapshotRevision, + outcome: 'complete' + } +} + +function snapshot(revision: number, tabIds: readonly string[]): RemoteWorkspaceSnapshot { + return { + namespace: 'workspace', + revision, + updatedAt: revision, + schemaVersion: 1, + session: { + activeWorktreePath: PATH, + activeTabId: tabIds[0] ?? null, + tabsByWorktreePath: { + [PATH]: tabIds.map((tabId, index) => ({ + id: tabId, + worktreePath: PATH, + ptyId: `pty-${tabId}`, + title: `Terminal ${index + 1}`, + customTitle: null, + color: null, + sortOrder: index, + createdAt: index + 1 + })) + }, + terminalLayoutsByTabId: {}, + activeWorktreePathsOnShutdown: [], + activeTabIdByWorktreePath: { [PATH]: tabIds[0] ?? null }, + remoteSessionIdsByTabId: Object.fromEntries(tabIds.map((id) => [id, `pty-${id}`])), + lastVisitedAtByWorktreePath: { [PATH]: revision }, + defaultTerminalTabsAppliedByWorktreePath: { [PATH]: true } + } + } satisfies RemoteWorkspaceSnapshot +} + +type TestStore = ReturnType + +async function applySnapshot(store: TestStore, snap: RemoteWorkspaceSnapshot): Promise { + await applyDirectSshRemoteWorkspaceSnapshot({ + store, + snapshot: snap, + token: token(snap.revision), + arrival: 1, + isArrivalCurrent: () => true, + isPreparationTokenCurrent: () => true, + waitForWorkspaceSessionReady: async () => true, + finalizeHydratedTerminals: () => 0 + }) +} + +function seedCatalog(store: TestStore): void { + store.setState({ + worktreesByRepo: { + repoA: [ + makeWorktree({ + id: WORKTREE_ID, + repoId: 'repoA', + path: PATH, + hostId: `ssh:${TARGET_ID}` + } as never) + ] + }, + repos: [ + { + id: 'repoA', + path: '/srv/proj', + displayName: 'Proj', + badgeColor: '#000', + addedAt: 0, + connectionId: TARGET_ID + } as never + ], + // Why: only the IPC-backed reconnect is stubbed; hydration bookkeeping stays real because it is + // the very signal under test. + reconnectPersistedTerminals: (async () => {}) as never, + setRemoteWorkspaceSyncStatus: (() => {}) as never + }) +} + +/** A relaunch: the workspace's tab rows and this session's host answers are both gone. */ +function simulateColdStart(store: TestStore): void { + const live = store.getState() + const { [WORKTREE_ID]: _tabs, ...tabsByWorktree } = live.tabsByWorktree + const { [WORKTREE_ID]: _unified, ...unifiedTabsByWorktree } = live.unifiedTabsByWorktree + store.setState({ + tabsByWorktree, + unifiedTabsByWorktree, + remoteWorkspaceHydratedTargetIds: new Set() + }) +} + +function terminalTabIds(store: TestStore): string[] { + return (store.getState().tabsByWorktree[WORKTREE_ID] ?? []).map((tab) => tab.id) +} + +describe('a fresh client meeting a workspace the host already owns', () => { + it('holds one tab when the host still reports one tab', async () => { + const store = createTestStore() + seedCatalog(store) + + // Yesterday: the host owns exactly one terminal in this workspace. + await applySnapshot(store, snapshot(1, ['T1'])) + expect(terminalTabIds(store)).toEqual(['T1']) + + // This morning: the app restarts and activates the workspace before SSH has answered. + simulateColdStart(store) + ensureWorktreeHasInitialTerminal(store.getState(), WORKTREE_ID) + + // The host answers, still holding only T1. + await applySnapshot(store, snapshot(2, ['T1'])) + + expect( + terminalTabIds(store), + 'the launch seeded a terminal into the gap before the host answered' + ).toEqual(['T1']) + }) + + it('adopts every tab the host owns instead of replacing them with a seeded one', async () => { + const store = createTestStore() + seedCatalog(store) + + // A client with no local row for this workspace, activating it before the snapshot lands. + expect(store.getState().tabsByWorktree[WORKTREE_ID]).toBeUndefined() + ensureWorktreeHasInitialTerminal(store.getState(), WORKTREE_ID) + + await applySnapshot(store, snapshot(1, ['T1', 'T2', 'T3'])) + + expect( + terminalTabIds(store), + 'declining to seed must not also cost the user the tabs the host really owns' + ).toEqual(['T1', 'T2', 'T3']) + }) +}) diff --git a/src/renderer/src/lib/resume-sleeping-agent-session-direct-ssh-hydration-gap.test.ts b/src/renderer/src/lib/resume-sleeping-agent-session-direct-ssh-hydration-gap.test.ts new file mode 100644 index 00000000000..be73993c49c --- /dev/null +++ b/src/renderer/src/lib/resume-sleeping-agent-session-direct-ssh-hydration-gap.test.ts @@ -0,0 +1,135 @@ +/** + * STA-3500: a direct-SSH workspace's tab rows only reach the renderer after connect, via + * remoteWorkspace.get → merge — a network round trip. The startup resume sweep fires before that + * lands, sees no pane owning a sleeping record, and cold-resumes a session the host is still + * running. STA-3498 counted five concurrent `claude` processes on one transcript that way. + * + * The paired-runtime flavor already parks per pane (host-mirrored-pane-liveness.ts), but that guard + * only recognises `web-terminal-*` surface ids, so direct SSH fell straight through it. + * + * Both halves matter: the sweep must decline while the host is unanswered, and it must still wake a + * genuinely sleeping agent once the host answers. + */ +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { SleepingAgentSessionRecord } from '../../../shared/agent-session-resume' +import { useAppStore } from '@/store' +import { makeWorktree } from '@/store/slices/store-test-helpers' +import { resumeSleepingAgentSessionsForWorktree } from './resume-sleeping-agent-session' + +const initialAppStoreState = useAppStore.getState() + +const TARGET_ID = 'ssh-target-1' +const PATH = '/srv/proj/feature' +const WORKTREE_ID = `repoSsh::${PATH}` +const LOCAL_WORKTREE_ID = 'repoLocal::/home/dev/proj/feature' + +afterEach(() => { + vi.unstubAllGlobals() + useAppStore.setState(initialAppStoreState, true) +}) + +function makeRecord(worktreeId: string): SleepingAgentSessionRecord { + return { + paneKey: 'tab-1:leaf-1', + tabId: 'tab-1', + worktreeId, + agent: 'claude', + providerSession: { key: 'session_id', id: 'sess-remote-1' }, + prompt: 'finish the migration', + state: 'working', + origin: 'quit', + capturedAt: 1, + updatedAt: 1 + } +} + +/** A cold start: the catalog knows the SSH workspace, but no tab row has arrived for it yet. */ +function seedColdDirectSshStart(): SleepingAgentSessionRecord { + const record = makeRecord(WORKTREE_ID) + useAppStore.setState({ + repos: [ + { + id: 'repoSsh', + path: '/srv/proj', + displayName: 'Proj', + badgeColor: '#000', + addedAt: 0, + connectionId: TARGET_ID + }, + { + id: 'repoLocal', + path: '/home/dev/proj', + displayName: 'Local', + badgeColor: '#000', + addedAt: 0 + } + ], + worktreesByRepo: { + repoSsh: [ + makeWorktree({ + id: WORKTREE_ID, + repoId: 'repoSsh', + path: PATH, + hostId: `ssh:${TARGET_ID}` + } as never) + ], + repoLocal: [ + makeWorktree({ + id: LOCAL_WORKTREE_ID, + repoId: 'repoLocal', + path: '/home/dev/proj/feature', + hostId: 'local' + } as never) + ] + }, + tabsByWorktree: {}, + sleepingAgentSessionsByPaneKey: { [record.paneKey]: record } + } as never) + return record +} + +function resumedTabCount(worktreeId: string): number { + return (useAppStore.getState().tabsByWorktree[worktreeId] ?? []).length +} + +describe('sleeping-agent resume across the direct-SSH hydration gap', () => { + it('does not resume a remote session before the host has answered', () => { + const record = seedColdDirectSshStart() + + const launched = resumeSleepingAgentSessionsForWorktree(WORKTREE_ID) + + expect(launched, 'cold-resumed a session the host may still be running').toBe(0) + expect(resumedTabCount(WORKTREE_ID)).toBe(0) + // Absence of evidence must not authorise deletion either — the record has to survive to wake later. + expect(useAppStore.getState().sleepingAgentSessionsByPaneKey[record.paneKey]).toBe(record) + }) + + it('wakes the same session once the host answers and holds no pane for it', () => { + const record = seedColdDirectSshStart() + resumeSleepingAgentSessionsForWorktree(WORKTREE_ID) + + useAppStore.getState().markRemoteWorkspaceHydrated(TARGET_ID) + const launched = resumeSleepingAgentSessionsForWorktree(WORKTREE_ID) + + expect(launched, 'the deferred agent never woke after the host answered').toBe(1) + const state = useAppStore.getState() + expect(state.tabsByWorktree[WORKTREE_ID]?.[0]?.launchAgent).toBe('claude') + expect(state.sleepingAgentSessionsByPaneKey[record.paneKey]).toBeUndefined() + }) + + it('leaves a purely local workspace resuming with no added latency', () => { + seedColdDirectSshStart() + const localRecord = { + ...makeRecord(LOCAL_WORKTREE_ID), + paneKey: 'tab-2:leaf-1', + tabId: 'tab-2' + } + useAppStore.setState({ + sleepingAgentSessionsByPaneKey: { [localRecord.paneKey]: localRecord } + } as never) + + // No hydration marked anywhere: a local workspace's host is this client, and it has answered. + expect(resumeSleepingAgentSessionsForWorktree(LOCAL_WORKTREE_ID)).toBe(1) + expect(resumedTabCount(LOCAL_WORKTREE_ID)).toBe(1) + }) +}) diff --git a/src/renderer/src/lib/resume-sleeping-agent-session.ts b/src/renderer/src/lib/resume-sleeping-agent-session.ts index bde0cca02c9..1865dc747af 100644 --- a/src/renderer/src/lib/resume-sleeping-agent-session.ts +++ b/src/renderer/src/lib/resume-sleeping-agent-session.ts @@ -14,6 +14,7 @@ import { type ResumeSleepingAgentSessionsOptions } from './sleeping-agent-session-launch' import { findUnhydratedHostMirrorForPane } from './host-mirrored-pane-liveness' +import { resolveWorkspaceTerminalHostAuthority } from './workspace-terminal-host-authority' import { parkUntilHostSessionMirrorHydrates } from '@/runtime/host-session-mirror-hydration' export type { ResumeSleepingAgentSessionsOptions } from './sleeping-agent-session-launch' @@ -171,6 +172,14 @@ export function resumeSleepingAgentSessionsForWorktree( options?: ResumeSleepingAgentSessionsOptions ): number { const state = useAppStore.getState() + // Why: every branch below reads local rows as the verdict on what the execution host is running, + // and before it answers "I hold no pane for this record" is `unverifiable`, not `exited`. Resuming + // on it forks a second agent onto a transcript the host is still writing (STA-3500). Declining is + // recoverable — the record survives, and the caller re-runs this sweep once the verdict lands. + // Paired-runtime workspaces keep their own per-pane mirror park below, which is finer-grained. + if (resolveWorkspaceTerminalHostAuthority(state, worktreeId) === 'unverifiable') { + return 0 + } const worktreeRecords = Object.values(state.sleepingAgentSessionsByPaneKey) .filter((record) => record.worktreeId === worktreeId) .sort((a, b) => a.capturedAt - b.capturedAt || a.updatedAt - b.updatedAt) diff --git a/src/renderer/src/lib/workspace-terminal-host-authority.test.ts b/src/renderer/src/lib/workspace-terminal-host-authority.test.ts new file mode 100644 index 00000000000..c7c5bc60f7e --- /dev/null +++ b/src/renderer/src/lib/workspace-terminal-host-authority.test.ts @@ -0,0 +1,228 @@ +/** + * The seeding predicate answers one question for both remote flavors: does some other party own + * terminal creation for this workspace right now? Three answers, and only `none` seeds. + */ +import { describe, expect, it, vi } from 'vitest' +import type * as AgentStatusModule from '@/lib/agent-status' +import { createTestStore, makeWorktree } from '@/store/slices/store-test-helpers' +import { ensureWorktreeHasInitialTerminal } from '@/lib/worktree-initial-terminal-seeding' +import { resolveWorkspaceTerminalHostAuthority } from '@/lib/workspace-terminal-host-authority' + +vi.mock('sonner', () => ({ toast: { info: vi.fn(), success: vi.fn(), error: vi.fn() } })) +vi.mock('@/lib/agent-status', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, detectAgentStatusFromTitle: vi.fn().mockReturnValue(null) } +}) + +type TestStore = ReturnType + +const LOCAL_WORKTREE_ID = 'repoLocal::/home/dev/proj/feature' +const SSH_WORKTREE_ID = 'repoSsh::/srv/proj/feature' +const PAIRED_WORKTREE_ID = 'repoPaired::/srv/proj/paired' +const TARGET_ID = 'ssh-target-1' +const ENVIRONMENT_ID = 'runtime-env-1' + +function repo(id: string, path: string, connectionId?: string): never { + return { + id, + path, + displayName: id, + badgeColor: '#000', + addedAt: 0, + ...(connectionId ? { connectionId } : {}) + } as never +} + +function seedLocal(store: TestStore): void { + store.setState({ + repos: [repo('repoLocal', '/home/dev/proj')], + worktreesByRepo: { + repoLocal: [ + makeWorktree({ + id: LOCAL_WORKTREE_ID, + repoId: 'repoLocal', + path: '/home/dev/proj/feature', + hostId: 'local' + } as never) + ] + } + }) +} + +function seedDirectSsh(store: TestStore): void { + store.setState({ + repos: [repo('repoSsh', '/srv/proj', TARGET_ID)], + worktreesByRepo: { + repoSsh: [ + makeWorktree({ + id: SSH_WORKTREE_ID, + repoId: 'repoSsh', + path: '/srv/proj/feature', + hostId: `ssh:${TARGET_ID}` + } as never) + ] + } + }) +} + +function terminalTabCount(store: TestStore, worktreeId: string): number { + return (store.getState().tabsByWorktree[worktreeId] ?? []).length +} + +describe('workspace terminal seeding authority', () => { + it('seeds a purely local workspace immediately, exactly once', () => { + const store = createTestStore() + seedLocal(store) + + expect(resolveWorkspaceTerminalHostAuthority(store.getState(), LOCAL_WORKTREE_ID)).toBe('none') + + // No await, no host round trip: the client is the execution host for a local workspace. + const tabId = ensureWorktreeHasInitialTerminal(store.getState(), LOCAL_WORKTREE_ID) + expect(tabId).toBeTruthy() + expect(terminalTabCount(store, LOCAL_WORKTREE_ID)).toBe(1) + + // A second activation pass must not add another. + ensureWorktreeHasInitialTerminal(store.getState(), LOCAL_WORKTREE_ID) + expect(terminalTabCount(store, LOCAL_WORKTREE_ID)).toBe(1) + }) + + it('distinguishes an unanswered SSH host from one that answered with nothing', () => { + const store = createTestStore() + seedDirectSsh(store) + + expect(resolveWorkspaceTerminalHostAuthority(store.getState(), SSH_WORKTREE_ID)).toBe( + 'unverifiable' + ) + expect(ensureWorktreeHasInitialTerminal(store.getState(), SSH_WORKTREE_ID)).toBeNull() + expect(terminalTabCount(store, SSH_WORKTREE_ID)).toBe(0) + + // The host answers and holds nothing here. That is positive evidence, so the workspace seeds. + store.getState().markRemoteWorkspaceHydrated(TARGET_ID) + expect(resolveWorkspaceTerminalHostAuthority(store.getState(), SSH_WORKTREE_ID)).toBe('none') + expect(ensureWorktreeHasInitialTerminal(store.getState(), SSH_WORKTREE_ID)).toBeTruthy() + expect(terminalTabCount(store, SSH_WORKTREE_ID)).toBe(1) + }) + + it.each(['offline', 'error'] as const)( + 'falls back to none when a sync terminates in %s without ever hydrating', + (phase) => { + // The regression this guards: remoteWorkspaceHydratedTargetIds is add-only in practice + // (clearRemoteWorkspaceHydrated has no production caller), so without a floor one failed sync + // leaves every git worktree on this target terminal-less and its sleeping agents unresumable + // for the rest of the app session — strictly worse than the pre-gate behaviour, and escapable + // only by creating a tab by hand. + const store = createTestStore() + seedDirectSsh(store) + store.getState().setRemoteWorkspaceSyncStatus(TARGET_ID, { phase, direction: 'pull' }) + + expect(resolveWorkspaceTerminalHostAuthority(store.getState(), SSH_WORKTREE_ID)).toBe('none') + expect(ensureWorktreeHasInitialTerminal(store.getState(), SSH_WORKTREE_ID)).toBeTruthy() + expect(terminalTabCount(store, SSH_WORKTREE_ID)).toBe(1) + } + ) + + it('still declines while a sync is in flight', () => { + // The floor must not swallow the wait it exists to bound: 'pulling' is an attempt in progress, + // not one that terminated without an answer. + const store = createTestStore() + seedDirectSsh(store) + store + .getState() + .setRemoteWorkspaceSyncStatus(TARGET_ID, { phase: 'pulling', direction: 'pull' }) + + expect(resolveWorkspaceTerminalHostAuthority(store.getState(), SSH_WORKTREE_ID)).toBe( + 'unverifiable' + ) + expect(ensureWorktreeHasInitialTerminal(store.getState(), SSH_WORKTREE_ID)).toBeNull() + }) + + it('keeps a hydrated target on none even if a later sync errors', () => { + // Once the host has answered, a subsequent transport error is not evidence it holds nothing new + // — but it is also not a reason to start refusing. The hydrated branch wins. + const store = createTestStore() + seedDirectSsh(store) + store.getState().markRemoteWorkspaceHydrated(TARGET_ID) + store.getState().setRemoteWorkspaceSyncStatus(TARGET_ID, { phase: 'error', direction: 'pull' }) + + expect(resolveWorkspaceTerminalHostAuthority(store.getState(), SSH_WORKTREE_ID)).toBe('none') + }) + + it('treats a conflicting host snapshot as unanswered', () => { + const store = createTestStore() + seedDirectSsh(store) + store.getState().markRemoteWorkspaceHydrated(TARGET_ID) + store.getState().setRemoteWorkspaceSyncStatus(TARGET_ID, { phase: 'conflict' }) + + expect(resolveWorkspaceTerminalHostAuthority(store.getState(), SSH_WORKTREE_ID)).toBe( + 'unverifiable' + ) + expect(ensureWorktreeHasInitialTerminal(store.getState(), SSH_WORKTREE_ID)).toBeNull() + expect(terminalTabCount(store, SSH_WORKTREE_ID)).toBe(0) + }) + + it('leaves terminal creation to the host of a paired runtime workspace', () => { + const store = createTestStore() + store.setState({ + repos: [repo('repoPaired', '/srv/proj')], + worktreesByRepo: { + repoPaired: [ + makeWorktree({ + id: PAIRED_WORKTREE_ID, + repoId: 'repoPaired', + path: '/srv/proj/paired', + hostId: `runtime:${ENVIRONMENT_ID}`, + runtimeOwnerEnvironmentId: ENVIRONMENT_ID + } as never) + ] + } + }) + + expect(resolveWorkspaceTerminalHostAuthority(store.getState(), PAIRED_WORKTREE_ID)).toBe('live') + expect(ensureWorktreeHasInitialTerminal(store.getState(), PAIRED_WORKTREE_ID)).toBeNull() + expect(terminalTabCount(store, PAIRED_WORKTREE_ID)).toBe(0) + }) + + /** + * #15556: the guard this replaces asked "am I a client of a live paired session?". Rival detected + * publications make the owning environment unnameable, so that guard answered "no" and the client + * seeded a local terminal beside the one the host was creating. Ownership is unresolved, not local. + */ + it('does not seed locally when the runtime owner cannot be named', () => { + const store = createTestStore() + store.setState({ + repos: [repo('repoPaired', '/srv/proj')], + worktreesByRepo: {}, + detectedWorktreesByRepo: { + repoPaired: { + authoritative: true, + worktrees: [ + makeWorktree({ + id: PAIRED_WORKTREE_ID, + repoId: 'repoPaired', + path: '/srv/proj/paired', + hostId: `runtime:${ENVIRONMENT_ID}`, + runtimeOwnerEnvironmentId: ENVIRONMENT_ID + } as never) + ] + }, + repoPairedRival: { + authoritative: true, + worktrees: [ + makeWorktree({ + id: PAIRED_WORKTREE_ID, + repoId: 'repoPairedRival', + path: '/srv/proj/paired', + hostId: 'local' + } as never) + ] + } + } as never + }) + + expect(resolveWorkspaceTerminalHostAuthority(store.getState(), PAIRED_WORKTREE_ID)).toBe( + 'unverifiable' + ) + expect(ensureWorktreeHasInitialTerminal(store.getState(), PAIRED_WORKTREE_ID)).toBeNull() + expect(terminalTabCount(store, PAIRED_WORKTREE_ID)).toBe(0) + }) +}) diff --git a/src/renderer/src/lib/workspace-terminal-host-authority.ts b/src/renderer/src/lib/workspace-terminal-host-authority.ts new file mode 100644 index 00000000000..d12ad08dbd6 --- /dev/null +++ b/src/renderer/src/lib/workspace-terminal-host-authority.ts @@ -0,0 +1,165 @@ +import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../shared/constants' +import { parseExecutionHostId } from '../../../shared/execution-host' +import { parseWorkspaceKey } from '../../../shared/workspace-scope' +import type { HostLiveTerminalProbeVerdict } from '@/runtime/host-live-terminal-probe' +import type { RemoteWorkspaceSyncStatus } from '@/store/slices/ssh' +import { isWebRuntimeSessionActive } from '@/runtime/web-runtime-session' +import { + getExecutionHostIdForWorktree, + getRuntimeEnvironmentIdForWorktree, + type WorktreeRuntimeOwnerState +} from '@/lib/worktree-runtime-owner' + +/** + * Who holds a workspace's terminals right now, in the same three-verdict vocabulary the renderer + * already uses for host terminal inventory ({@link HostLiveTerminalProbeVerdict}) — aliased rather + * than restated so the two cannot drift: + * + * - `live` — a remote execution host owns terminal creation here. It supplies the surface itself. + * - `unverifiable` — the workspace has a remote execution host with a sync still in flight or not + * yet attempted. It is bounded: a sync that terminates without an answer resolves `none` rather + * than refusing forever (see resolveDirectSshAuthority). + * - `none` — no remote host holds terminals here: either the workspace is local (this client *is* + * the execution host, and its own tab rows are the whole truth) or the host answered and holds + * nothing. + * + * `unverifiable` is a verdict, never a synonym for `none`. Two client behaviours read local rows as + * the verdict on what the host is running, and both are wrong before it answers: + * + * - seeding an initial terminal adds one tab per launch (STA-4658) — the snapshot then arrives, the + * merge rightly keeps the tab it was never told about, and the union uploads as the new host truth; + * - resuming a sleeping agent forks a second `claude --resume` onto a transcript the host is still + * writing (STA-3500, STA-3498, STA-3374), which no later evidence can undo. + * + * See docs/reference/ssh-execution-boundary.md. + */ +export type WorkspaceTerminalHostAuthority = HostLiveTerminalProbeVerdict + +export type WorkspaceTerminalHostAuthorityState = WorktreeRuntimeOwnerState & { + remoteWorkspaceHydratedTargetIds?: ReadonlySet + remoteWorkspaceSyncStatusByTargetId?: Record +} + +/** A sync attempt that has stopped without an answer. `unverifiable` is the honest verdict about the + * host, but holding it forever is not a verdict — it is a refusal to act, and `remoteWorkspaceHydratedTargetIds` + * is add-only in practice (`clearRemoteWorkspaceHydrated` has no production caller), so nothing + * would ever lift it. Four paths reach here: local-hydration timeout, a null `remoteWorkspace.get`, + * a falsy apply token, and never connecting at all. */ +const TERMINATED_WITHOUT_ANSWER_PHASES = new Set(['offline', 'error']) + +function resolveDirectSshAuthority( + state: WorkspaceTerminalHostAuthorityState, + targetId: string +): WorkspaceTerminalHostAuthority { + const phase = state.remoteWorkspaceSyncStatusByTargetId?.[targetId]?.phase + if (state.remoteWorkspaceHydratedTargetIds?.has(targetId)) { + // Why: the same pair use-app-session-persistence.ts gates uploads on. A conflicting snapshot + // means the client's picture is not the host's, so it is no basis for deciding the host holds + // nothing. + return phase === 'conflict' ? 'unverifiable' : 'none' + } + if (phase !== undefined && TERMINATED_WITHOUT_ANSWER_PHASES.has(phase)) { + // The bounded floor. Without it a single failed sync leaves every git worktree on this target + // terminal-less and its sleeping agents unresumable for the rest of the app session — strictly + // worse than the pre-gate behaviour, and only escapable by creating a tab by hand. Declining to + // seed is meant to be a wait, not a permanent refusal. + return 'none' + } + // Not connected, still pulling, or not yet attempted — "we could not ask", never "nothing there". + return 'unverifiable' +} + +/** + * The one host-authority question the seeding and sleeping-agent-resume paths ask, for both remote + * flavors: does some other party own this workspace's terminals right now? + * + * Deliberately an ownership question, not a client-liveness one — the guard it replaces asked "am I + * a client of a live paired session?", which a host desktop window answers "no" while a paired + * client answers "yes", so both seeded (#15556). + */ +export function resolveWorkspaceTerminalHostAuthority( + state: WorkspaceTerminalHostAuthorityState, + worktreeId: string | null | undefined +): WorkspaceTerminalHostAuthority { + if (!worktreeId || worktreeId === FLOATING_TERMINAL_WORKTREE_ID) { + return 'none' + } + if (isWebRuntimeSessionActive(getRuntimeEnvironmentIdForWorktree(state, worktreeId))) { + return 'live' + } + const host = parseExecutionHostId(getExecutionHostIdForWorktree(state, worktreeId)) + if (host?.kind === 'runtime') { + // A runtime host we could not name (rival detected publications, unhydrated catalog) is unasked. + return 'unverifiable' + } + if (host?.kind === 'ssh' && parseWorkspaceKey(worktreeId)?.type !== 'folder') { + // Why the git-worktree narrowing: the snapshot replaces exactly DirectSshTargetScope.gitWorktreeIds + // (remote-workspace-snapshot-apply.ts). A folder workspace's rows are never replaced by the host, + // so waiting on an answer that will never name them would leave it terminal-less for good. + return resolveDirectSshAuthority(state, host.targetId) + } + // Local, or outside the host's replace scope: this client is the execution host and its own rows are + // the whole truth. Absence of a catalog row is not evidence of a remote owner, and refusing to act + // on it would strand every workspace whose repo has not landed yet. + return 'none' +} + +/** + * Every slice the resolution above reads. Resolution walks the owner catalogs (and, for an active + * `ssh:` selection, `worktreesByRepo` uncached via resolveSelectedHostRoute), so a Zustand selector + * must not run it per store write — STA-3363 is the same shape. Same retained-selector memo as + * createConnectionIdForFileSelector. + */ +const AUTHORITY_INPUT_KEYS = [ + 'activeWorktreeId', + 'activeWorkspaceExecutionHostId', + 'detectedWorktreesByRepo', + 'folderWorkspaces', + 'projectGroups', + 'remoteWorkspaceHydratedTargetIds', + 'remoteWorkspaceSyncStatusByTargetId', + 'removedRuntimeEnvironmentIds', + 'repos', + 'restoredRuntimeHostIdByWorkspaceSessionKey', + 'runtimeEnvironmentCatalogHydrated', + 'runtimeEnvironments', + 'settings', + 'worktreesByRepo' +] as const satisfies readonly (keyof WorkspaceTerminalHostAuthorityState)[] + +/** Completeness, not just membership. The `satisfies` above only proves each listed key EXISTS on + * the state; a field added to WorkspaceTerminalHostAuthorityState and forgotten from the list would + * type-check while making the memo return a stale verdict — the failure mode is silent and looks + * like "the gate did not fire". This assignment fails to compile unless every key is listed. */ +type MissingAuthorityInputKey = Exclude< + keyof WorkspaceTerminalHostAuthorityState, + (typeof AUTHORITY_INPUT_KEYS)[number] +> +/** Errors with the missing key names when the union is not empty. Deliberately NOT + * `const x: MissingAuthorityInputKey[] = []` — an empty array literal is assignable to every array + * type, so that spelling passes no matter what is missing. */ +type AssertNoMissingAuthorityInputKey = T +export type AuthorityInputKeysAreComplete = + AssertNoMissingAuthorityInputKey + +type AuthorityInputs = readonly unknown[] + +function captureAuthorityInputs(state: WorkspaceTerminalHostAuthorityState): AuthorityInputs { + return AUTHORITY_INPUT_KEYS.map((key) => state[key]) +} + +export function createWorkspaceTerminalHostAuthoritySelector( + worktreeId: string | null | undefined +): (state: WorkspaceTerminalHostAuthorityState) => WorkspaceTerminalHostAuthority { + let previousInputs: AuthorityInputs | null = null + let previousResult: WorkspaceTerminalHostAuthority = 'none' + return (state) => { + const inputs = captureAuthorityInputs(state) + if (previousInputs?.every((value, index) => value === inputs[index]) === true) { + return previousResult + } + previousInputs = inputs + previousResult = resolveWorkspaceTerminalHostAuthority(state, worktreeId) + return previousResult + } +} diff --git a/src/renderer/src/lib/worktree-initial-terminal-seeding.ts b/src/renderer/src/lib/worktree-initial-terminal-seeding.ts index 2775c5c5f68..c3d58b190ba 100644 --- a/src/renderer/src/lib/worktree-initial-terminal-seeding.ts +++ b/src/renderer/src/lib/worktree-initial-terminal-seeding.ts @@ -7,9 +7,8 @@ import { createSequencedSetupAgentCommands } from '../../../shared/setup-agent-s import { getSetupRunnerCommandPlatformForPath } from '../../../shared/setup-runner-command' import { agentKindToTuiAgent } from '../../../shared/agent-kind' import { useAppStore } from '@/store' -import { isWebRuntimeSessionActive } from '@/runtime/web-runtime-session' import { queueHookCommandsForFirstWorktreeTab } from '@/lib/hook-command-delayed-delivery' -import { getRuntimeEnvironmentIdForWorktree } from '@/lib/worktree-runtime-owner' +import { resolveWorkspaceTerminalHostAuthority } from '@/lib/workspace-terminal-host-authority' import { initialAgentTabViewModeProps } from './native-chat-initial-view-mode' import { getConnectionId } from '@/lib/connection-context' import { isNativeChatTranscriptLocalReadable } from '@/lib/native-chat-transcript-readability' @@ -71,11 +70,9 @@ export function ensureWorktreeHasInitialTerminal( } const backendStartupTerminalSpawned = opts?.backendStartupTerminalSpawned === true - // Why: explicit spawn evidence survives the new-worktree ownership race; active web sessions provide the same authority for later activations. - if ( - backendStartupTerminalSpawned || - isWebRuntimeSessionActive(getRuntimeEnvironmentIdForWorktree(ownerState, worktreeId)) - ) { + const hostAuthority = resolveWorkspaceTerminalHostAuthority(ownerState, worktreeId) + // Why: explicit spawn evidence survives the new-worktree ownership race; a host that owns terminal creation provides the same authority for later activations. + if (backendStartupTerminalSpawned || hostAuthority === 'live') { const existingTerminalTabId = store.tabsByWorktree[worktreeId]?.[0]?.id if (existingTerminalTabId && (setup || issueCommand)) { queueSetupAndIssueCommands( @@ -124,10 +121,12 @@ export function ensureWorktreeHasInitialTerminal( // deactivation hooks for pane moves and retirement, where re-seeding is the wanted outcome. const shouldHonourClosedTerminalTombstone = Object.hasOwn(store.tabsByWorktree, worktreeId) && opts?.reseedEmptiedWorkspace !== true - const shouldAutoCreate = shouldAutoCreateInitialTerminal( - renderableTabCount, - shouldHonourClosedTerminalTombstone - ) + // Why: an execution host that has not answered is not a host with no terminals; seeding into that + // gap is what adds a tab per launch (STA-4658). Explicit launch work below is a request to create + // a terminal now, so it stays ungated. + const shouldAutoCreate = + hostAuthority === 'none' && + shouldAutoCreateInitialTerminal(renderableTabCount, shouldHonourClosedTerminalTombstone) const shouldCreateForExplicitWork = renderableTabCount === 0 && hasExplicitLaunchWork if (!shouldAutoCreate && !shouldCreateForExplicitWork) { const existingTerminalTabId = store.tabsByWorktree[worktreeId]?.[0]?.id diff --git a/tests/e2e/ssh-cold-hydration-gap-tab-seeding.spec.ts b/tests/e2e/ssh-cold-hydration-gap-tab-seeding.spec.ts new file mode 100644 index 00000000000..31ade2e1e2d --- /dev/null +++ b/tests/e2e/ssh-cold-hydration-gap-tab-seeding.spec.ts @@ -0,0 +1,295 @@ +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 { + return page.evaluate( + (id) => (window.__store?.getState().tabsByWorktree[id] ?? []).map((tab) => tab.id), + worktreeId + ) +} + +async function isTargetHydrated(page: Page, targetId: string): Promise { + return page.evaluate( + (id) => window.__store?.getState().remoteWorkspaceHydratedTargetIds.has(id) === true, + 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 { + 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 { + 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 { + 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('still seeds one tab for a client with no local row, because hydration is marked even when adoption wrote nothing', 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) + await expect + .poll(() => isTargetHydrated(freshLaunch.page, rejoined.targetId), { + timeout: 120_000, + message: 'the fresh client never hydrated the host workspace' + }) + .toBe(true) + const rejoinedTabIds = await waitForSettledTabIds(freshLaunch.page, rejoined.worktreeId) + + // Documents what HEAD actually does, so the spec is honest rather than red. Two facts, and + // the helper's own tab is switched off above so both are the product's doing: + // 1. the seeding predicate DOES fire with no local row — one tab is created from nothing; + // 2. none of the host's tabs are adopted, so that one tab replaces three. + // Why the host-authority gate does not prevent (1): 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. Fixing that needs per-worktree resolution rather than a per-target + // "some apply finished" flag; until then the seeding half has no measurable effect here. + // (1) is the seeding path reached by its intended precondition; a restart never reaches it + // because local state always restores the row first. (2) is a separate defect. Expected + // behaviour is pinned by the fixme below. + expect( + rejoinedTabIds.length, + `a client with no local row should seed exactly one tab and adopt none of the host's ${BASELINE_TAB_COUNT}: got ${rejoinedTabIds.length}` + ).toBe(1) + } finally { + if (freshApp) { + await fresh.close(freshApp) + } + if (seedingApp) { + await seeding.close(seedingApp) + } + await fresh.dispose() + await seeding.dispose() + cleanupDockerSshRelayTarget(target) + } + }) + + // The behaviour the test above SHOULD assert once the drop is fixed: a client with no local row + // is exactly the case the host snapshot exists to serve, so it must end up holding the host's + // tabs rather than an empty workspace. Kept as a fixme so the gap stays visible without putting + // a knowingly-red spec in the lane. Suspected 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. + {}) => {}) +})