mirror of
https://github.com/stablyai/orca.git
synced 2026-10-18 16:03:17 +00:00
fix(terminal): resume hibernated agents that reattach with no payload
When agent hibernation is on, a stopped (done) agent's PTY is killed and a passive sleeping record is kept; returning to the worktree relies on the pane reattaching on remount. On the daemon path (Windows/local worktrees) the daemon can reattach the hibernation-killed session as already-live (isReattach, isNew:false) and return no snapshot/replay/coldRestore — and, being a reattach rather than a fresh spawn, it silently drops the --resume command passed on connect. The renderer adopted that empty session, leaving a blank terminal with nothing running and the sidebar history still pointing at the dead tab (the sleeping record never cleared). A reopened pane that owns a resumable slept session must always be re-driven with its resume command, never left as a bare empty attach. handleReattachResult now discards a contentless isReattach for a pane with a hibernation record and re-drives the prepared resume. This excludes the healthy cases: a fresh session the daemon created (isReattach falsy — it already ran the command) and a live reattach (carries a snapshot/replay). Forward the isReattach signal the transport was dropping so the two cases are distinguishable. Adds a deterministic regression test that fails without the guard and passes with it.
This commit is contained in:
@@ -6239,6 +6239,93 @@ describe('connectPanePty', () => {
|
||||
}
|
||||
})
|
||||
|
||||
it('re-runs the resume command when a hibernated local session reattaches with no payload', async () => {
|
||||
// Why (regression): on Windows the daemon can reattach a hibernation-killed local
|
||||
// session as isNew:false with no snapshot/coldRestore, silently dropping the --resume
|
||||
// command it only runs for a fresh session. The old code adopted that empty reattach,
|
||||
// stranding the tab blank with nothing running. The pane owns a resumable slept
|
||||
// session, so a contentless isReattach must re-drive the prepared resume instead.
|
||||
const pendingTimeouts: (() => void)[] = []
|
||||
const originalSetTimeout = globalThis.setTimeout
|
||||
globalThis.setTimeout = vi.fn((fn: () => void) => {
|
||||
pendingTimeouts.push(fn)
|
||||
return 999 as unknown as ReturnType<typeof setTimeout>
|
||||
}) as unknown as typeof setTimeout
|
||||
|
||||
try {
|
||||
const { connectPanePty } = await import('./pty-connection')
|
||||
const paneKey = makePaneKey('tab-1', LEAF_2)
|
||||
// Seed the reattach session id so getPtyId() matches the adopted reattach payload.
|
||||
const transport = createMockTransport('restored-session')
|
||||
transport.connect.mockImplementation(async (opts: { sessionId?: string }) => {
|
||||
if (opts.sessionId) {
|
||||
// Bare zombie reattach: adopted an existing session, no replay payload.
|
||||
return { id: opts.sessionId, isReattach: true }
|
||||
}
|
||||
const onPtySpawn = createdTransportOptions[0]?.onPtySpawn as
|
||||
| ((ptyId: string) => void)
|
||||
| undefined
|
||||
onPtySpawn?.('fresh-resume-pty')
|
||||
return 'fresh-resume-pty'
|
||||
})
|
||||
transportFactoryQueue.push(transport)
|
||||
mockStoreState = {
|
||||
...mockStoreState,
|
||||
tabsByWorktree: { 'wt-1': [{ id: 'tab-1', ptyId: 'restored-session' }] },
|
||||
settings: {
|
||||
...mockStoreState.settings,
|
||||
agentCmdOverrides: {}
|
||||
},
|
||||
sleepingAgentSessionsByPaneKey: {
|
||||
[paneKey]: {
|
||||
paneKey,
|
||||
tabId: 'tab-1',
|
||||
worktreeId: 'wt-1',
|
||||
agent: 'codex',
|
||||
providerSession: { key: 'session_id', id: 'codex-session-1' },
|
||||
prompt: 'finish the task',
|
||||
// Mirrors the user's scenario: a stopped/completed agent that hibernated.
|
||||
state: 'done',
|
||||
origin: 'worktree-sleep',
|
||||
capturedAt: 1,
|
||||
updatedAt: 1
|
||||
}
|
||||
}
|
||||
} as StoreState
|
||||
const pane = createPane(2)
|
||||
const manager = createManager(2)
|
||||
const deps = createDeps({
|
||||
restoredLeafId: LEAF_2,
|
||||
restoredPtyIdByLeafId: { [LEAF_2]: 'restored-session' }
|
||||
})
|
||||
|
||||
connectPanePty(pane as never, manager as never, deps as never)
|
||||
await flushAsyncTicks(20)
|
||||
for (const fn of pendingTimeouts) {
|
||||
fn()
|
||||
}
|
||||
|
||||
// The empty reattach is discarded and a fresh resume is spawned.
|
||||
expect(transport.connect).toHaveBeenCalledTimes(2)
|
||||
expect(transport.connect).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
expect.objectContaining({
|
||||
command: "codex '--dangerously-bypass-approvals-and-sandbox' 'resume' 'codex-session-1'",
|
||||
env: expect.objectContaining({
|
||||
ORCA_PANE_KEY: paneKey,
|
||||
ORCA_AGENT_LAUNCH_TOKEN: expect.stringMatching(new RegExp(`^${UUID_RE}$`))
|
||||
})
|
||||
})
|
||||
)
|
||||
// The dead session is not adopted as the pane's live PTY.
|
||||
expect(deps.clearExitedPanePtyLayoutBinding).toHaveBeenCalledWith(2, 'restored-session')
|
||||
expect(deps.clearTabPtyId).toHaveBeenCalledWith('tab-1', 'restored-session')
|
||||
expect(deps.syncPanePtyLayoutBinding).not.toHaveBeenCalledWith(2, 'restored-session')
|
||||
} finally {
|
||||
globalThis.setTimeout = originalSetTimeout
|
||||
}
|
||||
})
|
||||
|
||||
it('clears the pending serializer when disposed before non-deferred SSH reattach expiry resolves', async () => {
|
||||
const { connectPanePty } = await import('./pty-connection')
|
||||
const reattach = createDeferred<undefined>()
|
||||
|
||||
@@ -7165,6 +7165,30 @@ export function connectPanePty(
|
||||
if (!isCurrentReattachPayload()) {
|
||||
return false
|
||||
}
|
||||
// Strict precedence snapshot > replay > coldRestore: paint exactly one, else overlapping tails duplicate TUI output on worktree switch.
|
||||
const hasStructuralReplay = Boolean(
|
||||
connectResult?.snapshot || connectResult?.replay || connectResult?.coldRestore
|
||||
)
|
||||
// Why: a bare reattach (daemon adopted an existing session, isNew=false) with no
|
||||
// replayable payload means the daemon reattached a session we hibernated/killed but
|
||||
// could not cold-restore — and, being a reattach not a fresh spawn, it silently
|
||||
// dropped the --resume command we passed on connect. Adopting it strands the pane
|
||||
// blank with nothing running (the hibernation-wake blank-terminal bug). This pane
|
||||
// owns a resumable slept session, so re-drive it with the prepared resume instead.
|
||||
// Excluded: a fresh session the daemon created (isReattach falsy — it already ran the
|
||||
// command) and a live reattach (carries a snapshot/replay).
|
||||
if (!hasStructuralReplay && connectResult?.isReattach && coldRestoreStartup?.hasSleepingRecord) {
|
||||
if (staleSessionId) {
|
||||
deps.clearExitedPanePtyLayoutBinding(pane.id, staleSessionId)
|
||||
deps.clearTabPtyId(deps.tabId, staleSessionId)
|
||||
} else {
|
||||
deps.syncPanePtyLayoutBinding(pane.id, null)
|
||||
}
|
||||
startFreshColdRestoreAgentResume(coldRestoreStartup, {
|
||||
forceBlankRestoredViewport: true
|
||||
})
|
||||
return false
|
||||
}
|
||||
setPanePtyFitBinding(ptyId)
|
||||
reportPanePtyVisibility(ptyId, deps.isVisibleRef.current)
|
||||
registerSideEffectFactConsumerForPty(ptyId)
|
||||
@@ -7177,10 +7201,6 @@ export function connectPanePty(
|
||||
// Why: mobile streaming needs xterm's exact screen state; install the serializer + lastTitle source for main-process hydration parity.
|
||||
registerPaneSerializerFor(ptyId)
|
||||
|
||||
// Strict precedence snapshot > replay > coldRestore: paint exactly one, else overlapping tails duplicate TUI output on worktree switch.
|
||||
const hasStructuralReplay = Boolean(
|
||||
connectResult?.snapshot || connectResult?.replay || connectResult?.coldRestore
|
||||
)
|
||||
let reattachPayloadApplied = !hasStructuralReplay
|
||||
const applyReattachPayload = async (): Promise<void> => {
|
||||
if (!isCurrentReattachPayload()) {
|
||||
|
||||
@@ -42,6 +42,12 @@ export type PtyConnectResult = {
|
||||
/** The requested session exited while it had no primary pane handler. Its
|
||||
* buffered final data/exit were delivered, so callers must not fresh-spawn. */
|
||||
exitedBeforeAttach?: boolean
|
||||
/** The provider adopted an existing session (daemon `isNew: false`) rather
|
||||
* than creating a fresh one. When true with no snapshot/replay/coldRestore,
|
||||
* the reattach carried no content and any startup `command` was dropped
|
||||
* (the daemon only runs it for a newly created session) — the caller must
|
||||
* recover a resumable slept session instead of adopting a dead one. */
|
||||
isReattach?: boolean
|
||||
launchAgent?: TuiAgent
|
||||
launchConfig?: SleepingAgentLaunchConfig
|
||||
snapshot?: string
|
||||
|
||||
@@ -1353,6 +1353,7 @@ describe('createIpcPtyTransport', () => {
|
||||
|
||||
expect(result).toEqual({
|
||||
id: 'pty-reattach',
|
||||
isReattach: true,
|
||||
launchAgent: 'droid',
|
||||
snapshot: 'snapshot data',
|
||||
snapshotCols: 132,
|
||||
@@ -1377,6 +1378,7 @@ describe('createIpcPtyTransport', () => {
|
||||
|
||||
expect(result).toEqual({
|
||||
id: 'pty-unknown-launch-agent',
|
||||
isReattach: true,
|
||||
snapshot: undefined,
|
||||
snapshotCols: undefined,
|
||||
snapshotRows: undefined,
|
||||
|
||||
@@ -743,6 +743,10 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra
|
||||
if (spawnResult.isReattach || spawnResult.coldRestore || spawnResult.sessionExpired) {
|
||||
return {
|
||||
id: spawnResult.id,
|
||||
// Why: forward isReattach so a bare reattach with no replay payload is
|
||||
// distinguishable from a fresh session that ran our startup command; the
|
||||
// former dropped the command and must recover a resumable slept session.
|
||||
...(spawnResult.isReattach ? { isReattach: true } : {}),
|
||||
...(resultLaunchAgent ? { launchAgent: resultLaunchAgent } : {}),
|
||||
...(spawnResult.launchConfig ? { launchConfig: spawnResult.launchConfig } : {}),
|
||||
snapshot: spawnResult.snapshot,
|
||||
|
||||
Reference in New Issue
Block a user