mirror of
https://github.com/stablyai/orca.git
synced 2026-10-31 00:57:45 +00:00
fix(terminal): a pane a relaunched app has not reopened keeps its history and folder when slept (#33425)
* fix(terminal): a pane a relaunched app has not reopened keeps its history and folder when slept After an app relaunch, a terminal the window had not shown again was known to the daemon client only from its terminal list: no history writer and, for a WSL pane, no distro or pane key. Its Sleep then wrote no final checkpoint at all (only output.log), so Wake restored from older output and, for a WSL pane on Windows, reopened at the spawn folder (awin verify-10, X2). #33398 learned those facts in `reconcileOnStartup`, which nothing in the app calls. Sleep now adopts such a session first: one `listSessions`, then the same history reconciliation, distro and pane key a startup adoption records. Its final checkpoint then lands with the shell's folder. Sessions the client spawned or reattached skip this. Co-Authored-By: Claude <noreply@anthropic.com> * fix(terminal): Sleep never re-adopts a session this client suspended after a seed failure Review of #33425: adopting an unattached session before Sleep also caught sessions this client already holds but suspended after a history seed failed (an old daemon, an oversized seed, a transfer failure). Re-adopting one wrote its fresh-only screen as the authoritative checkpoint, so Wake lost everything from before the restore. Sleep now adopts only sessions the client does not already hold as active. A session it does adopt becomes active only once its history writer is open, so an exit during the reconcile leaves it out. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -700,6 +700,50 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => {
|
||||
}
|
||||
})
|
||||
|
||||
// Review of #33425: Sleep's adoption of an unattached session must not re-adopt one this
|
||||
// client suspended after a seed failure, or Wake loses everything before the restore.
|
||||
it('keeps a seed-failed session suspended when it is slept after an adapter restart', async () => {
|
||||
const sessionId = 'cold-restore-seed-failure-sleep'
|
||||
const sessionDir = join(historyDir, getHistorySessionDirName(sessionId))
|
||||
mkdirSync(sessionDir, { recursive: true })
|
||||
writeFileSync(
|
||||
join(sessionDir, 'meta.json'),
|
||||
JSON.stringify({
|
||||
cwd: '/tmp',
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
startedAt: '2026-07-10T08:00:00Z',
|
||||
endedAt: null,
|
||||
exitCode: null
|
||||
})
|
||||
)
|
||||
writeFileSync(join(sessionDir, 'scrollback.bin'), 'recovered marker\r\n')
|
||||
|
||||
const first = new DaemonPtyAdapter({ socketPath, tokenPath, historyPath: historyDir })
|
||||
const originalWriteSync = HeadlessEmulator.prototype.writeSync
|
||||
const writeSyncSpy = vi
|
||||
.spyOn(HeadlessEmulator.prototype, 'writeSync')
|
||||
.mockImplementation(function (this: HeadlessEmulator, data) {
|
||||
return data.includes('recovered marker') ? false : originalWriteSync.call(this, data)
|
||||
})
|
||||
try {
|
||||
await first.spawn({ cols: 80, rows: 24, sessionId })
|
||||
lastSubprocess._simulateData('fresh-only output\r\n')
|
||||
await first.disconnectOnly()
|
||||
|
||||
historyAdapter = new DaemonPtyAdapter({ socketPath, tokenPath, historyPath: historyDir })
|
||||
await historyAdapter.spawn({ cols: 80, rows: 24, sessionId })
|
||||
await historyAdapter.shutdown(sessionId, { immediate: true, keepHistory: true })
|
||||
|
||||
expect(existsSync(join(sessionDir, 'checkpoint.json'))).toBe(false)
|
||||
expect(readFileSync(join(sessionDir, 'scrollback.bin'), 'utf8')).toContain(
|
||||
'recovered marker'
|
||||
)
|
||||
} finally {
|
||||
writeSyncSpy.mockRestore()
|
||||
}
|
||||
})
|
||||
|
||||
it('does not cold-restore for clean shutdown (endedAt set)', async () => {
|
||||
const sessionId = 'clean-exit'
|
||||
const sessionDir = join(historyDir, getHistorySessionDirName(sessionId))
|
||||
|
||||
@@ -162,6 +162,28 @@ describe('a sleeping bridge WSL pane keeps its shell folder', () => {
|
||||
expect(woken.coldRestore).toBeTruthy()
|
||||
})
|
||||
|
||||
// awin verify-10, X2: production never calls reconcileOnStartup, so after a relaunch a pane the
|
||||
// window has not reopened is known only from the inventory; its Sleep wrote no checkpoint at all.
|
||||
it('reads it for a pane a relaunched app only listed, never adopted or reattached', async () => {
|
||||
const before = new DaemonPtyAdapter({ socketPath, tokenPath, historyPath: historyDir() })
|
||||
const { id } = await before.spawn({
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
cwd: WSL_CWD,
|
||||
env: { ORCA_PANE_KEY: PANE_KEY },
|
||||
sessionId: 'repo::/repo@@listed'
|
||||
})
|
||||
lastSubprocess._simulateData('$ cd deep/er\r\n')
|
||||
await before.disconnectOnly()
|
||||
adapter = new DaemonPtyAdapter({ socketPath, tokenPath, historyPath: historyDir() })
|
||||
expect((await adapter.listProcesses()).map((process) => process.id)).toContain(id)
|
||||
|
||||
await adapter.shutdown(id, { immediate: true, keepHistory: true })
|
||||
|
||||
expect(readRows).toHaveBeenCalledWith('Ubuntu', expect.any(Number), { running: true })
|
||||
expect(checkpointCwd(id)).toBe(`${WSL_CWD}\\deep\\er`)
|
||||
})
|
||||
|
||||
it('keeps the spawn folder when the distro cannot be read', async () => {
|
||||
readRows.mockResolvedValue(null)
|
||||
adapter = new DaemonPtyAdapter({ socketPath, tokenPath, historyPath: historyDir() })
|
||||
|
||||
@@ -12,6 +12,7 @@ import type {
|
||||
PtyProcessInspectionOptions
|
||||
} from '../providers/pty-process-inspection'
|
||||
import { clientOnlyUnverifiableInspection } from '../../shared/terminal-process-inspection'
|
||||
import { remainingDaemonRequestTimeoutMs } from './daemon-request-deadline'
|
||||
|
||||
export abstract class DaemonPtyProcessInspection extends DaemonPtyBufferSnapshots {
|
||||
// Detached daemon PTYs can host agents; cleanup must not treat every foreground process as an idle shell.
|
||||
@@ -129,25 +130,65 @@ export abstract class DaemonPtyProcessInspection extends DaemonPtyBufferSnapshot
|
||||
killed.push(session.sessionId)
|
||||
} else {
|
||||
alive.push(session.sessionId)
|
||||
// Include background sessions in the final disconnect checkpoint.
|
||||
this.activeSessionIds.add(session.sessionId)
|
||||
// Why: after a relaunch a pane the window has not reopened still sleeps, and reading its
|
||||
// shell's folder then needs what only the daemon still knows about it.
|
||||
if (session.wslDistro) {
|
||||
this.wslDistrosBySessionId.set(session.sessionId, session.wslDistro)
|
||||
}
|
||||
if (session.wslDistro && session.paneKey) {
|
||||
this.wslPaneKeysBySessionId.set(session.sessionId, session.paneKey)
|
||||
}
|
||||
await this.reconcileLiveSessionHistory(session).catch((err) =>
|
||||
console.warn('[history] live-session reconciliation failed:', session.sessionId, err)
|
||||
)
|
||||
await this.adoptLiveSession(session)
|
||||
}
|
||||
}
|
||||
|
||||
return { alive, killed }
|
||||
}
|
||||
|
||||
/**
|
||||
* A session this client never spawned or reattached, such as one a relaunched app's window has
|
||||
* not reopened: adopted before its Sleep, or that Sleep writes no checkpoint and keeps no folder.
|
||||
*/
|
||||
protected async adoptListedSessionForSleep(
|
||||
sessionId: string,
|
||||
deadlineMs?: number
|
||||
): Promise<void> {
|
||||
// Why active too: a session this client suspended after a seed failure must stay suspended,
|
||||
// or its fresh-only screen would replace the history it was restored from.
|
||||
if (
|
||||
!this.historyManager ||
|
||||
this.historyManager.hasWriter(sessionId) ||
|
||||
this.activeSessionIds.has(sessionId)
|
||||
) {
|
||||
return
|
||||
}
|
||||
const result = await this.client.request<ListSessionsResult>(
|
||||
'listSessions',
|
||||
undefined,
|
||||
remainingDaemonRequestTimeoutMs(deadlineMs)
|
||||
)
|
||||
const session = result.sessions.find((entry) => entry.sessionId === sessionId && entry.isAlive)
|
||||
if (session) {
|
||||
await this.adoptLiveSession(session, { activeOnlyIfReconciled: true })
|
||||
}
|
||||
}
|
||||
|
||||
private async adoptLiveSession(
|
||||
session: SessionInfo,
|
||||
opts: { activeOnlyIfReconciled?: boolean } = {}
|
||||
): Promise<void> {
|
||||
// Include background sessions in the final disconnect checkpoint.
|
||||
if (!opts.activeOnlyIfReconciled) {
|
||||
this.activeSessionIds.add(session.sessionId)
|
||||
}
|
||||
// Sleep reads a WSL shell's folder by what only the daemon still knows about the pane.
|
||||
if (session.wslDistro) {
|
||||
this.wslDistrosBySessionId.set(session.sessionId, session.wslDistro)
|
||||
if (session.paneKey) {
|
||||
this.wslPaneKeysBySessionId.set(session.sessionId, session.paneKey)
|
||||
}
|
||||
}
|
||||
await this.reconcileLiveSessionHistory(session).catch((err) =>
|
||||
console.warn('[history] live-session reconciliation failed:', session.sessionId, err)
|
||||
)
|
||||
// Why after: an exit seen during the reconcile closed the writer; that session is not live.
|
||||
if (opts.activeOnlyIfReconciled && this.historyManager?.hasWriter(session.sessionId)) {
|
||||
this.activeSessionIds.add(session.sessionId)
|
||||
}
|
||||
}
|
||||
|
||||
protected async reconcileLiveSessionHistory(session: SessionInfo): Promise<void> {
|
||||
const historyManager = this.historyManager
|
||||
const historyReader = this.historyReader
|
||||
|
||||
@@ -17,6 +17,11 @@ export const LIVENESS_PROBE_TIMEOUT_MS = 2_000
|
||||
const MAX_TOMBSTONES = 1000
|
||||
|
||||
export abstract class DaemonPtySessionControl extends DaemonPtySessionInput {
|
||||
protected abstract adoptListedSessionForSleep(
|
||||
sessionId: string,
|
||||
deadlineMs?: number
|
||||
): Promise<void>
|
||||
|
||||
async attach(id: string): Promise<Pick<PtySpawnResult, 'providerSequence' | 'lastTitle'> | void> {
|
||||
await this.ensureConnected()
|
||||
if (!this.canDelegateBackgroundToDaemon) {
|
||||
@@ -93,6 +98,11 @@ export abstract class DaemonPtySessionControl extends DaemonPtySessionInput {
|
||||
if (opts.keepHistory && this.disconnectOnlyPromise) {
|
||||
throw new Error('Cannot keep history after daemon disconnect has started')
|
||||
}
|
||||
if (opts.keepHistory) {
|
||||
// Outside the spawn lock, which adopting the session's history takes itself.
|
||||
await this.ensureConnected(opts.deadlineMs)
|
||||
await this.adoptListedSessionForSleep(id, opts.deadlineMs).catch(() => {})
|
||||
}
|
||||
const shutdown = this.withHistorySpawnLock(id, () => this.shutdownWithHistoryLock(id, opts))
|
||||
if (!opts.keepHistory) {
|
||||
await shutdown
|
||||
|
||||
Reference in New Issue
Block a user