feat(cli): add audio bridge for attachments - #8332
Conversation
E2E test reportBaselineWith the installed CLI and a text-only primary model, a headless Feature verification
Automated verification
Scope notesManual E2E was run on macOS. Windows and Linux manual runs, tool-result audio, channel-specific attachment plumbing, video, and realtime dictation were not tested because they are outside this change's scope. |
|
Gate re-run at Template ✓ — all sections present, bilingual. Problem — observed, not theoretical: on the merge base a text-only primary model returns Direction — aligned: this is the audio twin of the shipped vision bridge. It reuses the existing voice-model setting ( Size — core paths touched ( Approach — no scope creep in the final delta. The two commits since the last gate are pure main syncs; no conflict-resolution edits touched PR code, so the approach assessment stands unchanged: one shared fail-closed bridge service consumed by all three entry points, mirroring the vision bridge's shape. Risk — Stage 1e still matches Moving on to code review. 🔍 中文说明在 模板 ✓ —— 各节齐全，双语。 问题 —— 已观测、非理论：merge base 上纯文本主模型对 WAV 方向 —— 对齐：这是已上线 vision bridge 的音频孪生体，复用现有语音模型设置（ 规模 —— 触及核心路径（ 方案 —— 最终增量无夹带。上次门禁后的两个提交纯属 main 同步，冲突解决未触及 PR 代码，方案评估不变：一个共享 fail-closed bridge 服务供三个入口复用，与 vision bridge 形态一致。 风险 —— Stage 1e 仍命中 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewReviewed at
Still open from earlier rounds, none blocking: R20-1 (the ACP resolution-failure branch replaces the audio correctly but emits no user-facing notice — disclosure consistency, not a leak) plus the deferred Suggestion threads on test-assertion strength, recorded per the late-round guidance. TestingUnattended run — no PR code built or executed here. Evidence carried: the PR's own CI at the reviewed commit (table below), and the maintainer's verification record in this thread — four independent local verifications, the most recent a real local stack at
All four Sandboxed verification, if a maintainer wants machine-produced proof at this exact head: 中文说明代码审查在
早期轮次遗留、均不阻塞：R20-1（ACP 解析失败分支正确替换了音频但无用户可见通知——属披露一致性，非泄漏）及测试断言强度类 Suggestion 线程，按后期轮次指引留档。 测试无人值守运行——未构建、未执行 PR 代码。本节证据：该提交的 PR 自身 CI（见英文部分标记区表格），以及本帖维护者的验证记录——四次独立本地验证，最近一次为 该 head 的四个 如维护者想要当前 head 上的机器化证据：以 sponsored run 方式触发 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 3/5 — clean review at Stepping back, honestly: there is nothing left to review here, which is itself the finding. The delta since the last gate is two clean upstream syncs — I verified file-for-file that no conflict resolution touched PR code — and the surrounding review history is the unusually thorough kind: 21+ review rounds, the one Critical that surfaced late (R19-1, the ACP bridge blind to the active route override) fixed and test-pinned two commits before the last gate, and four independent maintainer verifications in this thread, the most recent on a real local stack at What is actually left, none of it code and none of it for the bot to decide:
⏸️ Deferring to @wenshao — the review is clean and the maintainer's approval stands at this head; the bot defers purely on the Stage 0 policy cap for core-touching fork PRs, and the remaining step is the human merge decision. 中文说明置信度：3/5 —— 退一步看：这里已经没有可再审查的东西——这本身就是结论。上次门禁后的增量是两次干净的上游同步（已逐文件核验冲突解决未触及 PR 代码），而围绕它的评审历史异常扎实：21+ 轮评审，后期浮现的唯一 Critical（R19-1——ACP bridge 对活动路由 override 视而不见）已在上一门禁前两个提交修复并有测试锁定，本帖中还有四次维护者独立验证，最近一次是 真正剩下的事，都不是代码，也不由机器人决定：
⏸️ 转交 @wenshao —— 审查干净、维护者批准已在当前 head 上；机器人的 defer 纯粹出于 Stage 0 对触及核心的 fork PR 的政策上限，剩余步骤是人的合入决定。 — Qwen Code · qwen3.8-max Reviewed at |
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。本 PR 来自 fork，首轮处理将由下一次定时扫描执行（通常几分钟内）。移除 |
Review:
|
- Fix ACP @image regression: images are now unconditional in the text-path collector and re-validation loop, only audio is gated behind preserveUnsupportedAudioForBridge - Add MAX_AUDIO_PARTS_PER_TURN cap (4) and transcript length clamp - Check voice transport in runAudioBridge for consistent gating - Fix circular failure notice: report first concrete reason - Add early signal.aborted return matching the vision bridge - Bail on cancellation in headless after audio bridge - Use INFO (not ERROR) for partial-success bridge notices - Canonicalize non-standard audio MIME subtypes (x-wav, x-m4a, etc.) - Unexport isAudioPart (unused outside the module) - Add tests: image-capable ACP, oversized audio, partial conversion, pre-aborted, transport gating, per-turn cap, concrete error reason
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Review feedback addressed🔴 Blocking: ACP
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
已审查。 建议见行内评论。 未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： No action requiredAll four automated-reviewer suggestions were already addressed in commit
中文说明无需操作自动化审查器的四条建议均已在提交
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
已审查。 建议见行内评论。 未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： No code changes required this roundAll four inline findings from the automated reviewer were already addressed by @DragonnZhang in commit
The remaining
Verification
中文说明本轮无需代码变更自动化审查器提出的四个行内发现已全部由 @DragonnZhang 在提交
剩余的
验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
Review:
|
Round 41 — both Criticals verified real and fixed (head
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Deferred under the convergence posture (round 42, not a blocker) — recorded, not requested in this round:
packages/cli/src/ui/hooks/useGeminiStream.test.tsx:5694 — [review] history exactly-once claim asserted as at-least-once (toBeGreaterThan(0))packages/cli/src/ui/hooks/useGeminiStream.test.tsx:6791 — [probe] rewritten bridge-failure test drops the text part; fail-closed text survival unguarded (mutation ships green)packages/core/src/utils/fileUtils.ts:1294 — [probe] no exactly-10-MiB read-time boundary test; <= to < mutation ships green and wrongly rejects exact-cap attachmentspackages/cli/src/services/audio-bridge-service.ts:215 — [review] MAX_AUDIO_PARTS_PER_TURN enforced per call, not per turn, on drain/steer pathspackages/cli/src/acp-integration/session/Session.test.ts:8248 — [review] cancel-during-bridge tests never assert the user's text part survives the fail-closed abort pathpackages/cli/src/acp-integration/session/Session.test.ts:8642 — [review] mixed audio+image test never asserts the image part reaches the vision bridgepackages/cli/src/nonInteractiveCli.test.ts:1852 — [review] media-removal tests never pin the exact-route to bare selector flip on modelOverridepackages/cli/src/nonInteractiveCli.ts:1435 — [review] override resolution failure silently drops the override with no user-visible noticepackages/cli/src/services/audio-bridge-service.ts:96 — [review] notice attributes all failed files to the first failure reason (mixed reasons misreported)packages/cli/src/ui/hooks/useGeminiStream.test.tsx:1588 — [review] cancellation test pins INFO type only, not the audio egress disclosure textpackages/cli/src/ui/hooks/useGeminiStream.test.tsx:5486 — [review] drain-recovery tests never re-check the re-queue channel after successful re-deliverypackages/cli/src/ui/hooks/useGeminiStream.test.tsx:6346 — [probe] reconcile test pins the audio half only; the image segment's fate unguarded (mutation ships green)packages/core/src/core/client.test.ts:6796 — [probe] onSteerResolved notify-before-send ordering asserted order-blind (mutation ships green)
Convergence: round 42 posted 6 inline comment(s), 6 of them reported for the first time; the previous round posted 2 (2 new). Findings keep coming back to the same files: packages/cli/src/ui/hooks/useGeminiStream.ts (findings in round 41; 4 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push keeps the loop from re-deriving the same set; this PR's reviews already resolve to a critical posting floor. (Observation only — nothing was withheld from this review because of this observation.)
[Critical] R10-5 (still standing, carried from round 10, re-verified at this commit): packages/cli/src/nonInteractiveCli.ts — headless mid-loop and drain-item skill overrides are stored bare (itemModelOverride = override, ~:2916) and sent bare (modelOverride: itemModelOverride, :2827); the send-time NUL exact-route suffix applies only to inlineModelOverride (:1683-1688). Core resolves exact routes only for trailing-NUL selectors, so requestModalities falls back to the session config and slimCompactionInput keeps inline audio when the SESSION model is audio-capable — preserved raw audio rides the continuation send to a text-only skill target with no marker and no notice, while the interactive/ACP twins gate media routing on the target's modalities. This round's diff still does not touch the drain-item/mid-loop path (no hasAudioParts/runAudioBridge/applyToolResultMediaGate anywhere in it). Fix: route skill-override sends carrying surviving media through the target's modalities fail-closed (NUL exact route when media survives, or bridge/clamp before the bare send), matching the interactive/ACP twins.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
收敛姿态下延后（第 42 轮，非阻断）——已记录，本轮不要求修改：共 13 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 42 轮发布了 6 条行内评论，其中 6 条是首次提出；上一轮发布了 2 条（其中 2 条首次提出）。发现反复回到同一批文件：packages/cli/src/ui/hooks/useGeminiStream.ts（第 41 轮已出过发现，本轮又有 4 条）。新发现的产出速度没有下降。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送，可以避免循环反复推导同一组发现；本 PR 的评审已解析为 critical 发布下限。（仅为观察——本轮评审未因此扣留任何内容。）
[Critical] R10-5 (still standing, carried from round 10, re-verified at this commit): packages/cli/src/nonInteractiveCli.ts — headless mid-loop and drain-item skill overrides are stored bare (itemModelOverride = override, ~:2916) and sent bare (modelOverride: itemModelOverride, :2827); the send-time NUL exact-route suffix applies only to inlineModelOverride (:1683-1688). Core resolves exact routes only for trailing-NUL selectors, so requestModalities falls back to the session config and slimCompactionInput keeps inline audio when the SESSION model is audio-capable — preserved raw audio rides the continuation send to a text-only skill target with no marker and no notice, while the interactive/ACP twins gate media routing on the target's modalities. This round's diff still does not touch the drain-item/mid-loop path (no hasAudioParts/runAudioBridge/applyToolResultMediaGate anywhere in it). Fix: route skill-override sends carrying surviving media through the target's modalities fail-closed (NUL exact route when media survives, or bridge/clamp before the bare send), matching the interactive/ACP twins.
— qwen3.8-max via Qwen Code /review (v0.22.0)
…s on abort Round-42 fixes for six Criticals: - R42-1: handleResolvedSteer no longer nulls the retry store when the Steer send is restored; it reinstates the outer turn's payload so the errored outer turn stays Ctrl+Y-retryable (exactly-once preserved). - R42-2: payloadCarriesRoutedMedia counts nested functionResponse.parts media, so a Retry of a failed nested-media continuation keeps the trailing-NUL exact-route marker instead of sending the bare selector into session-modality slimming. - R42-3: the image-only capability-probe failure clears the inline override (R33-2 invariant, mirroring the audio branch), guarded by !mediaRouted so an already-routed audio route is not wiped; the cleared-override state with no delivery path now fails closed visibly with a pristine capture for Retry. - R42-4: both stop-loop drain branches re-check pendingSend.signal after the awaited continuation claim (which converts an abort to 'unavailable') and persist the dequeued messages before settling cancelled, instead of letting #runStopContinuation's initial-send preservation no-op drop them. - R42-5: the finalize loop's abort override only recomputes from the raw parts when the bridge conversion did not run, so completed transcripts are no longer discarded. - R42-6: applyToolResultMediaGate returns a pristine capture alongside its substitution and the function-response/Retry branches surface it, so the retry store keeps the media-bearing payload instead of resending the marker forever. Tests: 5 new regression tests (each negative-controlled against the unpatched code), 1 updated steer-restore test, 2 pre-existing expectations corrected to the fixed behavior.
|
Round 42 — all 6 Criticals verified; 6/6 real, all fixed in Every thread was verified against R42-1 — boundary-drain retry-store clobber ( R42-2 — Ctrl+Y Retry of a failed nested-media continuation loses the NUL exact-route marker ( R42-3 — image-only probe failure never clears the inline override (
R42-4 — abort race against the awaited continuation claim ( R42-5 — finalize loop's abort override discards completed transcripts ( R42-6 — resolved-but-unsupported gate substitution destroys nested media without a pristine capture ( Verification (all under the build lock): Flake note: under 2-core contention, |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): chunk 27: none — wait, per the brief I should write no such line when nothing was cut short. Nothing was cut short.; "agent reverse-audit (round 2)": none — the chunk (diff lines 5504-5899) was read in full in one untruncated read, and every follow-up check above completed.; "agent reverse-audit (round 2)": identifying the exact origin of all four resolveForModel calls asserted in 'rechecks earlier native audio after a later route resolution fails' (the test pass….
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Deferred under the convergence posture (round 43, not a blocker) — recorded, not requested in this round:
packages/cli/src/nonInteractiveCli.ts:1435 — [review] inline override silently dropped on resolution failure — no noticepackages/cli/src/ui/hooks/useGeminiStream.test.tsx:5978 — [review] exactly-once history asserted only with toBeGreaterThan(0)packages/core/src/core/client.ts:4059 — [review] three of four steerRouteOverride continuation sites untestedpackages/cli/src/ui/hooks/useGeminiStream.ts:3984 — [review] client-driven drains never stamp the route — media-free continuations lose the exact routedocs/design/2026-08-01-audio-bridge.md:34 — [review] design doc scope contradicts the nested tool-result gatepackages/cli/src/acp-integration/session/Session.test.ts:8944 — [review] mixed-media ACP test tolerates a raw-image leak (arrayContaining)packages/cli/src/acp-integration/session/Session.test.ts:9512 — [review] no test pins preserveUnsupportedImageForBridge absencepackages/cli/src/acp-integration/session/Session.test.ts:14001 — [review] pinned probe counts lock in the double capability-probepackages/cli/src/services/audio-bridge-service.test.ts:282 — [review] all-failed-after-egress notice drops the failure reasonpackages/cli/src/ui/hooks/useGeminiStream.test.tsx:5986 — [review] bridge-succeeded exactly-once test never runs the bridgepackages/cli/src/ui/hooks/useGeminiStream.test.tsx:6413 — [review] re-bridge retry test lacks not.toContain('audio/wav')packages/cli/src/ui/hooks/useGeminiStream.ts:1898 — [review] gate catch mislabels resolution failure as 'does not support'packages/cli/src/acp-integration/session/Session.test.ts:8447 — [review] oversized-audio tests juggle an env var that never firespackages/cli/src/services/audio-bridge-service.ts:215 — [review] 'maximum N per turn' cap is per bridge invocationpackages/cli/src/ui/hooks/useGeminiStream.test.tsx:1150 — [review] inline-override nested-media tests never pin the continuation routepackages/cli/src/ui/hooks/useGeminiStream.test.tsx:6640 — [review] recheck test never asserts the drained image survivespackages/cli/src/ui/hooks/useGeminiStream.test.tsx:6876 — [review] fallback-phase send pinned only by transcript substringspackages/cli/src/ui/hooks/useGeminiStream.test.tsx:7260 — [review] no test pins user text surviving the vision fail-closed fallback
Convergence: round 43 posted 8 inline comment(s), 8 of them reported for the first time; the previous round posted 6 (6 new). Findings keep coming back to the same files: packages/cli/src/ui/hooks/useGeminiStream.ts (findings in round 42; 8 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push keeps the loop from re-deriving the same set; this PR's reviews already resolve to a critical posting floor. (Observation only — nothing was withheld from this review because of this observation.)
[Critical] R10-5 (still standing, carried from round 10, re-verified at this commit): packages/cli/src/nonInteractiveCli.ts — headless mid-loop and drain-item skill-tool overrides are stored bare (:2612, :2915) and sent bare (:2499, :2827); the send-time NUL exact-route suffix applies only to inlineModelOverride (:1683-1688). Core resolves exact routes only for trailing-NUL selectors (geminiChat.ts:2318), so requestModalities falls back to the session config and slimCompactionInput keeps inline audio when the SESSION model is audio-capable — preserved raw audio rides the continuation send to a text-only skill target with no marker and no notice, while the interactive/ACP twins gate media routing on the target's modalities. The \0 guards at :2609/:2910 reject mid-item override changes against a stamped value but never stamp drain-item sends. witness: round-38 probe reproduced the raw-audio ride end-to-end; re-verified statically at the reviewed commit this round. Fix: route skill-override sends carrying surviving media through the target's modalities fail-closed (NUL exact route when media survives, or bridge/clamp before the bare send), matching the interactive/ACP twins.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：chunk 27：none — wait, per the brief I should write no such line when nothing was cut short. Nothing was cut short.；"agent reverse-audit (round 2)"：none — the chunk (diff lines 5504-5899) was read in full in one untruncated read, and every follow-up check above completed.；"agent reverse-audit (round 2)"：identifying the exact origin of all four resolveForModel calls asserted in 'rechecks earlier native audio after a later route resolution fails' (the test pass…。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
收敛姿态下延后（第 43 轮，非阻断）——已记录，本轮不要求修改：共 18 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 43 轮发布了 8 条行内评论，其中 8 条是首次提出；上一轮发布了 6 条（其中 6 条首次提出）。发现反复回到同一批文件：packages/cli/src/ui/hooks/useGeminiStream.ts（第 42 轮已出过发现，本轮又有 8 条）。新发现的产出速度没有下降。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送，可以避免循环反复推导同一组发现；本 PR 的评审已解析为 critical 发布下限。（仅为观察——本轮评审未因此扣留任何内容。）
[Critical] R10-5 (still standing, carried from round 10, re-verified at this commit): packages/cli/src/nonInteractiveCli.ts — headless mid-loop and drain-item skill-tool overrides are stored bare (:2612, :2915) and sent bare (:2499, :2827); the send-time NUL exact-route suffix applies only to inlineModelOverride (:1683-1688). Core resolves exact routes only for trailing-NUL selectors (geminiChat.ts:2318), so requestModalities falls back to the session config and slimCompactionInput keeps inline audio when the SESSION model is audio-capable — preserved raw audio rides the continuation send to a text-only skill target with no marker and no notice, while the interactive/ACP twins gate media routing on the target's modalities. The \0 guards at :2609/:2910 reject mid-item override changes against a stamped value but never stamp drain-item sends. witness: round-38 probe reproduced the raw-audio ride end-to-end; re-verified statically at the reviewed commit this round. Fix: route skill-override sends carrying surviving media through the target's modalities fail-closed (NUL exact route when media survives, or bridge/clamp before the bare send), matching the interactive/ACP twins.
— qwen3.8-max via Qwen Code /review (v0.22.0)
…actly-once Address the round-43 review findings (all verified real, each with a negative-control regression test): - R43-1: normalize MIME case in the nested tool-result media detector/replacer and in core route slimming (supportsMimeType / isNonImageMime). RFC 6838 types are case-insensitive; uppercase MCP MIME slipped past the gate and was placeholder-substituted even on capable routes. - R43-6: detect fileData carriers nested in functionResponse.parts alongside inlineData, matching core's convertToFunctionResponse and slimming/microcompact media tests. - R43-7: clamp accepted nested media in the tool-result gate with the QWEN_CODE_MAX_INLINE_MEDIA_BYTES ceiling, like every other routing path added by this PR. - R43-2: capability-probe top-level images against media-routed (non-inline) overrides and fail closed / bridge instead of passing raw images into the exact route. - R43-3: report drain routing only when a route survived the drain; a sticky mediaRouted with a cleared selector let core fall back to the stale pre-drain override. - R43-4: prefer the drain's fully-pristine capture over the gate capture when both exist in the retry store. - R43-8: add SteerInput.onAccept and invalidate the retry store symmetrically on accept, so an accepted steer cannot be Ctrl+Y-redelivered when a later continuation fails. - R43-10: advance the prompt counter on Retry so a retry's minted prompt_id cannot be re-minted by a later Notification drain and exact-routed into the stale override. Tests: useGeminiStream 277/277, core compactionInputSlimming+client 403/403, cli tsc clean; every fix negative-controlled.
|
Round 43 — all 8 Criticals verified against the code; 8/8 real, all fixed in Every finding was re-derived from the source (not the review text), and each fix has a negative-control regression test that was confirmed to fail on the unpatched code with the reviewer's symptom. R43-1 — case-sensitive MIME in the nested tool-result detector (real, fixed) R43-2 — top-level images never probed against non-inline media-routed overrides (real, fixed) R43-3 — sticky R43-4 — R43-6 — nested detector models the carrier as inlineData-only (real, fixed) R43-7 — supported nested media bypasses R43-8 — accepted steers linger in the retry store (real, fixed) R43-10 — prompt_id stamp collides for Retry and Notification (real, fixed) Verification (all under the shared build lock)
Pending review with 550 drafts still open — inline replies remain blocked (422), so this consolidated comment serves as the per-thread response; threads resolved via GraphQL. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 2)": none — but one candidate examined and dropped, for the record: #processSlashCommandResult passes no getModelOverride to #resolvePrompt / #applyBridgeConver…; chunk 5: executing the four touched tests in Session.test.ts — the review worktree has no node_modules or built workspace dist/, and a full install + build exceeds the r….
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Deferred under the convergence posture (round 44, not a blocker) — recorded, not requested in this round:
packages/cli/src/nonInteractiveCli.ts:1435 — [review] headless silently drops an unresolvable inline /model override — no user-facing noticepackages/cli/src/ui/hooks/useGeminiStream.ts:2024 — [review] tool-result gate mislabels a resolution failure as 'does not support images/audio'packages/core/src/core/client.ts:3903 — [review] steerRouteOverride continuation sites untested (3 of 4 send sites)packages/cli/src/acp-integration/session/Session.ts:7128 — [probe] recheck's emit-notices-before-abort egress disclosure has no testpackages/cli/src/nonInteractiveCli.ts:1369 — [probe] bridgeImagesForInlineOverride catch branch untestedpackages/cli/src/acp-integration/session/Session.ts:7517 — [review] 'maximum 4 per turn' audio cap is enforced per drained message on the mid-turn pathpackages/cli/src/acp-integration/session/Session.ts:7556 — [review] abort fail-closed path markers audio but silently strips imagespackages/cli/src/services/audio-bridge-service.ts:282 — [review] notice drops actionable runtime transcription errors after egresspackages/cli/src/services/audio-bridge-service.ts:119 — [review] oversized audio on an audio-capable target is silently clamped — no noticepackages/cli/src/ui/hooks/useGeminiStream.test.tsx:6681 — [review] client-steer re-bridge test pins a state production cannot produce (never settles)packages/cli/src/ui/hooks/useGeminiStream.test.tsx:7013 — [probe] recheck test never asserts the drained image survives into the final sendpackages/cli/src/ui/hooks/useGeminiStream.test.tsx:7679 — [probe] mid-drain reconcile test cannot distinguish re-bridge from segment droppackages/cli/src/ui/hooks/useGeminiStream.test.tsx:8289 — [probe] no test pins user text surviving the vision fail-closed fallbackpackages/cli/src/ui/hooks/useGeminiStream.ts:1555 — [review] fail-closed image branch's retry invariant/comment wrong for media-routed overridespackages/cli/src/nonInteractiveCli.test.ts:2305 — [probe] mixed-media test never pins runVisionBridge's parts argumentpackages/cli/src/ui/hooks/useGeminiStream.ts:2327 — [probe] tool-result gate skips inline/skill overrides that never routed media — silent slimming
Convergence: round 44 posted 1 inline comment(s), 1 of them reported for the first time; the previous round posted 8 (8 new). Findings keep coming back to the same files: packages/cli/src/ui/hooks/useGeminiStream.ts (findings in round 43; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
[Critical] R10-5 (still standing, carried from round 10, re-verified at this commit): packages/cli/src/nonInteractiveCli.ts — headless mid-loop and drain-item skill-tool overrides are stored bare (:2612, :2916) and sent bare (:2499, :2827); the send-time NUL exact-route suffix applies only to inlineModelOverride (:1683-1688). Core resolves exact routes only for trailing-NUL selectors, so requestModalities falls back to the session config and slimCompactionInput keeps inline audio when the SESSION model is audio-capable — preserved raw audio rides the continuation send to a text-only skill target with no marker and no notice, while the interactive/ACP twins gate media routing on the target's modalities. witness: round-38 probe reproduced the raw-audio ride end-to-end; re-verified statically at this commit in rounds 43 and 44 (code unchanged). Fix: route skill-override sends carrying surviving media through the target's modalities fail-closed (NUL exact route when media survives, or bridge/clamp before the bare send), matching the interactive/ACP twins.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 2)"：none — but one candidate examined and dropped, for the record: #processSlashCommandResult passes no getModelOverride to #resolvePrompt / #applyBridgeConver…；chunk 5：executing the four touched tests in Session.test.ts — the review worktree has no node_modules or built workspace dist/, and a full install + build exceeds the r…。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
收敛姿态下延后（第 44 轮，非阻断）——已记录，本轮不要求修改：共 16 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 44 轮发布了 1 条行内评论，其中 1 条是首次提出；上一轮发布了 8 条（其中 8 条首次提出）。发现反复回到同一批文件：packages/cli/src/ui/hooks/useGeminiStream.ts（第 43 轮已出过发现，本轮又有 1 条）。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。（仅为观察——本轮评审未因此扣留任何内容。）
[Critical] R10-5 (still standing, carried from round 10, re-verified at this commit): packages/cli/src/nonInteractiveCli.ts — headless mid-loop and drain-item skill-tool overrides are stored bare (:2612, :2916) and sent bare (:2499, :2827); the send-time NUL exact-route suffix applies only to inlineModelOverride (:1683-1688). Core resolves exact routes only for trailing-NUL selectors, so requestModalities falls back to the session config and slimCompactionInput keeps inline audio when the SESSION model is audio-capable — preserved raw audio rides the continuation send to a text-only skill target with no marker and no notice, while the interactive/ACP twins gate media routing on the target's modalities. witness: round-38 probe reproduced the raw-audio ride end-to-end; re-verified statically at this commit in rounds 43 and 44 (code unchanged). Fix: route skill-override sends carrying surviving media through the target's modalities fail-closed (NUL exact route when media survives, or bridge/clamp before the bare send), matching the interactive/ACP twins.
— qwen3.8-max via Qwen Code /review (v0.22.0)
Follow-up to 5709c7d (R44-1): replaceNestedFunctionResponseMedia replaced a structured (non-string) response.output with the fail-closed note, erasing the tool result while the note claimed only the media was omitted. Core's convertToFunctionResponse passes tool-supplied functionResponse parts through verbatim, so response.output can be structured; stringify it and append the note instead. Negative-control test pins the structured value surviving the fail-close.
|
Fixed in The first fix handled the string-output case but not the structured one: core's Negative control: |
fileUtils.ts (utils/) imported DEFAULT_MAX_AUDIO_BRIDGE_BYTES from core/inlineMediaLimit.ts, tripping the new architecture/no-core-utils-upward-import lint rule that requires utils/ to stay a leaf layer. Move the constant definition into a new leaf module utils/audio-bridge-constants.ts (no imports, mirrors vision-bridge-constants.ts) and re-export it from core/inlineMediaLimit.ts so the public API — including the core barrel and cli consumers — is unchanged.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): chunk 23: running the new drain/restore tests in packages/cli to confirm they pass (worktree lacks node_modules/dist; install+build exceeds budget).
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Deferred under the convergence posture (round 45, not a blocker) — recorded, not requested in this round:
packages/cli/src/acp-integration/session/Session.ts:5521 — [review] The cancelled-drain preservation sequence…packages/core/src/core/client.test.ts:7004 — [review] The new onSteerResolved contract test asserts only that…packages/cli/src/ui/hooks/useGeminiStream.test.tsx:5767 — [review] 'restores later messages when cancellation races with @…packages/cli/src/ui/hooks/useGeminiStream.test.tsx:7890 — [review] 'routes a top-level image an image-capable media-routed…packages/cli/src/ui/hooks/useGeminiStream.test.tsx:7898 — [review] 'does not report drain routing when the route failed…packages/cli/src/ui/hooks/useGeminiStream.ts:2050 — [review] applyToolResultMediaGate conflates a resolution FAILURE…packages/cli/src/ui/hooks/useGeminiStream.ts:2031 — [review] applyToolResultMediaGate early-returns unless…packages/cli/src/nonInteractiveCli.ts:1423 — [review] When the fail-closed capability check cannot resolve the…packages/cli/src/ui/hooks/useGeminiStream.test.tsx:855 (+2 locations) — [review] The capability-probe harness wiring (~15-25 lines:…packages/cli/src/ui/hooks/useGeminiStream.test.tsx:8422 — [review] The rewritten mid-turn vision-bridge-failure test dropped…packages/cli/src/nonInteractiveCli.ts:1385 — [review] The new bridgeImagesForInlineOverride catch branch…packages/cli/src/services/audio-bridge-service.ts:215 — [review] MAX_AUDIO_PARTS_PER_TURN ('maximum 4 per turn') is…packages/cli/src/acp-integration/session/Session.ts:5966 — [review] The empty-drain fallback branch in prepareBeforeCompressio…packages/cli/src/services/audio-bridge-service.test.ts:310 — [probe] The Sent ${result.egressCount} count in the…packages/cli/src/services/audio-bridge-service.ts:100 — [probe] formatAudioBridgeNotice 's zero-conversion branch…
Convergence: round 45 posted 3 inline comment(s), 3 of them reported for the first time; the previous round posted 1 (1 new). Findings keep coming back to the same files: packages/cli/src/ui/hooks/useGeminiStream.ts (findings in round 44; 2 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push keeps the loop from re-deriving the same set; this PR's reviews already resolve to a critical posting floor. (Observation only — nothing was withheld from this review because of this observation.)
[Critical] R10-5 (still standing, carried from round 10, re-verified at this commit): packages/cli/src/nonInteractiveCli.ts — headless mid-loop and drain-item skill-tool overrides are stored bare (:2600, :2904) and sent bare (:2815 and the mid-loop send); the send-time NUL exact-route suffix applies only to inlineModelOverride (:1671-1676). Core resolves exact routes only for trailing-NUL selectors, so requestModalities falls back to the session config and slimCompactionInput keeps inline audio when the SESSION model is audio-capable — preserved raw audio rides the continuation send to a text-only skill target with no marker and no notice, while the interactive/ACP twins gate media routing on the target's modalities. This round's diff does not touch the drain-item/mid-loop path (no hunks between :2400 and :2916). witness: round-38 probe reproduced the raw-audio ride end-to-end; re-verified statically at this commit in rounds 43, 44 and 45 (code unchanged). Fix: route skill-override sends carrying surviving media through the target's modalities fail-closed (NUL exact route when media survives, or bridge/clamp before the bare send), matching the interactive/ACP twins.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：chunk 23：running the new drain/restore tests in packages/cli to confirm they pass (worktree lacks node_modules/dist; install+build exceeds budget)。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
收敛姿态下延后（第 45 轮，非阻断）——已记录，本轮不要求修改：共 15 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 45 轮发布了 3 条行内评论，其中 3 条是首次提出；上一轮发布了 1 条（其中 1 条首次提出）。发现反复回到同一批文件：packages/cli/src/ui/hooks/useGeminiStream.ts（第 44 轮已出过发现，本轮又有 2 条）。新发现的产出速度没有下降。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送，可以避免循环反复推导同一组发现；本 PR 的评审已解析为 critical 发布下限。（仅为观察——本轮评审未因此扣留任何内容。）
[Critical] R10-5 (still standing, carried from round 10, re-verified at this commit): packages/cli/src/nonInteractiveCli.ts — headless mid-loop and drain-item skill-tool overrides are stored bare (:2600, :2904) and sent bare (:2815 and the mid-loop send); the send-time NUL exact-route suffix applies only to inlineModelOverride (:1671-1676). Core resolves exact routes only for trailing-NUL selectors, so requestModalities falls back to the session config and slimCompactionInput keeps inline audio when the SESSION model is audio-capable — preserved raw audio rides the continuation send to a text-only skill target with no marker and no notice, while the interactive/ACP twins gate media routing on the target's modalities. This round's diff does not touch the drain-item/mid-loop path (no hunks between :2400 and :2916). witness: round-38 probe reproduced the raw-audio ride end-to-end; re-verified statically at this commit in rounds 43, 44 and 45 (code unchanged). Fix: route skill-override sends carrying surviving media through the target's modalities fail-closed (NUL exact route when media survives, or bridge/clamp before the bare send), matching the interactive/ACP twins.
— qwen3.8-max via Qwen Code /review (v0.22.0)
…s, settle accepted hook-path steers
Round-45 follow-ups to the audio-bridge media routing:
R45-3: replaceNestedFunctionResponseMedia spread a non-plain-object
functionResponse.response (array/scalar/string returned by verbatim
pass-through tool results) into the rebuilt object, silently mangling it
(array -> numeric-keyed object) or erasing it (scalar spreads to {}).
Spread only plain objects; fold any other shape into the appended note via
stringifyStructuredToolOutput so the tool output survives the fail-close.
Negative-control test pins array/number/string responses surviving.
R45-2: the headless loop had no counterpart to useGeminiStream's
applyToolResultMediaGate, so nested functionResponse.parts media returned
mid-turn was exact-routed to the persisted NUL-stamped override and silently
placeholder-substituted by core slimming when the target lacked the
modality. Add the gate to the main-turn and drain-turn continuation sends:
detect, fail-closed-resolve the selector, substitute the unsupported
modality with a visible marker + emitBridgeNotice, clamp survivors.
Negative-control test pins the nested image being fail-closed instead of
riding the audio-only route raw.
R45-1: the hook-path drain installed onRestore but not the onAccept
counterpart handleResolvedSteer installs for the core-driven path, so an
accepted hook-path steer left its pristine payload in the retry store and
Ctrl+Y could re-inject it when a later continuation of the same request
failed. Install onAccept to hand back the superseded outer payload under the
identity guard, symmetric with handleResolvedSteer; guarded so a
continuation with no preceding payload keeps the composite as the recovery
channel. Negative-control test pins the retry restarting the outer payload
instead of re-delivering the accepted steer.
The nested-media helpers move to a shared leaf module
(utils/nested-function-response-media.ts) so the hook and the headless loop
gate with one implementation.
Round 45 — all 3 Critical findings verified real and fixed in
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): chunk 13: could not execute the two tests — no node_modules in the review worktree or parent checkout, and a full monorepo npm install + workspace builds exceeded the….
Not reviewed: reverse audit — stopped before round 5 by the review time budget.
Deferred under the convergence posture (round 46, not a blocker) — recorded, not requested in this round:
packages/cli/src/nonInteractiveCli.test.ts:1946 (+1 locations) — [review] Both clamp tests never assert the override selector's fate…packages/cli/src/nonInteractiveCli.test.ts:2188 (+1 locations) — [review] The audio fail-closed test never asserts the send's…packages/cli/src/nonInteractiveCli.test.ts:2510 (+1 locations) — [probe] 'gates nested tool-result media against the persisted…packages/cli/src/nonInteractiveCli.ts:1363 (+1 locations) — [review] The pre-existing per-turn vision-bridge else branch…packages/cli/src/nonInteractiveCli.ts:1428 (+1 locations) — [review] Headless resolution-failure branch silently drops an…packages/cli/src/nonInteractiveCli.ts:2994 (+1 locations) — [review] The headless drain-item call site of…packages/cli/src/ui/hooks/useGeminiStream.test.tsx:5493 (+1 locations) — [review] No test pins the (includeDeferred=false, goalTurnActive)…packages/cli/src/ui/hooks/useGeminiStream.test.tsx:5677 (+1 locations) — [review] 'drops a queued replacement prompt when a later goal…packages/cli/src/ui/hooks/useGeminiStream.test.tsx:5874 (+1 locations) — [review] 'restores later messages when cancellation races with @…packages/cli/src/ui/hooks/useGeminiStream.test.tsx:6336 (+1 locations) — [review] The failed-bridge drain send (send #1) never asserts the…packages/cli/src/ui/hooks/useGeminiStream.test.tsx:6353 (+2 locations) — [review] The retry-payload strip assertions in both audio-failure…packages/cli/src/ui/hooks/useGeminiStream.test.tsx:6379 (+1 locations) — [review] The re-drain redelivery assertions in 're-bridges mid-turn…packages/cli/src/ui/hooks/useGeminiStream.test.tsx:6569 (+1 locations) — [probe] 'delivers a failed drained steer exactly once when the…packages/cli/src/ui/hooks/useGeminiStream.test.tsx:6587 (+1 locations) — [probe] The history-side exactly-once claim is asserted only as…packages/cli/src/ui/hooks/useGeminiStream.test.tsx:7101 (+1 locations) — [probe] 'hands the retry store back to the outer payload once a…packages/cli/src/ui/hooks/useGeminiStream.test.tsx:7200 (+1 locations) — [review] 're-bridges a client-driven steer drain via Ctrl+Y when…packages/cli/src/ui/hooks/useGeminiStream.test.tsx:7201 (+1 locations) — [probe] 're-bridges a client-driven steer drain via Ctrl+Y' never…packages/cli/src/ui/hooks/useGeminiStream.test.tsx:7279 (+1 locations) — [probe] None of the new retry-store tests pins the hand-back…packages/cli/src/ui/hooks/useGeminiStream.test.tsx:7518 (+1 locations) — [probe] 'rechecks earlier drained audio against a later full-turn…packages/cli/src/ui/hooks/useGeminiStream.test.tsx:7541 (+2 locations) — [probe] In 'keeps a failed skill route fallback sticky across…- …and 6 more (see the run report)
Convergence: round 46 posted 3 inline comment(s), 3 of them reported for the first time; the previous round posted 3 (3 new). Findings keep coming back to the same files: packages/cli/src/nonInteractiveCli.ts (findings in round 45; 2 more now); packages/cli/src/ui/hooks/useGeminiStream.ts (findings in round 45; 1 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push keeps the loop from re-deriving the same set; this PR's reviews already resolve to a critical posting floor. (Observation only — nothing was withheld from this review because of this observation.)
[Critical] R10-5 (still standing, carried from round 10, re-verified at this commit): packages/cli/src/nonInteractiveCli.ts — headless mid-loop and drain-item skill-tool overrides are stored bare and sent bare; the send-time NUL exact-route suffix applies only to inlineModelOverride, so core resolves these sends without exact-route semantics: requestModalities falls back to the session config and slimCompactionInput keeps inline audio when the SESSION model is audio-capable — preserved raw audio rides the continuation send to a text-only skill target with no marker and no notice, while the interactive/ACP twins gate media routing on the target's modalities. This round's new applyToolResultMediaGate early-returns for non-NUL selectors (verified), so the bare-skill-override ride is unchanged. Witness: round-38 probe reproduced the raw-audio ride end-to-end; re-verified statically at this commit in rounds 43-45 and again this round (the NUL suffix applies only to inlineModelOverride, and the new gate early-returns for bare selectors). Fix: route skill-override sends carrying surviving media through the target's modalities fail-closed (NUL exact route when media survives, or bridge/clamp before the bare send), matching the interactive/ACP twins.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：chunk 13：could not execute the two tests — no node_modules in the review worktree or parent checkout, and a full monorepo npm install + workspace builds exceeded the…。
未审查：反向审计——评审时间预算不足，未能开始第 5 轮。
收敛姿态下延后（第 46 轮，非阻断）——已记录，本轮不要求修改：共 26 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 46 轮发布了 3 条行内评论，其中 3 条是首次提出；上一轮发布了 3 条（其中 3 条首次提出）。发现反复回到同一批文件：packages/cli/src/nonInteractiveCli.ts（第 45 轮已出过发现，本轮又有 2 条）；packages/cli/src/ui/hooks/useGeminiStream.ts（第 45 轮已出过发现，本轮又有 1 条）。新发现的产出速度没有下降。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送，可以避免循环反复推导同一组发现；本 PR 的评审已解析为 critical 发布下限。（仅为观察——本轮评审未因此扣留任何内容。）
[Critical] R10-5 (still standing, carried from round 10, re-verified at this commit): packages/cli/src/nonInteractiveCli.ts — headless mid-loop and drain-item skill-tool overrides are stored bare and sent bare; the send-time NUL exact-route suffix applies only to inlineModelOverride, so core resolves these sends without exact-route semantics: requestModalities falls back to the session config and slimCompactionInput keeps inline audio when the SESSION model is audio-capable — preserved raw audio rides the continuation send to a text-only skill target with no marker and no notice, while the interactive/ACP twins gate media routing on the target's modalities. This round's new applyToolResultMediaGate early-returns for non-NUL selectors (verified), so the bare-skill-override ride is unchanged. Witness: round-38 probe reproduced the raw-audio ride end-to-end; re-verified statically at this commit in rounds 43-45 and again this round (the NUL suffix applies only to inlineModelOverride, and the new gate early-returns for bare selectors). Fix: route skill-override sends carrying surviving media through the target's modalities fail-closed (NUL exact route when media survives, or bridge/clamp before the bare send), matching the interactive/ACP twins.
— qwen3.8-max via Qwen Code /review (v0.22.0)
…gates in headless path R46-1: settle accepts a steer as soon as its push lands — before any model content. When that send then errors with no model content, the accepted steer never reached the model, yet the onAccept hand-back plus the retry's orphan strip stranded it (neither delivered nor re-queued). Track per-send model content (StreamContentTracker) and undo the accept hand-back on a retryable no-content failure (undoAcceptedSteerHandBackIfNeeded) so Ctrl+Y's strip-then-push re-delivers the composite exactly once — for both the hook-path steer and the client-driven drain twin (handleResolvedSteer). R46-2: headless resolution-failure branch — with no vision bridge and a text-only session model, images rode the session route and were silently placeholder-substituted. Mirror the interactive R33-2 branch after the clamp block: replace the images with a visible "Image was not sent: …" marker and emit a bridge notice; correct the inaccurate comment. R46-3: headless applyToolResultMediaGate early-returned for any non-NUL selector, so bare fail-closed/text-only overrides shipped nested tool-result media raw. Gate against ANY persisted override selector (fail-close-resolving bare selectors, reusing the first-turn probe verdict). Tests: 2 hook steer-recovery regression tests, 1 headless fail-closed-image test, 1 headless bare-gate test (all fail with their fix disabled).
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): chunk 17: could not execute the new test to confirm it passes — the review worktree has no node_modules and a full npm ci + workspace npm run build (required by the…; chunk 18: could not execute useGeminiStream.test.tsx — the shared review worktree has no node_modules and installing there would disturb concurrent agents; verdict is fro…; chunk 19: executed the three tests under vitest to confirm green (worktree has no node_modules , and a full monorepo install + build exceeds the tool budget); verificati….
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Deferred under the convergence posture (round 47, not a blocker) — recorded, not requested in this round:
packages/cli/src/nonInteractiveCli.ts:3050 — [review] headless drain-item media gate has no paired testpackages/cli/src/ui/hooks/useGeminiStream.ts:5961 — [probe] skill-tool undefined-clear executing path untested (mutation survives)packages/cli/src/nonInteractiveCli.ts:1931 — [review] headless gate-time probe branch untestedpackages/cli/src/ui/hooks/useGeminiStream.test.tsx:5699 — [probe] goal-drain test missing handleSlashCommand dispatch assertionspackages/cli/src/ui/hooks/useGeminiStream.ts:1771 — [probe] NUL full-turn override: fail-closed else arm ships raw images into the exact routepackages/cli/src/nonInteractiveCli.test.ts:2584 — [review] headless gate tests don't pin functionResponse preservationpackages/cli/src/ui/hooks/useGeminiStream.test.tsx:6551 — [review] exactly-once test missing steer-text negative on retry payloadpackages/cli/src/ui/hooks/useGeminiStream.test.tsx:6587 — [review] exactly-once test's history assertion is >=1, not exactly onepackages/cli/src/ui/hooks/useGeminiStream.test.tsx:8517 — [probe] mid-drain fail-closed test can't see a dropped segmentpackages/cli/src/ui/hooks/useGeminiStream.test.tsx:9014 — [probe] text-retention assertion deleted by the bridge-failure test rewritepackages/cli/src/ui/hooks/useGeminiStream.ts:1683 — [probe] redundant second capability probe for the same routepackages/cli/src/ui/hooks/useGeminiStream.ts:1921 — [review] gate mislabels resolution failures as unsupported modalitiespackages/cli/src/nonInteractiveCli.test.ts:1885 — [probe] clamp tests don't pin the bare-selector invariantpackages/cli/src/ui/hooks/useGeminiStream.test.tsx:7132 — [probe] retry-payload assertions fabricated by the bridge mock
Convergence: round 47 posted 6 inline comment(s), 6 of them reported for the first time; the previous round posted 3 (3 new). Findings keep coming back to the same files: packages/cli/src/ui/hooks/useGeminiStream.ts (findings in round 46; 4 more now); packages/cli/src/nonInteractiveCli.ts (findings in round 46; 1 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push keeps the loop from re-deriving the same set; this PR's reviews already resolve to a critical posting floor. (Observation only — nothing was withheld from this review because of this observation.)
[Critical] R10-5 (carried from round 10, still standing at this commit — re-checked against HEAD 96fe03e): headless mid-loop and drain-item skill overrides are stored bare (modelOverride = override in the setter at ~:2731) and sent bare (mid-loop send ~:2618, drain-item send :2955); the send-time NUL exact-route suffix applies only to inlineModelOverride (:1711-1719). The nested tool-result media gate added this round polices only nested functionResponse.parts media for the override route — it does not stamp or gate TOP-LEVEL media on skill-override sends. Core resolves exact routes only for trailing-NUL selectors (geminiChat.ts ~:2566-2572), so requestModalities falls back to the session config and slimCompactionInput keeps inline audio when the SESSION model is audio-capable — preserved raw audio rides the continuation send to a text-only skill target with no marker and no notice, while the interactive/ACP twins gate media routing on the target's modalities. A round-38 probe reproduced the raw-audio ride end-to-end; the mechanism was re-verified statically at this commit. Fix: route skill-override sends carrying surviving media through the target's modalities fail-closed (NUL exact route when media survives, or bridge/clamp before the bare send), matching the interactive/ACP twins.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：chunk 17：could not execute the new test to confirm it passes — the review worktree has no node_modules and a full npm ci + workspace npm run build (required by the…；chunk 18：could not execute useGeminiStream.test.tsx — the shared review worktree has no node_modules and installing there would disturb concurrent agents; verdict is fro…；chunk 19：executed the three tests under vitest to confirm green (worktree has no node_modules , and a full monorepo install + build exceeds the tool budget); verificati…。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
收敛姿态下延后（第 47 轮，非阻断）——已记录，本轮不要求修改：共 14 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 47 轮发布了 6 条行内评论，其中 6 条是首次提出；上一轮发布了 3 条（其中 3 条首次提出）。发现反复回到同一批文件：packages/cli/src/ui/hooks/useGeminiStream.ts（第 46 轮已出过发现，本轮又有 4 条）；packages/cli/src/nonInteractiveCli.ts（第 46 轮已出过发现，本轮又有 1 条）。新发现的产出速度没有下降。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送，可以避免循环反复推导同一组发现；本 PR 的评审已解析为 critical 发布下限。（仅为观察——本轮评审未因此扣留任何内容。）
[Critical] R10-5 (carried from round 10, still standing at this commit — re-checked against HEAD 96fe03e): headless mid-loop and drain-item skill overrides are stored bare (modelOverride = override in the setter at ~:2731) and sent bare (mid-loop send ~:2618, drain-item send :2955); the send-time NUL exact-route suffix applies only to inlineModelOverride (:1711-1719). The nested tool-result media gate added this round polices only nested functionResponse.parts media for the override route — it does not stamp or gate TOP-LEVEL media on skill-override sends. Core resolves exact routes only for trailing-NUL selectors (geminiChat.ts ~:2566-2572), so requestModalities falls back to the session config and slimCompactionInput keeps inline audio when the SESSION model is audio-capable — preserved raw audio rides the continuation send to a text-only skill target with no marker and no notice, while the interactive/ACP twins gate media routing on the target's modalities. A round-38 probe reproduced the raw-audio ride end-to-end; the mechanism was re-verified statically at this commit. Fix: route skill-override sends carrying surviving media through the target's modalities fail-closed (NUL exact route when media survives, or bridge/clamp before the bare send), matching the interactive/ACP twins.
— qwen3.8-max via Qwen Code /review (v0.22.0)
…ld images
Rework the R46 steer-recovery machinery to key on settle OUTCOMES and drain
ORIGIN instead of lastPromptRef identity / enclosing-tracker state:
- handleResolvedSteer records settledVia ('accept'|'restore') on the undo
entry and invalidates it on restore: a restored drain is owned by the
re-queue, so the retry store is never re-armed alongside it (R47-1).
- handleResolvedSteer no longer gates on retryParts: the common drain shape
(text steers, clean media) stores the drain's parts and gets the same
swap/onAccept/onRestore/undo bookkeeping as the hook path (R47-2).
- Client drains mint a per-drain StreamContentTracker at resolution; the
client undo branch keys on it (plus owner-send identity) instead of the
enclosing send's tracker, which is always true at boundary-drain time
(R47-3).
- Hook-path onAccept is installed unconditionally: with no outer payload to
hand back (goal-first sessions) accept trims the steer segment from the
stored composite so a consumed steer is never re-injected by Ctrl+Y;
zero-content failures still re-arm the composite via the undo (R47-4).
- Tests drive core's real settle ordering (settleSteerInput in finally
before the enclosing failure surfaces, content before boundary drains);
the R46 test that never settled is replaced by faithful variants (R47-5).
- nonInteractiveCli no-vision-bridge branch appends a model-facing marker:
an image-only payload no longer collapses to an empty user turn (R47-6).
Negative controls verified for all six: each fix reverted individually
fails its regression test with the reported symptom.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): chunk 17: executing the three new tests in packages/cli (worktree has no node_modules/dist; install+build exceeds remaining tool budget) — correctness was verified by s….
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Deferred under the convergence posture (round 48, not a blocker) — recorded, not requested in this round:
packages/cli/src/nonInteractiveCli.test.ts:1839 — [review] three override-path tests never assert modelOverride — bare-vs-NUL semantics unpinned where conditional on surviving mediapackages/cli/src/nonInteractiveCli.test.ts:2657 — [review] exact-route persistence test never asserts the continuation's modelOverride — a stamp-drop regression ships greenpackages/cli/src/ui/hooks/useGeminiStream.test.tsx:3720 — [review] vacuous batchId contrast — committed setup-tool group existence never pinnedpackages/cli/src/ui/hooks/useGeminiStream.test.tsx:5677 — [probe] goal-clear drop test cannot observe its named behavior — removing '/goal clear' leaves it passingpackages/cli/src/ui/hooks/useGeminiStream.test.tsx:6217 (+8 locations) — [review] makeCompletedToolCall factory duplicated 7x across the drain-test cluster (+1 similar copy)packages/cli/src/ui/hooks/useGeminiStream.test.tsx:6579 — [review] exactly-once restore tests never re-check terminal state after re-drain deliverypackages/cli/src/ui/hooks/useGeminiStream.test.tsx:6923 — [review] retry re-bridge test missing raw-audio absence assertionpackages/cli/src/ui/hooks/useGeminiStream.test.tsx:7311 — [review] bridge mock fabricates the functionResponse output — testTool assertion blind to the stored compositepackages/cli/src/ui/hooks/useGeminiStream.test.tsx:9299 — [probe] text-retention assertion deleted by the bridge-failure test rewrite (mutation ships green)
Convergence: round 48 posted 4 inline comment(s), 4 of them reported for the first time; the previous round posted 6 (6 new). Findings keep coming back to the same files: packages/cli/src/ui/hooks/useGeminiStream.ts (findings in round 47; 4 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
[Critical] R10-5 (carried from round 10, still standing at HEAD ba76096 — re-checked against the code this round): headless mid-loop and drain-item skill overrides are stored bare (modelOverride = override in the setter at ~:2743) and sent bare (mid-loop send :2630, drain-item send :1900) polices only nested :2966); the NUL exact-route suffix at send time (:1727) applies only to inlineModelOverride. The nested tool-result media gate (functionResponse.parts media — it does not stamp or gate TOP-LEVEL media on skill-override sends. Core resolves exact routes only for trailing-NUL selectors (geminiChat.ts ~:2565-2570), so requestModalities falls back to the session config and slimCompactionInput keeps inline audio when the SESSION model is audio-capable — preserved raw audio rides the continuation send to a text-only skill target with no marker and no notice, while the interactive/ACP twins gate media routing on the target's modalities. The author reply claims a fix in 9a3e34f, but the code at HEAD still shows bare skill-override storage/sends and an inline-only NUL stamp — the mechanism still fires. Fix: route skill-override sends carrying surviving media through the target's modalities fail-closed (NUL exact route when media survives, or bridge/clamp before the bare send), matching the interactive/ACP twins.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：chunk 17：executing the three new tests in packages/cli (worktree has no node_modules/dist; install+build exceeds remaining tool budget) — correctness was verified by s…。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
收敛姿态下延后（第 48 轮，非阻断）——已记录，本轮不要求修改：共 9 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 48 轮发布了 4 条行内评论，其中 4 条是首次提出；上一轮发布了 6 条（其中 6 条首次提出）。发现反复回到同一批文件：packages/cli/src/ui/hooks/useGeminiStream.ts（第 47 轮已出过发现，本轮又有 4 条）。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。（仅为观察——本轮评审未因此扣留任何内容。）
[Critical] R10-5 (carried from round 10, still standing at HEAD ba76096 — re-checked against the code this round): headless mid-loop and drain-item skill overrides are stored bare (modelOverride = override in the setter at ~:2743) and sent bare (mid-loop send :2630, drain-item send :1900) polices only nested :2966); the NUL exact-route suffix at send time (:1727) applies only to inlineModelOverride. The nested tool-result media gate (functionResponse.parts media — it does not stamp or gate TOP-LEVEL media on skill-override sends. Core resolves exact routes only for trailing-NUL selectors (geminiChat.ts ~:2565-2570), so requestModalities falls back to the session config and slimCompactionInput keeps inline audio when the SESSION model is audio-capable — preserved raw audio rides the continuation send to a text-only skill target with no marker and no notice, while the interactive/ACP twins gate media routing on the target's modalities. The author reply claims a fix in 9a3e34f, but the code at HEAD still shows bare skill-override storage/sends and an inline-only NUL stamp — the mechanism still fires. Fix: route skill-override sends carrying surviving media through the target's modalities fail-closed (NUL exact route when media survives, or bridge/clamp before the bare send), matching the interactive/ACP twins.
— qwen3.8-max via Qwen Code /review (v0.22.0)
… and retryable R48-1: extend the image capability probe and the no-bridge fail-closed gate to any active override (drop the inline/stamp restriction) so unstamped skill-tool overrides no longer escape media policing (mirror the headless twin). R48-2: the drain recheck pass only fills an absent pristine capture, keeping the first-pass full-pristine media for Retry instead of the degraded variant. R48-3: thread the accumulated resolution failure into each drain segment (inheritedResolutionFailed) so clamp / fail-closed protections persist across segments in both drain passes. R48-4: add overrideCleared to the drain/SteerInput contract; core steerRouteOverride degrades to the session model instead of resurrecting the cleared pre-drain selector.
# Conflicts: # packages/cli/src/acp-integration/session/Session.test.ts # packages/cli/src/acp-integration/session/Session.ts
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Deferred under the convergence posture (round 49, not a blocker) — recorded, not requested in this round:
packages/cli/src/services/audio-bridge-service.ts:284 — [review] Bridge notice never surfaces the sanitized per-part failure reasonpackages/cli/src/ui/hooks/useGeminiStream.ts:1961 (+2 locations) — [review] Both media gates blame 'does not support' when the override failed to resolve (TUI + headless twin)docs/design/2026-08-01-audio-bridge.md:31 — [review] Design doc Scope contradicts the tool-result media gate this PR addspackages/cli/src/services/audio-bridge-service.ts:120 — [review] The 4-audio-per-turn cap does not apply to the native passthrough branchpackages/cli/src/ui/hooks/useGeminiStream.test.tsx:6599 — [review] Exactly-once steer test's transcript-side assertion is too weak to catch a double-addpackages/cli/src/ui/hooks/useGeminiStream.test.tsx:9873 — [review] Rewritten fail-closed test dropped the only user-text-survival assertionpackages/cli/src/ui/hooks/useGeminiStream.test.tsx:8138 — [review] Recheck test never asserts the already-routed image survives the recheckpackages/cli/src/ui/hooks/useGeminiStream.test.tsx:885 — [review] Capability-route mock scaffold copy-pasted across 13 new tests
Convergence: round 49 posted 5 inline comment(s), 5 of them reported for the first time; the previous round posted 4 (4 new). Findings keep coming back to the same files: packages/cli/src/ui/hooks/useGeminiStream.ts (findings in round 48; 4 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push keeps the loop from re-deriving the same set; this PR's reviews already resolve to a critical posting floor. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
收敛姿态下延后（第 49 轮，非阻断）——已记录，本轮不要求修改：共 8 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 49 轮发布了 5 条行内评论，其中 5 条是首次提出；上一轮发布了 4 条（其中 4 条首次提出）。发现反复回到同一批文件：packages/cli/src/ui/hooks/useGeminiStream.ts（第 48 轮已出过发现，本轮又有 4 条）。新发现的产出速度没有下降。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送，可以避免循环反复推导同一组发现；本 PR 的评审已解析为 critical 发布下限。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.0)
…rs on auth errors, gate bare overrides and untyped nested media - R49-1: stamp mediaRoutedPromptIdRef in the core-driven boundary drain's onSteerResolved wrapper when the drain routed media, so the steered turn's media-free tool continuations keep the exact route (mirrors the hook-path drain's metadata.steerMediaRouted stamp). Without it they send the bare selector, re-enabling the fallback chain mid-turn. - R49-2: run undoAcceptedSteerHandBackIfNeeded and set lastPromptErroredRef in the UnauthorizedError catch branch too, so an accepted-but-never-seen steer keeps its Ctrl+Y recovery channel on 401 exactly like any other non-abort failure. - R49-3: gate ANY active override in applyToolResultMediaGate (the headless twin already does, R46-3) — bare inline/skill overrides own continuation sends too, and their bare selectors resolve request modalities from the session config, silently placeholder-substituting nested media. When the override supports a detected modality, establish the route so surviving nested media exact-routes to the override instead of being slimmed against session modalities. - R49-4: snapshot both undo branches' preconditions before writing lastPromptRef; when a hook-path steer AND a client-driven drain both settled accepted without model content, store the merged payload so the retry's strip-then-push re-delivers both steers exactly once. - R49-5: detect MIME-less nested media carriers (hasUntyped) and fail them closed visibly in both gates — core's slimming resolves a missing MIME to DEFAULT_MIME, which matches no modality and placeholder-substitutes the media on every route.
# Conflicts: # packages/cli/src/acp-integration/session/Session.ts
…is union-safe The Retry branch ternary unions the gate result with an inline fallback object; TS2339 flagged gated.mediaRouted because only the awaited gate member declared it (the literal-branch property normalization applies only when both ternary arms are object literals). Add mediaRouted: undefined to the fallback member, matching the gate's optional property.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Unresolved, please confirm:
- [Critical] 128 older-round inline blocker threads (rounds 1-48): every anchored blocker is either outdated (anchored lines rewritten) or on files changed since the comment, and individual mechanism traces of all 128 were not feasible within this round…
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): chunk 18: none — though I did not execute the test suite (logic traced statically; assertions are internally consistent), and test 2's accept() generator and final asse…; chunk 23: none — no checks were cut short.; chunk 4: executing the new tests ( cd packages/cli && npx vitest run src/acp-integration/session/Session.test.ts ) to confirm they pass — the review worktree has no nod….
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Deferred under the convergence posture (round 50, not a blocker) — recorded, not requested in this round:
packages/cli/src/acp-integration/session/Session.test.ts:10350 — [review] image-capable no-bridge test doesn't pin the flag's absencepackages/cli/src/ui/hooks/useGeminiStream.test.tsx:8011 — [review] no-retryParts steer test misses the outer-prompt-absence assertionpackages/cli/src/utils/nested-function-response-media.ts:29 — [review] walkers throw TypeError on null/undefined nested part entriespackages/cli/src/utils/nested-function-response-media.ts:130 — [review] new 236-line gate module has zero testspackages/cli/src/nonInteractiveCli.ts:1975 — [review] hasUntyped fail-closed branch has zero test coveragepackages/cli/src/services/audio-bridge-service.ts:186 — [review] actionable config errors truncated at 200 chars, losing the escape hatchpackages/cli/src/acp-integration/session/Session.ts:7975 — [review] abort after a successful conversion discards completed transcriptspackages/cli/src/ui/hooks/useGeminiStream.ts:1916 — [review] no-override gate path: unclamped nested blobs and silent-slim after visible fail-closepackages/cli/src/ui/hooks/useGeminiStream.ts:1458 — [review] fail-closed retry re-derivation dead under skill-tool overridespackages/cli/src/ui/hooks/useGeminiStream.ts:1990 — [review] gate conflates resolution failure with 'does not support'packages/cli/src/nonInteractiveCli.test.ts:2686 — [review] headless exact-route test never pins the continuation's stamppackages/cli/src/nonInteractiveCli.ts:2008 — [review] headless gate clamp has no witness (TUI twin is pinned)packages/cli/src/ui/hooks/useGeminiStream.test.tsx:6602 — [review] exactly-once steer test asserts history with toBeGreaterThan(0)packages/cli/src/ui/hooks/useGeminiStream.test.tsx:7117 — [review] hook-path hand-back test's steer-absence guard is vacuouspackages/cli/src/ui/hooks/useGeminiStream.test.tsx:9225 — [review] R48-2 retryParts witness pins the audio half onlypackages/cli/src/ui/hooks/useGeminiStream.test.tsx:9963 — [review] fail-closed textOnly spread has no witnesspackages/cli/src/ui/hooks/useGeminiStream.ts:5285 — [review] UnauthorizedError recovery channel (R49-2 fix) has zero test coveragedocs/design/2026-08-01-audio-bridge.md:34 — [review] design doc claims nested tool-result audio is unchanged — this PR changes itpackages/cli/src/acp-integration/session/Session.test.ts:14767 — [review] recheck test never pins sibling-image survival on abort
Convergence: round 50 posted 3 inline comment(s), 2 of them reported for the first time; the previous round posted 5 (5 new). Findings keep coming back to the same files: packages/cli/src/ui/hooks/useGeminiStream.ts (findings in round 49; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)
[Critical] R50-3 (media-route stamp lifecycle): the inline location (useGeminiStream.ts:4605-4607) is occupied by the open R43-10 thread, so this confirmed class finding posts in the body. The media-route prompt stamp (mediaRoutedPromptIdRef) is a single session-wide slot whose write lifecycle does not match turn ownership — three confirmed modes: (a) OVERWRITE — a concurrent ?btw side question admitted during Responding skips the turn-start reset and unconditionally overwrites the foreground turn's stamp, contradicting the invariant in the comment directly above the write; (b) LINGERING — the write precedes the !shouldProceed / goal-claim-deferred early returns while startNewPrompt() advances AFTER them, so a send that stamped then aborted (Ctrl+C during the vision phase after audio routed) leaves a live stamp on a prompt_id the counter re-mints — every later media-free Notification drain exact-routes into the stale route's fail-closed resolution until the next user turn; (c) RETRY — a Ctrl+Y retry under an established route mints a fresh prompt_id but never re-stamps (the retry branch reports mediaRouted only when establishingRoute, false for an established route; retryLastPrompt passes no metadata), so the retried turn's media-free continuations send the bare selector and split the media-routed turn across models. Deterministic ref-state traces at the reviewed commit; exactly two stamp writers exist (:4606 and the onSteerResolved drain writer ~:5052) and neither fires for modes (b)/(c). Fix: scope stamp ownership to the owning turn — guard the write against sends that skip the turn-start reset, move it below the early returns, and re-stamp when a Retry belongs to an established media route; witness: three tests beside 'does not exact-route a media-free notification onto a retry stamp', one per mode, each going red when its guard is removed.
中文说明
仅完成部分审查，审查缺口已披露。
未决，请确认：共 1 条（原文未翻译，列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：chunk 18：none — though I did not execute the test suite (logic traced statically; assertions are internally consistent), and test 2's accept() generator and final asse…；chunk 23：none — no checks were cut short.；chunk 4：executing the new tests ( cd packages/cli && npx vitest run src/acp-integration/session/Session.test.ts ) to confirm they pass — the review worktree has no nod…。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
收敛姿态下延后（第 50 轮，非阻断）——已记录，本轮不要求修改：共 19 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 50 轮发布了 3 条行内评论，其中 2 条是首次提出；上一轮发布了 5 条（其中 5 条首次提出）。发现反复回到同一批文件：packages/cli/src/ui/hooks/useGeminiStream.ts（第 49 轮已出过发现，本轮又有 1 条）。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。（仅为观察——本轮评审未因此扣留任何内容。）
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff，除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上；并会一直如此，直到某一轮的标记重新带上锚点，或落地的嫁接能被运行该轮的评审使用。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
[Critical] R50-3 (media-route stamp lifecycle): the inline location (useGeminiStream.ts:4605-4607) is occupied by the open R43-10 thread, so this confirmed class finding posts in the body. The media-route prompt stamp (mediaRoutedPromptIdRef) is a single session-wide slot whose write lifecycle does not match turn ownership — three confirmed modes: (a) OVERWRITE — a concurrent ?btw side question admitted during Responding skips the turn-start reset and unconditionally overwrites the foreground turn's stamp, contradicting the invariant in the comment directly above the write; (b) LINGERING — the write precedes the !shouldProceed / goal-claim-deferred early returns while startNewPrompt() advances AFTER them, so a send that stamped then aborted (Ctrl+C during the vision phase after audio routed) leaves a live stamp on a prompt_id the counter re-mints — every later media-free Notification drain exact-routes into the stale route's fail-closed resolution until the next user turn; (c) RETRY — a Ctrl+Y retry under an established route mints a fresh prompt_id but never re-stamps (the retry branch reports mediaRouted only when establishingRoute, false for an established route; retryLastPrompt passes no metadata), so the retried turn's media-free continuations send the bare selector and split the media-routed turn across models. Deterministic ref-state traces at the reviewed commit; exactly two stamp writers exist (:4606 and the onSteerResolved drain writer ~:5052) and neither fires for modes (b)/(c). Fix: scope stamp ownership to the owning turn — guard the write against sends that skip the turn-start reset, move it below the early returns, and re-stamp when a Retry belongs to an established media route; witness: three tests beside 'does not exact-route a media-free notification onto a retry stamp', one per mode, each going red when its guard is removed.
— qwen3.8-max via Qwen Code /review (v0.22.2)
| function nestedPartCarriesMedia(inner: Part): boolean { | ||
| const hasInline = | ||
| typeof inner.inlineData?.data === 'string' && |
There was a problem hiding this comment.
[Critical] R49-5: The tool-result media gates are still keyed on a hand-enumerated MIME class list (image/, audio/, missing-MIME) over an unbounded entrance space of tool-supplied MIME strings — this round re-confirmed the finding with four new demonstrated corners. Any carrier outside that list — video/*, application/pdf, application/octet-stream (the MCP default for a MIME-less embedded resource), an empty-string mimeType (MCP image/audio blocks pass mimeType through verbatim), or a depth-2 nested functionResponse carrier — bypasses both gates (TUI useGeminiStream.ts:1924, headless nonInteractiveCli.ts:1929) entirely: no visible fail-closed marker, no ERROR item, no clamp. Core route slimming then placeholder-substitutes the media on the exact route, so the model answers about tool-result media it never received with nothing disclosed to the user. Probe against this commit's built modules: detectNestedFunctionResponseMedia returns {hasImage:false, hasAudio:false, hasUntyped:false} for video/mp4, application/octet-stream and '' (the missing-MIME control reports hasUntyped:true), and slimCompactionInput substitutes [document: video/mp4] / [document: ] placeholders — for ''-MIME on every route, including an all-capable one. Enumerating MIME prefixes never converges: key the gates on carrier presence (any nested media carrier), classify each MIME against the resolved route's authoritative modality map (core supportsMimeType semantics, normalizing empty/whitespace MIME to absent), fail closed visibly for anything the route cannot consume, and recurse into nested functionResponse parts the way core's transformPart does so the marker and the byte ceiling hold at every depth slimming can reach. A colocated unit test should assert detect reports carriers for video/mp4, octet-stream, ''-MIME and depth-2 shapes and that replace/clamp substitute them; removing the classification must turn it red.
中文说明
[Critical] R49-5：工具结果媒体门控仍然基于手工枚举的 MIME 类别清单（image/、audio/、缺失 MIME）来检查工具提供的、无界的 MIME 字符串输入空间——本轮审计重新确认了该问题，并新增四个已实证的绕过入口。凡不在该清单内的载体——video/*、application/pdf、application/octet-stream（MCP 对未声明 MIME 的内嵌资源的默认值）、空字符串 mimeType（MCP 图像/音频块原样透传 mimeType）、二层嵌套的 functionResponse 载体——都会完全绕过两个门控（TUI useGeminiStream.ts:1924、headless nonInteractiveCli.ts:1929）：没有可见的失败关闭标记、没有 ERROR 项、也没有字节上限检查。随后核心路由瘦身会在精确路由上把该媒体替换为占位符，模型实际上是在回答它从未收到的工具结果媒体，而用户看不到任何提示。已在本 commit 构建的模块上探针验证：detectNestedFunctionResponseMedia 对 video/mp4、application/octet-stream、'' 均返回 {hasImage:false, hasAudio:false, hasUntyped:false}（缺失 MIME 的对照组会报告 hasUntyped:true），且 slimCompactionInput 会把这些载体替换为 [document: video/mp4] / [document: ] 占位符——空字符串 MIME 在所有路由（包括全能力路由）上都会被替换。枚举 MIME 前缀永远不会收敛：建议把门控改为基于载体存在性（任意嵌套媒体载体），按解析出的路由的权威模态表（核心 supportsMimeType 语义，把空/空白 MIME 归一化为缺失）对每个 MIME 分类，对路由无法消费的载体做可见的失败关闭替换，并像核心 transformPart 一样递归进入嵌套的 functionResponse parts，使标记与字节上限在瘦身可达的每一层都生效。请补充同目录单元测试，断言 detect 能报告 video/mp4、octet-stream、''-MIME 和二层嵌套形状的载体、且 replace/clamp 会替换它们；移除该分类逻辑后测试必须变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| // Clamp whatever nested media survives the gate — the exact-route path | ||
| // would otherwise skip QWEN_CODE_MAX_INLINE_MEDIA_BYTES. | ||
| return normalizePartList(clampNestedFunctionResponseMedia(result)); |
There was a problem hiding this comment.
[Critical] The headless applyToolResultMediaGate validates nested tool-result media against the override but never establishes the exact route for the continuation: when media survives under a BARE persisted selector, nothing NUL-stamps modelOverride — the TUI twin sets mediaRoutedOverrideRef and returns mediaRouted: true, which stamps \0 at send time. The continuation sends the bare selector, core resolves requestModalities from the SESSION config, and slimCompactionInput silently placeholder-substitutes the very media the gate just validated. Concretely: a headless run with a text-only session model and an inline override to a media-capable model whose first turn carries no media (modelOverride persists bare — the init stamp at :1725-1730 fires only when first-turn media survives); a tool mid-turn returns an image nested in functionResponse.parts; the gate resolves the bare selector, sees the override supports images and passes the image through with no notice; the continuation send passes the bare selector unchanged; the nested image becomes an [image: image/png] placeholder — the model answers about an image it never received, with nothing on stderr or in the JSON stream. Probe against this commit's slimCompactionInput: session modalities {} → the nested image becomes [{"text":"[image: image/png]"}] while the image route survives; grep confirms no send-time \0 stamping exists in this file. Have the gate report whether supported media survived (e.g. return { parts, mediaSurvived }) and NUL-stamp the persisted selector at both call sites when media survived under a bare selector (modelOverride = ${routeSelector}\0``, same for the drain loop's itemModelOverride), so the continuation exact-routes and request modalities come from the resolved route. Extend `gates nested tool-result media against the persisted exact-route override` to assert the continuation send's `modelOverride` ends with `\0` — mutation-tested: withholding the stamp on continuations keeps every existing test green, and the added assertion turns that mutant red.
中文说明
[Critical] headless 的 applyToolResultMediaGate 会按 override 校验嵌套的工具结果媒体，但从不为后续请求（continuation）建立精确路由：当媒体在 BARE（未加 NUL 标记的）持久化选择器下存活时，没有任何代码给 modelOverride 加 \0 标记——而 TUI 的对应实现会设置 mediaRoutedOverrideRef 并返回 mediaRouted: true（发送时打 \0）。于是 continuation 以裸选择器发送，核心按 SESSION 配置解析 requestModalities，slimCompactionInput 会把门控刚刚校验过的那个媒体静默替换为占位符。具体场景：文本-only 会话模型 + 内联 override 到一个支持媒体的模型、且第一轮不带媒体（modelOverride 保持裸状态——:1725-1730 的初始打点只在第一轮媒体存活时触发）；工具在回合中途返回嵌套在 functionResponse.parts 里的图像；门控解析裸选择器、确认 override 支持图像后放行且不发任何提示；continuation 原样以裸选择器发送；嵌套图像变成 [image: image/png] 占位符——模型回答了一张它从未收到的图像，而 stderr 和 JSON 流上什么都没有。已在本 commit 的 slimCompactionInput 上探针验证：会话模态为 {} 时嵌套图像变成 [{"text":"[image: image/png]"}]，而 image 路由可以存活；grep 确认本文件中不存在发送时的 \0 打点。建议让门控返回媒体是否存活（如 { parts, mediaSurvived }），并在媒体于裸选择器下存活时对两个调用点的持久化选择器加 \0 标记（modelOverride = ${routeSelector}\0``，drain 循环中的 itemModelOverride 同理），使 continuation 走精确路由、请求模态取自解析出的路由。请在 `gates nested tool-result media against the persisted exact-route override` 中补充断言：continuation 发送的 `modelOverride` 以 `\0` 结尾——已做变异测试：从 continuation 上剥离该标记时现有测试全部仍为绿色，加上该断言后该变异变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| metadata?.onAdmissionFailed?.(); | ||
| throw error; |
There was a problem hiding this comment.
[Critical] submitQuery's prepare-throw catch fires only onAdmissionFailed, never onDeliveryFailed — so a tool continuation whose prepare throws never calls drainedSteer?.restore(), the only channel that re-queues steer messages the continuation destructively drained from the queue. The new applyToolResultMediaGate introduces the first throwing path into ToolResult prepare: the walker throws TypeError on a null/undefined nested part entry (untyped extension-supplied parts that core passes through verbatim — probe on this commit: detectNestedFunctionResponseMedia({functionResponse:{parts:[null]}}) → TypeError: Cannot read properties of null (reading 'inlineData')). The drained user message is neither sent, recorded, re-queued, nor Ctrl+Y-retryable (lastPromptRef is written only after prepare succeeds): the user's typed input silently vanishes behind a generic admission-failed error. Every other submitQuery failure branch (concurrency guards, !shouldProceed, stream errors) calls onDeliveryFailed; only this catch skips it, and before this PR, function-response prepare could not throw. Re-queue the attached steer before rethrowing: add metadata?.steerInput?.restore(); next to metadata?.onAdmissionFailed?.(); (restore is idempotent via its settled flag), matching every other delivery-failure path. Add a test: with a mid-turn message queued, drive a tool continuation whose tool result nests a null part entry so the gate throws, and assert the drained text is re-queued after submitQuery rejects; removing the added restore call must turn it red.
中文说明
[Critical] submitQuery 的 prepare 抛错 catch 只触发 onAdmissionFailed，从不触发 onDeliveryFailed——因此 prepare 抛错的工具 continuation 从不调用 drainedSteer?.restore()，而这是把被该 continuation 从队列中破坏性取出（destructively drained）的 steer 消息重新入队的唯一通道。新增的 applyToolResultMediaGate 给 ToolResult prepare 引入了第一个会抛异常的路径：walker 在嵌套 part 条目为 null/undefined 时抛 TypeError（core 会原样透传扩展提供的未定型 parts——在本 commit 上探针验证：detectNestedFunctionResponseMedia({functionResponse:{parts:[null]}}) → TypeError: Cannot read properties of null (reading 'inlineData')）。被取出的用户消息既不会被发送、不会被记录、不会重新入队、也无法 Ctrl+Y 重试（lastPromptRef 只在 prepare 成功后写入）：用户键入的输入在一个笼统的 admission-failed 错误背后静默消失。submitQuery 的其他每个失败分支（并发守卫、!shouldProceed、流错误）都会调用 onDeliveryFailed，唯独这个 catch 没有；而且在本 PR 之前，function-response 的 prepare 不可能抛异常。建议在重新抛出前先把附带的 steer 重新入队：在 metadata?.onAdmissionFailed?.(); 旁加上 metadata?.steerInput?.restore();（restore 通过 settled 标志保证幂等），与其他所有投递失败路径保持一致。请补充测试：队列中存在回合中途消息时，驱动一个工具结果嵌套 null part 条目、使门控抛错的工具 continuation，并断言 submitQuery 拒绝后被取出的文本已重新入队；移除新增的 restore 调用后该测试必须变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
7 participants
What this PR does
Adds an audio bridge for user-supplied attachments when the primary model does not support audio. Interactive and headless
@attachments and ACP audio prompts are transcribed through the configured batch voice model, then replaced with an explicitly untrusted machine-transcription block before the primary-model request.The bridge leaves audio unchanged for audio-capable primary models, reports when audio is sent to the transcription provider, and fails closed with a safe unavailable-content note when configuration, size limits, cancellation, or transcription prevents conversion. It also handles IPv4-translated IPv6 DNS answers used by local proxy resolvers without weakening private-network rejection.
Why it's needed
Qwen Code already has a vision bridge for images that a text-only primary model cannot consume, but equivalent user-supplied audio was either dropped as unsupported or only handled on a narrower ACP path. This makes configured voice transcription consistently available across the main prompt entry points while preventing raw unsupported audio or untrusted transcript instructions from silently reaching the primary model.
Reviewer Test Plan
How to verify
/model --voice, select a text-only primary model, and attach an MP3 or WAV file with@in both interactive and headless modes. Confirm that an audio-egress notice appears and that the primary model answers from the transcript without receiving raw audio.@audio path under the same configuration. Confirm that it follows the same conversion and disclosure behavior.Evidence (Before & After)
Before: a text-only primary model could not use an interactive or headless
@audio attachment and reported the audio as unavailable.After: a local headless MP3 run emitted
Converted 1 audio file(s) to text via qwen3-asr-flash. Your audio was sent to that model.and the primary model returned the expected transcript, including the spoken verification code4279.Tested on
Environment (optional)
macOS, Node.js 24, source CLI through
npm run dev, local fake OpenAI-compatible endpoints for deterministic end-to-end coverage, and a live DashScopeqwen3-asr-flashMP3 transcription smoke test.Risk & Scope
/model --voicecommand are reused.Linked Issues
N/A
中文说明
本 PR 做了什么
为主模型不支持音频时的用户附件新增 audio bridge。交互式和 headless 的
@附件以及 ACP 音频提示会通过已配置的批量语音模型转录，并在发给主模型前替换成明确标注为不可信的机器转录文本块。当主模型本身支持音频时，bridge 会保持音频不变；当音频被发送给转录服务商时会向用户提示；如果配置、大小限制、取消或转录失败导致无法转换，则会安全失败，只提供内容不可用的说明。本 PR 还兼容本地代理 DNS 返回的 IPv4-translated IPv6 地址，同时不会放宽对私网地址的拒绝。
为什么需要
Qwen Code 已经可以通过 vision bridge 处理纯文本主模型无法消费的图片，但用户提供的音频此前要么被作为不支持的内容丢弃，要么只在较窄的 ACP 路径中处理。本 PR 让已配置的语音转录在主要提示入口中保持一致，同时避免原始的不支持音频或转录内容中的不可信指令静默进入主模型。
Reviewer Test Plan
如何验证
/model --voice配置批量转录模型，选择纯文本主模型，并在交互式与 headless 模式中通过@附加 MP3 或 WAV 文件。确认界面出现音频出站提示，且主模型基于转录作答而没有收到原始音频。@音频路径的 ACP 提示。确认其转换与披露行为一致。证据（修改前后）
修改前：纯文本主模型无法使用交互式或 headless 的
@音频附件，并会报告音频内容不可用。修改后：本地 headless MP3 测试输出了
Converted 1 audio file(s) to text via qwen3-asr-flash. Your audio was sent to that model.，主模型随后返回了预期转录，其中包含语音验证码4279。测试平台
环境（可选）
macOS、Node.js 24、通过
npm run dev运行源码 CLI、使用本地模拟 OpenAI-compatible 端点进行确定性的端到端覆盖，并使用真实 DashScopeqwen3-asr-flash完成 MP3 转录 smoke test。风险与范围
/model --voice命令。关联 Issue
N/A