Harden daemon degraded-spawn handling (follow-up to #6830) for #6814 (#6866)

* Harden merged #6830 daemon degraded-mode handling

- shutdownFallbackSessions is now best-effort: a single un-killable local PTY
  no longer throws and aborts the daemon restart (which would leave the user's
  recovery path unusable, recreating the original lockup). Logs and continues.
- checkPtySpawnHealth retries once (timeout raised 2s->4s) so a transient stall
  on a busy machine right after an upgrade does not mis-classify a healthy
  daemon as unable to spawn PTYs and silently drop new terminals to the local
  provider without daemon persistence.
- Surface degraded mode: DegradedDaemonPtyProvider exposes isDegraded, and
  pty:management:listSessions returns { degraded } so the session UI can warn
  instead of it being a silent console.warn. Clearer actionable warn message.

Refs #6814.

Co-authored-by: Orca <help@stably.ai>

* Add #6814 daemon failure-mode classification test

Drives the real DaemonServer + checkDaemonHealth over a real socket to lock in
the healthy / degraded(pty-spawn-unhealthy) / wedged(unreachable) / unreachable
classification. Documents the load-bearing boundary that the degraded-daemon
fallback rescues a degraded daemon but NOT a fully wedged one.

Co-authored-by: Orca <help@stably.ai>

* Add degraded flag to web preload listSessions stub (typecheck parity)

Co-authored-by: Orca <help@stably.ai>

---------

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Jinwoo Hong
2026-06-30 01:29:45 -07:00
committed by GitHub
co-authored by Orca
parent f769fc7052
commit 6cdd2ed946
10 changed files with 316 additions and 33 deletions
+1 -1
View File
@@ -240,7 +240,7 @@ function createOutOfProcessLauncher(runtimeDir: string): DaemonLauncher {
if (liveSessionCount !== null && liveSessionCount > 0) {
if (health === 'pty-spawn-unhealthy') {
console.warn(
`[daemon] Preserving daemon that failed the PTY spawn health check because it owns ${liveSessionCount} live session${liveSessionCount === 1 ? '' : 's'}; fresh terminals will use the local provider until the daemon is restarted`
`[daemon] DEGRADED MODE: preserving daemon that failed the PTY spawn health check because it owns ${liveSessionCount} live session${liveSessionCount === 1 ? '' : 's'}. Existing sessions keep working; fresh terminals run on the local provider WITHOUT daemon persistence until you restart the daemon (Manage Sessions → Restart).`
)
return createPreservedDaemonHandle(
runtimeDir,
@@ -205,18 +205,29 @@ describe('DegradedDaemonPtyProvider', () => {
expect(provider.hasPty(fresh.id)).toBe(false)
})
it('throws instead of counting fallback sessions that fail to shut down', async () => {
it('is best-effort: counts only successful shutdowns and never throws (keeps restart alive)', async () => {
const current = createDaemonAdapter('daemon')
const fallback = createProvider('fallback')
const provider = new DegradedDaemonPtyProvider({ current, legacy: [], fallback })
const fresh = await provider.spawn({ cols: 80, rows: 24 })
vi.mocked(fallback.shutdown).mockRejectedValueOnce(new Error('still alive'))
const stuck = await provider.spawn({ sessionId: 'stuck', cols: 80, rows: 24 })
await provider.spawn({ sessionId: 'ok', cols: 80, rows: 24 })
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
vi.mocked(fallback.shutdown).mockImplementation(async (id: string) => {
if (id === stuck.id) {
throw new Error('still alive')
}
})
await expect(provider.shutdownFallbackSessions()).rejects.toThrow(
'Failed to shut down 1 fallback PTY session(s)'
)
// Why: a single un-killable local PTY must not abort the daemon restart.
const killedCount = await provider.shutdownFallbackSessions()
expect(provider.hasPty(fresh.id)).toBe(true)
// Best-effort: the one that shut down is counted, the stuck one is not, and
// crucially it does not throw — so the daemon restart sequence proceeds.
expect(killedCount).toBe(1)
expect(warn).toHaveBeenCalled()
expect(fallback.shutdown).toHaveBeenCalledWith('stuck', { immediate: true })
expect(fallback.shutdown).toHaveBeenCalledWith('ok', { immediate: true })
warn.mockRestore()
})
it('fans synthetic exits for discovered current-daemon sessions only', async () => {
@@ -8,6 +8,10 @@ type ManagedPtyProvider = IPtyProvider & {
export class DegradedDaemonPtyProvider implements IPtyProvider {
readonly routesFreshSpawnsToLocalProvider = true
// Why: the preserved daemon answers protocol but cannot spawn fresh PTYs.
// Surfaced (e.g. via pty:management:listSessions) so the UI can warn that
// new terminals are running without daemon persistence until a restart.
readonly isDegraded = true
private current: DaemonPtyAdapter
private legacy: DaemonPtyAdapter[]
@@ -237,11 +241,19 @@ export class DegradedDaemonPtyProvider implements IPtyProvider {
this.sessionProviders.delete(id)
})
)
const failedCount = results.filter((result) => result.status === 'rejected').length
if (failedCount > 0) {
throw new Error(`Failed to shut down ${failedCount} fallback PTY session(s)`)
// Why: this runs first in the daemon-restart sequence. A throw here would
// abort the whole restart and leave "Restart daemon" — the user's recovery
// path for a wedged terminal — unusable, recreating the original lockup. So
// it is best-effort: log failures, keep restarting, and only count the
// sessions that actually shut down.
const failed = results.filter((result) => result.status === 'rejected')
if (failed.length > 0) {
console.warn(
`[daemon] ${failed.length} local fallback PTY session(s) failed to shut down during daemon restart; continuing restart`,
...failed.map((result) => (result as PromiseRejectedResult).reason)
)
}
return ids.length
return results.length - failed.length
}
getCurrentDaemonSessionIds(): string[] {
@@ -0,0 +1,121 @@
// Regression coverage for issue #6814 (terminal lockup after upgrade).
//
// Drives the real DaemonServer + checkDaemonHealth client over a real unix
// socket to lock in how each post-upgrade daemon failure mode is classified,
// and therefore which ones the degraded-daemon fallback actually rescues:
// - degraded (answers protocol, cannot spawn) -> pty-spawn-unhealthy -> rescued
// - wedged (event loop hung, health RPC never returns) -> unreachable -> NOT
// rescued by the degraded provider; falls to the preserve-or-replace path.
// This boundary is load-bearing: the degraded fallback only helps a daemon that
// still answers protocol, so a regression that widened it to wedged daemons
// would silently strand fresh terminals on a daemon that cannot serve them.
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { mkdtempSync, rmSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { basename, join } from 'node:path'
import { DaemonServer } from './daemon-server'
import { checkDaemonHealth } from './daemon-health'
import type { SubprocessHandle } from './session'
function createMockSubprocess(): SubprocessHandle {
return {
pid: 55555,
getForegroundProcess: () => null,
write() {},
resize() {},
kill() {},
forceKill() {},
signal() {},
onData() {},
onExit() {},
dispose() {}
}
}
function daemonTestSocketPath(dir: string): string {
return process.platform === 'win32'
? `\\\\.\\pipe\\${basename(dir)}-daemon.sock`
: join(dir, 'daemon.sock')
}
describe('issue #6814 repro: daemon failure-mode classification', () => {
let dir: string
let socketPath: string
let tokenPath: string
beforeEach(() => {
dir = mkdtempSync(join(tmpdir(), 'issue-6814-repro-'))
socketPath = daemonTestSocketPath(dir)
tokenPath = join(dir, 'daemon.token')
})
afterEach(() => {
rmSync(dir, { recursive: true, force: true })
})
// The good case: daemon answers hello AND the PTY spawn probe succeeds.
it('HEALTHY: a daemon that can spawn PTYs classifies as healthy', async () => {
const server = new DaemonServer({
socketPath,
tokenPath,
ptySpawnHealthCheck: vi.fn(async () => {}),
spawnSubprocess: () => createMockSubprocess()
})
await server.start()
try {
await expect(checkDaemonHealth(socketPath, tokenPath)).resolves.toBe('healthy')
} finally {
await server.shutdown()
}
})
// Symptom B, degraded: this is the case #6830 RESCUES. The daemon answers
// protocol but its PTY spawn probe throws (deleted cwd / stale native PTY
// after an upgrade), so fresh terminals would open frozen with no cursor.
it('DEGRADED: protocol-alive daemon that cannot spawn PTYs classifies as pty-spawn-unhealthy', async () => {
const server = new DaemonServer({
socketPath,
tokenPath,
ptySpawnHealthCheck: vi.fn(async () => {
throw new Error('chdir(2) failed.: No such file or directory')
}),
spawnSubprocess: () => createMockSubprocess()
})
await server.start()
try {
// -> #6830 marks this daemon degraded and routes fresh spawns to the
// local provider instead of the no-cursor daemon pane.
await expect(checkDaemonHealth(socketPath, tokenPath)).resolves.toBe('pty-spawn-unhealthy')
} finally {
await server.shutdown()
}
})
// The limit of #6830: a fully WEDGED daemon (event loop hung — health RPC
// never returns) cannot be distinguished by a richer status. It times out
// and classifies as 'unreachable', the SAME bucket as a dead daemon.
it('WEDGED: a daemon whose health RPC never resolves classifies as unreachable (NOT degraded)', async () => {
const server = new DaemonServer({
socketPath,
tokenPath,
// Why: simulate a hung event loop — the probe never settles.
ptySpawnHealthCheck: vi.fn(() => new Promise<void>(() => {})),
spawnSubprocess: () => createMockSubprocess()
})
await server.start()
try {
const health = await checkDaemonHealth(socketPath, tokenPath)
// This is the key finding: wedged != degraded. #6830's degraded fallback
// does NOT engage here; recovery still depends on the unreachable-path
// (preserve-if-live-else-replace) logic, not the degraded provider.
expect(health).toBe('unreachable')
} finally {
await server.shutdown()
}
}, 15000)
// No daemon at all (or token missing) -> unreachable.
it('UNREACHABLE: no daemon listening classifies as unreachable', async () => {
await expect(checkDaemonHealth(socketPath, tokenPath)).resolves.toBe('unreachable')
})
})
+58 -1
View File
@@ -59,7 +59,7 @@ vi.mock('../providers/agent-foreground-process', () => ({
resolveAgentForegroundProcess: resolveAgentForegroundProcessMock
}))
import { createPtySubprocess } from './pty-subprocess'
import { createPtySubprocess, checkPtySpawnHealth } from './pty-subprocess'
const ORCA_SHELL_WRAPPER_ENV = [
'ORCA_ATTRIBUTION_SHIM_DIR',
@@ -2191,3 +2191,60 @@ describe('createPtySubprocess', () => {
})
})
})
describe('checkPtySpawnHealth (retry on transient failure)', () => {
let previousUserDataPath: string | undefined
let userDataPath: string
beforeEach(() => {
spawnMock.mockReset()
previousUserDataPath = process.env.ORCA_USER_DATA_PATH
userDataPath = mkdtempSync(join(tmpdir(), 'daemon-pty-health-test-'))
process.env.ORCA_USER_DATA_PATH = userDataPath
})
afterEach(() => {
if (previousUserDataPath === undefined) {
delete process.env.ORCA_USER_DATA_PATH
} else {
process.env.ORCA_USER_DATA_PATH = previousUserDataPath
}
rmSync(userDataPath, { recursive: true, force: true })
})
// Why: a busy machine right after an upgrade can make one probe fail; the
// retry must keep a genuinely healthy daemon out of degraded mode. Windows
// short-circuits checkPtySpawnHealth, so this is a POSIX-only behavior.
itOnPosixHost('retries once and resolves when the first probe fails but the second succeeds', async () => {
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
spawnMock
.mockImplementationOnce(() => {
const proc = mockPtyProcess()
queueMicrotask(() => proc._simulateExit(1))
return proc
})
.mockImplementationOnce(() => {
const proc = mockPtyProcess()
queueMicrotask(() => proc._simulateExit(0))
return proc
})
await expect(checkPtySpawnHealth()).resolves.toBeUndefined()
expect(spawnMock).toHaveBeenCalledTimes(2)
expect(warn).toHaveBeenCalled()
warn.mockRestore()
})
itOnPosixHost('rejects after exhausting retries when every probe fails', async () => {
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
spawnMock.mockImplementation(() => {
const proc = mockPtyProcess()
queueMicrotask(() => proc._simulateExit(1))
return proc
})
await expect(checkPtySpawnHealth()).rejects.toThrow(/exited with code 1/)
expect(spawnMock).toHaveBeenCalledTimes(2)
warn.mockRestore()
})
})
+45 -16
View File
@@ -58,7 +58,12 @@ const PANE_IDENTITY_ENV_KEYS = [
const FOREGROUND_AGENT_CACHE_TTL_MS = 1000
const SHELL_FOREGROUND_REFRESH_RETRY_MS = 5_000
const STARTUP_AGENT_FOREGROUND_BOOTSTRAP_MS = 5_000
const PTY_SPAWN_HEALTH_TIMEOUT_MS = 2_000
const PTY_SPAWN_HEALTH_TIMEOUT_MS = 4_000
// Why: a busy machine right after an upgrade can make one short-lived shell
// spawn slow. Retry once before declaring the daemon unable to spawn PTYs, so
// a transient stall does not silently route every fresh terminal to the local
// fallback (losing daemon persistence) until a manual restart.
const PTY_SPAWN_HEALTH_RETRY_ATTEMPTS = 2
const PENDING_PRE_LISTENER_DATA_MAX_CHARS = 512 * 1024
export type PtySubprocessOptions = {
@@ -318,21 +323,9 @@ function formatPtySpawnError(err: unknown, shellPath: string, spawnCwd: string):
}
/**
* Runs a short native PTY spawn probe for daemon health checks.
* Runs one short native PTY spawn probe (spawn `/bin/sh -c 'exit 0'`).
*/
export async function checkPtySpawnHealth(): Promise<void> {
if (process.platform === 'win32') {
return
}
// Why: Linux/macOS daemons can outlive an app update with a deleted cwd or
// stale native PTY path. A real short-lived spawn catches that before the
// main process routes fresh panes to a daemon that cannot create terminals.
if (process.platform === 'darwin') {
ensureNodePtySpawnHelperExecutable()
}
preflightUnixPtySpawnEnvironment()
function runSinglePtySpawnHealthProbe(): Promise<void> {
const cwd = isExistingDirectory(process.env.ORCA_USER_DATA_PATH)
? process.env.ORCA_USER_DATA_PATH
: getDefaultCwd()
@@ -353,7 +346,7 @@ export async function checkPtySpawnHealth(): Promise<void> {
throw formatPtySpawnError(err, '/bin/sh', cwd)
}
await new Promise<void>((resolve, reject) => {
return new Promise<void>((resolve, reject) => {
let settled = false
let exitDisposable: { dispose(): void } | undefined
const finish = (error?: Error, opts?: { kill?: boolean }): void => {
@@ -394,6 +387,42 @@ export async function checkPtySpawnHealth(): Promise<void> {
})
}
/**
* Runs a short native PTY spawn probe for daemon health checks, retrying once
* so a transient stall (e.g. a busy machine right after an upgrade) does not
* mis-classify a healthy daemon as unable to spawn PTYs.
*/
export async function checkPtySpawnHealth(): Promise<void> {
if (process.platform === 'win32') {
return
}
// Why: Linux/macOS daemons can outlive an app update with a deleted cwd or
// stale native PTY path. A real short-lived spawn catches that before the
// main process routes fresh panes to a daemon that cannot create terminals.
if (process.platform === 'darwin') {
ensureNodePtySpawnHelperExecutable()
}
preflightUnixPtySpawnEnvironment()
let lastError: unknown
for (let attempt = 1; attempt <= PTY_SPAWN_HEALTH_RETRY_ATTEMPTS; attempt++) {
try {
await runSinglePtySpawnHealthProbe()
return
} catch (err) {
lastError = err
if (attempt < PTY_SPAWN_HEALTH_RETRY_ATTEMPTS) {
console.warn(
`[daemon] PTY spawn health probe attempt ${attempt} failed; retrying`,
err instanceof Error ? err.message : err
)
}
}
}
throw lastError instanceof Error ? lastError : new Error(String(lastError))
}
/**
* Normalizes node-pty foreground process strings to executable basenames.
*/
+44
View File
@@ -43,6 +43,23 @@ vi.mock('../daemon/daemon-pty-router', () => {
return { DaemonPtyRouter }
})
// Why: the handler also branches on `provider instanceof DegradedDaemonPtyProvider`
// (for getAllAdapters) and reports `degraded` from it. The real constructor
// subscribes to adapter events, so keep only the accessors pty-management uses.
vi.mock('../daemon/degraded-daemon-pty-provider', () => {
class DegradedDaemonPtyProvider {
readonly isDegraded = true
private allAdapters: unknown[]
constructor(opts: { current: unknown; legacy: unknown[] }) {
this.allAdapters = [opts.current, ...opts.legacy]
}
getAllAdapters() {
return this.allAdapters
}
}
return { DegradedDaemonPtyProvider }
})
type HandlerMap = Record<string, (event: unknown, args?: unknown) => unknown>
function buildHandlerMap(): HandlerMap {
@@ -107,6 +124,15 @@ async function makeRouter(current: MockAdapter, legacy: MockAdapter[] = []) {
return new DaemonPtyRouter({ current: current as never, legacy: legacy as never })
}
async function makeDegradedProvider(current: MockAdapter, legacy: MockAdapter[] = []) {
const { DegradedDaemonPtyProvider } = await import('../daemon/degraded-daemon-pty-provider')
return new DegradedDaemonPtyProvider({
current: current as never,
legacy: legacy as never,
fallback: undefined as never
})
}
describe('pty:management IPC handlers', () => {
beforeEach(() => {
getDaemonProviderMock.mockReset()
@@ -128,15 +154,33 @@ describe('pty:management IPC handlers', () => {
const handlers = buildHandlerMap()
const result = (await handlers['pty:management:listSessions']({})) as {
sessions: DaemonSessionInfo[]
degraded: boolean
}
expect(result.sessions).toHaveLength(3)
expect(result.degraded).toBe(false)
const byId = new Map(result.sessions.map((s) => [s.sessionId, s]))
expect(byId.get('new-1')?.protocolVersion).toBe(5)
expect(byId.get('new-2')?.protocolVersion).toBe(5)
expect(byId.get('old-1')?.protocolVersion).toBe(3)
})
it('reports degraded mode and still lists sessions when the daemon cannot spawn fresh PTYs', async () => {
const current = makeAdapter(5, [makeSession('preserved-1')])
const { registerDaemonManagementHandlers } = await importFresh()
getDaemonProviderMock.mockReturnValue(await makeDegradedProvider(current))
registerDaemonManagementHandlers()
const handlers = buildHandlerMap()
const result = (await handlers['pty:management:listSessions']({})) as {
sessions: DaemonSessionInfo[]
degraded: boolean
}
expect(result.degraded).toBe(true)
expect(result.sessions.map((s) => s.sessionId)).toEqual(['preserved-1'])
})
it('returns empty list when no daemon provider is installed', async () => {
getDaemonProviderMock.mockReturnValue(null)
+9 -2
View File
@@ -32,6 +32,13 @@ function getDaemonAdapters(): DaemonPtyAdapter[] {
return [provider]
}
// Why: surface degraded mode (daemon alive but cannot spawn fresh PTYs) so the
// session-management UI can warn that new terminals lack daemon persistence
// until the daemon is restarted, instead of it being a silent console.warn.
function isDaemonDegraded(): boolean {
return getDaemonProvider() instanceof DegradedDaemonPtyProvider
}
async function collectSessions(adapters: DaemonPtyAdapter[]): Promise<DaemonSessionInfo[]> {
const results = await Promise.allSettled(
adapters.map(async (adapter) => {
@@ -53,9 +60,9 @@ export function registerDaemonManagementHandlers(): void {
ipcMain.handle(
'pty:management:listSessions',
async (): Promise<{ sessions: DaemonSessionInfo[] }> => {
async (): Promise<{ sessions: DaemonSessionInfo[]; degraded: boolean }> => {
const sessions = await collectSessions(getDaemonAdapters())
return { sessions }
return { sessions, degraded: isDaemonDegraded() }
}
)
+3 -1
View File
@@ -602,7 +602,9 @@ export type PtyManagementSession = {
}
export type PtyManagementApi = {
listSessions: () => Promise<{ sessions: PtyManagementSession[] }>
// `degraded` is true when the daemon is alive but cannot spawn fresh PTYs, so
// new terminals run on the local provider without daemon persistence.
listSessions: () => Promise<{ sessions: PtyManagementSession[]; degraded: boolean }>
killAll: () => Promise<{ killedCount: number; remainingCount: number }>
killOne: (args: { sessionId: string }) => Promise<{ success: boolean }>
restart: () => Promise<{ success: boolean }>
+1 -1
View File
@@ -2573,7 +2573,7 @@ function createPtyApi(): NonNullable<Partial<PreloadApi>['pty']> {
settlePaneSerializer: () => Promise.resolve(),
clearPendingPaneSerializer: () => Promise.resolve(),
management: {
listSessions: () => Promise.resolve({ sessions: [] }),
listSessions: () => Promise.resolve({ sessions: [], degraded: false }),
killAll: () => Promise.resolve({ killedCount: 0, remainingCount: 0 }),
killOne: () => Promise.resolve({ success: false }),
restart: () => Promise.resolve({ success: false })