fix(core,server,web,cli): compaction tolerance, retry parity, and failure classification (#174)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -61,6 +61,7 @@ export type ErrorSource =
|
||||
| "session"
|
||||
| "llm"
|
||||
| "environment"
|
||||
| "compaction"
|
||||
| "usage"
|
||||
| "title"
|
||||
| "subagent"
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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) };
|
||||
}
|
||||
|
||||
|
||||
@@ -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)", () => {
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user