diff --git a/config/scripts/run-ssh-docker-e2e.mjs b/config/scripts/run-ssh-docker-e2e.mjs index fc0d628ab78b..2eea3575fde1 100644 --- a/config/scripts/run-ssh-docker-e2e.mjs +++ b/config/scripts/run-ssh-docker-e2e.mjs @@ -78,6 +78,7 @@ const result = spawnSync( 'tests/e2e/ssh-reconnect-tab-destruction.spec.ts', 'tests/e2e/ssh-restart-tab-accumulation.spec.ts', 'tests/e2e/ssh-skill-installation.spec.ts', + 'tests/e2e/ssh-stale-resume-execution-host-scope.spec.ts', 'tests/e2e/ssh-terminal-window-wake-stale-grid-repro.spec.ts', '--config', 'tests/playwright.config.ts', diff --git a/src/renderer/src/components/terminal-pane/pty-connection/cold-restore-resume-startup.ts b/src/renderer/src/components/terminal-pane/pty-connection/cold-restore-resume-startup.ts index c719e4a83ef9..d15231fca7d0 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/cold-restore-resume-startup.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/cold-restore-resume-startup.ts @@ -2,6 +2,10 @@ import { useAppStore } from '@/store' import { createBrowserUuid } from '@/lib/browser-uuid' import { buildAgentResumeStartupPlan } from '@/lib/tui-agent-startup' import { resolveAgentResumeLaunchTarget } from '@/lib/agent-resume-launch-target' +import { + agentResumeOriginNamesAnotherExecutionHost, + sleepingRecordNamesAnotherExecutionHost +} from '@/lib/sleeping-record-execution-host-scope' import { resolveTuiAgentLaunchArgs, resolveTuiAgentLaunchEnv @@ -37,6 +41,25 @@ export function bindBuildColdRestoreAgentResumeStartup(session: ConnectPanePtySe if (!providerSession) { return null } + // Why: this is the second issuer of `--resume`, and the one that handles a quit/live record + // whose pane still exists — the sweep hands those here rather than launching them. A session id + // names a transcript on the machine that captured it, so replaying one over a pane now attached + // to a different host answers `No conversation found`. Returning null leaves the pane with a + // plain shell and the record intact, which the user can resume by hand. + // + // Two sources are consulted because either can be the one that knows. `session.executionHostId` + // is the pane's own transport and is authoritative when set, but it is still unresolved on an + // early reattach frame — and failing open on that frame is precisely when a wrong resume slips + // out. The catalog's answer for the record's worktree covers that window. + if ( + agentResumeOriginNamesAnotherExecutionHost( + useLiveEntry ? entry.connectionId : sleepingRecord?.connectionId, + session.executionHostId + ) || + (sleepingRecord && sleepingRecordNamesAnotherExecutionHost(sleepingRecord, state)) + ) { + return null + } const matchingSleepingLaunchConfig = sleepingRecord?.launchConfig && (!useLiveEntry || diff --git a/src/renderer/src/lib/resume-sleeping-agent-session-execution-host-scope.test.ts b/src/renderer/src/lib/resume-sleeping-agent-session-execution-host-scope.test.ts new file mode 100644 index 000000000000..54b5459a8cf5 --- /dev/null +++ b/src/renderer/src/lib/resume-sleeping-agent-session-execution-host-scope.test.ts @@ -0,0 +1,239 @@ +/** + * A provider session id names a transcript in ONE machine's agent state directory. Replaying a + * record captured on host A as a `--resume` executed on host B answers + * `No conversation found with session ID: ` at best, and at worst reopens an unrelated + * transcript that happens to share the id. + * + * Nothing in the resume path was host-scoped: `worktreeId` is `repoId::path` with no host component + * (shared/worktree/host-qualified-identity.ts), sleeping records are `'sleepingAgentKeyed'` so the + * boot-time host-contention parking never arbitrates them and every partition's records merge into + * one map, and `launchSleepingAgentSession` resolves its launch target from the *current* catalog. + * + * Both directions matter. The sweep must decline when the record names another machine, and it must + * still resume everything it cannot positively rule out — a gate that refuses on absent evidence + * would strand every record captured before the stamp existed. + */ +import { afterEach, describe, expect, it } from 'vitest' +import type { SleepingAgentSessionRecord } from '../../../shared/agent-session-resume' +import type { ExecutionHostId } from '../../../shared/execution-host' +import { useAppStore } from '@/store' +import { makeWorktree, TEST_REPO } from '@/store/slices/store-test-helpers' +import { resumeSleepingAgentSessionsForWorktree } from './resume-sleeping-agent-session' +import { + agentResumeOriginNamesAnotherExecutionHost, + sleepingRecordNamesAnotherExecutionHost +} from './sleeping-record-execution-host-scope' +import type { WorktreeRuntimeOwnerState } from './worktree-runtime-owner' + +const initialAppStoreState = useAppStore.getState() + +const TARGET_ID = 'openclaw' +const REMOTE_PATH = '/home/neil/projects/orca-test123' +const WORKTREE_ID = `repo-1::${REMOTE_PATH}` +const SESSION_ID = '87987465-66f6-4967-bf3f-0659565cbcc5' + +afterEach(() => { + useAppStore.setState(initialAppStoreState, true) +}) + +function makeRecord( + overrides: Partial = {} +): SleepingAgentSessionRecord { + return { + paneKey: 'tab-1:leaf-1', + tabId: 'tab-1', + worktreeId: WORKTREE_ID, + agent: 'claude', + providerSession: { key: 'session_id', id: SESSION_ID }, + prompt: 'finish the task', + state: 'working', + origin: 'quit', + capturedAt: 1, + updatedAt: 1, + ...overrides + } +} + +/** A catalog that resolves WORKTREE_ID to exactly `hostId`, with no tab rows for it. */ +function catalogOwnedBy(hostId: ExecutionHostId): WorktreeRuntimeOwnerState { + const connectionId = hostId.startsWith('ssh:') + ? decodeURIComponent(hostId.slice('ssh:'.length)) + : undefined + return { + repos: [ + { + id: 'repo-1', + ...(connectionId ? { connectionId } : {}), + ...(hostId.startsWith('runtime:') ? { executionHostId: hostId } : {}) + } + ], + worktreesByRepo: { + 'repo-1': [makeWorktree({ id: WORKTREE_ID, repoId: 'repo-1', path: REMOTE_PATH, hostId })] + } + } +} + +describe('sleepingRecordNamesAnotherExecutionHost', () => { + it.each([ + ['an SSH record on a different SSH target', 'other-target', `ssh:${TARGET_ID}`], + ['an SSH record on the local host', TARGET_ID, 'local'], + ['a local-or-runtime record on an SSH host', null, `ssh:${TARGET_ID}`] + ] as const)('refuses %s', (_label, connectionId, hostId) => { + const record = makeRecord({ connectionId }) + expect(sleepingRecordNamesAnotherExecutionHost(record, catalogOwnedBy(hostId))).toBe(true) + }) + + it.each([ + ['the same SSH target', TARGET_ID, `ssh:${TARGET_ID}`], + ['a target id needing URI encoding', 'my host', 'ssh:my%20host'], + ['a local record on the local host', null, 'local'], + // A paired client renames its host's workspaces — including that host's SSH ones — into its own + // runtime namespace, so a runtime answer is no evidence about the machine holding the transcript. + ['an SSH record whose workspace now reads as a paired runtime', TARGET_ID, 'runtime:env-1'], + ['a local-or-runtime record on a paired runtime', null, 'runtime:env-1'] + ] as const)('allows %s', (_label, connectionId, hostId) => { + const record = makeRecord({ connectionId }) + expect(sleepingRecordNamesAnotherExecutionHost(record, catalogOwnedBy(hostId))).toBe(false) + }) + + it.each([ + ['never stamped', undefined], + ['stamped with whitespace', ' '] + ] as const)('fails open on a record %s', (_label, connectionId) => { + // #9030 leaves SSH orphans unstamped. Refusing on absent evidence would strand every record + // captured before the stamp existed, which is a worse failure than the one being fixed. + const record = makeRecord(connectionId === undefined ? {} : { connectionId }) + expect( + sleepingRecordNamesAnotherExecutionHost(record, catalogOwnedBy(`ssh:${TARGET_ID}`)) + ).toBe(false) + }) + + it.each([ + ['an SSH record', TARGET_ID], + ['a local-or-runtime record', null] + ] as const)( + 'fails open for %s when the catalog has no row for the worktree', + (_label, connectionId) => { + // The routing resolver answers `'local'` for a worktree it has no row for. Read as a host, that + // would make every SSH record look foreign until its repo row lands — a gate that never resumes + // yours is the inverse of the defect and worse. + const record = makeRecord({ connectionId }) + expect( + sleepingRecordNamesAnotherExecutionHost(record, { repos: [], worktreesByRepo: {} }) + ).toBe(false) + } + ) + + it('still refuses an SSH record once a repo row positively names the worktree local', () => { + const record = makeRecord({ connectionId: TARGET_ID }) + expect( + sleepingRecordNamesAnotherExecutionHost(record, { + repos: [{ id: 'repo-1' }], + worktreesByRepo: {} + }) + ).toBe(true) + }) +}) + +describe('agentResumeOriginNamesAnotherExecutionHost', () => { + // The pane cold-restore path asks the same question against the transport the pane is attached to + // rather than the catalog, so the host-pair form is exported and pinned separately. + it.each([ + ['an SSH origin against another SSH pane', TARGET_ID, 'ssh:elsewhere', true], + ['an SSH origin against a local pane', TARGET_ID, 'local', true], + ['a local origin against an SSH pane', null, `ssh:${TARGET_ID}`, true], + ['an SSH origin against its own pane', TARGET_ID, `ssh:${TARGET_ID}`, false], + ['a local origin against a local pane', null, 'local', false], + ['an SSH origin against a paired-runtime pane', TARGET_ID, 'runtime:env-1', false] + ] as const)('reports %s as %s', (_label, originConnectionId, hostId, expected) => { + expect(agentResumeOriginNamesAnotherExecutionHost(originConnectionId, hostId)).toBe(expected) + }) + + it.each([null, undefined])( + 'fails open when the pane has no resolved execution host (%s)', + (hostId) => { + // A pane whose owner is still unresolved is not evidence of a different machine. + expect(agentResumeOriginNamesAnotherExecutionHost(TARGET_ID, hostId)).toBe(false) + } + ) +}) + +/** The SSH workspace after its host has answered, so terminal-host authority is decided and the + * sweep is allowed to act. Without the hydration mark the sweep declines for an unrelated reason + * and every assertion below would pass vacuously. */ +function seedAnsweredSshWorkspace(...records: SleepingAgentSessionRecord[]): void { + useAppStore.setState({ + repos: [{ ...TEST_REPO, id: 'repo-1', path: '/home/neil/projects', connectionId: TARGET_ID }], + worktreesByRepo: { + 'repo-1': [ + makeWorktree({ + id: WORKTREE_ID, + repoId: 'repo-1', + path: REMOTE_PATH, + hostId: `ssh:${TARGET_ID}` + }) + ] + }, + tabsByWorktree: {}, + sleepingAgentSessionsByPaneKey: Object.fromEntries( + records.map((record) => [record.paneKey, record]) + ) + }) + useAppStore.getState().markRemoteWorkspaceHydrated(TARGET_ID) +} + +describe('the resume sweep under execution-host scope', () => { + it('declines a locally captured session id rather than issuing it on the SSH host', () => { + const record = makeRecord({ connectionId: null }) + seedAnsweredSshWorkspace(record) + + expect( + resumeSleepingAgentSessionsForWorktree(WORKTREE_ID), + 'issued --resume for a local session id against the SSH host' + ).toBe(0) + expect(useAppStore.getState().tabsByWorktree[WORKTREE_ID] ?? []).toHaveLength(0) + }) + + it('preserves the declined record so the session stays resumable by hand', () => { + const record = makeRecord({ connectionId: 'a-different-target' }) + seedAnsweredSshWorkspace(record) + + resumeSleepingAgentSessionsForWorktree(WORKTREE_ID) + resumeSleepingAgentSessionsForWorktree(WORKTREE_ID) + + // Declining is recoverable only if the record survives; deleting it on a host disagreement + // would destroy the user's only handle on that transcript. + expect(useAppStore.getState().sleepingAgentSessionsByPaneKey[record.paneKey]).toBe(record) + }) + + it('still resumes a session captured on the host that owns the workspace', () => { + const record = makeRecord({ connectionId: TARGET_ID }) + seedAnsweredSshWorkspace(record) + + expect(resumeSleepingAgentSessionsForWorktree(WORKTREE_ID)).toBe(1) + expect(useAppStore.getState().sleepingAgentSessionsByPaneKey[record.paneKey]).toBeUndefined() + }) + + it('still resumes a legacy record that names no host at all', () => { + const record = makeRecord() + seedAnsweredSshWorkspace(record) + + expect(resumeSleepingAgentSessionsForWorktree(WORKTREE_ID)).toBe(1) + }) + + it('declines only the foreign record and resumes its native sibling', () => { + const foreign = makeRecord({ + paneKey: 'tab-1:leaf-1', + tabId: 'tab-1', + connectionId: null, + providerSession: { key: 'session_id', id: 'session-from-the-laptop' } + }) + const native = makeRecord({ paneKey: 'tab-2:leaf-1', tabId: 'tab-2', connectionId: TARGET_ID }) + seedAnsweredSshWorkspace(foreign, native) + + expect(resumeSleepingAgentSessionsForWorktree(WORKTREE_ID)).toBe(1) + const state = useAppStore.getState() + expect(state.sleepingAgentSessionsByPaneKey[foreign.paneKey]).toBe(foreign) + expect(state.sleepingAgentSessionsByPaneKey[native.paneKey]).toBeUndefined() + }) +}) diff --git a/src/renderer/src/lib/resume-sleeping-agent-session.ts b/src/renderer/src/lib/resume-sleeping-agent-session.ts index 0610eb9cf8bc..50fb378888f9 100644 --- a/src/renderer/src/lib/resume-sleeping-agent-session.ts +++ b/src/renderer/src/lib/resume-sleeping-agent-session.ts @@ -21,6 +21,7 @@ import { type UnhydratedHostMirror } from './host-mirrored-pane-liveness' import { parkUntilHostMirrorHandleLands } from './host-mirror-handle-gap-wait' +import { sleepingRecordNamesAnotherExecutionHost } from './sleeping-record-execution-host-scope' import { resolveWorkspaceTerminalHostAuthority } from './workspace-terminal-host-authority' import { parkUntilHostSessionMirrorHydrates } from '@/runtime/host-session-mirror-hydration' @@ -262,6 +263,14 @@ export function resumeSleepingAgentSessionsForWorktree( state.clearSleepingAgentSession(record.paneKey) continue } + // Why this is a `continue` and not a clear: the id is a valid locator on the machine that + // captured it, so the record is evidence, not garbage — deleting it on the strength of a host + // disagreement would destroy the user's only handle on that transcript. Declining costs an + // automatic wake the user can re-issue by hand; issuing `--resume` on the wrong machine is + // `No conversation found` at best and a forked transcript at worst. + if (sleepingRecordNamesAnotherExecutionHost(record, currentState)) { + continue + } const unhydratedMirror = findUnhydratedHostMirrorForPane(record, currentState) if (unhydratedMirror) { // Why: pane ownership is undecidable until the mirror answers, and every diff --git a/src/renderer/src/lib/sleeping-record-execution-host-scope.ts b/src/renderer/src/lib/sleeping-record-execution-host-scope.ts new file mode 100644 index 000000000000..8581a690f65f --- /dev/null +++ b/src/renderer/src/lib/sleeping-record-execution-host-scope.ts @@ -0,0 +1,78 @@ +import type { SleepingAgentSessionRecord } from '../../../shared/agent-session-resume' +import { + parseExecutionHostId, + toSshExecutionHostId, + type ExecutionHostId +} from '../../../shared/execution-host' +import { + getKnownExecutionHostIdForWorktree, + type WorktreeRuntimeOwnerState +} from './worktree-runtime-owner' + +/** + * Does this record's `--resume` locator belong to a different machine than the one the resume would + * run on? + * + * A provider session id names a transcript in one machine's agent state directory, but nothing + * else in the resume path is host-scoped: `worktreeId` is `repoId::path` with no host component + * (shared/worktree/host-qualified-identity.ts), sleeping records are `'sleepingAgentKeyed'` so the + * boot-time host-contention parking never arbitrates them and every partition's records merge into + * one map, and `launchSleepingAgentSession` resolves its launch target from the *current* catalog. + * A record captured on host A therefore reaches a launch on host B, which answers + * `No conversation found with session ID`. + * + * Deliberately fails open. It reports only a positively-known disagreement about the machine, + * because the alternative — refusing whenever the hosts cannot be compared — would strand every + * legitimate resume whose capture predates the stamp. + * + * "Fail open" names a direction for THIS decision, never a house style, and the safe direction is + * inverted a few files away. Here the destructive act is *attempting* a resume — a wrong one can + * fork a transcript, which is unrecoverable, while a refusal keeps the record and the user can + * resume by hand. So an unhydrated catalog must not be read as a host verdict. In + * `workspace-session-terminal-buffers.ts` the destructive act is the opposite: declining to capture + * loses the only scrollback copy, so an unknown repo is treated as remote. Same window, opposite + * default, both correct. A reader pattern-matching one onto the other will get this backwards. + * + * The four unknowns this fails open on: + * + * - `undefined` is "never stamped", not "local" (#9030 leaves SSH orphans unstamped). + * - `null` is "local **or** paired runtime": a `remote:@@` PTY is stamped null too + * (agent-status-connection-ownership.ts), so null cannot rule a runtime host out — only an + * `ssh:` one, which is unambiguously another machine. + * - A current host of `runtime:*` is no evidence either way, because a paired client relabels its + * host's workspaces — including that host's own SSH ones — into its runtime namespace. + * - A current host of `null` is a catalog with no row for the worktree. The routing resolver + * answers `'local'` there, which is the right default for issuing an operation and would read + * here as a positive host — so the worktree form below asks the resolver that keeps the silence. + */ +export function agentResumeOriginNamesAnotherExecutionHost( + originConnectionId: string | null | undefined, + currentExecutionHostId: ExecutionHostId | null | undefined +): boolean { + if (originConnectionId === undefined) { + return false + } + const originTargetId = originConnectionId === null ? null : originConnectionId.trim() + if (originTargetId === '') { + return false + } + const currentHost = parseExecutionHostId(currentExecutionHostId) + if (!currentHost || currentHost.kind === 'runtime') { + return false + } + if (currentHost.kind === 'ssh') { + return originTargetId === null || toSshExecutionHostId(originTargetId) !== currentHost.id + } + return originTargetId !== null +} + +/** The worktree-scoped form the activation sweep asks, resolving the host from the catalog. */ +export function sleepingRecordNamesAnotherExecutionHost( + record: SleepingAgentSessionRecord, + state: WorktreeRuntimeOwnerState +): boolean { + return agentResumeOriginNamesAnotherExecutionHost( + record.connectionId, + getKnownExecutionHostIdForWorktree(state, record.worktreeId) + ) +} diff --git a/src/renderer/src/lib/worktree-runtime-owner.test.ts b/src/renderer/src/lib/worktree-runtime-owner.test.ts index 995d7c15715c..02ea0682e6b4 100644 --- a/src/renderer/src/lib/worktree-runtime-owner.test.ts +++ b/src/renderer/src/lib/worktree-runtime-owner.test.ts @@ -3,6 +3,7 @@ import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../shared/constants' import { getExplicitRuntimeEnvironmentIdForWorktree, getExecutionHostIdForWorktree, + getKnownExecutionHostIdForWorktree, getRuntimeEnvironmentIdForWorktree, getRuntimeSessionMirrorEnvironmentIds, getSettingsForWorktreeRuntimeOwner, @@ -610,3 +611,39 @@ describe('active workspace host selection', () => { ) }) }) + +describe('getKnownExecutionHostIdForWorktree', () => { + const emptyCatalog: WorktreeRuntimeOwnerState = { repos: [], worktreesByRepo: {} } + + it('reports silence, not local, for a git worktree with no repo row', () => { + expect(getKnownExecutionHostIdForWorktree(emptyCatalog, 'missing-repo::wt')).toBeNull() + // The routing form keeps substituting the default in the same state. + expect(getExecutionHostIdForWorktree(emptyCatalog, 'missing-repo::wt')).toBe('local') + }) + + it('reports silence for a folder workspace with no folder-workspace row', () => { + expect(getKnownExecutionHostIdForWorktree(emptyCatalog, 'folder:missing')).toBeNull() + expect(getExecutionHostIdForWorktree(emptyCatalog, 'folder:missing')).toBe('local') + }) + + it.each([ + ['an ownerless repo row', { repos: [{ id: 'r' }] }, 'r::wt', 'local'], + ['an SSH repo row', { repos: [{ id: 'r', connectionId: 'box' }] }, 'r::wt', 'ssh:box'], + [ + 'a per-worktree host', + { worktreesByRepo: { r: [{ id: 'r::wt', repoId: 'r', hostId: 'ssh:box' }] } }, + 'r::wt', + 'ssh:box' + ], + [ + 'a folder-workspace row', + { folderWorkspaces: [{ id: 'f', projectGroupId: 'g' }] }, + 'folder:f', + 'local' + ], + ['the floating workspace', {}, FLOATING_TERMINAL_WORKTREE_ID, 'local'] + ] as const)('answers positively for %s', (_label, catalog, worktreeId, expected) => { + expect(getKnownExecutionHostIdForWorktree(catalog, worktreeId)).toBe(expected) + expect(getExecutionHostIdForWorktree(catalog, worktreeId)).toBe(expected) + }) +}) diff --git a/src/renderer/src/lib/worktree-runtime-owner.ts b/src/renderer/src/lib/worktree-runtime-owner.ts index 473b306eeaff..091cf246a9f3 100644 --- a/src/renderer/src/lib/worktree-runtime-owner.ts +++ b/src/renderer/src/lib/worktree-runtime-owner.ts @@ -14,6 +14,7 @@ import { } from './worktree-runtime-owner-index' import { getSingleFocusedRuntimeEnvironmentId } from './single-runtime-legacy-owner' import { + findFolderWorkspaceOwner, getExecutionHostIdForFolderWorkspace, getExplicitRuntimeEnvironmentIdForFolderWorkspace, getRuntimeEnvironmentIdForFolderWorkspace @@ -157,10 +158,26 @@ export function getExplicitRuntimeEnvironmentIdForWorktree( return getExplicitRuntimeEnvironmentIdFromHost(getRepoExecutionHostId(repo)) } -export function getExecutionHostIdForWorktree( +function getFocusedRuntimeOrLocalExecutionHostId( + state: WorktreeRuntimeOwnerState +): ExecutionHostId { + const environmentId = getSingleFocusedRuntimeEnvironmentId(state) + return environmentId ? `runtime:${encodeURIComponent(environmentId)}` : 'local' +} + +/** + * The catalog's answer, or `null` when it has none: no row names an owner for this worktree (a git + * worktree without a repo row, a folder workspace without a folder-workspace row) and nothing more + * specific — active-workspace host, detected owner, per-worktree host — applies either. A row that + * exists and names no owner is a positive `'local'`; a row that has not landed is silence. + * {@link getExecutionHostIdForWorktree} papers over that silence with the focused-runtime-or-local + * default, which is the right answer for routing an operation and the wrong one for a caller that + * reads the host as evidence. + */ +export function getKnownExecutionHostIdForWorktree( state: WorktreeRuntimeOwnerState, worktreeId: string | null | undefined -): ExecutionHostId { +): ExecutionHostId | null { if (!worktreeId) { return 'local' } @@ -173,7 +190,11 @@ export function getExecutionHostIdForWorktree( } const workspaceScope = parseWorkspaceKey(worktreeId) if (workspaceScope?.type === 'folder') { - return getExecutionHostIdForFolderWorkspace(state, workspaceScope.folderWorkspaceId) + const hostId = getExecutionHostIdForFolderWorkspace(state, workspaceScope.folderWorkspaceId) + // Why: the folder resolver substitutes `'local'` for a missing row the same way this one does. + return hostId === 'local' && !findFolderWorkspaceOwner(state, workspaceScope.folderWorkspaceId) + ? null + : hostId } const hasDetectedOwner = hasIndexedDetectedWorktree(state.detectedWorktreesByRepo, worktreeId) if (hasDetectedOwner) { @@ -196,12 +217,24 @@ export function getExecutionHostIdForWorktree( } const repoId = worktree?.repoId ?? getRepoIdFromWorktreeId(worktreeId) const repo = findRepoRecord(state.repos, repoId) - const hasExplicitOwner = Boolean(repo?.executionHostId?.trim() || repo?.connectionId?.trim()) - if (repo && hasExplicitOwner) { + if (!repo) { + return null + } + const hasExplicitOwner = Boolean(repo.executionHostId?.trim() || repo.connectionId?.trim()) + if (hasExplicitOwner) { return getRepoExecutionHostId(repo) } - const environmentId = getSingleFocusedRuntimeEnvironmentId(state) - return environmentId ? `runtime:${encodeURIComponent(environmentId)}` : 'local' + return getFocusedRuntimeOrLocalExecutionHostId(state) +} + +export function getExecutionHostIdForWorktree( + state: WorktreeRuntimeOwnerState, + worktreeId: string | null | undefined +): ExecutionHostId { + return ( + getKnownExecutionHostIdForWorktree(state, worktreeId) ?? + getFocusedRuntimeOrLocalExecutionHostId(state) + ) } export function getSettingsForWorktreeRuntimeOwner( diff --git a/tests/e2e/ssh-stale-resume-execution-host-scope.spec.ts b/tests/e2e/ssh-stale-resume-execution-host-scope.spec.ts new file mode 100644 index 000000000000..03e72adfd98a --- /dev/null +++ b/tests/e2e/ssh-stale-resume-execution-host-scope.spec.ts @@ -0,0 +1,306 @@ +/** + * A provider session id names a transcript in ONE machine's agent state directory. Orca issued one + * against the wrong machine and the agent answered + * `No conversation found with session ID: ` in the user's remote terminal. + * + * Nothing in the resume path was host-scoped. `worktreeId` is `repoId::path` with no host component, + * sleeping records merge across every host partition at boot without retaining which one they came + * from, and the launch path resolves its target from the *current* catalog — so a record captured on + * host A reaches a `--resume` executed on host B. + * + * This lane proves it at the only altitude that settles the question: the argv that actually lands + * on the remote machine. Both tests restart the app across a relay kill (the shape of an Orca + * update, which is what the user did) and read the stub agent's argv ledger out of the container. + * + * - foreign stamp → the ledger must hold no `--resume`, and the record must survive so the user + * can still resume by hand. It must still hold Orca's ordinary `--version` + * probe, or the lane would pass on an app that never reached the host at all. + * - matching stamp → the ledger must contain `--resume `. + * + * The second is not a nicety. Without it the first passes on any app that resumes nothing at all, + * which is exactly the failure mode a refuse-everything gate would ship. + */ +import type { ElectronApplication, TestInfo } from '@stablyai/playwright-test' +import { test, expect } from './helpers/orca-app' +import { createRestartSession } from './helpers/orca-restart' +import { connectDockerSshRelayTarget } from './helpers/docker-ssh-relay-connection' +import { killDockerSshRelayDaemon } from './helpers/docker-ssh-relay-faults' +import { + cleanupDockerSshRelayTarget, + execDockerSshRelayTargetCommand, + startDockerSshRelayTarget, + writeDockerSshRelayTargetFile, + type DockerSshRelayTarget +} from './helpers/docker-ssh-relay-target' +import { waitForActiveWorktree, waitForSessionReady } from './helpers/store' +import { + waitForActivePaneHookDescriptor, + waitForActivePanePtyId, + waitForActiveTerminalManager +} from './helpers/terminal' + +const RUN_DOCKER_SSH = process.env.ORCA_E2E_SSH_DOCKER === '1' + +const SESSION_ID = 'e2e-stale-resume-87987465' +const ARGV_LEDGER = '/tmp/orca-e2e-claude-argv.log' +/** Stands in for a record the user carried over from another machine: its transcript is not on this + * host under this id. Any value that is not the connected target's works. */ +const FOREIGN_CONNECTION_ID = 'orca-e2e-some-other-host' +/** Resolve the stamp to the connected target's own id, which is only minted during connect. */ +const STAMP_OWNING_HOST = Symbol('stamp-owning-host') +/** How long a `--resume` gets to reach the host once the relaunched pane holds its PTY. The resume + * is typed into that very shell, so anything the gate let through lands well inside this. */ +const RESUME_GRACE_MS = 20_000 + +test.use({ seedTestRepo: false }) + +/** A `claude` that records the argv it was invoked with and then holds the PTY open the way the real + * binary does. The ledger outlives the pane, and is appended to rather than truncated so a second + * invocation is visible as a second line. */ +function installRemoteClaudeArgvLedger(target: DockerSshRelayTarget): void { + writeDockerSshRelayTargetFile( + target, + '/usr/local/bin/claude', + [ + '#!/bin/sh', + `printf 'ARGV [%s] pid=%s ppid=%s %s\\n' "$(date +%s)" "$$" "$PPID" "$*" >> ${ARGV_LEDGER}`, + 'exec cat', + '' + ].join('\n') + ) + execDockerSshRelayTargetCommand(target, 'chmod 755 /usr/local/bin/claude') +} + +function readRemoteArgvLedger(target: DockerSshRelayTarget): string { + return execDockerSshRelayTargetCommand(target, `cat ${ARGV_LEDGER} 2>/dev/null || true`).trim() +} + +/** The lines appended since `baseline`. The ledger is append-only and the first launch already wrote + * its own `--version` probe to it, so "non-empty" says nothing about the relaunch — only the tail + * beyond what was there at quit does. Reading the whole ledger here is exactly the race that let the + * control case read two `--version` lines and give up before the resume was typed. */ +function ledgerLinesSince(ledger: string, baseline: string): string { + return ledger.startsWith(baseline) ? ledger.slice(baseline.length).trim() : ledger +} + +/** Poll the relaunch's ledger lines until `until` holds or the budget runs out. Returns them either + * way: the negative case asserts on what did NOT arrive, so this must not throw. */ +async function settleRemoteArgvLedger( + target: DockerSshRelayTarget, + baseline: string, + budgetMs: number, + until: (fresh: string) => boolean +): Promise { + const deadline = Date.now() + budgetMs + for (;;) { + const fresh = ledgerLinesSince(readRemoteArgvLedger(target), baseline) + if (until(fresh) || Date.now() >= deadline) { + return fresh + } + await new Promise((resolve) => setTimeout(resolve, 2_000)) + } +} + +/** + * One full incident replay: capture a sleeping agent record on the SSH worktree stamped with + * `stamp`, quit, kill the relay so no PTY can be reclaimed (without that the pane's live PTY + * suppresses the resume and the test proves nothing), relaunch, and report what reached the remote. + */ +async function resumeAcrossRestart( + testInfo: TestInfo, + target: DockerSshRelayTarget, + stamp: string | typeof STAMP_OWNING_HOST, + resumeBudgetMs: number +): Promise<{ + ledger: string + recordSurvived: boolean + diagnostics: { + recordStamp: string + entryStamp: string + ledgerBeforeQuit: string + ledgerAfterQuit: string + } +}> { + const restart = createRestartSession(testInfo) + let firstApp: ElectronApplication | null = null + let secondApp: ElectronApplication | null = null + try { + const firstLaunch = await restart.launch() + firstApp = firstLaunch.app + await waitForSessionReady(firstLaunch.page) + const remote = await connectDockerSshRelayTarget(firstLaunch.page, target) + await expect + .poll(() => waitForActiveWorktree(firstLaunch.page), { timeout: 60_000 }) + .toBe(remote.worktreeId) + await waitForActiveTerminalManager(firstLaunch.page, 60_000) + const descriptor = await waitForActivePaneHookDescriptor(firstLaunch.page, 60_000) + + // Why seeded rather than driven by a real agent: a real `claude` run needs an install and auth + // in the container. This is the same store entry the hook server writes, so the capture, + // persistence and resume paths under test are the production ones. + await firstLaunch.page.evaluate( + ({ paneKey, worktreeId, providerSessionId, connectionId }) => { + window.__store?.getState().setAgentStatus( + paneKey, + { state: 'working', prompt: 'finish the task', agentType: 'claude' }, + 'Claude', + undefined, + { worktreeId, connectionId }, + { + providerSession: { key: 'session_id', id: providerSessionId }, + launchConfig: { agentCommand: 'claude', agentArgs: '', agentEnv: {} } + } + ) + }, + { + paneKey: descriptor.paneKey, + worktreeId: remote.worktreeId, + providerSessionId: SESSION_ID, + connectionId: stamp === STAMP_OWNING_HOST ? remote.targetId : stamp + } + ) + + await firstLaunch.page.evaluate(() => window.dispatchEvent(new Event('beforeunload'))) + await expect + .poll( + () => + firstLaunch.page.evaluate( + async ({ targetId, sessionId }) => { + // The SSH worktree's rows live in the `ssh:` partition, globals in `local`. + const [local, host] = await Promise.all([ + window.api.session.get(), + window.api.session.get(`ssh:${targetId}`) + ]) + return [ + ...Object.values(local.sleepingAgentSessionsByPaneKey ?? {}), + ...Object.values(host.sleepingAgentSessionsByPaneKey ?? {}) + ].some((record) => record.providerSession.id === sessionId) + }, + { targetId: remote.targetId, sessionId: SESSION_ID } + ), + { timeout: 30_000, message: 'the sleeping agent record was never persisted before quit' } + ) + .toBe(true) + + const ledgerBeforeQuit = readRemoteArgvLedger(target) + + await restart.close(firstApp) + firstApp = null + // The shape of an Orca update: the relay and every PTY under it are gone, so nothing is + // reclaimable and the sleeping record is the only way the agent comes back. + killDockerSshRelayDaemon(target) + const ledgerAfterQuit = readRemoteArgvLedger(target) + + const secondLaunch = await restart.launch() + secondApp = secondLaunch.app + await waitForSessionReady(secondLaunch.page, 60_000) + await expect + .poll(() => waitForActiveWorktree(secondLaunch.page), { timeout: 90_000 }) + .toBe(remote.worktreeId) + await waitForActiveTerminalManager(secondLaunch.page, 90_000) + // The cold-restore decision is made before the replacement PTY is spawned, so a bound PTY + // means the gate has already ruled on this record — after this, waiting is only for the + // typed command to travel. + await waitForActivePanePtyId(secondLaunch.page, 90_000) + // Orca's per-launch `claude --version` probe proves the relaunch reached the host at all; the + // negative case is vacuous without it. + await settleRemoteArgvLedger(target, ledgerAfterQuit, 90_000, (fresh) => + fresh.includes('--version') + ) + const ledger = await settleRemoteArgvLedger(target, ledgerAfterQuit, resumeBudgetMs, (fresh) => + fresh.includes('--resume') + ) + // Why this is reported rather than merely asserted: the two host stamps are what the gate reads, + // so a failure that does not name them cannot be told apart from the gate simply not running. + const diagnostics = await secondLaunch.page.evaluate((sessionId) => { + const state = window.__store?.getState() + const record = Object.values(state?.sleepingAgentSessionsByPaneKey ?? {}).find( + (candidate) => candidate.providerSession.id === sessionId + ) + const entry = Object.values(state?.agentStatusByPaneKey ?? {}).find( + (candidate) => candidate.providerSession?.id === sessionId + ) + return { + recordStamp: record ? String(record.connectionId) : 'no-record', + entryStamp: entry ? String(entry.connectionId) : 'no-entry' + } + }, SESSION_ID) + return { + ledger, + recordSurvived: diagnostics.recordStamp !== 'no-record', + diagnostics: { ...diagnostics, ledgerBeforeQuit, ledgerAfterQuit } + } + } finally { + if (secondApp) { + await restart.close(secondApp) + } + if (firstApp) { + await restart.close(firstApp) + } + await restart.dispose() + } +} + +test.describe('SSH sleeping-agent resume execution-host scope', () => { + test.skip(!RUN_DOCKER_SSH, 'Set ORCA_E2E_SSH_DOCKER=1 to run Docker-backed SSH tests.') + test.skip(process.platform === 'win32', 'Docker SSH tests use POSIX ssh tooling.') + test.describe.configure({ mode: 'serial' }) + + test("does not issue another host's session id against the SSH host", async (// oxlint-disable-next-line no-empty-pattern -- This restart test owns both Electron launches. + {}, testInfo) => { + test.setTimeout(600_000) + let target: DockerSshRelayTarget | null = null + try { + target = startDockerSshRelayTarget(testInfo) + installRemoteClaudeArgvLedger(target) + + const result = await resumeAcrossRestart( + testInfo, + target, + FOREIGN_CONNECTION_ID, + RESUME_GRACE_MS + ) + + // Why not an empty ledger: Orca legitimately probes `claude --version` on the remote to + // detect installed agents, once per launch. That is not a resume. The defect is `--resume` + // carrying an id this machine never wrote, so that is what must be absent. `result.ledger` + // is only what the relaunch appended, so the first launch's probe cannot satisfy this. + expect( + result.ledger, + `Orca ran the agent on the SSH host with a session id captured on another machine.\nrecord stamp: ${result.diagnostics.recordStamp}\nlive entry stamp: ${result.diagnostics.entryStamp}\nledger before quit: ${JSON.stringify(result.diagnostics.ledgerBeforeQuit)}\nledger after quit+relay kill: ${JSON.stringify(result.diagnostics.ledgerAfterQuit)}` + ).not.toContain('--resume') + expect(result.ledger).not.toContain(SESSION_ID) + // The relaunch's lines must not be empty either, or this proves only that the agent never + // ran at all. + expect( + result.ledger, + 'the stub agent was never invoked by the relaunch, so the lane proves nothing' + ).toContain('--version') + // Declining is only recoverable if the record survives; deleting it on a host disagreement + // would destroy the user's only handle on that transcript. + expect(result.recordSurvived, 'the declined record was discarded, not preserved').toBe(true) + } finally { + cleanupDockerSshRelayTarget(target) + } + }) + + test('still resumes a session captured on the SSH host that owns the workspace', async (// oxlint-disable-next-line no-empty-pattern -- This restart test owns both Electron launches. + {}, testInfo) => { + test.setTimeout(600_000) + let target: DockerSshRelayTarget | null = null + try { + target = startDockerSshRelayTarget(testInfo) + installRemoteClaudeArgvLedger(target) + + // The control for the test above: the same machinery, one field different, and the resume + // must still land on the remote. + const result = await resumeAcrossRestart(testInfo, target, STAMP_OWNING_HOST, 90_000) + + expect(result.ledger, 'the legitimate resume never reached the SSH host').toContain( + `--resume ${SESSION_ID}` + ) + } finally { + cleanupDockerSshRelayTarget(target) + } + }) +})