fix(compact): harden pruning integration (round 2)
This commit is contained in:
@@ -31,7 +31,7 @@ Session log (per session):
|
||||
- **turns pair and nest** — `turn/start` opens a turn, `turn/end` closes the matching one; no overlapping turns.
|
||||
- **steps nest in turns** — `step/start` opens a step in the open turn; `step/end` closes the matching step.
|
||||
- **chunks belong to an open step** — `step/start` precedes its `assistant/chunk`s.
|
||||
- **an appended `tool/result` needs a prior `tool/call`** — fresh `surfaceOp: 'append'` results name the open step and consume its pending call, while a provenance-backed single-node `replace` is a turn-enclosed surface rewrite of an already-executed result. A `tool/call` may still have no result when the execution pipeline throws.
|
||||
- **an appended `tool/result` needs a prior `tool/call`** — fresh `surfaceOp: 'append'` results name the open step and consume its pending call. A replacement exemption applies only to a provenance-backed rewrite of one current `tool/result` node whose complete data is identical except for `content`; it must still be turn-enclosed. A `tool/call` may still have no result when the execution pipeline throws.
|
||||
- **provenance sources are valid and unambiguous** — `sourceEventSeqs` contains unique earlier known seqs; only `assistant/message` may carry an explicit empty list, which denotes a known empty provider stream rather than absent legacy provenance.
|
||||
|
||||
Agent status (per agent):
|
||||
|
||||
@@ -7,6 +7,7 @@
|
||||
* @module @deepseek-ai/dsh-invariants
|
||||
*/
|
||||
|
||||
import { isDeepStrictEqual } from 'node:util'
|
||||
import type { Context } from 'cordis'
|
||||
import { carrierKeyOf, isScopeCarrier } from '@deepseek-ai/dsh-scope'
|
||||
import { assertNever, HarnessError } from '@deepseek-ai/dsh-llm'
|
||||
@@ -50,13 +51,14 @@ interface SessionTrace {
|
||||
pendingCalls: Set<CallId>
|
||||
/** Every seq seen so far — validates `sourceEventSeqs` references. */
|
||||
knownSeqs: Set<number>
|
||||
/**
|
||||
* The seqs currently on the surface linked list, in linked-list order
|
||||
* (head to tail). A replace reorders this relative to seq order (the new
|
||||
* node takes the replaced range's position), so range validation is
|
||||
* positional, not by seq comparison.
|
||||
*/
|
||||
surface: number[]
|
||||
/** Current surface nodes in linked-list order, with immutable event identity. */
|
||||
surface: SurfaceTraceNode[]
|
||||
}
|
||||
|
||||
/** Immutable identity retained only while an event is on the current surface. */
|
||||
interface SurfaceTraceNode {
|
||||
seq: number
|
||||
event: SessionEvent<SurfaceEventType>
|
||||
}
|
||||
|
||||
/** One accepted event's deferred mutation of a live session trace. */
|
||||
@@ -70,8 +72,9 @@ interface SessionTraceTransition {
|
||||
| { kind: 'clear' }
|
||||
/** The event's mutation of the derived surface order. */
|
||||
surface:
|
||||
| { kind: 'none' | 'append' }
|
||||
| { kind: 'replace'; start: number; count: number }
|
||||
| { kind: 'none' }
|
||||
| { kind: 'append'; node: SurfaceTraceNode }
|
||||
| { kind: 'replace'; start: number; count: number; node: SurfaceTraceNode }
|
||||
/** The committed event sequence to add to the known-sequence set. */
|
||||
seq: number
|
||||
}
|
||||
@@ -85,6 +88,18 @@ function requireOpenStep(trace: SessionTrace, kind: string, turn: number, step:
|
||||
}
|
||||
}
|
||||
|
||||
/** Compare future-safe tool-result data while deliberately excluding content. */
|
||||
function sameToolResultDataExceptContent(
|
||||
original: SessionEvent<'tool/result'>['data'],
|
||||
replacement: SessionEvent<'tool/result'>['data'],
|
||||
): boolean {
|
||||
const originalRest = { ...original } as Record<string, unknown>
|
||||
const replacementRest = { ...replacement } as Record<string, unknown>
|
||||
delete originalRest['content']
|
||||
delete replacementRest['content']
|
||||
return isDeepStrictEqual(originalRest, replacementRest)
|
||||
}
|
||||
|
||||
/** Validate one candidate event without mutating the committed session trace. */
|
||||
function validateEvent(trace: SessionTrace, event: SessionEvent): SessionTraceTransition {
|
||||
// seq is strictly monotonic — the spine of replay equivalence. lastSeq
|
||||
@@ -139,14 +154,14 @@ function validateEvent(trace: SessionTrace, event: SessionEvent): SessionTraceTr
|
||||
// positional range — every shadowed node must appear in sourceEventSeqs.
|
||||
if (se.surfaceOp !== undefined) {
|
||||
if (se.surfaceOp === 'append') {
|
||||
surface = { kind: 'append' }
|
||||
surface = { kind: 'append', node: { seq: event.seq, event: se } }
|
||||
} else {
|
||||
const { start, end } = se.surfaceOp
|
||||
const startIdx = trace.surface.indexOf(start)
|
||||
const startIdx = trace.surface.findIndex(node => node.seq === start)
|
||||
if (startIdx === -1) {
|
||||
throw new InvariantError(`surface replace: start seq ${start} is not on the surface`)
|
||||
}
|
||||
const endIdx = trace.surface.indexOf(end)
|
||||
const endIdx = trace.surface.findIndex(node => node.seq === end)
|
||||
if (endIdx === -1) {
|
||||
throw new InvariantError(`surface replace: end seq ${end} is not on the surface`)
|
||||
}
|
||||
@@ -155,13 +170,18 @@ function validateEvent(trace: SessionTrace, event: SessionEvent): SessionTraceTr
|
||||
}
|
||||
// Every node the replace shadows (surface positions [startIdx, endIdx]
|
||||
// inclusive) must appear in sourceEventSeqs — the provenance contract.
|
||||
const shadowed = trace.surface.slice(startIdx, endIdx + 1)
|
||||
const shadowed = trace.surface.slice(startIdx, endIdx + 1).map(node => node.seq)
|
||||
const recorded = new Set(se.sourceEventSeqs ?? [])
|
||||
const missing = shadowed.filter(seq => !recorded.has(seq))
|
||||
if (missing.length > 0) {
|
||||
throw new InvariantError(`surface replace: sourceEventSeqs must include every shadowed surface node; missing ${missing.join(', ')}`)
|
||||
}
|
||||
surface = { kind: 'replace', start: startIdx, count: shadowed.length }
|
||||
surface = {
|
||||
kind: 'replace',
|
||||
start: startIdx,
|
||||
count: shadowed.length,
|
||||
node: { seq: event.seq, event: se },
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -232,15 +252,26 @@ function validateEvent(trace: SessionTrace, event: SessionEvent): SessionTraceTr
|
||||
break
|
||||
}
|
||||
case 'tool/result': {
|
||||
// A replacement rewrites an already-executed result whose recorded
|
||||
// turn/step can be closed. Surface provenance above validates the rewrite;
|
||||
// only fresh appends consume an open step's pending call.
|
||||
// Only a content-only rewrite of one CURRENT tool-result node may bypass
|
||||
// open-step/pending-call checks. The trace retains immutable surface event
|
||||
// identity, so this validation never indexes a mutable or stale session.
|
||||
if (se.surfaceOp !== undefined && se.surfaceOp !== 'append') {
|
||||
if (trace.openTurn === null) {
|
||||
throw new InvariantError(
|
||||
'tool/result surface replacement appended outside any open turn',
|
||||
)
|
||||
}
|
||||
const { start, end } = se.surfaceOp
|
||||
if (start !== end) {
|
||||
throw new InvariantError('tool/result surface replacement must rewrite exactly one current node')
|
||||
}
|
||||
const original = trace.surface.find(node => node.seq === start)?.event
|
||||
if (original?.type !== 'tool/result') {
|
||||
throw new InvariantError('tool/result surface replacement must target a current tool/result')
|
||||
}
|
||||
if (!sameToolResultDataExceptContent(original.data, event.data)) {
|
||||
throw new InvariantError('tool/result surface replacement may change only content')
|
||||
}
|
||||
break
|
||||
}
|
||||
requireOpenStep(trace, 'tool/result', event.data.turn, event.data.step)
|
||||
@@ -302,10 +333,14 @@ function applyTransition(trace: SessionTrace, transition: SessionTraceTransition
|
||||
case 'none':
|
||||
break
|
||||
case 'append':
|
||||
trace.surface.push(transition.seq)
|
||||
trace.surface.push(transition.surface.node)
|
||||
break
|
||||
case 'replace':
|
||||
trace.surface.splice(transition.surface.start, transition.surface.count, transition.seq)
|
||||
trace.surface.splice(
|
||||
transition.surface.start,
|
||||
transition.surface.count,
|
||||
transition.surface.node,
|
||||
)
|
||||
break
|
||||
/* v8 ignore next -- validateEvent produces this closed transition union */
|
||||
default:
|
||||
|
||||
@@ -478,6 +478,39 @@ describe('HMR safety', () => {
|
||||
})
|
||||
|
||||
describe('surface invariants', () => {
|
||||
async function toolResultRewriteFixture() {
|
||||
const { ctx } = await setup()
|
||||
const session = ctx.sessions.create()
|
||||
session.append('turn/start', { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } })
|
||||
const unrelated = session.append('user/message', {
|
||||
content: [{ type: 'text', text: 'request' }],
|
||||
source: { kind: 'user' },
|
||||
}, { surfaceOp: 'append' })
|
||||
session.append('step/start', { turn: 1, step: 1 })
|
||||
session.append('tool/call', {
|
||||
turn: 1,
|
||||
step: 1,
|
||||
callId: CallId('rewrite'),
|
||||
name: 'echo',
|
||||
arguments: '{}',
|
||||
})
|
||||
const originalData = {
|
||||
turn: 1,
|
||||
step: 1,
|
||||
callId: CallId('rewrite'),
|
||||
content: [{ type: 'text' as const, text: 'original' }],
|
||||
isError: true,
|
||||
error: { name: 'ExitError', code: 'EXIT_1' },
|
||||
meta: { presentation: { kind: 'terminal', output: 'full output' } },
|
||||
futureField: { nested: ['preserve', 1] },
|
||||
}
|
||||
const original = session.append('tool/result', originalData, { surfaceOp: 'append' })
|
||||
session.append('step/end', { turn: 1, step: 1 })
|
||||
session.append('turn/end', { turn: 1, reason: { kind: 'completed' } })
|
||||
session.append('turn/start', { turn: 2, trigger: { kind: 'message', source: { kind: 'user' } } })
|
||||
return { session, unrelated, original }
|
||||
}
|
||||
|
||||
it('accepts well-formed surface metadata', async () => {
|
||||
const { ctx } = await setup()
|
||||
const session = ctx.sessions.create()
|
||||
@@ -501,27 +534,7 @@ describe('surface invariants', () => {
|
||||
})
|
||||
|
||||
it('treats a provenance-backed tool-result replacement as a turn-enclosed rewrite', async () => {
|
||||
const { ctx } = await setup()
|
||||
const session = ctx.sessions.create()
|
||||
session.append('turn/start', { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } })
|
||||
session.append('step/start', { turn: 1, step: 1 })
|
||||
session.append('tool/call', {
|
||||
turn: 1,
|
||||
step: 1,
|
||||
callId: CallId('rewrite'),
|
||||
name: 'echo',
|
||||
arguments: '{}',
|
||||
})
|
||||
const original = session.append('tool/result', {
|
||||
turn: 1,
|
||||
step: 1,
|
||||
callId: CallId('rewrite'),
|
||||
content: [{ type: 'text', text: 'original' }],
|
||||
isError: false,
|
||||
}, { surfaceOp: 'append' })
|
||||
session.append('step/end', { turn: 1, step: 1 })
|
||||
session.append('turn/end', { turn: 1, reason: { kind: 'completed' } })
|
||||
session.append('turn/start', { turn: 2, trigger: { kind: 'message', source: { kind: 'user' } } })
|
||||
const { session, original } = await toolResultRewriteFixture()
|
||||
|
||||
expect(() => session.append('tool/result', {
|
||||
...original.data,
|
||||
@@ -532,6 +545,47 @@ describe('surface invariants', () => {
|
||||
})).not.toThrow()
|
||||
})
|
||||
|
||||
it('rejects a tool-result replacement targeting an unrelated current node', async () => {
|
||||
const { session, unrelated, original } = await toolResultRewriteFixture()
|
||||
expect(() => session.append('tool/result', {
|
||||
...original.data,
|
||||
content: [{ type: 'text', text: 'forged' }],
|
||||
}, {
|
||||
surfaceOp: { op: 'replace', start: unrelated.seq, end: unrelated.seq },
|
||||
sourceEventSeqs: [unrelated.seq],
|
||||
})).toThrow(/must target a current tool\/result/)
|
||||
})
|
||||
|
||||
it('rejects a multi-node tool-result replacement even with complete provenance', async () => {
|
||||
const { session, unrelated, original } = await toolResultRewriteFixture()
|
||||
expect(() => session.append('tool/result', {
|
||||
...original.data,
|
||||
content: [{ type: 'text', text: 'forged' }],
|
||||
}, {
|
||||
surfaceOp: { op: 'replace', start: unrelated.seq, end: original.seq },
|
||||
sourceEventSeqs: [unrelated.seq, original.seq],
|
||||
})).toThrow(/must rewrite exactly one current node/)
|
||||
})
|
||||
|
||||
it.each([
|
||||
['callId', { callId: CallId('forged') }],
|
||||
['turn', { turn: 2 }],
|
||||
['step', { step: 2 }],
|
||||
['error', { error: { name: 'ExitError', code: 'DIFFERENT' } }],
|
||||
['meta', { meta: { presentation: { kind: 'generic' } } }],
|
||||
['future data', { futureField: { nested: ['changed'] } }],
|
||||
])('rejects a content rewrite with altered %s', async (_label, altered) => {
|
||||
const { session, original } = await toolResultRewriteFixture()
|
||||
expect(() => session.append('tool/result', {
|
||||
...original.data,
|
||||
...altered,
|
||||
content: [{ type: 'text', text: 'pruned' }],
|
||||
}, {
|
||||
surfaceOp: { op: 'replace', start: original.seq, end: original.seq },
|
||||
sourceEventSeqs: [original.seq],
|
||||
})).toThrow(/may change only content/)
|
||||
})
|
||||
|
||||
it('accepts known-empty assistant provenance and rejects empty provenance elsewhere', async () => {
|
||||
const { ctx } = await setup()
|
||||
const session = ctx.sessions.create()
|
||||
|
||||
Reference in New Issue
Block a user