From d29d2ba7f370785c5393e02c90615f483a78af8a Mon Sep 17 00:00:00 2001 From: Yaowei Zheng Date: Mon, 27 Jul 2026 22:45:44 +0800 Subject: [PATCH] fix(core): reject empty compaction summaries and offer no tools to compaction requests (#84) Co-authored-by: Claude Fable 5 --- packages/core/src/engine/context-engine.ts | 186 +++++++-- packages/core/src/trace/resume.ts | 32 +- packages/core/test/compaction.test.ts | 427 +++++++++++++++++++++ packages/core/test/replay.test.ts | 38 +- packages/docs/content/agent-loop.en.md | 2 + packages/docs/content/agent-loop.zh.md | 2 + 6 files changed, 636 insertions(+), 51 deletions(-) diff --git a/packages/core/src/engine/context-engine.ts b/packages/core/src/engine/context-engine.ts index 8812916..79b9b4d 100644 --- a/packages/core/src/engine/context-engine.ts +++ b/packages/core/src/engine/context-engine.ts @@ -191,6 +191,19 @@ export interface ContextEngineDeps { /** Whether compaction is possible; when not `ok`, `compact()` is a no-op and yields no messages (see ContextEngine.compactability). */ export type CompactAvailability = "ok" | "unsupported" | "empty" | "just_compacted"; +/** + * Maximum summarize attempts when the compaction response is rejected as an invalid summary + * (empty extracted text, or the model answered with tool calls — issue #83/#84). Deliberately + * separate from (and larger than) `compactionMaxReconnects`: that cap governs transport-level + * timeout/malformed attempts that were never committed and back off exponentially, while a + * rejection is a well-formed committed response — the request itself works, the model just + * didn't produce a summary, so the repaired input is resent immediately with no backoff. And + * since the compaction request keeps the session's toolset (the prefix cache must stay valid, + * see summarizeContext), a model insisting on tools deserves several chances. + * Beyond this many rejected attempts the compaction fails (original context kept). + */ +const MAX_SUMMARY_REJECTIONS = 5; + /** Result of executing one LLM turn (the return value of runTurn). */ interface TurnResult { /** All tool outputs for this turn, reordered to match the original tool_call order (for the next turn's LLM input). */ @@ -1007,13 +1020,22 @@ export class ContextEngine { * `summarize` compaction: appends the compaction Prompt to the **old** LLM object (first * folding in all of this turn's tool results when mid-Task, to keep tool_use/tool_result * pairing), then extracts the `[summary]` and wraps it as `[context_summary]` user text. The + * compaction request carries the session's toolset **unchanged** — the request prefix must + * stay byte-identical to ordinary turns so the provider's prompt cache remains valid; + * compaction runs exactly when the context is largest, where re-billing the whole + * transcript uncached costs tens of times more (issue #84 — this is why tools are *not* + * omitted and no `tool_choice` override is used). The * compaction request's streamed output is not pushed to the Human output stream (it emits * paired compaction events, plus the compaction request's `token_usage` — positioned between * the two events, so the frontend can count compaction cost into its stats), but it is written - * to the old Trace. timeout/malformed reconnect via the existing retry mechanism under the - * compaction-specific cap (`compactionMaxReconnects`, tighter than the turn loop's ladder), - * collapsing to failed once retries are exhausted; on failure/abort, the original context and - * Trace index are kept — it does not fall back to discard. + * to the old Trace. Compaction succeeds only with a **valid summary** — non-empty extracted + * text and no tool calls in the response. An invalid summary is rejected: any tool calls the + * model issued are answered with synthesized failed outputs (pairing repair, see the loop + * body) and the repaired input is resent immediately, up to MAX_SUMMARY_REJECTIONS attempts, + * then the compaction fails. timeout/malformed reconnect via the existing retry mechanism + * under the compaction-specific cap (`compactionMaxReconnects`, tighter than the turn loop's + * ladder), collapsing to failed once retries are exhausted; on failure/abort, the original + * context and Trace index are kept — it does not fall back to discard. * Docs: /docs/agent-loop § "Compaction". */ private async *summarizeContext( @@ -1030,30 +1052,91 @@ export class ContextEngine { // executed and aren't recorded again, while carry-over's not-yet-written synthetic content // (flatten text, backfilled placeholders) and the compaction Prompt are written now. const prompt = userText(settings.prompt); - const input = [...pendingToolOutputs, prompt]; + const baseInput = [...pendingToolOutputs, prompt]; + let input = baseInput; await this.write(prompt); + // Synthesized outputs answering the latest rejected attempt's tool calls, not yet carried + // by a committed request: prepended to the retry input, and stashed as carry-over should + // the compaction be abandoned first (see stashRepairs). + let pendingRepairs: OmniMessage[] = []; + // Two independent retry budgets: transport-level timeout/malformed attempts (never + // committed) follow the compaction-specific reconnect cap with the exponential backoff + // ladder; invalid-summary rejections (committed, well-formed responses that just aren't + // summaries) get the larger dedicated cap and resend immediately — see + // MAX_SUMMARY_REJECTIONS and the rejection branch below. let reconnects = 0; + let rejections = 0; for (;;) { if (signal?.aborted) { + this.stashRepairs(pendingRepairs); yield* this.emitCompactionEnd(reason, "summarize", "aborted"); return { status: "aborted" }; } const attempt = await this.runCompactionRequest(input, signal, reconnects); if (attempt.status === "completed") { - // The compaction request's token_usage is pushed to the Human output stream (already - // written to Trace in runCompactionRequest, so here it's only yielded, not rewritten); - // the frontend uses this to count compaction cost into stats and display it on the - // compaction-complete line. - if (attempt.usage) yield attempt.usage; - // Lenient extraction: if the output lacks a [summary] tag, use the entire compaction - // output as-is rather than treating it as a failure. - const summary = userText(buildContextSummaryText(extractSummary(attempt.text))); - yield* this.emitCompactionEnd(reason, "summarize", "completed"); - await this.startNewContext(); - return { status: "completed", summary }; + // The attempt was committed by AgentHub, so whatever its input carried — including + // repairs synthesized for a previous rejection — is now in history and must not be + // resent. + pendingRepairs = []; + // A completed response counts as a compaction success only when it is a **usable + // summary**: the extracted text is non-empty and the response called no tool. The + // extraction itself stays lenient (output without a [summary] tag is used verbatim), + // but committing an empty `[context_summary]` would discard the whole context and + // lose the task state, and a tool-calling response is not a summary at all — with the + // session's tools offered (prefix-cache invariant), a model deciding to use one is a + // live possibility, not just a hallucination (issue #83). + const summaryText = extractSummary(attempt.text); + if (summaryText !== "" && attempt.toolCalls.length === 0) { + // The compaction request's token_usage is pushed to the Human output stream (already + // written to Trace in runCompactionRequest, so here it's only yielded, not rewritten); + // the frontend uses this to count compaction cost into stats and display it on the + // compaction-complete line. Only the adopted attempt's usage is surfaced: rejected + // attempts still feed observeTokenUsage (Session cumulative cost and context + // tracking stay correct), so the displayed compaction cost deliberately understates + // the true spend when retries happened — chosen so the line reflects the attempt + // that produced the summary. + if (attempt.usage) yield attempt.usage; + const summary = userText(buildContextSummaryText(summaryText)); + yield* this.emitCompactionEnd(reason, "summarize", "completed"); + await this.startNewContext(); + return { status: "completed", summary }; + } + // Rejected. Tool calls were never dispatched, yet the assistant turn holding them IS + // committed on the live LLM object — leaving them unanswered would get every + // subsequent request rejected by the provider (unanswered tool_use, issue #33): the + // exact state this file's other safety nets exist to prevent. Answer each call with a + // synthesized failed output (the same shape executeOne uses), written to Trace so + // resume replays the identical pairing, and prepended to the retry input so the + // provider sees tool_use/tool_result paired. The empty-text rejection needs no repair: + // that committed turn is plain assistant text/thinking, and re-sending the compaction + // Prompt on top of it is structurally sound. + rejections += 1; + pendingRepairs = attempt.toolCalls.map((tc) => + toolCallOutput({ + output: "[tool error] the compaction request expects a summary, not tool calls", + toolCallId: tc.payload.tool_call_id, + stopReason: "failed", + }), + ); + for (const repair of pendingRepairs) await this.write(repair); + // Rebuild from baseInput rather than appending: everything the rejected attempt's + // input carried is committed, so only the fresh repairs and the Prompt go out again. + input = pendingRepairs.length > 0 ? [...pendingRepairs, ...baseInput] : baseInput; + if (rejections >= MAX_SUMMARY_REJECTIONS) { + this.stashRepairs(pendingRepairs); + yield* this.emitCompactionEnd(reason, "summarize", "failed"); + return { status: "failed" }; + } + // A rejection is model behavior, not a transport failure: the request pipeline is + // healthy, so the repaired input is resent immediately — no backoff and no + // retry_in_ms announcement (the rejected attempt's request_end carries status + // completed, for which plannedRetryDelayMs yields nothing). The exponential ladder + // below belongs to transport failures only. + continue; } if (attempt.status === "aborted") { + this.stashRepairs(pendingRepairs); yield* this.emitCompactionEnd(reason, "summarize", "aborted"); return { status: "aborted" }; } @@ -1062,21 +1145,24 @@ export class ContextEngine { // completed/failed/aborted set, the original context is kept, and the host learns // about the credential problem from the request's own terminal status (a turn-loop // request will surface it; the compaction request_end is Trace-only). + this.stashRepairs(pendingRepairs); yield* this.emitCompactionEnd(reason, "summarize", "failed"); return { status: "failed" }; } - // timeout / malformed: retried via reconnect. The compaction request was never committed - // by AgentHub (case B), so the original input is resent unchanged. Compaction uses its - // own, tighter cap (not the shared maxReconnects): a failed compaction keeps the - // original context and retries on the next trigger, so failing fast beats holding the - // session through the full exponential ladder. + // timeout / malformed: retried via reconnect — transport-level, never committed by + // AgentHub (case B), so the input (any pending repairs included) is resent unchanged. + // Compaction uses its own, tighter cap (not the shared maxReconnects): a failed + // compaction keeps the original context and retries on the next trigger, so failing + // fast beats holding the session through the full exponential ladder. if (reconnects >= this.compactionMaxReconnects) { + this.stashRepairs(pendingRepairs); yield* this.emitCompactionEnd(reason, "summarize", "failed"); return { status: "failed" }; } reconnects += 1; const ok = await this.backoff(reconnects, signal); if (!ok) { + this.stashRepairs(pendingRepairs); yield* this.emitCompactionEnd(reason, "summarize", "aborted"); return { status: "aborted" }; } @@ -1084,19 +1170,44 @@ export class ContextEngine { } /** - * Issues one compaction request (an ordinary LLM Request): consumes the old LLM object's - * streamed output but **does not push it to the Human output stream** (except `token_usage` - * — captured and handed back via the return value for summarizeContext to yield); complete + * Holds synthesized repair outputs as carry-over when a summarize compaction is abandoned + * (failed/aborted) while the latest rejected attempt's tool calls are still unanswered: the + * next run's first request (or the next manual compaction, which folds carry-over in) sends + * them ahead of everything else, completing the tool_use/tool_result pairing on the live + * LLM object that the provider would otherwise reject every subsequent request over. The + * repairs were already written to Trace at synthesis time, and carry-over is never rewritten + * at send time, so no duplicate Trace entries arise. + */ + private stashRepairs(repairs: OmniMessage[]): void { + if (repairs.length === 0) return; + this.pendingCarryOver = [...repairs, ...this.pendingCarryOver]; + } + + /** + * Issues one compaction request — an ordinary LLM Request through the same object and the + * same frozen config as every other turn (the toolset is deliberately identical: a changed + * tool list would change the request prefix and invalidate the provider's prompt cache at + * the moment the context is largest, issue #84). Consumes the old LLM object's streamed + * output but **does not push it to the Human output stream** (except `token_usage` — + * captured and handed back via the return value for summarizeContext to yield); complete * messages and events are written to the old Trace; complete text segments are collected as - * the compaction output. Token usage is counted into the Session cumulative totals (recorded - * via observeTokenUsage, for the new object to carry forward). + * the compaction output, and `toolCalls` collects the response's real tool requests (never + * dispatched — summarizeContext rejects such a response as not-a-summary and answers each + * call with a synthesized failed output). + * Token usage is counted into the Session + * cumulative totals (recorded via observeTokenUsage, for the new object to carry forward). */ private async runCompactionRequest( input: OmniMessage[], signal?: AbortSignal, - /** Retries already performed by the compaction loop (its request_end announces the next planned backoff too). */ + /** Transport retries already performed by the compaction loop (its request_end announces the next planned backoff too). */ reconnectsSoFar = 0, - ): Promise<{ status: StopReason; text: string; usage: OmniMessage | null }> { + ): Promise<{ + status: StopReason; + text: string; + toolCalls: OmniMessage[]; + usage: OmniMessage | null; + }> { // The compaction request is itself an ordinary Request, emitting paired request events — // written to the (old) Trace only, not pushed to the stream, keeping the compaction process // invisible to Human. @@ -1106,6 +1217,7 @@ export class ContextEngine { ...(signal ? { signal } : {}), }); let text = ""; + const toolCalls: OmniMessage[] = []; let usage: OmniMessage | null = null; for (;;) { const res = await gen.next(); @@ -1114,7 +1226,9 @@ export class ContextEngine { // request_end, under the compaction cap. Compaction request events are written to // the old Trace only (never streamed), so retry_in_ms lands in the Trace record — // no live countdown renders for compaction; the frontend only sees the - // compaction event pair. + // compaction event pair. A rejected summary ends `completed`, for which + // plannedRetryDelayMs yields nothing — rejection resends are immediate (see + // summarizeContext), so no wait is ever announced for them. await this.write( requestEnd( res.value.status, @@ -1126,13 +1240,21 @@ export class ContextEngine { ), ), ); - return { status: res.value.status, text, usage }; + return { status: res.value.status, text, toolCalls, usage }; } const msg = res.value; await this.write(msg); if (this.observeTokenUsage(msg)) usage = msg; - if (isCompleteModelMessage(msg) && msg.payload.type === "text") { - text += (msg.payload as TextPayload).text; + if (isCompleteModelMessage(msg)) { + if (msg.payload.type === "text") { + text += (msg.payload as TextPayload).text; + } else if (msg.payload.type === "tool_call") { + // Same filter as the turn loop: a tool_call synthesized to close out an interruption + // carries a non-completed stop_reason — it is structural closure, not a real request, + // and gets no paired output. + const tc = msg as OmniMessage; + if (tc.payload.stop_reason === "completed") toolCalls.push(tc); + } } } } diff --git a/packages/core/src/trace/resume.ts b/packages/core/src/trace/resume.ts index d2296c5..46f014c 100644 --- a/packages/core/src/trace/resume.ts +++ b/packages/core/src/trace/resume.ts @@ -226,14 +226,21 @@ export function resumeTrace(messages: OmniMessage[]): ResumeResult { renderMessages: [], meta, }; - if (p.mode === "summarize") { - // Reconstruct the summary from the compaction request's output (the assistant text of - // the last completed Request). Always rebuilt in the current [context_summary] form — - // extractSummary itself still accepts the old tags an old Trace may contain. - const summaryText = lastCompletedRequestText(messages); - result.pendingSummary = userText(buildContextSummaryText(extractSummary(summaryText))); + if (p.mode !== "summarize") return result; + // Reconstruct the summary from the compaction request's output (the assistant text of + // the last completed Request). Always rebuilt in the current [context_summary] form — + // extractSummary itself still accepts the old tags an old Trace may contain. + const summaryText = extractSummary(lastCompletedRequestText(messages)); + if (summaryText !== "") { + result.pendingSummary = userText(buildContextSummaryText(summaryText)); + return result; } - return result; + // The compaction output extracts to an **empty** summary: only a pre-#83 engine could + // have written such a "completed" closure (compaction now fails instead of committing an + // empty summary, which would erase the task state). Mirror the current contract: void + // the closure and fall through to turn-by-turn replay — the original context this file + // still holds is restored, and the committed compaction turn stays in history like any + // committed turn. } } @@ -320,7 +327,11 @@ export function resumeTrace(messages: OmniMessage[]): ResumeResult { committedCallIds.add(p.tool_call_id); } } - sessionTurns += 1; + // A committed request inside a compaction span enters history as usual (AgentHub + // committed it — e.g. an invalid-summary attempt of a compaction that then failed), + // but does not count as a Session turn: in-process only runTurn increments the + // counter, never runCompactionRequest. + if (!inCompaction) sessionTurns += 1; snapshot = []; outputs = []; inRequest = false; @@ -433,8 +444,9 @@ function lastCompletedRequestText(messages: OmniMessage[]): string { { // A completed request with empty text still overwrites (we take the text of “the last // completed request”, even if empty) — otherwise a textless compaction output would fall - // back to an earlier turn's normal reply and get mistakenly injected as the summary; the - // in-process path yields an empty summary here (extractSummary(“”)). + // back to an earlier turn's normal reply and get mistakenly injected as the summary. The + // caller treats the resulting empty extract as a void closure (a pre-#83 trace shape) + // and replays the original context instead. if (msg.payload.status === "completed") text = current; inRequest = false; } diff --git a/packages/core/test/compaction.test.ts b/packages/core/test/compaction.test.ts index bbd8c54..854e6bb 100644 --- a/packages/core/test/compaction.test.ts +++ b/packages/core/test/compaction.test.ts @@ -8,6 +8,12 @@ * - summarize: appends a compaction prompt to the old LLM (merging in all of this round's tool * results first if mid-task); the summary is wrapped as a `[context_summary]` user text and fed * as the first input to the new LLM instance; on failure the original context is kept, never downgraded to discard. + * The compaction request carries the session's toolset unchanged (the prompt-cache prefix must + * stay byte-identical, #84), and a completed response only counts as success with a valid + * summary — non-empty extracted text and no tool calls (issue #83). A rejected response has its + * tool calls answered by synthesized failed outputs (pairing repair) and is retried under a + * dedicated cap of 5 rejections; then the compaction fails. Transport timeout/malformed + * attempts keep the shared reconnect cap. * - discard: deferred until task end if mid-task; sends no compaction request, just swaps in a new LLM instance directly. * - Process visibility: the compaction request's streamed output is never surfaced to the human, * only the paired compaction events are emitted; the dialogue is written to the old trace, and @@ -21,6 +27,7 @@ import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { assistantText, sessionMeta, + thinkingMessage, tokenUsage, toolCall, toolCallOutput, @@ -43,6 +50,8 @@ import type { } from "../src/interfaces.js"; import { ContextEngine } from "../src/engine/context-engine.js"; import type { CompactionSettings } from "../src/engine/context-engine.js"; +import { GenerativeModel } from "../src/llm/index.js"; +import type { UniConfig, UniEvent, UniMessage } from "@prismshadow/agenthub"; import { Writer, readTrace } from "../src/trace/index.js"; // --------------------------------------------------------------------------- @@ -457,6 +466,424 @@ describe("context compaction", () => { expect(retryPlans).toEqual([undefined, 1, 2, undefined]); }); + it("an empty compaction response (thinking only, no text) is rejected: 5 attempts, then failed with the context kept", async () => { + // Issue #83: the compaction request completes but yields no text. Committing the empty + // summary would discard the whole context and lose the task state — the response is + // rejected and retried under the dedicated rejection cap (5 attempts, #84), then the + // compaction fails while the original context and Trace file stay current. + const empty = (n: number): ScriptedResponse => ({ + messages: [thinkingMessage(`pondering, attempt ${n}, no text`)], + }); + const llm1 = new ScriptedLLM( + [ + { messages: [assistantText("answer one"), usage(150, 150)] }, + // Five completed-but-empty compaction attempts: the dedicated cap allows exactly 5 + // rejections. compactionMaxReconnects is 1 here on purpose — rejections must NOT + // consume the transport reconnect budget, or the loop would stop after 2 attempts. + empty(1), + empty(2), + empty(3), + empty(4), + empty(5), + // Original context kept: the next run stays on this instance (usage under the + // threshold here so the failed compaction isn't immediately retriggered). + { messages: [assistantText("continuing on the old context"), usage(60, 370)] }, + ], + "llm1", + ); + let created = 0; + const trace = new Writer({ tracesDir: traces, sessionId: "sess_empty" }); + const engine = new ContextEngine({ + llm: llm1, + environment: fakeEnvironment, + trace, + sessionMeta: metaMessage, + compaction: settings(), + createLLM: () => { + created += 1; + return new ScriptedLLM([], "llm2"); + }, + compactionMaxReconnects: 1, + reconnectBackoffMs: 1, + }); + const oldPath = trace.currentPath(); + + const out1 = await collect(engine.run([userText("task one")], { approve: allowAll })); + + // Exactly one event pair, ending failed — an empty summary is never a completed compaction. + const events = compactionEvents(out1); + expect( + events.map((e) => `${e.type}:${(e as Partial).status ?? ""}`), + ).toEqual(["compaction_begin:", "compaction_end:failed"]); + // No rejected attempt's token_usage is surfaced (only a successful compaction yields + // its usage between the paired events). + const types1 = payloadTypes(out1); + const between = out1.slice( + types1.indexOf("compaction_begin") + 1, + types1.lastIndexOf("compaction_end"), + ); + expect(between.filter((m) => (m.payload as { type?: string }).type === "token_usage")).toEqual( + [], + ); + // Turn + exactly five compaction attempts (the 5th rejection exhausts the cap, no 6th + // request), each resending the prompt unchanged — an empty rejection needs no repair. + expect(llm1.calls).toHaveLength(6); + for (let i = 1; i <= 5; i += 1) { + expect(llm1.calls[i]!.map(textOf)).toEqual(["COMPACT NOW"]); + } + // Rejection resends are immediate, never announced: no compaction request_end carries a + // retry_in_ms (they all end `completed`, unlike the transport ladder's timeout ends). + const rejectionPlans = (await readTrace(oldPath)) + .filter((m) => (m.payload as { type?: string }).type === "request_end") + .map((m) => (m.payload as { retry_in_ms?: number }).retry_in_ms); + expect(rejectionPlans.every((p) => p === undefined)).toBe(true); + // No LLM swap and no Trace rotation: the old file is still current. + expect(created).toBe(0); + expect(trace.currentPath()).toBe(oldPath); + + // Subsequent turns still run on the original context: the same instance serves the next + // run and its input is the plain new prompt — no [context_summary] injected. + await collect(engine.run([userText("task two")], { approve: allowAll })); + expect(llm1.calls).toHaveLength(7); + expect(llm1.calls[6]!.map(textOf)).toEqual(["task two"]); + expect(trace.currentPath()).toBe(oldPath); + expect(await readdir(dirname(oldPath))).toEqual(["sess_empty_001.jsonl"]); + }); + + it("tool-calling rejections exhaust the 5-attempt cap: every call is paired, and the next ordinary turn stays clean", async () => { + // A tool-calling response is not a summary — even when it also carries plausible summary + // text — but its assistant turn IS committed on the live LLM object. Each rejection's + // calls are answered with synthesized failed outputs (written to Trace, prepended to the + // retried input), so no tool_use ever dangles: after the compaction fails, the same + // object must still serve ordinary turns with a well-formed history (#84 review). + const callWith = (id: string): ScriptedResponse => ({ + messages: [ + assistantText("[summary]looks plausible[/summary]"), + toolCall({ name: "t", arguments: "{}", toolCallId: id }), + ], + }); + const llm1 = new ScriptedLLM( + [ + { messages: [assistantText("answer"), usage(150, 150)] }, + callWith("c1"), + callWith("c2"), + callWith("c3"), + callWith("c4"), + callWith("c5"), + { messages: [assistantText("still on the old context"), usage(60, 300)] }, + ], + "llm1", + ); + let created = 0; + const trace = new Writer({ tracesDir: traces, sessionId: "sess_paired" }); + const engine = new ContextEngine({ + llm: llm1, + environment: fakeEnvironment, + trace, + sessionMeta: metaMessage, + compaction: settings(), + createLLM: () => { + created += 1; + return new ScriptedLLM([], "llm2"); + }, + compactionMaxReconnects: 1, + reconnectBackoffMs: 1, + }); + const oldPath = trace.currentPath(); + + const out = await collect(engine.run([userText("go")], { approve: allowAll })); + const events = compactionEvents(out); + expect(events[1]).toMatchObject({ type: "compaction_end", status: "failed" }); + // The rejected tool calls are never approved or executed, and nothing of the compaction + // dialogue (repairs included) reaches the output stream. + expect(payloadTypes(out)).not.toContain("approval_decision"); + expect(payloadTypes(out)).not.toContain("tool_call_output"); + expect(created).toBe(0); + + // Five attempts; from the second on, the input leads with the repair answering the + // previous rejection's call, then re-issues the prompt. + expect(llm1.calls).toHaveLength(6); + for (let attempt = 2; attempt <= 5; attempt += 1) { + const retry = llm1.calls[attempt]!; + expect(payloadTypes(retry)).toEqual(["tool_call_output", "text"]); + const repair = retry[0]!.payload as { + tool_call_id: string; + output: string; + stop_reason?: string; + }; + expect(repair.tool_call_id).toBe(`c${attempt - 1}`); + expect(repair.output).toBe( + "[tool error] the compaction request expects a summary, not tool calls", + ); + expect(repair.stop_reason).toBe("failed"); + expect(textOf(retry[1]!)).toBe("COMPACT NOW"); + } + + // All five synthesized repairs are written to the (old) Trace for replay to mirror. + const repairIds = (await readTrace(oldPath)) + .filter((m) => { + const p = m.payload as { type?: string; output?: string }; + return ( + p.type === "tool_call_output" && + p.output === "[tool error] the compaction request expects a summary, not tool calls" + ); + }) + .map((m) => (m.payload as { tool_call_id: string }).tool_call_id); + expect(repairIds).toEqual(["c1", "c2", "c3", "c4", "c5"]); + + // The next ordinary turn on the SAME engine runs cleanly: the final rejection's repair is + // held as carry-over and leads the next request, so the committed history the LLM sees + // never leaves c5's tool_use unanswered — no dangling pairing, no [context_summary]. + await collect(engine.run([userText("next")], { approve: allowAll })); + expect(llm1.calls).toHaveLength(7); + const nextTurn = llm1.calls[6]!; + expect(payloadTypes(nextTurn)).toEqual(["tool_call_output", "text"]); + expect((nextTurn[0]!.payload as { tool_call_id: string }).tool_call_id).toBe("c5"); + expect(textOf(nextTurn[1]!)).toBe("next"); + // Carry-over is spent: it does not leak into later runs. + await collect(engine.run([userText("later")], { approve: allowAll })); + expect(llm1.calls[7]!.map(textOf)).toEqual(["later"]); + }); + + it("a valid summary on the 5th and final allowed attempt completes the compaction", async () => { + // Counting pin for the rejection cap: four rejected attempts spend the budget but the 5th + // attempt still gets its chance — a valid summary there succeeds (5 rejections would fail). + const llm1 = new ScriptedLLM( + [ + { messages: [assistantText("answer"), usage(150, 150)] }, + // Attempts 1-4: empty (thinking only) -> rejected, retried. + { messages: [thinkingMessage("blank stare 1"), usage(160, 310)] }, + { messages: [thinkingMessage("blank stare 2")] }, + { messages: [thinkingMessage("blank stare 3")] }, + { messages: [thinkingMessage("blank stare 4")] }, + // Attempt 5: a real summary -> the compaction completes with THIS attempt's output. + { messages: [assistantText("[summary]fifth attempt wins[/summary]"), usage(170, 480)] }, + ], + "llm1", + ); + const llm2 = new ScriptedLLM([{ messages: [assistantText("fresh"), usage(20, 500)] }], "llm2"); + let factoryTokens: TokenCounts | null = null; + const engine = new ContextEngine({ + llm: llm1, + environment: fakeEnvironment, + compaction: settings(), + createLLM: (tokens) => { + factoryTokens = tokens; + return llm2; + }, + maxReconnects: 1, + reconnectBackoffMs: 1, + }); + + const out = await collect(engine.run([userText("task one")], { approve: allowAll })); + const events = compactionEvents(out); + expect(events).toHaveLength(2); + expect(events[1]).toMatchObject({ type: "compaction_end", status: "completed" }); + // Only the adopted attempt's token_usage is surfaced between the paired events; rejected + // attempts' usage still feeds the Session cumulative totals (see below) but is not shown. + const types = payloadTypes(out); + const between = out.slice( + types.indexOf("compaction_begin") + 1, + types.lastIndexOf("compaction_end"), + ); + const usageBetween = between.filter( + (m) => (m.payload as { type?: string }).type === "token_usage", + ); + expect(usageBetween).toHaveLength(1); + expect((usageBetween[0]!.payload as TokenUsagePayload).request.total).toBe(170); + // Session cumulative tokens carried into the new instance include the rejected attempts' usage. + expect(factoryTokens).toMatchObject({ total: 480 }); + + // The new context opens with the 5th attempt's summary. + await collect(engine.run([userText("task two")], { approve: allowAll })); + expect(llm1.calls).toHaveLength(6); + expect(llm2.calls[0]!.map(textOf)).toEqual([ + "[context_summary]\nfifth attempt wins\n[/context_summary]", + "task two", + ]); + }); + + it("a tool-calling rejection is repaired and the retry's summary completes the compaction", async () => { + // The pairing repair (#84 review): the rejected attempt's assistant turn — committed by + // the stateful LLM — ends in tool_use blocks that were never dispatched. Before retrying, + // the engine answers each with a synthesized failed tool_call_output (written to Trace), + // prepended to the retried input so the provider sees tool_use/tool_result paired, and + // the retry then has a real chance to summarize. + const llm1 = new ScriptedLLM( + [ + { messages: [assistantText("answer"), usage(150, 150)] }, + // Attempt 1: rejected — the model reached for a tool instead of summarizing. + { + messages: [ + assistantText("[summary]tempting[/summary]"), + toolCall({ name: "t", arguments: "{}", toolCallId: "c1" }), + ], + }, + // Attempt 2 (carrying the repair): a real summary. + { messages: [assistantText("[summary]real summary[/summary]"), usage(170, 400)] }, + ], + "llm1", + ); + const llm2 = new ScriptedLLM([{ messages: [assistantText("fresh"), usage(20, 420)] }], "llm2"); + const trace = new Writer({ tracesDir: traces, sessionId: "sess_repair" }); + const engine = new ContextEngine({ + llm: llm1, + environment: fakeEnvironment, + trace, + sessionMeta: metaMessage, + compaction: settings(), + createLLM: () => llm2, + reconnectBackoffMs: 1, + }); + const oldPath = trace.currentPath(); + + const out = await collect(engine.run([userText("task one")], { approve: allowAll })); + expect(compactionEvents(out)[1]).toMatchObject({ + type: "compaction_end", + status: "completed", + }); + + // The retried input answers the rejected attempt's call first, then re-issues the prompt. + expect(llm1.calls).toHaveLength(3); + const retry = llm1.calls[2]!; + expect(payloadTypes(retry)).toEqual(["tool_call_output", "text"]); + const repair = retry[0]!.payload as { + tool_call_id: string; + output: string; + stop_reason?: string; + }; + expect(repair.tool_call_id).toBe("c1"); + expect(repair.output).toBe( + "[tool error] the compaction request expects a summary, not tool calls", + ); + expect(repair.stop_reason).toBe("failed"); + expect(textOf(retry[1]!)).toBe("COMPACT NOW"); + + // The repair belongs to the compaction dialogue: written to the old Trace (so replay + // mirrors the pairing), never pushed to the output stream. + const repairsInTrace = (await readTrace(oldPath)).filter((m) => { + const p = m.payload as { type?: string; tool_call_id?: string }; + return p.type === "tool_call_output" && p.tool_call_id === "c1"; + }); + expect(repairsInTrace).toHaveLength(1); + expect(payloadTypes(out)).not.toContain("tool_call_output"); + + // The new context opens with the retry's summary. + await collect(engine.run([userText("task two")], { approve: allowAll })); + expect(llm2.calls[0]!.map(textOf)).toEqual([ + "[context_summary]\nreal summary\n[/context_summary]", + "task two", + ]); + }); + + it("a non-completed tool_call (interruption-closure shape) does not reject the summary", async () => { + // Same filter as the ordinary turn loop: only stop_reason === "completed" tool_calls are + // real requests. A tool_call synthesized to close out an interruption carries the + // interruption reason — it is structural, gets no synthesized repair, and must not cost + // a rejection attempt. + const llm1 = new ScriptedLLM( + [ + { messages: [assistantText("answer"), usage(150, 150)] }, + { + messages: [ + toolCall({ name: "t", arguments: "", toolCallId: "cz", stopReason: "timeout" }), + assistantText("[summary]still fine[/summary]"), + ], + }, + ], + "llm1", + ); + const llm2 = new ScriptedLLM([{ messages: [assistantText("ok"), usage(10, 200)] }], "llm2"); + const trace = new Writer({ tracesDir: traces, sessionId: "sess_closure" }); + const engine = new ContextEngine({ + llm: llm1, + environment: fakeEnvironment, + trace, + sessionMeta: metaMessage, + compaction: settings(), + createLLM: () => llm2, + }); + const oldPath = trace.currentPath(); + + const out = await collect(engine.run([userText("go")], { approve: allowAll })); + expect(compactionEvents(out)[1]).toMatchObject({ + type: "compaction_end", + status: "completed", + }); + // One compaction attempt, no retry, and no repair synthesized for the closure call. + expect(llm1.calls).toHaveLength(2); + const closureRepairs = (await readTrace(oldPath)).filter((m) => { + const p = m.payload as { type?: string; tool_call_id?: string }; + return p.type === "tool_call_output" && p.tool_call_id === "cz"; + }); + expect(closureRepairs).toEqual([]); + }); + + it("the compaction request carries exactly the same tools as ordinary requests (prompt-cache pin)", async () => { + // Owner constraint (#84): compaction runs exactly when the context is at its largest, and + // the provider's prompt cache only holds if the request prefix — the tool list included — + // stays byte-identical to the ordinary turns'. The engine passes no per-request tool + // override of any kind, and GenerativeModel serves every request from the same frozen + // config, so the compaction request's tools are the session's tools, verbatim. + const configs: (UniConfig | undefined)[] = []; + const scripted = [ + // Turn 1 answer: usage above the compaction threshold (total 151 >= 100). + { text: "answer one", promptTokens: 150 }, + // Compaction request: a valid summary. + { text: "[summary]s[/summary]", promptTokens: 160 }, + ]; + class CapturingModel extends GenerativeModel { + protected override openStream( + _uni: UniMessage, + _signal: AbortSignal, + config?: UniConfig, + ): AsyncIterable { + configs.push(config); + const next = scripted.shift()!; + return (async function* () { + const event: UniEvent = { + role: "assistant", + event_type: "delta", + content_items: [{ type: "text", text: next.text }], + finish_reason: "stop", + usage_metadata: { + cached_tokens: 0, + prompt_tokens: next.promptTokens, + thoughts_tokens: 0, + response_tokens: 1, + }, + }; + yield event; + })(); + } + } + const model = new CapturingModel({ + modelId: "claude-sonnet-4-6", + tools: [ + { name: "exec_command", description: "run a command" }, + { name: "read_file", description: "read a file" }, + ], + }); + const engine = new ContextEngine({ + llm: model, + environment: fakeEnvironment, + compaction: settings(), + createLLM: () => new ScriptedLLM([], "llm2"), + }); + + const out = await collect(engine.run([userText("go")], { approve: allowAll })); + expect(compactionEvents(out)[1]).toMatchObject({ + type: "compaction_end", + status: "completed", + }); + expect(configs).toHaveLength(2); + // Identical config object -> identical serialized prefix; the tool list is present and + // unchanged (not omitted, not [], no tool_choice override). + expect(configs[1]).toBe(configs[0]); + expect(configs[1]?.tools?.map((t) => t.name)).toEqual(["exec_command", "read_file"]); + expect(configs[1] !== undefined && "tool_choice" in configs[1]).toBe(false); + }); + it("session turns reaching (==) the threshold compact at task end — no waiting for the next task", async () => { const llm1 = new ScriptedLLM( [ diff --git a/packages/core/test/replay.test.ts b/packages/core/test/replay.test.ts index 3bdd05f..fc78c0c 100644 --- a/packages/core/test/replay.test.ts +++ b/packages/core/test/replay.test.ts @@ -6,7 +6,9 @@ * tool_calls; trailing input is kept as-is as carry-over. * - Pairing fallback: committed tool_calls with no paired output get an interrupted-state placeholder. * - Compaction wrap-up (file level): summarize rebuilds [context_summary], discard leaves no - * pending input; failed compaction rounds are dropped by the generic rule. + * pending input; failed compaction rounds are dropped by the generic rule; a "completed" + * summarize closure whose output extracts to an empty summary (a pre-#83 trace shape) is + * voided and the original context replays. * - Tolerates a truncated trailing line left by an abnormal process exit. * - Round-trip: a Trace written out by the engine, once replayed, matches the history the model actually received. */ @@ -522,10 +524,13 @@ describe("resumeTrace regressions (PR #39 review)", () => { expect(result.carryOver).toEqual([]); }); - it("closed context (summarize) with a textless compaction output yields an empty summary", () => { - // The compaction request completed but produced no text (e.g. thinking-only): the summary is - // empty, and must not fall back to an earlier round's ordinary answer (consistent with the - // in-process extractSummary("") behavior). + it("closed context (summarize) with a textless compaction output voids the closure and replays the original context", () => { + // The compaction request completed but produced no text (e.g. thinking-only): only a + // pre-#83 engine wrote such a "completed" closure — the engine now fails the compaction + // instead of committing an empty summary that would erase the task state. Resume mirrors + // that contract: no empty [context_summary] is fabricated, nothing falls back to an + // earlier round's ordinary answer, and the original context held by this file is + // reconstructed (the committed compaction turn stays in history like any committed turn). const result = resumeTrace([ meta(), userText("hello"), @@ -541,9 +546,24 @@ describe("resumeTrace regressions (PR #39 review)", () => { tokenUsage(usage(20), usage(20)), compactionEnd({ reason: "context", mode: "summarize", status: "completed" }), ]); - expect(result.contextClosed).toBe(true); - const summary = result.pendingSummary!.payload as { text: string }; - expect(summary.text).toBe("[context_summary]\n\n[/context_summary]"); - expect(summary.text).not.toContain("42"); + expect(result.contextClosed).toBe(false); + expect(result.pendingSummary).toBeUndefined(); + // Full original history, including the committed compaction exchange (AgentHub committed + // it, so the next request builds on top of it). + expect(result.history.map((m) => (m.payload as { type?: string }).type)).toEqual([ + "text", + "text", + "text", + "thinking", + ]); + expect(textsOf(result.history.slice(0, 3))).toEqual([ + "hello", + "The answer is 42.", + "please summarize", + ]); + // The committed compaction request does not count as a Session turn (in-process, only real + // turns increment the counter). + expect(result.sessionTurns).toBe(1); + expect(result.carryOver).toEqual([]); }); }); diff --git a/packages/docs/content/agent-loop.en.md b/packages/docs/content/agent-loop.en.md index 2ba7d9b..7a6631e 100644 --- a/packages/docs/content/agent-loop.en.md +++ b/packages/docs/content/agent-loop.en.md @@ -116,6 +116,8 @@ Three triggers (`compaction_begin.reason`): Two modes: `summarize` (default) appends the compaction Prompt to the old context, extracts the `[summary]`, wraps it as a `[context_summary]` user text and continues in a **fresh model context**; `discard` simply drops the old context. System markers are written as `[tag]…[/tag]`; the earlier angle-bracket form (``, ``, …) is still recognized when reading old Traces and old persisted compaction prompts. Compaction rotates the [Trace file](/sessions-and-traces) (`_002`, `_003`, …) — one Trace file always equals one complete model context. `compactability()` probes feasibility before `session.compact()` (`ok | unsupported | empty | just_compacted`). +The compaction request keeps the session's toolset **unchanged** — the request prefix (tool list included) stays byte-identical to ordinary turns, so the provider's prompt cache remains valid at the moment the context is largest. Compaction still succeeds only with a valid summary: a response that calls a tool or whose extracted summary is empty is rejected — any tool calls are answered with synthesized failed outputs (keeping `tool_use`/`tool_result` pairing intact) and the repaired request is resent immediately (a rejection is model behavior, not a transport failure, so no backoff applies), up to 5 rejected attempts; then the compaction ends `failed`, keeping the original context and Trace file until the next trigger. Transport `timeout`/`malformed` attempts follow the compaction-specific reconnect cap and backoff ladder described under "Automatic reconnect" above. + ## Concurrency model - Within a turn: approvals are sequential, execution is concurrent, and the next turn's input keeps the original order; diff --git a/packages/docs/content/agent-loop.zh.md b/packages/docs/content/agent-loop.zh.md index 05fa9cf..d2f037d 100644 --- a/packages/docs/content/agent-loop.zh.md +++ b/packages/docs/content/agent-loop.zh.md @@ -113,6 +113,8 @@ interface CompactionSettings { 两种模式：`summarize`(默认)向旧上下文追加压缩 Prompt，提取 `[summary]` 后包装为 `[context_summary]` 用户文本，在**全新的模型上下文**中继续；`discard` 直接丢弃旧上下文。系统标记统一写作 `[tag]…[/tag]`；读取旧 Trace 与旧压缩 Prompt 时仍识别早期的尖括号形式（``、`` 等）。压缩时 [Trace 文件随之轮转](/sessions-and-traces)(`_002`、`_003`……)，一个 Trace 文件恒等于一个完整模型上下文。`session.compact()` 前可用 `compactability()` 探询可行性(`ok | unsupported | empty | just_compacted`)。 +压缩请求**保持会话工具集不变**——请求前缀（含工具列表）与普通轮次逐字节一致，确保上下文最大的时刻提供商的提示词缓存依然有效。只有得到有效摘要，压缩才算成功：若响应中出现工具调用、或提取出的摘要为空，则判为无效并重试——工具调用会先以合成的失败输出逐一应答（保持 `tool_use`/`tool_result` 配对完整），修复后的请求**立即重发**（无效摘要是模型行为而非传输故障，不做退避），最多允许 5 次无效尝试，之后压缩以 `failed` 结束，保留原上下文与 Trace 文件，等待下次触发。传输层 `timeout`/`malformed` 仍走上文「自动重连」一节所述的压缩专用重连上限与退避阶梯。 + ## 并发模型 - 同一轮内：审批逐个、执行并发、下一轮输入按原始顺序；