Skip to content

Commit fecf8c0

Browse files
committed
fix(agent-runtime): never wipe chat history on compaction
Two verified paths replaced the model's entire context with nothing: 1. /compact built its replacement summary from fullResponse without checking it was non-empty. A silent empty stop (finish part with a real reason, no content) yields no recovery chunk, so the whole history was replaced by a single message whose summary carried nothing — every earlier turn gone. Guard the replacement on fullResponse.trim() and keep the history (the forced next step retries the summary) when the model returned nothing. 2. Compaction identified its own summary by content: a user message containing <conversation_summary> and the header sentence. A user message quoting those markers (asking about this very mechanism, pasting a summary back) matched, findLast picked it over the real summary, and the real summary was then neither re-parsed nor kept — every earlier turn vanished at the next compaction. The inlined copy in the context-pruner matched the bare tag alone, so a message merely mentioning the tag wiped memory there too. Stamp summaries with a CONVERSATION_SUMMARY tag and recognize them by provenance; keep a legacy fallback requiring the full envelope (open+close tag, header, <historical_memory>) so pre-tag summaries still fold in. The pruner's tag-alone match — the parity test's deliberate divergence — closes: a bare tag mention no longer qualifies in either implementation. Refs #1166
1 parent 28663a3 commit fecf8c0

5 files changed

Lines changed: 210 additions & 19 deletions

File tree

