Repository navigation
Conversation
b6d612d to
e807efd
Compare
[bot] 🏷 PR #3047 审核结论(对应根因 issue #3046) 审核对象:最终 head 八问:
验证细节:
结论:审核通过,未发现阻塞问题。 kernel 变更按 Kernel Boundary 约定需人工合并——@OMSociety 请自行合并:#3047 |
…tall warning Tier-1 beautification for #3047 (reference: tintinweb pi-subagents fork 0.19.0-fork.6): - New delegate-render module: renderShell:'self' + renderCall/renderResult on acp_delegate / acp_delegate_wait / acp_delegate_cancel. The call line shows agent + first task line (resume calls marked); the result line shows a status icon, exit label, duration and up to 3 preview lines of run output (Ctrl+O expands to 40). Renderers fall back to plain text on bad input, never throw. - Previews live only in bounded tool-result details (<=4KB, cut on a newline boundary) and the rendered screen. Model-facing content text is unchanged: task title + result file path. - Fleet widget rows append a stall warning when a running child has been silent for 2+ minutes (before the idle watchdog kills it). - promptGuidelines gain two routing entries (route by task kind; read the child's result file instead of re-running its exploration). - exitLabel moved to delegate-render.ts and re-exported from delegate-tool.js (public surface preserved). - Hosts < pi-coding-agent 0.62 ignore the renderer fields (< 0.67.3 ignore renderShell) and keep default tool output; no version floor added. pi-subagents 0.1.5 -> 0.2.0 (CI-enforced for src changes).
…tall warning Tier-1 beautification for #3047 (reference: tintinweb pi-subagents fork 0.19.0-fork.6): - New delegate-render module: renderShell:'self' + renderCall/renderResult on acp_delegate / acp_delegate_wait / acp_delegate_cancel. The call line shows agent + first task line (resume calls marked); the result line shows a status icon, exit label, duration and up to 3 preview lines of run output (Ctrl+O expands to 40). Renderers fall back to plain text on bad input, never throw. - Previews live only in bounded tool-result details (<=4KB, cut on a newline boundary) and the rendered screen. Model-facing content text is unchanged: task title + result file path. - Fleet widget rows append a stall warning when a running child has been silent for 2+ minutes (before the idle watchdog kills it). - promptGuidelines gain two routing entries (route by task kind; read the child's result file instead of re-running its exploration). - exitLabel moved to delegate-render.ts and re-exported from delegate-tool.js (public surface preserved). - Hosts < pi-coding-agent 0.62 ignore the renderer fields (< 0.67.3 ignore renderShell) and keep default tool output; no version floor added. pi-subagents 0.1.5 -> 0.2.0 (CI-enforced for src changes).
|
冲突 ,ework重新基于最新master review一次 走最新的规则 |
…et membership (#3046) stripOrphanedToolResults / stripOrphanedToolCalls collected ids into a Set, so a half survived whenever ANY message carried its id - including a duplicate id from another turn or a declaration on the other side of the fold. Count open calls/results in render order instead: results claim the nearest earlier unanswered call, calls claim the nearest later unanswered result. Verified: new kernel/tests/issue3046-duplicate-tool-call-ids.test.ts fails on master at both pairing assertions and passes here; orphan-fixes, tool-pairs, tool-pair-integrity, state-prune, reasoning-pairs stay green (63/63).
…3046) Records the constraint learned from production: a role:tool message must be preceded by the assistant declaring its tool_call_id, and every declared id must be answered exactly once later. Validator clause is order+multiplicity aware and runs before the tools early-return.
…wner feedback: double it)
e807efd to
1c221a7
Compare
(ai: vllm-qwen/qwen3.8-27b) 冲突已解决 + 基于最新 master 的独立复审冲突处理:PR 分支已 rebase 到最新 master(
结构化八问(基于最新 master 重新独立审过,不复用旧结论)
行为变更披露(旧值 → 新值)
验证证据
遗留项(不另开 issue,留在 #3046 清单内)作者声明的超范围部分我核过属实: 结论冲突解决完毕,独立复审通过。本 PR 含 |
(ai: vllm-qwen/qwen3.8-27b) 冲突处理完成,rebase 后已按最新规则重新审核一、冲突点与解法(4 处,全部是元数据/编号类,无逻辑冲突)
rebase 到最新 master(bd2aa6e),拆成 4 个提交并保留原作者署名:主修复 / root lockfile 同步 / kernel lockfile 同步 / wire-contract 账本。force-with-lease 推到作者 fork 的 PR 分支(PR 开了 maintainer_can_modify),PR 描述同步更正(版本号、WC-018、rebase note)。推送后 PR 恢复可合并状态。 二、重新审核结论(基于 rebase 后 head 1c221a7 独立重审,非复读旧结论)
行为变更披露(PR 描述已有):重复 id 输入下,重建请求现在可能丢弃以前会保留的某些工具半段——正是触发 400 的那些;良构输入输出字节级不变(no-op 测试钉住)。 测试质量:3 个新测试都有牙(test2 把修复回退成旧的 Set 逻辑即失败;test3 钉字节级恒等与顺序), 三、CI 状态:红 lane 与本 PR 无关(已取证)本 PR 相关的所有守卫 lane(kernel-guard、lock-sync、static-gates、config-surface-guard、各 e2e 专项)全绿。6 个红的 test 分片 lane 经排查是仓库级抽风集群:
说明:我的 PAT 无权调 rerun-failed(API 返回 404)。要拿干净绿,请在 Actions 页面点 Re-run failed jobs,或等 #3188 修好后自然转绿。 四、判定代码层面本 PR 可合;因触碰 |
…utify feat: acp_delegate 工具块紧凑化 + 输出预览 + 停滞警告(#3047 核心美化)
(ai: deepseek/deepseek-v4.1-flash)
kernel-change: the unpaired half is produced by the kernel's own fold/render pipeline — a fold covers one instance's partner and the guard then re-pairs that half against a different message carrying the same id — and host-minted tool-call ids must stay byte-exact on the wire (#1039), so the kernel cannot rewrite, re-mint or de-duplicate ids to make a set lookup work. The only thing it can do is refuse to build the invalid shape, which is exactly what these guards exist for ("drop the dangling run rather than ship a 400-triggering half-pair").
Summary
kernel/src/prune.tspaired tool calls with tool results by id-set membership. Host ids are not guaranteed unique — a host that numbers callsname:indexre-issues the same id on later turns — so a half survived whenever any message carried its id, including a duplicate on the other side of the fold or a declaration that comes later in the request. The rebuilt body then went to the wire with arole:"tool"message before any assistant declared itstool_call_id, and the upstream refused the whole conversation:The mirror shape (an assistant
tool_callsentry no later tool message answers) is the same defect with the other half surviving. Both were hit in production on this machine, and both sessions became unusable: every later turn rebuilt the same request and got HTTP 400.Fixes #3046.
What changed
kernel/src/prune.ts—stripOrphanedToolResults/stripOrphanedToolCallsnow pair by order and multiplicity: a counter of open (unanswered) calls and a counter of open results, each half claiming the nearest partner, never a distant same-id twin. The result side walks forward (a tool result may only be claimed by a call that precedes it); the call side walks backward (a call may only claim a result that follows it). Id-less messages keep their previous behavior, andcompresscalls still stay regardless of pairing.kernel/tests/issue3046-duplicate-tool-call-ids.test.ts(new) — a minimal fixture for each half, plus a no-op case proving complete duplicate-id pairs are neither dropped nor reordered.kernel/package.json0.0.123 → 0.0.124 (master had already reached 0.0.123 via fix(#2886): make recent-zone protection pair-aware (prevent orphaned function_call) #2892), with root and kernel lockfiles synced.tests/wire-contract-fakes.ts/tests/wire-contract.test.ts—WC-018(openai-chat) records the constraint learned from production, with a probe body that trips it. The clause is order + multiplicity aware and runs before the tools early-return, so it binds on any messages array. (Renumbered from WC-017 at rebase: [bug] Codex CLI 经 bili 0.1.196 转发时返回空 turn(exit 0 + 0 字节输出;直连正常) #3063 took WC-017 for the codex-lane stream rule; all references in this PR track the rename.)Root cause
A set cannot distinguish two instances that share an id, and membership says nothing about order. The halves themselves are separated by the compression path (the fold covers one instance's partner; the integrity-withdrawal carve can leave the partner visible), but this guard is the last decision point before the wire — so the invalid shape reached the provider.
Test
On
origin/masterboth pairing tests fail at the assertion that names the defect:With the patch they pass, along with the no-op case.
Verification
node --import tsx --test kernel/tests/issue3046-duplicate-tool-call-ids.test.ts kernel/tests/orphan-fixes.test.ts kernel/tests/tool-pairs.test.ts kernel/tests/tool-pair-integrity.test.ts kernel/tests/state-prune.test.ts kernel/tests/reasoning-pairs.test.ts→ 63 pass / 0 fail (8 suites)node --import tsx --test tests/wire-contract.test.ts→ 18 pass / 0 failnpm run typecheck→ exit 0npx prettier --check kernel/src/prune.ts kernel/tests/issue3046-duplicate-tool-call-ids.test.ts→ cleancopilot-schema-contract,wire-contract-go-session,prefix-affinity-orphan,e2e-responses-offhead-system, the orphan/strict-echo loop suites) unchanged behavior.Behavior change
Disclosed: a rebuilt request can now lose a half it previously kept. Only messages that cannot be paired in render order are removed, i.e. exactly the ones that would have tripped the upstream contract; complete pairs, id-less messages and
compresscalls are untouched (pinned by the no-op test).Config surface
None — no new config field, env var, CLI flag, or persisted field. No id is minted, rewritten, or de-duplicated.
Related
kernel/src/recommend.ts); fix(#2886): make recent-zone protection pair-aware (prevent orphaned function_call) #2892 makes that protection pair-aware and deliberately adds no wire cleaning, and it cannot fix this one.src/tool-pair-order.ts, Responses wire).PLUGIN.mdobligation 5). This is the neighbour: ids that are stable but not unique.stripOrphanedToolResults(then a content-hash collision), i.e. precedent that this guard is maintained where it is.Out of scope
Other id-keyed helpers in the same pipeline share the uniqueness assumption —
pairSafeAnchorIndex/adjustBoundariesForToolPairsinkernel/src/prune.tsandcomputeIntegrityWithdrawalsin the compressor. They are how the stray halves stayed visible, but they do not decide what goes on the wire; happy to file that separately if you want it tracked.Reported by an AI agent on behalf of a user; every artifact above was produced by running the code in this branch and on
origin/master. Kernel change — flagging for owner review per the Kernel Boundary rule.Rebase note (2026-10-14)
Rebased onto
master@ bd2aa6e after #3055/#3063 landed. Conflicts were metadata-only: kernel version line (→ 0.0.124), both lockfiles, and the WIRE_RULES id allocation (→ WC-018, all references updated). No logic change versus the originally reviewed diff.