Skip to content

fix: zero-injection session identity — outbound tool_use witness routing (#1685) - #1692

Merged
ranxianglei merged 6 commits into
masterfrom
2026-09-29_zero-id-ring-routing
Sep 30, 2026
Merged

fix: zero-injection session identity — outbound tool_use witness routing (#1685)#1692
ranxianglei merged 6 commits into
masterfrom
2026-09-29_zero-id-ring-routing

Conversation

@ranxianglei

@ranxianglei ranxianglei commented Sep 29, 2026

Copy link
Copy Markdown
Owner

Fixes #1685
Fixes #1611
Supersedes #1614 and #1620 (please close those when this lands).

What

The model no longer sees ANY conversation id, in any form. Every model-visible carrier is removed:

Session identity moves fully to machine channels; the MCP tool leg routes id-less POSTs by a new outbound tool_use witness ring: the proxy is the only path model output takes, so every complete bili tool call it streams out is recorded as sha256(normalizeToolName(name) + NUL + stableStringify(args minus conversation_id)) per session (ring cap 32, TTL 10min, hash index evicted in sync).

Routing ladder (handlePluginTool)

  1. unique witness → wins, even over a differing body id (claude-code subagent correctness: shim env binds the main session, the subagent's tool_use streamed in the sub session — claudeSubagentSplit)
  2. collision, no id → loud 400 refuse-to-guess
  3. no witness → body conversationId (extensions / legacy — unchanged behavior, resolveConversation incl. canonical-id fallback)
  4. neither → single-active arbitration inside the 10min window, else loud 400

Recording points (observe-only, bytes forwarded verbatim per #1039)

All five plugin response lanes: openai chat fragments, anthropic input_json_delta, google whole-part functionCall, responses output_item.added+function_call_arguments.done, non-stream JSON. Witness keys strip a legacy conversation_id arg echo on both record and lookup sides, so old-model echoes never fork the key.

Compat

  • src/mcp.ts shim: no longer hard-fails id-less tool calls — forwards them; per-call override and keepForSearch preserved
  • delete args.conversation_id legacy strip kept in handlePluginTool (old models may still echo the param)
  • kernel execution surface untouched (wire-mode conversation_id in executeSearchContextTarget still works)

Why this shape

Evidence

  • npm run typecheck ✅ npm run build ✅ (9.43MB ESM)
  • New tests: tests/tool-ring.test.ts (18 cases: hash normalization/parity, capacity eviction, collision sets, full routing ladder incl. witness>body, arb, loud-400s) and tests/tool-witness-pipes.test.ts (6 cases: all five lanes record; legacy echo parity) — all green
  • Flipped to the new contract: tests/issue760-percall-conversation.test.ts 8/8, tests/plugin-protocol.test.ts + tests/search-context-cross-session.test.ts 24/24, tests/wire-contract.test.ts 11/11 (goldens regenerated)
  • Full-suite sweep (356 files, parallel): only the 5 pre-existing local failures (omp-native / opencode-native / opencode-v2 / pi-native / plugin-agent) — verified identical on local master via git stash; dsh-native also hangs on local master (CI green = environmental). Zero regressions from this change.

version untouched; no wire bytes altered on the request side.

This was referenced Sep 29, 2026
Closed
Closed
Closed
@github-actions

github-actions Bot commented Sep 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

📦 Built Package Artifact

Branch: 2026-09-29_zero-id-ring-routing (86e1811)

Option A — Install this exact build (recommended)

npm install -g billion-context@0.0.1715-pr.1692.3e18253

This exact version is immutable and reproducible. The rolling pr tag tracks the newest test build across all PRs.

Option B — Download artifact

  1. Download the artifact from the Actions run
  2. Extract the tarball and install:
tar xzf billion-context-pr1692.tgz
npm install -g package

Install instructions are refreshed on each push.

@ranxianglei

ranxianglei commented Sep 29, 2026

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 Reviewing PR #1692 now — pulling the branch (2026-09-29_zero-id-ring-routing @ 71f3aa73), walking the diff against #1685/#1611, re-running checks, and checking CI. Full review report to follow in this thread.

@ranxianglei ranxianglei added the p1 label Sep 29, 2026
@ranxianglei

ranxianglei commented Sep 29, 2026

Copy link
Copy Markdown
Owner Author

Review follow-up in 5aeec47:

  • duplicate-hash index safety: dropFromIndexIfAbsent — a hash surviving in any ring slot keeps its hashIndex membership when an OLDER duplicate copy is evicted (capacity path) or swept (TTL path). Previously the old copy's removal dropped the index for the newer copy → premature witness miss (safe direction — falls back to body id / arbitration — but wrong, and it defeats the dedup design).
  • TTL pinned by test (was an acceptance item without coverage): fake-clock seam _setToolRingClockForTest; match at exactly 10min, miss at 10min+1ms, ring map freed on expiry.
  • negative tests: empty-name witness is a no-op (unit + openai pipe lane).

Local: 30/30 green in tool-ring + tool-witness-pipes, typecheck + build green.

ranxianglei pushed a commit that referenced this pull request Sep 29, 2026
ework-agent
fix: tool-ring witnessHash rejects non-object argument JSON without t…
785dfde 
…hrowing (#1692 review)

The legacy-conversation_id strip ran '"conversation_id" in obj' on the
parsed argument JSON. A degenerate but reachable payload (arguments of
null/scalar from a misbehaving upstream, e.g. the google whole-part lane
stringifying a null args object) parses to a non-object and made the
'in' operator throw TypeError inside recordToolWitness — on the hot SSE
stream path (settleWitnesses runs at stream end AND in the catch), where
an observe-only recorder must never perturb the response. Now such
payloads return an empty hash = not recorded, matching the existing
unparseable-JSON contract. Regression test added.
ranxianglei pushed a commit that referenced this pull request Sep 29, 2026
ework-agent
chore: drop dead ensureCanonicalId import left by zero-injection chan…
c39df22 
…ge (#1692 review)

The zero-injection commit removed every call site in server.ts but left
the token in the session.js import list. Typecheck passes only because
noUnusedLocals is off; removing it keeps the import honest.
@ranxianglei

ranxianglei commented Sep 29, 2026

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 Independent review complete — mergeable. Two small drive-by fixes added as commits 785dfdef + c39df222; CI on head c39df222 is 13/13 green. Full notes below.

① Original issues (#1685 / #1611) — direction verified

  • conversation-id line in the static system prompt breaks the upstream prefix cache on fork/resume/session-switch (cached≈0 despite byte-identical history) #1611 root cause confirmed: the id line lived in the STATIC system prompt, so fork/--continue changed system bytes → provider prefix-cache LCP broke inside system → cached=0 on a byte-identical history. Zero-injection removes the mechanism at its source (no per-conversation bytes anywhere model-visible), not a symptom patch. Correct shape.
  • Identity moving fully to machine channels + witness routing closes the trust-chain hole (a hallucinated/echoed id can no longer misroute a tool call). The routing ladder in handlePluginTool was traced case-by-case: unique witness > body id; collision+no-id → loud 400; collision+body id → body id; no witness → body id / single-active arbitration / loud 400. All six paths behave as specified, including the claude-code subagent case (claudeSubagentSplit) where the witness must beat the shim's main-session env binding.
  • resolveConversation("") edge (dead session via arb/witness) falls through to the downstream if (!session) guard → loud 400. Safe.

② Regression audit — existing behaviors touched, verdicts

Touched behavior Verdict
Wire system note [Your bili conversation id: …] (all 5 server.ts sites) Intended removal — disclosed in PR body; this IS the fix. Request side byte-stable across forks by design.
MCP manifest conversation_id params (#760) Intended removal — goldens show pure property deletions only, regenerated per the wire-contract ledger rule.
search_context cross-conversation param (#841) Intended removal — same.
MCP shim id-less tool calls: hard-fail → forward Behavior change, disclosed. keepForSearch and per-call override semantics preserved (verified in src/mcp.ts: keepForSearch = search_context && perCall && env conversationId bound; routeOverride logic intact).
Id-less POSTs with no witness but multiple fresh conversations New explicit proxy-side refusal (loud 400 with count) where previously the shim failed first — same fail direction, better message.
Kernel execution surface (executeSearchContextTarget wire-mode conversation_id) Untouched — verified still works.
Observe-only invariant (#1039): forwarded bytes verbatim Holds on all five lanes — EXCEPT one defect found below (threw, didn't alter bytes).
High-risk patterns (fail-fast→swallow, threshold/timing drift, silent config clobber) None found.

③ Code review — defects found & fixed directly on the branch

  1. 785dfdef — witnessHash threw on non-object argument JSON. The legacy strip ran "conversation_id" in obj on parsed args; a degenerate-but-reachable payload (arguments of null/scalar — e.g. the google whole-part lane stringifying a null args object, or a misbehaving upstream) parses to a non-object → TypeError: Cannot use 'in' operator to search for 'conversation_id' in null. This sits on the hot SSE path (settleWitnesses runs at stream end AND in the catch), where an observe-only recorder must never perturb response finalization. Fix: reject with empty hash (= not recorded), matching the existing unparseable-JSON contract. Regression test added covering "null", "42", '"str"', "true".
  2. c39df222 — dead import. ensureCanonicalId remained in the server.ts session.js import list after all call sites were removed (typecheck passes only because noUnusedLocals is off). Removed.

On your 5aeec47 follow-up: both fixes independently verified correct. I had reproduced both original defects locally before it landed (eviction index loss on mid-ring duplicates; TTL untested) — notably dropFromIndexIfAbsent covering both the capacity loop and sweep() is more complete than my local parallel fix, which only caught the capacity path; my stashed version was discarded as redundant. The fake-clock seam, boundary-pinned TTL tests (match AT 10min, miss at +1ms, ring freed), and nameless no-op tests are all sound.

④ Test validity

Real and biting — no tautologies found: routing-ladder tests drive actual handlePluginTool HTTP responses against live sessions; ring tests assert internal index state; pipe tests push real SSE chunks through pipePluginChatWithStrip/responses/json lanes. Gaps from the acceptance list are now closed: TTL pinned by test, duplicate-hash regression on both paths, nameless no-op (unit + pipe lane), plus my scalar-guard regression.

⑤ Diff cleanliness

Clean — every line maps to the PR's purpose; goldens are legitimate deletions; README en/zh kept in sync; version untouched; linear history, each commit labeled.

Verification evidence (head c39df222)

  • CI: 13/13 success (ubuntu 22/24 + windows 22/24 full matrix, e2e, hermetic registry, build-artifact, version-guard, dsh/opencode/pi/codex native lanes)
  • Local: typecheck ✅ · build ✅ (9.43MB ESM, dist-import-annotations OK) · tool-ring+tool-witness-pipes 31/31 ✅ · flipped suites (issue760 / plugin-protocol / search-context-cross-session / wire-contract ±golden) 48/48 ✅ · full suite 3257 pass / 2 fail / 12 skip — the 2 fails are wall-clock timing tests (fix-437-idle-timeout, issue1452-socket-hardening) that pass 15/15 in isolation = load flakes under my sandbox, not regressions (CI ran them green too)

Behavior-change flags (old → new, all intended/disclosed)

  1. Model-visible conversation id: present (wire note + MCP params) → absent everywhere (the fix itself).
  2. Id-less MCP tool call: shim hard-fail → forwarded, routed by witness/arbitration with loud 400 fallbacks.

Housekeeping: closing #1614/#1620 stays with you post-merge. Per the auto-merge gate this is wire/message-shape territory anyway — human merge.

中文摘要：独立审查通过，直接在 PR 分支补了两个小修——witnessHash 对非对象参数 JSON（null/标量）会在 SSE 流路径抛 TypeError，已加守卫并补回归测试；另清理了零注入改动遗留的死导入。与作者 5aeec47 的修复互不重叠且已逐条验证正确。CI 13/13 全绿、本地全量仅 2 个负载型 flake（隔离复跑通过），可以合并。

@ranxianglei

ranxianglei commented Sep 29, 2026

Copy link
Copy Markdown
Owner Author

更新了 master 再看下

@ranxianglei

ranxianglei commented Sep 29, 2026

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 已按更新后的 master 复查完毕（期间我的会话被平台重启了一次，补报进度）。

你已直接把 master merge 进 PR 分支（ca31ad27，parents = c39df222 + 75c1e75c v0.1.173）——我这边原本准备推一个线性 rebase，发现你的 merge 已覆盖且与我 rebase 后的树 逐字节一致（git diff 为空），所以没有再推送任何东西，以你的 merge 拓扑为准。结论：对新版 master 验证通过，仍可合并。

新 master 与本 PR 的交互面检查

验证证据（树 = ca31ad27）

  • CI：13/13 success（ubuntu 22/24 + windows 22/24 全矩阵、e2e、hermetic registry、build-artifact、version-guard、dsh/opencode/pi/codex native lanes）
  • 本地（rebase 后同树）：typecheck ✅ · build ✅（dist-import-annotations OK）· tool-ring + tool-witness-pipes + issue760 + plugin-protocol + search-context-cross-session + wire-contract ±golden 79/79 ✅ · 全量套件 3311 pass / 1 fail / 跳过若干 —— 唯一失败仍是那个负载型墙钟 flake（fix-437-idle-timeout「healthy stream longer than timeout is NOT aborted」），隔离复跑通过，CI 矩阵里也是绿的。顺带一提：master 上的 9b534a94（stall-guard budget margin）正是这类测试的加固方向。

当前分支状态

ca31ad27 = master(v0.1.173) ⊕ 4 个内容提交（原始实现 → 你的 duplicate-hash/TTL 修复 → 我的 witnessHash 非对象守卫 → 死导入清理）。diff 干净、version 未动（随 master 已是 0.1.173）。

中文摘要：已对照更新后的 master（v0.1.173，含端口分区/SSE keep-alive 等 47 个提交）重新审查并全量验证——新提交与 witness 路由零代码交集，CI 13/13 全绿、本地全量仅 1 个已知负载型 flake，PR 现在基于最新 master，可以合并。

@ranxianglei
ranxianglei force-pushed the 2026-09-29_zero-id-ring-routing branch from ca31ad2 to 586671f Compare September 30, 2026 08:08
ranxianglei pushed a commit that referenced this pull request Sep 30, 2026
ework-agent ework-agent
fix: tool-ring witnessHash rejects non-object argument JSON without t…
4075d74 
…hrowing (#1692 review)

The legacy-conversation_id strip ran '"conversation_id" in obj' on the
parsed argument JSON. A degenerate but reachable payload (arguments of
null/scalar from a misbehaving upstream, e.g. the google whole-part lane
stringifying a null args object) parses to a non-object and made the
'in' operator throw TypeError inside recordToolWitness — on the hot SSE
stream path (settleWitnesses runs at stream end AND in the catch), where
an observe-only recorder must never perturb the response. Now such
payloads return an empty hash = not recorded, matching the existing
unparseable-JSON contract. Regression test added.
ranxianglei pushed a commit that referenced this pull request Sep 30, 2026
ework-agent ework-agent
chore: drop dead ensureCanonicalId import left by zero-injection chan…
65b68dc 
…ge (#1692 review)

The zero-injection commit removed every call site in server.ts but left
the token in the session.js import list. Typecheck passes only because
noUnusedLocals is off; removing it keeps the import honest.
ranxianglei pushed a commit that referenced this pull request Sep 30, 2026
ework-agent
test: flip #1712 range-variant pin to the zero-injection contract (#1685
134027d 
)

decompress-empty-range.test.ts pinned 'search_context keeps
conversation_id' from master (post-#1713). Under #1692 zero-injection
the param is intentionally gone from every served array — the
rebase surfaced the stale pin; flipped to assert absence.
@ranxianglei ranxianglei mentioned this pull request Sep 30, 2026
Closed
ranxianglei and others added 6 commits September 30, 2026 19:14
@ranxianglei ework-agent
fix: zero-injection session identity — route id-less MCP tool calls b…
8417342 
…y outbound tool_use witness (#1685)

- remove every model-visible conversation id: the wire system note
  (withConversationIdNote), MCP manifest conversation_id params (#760),
  search_context cross-conversation description (#841)
- new src/tool-ring.ts: per-session ring of
  sha256(name + NUL + stableStringify(args minus conversation_id)) over
  outbound tool_use; capacity 32, TTL 10min, hash-index evicted in sync
- record witnesses on all five plugin response lanes (openai/anthropic/
  google chat SSE, responses SSE, non-stream JSON) — observe-only, bytes
  still forwarded verbatim (#1039)
- handlePluginTool routing ladder: unique witness wins (even over a
  differing body id — claude-code subagent correctness); collision with
  no id -> loud 400 refuse-to-guess; no witness -> body id path; neither
  -> single-active arbitration inside the TTL window, else loud 400
- mcp shim no longer hard-fails id-less tool calls: forwards them for
  proxy-side routing; per-call override + legacy strip kept for compat
- wire-contract goldens regenerated (conversation_id param gone)
- README zh/en kimi lane notes updated

Fixes #1685
Fixes #1611
@ranxianglei ework-agent
fix(#1685 review): duplicate-hash eviction/sweep must keep the witnes…
a6d7733 
…s index; pin TTL with a fake clock

- tool-ring.ts: dropFromIndexIfAbsent — index membership is only dropped when
  no surviving ring slot carries the hash. Previously evicting/sweeping the
  OLDER copy of a duplicated hash (same name+args witnessed twice, e.g. a
  retried compress) killed the index entry of the surviving newer copy →
  premature witness miss (safe direction — falls back to body id/arbitration —
  but wrong). Applies to both the capacity-evict loop and sweep().
- clock seam (_setToolRingClockForTest) + TTL tests: match AT the 10min
  boundary, miss 1ms past, ring freed on expiry
- regression tests for both duplicate-hash paths (capacity + sweep)
- negative tests: empty-name witness is a no-op (unit + pipe lane)
ework-agent
fix: tool-ring witnessHash rejects non-object argument JSON without t…
bc902c3 
…hrowing (#1692 review)

The legacy-conversation_id strip ran '"conversation_id" in obj' on the
parsed argument JSON. A degenerate but reachable payload (arguments of
null/scalar from a misbehaving upstream, e.g. the google whole-part lane
stringifying a null args object) parses to a non-object and made the
'in' operator throw TypeError inside recordToolWitness — on the hot SSE
stream path (settleWitnesses runs at stream end AND in the catch), where
an observe-only recorder must never perturb the response. Now such
payloads return an empty hash = not recorded, matching the existing
unparseable-JSON contract. Regression test added.
ework-agent
chore: drop dead ensureCanonicalId import left by zero-injection chan…
3fbcf67 
…ge (#1692 review)

The zero-injection commit removed every call site in server.ts but left
the token in the session.js import list. Typecheck passes only because
noUnusedLocals is off; removing it keeps the import honest.
ework-agent ework-agent
chore: regenerate wire-contract goldens after rebase (#1685)
c93503f 
Rebased onto master (post #1713 NO_RANGE manifest locals, post #1745):
goldens now reflect zero-injection manifests — every conversation_id
param block removed (routing param, search_context cross-session param,
one tool left with empty properties). Regenerated via
scripts/update-wire-contract-goldens.ts; manifest arrays keep the
NO_RANGE-aware locals instead of the pre-rebase plain BILI_ACP_TOOLS_*
arrays.
ework-agent ework-agent
test: flip #1712 range-variant pin to the zero-injection contract (#1685
86e1811 
)

decompress-empty-range.test.ts pinned 'search_context keeps
conversation_id' from master (post-#1713). Under #1692 zero-injection
the param is intentionally gone from every served array — the
rebase surfaced the stale pin; flipped to assert absence.
@ranxianglei
ranxianglei force-pushed the 2026-09-29_zero-id-ring-routing branch from 134027d to 86e1811 Compare September 30, 2026 11:26
@ranxianglei
ranxianglei merged commit 80b50b7 into master Sep 30, 2026
13 checks passed
@ranxianglei ranxianglei mentioned this pull request Sep 30, 2026
Merged
@github-actions github-actions Bot mentioned this pull request Sep 30, 2026
Merged
AEmbers pushed a commit to AEmbers/billion-context that referenced this pull request Sep 30, 2026
@ranxianglei
test: e2e the ranxianglei#1685 plugin tool routing ladder + MCP shim …
1872c3b 
…lane

Sediment of the 2026-09-30 real-machine verification matrix (issue ranxianglei#1611 /
PR ranxianglei#1692 live session): every scenario was first probed against a
production instance with curl, then pinned hermetically.

- routing ladder: single-active arb → outbound tool_use witness (plain-JSON
  recording lane, 5th of ranxianglei#1692) beats a two-active refuse-to-guess → loud
  404 on an unknown body conversation id (ranxianglei#1158 diagnostic)
- MCP shim leg: spawns the real src/mcp.ts over stdio JSON-RPC the way a
  codex host drives it (BILI_MCP_PROXY + BILI_CONVERSATION_ID env only) and
  routes a tools/call through the body-id lane
- hermetic: XDG_STATE_HOME redirected before server import — boot's
  loadConversations() would otherwise read the developer's production
  plugin-conversations.json and count live sessions as fresh conversations,
  breaking single-active arbitration assertions (auto-restart.test.ts
  env-first pattern)
This was referenced Oct 1, 2026
Closed
Closed
@ranxianglei ranxianglei mentioned this pull request Oct 6, 2026
Merged
@ranxianglei
ranxianglei deleted the 2026-09-29_zero-id-ring-routing branch October 15, 2026 05:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

p1

Projects

None yet

Milestone

No milestone

1 participant

@ranxianglei