fix(agent): correct session-prefix composition-order docs; scrub messagePrefix in snapshots

Two ds-review-bot findings:

The seam JSDoc claimed extending 'await next()' composes in
registration order — false for the append form: the waterfall unwinds
innermost-first, so appending places later-registered contributions
first. The canonical contribution is now documented as the PREPEND
'[mine, ...await next()]' (registration order on the wire), with the
append form's reverse-order behavior stated explicitly; the ordering
test now uses the canonical pattern in both listeners.

scrubRequestHeaders tokenized only system/tools, so a fixture recording
a composed session prefix would carry its raw text (workspace-specific
churn/leak). The scrubber now maps each header/delta messagePrefix
entry to a {{messagePrefix}} token — count stays a structural fact,
absence stays absent, the empty-array transition stays visible — with
normalize.spec coverage for the header, delta, absence, and odd-shape
paths.
This commit is contained in:
Yichen Jiang
2026-07-08 16:16:08 +08:00
parent b04cfe3a41
commit 77333c0a19
6 changed files with 76 additions and 24 deletions

View File

@@ -352,23 +352,24 @@ describe('agent/session-prefix', () => {
expect(agent.session.deriveMessages()[0]).toEqual({ role: 'user', content: [{ type: 'text', text: 'go' }] })
})
it('contributions compose across listeners in registration order', async () => {
it('the canonical prepend pattern composes contributions in registration order', async () => {
const adapter = new MockAdapter([textResponse('ok')])
const ctx = await harness(adapter)
const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' })
// Both listeners use the canonical `[mine, ...await next()]` prepend: the
// waterfall unwinds innermost-first (the second listener's array is built
// first), so prepending puts the FIRST-registered contribution first.
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/session-prefix', async (_agent, _prefix, _signal, next): Promise<Message[]> => {
return [...await next(), { role: 'user', content: [{ type: 'text', text: 'second' }] }]
return [{ role: 'user', content: [{ type: 'text', text: 'second' }] }, ...await next()]
})
send(agent, 'hi')
await waitForIdle(ctx, agent)
// Registration order composes: the first listener runs last on the way
// 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'])
})

View File

