fix(subagents): enforce UTF-8 snapshot budgets - #238
Conversation
535806d to
3eb2568
Compare
somewan820
left a comment
somewan820
left a comment
There was a problem hiding this comment.
P1 Must-Fix：extensions/subagents/src/manager.ts:427-435 的 persistExactResult() 在 settle() 中执行，未处理 writer 错误；生产 persistResultArtifact() 可因权限、symlink、磁盘满或 collision 抛错，导致 settle() 在 notify()/onSettled() 前失败，waiter/result delivery 可能永远收不到终态。请让 artifact cache fail-soft，或保证 settlement 先终态通知再处理缓存失败，并补充 failure-path tests。
P1 Must-Fix：exact-result artifact 没有 retention/aggregate size limit，长期运行会持续积累唯一结果并耗尽 cache volume。请加入 bounded cleanup/retention 及写入/清理失败恢复。
P2 Should-Fix：enforceSnapshotBudget() 将有损 projection 写回 live entry.snapshot，transcript omits 后 takeover UI 无 rehydrate 路径，历史会静默消失。执行 snapshot 与 UI/model projection 应分离，或提供显式 rehydrate/omission 状态。P3：snapshot parsers 当前无生产调用方。静态审查，未运行测试。
github-actions
Bot
added
the
area:subagents
3a0cc46 to
51b6a22
Compare
|
感谢审查，已在最新提交 中处理这些问题。 P1：artifact persistence 不再阻断 settlement settle() 现在先完成 terminal status、retained result、projection 更新、 notify() 、 onSettled() 、waiter 唤醒及并发槽释放，再将 artifact persistence 调度为异步的可选恢复工作。因此权限错误、symlink、磁盘满、collision、lock contention 或 writer throw 都不会阻止终态通知，也不会让 waiter/result delivery 永久等待。 延迟 writer 会捕获 cache/filesystem 异常，严格验证 ref 只能是 { version: 1, digest } ，验证 digest 与 exact result 内容匹配，并使用 run generation guard 防止旧 run 的延迟 writer 把 artifact 附加到重启后的新 run。失败时不会回滚 terminal status 或 finalText 。 新增/更新的 manager failure‑path tests 验证了 writer throw、wrong digest 和非法 ref 场景下： waitFor() 返回、终态可从 per‑id listener 观察、 onSettled() 被调用，且后续仍可 spawn。 P1：artifact cache bounded retention cache 现在有硬性 retention 上限：最多 64 个 artifact 文件、最多 64 MiB payload；自定义 maxFiles / maxBytes 不能超过 hard cap。retention eviction 和 stale metadata cleanup 在 cache lock 内完成。超大写入在触碰 cache 前拒绝，不留下 partial artifact 或 .tmp ，后续正常写入仍可继续。 锁获取使用有界重试；ownership 不确定、PID reuse、lock contention、symlink、unknown/recovery entry 或平台无法提供安全 no‑follow 操作时均 fail‑closed。release cleanup 失败也不会把已成功发布的 ref 变成失败。 P2：canonical snapshot 与 projection 分离 entry.snapshot 保持 canonical event‑folding state， entry.projection 是独立的 aggregate‑bounded read model；projection 不再写回 canonical snapshot。takeover 继续使用 view.getFull?.(id) ?? view.get(id) 。 projection rebuild 失败时会清除旧 projection，读取方回退到最新 canonical state，避免 settled 条目继续显示旧的 running 。exact‑result recovery 只使用经过校验的 artifact 或明确完整的 canonical retained finalText ，不会把 projection 当作 recovery source。新增 finalTextTruncated 标记：当 artifact 不可用且 retained text 被 1 MiB cap 截断时， subagent_result 会明确报告 exact result unavailable，而不是返回截断前缀。 P3：snapshot projection 已有生产调用方 manager 会在 entry 注册和 lifecycle 更新时调用 snapshot projection；canonical snapshot 仍供 event folding 和 takeover rehydration 使用，model‑facing 工具使用 bounded projection。 验证 已执行： text node --test --experimental-strip-types tests/extensions/subagents/manager.test.ts tests/extensions/subagents/result-artifact.test.ts tests/extensions/subagents/snapshot.test.ts tests/extensions/subagents/takeover.test.ts tests/extensions/subagents/index.test.ts 结果：88 tests，74 passed，0 failed，14 skipped。 另已执行并通过： text bun run typecheck |
tt-a1i
left a comment
tt-a1i
left a comment
There was a problem hiding this comment.
复审 exact head b3d7d4665f160e0e3eb8fcaba73c27f536025885。目标是用UTF-8字节预算限制子代理展示快照，同时通过canonical state/受保护artifact保留恢复能力。旧review中的settlement被缓存异常阻塞、缓存无上限、展示投影覆盖canonical state三项都有实质改进，不重复计入。
Standards
独立人工审查无额外架构要求；现有自动守卫确认1项：新增subagent_result没有完成child分类，也未纳入现有tool surface，违反已有工具归属契约。请在现有表中补齐，不要新增一层权限系统。
Spec
3项已复现P2：
- Darwin使用/dev/fd/目录句柄拼子路径，macOS上的cache读写失败。
- 超过16KiB的单行结果无法通过line offset/limit恢复；limit=1仍只有截断提示。
- truncateUtf8Head把四字节字符的3字节前缀解码为替换字符，却误判为完整UTF-8前缀。
详见行内。它们直接影响本PR承诺的可恢复性、兼容性和UTF-8正确性；无需扩展成新存储框架。
验证
Node24/macOS exact checkout：check因index.ts/manager.ts格式失败；完整test失败，明确包含child drift guard和Mac artifact错误。独立重跑guard仍报subagent_result未分类；Mac artifact专项仍报ENOENT /dev/fd/.../cache。独立pageResultText复现20KiB单行只返回60-byte提示；truncateUtf8Head("😀",3)返回替换字符，4096字节真实预算同样复现。GitHub Node22/24 CI也为失败。不把其他并行运行出现的setup超时直接归因本PR；未做实际Pi UI或发布验收。
请修复这些边界并跑通check/test后再复审。本轮未修改源码、提交或合并。
| }); | ||
|
|
||
| pi.registerTool({ | ||
| name: "subagent_result", |
There was a problem hiding this comment.
[P2] 将新工具纳入现有归属和 child 权限分类
新增 subagent_result 后，extensions/shared/child-session.ts 的 child-safe/excluded 列表都没有它，OPENPI_TOOL_SURFACE.subagents 也未登记。当前现有 every registered package tool is classified ... 测试稳定失败，明确报该工具未分类。请按真实用途加入已有 parent-only/child-safe 分类及 subagents 工具可见性表，验证关闭能力和子会话边界，不应让新工具绕开已建立的登记契约。
| function descriptorRelativePath(fd: number, fallback: string) { | ||
| if (process.platform === "linux") return `/proc/self/fd/${fd}`; | ||
| if ( | ||
| process.platform === "darwin" || |
There was a problem hiding this comment.
[P2] Darwin 的 /dev/fd 不能当作 Linux 的目录遍历入口
本机 macOS 实测目录fd本身可fstat，/dev/fd/<fd>也显示directory，但访问/dev/fd/<fd>/package.json即ENOENT；它不像Linux /proc/self/fd/<fd>支持继续拼接子路径。因此这里声明支持darwin后，openCheckedDirectory中的mkdir/read始终失败，artifact专项稳定报ENOENT mkdir /dev/fd/14/cache，不是测试并行噪声。请验证Mac可用、具有同等安全边界的实现；如果决定暂不支持，需在I/O前明确fail-closed并披露精确恢复的退化，不能仅跳过测试或声称兼容性未变。未推断其他BSD行为。
| truncated: false, | ||
| }; | ||
| } | ||
| const suffix = `\n[page truncated at ${maxBytes} bytes; reduce the requested range]`; |
There was a problem hiding this comment.
[P2] 为超长单行保留可继续读取的恢复路径
当前工具只有行offset/limit，超出字节预算后使用的truncateHead不会截取首条超长行的部分内容。真实输入20KiB单行+尾标记，offset0/limit200与offset0/limit1都只返回60字节的截断提示；offset1已经EOF。用户按提示reduce range也无法取得正文，违反exact recovery用途。请提供最小行内继续读取方式或等价受保护恢复路径，同时保持页面字节上限，并补单行超限的完整恢复测试。
| let end = maxBytes; | ||
| while (end > 0) { | ||
| const candidate = bytes.subarray(0, end).toString("utf8"); | ||
| if (byteLength(candidate) === end) return candidate; |
There was a problem hiding this comment.
[P2] 解码后字节数相同不代表 UTF-8 边界完整
truncateUtf8Head("😀", 3)实际返回U+FFFD替换字符：四字节emoji的前三字节解码成一个三字节replacement，恰好满足byteLength(candidate) === end，因此提前返回了并非原文前缀的字符串。真实4096预算下，4093个ASCII字符后接emoji也会出现同样损坏。现测试只有三字节中文，漏掉了此情形。请按codepoint边界回退或采用严格解码验证，并覆盖四字节字符各切割位置；不要让有界裁剪改写原文内容。
|
当前 head b3d7d46 已复审，Changes Requested。旧的缓存失败阻塞settlement、无缓存上限、canonical/projection混用已有实质修复；新确认4项P2：工具分类遗漏、Mac缓存路径不可用、超长单行无法恢复、四字节UTF-8截断产生替换字符。check/test及必需CI当前均非绿色，详见 #238 (review) 。建议在现有边界内逐项修，不增加新框架。本轮未改源码或合并。 |
Keep canonical settlement separate from bounded projection, persist artifacts fail-soft, and recover through subagent_result(id, offset, limit) instead of cache pathnames.
Keep exact-result paging, UTF-8 truncation, Darwin cache paths, and child-tool classification within the existing subagent surface.
ba54ec5 to
781775e
Compare
tt-a1i
left a comment
tt-a1i
left a comment
There was a problem hiding this comment.
复审 exact head 781775e9acf419d34bff904c1e1353229251202e。目标是给 Direct Subagent snapshot 建立聚合 UTF-8 硬上限，同时保留 exact result 的恢复能力；价值是避免长 Session 的模型/UI read model 持续膨胀。当前提交已经修好工具分类、四字节 UTF-8 截断和超长单行分页，但仍有跨平台恢复、fail-open 和生命周期问题。
Standards
2 项：projection rebuild 失败会回退到完整 canonical snapshot，硬资源上限 fail-open；artifact persistence 用未受 Session Scope 管理的裸 setTimeout，shutdown 不取消/等待，且测试把 onSettled 内断言异常吞掉了。
Spec
2 项：exact artifact 现在仅支持 Linux，macOS 上超过 1 MiB 的终态结果不可恢复；Issue #180 要求的旧版/超大 persisted snapshot restore 边界仍没有实现，当前 projection 只存在于进程内 manager。另有 line-offset 转 byte cursor 时文本提示的 cursor 不是绝对值。
验证：macOS exact checkout bun run check 通过；manager/snapshot/result-artifact 专项 47 passed、14 个 Linux-only skipped；独立分页复现得到文本 next 38，结构化 absolute nextByteOffset=45。GitHub Node 22/24 仍各失败 2 项，因此必需 gate 未通过。建议先缩小并闭合现有边界，不要继续扩充存储框架。
| const RESULT_CACHE_RECOVERY_NAME = | ||
| /^\.retention-lock\.recovery\.[0-9a-f-]{36}$/iu; | ||
| const RESULT_ARTIFACT_TEMP_NAME = /^\.[a-f0-9]{64}\.[0-9a-f-]{36}\.tmp$/u; | ||
| const SUPPORTED_DESCRIPTOR_PLATFORMS = new Set(["linux"]); |
There was a problem hiding this comment.
[P1] 这不是修复 Darwin cache，而是把 exact-result artifact 限定为 Linux。macOS 上 persist 会抛出 unsupported，read 会变成 miss；manager 又只保留 1 MiB finalText，所以更大的 settled result 会被 subagent_result 明确判为 exact unavailable。Issue #180 要求 exact result artifacts 保持可恢复，PR body 也未披露 Linux-only。最小处理可以明确收窄 PR/Issue 范围并保留 #180 未完成；若仍声称 Fixes #180，则需要经验证的 Pi-native/平台安全恢复机制。
| return true; | ||
| } catch { | ||
| // Never expose a projection from an older lifecycle state after a | ||
| // failed rebuild; readers fall back to the canonical snapshot. |
There was a problem hiding this comment.
[P1] 投影重建失败时清空所有 bounded projection，而 view.list/get 随后用 entry.projection ?? entry.snapshot 回退到完整 canonical snapshot。资源上限自身一旦失败，模型/UI 就收到未受 aggregate budget 限制的数据，属于 fail-open。请保留最后一个安全 projection 或返回明确的有界失败状态，不能把 canonical full snapshot 当普通 reader fallback。
| // Artifact persistence is optional recovery work. Schedule it only | ||
| // after settlement has notified waiters and hooks, so a slow or broken | ||
| // writer can never hold the lifecycle path or a concurrency slot. | ||
| setTimeout(() => { |
There was a problem hiding this comment.
[P2] 这个裸 setTimeout 不属于 manager Scope；disposeAll() 不取消也不等待它，所以 Session 关闭后仍可能写 cache、修改 entry 并触发 notify。它也保证 onSettled/delivery clone 发生在 artifact 关联之前。当前“persisted before settlement”测试在 onSettled 里抛出的断言会被生产代码 catch 掉，因此是假阳性。请跟踪并在 teardown 取消/有界等待该任务，同时用不会被生产 catch 吞掉的外部观测验证顺序。
| ); | ||
| } | ||
|
|
||
| const noticeFor = (next: number) => `\n[page truncated; next ${next}]`; |
There was a problem hiding this comment.
[P2] line-offset 页面截断时，这个 notice 使用 page-local end，但返回的 nextByteOffset 是加上 pageStartByte 的绝对 cursor。复现 prefix\n + 长行、offset:1,maxBytes:64：正文提示 next 38，details 返回 nextByteOffset:45。Pi 提供给模型的可靠正文不能指向另一个位置；请让提示使用 absolute cursor，或删掉正文中的数值并只暴露一个可消费的权威 cursor。
tt-a1i
left a comment
tt-a1i
left a comment
There was a problem hiding this comment.
Exact-head review: ef93764
[P1] Exact terminal-result recovery remains unavailable on supported non-Linux hosts. extensions/subagents/src/result-artifact.ts:60 and 233-246 explicitly allow only process.platform linux and throw elsewhere. manager.ts:632-637 still truncates results above the retained prefix, so on macOS/Windows subagent_result cannot recover the exact value promised by this PR and Issue #180.
[P1] Retention can also invalidate a reference that the live manager still exposes. The cache is bounded to 64 MiB, and trimResultArtifactCache() at result-artifact.ts:902-937 protects only the incoming artifact while deleting older files. The manager may retain up to 64 settled entries, but two distinct results around 40 MiB cause the second write to evict the first even though the first snapshot still publishes its artifact reference. That entry then has neither full in-memory text nor a recoverable artifact.
Please provide fail-closed exact recovery on all supported platforms and make retention aware of live references, or make the contract explicitly unavailable before truncating the only exact copy. Add macOS/Windows coverage and a multi-live-entry aggregate-size regression. Also close Issue #180s required bounded restore behavior for legacy/oversized persisted snapshots rather than only restoring counters.
Verification boundary: exact-head Standards + Spec review with source-level boundary reproduction; existing older-head reviews do not cover this exact revision.
3 participants
概要
验证
bun test tests/extensions/subagents/snapshot.test.ts tests/extensions/subagents/manager.test.ts tests/extensions/subagents/index.test.tsbun run lintbun run typecheckFixes #180