diff --git a/CHANGELOG.md b/CHANGELOG.md index 95a1a91..a592abc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Changelog ## Unreleased (master, since v0.1.38) +- **fix(compress): 彻底移除 `content` 参数的 JSON 字符串形式（closes #273）** — 撤销此前为兼容非严格工具 provider 加的 `Type.Union([Array, String])` 兜底：schema 改回仅 `Type.Array(RangeSpec)`，删除 `tailRepair`/`repairContentTail`/`jsonParseError` 及 `describeDiagnostics` 的字符串解析分支，`normalizeRanges` 改为纯数组校验。非数组 `content`（含 JSON 编码的数组字符串）现在直接抛错（pi 标 `isError:true`，计入失败上限）。权衡：会把数组参数字符串化的非严格 provider 现在在 schema 层被拒，不再进兜底解析 - **fix(delegate): 并发完成的 delegate 通知合并为单条批量消息（closes #157）** — 多个 subagent 同时（或主窗口正忙时接连）完成时，每条完成各自 `sendUserMessage(deliverAs: followUp)` 注入一条通知，N 个完成的 delegate 吃掉 N 个完整模型轮次，且模型已收尾后通知仍在持续涌入。现在 finalize 不再直接注入，而是进入 2s 尾沿防抖窗口（自首个排队完成起硬上限 10s，防连续错峰完成饿死投递）；窗口关闭时 `flushDelegateNotifications` 发**一条**批量消息：头部计数（`[acp_delegate] 3 delegates finished (2 completed, 1 FAILED)`）+ 每个 run 一节（状态/exit code/超时注记/任务/结果文件/失败错误摘录）+ 单一尾部（仍在跑的 delegate 数、session delegate usage、收尾指令）。窗口期内获得 waiter 或被 `acp_delegate_wait`/`acp_delegate_cancel` 消费的 run 自动出批（不重复投递）；发送失败不置 delivered，run 留在未送达集由后续 carrier 补投（`findUndeliveredRuns` 将排队中视为已排程而非丢失）；单 run flush 保留原单条格式。净效果：N 个同时完成 → 1 个模型轮次 - **feat(delegate): 失败/取消保留日志 + 失败诊断 + `resumeFrom` 续跑中断的 run (closes #235)** — 此前 cancel 与 spawn error 路径直接删除 `.out`/`.activity` 文件，失败通知只有 exit code（信号被丢弃、stderr 可能为空、activity 轨迹不可见）。现在：① 所有终止路径保留文件（spawn error 把错误写入 `.out`；cancel 回填部分回复，cancel/wait 结果明确给出文件路径）；② 失败通知带 exit 信号（`exit SIGTERM`）、stderr、activity 日志尾部（400 字符）与 activity 文件路径；③ pi 宿主 delegate 用 `--session /.session.jsonl` + `--session-dir` 持久化自身会话（omp 不变，保持 `--no-session`），新增 `resumeFrom: ""` 参数让新 run 恢复原会话（原任务 + 已执行的 tool calls + 部分结果）从中断处继续——`task` 变为可选（提供时作为本次追加指引）；校验：原 run 不得仍在运行、session 文件必须存在、非 pi 宿主拒绝 - **fix(floor): provider-usage floor 跳过压缩后一轮的 stale anchor(#258 评审)** — pi 的 `getContextUsage()` 锚定最后一条有效 assistant usage;成功 compress 落在锚点之后时,下一个 LLM 调用仍以压缩前的大数字 floor,会在刚缩小的上下文上重跑 emergency(nudge 注入 + 工具结果机械截断),面板也继续显示压缩前数字,直到下一条 usage 到达。新增 `src/floor-stale.ts`(`usageAnchorPredatesCompression`):锚点(跳过 aborted/error/全零)早于最后一条成功 compress toolResult(失败/0-block no-op 不算)时跳过 floor,context transform / `acp_status` / `/acp` 三处一致(面板 sessionTokens 同步);对齐 pi 自身 compaction 的 "usage source must be post-compaction" 检查。已知接受:provider 永不报 usage 时(omp #18 tree-sum 回退)floor 仍会永久高位 diff --git a/src/compress-tool.ts b/src/compress-tool.ts index 83bb5af..3318853 100644 --- a/src/compress-tool.ts +++ b/src/compress-tool.ts @@ -8,7 +8,7 @@ import type { AcpRuntime } from "./runtime.js"; import { MAX_COMPRESS_ATTEMPTS } from "./runtime.js"; import { debug, logError, logInfo, logThrow, logWarn } from "./log.js"; import { estimateTokens, collectCoveredMessageIds, collectImageTokens, modelSupportsImages, lastUserMessageId } from "./tokens.js"; -import { defaultCountTokens, parseCompressArgs, viableRanges, formatRanges, type CompressionBlock, type CompressionState, type CompressParseDiagnostics, type NudgeDecision } from "acp-kernel"; +import { defaultCountTokens, viableRanges, formatRanges, type CompressionBlock, type CompressionState, type NudgeDecision } from "acp-kernel"; import { getSystemPromptText } from "./compat.js"; import { OMP_UNSUPPORTED_MESSAGE } from "./omp.js"; @@ -25,16 +25,7 @@ const RangeSpec = Type.Object({ const CompressParams = Type.Object({ topic: Type.Optional(Type.String({ description: "Fallback topic for entries without their own. Omit when each content entry specifies its own topic." })), - content: Type.Union([ - Type.Array(RangeSpec), - // Non-strict-tool providers (vLLM openai-completions, supportsStrictTools: - // false) sometimes stringify nested array arguments — session - // 01a00a38 died on exactly this: pi's typebox validation rejected - // "[{\"topic\":...}]" with "content.0: must be object" and the turn's - // only compress attempt was lost. Accept the JSON-encoded form and parse - // it in normalizeRanges below. - Type.String({ description: "JSON-encoded array of ranges — accepted because non-strict-tool providers sometimes stringify array arguments; parsed automatically." }), - ], { description: "One or more ranges to compress, each with start/end boundaries and a summary. When compressing multiple unrelated ranges in one call, give each its own topic." }), + content: Type.Array(RangeSpec, { description: "One or more ranges to compress, each with start/end boundaries and a summary. When compressing multiple unrelated ranges in one call, give each its own topic." }), summaryMaxChars: Type.Optional(Type.Number({ description: "Override max summary length (default max: 20000 chars). Use when content is important and needs more detail — don't lose critical info just to fit the limit." })), }); @@ -70,84 +61,23 @@ export function makeCompressTool(runtime: AcpRuntime): ToolDefinition; -// Normalize the compress args via the kernel's lenient parser (fenced / -// trailing-comma / raw-newline / double-stringified / truncated-salvage). -// Returns an error string on bad input — handleCompress THROWS it so pi marks -// the toolResult isError:true, which is what makes the outcome count toward -// the failure cap (a returned string would land as isError:false and count -// as neutral). An empty array passes through (the call site returns "No -// ranges provided."). +// Array-only (JSON-string form removed per #273). Returns an error string on +// bad input — handleCompress throws it so pi marks the result isError:true and +// it counts toward the failure cap. The Array.isArray guard is defensive for +// direct execute() callers (tests) that bypass pi's typebox validation. export function normalizeRanges(args: CompressArgs): RangeEntry[] | string { - const effective = repairContentTail(args); - const { ranges, diagnostics } = parseCompressArgs(effective); - if (ranges.length === 0) { - if (Array.isArray(effective.content) && effective.content.length === 0) return []; - return describeDiagnostics(diagnostics, effective.content); - } - return ranges.map((r) => ({ startId: r.startRef, endId: r.endRef, summary: r.summary, topic: r.topic })); -} - -// Qwen-family models in non-strict tool-call mode sometimes emit the `content` -// array as a JSON-encoded string whose LAST entry object is missing its closing -// `}` (tail `"]` instead of `"}]`). The kernel's lenient parser then drops that -// last range — or every range, when it is the only one — and reports a -// misleading "truncated"/"no-valid-ranges" diagnostic. Repair the brace before -// delegating so the whole array parses. Args are returned unchanged when the -// repair does not apply. -function repairContentTail(args: CompressArgs): CompressArgs { - if (typeof args.content !== "string") return args; - const repaired = tailRepair(args.content); - return repaired === undefined ? args : { ...args, content: repaired }; -} - -// Deterministic tail-repair: if the trimmed string ends with `"]` and the char -// before it is a closing `"`, retry the parse with `"}]` appended. A valid JSON -// array never still parses after appending `}`, so this has no false positives. -export function tailRepair(s: string): string | undefined { - const t = s.trimEnd(); - if (!t.endsWith("]")) return undefined; - const body = t.slice(0, -1).trimEnd(); - if (!body.endsWith('"')) return undefined; - const candidate = body + "}]"; - try { - if (Array.isArray(JSON.parse(candidate))) return candidate; - } catch { - // not the missing-brace case - } - return undefined; -} - -function describeDiagnostics(diagnostics: CompressParseDiagnostics, content: CompressArgs["content"]): string { - const shape = typeof content === "string" - ? "a JSON-encoded string (non-strict-tool providers stringify array arguments)" - : content === null ? "null" : `a ${typeof content}`; - const base = `Invalid compress content (${diagnostics.kind}): got ${shape}`; - if (diagnostics.kind === "truncated") { - return `${base}; the input was truncated and no complete ranges could be recovered. Shorten the summary or split into smaller ranges.`; - } - if (diagnostics.invalidItems > 0) { - return `${base}; ${diagnostics.invalidItems} entr${diagnostics.invalidItems === 1 ? "y was" : "ies were"} dropped as invalid. Each range must be an object with string fields startId, endId, summary.`; - } - const parseErr = jsonParseError(content); - if (parseErr !== undefined) { - return `${base}; the JSON failed to parse: ${parseErr}. Fix the malformed JSON (e.g. a missing closing brace or quote) and retry.`; + const content = args.content; + if (!Array.isArray(content)) { + return `Invalid compress content: content must be an ARRAY of {startId, endId, summary} objects — JSON-encoded string arrays are no longer accepted, pass the array directly. Got ${typeof content}.`; } - return `${base}. content must be an ARRAY of {startId, endId, summary} objects.`; -} - -// Short diagnostic for why a JSON-shaped string fails to parse, or undefined -// when it parses fine or is not JSON-shaped. Gives the model a retryable signal -// (the parser's own position) instead of the misleading "must be an ARRAY". -function jsonParseError(content: CompressArgs["content"]): string | undefined { - if (typeof content !== "string") return undefined; - const t = content.trim(); - if (!t.startsWith("{") && !t.startsWith("[")) return undefined; - try { - JSON.parse(t); - return undefined; - } catch (e) { - return e instanceof Error ? e.message : String(e); + if (content.length === 0) return []; + for (let i = 0; i < content.length; i++) { + const r = content[i]; + if (!r || typeof r !== "object" || typeof r.startId !== "string" || typeof r.endId !== "string" || typeof r.summary !== "string") { + return `Invalid compress content: entry ${i} is not a valid range. Each range must be an object with string fields startId, endId, summary.`; + } } + return content.map((r) => ({ startId: r.startId, endId: r.endId, summary: r.summary, topic: r.topic })); } /** Panel block count ("… (~N reclaimed, B blocks)"), or -1 for non-panels. */ diff --git a/tests/compress-retry.test.ts b/tests/compress-retry.test.ts index b507e93..1193b7e 100644 --- a/tests/compress-retry.test.ts +++ b/tests/compress-retry.test.ts @@ -11,7 +11,8 @@ import { isCompressSuccessText, isCompressNoopText } from "../src/compress-tool. // stringified the array). // // Behavior under test: -// 1. compress-tool accepts a JSON-encoded string for content (root cause). +// 1. compress-tool rejects non-array (JSON-string) content with a thrown +// error — the JSON-encoded string form was removed per #273. // 2. Argument errors THROW (pi only marks thrown tool errors isError:true — // a returned error string would be isError:false: not counted + counter // reset). @@ -141,28 +142,7 @@ test("noteCompressOutcomes: counts, caps, resets on success, resets per turn, ne // ─── unit: normalizeRanges via the tool ───────────────────────────────────── -test("compress tool accepts JSON-encoded string content (non-strict-tool providers)", async () => { - const { api, handlers } = captureApi(); - createAcpExtension({ modelContextLimit: 200_000 })(api as any); - const stateFile = "/tmp/pai-acp-retry-str.session.json"; - await rm(`${stateFile}.acp.json`, { force: true }); - const entries = [userMsg("e1", ZH)]; - const ctx = fakeCtx(() => entries, stateFile); - await fire(handlers, ctx); // assigns refs - - const compressTool = api.tools.find((t: any) => t.name === "compress")!; - const out = await compressTool.execute( - "tc1", - // exactly what session 01a00a38's model sent: a JSON-encoded array string - { content: JSON.stringify([{ startId: "m00001", endId: "m00001", summary: "compressed from string form" }]) }, - undefined, undefined, ctx, - ); - const text = typeof out === "string" ? out : out.content?.[0]?.text ?? String(out); - assert.ok(/ACP \|/.test(text), `expected success panel: ${text}`); - await rm(`${stateFile}.acp.json`, { force: true }); -}); - -test("compress tool THROWS on garbage string content (isError:true → counted by the outcome tracker)", async () => { +test("compress tool THROWS on non-array (JSON-string) content (isError:true → counted by the outcome tracker)", async () => { const { api, handlers } = captureApi(); createAcpExtension({ modelContextLimit: 200_000 })(api as any); const stateFile = "/tmp/pai-acp-retry-str2.session.json"; @@ -174,9 +154,11 @@ test("compress tool THROWS on garbage string content (isError:true → counted b const compressTool = api.tools.find((t: any) => t.name === "compress")!; // pi-agent-core marks only THROWN tool errors isError:true; returning the // error string would be isError:false (not counted as a failure + counter - // reset), so the tool must reject. + // reset), so the tool must reject. The JSON-string form was removed per + // #273, so any non-array content (a JSON-encoded array string included) is + // rejected with a clear "must be an ARRAY" error. await assert.rejects( - () => compressTool.execute("tc1", { content: "not json {" }, undefined, undefined, ctx), + () => compressTool.execute("tc1", { content: JSON.stringify([{ startId: "m00001", endId: "m00001", summary: "s" }]) }, undefined, undefined, ctx), /Invalid compress content[\s\S]*ARRAY/, ); await rm(`${stateFile}.acp.json`, { force: true }); diff --git a/tests/compress-tail-repair.test.ts b/tests/compress-tail-repair.test.ts deleted file mode 100644 index 16eca68..0000000 --- a/tests/compress-tail-repair.test.ts +++ /dev/null @@ -1,159 +0,0 @@ -import { test } from "node:test"; -import assert from "node:assert/strict"; -import { rm } from "node:fs/promises"; -import { createAcpExtension } from "../src/index.js"; -import { normalizeRanges, tailRepair } from "../src/compress-tool.js"; - -// issue #253: Qwen-family non-strict tool calls drop the last entry's closing -// `}` (tail `"]` instead of `"}]`). The kernel parser then drops the last range -// (or every range, when it is the only one) and reports a misleading -// "truncated"/"no-valid-ranges" + "must be an ARRAY" diagnostic. The adapter -// repairs the brace before delegating and, when the input is array-shaped but -// still unparseable, reports the parser's own diagnostic instead. - -// Build a content-array string, then drop the last entry's closing `}`. -function dropLastBrace(arr: unknown[]): string { - const s = JSON.stringify(arr); - return s.slice(0, s.length - 2) + "]"; -} - -// ─── unit: tailRepair (the deterministic repair) ──────────────────────────── - -test("tailRepair recovers a single entry missing its closing `}`", () => { - const broken = dropLastBrace([{ startId: "m00001", endId: "m00010", summary: "s" }]); - assert.ok(broken.endsWith('"]') && !broken.endsWith('"}]'), "precondition: tail is `\"]`"); - const repaired = tailRepair(broken); - assert.equal(repaired, JSON.stringify([{ startId: "m00001", endId: "m00010", summary: "s" }])); -}); - -test("tailRepair recovers a multi-entry array whose LAST entry is missing `}`", () => { - const arr = [ - { startId: "m00001", endId: "m00010", summary: "a" }, - { startId: "m00011", endId: "m00020", summary: "b" }, - { startId: "m00021", endId: "m00030", summary: "c" }, - ]; - const repaired = tailRepair(dropLastBrace(arr)); - assert.deepEqual(JSON.parse(repaired!), arr); -}); - -test("tailRepair has no false positives on well-formed inputs", () => { - // A valid object array ends in `}]` → body ends in `}`, not `"`. - assert.equal(tailRepair('[{"startId":"m1","endId":"m2","summary":"x"}]'), undefined); - // A valid string array: appending `}` yields invalid JSON. - assert.equal(tailRepair('["a"]'), undefined); - // Empty array: body `[` does not end in `"`. - assert.equal(tailRepair("[]"), undefined); - // Not array-shaped at all. - assert.equal(tailRepair('{"content": []}'), undefined); - assert.equal(tailRepair("not json"), undefined); - // A genuine mid-array defect (missing brace between entries) is NOT the tail - // case — left untouched for the accurate error path to report. - assert.equal(tailRepair('[{"startId":"m1","endId":"m2","summary":"a"{"startId":"m3","endId":"m4","summary":"b"}]'), undefined); -}); - -test("tailRepair tolerates trailing whitespace after the `]`", () => { - const broken = dropLastBrace([{ startId: "m1", endId: "m2", summary: "s" }]) + " \n"; - assert.equal(tailRepair(broken), JSON.stringify([{ startId: "m1", endId: "m2", summary: "s" }])); -}); - -// ─── unit: normalizeRanges (repair wired in + accurate errors) ────────────── - -test("normalizeRanges repairs a single-entry missing-`}` payload (the #253 failure)", () => { - const summary = "Auth exploration: src/auth/login.ts:12, src/auth/token.ts:45. Chose JWT over session because stateless. saved at /home/dog/tmp/comments_early.json."; - const broken = dropLastBrace([{ startId: "m00001", endId: "m00010", summary }]); - const out = normalizeRanges({ content: broken }); - assert.ok(Array.isArray(out), `expected ranges, got error: ${out}`); - assert.equal(out.length, 1); - assert.deepEqual(out[0], { startId: "m00001", endId: "m00010", summary, topic: undefined }); -}); - -test("normalizeRanges recovers ALL ranges when the last entry is missing `}`", () => { - const broken = dropLastBrace([ - { startId: "m00001", endId: "m00010", summary: "first" }, - { startId: "m00011", endId: "m00020", summary: "second" }, - { startId: "m00021", endId: "m00030", summary: "third" }, - ]); - const out = normalizeRanges({ content: broken }); - assert.ok(Array.isArray(out), `expected ranges, got error: ${out}`); - assert.equal(out.length, 3, "the previously-dropped last range is recovered"); - assert.deepEqual(out.map((r) => `${r.startId}..${r.endId}`), ["m00001..m00010", "m00011..m00020", "m00021..m00030"]); -}); - -test("normalizeRanges applies the top-level topic to repaired ranges", () => { - const broken = dropLastBrace([{ startId: "m00001", endId: "m00005", summary: "s" }]); - const out = normalizeRanges({ topic: "Auth", content: broken }); - assert.ok(Array.isArray(out)); - assert.equal(out[0].topic, "Auth"); -}); - -test("normalizeRanges leaves a well-formed string array untouched", () => { - const s = JSON.stringify([{ startId: "m1", endId: "m2", summary: "x" }]); - const out = normalizeRanges({ content: s }); - assert.ok(Array.isArray(out)); - assert.equal(out.length, 1); - assert.equal(out[0].startId, "m1"); -}); - -test("array-shaped but unparseable (non-tail defect) → parser diagnostic, not 'must be an ARRAY'", () => { - const broken = '[{"startId": "m1", "endId": "m2", "summary": "unterminated'; - const out = normalizeRanges({ content: broken }); - assert.equal(typeof out, "string", `expected an error string, got: ${JSON.stringify(out)}`); - assert.match(out, /failed to parse/); - assert.doesNotMatch(out, /must be an ARRAY/); -}); - -test("non-array-shaped garbage still reports 'must be an ARRAY' (unchanged)", () => { - const out = normalizeRanges({ content: "not json {" }); - assert.equal(typeof out, "string"); - assert.match(out, /must be an ARRAY/); -}); - -// ─── integration: the repaired payload actually compresses end-to-end ─────── - -test("compress tool succeeds on a missing-`}` string payload (end-to-end)", async () => { - const handlers = new Map any)[]>(); - const api: any = { - on(event: string, handler: (e: any, ctx: any) => any) { - const list = handlers.get(event) ?? []; - list.push(handler); - handlers.set(event, list); - }, - tools: [] as any[], - commands: new Map(), - registerTool(tool: any) { this.tools.push(tool); }, - registerCommand(name: string, options: any) { this.commands.set(name, options); }, - }; - createAcpExtension({ modelContextLimit: 200_000 })(api); - const stateFile = "/tmp/pai-acp-tail-repair-e2e.session.json"; - await rm(`${stateFile}.acp.json`, { force: true }); - - const ZH = "中".repeat(6000); - const userMsg = (id: string, text: string) => - ({ type: "message", id, parentId: null, timestamp: "", message: { role: "user", content: text, timestamp: Date.now() } }); - const entries = [userMsg("e1", ZH)]; - const ctx: any = { - mode: "rpc", - hasUI: false, - ui: { notify: () => {}, confirm: async () => true, select: async () => undefined, input: async () => "", setStatus: () => {} }, - model: { contextWindow: 200_000, id: "test-model" }, - getContextUsage: () => null, - sessionManager: { - buildContextEntries: () => entries, - getSessionId: () => "tail-repair-session", - getSessionFile: () => stateFile, - }, - }; - await handlers.get("context")![0]!({ type: "context", messages: [] }, ctx); // assign refs - - const compressTool = api.tools.find((t: any) => t.name === "compress")!; - const broken = dropLastBrace([{ startId: "m00001", endId: "m00001", summary: "compressed via tail repair" }]); - // Before the fix this REJECTED with "no-valid-ranges ... must be an ARRAY" - // (the single entry's missing `}` made the kernel parser drop it). After the - // fix the payload parses, so the tool resolves with a panel — never the - // misleading parse error. - const out = await compressTool.execute("tc1", { content: broken }, undefined, undefined, ctx); - const text = typeof out === "string" ? out : out.content?.[0]?.text ?? String(out); - assert.match(text, /▣ ACP \|/, `expected a panel (payload accepted), got: ${text}`); - assert.doesNotMatch(text, /must be an ARRAY|no-valid-ranges/, `misleading parse error regressed: ${text}`); - await rm(`${stateFile}.acp.json`, { force: true }); -});