Skip to content

feat(offload): add bounded tool-result recovery#448

Open
quan020406 wants to merge 3 commits into
TencentCloud:mainfrom
quan020406:feat/tool-result-offload-read
Open

feat(offload): add bounded tool-result recovery#448
quan020406 wants to merge 3 commits into
TencentCloud:mainfrom
quan020406:feat/tool-result-offload-read

Conversation

@quan020406

Copy link
Copy Markdown

Description | 描述

本 PR 从 #410 中拆分,仅聚焦工具结果卸载与按需读取行为。

主要改动:

  • 在大工具结果进入 prompt-facing history 前进行确定性序列化和 token 估算。
  • 超过 inlineToolResultMaxTokens 时,将完整结果写入 refs/*.md,prompt 中仅保留摘要、result_ref、内容 hash 和恢复指引。
  • 新增 tdai_offload_read,支持按 query、行号范围和 token 预算读取最小必要片段。
  • 为 ref 路径增加单层 refs/*.md 校验,并要求存在活动 session。
  • L1 后续处理复用已生成的 result_ref,避免重复写入完整结果。

不包含以下内容:

  • recall injection placement
  • injected history cleanup
  • cache epoch
  • task snapshot / MMD snapshot 逻辑

Related Issue | 关联 Issue

Related to #120

#410 拆分,按评审意见收敛为独立的工具结果卸载/读取 PR。

Change Type | 修改类型

  • Bug fix | Bug 修复
  • New feature | 新功能
  • Documentation update | 文档更新
  • Code optimization | 代码优化

Self-test Checklist | 自测清单

  • Verified locally | 本地验证通过
  • No existing features affected | 无影响现有功能

验证命令:

  • npm.cmd test
  • npm.cmd run build:plugin
  • node --check dist\index.mjs
  • git diff --check

验证结果:

  • 8 个测试文件、73 个测试通过。
  • 插件构建通过。
  • dist/index.mjs 语法检查通过。
  • diff whitespace 检查通过。

Additional Notes | 其他说明

该 PR 与 #375 独立,评审范围限定在有界卸载与恢复行为本身,不包含 recall 注入布局调整。

@Maxwell-Code07

Copy link
Copy Markdown
Collaborator

Thank you for submitting this PR and participating in Tencent Rhino-bird Open-source Training Program!
We have successfully received your submission. The program is currently in full swing, and we will complete the Code Review for you as soon as possible. Please keep an eye on the status notifications for this PR so you can follow up promptly once the review feedback is provided.
Thanks again for your contribution and open-source spirit! 🚀

@YOMXXX YOMXXX left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot approve this as-is.

tdai_offload_read needs a stricter session boundary. The new tool reads full offloaded tool-result refs, but when ctx.sessionKey is missing it falls back to _lastActiveMgr:

const mgr = ctx?.sessionKey ? await _resolveSession(ctx.sessionKey, ctx?.sessionId) : _lastActiveMgr;

That is not safe for a ref-reading tool. A caller without a session key can read from whichever session happened to be active last, as long as it supplies a refs/*.md name. Tool-result refs may contain raw command output, file contents, diagnostics, or other sensitive task data, so this should not inherit the looser fallback pattern used by internal lifecycle hooks.

Please require an active ctx.sessionKey (or another explicit, validated session binding) for tdai_offload_read, and add tests covering:

  • missing ctx.sessionKey is rejected instead of using _lastActiveMgr
  • a valid session can read its own ref
  • traversal/nested/absolute paths remain rejected
  • requested max_tokens cannot exceed the configured hard limit

There is also a correctness gap worth testing: when replacePromptFacingToolResult() cannot find the matching tool-result message, the raw large result may remain in event.messages even though event.result was replaced. The front-offload path should either guarantee replacement or log/fallback in a way that does not leave the prompt-facing history unbounded.

@quan020406

Copy link
Copy Markdown
Author

Addressed in e760342.

tdai_offload_read now requires an explicit ctx.sessionKey and no longer falls back to _lastActiveMgr. The read is resolved only through the validated session registry entry.

Added coverage for:

  • rejecting a missing ctx.sessionKey;
  • reading a ref from the explicitly bound session;
  • traversal, nested, and absolute ref-path rejection;
  • capping requested max_tokens at the configured hard limit.

Also made front-offload replacement atomic: event.result is replaced only after the matching prompt-facing tool-result message is replaced. If no matching message exists, the raw result is retained and a warning is logged for normal L3 handling.

Validation:

  • npm.cmd test (76/76)
  • npm.cmd run build:plugin
  • git diff --check

@quan020406

Copy link
Copy Markdown
Author

Fixed in 9fc5563.

The ref directory is agent-scoped, so requiring a valid sessionKey alone was insufficient. Refs now carry an owner sessionKey marker when they are written, and tdai_offload_read validates that marker against the caller's ctx.sessionKey before returning content.

A ref written by agent:main:session-1 is now rejected for agent:main:session-2 with the same not-found response used for missing refs. The owning session can still read it.

Legacy refs without an owner marker are intentionally treated as unreadable through tdai_offload_read, which is the safe default.

Validation:

  • npm.cmd test (76/76)
  • npm.cmd run build:plugin
  • git diff --check

@quan020406 quan020406 requested a review from YOMXXX July 11, 2026 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants