diff --git a/agents/context-pruner.ts b/agents/context-pruner.ts index 187077c476..42634ef6dd 100644 --- a/agents/context-pruner.ts +++ b/agents/context-pruner.ts @@ -449,7 +449,20 @@ const definition: AgentDefinition = { function isConversationSummary(message: Message): boolean { if (message.role !== 'user') return false - return getTextContent(message).includes('') + // Provenance tag first — see CONVERSATION_SUMMARY_TAG in + // packages/agent-runtime/src/compact-history.ts for why identity by + // content alone is unsafe (a user message quoting the markers used to + // steal the summary's identity and erase the older memory). + if (message.tags?.includes('CONVERSATION_SUMMARY')) return true + // Legacy fallback for summaries written before the tag existed: require + // the full envelope, not just the bare tag a user can easily send. + const text = getTextContent(message) + return ( + text.includes('') && + text.includes('') && + text.includes(SUMMARY_HEADER) && + text.includes('') + ) } function extractSummaryContent(message: Message): string { @@ -854,6 +867,7 @@ ${SUMMARY_DISCLAIMER}`, role: 'user', content: summaryContentParts, sentAt: now, + tags: ['CONVERSATION_SUMMARY'], } const continuationMessage: UserMessage = { diff --git a/packages/agent-runtime/src/__tests__/compact-history.test.ts b/packages/agent-runtime/src/__tests__/compact-history.test.ts index b38e8b3ac0..71ff43e44f 100644 --- a/packages/agent-runtime/src/__tests__/compact-history.test.ts +++ b/packages/agent-runtime/src/__tests__/compact-history.test.ts @@ -188,6 +188,67 @@ describe('compactMessages', () => { ).toHaveLength(1) }) + it('does not let a user message quoting the markers steal the summary identity', () => { + // The 2026-08-31 wipe: a user message containing BOTH the tag and the + // header (asking about this very mechanism, pasting a summary back) + // matched isConversationSummary, so findLast picked the quote over the + // real summary. The real summary was then neither re-parsed nor kept as + // history — every earlier turn vanished from the model's context. + const first = compact([ + user('the original request about auth', ['USER_PROMPT']), + assistant('refactored the auth module'), + ]) + const quote = user( + 'what is ? e.g. "This is a summary of the conversation so far. The original messages have been condensed to save context space." — explain it', + ['USER_PROMPT'], + ) + const second = compactMessages({ + messages: [...first, assistant('more work'), quote], + }) + + // The real memory survives the second compaction. + expect(second.stats.previous_summary_entry_count).toBeGreaterThan(0) + expect(textOf(second.messages[0])).toContain('the original request about auth') + // The quote is the live prompt, preserved as a real message — not eaten. + // (It comes back re-stamped with a fresh sentAt, so compare content.) + expect(textOf(second.messages.at(-1)!)).toContain('what is ') + }) + + it('still recognizes a legacy summary by its full envelope', () => { + // Summaries written before the CONVERSATION_SUMMARY tag existed carry no + // tag, so identity falls back to the full envelope — open tag, header, + // close tag AND . A bare tag-plus-header quote must + // not qualify. + const legacySummary = user( + '\nThis is a summary of the conversation so far. The original messages have been condensed to save context space.\n\n\n[USER]\nthe legacy request\n\n', + ) + const result = compactMessages({ + messages: [legacySummary, assistant('and then some work')], + }) + + expect(result.stats.previous_summary_entry_count).toBeGreaterThan(0) + expect(textOf(result.messages[0])).toContain('the legacy request') + }) + + it('does not fold in a quote that has the tag and header but no memory block', () => { + // A tag-plus-header quote reproduces the pre-tag identity check. It lacks + // , so the legacy fallback must reject it — the memory + // it would have "been" belongs to the real, newer summary. + const first = compact([ + user('the original request', ['USER_PROMPT']), + assistant('working on it'), + ]) + const quote = user( + '\nThis is a summary of the conversation so far. The original messages have been condensed to save context space.', + ) + const result = compactMessages({ + messages: [...first, quote], + }) + + expect(result.stats.previous_summary_entry_count).toBeGreaterThan(0) + expect(textOf(result.messages[0])).toContain('the original request') + }) + it('spends the two budgets independently: a flood of tool work keeps user prompts', () => { const { messages, stats } = compactMessages({ messages: [ diff --git a/packages/agent-runtime/src/__tests__/context-pruner-parity.test.ts b/packages/agent-runtime/src/__tests__/context-pruner-parity.test.ts index 7ea7854e84..9fc4ba160a 100644 --- a/packages/agent-runtime/src/__tests__/context-pruner-parity.test.ts +++ b/packages/agent-runtime/src/__tests__/context-pruner-parity.test.ts @@ -392,14 +392,16 @@ describe('context-pruner parity', () => { }) /** - * Deliberate divergence #2. The pruner treats any user message containing - * `` as a memory artifact — dropping it from the - * history and re-parsing it as entries — which silently eats a user message - * that merely mentions the tag. The runtime additionally requires the header - * its own envelope always carries. This matters more here because the - * cache-expiry trigger compacts on ordinary idle turns. + * Former divergence #2, closed. The pruner used to treat any user message + * containing `` as a memory artifact — dropping it + * from the history and re-parsing it as entries, which silently ate a user + * message that merely mentions the tag, and (as findLast picks the LAST + * match) let a quoting message steal the real summary's identity and erase + * the older memory. Both implementations now recognize summaries by the + * CONVERSATION_SUMMARY tag they stamp, with a legacy full-envelope fallback + * that a bare tag mention does not satisfy. */ - it('keeps a user message that only mentions the tag, where the pruner eats it', () => { + it('keeps a user message that only mentions the tag, in both implementations', () => { const history: Message[] = [ user('why does it emit around the memory?'), assistant('because the model needs a delimiter'), @@ -412,7 +414,7 @@ describe('context-pruner parity', () => { const prunerMemory = textOfFirst(runPruner(history)) expect(runtimeMemory).toContain('why does it emit') - expect(prunerMemory).not.toContain('why does it emit') + expect(prunerMemory).toContain('why does it emit') }) it('matches the pruner when a budget evicts old entries', () => { diff --git a/packages/agent-runtime/src/__tests__/main-prompt.test.ts b/packages/agent-runtime/src/__tests__/main-prompt.test.ts index 841c5953a0..3028c69201 100644 --- a/packages/agent-runtime/src/__tests__/main-prompt.test.ts +++ b/packages/agent-runtime/src/__tests__/main-prompt.test.ts @@ -445,4 +445,101 @@ describe('mainPrompt', () => { expect(output.type).toBeDefined() // Output should exist even for empty response }) + + it('does not replace the history with an empty summary on /compact', async () => { + // A silent empty stop yields no recovery chunk, so an unguarded /compact + // replacement would collapse the whole history into one summary message + // carrying nothing — every earlier turn gone from the model's context. + mockAgentStream([]) + + const sessionState = getInitialSessionState(mockFileContext) + sessionState.mainAgentState.messageHistory = [ + { + role: 'user' as const, + content: [{ type: 'text' as const, text: 'earlier turn: fix the login bug' }], + sentAt: 1, + }, + { + role: 'assistant' as const, + content: [{ type: 'text' as const, text: 'fixed it' }], + sentAt: 2, + }, + ] + const action = { + type: 'prompt' as const, + prompt: '/compact', + sessionState, + fingerprintId: 'test', + costMode: 'normal' as const, + promptId: 'test', + toolResults: [], + } + + const { sessionState: newSessionState } = await mainPrompt({ + ...mainPromptBaseParams, + action, + localAgentTemplates: mockLocalAgentTemplates, + }) + + // The history survives: no empty summary message, earlier turns intact. + const history = newSessionState.mainAgentState.messageHistory + expect( + history.some((m) => textOfHistoryMessage(m).includes('The following is a summary')), + ).toBe(false) + expect( + history.some((m) => textOfHistoryMessage(m).includes('earlier turn: fix the login bug')), + ).toBe(true) + }) + + it('still replaces the history on /compact when the model produced a summary', async () => { + mockAgentStream([{ type: 'text', text: 'Summary: the user asked to fix the login bug, which was fixed.' }]) + + const sessionState = getInitialSessionState(mockFileContext) + sessionState.mainAgentState.messageHistory = [ + { + role: 'user' as const, + content: [{ type: 'text' as const, text: 'earlier turn: fix the login bug' }], + sentAt: 1, + }, + { + role: 'assistant' as const, + content: [{ type: 'text' as const, text: 'fixed it' }], + sentAt: 2, + }, + ] + const action = { + type: 'prompt' as const, + prompt: '/compact', + sessionState, + fingerprintId: 'test', + costMode: 'normal' as const, + promptId: 'test', + toolResults: [], + } + + const { sessionState: newSessionState } = await mainPrompt({ + ...mainPromptBaseParams, + action, + localAgentTemplates: mockLocalAgentTemplates, + }) + + const history = newSessionState.mainAgentState.messageHistory + expect(history).toHaveLength(1) + expect(textOfHistoryMessage(history[0])).toContain( + 'Summary: the user asked to fix the login bug', + ) + }) }) + +function textOfHistoryMessage(message: { content: unknown }): string { + const content = message.content as unknown + if (typeof content === 'string') return content + if (Array.isArray(content)) { + return content + .map((part: { type?: string; text?: string }) => + part.type === 'text' && typeof part.text === 'string' ? part.text : '', + ) + .join('\n') + } + return '' +} diff --git a/packages/agent-runtime/src/compact-history.ts b/packages/agent-runtime/src/compact-history.ts index cbf5d8ac56..601b4f4119 100644 --- a/packages/agent-runtime/src/compact-history.ts +++ b/packages/agent-runtime/src/compact-history.ts @@ -366,25 +366,41 @@ const SCAFFOLDING_TAGS = [ 'SUBAGENT_SPAWN', ] +/** Message tag stamped on the summary message this module (and the inlined + * copy in agents/context-pruner.ts) produces, so the next compaction finds + * the real memory artifact by provenance instead of by content. Identity by + * content is unsafe: a USER message that quotes the summary markers — asking + * about this very mechanism, pasting an old summary back — used to be taken + * for the real one. The `findLast` then picked the quote, the actual summary + * was dropped with the rest of the history, and every earlier turn vanished + * from the model's context at the next compaction. */ +export const CONVERSATION_SUMMARY_TAG = 'CONVERSATION_SUMMARY' + /** * Recognizes a memory artifact this module (or the context-pruner) produced. * - * Both markers are required, and that is the point. A summary is dropped from - * the history and re-parsed into entries, so anything mistaken for one is - * silently eaten — and the bare `` tag is a string a user - * can easily send, most obviously when asking about this very code. Requiring - * the header too means only text that reproduces our envelope qualifies. + * The tag is the identity: only messages this pass itself produced carry it, + * and a user message can never gain it by content alone. The envelope check + * below is a legacy fallback only — summaries written before the tag existed + * carry no marker, so re-summarizing them (which preserves their text as an + * entry, nested once) beats losing them. It requires the FULL envelope + * including ``: a user message that merely quotes the tag + * and the header must not qualify. * - * The context-pruner matches on the tag alone. That is a deliberate divergence - * (see the parity test): it matters much more here, because the cache-expiry - * trigger compacts on ordinary idle turns rather than only near the context - * limit, so a user message can meet a compaction pass within minutes. + * The context-pruner matches on the tag alone in its pre-tag copy. That is a + * deliberate divergence to port (see the parity test): a bare + * `` in a user message is easy to send, most obviously + * when asking about this very code. */ function isConversationSummary(message: Message): boolean { if (message.role !== 'user') return false + if (message.tags?.includes(CONVERSATION_SUMMARY_TAG)) return true const text = getTextContent(message) return ( - text.includes('') && text.includes(SUMMARY_HEADER) + text.includes('') && + text.includes('') && + text.includes(SUMMARY_HEADER) && + text.includes('') ) } @@ -743,6 +759,7 @@ ${SUMMARY_DISCLAIMER}`, role: 'user', content: [textPart, ...imageParts], sentAt, + tags: [CONVERSATION_SUMMARY_TAG], } }