simplify(session): fold trace-only usage/error events into load-bearing events

The session event vocabulary carried two standalone trace-only events that were not load-bearing as separate records. Fold their facts into nearby load-bearing events and delete the standalone variants. - Token usage now rides on `assistant/message` as an optional `usage` field — the assembled model output and its accounting travel together. The loop folds `assembler.usage` onto the append instead of emitting a separate `usage` event. - The max-tokens path is the no-data-loss host: a step cut off with usage but EMPTY content (e.g. only a dropped tool call) previously emitted a standalone `usage`; it now records an empty-content `assistant/message { content: [], usage }`. `deriveMessages()` skips empty-content assistant messages, so the usage host never injects a spurious content-less assistant turn into the provider transcript. A step with neither content nor usage appends nothing. - An operational error's step number now rides on `turn/end.reason` for `kind: 'error'` (`{ kind: 'error', step, message, code? }`) — the durable turn outcome ACP and resume already consume. `failTurn` sets the reason directly (no separate session `error` event). `agent/error` + logging are unchanged for live diagnostics. - No format-version bump: pre-release, no persisted data, so per the format policy there is nothing to migrate or reject (the RFC's "refresh the format version" criterion over-reached). `version` stays 1. - ACP fixtures + goldens re-recorded (keyless replay): dropped standalone usage/error lines, usage folded onto assistant/message, error step on turn/end.reason. RFC moved proposed -> implemented with an implementation note recording the two scope refinements.
2026-06-21 10:00:06 +08:00
parent c6ed980d6f
commit 2be60b9a22
30 changed files with 480 additions and 470 deletions
--- a/packages/core/agent-loop/src/loop.ts
+++ b/packages/core/agent-loop/src/loop.ts
@@ -154,7 +154,7 @@ export interface LoopHandle {
 *       stream ctx.llm.stream(req)                    ⟵ waterfall llm/stream (raw chunks)
 *         session('assistant/chunk'); emit agent/stream-chunk
 *       msg = waterfall agent/step-result             ⟵ BEFORE the log append, so the
- *       session('assistant/message','usage')             session records what actually ran
+ *       session('assistant/message' {content, usage?})   session records what actually ran
 *       each tool-call in msg (sequential, abort-checked):
 *         session('tool/call'); ctx.tools.execute()   ⟵ waterfall tools/execute
 *         session('tool/result')
@@ -313,41 +313,24 @@ async function runTurn(ctx: Context, agent: ReactLoopAgent, handle: LoopHandle,
    return false
  }

-  // Record a step/turn failure exactly once: append the single `error` event
-  // (only while the turn is still open — see below), set the error reason, and
-  // emit agent/error (contained — trap: a throwing agent/error listener must not
-  // re-escape and strand the turn). Disposal and abort set `reason` directly
-  // without calling this (no `error` event for those — they are not failures).
+  // Record a step/turn failure exactly once: set the error reason (carrying the
+  // failing `step` — the durable failure lives entirely on turn/end.reason, there
+  // is no separate session error event) and emit agent/error (contained — trap: a
+  // throwing agent/error listener must not re-escape and strand the turn).
+  // Disposal and abort set `reason` directly without calling this (they are not
+  // failures).
  const failTurn = (err: CodedError): void => {
    if (errorReported) return
    errorReported = true
-    // Only append the session `error` INSIDE the turn (before turn/end). If the
-    // turn has already ended — the only way here is a throwing agent/turn-end
-    // listener after closeTurn(true) already appended turn/end — appending now
-    // would land the error AFTER the last turn/end, where the persistence
-    // backend treats it as a crash tail and drops it on resume (the turn-enclosure RFC). In
-    // that case report via agent/error + the logger only; the turn is balanced.
-    if (!turnEnded) {
-      // Set `reason` BEFORE the append: Session.append pushes the error event
-      // before notifying session/event listeners, so a throwing listener would
-      // otherwise leave `reason` unset (and closeTurn would record the wrong
-      // reason / the outer catch would skip closeTurn). The append is contained
-      // — the error event is already in the log either way; a throwing listener
-      // must not abort finalization.
-      reason = { kind: 'error', ...errorData(err) }
-      try {
-        session.append('error', { turn, step, ...errorData(err) })
-      } catch (appendError: unknown) {
-        ctx.logger.warn(`agent "${agent.id}": session/event listener threw on the error event at turn ${turn}: ${toError(appendError).message}`)
-      }
-    } else {
-      ctx.logger.warn(`agent "${agent.id}": agent/turn-end listener threw after turn ${turn} closed: ${err.message}`)
-    }
+    // Set `reason` here so the durable failure is captured before closeTurn
+    // appends turn/end. The step number rides along so the operational error's
+    // location survives in the durable log.
+    reason = { kind: 'error', step, ...errorData(err) }
    try {
      ctx.emit('agent/error', agent, turn, step, err)
    } catch {
-      // contained: the error is already logged; a throwing agent/error
-      // listener must not prevent the turn from closing.
+      // contained: the error is already captured on `reason`; a throwing
+      // agent/error listener must not prevent the turn from closing.
    }
  }

@@ -608,11 +591,14 @@ async function runStep(
  if (assembler.finish.kind === 'max-tokens') {
    let message: Message = withoutToolCalls(assembler.message())
    message = withoutToolCalls(await ctx.waterfall('agent/step-result', agent, turn, step, message, () => Promise.resolve(message)))
-    if (message.content.length > 0) {
-      session.append('assistant/message', { turn, step, content: message.content })
-    }
-    if (assembler.usage) {
-      session.append('usage', { turn, step, usage: assembler.usage })
+    // Fire the assistant/message when there is content OR usage: a max-tokens
+    // step can be cut off with empty content but still carry token accounting,
+    // and assistant/message is the only host for usage (there is no standalone
+    // usage event). An empty-content assistant/message is skipped by
+    // deriveMessages(), so hosting usage on it never injects a spurious assistant
+    // turn into derived history.
+    if (message.content.length > 0 || assembler.usage) {
+      session.append('assistant/message', { turn, step, content: message.content, ...(assembler.usage ? { usage: assembler.usage } : {}) })
    }
    return { hadToolCalls: false, finish: assembler.finish }
  }
@@ -623,10 +609,7 @@ async function runStep(
  let message: Message = assembler.message()
  message = await ctx.waterfall('agent/step-result', agent, turn, step, message, () => Promise.resolve(message))

-  session.append('assistant/message', { turn, step, content: message.content })
-  if (assembler.usage) {
-    session.append('usage', { turn, step, usage: assembler.usage })
-  }
+  session.append('assistant/message', { turn, step, content: message.content, ...(assembler.usage ? { usage: assembler.usage } : {}) })

  // --- Tool execution (sequential; parallel execution is a TODO) ---
  // ToolRegistry.execute converts tool failures (including aborts) into
--- a/packages/core/agent-loop/tests/coverage-edges.spec.ts
+++ b/packages/core/agent-loop/tests/coverage-edges.spec.ts
@@ -213,9 +213,9 @@ describe('toError normalization', () => {
    expect(errors).toHaveLength(1)
    expect(errors[0]!.message).toBe('naked string error')
    // A non-Error throw is wrapped in a HarnessError with code UNKNOWN, so the
-    // session error event carries a routable code instead of degrading.
-    const errorEvent = agent.session.events.find(e => e.type === 'error')
-    expect(errorEvent?.type === 'error' && errorEvent.data.code).toBe('UNKNOWN')
+    // turn-end error reason carries a routable code instead of degrading.
+    const turnEnd = agent.session.events.find(e => e.type === 'turn/end')
+    expect(turnEnd?.type === 'turn/end' && turnEnd.data.reason.kind === 'error' && turnEnd.data.reason.code).toBe('UNKNOWN')
  })

  it('normalizes non-Error throws from agent/request waterfall via inline toError in runStep catch', async () => {
@@ -240,8 +240,8 @@ describe('toError normalization', () => {
    expect(errors).toHaveLength(1)
    // String() of { code: 500 } is '[object Object]'
    expect(errors[0]!.message).toBe('[object Object]')
-    const errorEvent = agent.session.events.find(e => e.type === 'error')
-    expect(errorEvent?.type === 'error' && errorEvent.data.code).toBe('UNKNOWN')
+    const turnEnd = agent.session.events.find(e => e.type === 'turn/end')
+    expect(turnEnd?.type === 'turn/end' && turnEnd.data.reason.kind === 'error' && turnEnd.data.reason.code).toBe('UNKNOWN')
  })
 })

@@ -268,11 +268,11 @@ describe('coded error data emission', () => {
    expect(errors).toHaveLength(1)
    expect(errors[0]!.message).toBe('server overloaded')

-    // session error event includes the code
-    const errorEvent = agent.session.events.find(e => e.type === 'error')
-    expect(errorEvent).toBeDefined()
-    if (errorEvent!.type === 'error') {
-      expect(errorEvent!.data.code).toBe('RATE_LIMIT')
+    // turn-end error reason includes the code
+    const turnEnd = agent.session.events.find(e => e.type === 'turn/end')
+    expect(turnEnd).toBeDefined()
+    if (turnEnd?.type === 'turn/end' && turnEnd.data.reason.kind === 'error') {
+      expect(turnEnd.data.reason.code).toBe('RATE_LIMIT')
    }
  })
 })
--- a/packages/core/agent-loop/tests/loop.spec.ts
+++ b/packages/core/agent-loop/tests/loop.spec.ts
@@ -58,11 +58,13 @@ describe('agent loop', () => {

    const types = agent.session.events.map(e => e.type)
    // turn/start opens the turn, THEN the queued user message is recorded inside
-    // it (every event is turn-enclosed), then assembled message + usage.
+    // it (every event is turn-enclosed), then the assembled message (carrying the
+    // step's usage).
    expect(types[0]).toBe('turn/start')
    expect(types[1]).toBe('user/message')
    expect(types).toContain('assistant/message')
-    expect(types).toContain('usage')
+    const assistantMessage = agent.session.events.find(e => e.type === 'assistant/message')
+    expect(assistantMessage?.type === 'assistant/message' && assistantMessage.data.usage).toEqual({ inputTokens: 10, outputTokens: 'hello there'.length })
    expect(types.at(-1)).toBe('turn/end')

    // derived history: user + assistant
@@ -442,6 +444,47 @@ describe('agent loop', () => {
    expect(agent.session.events.some(e => e.type === 'tool/call')).toBe(false)
    expect(agent.session.deriveMessages()).toEqual([{ role: 'user', content: [{ type: 'text', text: 'go' }] }])
    expect(reasons).toEqual([{ kind: 'max-tokens' }])
+    // No-data-loss: a max-tokens step whose only content was a dropped tool call
+    // has EMPTY assistant content, but its usage must still be represented. It
+    // rides on an (empty-content) assistant/message — there is no standalone
+    // usage event — and that empty message is skipped by deriveMessages(), so
+    // the derived history above is NOT corrupted by a spurious assistant turn.
+    const assistantMessage = agent.session.events.find(e => e.type === 'assistant/message')
+    expect(assistantMessage?.type === 'assistant/message' && assistantMessage.data).toEqual({
+      turn: 1, step: 1, content: [], usage: { inputTokens: 10, outputTokens: 5 },
+    })
+  })
+
+  it('appends no assistant/message for a max-tokens step with empty content and no usage', async () => {
+    // A max-tokens step truncated to a dropped tool call AND with no usage chunk
+    // has nothing to record: empty content and no accounting → no assistant/message
+    // (the empty-content host exists only to carry usage). The turn still ends
+    // max-tokens.
+    const callId = CallId('c1')
+    const adapter = new MockAdapter([[
+      { type: 'block-start', index: 0, blockType: 'tool-call' },
+      { type: 'tool-call-delta', index: 0, id: callId, name: 'echo', argumentsDelta: '{"text":"x"}' },
+      { type: 'block-end', index: 0, block: { type: 'tool-call', id: callId, name: 'echo', arguments: '{"text":"x"}' } },
+      { type: 'finish', reason: { kind: 'max-tokens' } },
+    ]])
+    const ctx = await harness(adapter)
+    ctx.tools.register(defineTool({
+      name: 'echo',
+      description: '',
+      parameters: { text: { type: 'string' } },
+      async execute() { return [{ type: 'text', text: 'should not run' }] },
+    }))
+    const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' })
+
+    const reasons: TurnEndReason[] = []
+    ctx.on('agent/turn-end', (_agent, _turn, reason) => void reasons.push(reason))
+
+    send(agent, 'go')
+    await waitForIdle(ctx, agent)
+
+    expect(reasons).toEqual([{ kind: 'max-tokens' }])
+    expect(agent.session.events.some(e => e.type === 'assistant/message')).toBe(false)
+    expect(agent.session.deriveMessages()).toEqual([{ role: 'user', content: [{ type: 'text', text: 'go' }] }])
  })

  it('keeps safe max-tokens assistant content while dropping truncated tool calls', async () => {
@@ -563,7 +606,10 @@ describe('agent loop', () => {
    expect(errors).toHaveLength(1)
    expect(errors[0]!.message).toContain('script exhausted')
    expect(reasons[0]).toMatchObject({ kind: 'error' })
-    expect(agent.session.events.some(e => e.type === 'error')).toBe(true)
+    // The durable failure lives entirely on turn/end.reason (with the failing
+    // step), not a standalone error event.
+    const turnEnd = agent.session.events.find(e => e.type === 'turn/end')
+    expect(turnEnd?.type === 'turn/end' && turnEnd.data.reason).toMatchObject({ kind: 'error', step: 1 })
  })

  it('disposing the loop fiber mid-turn stops the loop (HMR safety)', async () => {
--- a/packages/core/agent-loop/tests/review-fixes.spec.ts
+++ b/packages/core/agent-loop/tests/review-fixes.spec.ts
@@ -492,11 +492,13 @@ describe('HIGH: a finish-error stream chunk ends the turn as error, not complete
    send(agent, 'go')
    await waitForIdle(ctx, agent)

-    expect(reasons).toEqual([{ kind: 'error', message: 'provider 401', code: 'AUTH' }])
+    expect(reasons).toEqual([{ kind: 'error', step: 1, message: 'provider 401', code: 'AUTH' }])

    const events = [...agent.session.events]
-    expect(events.some(event => event.type === 'error'
-      && event.data.message === 'provider 401' && event.data.code === 'AUTH')).toBe(true)
+    // The durable failure lives on turn/end.reason (with the failing step), not
+    // a standalone error event.
+    const turnEnd = events.find(event => event.type === 'turn/end')
+    expect(turnEnd?.type === 'turn/end' && turnEnd.data.reason).toEqual({ kind: 'error', step: 1, message: 'provider 401', code: 'AUTH' })
    // Crucially: no assistant/message was logged for the failed step.
    expect(events.some(event => event.type === 'assistant/message')).toBe(false)
  })
@@ -515,7 +517,7 @@ describe('HIGH: a finish-error stream chunk ends the turn as error, not complete
    send(agent, 'go')
    await waitForIdle(ctx, agent)

-    expect(reasons).toEqual([{ kind: 'error', message: 'model stream aborted', code: 'ABORTED' }])
+    expect(reasons).toEqual([{ kind: 'error', step: 1, message: 'model stream aborted', code: 'ABORTED' }])
    expect([...agent.session.events].some(event => event.type === 'assistant/message')).toBe(false)
  })

@@ -533,7 +535,7 @@ describe('HIGH: a finish-error stream chunk ends the turn as error, not complete
    send(agent, 'go')
    await waitForIdle(ctx, agent)

-    expect(reasons).toEqual([{ kind: 'error', message: 'codeless failure' }])
+    expect(reasons).toEqual([{ kind: 'error', step: 1, message: 'codeless failure' }])
  })
 })

@@ -592,7 +594,7 @@ describe('P1-5: a started turn (and any open step) is always closed on a boundar
      turnEnd: e.filter(x => x.type === 'turn/end').length,
      stepStart: e.filter(x => x.type === 'step/start').length,
      stepEnd: e.filter(x => x.type === 'step/end').length,
-      errors: e.filter(x => x.type === 'error').length,
+      errors: e.filter(x => x.type === 'turn/end' && x.data.reason.kind === 'error').length,
      lastTurnEnd: e.findLast(x => x.type === 'turn/end'),
    }
  }
@@ -611,10 +613,10 @@ describe('P1-5: a started turn (and any open step) is always closed on a boundar
    await waitForIdle(ctx, agent)

    const c = boundaryCounts(agent)
-    // turn opened and closed; no step ran; exactly one error logged + emitted.
+    // turn opened and closed; no step ran; exactly one error turn-end + emitted.
    expect(c).toMatchObject({ turnStart: 1, turnEnd: 1, stepStart: 0, stepEnd: 0, errors: 1 })
    expect(errors.map(e => e.message)).toEqual(['boom turn-start'])
-    expect(c.lastTurnEnd?.type === 'turn/end' && c.lastTurnEnd.data.reason).toEqual({ kind: 'error', message: 'boom turn-start' })
+    expect(c.lastTurnEnd?.type === 'turn/end' && c.lastTurnEnd.data.reason).toEqual({ kind: 'error', step: 0, message: 'boom turn-start' })
    // model was never called (we threw before the step's request).
    expect(adapter.requests).toHaveLength(0)
  })
@@ -664,7 +666,7 @@ describe('P1-5: a started turn (and any open step) is always closed on a boundar
    expect(c.turnStart).toBe(1)
    expect(c.turnEnd).toBe(1)
    expect(c.stepStart).toBe(c.stepEnd)
-    expect(c.lastTurnEnd?.type === 'turn/end' && c.lastTurnEnd.data.reason).toMatchObject({ kind: 'error', message: 'provider 500' })
+    expect(c.lastTurnEnd?.type === 'turn/end' && c.lastTurnEnd.data.reason).toMatchObject({ kind: 'error', step: 1, message: 'provider 500' })

    // loop survives: a second turn runs to completion (invariants oracle would
    // throw on its turn/start if turn 1 had been left open).
@@ -701,8 +703,8 @@ describe('P1-5: a started turn (and any open step) is always closed on a boundar
    expect(turnStarts).toBe(1)
    expect(turnEnds).toBe(1) // balanced — the turn was closed despite disposal
    expect(reasons).toEqual([{ kind: 'disposed' }])
-    // no error event: disposal is not a failure.
-    expect(e.some(x => x.type === 'error')).toBe(false)
+    // no error reason: disposal is not a failure.
+    expect(e.some(x => x.type === 'turn/end' && x.data.reason.kind === 'error')).toBe(false)
  })

  it('preserves reason disposed when the turn-end emit throws during disposal (outer-catch disposed branch)', async () => {
@@ -741,9 +743,10 @@ describe('P1-5: a started turn (and any open step) is always closed on a boundar
    expect(e.filter(x => x.type === 'turn/start')).toHaveLength(1)
    const turnEnd = e.findLast(x => x.type === 'turn/end')
    expect(turnEnd?.type === 'turn/end' && turnEnd.data.reason).toEqual({ kind: 'disposed' })
-    // The throwing turn-end listener is contained: no error event is logged and
-    // no agent/error is emitted (disposal is not a failure; the throw is swallowed).
-    expect(e.some(x => x.type === 'error')).toBe(false)
+    // The throwing turn-end listener is contained: the turn/end carries the
+    // disposed reason (not an error) and no agent/error is emitted (disposal is
+    // not a failure; the throw is swallowed).
+    expect(e.some(x => x.type === 'turn/end' && x.data.reason.kind === 'error')).toBe(false)
    expect(errorEmits).toHaveLength(0)
  })

@@ -840,11 +843,11 @@ describe('P1-5: a started turn (and any open step) is always closed on a boundar
    await waitForIdle(ctx, agent)

    const c = boundaryCounts(agent)
-    // step opened and closed; exactly one error; turn balanced; turn ends error.
+    // step opened and closed; exactly one error turn-end; turn balanced.
    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' })
+      .toEqual({ kind: 'error', step: 1, message: 'boom step-end' })

    // step/end precedes turn/end (ordering contract)
    const e = [...agent.session.events]
@@ -882,12 +885,12 @@ describe('P1-5: a started turn (and any open step) is always closed on a boundar
    await waitForIdle(ctx, agent)

    const c = boundaryCounts(agent)
-    // exactly one error event + one agent/error emit, despite two failTurn calls.
+    // exactly one error turn-end + one agent/error emit, despite two failTurn calls.
    expect(c.errors).toBe(1)
    expect(errors.map(e => e.message)).toEqual(['provider down'])
    expect(c.turnStart).toBe(1)
    expect(c.turnEnd).toBe(1) // single turn/end, balanced
-    expect(c.lastTurnEnd?.type === 'turn/end' && c.lastTurnEnd.data.reason).toMatchObject({ kind: 'error', message: 'provider down' })
+    expect(c.lastTurnEnd?.type === 'turn/end' && c.lastTurnEnd.data.reason).toMatchObject({ kind: 'error', step: 1, message: 'provider down' })

    // loop survives the compound failure.
    send(agent, 'again')
@@ -895,42 +898,6 @@ describe('P1-5: a started turn (and any open step) is always closed on a boundar
    expect(boundaryCounts(agent).turnEnd).toBe(2)
  })

-  it('a throwing session/event listener on the error event still closes the turn (finalizer containment)', async () => {
-    // failTurn appends the `error` event; Session.append pushes it BEFORE
-    // notifying session/event listeners, so a throwing listener leaves `error`
-    // in the log but must NOT abort finalization — `reason` is set before the
-    // append and the throw is contained, so closeTurn(false) still runs and
-    // turn/end is appended (the turn is balanced, not left open).
-    // Plain harness (no invariants oracle): the throwing listener is itself a
-    // session/event subscriber. A finish-error drives the boundary-error path.
-    const errorStream: StreamChunk[] = [{ type: 'finish', reason: { kind: 'error', message: 'provider down' } }]
-    const adapter = new MockAdapter([errorStream, textResponse('turn 2 ok')])
-    const ctx = await harness(adapter)
-    const agent = ctx.agentLoop.create(AgentId('a-errthrow'), { model: 'mock' })
-
-    let threw = false
-    ctx.on('session/event', (_s, event) => {
-      if (!threw && event.type === 'error') { threw = true; throw new Error('boom error-event listener') }
-    })
-
-    send(agent, 'go')
-    await waitForIdle(ctx, agent)
-
-    const e = [...agent.session.events]
-    // The error event is in the log (pushed before the listener threw)…
-    expect(e.some(x => x.type === 'error')).toBe(true)
-    // …and the turn was still closed with the error reason (finalization did not
-    // abort): the last event is turn/end carrying the error reason.
-    const last = e.at(-1)
-    expect(last?.type).toBe('turn/end')
-    expect(last?.type === 'turn/end' && last.data.reason).toMatchObject({ kind: 'error', message: 'provider down' })
-
-    // loop survives: a second turn runs normally.
-    send(agent, 'again')
-    await waitForIdle(ctx, agent)
-    expect(adapter.requests).toHaveLength(2)
-  })
-
  it('a throwing session/event listener on step/end during finalization still appends turn/end', async () => {
    // A throwing agent/step-start listener drives the outer catch, which calls
    // closeStep() during finalization. closeStep appends step/end; a