From e0a47254af9ed6419440c507d00bd3ade7b6756a Mon Sep 17 00:00:00 2001 From: Yaowei Zheng Date: Wed, 29 Jul 2026 16:16:15 +0800 Subject: [PATCH] feat(server,web): page the cost center's error table, abbreviate the source, drop command-tool noise (#103) Co-authored-by: Claude Opus 5 (1M context) --- packages/docs/content/server-api.en.md | 1 + packages/docs/content/server-api.zh.md | 1 + packages/server/src/api/types.ts | 14 +- packages/server/src/db/repos/errors.ts | 6 +- packages/server/src/http/routes/usage.ts | 32 ++- .../src/runtime/stream-error-watcher.ts | 53 ++++- packages/server/src/services/usage-service.ts | 47 ++++- packages/server/test/authz.test.ts | 3 + packages/server/test/errors.test.ts | 183 ++++++++++++++++-- packages/server/test/id-validation.test.ts | 1 + packages/server/test/session-manager.test.ts | 6 +- packages/server/test/usage.test.ts | 100 ++++++++++ packages/web/src/api/endpoints.ts | 21 ++ .../web/src/features/usage/errors-panel.tsx | 148 ++++++++++++-- .../web/src/features/usage/usage-page.tsx | 13 +- packages/web/src/lib/strings-en.ts | 5 + packages/web/src/lib/strings.ts | 5 + 17 files changed, 598 insertions(+), 41 deletions(-) diff --git a/packages/docs/content/server-api.en.md b/packages/docs/content/server-api.en.md index 8d31594..61c10d2 100644 --- a/packages/docs/content/server-api.en.md +++ b/packages/docs/content/server-api.en.md @@ -141,6 +141,7 @@ On Session creation, `modelId` and `provider` are both-or-neither: send the comp | Method | Path | Description | | --- | --- | --- | | GET | /usage | Usage statistics; query parameters `from`, `to`, `groupBy`, `agentId`, `provider`, `modelId` | +| GET | /usage/errors | One page of the error detail table (newest first): `offset`, `limit`, plus the same `from` / `to` / `agentId` filter → `{items, total}` | | GET | /agents/:agentId/traces | Date → Session drill-down structure of Trace files | | GET | /agents/:agentId/traces/:sessionId/:index | Read Trace events (`offset` / `limit` pagination) | | GET | /agents/:agentId/traces/:sessionId/:index/analysis | Trace performance analysis | diff --git a/packages/docs/content/server-api.zh.md b/packages/docs/content/server-api.zh.md index 7ea8ecc..18a19da 100644 --- a/packages/docs/content/server-api.zh.md +++ b/packages/docs/content/server-api.zh.md @@ -141,6 +141,7 @@ Schedule 写操作仅限 Owner。新建 Session 模式的任务，`modelId` 与 | 方法 | 路径 | 说明 | | --- | --- | --- | | GET | /usage | 用量统计，查询参数 `from`、`to`、`groupBy`、`agentId`、`provider`、`modelId` | +| GET | /usage/errors | 异常明细表分页（按时间倒序）：`offset`、`limit`，以及与看板一致的 `from` / `to` / `agentId` 过滤 → `{items, total}` | | GET | /agents/:agentId/traces | Trace 文件的日期 → Session 下钻结构 | | GET | /agents/:agentId/traces/:sessionId/:index | 读取 Trace 事件（`offset` / `limit` 分页） | | GET | /agents/:agentId/traces/:sessionId/:index/analysis | Trace 性能分析结果 | diff --git a/packages/server/src/api/types.ts b/packages/server/src/api/types.ts index e1fbb5a..0d4bf84 100644 --- a/packages/server/src/api/types.ts +++ b/packages/server/src/api/types.ts @@ -1085,10 +1085,22 @@ export interface UsageErrors { unexpected: number; /** The most frequent source · code (null when there are no errors). */ topCode: UsageErrorCount | null; - /** Most recent N items (reverse chronological). */ + /** Most recent N items (reverse chronological) — the first page; older ones come from `GET /usage/errors`. */ recent: UsageErrorItem[]; } +/** + * GET /api/projects/:projectId/usage/errors — one page of the error detail table, newest + * first. The dashboard response above already carries the first page; this exists so + * "show me earlier ones" does not have to refetch the whole aggregate. It takes the same + * date/agent filter as the dashboard, so a page never widens what the summary counted. + */ +export interface UsageErrorsPage { + items: UsageErrorItem[]; + /** Filtered row count, so the caller knows when it has reached the end. */ + total: number; +} + export interface UsageResponse { summary: { today: UsageBucket; diff --git a/packages/server/src/db/repos/errors.ts b/packages/server/src/db/repos/errors.ts index 209748d..1de52b8 100644 --- a/packages/server/src/db/repos/errors.ts +++ b/packages/server/src/db/repos/errors.ts @@ -194,15 +194,15 @@ export class ErrorsRepo { } /** The most recent `limit` entries (reverse chronological order). */ - recent(projectId: string, f: ErrorFilter = {}, limit = 20): ErrorItem[] { + recent(projectId: string, f: ErrorFilter = {}, limit = 20, offset = 0): ErrorItem[] { const { where, params } = this.conds(projectId, f); const rows = this.db .prepare( `SELECT ts, source, code, kind, message FROM error_records WHERE ${where} - ORDER BY id DESC LIMIT :limit`, + ORDER BY id DESC LIMIT :limit OFFSET :offset`, ) - .all({ ...params, limit }); + .all({ ...params, limit, offset }); return rows.map((r) => ({ ts: r.ts as string, source: r.source as string, diff --git a/packages/server/src/http/routes/usage.ts b/packages/server/src/http/routes/usage.ts index e1d00b4..62e31d0 100644 --- a/packages/server/src/http/routes/usage.ts +++ b/packages/server/src/http/routes/usage.ts @@ -1,12 +1,14 @@ /** * Usage statistics routes: * GET /api/projects/:p/usage?from&to&groupBy&agentId&provider&modelId - * (model filter is paired: provider and modelId are given together). + * (model filter is paired: provider and modelId are given together); + * GET /api/projects/:p/usage/errors?offset&limit&from&to&agentId — one page of the error + * detail table, for paging back past the first page the dashboard already returns. */ import { Hono } from "hono"; import type { UsageGroupBy } from "../../api/types.js"; import type { AppEnv } from "../../auth/middleware.js"; -import { badRequest, optionalDateParam, requireValidId } from "../validate.js"; +import { badRequest, optionalDateParam, paginationQuery, requireValidId } from "../validate.js"; import type { AppDeps } from "../../app.js"; const GROUP_BYS: readonly UsageGroupBy[] = ["date", "agent", "model", "session"]; @@ -44,5 +46,31 @@ export function usageRoutes(deps: AppDeps): Hono { ); }); + // One page of the error detail table, newest first. The dashboard response above already + // carries the first page; this serves "show me earlier ones" without refetching the whole + // aggregate. Takes the date/agent filter only — the model filter never applied to errors + // (HTTP and process errors have no Model dimension), so accepting it here would imply a + // narrowing the summary above does not do. + app.get("/errors", (c) => { + const projectId = requireValidId(c, "projectId"); + deps.projectService.requireProjectAccess(c.var.user.userId, projectId); + const { offset, limit } = paginationQuery(c); + const from = optionalDateParam(c.req.query("from"), "from"); + const to = optionalDateParam(c.req.query("to"), "to"); + const agentId = c.req.query("agentId"); + return c.json( + deps.usageService.queryErrors(projectId, { + offset, + limit, + // Same admin-only rule as the dashboard: a regular member seeing another tenant's + // unattributed errors would be a cross-tenant leak. + includeGlobalErrors: c.var.user.isAdmin, + ...(from !== undefined ? { from } : {}), + ...(to !== undefined ? { to } : {}), + ...(agentId !== undefined && agentId !== "" ? { agentId } : {}), + }), + ); + }); + return app; } diff --git a/packages/server/src/runtime/stream-error-watcher.ts b/packages/server/src/runtime/stream-error-watcher.ts index 344d988..6f1dcce 100644 --- a/packages/server/src/runtime/stream-error-watcher.ts +++ b/packages/server/src/runtime/stream-error-watcher.ts @@ -34,7 +34,10 @@ * * Environment (source = `environment`): reads `tool_call_output`'s stop_reason ∈ * {failed, timeout} → expected (the error is fed back to the model, and the Agent - * adjusts on its own; `aborted` is denial/interruption, not recorded). + * adjusts on its own; `aborted` is denial/interruption, not recorded). Exactly one shape is + * dropped: a command tool's ordinary non-zero exit, which is how a shell command reports + * information rather than a fault — see isOrdinaryCommandExit; the same tools' environment + * faults (killed by a signal, spawn failure, tool timeout) still record. * `tool_call_output` only has tool_call_id, no tool name, so `tool_call_id → tool name` * is cached (tool_call always arrives before its output), and the tool name is written * into code (`tool_failed:exec_command`) — so the stats dashboard's "most common error @@ -104,6 +107,50 @@ function isToolFailure(s: unknown): s is ToolFailure { return s === "failed" || s === "timeout"; } +/** + * The command tools. Both funnel their exit status through core's `resultForExit`, which maps + * *any* non-zero exit to `failed` — and a non-zero exit is how shell commands return + * information, not a fault: `grep` exits 1 when nothing matches, `test -f` when the file is + * absent, `diff` when files differ. Recording those made the cost center's error table mostly a + * log of ordinary Agent work, burying the real errors and eating the row cap that exists to + * protect them. (Only the row cap: the recorder's dedup key carries the error code, so this + * noise could ever have crowded out only *itself*.) The Agent sees the failure in the tool + * output and adjusts — nothing about it needs a human. + * + * Both belong here: `input_command` is how a backgrounded `exec_command` is polled for its + * eventual exit, so covering only one would record the *same* command's non-zero exit when it + * finishes in the background and drop it when it finishes in the foreground. + */ +const COMMAND_TOOLS: ReadonlySet = new Set(["exec_command", "input_command"]); + +/** + * `resultForExit`'s terminal marker for an ordinary non-zero exit. Anchored to the end because + * the marker is a `note`, which Environment appends after the (possibly truncated) output — a + * command's own stdout may contain anything, including this text. + */ +const ORDINARY_EXIT_NOTE = /\[exit code: [^\]\n]*\]\s*$/; + +/** + * Whether a command-tool failure is just an ordinary non-zero exit — the one failure shape the + * error table is better off without (see COMMAND_TOOLS). + * + * The discrimination is by note rather than by tool name, because `failed` from these tools is + * not only "the command exited non-zero": it also covers termination by signal (an OOM kill, a + * segfault), a spawn failure (nonexistent workdir, EMFILE, an unresolvable shell), a missing + * command session manager, and a tool timeout. Those are config/environment faults — the + * recorder's own "needs a human" category — and no amount of Agent self-correction resolves + * them, so they must keep landing in the table. `resultForExit` writes the exit code as the + * output's last note, which tells the two apart precisely. + * + * An unknown name (tool_call evicted from the cache, or never seen) still qualifies: only these + * two tools ever emit that marker, so dropping it beats recording a nameless + * `tool_failed:unknown` for exactly the noise this exists to remove. + */ +function isOrdinaryCommandExit(name: string | undefined, output: string): boolean { + if (name !== undefined && !COMMAND_TOOLS.has(name)) return false; + return ORDINARY_EXIT_NOTE.test(output); +} + /** A user-interrupt abort message (core's `aborted by user` / `aborted during …`): not a failure reason. */ function isUserAbortReason(reason: string): boolean { return /^aborted\b/i.test(reason); @@ -285,9 +332,11 @@ export class StreamErrorWatcher { const name = this.toolNames.get(key); this.toolNames.delete(key); // This call has settled: dequeue it, the cache only keeps in-flight calls if (!isToolFailure(p.stop_reason)) return; // completed / aborted (denial, user interrupt) are not errors + const output = p.output ?? ""; + if (isOrdinaryCommandExit(name, output)) return; // a shell's exit status is information, not a fault this.errors.record({ source: "environment", - err: toolFailureText(p.output ?? ""), + err: toolFailureText(output), ctx: this.ctxFor(origin), // The tool name goes into code: so the stats dashboard's "most common error code" and table can show which tool failed. code: `tool_${p.stop_reason}:${name ?? "unknown"}`, diff --git a/packages/server/src/services/usage-service.ts b/packages/server/src/services/usage-service.ts index 18421d3..4cbacce 100644 --- a/packages/server/src/services/usage-service.ts +++ b/packages/server/src/services/usage-service.ts @@ -19,6 +19,7 @@ import type { UsageBucket, UsageErrors, + UsageErrorsPage, UsageGroupBy, UsageGroupRow, UsageResponse, @@ -32,7 +33,12 @@ import type { } from "../db/repos/usage.js"; import { formatLocalDate, localDateMinusDays } from "../internal/dates.js"; -/** Number of most-recent entries kept in the error detail table. */ +/** + * Number of most-recent entries kept in the error detail table. Also the page size the whole + * feature runs on, but nothing needs to hard-code it: `errors.recent` is exactly this many rows + * whenever a second page exists, so the client derives its page size from the response instead + * of holding a constant that could drift out of step with this one. + */ const ERROR_RECENT_N = 20; /** The three pricing buckets (usd_per_mtok convention), returned by the pricing lookup callback. */ @@ -61,6 +67,17 @@ export interface UsageQuery { includeGlobalErrors?: boolean; } +/** One page of the error detail table (see {@link UsageService.queryErrors}). */ +export interface UsageErrorsQuery { + offset: number; + limit: number; + from?: string; + to?: string; + agentId?: string; + /** Admin only: include errors with no Project attribution (see the ErrorsRepo file header). */ + includeGlobalErrors?: boolean; +} + /** Cost formula: sum of the three buckets, in USD per million Tokens. */ function costOf(sums: UsageModelSums, rates: PricingRates): number { return ( @@ -164,6 +181,34 @@ export class UsageService { }; } + /** + * One page of the error detail table, newest first. The dashboard's own response already + * carries the first page (`errors.recent`); this serves the "show me earlier ones" paging, + * where refetching the whole aggregate to move one page would be wasteful. `total` is the + * filtered row count, so the caller knows when it has reached the end. + * + * Takes the same filter the dashboard applies — date + agent, and admin-only visibility of + * unattributed errors — so a page never widens what the summary above it counted. + * + * Paging is by offset into a newest-first table that grows at its head, so errors recorded + * between two page requests shift every row down and a page can re-show rows already seen. + * Accepted deliberately: this is a diagnostic table read at a moment in time, not a feed to + * be walked exhaustively, and keying off the newest row's id would cost a cursor the caller + * has no other use for. + */ + queryErrors(projectId: string, q: UsageErrorsQuery): UsageErrorsPage { + const f: ErrorFilter = { + ...(q.agentId !== undefined ? { agentId: q.agentId } : {}), + ...(q.from !== undefined ? { from: q.from } : {}), + ...(q.to !== undefined ? { to: q.to } : {}), + ...(q.includeGlobalErrors === true ? { includeGlobal: true } : {}), + }; + return { + items: this.errors.recent(projectId, f, q.limit, q.offset), + total: this.errors.summary(projectId, f).total, + }; + } + /** Error statistics: summary info (total / unexpected / most common error code) + the last N entries, all filtered by the selected range. */ private foldErrors(projectId: string, f: ErrorFilter): UsageErrors { const { total, unexpected } = this.errors.summary(projectId, f); diff --git a/packages/server/test/authz.test.ts b/packages/server/test/authz.test.ts index 67edad1..eef1809 100644 --- a/packages/server/test/authz.test.ts +++ b/packages/server/test/authz.test.ts @@ -45,6 +45,9 @@ describe("authz", () => { expect((await outsider.get(`/api/projects/${projectId}/agents`)).status).toBe(404); expect((await outsider.get(`/api/projects/${projectId}/members`)).status).toBe(404); expect((await outsider.get(`/api/projects/${projectId}/usage`)).status).toBe(404); + // The error table's paging route is checked separately from /usage: it is the one place a + // reader can walk past the dashboard's first page, so it must refuse the same way. + expect((await outsider.get(`/api/projects/${projectId}/usage/errors`)).status).toBe(404); }); it("member can read models (masked) but not write; owner can write", async () => { diff --git a/packages/server/test/errors.test.ts b/packages/server/test/errors.test.ts index 6683441..c3b3c72 100644 --- a/packages/server/test/errors.test.ts +++ b/packages/server/test/errors.test.ts @@ -24,7 +24,7 @@ import { withOrigin, } from "@prismshadow/penguin-core"; import type { OmniMessage } from "@prismshadow/penguin-core"; -import type { ProjectCreateResponse, UsageResponse } from "../src/api/types.js"; +import type { ProjectCreateResponse, UsageErrorsPage, UsageResponse } from "../src/api/types.js"; import { openDatabase } from "../src/db/database.js"; import { ErrorsRepo } from "../src/db/repos/errors.js"; import type { ErrorRecordInsert } from "../src/db/repos/errors.js"; @@ -526,11 +526,102 @@ describe("stream-error-watcher (LLM / Environment errors)", () => { const call = (name: string, id: string) => toolCall({ name, arguments: "{}", toolCallId: id }); - it("tool failed / timeout → environment + expected, code carries the tool name", () => { + it("a command tool's ordinary non-zero exit is not recorded: an exit status is information", () => { + // Both command tools end in resultForExit, which maps ANY non-zero exit to `failed`, so + // grep finding nothing (exit 1), `test -f` on a missing file, or a diff that differs would + // all land in the cost center and bury the real errors. input_command is covered alongside + // exec_command because it is how a backgrounded command is polled for its eventual exit — + // dropping one but not the other would depend on where the command happened to finish. + // Every other tool still records, whatever its output says. const got = feed([ call("exec_command", "tc-1"), toolCallOutput({ - output: "ls: /nope: No such file or directory\n[tool error] exit code 2", + output: "grep: no match\n[exit code: 1]", + toolCallId: "tc-1", + stopReason: "failed", + }), + call("input_command", "tc-2"), + toolCallOutput({ + output: "make: *** [build] Error 2\n[exit code: 2]", + toolCallId: "tc-2", + stopReason: "failed", + }), + call("write_file", "tc-3"), + toolCallOutput({ + output: "EACCES\n[exit code: 1]", + toolCallId: "tc-3", + stopReason: "failed", + }), + ]); + expect(got.map((r) => r.code)).toEqual(["tool_failed:write_file"]); + }); + + it("a command tool killed by a signal, or that never spawned, is still recorded", () => { + // `failed` from these tools is not only "exited non-zero": an OOM kill or a segfault + // (resultForExit's signal branch) and a spawn failure (nonexistent workdir, EMFILE, an + // unresolvable shell) are config/environment faults — the recorder's "needs a human" + // category — that no amount of Agent self-correction gets around. Only the exit marker + // separates them, which is why the rule reads the note rather than the tool name. + const got = feed([ + call("exec_command", "tc-1"), + toolCallOutput({ + output: "cc1plus: out of memory\n[terminated by signal SIGKILL]", + toolCallId: "tc-1", + stopReason: "failed", + }), + call("input_command", "tc-2"), + toolCallOutput({ + output: "[spawn error: ENOENT: no such file or directory, posix_spawn '/bin/nope']", + toolCallId: "tc-2", + stopReason: "failed", + }), + ]); + expect(got.map((r) => r.code)).toEqual([ + "tool_failed:exec_command", + "tool_failed:input_command", + ]); + }); + + it("a command tool's timeout and a missing session manager are recorded (neither is an exit)", () => { + // Environment finalizes a tool timeout as stop_reason `failed` plus its own note — it never + // emits stop_reason "timeout" for these tools — so a hung command surfaces as tool_failed + // with no exit marker to drop it. A missing command session manager is a server + // misconfiguration and produces no exit marker either. + const got = feed([ + call("exec_command", "tc-1"), + toolCallOutput({ + output: "still building…\n[tool timeout: exceeded 60000ms]", + toolCallId: "tc-1", + stopReason: "failed", + }), + call("input_command", "tc-2"), + toolCallOutput({ + output: "[input_command unavailable: no command session manager configured]", + toolCallId: "tc-2", + stopReason: "failed", + }), + ]); + expect(got.map((r) => r.code)).toEqual([ + "tool_failed:exec_command", + "tool_failed:input_command", + ]); + }); + + it("an exit marker with no cached tool name is dropped, not filed under tool_failed:unknown", () => { + // Only the command tools ever write that marker, so a cache miss (the tool_call evicted, or + // never seen) is still that noise — recording it nameless would defeat the exclusion. + const got = feed([ + toolCallOutput({ output: "[exit code: 1]", toolCallId: "tc-1", stopReason: "failed" }), + toolCallOutput({ output: "boom", toolCallId: "tc-2", stopReason: "failed" }), + ]); + expect(got.map((r) => [r.code, r.message])).toEqual([["tool_failed:unknown", "boom"]]); + }); + + it("tool failed / timeout → environment + expected, code carries the tool name", () => { + const got = feed([ + call("write_file", "tc-1"), + toolCallOutput({ + output: "EACCES: permission denied\n[tool error] write failed", toolCallId: "tc-1", stopReason: "failed", }), @@ -545,19 +636,19 @@ describe("stream-error-watcher (LLM / Environment errors)", () => { expect(got[0]).toMatchObject({ source: "environment", kind: "expected", // error fed back to the model; the Agent adjusts on its own — no human needed - code: "tool_failed:exec_command", + code: "tool_failed:write_file", project_id: "p1", agent_id: "a1", session_id: "s1", }); - expect(got[0]!.message).toContain("[tool error] exit code 2"); // the actual error text + expect(got[0]!.message).toContain("[tool error] write failed"); // the actual error text expect(got[1]).toMatchObject({ code: "tool_timeout:read_file", kind: "expected" }); }); it("tool aborted (denial / user interrupt) and completed are not recorded", () => { expect( feed([ - call("exec_command", "tc-1"), + call("write_file", "tc-1"), toolCallOutput({ output: "Tool call denied by user.", toolCallId: "tc-1", @@ -571,20 +662,20 @@ describe("stream-error-watcher (LLM / Environment errors)", () => { it("parallel tools: each tool_call_id maps to its own name despite out-of-order outputs", () => { const got = feed([ - call("exec_command", "tc-1"), + call("write_file", "tc-1"), call("read_file", "tc-2"), call("write_file", "tc-3"), toolCallOutput({ output: "boom-2", toolCallId: "tc-2", stopReason: "failed" }), toolCallOutput({ output: "ok", toolCallId: "tc-3", stopReason: "completed" }), toolCallOutput({ output: "boom-1", toolCallId: "tc-1", stopReason: "failed" }), ]); - expect(got.map((r) => r.code)).toEqual(["tool_failed:read_file", "tool_failed:exec_command"]); + expect(got.map((r) => r.code)).toEqual(["tool_failed:read_file", "tool_failed:write_file"]); expect(got.map((r) => r.message)).toEqual(["boom-2", "boom-1"]); }); it("a child session's tool failure: no name mix-up with the parent's equal tool_call_id", () => { const got = feed([ - call("exec_command", "tc-1"), // parent session + call("read_file", "tc-1"), // parent session withOrigin(call("write_file", "tc-1"), "session-child"), // sub-session happens to share the same id withOrigin( toolCallOutput({ output: "child boom", toolCallId: "tc-1", stopReason: "failed" }), @@ -598,12 +689,12 @@ describe("stream-error-watcher (LLM / Environment errors)", () => { message: "child boom", session_id: "s1", // this test didn't feed the sub-session's session_meta → attribution falls back to the parent ctx (see the "attribution" test cases below) }); - expect(got[1]).toMatchObject({ code: "tool_failed:exec_command", message: "parent boom" }); + expect(got[1]).toMatchObject({ code: "tool_failed:read_file", message: "parent boom" }); }); it("overlong tool output: message takes the tail (the reason is at the end)", () => { const got = feed([ - call("exec_command", "tc-1"), + call("write_file", "tc-1"), toolCallOutput({ output: `${"x".repeat(2000)}\n[tool error] boom`, toolCallId: "tc-1", @@ -620,7 +711,7 @@ describe("stream-error-watcher (LLM / Environment errors)", () => { expect( feed([ assistantText("normal output"), - call("exec_command", "tc-1"), + call("write_file", "tc-1"), partialToolCallOutput({ eventType: "stop", toolCallId: "tc-1", stopReason: "failed" }), ]), ).toHaveLength(0); @@ -649,7 +740,7 @@ describe("stream-error-watcher (LLM / Environment errors)", () => { it("a child session's tool failure attributes to it (code still carries the tool name)", () => { const got = feed([ childMeta("session-child", "/data/agents/agent-child/agent_state"), - withOrigin(call("exec_command", "tc-1"), "session-child"), + withOrigin(call("write_file", "tc-1"), "session-child"), withOrigin( toolCallOutput({ output: "child boom", toolCallId: "tc-1", stopReason: "failed" }), "session-child", @@ -658,7 +749,7 @@ describe("stream-error-watcher (LLM / Environment errors)", () => { expect(got).toHaveLength(1); expect(got[0]).toMatchObject({ source: "environment", - code: "tool_failed:exec_command", + code: "tool_failed:write_file", message: "child boom", agent_id: "agent-child", session_id: "session-child", @@ -678,7 +769,7 @@ describe("stream-error-watcher (LLM / Environment errors)", () => { toolCallOutput({ output: "child tool boom", toolCallId: "tc-9", stopReason: "failed" }), "session-child", ), - call("exec_command", "tc-9"), // parent session happens to share the same id + call("read_file", "tc-9"), // parent session happens to share the same id toolCallOutput({ output: "parent tool boom", toolCallId: "tc-9", stopReason: "failed" }), requestEnd("failed"), // the parent session's LLM failure only wraps up now abortEvent("llm request error: 500 upstream"), @@ -687,7 +778,7 @@ describe("stream-error-watcher (LLM / Environment errors)", () => { expect(got.map((r) => [r.code, r.agent_id, r.session_id])).toEqual([ ["llm_timeout", "agent-child", "session-child"], ["tool_failed:write_file", "agent-child", "session-child"], - ["tool_failed:exec_command", "a1", "s1"], + ["tool_failed:read_file", "a1", "s1"], ["llm_failed", "a1", "s1"], ]); }); @@ -822,6 +913,66 @@ describe("HTTP onError persistence (integration)", () => { expect(adminBody.errors.recent[0]).toMatchObject({ code: "invalid_credentials" }); }); + it("the paged error route pages inside the caller's own tenant, at every offset", async () => { + // The point of the route is that paging back is not a way around the dashboard's isolation: + // the rows a member can reach at offset N are the same set the summary counted, never + // another tenant's and never the unattributed ones. Seeded directly so the interleaving is + // exact — going through HTTP would collapse repeats into the recorder's dedup window. + const repo = new ErrorsRepo(t.deps.db); + const seed = (owner: string | null, code: string) => + repo.insert({ + ts: "2026-07-27T00:00:00.000Z", + date: "2026-07-27", + projectId: owner, + agentId: "a1", + sessionId: "s1", + source: "http", + kind: "expected", + code, + status: 404, + message: code, + }); + const foreign = (await ( + await api.post("/api/projects", { projectId: "err_user-tenant_2", name: "Other tenant" }) + ).json()) as ProjectCreateResponse; + // Interleaved, so a mistaken filter would show up inside the first page rather than past it. + seed(projectId, "mine_0"); + seed(foreign.project.projectId, "theirs_0"); + seed(null, "unattributed_0"); + seed(projectId, "mine_1"); + seed(projectId, "mine_2"); + + const pageOf = async (offset: number, limit: number) => + (await ( + await api.get(`/api/projects/${projectId}/usage/errors?offset=${offset}&limit=${limit}`) + ).json()) as UsageErrorsPage; + + const first = await pageOf(0, 2); + expect(first.total).toBe(3); // the filtered count, not the table's + expect(first.items.map((e) => e.code)).toEqual(["mine_2", "mine_1"]); // newest first + const second = await pageOf(2, 2); + expect(second.total).toBe(3); + expect(second.items.map((e) => e.code)).toEqual(["mine_0"]); + expect((await pageOf(3, 2)).items).toEqual([]); // past the end is empty, not an error + + // The admin is the only one who reaches the unattributed rows — the same rule the dashboard + // applies, so paging cannot be used to slip past it. + const adminApi = apiClient(t.app, (await loginAdmin(t.app)).cookie); + const adminPage = (await ( + await adminApi.get(`/api/projects/default_project/usage/errors?offset=0&limit=20`) + ).json()) as UsageErrorsPage; + expect(adminPage.items.map((e) => e.code)).toEqual(["unattributed_0"]); + }); + + it("the paged error route rejects a page it cannot serve rather than guessing", async () => { + expect((await api.get(`/api/projects/${projectId}/usage/errors?offset=-1`)).status).toBe(400); + expect((await api.get(`/api/projects/${projectId}/usage/errors?limit=0`)).status).toBe(400); + expect((await api.get(`/api/projects/${projectId}/usage/errors?limit=1001`)).status).toBe(400); + expect((await api.get(`/api/projects/${projectId}/usage/errors?from=2026-13-01`)).status).toBe( + 400, + ); + }); + it("Project deletion cascade-cleans that Project's error records", async () => { await api.get(`/api/projects/${projectId}/agents/agent-nope/sessions`); expect(errorRows().filter((r) => r.project_id === projectId)).toHaveLength(1); diff --git a/packages/server/test/id-validation.test.ts b/packages/server/test/id-validation.test.ts index 27fc2cb..19564b7 100644 --- a/packages/server/test/id-validation.test.ts +++ b/packages/server/test/id-validation.test.ts @@ -97,6 +97,7 @@ describe("id-validation", () => { expect((await attacker.get(`/api/projects/..%2Fetc/agents`)).status).toBe(404); expect((await attacker.get(`/api/projects/..%2Fetc/models`)).status).toBe(404); expect((await attacker.get(`/api/projects/..%2Fetc/usage`)).status).toBe(404); + expect((await attacker.get(`/api/projects/..%2Fetc/usage/errors`)).status).toBe(404); expect((await attacker.get(`/api/projects/..%2Fetc/members`)).status).toBe(404); expect((await attacker.delete(`/api/projects/..%2F${victimProject}`)).status).toBe(404); }); diff --git a/packages/server/test/session-manager.test.ts b/packages/server/test/session-manager.test.ts index b791d9c..917d44c 100644 --- a/packages/server/test/session-manager.test.ts +++ b/packages/server/test/session-manager.test.ts @@ -51,7 +51,7 @@ const ROW: SessionRow = { }; /** A simple, scriptable fake Session: run yields one tool_call and requests approval for it. */ -function approvalFakeSession(sessionId: string, toolName = "exec_command"): RuntimeSession { +function approvalFakeSession(sessionId: string, toolName = "write_file"): RuntimeSession { return { sessionId, toolPermission: (name) => (name === "read_tool" ? "r" : "rw"), @@ -197,7 +197,7 @@ describe("session-manager", () => { skipReconnectWait: () => false, async *run(): AsyncGenerator { yield requestBegin(); - yield toolCall({ name: "exec_command", arguments: "{}", toolCallId: "tc-1" }); + yield toolCall({ name: "write_file", arguments: "{}", toolCallId: "tc-1" }); yield toolCallOutput({ output: "ls: /nope\n[tool error] exit code 2", toolCallId: "tc-1", @@ -213,7 +213,7 @@ describe("session-manager", () => { await waitFor(() => manager.statusOf("session-1") === "idle" && captured.length >= 2); expect(captured.map((a) => [a.source, a.code, a.kind])).toEqual([ - ["environment", "tool_failed:exec_command", "expected"], // error fed back to the model; the Agent adjusts on its own + ["environment", "tool_failed:write_file", "expected"], // error fed back to the model; the Agent adjusts on its own ["llm", "llm_failed", "unexpected"], // not retryable, requires human intervention ]); expect(captured[0]!.ctx).toEqual({ projectId: "p1", agentId: "a1", sessionId: "session-1" }); diff --git a/packages/server/test/usage.test.ts b/packages/server/test/usage.test.ts index d6bc05e..7b273be 100644 --- a/packages/server/test/usage.test.ts +++ b/packages/server/test/usage.test.ts @@ -277,3 +277,103 @@ describe("usage-service (cost computed on the fly)", () => { expect(byAgent.success.map((s) => s.modelId)).toEqual(["m1"]); }); }); + +describe("usage-service.queryErrors (error table paging)", () => { + let db: DatabaseSync; + let errors: ErrorsRepo; + let service: UsageService; + + beforeEach(() => { + db = openDatabase(":memory:"); + errors = new ErrorsRepo(db); + service = new UsageService(new UsageRepo(db), errors, async () => undefined); + // 25 rows, oldest first — so "newest first" ordering is observable across a page boundary. + for (let i = 0; i < 25; i += 1) { + errors.insert({ + ts: `2026-07-27T00:00:${String(i).padStart(2, "0")}.000Z`, + date: "2026-07-27", + projectId: "p1", + agentId: "a1", + sessionId: "s1", + source: "http", + kind: "expected", + code: `code_${i}`, + status: 400, + message: `m${i}`, + }); + } + }); + afterEach(() => db.close()); + + it("pages newest-first and reports the filtered total, so the caller knows where the end is", () => { + const first = service.queryErrors("p1", { offset: 0, limit: 20 }); + expect(first.total).toBe(25); + expect(first.items).toHaveLength(20); + expect(first.items[0]!.code).toBe("code_24"); // newest + const second = service.queryErrors("p1", { offset: 20, limit: 20 }); + expect(second.total).toBe(25); + // The tail is the five oldest, still descending, with no overlap against page one. + expect(second.items.map((e) => e.code)).toEqual([ + "code_4", + "code_3", + "code_2", + "code_1", + "code_0", + ]); + }); + + it("past the end is empty rather than an error, and total stays the full count", () => { + const page = service.queryErrors("p1", { offset: 100, limit: 20 }); + expect(page.items).toEqual([]); + expect(page.total).toBe(25); + }); + + it("filters before it offsets, so a later page never slides onto rows the summary excluded", () => { + // Five newer rows from another Agent, i.e. sitting at the head of the unfiltered table. If + // the offset were counted over that table and the filter applied afterwards, both pages + // below would come back shifted (and page two short); filtering first is what makes the + // 25-row filtered set page cleanly as 20 + 5. + for (let i = 0; i < 5; i += 1) { + errors.insert({ + ts: `2026-07-28T00:00:0${i}.000Z`, + date: "2026-07-28", + projectId: "p1", + agentId: "other", + sessionId: "s2", + source: "http", + kind: "expected", + code: `other_${i}`, + status: 400, + message: "m", + }); + } + const scoped = { offset: 0, limit: 20, agentId: "a1" }; + const first = service.queryErrors("p1", scoped); + expect(first.total).toBe(25); // the other Agent's five are outside the count, not just the page + expect(first.items[0]!.code).toBe("code_24"); + expect(first.items.at(-1)!.code).toBe("code_5"); + const second = service.queryErrors("p1", { ...scoped, offset: 20 }); + expect(second.items.map((e) => e.code)).toEqual([ + "code_4", + "code_3", + "code_2", + "code_1", + "code_0", + ]); + // Unfiltered, the same offset lands on entirely different rows — the filter is doing work. + const unscoped = service.queryErrors("p1", { offset: 20, limit: 20 }); + expect(unscoped.total).toBe(30); + expect(unscoped.items.map((e) => e.code)).toEqual([ + "code_9", + "code_8", + "code_7", + "code_6", + "code_5", + "code_4", + "code_3", + "code_2", + "code_1", + "code_0", + ]); + }); +}); diff --git a/packages/web/src/api/endpoints.ts b/packages/web/src/api/endpoints.ts index 842a02c..979e949 100644 --- a/packages/web/src/api/endpoints.ts +++ b/packages/web/src/api/endpoints.ts @@ -63,6 +63,7 @@ import type { UiPrefs, UpdateCheckResponse, UpdateRunResponse, + UsageErrorsPage, UsageGroupBy, UsageResponse, VaultResponse, @@ -360,6 +361,26 @@ export const importAgentTrace = (projectId: string, agentId: string, body: Trace // Usage statistics ---------------------------------------------------------------------- +/** + * One page of the cost center's error table (newest first). The dashboard response already + * carries the first page; this is for paging back to earlier ones without refetching the + * whole aggregate. Takes the dashboard's date/agent filter only — the model filter never + * applied to errors. + */ +export const getUsageErrors = ( + projectId: string, + params: { offset: number; limit: number; from?: string; to?: string; agentId?: string }, +) => + apiFetch(`/api/projects/${encodeURIComponent(projectId)}/usage/errors`, { + query: { + offset: String(params.offset), + limit: String(params.limit), + from: params.from, + to: params.to, + agentId: params.agentId, + }, + }); + export const getUsage = ( projectId: string, params: { diff --git a/packages/web/src/features/usage/errors-panel.tsx b/packages/web/src/features/usage/errors-panel.tsx index 967bfe6..7e8ab3c 100644 --- a/packages/web/src/features/usage/errors-panel.tsx +++ b/packages/web/src/features/usage/errors-panel.tsx @@ -9,9 +9,11 @@ * exceptions) is a prominent rose; expected (HttpError, business 4xx) recedes into gray. * The outer frame is provided by the caller's ChartCard (full width, below the four business charts). */ -import { useState } from "react"; -import type { UsageErrors } from "@prismshadow/penguin-server/api"; +import { useEffect, useState } from "react"; +import type { UsageErrorItem, UsageErrors } from "@prismshadow/penguin-server/api"; +import * as api from "../../api/endpoints"; import { S } from "../../lib/strings"; +import { apiErrorText } from "../../lib/api-error"; import { formatDateTime } from "../../lib/format"; import { Badge } from "../../components/ui/badge"; import { Empty } from "./usage-charts"; @@ -28,6 +30,18 @@ function kindOf(kind: string): ErrorKindKey { return kind === "unexpected" ? "unexpected" : "expected"; } +/** + * Source labels are abbreviated so the bracket stays narrow beside a long code: + * `[env] tool_failed:exec_command`, `[http] password`. Only `environment` needs shortening + * -- every other source is already short enough to read at a glance. + */ +const SOURCE_ABBREV: Readonly> = { environment: "env" }; + +/** `[env] tool_failed:read_file` -- the bracketed source followed by the raw error code. */ +function sourceCode(source: string, code: string): string { + return `[${SOURCE_ABBREV[source] ?? source}] ${code}`; +} + /** A single small stat: name + value, one row side by side (not turned into a chart). */ function Stat({ label, @@ -67,10 +81,79 @@ function Th({ children, className = "" }: { children: React.ReactNode; className * and clicking again collapses it. The full text is also in the hover title. Cells align to the * top so an expanded multi-line message keeps the row tidy; the table scrolls past max height. */ -export function ErrorsPanel({ errors }: { errors: UsageErrors }) { +export function ErrorsPanel({ + errors, + projectId, + filters, +}: { + errors: UsageErrors; + projectId: string; + /** + * The dashboard's own date/agent filter — a page must never widen what the summary counted. + * Memoize it at the call site: the fetch effect depends on the object's identity. + */ + filters: { from?: string; to?: string; agentId?: string }; +}) { const { total, unexpected, topCode, recent } = errors; - // Message rows expanded to their full text (index into `recent`); one line each by default. + // Page size is read off the first page rather than duplicating the server's ERROR_RECENT_N: + // whenever a second page exists at all, `recent` is exactly that many rows, so the two cannot + // drift apart into skipping or repeating rows. (Empty means a single empty page anyway.) + const pageSize = Math.max(1, recent.length); + // Paging: page 0 is the `recent` the dashboard response already carried, so it costs no + // request; later pages are fetched on demand. + const [page, setPage] = useState(0); + const [items, setItems] = useState(recent); + // The row count the pager counts pages against: seeded from the dashboard snapshot, then + // replaced by each page's own total. The snapshot goes stale (rows evicted by the row cap, an + // Agent deleted between load and click), and pinning to it computes a page count that can + // strand the caller on a page the data no longer has. + const [pagedTotal, setPagedTotal] = useState(total); + const [pageError, setPageError] = useState(null); + const [loading, setLoading] = useState(false); + // A new dashboard response invalidates the offsets being paged through, so paging goes back to + // page 0 — whose rows the effect below restores from that same response. Today the panel is + // remounted on every filter change, which resets this anyway; the reset does not rely on that. + useEffect(() => setPage(0), [recent]); + + // Every value this effect reads is a dep, the page-0 branch's included: one left out would be + // served from a stale closure, and the repo has no lint rule that would catch it. + useEffect(() => { + // Page 0 is restored, never fetched — the dashboard response already carries it. Restoring + // is the whole point of the early return: leaving `items` untouched would keep the previous + // page's rows and its error on screen under a "page 1" label, with no way out short of + // changing a filter. + if (page === 0) { + setItems(recent); + setPagedTotal(total); + setPageError(null); + return; + } + let cancelled = false; + setLoading(true); + setPageError(null); + api + .getUsageErrors(projectId, { offset: page * pageSize, limit: pageSize, ...filters }) + .then((res) => { + if (cancelled) return; + setItems(res.items); + setPagedTotal(res.total); + }) + .catch((e: unknown) => { + // Keep the rows already on screen rather than blanking the table under an error. + if (!cancelled) setPageError(apiErrorText(e)); + }) + .finally(() => { + if (!cancelled) setLoading(false); + }); + return () => { + cancelled = true; + }; + }, [page, pageSize, projectId, filters, recent, total]); + + const pageCount = Math.max(1, Math.ceil(pagedTotal / pageSize)); + // Message rows expanded to their full text (index into the current page); one line each by default. const [expanded, setExpanded] = useState>(new Set()); + useEffect(() => setExpanded(new Set()), [items]); const toggle = (i: number) => setExpanded((prev) => { const next = new Set(prev); @@ -94,13 +177,13 @@ export function ErrorsPanel({ errors }: { errors: UsageErrors }) { {topCode && ( )} {/* Recent-errors table */} - {recent.length === 0 ? ( + {items.length === 0 ? ( ) : (
@@ -109,7 +192,7 @@ export function ErrorsPanel({ errors }: { errors: UsageErrors }) { {S.common.time} {/* Wide enough to fully fit the longest error code: a tool - failure's code carries the tool name (e.g. environment · + failure's code carries the tool name (e.g. [env] tool_failed:exec_command), and truncating it would hide which tool failed. */} {S.usage.errorsColCode} {S.usage.errorsColKind} @@ -117,7 +200,7 @@ export function ErrorsPanel({ errors }: { errors: UsageErrors }) { - {recent.map((e, i) => { + {items.map((e, i) => { const key = kindOf(e.kind); return ( - - {e.source} · {e.code} - + {sourceCode(e.source, e.code)} {kindLabel(key)} @@ -155,6 +236,51 @@ export function ErrorsPanel({ errors }: { errors: UsageErrors }) {
)} + + {/* Pager: once there is more than one page — and unconditionally while paged away from + the first, so a later page that came back empty or shrank below the page count still + offers the way back instead of stranding the reader on a bare table. Kept outside the + scroll box so it stays reachable without scrolling to the bottom of the rows. */} + {(pageCount > 1 || page > 0) && ( +
+ {pageError !== null && {pageError}} + + {S.usage.errorsPageOf(page + 1, pageCount, pagedTotal)} + + setPage((p) => Math.max(0, p - 1))} + /> + = pageCount || loading} + onClick={() => setPage((p) => p + 1)} + /> +
+ )} ); } + +/** Pager step button: same recessive treatment as the rest of the panel's chrome. */ +function PagerButton({ + label, + disabled, + onClick, +}: { + label: string; + disabled: boolean; + onClick: () => void; +}) { + return ( + + ); +} diff --git a/packages/web/src/features/usage/usage-page.tsx b/packages/web/src/features/usage/usage-page.tsx index a20a06f..b180629 100644 --- a/packages/web/src/features/usage/usage-page.tsx +++ b/packages/web/src/features/usage/usage-page.tsx @@ -12,7 +12,7 @@ * (stats + a recent-errors table). * Currency follows the user's settings; a row with unconfigured pricing shows its cost as "—". */ -import { useCallback, useEffect, useState } from "react"; +import { useCallback, useEffect, useMemo, useState } from "react"; import { useSearchParams } from "react-router"; import type { ModelRefDto, UsageBucket, UsageResponse } from "@prismshadow/penguin-server/api"; import * as api from "../../api/endpoints"; @@ -168,6 +168,15 @@ export function UsagePage() { void load(); }, [load]); + // The error panel pages against this filter and depends on the object's identity, so it is + // built once per filter value rather than fresh on every render — a new object each render + // would refetch the current page on any unrelated state change (hovering a chart bucket). + // The model filter is deliberately absent: it never applied to errors. + const errorFilters = useMemo( + () => ({ from, to, ...(agentFilter ? { agentId: agentFilter } : {}) }), + [from, to, agentFilter], + ); + if (!projectId) return null; // Model filter candidates and the currently selected item's index (the option value uses the index, avoiding concatenating an id as the key). @@ -294,7 +303,7 @@ export function UsagePage() { {/* Errors (a single full-width panel: stats + a recent-errors table) */} {data && ( - + )} diff --git a/packages/web/src/lib/strings-en.ts b/packages/web/src/lib/strings-en.ts index 4776afb..8300c1b 100644 --- a/packages/web/src/lib/strings-en.ts +++ b/packages/web/src/lib/strings-en.ts @@ -868,6 +868,11 @@ When done, open index.html in a browser and self-test once.`, errorsColKind: "Type", errorsColMessage: "Message", errorsEmpty: "No errors", + /** Detail-table pager: newer/older step back through pages of the same filtered set. */ + errorsNewer: "Newer", + errorsOlder: "Older", + errorsPageOf: (page: number, pages: number, total: number) => + `Page ${page} / ${pages} · ${total} total`, }, traces: { diff --git a/packages/web/src/lib/strings.ts b/packages/web/src/lib/strings.ts index d655bf9..da39b3a 100644 --- a/packages/web/src/lib/strings.ts +++ b/packages/web/src/lib/strings.ts @@ -846,6 +846,11 @@ Penguin 视觉风格（见 web-design 技能），深色/浅色主题（ + `第 ${page} / ${pages} 页 · 共 ${total} 条`, }, traces: {