From 6f60b242bdb71b721dbdc6b39647dd079e788847 Mon Sep 17 00:00:00 2001 From: Yaowei Zheng Date: Tue, 4 Aug 2026 17:27:45 +0800 Subject: [PATCH] fix(core,server,web,cli): compaction tolerance, retry parity, and failure classification (#174) Co-authored-by: Claude Fable 5 --- packages/cli/src/i18n.ts | 15 +- packages/cli/src/render.ts | 20 +- packages/cli/test/render.test.ts | 25 +- packages/core/src/engine/context-engine.ts | 253 +++++++++++------- .../src/environment/tools/describe-image.ts | 4 +- packages/core/src/interfaces.ts | 11 +- packages/core/src/llm/generative-model.ts | 10 +- packages/core/src/omnimessage/builders.ts | 21 +- .../src/omnimessage/markers/engine-blocks.ts | 26 +- packages/core/src/omnimessage/types.ts | 49 +++- packages/core/src/state/default-config.ts | 22 +- packages/core/test/compaction.test.ts | 228 ++++++++++++---- packages/core/test/describe-image.test.ts | 2 +- packages/core/test/engine.test.ts | 32 ++- packages/core/test/llm.test.ts | 8 +- packages/core/test/markers.test.ts | 17 ++ packages/core/test/session-title.test.ts | 2 +- packages/docs/content/agent-loop.en.md | 6 +- packages/docs/content/agent-loop.zh.md | 6 +- packages/docs/content/omni-message.en.md | 16 +- packages/docs/content/omni-message.zh.md | 12 +- packages/server/src/db/schema.ts | 2 +- packages/server/src/runtime/error-recorder.ts | 1 + .../src/runtime/stream-error-watcher.ts | 60 ++++- .../src/services/project-config-service.ts | 3 +- packages/server/test/errors.test.ts | 77 +++++- packages/server/test/model-probe.test.ts | 6 +- .../src/features/chat/compaction-banner.tsx | 2 +- packages/web/src/lib/omni/stream-model.ts | 23 +- packages/web/src/lib/strings-en.ts | 8 +- packages/web/src/lib/strings.ts | 8 +- packages/web/test/stream-model.test.ts | 92 +++++-- 32 files changed, 741 insertions(+), 326 deletions(-) diff --git a/packages/cli/src/i18n.ts b/packages/cli/src/i18n.ts index fae9746..ed72e99 100644 --- a/packages/cli/src/i18n.ts +++ b/packages/cli/src/i18n.ts @@ -171,7 +171,12 @@ export interface Messages { * line: total = Session cumulative, delta = consumed by this compaction, carrying its own * sign); when present it is appended at the end of the line, e.g. ` · tokens 14k (+6k)`. */ - compactionStop(mode: string, status: string, tokens?: { total: string; delta: string }): string; + compactionStop( + mode: string, + status: string, + tokens?: { total: string; delta: string }, + errorMessage?: string, + ): string; /** Prompt shown when `/compact` has nothing to compact (session just started / two consecutive compactions). */ compactNothing(): string; /** Dim line announcing one goal round (printed before the round runs). */ @@ -406,12 +411,12 @@ const en: Messages = { mode === "discard" ? `[compaction] discarding context (${reason})…` : `[compaction] summarizing context (${reason})…`, - compactionStop: (mode, status, tokens) => + compactionStop: (mode, status, tokens, errorMessage) => (status === "completed" ? mode === "discard" ? "[compaction] done; old context discarded" : "[compaction] done; continuing with the summarized context" - : `[compaction] ${status}; keeping the current context`) + + : `[compaction] ${status}${errorMessage !== undefined ? ` (${errorMessage})` : ""}; keeping the current context`) + (tokens ? ` · tokens ${tokens.total} (${tokens.delta})` : ""), compactNothing: () => "[compaction] nothing to compact yet", goalRound: (round) => `[goal] round ${round}`, @@ -620,12 +625,12 @@ const zh: Messages = { mode === "discard" ? `[压缩] 正在丢弃旧上下文（${reason}）……` : `[压缩] 正在总结压缩上下文（${reason}）……`, - compactionStop: (mode, status, tokens) => + compactionStop: (mode, status, tokens, errorMessage) => (status === "completed" ? mode === "discard" ? "[压缩] 完成，旧上下文已丢弃" : "[压缩] 完成，已切换到摘要后的新上下文" - : `[压缩] ${status === "aborted" ? "已中断" : "失败"}，保留当前上下文`) + + : `[压缩] ${status === "aborted" ? "已中断" : `失败${errorMessage !== undefined ? `（${errorMessage}）` : ""}`}，保留当前上下文`) + (tokens ? ` · tokens ${tokens.total} (${tokens.delta})` : ""), compactNothing: () => "[压缩] 当前上下文为空，无需压缩", goalRound: (round) => `[目标] 第 ${round} 轮`, diff --git a/packages/cli/src/render.ts b/packages/cli/src/render.ts index 352c12c..adfbc08 100644 --- a/packages/cli/src/render.ts +++ b/packages/cli/src/render.ts @@ -380,8 +380,8 @@ export class StreamRenderer { private taskLastReqEndMs: number | null = null; /** Retryable terminal state (failed/timeout/malformed) of the previous request: the next request_begin is a retry, at which point a notice is printed. */ private pendingRetry: "failed" | "timeout" | "malformed" | null = null; - /** Number of retries already initiated (increments on consecutive failures, reset once a request completes normally). */ - private reconnectRun = 0; + /** The pending failure's attempt ordinal (request_end.attempt — the core's authoritative count, printed on the retry line). */ + private pendingRetryAttempt: number | undefined; constructor(out: NodeJS.WritableStream = process.stdout, t: Messages = defaultMessages()) { this.out = out; @@ -688,7 +688,7 @@ export class StreamRenderer { } else if (payload.type === "abort") { // Run ended (user interrupt / retries exhausted): clear any pending retry state so the next run doesn't mistakenly print a retry line. this.pendingRetry = null; - this.reconnectRun = 0; + this.pendingRetryAttempt = undefined; this.finishLine(); this.out.write(`${formatAbort(payload as AbortPayload, this.t)}\n`); this.lastLineKey = null; @@ -698,9 +698,12 @@ export class StreamRenderer { // retries are exhausted, there's no retry after the last failure, only an abort // explaining why). if (this.pendingRetry) { - this.reconnectRun += 1; + // The failed request_end stamped its authoritative attempt ordinal — print it + // (the CLI renders live streams only, which always come from a stamping core). + const attempt = this.pendingRetryAttempt ?? 1; + this.pendingRetryAttempt = undefined; this.finishLine(); - this.out.write(`${dim(this.t.reconnectLabel(this.pendingRetry, this.reconnectRun))}\n`); + this.out.write(`${dim(this.t.reconnectLabel(this.pendingRetry, attempt))}\n`); this.lastLineKey = null; this.pendingRetry = null; } @@ -725,9 +728,10 @@ export class StreamRenderer { // reset the counter mid-ladder so a mixed run renumbers back to retry #1. if (p.status === "failed" || p.status === "timeout" || p.status === "malformed") { this.pendingRetry = p.status; + this.pendingRetryAttempt = p.attempt; } else { this.pendingRetry = null; - this.reconnectRun = 0; + this.pendingRetryAttempt = undefined; } } else if (payload.type === "compaction_begin") { // Paired compaction events: begin signals compaction is in progress. @@ -751,7 +755,9 @@ export class StreamRenderer { } : undefined; this.compactionTokens = 0; - this.out.write(`${dim(this.t.compactionStop(p.mode, p.status, tokens))}\n`); + this.out.write( + `${dim(this.t.compactionStop(p.mode, p.status, tokens, p.error_message))}\n`, + ); this.lastLineKey = null; } return; diff --git a/packages/cli/test/render.test.ts b/packages/cli/test/render.test.ts index 2824169..08c2a3e 100644 --- a/packages/cli/test/render.test.ts +++ b/packages/cli/test/render.test.ts @@ -281,50 +281,51 @@ describe("StreamRenderer", () => { expect(stripAnsi(text())).toBe("[tool-c3] -> line1\n"); }); - it("prints the retry line only when the retry request actually begins", () => { + it("prints the retry line, with the stamped attempt, only when the retry request actually begins", () => { const { stream, text } = collector(); const r = new StreamRenderer(stream, t); r.handle(requestBegin()); - r.handle(requestEnd("malformed")); + r.handle(requestEnd("malformed", { attempt: 1 })); expect(stripAnsi(text())).toBe(""); // the failure itself prints nothing; only the retry's start does r.handle(requestBegin()); // retry #1 begins expect(stripAnsi(text())).toContain("retry #1"); - r.handle(requestEnd("timeout")); + r.handle(requestEnd("timeout", { attempt: 2 })); r.handle(requestBegin()); // retry #2 begins expect(stripAnsi(text())).toContain("retry #2"); // Retry #2 fails again and retries are exhausted: no next request_begin, only abort — no retry #3 appears. - r.handle(requestEnd("malformed")); + r.handle(requestEnd("malformed", { attempt: 3 })); r.handle(abortEvent("malformed response failed after 2 retries")); expect(stripAnsi(text())).not.toContain("retry #3"); - // The first request of the next run is not a retry, so it prints nothing; a new failure after it counts from 1 again. + // The first request of the next run is not a retry, so it prints nothing; the next run's + // failures are stamped from 1 again. r.handle(requestBegin()); - r.handle(requestEnd("timeout")); + r.handle(requestEnd("timeout", { attempt: 1 })); r.handle(requestBegin()); const lines = stripAnsi(text()); expect(lines.match(/retry #1/g)).toHaveLength(2); expect(lines).not.toContain("retry #3"); }); - it("prints the retry line for a failed request too, and keeps counting across a mixed ladder", () => { + it("prints the retry line for a failed request too, straight from the stamped ordinal", () => { // The engine reconnects on `failed` as well, so the CLI has to say so — otherwise the - // session goes quiet for the whole ladder. And because `failed` used to reset the - // counter, a mixed timeout → failed → timeout run renumbered back to retry #1. + // session goes quiet for the whole ladder. The number is the event's own attempt, so a + // mixed timeout → failed → timeout run keeps counting without any client-side state. const { stream, text } = collector(); const r = new StreamRenderer(stream, t); r.handle(requestBegin()); - r.handle(requestEnd("failed", "Upstream HTTP/2 stream failed")); + r.handle(requestEnd("failed", { errorMessage: "Upstream HTTP/2 stream failed", attempt: 1 })); r.handle(requestBegin()); // retry #1 begins let lines = stripAnsi(text()); expect(lines).toContain("the model provider returned an error"); expect(lines).toContain("retry #1"); - r.handle(requestEnd("timeout")); + r.handle(requestEnd("timeout", { attempt: 2 })); r.handle(requestBegin()); // retry #2 begins — the count does not restart lines = stripAnsi(text()); expect(lines).toContain("connection timed out"); expect(lines).toContain("retry #2"); // `auth` is terminal: the engine never retries it, so the next request_begin (a new run) // must not be announced as a retry. - r.handle(requestEnd("auth", "401 invalid x-api-key")); + r.handle(requestEnd("auth", { errorMessage: "401 invalid x-api-key", attempt: 3 })); r.handle(requestBegin()); expect(stripAnsi(text())).not.toContain("retry #3"); }); diff --git a/packages/core/src/engine/context-engine.ts b/packages/core/src/engine/context-engine.ts index 21db464..b64905d 100644 --- a/packages/core/src/engine/context-engine.ts +++ b/packages/core/src/engine/context-engine.ts @@ -183,10 +183,14 @@ export interface ContextEngineDeps { /** Ceiling (ms) for a single reconnect backoff wait. Defaults to 30000. */ reconnectBackoffMaxMs?: number; /** - * Maximum retries for a failing compaction request (instead of the shared `maxReconnects`; - * same statuses, shorter budget — see RETRY_STATUSES). Compaction failure is already - * graceful — the original context is kept and compaction retries at the next trigger — so a - * broken request fails fast (~1.75s of backoff). Defaults to 3. + * Maximum retries for a failing compaction request — one budget for every failure kind: + * the transport statuses (see RETRY_STATUSES) and a committed response that isn't a usable + * summary (empty, or tool calls) all draw from it; only `auth` stops without retrying. + * Defaults to the shared `maxReconnects`: a compaction request is an ordinary LLM request + * and deserves the same patience (issue #170 — the earlier tighter budget made a + * struggling provider fail compaction fast, and a session whose every turn re-triggers + * compaction is stuck). Failure stays graceful either way: the original context is kept + * and compaction retries at the next trigger. */ compactionMaxReconnects?: number; /** @@ -225,17 +229,19 @@ const carriesSteering = (m: OmniMessage): boolean => { 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 the retryable - * attempts that were never committed (failed/timeout/malformed) 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). + * Corrective note prepended to the re-sent compaction Prompt after a committed-but-unusable + * response (empty summary, or tool calls — issues #83/#170). The unusable + * response is committed on the live LLM object and can only be *appended* to (rewriting the + * prefix would invalidate the provider's prompt cache at the moment the context is largest — + * the same invariant that pins the toolset, issue #84); without an explicit correction the + * model sees its own bad output as the freshest example and copies it verbatim on every + * retry (issue #170: deepseek-v4-flash kept writing the body after `[/summary]`). + * Exported for unit tests. */ -const MAX_SUMMARY_REJECTIONS = 5; +export const SUMMARY_RETRY_GUIDANCE = + "Your previous reply was not a usable summary. Reply again with text only, no tool " + + "calls, in exactly this format and nothing after it:\n\n" + + "[summary]put the summary text here...[/summary]"; /** Result of executing one LLM turn (the return value of runTurn). */ interface TurnResult { @@ -342,9 +348,11 @@ function downgradeCarriedGoalInput(msg: OmniMessage): OmniMessage { * own way lands here; retrying a genuinely permanent error costs the ladder and ends the same * way, while aborting a transient one destroys the turn. * - * Both loops retry the same set. What differs is the budget: compaction runs on - * `compactionMaxReconnects` (shorter than the turn loop's), because a compaction that gives - * up keeps the original context and tries again at the next trigger. + * Both loops retry the same set on the same backoff ladder, and by default with the same + * budget: compaction runs on `compactionMaxReconnects`, which follows `maxReconnects` unless + * set explicitly (issue #170 — a compaction request is an ordinary LLM request and gets the + * turn loop's patience; compaction additionally routes a committed-but-unusable summary + * through the same budget, see summarizeContext). */ const RETRY_STATUSES: readonly StopReason[] = ["failed", "timeout", "malformed"]; @@ -406,7 +414,7 @@ export class ContextEngine { this.maxReconnects = deps.maxReconnects ?? 5; this.reconnectBackoffMs = deps.reconnectBackoffMs ?? 250; this.reconnectBackoffMaxMs = deps.reconnectBackoffMaxMs ?? 30_000; - this.compactionMaxReconnects = deps.compactionMaxReconnects ?? 3; + this.compactionMaxReconnects = deps.compactionMaxReconnects ?? this.maxReconnects; this.llm = deps.llm; // Session resumption: apply the initial state derived from replay. const init = deps.initialState; @@ -619,7 +627,7 @@ export class ContextEngine { // else — `failed` included — goes to the reconnect loop below. if (turn.outcome.status === "auth") { this.pendingCarryOver = this.buildCarryOver(attemptInput, turn); - yield* this.emitAbort(`llm request error: ${turn.outcome.message ?? "unknown"}`); + yield* this.emitAbort(`llm request error: ${turn.outcome.errorMessage ?? "unknown"}`); return; } // Completed normally. @@ -653,7 +661,7 @@ export class ContextEngine { turn.outcome.status === "malformed" ? `malformed response failed after ${this.maxReconnects} retries` : turn.outcome.status === "failed" - ? `llm request failed after ${this.maxReconnects} retries: ${turn.outcome.message ?? "unknown"}` + ? `llm request failed after ${this.maxReconnects} retries: ${turn.outcome.errorMessage ?? "unknown"}` : `reconnect failed after ${this.maxReconnects} retries`; yield* this.emitAbort(reason); return; @@ -967,16 +975,21 @@ export class ContextEngine { // (the errors panel) can learn the real reason (e.g. a quota code). When the // engine will retry in-run, the planned backoff rides along as retry_in_ms // (the frontend's live countdown); absent on final failures and completions. - const stopEvt = requestEnd( - outcome.status, - outcome.message, - this.plannedRetryDelayMs( - outcome, - reconnectsSoFar, - this.maxReconnects, - RETRY_STATUSES, - ), + const retryInMs = this.plannedRetryDelayMs( + outcome, + reconnectsSoFar, + this.maxReconnects, + RETRY_STATUSES, ); + const stopEvt = requestEnd(outcome.status, { + ...(outcome.errorMessage !== undefined ? { errorMessage: outcome.errorMessage } : {}), + // The authoritative attempt ordinal (1-based, within this retry run); a clean + // first-try completion stays unstamped so the common case adds no noise. + ...(outcome.status !== "completed" || reconnectsSoFar > 0 + ? { attempt: reconnectsSoFar + 1 } + : {}), + ...(retryInMs !== undefined ? { retryInMs } : {}), + }); queue.push(stopEvt); await this.write(stopEvt); break; @@ -1230,15 +1243,17 @@ export class ContextEngine { * 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 + * paired compaction events, plus every attempt's `token_usage` — positioned between the two + * events, so the frontend stats and the server's usage records count the compaction's true + * spend, rejected attempts included), but it is written * 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. failed/timeout/malformed reconnect under the compaction-specific - * cap (`compactionMaxReconnects` — a shorter budget than the turn loop's, not a narrower - * set), collapsing to failed once retries are exhausted; on failure/abort, the original + * text and no tool calls in the response. Everything short of that is one kind of failure, + * handled exactly like an ordinary LLM request's (issue #170): an unusable committed + * response (empty summary, or tool calls — answered with synthesized failed outputs and + * retried behind a corrective note, see the loop body) and the transport failures + * (failed/timeout/malformed) all reconnect under the one `compactionMaxReconnects` budget + * (defaulting to the turn loop's budget and ladder — see RETRY_STATUSES); only `auth` stops + * without retrying. Once the budget is exhausted the compaction fails; on failure/abort, the original * context and Trace index are kept — it does not fall back to discard. The first **committed** * attempt absorbs `pendingToolOutputs` into the old context's history (issue #85): later * resends carry only the repairs and the Prompt, and the result's `committed` flag tells @@ -1271,24 +1286,40 @@ export class ContextEngine { // clean end — none of those reach the stateful commit). Returned as `committed`: the // callers' two-case carry rule branches on it. let committed = false; - // Synthesized outputs answering the latest rejected attempt's tool calls, not yet carried + // Synthesized outputs answering the latest unusable 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: retryable attempts (failed/timeout/malformed, 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. + // One retry budget for every failure: an unusable committed response (empty summary / + // tool calls) counts exactly like a transport failure (issue #170) — same counter, same + // exponential ladder — and only `auth` stops without retrying. let reconnects = 0; - let rejections = 0; + // The compaction_end event's share of the RetryDetail block (also what the server's + // error record carries): the final attempt ordinal, and the last failure's detail. + let attempts = 0; + let lastError: string | undefined; for (;;) { if (signal?.aborted) { this.stashRepairs(pendingRepairs); - yield* this.emitCompactionEnd(reason, "summarize", "aborted"); + yield* this.emitCompactionEnd( + reason, + "summarize", + "aborted", + attempts > 0 ? { attempt: attempts } : undefined, + ); return { status: "aborted", committed }; } const attempt = await this.runCompactionRequest(input, signal, reconnects); + attempts += 1; + // Every attempt's token_usage is pushed to the Human output stream (already written to + // Trace in runCompactionRequest, so it's only yielded here, never rewritten): the frontend + // stats and the server's usage records then carry the compaction's true spend — failed + // attempts burn real tokens (issue #170), and surfacing only the adopted attempt's usage + // understated the cost center. + if (attempt.usage) yield attempt.usage; + // A committed-but-unusable response (empty summary or tool calls): its retry input must + // be rebuilt below — repairs + corrective note + Prompt — instead of resent unchanged. + let unusable = false; if (attempt.status === "completed") { // 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 @@ -1306,30 +1337,26 @@ export class ContextEngine { // 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"); + yield* this.emitCompactionEnd(reason, "summarize", "completed", { attempt: attempts }); await this.startNewContext(); return { status: "completed", summary, committed }; } - // Rejected. Tool calls were never dispatched, yet the assistant turn holding them IS + // Not a summary — one more failed attempt, sharing the reconnect budget below. 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: + // provider sees tool_use/tool_result paired. The empty-text case 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; + unusable = true; + lastError = + attempt.toolCalls.length > 0 + ? "the response called tools instead of writing a summary" + : "the response contained no usable summary"; pendingRepairs = attempt.toolCalls.map((tc) => toolCallOutput({ output: "[tool error] the compaction request expects a summary, not tool calls", @@ -1338,53 +1365,61 @@ export class ContextEngine { }), ); for (const repair of pendingRepairs) await this.write(repair); - // Rebuild from the (shrunken) base 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, ...base] : base; - if (rejections >= MAX_SUMMARY_REJECTIONS) { - this.stashRepairs(pendingRepairs); - yield* this.emitCompactionEnd(reason, "summarize", "failed"); - return { status: "failed", committed }; - } - // 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") { + } else if (attempt.status === "aborted") { this.stashRepairs(pendingRepairs); - yield* this.emitCompactionEnd(reason, "summarize", "aborted"); + yield* this.emitCompactionEnd(reason, "summarize", "aborted", { attempt: attempts }); return { status: "aborted", committed }; - } - if (attempt.status === "auth") { - // `auth` folds into `failed` here: the compaction event pair keeps its + } else if (attempt.status === "auth") { + // `auth` is the one status that never retries: credentials don't heal on a ladder. + // It folds into `failed` here: the compaction event pair keeps its // 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"); + yield* this.emitCompactionEnd(reason, "summarize", "failed", { + attempt: attempts, + ...(attempt.errorMessage !== undefined ? { errorMessage: attempt.errorMessage } : {}), + }); return { status: "failed", committed }; + } else { + // Transport failure: keep its detail as the last error of record. + lastError = attempt.errorMessage; } - // failed / timeout / malformed: retried via reconnect — none of them is 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 compaction - // that ends up failing keeps the original context and tries again on the next trigger, - // so a short ladder here beats holding the session through the full one. + // One failure path for everything else — unusable summaries and the transport statuses + // (failed / timeout / malformed, never committed by AgentHub) — treated like an + // ordinary LLM request's failures: the same budget (defaulting to the shared + // maxReconnects, issue #170) and the same exponential ladder. An unusable attempt's + // request_end carries status completed, for which no retry_in_ms is announced — the + // backoff wait still happens. if (reconnects >= this.compactionMaxReconnects) { this.stashRepairs(pendingRepairs); - yield* this.emitCompactionEnd(reason, "summarize", "failed"); + yield* this.emitCompactionEnd(reason, "summarize", "failed", { + attempt: attempts, + ...(lastError !== undefined ? { errorMessage: lastError } : {}), + }); return { status: "failed", committed }; } reconnects += 1; const ok = await this.backoff(reconnects, signal); if (!ok) { this.stashRepairs(pendingRepairs); - yield* this.emitCompactionEnd(reason, "summarize", "aborted"); + yield* this.emitCompactionEnd(reason, "summarize", "aborted", { attempt: attempts }); return { status: "aborted", committed }; } + if (unusable) { + // Rebuild from the (shrunken) base rather than appending: everything the unusable + // attempt's input carried is committed — the live object's history can only grow, so + // the retry appends the fresh repairs, a corrective note, and the Prompt. The note + // (written to Trace like the Prompt, and only when a retry actually follows) is what + // breaks the copy-my-own-mistake loop: the model's freshest example is its committed + // bad output, and rewriting history to hide it would invalidate the provider's + // prompt cache (issue #84) — correcting forward is the one cache-safe option + // (issue #170). Transport failures skip this: nothing was committed, so their input + // is resent unchanged (any pending repairs included). + const guidance = userText(SUMMARY_RETRY_GUIDANCE); + await this.write(guidance); + input = [...pendingRepairs, guidance, ...base]; + } } } @@ -1426,6 +1461,8 @@ export class ContextEngine { text: string; toolCalls: OmniMessage[]; usage: OmniMessage | null; + /** Error detail (LLMOutcome.errorMessage) on non-completed statuses — becomes compaction_end.error_message when this failure ends the compaction. */ + errorMessage?: string; }> { // 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 @@ -1448,19 +1485,32 @@ export class ContextEngine { // 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, - res.value.message, - this.plannedRetryDelayMs( - res.value, - reconnectsSoFar, - this.compactionMaxReconnects, - RETRY_STATUSES, - ), - ), + const retryInMs = this.plannedRetryDelayMs( + res.value, + reconnectsSoFar, + this.compactionMaxReconnects, + RETRY_STATUSES, ); - return { status: res.value.status, text, toolCalls, usage }; + await this.write( + requestEnd(res.value.status, { + ...(res.value.errorMessage !== undefined + ? { errorMessage: res.value.errorMessage } + : {}), + // Same stamping rule as the turn loop; for compaction the ordinal counts every + // retry kind (transport and unusable-summary alike share one budget). + ...(res.value.status !== "completed" || reconnectsSoFar > 0 + ? { attempt: reconnectsSoFar + 1 } + : {}), + ...(retryInMs !== undefined ? { retryInMs } : {}), + }), + ); + return { + status: res.value.status, + text, + toolCalls, + usage, + ...(res.value.errorMessage !== undefined ? { errorMessage: res.value.errorMessage } : {}), + }; } const msg = res.value; await this.write(msg); @@ -1512,13 +1562,14 @@ export class ContextEngine { await this.write(msg); } - /** Yields and records a compaction stop event (carrying the result status; non-completed means compaction was abandoned). */ + /** Yields and records a compaction stop event (carrying the result status — non-completed means compaction was abandoned — plus its share of the RetryDetail block: final attempt ordinal, and the last error detail on failures). */ private async *emitCompactionEnd( reason: CompactionReason, mode: CompactionMode, status: StopReason, + detail?: { attempt?: number; errorMessage?: string }, ): AsyncGenerator { - const msg = compactionEnd({ reason, mode, status }); + const msg = compactionEnd({ reason, mode, status, ...detail }); yield msg; await this.write(msg); } diff --git a/packages/core/src/environment/tools/describe-image.ts b/packages/core/src/environment/tools/describe-image.ts index abb9baa..ccfa503 100644 --- a/packages/core/src/environment/tools/describe-image.ts +++ b/packages/core/src/environment/tools/describe-image.ts @@ -103,7 +103,9 @@ export function createDescribeImageTool( if (signal?.aborted) return { stopReason: "aborted" }; if (!outcome || outcome.status !== "completed") { const detail = - outcome && "message" in outcome && outcome.message ? `: ${outcome.message}` : ""; + outcome && "errorMessage" in outcome && outcome.errorMessage + ? `: ${outcome.errorMessage}` + : ""; yield delta( `${streamedAny ? "\n" : ""}Vision model (${describer.modelId}) request ${outcome?.status ?? "failed"}${detail}`, ); diff --git a/packages/core/src/interfaces.ts b/packages/core/src/interfaces.ts index 510da42..7d2d250 100644 --- a/packages/core/src/interfaces.ts +++ b/packages/core/src/interfaces.ts @@ -148,7 +148,7 @@ export interface GenerativeModelParameters { * `context_engine`; * - `aborted`: user-initiated interruption — stop and hand back to the user; * - `failed`: an error the retry classifier did not judge transient (params, etc.) — still - * retried by `context_engine` within the same run (`message` provides the display text). + * retried by `context_engine` within the same run (`errorMessage` provides the display text). * The classification stays honest — this is reported as `failed`, not relabelled a * timeout — while the *policy* retries it, because that classifier is an allowlist and a * gateway phrasing a transient fault its own way lands here; @@ -162,12 +162,13 @@ export interface GenerativeModelParameters { export interface LLMOutcome { status: StopReason; /** - * Failure detail (`describeError` text): present on `failed` / `auth`, and on `timeout` / + * Error detail (`describeError` text): present on `failed` / `auth`, and on `timeout` / * `malformed` when a concrete transport/provider error was caught (a plain idle timeout - * has none). Carried onto the `request_end` event so observability (the Cost center's - * errors panel) can show the real reason behind a retried request. + * has none). Carried onto the `request_end` event as `error_message` — one name across + * the internal outcome and the wire — so observability (the Cost center's errors panel) + * can show the real reason behind a retried request. */ - message?: string; + errorMessage?: string; } /** diff --git a/packages/core/src/llm/generative-model.ts b/packages/core/src/llm/generative-model.ts index 415a5fa..a1bc8ae 100644 --- a/packages/core/src/llm/generative-model.ts +++ b/packages/core/src/llm/generative-model.ts @@ -1107,7 +1107,7 @@ export class GenerativeModel implements LLMInterface { try { uniMessage = mergeOmniToUniMessage(params.newMessages); } catch (err) { - return { status: "failed", message: describeError(err) }; + return { status: "failed", errorMessage: describeError(err) }; } const translator = new EventTranslator(this.toolCallIds); @@ -1214,7 +1214,7 @@ export class GenerativeModel implements LLMInterface { // malformed to reconnect and retry — must not be classified as failed. outcome = { status: "malformed", - message: describeError(error), + errorMessage: describeError(error), }; } else if (isAuthenticationError(error)) { // Credentials failure: its own terminal status so hosts can tell "update this @@ -1222,17 +1222,17 @@ export class GenerativeModel implements LLMInterface { // fixed at Session creation; the credential is read from the current Project // config on load) apart from a one-off failure. Checked before the retryable // branch as a belt — isRetryableError itself already refuses auth signals. - outcome = { status: "auth", message: describeError(error) }; + outcome = { status: "auth", errorMessage: describeError(error) }; } else if (isRetryableError(error)) { // Network drop / transient provider rejection -> needs reconnection. The detail // (e.g. "403 … (insufficient_user_quota)") rides on the outcome so observability // (request_end -> the Cost center's errors panel) shows the real reason behind a // retried request, not just "timeout". - outcome = { status: "timeout", message: describeError(error) }; + outcome = { status: "timeout", errorMessage: describeError(error) }; } else if ((error as { name?: string })?.name === "AbortError") { outcome = { status: "aborted" }; // Fallback: an unexpected abort (neither timeout nor user) } else { - outcome = { status: "failed", message: describeError(error) }; + outcome = { status: "failed", errorMessage: describeError(error) }; } } finally { clearTimer(); diff --git a/packages/core/src/omnimessage/builders.ts b/packages/core/src/omnimessage/builders.ts index 4240db0..ef9da97 100644 --- a/packages/core/src/omnimessage/builders.ts +++ b/packages/core/src/omnimessage/builders.ts @@ -273,20 +273,21 @@ export function requestBegin(): OmniMessage { /** * request end event: carries the terminal state (`completed` means this turn was already - * committed to AgentHub), plus — on non-completed statuses — the failure detail (from - * LLMOutcome.message) and, when the engine will retry in-run, the planned backoff wait - * (`retry_in_ms`, rendered by the Web App as a live countdown). + * committed to AgentHub), plus the unified retry detail block (see RequestRetryDetail): + * the error detail (from LLMOutcome.errorMessage), the 1-based attempt ordinal, and — when the + * engine will retry in-run — the planned backoff wait (`retry_in_ms`, rendered by the Web + * App as a live countdown). This builder is the one place the block is stamped. */ export function requestEnd( status: StopReason, - message?: string, - retryInMs?: number, + retry: { errorMessage?: string; attempt?: number; retryInMs?: number } = {}, ): OmniMessage { return event({ type: "request_end", status, - ...(message !== undefined ? { message } : {}), - ...(retryInMs !== undefined ? { retry_in_ms: retryInMs } : {}), + ...(retry.errorMessage !== undefined ? { error_message: retry.errorMessage } : {}), + ...(retry.attempt !== undefined ? { attempt: retry.attempt } : {}), + ...(retry.retryInMs !== undefined ? { retry_in_ms: retry.retryInMs } : {}), }); } @@ -306,17 +307,21 @@ export function compactionBegin(args: { }); } -/** compaction end event: carries the compaction result (non-`completed` means compaction was abandoned and the original context is kept). */ +/** compaction end event: carries the compaction result (non-`completed` means compaction was abandoned and the original context is kept), plus its share of the RetryDetail block (final attempt ordinal; last error_message detail on failures). */ export function compactionEnd(args: { reason: CompactionReason; mode: CompactionMode; status: StopReason; + attempt?: number; + errorMessage?: string; }): OmniMessage { return event({ type: "compaction_end", reason: args.reason, mode: args.mode, status: args.status, + ...(args.attempt !== undefined ? { attempt: args.attempt } : {}), + ...(args.errorMessage !== undefined ? { error_message: args.errorMessage } : {}), }); } diff --git a/packages/core/src/omnimessage/markers/engine-blocks.ts b/packages/core/src/omnimessage/markers/engine-blocks.ts index 753b58c..ebcd890 100644 --- a/packages/core/src/omnimessage/markers/engine-blocks.ts +++ b/packages/core/src/omnimessage/markers/engine-blocks.ts @@ -4,7 +4,7 @@ * host — but their spelling belongs here with the rest of the markers so the engine and the * resume path cannot drift apart (they used to duplicate the `[context_summary]` template). */ -import { dualFormPatterns, markerBlock, matchDualForm } from "./block.js"; +import { dualFormPatterns, markerBlock, matchDualForm, stripMarkerBlocks } from "./block.js"; import { MARKER_TAGS, TRANSCRIPT_TAGS } from "./tags.js"; /** Wraps a compaction summary as the new context's first input: `[context_summary]…[/context_summary]`. */ @@ -12,19 +12,33 @@ export function buildContextSummaryText(summary: string): string { return markerBlock(MARKER_TAGS.contextSummary, summary); } -const SUMMARY_PATTERNS = dualFormPatterns(MARKER_TAGS.summary, "([\\s\\S]*?)"); +const SUMMARY_PATTERNS = dualFormPatterns(MARKER_TAGS.summary, "([\\s\\S]*?)", "g"); /** - * Extracts the summary the model wrote inside `[summary]…[/summary]` during compaction; when - * the tag is missing, leniently uses the entire output as-is (not treated as a failure). + * Extracts the summary the model wrote during compaction, with a tolerance ladder (issue #170: + * some models — deepseek-v4-flash observed — treat the tags as a "title" and write the body + * outside them): + * 1. the first `[summary]…[/summary]` pair whose content is non-empty once stray summary + * tags are stripped (so a nested `[summary]` line doesn't hide the text after it); + * 2. otherwise, everything left after stripping the (empty) summary blocks and stray tags — + * rescues output shaped `[summary]\n[/summary]\n`; + * 3. tagless output stays used verbatim. An empty return means the output was genuinely + * empty — the caller treats that as a failed attempt. + * Step 1 preserves the historical result for every healthy output, so resume re-extracting + * from an old Trace reconstructs the same summary the live session used. * * The legacy `` form must stay accepted **indefinitely**: * `compaction.prompt` is persisted in every existing agent's `system_config.yaml`, so old * agents keep instructing the model to use the angle-bracket tags. */ export function extractSummary(raw: string): string { - const match = matchDualForm(SUMMARY_PATTERNS, raw); - return (match ? match[1]! : raw).trim(); + for (const pattern of SUMMARY_PATTERNS) { + for (const match of raw.matchAll(pattern)) { + const inside = stripMarkerBlocks(match[1]!, MARKER_TAGS.summary).trim(); + if (inside !== "") return inside; + } + } + return stripMarkerBlocks(raw, MARKER_TAGS.summary).trim(); } /** One transcribed line inside a turn block: an inner tag, optional attributes, and the body. */ diff --git a/packages/core/src/omnimessage/types.ts b/packages/core/src/omnimessage/types.ts index 4837fc5..31e595c 100644 --- a/packages/core/src/omnimessage/types.ts +++ b/packages/core/src/omnimessage/types.ts @@ -280,29 +280,47 @@ export interface RequestBeginPayload { type: "request_begin"; } -export interface RequestEndPayload { - type: "request_end"; - /** Terminal state of this Request (reuses the six StopReason values, sharing its source with this turn's complete message's stop_reason / LLMOutcome; `auth` is the credentials-failure signal hosts key on). */ - status: StopReason; +/** + * The unified retry/failure detail block shared by `request_end` and `compaction_end` — + * one standard group of fields, stamped by the builders (the way `withOrigin` stamps + * `origin`) rather than accreting as scattered ad-hoc parameters. Every field is optional + * and additive: old Traces replay unchanged. + */ +export interface RetryDetail { /** - * Failure detail from `LLMOutcome.message`, present only on non-completed statuses: the - * real reason behind a retried/failed Request (e.g. `403 … (insufficient_user_quota)`), - * for observability — the server's error records / Cost center read it here because a - * retried request never produces an abort event. Additive: old Traces without it replay - * unchanged. + * Error detail, one name across the stack (`LLMOutcome.errorMessage` internally): present + * only on non-completed statuses — the real reason behind a retried/failed Request (e.g. + * `403 … (insufficient_user_quota)`), for observability — the server's error records / + * Cost center read it here because a retried request never produces an abort event. Not + * plain `message`: in this protocol "message" means an OmniMessage / model output, and + * this field is neither. (Traces written before the rename carry it as `message`; nothing + * reads that field semantically after the fact, so no dual-read is kept.) */ - message?: string; + error_message?: string; + /** + * 1-based ordinal of this Request within its retry run — the authoritative retry count + * the CLI/Web display verbatim. Stamped on every non-completed request_end and on a + * completed one that needed retries; absent on a clean first-try completion (the common + * case stays noise-free) and in old Traces. + */ + attempt?: number; /** * Planned in-run retry wait (ms) — present ONLY when the engine will retry this failure * within the same run (status `timeout`/`malformed` with attempts remaining under the * applicable cap). Computed by the same formula as the actual backoff sleep * (`reconnectDelayMs`), so the announced wait and the real one cannot drift; the Web App * renders it as a live countdown to the next attempt. Absent on final failures (an abort - * follows instead) and on completed requests. Additive: old Traces replay unchanged. + * follows instead) and on completed requests. */ retry_in_ms?: number; } +export interface RequestEndPayload extends RetryDetail { + type: "request_end"; + /** Terminal state of this Request (reuses the six StopReason values, sharing its source with this turn's complete message's stop_reason / LLMOutcome; `auth` is the credentials-failure signal hosts key on). */ + status: StopReason; +} + /** Compaction trigger reason: context threshold / turn-count threshold / user-initiated request. */ export type CompactionReason = "context" | "turns" | "manual"; @@ -328,7 +346,14 @@ export interface CompactionBeginPayload { turns: number; } -export interface CompactionEndPayload { +/** + * Inherits the shared RetryDetail block: `attempt` is the final attempt's 1-based ordinal + * (failed attempts and retries included — stamped by summarize mode), `error_message` is + * the last failure's detail (present only when `status` is `failed`), and `retry_in_ms` is + * never stamped here (compaction retries are announced on the compaction request's own + * request_end, which is Trace-only). + */ +export interface CompactionEndPayload extends RetryDetail { type: "compaction_end"; reason: CompactionReason; mode: CompactionMode; diff --git a/packages/core/src/state/default-config.ts b/packages/core/src/state/default-config.ts index 9dcdb69..5242cf9 100644 --- a/packages/core/src/state/default-config.ts +++ b/packages/core/src/state/default-config.ts @@ -156,18 +156,20 @@ Skills are reusable instruction packages at /agents//age - Session ID: {{SESSION_ID}}`; /** - * Built-in default compaction Prompt (summarize mode): tells the model that after - * compaction the raw transcript is no longer visible and the - * summary is the only record, so it must include everything needed to continue the task, - * and no tools may be called while writing the summary. + * Built-in default compaction Prompt (summarize mode): tells the model the summary will + * replace the transcript as its only record (so it must include everything needed to + * continue the task) and that no tools may be called. The format is shown as a concrete + * example rather than described in prose — some models treat the tags as a "title" and + * write the body after the closing tag (issue #170); extraction salvages that shape, but + * generating it right beats repairing it. Persisted per-agent in system_config.yaml — + * existing agents keep their stored prompt. */ export const DEFAULT_COMPACTION_PROMPT = - "You have a partial transcript of the task above. Write a summary of it wrapped in " + - "`[summary][/summary]` tags. This summary will replace the transcript: in the next " + - "context window the raw transcript above will no longer be visible and this summary " + - "will be its only record, so include everything needed to continue the task — the " + - "original request, current state, next steps, and any learnings. Do not call any " + - "tools while writing the summary; respond with text only."; + "Summarize the task transcript above. The summary will replace the transcript as its " + + "only record, so include everything needed to continue the task — the original request, " + + "current state, next steps, and any learnings. Do not call any tools; reply with text " + + "only, in exactly this format and nothing after it:\n\n" + + "[summary]put the summary text here...[/summary]"; /** * Default built-in system tools: file reading/editing/writing first, then bash execution diff --git a/packages/core/test/compaction.test.ts b/packages/core/test/compaction.test.ts index e65b8e7..03f550d 100644 --- a/packages/core/test/compaction.test.ts +++ b/packages/core/test/compaction.test.ts @@ -49,7 +49,7 @@ import type { LLMInterface, LLMOutcome, } from "../src/interfaces.js"; -import { ContextEngine } from "../src/engine/context-engine.js"; +import { ContextEngine, SUMMARY_RETRY_GUIDANCE } 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"; @@ -78,7 +78,7 @@ class ScriptedLLM implements LLMInterface { this.calls.push(params.newMessages); const next = this.responses.shift(); if (!next) { - return { status: "failed", message: `${this.label}: no scripted response` }; + return { status: "failed", errorMessage: `${this.label}: no scripted response` }; } for (const msg of next.messages) yield msg; return next.outcome ?? { status: "completed" }; @@ -308,7 +308,7 @@ describe("context compaction", () => { messages: [toolCall({ name: "t", arguments: "{}", toolCallId: "c1" }), usage(150, 150)], }, // Compaction request fails on the one status no ladder can fix (a rejected credential). - { messages: [], outcome: { status: "auth", message: "auth error" } }, + { messages: [], outcome: { status: "auth", errorMessage: "auth error" } }, // Original context is kept: the task continues, tool outputs feed back into the old instance as usual (context usage keeps growing). { messages: [assistantText("finished on old context"), usage(190, 340)] }, // Second trigger (context still over the limit) -> retries compaction at the boundary, this time succeeding. @@ -415,12 +415,55 @@ describe("context compaction", () => { const out = await collect(engine.run([userText("go")], { approve: allowAll })); const events = compactionEvents(out); - expect(events[1]).toMatchObject({ type: "compaction_end", status: "failed" }); + // The end event reports the attempts spent (issue #170's cost-center row message). + expect(events[1]).toMatchObject({ + type: "compaction_end", + status: "failed", + attempt: 2, + }); // The retry resends the original input (tool results + prompt; here there are no tool results, just the prompt). expect(llm1.calls).toHaveLength(3); expect(payloadTypes(llm1.calls[2]!)).toEqual(["text"]); }); + it("compaction transport retries default to the shared maxReconnects budget", async () => { + // Issue #170: without an explicit compactionMaxReconnects, the compaction request gets + // the same retry budget as the turn loop (here 4) — under the old tighter default of 3 + // this script would have failed before reaching the 5th, succeeding attempt. + const failing = (n: number): ScriptedResponse => ({ + messages: [], + outcome: { status: "failed", errorMessage: `blip ${n}` }, + }); + const llm1 = new ScriptedLLM( + [ + { messages: [assistantText("answer"), usage(150, 150)] }, + failing(1), + failing(2), + failing(3), + failing(4), + { messages: [assistantText("[summary]after the shared ladder[/summary]"), usage(30, 300)] }, + ], + "llm1", + ); + const llm2 = new ScriptedLLM([], "llm2"); + const engine = new ContextEngine({ + llm: llm1, + environment: fakeEnvironment, + compaction: settings(), + createLLM: () => llm2, + maxReconnects: 4, + reconnectBackoffMs: 1, + }); + + const out = await collect(engine.run([userText("go")], { approve: allowAll })); + expect(compactionEvents(out)[1]).toMatchObject({ + type: "compaction_end", + status: "completed", + attempt: 5, + }); + expect(llm1.calls).toHaveLength(6); + }); + it("a failed compaction request takes the ladder and can recover on a later attempt", async () => { // The compaction loop retries the same statuses the turn loop does: `failed` is where a // transient fault lands whenever the classifier doesn't recognize the gateway's wording, @@ -429,7 +472,7 @@ describe("context compaction", () => { const llm1 = new ScriptedLLM( [ { messages: [assistantText("answer"), usage(150, 150)] }, - { messages: [], outcome: { status: "failed", message: "502 upstream" } }, + { messages: [], outcome: { status: "failed", errorMessage: "502 upstream" } }, { messages: [assistantText("[summary]recovered[/summary]")] }, ], "llm1", @@ -455,11 +498,11 @@ describe("context compaction", () => { expect(payloadTypes(llm1.calls[2]!)).toEqual(["text"]); }); - it("the compaction loop uses its own cap, not the shared maxReconnects ladder", async () => { - // Compaction failure has graceful semantics (the original context is kept, compaction - // retries on the next trigger), so it fails fast: with compactionMaxReconnects=2, the - // compaction request runs 1+2 attempts and gives up — even though maxReconnects is far - // larger and would have kept the session stalled through the full exponential ladder. + it("an explicit compactionMaxReconnects overrides the shared maxReconnects budget", async () => { + // The dep still allows a caller to bound compaction retries independently: with + // compactionMaxReconnects=2, the compaction request runs 1+2 attempts and gives up — + // even though maxReconnects is far larger (the default merely *follows* maxReconnects, + // it does not override an explicit value). const llm1 = new ScriptedLLM( [ { messages: [assistantText("answer"), usage(150, 150)] }, @@ -495,26 +538,31 @@ describe("context compaction", () => { // The Trace-written compaction request_ends announce the planned backoff under the // COMPACTION cap (base 1ms: 1, then 2), with none on the final failure — these events // are never streamed, so the value lands in the Trace record only. - const retryPlans = recorded - .filter((m) => (m.payload as { type?: string }).type === "request_end") - .map((m) => (m.payload as { retry_in_ms?: number }).retry_in_ms); + const compactionEnds = recorded.filter( + (m) => (m.payload as { type?: string }).type === "request_end", + ); + const retryPlans = compactionEnds.map( + (m) => (m.payload as { retry_in_ms?: number }).retry_in_ms, + ); expect(retryPlans).toEqual([undefined, 1, 2, undefined]); + // The attempt ordinal rides the same events: the turn's clean completion stays + // unstamped, the three compaction failures count 1..3. + const attempts = compactionEnds.map((m) => (m.payload as { attempt?: number }).attempt); + expect(attempts).toEqual([undefined, 1, 2, 3]); }); - 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. + it("an empty compaction response (thinking only, no text) is retried like any failure: 5 attempts, then failed with the context kept", async () => { + // Issue #83/#170: the compaction request completes but yields no text. Committing the + // empty summary would discard the whole context and lose the task state — the response + // counts as one more failed attempt on the unified reconnect budget, and once the budget + // is exhausted 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. + // Five completed-but-empty compaction attempt: 4 retries of budget allow exactly 5. empty(1), empty(2), empty(3), @@ -538,7 +586,7 @@ describe("context compaction", () => { created += 1; return new ScriptedLLM([], "llm2"); }, - compactionMaxReconnects: 1, + compactionMaxReconnects: 4, reconnectBackoffMs: 1, }); const oldPath = trace.currentPath(); @@ -546,12 +594,17 @@ describe("context compaction", () => { 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. + // The end event reports the attempts spent (issue #170). 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). + expect(events[1]).toMatchObject({ + attempt: 5, + error_message: "the response contained no usable summary", + }); + // These scripted attempts carry no token_usage of their own, so none appears between the + // paired events (attempts that do carry usage surface it — see the 5th-attempt test). const types1 = payloadTypes(out1); const between = out1.slice( types1.indexOf("compaction_begin") + 1, @@ -560,14 +613,16 @@ describe("context compaction", () => { 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. + // Turn + exactly five compaction attempts (the 5th exhausts the budget, no 6th + // request); each retry leads with the corrective note, then the prompt — an empty + // response 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"]); + expect(llm1.calls[1]!.map(textOf)).toEqual(["COMPACT NOW"]); + for (let i = 2; i <= 5; i += 1) { + expect(llm1.calls[i]!.map(textOf)).toEqual([SUMMARY_RETRY_GUIDANCE, "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). + // An unusable attempt's request_end carries status `completed`, for which no retry_in_ms + // is ever announced (the backoff wait still ran between attempts). 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); @@ -585,9 +640,9 @@ describe("context compaction", () => { 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 () => { + it("tool-calling responses exhaust the retry budget: 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 + // text — but its assistant turn IS committed on the live LLM object. Each such attempt'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). @@ -621,7 +676,7 @@ describe("context compaction", () => { created += 1; return new ScriptedLLM([], "llm2"); }, - compactionMaxReconnects: 1, + compactionMaxReconnects: 4, reconnectBackoffMs: 1, }); const oldPath = trace.currentPath(); @@ -635,12 +690,18 @@ describe("context compaction", () => { expect(payloadTypes(out)).not.toContain("tool_call_output"); expect(created).toBe(0); + // The end event reports the attempts spent and the last failure (issue #170's + // cost-center row message and the frontend banner detail). + expect(events[1]).toMatchObject({ + attempt: 5, + error_message: "the response called tools instead of writing a summary", + }); // Five attempts; from the second on, the input leads with the repair answering the - // previous rejection's call, then re-issues the prompt. + // previous attempt's call, then the corrective note, then 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"]); + expect(payloadTypes(retry)).toEqual(["tool_call_output", "text", "text"]); const repair = retry[0]!.payload as { tool_call_id: string; output: string; @@ -651,7 +712,8 @@ describe("context compaction", () => { "[tool error] the compaction request expects a summary, not tool calls", ); expect(repair.stop_reason).toBe("failed"); - expect(textOf(retry[1]!)).toBe("COMPACT NOW"); + expect(textOf(retry[1]!)).toBe(SUMMARY_RETRY_GUIDANCE); + expect(textOf(retry[2]!)).toBe("COMPACT NOW"); } // All five synthesized repairs are written to the (old) Trace for replay to mirror. @@ -681,8 +743,9 @@ describe("context compaction", () => { }); 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). + // Counting pin for the unified budget: four failed attempts spend the 4 retries, but the + // 5th (last allowed) attempt still gets its chance — a valid summary there succeeds + // (a 5th failure would exhaust the budget instead). const llm1 = new ScriptedLLM( [ { messages: [assistantText("answer"), usage(150, 150)] }, @@ -706,7 +769,7 @@ describe("context compaction", () => { factoryTokens = tokens; return llm2; }, - maxReconnects: 1, + compactionMaxReconnects: 4, reconnectBackoffMs: 1, }); @@ -714,8 +777,10 @@ describe("context compaction", () => { 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. + // Every attempt's token_usage is surfaced between the paired events — rejected attempts + // burn real tokens, and hiding them understated the cost center (issue #170). Here the + // 1st (rejected) and 5th (adopted) attempts carry usage. + expect(events[1]).toMatchObject({ attempt: 5 }); const types = payloadTypes(out); const between = out.slice( types.indexOf("compaction_begin") + 1, @@ -724,8 +789,9 @@ describe("context compaction", () => { 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); + expect(usageBetween.map((m) => (m.payload as TokenUsagePayload).request.total)).toEqual([ + 160, 170, + ]); // Session cumulative tokens carried into the new instance include the rejected attempts' usage. expect(factoryTokens).toMatchObject({ total: 480 }); @@ -778,10 +844,11 @@ describe("context compaction", () => { status: "completed", }); - // The retried input answers the rejected attempt's call first, then re-issues the prompt. + // The retried input answers the rejected attempt's call first, then carries the + // corrective note and re-issues the prompt. expect(llm1.calls).toHaveLength(3); const retry = llm1.calls[2]!; - expect(payloadTypes(retry)).toEqual(["tool_call_output", "text"]); + expect(payloadTypes(retry)).toEqual(["tool_call_output", "text", "text"]); const repair = retry[0]!.payload as { tool_call_id: string; output: string; @@ -792,7 +859,8 @@ describe("context compaction", () => { "[tool error] the compaction request expects a summary, not tool calls", ); expect(repair.stop_reason).toBe("failed"); - expect(textOf(retry[1]!)).toBe("COMPACT NOW"); + expect(textOf(retry[1]!)).toBe(SUMMARY_RETRY_GUIDANCE); + expect(textOf(retry[2]!)).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. @@ -897,6 +965,7 @@ describe("context compaction", () => { created += 1; return new ScriptedLLM([], "llm2"); }, + compactionMaxReconnects: 4, reconnectBackoffMs: 1, }); const oldPath = trace.currentPath(); @@ -908,15 +977,16 @@ describe("context compaction", () => { // is the whole story. expect(payloadTypes(out1)).not.toContain("abort"); expect(llm1.calls).toHaveLength(6); - // Attempt 1 folds the turn's tool output in; attempt 2 carries the repair + Prompt but - // NOT the absorbed output; attempts 3-5 are Prompt-only. + // Attempt 1 folds the turn's tool output in; attempt 2 carries the repair + note + Prompt + // but NOT the absorbed output; attempts 3-5 are note + Prompt. expect(payloadTypes(llm1.calls[1]!)).toEqual(["tool_call_output", "text"]); expect((llm1.calls[1]![0]!.payload as { tool_call_id?: string }).tool_call_id).toBe("ct"); - expect(payloadTypes(llm1.calls[2]!)).toEqual(["tool_call_output", "text"]); + expect(payloadTypes(llm1.calls[2]!)).toEqual(["tool_call_output", "text", "text"]); expect((llm1.calls[2]![0]!.payload as { tool_call_id?: string }).tool_call_id).toBe("c1"); - expect(textOf(llm1.calls[2]![1]!)).toBe("COMPACT NOW"); + expect(textOf(llm1.calls[2]![1]!)).toBe(SUMMARY_RETRY_GUIDANCE); + expect(textOf(llm1.calls[2]![2]!)).toBe("COMPACT NOW"); for (let attempt = 3; attempt <= 5; attempt += 1) { - expect(llm1.calls[attempt]!.map(textOf)).toEqual(["COMPACT NOW"]); + expect(llm1.calls[attempt]!.map(textOf)).toEqual([SUMMARY_RETRY_GUIDANCE, "COMPACT NOW"]); } // Original context kept: no LLM swap, no Trace rotation. expect(created).toBe(0); @@ -955,16 +1025,17 @@ describe("context compaction", () => { environment: fakeEnvironment, compaction: settings(), createLLM: () => new ScriptedLLM([], "llm2"), + compactionMaxReconnects: 4, reconnectBackoffMs: 1, }); const out = await collect(engine.run([userText("go")], { approve: allowAll })); expect(compactionEvents(out)[1]).toMatchObject({ type: "compaction_end", status: "failed" }); expect(llm1.calls).toHaveLength(7); - // Attempt 1 folds the outputs; attempts 2-5 are Prompt-only (absorbed by the first commit). + // Attempt 1 folds the outputs; attempts 2-5 are note + Prompt (absorbed by the first commit). expect(payloadTypes(llm1.calls[1]!)).toEqual(["tool_call_output", "text"]); for (let attempt = 2; attempt <= 5; attempt += 1) { - expect(llm1.calls[attempt]!.map(textOf)).toEqual(["COMPACT NOW"]); + expect(llm1.calls[attempt]!.map(textOf)).toEqual([SUMMARY_RETRY_GUIDANCE, "COMPACT NOW"]); } // The continuation input is the repair answering c5 — nothing else: no absorbed outputs, // no prompt. @@ -1044,8 +1115,8 @@ describe("context compaction", () => { expect(events[1]).toMatchObject({ type: "compaction_end", status: "aborted" }); expect(payloadTypes(out)).toContain("abort"); expect(llm1.calls).toHaveLength(3); - // Attempt 2 carried the repair + Prompt (not the absorbed outputs). - expect(payloadTypes(llm1.calls[2]!)).toEqual(["tool_call_output", "text"]); + // Attempt 2 carried the repair + note + Prompt (not the absorbed outputs). + expect(payloadTypes(llm1.calls[2]!)).toEqual(["tool_call_output", "text", "text"]); expect((llm1.calls[2]![0]!.payload as { tool_call_id?: string }).tool_call_id).toBe("c1"); // The next run leads with the still-unanswered repair; the absorbed turn outputs are @@ -1255,6 +1326,46 @@ describe("context compaction", () => { ); }); + it("rescues a summary written outside the tags (issue #170) on the first attempt", async () => { + // deepseek-v4-flash writes `[summary]\n[/summary]` as a "title" and the body after the + // closing tag. Extraction salvages the outside text, so the very first attempt succeeds — + // no rejection loop, no burned retries. + const llm1 = new ScriptedLLM( + [ + { messages: [assistantText("answer"), usage(150, 150)] }, + { + messages: [ + assistantText("[summary]\n[/summary]\nRescued body outside the tags."), + usage(40, 200), + ], + }, + ], + "llm1", + ); + const llm2 = new ScriptedLLM([{ messages: [assistantText("fresh"), usage(10, 210)] }], "llm2"); + const engine = new ContextEngine({ + llm: llm1, + environment: fakeEnvironment, + compaction: settings(), + createLLM: () => llm2, + reconnectBackoffMs: 1, + }); + + const out = await collect(engine.run([userText("task one")], { approve: allowAll })); + expect(compactionEvents(out)[1]).toMatchObject({ + type: "compaction_end", + status: "completed", + attempt: 1, + }); + // No rejection retry: the turn request plus a single compaction attempt. + expect(llm1.calls).toHaveLength(2); + await collect(engine.run([userText("task two")], { approve: allowAll })); + expect(llm2.calls[0]!.map(textOf)).toEqual([ + "[context_summary]\nRescued body outside the tags.\n[/context_summary]", + "task two", + ]); + }); + it("manual compaction skips threshold checks and reuses the same flow", async () => { const llm1 = new ScriptedLLM( [ @@ -1356,6 +1467,7 @@ describe("context compaction", () => { environment: fakeEnvironment, compaction: settings(), createLLM: () => new ScriptedLLM([], "llm2"), + compactionMaxReconnects: 4, reconnectBackoffMs: 1, }); await collect(engine.run([userText("start")], { approve: allowAll })); @@ -1368,11 +1480,11 @@ describe("context compaction", () => { const out = await collect(engine.compact()); expect(compactionEvents(out)[1]).toMatchObject({ type: "compaction_end", status: "failed" }); expect(llm1.calls).toHaveLength(6); - // Attempt 1 folded the carry-over in (and committed it); later attempts resend the Prompt - // alone — the absorbed carry-over never again. + // Attempt 1 folded the carry-over in (and committed it); later attempts resend the note + + // Prompt — the absorbed carry-over never again. expect(llm1.calls[1]!.map(textOf)).toEqual(["pending question", "COMPACT NOW"]); for (let attempt = 2; attempt <= 5; attempt += 1) { - expect(llm1.calls[attempt]!.map(textOf)).toEqual(["COMPACT NOW"]); + expect(llm1.calls[attempt]!.map(textOf)).toEqual([SUMMARY_RETRY_GUIDANCE, "COMPACT NOW"]); } // The carry-over is consumed at the seam — the next run leads with the stashed repair diff --git a/packages/core/test/describe-image.test.ts b/packages/core/test/describe-image.test.ts index f6d4028..687cb9a 100644 --- a/packages/core/test/describe-image.test.ts +++ b/packages/core/test/describe-image.test.ts @@ -177,7 +177,7 @@ describe("describe_image (the text-only-model variant of read_image)", () => { it("vision model request failure: failed with the status and message", async () => { await writeFile(path.join(tmp, "a.png"), PNG_1X1); - const { llm } = fakeLLM("", { status: "failed", message: "401 unauthorized" }); + const { llm } = fakeLLM("", { status: "failed", errorMessage: "401 unauthorized" }); const { result, text } = await run({ source: "a.png" }, tmp, { modelId: "vis-1", createLLM: () => llm, diff --git a/packages/core/test/engine.test.ts b/packages/core/test/engine.test.ts index 9f9de15..9a3708c 100644 --- a/packages/core/test/engine.test.ts +++ b/packages/core/test/engine.test.ts @@ -684,7 +684,7 @@ describe("ContextEngine ReAct loop (mock LLM, approve callback)", () => { yield partialText("start", ""); yield partialText("delta", "half a thought"); // `auth`: the one LLM status that still exits straight to the flatten path. - return { status: "auth", message: "boom" }; + return { status: "auth", errorMessage: "boom" }; } yield assistantText("ok"); yield tokenUsage(emptyTokenCounts(), { @@ -1646,7 +1646,7 @@ describe("ContextEngine LLM timeout / network interruption (PRN-012)", () => { toolCallId: "tc-broken", stopReason: "malformed", }); - return { status: "malformed", message: "incomplete stream" }; + return { status: "malformed", errorMessage: "incomplete stream" }; } yield assistantText("done"); yield tokenUsage(emptyTokenCounts(), { @@ -1799,7 +1799,7 @@ describe("ContextEngine LLM timeout / network interruption (PRN-012)", () => { async *streamGenerate(params) { calls += 1; inputs.push(params.newMessages); - return { status: "failed", message: "400 unknown parameter: max_output_tokens" }; + return { status: "failed", errorMessage: "400 unknown parameter: max_output_tokens" }; }, }; const environment = new Environment({ @@ -1884,7 +1884,7 @@ describe("ContextEngine LLM timeout / network interruption (PRN-012)", () => { // eslint-disable-next-line require-yield async *streamGenerate() { calls += 1; - return { status: "auth", message: "401 invalid x-api-key" }; + return { status: "auth", errorMessage: "401 invalid x-api-key" }; }, }; const environment = new Environment({ @@ -1898,7 +1898,9 @@ describe("ContextEngine LLM timeout / network interruption (PRN-012)", () => { // The request's own terminal status is the host signal (streams to the web). const end = all.find((m) => (m.payload as { type?: string }).type === "request_end"); expect((end!.payload as { status?: string }).status).toBe("auth"); - expect((end!.payload as { message?: string }).message).toBe("401 invalid x-api-key"); + expect((end!.payload as { error_message?: string }).error_message).toBe( + "401 invalid x-api-key", + ); // No planned retry is announced for a terminal failure. expect((end!.payload as { retry_in_ms?: number }).retry_in_ms).toBeUndefined(); const abort = all.find((m) => (m.payload as { type?: string }).type === "abort"); @@ -2037,7 +2039,10 @@ describe("ContextEngine LLM timeout / network interruption (PRN-012)", () => { if (calls === 1) { // A retryable provider rejection: the detail must reach observability via the // event — a retried request never produces an abort to carry it. - return { status: "timeout", message: "403 quota exceeded (insufficient_user_quota)" }; + return { + status: "timeout", + errorMessage: "403 quota exceeded (insufficient_user_quota)", + }; } yield assistantText("ok"); yield tokenUsage(emptyTokenCounts(), { @@ -2057,16 +2062,21 @@ describe("ContextEngine LLM timeout / network interruption (PRN-012)", () => { const all = await collectRun(engine, [userText("go")], allowAll); const ends = all.filter((m) => (m.payload as { type?: string }).type === "request_end") as { - payload: { status?: string; message?: string; retry_in_ms?: number }; + payload: { status?: string; error_message?: string; attempt?: number; retry_in_ms?: number }; }[]; expect(ends).toHaveLength(2); expect(ends[0]!.payload.status).toBe("timeout"); - expect(ends[0]!.payload.message).toBe("403 quota exceeded (insufficient_user_quota)"); - // The engine will retry: the planned backoff is announced (base 0 here -> 0ms). + expect(ends[0]!.payload.error_message).toBe("403 quota exceeded (insufficient_user_quota)"); + // The engine will retry: the planned backoff is announced (base 0 here -> 0ms), and the + // authoritative attempt ordinal is stamped (this was the run's 1st request). expect(ends[0]!.payload.retry_in_ms).toBe(0); + expect(ends[0]!.payload.attempt).toBe(1); expect(ends[1]!.payload.status).toBe("completed"); - expect(ends[1]!.payload.message).toBeUndefined(); + expect(ends[1]!.payload.error_message).toBeUndefined(); expect(ends[1]!.payload.retry_in_ms).toBeUndefined(); + // A completion that needed retries still carries its ordinal ("recovered on attempt 2"); + // only a clean first-try completion stays unstamped. + expect(ends[1]!.payload.attempt).toBe(2); }); it("request_end announces the planned backoff (retry_in_ms) from the shared ladder; absent once the cap is reached", async () => { @@ -2178,7 +2188,7 @@ describe("ContextEngine LLM timeout / network interruption (PRN-012)", () => { // exhausted-retries test below). yield thinkingMessage("half-thought", "failed"); yield assistantText("half-text", "failed"); - return { status: "auth", message: "boom" }; + return { status: "auth", errorMessage: "boom" }; } yield assistantText("ok"); yield tokenUsage(emptyTokenCounts(), { diff --git a/packages/core/test/llm.test.ts b/packages/core/test/llm.test.ts index 609a485..859ad94 100644 --- a/packages/core/test/llm.test.ts +++ b/packages/core/test/llm.test.ts @@ -1573,7 +1573,7 @@ describe("GenerativeModel.streamGenerate outcome classification (PRN-013)", () = model.streamGenerate({ newMessages: [userText("go")] }), ); expect(outcome.status).toBe("malformed"); - expect(outcome.message).toContain("Unexpected token"); + expect(outcome.errorMessage).toContain("Unexpected token"); const complete = messages.find((m) => typeOf(m) === "text"); expect((complete!.payload as TextPayload).text).toBe("hi"); expect((complete!.payload as TextPayload).stop_reason).toBe("malformed"); @@ -1634,7 +1634,7 @@ describe("GenerativeModel.streamGenerate outcome classification (PRN-013)", () = // 401 = credentials failure: its own stop reason so hosts can gate input until the // model's API key is updated (same engine behavior as failed). expect(outcome.status).toBe("auth"); - expect(outcome.message).toContain("invalid api key"); + expect(outcome.errorMessage).toContain("invalid api key"); expect(messages.map(typeOf)).not.toContain("token_usage"); // A genuinely non-retryable parameter error stays a plain failure. @@ -1663,7 +1663,7 @@ describe("GenerativeModel.streamGenerate outcome classification (PRN-013)", () = model.streamGenerate({ newMessages: [userText("go")] }), ); expect(outcome.status).toBe("auth"); - expect(outcome.message).toContain("subscription key invalid"); + expect(outcome.errorMessage).toContain("subscription key invalid"); expect(messages.map(typeOf)).not.toContain("token_usage"); }); @@ -1681,7 +1681,7 @@ describe("GenerativeModel.streamGenerate outcome classification (PRN-013)", () = expect(outcome.status).toBe("timeout"); // The real reason rides on the outcome (request_end -> the Cost center's errors panel // shows it): a retried quota rejection must not surface as a bare "timeout". - expect(outcome.message).toContain("insufficient_user_quota"); + expect(outcome.errorMessage).toContain("insufficient_user_quota"); expect(messages.map(typeOf)).not.toContain("token_usage"); }); diff --git a/packages/core/test/markers.test.ts b/packages/core/test/markers.test.ts index 6183ca5..4f302c9 100644 --- a/packages/core/test/markers.test.ts +++ b/packages/core/test/markers.test.ts @@ -132,6 +132,23 @@ describe("engine blocks ([turn_aborted] / [turn_retried] / [context_summary] / [ expect(extractSummary("[summary]a[/summary] b")).toBe("a"); }); + it("extractSummary rescues content the model wrote outside the tags (issue #170)", () => { + // deepseek-v4-flash writes the tags as a "title" and the body after the closing tag. + expect(extractSummary("[summary]\n[/summary]\nThe actual summary body.")).toBe( + "The actual summary body.", + ); + expect(extractSummary("[summary]\n[summary][/summary]\nBody after a doubled open tag.")).toBe( + "Body after a doubled open tag.", + ); + // A stray opening tag inside the pair doesn't hide the text that follows it. + expect(extractSummary("[summary]\n[summary]\nBody inside.[/summary]")).toBe("Body inside."); + // An empty current-form pair doesn't shadow a non-empty legacy pair. + expect(extractSummary("[summary][/summary]legacy body")).toBe("legacy body"); + // Genuinely empty output still extracts to "" — the engine treats that as a failed attempt. + expect(extractSummary("[summary][/summary]")).toBe(""); + expect(extractSummary("[summary]\n[/summary]")).toBe(""); + }); + it("turn blocks round-trip through unwrapSyntheticBlock, both forms, single-level", () => { const lines = [transcribeUserInput("go"), transcribeToolCall("read_file", "t1", "{}")]; const aborted = buildTurnAbortedBlock(lines); diff --git a/packages/core/test/session-title.test.ts b/packages/core/test/session-title.test.ts index a865c31..d2ee8d1 100644 --- a/packages/core/test/session-title.test.ts +++ b/packages/core/test/session-title.test.ts @@ -97,7 +97,7 @@ describe("session-title", () => { assistantText("partial"), tokenUsage(emptyTokenCounts(), { cache_read: 0, cache_write: 0, output: 1, total: 1 }), ], - { status: "failed", message: "401" }, + { status: "failed", errorMessage: "401" }, ), { userText: "u", assistantText: "a" }, ); diff --git a/packages/docs/content/agent-loop.en.md b/packages/docs/content/agent-loop.en.md index ffdb842..0c1e198 100644 --- a/packages/docs/content/agent-loop.en.md +++ b/packages/docs/content/agent-loop.en.md @@ -103,7 +103,7 @@ Goal mode is the exception because its objective is re-injected as text every ro ## Automatic reconnect -Every LLM-side failure except `auth` triggers an in-run reconnect — `timeout` (network timeouts, transport disconnects, rate limits, 5xx, transient provider quota errors), `malformed` (truncated streams, JSON parse failures), and **`failed` as well**. `failed` retries even though the classifier judged it non-transient: that judgement is an allowlist of known codes, statuses and message vocabulary, so a gateway phrasing a transient fault its own way (`Upstream HTTP/2 stream failed`, say) lands there and used to kill the turn. Retrying a genuinely permanent error costs the ladder and ends the same way; aborting a transient one destroys the turn. Note this changes the *policy*, not the *taxonomy*: a `failed` request is still recorded as `failed` on its `request_end` and in the Cost center, rather than being relabelled a timeout. On a reconnect the engine re-sends the original input plus a `[turn_retried]` block carrying the previous partial output, so tools are never re-executed. Default limit is 5 reconnects with exponential backoff under a ceiling (base 250ms, cap 30s: 250ms, 500ms, 1s, 2s, 4s ≈ 7.75s of total patience — one shared schedule growing toward the slower retryable classes, with the first steps as fast as a transport blip needs); beyond that the turn settles as `failed`. Each failure's `request_end` announces the planned wait as `retry_in_ms` (same formula as the sleep), which the Web App renders as a live countdown with "retry now" (skips the remaining wait via `Session.skipReconnectWait` — the attempt counter is unchanged) and "give up" (the ordinary abort; the engine's abort-during-backoff path ends the turn) controls; the CLI prints its own `[retry]` line. All three retryable statuses render identically — a retry the user cannot see is a stalled session with no explanation and no way out. Compaction requests retry the same statuses under their own tighter cap (3 retries): a compaction that gives up keeps the original context and tries again at the next trigger, so a short ladder there beats stalling the session behind the full one. Authentication errors are classified before any retry heuristic and never retry: the request ends with its own terminal status `auth` (only the model reference is fixed at Session creation — credentials are read from the current Project config when the Session loads), and the Web App disables that Session's composer until the model's credential is updated (which auto-unlocks it) or the notice is dismissed for a retry. Tool errors are never retried — they are fed back to the model as `tool_call_output` and the model decides what to do next. +Every LLM-side failure except `auth` triggers an in-run reconnect — `timeout` (network timeouts, transport disconnects, rate limits, 5xx, transient provider quota errors), `malformed` (truncated streams, JSON parse failures), and **`failed` as well**. `failed` retries even though the classifier judged it non-transient: that judgement is an allowlist of known codes, statuses and message vocabulary, so a gateway phrasing a transient fault its own way (`Upstream HTTP/2 stream failed`, say) lands there and used to kill the turn. Retrying a genuinely permanent error costs the ladder and ends the same way; aborting a transient one destroys the turn. Note this changes the *policy*, not the *taxonomy*: a `failed` request is still recorded as `failed` on its `request_end` and in the Cost center, rather than being relabelled a timeout. On a reconnect the engine re-sends the original input plus a `[turn_retried]` block carrying the previous partial output, so tools are never re-executed. Default limit is 5 reconnects with exponential backoff under a ceiling (base 250ms, cap 30s: 250ms, 500ms, 1s, 2s, 4s ≈ 7.75s of total patience — one shared schedule growing toward the slower retryable classes, with the first steps as fast as a transport blip needs); beyond that the turn settles as `failed`. Each failure's `request_end` announces the planned wait as `retry_in_ms` (same formula as the sleep) and stamps `attempt`, the authoritative 1-based ordinal of the request within its retry run (the CLI and Web App display it verbatim); the Web App renders the wait as a live countdown with "retry now" (skips the remaining wait via `Session.skipReconnectWait` — the attempt counter is unchanged) and "give up" (the ordinary abort; the engine's abort-during-backoff path ends the turn) controls; the CLI prints its own `[retry]` line. All three retryable statuses render identically — a retry the user cannot see is a stalled session with no explanation and no way out. A compaction request is an ordinary LLM request and by default retries on the same cap and ladder (an unusable summary draws on the same budget — see "Context compaction"); a compaction that gives up keeps the original context and tries again at the next trigger. Authentication errors are classified before any retry heuristic and never retry: the request ends with its own terminal status `auth` (only the model reference is fixed at Session creation — credentials are read from the current Project config when the Session loads), and the Web App disables that Session's composer until the model's credential is updated (which auto-unlocks it) or the notice is dismissed for a retry. Tool errors are never retried — they are fed back to the model as `tool_call_output` and the model decides what to do next. ## Compaction @@ -126,9 +126,9 @@ Three triggers (`compaction_begin.reason`): | `turns` | Session turn count ≥ `maxSessionTurns` (default -1 = unlimited) | | `manual` | the user runs `/compact` or calls `session.compact()` | -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`). +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. Summary extraction applies a tolerance ladder: the first non-empty `[summary]` tag pair wins; when every pair is empty, the text left after stripping the tags is used instead (rescuing models that write the body after the closing tag); with no tags at all, the whole output is used verbatim. 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. Retryable attempts (`failed`/`timeout`/`malformed`, the same set the turn loop retries) follow the compaction-specific reconnect cap and backoff ladder described under "Automatic reconnect" above; only `auth` stops the compaction at once. The first **committed** attempt — adopted or rejected — also absorbs whatever turn input was folded into the compaction request (mid-task tool results, or the carry-over a manual `/compact` folds in) into the old context's history: retries resend only the repairs and the Prompt, and the continuation after an abandoned compaction never resends the absorbed input. +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 counts as one more failed attempt — any tool calls are answered with synthesized failed outputs (keeping `tool_use`/`tool_result` pairing intact), and the resent request carries a corrective note ahead of the compaction Prompt (committed history can only be appended to; rewriting it would invalidate the prompt cache). Every failure — unusable summaries and the `failed`/`timeout`/`malformed` transport statuses alike — draws on the one reconnect budget and backoff ladder described under "Automatic reconnect" above; only `auth` stops the compaction at once. Once the budget is exhausted the compaction ends `failed`, keeping the original context and Trace file until the next trigger; `compaction_end` reuses the unified retry detail block — `attempt` (the final attempt's ordinal, failed attempts included) and, on failure, the last `error_message` detail (shown on the chat banner and the CLI line) — and a failed compaction also lands in the cost center as a `compaction_failed` error record. The first **committed** attempt — adopted or rejected — also absorbs whatever turn input was folded into the compaction request (mid-task tool results, or the carry-over a manual `/compact` folds in) into the old context's history: retries resend only the repairs and the Prompt, and the continuation after an abandoned compaction never resends the absorbed input. ## Concurrency model diff --git a/packages/docs/content/agent-loop.zh.md b/packages/docs/content/agent-loop.zh.md index 542ac2d..15a5ec6 100644 --- a/packages/docs/content/agent-loop.zh.md +++ b/packages/docs/content/agent-loop.zh.md @@ -100,7 +100,7 @@ Task 运行期间，宿主可通过 `session.steer(input)` 排队一条用户消 ## 自动重连 -除 `auth` 外，LLM 侧的所有失败都会触发引擎内自动重连——`timeout`（网络超时、传输层断连、限流、5xx、瞬时的供应商额度错误）、`malformed`（流截断、JSON 解析失败），**以及 `failed`**。`failed` 也重试，尽管分类器判定它不是瞬时错误：那个判定本质是一张允许清单（已知错误码、状态码与消息措辞），所以用自己说法描述瞬时故障的网关（例如 `Upstream HTTP/2 stream failed`）会落到这一档，此前会直接终止本轮。重试一个真正的永久错误，代价是走完退避梯度后以同样的方式收场；而把瞬时错误直接中断，则毁掉这一轮。注意改的是**策略**而非**分类**：`failed` 请求在 `request_end` 与成本中心里仍然记为 `failed`，不会被改标成超时。重连时同一次 `run` 内重发原始输入，并附加 `[turn_retried]` 块携带上一次的部分输出，避免工具重复执行。默认最多重连 5 次，指数退避并设上限(基数 250ms、上限 30s：250ms、500ms、1s、2s、4s，总耐心约 7.75s——所有可重试类别共用一张时间表，向较慢的类别递增，头几步仍与传输层抖动所需的一样快)；超限后该轮以 `failed` 收场。每次失败的 `request_end` 会以 `retry_in_ms` 宣告计划中的等待(与实际休眠同一公式)，Web App 据此实时倒计时，并提供「立即重试」(经 `Session.skipReconnectWait` 跳过剩余等待——重试计数不变)与「放弃」(普通中断；引擎的退避中中断路径结束本轮)两个内联按钮，CLI 则打印自己的 `[重试]` 行。三种可重试终态的渲染完全一致——用户看不见的重试，等于一次没有任何解释、也无从退出的卡顿。压缩请求重试同样的终态，只是用独立的更小上限(重试 3 次)：压缩放弃后会保留原上下文、等下一次触发再试，所以那里用一段短梯度好过让会话干等完整的一段。鉴权错误在任何重试启发式之前判定、从不重试：请求以专属终态 `auth` 收场(Session 锁定的只是模型引用，凭据在会话装载时取自当前 Project 配置)，Web App 据此禁用该 Session 的输入框，直到该模型的凭据被更新(更新后自动解锁)或用户点击「重试」。工具错误从不重试——它们作为 `tool_call_output` 反馈给模型，由模型决定下一步。 +除 `auth` 外，LLM 侧的所有失败都会触发引擎内自动重连——`timeout`（网络超时、传输层断连、限流、5xx、瞬时的供应商额度错误）、`malformed`（流截断、JSON 解析失败），**以及 `failed`**。`failed` 也重试，尽管分类器判定它不是瞬时错误：那个判定本质是一张允许清单（已知错误码、状态码与消息措辞），所以用自己说法描述瞬时故障的网关（例如 `Upstream HTTP/2 stream failed`）会落到这一档，此前会直接终止本轮。重试一个真正的永久错误，代价是走完退避梯度后以同样的方式收场；而把瞬时错误直接中断，则毁掉这一轮。注意改的是**策略**而非**分类**：`failed` 请求在 `request_end` 与成本中心里仍然记为 `failed`，不会被改标成超时。重连时同一次 `run` 内重发原始输入，并附加 `[turn_retried]` 块携带上一次的部分输出，避免工具重复执行。默认最多重连 5 次，指数退避并设上限(基数 250ms、上限 30s：250ms、500ms、1s、2s、4s，总耐心约 7.75s——所有可重试类别共用一张时间表，向较慢的类别递增，头几步仍与传输层抖动所需的一样快)；超限后该轮以 `failed` 收场。每次失败的 `request_end` 会以 `retry_in_ms` 宣告计划中的等待(与实际休眠同一公式)、以 `attempt` 标注这是本轮第几次尝试(权威序号,CLI 与 Web 的重试行直接显示它)，Web App 据此实时倒计时，并提供「立即重试」(经 `Session.skipReconnectWait` 跳过剩余等待——重试计数不变)与「放弃」(普通中断；引擎的退避中中断路径结束本轮)两个内联按钮，CLI 则打印自己的 `[重试]` 行。三种可重试终态的渲染完全一致——用户看不见的重试，等于一次没有任何解释、也无从退出的卡顿。压缩请求是一次普通的 LLM 请求，默认沿用同一重连上限与退避阶梯（无效摘要也计入同一预算，见「上下文压缩」一节）；压缩放弃后保留原上下文、等下一次触发再试。鉴权错误在任何重试启发式之前判定、从不重试：请求以专属终态 `auth` 收场(Session 锁定的只是模型引用，凭据在会话装载时取自当前 Project 配置)，Web App 据此禁用该 Session 的输入框，直到该模型的凭据被更新(更新后自动解锁)或用户点击「重试」。工具错误从不重试——它们作为 `tool_call_output` 反馈给模型，由模型决定下一步。 ## 上下文压缩(Compaction) @@ -123,9 +123,9 @@ interface CompactionSettings { | `turns` | Session 轮数 ≥ `maxSessionTurns`(默认 -1，即不限) | | `manual` | 用户执行 `/compact` 或调用 `session.compact()` | -两种模式：`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`)。 +两种模式：`summarize`(默认)向旧上下文追加压缩 Prompt，提取 `[summary]` 后包装为 `[context_summary]` 用户文本，在**全新的模型上下文**中继续；`discard` 直接丢弃旧上下文。系统标记统一写作 `[tag]…[/tag]`；读取旧 Trace 与旧压缩 Prompt 时仍识别早期的尖括号形式（``、`` 等）。摘要提取自带容忍阶梯：优先取第一个非空的 `[summary]` 标签对；标签对全为空时，改取剥离标签后剩余的全部文本（兼容把正文写在闭合标签之后的模型）；完全没有标签则整段输出照用。压缩时 [Trace 文件随之轮转](/sessions-and-traces)(`_002`、`_003`……)，一个 Trace 文件恒等于一个完整模型上下文。`session.compact()` 前可用 `compactability()` 探询可行性(`ok | unsupported | empty | just_compacted`)。 -压缩请求**保持会话工具集不变**——请求前缀（含工具列表）与普通轮次逐字节一致，确保上下文最大的时刻提供商的提示词缓存依然有效。只有得到有效摘要，压缩才算成功：若响应中出现工具调用、或提取出的摘要为空，则判为无效并重试——工具调用会先以合成的失败输出逐一应答（保持 `tool_use`/`tool_result` 配对完整），修复后的请求**立即重发**（无效摘要是模型行为而非传输故障，不做退避），最多允许 5 次无效尝试，之后压缩以 `failed` 结束，保留原上下文与 Trace 文件，等待下次触发。可重试的结束态（`failed`/`timeout`/`malformed`，与轮次循环同一套）走上文「自动重连」一节所述的压缩专用重连上限与退避阶梯；只有 `auth` 会让压缩当场停止。首个**已提交**的尝试（无论被采纳还是被判无效）同时会把折叠进压缩请求的本轮输入（任务中途的工具结果、手动 `/compact` 折叠的补发内容）吸收进旧上下文：重试只重发修复输出与压缩 Prompt，压缩放弃后的续跑也不再重发已吸收的输入。 +压缩请求**保持会话工具集不变**——请求前缀（含工具列表）与普通轮次逐字节一致，确保上下文最大的时刻提供商的提示词缓存依然有效。只有得到有效摘要，压缩才算成功：若响应中出现工具调用、或提取出的摘要为空，视同一次普通的失败尝试——工具调用先以合成的失败输出逐一应答（保持 `tool_use`/`tool_result` 配对完整），重发的请求在压缩 Prompt 前附加一条纠正说明（已提交的历史只能追加、不能改写，改写会使提示词缓存失效）。所有失败——无效摘要与 `failed`/`timeout`/`malformed` 传输失败——共用上文「自动重连」一节的同一重连预算与退避阶梯；只有 `auth` 会让压缩当场停止。预算耗尽后压缩以 `failed` 结束，保留原上下文与 Trace 文件，等待下次触发；`compaction_end` 复用统一的重试详情块——`attempt`(最终尝试序号,失败尝试也计入)与失败时的 `error_message` 详情(聊天横幅与 CLI 直接展示)，压缩失败还会作为一条 `compaction_failed` 错误记录进入成本中心。首个**已提交**的尝试（无论被采纳还是被判无效）同时会把折叠进压缩请求的本轮输入（任务中途的工具结果、手动 `/compact` 折叠的补发内容）吸收进旧上下文：重试只重发修复输出与压缩 Prompt，压缩放弃后的续跑也不再重发已吸收的输入。 ## 并发模型 diff --git a/packages/docs/content/omni-message.en.md b/packages/docs/content/omni-message.en.md index 0ab9eb5..e387f15 100644 --- a/packages/docs/content/omni-message.en.md +++ b/packages/docs/content/omni-message.en.md @@ -190,10 +190,18 @@ interface RequestBeginPayload { interface RequestEndPayload { type: "request_end"; status: StopReason; // "completed" is the mechanical commit criterion for replay - message?: string; // failure detail (LLMOutcome.message), non-completed only: - // the real reason behind a retried/failed Request (e.g. a - // provider quota code) — read by the Cost center's errors - // panel; additive, old Traces replay unchanged + // The unified RetryDetail block below is stamped in one place by the builders; every + // field is additive — old Traces replay unchanged. compaction_end reuses the same + // block (attempt = final attempt ordinal, error_message = the last failure's detail). + error_message?: string; // error detail (LLMOutcome.errorMessage internally — one + // name across the stack), non-completed only: the real + // reason behind a retried/failed Request (e.g. a provider + // quota code) — read by the Cost center's errors panel + attempt?: number; // 1-based ordinal of this request within its retry run (the + // authoritative retry count): stamped on failures and on a + // completion that needed retries; absent on a clean first try + retry_in_ms?: number; // planned reconnect wait (ms), present only when the engine + // will retry in-run — the Web App renders it as a countdown } interface ApprovalDecisionPayload { diff --git a/packages/docs/content/omni-message.zh.md b/packages/docs/content/omni-message.zh.md index b2e8af0..5e15bc9 100644 --- a/packages/docs/content/omni-message.zh.md +++ b/packages/docs/content/omni-message.zh.md @@ -189,9 +189,15 @@ interface RequestBeginPayload { interface RequestEndPayload { type: "request_end"; status: StopReason; // completed 是回放判定「该轮已提交」的机械标准 - message?: string; // 失败详情(LLMOutcome.message),仅非 completed 携带: - // 被重试/失败的 Request 背后的真实原因(如供应商额度码), - // 供成本中心错误面板读取;增量字段,旧 Trace 回放不受影响 + // 以下为统一的重试详情块 RetryDetail(由 builder 一处盖章;均为增量字段,旧 Trace 回放 + // 不受影响;compaction_end 复用同一块——attempt 为最终尝试序号,error_message 为失败详情) + error_message?: string; // 错误详情(内部 LLMOutcome.errorMessage,内外同名),仅非 + // completed 携带:被重试/失败的 Request 背后的真实原因 + // (如供应商额度码),供成本中心错误面板读取 + attempt?: number; // 本次重试序列内的第几次请求(1 起,权威计数):失败请求 + // 与经重试后成功的请求携带;首发即成功不携带 + retry_in_ms?: number; // 计划中的重连等待(毫秒),仅当引擎将在本轮内重试时携带, + // Web App 据此渲染倒计时 } interface ApprovalDecisionPayload { diff --git a/packages/server/src/db/schema.ts b/packages/server/src/db/schema.ts index bb5c920..aa519bc 100644 --- a/packages/server/src/db/schema.ts +++ b/packages/server/src/db/schema.ts @@ -82,7 +82,7 @@ CREATE TABLE IF NOT EXISTS error_records ( -- server-side error capture (the project_id TEXT, -- nullable: sign-in/registration and process-level errors have no Project context agent_id TEXT, session_id TEXT, - source TEXT NOT NULL, -- http | session | usage | title | subagent | process | llm | environment | schedule + source TEXT NOT NULL, -- http | session | usage | title | subagent | process | llm | environment | compaction | schedule kind TEXT NOT NULL, -- expected (HttpError, business 4xx) | unexpected (500/runtime) code TEXT NOT NULL, -- HttpError.code / internal / session_run_failed / ... status INTEGER, -- HTTP status code; NULL for non-HTTP sources diff --git a/packages/server/src/runtime/error-recorder.ts b/packages/server/src/runtime/error-recorder.ts index d1b82ed..c759965 100644 --- a/packages/server/src/runtime/error-recorder.ts +++ b/packages/server/src/runtime/error-recorder.ts @@ -61,6 +61,7 @@ export type ErrorSource = | "session" | "llm" | "environment" + | "compaction" | "usage" | "title" | "subagent" diff --git a/packages/server/src/runtime/stream-error-watcher.ts b/packages/server/src/runtime/stream-error-watcher.ts index dd8b1ef..7bbdb2c 100644 --- a/packages/server/src/runtime/stream-error-watcher.ts +++ b/packages/server/src/runtime/stream-error-watcher.ts @@ -4,7 +4,8 @@ * converges them into the message stream (LLM and Environment handle errors * internally and never throw), so a try/catch can't catch a single one. This watcher * hooks onto SessionManager's drive, inspects messages one by one, and fishes them out - * into error_records (source = `llm` / `environment`), matching usage-recorder's shape: + * into error_records (source = `llm` / `environment` / `compaction`), matching + * usage-recorder's shape: * recognizes only a few payload types, no-op on the rest. **One instance per run/compact** * (its state wraps up accordingly, see close). * @@ -27,14 +28,14 @@ * - `aborted` / `completed` are not recorded (the former is a user-initiated interrupt, * not an error). * - * The message uses the real reason: `request_end` carries the failure detail on - * non-completed statuses (`message`, from LLMOutcome), and the `abort` event's reason is + * The message uses the real reason: `request_end` carries the error detail on + * non-completed statuses (`error_message`, from LLMOutcome), and the `abort` event's reason is * core's failure-reason prose (e.g. `llm request error: 401 …` / `malformed response * failed after N retries`). A `request_end` failure is first held pending (status + its * own detail), not persisted immediately, and is resolved at the next request boundary: * - Immediately followed by `abort` → use its reason as the message (the real reason); * - Immediately followed by `request_begin` (the engine is retrying — no abort will ever - * arrive) → use the staged request_end's own `message` (the real detail, e.g. a quota + * arrive) → use the staged request_end's own `error_message` detail (e.g. a quota * code), falling back to the generic status text when it carried none. This boundary is * also what tells a survived `failed` from an exhausted one (see the classification above); * - Still unresolved when the run ends → close persists it the same way. @@ -295,17 +296,23 @@ export class StreamErrorWatcher { type?: string; status?: StopReason; reason?: string | null; - message?: string; + error_message?: string; }; const key = originKey(msg); + if (p.type === "compaction_end") { + this.observeCompactionEnd(msg, key); + return; + } if (p.type === "request_end") { this.flush(key); // Defensive: if a previous failure is still pending (normally resolved by request_begin), persist it first if (isLlmFailure(p.status)) { this.pending.set(key, { status: p.status, - // The event's own failure detail (LLMOutcome.message): the message of record when - // no abort follows (the retry path). - ...(typeof p.message === "string" && p.message.trim() ? { message: p.message } : {}), + // The event's own error detail (request_end.error_message): the message of record + // when no abort follows (the retry path). + ...(typeof p.error_message === "string" && p.error_message.trim() + ? { message: p.error_message } + : {}), }); } return; @@ -356,6 +363,43 @@ export class StreamErrorWatcher { }); } + /** + * A failed compaction becomes its own error record (source = `compaction`) — a compaction + * is an ordinary LLM request whose failures (unusable summary and transport alike) core + * retries under the standard budget, so a `failed` end means the retries ran out + * (issue #170). This is the single record a failed compaction produces: the compaction + * request's own request_begin/request_end pair is written to Trace only and never reaches + * this stream, so the llm-source records above cannot double-count it. `completed` is not + * an error and `aborted` is a user interrupt — neither is recorded. kind is unexpected for + * the same reason `llm_failed` is: the budget is exhausted, and while the original context + * is kept, the trigger still holds — the session keeps re-entering compaction until a + * human changes something. The message carries the attempts and the last error detail + * (compaction_end's share of the RetryDetail block) so the cost center shows what failed. + */ + private observeCompactionEnd(msg: OmniMessage, key: string): void { + const p = msg.payload as { + mode?: string; + reason?: string; + status?: StopReason; + attempt?: number; + error_message?: string; + }; + if (p.status !== "failed") return; + const attempts = + typeof p.attempt === "number" && p.attempt > 0 + ? ` after ${p.attempt} attempt${p.attempt === 1 ? "" : "s"}` + : ""; + const detail = + typeof p.error_message === "string" && p.error_message ? `: ${p.error_message}` : ""; + this.errors.record({ + source: "compaction", + err: `${p.mode ?? "summarize"} compaction failed${attempts}${detail}; trigger ${p.reason ?? "unknown"}, original context kept.`, + ctx: this.ctxFor(key), + code: "compaction_failed", + kind: "unexpected", + }); + } + // —— Environment (tool execution) —— private observeTool(msg: OmniMessage): void { diff --git a/packages/server/src/services/project-config-service.ts b/packages/server/src/services/project-config-service.ts index f93895d..d0cc8e4 100644 --- a/packages/server/src/services/project-config-service.ts +++ b/packages/server/src/services/project-config-service.ts @@ -608,7 +608,8 @@ export function probeVerdict( ): { ok: true } | { ok: false; message: string } { if (outcome.status === "completed") return { ok: true }; if (outcome.status === "malformed" && sawContent) return { ok: true }; - const detail = "message" in outcome && outcome.message ? outcome.message : outcome.status; + const detail = + "errorMessage" in outcome && outcome.errorMessage ? outcome.errorMessage : outcome.status; return { ok: false, message: String(detail).slice(0, 300) }; } diff --git a/packages/server/test/errors.test.ts b/packages/server/test/errors.test.ts index e2aa0bb..f9a76f7 100644 --- a/packages/server/test/errors.test.ts +++ b/packages/server/test/errors.test.ts @@ -15,6 +15,7 @@ import type { DatabaseSync } from "node:sqlite"; import { abortEvent, assistantText, + compactionEnd, partialToolCallOutput, requestBegin, requestEnd, @@ -414,7 +415,7 @@ describe("stream-error-watcher (LLM / Environment errors)", () => { // a bucket would let a real credential failure be dropped as a duplicate. const got = feed([ requestBegin(), - requestEnd("auth", "401 invalid x-api-key (invalid_api_key)"), + requestEnd("auth", { errorMessage: "401 invalid x-api-key (invalid_api_key)" }), abortEvent("llm request error: 401 invalid x-api-key (invalid_api_key)"), ]); expect(got).toHaveLength(1); @@ -432,7 +433,9 @@ describe("stream-error-watcher (LLM / Environment errors)", () => { // request_begin proves another attempt happened — that one is expected, like a timeout. const got = feed([ requestBegin(), - requestEnd("failed", "Upstream HTTP/2 stream failed (upstream_http2_stream_error)"), + requestEnd("failed", { + errorMessage: "Upstream HTTP/2 stream failed (upstream_http2_stream_error)", + }), requestBegin(), requestEnd("completed"), ]); @@ -453,9 +456,9 @@ describe("stream-error-watcher (LLM / Environment errors)", () => { // the one that never does. const got = feed([ requestBegin(), - requestEnd("failed", "Upstream HTTP/2 stream failed"), + requestEnd("failed", { errorMessage: "Upstream HTTP/2 stream failed" }), requestBegin(), // The retry: resolves the failure above as recovered. - requestEnd("auth", "401 invalid x-api-key"), + requestEnd("auth", { errorMessage: "401 invalid x-api-key" }), abortEvent("llm request error: 401 invalid x-api-key"), ]); expect(got.map((r) => [r.code, r.kind])).toEqual([ @@ -471,7 +474,9 @@ describe("stream-error-watcher (LLM / Environment errors)", () => { // "timed out" status text. This is what the Cost center shows for a retried quota 403. const got = feed([ requestBegin(), - requestEnd("timeout", "403 no active subscription (insufficient_user_quota)"), + requestEnd("timeout", { + errorMessage: "403 no active subscription (insufficient_user_quota)", + }), requestBegin(), requestEnd("completed"), ]); @@ -487,7 +492,7 @@ describe("stream-error-watcher (LLM / Environment errors)", () => { it("an abort reason still outranks the staged request_end detail (failed exit path)", () => { const got = feed([ requestBegin(), - requestEnd("failed", "401 invalid x-api-key (invalid_api_key)"), + requestEnd("failed", { errorMessage: "401 invalid x-api-key (invalid_api_key)" }), abortEvent("llm request error: 401 invalid x-api-key (invalid_api_key)"), ]); expect(got).toHaveLength(1); @@ -500,7 +505,7 @@ describe("stream-error-watcher (LLM / Environment errors)", () => { // request_end detail IS the failure's reason — prefer it over the generic status text. const got = feed([ requestBegin(), - requestEnd("timeout", "403 quota exceeded (insufficient_user_quota)"), + requestEnd("timeout", { errorMessage: "403 quota exceeded (insufficient_user_quota)" }), abortEvent("aborted during reconnect backoff"), ]); expect(got).toHaveLength(1); @@ -843,6 +848,64 @@ describe("stream-error-watcher (LLM / Environment errors)", () => { expect(got).toHaveLength(1); expect(got[0]).toMatchObject({ code: "llm_failed", agent_id: "a1", session_id: "s1" }); }); + + // —— Compaction —— + + it("a failed compaction records one error row; the message carries attempts and the error", () => { + // Issue #170: a compaction is an ordinary LLM request whose failures core retries under + // the standard budget — a failed end means the retries ran out, and the cost center's + // row shows how many attempts were spent and what the last failure was. + const got = feed([ + compactionEnd({ + reason: "context", + mode: "summarize", + status: "failed", + attempt: 5, + errorMessage: "the response contained no usable summary", + }), + ]); + expect(got).toHaveLength(1); + expect(got[0]).toMatchObject({ + source: "compaction", + kind: "unexpected", + code: "compaction_failed", + message: + "summarize compaction failed after 5 attempts: the response contained no usable summary; trigger context, original context kept.", + project_id: "p1", + agent_id: "a1", + session_id: "s1", + }); + }); + + it("compaction completed / aborted are not errors; an old-core failed still records", () => { + const got = feed([ + compactionEnd({ reason: "context", mode: "summarize", status: "completed", attempt: 1 }), + compactionEnd({ reason: "manual", mode: "summarize", status: "aborted", attempt: 2 }), + // An old core's compaction_end has no attempt/error fields at all. + compactionEnd({ reason: "turns", mode: "summarize", status: "failed" }), + ]); + expect(got).toHaveLength(1); + expect(got[0]).toMatchObject({ + code: "compaction_failed", + message: "summarize compaction failed; trigger turns, original context kept.", + }); + }); + + it("a child session's failed compaction attributes to the child Agent/Session", () => { + const got = feed([ + childMeta("session-child", "/data/agents/agent-child/agent_state"), + withOrigin( + compactionEnd({ reason: "context", mode: "summarize", status: "failed", attempt: 6 }), + "session-child", + ), + ]); + expect(got).toHaveLength(1); + expect(got[0]).toMatchObject({ + code: "compaction_failed", + agent_id: "agent-child", + session_id: "session-child", + }); + }); }); describe("HTTP onError persistence (integration)", () => { diff --git a/packages/server/test/model-probe.test.ts b/packages/server/test/model-probe.test.ts index 64936c2..ae13b55 100644 --- a/packages/server/test/model-probe.test.ts +++ b/packages/server/test/model-probe.test.ts @@ -48,13 +48,13 @@ describe("probeVerdict", () => { }); it("fails a malformed ending with nothing received (broken response, not a working endpoint)", () => { - const verdict = probeVerdict({ status: "malformed", message: "unexpected EOF" }, false); + const verdict = probeVerdict({ status: "malformed", errorMessage: "unexpected EOF" }, false); expect(verdict).toEqual({ ok: false, message: "unexpected EOF" }); }); it("fails timeouts and errors even when content was streamed (flaky is not ok)", () => { expect(probeVerdict({ status: "timeout" }, true)).toEqual({ ok: false, message: "timeout" }); - expect(probeVerdict({ status: "failed", message: "401 unauthorized" }, true)).toEqual({ + expect(probeVerdict({ status: "failed", errorMessage: "401 unauthorized" }, true)).toEqual({ ok: false, message: "401 unauthorized", }); @@ -62,7 +62,7 @@ describe("probeVerdict", () => { }); it("truncates long failure messages to 300 characters", () => { - const verdict = probeVerdict({ status: "failed", message: "x".repeat(500) }, false); + const verdict = probeVerdict({ status: "failed", errorMessage: "x".repeat(500) }, false); expect(verdict.ok).toBe(false); if (!verdict.ok) expect(verdict.message).toHaveLength(300); }); diff --git a/packages/web/src/features/chat/compaction-banner.tsx b/packages/web/src/features/chat/compaction-banner.tsx index bfb76ac..721f000 100644 --- a/packages/web/src/features/chat/compaction-banner.tsx +++ b/packages/web/src/features/chat/compaction-banner.tsx @@ -23,6 +23,6 @@ export function CompactionBanner({ item }: { item: CompactionItem }) { const text = item.status === "completed" ? S.chat.compactionDone(item.mode) - : S.chat.compactionFailed(item.status ?? "failed"); + : S.chat.compactionFailed(item.status ?? "failed", item.errorMessage); return

{text}

; } diff --git a/packages/web/src/lib/omni/stream-model.ts b/packages/web/src/lib/omni/stream-model.ts index 54e6aed..119d199 100644 --- a/packages/web/src/lib/omni/stream-model.ts +++ b/packages/web/src/lib/omni/stream-model.ts @@ -220,7 +220,7 @@ export interface ReconnectItem { id: number; /** Trigger reason: timeout (timed out / disconnected), malformed (an incomplete or unparseable response), or failed (the provider returned an error). */ status: ReconnectStatus; - /** Which retry attempt this is (increments on consecutive failures within the same round; resets to 1 after a request finishes normally). */ + /** Which retry attempt this is — request_end.attempt, the core's authoritative 1-based ordinal (1 for Traces written before the field existed). */ attempt: number; /** The retry request has been sent (set true by the next request_begin). */ retrying: boolean; @@ -250,6 +250,8 @@ export interface CompactionItem { /** True between begin and end (renders a "compaction in progress" banner). */ running: boolean; status?: StopReason; + /** Last failure detail from compaction_end.error_message (its share of the RetryDetail block; present on failed ends from new cores). */ + errorMessage?: string; } export interface TaskStatsItem { @@ -375,8 +377,6 @@ export interface StreamModel { * dispatches it via `void executeOne`, which doesn't block the streaming loop — execution happens between two Requests). */ openApprovalWaitMs: number; - /** Consecutive reconnect-failure count (incremented when request_end carries a status the engine reconnects on, reset to zero on any other terminal status). */ - reconnectRun: number; /** * Timestamp (ms) of the most recent auth failure: a main-session `request_end` with * status "auth" arrived on THIS model (a subagent's request events route to the nested @@ -459,7 +459,6 @@ function newModel(nested: boolean, localDecisions: Set): StreamModel { lastTsMs: 0, openRequestBeginMs: null, openApprovalWaitMs: 0, - reconnectRun: 0, lastAuthFailureMs: null, taskOpen: false, taskStartLocalMs: 0, @@ -723,10 +722,9 @@ function startTask(model: StreamModel, timestamp: string, nowMs: number): void { // server dies during a backoff window, the Trace's tail is // request_end(timeout) with no abort, and history rebuild would leave a // dangling "retrying…" — the new Task's first request_begin isn't its - // retry, so mark it gaveUp and reset the consecutive-failure count (the new Task's failures count from 1 again). + // retry, so mark it gaveUp. const waiting = findLastWaitingReconnect(model); if (waiting) waiting.gaveUp = true; - model.reconnectRun = 0; // Any unclosed Request start / approval wait left over from the previous Task isn't carried into this Task's LLM timing. model.openRequestBeginMs = null; model.openApprovalWaitMs = 0; @@ -1302,11 +1300,9 @@ function handleEvent(model: StreamModel, p: EventPayload, tsMs?: number, nowMs?: // placeholder resend goes only to the model, never written to Trace): finalize the executing cards. closeExecutingToolCards(model); // A reconnect hint waiting to retry: an interruption means retries are - // exhausted/abandoned, so mark it gaveUp (this interruption marker item gives the reason); - // the run has ended, so reset the consecutive-failure count. + // exhausted/abandoned, so mark it gaveUp (this interruption marker item gives the reason). const waiting = findLastWaitingReconnect(model); if (waiting) waiting.gaveUp = true; - model.reconnectRun = 0; const item: AbortItem = { kind: "abort", id: nextId(model) }; if (p.reason != null) item.reason = p.reason; model.items.push(item); @@ -1333,6 +1329,7 @@ function handleEvent(model: StreamModel, p: EventPayload, tsMs?: number, nowMs?: if (item) { item.running = false; item.status = p.status; + if (p.error_message !== undefined) item.errorMessage = p.error_message; } else { // Mid-stream join (missed the begin): append a completed banner directly. const created: CompactionItem = { @@ -1342,6 +1339,7 @@ function handleEvent(model: StreamModel, p: EventPayload, tsMs?: number, nowMs?: mode: p.mode, running: false, status: p.status, + ...(p.error_message !== undefined ? { errorMessage: p.error_message } : {}), }; model.items.push(created); } @@ -1404,12 +1402,13 @@ function handleEvent(model: StreamModel, p: EventPayload, tsMs?: number, nowMs?: // ladder with nothing on screen and no give-up control. It would also reset the counter // mid-ladder, renumbering a mixed timeout → failed → timeout run back to retry #1. if (isReconnectStatus(p.status)) { - model.reconnectRun += 1; const item: ReconnectItem = { kind: "reconnect", id: nextId(model), status: p.status, - attempt: model.reconnectRun, + // The core stamps the authoritative ordinal on every retryable request_end; only + // Traces written before the field existed lack it (rendered as attempt 1). + attempt: p.attempt ?? 1, retrying: false, }; // The engine announced its planned backoff: keep it with the CLIENT arrival time @@ -1419,8 +1418,6 @@ function handleEvent(model: StreamModel, p: EventPayload, tsMs?: number, nowMs?: item.arrivedAtMs = nowMs ?? Date.now(); } model.items.push(item); - } else { - model.reconnectRun = 0; } return; } diff --git a/packages/web/src/lib/strings-en.ts b/packages/web/src/lib/strings-en.ts index f1df81c..e0b02c8 100644 --- a/packages/web/src/lib/strings-en.ts +++ b/packages/web/src/lib/strings-en.ts @@ -852,8 +852,12 @@ Scenarios: mode === "discard" ? "[Compaction] done, old context discarded" : "[Compaction] done, switched to the summarized context", - compactionFailed: (status: string) => - `[Compaction] ${status === "aborted" ? "aborted" : "failed"}, keeping current context`, + compactionFailed: (status: string, errorMessage?: string): string => { + if (status === "aborted") return "[Compaction] aborted, keeping current context"; + return errorMessage !== undefined + ? `[Compaction] failed (${errorMessage}), keeping current context` + : "[Compaction] failed, keeping current context"; + }, unknownTool: "(unknown tool)", workRunning: "Running", workDone: "Done", diff --git a/packages/web/src/lib/strings.ts b/packages/web/src/lib/strings.ts index ee8fe18..cfd1e2f 100644 --- a/packages/web/src/lib/strings.ts +++ b/packages/web/src/lib/strings.ts @@ -831,8 +831,12 @@ Benchmark： compactionRunning: (mode: string) => `压缩进行中（${mode}）…`, compactionDone: (mode: string): string => mode === "discard" ? "[压缩] 完成，旧上下文已丢弃" : "[压缩] 完成，已切换到摘要后的新上下文", - compactionFailed: (status: string) => - `[压缩] ${status === "aborted" ? "已中断" : "失败"}，保留当前上下文`, + compactionFailed: (status: string, errorMessage?: string): string => { + if (status === "aborted") return "[压缩] 已中断，保留当前上下文"; + return errorMessage !== undefined + ? `[压缩] 失败（${errorMessage}），保留当前上下文` + : "[压缩] 失败，保留当前上下文"; + }, unknownTool: "（未知工具）", workRunning: "运行中", workDone: "运行完毕", diff --git a/packages/web/test/stream-model.test.ts b/packages/web/test/stream-model.test.ts index 42b9e16..07542da 100644 --- a/packages/web/test/stream-model.test.ts +++ b/packages/web/test/stream-model.test.ts @@ -355,7 +355,7 @@ describe("approvals and events", () => { const m = createStreamModel(); pushMessage(m, abortEvent("aborted by user")); expect(m.lastAuthFailureMs).toBeNull(); - const end = requestEnd("auth", "401 invalid x-api-key"); + const end = requestEnd("auth", { errorMessage: "401 invalid x-api-key" }); pushMessage(m, end); // The recorded time is the event's envelope timestamp (so a reload can compare it // against the Project's credentials-updated time). @@ -375,7 +375,7 @@ describe("approvals and events", () => { it("a later completed request clears the auth-dead state; a new auth failure re-arms it", () => { const m = createStreamModel(); - pushMessage(m, requestEnd("auth", "401")); + pushMessage(m, requestEnd("auth", { errorMessage: "401" })); expect(m.lastAuthFailureMs).not.toBeNull(); // The key was fixed and a request succeeded: the state must not outlive the success. pushMessage(m, userText("again")); @@ -383,7 +383,7 @@ describe("approvals and events", () => { pushMessage(m, requestEnd("completed")); expect(m.lastAuthFailureMs).toBeNull(); // A fresh auth failure re-arms (e.g. the replacement key is wrong too). - pushMessage(m, requestEnd("auth", "401")); + pushMessage(m, requestEnd("auth", { errorMessage: "401" })); expect(m.lastAuthFailureMs).not.toBeNull(); }); @@ -394,7 +394,7 @@ describe("approvals and events", () => { pushMessages(recovered, [ userText("go"), requestBegin(), - requestEnd("auth", "401 invalid x-api-key"), + requestEnd("auth", { errorMessage: "401 invalid x-api-key" }), abortEvent("llm request error: 401 invalid x-api-key"), userText("after fix"), requestBegin(), @@ -411,7 +411,7 @@ describe("approvals and events", () => { requestEnd("completed"), userText("later"), requestBegin(), - requestEnd("auth", "401 invalid x-api-key"), + requestEnd("auth", { errorMessage: "401 invalid x-api-key" }), abortEvent("llm request error: 401 invalid x-api-key"), ]); finalizeHistory(dead); @@ -427,7 +427,7 @@ describe("approvals and events", () => { it("a subagent-origin auth failure does NOT kill the parent session's input", () => { const m = createStreamModel(); - pushMessage(m, withOrigin(requestEnd("auth", "401"), "child1")); + pushMessage(m, withOrigin(requestEnd("auth", { errorMessage: "401" }), "child1")); // The failure belongs to the child session: its nested model carries the state, the // parent composer stays usable (the subagent simply surfaces as failed). expect(m.lastAuthFailureMs).toBeNull(); @@ -460,6 +460,28 @@ describe("approvals and events", () => { expect(banner).not.toHaveProperty("tokens"); }); + it("compaction_end carries its RetryDetail share onto the banner (error shown on failure)", () => { + const m = createStreamModel(); + pushMessage( + m, + compactionBegin({ reason: "context", mode: "summarize", context: 1000, turns: 3 }), + ); + const banner = items(m)[0] as CompactionItem; + pushMessage( + m, + compactionEnd({ + reason: "context", + mode: "summarize", + status: "failed", + attempt: 5, + errorMessage: "the response contained no usable summary", + }), + ); + expect(banner.running).toBe(false); + expect(banner.status).toBe("failed"); + expect(banner.errorMessage).toBe("the response contained no usable summary"); + }); + it("request_begin/end (normal final state) and main-session session_meta do not render", () => { const m = createStreamModel(); pushMessage(m, meta("session-x")); @@ -468,10 +490,11 @@ describe("approvals and events", () => { expect(items(m)).toHaveLength(0); }); - it("request_end final state timeout/malformed produces a retry notice item (with attempt number); request_begin marks it as resent", () => { + it("request_end final state timeout/malformed produces a retry notice item (with the stamped attempt); request_begin marks it as resent", () => { const m = createStreamModel(); pushMessage(m, requestBegin()); - pushMessage(m, requestEnd("malformed")); + // The core stamps the authoritative ordinal on every retryable request_end. + pushMessage(m, requestEnd("malformed", { attempt: 1 })); const retry = items(m)[0] as ReconnectItem; expect(retry).toMatchObject({ kind: "reconnect", @@ -482,17 +505,21 @@ describe("approvals and events", () => { // Retry request sent: the notice is marked as retrying. pushMessage(m, requestBegin()); expect(retry.retrying).toBe(true); - // Retry fails again: a second notice, attempt count increments. - pushMessage(m, requestEnd("timeout")); + // Retry fails again: a second notice, carrying the next ordinal. + pushMessage(m, requestEnd("timeout", { attempt: 2 })); const retry2 = items(m)[1] as ReconnectItem; expect(retry2).toMatchObject({ kind: "reconnect", status: "timeout", attempt: 2 }); - // Retry succeeds: no new entry, the consecutive-failure count resets to 0 — the next round's failure starts back at 1. + // Retry succeeds: no new entry; the next round's failure carries attempt 1 again. pushMessage(m, requestBegin()); pushMessage(m, requestEnd("completed")); expect(items(m)).toHaveLength(2); pushMessage(m, requestBegin()); - pushMessage(m, requestEnd("timeout")); + pushMessage(m, requestEnd("timeout", { attempt: 1 })); expect((items(m)[2] as ReconnectItem).attempt).toBe(1); + // A Trace written before the field existed lacks it: rendered as attempt 1. + pushMessage(m, requestBegin()); + pushMessage(m, requestEnd("timeout")); + expect((items(m)[3] as ReconnectItem).attempt).toBe(1); }); it("request_end(failed) renders a retry notice too, with its countdown inputs and give-up target", () => { @@ -503,7 +530,10 @@ describe("approvals and events", () => { pushMessage(m, requestBegin()); pushMessage( m, - requestEnd("failed", "Upstream HTTP/2 stream failed (upstream_http2_stream_error)", 4000), + requestEnd("failed", { + errorMessage: "Upstream HTTP/2 stream failed (upstream_http2_stream_error)", + retryInMs: 4000, + }), 111_000, ); const retry = items(m)[0] as ReconnectItem; @@ -521,39 +551,38 @@ describe("approvals and events", () => { expect(retry.retrying).toBe(true); }); - it("a mixed ladder keeps counting: failed no longer resets the attempt number mid-run", () => { - // `failed` used to fall through to the reset branch, so timeout → failed → timeout - // renumbered the third attempt back to #1 while the engine was on its third. + it("a mixed ladder keeps counting: the stamped ordinal carries across failure kinds", () => { + // The engine numbers timeout → failed → timeout as one run (all three draw on the same + // budget), and each notice shows the event's own ordinal. const m = createStreamModel(); pushMessage(m, requestBegin()); - pushMessage(m, requestEnd("timeout")); + pushMessage(m, requestEnd("timeout", { attempt: 1 })); pushMessage(m, requestBegin()); - pushMessage(m, requestEnd("failed", "502 bad gateway")); + pushMessage(m, requestEnd("failed", { errorMessage: "502 bad gateway", attempt: 2 })); pushMessage(m, requestBegin()); - pushMessage(m, requestEnd("timeout")); + pushMessage(m, requestEnd("timeout", { attempt: 3 })); expect((items(m) as ReconnectItem[]).map((i) => [i.status, i.attempt])).toEqual([ ["timeout", 1], ["failed", 2], ["timeout", 3], ]); - // A normal finish still resets it: the next run's first failure is #1 again. + // After a normal finish the next run's first failure is stamped #1 again. pushMessage(m, requestBegin()); pushMessage(m, requestEnd("completed")); pushMessage(m, requestBegin()); - pushMessage(m, requestEnd("failed")); + pushMessage(m, requestEnd("failed", { attempt: 1 })); expect((items(m)[3] as ReconnectItem).attempt).toBe(1); }); - it("request_end(auth) stays out of the ladder: terminal, so no retry notice and the count resets", () => { + it("request_end(auth) stays out of the ladder: terminal, so no retry notice", () => { // The one status the engine does not retry — an item would promise a countdown and a // "Retry now" that will never happen, on top of the composer already being gated. const m = createStreamModel(); pushMessage(m, requestBegin()); - pushMessage(m, requestEnd("timeout")); + pushMessage(m, requestEnd("timeout", { attempt: 1 })); pushMessage(m, requestBegin()); - pushMessage(m, requestEnd("auth", "401 invalid x-api-key")); + pushMessage(m, requestEnd("auth", { errorMessage: "401 invalid x-api-key", attempt: 2 })); expect((items(m) as ReconnectItem[]).filter((i) => i.kind === "reconnect")).toHaveLength(1); - expect(m.reconnectRun).toBe(0); }); it("retries exhausted: an arriving abort marks the waiting retry notice gaveUp and resets the consecutive-failure count", () => { @@ -582,7 +611,14 @@ describe("approvals and events", () => { // The engine announced a 4s backoff before retry #1; nowMs (the injected client clock) // is the countdown anchor — NOT the envelope timestamp, so server clock skew cannot // bend the ticker. - pushMessage(m, requestEnd("timeout", "403 quota (insufficient_user_quota)", 4000), 111_000); + pushMessage( + m, + requestEnd("timeout", { + errorMessage: "403 quota (insufficient_user_quota)", + retryInMs: 4000, + }), + 111_000, + ); const item = items(m)[0] as ReconnectItem; expect(item).toMatchObject({ kind: "reconnect", @@ -608,7 +644,7 @@ describe("approvals and events", () => { pushMessages(retried, [ userText("go"), requestBegin(), - requestEnd("timeout", "quota", 30_000), + requestEnd("timeout", { errorMessage: "quota", retryInMs: 30_000 }), requestBegin(), // the engine's retry — replayed immediately after requestEnd("completed"), ]); @@ -620,7 +656,7 @@ describe("approvals and events", () => { pushMessages(aborted, [ userText("go"), requestBegin(), - requestEnd("timeout", "quota", 30_000), + requestEnd("timeout", { errorMessage: "quota", retryInMs: 30_000 }), abortEvent("aborted during reconnect backoff"), // the user gave up mid-wait ]); finalizeHistory(aborted);