fix review findings: one frozen seed through the waterfall; stale agent/request docs
Codex diff review, round 1, two (A) findings: - The agent/request fallback resolved the RAW seed object — on later steps the session's cached header fold — so a delegating listener (await next(), mutate, return) could rewrite the fold in place and the change would compare as already-baseline: no delta logged, the persisted log unable to reconstruct the request (the dev invariant would fire on the divergence, but the log would still lie). One structuredClone'd, deep-frozen seed now serves both the listener chain and the fallback — in-place shaping after delegation throws — and Session.requestHeader() freezes its fold on update, so the leak class is unrepresentable from either side. Pinned by a loop-level delegating-mutator test. - Doc sweep for the old contract: agent README's event row (mutate GenerateOptions / tool filtering → frozen config seed, replacement out, logged header), compact-basic's module JSDoc (summarize routed through agent/request → direct one-shot at llm/stream), and architecture.md's event-domain line (request mutation → call-config shaping).
This commit is contained in:
@@ -694,17 +694,23 @@ async function runStep(
|
||||
// baseline, which is what keeps fork model-overrides and resume-time
|
||||
// reconfiguration correct. Later steps seed from the log's folded header,
|
||||
// which by then is exactly what this instance last logged.
|
||||
const seedConfig: LlmCallConfig = transmission.loggedHeader
|
||||
// One deep-cloned, frozen seed serves BOTH the listener chain and the
|
||||
// no-listener fallback: structuredClone decouples it from the session's
|
||||
// cached header fold (a raw reference would let a delegating listener
|
||||
// mutate the fold in place and silently skip the delta log), and the freeze
|
||||
// makes in-place shaping unrepresentable — a switch is a RETURNED
|
||||
// replacement, which the header event below records.
|
||||
const seedConfig: LlmCallConfig = deepFreeze(structuredClone(transmission.loggedHeader
|
||||
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion -- loggedHeader ⟹ a snapshot is in the log
|
||||
? session.requestHeader()!.config
|
||||
: { model: options.model ?? '' }
|
||||
: { model: options.model ?? '' }))
|
||||
|
||||
// Shape the call config: listeners return a replacement to switch model or
|
||||
// sampling (the seed is frozen — content shaping is not expressible here;
|
||||
// model-visible content flows through the log channels). The header event
|
||||
// below records whatever the request ACTUALLY uses, so a listener's switch
|
||||
// is a logged, reconstructable fact, never silent drift.
|
||||
const config = await ctx.waterfall('agent/request', agent, turn, step, deepFreeze({ ...seedConfig }), () => Promise.resolve(seedConfig))
|
||||
const config = await ctx.waterfall('agent/request', agent, turn, step, seedConfig, () => Promise.resolve(seedConfig))
|
||||
if (!config.model) {
|
||||
throw new Error(`agent "${agent.id}" has no model: set AgentOptions.model or supply one via the agent/request waterfall`)
|
||||
}
|
||||
|
||||
@@ -240,6 +240,35 @@ describe('request stability across the loop', () => {
|
||||
expectPrefixExtension(adapter.requests[0]!, adapter2.requests[0]!)
|
||||
})
|
||||
|
||||
it('a delegating listener cannot mutate the seed through next() — the fold stays log-true', async () => {
|
||||
const adapter = new MockAdapter([textResponse('one'), textResponse('two')])
|
||||
const ctx = await harness(adapter)
|
||||
const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' })
|
||||
|
||||
ctx.on('agent/request', async (_agent, _turn, _step, _config, next) => {
|
||||
const config = await next()
|
||||
// next() resolves the SAME frozen seed — in-place shaping after
|
||||
// delegation is unrepresentable, so a "mutate what next() returned"
|
||||
// listener cannot desync the log from the request (nor reach the
|
||||
// session's cached header fold, which is deep-cloned away and itself
|
||||
// frozen).
|
||||
expect(Object.isFrozen(config)).toBe(true)
|
||||
expect(() => { (config as { temperature?: number }).temperature = 0.9 }).toThrow(TypeError)
|
||||
return config
|
||||
})
|
||||
|
||||
send(agent, 'first')
|
||||
await waitForIdle(ctx, agent)
|
||||
send(agent, 'second')
|
||||
await waitForIdle(ctx, agent)
|
||||
|
||||
// No delta was logged (nothing really changed), and the session's own
|
||||
// fold is immutable state.
|
||||
expect(agent.session.events.filter(e => e.type === 'request/header-delta')).toHaveLength(0)
|
||||
expect(Object.isFrozen(agent.session.requestHeader())).toBe(true)
|
||||
expect(adapter.requests[1]!.temperature).toBeUndefined()
|
||||
})
|
||||
|
||||
it('THEOREM: every request rebuilds byte-equal from the session log alone', async () => {
|
||||
const adapter = new MockAdapter([
|
||||
toolCallResponse('c1', 'echo', { text: 'one' }, 'calling'),
|
||||
|
||||
@@ -44,7 +44,7 @@ Turn and step boundaries are NOT mirrored as `agent/*` emits: a consumer that ne
|
||||
- `agent/session-start` (emit) — fired once before the first turn; a listener seeds context via `agent.inject()` (it cannot veto startup).
|
||||
- `agent/prompt-submit` — decide what happens to one drained queued message before it becomes a `user/message`: `PromptDecision` = `allow` (optionally rewriting the prompt `content` or attaching `additionalContext`) or `block` (drop it; a batch whose every prompt is blocked opens a zero-step turn that ends `rejected`). Maps onto Claude Code's `UserPromptSubmit`.
|
||||
- `agent/pre-step` (serial) — mutate the session surface before the step opens and history is derived (compaction). Fires after `turn/start` and before `step/start`, so a listener's appended events land outside the step.
|
||||
- `agent/request` — mutate `GenerateOptions` before the model call (hooks, model switching, tool filtering)
|
||||
- `agent/request` — shape the call config before the model call: a frozen `LlmCallConfig` seed in, a replacement out (model switching, sampling overrides). Content is not shapeable here — every request is a pure function of the session log ([reconstructability RFC](../../../docs/rfc/implemented/architecture/2026-07-05-reconstructable-requests.md)); the loop logs whatever config the request actually uses as a `request/header*` event
|
||||
- `agent/step-result` — post-process the assembled assistant message before tool dispatch (validates what the log records)
|
||||
- `agent/turn-continuation` — override the continue/stop decision via `ContinuationDecision` = `{action:'stop'}` or `{action:'continue', reason?}` (a `continue` `reason` is recorded as next-step steering in the same turn — the typed `/goal` pattern). Force-continue `/loop`, force-stop budget guard.
|
||||
|
||||
|
||||
@@ -252,7 +252,11 @@ export class Session {
|
||||
*/
|
||||
requestHeader(): EpochHeader | undefined {
|
||||
if (this.headerFoldSeq < this.log.length) {
|
||||
this.headerFold = foldRequestHeader(this.log.slice(this.headerFoldSeq), this.headerFold)
|
||||
// Frozen on update: the fold is session state exposed by reference — a
|
||||
// consumer mutating it in place (instead of building a replacement)
|
||||
// would desync every later comparison against the log, so mutation
|
||||
// throws instead.
|
||||
this.headerFold = deepFreeze(foldRequestHeader(this.log.slice(this.headerFoldSeq), this.headerFold))
|
||||
this.headerFoldSeq = this.log.length
|
||||
}
|
||||
return this.headerFold
|
||||
|
||||
Reference in New Issue
Block a user