Skip to content

Commit 1ce7547

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 923cc79 commit 1ce7547

6 files changed

Lines changed: 222 additions & 21 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
@@ -361,25 +361,41 @@ const SCAFFOLDING_TAGS = [
361361
'SUBAGENT_SPAWN',
362362
]
363363

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

@@ -738,6 +754,7 @@ ${SUMMARY_DISCLAIMER}`,
738754
role: 'user',
739755
content: [textPart, ...imageParts],
740756
sentAt,
757+
tags: [CONVERSATION_SUMMARY_TAG],
741758
}
742759
}
743760

packages/agent-runtime/src/run-agent-step.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -559,11 +559,16 @@ export const runAgentStep = async (
559559
'agentStep',
560560
)
561561

562-
// Handle /compact command: replace message history with the summary
562+
// Handle /compact command: replace message history with the summary — but
563+
// only when the model actually produced one. An empty completion (silent
564+
// stop with a bare finish part; it yields no recovery chunk) must not
565+
// replace the history: the single summary message would carry nothing and
566+
// every earlier turn would be gone from the model's context. Leaving the
567+
// history intact lets the forced next step retry the summary.
563568
const wasCompacted =
564569
prompt &&
565570
(prompt.toLowerCase() === '/compact' || prompt.toLowerCase() === 'compact')
566-
if (wasCompacted) {
571+
if (wasCompacted && fullResponse.trim()) {
567572
agentState.messageHistory = [
568573
userMessage(
569574
withSystemTags(
@@ -572,6 +577,11 @@ export const runAgentStep = async (
572577
),
573578
]
574579
logger.debug({ summary: fullResponse }, 'Compacted messages')
580+
} else if (wasCompacted) {
581+
logger.warn(
582+
{ promptLength: prompt.length },
583+
'Compact requested but the model returned no summary; keeping the message history intact',
584+
)
575585
}
576586

577587
const hasNoToolResults =

0 commit comments

Comments
 (0)