mirror of
https://github.com/stablyai/orca.git
synced 2026-09-29 08:03:20 +00:00
fix(terminal): classify a startup-carried provider session as a resume spawn
The watchdog's resume exclusion keyed on the cold-restore override alone, but spawnIpcPty falls back to the transport options for resumeProviderSession. The sidebar 'resume a sleeping agent' flow clears its sleeping record at tab creation, so the pane mounts into a plain startFreshSpawn() with no override while main still re-issues --resume. A hung spawn there was remounted into a second --resume on one transcript. Carry the exclusion on the spawn promise rather than the arming call, so a remount that adopts a pending spawn cannot lose it.
This commit is contained in:
+3
-2
@@ -172,8 +172,9 @@ export function runDeferredSessionReattachChoice(session: ConnectPanePtySession)
|
||||
}
|
||||
recordPtyConnectDiagnostic(`pane=${session.pane.id} -> PENDING SPAWN`)
|
||||
session.armDirectSshPaneRetryTimeout(pendingSpawn, session.directSshRetryAttempt)
|
||||
// Why re-arm: the adopting instance needs its own settlement clock, or a hung
|
||||
// spawn it inherited would freeze this pane with no timer of its own.
|
||||
// Why re-arm: a spawn whose arming pane was disposed before it could arm has
|
||||
// no clock at all, and this pane would inherit the freeze. Already-armed
|
||||
// spawns no-op, and the resume exclusion rides the promise, not this call.
|
||||
armSpawnSettlementWatchdog(session, pendingSpawn)
|
||||
void pendingSpawn
|
||||
.then((spawnedPtyId) => {
|
||||
|
||||
@@ -0,0 +1,84 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { bindStartFreshSpawn } from './fresh-spawn-start'
|
||||
import { observeSpawnSettlement } from './unbound-pane-spawn-recovery'
|
||||
import { pendingSpawnByPaneKey, pendingSpawnGenerationByPaneKey } from './pty-connect-limits'
|
||||
|
||||
vi.mock('./unbound-pane-spawn-recovery', () => ({ observeSpawnSettlement: vi.fn() }))
|
||||
vi.mock('@/lib/pane-manager/pane-terminal-output-scheduler', () => ({
|
||||
writeTerminalOutput: vi.fn()
|
||||
}))
|
||||
vi.mock('../pty-buffer-serializer', () => ({ hasPtySerializer: vi.fn(() => false) }))
|
||||
vi.mock('@/store', () => ({
|
||||
useAppStore: { getState: () => ({ deleteStateByWorktreeId: {}, getTab: () => null }) }
|
||||
}))
|
||||
|
||||
function buildSession(overrides: Record<string, unknown> = {}): never {
|
||||
return {
|
||||
deps: { tabId: 'tab-1', worktreeId: 'wt-1', cwd: '/w', isVisibleRef: { current: true } },
|
||||
pane: { id: 1, leafId: 'leaf-1', terminal: {} },
|
||||
// Non-null keeps the spawn off the serializer pre-signal, which is not under test.
|
||||
runtimeEnvironmentId: 'rt-1',
|
||||
transportOptions: {},
|
||||
disposed: false,
|
||||
pendingSpawnKey: 'pane-key',
|
||||
tabGeneration: 0,
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
authoritativeReattachGeneration: 0,
|
||||
transportStreamGeneration: 0,
|
||||
kittyKeyboardModes: { reset: vi.fn() },
|
||||
isLegacyWorkerAutomaticResumeBlocked: () => false,
|
||||
clearPaneMode2031State: vi.fn(),
|
||||
clearHiddenOutputRestoreState: vi.fn(),
|
||||
resetFreshSpawnFollowOutput: vi.fn(),
|
||||
prepareFreshShellViewportForSpawn: vi.fn(),
|
||||
shouldDeclareHiddenAtSpawn: () => false,
|
||||
captureTransportOutputCallbacks: () => ({ generation: 0, callbacks: {} }),
|
||||
armDirectSshPaneRetryTimeout: vi.fn(),
|
||||
reportError: vi.fn(),
|
||||
finishReattachLiveDataDeferral: vi.fn(),
|
||||
transport: { connect: vi.fn(() => new Promise(() => {})), getPtyId: () => null },
|
||||
...overrides
|
||||
} as never
|
||||
}
|
||||
|
||||
function resumesProviderSessionForSpawn(session: never): boolean {
|
||||
bindStartFreshSpawn(session)
|
||||
void (session as unknown as { startFreshSpawn: () => void }).startFreshSpawn()
|
||||
return vi.mocked(observeSpawnSettlement).mock.calls[0]?.[2]?.resumesProviderSession === true
|
||||
}
|
||||
|
||||
describe('bindStartFreshSpawn resume classification', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
pendingSpawnByPaneKey.clear()
|
||||
pendingSpawnGenerationByPaneKey.clear()
|
||||
})
|
||||
|
||||
// The sidebar "resume a sleeping agent" flow clears its sleeping record at tab
|
||||
// creation, so the pane mounts into a PLAIN startFreshSpawn() with no
|
||||
// cold-restore override — while spawnIpcPty still replays the provider session
|
||||
// off the transport options. Classifying that as non-resume lets a hung spawn
|
||||
// be remounted into a SECOND --resume on one transcript.
|
||||
it('treats a startup-carried provider session as a resume spawn', () => {
|
||||
const session = buildSession({
|
||||
transportOptions: { resumeProviderSession: { key: 'session_id', id: 'sess-1' } }
|
||||
})
|
||||
|
||||
expect(resumesProviderSessionForSpawn(session)).toBe(true)
|
||||
})
|
||||
|
||||
it('treats a plain spawn as a non-resume spawn', () => {
|
||||
expect(resumesProviderSessionForSpawn(buildSession())).toBe(false)
|
||||
})
|
||||
|
||||
it('treats an explicit cold-restore override as a resume spawn', () => {
|
||||
const session = buildSession()
|
||||
bindStartFreshSpawn(session)
|
||||
void (session as unknown as { startFreshSpawn: (startup: unknown) => void }).startFreshSpawn({
|
||||
launchConfig: { agent: 'claude' }
|
||||
})
|
||||
|
||||
expect(vi.mocked(observeSpawnSettlement).mock.calls[0]?.[2]?.resumesProviderSession).toBe(true)
|
||||
})
|
||||
})
|
||||
@@ -321,7 +321,13 @@ export function bindStartFreshSpawn(session: ConnectPanePtySession): void {
|
||||
})
|
||||
session.armDirectSshPaneRetryTimeout(trackedPromise, session.directSshRetryAttempt)
|
||||
observeSpawnSettlement(session, trackedPromise, {
|
||||
resumesProviderSession: Boolean(coldRestoreOverride)
|
||||
// Why the transport options too: spawnIpcPty falls back to them for
|
||||
// `resumeProviderSession`, so a pane whose startup carries a provider
|
||||
// session (sidebar resume of a sleeping agent) re-issues --resume on
|
||||
// EVERY fresh spawn — with no cold-restore override to mark it.
|
||||
resumesProviderSession: Boolean(
|
||||
coldRestoreOverride ?? session.transportOptions?.resumeProviderSession
|
||||
)
|
||||
})
|
||||
// Why: split panes in the same tab can spawn concurrently. Key by pane
|
||||
// as well as tab so a remount cannot attach to a sibling setup pane's PTY.
|
||||
|
||||
+16
@@ -261,6 +261,22 @@ describe('cold-restore resume spawns', () => {
|
||||
expect(pendingSpawnByPaneKey.has('resume-key')).toBe(false)
|
||||
})
|
||||
|
||||
// The adopting remount arms with no options of its own. If the exclusion rode
|
||||
// the arming call instead of the spawn, this pane would re-issue --resume.
|
||||
it('never remounts a resume spawn adopted by a remount whose arming pane was disposed', () => {
|
||||
const promise = new Promise<string | null>(() => {})
|
||||
observeSpawnSettlement(
|
||||
buildSession({ deps: { tabId: 'tab-disposed' }, disposed: true }),
|
||||
promise,
|
||||
{ resumesProviderSession: true }
|
||||
)
|
||||
|
||||
armSpawnSettlementWatchdog(buildSession({ deps: { tabId: 'tab-adopter' } }), promise)
|
||||
vi.advanceTimersByTime(TRANSPORT_CONNECT_SETTLE_GRACE_MS)
|
||||
|
||||
expect(requestTerminalPaneRecovery).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('still remounts a non-resume spawn that hangs', () => {
|
||||
observeSpawnSettlement(
|
||||
buildSession({ deps: { tabId: 'tab-plain' }, pendingSpawnKey: 'plain-key' }),
|
||||
|
||||
+14
-4
@@ -14,6 +14,11 @@ import type { ConnectPanePtySession } from './connect-pane-pty-session'
|
||||
// Why a module-level set: a promise is already a unique identity, so arming from
|
||||
// both the fresh spawn and a remount's pending-spawn adoption cannot double-time it.
|
||||
const spawnSettlementTimedPromises = new WeakSet<Promise<unknown>>()
|
||||
// Why keyed on the spawn and not on the arming call: a remount adopts a pending
|
||||
// spawn it did not start, so a flag passed by the caller would be lost exactly
|
||||
// when the arming pane was disposed before it could arm — the one case where the
|
||||
// adopter's own arming is what runs.
|
||||
const resumeShapedSpawns = new WeakSet<Promise<unknown>>()
|
||||
|
||||
function remountUnboundPane(
|
||||
session: ConnectPanePtySession,
|
||||
@@ -68,7 +73,12 @@ export function observeSpawnSettlement(
|
||||
trackedPromise: Promise<string | null>,
|
||||
options: { resumesProviderSession?: boolean } = {}
|
||||
): void {
|
||||
armSpawnSettlementWatchdog(session, trackedPromise, options)
|
||||
// Recorded before arming, so a pane disposed too early to arm still hands the
|
||||
// flag to whichever remount adopts this spawn.
|
||||
if (options.resumesProviderSession === true) {
|
||||
resumeShapedSpawns.add(trackedPromise)
|
||||
}
|
||||
armSpawnSettlementWatchdog(session, trackedPromise)
|
||||
void trackedPromise.then((spawnedPtyId) => {
|
||||
if (spawnedPtyId) {
|
||||
return
|
||||
@@ -98,8 +108,7 @@ export function observeSpawnSettlement(
|
||||
* pane-key entry forever, which also freezes any remount that adopts it. */
|
||||
export function armSpawnSettlementWatchdog(
|
||||
session: ConnectPanePtySession,
|
||||
trackedPromise: Promise<string | null>,
|
||||
options: { resumesProviderSession?: boolean } = {}
|
||||
trackedPromise: Promise<string | null>
|
||||
): void {
|
||||
if (session.disposed || spawnSettlementTimedPromises.has(trackedPromise)) {
|
||||
return
|
||||
@@ -121,10 +130,11 @@ export function armSpawnSettlementWatchdog(
|
||||
// (it may own a recycled id). Remounting a hung one would put a SECOND
|
||||
// --resume on the same transcript. A stuck pane is recoverable; two agents
|
||||
// writing one conversation is not.
|
||||
if (options.resumesProviderSession === true) {
|
||||
if (resumeShapedSpawns.has(trackedPromise)) {
|
||||
warnTerminalLifecycleAnomaly('resume spawn never settled; remount withheld', {
|
||||
tabId,
|
||||
worktreeId: session.deps.worktreeId,
|
||||
leafId: session.deps.restoredLeafId ?? session.pane.leafId,
|
||||
paneId: session.pane.id,
|
||||
ptyId: null
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user