@@ -408,9 +408,14 @@ declare module 'cordis' {
* durable `context/message` paid once and prefix-cached thereafter.
*
* The seed is a frozen empty list; a contributing listener returns a NEW
* array extending `await next()` (`[...prefix, mine]` — never an in-place
* push), so contributions compose across plugins in registration order
* and compose deterministically for a fixed plugin set. Call `next()` to
* array — never an in-place push. The canonical contribution is a
* PREPEND, `[mine, ...await next()]`: the waterfall unwinds
* innermost-first (the LAST-registered listener's `next()` resolves
* first), so prepending yields registration order on the wire, and every
* plugin using it composes deterministically. The append form
* `[...await next(), mine]` is legal but places a contribution AFTER
* every later-registered plugin's — reverse registration order when all
* contributors append. Call `next()` to
* delegate, or return a list without it to short-circuit.
* @param agent - the agent whose session prefix is being composed.
* @param prefix - the frozen empty seed; return an extended replacement to contribute.

View File

@@ -13,8 +13,9 @@
* (deterministic — `seq = log.length`, part of the event-log contract).
*
* A separate, composable normalizer — {@link scrubRequestHeaders} — replaces
* the bulky request-header CONTENT (the composed system prompt and the tool
* schema list) with `{{system}}`/`{{tools}}` tokens. It is deliberately NOT
* the bulky request-header CONTENT (the composed system prompt, the tool
* schema list, and the session prefix) with
* `{{system}}`/`{{tools}}`/`{{messagePrefix}}` tokens. It is deliberately NOT
* folded into {@link normalizeSessionLog}: each suite's one header-pinning
* scenario compares that content verbatim, every other scenario composes the
* scrub in (the `pinsHeader` flag on the scenario table, consumed by the suite
@@ -30,6 +31,7 @@ const SESSION_ID = '{{sessionId}}'
const CWD = '{{cwd}}'
const SYSTEM = '{{system}}'
const TOOLS = '{{tools}}'
const MESSAGE_PREFIX = '{{messagePrefix}}'
/** A UUID v4 string, the shape `randomUUID()` produces for session ids. */
const UUID_RE = /[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}/gi
@@ -135,15 +137,22 @@ export function normalizeSessionLog(rawLog: string, ctx: NormalizeContext): stri
/**
* Replace request-header CONTENT in a session JSONL with stable tokens,
* keeping its structure: a `request/header` event's `data.header.system` →
* `{{system}}` and `data.header.tools` → `{{tools}}`; a
* `{{system}}`, `data.header.tools` → `{{tools}}`, and
* `data.header.messagePrefix` → one `{{messagePrefix}}` token per message
* (the session prefix is model-visible bulk — an AGENTS digest, a skills
* catalog — so its COUNT stays a structural fact while its text never lands
* in a fixture); a
* `request/header-delta` event keeps every structural fact — the system
* delta's `keepStart`/`keepEnd` line positions and inserted-line COUNT (one
* `{{system}}` token per inserted line), the tools delta's
* added/removed/changed tool NAMES — and tokenizes only the bulk (prompt
* text; each added/changed schema's fields other than `name` → `{{tools}}`),
* added/removed/changed tool NAMES, the prefix replacement's message COUNT —
* and tokenizes only the bulk (prompt
* text; each added/changed schema's fields other than `name` → `{{tools}}`;
* each replacement prefix message → `{{messagePrefix}}`),
* so two different deltas still compare different.
* Absent fields stay absent — WHETHER a header carried a system prompt or
* tools is behavior and stays visible; `config` and `reason` are small and
* Absent fields stay absent — WHETHER a header carried a system prompt,
* tools, or a prefix is behavior and stays visible; `config` and `reason`
* are small and
* stable, so they stay verbatim (a model swap churns every fixture by design
* — it invalidates the recorded responses; a prompt/schema edit churns none —
* replay never reads this content, see dsh-llm-replay).
@@ -166,9 +175,10 @@ export function scrubRequestHeaders(rawLog: string): string {
if (record.type === 'request/header') {
const header = data.header as Record<string, unknown> | null | undefined
if (header === null || typeof header !== 'object') return line
if (!('system' in header) && !('tools' in header)) return line
if (!('system' in header) && !('tools' in header) && !('messagePrefix' in header)) return line
if ('system' in header) header.system = SYSTEM
if ('tools' in header) header.tools = TOOLS
if (Array.isArray(header.messagePrefix)) header.messagePrefix = header.messagePrefix.map(() => MESSAGE_PREFIX)
return JSON.stringify(record)
}
if (record.type === 'request/header-delta') {
@@ -183,6 +193,10 @@ export function scrubRequestHeaders(rawLog: string): string {
if (Array.isArray(tools.added)) { tools.added = tools.added.map(scrubToolSchema); touched = true }
if (Array.isArray(tools.changed)) { tools.changed = tools.changed.map(scrubToolSchema); touched = true }
}
if (Array.isArray(data.messagePrefix)) {
data.messagePrefix = data.messagePrefix.map(() => MESSAGE_PREFIX)
touched = true
}
return touched ? JSON.stringify(record) : line
}
return line

View File

@@ -155,6 +155,38 @@ describe('scrubRequestHeaders', () => {
expect(toolsOnly).not.toContain('{{system}}')
})
it('scrubs the header session prefix to one token per message, keeping the count', () => {
const ev = headerEvent({
config: { model: 'm' },
messagePrefix: [
{ role: 'user', content: [{ type: 'text', text: 'workspace AGENTS digest' }] },
{ role: 'user', content: [{ type: 'text', text: 'skills catalog' }] },
],
})
const out = scrubRequestHeaders(`${headerLine}\n${ev}\n`)
expect(out).toContain('"messagePrefix":["{{messagePrefix}}","{{messagePrefix}}"]')
expect(out).not.toContain('AGENTS digest')
expect(out).not.toContain('skills catalog')
// Absence stays absent — a prefix-less header gains no token…
expect(scrubRequestHeaders(`${headerLine}\n${headerEvent({ system: 's' })}\n`)).not.toContain('{{messagePrefix}}')
// …and a non-array shape passes through untouched.
const odd = JSON.stringify({ type: 'request/header', seq: 4, time: 9, data: { header: { config: { model: 'm' }, messagePrefix: 'weird' }, reason: 'initial' } })
expect(scrubRequestHeaders(`${headerLine}\n${odd}\n`)).toContain('"messagePrefix":"weird"')
})
it('scrubs a header-delta prefix replacement to one token per message', () => {
const delta = JSON.stringify({
type: 'request/header-delta', seq: 8, time: 9,
data: { messagePrefix: [{ role: 'user', content: [{ type: 'text', text: 'leaked opener' }] }] },
})
const out = scrubRequestHeaders(`${headerLine}\n${delta}\n`)
expect(out).toContain('"messagePrefix":["{{messagePrefix}}"]')
expect(out).not.toContain('leaked opener')
// The empty-array transition-to-absence stays a structural fact.
const toNone = JSON.stringify({ type: 'request/header-delta', seq: 9, time: 9, data: { messagePrefix: [] } })
expect(scrubRequestHeaders(`${headerLine}\n${toNone}\n`)).toContain('"messagePrefix":[]')
})
it('leaves a delta with no scrubbable payload byte-identical (config-only, or non-array shapes)', () => {
const configOnly = JSON.stringify({ type: 'request/header-delta', seq: 8, time: 9, data: { config: { model: 'm2' } } })
const oddShapes = JSON.stringify({ type: 'request/header-delta', seq: 9, time: 9, data: { system: { insert: 'not-an-array' }, tools: null } })