feat(tokenless): add native dsh plugin - #2581
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d3fb7b370c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![qoderai[bot]](https://avatars.githubusercontent.com/in/1396158?s=60&v=4){kind=small}
d3fb7b3 to
e5d6a33
Compare
Forrest-ly
left a comment
Forrest-ly
left a comment
There was a problem hiding this comment.
总体评价
DSH 原生插件整体设计扎实：fail-open 语义一致、子进程边界完整（timeout/maxBuffer/signal）、raw/RPM/npm 四条打包链路验证齐全，CLI 参数与 tokenless-cli 契约逐项吻合。主要问题在环境错误归因：会把成功结果误判为失败并向 agent 注入误导性 "Skip retry" 指令——Codex review 已指出 raw-text 路径（dist/index.js:303），本次审查补充其未覆盖的第二个调用点和一处失效的 exit-code 防护，建议修复后合并。
审查结论
request changes
详细意见
🔴 必须修改（阻塞合并）
- [src/tokenless/adapters/tokenless/dsh/dist/index.js:321]（补充 Codex review）Codex P1 指向 303 行的 raw-text 归因，但压缩路径上还有第二个调用点：
classifyEnvironmentError(parsed)只在isError === false的成功结果上运行（312 行singleTextContent门控），扫描成功 JSON 的 stderr/error 字段。例如 exit 0 但 stderr 含 "… timed out …" 警告的构建命令、或把 error 字段当作普通数据的 API 响应，都会被注入[tokenless:env] … failed … Skip retry。仅按 Codex 建议限制 raw-text 路径而不处理此调用点，误报仍然存在。 - [src/tokenless/adapters/tokenless/dsh/dist/index.js:156-161]（补充 Codex review）
if (exitCode === 0 && text.length === 0) return undefined是失效防护：text 为空时 163 行if (!text) return undefined已返回，exitCode === 0分支从不生效；exit_code === 0 且 stderr 非空的成功结果仍会被模式匹配（/timeout/i连代码里的setTimeout都会命中）。防护的意图应是 "exit 0 不视为环境失败"。注意共享分类器common/hooks/hook_utils.py:312（classify_env_error）有同样的问题，本插件是其忠实移植——建议两处同步修复保持各 adapter 行为一致，或至少在 dsh 插件内先行收敛。- 修复建议：raw-text 归因仅在
result.isError === true时进行；结构化 JSON 仅在 exit_code/exitCode 非零或存在显式错误字段时归因；skip/enabled 门控移到归因计算之前。
- 修复建议：raw-text 归因仅在
🟡 建议修改（不阻塞但推荐）
- [src/tokenless/tests/test-dsh-adapter.sh] 测试缺口：PR 描述称覆盖 "fail-open behavior"，实际只测了 abort；缺少 binary 缺失/退出码非 0 的用例，也缺少 "无压缩收益"（CLI 输出 ≥ 输入）的用例。建议补两个 case 并断言 next() 被调用、原内容原样通过。另：fake binary 完全忽略 argv，CLI flag 正确性目前只靠人工核对。
- [src/tokenless/adapters/tokenless/dsh/dist/index.js:20-57] DEFAULT_SKIP_TOOLS / DEFAULT_SHELL_TOOLS / DEFAULT_THRESHOLDS 是
common/hooks/tool_categories.json的硬编码副本（已核对当前逐项一致），双份事实来源有漂移风险。建议加一个 parity 断言测试，或基于import.meta.url从../common/hooks/tool_categories.json读取（仍保持零运行时依赖）。 - [src/tokenless/adapters/tokenless/dsh/dist/index.js:301-308]
responseCompressionEnabled: false只关闭压缩；归因在 303-304 行先于门控计算，disabled 时仍会注入 additionalContexts，且没有独立开关。建议把归因一并纳入 enabled 门控，或提供独立配置项并在 manifest/文档中说明——在上述误报修复前，用户至少需要止血手段。 - [src/tokenless/adapters/tokenless/dsh/dist/index.js compressionArgs] 压缩默认开启 stash（不传
--no-stash），被截断内容以<<tokenless:HASH>>标记呈现、需tokenless retrieve恢复；本 PR 未给 DSH 侧接入 retrieve 指引。若 DSH 会话中的 agent 不懂得恢复，截断内容对模型即事实丢失。建议明确 DSH agent 的恢复路径文档，或在指引就绪前考虑默认noStash。 - [src/tokenless/Makefile:152-161]
build-dsh-plugin只检查文件存在；dist/index.js 是提交的源码而非构建产物，语法/导入错误要到 test-adapters 才暴露，而打包链路（package-raw/RPM/npm）理论上可单独运行。建议在验证接缝加一条低成本的 import 检查（测试脚本已有现成的 import 模式可复用）。
🟢 值得肯定
- fail-open 一致且可验证：CLI 缺失/失败/超时/无收益均回退原内容并经 next() 放行；execFile + argv 数组无 shell 注入面；timeout、maxBuffer、AbortSignal 三重边界齐全。
- 内容替换非常保守：单 text block + JSON 对象/数组 + 严格更短 + 输出重新 JSON.parse 校验，宿主原内容始终权威。
- CLI 参数（--agent-id/--session-id/--tool-use-id/--truncate-*/--max-depth/--no-stash）与 tokenless-cli CompressResponse 定义及 stdin/stdout 契约逐项核对一致。
- 打包集成完整：Makefile build/install/test、raw package.sh + verify-release.py、RPM spec %build/%install/%files、npm packaging 与 prebuilt 测试全部同步更新且带存在性断言。
- 工具清单与阈值同 tool_categories.json 完全对齐；测试覆盖压缩收益短路 next()、混合内容保持、取消传播与归因消息结构。
ikunkun-sys
left a comment
ikunkun-sys
left a comment
There was a problem hiding this comment.
针对当前 head e5d6a338 请求修改。插件构建、定向 adapter 测试、Raw manifest 校验及远端 CI 均已通过，但现有测试没有覆盖以下 DSH 宿主契约：
- 两个 P1：压缩命中会截断 post-execute waterfall；真实 DSH bash 非零退出无法触发环境错误归因。
- 一个 P2：Code Mode 嵌套调用会产生没有减少模型上下文的压缩统计和 stash。
详细复现依据见 inline comments。
Forrest-ly
left a comment
Forrest-ly
left a comment
There was a problem hiding this comment.
总体评价
整体质量较高：压缩主流程的 fail-open 设计严谨，Codex 上一轮提出的 P1（错误归因未按 isError 门控）已在 e5d6a338 正确修复并有测试钉死，打包链路（Makefile/raw/RPM/npm）完整且互相校验。主要问题在于：dsh 插件自带了一套与共享 ENV_PATTERNS 分类法不一致的错误模式与类目名，会造成跨 adapter 的归因行为分叉，建议合并前对齐。
审查结论
request changes
详细意见
🔴 必须修改（阻塞合并）
-
[src/tokenless/adapters/tokenless/dsh/dist/index.js:64-118] 环境错误分类法与共享 canonical 列表分叉。项目已有唯一权威来源
adapters/tokenless/common/hooks/hook_utils.py的ENV_PATTERNS（注释明确说明它是 codex 与 hook adapter 共享的"superset"，类目为ENV_DEPENDENCY_MISSING/ENV_PERMISSION/ENV_FILE_MISSING/ENV_NETWORK/ENV_PACKAGE_MISSING），而本 PR 新写了一份 JS 副本，存在两类分叉：- 类目名不一致：
ENV_PERMISSION_DENIED（:87）vs 共享ENV_PERMISSION；ENV_PATH_MISSING（:98）vsENV_FILE_MISSING；ENV_NETWORK_FAILURE（:110）vsENV_NETWORK。这些类目串会进入 agent 可见消息（且 manifest 已声明stats能力），同一故障在不同 adapter 下产生不同标签，现在就分叉、以后改就是破坏兼容。 - 模式覆盖不一致（已实测验证）：dash 风格的
/bin/sh: 1: jq: not found共享classify_env_error能归因为ENV_DEPENDENCY_MISSING，dsh 插件则漏检（缺/bin/sh:.*: not found模式）——这是 Debian/Ubuntu 容器里最常见的缺依赖报错形态。此外还缺cannot open .* for writing、curl: \(6\)/curl: \(7\)、ECONNREFUSED/ETIMEDOUT、Connection timed out、Failed to connect、Name or service not known、cannot find、does not exist、cannot import name等共享列表已有模式；同时新增了共享列表刻意未收录的宽泛/timeout/i（:107），对已报错但只是文本含 "timeout" 的结果容易误归因为网络故障。
修复建议：与
hook_utils.py的ENV_PATTERNS完全对齐（类目名、模式、hint 文案逐条镜像，并加注释指明 canonical 来源及"修改需双侧同步"）；更稳妥的做法是把模式表抽成共享 JSON（参照tool_categories.json的做法）供两侧读取。若确属有意分叉，请在代码与 PR 描述中说明理由。 - 类目名不一致：
🟡 建议修改（不阻塞但推荐）
- [src/tokenless/tests/test-dsh-adapter.sh] PR 描述声称覆盖 fail-open，但测试套件没有对应用例。现有用例覆盖了压缩、混合内容、取消（aborted signal）、错误归因，但从未触发
execFile失败路径。fail-open 是本插件最关键的安全属性，建议补充：tokenlessBin指向不存在的二进制（我手工验证过此路径行为正确：next()被调用、原结果保留）、CLI 非零退出、CLI 超时、CLI 输出不缩短（candidate 长度 >= 原文）四种情形。另外建议补skipTools命中但isError === true时仍附加归因、以及responseCompressionEnabled: false的用例。 - [src/tokenless/adapters/tokenless/dsh/dist/index.js:339-349] 压缩命中时直接返回 decision，不调用
next()，与其余路径（passWithAttribution总是先await next()）不对称。若 DSH 的tools/post-execute链上还有其他插件（例如未来其他组件也挂 post-execute），它们将永远看不到该结果。请与 DSH/Cordis 的 seam 契约确认"替换内容是否允许短路"：若允许，请在文件头注释中写明这是有意的替换语义；若链应组合执行，建议先await next()再覆盖返回 decision 的content（与passWithAttribution的写法一致）。 - [src/tokenless/adapters/tokenless/dsh/dist/index.js:20-62] skip/shell 工具表与阈值是硬编码副本，与
tool_categories.json存在漂移风险。当前值与 JSON 完全一致（已核对），但 JSON 的_meta自称是"all adapters 的分类与阈值唯一来源"，且曾为修正分类漂移而改版；一旦 JSON 更新，dsh 插件会静默落后。安装后 dsh bundle 与common/hooks/同处{datadir}/adapters/tokenless/之下，插件可在apply()时尝试加载../common/hooks/tool_categories.json并以现有硬编码值兜底；至少请加一个 parity 测试或注明 canonical 来源。另外 :42 注释"the same thresholds used by the shared hook adapter"目前只是巧合成立，建议一并修正表述。 - [src/tokenless/adapters/tokenless/dsh/dist/index.js:313,322]
responseCompressionEnabled: false只关闭压缩，不关闭错误归因注入（已实测确认）。从命名看用户可能期望该开关让插件完全静默。若属有意设计（开关只管压缩），请在配置说明中写明；否则建议拆独立开关（如errorAttributionEnabled）。 - [src/tokenless/CHANGELOG.md / CHANGELOG_zh.md] 缺少 changelog 条目。项目遵循 Keep a Changelog，
[Unreleased]目前为空，且此前的 adapter 变更（Qoder 原生化、Qwencode 等）均有中英文双条目。新增 native adapter 属于用户可见变更，请补记。
🟢 值得肯定
- Codex P1 修复到位：原始文本分类现在严格以
result?.isError === true为前提（:315），结构化结果仅在显式报告失败（非零 exit_code /success:false/ok:false）时才归因（:162-180）；successfulMatch、zeroExit、nonzeroExit三个测试把修复钉得很牢。 - fail-open 设计扎实：二进制缺失、CLI 出错、超时、无缩短、信号取消全部回落到
next()（我手工验证了二进制缺失路径），compressText要求 candidate 可被JSON.parse且严格更短才替换（:298-304），替换守卫（单一 text block、JSON object/array）保证混合内容/图片/错误内容零改动。 - 子进程卫生：
execFile无 shell（argv 注入面为零）、stdin 挂 error handler 防 EPIPE 冒泡、maxBuffer/timeout/AbortSignal 三重边界。 - 与共享分类法的一致性：
DEFAULT_SKIP_TOOLS、DEFAULT_SHELL_TOOLS、两套阈值与hook_utils.py的 fallback/tool_categories.json逐项一致。 - 打包完整且自校验：Makefile
build-dsh-plugin作为验证接缝、raw/RPM/npm 三处均校验产物存在，clean正确区分"生成的 package.json"与"入库的 dist/index.js"（后者是手写 ESM 源码而非构建产物，入库合理，Makefile 注释也已说明）。 - 本地
bash tests/test-dsh-adapter.sh通过（Node v22），CITest tokenless在 heade5d6a338上为 SUCCESS。
Forrest-ly
left a comment
Forrest-ly
left a comment
There was a problem hiding this comment.
总体评价
针对 head e5d6a338 独立审查并在本地（node v22.21.1）运行了新增测试与自写探针。结论：原 Codex bot P1（成功结果 raw-text 误归因）已修复并经实测验证；但插件与 DSH 宿主契约的对接仍有两处阻塞级问题（与 ikunkun-sys 的审查一致，我已对照 deepseek-harness 源码独立核实），另发现一处误归因边角与若干测试缺口。建议修复后合并。
已验证通过的部分
- 原 P1 修复落实：raw-text 归因（
dist/index.js:314-317）现在以result?.isError === true为门控；结构化路径改为classifyStructuredEnvironmentError（:162），仅在显式失败标记（exit_code 非零 /isError/success:false/ok:false）时归因。本地运行bash tests/test-dsh-adapter.sh通过；另用探针确认：成功文本含 "permission denied"、{"exit_code":0,"stderr":"…permission denied…"}均不再产生 attribution。 - fail-open 语义正确：手工探针覆盖 CLI 缺失、无压缩收益（输出 ≥ 输入）、输出非法 JSON 三种场景，均经
next()放行且保留原内容。 - 子进程边界完整：timeout / maxBuffer / AbortSignal 三重约束，
execFile+ argv 数组无 shell 注入面，内容走 stdin。 - 打包集成完整一致：Makefile build/install/test、raw
package.sh+verify-release.py、RPM spec、npm packaging 四处链路同步更新且带存在性断言；test-dsh-adapter.sh已接入make test-adapters。
必须修改（阻塞合并）
- [P1] 压缩命中短路
next()，截断tools/post-executewaterfall（dist/index.js:343-349，与 ikunkun-sys 第一项一致）。DSH 自带 spill policy 的写法是先const decision = await next()再对 accepted content 做变换（packages/spill/spill-policy/src/index.ts），证明该 seam 是 waterfall 组合。当前实现在压缩命中时直接返回accept，且测试显式断言 "next() must not run"——任何注册在 Tokenless 之后的 listener（包括可能返回block的 policy）在该结果上都不会运行，其决定被整体丢弃。建议：先await next()，仅当下游决定为 accept 且内容为单 text block 时替换为压缩结果，block/value 等决定透传，并同步修正测试预期。 - [P1] 结构化失败识别与 DSH 真实结果契约不符（
dist/index.js:335，核实 ikunkun-sys 第二项）。DSH tool-bash 的 canonical 结果在result.value：{ kind:'foreground', exitCode: integer|null, timedOut, stdout:{text,truncated,…}, stderr:{text,…} }；非零退出以isError:false+ 展示文本中的[exit code: N]标记呈现（packages/shell/tool-bash/src/index.tscanonicalBashResult与 output schema）。因此 (a) 真实 bash 失败的 content 不是 JSON，JSON.parse失败后直接放行、无任何归因——即本 PR 主打的 shell 错误归因在真实宿主上不触发；(b) 即使 JSON 化，stdout/stderr是对象而非字符串，:174-176 的typeof part === 'string'过滤会全部丢弃；(c) 新增测试的{"exit_code":1,"stderr":"…"}fixture 是虚构契约。建议：从result.value读取exitCode/timedOut（并兼容嵌套stderr.text/stdout.text）做归因，content-text 解析仅作为其他工具的兜底；测试改用真实 value 形状。 - [P1] 非数字字符串 exit code 被当作失败标记（
dist/index.js:164-165，自写探针发现）。成功结果{"exit_code":"N/A","stderr":"…permission denied…"}会被判定失败并注入ENV_PERMISSION_DENIED … Skip retry——成功数据中恰好带非数字 exit_code 字段 + 错误短语的普通内容会被误判。建议字符串分支仅在可解析为非零整数（如/^-?\d+$/）时视为非零退出，非数字字符串按"无显式退出信号"处理。
建议修改（不阻塞但推荐）
- [P2] Code Mode 子调用应跳过压缩（核实 ikunkun-sys 第三项）：
exec.parent !== undefined的子调用，其展示 content 不进入模型上下文（core/tools README "Only the program's outer logs and return value re-enter model context"），当前实现仍会启动 Tokenless CLI、写 stash 并记录 savings，产生无实际收益的统计与孤立 stash。DSH spill policy 正是因此跳过 parented executions。归因路径可保留，仅压缩路径排除。 - 测试缺口：
test-dsh-adapter.sh目前未覆盖 (a) skipped-tool 路径（如 Grep 成功 → 透传且不压缩；这是原 P1 的事发场景），(b) CLI 缺失 / 无收益 / 非法输出的 fail-open（行为我已手工验证正确，建议固化），(c) DSH 真实 bash value 形状的归因用例。 - 小项：skip 列表内工具在
isError === true时仍会注入 attribution（探针确认）——如果这是有意设计（真实失败仍提示勿盲目重试），建议补注释 + 测试固定；build-dsh-plugin目前只做存在性检查，可加一条低成本nodeimport/语法检查，让打包接缝提前暴露语法错误（与 Forrest-ly 同条建议）。
备注
Forrest-ly review 中两处行号与 e5d6a33 的实际代码不符：其提到的 :321 classifyEnvironmentError(parsed) 成功路径在当前 head 已由带显式失败标记门控的 classifyStructuredEnvironmentError 取代，:156-161 的失效防护代码不存在。探针验证 exit_code:0 + 可疑 stderr 不再产生归因，这两点可能基于较早版本，供作者参考避免重复修改。
值得肯定
- fail-open 一致性、子进程边界、无注入面做得扎实
- 内容替换策略非常保守（单 text block + JSON 对象/数组 + 严格更短 + 重新 parse 校验），宿主原内容始终权威
- 原 P1 修复附带了针对性回归测试（成功文本含错误短语 / 零退出结构化结果 / 非零退出结构化失败）
Forrest-ly
left a comment
Forrest-ly
left a comment
There was a problem hiding this comment.
总体评价
针对 head e5d6a338 独立审查并在本地（node v22.21.1）运行了新增测试与自写探针。结论：原 Codex bot P1（成功结果 raw-text 误归因）已修复并经实测验证；但插件与 DSH 宿主契约的对接仍有两处阻塞级问题（与 ikunkun-sys 的审查一致，我已对照 deepseek-harness 源码独立核实），另发现一处误归因边角与若干测试缺口。建议修复后合并。
已验证通过的部分
- 原 P1 修复落实：raw-text 归因（
dist/index.js:314-317）现在以result?.isError === true为门控；结构化路径改为classifyStructuredEnvironmentError（:162），仅在显式失败标记（exit_code 非零 /isError/success:false/ok:false）时归因。本地运行bash tests/test-dsh-adapter.sh通过；另用探针确认：成功文本含 "permission denied"、{"exit_code":0,"stderr":"…permission denied…"}均不再产生 attribution。 - fail-open 语义正确：手工探针覆盖 CLI 缺失、无压缩收益（输出 ≥ 输入）、输出非法 JSON 三种场景，均经
next()放行且保留原内容。 - 子进程边界完整：timeout / maxBuffer / AbortSignal 三重约束，
execFile+ argv 数组无 shell 注入面，内容走 stdin。 - 打包集成完整一致：Makefile build/install/test、raw
package.sh+verify-release.py、RPM spec、npm packaging 四处链路同步更新且带存在性断言；test-dsh-adapter.sh已接入make test-adapters。
必须修改（阻塞合并）
- [P1] 压缩命中短路
next()，截断tools/post-executewaterfall（dist/index.js:343-349，与 ikunkun-sys 第一项一致）。DSH 自带 spill policy 的写法是先const decision = await next()再对 accepted content 做变换（packages/spill/spill-policy/src/index.ts），证明该 seam 是 waterfall 组合。当前实现在压缩命中时直接返回accept，且测试显式断言 "next() must not run"——任何注册在 Tokenless 之后的 listener（包括可能返回block的 policy）在该结果上都不会运行，其决定被整体丢弃。建议：先await next()，仅当下游决定为 accept 且内容为单 text block 时替换为压缩结果，block/value 等决定透传，并同步修正测试预期。 - [P1] 结构化失败识别与 DSH 真实结果契约不符（
dist/index.js:335，核实 ikunkun-sys 第二项）。DSH tool-bash 的 canonical 结果在result.value：{ kind:'foreground', exitCode: integer|null, timedOut, stdout:{text,truncated,…}, stderr:{text,…} }；非零退出以isError:false+ 展示文本中的[exit code: N]标记呈现（packages/shell/tool-bash/src/index.tscanonicalBashResult与 output schema）。因此 (a) 真实 bash 失败的 content 不是 JSON，JSON.parse失败后直接放行、无任何归因——即本 PR 主打的 shell 错误归因在真实宿主上不触发；(b) 即使 JSON 化，stdout/stderr是对象而非字符串，:174-176 的typeof part === 'string'过滤会全部丢弃；(c) 新增测试的{"exit_code":1,"stderr":"…"}fixture 是虚构契约。建议：从result.value读取exitCode/timedOut（并兼容嵌套stderr.text/stdout.text）做归因，content-text 解析仅作为其他工具的兜底；测试改用真实 value 形状。 - [P1] 非数字字符串 exit code 被当作失败标记（
dist/index.js:164-165，自写探针发现）。成功结果{"exit_code":"N/A","stderr":"…permission denied…"}会被判定失败并注入ENV_PERMISSION_DENIED … Skip retry——成功数据中恰好带非数字 exit_code 字段 + 错误短语的普通内容会被误判。建议字符串分支仅在可解析为非零整数（如/^-?\d+$/）时视为非零退出，非数字字符串按"无显式退出信号"处理。
建议修改（不阻塞但推荐）
- [P2] Code Mode 子调用应跳过压缩（核实 ikunkun-sys 第三项）：
exec.parent !== undefined的子调用，其展示 content 不进入模型上下文（core/tools README "Only the program's outer logs and return value re-enter model context"），当前实现仍会启动 Tokenless CLI、写 stash 并记录 savings，产生无实际收益的统计与孤立 stash。DSH spill policy 正是因此跳过 parented executions。归因路径可保留，仅压缩路径排除。 - 测试缺口：
test-dsh-adapter.sh目前未覆盖 (a) skipped-tool 路径（如 Grep 成功 → 透传且不压缩；这是原 P1 的事发场景），(b) CLI 缺失 / 无收益 / 非法输出的 fail-open（行为我已手工验证正确，建议固化），(c) DSH 真实 bash value 形状的归因用例。 - 小项：skip 列表内工具在
isError === true时仍会注入 attribution（探针确认）——如果这是有意设计（真实失败仍提示勿盲目重试），建议补注释 + 测试固定；build-dsh-plugin目前只做存在性检查，可加一条低成本nodeimport/语法检查，让打包接缝提前暴露语法错误（与 Forrest-ly 同条建议）。
备注
Forrest-ly review 中两处行号与 e5d6a33 的实际代码不符：其提到的 :321 classifyEnvironmentError(parsed) 成功路径在当前 head 已由带显式失败标记门控的 classifyStructuredEnvironmentError 取代，:156-161 的失效防护代码不存在。探针验证 exit_code:0 + 可疑 stderr 不再产生归因，这两点可能基于较早版本，供作者参考避免重复修改。
值得肯定
- fail-open 一致性、子进程边界、无注入面做得扎实
- 内容替换策略非常保守（单 text block + JSON 对象/数组 + 严格更短 + 重新 parse 校验），宿主原内容始终权威
- 原 P1 修复附带了针对性回归测试（成功文本含错误短语 / 零退出结构化结果 / 非零退出结构化失败）
重复提交（与 03:23:08Z 的 review 完全相同，操作失误重复投递），以该条为准。
e5d6a33 to
4fc92c1
Compare
|
Addressed the latest Forrest and ikun reviews in 4fc92c1.
responseCompressionEnabled intentionally controls compression only; failure attribution remains active as a safety signal, now documented in code and pinned by a test. I did not add CHANGELOG entries because specs/documentation-standard.md §5 requires daily feature/fix PRs to leave changelog aggregation to release version-bump PRs. Validation passed: make lint; all Rust, hook, Python, and adapter suites with the current 0.7.6 build; raw and npm package tests under the standard 0022 packaging umask. |
Forrest-ly
left a comment
Forrest-ly
left a comment
There was a problem hiding this comment.
总体评价
针对 head 4fc92c1 的 re-review，结论：APPROVE。前两轮的两个 🔴 阻塞项均已确认修复，作者声称的 3 个修复点在代码与测试中均得到验证；本地运行 bash src/tokenless/tests/test-dsh-adapter.sh（Node v22.21.1）与 make build-dsh-plugin 全部通过，修复未引入新的阻塞问题。
🔴 阻塞项复核（均已修复）
- 第 1 轮：环境错误归因误报 — raw-text 归因现在仅在
result.isError === true时执行（adapters/tokenless/dsh/dist/index.js:343-345），压缩路径不再有扫描成功结果的第二个调用点。结构化归因仅在isError === false时进行，且只认 canonicalresult.value中的显式失败标记：数字/整数字符串exitCode !== 0、timedOut、isError、success === false、ok === false（classifyStructuredEnvironmentError）。exitCode === 0且 stdout 含敏感词不再注入归因（测试zeroExit）；非数字退出码（'N/A'）被拒绝（测试nonnumericExit）。 - 第 2 轮：ENV_PATTERNS 与
hook_utils.py分叉 — 类目名已与 canonical 列表一致（ENV_DEPENDENCY_MISSING / ENV_PERMISSION / ENV_FILE_MISSING / ENV_NETWORK / ENV_PACKAGE_MISSING）；5 组模式逐条与common/hooks/hook_utils.py核对一致，包括 dash 风格/bin/sh:.*: not found、curl (6)/(7)、ECONNREFUSED/ETIMEDOUT 等；过宽的/timeout/i已移除；镜像契约在注释中声明（dist/index.js:64-66）。
作者 3 个修复点验证
- Waterfall 顺序（
dist/index.js:356-380）：先await next()；block 决策与 canonical-value 替换决策原样透传（测试用strictEqual断言引用透传）；仅在kind === 'accept'、决策无自有value、单 text block 时替换展示内容；下游additionalContexts得到保留。此前“压缩命中短路返回、不调用next()”的不对称已消除。 - 归因改读 canonical
result.value：覆盖嵌套stdout.text/stderr.text；测试改用真实 DSH bash 结果形状。 - Code Mode 子调用（
exec.parent !== undefined）：保留环境归因，跳过压缩/stash/savings（测试parentFailure断言 tokenless 二进制未被调用）。
第 2 轮 🟡 建议复核
- fail-open 测试：已补齐（binary 缺失 / 非零退出 / 超时 / 无压缩收益 / 无效 JSON 输出），全部通过。
- tool_categories.json parity：
DEFAULT_SKIP_TOOLS/DEFAULT_SHELL_TOOLS/ 阈值与tool_categories.json完全一致；新增 parity 测试直接读取该 JSON 并断言插件行为，未来漂移会导致测试失败。 responseCompressionEnabled不门控归因：作者明确为有意设计——归因独立于压缩，配有代码注释与测试（disabledFailure），可接受。- changelog：仍缺。
src/tokenless/CHANGELOG.md与CHANGELOG_zh.md均有空的[Unreleased]段，本 PR 未新增条目，建议合并前补上（非阻塞）。
小观察（非阻塞，可选）
classifyStructuredEnvironmentError将stdout.text一并拼入扫描文本，而 Python canonicalclassify_env_error只扫 stderr+error。因仅在显式失败标记后触发，误报风险很低，但毕竟与 canonical 有语义差异，建议加注释说明或保持对齐。- JS
/which:\s+no/i对应 Python 的"which: no"（单空格字面子串），JS 版匹配略宽（\s+），影响可忽略。
Forrest-ly
left a comment
Forrest-ly
left a comment
There was a problem hiding this comment.
总体评价
针对新 head 4fc92c1（force-push 压缩后的单一 commit）独立审查，并在本地（Node v22.21.1）运行了适配测试与自写探针。上一轮三个 P1/P2（waterfall 截断、DSH canonical result.value 契约、非数字 exit_code、Code Mode 子调用压缩）均已正确修复并用回归测试钉死；ENV 分类表已与 hook_utils.py canonical 来源逐项对齐；raw/RPM/npm 打包链路完整。未发现阻塞问题，仅在新引入的 result.value 归因路径上有三个增量精度/覆盖建议。
审查结论
approve
详细意见
🔴 必须修改（阻塞合并）
无。
🟡 建议修改（不阻塞但推荐）
- [src/tokenless/adapters/tokenless/dsh/dist/index.js:205-208] timedOut 兜底归因为 ENV_NETWORK 会误导诊断。当
value.timedOut === true且 stderr/stdout 未命中任何模式时，兜底注入ENV_NETWORK (Network connectivity issue. Check DNS, proxy, and firewall settings.)。探针验证：exitCode: 124, timedOut: true, stderr: ""的纯超时（如长构建、sleep 超限）也会被标记为网络故障——"Skip retry" 建议本身正确，但类目/hint 会把 agent 引向 DNS/代理/防火墙排查。现有测试（timedOut 用例）把该行为钉成了 ENV_NETWORK。建议：无模式命中时不做归因；或确需覆盖超时时，先在hook_utils.pycanonical 表引入中性超时类目再双侧同步镜像，并同步调整测试。 - [src/tokenless/adapters/tokenless/dsh/dist/index.js:159-167]
result.error为纯字符串时被 errorText 丢弃。当前只处理error.message为字符串的对象形态；若宿主以result.error = "permission denied ..."的字符串形态报错（工具协议中常见），该消息完全不参与分类，只扫描 content。探针验证：isError: true+ 字符串 error + 无关 content → 无归因。方向安全（漏报而非误报），但错误归因这一主打能力会静默失效。建议：在 errorText 开头补if (typeof error === 'string') return error。 - [src/tokenless/adapters/tokenless/dsh/dist/index.js:182-188]
exitCode: null未视为失败标记。DSH 契约中exitCode为 integer|null（null 一般对应进程被信号杀死）；探针验证exitCode: null+ stderrConnection refused→ 无归因。若 DSH 保证 null 仅出现在 signal-kill 场景，建议将其纳入failed判定（或至少允许对可匹配 stderr 归因）；若属有意保守，请补注释说明意图并加一个 killed-process 探针用例钉住行为。
🟢 值得肯定
- 上一轮全部 P1/P2 经探针独立验证已修复：handler 先
await next()再仅替换 accepted display content（block / canonical value 决定原样透传、downstream additionalContexts 完整保留）；归因改读result.value的 exitCode/timedOut 及嵌套stdout.text/stderr.text；非数字字符串 exit_code 被/^-?\d+$/正确拒绝；parented Code Mode 子调用保留归因但跳过压缩/stash/savings。每条修复都有对应回归测试。 ENV_PATTERNS与hook_utils.pycanonical 表逐项一致（类目名、顺序、hint、模式集合，含 dash 的/bin/sh:.*: not found形态），注释明确了 canonical 来源与双侧同步要求；此前分叉的ENV_PERMISSION_DENIED/ENV_PATH_MISSING/ENV_NETWORK_FAILURE类目名与宽泛/timeout/i模式已消除。- 内容替换策略保持保守（单 text block + JSON 对象/数组 + 严格更短 + 重新 parse 校验 + 宿主的 downstream content 优先），fail-open 四路径（binary 缺失 / 非零退出 / 超时 / 无收益与非法输出）与 skip-tool、disabled 配置均有测试钉死；工具分类表与阈值通过运行时读取
tool_categories.json的 parity 测试防漂移。 build-dsh-plugin增加了 node import/契约校验接缝；Makefile、raw package.sh、verify-release.py、RPM spec、npm packaging 与两个打包测试全部同步更新且带存在性断言。本地bash tests/test-dsh-adapter.sh全量通过。
Forrest-ly
left a comment
Forrest-ly
left a comment
There was a problem hiding this comment.
总体评价
针对 head 4fc92c1 做了独立复审（含本地运行验证），确认既有两轮 APPROVE 的结论成立：此前所有 🔴/P1/P2 阻塞项均已修复并有测试钉死，未发现新的阻塞问题。本次增量发现仅一条用户文档缺口（🟡，既有 review 均未提及）。
审查结论
approve
独立验证记录
- 本地（Node v22.21.1）运行
bash src/tokenless/tests/test-dsh-adapter.sh、make build-dsh-plugin、bash src/tokenless/tests/test-package-raw.sh全部通过。 ENV_PATTERNS与 canonical 来源common/hooks/hook_utils.py逐条核对一致：5 个类目名、hint 文案、模式集合与顺序（含 dash 风格/bin/sh:.*: not found、curl (6)/(7)、ECONNREFUSED/ETIMEDOUT 等）；差异仅为大小写不敏感（方向安全）与既有 review 已记录的/which:\s+no/微宽。DEFAULT_SKIP_TOOLS（19 项）/DEFAULT_SHELL_TOOLS（11 项）/ shell 阈值 65536/128/8 / api 阈值 1048576/65536/32 与common/hooks/tool_categories.json逐项一致，parity 测试运行时读取该 JSON 防漂移。component.toml.in模板替换后可被 TOML 解析，8 个 adapter 块（含新增 dsh 的 bundle entry 与>=0.1.0-rc.2 <0.2.0compat）均正确；manifest.json.in与 dshpackage.json.in为合法 JSON。- 打包四链路（Makefile install / raw package.sh + verify-release.py / RPM spec / npm package-npm.js）对 dsh 三件套（package.json、cordis.patch.yml、dist/index.js）的包含与存在性断言完整一致。
- 核心插件复核：归因严格门控（raw-text 仅
isError === true，结构化仅认 canonicalresult.value显式失败标记，非数字 exitCode 被拒）；waterfall 先await next()再仅替换 accepted 单 text block 展示内容，block/value 决定原样透传；execFileargv 数组无注入面，timeout/maxBuffer/AbortSignal 边界完整，fail-open 四路径有测试。
详细意见
🔴 必须修改（阻塞合并）
无。
🟡 建议修改（不阻塞但推荐）
- [docs/user-guide/en/token-saving/tokenless/framework-integration.md 及 zh 对应文档] 新增 native adapter 是用户可见变更（RPM spec 描述已指引
anolisa adapter enable tokenless dsh --profile <profile>），但用户指南未同步：framework-integration.md 的 "Agent adapter support matrix" 表（en 约 :11-21 / zh 约 :11 起）与 "Enable one adapter" 的 enable 命令列表（en :73-84 / zh :72-84）均未包含 dsh；QUICKSTART.md 的 Agent/Setup 表（en 约 :117 / zh 约 :114）同样缺 dsh 行。建议按既有条目格式补充：dsh 行（plugin 生命周期接入、Cordis patch、不改 profile）、anolisa adapter enable tokenless dsh示例、兼容版本范围>=0.1.0-rc.2 <0.2.0。与既有 review 已指出的 changelog 缺口同属发布完整性事项，可一并补齐。
🟢 值得肯定
- 阻塞项修复质量高且每条都有回归测试钉死（waterfall 组合、canonical value 归因、非数字 exitCode 拒绝、Code Mode 子调用跳过压缩、fail-open 四路径）。
- 与共享 canonical 来源（
hook_utils.pyENV_PATTERNS、tool_categories.json）的镜像关系在注释中声明，并分别以注释契约与 parity 测试防漂移。 - 打包验证接缝
build-dsh-plugin含 node import/契约校验，raw/RPM/npm 四条链路同步更新且带存在性断言。
Forrest-ly
left a comment
Forrest-ly
left a comment
There was a problem hiding this comment.
总体评价
针对 head 4fc92c1 做了独立复审：既有 review 的所有阻塞项（归因门控、waterfall 组合、canonical value 归因、Code Mode 子调用、分类法 parity、fail-open 测试）均已确认修复并有测试钉死。本次另将插件行为与 DSH 官方 packages/core/tools README 的 PostToolDecision 契约逐项核对，未发现契约违背；增量发现一条：结构化归因路径在分类器行为层与 canonical classify_env_error 存在两处既有 review 未讨论的分叉。
审查结论
approve
详细意见
🔴 必须修改（阻塞合并）
无。
🟡 建议修改（不阻塞但推荐）
- [src/tokenless/adapters/tokenless/dsh/dist/index.js:202-208]
classifyStructuredEnvironmentError与 canonicalcommon/hooks/hook_utils.py:312（classify_env_error）存在两处分类器行为分叉，既有 review（含 ENV_PATTERNS parity 核查）均未覆盖：- timedOut → ENV_NETWORK 兜底（:205-208）：命令超时（
timedOut: true）且文本未命中任何模式时，注入 ENV_NETWORK 与 "Check DNS, proxy, and firewall settings" 提示。DSH 的timedOut是工具执行超时（README 中 execute wrapper 的 timeout 语义），并非网络概念——死循环构建、CPU-bound 命令、被 DSH 超时杀掉的sleep都会被标成网络故障，把 agent 引向 DNS/代理/防火墙排查。而同一失败在 hook adapter 下 canonical 分类器返回空——这正是上一轮 🔴（ENV_PATTERNS 分叉）要求消除的"同一故障、不同 adapter 不同标签"形态。测试（timedOutcase：exitCode 124 + stderr "command stopped after timeout" 断言 ENV_NETWORK）已把该行为钉死。 - stdout 纳入扫描文本（:202）：canonical 只扫
stderr + error，此处为stderr + error + stdout。失败命令的 stdout 若恰好含命中模式的数据文本（如输出中带 "permission denied" 字样），会误标类目，且与 hook adapter 行为分叉。
- 修复建议（二选一并保持双侧同步）：(a) 向 canonical 收敛——扫描文本去掉 stdout、移除 timedOut 兜底，timedOut 视为"失败但类目未知"（可保留归因消息但不套用网络类目与 hint）；(b) 若确要保留超时归因，先在 canonical
hook_utils.py增设中性类目（如 ENV_TIMEOUT + 中性 hint）再双侧镜像，维持分类法单一权威来源原则。无论哪种，:64 注释目前只声明模式表 parity，建议把 parity 契约扩展到分类器行为层并注明。
- timedOut → ENV_NETWORK 兜底（:205-208）：命令超时（
🟢 值得肯定
- 与 DSH 官方 README 契约核对无违背：waterfall 先
await next()，block / canonical value 决定原样透传，仅替换 accepted 单 text block 展示内容；归因附加到 block 决定时属于 README 明确允许的 "contexts explicitly supplied by the blocking decision"。 - 归因门控严格（raw-text 仅
isError === true、结构化仅认 canonicalresult.value显式失败标记、非数字 exitCode 拒绝），fail-open 五路径（binary 缺失 / 非零退出 / 超时 / 无收益 / 非法输出）齐全，且 fake binary 通过TOKENLESS_TEST_ARGS落盘 argv 做 CLI flag 断言，补上了既有 🟡 提出的缺口。 tool_categories.jsonparity 测试运行时读取 JSON 防漂移；component.toml.in / manifest.json.in 的 plugin_id / pluginId 双轨命名与 openclaw、claude-code 既有约定一致；raw/RPM/npm 四条打包链路对 dsh 三件套的包含与存在性断言完整。
Forrest-ly
left a comment
Forrest-ly
left a comment
There was a problem hiding this comment.
总体评价
针对 head 4fc92c1 独立审查：本地（Node v22.21.1）复跑 test-dsh-adapter.sh 全部通过，并自写探针验证。与 DSH 官方契约（deepseek-ai/deepseek-harness 的 packages/core/tools README 与 docs/subsystems/tools.md 生成区）逐项核对：listener 签名 (exec, result, next)、PostToolDecision 三形态（accept-content / accept-value / block）、"content 替换保留 canonical value"、"block 仅暴露 blocking decision 显式提供的 contexts" 均吻合；ENV_PATTERNS 镜像表与 tool_categories.json 分类/阈值也逐项对照 canonical 来源核实一致。既有 review 的全部阻塞项确认已修复且有测试钉死。本次增量发现三条既有 review 均未涉及的非阻塞问题（配置覆盖语义、压缩管线缺 TOON 二段、无最小尺寸门控），均已用探针实证。
审查结论
approve
详细意见
🔴 必须修改（阻塞合并）
无。
🟡 建议修改（不阻塞但推荐）
- [src/tokenless/adapters/tokenless/dsh/dist/index.js:340, :254]
skipTools/shellTools配置覆盖是整体替换语义，与 :17-19 注释宣称的 "callers may extend or replace" 不符，且会静默剥夺 lossless 安全默认。探针实证：设置skipTools: ['my_db_query']（用户意图：追加一个 lossless 工具）后，Read/Grep等全部默认内容获取工具立即失去 skip 保护、进入有损压缩；shellTools覆盖同理会把Bash从 shell 阈值降到 API 阈值。现有测试未覆盖任何配置覆盖路径。修复建议（选一）：(a) 改为合并语义，或新增extraSkipTools/extraShellTools用于扩展、skipTools保留整体替换语义；(b) 若确认只做替换，修正注释（删 "extend"），在 manifest/文档中写明"覆盖 skipTools 将同时失去默认 lossless 清单"，并补配置覆盖测试钉死所选语义。 - [src/tokenless/adapters/tokenless/dsh/dist/index.js:260] 压缩管线缺少 canonical 管线具备的
compress-toon二段。共享 hook adapter（common/hooks/compress_response_hook.pystep 15/16）为 compress-response → compress-toon 两段取更短者；本插件仅执行 compress-response（全文件 0 处 compress-toon，探针确认 argv 只有单一子命令）。同一负载下 dsh 用户获得的 token 节省将系统性低于 hook adapter 用户，tokenless stats的跨 adapter 节省对比也会因管线差异失真。若属 v1 原生插件的有意裁剪，建议在 PR 描述 Known limitations 中补记并在文档注明；否则按 hook adapter 模式补二段（同样 fail-open、同样"严格更短才替换"守卫）。 - [src/tokenless/adapters/tokenless/dsh/dist/index.js:309-330] 无最小尺寸门控：任意小的单 text block JSON 都会 spawn CLI 进程。探针实证：7 字符
{"a":1}与 199 字符 JSON 均触发 tokenless 子进程；而 canonical hook 对_MIN_RESPONSE_CHARS = 200以下直接跳过（compress_response_hook.py:82, :345）。小负载不可能压缩得更短，fail-open 后行为正确，但在工具调用密集的 dsh 会话中每次调用都多一次进程 spawn（CLI 启动 + stats 记录开销）。建议在compressText入口镜像 canonical 的 200 字符门控（或提供minResponseChars配置）提前返回；归因路径独立于压缩，不受此门控影响。
🟢 值得肯定
- 与 DSH 官方契约核对无违背且映射选择有据：
exec.parent为ToolExecutionToken（symbol），=== undefined判断正确识别 Code Mode 子调用；DSH agent registry 的enter()强制agent.id === agent.session.id，故exec.agent?.id转发为--session-id语义正确，与 hook adapter 从宿主 payload 取session_id的统计分组语义一致。 - 本地独立复跑
test-dsh-adapter.sh（22 组断言，Node v22.21.1）全部通过；fake binary 经TOKENLESS_TEST_ARGS落盘 argv，CLI flag 与 canonical 阈值 parity 断言可信。 ENV_PATTERNS与hook_utils.pycanonical 表、DEFAULT_SKIP_TOOLS/DEFAULT_SHELL_TOOLS/双阈值与tool_categories.json均经本次独立逐项核对一致；compress-responseargv 与tokenless-cli的CompressResponse参数定义逐项吻合。
Forrest-ly
left a comment
Forrest-ly
left a comment
There was a problem hiding this comment.
总体评价
针对 head 4fc92c1 独立审查：本地克隆 PR 分支（Node v22.21.1）复跑 bash tests/test-dsh-adapter.sh 全部通过，并自写探针对结构化归因路径做了行为验证。与 canonical 来源逐项核对：ENV_PATTERNS 镜像表与 common/hooks/hook_utils.py 一致（类目名/hint/模式集合，含 dash 风格 /bin/sh:.*: not found、curl (6)/(7)、ECONNREFUSED/ETIMEDOUT）；DEFAULT_SKIP_TOOLS（19 项）/DEFAULT_SHELL_TOOLS（11 项）/shell 65536/128/8、api 1048576/65536/32 与 common/hooks/tool_categories.json 逐项一致；canonical hook 的 compress-response → compress-toon 二段与 _MIN_RESPONSE_CHARS = 200 门控均已确认（对应既有 review 的两条管线建议）。既有 review 的全部阻塞项（归因门控、waterfall 截断、canonical value 归因、Code Mode 子调用、分类法分叉、fail-open 测试缺口）确认已修复且有测试钉死。本次增量发现两条既有 review（含 5 轮 APPROVE 与全部 inline comments）均未涉及的非阻塞问题：parity 测试对 API 层阈值失去保护、结构化归因丢弃无 message 的 error 对象。
审查结论
approve
详细意见
🔴 必须修改（阻塞合并）
无。
🟡 建议修改（不阻塞但推荐）
- [src/tokenless/tests/test-dsh-adapter.sh:325-331] parity 测试对 API 层阈值硬编码，
layer_3_api漂移不会触发测试失败。该 parity 块的目的是运行时读取共享来源防漂移：layer_1_skip工具表（:304）与layer_2_shell阈值（:312-314）都从tool_categories.json读取，但 API 默认工具的 argv 断言硬编码'1048576' / '65536' / '32'（:328-330）——而同一 JSON 中存在数值相同的layer_3_api.thresholds。若未来 canonical JSON 更新 layer_3_api 阈值，dsh 插件的DEFAULT_THRESHOLDS.api（dist/index.js:59-62）会静默落后且本测试仍通过——正是该 parity 块本应防止的漂移失效模式。建议仿照 shell 层改为String(categories.layer_3_api.thresholds.truncate_strings_at)等读取式断言。 - [src/tokenless/adapters/tokenless/dsh/dist/index.js:193-203] 无字符串
message的value.error对象被排除出扫描文本，与 canonical 分类器行为分叉。classifyStructuredEnvironmentError中，当value.error是无字符串message的对象（如{ code: 'EACCES', errno: 13 }）时，errorValue保持对象形态，被 :202-204 的.filter((part) => typeof part === 'string')丢弃。探针实证（Node v22.21.1）：exitCode: 1+error: {code:'EACCES',errno:13}+ 空 stderr → 无归因；而 canonicalclassify_env_error（hook_utils.py）以str(stderr) + str(error)构造扫描文本，同一负载在 hook adapter 侧会命中 "EACCES" 归因为 ENV_PERMISSION（已独立验证str({'code': 'EACCES', ...})含该模式）。这是继既有 review 指出的 timedOut 兜底、stdout 入扫描之后第三处分类器行为层分叉，方向安全（漏报而非误报），但同一故障跨 adapter 归因不一致。建议（二选一）：镜像 canonical 将 error 对象的字符串投影纳入扫描文本；或在 :64-66 镜像契约注释中明确"无 message 的 error 对象不参与扫描"属有意收窄，并补探针用例钉死所选行为。
🟢 值得肯定
- 既有全部阻塞项修复经本地独立复跑确认：head 4fc92c1 上
bash tests/test-dsh-adapter.sh全量通过；waterfall 先await next()、block/canonical-value 决定原样透传、仅替换 accepted 单 text block 展示内容；归因严格门控（raw-text 仅isError === true、结构化仅认 canonicalresult.value显式失败标记、非数字 exitCode 拒绝）；fail-open 五路径（binary 缺失/非零退出/超时/无收益/非法输出）齐全且经测试钉死。 ENV_PATTERNS与tool_categories.json的镜像关系经本次独立逐项核对一致，镜像契约在注释中声明（dist/index.js:64-66），parity 测试运行时读取 JSON 防漂移——上述两条增量建议仅是补全该机制的保护范围，不影响现有正确性。- 打包四链路（Makefile install / raw package.sh + verify-release.py / RPM spec / npm package-npm.js）对 dsh 三件套（package.json、cordis.patch.yml、dist/index.js）的包含与存在性断言完整；
build-dsh-plugin验证接缝含 node import + apply/name 契约检查，raw/RPM/npm 复用同一接缝。
ikunkun-sys
left a comment
ikunkun-sys
left a comment
There was a problem hiding this comment.
针对当前 head 4fc92c1c 请求修改。上一轮 waterfall 组合、canonical bash value 和 Code Mode 子调用问题均已确认修复，Node 22 adapter 测试与远端 CI 也通过；但当前结构化错误归因仍会把任意成功工具的业务数据误判为宿主失败。具体复现和影响见 inline comment。
Forrest-ly
left a comment
Forrest-ly
left a comment
There was a problem hiding this comment.
总体评价
针对 head 4fc92c1 复核 ikunkun-sys 新提出的 P1（结构化归因把任意成功工具的 value 当作通用失败协议）。本地 Node v22.21.1 直接 import 插件独立复现成立：除 reviewer 给出的 audit-record 场景外，另复现两种误报形态，且确认 responseCompressionEnabled: false 与 skipTools 均无法阻断注入。核对 DSH pinned commit 的契约（packages/core/tools/README.md，ToolExecutionResult）：成功结果为 { isError:false, value:JsonValue, ... }，失败为 { isError:true, error, ... } 且没有 value——value 不承载宿主级失败语义，exitCode/success/ok/timedOut 只是 shell/process 类工具自身 output schema 的字段。上一轮 APPROVE 认为"仅认显式失败标记"的门控已充分，本轮验证证明该前提对非 shell 工具不成立，故结论改为 request changes。
审查结论
request changes
详细意见
🔴 必须修改（阻塞合并）
-
[src/tokenless/adapters/tokenless/dsh/dist/index.js:346-348] 结构化归因作用于任意成功工具的
result.value，超出 DSH 成功契约（核实并支持 ikunkun-sys 本轮 P1）。当前对所有result.isError === false的结果调用classifyStructuredEnvironmentError(result.value)，但 DSH 契约下成功工具的value只是其 output schema 声明的任意 JsonValue，exitCode/success/ok/timedOut并非宿主级通用失败标志（只有 tool-bash 这类工具的契约声明{ kind:'foreground', exitCode, timedOut, stderr:{text,truncated}, ... }）。本地复现（Node v22.21.1，直接对 dist 驱动tools/post-executelistener）：- 成功业务工具返回
{ kind:'audit-record', exitCode:1, stderr:{ text:'permission denied was captured in the historical record' } }→ 注入[tokenless:env] GetProcessRecord failed: ENV_PERMISSION ... Skip retry.（复现 reviewer 场景）； - 业务数据仅含
timedOut: true、无任何错误文本 → 走 timedOut 兜底注入ENV_NETWORK ... Skip retry.（纯字段名误报，无需任何文本命中）； - 业务数据
success:false+error:{ message:'ENOENT: archived log no longer present' }→ 注入ENV_FILE_MISSING ... Skip retry.； - 上述注入在
responseCompressionEnabled: false下同样发生（归因先于且独立于压缩门控，已在代码 :343-354 与 :359-360 确认）；skipTools命中的工具同样会被注入（归因在 skip 判断之前计算）。
影响：归因消息断言"<工具> failed"并指令模型 "Skip retry"，而工具实际成功——把合法业务数据变成误导模型的失败指令，与归因作为可信安全信号的设计目标直接冲突；且当前无任何配置开关可关闭该路径。
修复建议（已本地验证）：把结构化归因门控到输出契约可确认的工具——复用现有
shellTools集合（DEFAULT_SHELL_TOOLS，可经config.shellTools覆盖），仅当shellTools.has(exec.name)时才解释result.value。模拟该门控后全场景通过：上述 1/2/3 误报消除；DSH bash 真阳性（Bash/bash，canonicalforegroundvalue）保留；isError: true的 raw-text 归因不受影响；经shellTools配置的自定义 shell 工具仍可归因。实现上可把 shellTools 提升到apply()中（当前在compressionArgs内每次调用从 config 重组，可顺带统一）。若倾向更精确的契约判定（如结合value.kind === 'foreground'），请说明覆盖哪些工具契约并补充测试钉死。测试建议：现有结构化归因用例全部使用name: 'Bash'（tests/test-dsh-adapter.sh:156 起），门控后不受影响；请补"非 shell 工具携带 exitCode/timedOut/success:false 业务数据不得产生归因"的用例钉死本次误报，并补shellTools配置扩展用例。 - 成功业务工具返回
🟡 建议修改（不阻塞但推荐）
- 本轮无新增非阻塞发现；上一轮 review 的两条非阻塞建议（parity 测试对 API 层阈值硬编码、无 message 的 error 对象不参与扫描）仍然有效，可随本次一并处理。
🟢 值得肯定
- shell 工具范围内的既有修复依然正确且被测试钉死：非数字 exitCode 拒绝归因、exit 0 不归因、canonical
foregroundvalue 归因、waterfall 组合、Code Mode 子调用跳过压缩保留归因、五路 fail-open 均未在本轮验证中发现回归。本轮问题仅在于结构化归因的适用范围超出了 DSH 成功契约，收敛门控即可，不需要重构归因逻辑。
- compress JSON tool results through the native post-execute seam - preserve unsafe or unsupported outputs with fail-open behavior - ship the DSH bundle through raw, RPM, and npm packages Signed-off-by: kongche-jbw <kongche.jbw@alibaba-inc.com>
4fc92c1 to
4736009
Compare
|
Review update for
Validated with the native DSH adapter test, lint, and the full Tokenless test suite against the branch binary. |
Forrest-ly
left a comment
Forrest-ly
left a comment
There was a problem hiding this comment.
Re-review（head 4736009）：通过。针对 ikunkun-sys / Forrest-ly 提出的 P1（成功业务工具的 result.value 被当作通用失败协议）逐条验证如下。
1. P1 修复验证 ✅
dist/index.js 中结构化归因现已严格门控：
const structuredError = result?.isError === false && shellTools.has(exec.name)
? classifyStructuredEnvironmentError(result.value)
: undefinedclassifyStructuredEnvironmentError只对配置shellTools契约内的工具生效（默认为共享 shell 工具分类表，可经 plugin config 覆盖）；- 成功业务工具的任意
value（exitCode/success/ok/timedOut等字段）不再被解释为宿主级失败。
本地独立复现（Node v22.21.1 直接 import 插件）：
- ikunkun-sys 的 audit-record 场景（
{ kind: 'audit-record', exitCode: 1, stderr: { text: 'permission denied...' } }）→ 不再注入[tokenless:env] ... Skip retry.； - 另两种误报形态（
timedOut: true记录型 value、success: false归档型 value）→ 均不再误归因； - 以上在
responseCompressionEnabled: true与false下均验证通过（归因计算先于压缩门控，路径独立）。
2. raw isError 归因路径不受影响 ✅
- 非 shell 工具
isError: true（error.message / content 文本）仍正确归因（ENV_DEPENDENCY_MISSING 等）； - canonical DSH bash 场景保持正确：exitCode 127 → ENV_DEPENDENCY_MISSING、exitCode 1 + stderr permission → ENV_PERMISSION、timedOut 124 + connection timed out → ENV_NETWORK；
- 负向守卫保持：exitCode 0、非数字 exitCode（'N/A'）、仅 stdout 含关键词均不误归因；
- 结构化与 raw 两条路径按
isError === true / false互斥，不会双重注入。
3. 新增测试真实有效 ✅
tests/test-dsh-adapter.sh 已覆盖作者声称的三类用例并全部通过：
- 三种 shell-shaped 业务 value（audit-record / latency-record / archived-result）断言
additionalContexts === undefined； - 自定义
shellTools: ['custom_process']配置下结构化归因生效（ENV_PERMISSION）； - canonical bash 用例及负向用例钉死。
本地执行结果（Node v22.21.1）：bash tests/test-dsh-adapter.sh ✅、make test-adapters ✅、make build-dsh-plugin ✅。
4. 无新问题引入 ✅
- 压缩关闭 / skip 工具 / Code Mode 子调用（
exec.parent）路径：归因保留、decision 形状不被改写（waterfall 透传），且不调用 tokenless CLI； - 打包侧（Makefile、RPM spec、raw package、npm script、verify-release.py）与既有方案一致，未发现回归。
非阻塞备注
shellTools配置为数组时是整体替换默认集（注释已说明 "extend or replace"），自定义后默认 shell 工具不再走结构化归因——属文档化语义，使用方配置时需注意；- JS 版
ENV_PATTERNS与common/hooks/hook_utils.py实质一致（均为大小写不敏感匹配），仅which:\s+no（JS）比字面量"which: no"（py）略宽；两边仍靠注释约定手工同步，建议后续考虑加一个机械 parity 测试钉死（不阻塞本 PR，属既有设计）。
LGTM，感谢快速修复。
Forrest-ly
left a comment
Forrest-ly
left a comment
There was a problem hiding this comment.
总体评价
针对 head 4736009 复核上轮 CHANGES_REQUESTED（结构化归因作用于任意成功工具的 result.value）与 ikunkun-sys 的 P1。修复方式与上轮建议完全一致且实现正确：shellTools 已提升到 apply() 中只构建一次（dist/index.js:346），结构化归因严格门控为 result?.isError === false && shellTools.has(exec.name)（dist/index.js:352-354），raw isError 归因对所有工具保留。本地 Node v22.21.1 独立复现上轮全部误报场景均已消除，真阳性全部保留，测试已将新行为钉死。无新增阻塞问题，同意合并。
审查结论
approve
详细意见
🔴 必须修改（阻塞合并）
无。
🟡 建议修改（不阻塞但推荐）
- [src/tokenless/adapters/tokenless/dsh/dist/index.js:159-167]（补充上轮"无 message 的 error 对象不参与扫描"）结构化路径已修复——
value.error无message时现在会JSON.stringify(error)参与扫描（errorObject用例以{ code: 'EACCES', errno: 13 }钉死 ENV_PERMISSION，已本地验证）。仅剩的对称性缺口在 raw 路径：errorText()对isError: true结果只取error.message，message 缺失时退回 content 文本；若宿主以无 message 的对象形式给出error且 content 未复述诊断信息，该失败不会获得归因。方向是安全的（漏报而非误报），可选打磨：errorText()对无 message 的 error 对象做同样的 stringify 兜底。不阻塞。
🟢 值得肯定
- 门控修复的实现质量高：完全按上轮建议复用
shellTools集合并提升作用域，compressionArgs的 shell/api 阈值选择与归因门控现在共用同一集合，消除了两处重组的不一致风险；toolSet对非法配置值不放宽默认集合。 - 上轮 4 个误报形态全部以用例钉死（tests/test-dsh-adapter.sh）：audit-record
exitCode:1+stderr 业务数据、timedOut:true纯字段名数据、success:false+ENOENT 业务数据、responseCompressionEnabled:false下的注入——本地独立复现 8 组场景（含skipTools命中、自定义shellTools恢复归因、配置替换默认集合后Bash不再归因）全部符合预期。 - 旧
timedOut兜底（无文本命中即注入 ENV_NETWORK）被移除是正确收敛：该兜底对非网络超时语义不准，现在超时真阳性仍可通过 stderr 模式命中（exitCode:124+connection timed out→ ENV_NETWORK 用例保留），无模式命中则不归因（unclassifiedTimeout用例钉死），与"归因必须是可信信号"的目标一致。 - 上轮两条遗留 🟡 均已落实：与
common/hooks/tool_categories.json的 parity 测试（skip 集合不触发 CLI、shell 集合使用共享阈值 argv 逐项断言）——另经本地双向核对，DEFAULT_SKIP_TOOLS/DEFAULT_SHELL_TOOLS/DEFAULT_THRESHOLDS与共享 JSON 完全一致；无 message error 对象的扫描修复见上。 - 回归面完整：canonical DSH bash 归因、非数字 exitCode 拒绝、exit 0 不归因、waterfall 组合（block/value 透传、downstream additionalContexts 保留）、Code Mode 父调用跳过压缩保留归因、五路 fail-open、raw 打包测试，本地全部通过（
test-dsh-adapter.sh、test-package-raw.sh、make build-dsh-plugin）。
3 participants
Why
Tokenless needs a DeepSeek Harness integration that participates in DSH's
native Cordis lifecycle instead of installing a bridge hook or editing profile
configuration directly. This bundle provides response compression and error
attribution through DSH's typed
tools/post-executeseam while keeping theTokenless CLI as the compression engine.
What changed
@anolisa/dsh-tokenlessnative bundle with nesteddsh.bundle.patchmetadata, a relocatable Cordis patch, and a dependency-freeESM plugin entry.
adapter manifest.
npm prebuilt packages.
cancellation, fail-open behavior, and plugin-owned error attribution.
Current functionality:
tokenless compress-response.object or array and the compressed JSON is strictly shorter.
thresholds, skipped tools, timeout, buffer size, binary path, agent id, and
stash behavior.
exec.signaland bound the child process witha timeout and output limit.
from host errors and canonical shell-tool values, then append a valid
plugin-owned user context that tells the agent not to retry blindly.
preserving Tokenless statistics attribution.
Known limitations:
deliberately excludes tool-argument replacement, so Tokenless cannot safely
rewrite commands through a native public seam yet.
JSON are not compressed. These paths continue unchanged.
compress-responseonly and does not add the hookadapter's TOON second stage.
CLI, but content is replaced only when the result is strictly smaller.
DSH continues with the original result.
>=0.1.0-rc.2 <0.2.0compatibility range.driver is available, the native package can still be registered through
DSH's own plugin CLI.
Related issue
no-issue: add the Tokenless implementation for the shared DSH adapter contract
Companion interface PR: #2580
User / Agent impact
After installing a build that includes #2580, users can run:
The plugin then compresses eligible DSH tool responses without changing tool
calls or profile files itself.
Risk and compatibility
Existing Tokenless adapters and non-DSH installations are unchanged. The new
adapter is inactive until explicitly enabled. Before rolling back after an
enable, remove the Tokenless package from each affected DSH profile or use the
companion ANOLISA driver's disable command.
Validation
cargo fmt --all -- --checkmake -C src/tokenless lintmake -C src/tokenless test-adaptersmake -C src/tokenless build-dsh-pluginbash src/tokenless/tests/test-dsh-adapter.shumask 022 && bash src/tokenless/tests/test-package-raw.shumask 022 && bash src/tokenless/tests/test-package-npm-prebuilt.shpython3 src/tokenless/packaging/raw/verify-release.pygit diff --checkDocumentation and rollback
The adapter manifests and package metadata describe the native contract; no
standalone guide is added in this change. Disable the adapter registrations and
revert this PR's feature commit to remove the bundle and packaging integration.