Repository navigation
fix(protected): match tool names case-insensitively across clients - #484
Conversation
📦 Built Package ArtifactBranch: Option A — Install from npm PR tag (recommended)In your adapter project: npm install acp-kernel@pr-484Each push to this PR publishes a new version under the Option B — Download artifact
tar xzf acp-kernel-pr484.tgz
npm install ./packageThis comment is automatically updated on each push. |
[bot] 🏷 Review #1(agent)—— 结论:方向正确、验证充分,可合并(待第 2 个独立 agent review + 人工合并)。 查重open/closed issue 与 PR 均检索(关键词 case-insensitive / casing / tool name matching):无同类在先工作,本 PR 是该根因的首个修复,来源标记(billion-context#1725)合规。 ① 原 issue 方向审billion-context#1725 的根因分析(floor 2/6)定位准确:全部工具名旋钮共用单一比较器 ② 回归审查(既有行为清点)触碰路径:
未发现无意回归。 ③ 代码审查最小 diff(函数体 4 行 + 注释),实现正确;注释写明了动机、单调性论证与来源 issue,符合「注释只用于必要处」规范。无 ④ 测试有效性审查5 条新测试有牙:正反断言齐全( ⑤ diff 整洁度2 文件,+96/-4,每一行都与本 PR 目的相关;无版本行、无锁文件、无空行重排。干净。 CI 与合并状态4 项检查全绿(test node24 / test node22 / pr-validation / build-artifact,commit 4689f50); 契约提示本 PR 触及 §7.3 契约 #6(protected-tool filtering / soft-recent-zone 语义)——human-gated,按 §6 需 ≥2 个独立 agent review,本条为第 1 个(通过);合并仅限人工。 Review 中发现的新问题(已立项)内核里还有一批针对 ACP 自身注入的协议工具名( 一句话中文摘要:修复了工具名匹配大小写敏感导致跨客户端(read vs Read)六个配置旋钮静默失效的问题(统一比较器两侧小写化,单调增宽、零默认值变更);我本地独立复验了 5 条回归测试旧码必挂、新码全过,CI 四项全绿,diff 干净——从代码与行为看可以合并,按规矩还需第 2 个独立 agent review 后由你人工合并。 |
|
Review #2(independent agent)—— 结论:同意 Review #1,可合并(人工)。以下为独立复验,非复读。 独立复验(全部亲手跑过)
正确性论证(自行推演,与 PR 一致)
范围纪律ACP 自身注入工具名( 状态提示
契约触及 §7.3 契约 #6(protected-tool filtering / soft-recent-zone)——human-gated。本条为第 2 个独立 agent review(通过)。合并仅限人工。 一句话:修复正确、单调、零默认值变更;测试经旧码互换取证;调用点无遗漏;范围边界干净 —— 从代码与行为角度可合,等你按流程人工合并。 |
Client hosts disagree on tool-name casing (opencode sends read, Claude/ZCode send Read). matchToolPattern compared with === / startsWith (case-sensitive), so every knob built on it -- protectedTools, protectedLatestTools, recent-zone exclusion, preserveRecentTools, absorb/crush excludeTools -- silently stopped matching on differently-cased clients. Fix: lowercase both sides before comparing. Monotonic change: only adds matches, never removes one that held under exact matching, so no prior protection/exclusion is lost. The lone theoretical edge (two distinct tools differing only by case) over-matches in the conservative direction. Regression tests fail on the old code and pass here. Fixes billion-context#1725. Pre-flight: typecheck clean; full suite 1207/1207; build ok; prettier clean.
4689f50 to
2cc21fa
Compare
[bot] 🏷 收到 Review #2 —— ≥2 独立 agent review 的要求现已满足(两条均通过),且「behind」提示我已核实并处理完毕,不是留给你合并前手动做的: behind 状态:已核实 + 已 rebase
当前状态(刚查)
剩余步骤(均为人工/后续流程)
一句话:两个独立 review 均通过、behind 已 rebase 修复且新 head CI 四项全绿、mergeable_state clean —— 万事俱备,只等你人工合并。 |
Fixes the case-sensitivity defect reported in billion-context#1725.
Problem
matchToolPatterncompared tool names with===(exact) andstartsWith(glob prefix) — both case-sensitive. Client hosts disagree on tool-name casing: opencode sendsread, Claude/ZCode sendRead. Because every knob built on this matcher shares the single comparator, a user's pattern silently stopped matching on any client whose casing differed from the pattern:NEVER_PRESERVE_RECENT_TOOLSis lowercase[decompress, search_context, read, bash]) never fired forRead/Bash, so those large results stayed pinned in the protected recent zone instead of being immediately compressible;preserveRecentTools:["read"]was a silent no-op on such clients (it subtracted an already-non-matching entry);protectedTools,protectedLatestTools, and absorb/crushexcludeTools.Fix
Lowercase both sides before comparing in
matchToolPattern. One change fixes all six knobs uniformly across clients.Monotonic / no regression: case-insensitivity only adds matches — any pair that matched under exact comparison still matches (
a===b ⇒ a.toLowerCase()===b.toLowerCase()). No prior protection or exclusion is lost. The lone theoretical edge (a client registering two distinct tools differing only by case) over-matches in the conservative direction (protect more / crush less), not destructively. Wire tool names are untouched (internal comparison only), so message-id/ref numbering, rendered tag format, token snapshots, nudge cadence, and prefix-cache stability are all unaffected.Contract note
This touches the protected-tool-filtering / soft-recent-zone semantics (§7.3 kernel contract) — flagged for human review. Per repo rules this needs ≥2 agent reviews before merge; merge is human-only. billion-context will bump its
acp-kernelpin in a follow-up once this version is published to npm.Pre-flight
npm run typecheck— cleannpm test— 1207/1207 passnpm run build— oknpm run format:check— cleantests/protect-recent.test.ts; verified they fail on the pre-fix code and pass here.