feat(agent-loop): turn-enclosure invariant + post-turn error model
Every session event now lives inside a turn (between turn/start and its turn/end). The loop records queued user/message events AFTER turn/start; an idle agent.inject() wraps its context/message in a one-shot injection turn. This makes the turn the single durability/replay boundary so a persistence backend can treat anything after the last turn/end as a crash tail without dropping legitimate between-turn context. A failure once the turn is already closed (rejecting session/flush, a throwing agent/turn-end listener) has no in-turn position for a session error event, so it is reported via agent/error + logger only; the turn stays balanced. failTurn appends an error event only while the turn is open. The dsh-invariants plugin enforces turn-enclosure via a default case: every non-boundary event type — including plugin-added merge-extensible keys — must sit inside an open turn or it throws. Documented in ADR 0017 + architecture.md.
This commit is contained in:
@@ -1,4 +1,4 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import { Context } from 'cordis'
|
||||
import { AgentId } from '@deepseek-ai/dsh-agent'
|
||||
import LlmService from '@deepseek-ai/dsh-llm'
|
||||
@@ -82,6 +82,80 @@ describe('LoopAgent', () => {
|
||||
expect(() => { agent.inject([{ type: 'text', text: 'too late' }]) }).toThrow('disposed')
|
||||
})
|
||||
|
||||
it('inject() decides enclosure from the LOG (open turn), not agent status', async () => {
|
||||
const adapter = new MockAdapter([textResponse('ok')])
|
||||
const ctx = await harness(adapter)
|
||||
const agent = ctx.agentLoop.create('a1', { model: 'mock' })
|
||||
|
||||
// Simulate an OPEN turn in the log while the agent is idle (status is not a
|
||||
// reliable open-turn signal). inject must append into that open turn, NOT
|
||||
// wrap a new one.
|
||||
agent.session.append('turn/start', { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } })
|
||||
agent.inject([{ type: 'text', text: 'mid' }], { source: { kind: 'plugin', plugin: 'p' } })
|
||||
expect(agent.session.events.filter(e => e.type === 'turn/start')).toHaveLength(1)
|
||||
expect(agent.session.events.at(-1)!.type).toBe('context/message')
|
||||
|
||||
// Close the turn; now inject must wrap its own one-shot injection turn.
|
||||
agent.session.append('turn/end', { turn: 1, reason: { kind: 'completed' } })
|
||||
agent.inject([{ type: 'text', text: 'after' }], { source: { kind: 'plugin', plugin: 'p' } })
|
||||
const starts = agent.session.events.filter(e => e.type === 'turn/start')
|
||||
expect(starts).toHaveLength(2)
|
||||
const last = starts[1]!
|
||||
expect(last.type === 'turn/start' && last.data.trigger.kind).toBe('injection')
|
||||
expect(agent.session.events.at(-1)!.type).toBe('turn/end') // turn-enclosed
|
||||
})
|
||||
|
||||
it('idle inject() contains a failing flush (logs, does not throw into the caller)', async () => {
|
||||
const adapter = new MockAdapter([textResponse('ok')])
|
||||
const ctx = await harness(adapter)
|
||||
// A persistence-like listener whose flush rejects.
|
||||
ctx.on('session/flush', () => { throw new Error('disk gone') })
|
||||
const warn = vi.spyOn(ctx.logger, 'warn').mockImplementation(() => undefined)
|
||||
const agent = ctx.agentLoop.create('a1', { model: 'mock' })
|
||||
|
||||
// inject() is synchronous and fires a fire-and-forget flush; a rejecting
|
||||
// flush must be contained (logged), never thrown into the caller.
|
||||
expect(() => { agent.inject([{ type: 'text', text: 'notice' }], { source: { kind: 'plugin', plugin: 'p' } }) }).not.toThrow()
|
||||
await new Promise(r => setTimeout(r, 20)) // let the contained flush settle
|
||||
expect(warn).toHaveBeenCalledWith(expect.stringContaining('flush after idle injection failed'))
|
||||
warn.mockRestore()
|
||||
})
|
||||
|
||||
it('idle inject() closes its one-shot turn AND still checkpoints even if the append throws', async () => {
|
||||
const adapter = new MockAdapter([textResponse('ok')])
|
||||
const ctx = await harness(adapter)
|
||||
const agent = ctx.agentLoop.create('a1', { model: 'mock' })
|
||||
let flushes = 0
|
||||
ctx.on('session/flush', () => { flushes += 1 })
|
||||
|
||||
// Non-serializable injected content makes Session.append throw AFTER
|
||||
// turn/start was recorded. The turn/end must still be appended (finally),
|
||||
// AND the durability checkpoint must still fire — the balanced turn is in
|
||||
// memory and a crash before the next turn/dispose would otherwise lose it.
|
||||
expect(() => {
|
||||
agent.inject([{ type: 'text', text: 'x', bad: 1n } as never], { source: { kind: 'plugin', plugin: 'p' } })
|
||||
}).toThrow(/non-JSON-serializable/)
|
||||
const types = agent.session.events.map(e => e.type)
|
||||
expect(types).toEqual(['turn/start', 'turn/end']) // balanced, no open turn
|
||||
await new Promise(r => setTimeout(r, 10)) // let the fire-and-forget flush run
|
||||
expect(flushes).toBe(1) // checkpoint fired despite the throw
|
||||
})
|
||||
|
||||
it('idle inject() with a non-serializable source opens no turn (nothing to close)', async () => {
|
||||
const adapter = new MockAdapter([textResponse('ok')])
|
||||
const ctx = await harness(adapter)
|
||||
const agent = ctx.agentLoop.create('a1', { model: 'mock' })
|
||||
|
||||
// A non-serializable source makes the turn/start append throw BEFORE the
|
||||
// event is pushed (Session.append validates before push), so NO turn opens.
|
||||
// The finally's isTurnOpen() guard sees no open turn and appends nothing —
|
||||
// the log stays empty, not left with a dangling turn/start.
|
||||
expect(() => {
|
||||
agent.inject([{ type: 'text', text: 'x' }], { source: { kind: 'plugin', plugin: 'p', bad: 1n } as never })
|
||||
}).toThrow(/non-JSON-serializable/)
|
||||
expect(agent.session.events).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('steer() when idle falls through to send() and starts a turn', async () => {
|
||||
const adapter = new MockAdapter([textResponse('ok')])
|
||||
const ctx = await harness(adapter)
|
||||
|
||||
@@ -57,9 +57,10 @@ describe('agent loop', () => {
|
||||
expect(order).toEqual(['agent/turn-start', 'agent/step-start', 'agent/step-end', 'agent/turn-end'])
|
||||
|
||||
const types = agent.session.events.map(e => e.type)
|
||||
// user message recorded before turn/start, assembled message + usage present
|
||||
expect(types[0]).toBe('user/message')
|
||||
expect(types[1]).toBe('turn/start')
|
||||
// turn/start opens the turn, THEN the queued user message is recorded inside
|
||||
// it (every event is turn-enclosed), then assembled message + usage.
|
||||
expect(types[0]).toBe('turn/start')
|
||||
expect(types[1]).toBe('user/message')
|
||||
expect(types).toContain('assistant/message')
|
||||
expect(types).toContain('usage')
|
||||
expect(types.at(-1)).toBe('turn/end')
|
||||
@@ -199,16 +200,22 @@ describe('agent loop', () => {
|
||||
expect(agent.session.events.some(e => e.type === 'user/message')).toBe(true)
|
||||
})
|
||||
|
||||
it('inject() appends context visible to the next request without starting a turn', async () => {
|
||||
it('inject() while idle wraps context in a one-shot turn, visible to the next request', async () => {
|
||||
const adapter = new MockAdapter([textResponse('ok')])
|
||||
const ctx = await harness(adapter)
|
||||
const agent = ctx.agentLoop.create('a1', { model: 'mock' })
|
||||
|
||||
agent.inject([{ type: 'text', text: 'file changed: a.ts' }], { source: { kind: 'plugin', plugin: 'watcher' } })
|
||||
// no turn started
|
||||
// The idle inject records a self-contained turn (turn/start → context/message
|
||||
// → turn/end) so the event stays turn-enclosed, but does NOT run the model.
|
||||
await new Promise(r => setTimeout(r, 20))
|
||||
expect(agent.status).toBe('idle')
|
||||
expect(adapter.requests).toHaveLength(0)
|
||||
const injectedTurn = agent.session.events.filter(e => e.type === 'turn/start')
|
||||
expect(injectedTurn).toHaveLength(1)
|
||||
const it0 = injectedTurn[0]!
|
||||
expect(it0.type === 'turn/start' && it0.data.trigger.kind).toBe('injection')
|
||||
expect(agent.session.events.at(-1)!.type).toBe('turn/end') // turn-enclosed
|
||||
|
||||
send(agent, 'go')
|
||||
await waitForIdle(ctx, agent)
|
||||
@@ -217,6 +224,38 @@ describe('agent loop', () => {
|
||||
expect(flat).toContain('<context source=\\"plugin\\">')
|
||||
})
|
||||
|
||||
it('inject() while running appends into the open turn (no extra synthetic turn)', async () => {
|
||||
const adapter = new MockAdapter([
|
||||
toolCallResponse('c1', 'noticer', {}, 'calling'),
|
||||
textResponse('done'),
|
||||
])
|
||||
const ctx = await harness(adapter)
|
||||
const agent = ctx.agentLoop.create('a1', { model: 'mock' })
|
||||
// A tool that injects mid-execution: at this point the agent is running, so
|
||||
// inject must append the context/message into the ALREADY-open turn rather
|
||||
// than wrap it in its own one-shot turn.
|
||||
ctx.tools.register(defineTool({
|
||||
name: 'noticer',
|
||||
description: 'injects a notice',
|
||||
parameters: {},
|
||||
async execute() {
|
||||
agent.inject([{ type: 'text', text: 'mid-turn notice' }], { source: { kind: 'plugin', plugin: 'x' } })
|
||||
return [{ type: 'text', text: 'ok' }]
|
||||
},
|
||||
}))
|
||||
|
||||
send(agent, 'go')
|
||||
await waitForIdle(ctx, agent)
|
||||
|
||||
// Exactly ONE turn ran (no synthetic injection turn), and the mid-turn
|
||||
// context/message sits inside it.
|
||||
const turnStarts = agent.session.events.filter(e => e.type === 'turn/start')
|
||||
expect(turnStarts).toHaveLength(1)
|
||||
const ts0 = turnStarts[0]!
|
||||
expect(ts0.type === 'turn/start' && ts0.data.trigger.kind).toBe('message')
|
||||
expect(agent.session.events.some(e => e.type === 'context/message')).toBe(true)
|
||||
})
|
||||
|
||||
it('agent/turn-continuation can force-continue (/loop pattern) and force-stop', async () => {
|
||||
// force-continue: model never calls tools, but a plugin forces 3 steps
|
||||
const adapter = new MockAdapter([
|
||||
|
||||
@@ -854,6 +854,39 @@ describe('P1-5: a started turn (and any open step) is always closed on a boundar
|
||||
expect(adapter.requests).toHaveLength(1)
|
||||
})
|
||||
|
||||
it('a throwing turn-end listener on a SUCCESSFUL turn leaves no event after turn/end (loadable log)', async () => {
|
||||
// Regression: a normal turn completes, closeTurn(true) appends turn/end and
|
||||
// emits agent/turn-end whose listener throws. The error must NOT be appended
|
||||
// as a session event after turn/end — that would sit past the commit
|
||||
// boundary and be dropped as a crash tail on resume (ADR 0017). It is
|
||||
// surfaced via agent/error instead, and the log's last event is turn/end.
|
||||
const adapter = new MockAdapter([textResponse('done'), textResponse('next ok')])
|
||||
const ctx = await balancedHarness(adapter)
|
||||
const agent = ctx.agentLoop.create('a-tend', { model: 'mock' })
|
||||
|
||||
let threw = false
|
||||
ctx.on('agent/turn-end', () => { if (!threw) { threw = true; throw new Error('boom turn-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)
|
||||
expect(c.turnEnd).toBe(1)
|
||||
expect(c.errors).toBe(0) // NO session error event (it would be post-turn/end)
|
||||
expect(agent.session.events.at(-1)?.type).toBe('turn/end') // last event is the boundary
|
||||
expect(errors.map(e => e.message)).toEqual(['boom turn-end']) // surfaced via agent/error
|
||||
// The whole log is loadable (nothing dropped): a fresh replay sees the turn.
|
||||
const replay = new Session(SessionId('replay'), [...agent.session.events])
|
||||
expect(replay.deriveMessages().map(m => m.role)).toEqual(['user', 'assistant'])
|
||||
|
||||
// loop survives.
|
||||
send(agent, 'again')
|
||||
await waitForIdle(ctx, agent)
|
||||
expect(boundaryCounts(agent).turnEnd).toBe(2)
|
||||
})
|
||||
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user