fix(scope): harden final ownership boundaries
This commit is contained in:
@@ -35,7 +35,7 @@ Values LEAVING the script (hook options/schemas, the script's return) are materi
|
||||
|
||||
## Cancellation, death, disposal
|
||||
|
||||
Per-run limits: a concurrency semaphore (`maxConcurrentAgents`), a total-`agent()` cap (`maxTotalAgents`), and a per-call item cap (`maxItemsPerCall`), all config. `cancel()` posts the cancel to the worker (its hooks start throwing `CANCELLED`; the script dies at its next await) and cancels every host-side child NOW on **both seam channels** — the shared request signal aborts AND each registered child's explicit `cancel()` is called host-side, because the seam leaves a provider free to honor either channel and a worker wedged in a synchronous spin could not relay its own per-child cancel RPCs (those later land as idempotent no-ops). Each provider-owned cancel callback is exception-contained independently, so a broken child cannot prevent peer cancellation or workflow settlement. The grace then arms: a run still unsettled `disposeGraceMs` later force-settles `cancelled` and the worker is **terminated**. A cancellation that lands before the body runs (the ready→go handshake) reports `cancelled` without executing anything; a worker `result` racing an in-flight host cancellation reports `cancelled` too (first-wins settlement — the seam-visible result had not settled when cancellation was requested); post-cancel `phase`/`log` narration is suppressed host-side, while cancelled children still deliver their paired `agent-end`.
|
||||
Per-run limits: a concurrency semaphore (`maxConcurrentAgents`), a total-`agent()` cap (`maxTotalAgents`), and a per-call item cap (`maxItemsPerCall`), all config. `cancel()` posts the cancel to the worker (its hooks start throwing `CANCELLED`; the script dies at its next await) and cancels every host-side child NOW on **both seam channels** — the shared request signal aborts AND each registered child's explicit `cancel()` is called host-side, because the seam leaves a provider free to honor either channel and a worker wedged in a synchronous spin could not relay its own per-child cancel RPCs (those later land as idempotent no-ops). Each provider-owned cancel callback is exception-contained independently, so a broken child cannot prevent peer cancellation or workflow settlement. The caller's optional start-signal callback is retained by exact identity only while the run is live and removed at the first settlement or teardown, so a long-lived signal cannot retain completed `WorkerRun` instances. The grace then arms: a run still unsettled `disposeGraceMs` later force-settles `cancelled` and the worker is **terminated**. A cancellation that lands before the body runs (the ready→go handshake) reports `cancelled` without executing anything; a worker `result` racing an in-flight host cancellation reports `cancelled` too (first-wins settlement — the seam-visible result had not settled when cancellation was requested); post-cancel `phase`/`log` narration is suppressed host-side, while cancelled children still deliver their paired `agent-end`.
|
||||
|
||||
A worker that dies unexpectedly (an OOM, a script reaching `process.exit` through the documented vm escape) settles the run `stopReason: 'error'` with the exit diagnostics — or `'cancelled'` when a cancel was in flight — and the host-side child registry is what winds every surviving child down. `dispose()` = cancel + immediate host-driven disposal of every registered child (a wedged worker can relay no dispose RPC, so child teardown overlaps the grace instead of starting after it; the worker's own dispose RPCs join the same per-child disposal) + bounded wait (result, then child-registry quiescence, capped by the grace) + unconditional `worker.terminate()`: the thread never outlives its run. Before an ordinary run settlement becomes observable, the host cancels every stray child on both channels too—even a fire-and-forget run still waiting on `started`, for which the worker has no handle yet—and `dispose()` then waits for their disposal (bounded by the grace) before returning. `agent-start`/`agent-end` pairing is host-guaranteed the same way: forwarded starts live in a ledger, worker-reported ends pair them on the graceful paths, and the termination paths (grace force-settle, worker death) synthesize the missing ends (outcome `cancelled`) before the run settles — a start still in flight across the force-settle can surface after `workflow/end`, immediately paired the same way.
|
||||
|
||||
|
||||
@@ -130,6 +130,9 @@ export class WorkerRun implements WorkflowRun {
|
||||
private readonly quiescenceWaiters: (() => void)[] = []
|
||||
/** The per-run abort fanout every child start request carries. */
|
||||
private readonly controller = new AbortController()
|
||||
/** External start signal and the exact callback installed on it, retained only until first settle/teardown. */
|
||||
private inputSignal: AbortSignal | undefined
|
||||
private inputSignalAbort: (() => void) | undefined
|
||||
private disposed: Promise<void> | undefined
|
||||
|
||||
constructor(
|
||||
@@ -159,8 +162,14 @@ export class WorkerRun implements WorkflowRun {
|
||||
})
|
||||
if (signal?.aborted) {
|
||||
this.cancel('workflow start signal already aborted')
|
||||
} else {
|
||||
signal?.addEventListener('abort', () => { this.cancel('workflow signal aborted') }, { once: true })
|
||||
} else if (signal !== undefined) {
|
||||
const onAbort = (): void => {
|
||||
this.detachInputSignal()
|
||||
this.cancel('workflow signal aborted')
|
||||
}
|
||||
this.inputSignal = signal
|
||||
this.inputSignalAbort = onAbort
|
||||
signal.addEventListener('abort', onAbort, { once: true })
|
||||
}
|
||||
}
|
||||
|
||||
@@ -217,6 +226,7 @@ export class WorkerRun implements WorkflowRun {
|
||||
*/
|
||||
dispose(): Promise<void> {
|
||||
this.disposed ??= (async () => {
|
||||
this.detachInputSignal()
|
||||
this.cancel('workflow disposed')
|
||||
for (const [callId, run] of [...this.children]) void this.disposeChild(callId, run)
|
||||
await Promise.race([
|
||||
@@ -522,10 +532,21 @@ export class WorkerRun implements WorkflowRun {
|
||||
return { value: null, stopReason: 'cancelled', error: `workflow run cancelled: ${reason}`, agentsStarted }
|
||||
}
|
||||
|
||||
/** First settle wins; disarms the grace timer. */
|
||||
/** Remove the exact abort callback installed on the caller's start signal. */
|
||||
private detachInputSignal(): void {
|
||||
const signal = this.inputSignal
|
||||
const onAbort = this.inputSignalAbort
|
||||
if (signal === undefined || onAbort === undefined) return
|
||||
this.inputSignal = undefined
|
||||
this.inputSignalAbort = undefined
|
||||
signal.removeEventListener('abort', onAbort)
|
||||
}
|
||||
|
||||
/** First settle wins; disarms the grace timer and releases the caller signal. */
|
||||
private settleResult(result: WorkflowResult): void {
|
||||
if (this.settled) return
|
||||
this.settled = true
|
||||
this.detachInputSignal()
|
||||
clearTimeout(this.graceTimer)
|
||||
this.settleResolve(result)
|
||||
}
|
||||
|
||||
@@ -585,6 +585,41 @@ describe('dsh-workflow-workerthread', () => {
|
||||
await second.dispose()
|
||||
})
|
||||
|
||||
it('removes the exact external abort callback on first settlement or teardown', async () => {
|
||||
const { ctx, parent } = await setup()
|
||||
const settledController = new AbortController()
|
||||
const settledAdd = vi.spyOn(settledController.signal, 'addEventListener')
|
||||
const settledRemove = vi.spyOn(settledController.signal, 'removeEventListener')
|
||||
const completed = ctx.workflows.start({ ...scripted('return 123'), parent, signal: settledController.signal })
|
||||
const settledAbort = settledAdd.mock.calls.find(([type]) => type === 'abort')?.[1]
|
||||
expect(typeof settledAbort).toBe('function')
|
||||
|
||||
await expect(completed.result).resolves.toMatchObject({ value: 123, stopReason: 'completed' })
|
||||
expect(settledRemove).toHaveBeenCalledWith('abort', settledAbort)
|
||||
const cancelAfterSettle = vi.spyOn(completed, 'cancel')
|
||||
settledController.abort()
|
||||
expect(cancelAfterSettle).not.toHaveBeenCalled()
|
||||
cancelAfterSettle.mockRestore()
|
||||
await completed.dispose()
|
||||
|
||||
const manual = await setup({ manual: true })
|
||||
const teardownController = new AbortController()
|
||||
const teardownAdd = vi.spyOn(teardownController.signal, 'addEventListener')
|
||||
const teardownRemove = vi.spyOn(teardownController.signal, 'removeEventListener')
|
||||
const tornDown = manual.ctx.workflows.start({
|
||||
...scripted("return await agent('job')"),
|
||||
parent: manual.parent,
|
||||
signal: teardownController.signal,
|
||||
})
|
||||
await waitFor(() => { expect(manual.provider.runs).toHaveLength(1) })
|
||||
const teardownAbort = teardownAdd.mock.calls.find(([type]) => type === 'abort')?.[1]
|
||||
expect(typeof teardownAbort).toBe('function')
|
||||
|
||||
const disposing = tornDown.dispose()
|
||||
expect(teardownRemove).toHaveBeenCalledWith('abort', teardownAbort)
|
||||
await disposing
|
||||
})
|
||||
|
||||
it('a child-start racing the host cancel is refused: no child starts after cancellation', async () => {
|
||||
const { ctx, parent, provider } = await setup({ manual: true })
|
||||
// Cancel from INSIDE the log listener: the worker has already posted
|
||||
|
||||
Reference in New Issue
Block a user