starred/orca
1
0
Fork 0
mirror of https://github.com/stablyai/orca.git synced 2026-10-23 00:03:52 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(terminals): serialize history-preserving stop ownership

Browse Source
This commit is contained in:
Jinwoo-H
2026-08-20 03:14:04 -04:00
parent 9773a84983
commit e0923f2cd4
6 changed files with 249 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/main/ipc/pty-runtime-kill-and-exit.test.ts
+6 -4
View File
@@ -166,7 +166,8 @@ describe('registerPtyHandlers', () => {
const runtime = {
setPtyController: vi.fn(),
markPtyStopRequested: vi.fn(),
markPtyHistoryPreservingStopRequested: vi.fn(),
markPtyHistoryPreservingStopRequested: vi.fn(() => 1),
preservePtyHistoryThroughLateExit: vi.fn(),
clearPtyHistoryPreservingStopRequested: vi.fn()
}
setLocalPtyProvider({
@@ -198,7 +199,8 @@ describe('registerPtyHandlers', () => {
handlers.get('pty:kill')!(null, { id: 'local-pty', keepHistory: true })
).rejects.toThrow('daemon unavailable')
expect(runtime.markPtyHistoryPreservingStopRequested).toHaveBeenCalledWith('local-pty')
expect(runtime.clearPtyHistoryPreservingStopRequested).toHaveBeenCalledWith('local-pty')
expect(runtime.preservePtyHistoryThroughLateExit).toHaveBeenCalledWith('local-pty', 1)
expect(runtime.clearPtyHistoryPreservingStopRequested).not.toHaveBeenCalled()
})
it('rejects runtime terminal IDs before unowned local provider routing', async () => {
const shutdown = vi.spyOn(getLocalPtyProvider(), 'shutdown')
@@ -215,7 +217,7 @@ describe('registerPtyHandlers', () => {
const runtime = {
setPtyController: vi.fn(),
onPtyExit: vi.fn(),
markPtyHistoryPreservingStopRequested: vi.fn(),
markPtyHistoryPreservingStopRequested: vi.fn(() => 1),
clearPtyHistoryPreservingStopRequested: vi.fn()
}
setLocalPtyProvider({
@@ -251,7 +253,7 @@ describe('registerPtyHandlers', () => {
})
expect(runtime.onPtyExit).toHaveBeenCalledWith('local-pty', -1, undefined)
expect(runtime.markPtyHistoryPreservingStopRequested).toHaveBeenCalledWith('local-pty')
expect(runtime.clearPtyHistoryPreservingStopRequested).toHaveBeenCalledWith('local-pty')
expect(runtime.clearPtyHistoryPreservingStopRequested).toHaveBeenCalledWith('local-pty', 1)
expect(runtime.markPtyHistoryPreservingStopRequested.mock.invocationCallOrder[0]).toBeLessThan(
runtime.onPtyExit.mock.invocationCallOrder[0]!
)
src/main/ipc/pty.ts
+23 -10
View File
@@ -4033,7 +4033,8 @@ export function registerPtyHandlers(
async function shutdownProviderAndDetectExit(
provider: IPtyProvider,
id: string,
opts: { immediate?: boolean; keepHistory?: boolean; deadlineMs?: number }
opts: { immediate?: boolean; keepHistory?: boolean; deadlineMs?: number },
onExitObserved?: () => void
): Promise<boolean> {
let providerExitObserved = false
const expectedIncarnationId = ptyIncarnationById.get(id)
@@ -4043,6 +4044,7 @@ export function registerPtyHandlers(
(!expectedIncarnationId || payload.incarnationId === expectedIncarnationId)
) {
providerExitObserved = true
onExitObserved?.()
}
})
try {
@@ -7722,9 +7724,9 @@ export function registerPtyHandlers(
throw new Error('Invalid PTY provider id')
}
runtime?.markPtyStopRequested?.(args.id)
if (args.keepHistory) {
runtime?.markPtyHistoryPreservingStopRequested(args.id)
}
let historyPreservingStopId = args.keepHistory
? runtime?.markPtyHistoryPreservingStopRequested(args.id)
: undefined
try {
const ownedConnectionId = ptyOwnership.get(args.id)
const parsedSshId = ownedConnectionId === undefined ? parseAppSshPtyId(args.id) : null
@@ -7749,13 +7751,24 @@ export function registerPtyHandlers(
const shutdownProvider = provider ?? getProviderForPty(args.id)
let providerExitObserved = false
try {
providerExitObserved = await shutdownProviderAndDetectExit(shutdownProvider, args.id, {
immediate: true,
keepHistory: args.keepHistory ?? false
})
providerExitObserved = await shutdownProviderAndDetectExit(
shutdownProvider,
args.id,
{
immediate: true,
keepHistory: args.keepHistory ?? false
},
() => {
providerExitObserved = true
}
)
} catch (err) {
if (!isPtyAlreadyGoneError(err)) {
// Why: a failed shutdown can leave the process alive (SSH relay grace window / local daemon); keep ownership/lease state so the user can retry.
if (historyPreservingStopId !== undefined && !providerExitObserved) {
runtime?.preservePtyHistoryThroughLateExit(args.id, historyPreservingStopId)
historyPreservingStopId = undefined
}
throw err
}
/* session already dead — cleanup below handles the rest */
@@ -7769,8 +7782,8 @@ export function registerPtyHandlers(
sendPtyExitToRenderer({ id: args.id, code: -1 })
}
} finally {
if (args.keepHistory) {
runtime?.clearPtyHistoryPreservingStopRequested(args.id)
if (historyPreservingStopId !== undefined) {
runtime?.clearPtyHistoryPreservingStopRequested(args.id, historyPreservingStopId)
}
}
})
src/main/runtime/orca-runtime-terminal-retirement.test.ts
+67 -2
View File
@@ -488,9 +488,9 @@ describe('OrcaRuntimeService terminal surface retirement', () => {
incarnationId: 'incarnation-left'
})
runtime.markPtyHistoryPreservingStopRequested('pty-left')
const stopRequestId = runtime.markPtyHistoryPreservingStopRequested('pty-left')
runtime.onPtyExit('pty-left', 0, 'incarnation-left')
runtime.clearPtyHistoryPreservingStopRequested('pty-left')
runtime.clearPtyHistoryPreservingStopRequested('pty-left', stopRequestId)
expect((await runtime.listMobileSessionTabs(`id:${WORKTREE_ID}`)).tabs).toEqual([
expect.objectContaining({ id: 'tab::left' }),
@@ -499,6 +499,71 @@ describe('OrcaRuntimeService terminal surface retirement', () => {
expect(setWorkspaceSession).not.toHaveBeenCalled()
})
it('preserves a surface until every overlapping history-preserving stop finishes', async () => {
const session = makePersistedSplitSession()
const setWorkspaceSession = vi.fn()
const runtime = new OrcaRuntimeService(
runtimeStore({
getWorkspaceSession: () => session,
setWorkspaceSession,
flushOrThrow: vi.fn()
})
)
runtime.attachWindow(1)
syncSplit(runtime)
runtime.registerPty('pty-left', WORKTREE_ID, null, {
tabId: 'tab',
leafId: 'left',
incarnationId: 'incarnation-left'
})
const firstStopRequestId = runtime.markPtyHistoryPreservingStopRequested('pty-left')
const secondStopRequestId = runtime.markPtyHistoryPreservingStopRequested('pty-left')
runtime.clearPtyHistoryPreservingStopRequested('pty-left', firstStopRequestId)
runtime.onPtyExit('pty-left', 0, 'incarnation-left')
runtime.clearPtyHistoryPreservingStopRequested('pty-left', secondStopRequestId)
expect((await runtime.listMobileSessionTabs(`id:${WORKTREE_ID}`)).tabs).toEqual([
expect.objectContaining({ id: 'tab::left' }),
expect.objectContaining({ id: 'tab::right' })
])
expect(setWorkspaceSession).not.toHaveBeenCalled()
})
it('consumes a failed stop lease when its late exit arrives', async () => {
const session = makePersistedSplitSession()
const runtime = new OrcaRuntimeService(
runtimeStore({
getWorkspaceSession: () => session,
setWorkspaceSession: vi.fn(),
flushOrThrow: vi.fn()
})
)
runtime.attachWindow(1)
syncSplit(runtime)
runtime.registerPty('pty-left', WORKTREE_ID, null, {
tabId: 'tab',
leafId: 'left',
incarnationId: 'incarnation-left'
})
const stopRequestId = runtime.markPtyHistoryPreservingStopRequested('pty-left')
runtime.preservePtyHistoryThroughLateExit('pty-left', stopRequestId)
runtime.onPtyExit('pty-left', 0, 'incarnation-left')
expect((await runtime.listMobileSessionTabs(`id:${WORKTREE_ID}`)).tabs).toEqual([
expect.objectContaining({ id: 'tab::left' }),
expect.objectContaining({ id: 'tab::right' })
])
runtime.onPtyExit('pty-left', 0, 'incarnation-left')
expect(
(await runtime.listMobileSessionTabs(`id:${WORKTREE_ID}`)).tabs.find(
(tab) => tab.id === 'tab::left'
)
).toBeUndefined()
})
it('ignores a delayed exit from an older incarnation of a reused PTY id', async () => {
const setWorkspaceSession = vi.fn()
const runtime = new OrcaRuntimeService(
src/main/runtime/orca-runtime.ts
+56 -14
View File
@@ -3091,9 +3091,12 @@ export class OrcaRuntimeService {
// Why: provider exit can beat surface registration; that exact dead incarnation must never publish.
private earlyExitedPtyIncarnations = new Map<string, PtyIncarnationId | null>()
private pendingPtyRegistrationIncarnations = new Map<string, PtyIncarnationId | null>()
// Why: exact-stop is the current sleep transaction boundary; its exit must
// leave the renderer's intentional sleeping surface available for wake.
private intentionalHandlelessPtyStops = new Map<string, string | null>()
// Why: every overlapping hibernation owns its lease until its stop settles.
private intentionalHandlelessPtyStops = new Map<
string,
Map<number, { incarnationId: string | null; awaitsLateExit: boolean }>
>()
private nextIntentionalHandlelessPtyStopId = 1
// Why: coalesces title/status-driven session.tabs emits so spinner churn
// doesn't fan out (and per-client JSON.stringify) a snapshot several times a
// second. Emit reads the latest snapshot, so only the freshest version ships.
@@ -15217,10 +15220,27 @@ export class OrcaRuntimeService {
exitIncarnationId ?? pendingIncarnation ?? pty?.incarnationId ?? null
)
}
const intentionalStopIncarnation = this.intentionalHandlelessPtyStops.get(ptyId)
const intentionalStops = this.intentionalHandlelessPtyStops.get(ptyId)
const preservesIntentionalHandlelessSurface =
this.intentionalHandlelessPtyStops.has(ptyId) &&
(intentionalStopIncarnation === null || intentionalStopIncarnation === incarnationId)
intentionalStops !== undefined &&
[...intentionalStops.values()].some(
(intentionalStop) =>
intentionalStop.incarnationId === null || intentionalStop.incarnationId === incarnationId
)
if (intentionalStops) {
for (const [requestId, intentionalStop] of intentionalStops) {
if (
intentionalStop.awaitsLateExit &&
(intentionalStop.incarnationId === null ||
intentionalStop.incarnationId === incarnationId)
) {
intentionalStops.delete(requestId)
}
}
if (intentionalStops.size === 0) {
this.intentionalHandlelessPtyStops.delete(ptyId)
}
}
advertisedUrlWatcher.unbindPty(ptyId)
// Clean up new mobile state for this PTY
this.mobileSubscribers.delete(ptyId)
@@ -18211,12 +18231,32 @@ export class OrcaRuntimeService {
this.stopRequestedPtyIds.add(ptyId)
}
markPtyHistoryPreservingStopRequested(ptyId: string): void {
this.intentionalHandlelessPtyStops.set(ptyId, this.ptysById.get(ptyId)?.incarnationId ?? null)
markPtyHistoryPreservingStopRequested(ptyId: string): number {
const incarnationId = this.ptysById.get(ptyId)?.incarnationId ?? null
const requestId = this.nextIntentionalHandlelessPtyStopId
this.nextIntentionalHandlelessPtyStopId += 1
const requests = this.intentionalHandlelessPtyStops.get(ptyId) ?? new Map()
requests.set(requestId, { incarnationId, awaitsLateExit: false })
this.intentionalHandlelessPtyStops.set(ptyId, requests)
return requestId
}
clearPtyHistoryPreservingStopRequested(ptyId: string): void {
this.intentionalHandlelessPtyStops.delete(ptyId)
preservePtyHistoryThroughLateExit(ptyId: string, requestId: number): void {
const request = this.intentionalHandlelessPtyStops.get(ptyId)?.get(requestId)
if (request) {
request.awaitsLateExit = true
}
}
clearPtyHistoryPreservingStopRequested(ptyId: string, requestId: number): void {
const requests = this.intentionalHandlelessPtyStops.get(ptyId)
if (!requests) {
return
}
requests.delete(requestId)
if (requests.size === 0) {
this.intentionalHandlelessPtyStops.delete(ptyId)
}
}
isPtyStopRequested(ptyId: string): boolean {
@@ -30798,15 +30838,17 @@ export class OrcaRuntimeService {
const stoppedPtyIds: string[] = []
for (const ptyId of [...expected].sort()) {
if (opts.keepHistory) {
this.markPtyHistoryPreservingStopRequested(ptyId)
}
const historyPreservingStopId = opts.keepHistory
? this.markPtyHistoryPreservingStopRequested(ptyId)
: null
try {
if (!(await this.ptyController.stopAndWait(ptyId, { keepHistory: opts.keepHistory }))) {
throw Object.assign(new Error('terminal_exact_stop_failed'), { ptyId })
}
} finally {
this.clearPtyHistoryPreservingStopRequested(ptyId)
if (historyPreservingStopId !== null) {
this.clearPtyHistoryPreservingStopRequested(ptyId, historyPreservingStopId)
}
}
stoppedPtyIds.push(ptyId)
}
src/renderer/src/store/slices/store-sleep-pane-hibernation.test.ts
+82
View File
@@ -332,6 +332,88 @@ describe('shutdownWorktreeTerminals (sleep) — agent status hygiene', () => {
expect(state.sleepingAgentSessionsByPaneKey[targetPaneKey]).toBeDefined()
})
it('does not clear a replacement pane that binds while hibernation is stopping', async () => {
const store = createTestStore()
const wt = 'repo1::/path/wt1'
const targetLeaf = '11111111-1111-4111-8111-111111111111'
const targetPaneKey = `tab-1:${targetLeaf}`
let finishKill: (() => void) | undefined
mockApi.pty.kill.mockImplementationOnce(
() =>
new Promise<void>((resolve) => {
finishKill = resolve
})
)
seedStore(store, {
worktreesByRepo: {
repo1: [makeWorktree({ id: wt, repoId: 'repo1', path: '/path/wt1' })]
},
tabsByWorktree: {
[wt]: [makeTab({ id: 'tab-1', worktreeId: wt, title: 'Codex', ptyId: 'pty-agent' })]
},
terminalLayoutsByTabId: {
'tab-1': {
root: { type: 'leaf', leafId: targetLeaf },
activeLeafId: targetLeaf,
expandedLeafId: null,
ptyIdsByLeafId: { [targetLeaf]: 'pty-agent' }
}
},
ptyIdsByTabId: { 'tab-1': ['pty-agent'] }
})
store
.getState()
.setAgentStatus(
targetPaneKey,
{ state: 'done', prompt: 'old run', agentType: 'codex' },
'Codex',
{ updatedAt: 2000, stateStartedAt: 1000 },
{ tabId: 'tab-1', worktreeId: wt },
{ providerSession: { key: 'session_id', id: 'old-session' } }
)
const shutdown = store.getState().shutdownCompletedAgentPaneForHibernation(wt, {
paneKey: targetPaneKey,
tabId: 'tab-1',
leafId: targetLeaf,
ptyId: 'pty-agent'
})
store.setState((state) => ({
tabsByWorktree: {
...state.tabsByWorktree,
[wt]: state.tabsByWorktree[wt]!.map((tab) =>
tab.id === 'tab-1' ? { ...tab, ptyId: 'pty-replacement' } : tab
)
},
terminalLayoutsByTabId: {
...state.terminalLayoutsByTabId,
'tab-1': {
...state.terminalLayoutsByTabId['tab-1']!,
ptyIdsByLeafId: { [targetLeaf]: 'pty-replacement' }
}
},
ptyIdsByTabId: { ...state.ptyIdsByTabId, 'tab-1': ['pty-replacement'] }
}))
store
.getState()
.setAgentStatus(
targetPaneKey,
{ state: 'working', prompt: 'replacement run', agentType: 'codex' },
'Codex',
{ updatedAt: 3000, stateStartedAt: 3000 },
{ tabId: 'tab-1', worktreeId: wt },
{ providerSession: { key: 'session_id', id: 'replacement-session' } }
)
finishKill?.()
await shutdown
expect(store.getState().ptyIdsByTabId['tab-1']).toEqual(['pty-replacement'])
expect(store.getState().agentStatusByPaneKey[targetPaneKey]).toMatchObject({
state: 'working',
providerSession: { key: 'session_id', id: 'replacement-session' }
})
})
it('keeps manual sleep worktree-wide', async () => {
const store = createTestStore()
const wt = 'repo1::/path/wt1'
src/renderer/src/store/slices/terminals.ts
+15
View File
@@ -2771,6 +2771,21 @@ export const createTerminalSlice: StateCreator<AppState, [], [], TerminalSlice>
}
}
const stateAfterShutdown = get()
const tabAfterShutdown = (stateAfterShutdown.tabsByWorktree[worktreeId] ?? []).find(
(candidate) => candidate.id === opts.tabId
)
const panePtyAfterShutdown =
stateAfterShutdown.terminalLayoutsByTabId[opts.tabId]?.ptyIdsByLeafId?.[opts.leafId]
if (
!tabAfterShutdown ||
tabAfterShutdown.createdAt !== tab.createdAt ||
(panePtyAfterShutdown !== undefined && panePtyAfterShutdown !== opts.ptyId)
) {
// Why: the stopped process is gone, but a replacement pane owns all later store cleanup.
return
}
set((s) => {
const existingPtyIds = s.ptyIdsByTabId[opts.tabId] ?? []
const shutdownPtyIdSet = new Set(rendererShutdownPtyIds)