From 3783f3e17824bdca1d7a293d5735845e9d999f0d Mon Sep 17 00:00:00 2001
From: Tianyi Cui <53024+tianyicui@users.noreply.github.com>
Date: Mon, 15 Jun 2026 17:43:33 +0800
Subject: [PATCH] fix(agent-loop): surface throwing step-end listener as turn
 error via failTurn
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

closeStep() previously caught and silently swallowed a throw from
agent/step-end emit. In the normal no-tool/no-steering path, this
caused runTurn to reach closeTurn(true) with reason still
{kind:completed}, so the session recorded a completed turn with
zero error events — even though a plugin had failed at a loop
boundary. This violates the contract that a throwing plugin is
contained as a turn error, not a silent success.

Now the catch calls failTurn(toError(error)), which appends the
single error event and sets reason={kind:error,…}. failTurn is
idempotent (errorReported guard), so existing error paths that
call failTurn after closeStep are unaffected.

Add regression test: a throwing agent/step-end listener during a
successful step now produces exactly one error event, a turn/end
with reason error, balanced boundaries (step/end before turn/end),
and a surviving loop.
---
 packages/agent-loop/src/loop.ts               |  9 +++--
 .../agent-loop/tests/review-fixes.spec.ts     | 40 +++++++++++++++++++
 2 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/packages/agent-loop/src/loop.ts b/packages/agent-loop/src/loop.ts
index 063ed21a9d..4b8c7c743a 100644
--- a/packages/agent-loop/src/loop.ts
+++ b/packages/agent-loop/src/loop.ts
@@ -180,9 +180,12 @@ async function runTurn(ctx: Context, agent: LoopAgent, handle: LoopHandle, turn:
     session.append('step/end', { turn, step })
     try {
       ctx.emit('agent/step-end', agent, turn, step)
-    } catch {
-      // contained: step/end is already recorded, so balance holds; a throwing
-      // step-end listener is the listener's bug, not the loop's.
+    } catch (error: unknown) {
+      // step/end is already recorded so balance holds; surface the throwing
+      // listener as a turn error via failTurn (idempotent). This prevents a
+      // throwing step-end listener from producing a silent "completed" turn
+      // when the step itself succeeded (the normal-path closeStep call).
+      failTurn(toError(error))
     }
   }
 
diff --git a/packages/agent-loop/tests/review-fixes.spec.ts b/packages/agent-loop/tests/review-fixes.spec.ts
index fb297307dd..3a10af6a77 100644
--- a/packages/agent-loop/tests/review-fixes.spec.ts
+++ b/packages/agent-loop/tests/review-fixes.spec.ts
@@ -854,6 +854,46 @@ describe('P1-5: a started turn (and any open step) is always closed on a boundar
     expect(adapter.requests).toHaveLength(1)
   })
 
+  it('a throwing agent/step-end listener during a successful step ends the turn as error, not completed', async () => {
+    // closeStep() must surface a throwing step-end listener via failTurn so the
+    // turn ends with reason error, not a silent "completed" with the throw
+    // swallowed. Regression test for the closeStep() catch that previously
+    // swallowed the throw in the normal (no-tool, no-steering) path.
+    const adapter = new MockAdapter([textResponse('all good'), textResponse('turn 2 ok')])
+    const ctx = await balancedHarness(adapter)
+    const agent = ctx.agentLoop.create('a-stepend-throw', { model: 'mock' })
+
+    let threw = false
+    ctx.on('agent/step-end', () => { if (!threw) { threw = true; throw new Error('boom step-end') } })
+    const errors: Error[] = []
+    ctx.on('agent/error', (_a, _t, _s, error) => void errors.push(error))
+
+    send(agent, 'go')
+    await waitForIdle(ctx, agent)
+
+    const c = boundaryCounts(agent)
+    // step opened and closed; exactly one error; turn balanced; turn ends error.
+    expect(c).toMatchObject({ turnStart: 1, turnEnd: 1, stepStart: 1, stepEnd: 1, errors: 1 })
+    expect(errors.map(e => e.message)).toEqual(['boom step-end'])
+    expect(c.lastTurnEnd?.type === 'turn/end' && c.lastTurnEnd.data.reason)
+      .toEqual({ kind: 'error', message: 'boom step-end' })
+
+    // step/end precedes turn/end (ordering contract)
+    const e = [...agent.session.events]
+    const stepEndIdx = e.findIndex(x => x.type === 'step/end')
+    const turnEndIdx = e.findIndex(x => x.type === 'turn/end')
+    expect(stepEndIdx).toBeGreaterThanOrEqual(0)
+    expect(stepEndIdx).toBeLessThan(turnEndIdx)
+
+    // loop survives: a subsequent turn runs to completion
+    send(agent, 'again')
+    await waitForIdle(ctx, agent)
+    const c2 = boundaryCounts(agent)
+    expect(c2.turnStart).toBe(2)
+    expect(c2.turnEnd).toBe(2)
+    expect(c2.stepStart).toBe(c2.stepEnd)
+  })
+
   it('a step error followed by a throwing turn-end listener logs the error exactly once (no double-report)', async () => {
     // The step fails (finish-error) → failTurn records ONE error and sets the
     // error reason. closeTurn(true) then appends turn/end and emits