workflow: make the seam's listener containment total

emitWorkflowEvent's catch rendered the thrown value with a bare
String(error), which itself throws when the value's toString /
Symbol.toPrimitive throws — breaking the documented containment
guarantee: such a listener could fail the run mid-emit, starve later
listeners, and turn the detached workflow/end settle hook into an
unhandled rejection. Render through a local total fallback instead
(String in a try, a fixed label when even coercion throws); local
because the seam sits below every engine and cannot import an engine's
renderer. Regression: a listener throwing a coercion-trap value — the
emit does not propagate and later listeners still run.
This commit is contained in:
imccyu
2026-07-09 18:13:34 +08:00
parent 773ecf03f5
commit 4a15c8a479
2 changed files with 41 additions and 6 deletions

View File

@@ -218,11 +218,12 @@ export abstract class WorkflowService extends Service {
* with its OWN structural clone of the payload (the payloads are plain JSON
* data by the seam contract), so a listener mutating what it received can
* corrupt neither the engine's live state nor any other listener's or later
* event's view; a thrown listener is logged (never propagated), so one bad
* subscriber can neither fail the engine mid-run, surface as an unhandled
* rejection on a detached settle hook, nor starve the listeners registered
* after it (cordis `emit` halts on the first throw — same guarantee as the
* subagent seam's lifecycle emits).
* event's view; a thrown listener is logged (never propagated — the logging
* itself is total, even for a thrown value whose own string coercion
* throws), so one bad subscriber can neither fail the engine mid-run,
* surface as an unhandled rejection on a detached settle hook, nor starve
* the listeners registered after it (cordis `emit` halts on the first throw
* — same guarantee as the subagent seam's lifecycle emits).
* @param name - the `workflow/*` event to dispatch.
* @param args - the event's payload, matching its declared signature.
*/
@@ -233,10 +234,28 @@ export abstract class WorkflowService extends Service {
// dispatch callback applies the payload tuple.
;(callback as (...payload: unknown[]) => void)(...structuredClone(args))
} catch (error: unknown) {
this.ctx.logger.warn(`workflow: ${name} listener threw: ${String(error)}`)
this.ctx.logger.warn(`workflow: ${name} listener threw: ${renderListenerError(error)}`)
}
}
}
}
/**
* Total renderer for a listener-thrown value: the containment catch must never
* itself throw, and `String(error)` does when the value's own `toString` /
* `Symbol.toPrimitive` throws. Local rather than an engine package's renderer
* — the seam sits below every engine and cannot import one.
* @param error - any thrown value.
* @returns `String(error)`, or a fixed label when even coercion throws.
*/
function renderListenerError(error: unknown): string {
try {
return String(error)
} catch {
// Only a throwing toString/Symbol.toPrimitive lands here; the fixed label
// keeps the containment guarantee total.
return '[unrenderable thrown value]'
}
}
export default WorkflowService

View File

@@ -102,6 +102,22 @@ describe('dsh-workflow (interface)', () => {
expect(String(warn.mock.calls[0]![0])).toContain('workflow/phase listener threw')
})
it('containment is total: a listener throwing a value whose coercion throws neither propagates nor starves later listeners', async () => {
const ctx = new Context()
await ctx.plugin(StubEngine)
const warn = vi.spyOn(ctx.logger, 'warn').mockImplementation(() => ctx.logger)
const reached: string[] = []
ctx.on('workflow/phase', () => {
throw { toString: () => { throw new Error('coercion trap') } }
})
ctx.on('workflow/phase', (_info, title) => { reached.push(title) })
const engine = ctx.workflows as StubEngine
expect(() => { engine.emit('workflow/phase', INFO, 'Scan') }).not.toThrow()
expect(reached).toEqual(['Scan'])
expect(warn).toHaveBeenCalledOnce()
expect(String(warn.mock.calls[0]![0])).toContain('[unrenderable thrown value]')
})
it('has the expected export surface (default = the abstract service class)', () => {
expect(WorkflowServiceDefault).toBe(WorkflowService)
})