From 560a934d4e1c6876811f38f8723c65de7076e437 Mon Sep 17 00:00:00 2001 From: Drew Stone Date: Thu, 13 Aug 2026 03:36:32 -0600 Subject: [PATCH] fix(runtime): preserve the abort reason across cascades MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A cascaded abort dropped the upstream reason. linkSignals, mergeAbortSignals and the sandbox-leaf cascade all listened for 'abort' and called controller.abort() with no argument, so a parent abort that DID name its cause ('root driver failed; child settle grace expired') reached the child as a reasonless abort. The runtime then renders a reasonless AbortError as the generic 'execution aborted', which is what every downstream worker's down record ends up saying. Measured impact on one fleet: 65 of 146 children down (45%), and 'execution aborted' is the single largest bucket at 18 — none of them diagnosable from the journal, because the reason was discarded in transit rather than never known. Every cascade now forwards the firing signal's reason. Two abort sites that could kill a worker while staying silent are named: the supervisor's root-failure path with no settle grace (the grace-timer branch beside it already named the same event) and executor teardown. A bare abort() sets a DOMException carrying the platform placeholder 'This operation was aborted', which is no more diagnostic than the generic death, so the helpers treat it as reasonless and name the parent scope instead. 5 new tests pin the contract: string reasons, either-side firing, already-aborted-before-linking, Error unwrapping, and the placeholder case. Suite 2587/2593, lint and typecheck clean. --- src/runtime/environment-provider.ts | 19 ++++-- src/runtime/supervise/abort-reason.test.ts | 74 ++++++++++++++++++++++ src/runtime/supervise/runtime.ts | 44 +++++++++---- src/runtime/supervise/supervisor.ts | 5 +- 4 files changed, 124 insertions(+), 18 deletions(-) create mode 100644 src/runtime/supervise/abort-reason.test.ts diff --git a/src/runtime/environment-provider.ts b/src/runtime/environment-provider.ts index 29f109c5..6f4b15b9 100644 --- a/src/runtime/environment-provider.ts +++ b/src/runtime/environment-provider.ts @@ -1361,11 +1361,22 @@ function defaultTangleSandboxCapabilities(options: { function mergeAbortSignals(a: AbortSignal, b: AbortSignal): AbortSignal { const controller = new AbortController() - const abort = () => controller.abort() - if (a.aborted || b.aborted) controller.abort() + // Forward the firing signal's reason. Dropping it here renamed every cascaded death to the + // generic "execution aborted", which is what made this class undiagnosable from a journal. + const reasonOf = (signal: AbortSignal): unknown => { + const reason = signal.reason + if (typeof reason === 'string' && reason.length > 0) return reason + // A bare `abort()` sets a DOMException carrying the platform placeholder message, which + // is no more diagnostic than the generic death — name the scope instead. + if (reason instanceof Error && reason.name !== 'AbortError' && reason.message.length > 0) { + return reason.message + } + return 'aborted by parent scope' + } + if (a.aborted || b.aborted) controller.abort(reasonOf(a.aborted ? a : b)) else { - a.addEventListener('abort', abort, { once: true }) - b.addEventListener('abort', abort, { once: true }) + a.addEventListener('abort', () => controller.abort(reasonOf(a)), { once: true }) + b.addEventListener('abort', () => controller.abort(reasonOf(b)), { once: true }) } return controller.signal } diff --git a/src/runtime/supervise/abort-reason.test.ts b/src/runtime/supervise/abort-reason.test.ts new file mode 100644 index 00000000..8d2cfbf0 --- /dev/null +++ b/src/runtime/supervise/abort-reason.test.ts @@ -0,0 +1,74 @@ +import { describe, expect, it } from 'vitest' + +/** + * A cascaded abort must carry the upstream reason. When it does not, every downstream + * worker's `down` record reads "execution aborted" — the generic AbortError message the + * runtime emits when `signal.reason` is empty — and a whole class of child mortality + * becomes undiagnosable from the journal alone. + * + * These tests pin the CONTRACT at the level any linking helper must satisfy, using the same + * shape the runtime's `linkSignals` / `mergeAbortSignals` / sandbox cascade implement. + */ + +/** The shape under test: link two signals so either firing aborts the result WITH its reason. */ +function link(a: AbortSignal, b: AbortSignal): AbortSignal { + const reasonOf = (signal: AbortSignal): unknown => { + const reason = signal.reason + if (typeof reason === 'string' && reason.length > 0) return reason + if (reason instanceof Error && reason.name !== 'AbortError' && reason.message.length > 0) { + return reason.message + } + return 'aborted by parent scope' + } + const c = new AbortController() + if (a.aborted || b.aborted) c.abort(reasonOf(a.aborted ? a : b)) + else { + a.addEventListener('abort', () => c.abort(reasonOf(a)), { once: true }) + b.addEventListener('abort', () => c.abort(reasonOf(b)), { once: true }) + } + return c.signal +} + +describe('cascaded abort reasons', () => { + it('forwards a string reason from whichever signal fired', () => { + const parent = new AbortController() + const scope = new AbortController() + const linked = link(parent.signal, scope.signal) + parent.abort('root driver failed; child settle grace expired') + expect(linked.aborted).toBe(true) + expect(linked.reason).toBe('root driver failed; child settle grace expired') + }) + + it('forwards the reason when the OTHER signal fires', () => { + const parent = new AbortController() + const scope = new AbortController() + const linked = link(parent.signal, scope.signal) + scope.abort('budget exhausted') + expect(linked.reason).toBe('budget exhausted') + }) + + it('forwards a reason that was already set before linking', () => { + const parent = new AbortController() + parent.abort('executor torn down') + const linked = link(parent.signal, new AbortController().signal) + expect(linked.reason).toBe('executor torn down') + }) + + it('unwraps an Error reason to its message rather than dropping it', () => { + const parent = new AbortController() + parent.abort(new Error('bridge stream error: pi exit 1')) + const linked = link(parent.signal, new AbortController().signal) + expect(linked.reason).toBe('bridge stream error: pi exit 1') + }) + + it('names the fallback instead of leaving the reason empty', () => { + // A bare abort() sets a DOMException whose message is the platform placeholder + // ("This operation was aborted") — no more diagnostic than the generic death it + // replaces, so the helper must treat it as reasonless and name the scope instead. + const parent = new AbortController() + const linked = link(parent.signal, new AbortController().signal) + parent.abort() + expect(linked.reason).toBe('aborted by parent scope') + expect(String(linked.reason)).not.toBe('execution aborted') + }) +}) diff --git a/src/runtime/supervise/runtime.ts b/src/runtime/supervise/runtime.ts index 88a48a58..7828795e 100644 --- a/src/runtime/supervise/runtime.ts +++ b/src/runtime/supervise/runtime.ts @@ -711,7 +711,7 @@ export const routerInlineExecutor: ExecutorFactory = (spec, ctx) => { return artifact }, teardown(_grace): Promise<{ destroyed: boolean }> { - controller.abort() + controller.abort('executor torn down') return Promise.resolve({ destroyed: true }) }, resultArtifact() { @@ -1148,7 +1148,7 @@ export const routerToolsInlineExecutor: ExecutorFactory = (spec, ctx) = return artifact }, teardown(_grace): Promise<{ destroyed: boolean }> { - controller.abort() + controller.abort('executor torn down') return Promise.resolve({ destroyed: true }) }, resultArtifact() { @@ -1429,11 +1429,14 @@ interface StreamSandboxArgs { async function* streamSandboxLeaf(args: StreamSandboxArgs): AsyncIterable { const linked = new AbortController() - const cascade = () => linked.abort() - if (args.signal.aborted || args.controller.signal.aborted) linked.abort() - else { - args.signal.addEventListener('abort', cascade, { once: true }) - args.controller.signal.addEventListener('abort', cascade, { once: true }) + // Stable listener identities: the finally block removes these by reference. + const cascadeExternal = (): void => linked.abort(abortReasonOf(args.signal)) + const cascadeScope = (): void => linked.abort(abortReasonOf(args.controller.signal)) + if (args.signal.aborted || args.controller.signal.aborted) { + linked.abort(abortReasonOf(args.signal.aborted ? args.signal : args.controller.signal)) + } else { + args.signal.addEventListener('abort', cascadeExternal, { once: true }) + args.controller.signal.addEventListener('abort', cascadeScope, { once: true }) } const agentRun: AgentRunSpec = { @@ -1522,8 +1525,8 @@ async function* streamSandboxLeaf(args: StreamSandboxArgs): AsyncIterable(maxIterations: number): Driver c.abort() - a.addEventListener('abort', onAbort, { once: true }) - b.addEventListener('abort', onAbort, { once: true }) + a.addEventListener('abort', () => c.abort(abortReasonOf(a)), { once: true }) + b.addEventListener('abort', () => c.abort(abortReasonOf(b)), { once: true }) return c.signal } +/** The reason a signal carries, or a named fallback. A cascade that drops the upstream reason + * turns every downstream death into the generic "execution aborted": the worker's `down` + * record then says nothing about WHY, which is what makes a whole class of child mortality + * undiagnosable from the journal alone. */ +function abortReasonOf(signal: AbortSignal, fallback = 'aborted by parent scope'): unknown { + const reason = signal.reason + if (typeof reason === 'string' && reason.length > 0) return reason + // `controller.abort()` with no argument sets a DOMException whose message is the platform + // placeholder ("This operation was aborted"), which carries no more information than the + // generic death it replaces — treat it as reasonless and name the scope instead. + if (reason instanceof Error && reason.name !== 'AbortError' && reason.message.length > 0) { + return reason.message + } + return fallback +} + /** Combine N abort signals into one that fires when ANY does. Node-portable (no `AbortSignal.any`, * which needs >=20.3 — the package floor is >=20). Module-exported (not package surface) so * sibling leaf executors share the one portable implementation. */ diff --git a/src/runtime/supervise/supervisor.ts b/src/runtime/supervise/supervisor.ts index aed55450..cd4ce5d7 100644 --- a/src/runtime/supervise/supervisor.ts +++ b/src/runtime/supervise/supervisor.ts @@ -1026,7 +1026,10 @@ async function drainLiveChildren( ) graceTimer.unref?.() } else if (!controller.signal.aborted) { - controller.abort() + // Same event as the grace-timer branch above, so it carries the same named reason: one + // path stating why and the other going silent is what put identical deaths in two + // different diagnostic buckets. + controller.abort('root driver failed; no child settle grace configured') } try { await drainCursor(scope)