mirror of
https://github.com/stablyai/orca.git
synced 2026-11-06 12:07:58 +00:00
fix(remote): opening a slept running agent on a host with no window resumes it, never a fresh launch (#29950)
A host that could open a window but had none left a workspace-slept running agent's record for a window wake that never came, so opening its tab from a paired client or phone launched a fresh agent with settings defaults (for example --dangerously-skip-permissions instead of the saved --permission-mode plan). The pane now resumes it with only the recorded flags. Before the spawn it durably claims the record by rewriting it into the host's own pane-owned capture form, which a window wake never relaunches (no second writer) and which still resumes the pane on a later open if the PTY is lost or the spawn fails. With a window present the window still owns the wake, and the pane opens a plain shell instead of a fresh agent.
This commit is contained in:
+159
-16
@@ -1,15 +1,18 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import { OrcaRuntimeService } from '../orca-runtime-test-mocks.spec'
|
||||
import { OrcaRuntimeService, electronMocks } from '../orca-runtime-test-mocks.spec'
|
||||
import type { SleepingAgentSessionRecord } from '../orca-runtime-test-mocks.spec'
|
||||
import {
|
||||
HEADLESS_LEAF_ID,
|
||||
HEADLESS_SECOND_LEAF_ID,
|
||||
TEST_WINDOW_ID,
|
||||
TEST_WORKTREE_ID,
|
||||
makeRuntimeStoreWithWorkspaceSession,
|
||||
makeWorkspaceSessionWithHeadlessTerminal
|
||||
} from '../orca-runtime-test-fixtures.spec'
|
||||
|
||||
const PANE_KEY = `host-tab:${HEADLESS_LEAF_ID}`
|
||||
// The host's own pane-owned capture form (#11598), which no window wake relaunches.
|
||||
const PANE_OWNED = { origin: 'worktree-sleep', state: 'done', restoreOnTabOpenOnly: true }
|
||||
|
||||
// A running agent's workspace sleep: the window's wake relaunches it, and a windowless host has none.
|
||||
function makeWorkingSleepStore(overrides: Partial<SleepingAgentSessionRecord> = {}) {
|
||||
@@ -40,8 +43,22 @@ function makeWorkingSleepStore(overrides: Partial<SleepingAgentSessionRecord> =
|
||||
return fixture
|
||||
}
|
||||
|
||||
function openPane(runtimeStore: unknown, windowStatus: 'blocked' | 'openable') {
|
||||
const spawn = vi.fn().mockResolvedValue({ id: 'persisted-pty' })
|
||||
// Why: the review's repro tab was launched as Claude, so a missed resume became a fresh agent.
|
||||
function withClaudeLaunchedTab(fixture: ReturnType<typeof makeWorkingSleepStore>): void {
|
||||
const session = structuredClone(fixture.getSession())
|
||||
const tab = session.tabsByWorktree[TEST_WORKTREE_ID]?.[0]
|
||||
if (tab) {
|
||||
tab.launchAgent = 'claude'
|
||||
}
|
||||
fixture.setSession(session)
|
||||
}
|
||||
|
||||
function openPane(
|
||||
runtimeStore: unknown,
|
||||
windowStatus: 'blocked' | 'openable',
|
||||
opts: { spawn?: () => Promise<{ id: string }>; withWindow?: boolean } = {}
|
||||
) {
|
||||
const spawn = vi.fn(opts.spawn ?? (() => Promise.resolve({ id: 'persisted-pty' })))
|
||||
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the fixture store implements the runtime store methods these tests reach.
|
||||
const runtime = new OrcaRuntimeService(runtimeStore as never, undefined, {
|
||||
getDesktopWindowStatus: () => windowStatus
|
||||
@@ -54,17 +71,35 @@ function openPane(runtimeStore: unknown, windowStatus: 'blocked' | 'openable') {
|
||||
listProcesses: async () => []
|
||||
})
|
||||
runtime.syncWindowGraph(0, { tabs: [], leaves: [] })
|
||||
const opened = runtime.activateMobileSessionTab(
|
||||
`id:${TEST_WORKTREE_ID}`,
|
||||
'host-tab',
|
||||
HEADLESS_LEAF_ID,
|
||||
{ notifyClients: false, navigation: 'caller', intent: 'user' }
|
||||
)
|
||||
return { opened, spawn }
|
||||
electronMocks.BrowserWindow.fromId.mockReturnValue(null)
|
||||
const opened = (async () => {
|
||||
if (opts.withWindow) {
|
||||
runtime.attachWindow(TEST_WINDOW_ID)
|
||||
runtime.markGraphReady(TEST_WINDOW_ID)
|
||||
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the runtime reads only isDestroyed from the window mock.
|
||||
electronMocks.BrowserWindow.fromId.mockReturnValue({ isDestroyed: () => false } as never)
|
||||
// A paired desktop client opening the worktree hydrates the host's tabs without a wake.
|
||||
await runtime.activateManagedWorktree(`id:${TEST_WORKTREE_ID}`, {
|
||||
notifyClients: false,
|
||||
clientKind: 'runtime'
|
||||
})
|
||||
}
|
||||
return runtime.activateMobileSessionTab(
|
||||
`id:${TEST_WORKTREE_ID}`,
|
||||
'host-tab',
|
||||
HEADLESS_LEAF_ID,
|
||||
{
|
||||
notifyClients: false,
|
||||
navigation: 'caller',
|
||||
intent: 'user'
|
||||
}
|
||||
)
|
||||
})()
|
||||
return { opened, spawn, runtime }
|
||||
}
|
||||
|
||||
function spawnedCommand(spawn: ReturnType<typeof vi.fn>): string {
|
||||
const args: unknown = spawn.mock.calls[0]?.[0]
|
||||
function spawnedCommand(spawn: ReturnType<typeof vi.fn>, call = 0): string {
|
||||
const args: unknown = spawn.mock.calls[call]?.[0]
|
||||
return typeof args === 'object' && args !== null && 'command' in args
|
||||
? String(args.command ?? '')
|
||||
: ''
|
||||
@@ -108,11 +143,119 @@ describe('opening a workspace-slept running agent on a host with no window', ()
|
||||
expect(spawnedCommand(spawn)).not.toContain('provider-session-1')
|
||||
})
|
||||
|
||||
it('leaves the resume to the window wake on a host that can open one', async () => {
|
||||
const { runtimeStore } = makeWorkingSleepStore()
|
||||
const { opened, spawn } = openPane(runtimeStore, 'openable')
|
||||
// Review repro: a host that could open a window but had none fell back to a fresh launch.
|
||||
it('resumes in the pane on a host that can open a window but has none', async () => {
|
||||
const fixture = makeWorkingSleepStore({
|
||||
launchConfig: { agentArgs: '--permission-mode plan', agentEnv: {} }
|
||||
})
|
||||
withClaudeLaunchedTab(fixture)
|
||||
const { opened, spawn } = openPane(fixture.runtimeStore, 'openable')
|
||||
|
||||
await opened
|
||||
expect(spawnedCommand(spawn)).not.toContain('provider-session-1')
|
||||
const command = spawnedCommand(spawn)
|
||||
expect(command).toContain('--resume')
|
||||
expect(command).toContain('provider-session-1')
|
||||
expect(command).toMatch(/--permission-mode'? '?plan/)
|
||||
expect(command).not.toContain('--dangerously-skip-permissions')
|
||||
// Why: a later window wake relaunches a running record (a second writer), never a pane-owned one.
|
||||
expect(fixture.getSession().sleepingAgentSessionsByPaneKey?.[PANE_KEY]).toMatchObject(
|
||||
PANE_OWNED
|
||||
)
|
||||
})
|
||||
|
||||
// Codex repro: deleting the record left a restarted host nothing to resume a lost PTY from.
|
||||
it('still resumes with the recorded flags after a host restart loses the PTY', async () => {
|
||||
const fixture = makeWorkingSleepStore({
|
||||
launchConfig: { agentArgs: '--permission-mode plan', agentEnv: {} }
|
||||
})
|
||||
withClaudeLaunchedTab(fixture)
|
||||
await openPane(fixture.runtimeStore, 'openable').opened
|
||||
|
||||
const restarted = openPane(fixture.runtimeStore, 'openable')
|
||||
await restarted.opened
|
||||
const command = spawnedCommand(restarted.spawn)
|
||||
expect(command).toContain('provider-session-1')
|
||||
expect(command).toMatch(/--permission-mode'? '?plan/)
|
||||
expect(command).not.toContain('--dangerously-skip-permissions')
|
||||
})
|
||||
|
||||
// Codex repro: spending the record before the spawn let an overlapping reconnect probe fresh-launch.
|
||||
it('keeps an automatic probe parked while the resume spawn is in flight', async () => {
|
||||
const fixture = makeWorkingSleepStore()
|
||||
withClaudeLaunchedTab(fixture)
|
||||
let landSpawn: (value: { id: string }) => void = () => {}
|
||||
let spawnStarted: () => void = () => {}
|
||||
const started = new Promise<void>((resolve) => {
|
||||
spawnStarted = resolve
|
||||
})
|
||||
const { opened, spawn, runtime } = openPane(fixture.runtimeStore, 'openable', {
|
||||
spawn: () => {
|
||||
spawnStarted()
|
||||
return new Promise((resolve) => {
|
||||
landSpawn = resolve
|
||||
})
|
||||
}
|
||||
})
|
||||
await started
|
||||
// Codex repro: a window attaching mid-spawn must already see the record as the pane's.
|
||||
expect(fixture.getSession().sleepingAgentSessionsByPaneKey?.[PANE_KEY]).toMatchObject(
|
||||
PANE_OWNED
|
||||
)
|
||||
|
||||
await runtime.activateMobileSessionTab(`id:${TEST_WORKTREE_ID}`, 'host-tab', HEADLESS_LEAF_ID, {
|
||||
notifyClients: false,
|
||||
navigation: 'caller',
|
||||
intent: 'automatic'
|
||||
})
|
||||
expect(spawn).toHaveBeenCalledOnce()
|
||||
landSpawn({ id: 'persisted-pty' })
|
||||
await opened
|
||||
expect(spawnedCommand(spawn)).toContain('provider-session-1')
|
||||
})
|
||||
|
||||
// Codex repro: the open that lost the claim spawned a plain shell that could swallow the resume.
|
||||
it('resumes from both of two overlapping opens of the pane, never a plain shell', async () => {
|
||||
const fixture = makeWorkingSleepStore()
|
||||
withClaudeLaunchedTab(fixture)
|
||||
const { opened, spawn, runtime } = openPane(fixture.runtimeStore, 'openable')
|
||||
const second = runtime.activateMobileSessionTab(
|
||||
`id:${TEST_WORKTREE_ID}`,
|
||||
'host-tab',
|
||||
HEADLESS_LEAF_ID,
|
||||
{ notifyClients: false, navigation: 'caller', intent: 'user' }
|
||||
)
|
||||
|
||||
await Promise.allSettled([opened, second])
|
||||
expect(spawn).toHaveBeenCalled()
|
||||
for (let call = 0; call < spawn.mock.calls.length; call += 1) {
|
||||
expect(spawnedCommand(spawn, call)).toContain('provider-session-1')
|
||||
}
|
||||
})
|
||||
|
||||
it('keeps the record for the next open when the pane resume fails to spawn', async () => {
|
||||
const fixture = makeWorkingSleepStore()
|
||||
const { opened, spawn } = openPane(fixture.runtimeStore, 'openable', {
|
||||
spawn: () => Promise.reject(new Error('spawn failed'))
|
||||
})
|
||||
|
||||
await expect(opened).rejects.toThrow('spawn failed')
|
||||
expect(spawnedCommand(spawn)).toContain('provider-session-1')
|
||||
expect(fixture.getSession().sleepingAgentSessionsByPaneKey?.[PANE_KEY]).toMatchObject(
|
||||
PANE_OWNED
|
||||
)
|
||||
const retried = openPane(fixture.runtimeStore, 'openable')
|
||||
await retried.opened
|
||||
expect(spawnedCommand(retried.spawn)).toContain('provider-session-1')
|
||||
})
|
||||
|
||||
it('opens a plain shell, not a fresh agent, when a window will wake the record', async () => {
|
||||
const fixture = makeWorkingSleepStore()
|
||||
withClaudeLaunchedTab(fixture)
|
||||
const { opened, spawn } = openPane(fixture.runtimeStore, 'openable', { withWindow: true })
|
||||
|
||||
await opened
|
||||
expect(spawn).toHaveBeenCalledOnce()
|
||||
expect(spawnedCommand(spawn)).toBe('')
|
||||
expect(fixture.getSession().sleepingAgentSessionsByPaneKey?.[PANE_KEY]).toBeDefined()
|
||||
})
|
||||
})
|
||||
|
||||
+24
-7
@@ -22,6 +22,8 @@ import type {
|
||||
MobileTerminalResumeCommand,
|
||||
OrcaRuntimeWithResolveMobileSessionTerminalCommand as MobileTerminalStartup
|
||||
} from '../../terminal/launch/mobile-terminal-startup'
|
||||
import type { OrcaRuntimeWithStopTerminalsForWorktree as SessionEditHost } from '../../terminal/stop/worktree-terminal-stop'
|
||||
import { claimWindowWakeRecordForPane, isWindowWakeSleepRecord } from './window-wake-sleep-record'
|
||||
import { runtimeWorktreeIdsEqual } from '../../runtime-worktree-path-identity'
|
||||
import {
|
||||
buildHeadlessMobileSessionTabGroups,
|
||||
@@ -38,6 +40,9 @@ export abstract class OrcaRuntimeWithApplyMobileSessionTabNavigation extends Orc
|
||||
protected abstract isHeadlessMobileSessionPublication(
|
||||
...args: Parameters<MobileSessionPreservation['isHeadlessMobileSessionPublication']>
|
||||
): ReturnType<MobileSessionPreservation['isHeadlessMobileSessionPublication']>
|
||||
protected abstract commitWorkspaceSessionEdit(
|
||||
...args: Parameters<SessionEditHost['commitWorkspaceSessionEdit']>
|
||||
): ReturnType<SessionEditHost['commitWorkspaceSessionEdit']>
|
||||
|
||||
protected applyMobileSessionTabNavigation(
|
||||
snapshot: RuntimeMobileSessionTabsResult,
|
||||
@@ -147,20 +152,18 @@ export abstract class OrcaRuntimeWithApplyMobileSessionTabNavigation extends Orc
|
||||
}
|
||||
// Why: an originless record predates capture origins and carries no proof of which pane or
|
||||
// host wrote it, so it is left (record kept) for the window's legacy cleanup (#29303).
|
||||
// A host that can never open a window (orcad, blocked serve) has no window wake, so the pane owns it.
|
||||
// A host with no window right now has no window wake, so the pane owns the record and claims it.
|
||||
const paneOwned =
|
||||
record.origin === 'live' ||
|
||||
record.origin === 'quit' ||
|
||||
(record.origin === 'worktree-sleep' &&
|
||||
(record.state === 'done' ||
|
||||
(this.getDesktopWindowStatusFn() === 'blocked' &&
|
||||
!this.getAvailableAuthoritativeWindow())))
|
||||
(record.state === 'done' || !this.getAvailableAuthoritativeWindow()))
|
||||
return paneOwned ? record : null
|
||||
}
|
||||
|
||||
/**
|
||||
* The pane's resume launch: undefined when it owns no record, `{}` (plain shell, record kept,
|
||||
* as the window does) when the record cannot be resumed here.
|
||||
* as the window does) when the record cannot be resumed here or the window will wake it.
|
||||
*/
|
||||
protected async resolvePaneOwnedResumeStartup(
|
||||
worktreeId: string,
|
||||
@@ -168,7 +171,9 @@ export abstract class OrcaRuntimeWithApplyMobileSessionTabNavigation extends Orc
|
||||
): Promise<MobileTerminalResumeCommand | Record<string, never> | undefined> {
|
||||
const record = this.getPaneOwnedResumeRecord(worktreeId, tab)
|
||||
if (!record) {
|
||||
return undefined
|
||||
const unowned = this.getPaneSleepingAgentRecord(worktreeId, tab)
|
||||
// Why: a fresh launch would drop the conversation and its recorded flags (#29672).
|
||||
return unowned && isWindowWakeSleepRecord(unowned) ? {} : undefined
|
||||
}
|
||||
// Why: nothing here spends the record, so a session another pane also records could be
|
||||
// resumed twice (two writers on one transcript), as could one already live in another pane.
|
||||
@@ -186,12 +191,24 @@ export abstract class OrcaRuntimeWithApplyMobileSessionTabNavigation extends Orc
|
||||
) {
|
||||
return {}
|
||||
}
|
||||
let resume: MobileTerminalResumeCommand | null
|
||||
try {
|
||||
const workspace = await this.resolveTerminalWorkspaceLaunchScope(`id:${worktreeId}`)
|
||||
return this.resolveMobileSessionResumeCommand(workspace, record) ?? {}
|
||||
resume = this.resolveMobileSessionResumeCommand(workspace, record)
|
||||
} catch {
|
||||
return {}
|
||||
}
|
||||
if (!resume || !isWindowWakeSleepRecord(record)) {
|
||||
return resume ?? {}
|
||||
}
|
||||
const hostId = this.getWorkspaceSessionHostIdForWorktree(worktreeId)
|
||||
const claimed = await claimWindowWakeRecordForPane(
|
||||
(edit) => this.commitWorkspaceSessionEdit(hostId, edit),
|
||||
// Why: a window that attached during the awaits above now owns this record's wake.
|
||||
() => Boolean(this.getAvailableAuthoritativeWindow()),
|
||||
record
|
||||
)
|
||||
return claimed ? resume : {}
|
||||
}
|
||||
|
||||
/** The host's hook store places this conversation in another pane not proven exited. */
|
||||
|
||||
@@ -0,0 +1,69 @@
|
||||
import {
|
||||
agentProviderSessionsEqual,
|
||||
type SleepingAgentSessionRecord
|
||||
} from '../../../../shared/agent-session-resume'
|
||||
import type { WorkspaceSessionState } from '../../../../shared/workspace-session-state-types'
|
||||
|
||||
/** A running agent's sleep record, which the window's worktree wake relaunches in a fresh tab. */
|
||||
export function isWindowWakeSleepRecord(record: SleepingAgentSessionRecord): boolean {
|
||||
return record.origin === 'worktree-sleep' && record.state !== 'done'
|
||||
}
|
||||
|
||||
/** The host's own pane-owned capture form (#11598), which no window wake relaunches. */
|
||||
function paneOwnedForm(record: SleepingAgentSessionRecord): SleepingAgentSessionRecord {
|
||||
return { ...record, state: 'done', restoreOnTabOpenOnly: true }
|
||||
}
|
||||
|
||||
function sameRecord(a: SleepingAgentSessionRecord, b: SleepingAgentSessionRecord): boolean {
|
||||
return (
|
||||
a.origin === b.origin &&
|
||||
a.state === b.state &&
|
||||
a.capturedAt === b.capturedAt &&
|
||||
a.agent === b.agent &&
|
||||
agentProviderSessionsEqual(a.agent, a.providerSession, b.providerSession)
|
||||
)
|
||||
}
|
||||
|
||||
type SessionEdit = (
|
||||
edit: (session: WorkspaceSessionState) => WorkspaceSessionState | null
|
||||
) => Promise<void>
|
||||
|
||||
/**
|
||||
* Durably hands a window-wake record to the pane about to resume it, before the spawn, unless a
|
||||
* window now owns its wake: a window attaching mid-spawn then cannot relaunch it as a second writer,
|
||||
* and a PTY lost later (or a failed spawn) still resumes the pane with its recorded flags on the
|
||||
* next open. False when nothing was claimed; the pane then opens a plain shell.
|
||||
*/
|
||||
export async function claimWindowWakeRecordForPane(
|
||||
commit: SessionEdit,
|
||||
windowOwnsWake: () => boolean,
|
||||
record: SleepingAgentSessionRecord
|
||||
): Promise<boolean> {
|
||||
const claimed = paneOwnedForm(record)
|
||||
let owned = false
|
||||
try {
|
||||
await commit((session) => {
|
||||
const stored = session.sleepingAgentSessionsByPaneKey?.[record.paneKey]
|
||||
// Why: an overlapping open of this pane claimed it first; resume alongside it, since a plain
|
||||
// shell spawned here could win the pane and swallow that open's resume.
|
||||
if (stored && sameRecord(stored, claimed)) {
|
||||
owned = true
|
||||
return null
|
||||
}
|
||||
if (!stored || !sameRecord(stored, record) || windowOwnsWake()) {
|
||||
return null
|
||||
}
|
||||
owned = true
|
||||
return {
|
||||
...session,
|
||||
sleepingAgentSessionsByPaneKey: {
|
||||
...session.sleepingAgentSessionsByPaneKey,
|
||||
[record.paneKey]: claimed
|
||||
}
|
||||
}
|
||||
})
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
return owned
|
||||
}
|
||||
Reference in New Issue
Block a user