‎agents/context-pruner.ts‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,20 @@ const definition: AgentDefinition = {
449449

450450
function isConversationSummary(message: Message): boolean {
451451
if (message.role !== 'user') return false
452-
return getTextContent(message).includes('<conversation_summary>')
452+
// Provenance tag first — see CONVERSATION_SUMMARY_TAG in
453+
// packages/agent-runtime/src/compact-history.ts for why identity by
454+
// content alone is unsafe (a user message quoting the markers used to
455+
// steal the summary's identity and erase the older memory).
456+
if (message.tags?.includes('CONVERSATION_SUMMARY')) return true
457+
// Legacy fallback for summaries written before the tag existed: require
458+
// the full envelope, not just the bare tag a user can easily send.
459+
const text = getTextContent(message)
460+
return (
461+
text.includes('<conversation_summary>') &&
462+
text.includes('</conversation_summary>') &&
463+
text.includes(SUMMARY_HEADER) &&
464+
text.includes('<historical_memory>')
465+
)
453466
}
454467

455468
function extractSummaryContent(message: Message): string {
@@ -854,6 +867,7 @@ ${SUMMARY_DISCLAIMER}`,
854867
role: 'user',
855868
content: summaryContentParts,
856869
sentAt: now,
870+
tags: ['CONVERSATION_SUMMARY'],
857871
}
858872

859873
const continuationMessage: UserMessage = {

‎packages/agent-runtime/src/__tests__/compact-history.test.ts‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,67 @@ describe('compactMessages', () => {
188188
).toHaveLength(1)
189189
})
190190

191+
it('does not let a user message quoting the markers steal the summary identity', () => {
192+
// The 2026-08-31 wipe: a user message containing BOTH the tag and the
193+
// header (asking about this very mechanism, pasting a summary back)
194+
// matched isConversationSummary, so findLast picked the quote over the
195+
// real summary. The real summary was then neither re-parsed nor kept as
196+
// history — every earlier turn vanished from the model's context.
197+
const first = compact([
198+
user('the original request about auth', ['USER_PROMPT']),
199+
assistant('refactored the auth module'),
200+
])
201+
const quote = user(
202+
'what is <conversation_summary>? e.g. "This is a summary of the conversation so far. The original messages have been condensed to save context space." — explain it',
203+
['USER_PROMPT'],
204+
)
205+
const second = compactMessages({
206+
messages: [...first, assistant('more work'), quote],
207+
})
208+
209+
// The real memory survives the second compaction.
210+
expect(second.stats.previous_summary_entry_count).toBeGreaterThan(0)
211+
expect(textOf(second.messages[0])).toContain('the original request about auth')
212+
// The quote is the live prompt, preserved as a real message — not eaten.
213+
// (It comes back re-stamped with a fresh sentAt, so compare content.)
214+
expect(textOf(second.messages.at(-1)!)).toContain('what is <conversation_summary>')
215+
})
216+
217+
it('still recognizes a legacy summary by its full envelope', () => {
218+
// Summaries written before the CONVERSATION_SUMMARY tag existed carry no
219+
// tag, so identity falls back to the full envelope — open tag, header,
220+
// close tag AND <historical_memory>. A bare tag-plus-header quote must
221+
// not qualify.
222+
const legacySummary = user(
223+
'<conversation_summary>\nThis is a summary of the conversation so far. The original messages have been condensed to save context space.\n\n<historical_memory>\n[USER]\nthe legacy request\n</historical_memory>\n</conversation_summary>',
224+
)
225+
const result = compactMessages({
226+
messages: [legacySummary, assistant('and then some work')],
227+
})
228+
229+
expect(result.stats.previous_summary_entry_count).toBeGreaterThan(0)
230+
expect(textOf(result.messages[0])).toContain('the legacy request')
231+
})
232+
233+
it('does not fold in a quote that has the tag and header but no memory block', () => {
234+
// A tag-plus-header quote reproduces the pre-tag identity check. It lacks
235+
// <historical_memory>, so the legacy fallback must reject it — the memory
236+
// it would have "been" belongs to the real, newer summary.
237+
const first = compact([
238+
user('the original request', ['USER_PROMPT']),
239+
assistant('working on it'),
240+
])
241+
const quote = user(
242+
'<conversation_summary>\nThis is a summary of the conversation so far. The original messages have been condensed to save context space.',
243+
)
244+
const result = compactMessages({
245+
messages: [...first, quote],
246+
})
247+
248+
expect(result.stats.previous_summary_entry_count).toBeGreaterThan(0)
249+
expect(textOf(result.messages[0])).toContain('the original request')
250+
})
251+
191252
it('spends the two budgets independently: a flood of tool work keeps user prompts', () => {
192253
const { messages, stats } = compactMessages({
193254
messages: [

‎packages/agent-runtime/src/__tests__/context-pruner-parity.test.ts‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -392,14 +392,16 @@ describe('context-pruner parity', () => {
392392
})
393393

394394
/**
395-
* Deliberate divergence #2. The pruner treats any user message containing
396-
* `<conversation_summary>` as a memory artifact — dropping it from the
397-
* history and re-parsing it as entries — which silently eats a user message
398-
* that merely mentions the tag. The runtime additionally requires the header
399-
* its own envelope always carries. This matters more here because the
400-
* cache-expiry trigger compacts on ordinary idle turns.
395+
* Former divergence #2, closed. The pruner used to treat any user message
396+
* containing `<conversation_summary>` as a memory artifact — dropping it
397+
* from the history and re-parsing it as entries, which silently ate a user
398+
* message that merely mentions the tag, and (as findLast picks the LAST
399+
* match) let a quoting message steal the real summary's identity and erase
400+
* the older memory. Both implementations now recognize summaries by the
401+
* CONVERSATION_SUMMARY tag they stamp, with a legacy full-envelope fallback
402+
* that a bare tag mention does not satisfy.
401403
*/
402-
it('keeps a user message that only mentions the tag, where the pruner eats it', () => {
404+
it('keeps a user message that only mentions the tag, in both implementations', () => {
403405
const history: Message[] = [
404406
user('why does it emit <conversation_summary> around the memory?'),
405407
assistant('because the model needs a delimiter'),
@@ -412,7 +414,7 @@ describe('context-pruner parity', () => {
412414
const prunerMemory = textOfFirst(runPruner(history))
413415

414416
expect(runtimeMemory).toContain('why does it emit')
415-
expect(prunerMemory).not.toContain('why does it emit')
417+
expect(prunerMemory).toContain('why does it emit')
416418
})
417419

418420
it('matches the pruner when a budget evicts old entries', () => {

‎packages/agent-runtime/src/__tests__/main-prompt.test.ts‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -445,4 +445,101 @@ describe('mainPrompt', () => {
445445

446446
expect(output.type).toBeDefined() // Output should exist even for empty response
447447
})
448+
449+
it('does not replace the history with an empty summary on /compact', async () => {
450+
// A silent empty stop yields no recovery chunk, so an unguarded /compact
451+
// replacement would collapse the whole history into one summary message
452+
// carrying nothing — every earlier turn gone from the model's context.
453+
mockAgentStream([])
454+
455+
const sessionState = getInitialSessionState(mockFileContext)
456+
sessionState.mainAgentState.messageHistory = [
457+
{
458+
role: 'user' as const,
459+
content: [{ type: 'text' as const, text: 'earlier turn: fix the login bug' }],
460+
sentAt: 1,
461+
},
462+
{
463+
role: 'assistant' as const,
464+
content: [{ type: 'text' as const, text: 'fixed it' }],
465+
sentAt: 2,
466+
},
467+
]
468+
const action = {
469+
type: 'prompt' as const,
470+
prompt: '/compact',
471+
sessionState,
472+
fingerprintId: 'test',
473+
costMode: 'normal' as const,
474+
promptId: 'test',
475+
toolResults: [],
476+
}
477+
478+
const { sessionState: newSessionState } = await mainPrompt({
479+
...mainPromptBaseParams,
480+
action,
481+
localAgentTemplates: mockLocalAgentTemplates,
482+
})
483+
484+
// The history survives: no empty summary message, earlier turns intact.
485+
const history = newSessionState.mainAgentState.messageHistory
486+
expect(
487+
history.some((m) => textOfHistoryMessage(m).includes('The following is a summary')),
488+
).toBe(false)
489+
expect(
490+
history.some((m) => textOfHistoryMessage(m).includes('earlier turn: fix the login bug')),
491+
).toBe(true)
492+
})
493+
494+
it('still replaces the history on /compact when the model produced a summary', async () => {
495+
mockAgentStream([{ type: 'text', text: 'Summary: the user asked to fix the login bug, which was fixed.' }])
496+
497+
const sessionState = getInitialSessionState(mockFileContext)
498+
sessionState.mainAgentState.messageHistory = [
499+
{
500+
role: 'user' as const,
501+
content: [{ type: 'text' as const, text: 'earlier turn: fix the login bug' }],
502+
sentAt: 1,
503+
},
504+
{
505+
role: 'assistant' as const,
506+
content: [{ type: 'text' as const, text: 'fixed it' }],
507+
sentAt: 2,
508+
},
509+
]
510+
const action = {
511+
type: 'prompt' as const,
512+
prompt: '/compact',
513+
sessionState,
514+
fingerprintId: 'test',
515+
costMode: 'normal' as const,
516+
promptId: 'test',
517+
toolResults: [],
518+
}
519+
520+
const { sessionState: newSessionState } = await mainPrompt({
521+
...mainPromptBaseParams,
522+
action,
523+
localAgentTemplates: mockLocalAgentTemplates,
524+
})
525+
526+
const history = newSessionState.mainAgentState.messageHistory
527+
expect(history).toHaveLength(1)
528+
expect(textOfHistoryMessage(history[0])).toContain(
529+
'Summary: the user asked to fix the login bug',
530+
)
531+
})
448532
})
533+
534+
function textOfHistoryMessage(message: { content: unknown }): string {
535+
const content = message.content as unknown
536+
if (typeof content === 'string') return content
537+
if (Array.isArray(content)) {
538+
return content
539+
.map((part: { type?: string; text?: string }) =>
540+
part.type === 'text' && typeof part.text === 'string' ? part.text : '',
541+
)
542+
.join('\n')
543+
}
544+
return ''
545+
}

‎packages/agent-runtime/src/compact-history.ts‎

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -366,25 +366,41 @@ const SCAFFOLDING_TAGS = [
366366
'SUBAGENT_SPAWN',
367367
]
368368

369+
/** Message tag stamped on the summary message this module (and the inlined
370+
* copy in agents/context-pruner.ts) produces, so the next compaction finds
371+
* the real memory artifact by provenance instead of by content. Identity by
372+
* content is unsafe: a USER message that quotes the summary markers — asking
373+
* about this very mechanism, pasting an old summary back — used to be taken
374+
* for the real one. The `findLast` then picked the quote, the actual summary
375+
* was dropped with the rest of the history, and every earlier turn vanished
376+
* from the model's context at the next compaction. */
377+
export const CONVERSATION_SUMMARY_TAG = 'CONVERSATION_SUMMARY'
378+
369379
/**
370380
* Recognizes a memory artifact this module (or the context-pruner) produced.
371381
*
372-
* Both markers are required, and that is the point. A summary is dropped from
373-
* the history and re-parsed into entries, so anything mistaken for one is
374-
* silently eaten — and the bare `<conversation_summary>` tag is a string a user
375-
* can easily send, most obviously when asking about this very code. Requiring
376-
* the header too means only text that reproduces our envelope qualifies.
382+
* The tag is the identity: only messages this pass itself produced carry it,
383+
* and a user message can never gain it by content alone. The envelope check
384+
* below is a legacy fallback only — summaries written before the tag existed
385+
* carry no marker, so re-summarizing them (which preserves their text as an
386+
* entry, nested once) beats losing them. It requires the FULL envelope
387+
* including `<historical_memory>`: a user message that merely quotes the tag
388+
* and the header must not qualify.
377389
*
378-
* The context-pruner matches on the tag alone. That is a deliberate divergence
379-
* (see the parity test): it matters much more here, because the cache-expiry
380-
* trigger compacts on ordinary idle turns rather than only near the context
381-
* limit, so a user message can meet a compaction pass within minutes.
390+
* The context-pruner matches on the tag alone in its pre-tag copy. That is a
391+
* deliberate divergence to port (see the parity test): a bare
392+
* `<conversation_summary>` in a user message is easy to send, most obviously
393+
* when asking about this very code.
382394
*/
383395
function isConversationSummary(message: Message): boolean {
384396
if (message.role !== 'user') return false
397+
if (message.tags?.includes(CONVERSATION_SUMMARY_TAG)) return true
385398
const text = getTextContent(message)
386399
return (
387-
text.includes('<conversation_summary>') && text.includes(SUMMARY_HEADER)
400+
text.includes('<conversation_summary>') &&
401+
text.includes('</conversation_summary>') &&
402+
text.includes(SUMMARY_HEADER) &&
403+
text.includes('<historical_memory>')
388404
)
389405
}
390406

@@ -743,6 +759,7 @@ ${SUMMARY_DISCLAIMER}`,
743759
role: 'user',
744760
content: [textPart, ...imageParts],
745761
sentAt,
762+
tags: [CONVERSATION_SUMMARY_TAG],
746763
}
747764
}
748765

0 commit comments

Comments
 (0)