fix(tokenless): Cosh-NG compatibility for response compression and rewrite hooks - #2238
fix(tokenless): Cosh-NG compatibility for response compression and rewrite hooks#2238Forrest-ly wants to merge 2 commits into
Conversation
![qoderai[bot]](https://avatars.githubusercontent.com/in/1396158?s=60&v=4){kind=small}
There was a problem hiding this comment.
- Cosh-NG 小响应路径下 env_attribution 行为与大响应不完全一致，可能丢失错误归因提示。
- rewrite_hook 在 Cosh-NG 场景对 PATH 中 tokenless/rtk 依赖较强，未完全复用现有二进制解析策略。
- COSH_NG_VERSION 环境变量解析对空白值场景未完全覆盖，容易与未配置状态混淆。
🤖 Generated by Qoder • View workflow run
| # env attribution for error cases (small size doesn't mean the | ||
| # error classification is unimportant to the agent). | ||
| if len(tool_response) < _MIN_RESPONSE_CHARS: |
There was a problem hiding this comment.
[P2] Cosh-NG 小响应时 env_attribution 行为差异
compress_response_hook.py:322-333：对于小响应且无 env_attribution，copilot-shell 分支直接 skip，而 Cosh-NG 分支也 skip。当前实现无法在这类错误场景下给 Cosh-NG 提供环境归因提示，行为与较大响应时不一致。建议将 env_attribution-only 输出路径抽取复用，确保在 Cosh-NG 下即使压缩未发生、小响应也能统一返回 additionalContext。
🤖 Generated by Qoder • Fix in Qoder
There was a problem hiding this comment.
The cited lines 322–333 are in the Copilot-Shell branch (parsing tool_response_raw as string), not the Cosh-NG path. The actual small-response + env_attribution logic is at lines 363–374, where Cosh-NG already emits additionalContext via build_cosh_ng_post_tool_output(replacement=None, additional_context=env_attribution) when env_attribution is present. The existing test test_cosh_ng_env_attribution_only covers this case and passes. No change needed.
| env = os.environ.copy() | ||
| env["HOME"] = str(self.home) | ||
| env["PATH"] = str(self.mock_rtk.parent) + ":" + env.get("PATH", "") |
There was a problem hiding this comment.
[P2] Cosh-NG 下 rewrite_hook PATH 依赖未覆盖
rewrite_hook.py:478-482：Cosh-NG 集成测试通过 PATH 注入 mock rtk/tokenless，但生产环境下若 PATH 中 tokenless/rtk 不可用，当前 Cosh-NG 分支仅依赖 PATH，不会退回现有 TOKENLESS* 解析逻辑。建议在 Cosh-NG 分支沿用 resolve_binary 结果（或显式校验 PATH）以避免运行时找不到可执行导致 rewrite 失效。
🤖 Generated by Qoder • Fix in Qoder
There was a problem hiding this comment.
The rewrite hook already uses resolve_binary() with fallback paths for both rtk (lines 142–144) and tokenless (lines 165–169). resolve_binary checks FHS fallback paths (/usr/libexec/anolisa/tokenless/rtk, etc.) before falling back to PATH. Additionally, _anchor_rtk_prefix (line 231) replaces bare rtk tokens with the resolved absolute path, so the rewritten command does not depend on PATH at execution time either. No change needed.
| """Detect Cosh-NG version from ``COSH_NG_VERSION`` environment variable. | ||
|
|
||
| Returns a (major, minor, patch) tuple or ``None`` if not set or unparseable. | ||
| """ | ||
| version_str = os.environ.get("COSH_NG_VERSION", "") |
There was a problem hiding this comment.
[P2] COSH_NG_VERSION 环境变量解析缺少空白值覆盖
hook_utils.py:670-687：detect_cosh_ng_version 对空字符串直接返回 None，而 compress_response_hook 中仅依据 cosh_ng_supports_replacement() 结果 fail-open。若 COSH_NG_VERSION 设置为仅空格/换行，当前实现仍视为“未配置”，行为与完全未设置相同。建议在 detect_cosh_ng_version 中 strip 后再判断空值，确保这类配置错误被视作“未配置”并便于后续诊断。
🤖 Generated by Qoder • Fix in Qoder
There was a problem hiding this comment.
Fixed in the latest commit. detect_cosh_ng_version now strips whitespace before checking emptiness: version_str = os.environ.get("COSH_NG_VERSION", "").strip(). Added a test case test_detect_version_whitespace_only to verify this behavior.
![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: 359ee65844
ℹ️ 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".
| """ | ||
| specific: dict = {"hookEventName": "PostToolUse"} | ||
| if replacement is not None: | ||
| specific["replacement"] = replacement |
There was a problem hiding this comment.
When this hook runs under current Cosh-NG, the runtime only extracts replacements from hookSpecificOutput.updated_tool_response or updatedToolResponse in pick_updated_tool_response and then applies that value during aggregate_post_tool_use; it never reads a replacement field. Emitting replacement here is therefore ignored, so large Cosh-NG PostToolUse responses pass through uncompressed even though the hook reports a replacement; emit updatedToolResponse/updated_tool_response or update the runtime contract consistently.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — this was a real bug. build_cosh_ng_post_tool_output was emitting replacement but Cosh-NG's pick_updated_tool_response reads updated_tool_response / updatedToolResponse. Fixed in the latest commit: the function now emits updated_tool_response (snake_case, matching the Cosh-NG convention). All related tests updated accordingly.
359ee65 to
4a1029c
Compare
4a1029c to
38fb244
Compare
The build_cosh_ng_post_tool_output function emitted a "replacement" key that Cosh-NG's pick_updated_tool_response never reads — it expects "updated_tool_response" / "updatedToolResponse". This caused compressed responses to be silently ignored under Cosh-NG. Also strip whitespace from COSH_NG_VERSION before parsing to handle misconfigured whitespace-only values. Assisted-by: Qoder:1.0.45 Signed-off-by: Forrest-ly <forrest.ly@example.com>
1 participant
Summary
This PR adds Cosh-NG runtime compatibility to the tokenless response compression and rewrite hooks:
tool_response({llmContent, returnDisplay}).llmContent; never includereturnDisplay.replacementfield in PostToolUse hook output for Cosh-NG.tool_inputandupdatedInputfields in PreToolUse hook output for cross-runtime compatibility.cosh-ngagent ID for stats attribution when running under Cosh-NG.Test Plan
src/tokenless/tests/test_cosh_ng_compat.pycovering runtime detection,llmContentextraction, output format building, version detection, replacement support, agent ID attribution, and hook integration.46 passed, 38 skippedon the available Python 3.8 interpreter (integration tests forcompress_response_hookare skipped on Python < 3.9, matching existing test behavior).