Repository navigation
[PR] fix(#5595): keep fork children sendable when raw-history rebuild emits empty tool-call arguments - #5602
Open
ranxianglei wants to merge 26 commits into
Open
ranxianglei wants to merge 26 commits into
ranxianglei wants to merge 26 commits into
Conversation
Converging master's red gates onto ci-fixes. lifecycle-ledger-guard is already fixed here (LIFECYCLE.md entries for BILI_SESSION_CONTENT_STORE_PIN / BILI_SPAWN_FINGERPRINT from the prior sentinel pass, preserved through the master merge); this commit clears the other two. docs-sync: #5319 documented the new 'chat' builtin prompt pack in CONFIGURATION.md/.zh-CN.md but never updated the single-source seed, so regeneration dropped it and the gate failed. Add 'chat' to the compress.promptPack seed (type/desc_en/desc_zh) and regenerate website/docs/index.html to match. unused-exports: CodexWindowSource (src/codex-models.ts) and PackInstallOptions (src/pack-install.ts) are referenced only inside their own modules and imported nowhere else; drop the leading export (INTERNAL_ONLY) as the guard prescribes. Verified locally: lifecycle-ledger-guard OK, check-unused-exports clean, gen-config-docs regenerate matches committed artifacts (zero drift).
…safe family gate) pack-install.ts took the last 3 npm stderr lines via split+slice(-3), tripping the #816/#1615 family gate in tests/text-safe.test.ts even though the receiver is an array of lines (code-unit-safe). Use a non-negative computed start index so the literal negative-slice token is gone; output is identical (last up-to-3 lines clamped to 300 chars). Inherited from #5340, which shipped without satisfying the gate.
Inherited red resolved by convergence, not rewrite: ci-fixes was 12 commits behind master and its two unique fix commits (cbdd67d negative-slice, 701688d docs-sync/unused-export) are strict subsets of what master already merged better (e030d3b+4a12c1a4 safePrefix+windows npm shim, 3491e2d dead export, b76f0c9 lifecycle reconcile). Both failing shards were inherited from master's own newer commits that ci-fixes lacked: - #5103 refusal-remedy test -> pinned by 892a378 - flushAll dirty content store -> fixed by 1f209ff (src/store.ts adopt) Conflicts (codex-models.ts, pack-install.ts) resolved toward master = superset. Resulting ci-fixes tree is byte-identical to origin/master (git diff empty).
…n (suite preload parity) The whole suite runs under tests/preload-test-env.mjs, which pins the #4313 message-timestamp display gate OFF so byte-comparing baselines keep their pre-feature wire bytes. The regen script only carried the Parity #1/#2 pins (image rescale, snapshot shadow) — a bare 'node --import tsx' regen baked time-stamped ACP tags plus the timestamp-aware ACP-TAGS doc variant into every per-turn digest (+157B constant per body, +58B per turn), so freshly regenerated goldens would red all four wires again on the next CI run. Discovered while converging the master red at c78ecb4 (#4599 sentinel cycle): the local regen drift was initially misread as an environment divergence until body-level forensics isolated the stamp attributes and the flag-conditional doc section in kernel/src/compress-tools.ts.
#5416 (c91ef1a, kernel 0.0.176) added absorbedDebt/absorbCount to the persisted nudge state (host booking in src/session.ts absorbAbortedTurnGrowth, reset in kernel applyCompression) — an intentional persisted-state schema change, declared in that commit ('kernel-change: nudge decision only ... no config surface added'). After any fold the persisted session now carries the two new keys, so the state digests drift by design. Regenerated via the now-parity-pinned scripts/update-storage-render-goldens.ts. Diff is surgical: per wire ONLY stateDigest, stateDigestAligned, sectionDigests.state and stateKeyDigests.nudge change; every per-turn sha/bytes/shaCiteNorm is byte-identical to the committed goldens (wire parity proof) and blockContents/stats/metadata sections are untouched. Both consuming suites green locally under the exact CI preload chain (17/17).
…th lost node_modules in #5430)
…win22s2 flake, 3 consecutive runs)
…iscussion, not node) The new blog pipeline (#5414) selected "{ node { number url } }" from the createDiscussion mutation, but GitHub's CreateDiscussionPayload exposes "discussion", not "node" — so the first master run of sync-discussions failed with "Field 'node' doesn't exist on type 'CreateDiscussionPayload'". Align the query and its result reads, and pin the selection in the unit test (positive + negative) so the wrong field cannot silently ship again.
The pipeline opens exactly one PR for the human merge step (POST /pulls), which requires the pull-requests:write permission; without it the write-back would 403 immediately after a discussion was created.
…pshot (#4599) The "PFA stamp must be on disk after restart" assertion flaked on CI load (ubuntu-22 shard1; reproduced 1/24 locally under concurrency, idle-green). Root cause: the test read the raw envelope while the still-running server could still fire leftover turn debounced saves, which overwrote the terminal snapshot without the just-set anonymousPrefixAffinity field. A real restart stops the old process writing, so drain (flushAll) + cancel leftover debounces (cancelAll) and swap in the reload store BEFORE reading. All four original byte assertions are preserved verbatim; a genuine persistence failure still trips them. Validated: idle 4/4 + 112 concurrent runs, 0 failures.
…py sweep #5555/#5556 intentionally reworded the nudge-appended notes (FIRST_SIGHT_DRAIN_NOTE/_EXTERNAL, SUMMARY_BUDGET_NOTE), the retrieve-tool description, the absorb/decompress receipts, the acp-status empty-ranges line, and the image-rescale note, but regenerated only the wire-contract goldens. The storage-render suite pins the persisted nudge/state digest, so all four wires drifted deterministically (blockContents unchanged; only state/stats/metadata moved) and reded 8 test shards on master d244fff. Regenerated tests/golden/storage-render/{chat,anthropic,responses,google}.json via scripts/update-storage-render-goldens.ts under its parity pins (BILI_IMAGE_RESCALE=0, BILI_SNAPSHOT_SHADOW=0, BILI_MESSAGE_TIMESTAMPS=0). Local regen stateDigests reproduce CI's actual drifted digests exactly (chat b7ba6bb1, anthropic e0e58faa, responses 134a78c7, google 9e9ddc00) => deterministic, not flake. Sentinel convergence onto ci-fixes for the master-red (#4599); flows to master via #5358.
…tbound OpenAI wire A fork child whose parent held multimodal/thinking content cannot be textually compared, so adoption degrades to a full raw-history rebuild whose unrecovered tool-call cores serialize as empty function.arguments - a hard provider 400 (InvalidArgument) that compaction/preflight cannot rescue, bricking the child. Enforce the provider contract at the two OpenAI finalize sites (proxy prepare + loop adapter), mirroring MALFORMED_PROXY_TOOL_ARGS_REPLAY: valid arguments pass byte-exact, only empty/non-object JSON is swapped for a parseable placeholder.
When durable storage is legacy-only (json) there are no unified tail rows, so the fork raw-tail reconstruction bails permanently - but it reported the transient 'sidecar-missing' reason, mislabeling a structural gap as a recoverable open failure. Report the actionable cause instead.
Contributor
📦 Built Package ArtifactBranch: Latest build — install this exact versionnpm install -g billion-context@0.0.7992-pr.5602.420ace1This exact version is immutable and reproducible. The rolling Recent builds (commit → version)
Download artifact
tar xzf billion-context-pr5602.tgz
npm install -g packageInstall instructions are refreshed on each push. |
ranxianglei
changed the title
Owner
Author
[bot] 🏷 (ai: vllm-qwen/qwen3.8-27b) 查重：不重复，但与在途 PR #5537（镜像 #5540）同机制家族、存在直接代码冲突 —— 先报这个，因为它影响合入顺序：
标签：bug / dsh / scope/wiring / p1（fork 子会话永久不可用 = owner 2026-10-19 修订的「fork 继承被永久放弃」类 p1；车道按报告环境取 dsh）。 CI 状态：head 初审已发现一处需修的小问题（F4）： 完整八问结论 + 本地验证结果随后补一条。 |
added 2 commits
November 3, 2026 17:48
…rage stamp Review follow-up on 0964efe: inside the !replicasAvailable() branch, liveStorageStamp === "legacy" is always true (the stamp reads legacy whenever the sidecar is absent/disabled), so the previous ternary's else branch was dead code and every unified-configured instance with a down sidecar (3-strike latch, corrupt-index boot-without-index) got stamped "durable storage is legacy-only" — mislabeling exactly the recoverable/ latched cases F4 exists to tell apart. Discriminate on the constructor-time resolution instead: new SessionStore.configuredUnified getter (enabled AND knob tier non-legacy AND no encryption hard-block; the block flag is now stored as a ctor-time field instead of a local const). Structurally legacy-only instances keep the actionable config message; unified-configured instances whose sidecar is down right now keep the #5188 sidecarDownReason() vocabulary. Tests: three store-level pins appended to tests/issue5595-fork-empty-tool-args.test.ts (legacy knob / full with a 3-strike-latched sidecar via the #4942 seam / encryption hard-block), including the conflation assertion itself (live stamp reads "legacy" while configuredUnified stays true).
Owner
Author
[bot] 🏷 (ai: vllm-qwen/qwen3.8-27b) Review 结论:可合(等新 head 八问
行为变更披露(旧→新):①出站 OpenAI body 中空/非对象 JSON 的 我补的 commit 合并期协调事项(两条,均不阻塞本 PR 单独成立):
CI:新 head 收工判定:代码与行为审毕,可合;等 c156bc0 CI 绿 + 上述两条协调事项落定(建议 owner 拍板 #5537 合并顺序)。 |
Owner
Author
|
(ai: vllm-qwen/qwen3.8-27b) 结论:REQUEST_CHANGES —— 一个阻塞项(CI 红即该问题,非继承红);其余通过。 阻塞发现
修法(选项 A,推荐):删掉 非阻塞发现(已验证,本分支无需动作)
回归审查(diff 触碰路径的既有行为,逐项结论)
八问
合并判定:现状不可合。 按选项 A 回退 adapter 侧守卫后分支其余部分干净——F1+F4 正确、双向测试齐全、覆盖所报复现。选项 B 需 owner 签核(见 🔶 评论)。 |
Owner
Author
|
🔶 需要决策 PR #5602(head c156bc0)被卡 diff: 选项:
请裁定 A/B。(Responses wire 同源暴露 |
Owner
Author
[bot] 🏷 审核完成 → REQUEST_CHANGES(现状不可合),已走 🔶 决策流。 结论:PR #5602(fix #5595 fork 空 tool-call arguments)有一个阻塞项,其余通过。
投递:完整审核 → PR #5602 评论;🔶 需要决策 评论 + |
Owner
Author
Owner
Author
…in prepare only Owner review of PR #5602: the hardenOpenaiToolCallArguments call added to the openai LOOP adapter's buildRequest broke #1039's tool-args-verbatim corpus (toolargs-adversarial 'C chat rebuild verbatim': bare empty / whitespace-only / unparseable-truncated args must ride byte-exact). The host re-sends its own history on the loop re-request, so any rewrite desyncs the session (#1039/#1960). Remove the guard from the adapter (loop path stays verbatim); it remains in the proxy-mode prepare path (prepare-openai), which covers this issue's fork-rebuild repro. Replace the now-invalid adapter-heals wiring test with a boundary pin asserting the loop forwards degenerate args verbatim. Builds on bb1b6e0's configuredUnified F4 refinement (kept) and its three F4 store-level pins.
This was referenced Nov 3, 2026
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #5595
What
A fork child whose parent held multimodal/thinking content cannot be textually compared, so adoption degrades to a full raw-history rebuild. That rebuild serializes unrecovered tool-call cores as empty
function.arguments, which OpenAI-compatible providers reject with a hard 400 (InvalidArgument:...function.arguments must be a valid JSON object string). Compaction/preflight cannot rescue it — it is a validation failure, not a size failure — so the forked session stays unusable forever.This PR makes the reported bricking go away at the boundary we own, plus fixes a misleading diagnostic on the same path.
Changes
F1 — outbound arg-validity guard (the availability fix). Enforces the provider contract that every OpenAI
tool_calls[].function.argumentsis a valid JSON object string, at BOTH finalize sites where bili assembles the outbound OpenAI body:src/server/prepare-openai.ts(proxy-mode rebuild)src/loop/adapter-openai.ts(compress-loop re-request)New
hardenOpenaiToolCallArgumentsinsrc/util.tsmirrors the existingMALFORMED_PROXY_TOOL_ARGS_REPLAYprecedent (src/loop/core.ts, which already fixes the model-produced non-JSON case): valid arguments pass byte-exact; only empty/non-object JSON is swapped for a parseable placeholder, and one warn line records how many were rebuilt. Wire fidelity holds (§7.3): no legitimate argument bytes change.F4 — name the permanent legacy-storage bail. When durable storage is legacy-only (
storage.unified="legacy"), there are no unified tail rows, so fork raw-tail reconstruction bails permanently — but it reported the transientsidecar-missingreason, mislabeling a structural gap as a recoverable open failure. It now reports the actionable cause.Scope & what is deliberately NOT here
kernel/src/wire/responses.ts, etc.) but were not hit by this report, andprepare-responses.tshas no single convergence point, so host-guarding only some branches would be a partial-coverage trap. The complete all-wires fix belongs in the kernel serializers — human-gated (Kernel Boundary) — and is tracked separately, not bundled here.src/plugin.ts:1264) collides with in-flight PR [PR] fix(plugin-fork): lift the thinking hard gate — thinking sessions can fork (#4266) #4900 (which rewrites that exact line) — deferred to avoid a hot-line conflict.In-flight overlap on the broader "fork adoption fails → child unusable" family: #4900 (thinking gate), #3024 (retryable snapshot), #5126 (false-success receipt), #5538 (oversized rebuild). This PR covers the distinct facets §4.2 (empty-args schema 400) + §4.3 (legacy no-reconstruction diagnostic) uncovered by those.
Verification
npm run typecheckclean (exit 0).tests/issue5595-fork-empty-tool-args.test.ts(8 tests incl. gate-negative byte-exactness + a livebuildRequestwiring test) passes.issue3661-malformed-proxy-args-replay,issue3926-empty-tool-calls,loop-adapters— 52 pass / 0 fail (no regression in the touched adapter/wire path). Full-suite verdict = remote CI on the pushed head.No config surface added/changed. No kernel change. No dependency added.