refactor(agent): replace the per-step advice seam with agent/session-prefix
Review discussion converged on the industry shape (Claude Code caches user context per conversation; Codex separates initial context from diffs; Kimi appends at continuation boundaries to protect prompt caching): stable openers belong in a compose-once prefix, mid-session changes belong in append-only history — not in a per-request slot. agent/session-prefix fires ONCE per loop instance, lazily on its first request-building step: the composed Message[] is deep-frozen, cached on the transmission bookkeeping, recorded as EpochHeader.messagePrefix on the anchoring 'initial'/'resume' snapshot, and reused verbatim for every request the instance sends — prefix stability is structural, not a producer discipline, and a resume recomposes with attributable drift. The request is messagePrefix + boundary snapshot. The per-step RequestAdvice/RequestAdviceContext surface and the messageSuffix header field are dropped: the tail slot had no consumer, and every current update pattern (new AGENTS.md discovered, memory update, skills change) routes through the existing append-only history channels — inject(), tools/post-execute additionalContext, prompt-submit additionalContext — each paid once and prefix-cached thereafter. The messagePrefix delta arm stays for codec totality; the loop never produces one in practice.
This commit is contained in:
@@ -1,14 +1,13 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { Context } from 'cordis'
|
||||
import LlmService, { CallId, type Message } from '@deepseek-ai/dsh-llm'
|
||||
import SessionStore, { foldRequestHeader, type SessionEvent, type TurnEndReason } from '@deepseek-ai/dsh-session'
|
||||
import SessionStore, { type SessionEvent, type TurnEndReason } from '@deepseek-ai/dsh-session'
|
||||
import SystemPrompt from '@deepseek-ai/dsh-system-prompt'
|
||||
import ToolRegistry, { defineTool, type PostToolDecision, type PreToolDecision } from '@deepseek-ai/dsh-tools'
|
||||
import AgentRegistry, {
|
||||
AgentId,
|
||||
type ContinuationDecision,
|
||||
type PromptDecision,
|
||||
type RequestAdvice,
|
||||
type SessionStartSource,
|
||||
} from '@deepseek-ai/dsh-agent'
|
||||
import AgentLoop, { type ReactLoopAgent } from '@deepseek-ai/dsh-agent-loop'
|
||||
@@ -311,58 +310,58 @@ describe('agent/session-start', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('agent/request-advice (RequestAdvice)', () => {
|
||||
it('frames the derived history: before precedes it, after follows it, and the header records both', async () => {
|
||||
const adapter = new MockAdapter([textResponse('ok')])
|
||||
describe('agent/session-prefix', () => {
|
||||
it('composes once per loop instance and fronts every request; the header records it; history stays untouched', async () => {
|
||||
const adapter = new MockAdapter([
|
||||
toolCallResponse('c1', 'echo', { text: 'ping' }),
|
||||
textResponse('done'),
|
||||
textResponse('again'),
|
||||
])
|
||||
const ctx = await harness(adapter)
|
||||
ctx.tools.register(defineTool({
|
||||
name: 'echo', description: 'echo', parameters: { text: { type: 'string' } },
|
||||
async execute(args) { return [{ type: 'text', text: String(args.text) }] },
|
||||
}))
|
||||
const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' })
|
||||
|
||||
const reminder: Message = { role: 'user', content: [{ type: 'text', text: '<system-reminder>catalog</system-reminder>' }] }
|
||||
const trailer: Message = { role: 'user', content: [{ type: 'text', text: 'trailing note' }] }
|
||||
ctx.on('agent/request-advice', async (_agent, _turn, _step, _messages, _context, next): Promise<RequestAdvice> => {
|
||||
const result = await next()
|
||||
return { before: [...result.before, reminder], after: [...result.after, trailer] }
|
||||
let composed = 0
|
||||
ctx.on('agent/session-prefix', async (_agent, _prefix, _signal, next): Promise<Message[]> => {
|
||||
composed += 1
|
||||
return [...await next(), reminder]
|
||||
})
|
||||
|
||||
send(agent, 'hi')
|
||||
send(agent, 'go')
|
||||
await waitForIdle(ctx, agent)
|
||||
send(agent, 'next turn')
|
||||
await waitForIdle(ctx, agent)
|
||||
|
||||
// The request carries before + derived history + after, in that order…
|
||||
const request = adapter.requests[0]!
|
||||
expect(request.messages).toEqual([
|
||||
reminder,
|
||||
{ role: 'user', content: [{ type: 'text', text: 'hi' }] },
|
||||
trailer,
|
||||
])
|
||||
// …the header event is their durable record…
|
||||
const headerEvent = events(agent).find(e => e.type === 'request/header')
|
||||
expect(headerEvent?.type === 'request/header' && headerEvent.data.header.messagePrefix).toEqual([reminder])
|
||||
expect(headerEvent?.type === 'request/header' && headerEvent.data.header.messageSuffix).toEqual([trailer])
|
||||
// …and they never become session history.
|
||||
expect(agent.session.deriveMessages()).toEqual([
|
||||
{ role: 'user', content: [{ type: 'text', text: 'hi' }] },
|
||||
{ role: 'assistant', content: [{ type: 'text', text: 'ok' }] },
|
||||
])
|
||||
// Three requests (two turns), ONE composition: the frozen product is
|
||||
// reused verbatim, so the prefix cannot drift mid-session.
|
||||
expect(adapter.requests).toHaveLength(3)
|
||||
expect(composed).toBe(1)
|
||||
for (const request of adapter.requests) {
|
||||
expect(request.messages[0]).toEqual(reminder)
|
||||
}
|
||||
// The anchoring snapshot is the prefix's durable record — and the ONLY
|
||||
// header event: reuse means no request/header-delta ever.
|
||||
const headerEvents = events(agent).filter(e => e.type === 'request/header' || e.type === 'request/header-delta')
|
||||
expect(headerEvents).toHaveLength(1)
|
||||
expect(headerEvents[0]?.type === 'request/header' && headerEvents[0].data.header.messagePrefix).toEqual([reminder])
|
||||
// Never session history: the derivation starts at the real user prompt.
|
||||
expect(agent.session.deriveMessages()[0]).toEqual({ role: 'user', content: [{ type: 'text', text: 'go' }] })
|
||||
})
|
||||
|
||||
it('contributions compose across listeners and see the read-only request facts', async () => {
|
||||
it('contributions compose across listeners in registration order', async () => {
|
||||
const adapter = new MockAdapter([textResponse('ok')])
|
||||
const ctx = await harness(adapter)
|
||||
const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' })
|
||||
|
||||
const seen: { system: string; boundaryRoles: string[]; sectionCount: number }[] = []
|
||||
ctx.on('agent/request-advice', async (_agent, _turn, _step, _messages, context, next): Promise<RequestAdvice> => {
|
||||
const result = await next()
|
||||
seen.push({
|
||||
system: context.system,
|
||||
boundaryRoles: context.boundaryMessages.map(m => m.role),
|
||||
sectionCount: context.assembly.sections.length,
|
||||
})
|
||||
return { before: [{ role: 'user', content: [{ type: 'text', text: 'first' }] }, ...result.before], after: result.after }
|
||||
ctx.on('agent/session-prefix', async (_agent, _prefix, _signal, next): Promise<Message[]> => {
|
||||
return [{ role: 'user', content: [{ type: 'text', text: 'first' }] }, ...await next()]
|
||||
})
|
||||
ctx.on('agent/request-advice', async (_agent, _turn, _step, _messages, _context, next): Promise<RequestAdvice> => {
|
||||
const result = await next()
|
||||
return { before: [...result.before, { role: 'user', content: [{ type: 'text', text: 'second' }] }], after: result.after }
|
||||
ctx.on('agent/session-prefix', async (_agent, _prefix, _signal, next): Promise<Message[]> => {
|
||||
return [...await next(), { role: 'user', content: [{ type: 'text', text: 'second' }] }]
|
||||
})
|
||||
|
||||
send(agent, 'hi')
|
||||
@@ -372,27 +371,21 @@ describe('agent/request-advice (RequestAdvice)', () => {
|
||||
// out (waterfall), so its prepend lands first.
|
||||
const texts = adapter.requests[0]!.messages.map(m => m.content[0]?.type === 'text' ? m.content[0].text : '')
|
||||
expect(texts).toEqual(['first', 'second', 'hi'])
|
||||
// The context carried the request facts: the rendered system prompt, the
|
||||
// boundary snapshot (exactly the drained user prompt), and the assembly.
|
||||
expect(seen).toHaveLength(1)
|
||||
expect(seen[0]!.boundaryRoles).toEqual(['user'])
|
||||
expect(typeof seen[0]!.system).toBe('string')
|
||||
})
|
||||
|
||||
it('with no contributions the header omits both fields and the request is the bare derivation', async () => {
|
||||
it('with no contributions the header omits messagePrefix and the request is the bare derivation', async () => {
|
||||
const adapter = new MockAdapter([textResponse('ok')])
|
||||
const ctx = await harness(adapter)
|
||||
const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' })
|
||||
|
||||
// A listener that delegates without contributing — the canonical no-op.
|
||||
ctx.on('agent/request-advice', async (_agent, _turn, _step, _messages, _context, next) => next())
|
||||
ctx.on('agent/session-prefix', async (_agent, _prefix, _signal, next) => next())
|
||||
|
||||
send(agent, 'hi')
|
||||
await waitForIdle(ctx, agent)
|
||||
|
||||
const headerEvent = events(agent).find(e => e.type === 'request/header')
|
||||
expect(headerEvent?.type === 'request/header' && 'messagePrefix' in headerEvent.data.header).toBe(false)
|
||||
expect(headerEvent?.type === 'request/header' && 'messageSuffix' in headerEvent.data.header).toBe(false)
|
||||
expect(adapter.requests[0]!.messages).toEqual([{ role: 'user', content: [{ type: 'text', text: 'hi' }] }])
|
||||
})
|
||||
|
||||
@@ -402,9 +395,9 @@ describe('agent/request-advice (RequestAdvice)', () => {
|
||||
const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' })
|
||||
|
||||
let mutationError: unknown
|
||||
ctx.on('agent/request-advice', async (_agent, _turn, _step, messages, _context, next): Promise<RequestAdvice> => {
|
||||
ctx.on('agent/session-prefix', async (_agent, prefix, _signal, next): Promise<Message[]> => {
|
||||
try {
|
||||
messages.before.push({ role: 'user', content: [{ type: 'text', text: 'smuggled' }] })
|
||||
prefix.push({ role: 'user', content: [{ type: 'text', text: 'smuggled' }] })
|
||||
} catch (error: unknown) {
|
||||
mutationError = error
|
||||
}
|
||||
@@ -418,30 +411,7 @@ describe('agent/request-advice (RequestAdvice)', () => {
|
||||
expect(adapter.requests[0]!.messages).toEqual([{ role: 'user', content: [{ type: 'text', text: 'hi' }] }])
|
||||
})
|
||||
|
||||
it('the read-only boundary context rejects in-place mutation before the request is built', async () => {
|
||||
const adapter = new MockAdapter([textResponse('ok')])
|
||||
const ctx = await harness(adapter)
|
||||
const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' })
|
||||
|
||||
let mutationError: unknown
|
||||
ctx.on('agent/request-advice', async (_agent, _turn, _step, _messages, context, next): Promise<RequestAdvice> => {
|
||||
try {
|
||||
const mutableBoundary = context.boundaryMessages as Message[]
|
||||
mutableBoundary.push({ role: 'user', content: [{ type: 'text', text: 'smuggled' }] })
|
||||
} catch (error: unknown) {
|
||||
mutationError = error
|
||||
}
|
||||
return next()
|
||||
})
|
||||
|
||||
send(agent, 'hi')
|
||||
await waitForIdle(ctx, agent)
|
||||
|
||||
expect(mutationError).toBeInstanceOf(TypeError)
|
||||
expect(adapter.requests[0]!.messages).toEqual([{ role: 'user', content: [{ type: 'text', text: 'hi' }] }])
|
||||
})
|
||||
|
||||
it('a per-step contribution change is logged as a header delta, so every request stays reconstructable', async () => {
|
||||
it('mutating a listener-held reference after composition cannot alter later requests (the cache is a frozen clone)', async () => {
|
||||
const adapter = new MockAdapter([
|
||||
toolCallResponse('c1', 'echo', { text: 'ping' }),
|
||||
textResponse('done'),
|
||||
@@ -453,26 +423,21 @@ describe('agent/request-advice (RequestAdvice)', () => {
|
||||
}))
|
||||
const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' })
|
||||
|
||||
let step = 0
|
||||
ctx.on('agent/request-advice', async (_agent, _turn, _step, _messages, _context, next): Promise<RequestAdvice> => {
|
||||
const result = await next()
|
||||
step += 1
|
||||
return { before: [...result.before, { role: 'user', content: [{ type: 'text', text: `reminder v${step}` }] }], after: result.after }
|
||||
})
|
||||
const held: Message = { role: 'user', content: [{ type: 'text', text: 'v1' }] }
|
||||
ctx.on('agent/session-prefix', async (_agent, _prefix, _signal, next): Promise<Message[]> => [...await next(), held])
|
||||
|
||||
send(agent, 'go')
|
||||
await waitForIdle(ctx, agent)
|
||||
|
||||
expect(adapter.requests[0]!.messages[0]).toEqual({ role: 'user', content: [{ type: 'text', text: 'reminder v1' }] })
|
||||
expect(adapter.requests[1]!.messages[0]).toEqual({ role: 'user', content: [{ type: 'text', text: 'reminder v2' }] })
|
||||
// Step 2's changed prefix rides a request/header-delta whose fold matches
|
||||
// what the second request actually sent.
|
||||
const delta = events(agent).find(e => e.type === 'request/header-delta')
|
||||
expect(delta?.type === 'request/header-delta' && delta.data.messagePrefix).toEqual([{ role: 'user', content: [{ type: 'text', text: 'reminder v2' }] }])
|
||||
expect(foldRequestHeader(agent.session.events)?.messagePrefix).toEqual([{ role: 'user', content: [{ type: 'text', text: 'reminder v2' }] }])
|
||||
// The listener mutates the object it contributed AFTER composition; the
|
||||
// cached prefix is a deep-frozen clone, so step 2's request is unchanged.
|
||||
held.content = [{ type: 'text', text: 'v2' }]
|
||||
expect(adapter.requests[1]!.messages[0]).toEqual({ role: 'user', content: [{ type: 'text', text: 'v1' }] })
|
||||
expect(events(agent).filter(e => e.type === 'request/header-delta')).toHaveLength(0)
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
describe('agent/turn-continuation (ContinuationDecision)', () => {
|
||||
it('a continue decision with a reason records next-step steering in the same turn', async () => {
|
||||
const adapter = new MockAdapter([textResponse('step 1 no tools'), textResponse('step 2')])
|
||||
|
||||
Reference in New Issue
Block a user