From 4a15c8a4794788be27a17d02a6ee1795f83375c9 Mon Sep 17 00:00:00 2001
From: imccyu <276526105+imccyu@users.noreply.github.com>
Date: Thu, 9 Jul 2026 18:13:34 +0800
Subject: [PATCH] workflow: make the seam's listener containment total
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.
---
 packages/workflow/workflow/src/index.ts       | 31 +++++++++++++++----
 .../workflow/workflow/tests/workflow.spec.ts  | 16 ++++++++++
 2 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/packages/workflow/workflow/src/index.ts b/packages/workflow/workflow/src/index.ts
index 288f6af952..0e5a03c438 100644
--- a/packages/workflow/workflow/src/index.ts
+++ b/packages/workflow/workflow/src/index.ts
@@ -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
diff --git a/packages/workflow/workflow/tests/workflow.spec.ts b/packages/workflow/workflow/tests/workflow.spec.ts
index b303c02962..a983e5a2b3 100644
--- a/packages/workflow/workflow/tests/workflow.spec.ts
+++ b/packages/workflow/workflow/tests/workflow.spec.ts
@@ -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)
   })