fix(tokenless): anchor rtk prefix in Hermes and OpenClaw adapters - #2249
fix(tokenless): anchor rtk prefix in Hermes and OpenClaw adapters#2249Forrest-ly wants to merge 1 commit into
Conversation
![qoderai[bot]](https://avatars.githubusercontent.com/in/1396158?s=60&v=4){kind=small}
There was a problem hiding this comment.
- TS 端 shellTokenize 的注释比实现更乐观，容易让调用方误以为提供 shlex 级别的 token 保证。
- anchorRtkPrefix 的“已知局限”仅记录在 Python 端文档里，建议在 TS 注释中同步，保持行为契约一致。
- Node 侧 anchorRtkPrefix 测试矩阵目前只覆盖理想 rewrites，未包含 Python 端注明的边界行为（非 wrapper 位置 rtk 也会被锚定），可以补一个回归用例以防未来重构偏离。
🤖 Generated by Qoder • View workflow run
| /** | ||
| * Tokenize a shell command string without a shell, preserving quoted strings, | ||
| * globs, fd redirections, and command substitutions as single tokens. | ||
| * Mirrors Python shlex.shlex(posix=False, whitespace_split=True, commenters=""). | ||
| */ | ||
| function shellTokenize(cmd: string): string[] | null { |
There was a problem hiding this comment.
[P2] shellTokenize 注释与实现略有偏差
openclaw/index.ts 中 shellTokenize 注释声称“globs、fd 重定向、命令替换作为单个 token 保留”，但当前实现只是按空白切分并在引号内跳过空白，对 2>&1、$(date) 等并无特殊处理，测试也依赖于这一行为。建议将注释改为更窄的描述（例如仅说明保留引号/空白分割、禁用注释），避免给调用方误导性的解析保证。
🤖 Generated by Qoder • Fix in Qoder
There was a problem hiding this comment.
Fixed in 1eba8d1. shellTokenize JSDoc updated — now describes only the guarantees the implementation actually provides (whitespace split + quoted span preservation), without over-claiming fd redirection / command substitution handling.
| /** | ||
| * Replace bare `rtk` wrapper tokens with the resolved absolute binary path. | ||
| * | ||
| * Ports the Python _anchor_rtk_prefix logic: swaps the first unquoted `rtk` | ||
| * token of each pipeline segment (at command start or right after a connective | ||
| * like `&&`/`||`/`;`/`|`/`&`, optionally behind env assignments or wrappers | ||
| * like `sudo`). Quoted patterns, globs, fd redirections, and command | ||
| * substitutions are never modified. Unparseable input is returned untouched. | ||
| */ | ||
| function anchorRtkPrefix(rewritten: string, resolvedRtkPath: string): string { |
There was a problem hiding this comment.
[P2] anchorRtkPrefix 行为已含已知局限但未在 TS 端说明
Python 版本 _anchor_rtk_prefix 文档明确记录“echo rtk done 这类位置会被误判为 wrapper”的已知局限，而 TS anchorRtkPrefix 的注释只描述理想行为。为避免调用方误解范围，建议在 TS 注释中同步补上一句相同局限说明，使两端行为契约一致。
🤖 Generated by Qoder • Fix in Qoder
There was a problem hiding this comment.
Fixed in 1eba8d1. anchorRtkPrefix JSDoc now documents the known limitation (matching Python behavior): a bare rtk token appearing as an argument rather than a command is incorrectly anchored. Added a regression test for this edge case.
| const RTK = "/home/user/.local/share/anolisa/tokenless/rtk"; | ||
|
|
||
| test("simple single command", () => { |
There was a problem hiding this comment.
[P2] Node 测试中逻辑复制缺少“已知局限”回归用例
test_openclaw_anchor.mjs 为避免 build step 直接复制了 anchorRtkPrefix 逻辑，但当前用例只覆盖“正常” rewrite 形状，未包含 Python 端注释里提到的已知局限（例如 echo rtk done 被视为 wrapper）的行为约束。建议补充一个“非 wrapper 位置的 rtk 也会被锚定”的测试，以防未来重构时在 TS 侧悄然改变这一边界行为而破坏 Hermes/Python 侧的一致性。
🤖 Generated by Qoder • Fix in Qoder
There was a problem hiding this comment.
Fixed in 1eba8d1. Extracted shellTokenize, anchorRtkPrefix, SEGMENT_OPS, and isEnvAssignment into a shared anchor-helpers.ts module. The test file now imports from the compiled production code (dist/anchor-helpers.js) instead of duplicating the implementation. Added the test to make test-integration so CI covers the production helpers. Also added regression tests for the known limitation (non-wrapper rtk anchoring).
![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: d2f2c3223c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| i = end + 1; | ||
| } else if (ch === '"') { | ||
| // double-quoted string: consume up to closing " | ||
| const end = cmd.indexOf('"', i + 1); |
There was a problem hiding this comment.
When a rewritten command contains a valid escaped double quote, such as rtk grep "foo \" bar" src/, this indexOf treats the escaped quote as the closing delimiter; the later real closing quote is then considered unmatched, shellTokenize returns null, and anchorRtkPrefix leaves the original bare rtk in place. In the trimmed-PATH OpenClaw environments this change is meant to fix, those rewritten commands still fail with rtk: command not found; the tokenizer needs to skip escaped quote characters instead of using the next raw quote.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 1eba8d1. shellTokenize now scans character-by-character inside double-quoted spans, skipping backslash-escaped characters (\", \\, etc.) instead of using indexOf('"'). Added regression tests for escaped double quotes and escaped backslashes.
ikunkun-sys
left a comment
ikunkun-sys
left a comment
There was a problem hiding this comment.
请求修改：当前版本仍有两项阻塞正确性问题，暂不可合并。
-
P1 — OpenClaw 对合法的转义双引号 rewrite 放弃锚定（
src/tokenless/adapters/tokenless/openclaw/index.ts:194）shellTokenize使用indexOf('"')查找引号，没有跳过\"。真实rtk 0.43.0对grep "foo\"bar" src/返回 exit 3 和rtk grep "foo\"bar" src/；当前解析返回null，导致裸rtk原样进入 trimmed-PATH 环境并以 127 失败。请正确处理转义引号，并用生产 helper 增加回归测试。 -
P1 — Hermes copy 安装在 shared hooks 版本错位时无法导入（
src/tokenless/adapters/tokenless/hermes/__init__.py:116、:155）候选目录只验证
hook_utils.py存在便停止搜索，随后无条件导入新私有符号_anchor_rtk_prefix。如果系统/XDG/user 路径中有受信任但较旧的 shared hooks，整个插件会抛出ImportError。本地tests/test_hermes_plugin_import.py在该场景下为 9 passed / 2 failed。请验证候选 API/版本并继续搜索兼容候选，或提供不依赖跨版本私有符号的回退，并补版本错位测试。
另有两项应一并处理：
src/tokenless/adapters/tokenless/openclaw/index.ts:223对包含单引号的 RTK 路径生成无效 shell quoting。src/tokenless/tests/test_openclaw_anchor.mjs:15复制而非导入生产 helper，且未接入make test/ tokenless CI，因此绿色 CI 无法防止生产实现漂移。
验证结果：test_rewrite_hook.py 11 passed；Node anchor tests 14 passed（但为复制实现）；Hermes import tests 9 passed / 2 failed。GitHub 当前适用 checks 全部通过，但不能覆盖以上阻塞问题。
|
已按 review 意见修复所有阻塞项，推送至 1eba8d1。逐项说明： P1 — OpenClaw 转义双引号 ✅ P1 — Hermes copy 安装版本错位 ✅ 单引号 RTK 路径 ✅ 测试复制 + CI 接入 ✅ 测试结果
|
ikunkun-sys
left a comment
ikunkun-sys
left a comment
There was a problem hiding this comment.
复审结论：仍有阻塞问题，暂不可合并。
已确认这轮修复覆盖了双引号字符串内的转义引号、RTK 路径中的单引号、生产 helper 复用，以及旧版 hook_utils 不再导致 Hermes 插件直接导入失败。不过新实现仍存在以下问题：
-
[P1] Hermes 的“降级模式”仍会执行其声称已禁用的压缩、TOON 和 env-check。
src/tokenless/adapters/tokenless/hermes/__init__.py:217-282在找不到兼容hook_utils时安装了不完整的本地替代实现，但on_pre_tool_call（594-605）、on_transform_tool_result（631-665）和register（703-713）都没有按_HOOK_UTILS_AVAILABLE做隔离。实际强制进入该分支后，_HOOK_UTILS_AVAILABLE == False，但 transform 仍调用 tokenless 并能返回截断后的内容；这与 720-724 行“response compression and TOON encoding disabled”的告警相反。固定阈值(65536, 128, 8)和不完整的 skip/tool 分类会在版本错配场景中静默改变本应保真的工具输出。建议在降级模式下只保留具有完整本地实现的 RTK rewrite，明确跳过 tokenless 的三个功能；或者让所有 fallback 与共享实现完全同语义，并补充断言这些 hook 在降级模式下不会被调用/不会改写结果的测试。 -
[P1] OpenClaw tokenizer 仍把引号外的反斜杠转义误判为未闭合引号。
src/tokenless/adapters/tokenless/openclaw/anchor-helpers.ts:33-58只在双引号内部跳过\\后的字符。有效 shell 命令rtk grep foo\\\"bar src/会得到shellTokenize(...) === null，因此anchorRtkPrefix原样返回裸rtk；当 RTK 不在 PATH、仅通过已解析绝对路径可用时，Hermes/OpenClaw 最终建议的命令仍会以 127 失败。RTK rewrite 会保留被 rewrite 命令的原始参数，所以这类参数能够进入这里。请也在非单引号上下文处理反斜杠转义，并为该输入补回归测试。 -
[P2] 新增的生产 helper 测试尚未真正接入 CI，且
make test-integration在干净检出上也不能独立运行。 测试导入adapters/tokenless/openclaw/dist/anchor-helpers.js，但src/tokenless/Makefile:186-194的test-integration不依赖build-openclaw-plugin；干净检出时dist不存在，直接运行会报ERR_MODULE_NOT_FOUND。同时.github/workflows/ci.yaml:811-815仍只手动执行两个 Python 测试，没有运行该 Node 测试。请让测试目标先构建 OpenClaw 插件，并在 tokenless CI job 中调用该目标或显式执行 build +node --test。
验证结果：手动执行 make build-openclaw-plugin 后，OpenClaw Node 测试 19/19、Hermes 测试 13/13、rewrite hook 测试 11/11、make test-integration 全部通过；当前 GitHub checks 也全部通过。但上述场景未被现有测试/CI 覆盖。
|
已按第二轮 review 修复所有阻塞项，推送至 e277db8。逐项说明： P1 — Hermes 降级模式隔离 ✅ P1 — 引号外反斜杠转义 ✅ P2 — CI 接入 Node 测试 ✅
测试结果
|
kongche-jbw
left a comment
kongche-jbw
left a comment
There was a problem hiding this comment.
Review baseline: 5f1c665a0289ee0315ca1c2615f38916af5b790d...e277db8279af25b2cf212c75e226d7dce467ae0c
[P1] 降级模式会绕过 rtk 最低版本守卫
src/tokenless/adapters/tokenless/hermes/__init__.py:278 的本地 _parse_version
使用 re.match，但 rtk --version 的输出形如 rtk 0.34.0。当 shared
hook_utils 缺失或版本不兼容时，该分支会把版本解析为 None，随后仍用低于
0.35.0 的 rtk 生成并建议重写命令。这绕过了 _MIN_RTK_VERSION，会让 Hermes
在降级场景执行明确不受支持的 rewrite 协议。
Possible direction: 与共享实现一致地搜索版本号，并增加降级模式下模拟
rtk 0.34.0 的 _try_rewrite 回归测试，断言不返回 block directive。
ikunkun-sys
left a comment
ikunkun-sys
left a comment
There was a problem hiding this comment.
复审结论：上一轮提出的 Hermes 功能隔离、OpenClaw 引号外转义和 Node CI 接入均已修复，但当前 head e277db8279af25b2cf212c75e226d7dce467ae0c 仍有阻塞问题，暂不可合并。
-
[P1] Hermes 降级模式会绕过 RTK 最低版本保护（
src/tokenless/adapters/tokenless/hermes/__init__.py:278-282、:514-522）本地 fallback 使用
re.match，而 RTK 的--version输出带程序名前缀，例如rtk 0.34.0。该字符串会被解析为None，随后_try_rewrite不会进入< 0.35.0的拒绝分支。我强制插件进入降级模式并模拟上述版本输出，实际得到parsed=None，且_try_rewrite(...)仍返回了blockdirective。这会在 shared hooks 版本错配时启用明确不受支持的 rewrite 协议。请让 fallback 与共享parse_version的搜索语义一致，并增加降级模式下rtk 0.34.0必须跳过 rewrite 的回归测试。 -
[P2] 声称保留的降级 RTK rewrite 未覆盖仓库支持的安装布局（
src/tokenless/adapters/tokenless/hermes/__init__.py:222-242、:346-351）降级分支把
_RTK_LOCAL_SHARE/_RTK_LOCAL_LIB置空，自己的resolve_binary也没有共享_known_binary_paths中的~/.local/libexec/anolisa/tokenless/rtk、~/.local/lib/anolisa/libexec/tokenless/rtk和/usr/local/libexec/anolisa/tokenless/rtk等路径。在 PATH 被裁剪、RTK 仅存在于受支持的~/.local/libexec/anolisa/tokenless/rtk时，实测_resolve_binary返回None。这正是本 PR 要兼容的运行环境，会让所谓“only RTK rewrite remains active”静默退化为 rewrite 也不可用。请让本地 resolver 覆盖与共享实现相同的支持路径，并针对 PATH 为空、RTK 仅位于 libexec 的场景测试。 -
[P2] 新增的 Hermes 降级测试无法证明 guard 生效（
src/tokenless/tests/test_hermes_plugin_import.py:247-273）transform 用例传入的 JSON 只有几十字符，未到
_MIN_RESPONSE_LEN=200；同时测试环境通常也解析不到已安装的 tokenless。因此即使删除本次新增的_HOOK_UTILS_AVAILABLEguards，两个 callback 仍会经_have(...) == False、短响应或 env-check 返回空而得到None，测试依然可能通过。请 mock_have为真，并把_env_check/ compression 路径设为调用即失败（或断言调用计数为零），让回归测试能区分有无 guard。
验证结果：本地 make test-integration 全部通过（Hermes 14/14、OpenClaw Node 20/20，其他 hook 测试也通过）；GitHub Test tokenless 在当前 head 成功，且其中 Run OpenClaw anchor helper tests (Node) 步骤已成功执行。现有绿色结果未覆盖上述降级版本/路径场景。
|
已按第三轮 review 修复所有问题，推送至 03040ce。逐条说明： P1 — Hermes 降级模式绕过 RTK 最低版本保护 ✅ 采纳并修复
P2 — 降级 RTK rewrite 未覆盖 libexec 安装布局 ✅ 采纳并修复
P2 — 降级测试无法证明 guard 生效 ✅ 采纳并修复
测试结果
|
ikunkun-sys
left a comment
ikunkun-sys
left a comment
There was a problem hiding this comment.
复审结论：上一轮的版本解析 P1 已修复，但当前 head 03040ced0580d7e35606b6f3047f8c1fb08ed96c 仍不满足合并条件。
-
[P2，仓库规则要求合并前处理] 同一 PR 内引入的问题仍以独立 fix commits 保留。 当前分支在初始提交
d2f2c322后保留了1eba8d19、e277db82、03040ced三个用于修复本 PR 自身问题的独立提交。根目录AGENTS.md:356-369明确要求这类修改使用fixup并rebase --autosquash，且禁止保留 standalone fix commit。当前历史需要在合并前按该规则整理。 -
[P2] 降级 resolver 仍未覆盖其声称同步的 legacy 安装路径。
src/tokenless/adapters/tokenless/hermes/__init__.py:242-260增加了新的 bin/libexec 布局，但遗漏了共享实现明确要求暂时保留的~/.local/share/anolisa/tokenless/<name>和~/.local/lib/anolisa/tokenless/<name>（common/hooks/hook_utils.py:20-26、:103-113）。这尤其影响“新插件 + 旧 shared hooks/旧安装”的降级场景：在 PATH 为空、RTK 仅位于 legacy share 路径时，我的探针仍得到_resolve_binary(...) == None，RTK rewrite 会被静默禁用。 -
[P2] 更新后的 guard 测试仍不能区分 guard 是否存在。
src/tokenless/tests/test_hermes_plugin_import.py:247-285只把顶层_havemock 为True；guard 被绕过后，_env_check、_compress_response和_encode_toon会再次调用真实_resolve_binary，在未安装 tokenless 的测试环境中仍返回None。我将_HOOK_UTILS_AVAILABLE改为True模拟删除 guard，两个 callback 依然都返回None，因此当前测试仍会误报通过。需要让下游调用在被触发时可观察（例如计数或立即失败），才能证明降级分支确实在 guard 处停止。 -
[P2] resolver 测试会写入真实用户主目录。
src/tokenless/tests/test_hermes_plugin_import.py:336-353在~/.local/libexec/anolisa/tokenless/下创建可执行文件，只删除文件、不清理所建目录。该测试会污染开发机，并在只读或受限 HOME 环境中失败；测试路径应完全留在self.tmp，通过 mock home/path resolution 来覆盖已知布局。
验证结果：本地 make test-integration 全部通过（Hermes 16/16、OpenClaw Node 20/20，其他 hook 测试也通过），git diff --check 通过；当前 GitHub checks 全部通过，Test tokenless 中的 Node 步骤已成功执行。上述问题属于现有绿色测试未能覆盖的兼容性、测试隔离和仓库历史规则缺口。
Both adapters returned rtk's rewrite output verbatim, so bare `rtk` tokens in the result failed with exit 127 in agent runtimes whose PATH lacked the rtk location — the same failure mode fixed for the shared hook in PR alibaba#1975. - Move _anchor_rtk_prefix/_is_env_assignment/_SEGMENT_OPS into hook_utils.py so rewrite_hook.py and hermes/__init__.py share one implementation; rewrite_hook.py now imports rather than defines them. - hermes/__init__.py _try_rewrite: call _anchor_rtk_prefix on the rtk output before building the block directive. - openclaw/index.ts: port the anchor logic (shellTokenize + anchorRtkPrefix + isEnvAssignment + SEGMENT_OPS) with posix=False semantics — quoted strings, globs, fd redirections, and command substitutions are preserved verbatim; call anchorRtkPrefix in tryRtkRewrite before returning the result. - Add tests/test_openclaw_anchor.mjs covering the full case matrix: simple rewrite, multiple &&-separated segments, sudo wrapper, env assignments, single &, quoted rtk pattern, unquoted glob, hash arg, fd merge (2>&1), fd redirect (2>/dev/null), command substitution $(date), spaced path quoting, no-rtk passthrough, unmatched quote. Co-authored-by: multica-agent <github@multica.ai>
03040ce to
d902c1a
Compare
|
已按第四轮 review 修复全部 4 项 P2，分支历史已按仓库规则整理，当前 head 为单个 commit d902c1a。逐条说明： P2-1 — 同一 PR 内的 fix commits 未合并 ✅ 已整理
P2-2 — 降级 resolver 遗漏 legacy 安装路径 ✅ 已修复
已按您的探针场景验证：新插件 + 旧 shared hooks（降级模式）+ PATH 为空 + RTK 仅位于 legacy share 路径时， P2-3 — guard 测试无法区分 guard 是否存在 ✅ 已重写
P2-4 — resolver 测试写入真实主目录 ✅ 已重写 原
测试后已验证真实 测试结果（head d902c1a）
请重新 review，谢谢！ |
3 participants
Description
Follow-up to #1975. That PR fixed the bare
rtkprefix in the sharedcommon/hooks/rewrite_hook.py, but two adapters shipped independent rewrite paths with the same latent defect:adapters/tokenless/hermes/__init__.py,_try_rewrite): returnedproc.stdout.strip()verbatim without anchoring thertkprefix.adapters/tokenless/openclaw/index.ts,tryRtkRewrite): returnedresult.stdout?.trim()verbatim without anchoring.In an agent runtime whose tool shell PATH lacks the rtk location, every rewritten command failed with exit 127 — the same failure mode observed in Qoder IDE sessions.
Changes
common/hooks/hook_utils.py: Extract_anchor_rtk_prefix,_is_env_assignment, and_SEGMENT_OPSfromrewrite_hook.pyintohook_utils.pyas the single shared implementation.common/hooks/rewrite_hook.py: Import_anchor_rtk_prefixfromhook_utilsinstead of defining it locally. Remove the now-duplicate definitions.hermes/__init__.py: Import_anchor_rtk_prefixfromhook_utilsand call it in_try_rewritebefore building the block directive.openclaw/index.ts: Port the anchor logic (shellTokenize,anchorRtkPrefix,isEnvAssignment,SEGMENT_OPS) with posix=False semantics — quoted strings, globs, fd redirections, and command substitutions are preserved verbatim. CallanchorRtkPrefixintryRtkRewritebefore returning the result.tests/test_openclaw_anchor.mjs: New Node.js test file covering the full case matrix matchingtest_rewrite_hook.py: simple rewrite, multiple&&-separated segments,sudowrapper, env assignments, single&, quoted rtk pattern, unquoted glob, hash argument, fd merge (2>&1), fd redirect (2>/dev/null), command substitution$(date), spaced path quoting, no-rtk passthrough, unmatched-quote passthrough.Testing
Closes #2123