Repository navigation
[PR] fix: register manifest-advertised conditional tools in the opencode V2 lane (#2443) - #2445
ranxianglei wants to merge 1 commit into
Conversation
📦 Built Package ArtifactBranch: Option A — Install this exact build (recommended)npm install -g billion-context@0.0.2768-pr.2445.be74b8dThis exact version is immutable and reproducible. The rolling Option B — Download artifact
tar xzf billion-context-pr2445.tgz
npm install -g packageInstall instructions are refreshed on each push. |
[bot] 🏷 Review 完成 — 结论:拦下(不建议合并)。方向正确,但核心机制在真实 proxy wire 上不工作:合并后真机上 验证过程(每步实测,非推断)
对 PR 自提问题(transform 累积语义)的回答源码级核验(@opencode/cli v2.0.3 tag = 本地二进制;v2.0.26 最新 tag): 修法建议(root-cause 层,最小)
顺带发现(非阻塞)
结构化八问
判定:拦下。 待你批准后我直接在 PR 分支补 commit(fetchManifest 解包 + fixture 对齐 + 真机探测重验),然后继续 review 流程。 |
d1ce3a6 to
5627a3a
Compare
What / why
The opencode V2 lane (
src/agent/opencode-v2.ts) registered only its static bundled list — the 5 core ACP tools +absorb— and never consulted the proxy's plugin manifest. That list has noacp_retrieve. So in plugin mode with CCR enabled, an[acp-stored]placeholder advertisedacp_retrieve("mNNNNN")as its restore path, but the tool was not in the model's catalog — an unreachable pointer.retrieveCallsstayed at 0 whilestoredBytesgrew; the only workaround wasccr.enabled:false.The dsh and pi lanes already consume
GET /__bili/plugin/manifestand register its tool surface (#1192); the opencode V2 lane was the outlier that didn't. This makes it do the same.How
On the first routed request — when
state.proxyBasebecomes known (native bootstrap / per-request routing discover it lazily, so setup-time isn't reliable) — fetch the manifest once and register whatever it advertises beyond the static set:TOOL_DELTA_RETRY_MS = 15000, pi/dsh parity);In practice the delta is
acp_retrievewhile CCR is armed andacp_rulewhile rules are on. The executor is factored intomakeToolExecutor(toolName)shared by both the static and delta registrations (behavior unchanged).Behavior change (disclosed)
acp_retrieve/acp_rule; CCR placeholders pointed at an unreachable tool.Config surface
None. No new env var, config field, or CLI flag. Uses the existing
ccr.*/ rule config the manifest already reflects.Tests
New regression test (
tests/opencode-v2.test.ts): a fake proxy serving a manifest that advertisesacp_retrieve→ asserts the base set is intact before traffic,acp_retrieveappears exactly once after the first routed request with the faithful production schema/options, and its executor forwards to the live dispatcher. Existing suite (incl. the inert/kill-switch/no-proxy paths that now exercise the degrade branch) stays green. Targeted run: 32/32; typecheck clean on the rebased head.Rebased onto current master (resolves a clean union with #2399's fork-adoption seam in the same hook — both coexist).
Needs human review (per §7.4 — load-bearing tool-surface change)
One assumption worth confirming against real OpenCode 2.0.x: the fix issues a second
ctx.tool.transform(...)call (after the synchronous setup-time one) to add the delta. This relies on the transform editor accumulating / upserting by tool name across calls. Multi-plugin coexistence implies accumulation (a replace would stomp other plugins' tools), and a name-keyed catalog upserts rather than duplicating — so this should hold — but it is not pinned by any in-repo fixture. If real 2.0.x instead replaces the plugin's tool set on a second call, the delta registration would need to re-add the base set too (or move to a reload-based refresh). Please sanity-check on a live host before merge.Closes #2443