fix(runtime): stop a throwing client-event listener from wedging worktree sleep + harden fan-out (#10052)

This commit is contained in:
OrcaWin 2026-07-22 21:54:33 -07:00 committed by GitHub
parent 8cb0b8dcde
commit 8685cdb3fb
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 108 additions and 19 deletions

View File

@ -27951,6 +27951,77 @@ describe('OrcaRuntimeService', () => {
}
})
it('releases the worktree terminal mutation when a wake client-event listener throws', async () => {
const runtime = new OrcaRuntimeService(store)
const secondListenerEvents: RuntimeClientEvent[] = []
// Why: a broken paired-client relay can throw synchronously while delivering the wake
// notification. That must not abort the wake or (regression) leak the per-worktree terminal
// mutation acquired in acquireWorktreeTerminalSpawn, or every later sleep wedges for 12s.
runtime.onClientEvent((event) => {
if (event.type === 'worktreeTerminalSleepState' && event.phase === 'woken') {
throw new Error('relay_send_failed')
}
})
runtime.onClientEvent((event) => secondListenerEvents.push(event))
const processLists = [[{ id: 'pty-1', cwd: TEST_WORKTREE_PATH, title: 'Claude' }], [], []]
runtime.setPtyController({
write: () => true,
kill: () => false,
stopAndWait: async (ptyId) => {
runtime.onPtyExit(ptyId, -1)
return true
},
getForegroundProcess: async () => null,
listProcesses: async () => processLists.shift() ?? []
})
// Sleep leaves the worktree in a 'sleeping' state so the next spawn emits the 'woken' event.
await runtime.sleepTerminalsForWorktree(`id:${TEST_WORKTREE_ID}`)
// The wake acquires the mutation and emits 'woken'; a throwing subscriber must not surface.
const releaseSpawn = await runtime.acquireWorktreeTerminalSpawn(TEST_WORKTREE_ID)
releaseSpawn()
// Isolation: the second subscriber still received the 'woken' event.
expect(
secondListenerEvents.some(
(event) => event.type === 'worktreeTerminalSleepState' && event.phase === 'woken'
)
).toBe(true)
// Regression: the mutation was released, so a subsequent sleep converges instead of throwing
// terminal_worktree_sleep_timeout.
await expect(
runtime.sleepTerminalsForWorktree(`id:${TEST_WORKTREE_ID}`)
).resolves.toMatchObject({ postStopVerified: true })
})
it('isolates a throwing subscriber across runtime listener fan-out', () => {
const runtime = new OrcaRuntimeService(store)
const delivered: number[] = []
// Why: the shared notifyRuntimeListeners guard must let sibling fan-outs (here mobile
// notifications) survive a throwing subscriber, not just the client-event path.
runtime.onNotificationDispatched(() => {
throw new Error('subscriber_send_failed')
})
runtime.onNotificationDispatched((event) => {
delivered.push(event.notificationSeq ?? -1)
})
expect(() =>
runtime.dispatchMobileNotification({
type: 'notification',
source: 'test',
title: 'Test',
body: 'Body',
worktreeId: TEST_WORKTREE_ID
})
).not.toThrow()
// The second subscriber still received the event despite the first throwing.
expect(delivered).toHaveLength(1)
})
it('keeps the original committed disposition across an idempotent retry', async () => {
const runtime = new OrcaRuntimeService(store)
const events: RuntimeClientEvent[] = []

View File

@ -3534,9 +3534,9 @@ export class OrcaRuntimeService {
}
private emitClientEvent(event: RuntimeClientEvent): void {
for (const listener of this.clientEventListeners) {
listener(event)
}
// Why: a throwing subscriber here once escaped acquireWorktreeTerminalSpawn after it took the
// per-worktree terminal mutation, leaking it and wedging that worktree's sleep until restart.
notifyRuntimeListeners(this.clientEventListeners, (listener) => listener(event), 'client-event')
}
private notifyWorktreesChanged(repoId: string): void {
@ -7600,7 +7600,13 @@ export class OrcaRuntimeService {
...(cwdChanged && cwd !== null ? { cwd } : {})
}
for (const listener of listeners) {
listener(data, meta)
try {
listener(data, meta)
} catch (error) {
// Why: inlined rather than via notifyRuntimeListeners to avoid a per-chunk closure
// allocation on the terminal-output hot path; isolation semantics match the helper.
console.error('[runtime] pty-data listener threw', error)
}
}
}
return outputSequence
@ -8583,9 +8589,7 @@ export class OrcaRuntimeService {
if (!listeners) {
return
}
for (const listener of listeners) {
listener({ mode, cols, rows })
}
notifyRuntimeListeners(listeners, (listener) => listener({ mode, cols, rows }), 'fit-override')
}
serializeTerminalBuffer(
@ -9860,12 +9864,13 @@ export class OrcaRuntimeService {
dispatchMobileNotification(event: MobileNotificationEvent): void {
const seq = this.mobileNotificationReplay.record(event)
for (const listener of this.notificationListeners) {
// Why: surface the desktop-assigned seq to live listeners so they can
// watermark the last event delivered and feed it back to getMissedSince
// on reconnect (idempotent catch-up, no duplicate local pushes).
listener({ ...event, notificationSeq: seq })
}
// Why: surface the desktop-assigned seq to live listeners so they can watermark the last event
// delivered and feed it back to getMissedSince on reconnect (idempotent catch-up, no dupes).
notifyRuntimeListeners(
this.notificationListeners,
(listener) => listener({ ...event, notificationSeq: seq }),
'mobile-notification'
)
}
// Returns notifications dispatched after lastSeenSeq. Idempotent: the same
@ -10791,9 +10796,7 @@ export class OrcaRuntimeService {
this.notifier?.terminalDriverChanged(ptyId, next)
const listeners = this.driverListeners.get(ptyId)
if (listeners) {
for (const listener of listeners) {
listener(next)
}
notifyRuntimeListeners(listeners, (listener) => listener(next), 'pty-driver')
}
}
@ -12366,9 +12369,7 @@ export class OrcaRuntimeService {
if (!listeners) {
return
}
for (const listener of listeners) {
listener(event)
}
notifyRuntimeListeners(listeners, (listener) => listener(event), 'pty-resize')
}
// Why: Section 7.2 — the runtime detects agent exit directly and updates
@ -29693,6 +29694,23 @@ async function waitForWorktreeTerminalMutation(
}
}
}
// Why: listener fan-out is best-effort delivery. One subscriber throwing synchronously — e.g. a
// paired-client relay whose stream is closed — must never abort the emitting operation or leak
// state (a lock/mutation) the caller holds across the emit. Isolate every listener and log.
function notifyRuntimeListeners<L>(
listeners: Iterable<L>,
deliver: (listener: L) => void,
context: string
): void {
for (const listener of listeners) {
try {
deliver(listener)
} catch (error) {
console.error(`[runtime] ${context} listener threw`, error)
}
}
}
// Why (§3.3): 30s freshness window reuses a recent fetch for repeat create/dispatch on the same repo+remote; short enough a changed remote is seen next action.
const FETCH_FRESHNESS_MS = 30_000
// Why: bound fetches so a Windows credential-manager GUI hang (STA-1292) can't wedge worktree creation; parity with the exact-base refresh sibling.