fix(java): read control response subtype from payload - #9924
Conversation
|
Thanks for the PR! Template looks good ✓ Problem: real and documented. The linked tracking issue #8835 carries the Direction: aligned — wire-format correctness for the Java SDK; a session reader that cannot observe control errors misreads its own protocol. No direct Java SDK entry in the upstream CHANGELOG, but the area is clearly in scope. Size: not a core path ( Approach: right-sized v2. The earlier iteration that stopped the turn on a control error (the round-1 Critical) is gone, replaced by a small Risk: no elevated risk signals (no high-risk path matches). Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：真实且已记录在案。关联跟踪 issue #8835 中的 方向：对齐——这是 Java SDK 的 wire format 正确性；无法感知 control error 的 session 读取器是在误读自己的协议。上游 CHANGELOG 没有直接提及 Java SDK，但该领域显然在范围内。 规模：非核心路径（ 方案：恰当的 v2。早先"遇到 control error 即停止当前 turn"的迭代（第 1 轮 Critical）已被替换：新增一个小的 风险：无升级风险信号（未命中高风险路径）。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewIndependent take before reading the diff: derive the subtype from the nested What I checked:
No blockers, no convention violations. Round 7's three recorded deferrals — Test evidence (PR's own CI — static review only; per gate rules PR code is never executed here)All checks on the reviewed commit have completed — nothing pending, nothing failing. The full Java matrix is green (ubuntu Java 11/17/21, macOS Java 21, Windows Java 21), and the daemon E2E, Node unit lane, integration lanes, and scans all passed. The remaining checks on this commit are conditional bot-orchestration jobs that completed as skipped.
From the The author's machine has no JVM/Maven — that is their stated circumstance, not evidence; the authoritative runtime signal is the five-platform matrix above. Nothing here is user-visible (SDK-internal parsing and a log line), so no TUI/real-scenario capture applies. 中文说明代码审查读 diff 之前的独立方案：从嵌套的 核查内容：
无阻断项，无规范违规。第 7 轮记录的 3 条延后项—— 测试证据（PR 自己的 CI——仅静态审查；按门禁规则此处绝不执行 PR 代码）被审查 commit 上的所有检查已完成——无 pending、无失败。Java 全矩阵绿（ubuntu Java 11/17/21、macOS Java 21、Windows Java 21），daemon E2E、Node 单元、集成通道和扫描全部通过；该 commit 上其余检查为条件性 bot 编排任务，均为 skipped。CI 表格见英文版上方（机器可读区域，CI 稳定后由 finalize 任务就地更新）。 摘自 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean across every stage; the only remainder is round 7's three non-blocking hygiene deferrals. Stepping back: this started as a dead-branch fix flagged by the repo-hygiene report and went through seven review rounds; what's left is the minimal correct version of it. The subtype now comes from the level the CLI actually writes, legacy envelope-less messages keep their old behavior, and the turn drains to its Note for the thread: my earlier changes-requested review on this PR came from a prior iteration of the diff (the stop-on-error behavior) that no longer exists; this approval supersedes it. 中文说明置信度：4/5 —— 各阶段均干净；仅剩第 7 轮记录的 3 条非阻断卫生项。 退一步看：这原本是仓库卫生报告标记的死分支修复，经过 7 轮 review，现在留下的是它最小且正确的形态。subtype 现在从 CLI 实际写入的层级读取，无信封的旧式消息保持原有行为，turn 会 drain 到 线程说明：我此前对本 PR 的 request-changes 评审来自 diff 的早期迭代（遇错即停的行为），该行为已不存在；本次批准取代之前的评审。 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
| log.info("control_response error: {}", jsonObject.toJSONString()); | ||
| return "error".equals(jsonObject.getString("subtype")); | ||
| return true; |
There was a problem hiding this comment.
[Critical] This diff makes the previously-dead error branch reachable, and returning true here stops the sendPrompt read loop mid-turn — but the interrupted turn's remaining assistant/result lines stay in ProcessTransport's shared BufferedReader, so the session desynchronizes. The next sendPrompt consumes the previous turn's leftovers and completes with the previous turn's result (every subsequent turn shifted by one), and a later non-reading setModel/interrupt reads a stale non-control line and throws a wrapped NullPointerException from cliControlResponse.getResponse().getSubtype() (Session.java:156). sendPrompt returns normally with no exception, so the caller cannot tell the turn was cut short.
Trigger: while sendPrompt is blocking, a concurrent control request — the transport.isReading() → inputNoWaitResponse path in processControlRequest, e.g. setModel from another thread — is rejected by the CLI, which emits a nested response.subtype:"error" control_response (ControlDispatcher.sendErrorResponse) while the turn itself continues. Before this diff the branch was unreachable (the CLI never emitted a top-level subtype), so the loop always drained to the turn's own result and self-healed; the desync is newly activated by this change.
Witness — live probe against the real Session/ProcessTransport with a fake CLI; it flips when this hunk is reverted to the merge base:
PR arm:
turn1 sendPrompt returned: done=true exception=null assistantUuids=[a1] resultUuids=[]
turn2 sendPrompt returned: assistantUuids=[a2] resultUuids=[r1]
post-abort setModel threw ...SessionControlException ... cause=java.lang.NullPointerException:
Cannot invoke "CLIControlResponse$Response.getSubtype()" because the return value of
"CLIControlResponse.getResponse()" is null
BASE arm (hunk reverted):
turn1 sendPrompt returned: assistantUuids=[a1, a2] resultUuids=[r1]
turn2 sendPrompt returned: assistantUuids=[a3] resultUuids=[r2]
post-abort setModel returned = Optional[true]
Handle the control failure without truncating the turn stream: throw from the callback (the existing catch wraps it as SessionSendPromptException, and the control_request branch already relies on throwing) so the caller learns the session may be tainted, or keep reading (return false) and surface the error through the consumers. If stop-on-error is deliberately kept, the session must drain/discard the transport up to the interrupted turn's result before the next operation.
中文说明
这个 diff 让原本不可达的 error 分支变为可达，而在此处 return true 会在 turn 中途停止 sendPrompt 的读取循环——但被中断 turn 剩余的 assistant/result 行仍留在 ProcessTransport 共享的 BufferedReader 中，导致会话失步：下一次 sendPrompt 会消费上一个 turn 的残留输出、并以该 turn 的 result 作为自己的结果结束（之后每个 turn 都错位一个）；随后非读取状态下的 setModel/interrupt 会读到残留的非 control 行，在 cliControlResponse.getResponse().getSubtype()（Session.java:156）处抛出包装后的 NullPointerException。sendPrompt 正常返回、没有异常，调用方无法察觉 turn 被截断。
触发方式：sendPrompt 阻塞期间，并发的 control 请求（processControlRequest 中 transport.isReading() → inputNoWaitResponse 路径，例如另一线程调用 setModel）被 CLI 拒绝，CLI 在 turn 继续执行的同时发出嵌套的 response.subtype:"error" control_response（ControlDispatcher.sendErrorResponse）。此 diff 之前该分支不可达（CLI 从未发出顶层 subtype），循环总会读到当前 turn 自己的 result 而自愈；失步是本改动新引入的。
证据——用假 CLI 进程驱动真实 Session/ProcessTransport 的实测探针；将本 hunk 回退到 merge base 后结果翻转：
PR arm:
turn1 sendPrompt returned: done=true exception=null assistantUuids=[a1] resultUuids=[]
turn2 sendPrompt returned: assistantUuids=[a2] resultUuids=[r1]
post-abort setModel threw ...SessionControlException ... cause=java.lang.NullPointerException:
Cannot invoke "CLIControlResponse$Response.getSubtype()" because the return value of
"CLIControlResponse.getResponse()" is null
BASE arm（hunk 回退后）:
turn1 sendPrompt returned: assistantUuids=[a1, a2] resultUuids=[r1]
turn2 sendPrompt returned: assistantUuids=[a3] resultUuids=[r2]
post-abort setModel returned = Optional[true]
建议不要在 control 失败时截断 turn 流：从回调中抛出异常（现有的 catch 会包装为 SessionSendPromptException，control_request 分支已经依赖抛异常机制），让调用方知道会话可能已被污染；或继续读取（return false）并通过 consumers 上报错误。如果确实要保留 error 即停，则必须在下一次操作前排空/丢弃传输流直到被中断 turn 的 result。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| String responseSubtype = Optional.ofNullable(controlResponse) | ||
| .map(CLIControlResponse::getResponse) | ||
| .map(CLIControlResponse.Response::getSubtype) | ||
| .orElse(jsonObject.getString("subtype")); |
There was a problem hiding this comment.
[Suggestion] The legacy top-level subtype fallback this PR explicitly keeps has no test — the mutation .orElse(jsonObject.getString("subtype")) → .orElse(null) survives the entire committed suite (verified by a mutant run in a scratch tree). If a future edit drops the fallback, a flat-shape message {"type":"control_response","subtype":"error"} (no nested response object) would be treated as non-error and sendPrompt would keep consuming the stream instead of stopping, with no unit or CI test failing — integration tests are excluded from mvn test by surefire excludedGroups, and they talk to a real CLI that only emits the nested shape.
Probe flip: the legacy flat error shape processed 2 lines under the mutant vs 1 (correct stop) on unmodified PR code.
Add a FakeTransport test feeding one legacy top-level-error control_response (no response object) followed by a decoy assistant line, and assert transport.getProcessedPromptLineCount() == 1.
中文说明
本 PR 明确保留的旧顶层 subtype 兜底没有测试——变异 .orElse(jsonObject.getString("subtype")) → .orElse(null) 在整个已提交测试套件下存活（已在 scratch tree 中做变异验证）。如果未来有人删掉这个兜底，扁平形状的消息 {"type":"control_response","subtype":"error"}（无嵌套 response 对象）会被当作非错误，sendPrompt 继续读取流而不停止，且没有任何单测或 CI 测试失败——integration 测试被 surefire excludedGroups 排除在 mvn test 之外，而且它们连接的是只发嵌套形状的真实 CLI。
探针翻转：该扁平 error 形状在变异体下处理了 2 行，未改动的 PR 代码下处理 1 行（正确停止）。
建议新增一个 FakeTransport 测试：先喂一条顶层 error 的 control_response（无 response 对象），再喂一条诱饵 assistant 消息，断言 transport.getProcessedPromptLineCount() == 1。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| if (!"error".equals(responseSubtype)) { | ||
| return false; |
There was a problem hiding this comment.
[Suggestion] The non-error continuation branch of the rewritten condition has no unit test — the mutant "always stop on a control_response" (this condition forced to false) leaves every committed test green, including the new regression test, which only feeds the error case. Concrete discriminator: a control_response with nested response.subtype:"success" followed by a result message — correct code processes both lines (count 2); the mutant stops at line 1 (count 1) and nothing fails. This is a realistic path: session.interrupt() mid-prompt (as sendPromptAndInterruptContinueSuccessfully does) sends a success control_response into the very stream this loop reads, so a mutation that stops on every control_response would silently truncate the prompt's remaining assistant/result output. Only @Tag("integration") tests cover this, and surefire excludes that tag from mvn test/CI.
Probe flip: nested-success control_response + result processed 1 line under the mutant vs 2 on unmodified PR code.
Add a FakeTransport test feeding a nested subtype:"success" control_response followed by a result message, and assert getProcessedPromptLineCount() == 2.
中文说明
重写后条件的非错误继续分支没有单测——变异体"对所有 control_response 都停止"（将该条件强制为 false）下所有已提交测试仍全绿，包括新增的回归测试（它只喂了 error 场景）。具体判别输入：嵌套 response.subtype:"success" 的 control_response 后跟一条 result 消息——正确代码处理两行（计数 2）；变异体在第 1 行就停止（计数 1）且没有测试失败。这是真实路径：prompt 进行中调用 session.interrupt()（如 sendPromptAndInterruptContinueSuccessfully 所做）会向本循环正在读取的同一个流发送 success control_response，因此"逢 control_response 必停"的变异会悄悄截断 prompt 剩余的 assistant/result 输出。只有 @Tag("integration") 测试覆盖该路径，而 surefire 将其排除在 mvn test/CI 之外。
探针翻转：嵌套 success control_response + result 在变异体下处理 1 行，未改动的 PR 代码下处理 2 行。
建议新增一个 FakeTransport 测试：喂嵌套 subtype:"success" 的 control_response 后跟 result 消息，断言 getProcessedPromptLineCount() == 2。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| if (!"error".equals(responseSubtype)) { | ||
| return false; | ||
| } else { | ||
| log.info("control_response error: {}", jsonObject.toJSONString()); |
There was a problem hiding this comment.
[Suggestion] The error abort this diff activates drops the error on both sides. (1) No caller-visible signal: sendPrompt returns normally with no exception, the default SessionEventSimpleConsumers.onControlResponse is an empty method body, and the sole trace is this INFO-level log — a level WARN/ERROR-filtered pipelines never see. Before this diff the branch was unreachable for the real (nested) wire format; the diff makes it the normal error path, so an integrator whose turn is aborted sees the prompt just end early with partial output and nothing anywhere says why. (2) The production model cannot carry the reason at all: real CLI error envelopes put the message under response.error (ControlDispatcher.sendErrorResponse), but CLIControlResponse.Response has only requestId/subtype/response fields, so fastjson2 silently drops the error member.
Witness — probe parsing a real envelope {"type":"control_response","response":{"subtype":"error","request_id":"set-model","error":"Invalid model specified: nonexistent-model"}} (fastjson2 2.0.60, --release 11, Session.java's exact parse call): model.reserializedGraph={"type":"control_response","response":{"request_id":"set-model","subtype":"error"}}, reasonReachableProgrammatically=false; adding an error accessor to Response flips it to true.
Surface the failure — throw from the callback so the existing catch wraps it as SessionSendPromptException (or at least log at WARN/ERROR), and add an error field with getter/setter to CLIControlResponse.Response mapping the wire's response.error so the abort reason is carryable to callers/consumers.
中文说明
这个 diff 激活的 error 中止路径在两个层面都把错误丢掉了。(1) 调用方无感知：sendPrompt 正常返回、没有异常，默认的 SessionEventSimpleConsumers.onControlResponse 是空方法体，唯一的痕迹是这条 INFO 级日志——WARN/ERROR 过滤的日志管道根本看不到。此 diff 之前该分支对真实（嵌套）wire format 不可达；diff 之后它成为正常错误路径，集成方只会看到 prompt 提前结束、输出残缺，任何地方都没有原因。(2) 生产模型根本无法携带原因：真实 CLI error 信封把消息放在 response.error（ControlDispatcher.sendErrorResponse），但 CLIControlResponse.Response 只有 requestId/subtype/response 字段，fastjson2 会静默丢弃 error 成员。
证据——探针解析真实信封 {"type":"control_response","response":{"subtype":"error","request_id":"set-model","error":"Invalid model specified: nonexistent-model"}}（fastjson2 2.0.60、--release 11、Session.java 的原解析调用）：model.reserializedGraph={"type":"control_response","response":{"request_id":"set-model","subtype":"error"}}、reasonReachableProgrammatically=false；给 Response 增加 error 访问器后翻转为 true。
建议把失败暴露出来——从回调抛出异常，让现有 catch 包装为 SessionSendPromptException（至少也改用 WARN/ERROR 级别日志），并给 CLIControlResponse.Response 增加映射 wire response.error 的 error 字段（含 getter/setter），使中止原因可以被调用方/消费者拿到。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| String responseSubtype = Optional.ofNullable(controlResponse) | ||
| .map(CLIControlResponse::getResponse) | ||
| .map(CLIControlResponse.Response::getSubtype) | ||
| .orElse(jsonObject.getString("subtype")); |
There was a problem hiding this comment.
[Suggestion] The legacy top-level fallback is unreachable whenever the nested response object exists: CLIControlResponse.Response.subtype is field-initialized to "success", so Optional.map(...getSubtype) never yields empty for a parsed nested object and .orElse(jsonObject.getString("subtype")) is never evaluated. A mixed shape {"type":"control_response","subtype":"error","response":{"request_id":"r1"}} (top-level error subtype, nested object without its own subtype) therefore yields responseSubtype = "success" and the control error is silently swallowed — the exact bug this PR fixes, re-opened for that shape. No emitter in this repo produces the mixed shape today (swept ControlDispatcher success/error emitters, BaseJsonOutputAdapter.emitControlError, TS SDK Query.sendControlResponse, and both protocol types), so this is latent — but the fallback reads as unconditional and will mislead the next person touching this code.
Witness — probe on unmodified PR code: feeding the mixed shape, the read loop did not stop (processed=2).
Either make the fallback key-presence-based from the raw JSON, e.g. Optional.ofNullable(jsonObject.getJSONObject("response")).map(r -> r.getString("subtype")).orElseGet(() -> jsonObject.getString("subtype")), or keep the parsed-wrapper form and add a comment stating the fallback only applies when the nested response object is entirely absent due to the "success" default.
中文说明
只要嵌套 response 对象存在，旧顶层兜底就不可达：CLIControlResponse.Response.subtype 的字段初始值是 "success"，对已解析出的嵌套对象 Optional.map(...getSubtype) 永远不会为空，因此 .orElse(jsonObject.getString("subtype")) 永远不会被求值。混合形状 {"type":"control_response","subtype":"error","response":{"request_id":"r1"}}（顶层 error subtype + 嵌套对象无自己的 subtype）会得到 responseSubtype = "success"，control 错误被静默吞掉——正是本 PR 修复的 bug 在该形状下复现。当前仓库没有任何发送方产生这种混合形状（已扫 ControlDispatcher success/error 发送器、BaseJsonOutputAdapter.emitControlError、TS SDK Query.sendControlResponse 和两个 protocol 类型），所以目前是潜在的——但兜底读起来像是无条件的，会误导下一个改这段代码的人。
证据——未改动 PR 代码上的探针：喂入混合形状后读取循环没有停止（processed=2）。
建议要么让兜底基于键是否存在、直接从原始 JSON 读取，例如 Optional.ofNullable(jsonObject.getJSONObject("response")).map(r -> r.getString("subtype")).orElseGet(() -> jsonObject.getString("subtype"))；要么保留解析包装形式并加注释说明：由于 "success" 默认值的存在，兜底只在嵌套 response 对象完全缺失时生效。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| "{\"type\":\"control_response\",\"response\":{\"request_id\":\"set-model\",\"subtype\":\"error\"," | ||
| + "\"response\":{\"message\":\"set model failed\"}}}", |
There was a problem hiding this comment.
[Suggestion] The error fixture does not match the CLI wire format it simulates: the CLI puts the failure under response.error with no payload (ControlDispatcher.sendErrorResponse / BaseJsonOutputAdapter.emitControlError, ControlErrorResponse type), never under a nested response.response payload — and CLIControlResponse.Response has no error field, so real error envelopes deserialize with Response.response == null. The subtype check under test reads only the envelope, so the test passes today either way; but if a future refactor derives the error decision through the payload (or requires a non-null controlResponse.getResponse().getResponse()), this fixture — which has a payload — would still pass while every real CLI error response (payload absent) would break the stop-on-error behaviour the test exists to guard.
| "{\"type\":\"control_response\",\"response\":{\"request_id\":\"set-model\",\"subtype\":\"error\"," | |
| + "\"response\":{\"message\":\"set model failed\"}}}", | |
| "{\"type\":\"control_response\",\"response\":{\"request_id\":\"set-model\",\"subtype\":\"error\"," | |
| + "\"error\":\"set model failed\"}}", |
中文说明
该 error 测试数据与它模拟的 CLI wire format 不一致：CLI 把失败信息放在 response.error 下且没有 payload（ControlDispatcher.sendErrorResponse / BaseJsonOutputAdapter.emitControlError、ControlErrorResponse 类型），从不放在嵌套的 response.response payload 下——而且 CLIControlResponse.Response 没有 error 字段，真实 error 信封反序列化后 Response.response == null。被测的 subtype 检查只读信封层，所以现在测试无论哪种形状都通过；但如果未来重构改为通过 payload 判断错误（或要求 controlResponse.getResponse().getResponse() 非空），这个带 payload 的测试数据仍会通过，而所有真实 CLI error 响应（无 payload）都会破坏该测试本要守护的 error 即停行为。
— qwen3.8-max via Qwen Code /review (v0.22.0)
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.
中文说明
已审查。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| if ("error".equals(getControlResponseSubtype(jsonObject, controlResponse))) { | ||
| log.warn("control_response error: {}", jsonObject.toJSONString()); | ||
| } |
There was a problem hiding this comment.
[Suggestion] Now that this branch returns false unconditionally, the only observable effect of the subtype detection is this log.warn — and none of the three new tests observes it: they assert only getProcessedPromptLineCount() and consumer invocation counts, which the unconditional return false satisfies regardless of detection. Mutations verified to survive the committed suite (scratch Maven runs at this commit): replace getControlResponseSubtype's body with return null; (SessionTest 4/4 green, no WARN emitted), delete this entire if block (green), or delete the helper's nested response.subtype branch (green — masked by the envelope fallback). The nested-error drain test also stays green under the pre-PR top-level-only read; only sendPromptUsesTopLevelSubtypeWhenNestedSubtypeIsMissing catches a full revert. A future edit that breaks nested-subtype reading therefore ships with every test green, and mid-turn control errors are silently swallowed without the warning this PR exists to emit — while test names like ...NestedError imply the path is covered.
Fix: assert the one observable effect — attach a capturing appender to the Session logger (logback-classic is already a test dependency; e.g. a ch.qos.logback.core.read.ListAppender on (ch.qos.logback.classic.Logger) LoggerFactory.getLogger(Session.class)) and assert control_response error fires once for each error fixture (nested and top-level) and zero times for the nested-success fixture. A probe verified this assertion flips between the mutants and the correct code.
Witness (scratch-tree mutant runs):
mutant(helper body -> return null): SessionTest Tests run: 4, Failures: 0 (no WARN)
mutant(delete if-block): Tests run: 4, Failures: 0
probe fix under mutant: warnsOnNestedErrorControlResponse expected: <1> but was: <0>
中文说明
在该分支 return false 变为无条件之后，subtype 检测唯一可观测的效果就是这条 log.warn——而三个新测试都没有观测它：它们只断言 getProcessedPromptLineCount() 和 consumer 调用次数，无条件的 return false 无论如何都满足这些断言。已在本 commit 的 scratch Maven 运行中验证存活的变异体：把 getControlResponseSubtype 方法体替换为 return null;（SessionTest 4/4 全绿，无 WARN 输出）、删除整个 if 块（全绿）、删除 helper 中嵌套 response.subtype 分支（全绿——被 envelope 兜底掩盖）。嵌套 error 排空测试即使在改动前只读顶层 subtype 的代码上也是绿的；只有 sendPromptUsesTopLevelSubtypeWhenNestedSubtypeIsMissing 能捕获完整回退。因此，未来破坏嵌套 subtype 读取的改动会在所有测试全绿的情况下合入，turn 中途的 control error 将被静默吞掉、不再输出本 PR 要补上的警告——而 ...NestedError 之类的测试名却让人以为该路径已有覆盖。
修复：断言检测的唯一可观测效果——给 Session 的 logger 挂一个捕获 appender（logback-classic 已是测试依赖，例如把 ch.qos.logback.core.read.ListAppender 挂在 (ch.qos.logback.classic.Logger) LoggerFactory.getLogger(Session.class) 上），断言两个 error 测试数据（嵌套与顶层）各触发一次 control_response error、嵌套 success 数据触发零次。探针已验证该断言在变异体与正确代码之间翻转。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| return Optional.ofNullable(controlResponse) | ||
| .map(CLIControlResponse::getResponse) | ||
| .map(CLIControlResponse.Response::getSubtype) | ||
| .orElse(null); |
There was a problem hiding this comment.
[Suggestion] This third fallback can never change the outcome at the helper's only call site: controlResponse is parsed from the same jsonObject the raw-JSON branches read, Response.subtype maps the same subtype key (no @JSONField alias), and the field is initialized to "success". So: no nested response object → the typed parse also has getResponse() == null (same null); a nested object without a subtype key and no top-level subtype → returns "success" where branches 1–2 return null, but "error".equals(...) is false either way; a nested object with subtype → the first branch already returned. A scratch-tree probe confirmed the dead weight: replacing this fallback with return null; leaves 6/6 tests green and both error fixtures still emit byte-identical WARN lines. The redundancy also masks regressions — deleting the nested branch keeps every test green precisely because this fallback absorbs the miss, hiding exactly the regressions the companion comment on the if block asks tests to catch.
Fix: drop this fallback (and the then-unused controlResponse parameter, updating the call site) — behavior at the call site is unchanged by the case analysis above. If a third source was meant to guard against a parse discrepancy, name that case in a comment instead.
Witness:
fallback replaced with `return null;`: Tests run: 6, Failures: 0; both WARN lines still emitted byte-identical
中文说明
第三个兜底在 helper 的唯一调用点处永远不可能改变结果：controlResponse 就是从原始 JSON 分支所读取的同一个 jsonObject 解析而来，Response.subtype 映射的是同一个 subtype 键（无 @JSONField 别名），且字段初始值为 "success"。因此：无嵌套 response 对象 → 类型化解析同样得到 getResponse() == null（同样是 null）；嵌套对象存在但无 subtype 键且无顶层 subtype → 在第 1、2 支返回 null 处返回 "success"，但 "error".equals(...) 两种情况都为 false；嵌套对象带 subtype → 第一支已经返回。scratch tree 探针证实了这段死代码：把该兜底替换为 return null; 后 6/6 测试全绿，两个 error 测试数据仍输出字节级一致的 WARN。这种冗余还会掩盖回归——删除嵌套分支后所有测试依然全绿，正是因为这个兜底吸收了缺失，恰好隐藏了 if 块那条评论要求测试捕获的那类回归。
修复：移除该兜底（以及随之不再使用的 controlResponse 参数，并同步更新调用点）——按上面的分支分析，调用点行为不变。如果第三个来源本意是防某种解析差异，请改用注释说明该场景。
— qwen3.8-max via Qwen Code /review (v0.22.0)
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. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
中文说明
已审查。 建议见行内评论。 1 条建议级发现无法锚定到改动行，已丢弃；此处无需进一步处理。
— qwen3.8-max via Qwen Code /review (v0.22.0)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| appender.stop(); | ||
| } | ||
|
|
||
| private static boolean hasControlResponseErrorWarning(ListAppender<ILoggingEvent> appender) { |
There was a problem hiding this comment.
[Suggestion] The matcher checks only the static prefix, so dropping or narrowing the payload argument of log.warn("control_response error: {}", jsonObject.toJSONString()) — a statement inside this PR's own changed hunk — still passes both error tests (both mutants verified green). The typed CLIControlResponse carries no error field, so this payload is the only place the SDK surfaces the CLI's failure text; a silent narrowing ships degraded diagnostics with nothing to catch it. Require the matched warning to contain the fixture's error text, e.g. && event.getFormattedMessage().contains("set model failed") — the fixture already provides the string.
中文说明
[Suggestion] 该匹配只检查静态前缀，因此删掉或收窄 log.warn("control_response error: {}", jsonObject.toJSONString()) 的 payload 参数——该语句就在本 PR 改动的 hunk 内——两个 error 测试仍会全部通过（两种变异均实测通过）。CLIControlResponse 类型中没有 error 字段，这个 payload 是 SDK 向用户暴露 CLI 失败信息的唯一位置；悄悄收窄它会发布降级的诊断信息且无任何测试拦截。建议要求匹配到的警告包含 fixture 中的错误文本，例如 && event.getFormattedMessage().contains("set model failed")——fixture 已经提供了该字符串。
— qwen3.8-max via Qwen Code /review (v0.22.0)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): "agent 4": none — no check was cut short..
中文说明
未探索到全部深度（达到工具调用预算）："agent 4"：none — no check was cut short.。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
Addressed the latest review round in commits
Resolved the stale threads. Java runtime verification is via the CI matrix (no local Maven). @qwen-code /review |
|
@qwen-code /triage |
|
@qwen-code /review\n\nThe latest review reports no findings on commit cd80147. Please refresh the review decision for this PR. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Deferred under the convergence posture (round 6, not a blocker) — recorded, not requested in this round:
packages/sdk-java/qwencode/src/main/java/com/alibaba/qwen/code/cli/session/Session.java:295 — [review] subtype 双读取者：类型化对象旁重新从原始 JSON 推导；顶层兜底固化了任何生产者都未发出的 wire 形状packages/sdk-java/qwencode/src/main/java/com/alibaba/qwen/code/cli/session/Session.java:295 — [review] 缺少注释说明为何必须读原始 JSON（类型化 Response.subtype 默认 "success"、无旧顶层字段映射）
中文说明
收敛姿态下延后（第 6 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
Use the parsed control response wrapper when deciding whether a CLI control response is an error, matching the nested wire format and preserving the legacy top-level fallback. Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Use the real control-response error envelope in session tests so the subtype handling coverage matches the CLI wire format. Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com
Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com
Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com
Cover missing response payloads, non-error subtype boundaries, and warning payload content so subtype detection regressions are observable. Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ccbab0a to
15a5fb5
Compare
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push，因为这会使已有的评审评论失效。另外，供日后参考：作为集成流程的一部分，机器人始终会自动将所有改动压缩（squash）为单个提交。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Deferred under the convergence posture (round 7, not a blocker) — recorded, not requested in this round:
packages/sdk-java/qwencode/src/main/java/com/alibaba/qwen/code/cli/session/Session.java:295 — [review] 嵌套优先于顶层 subtype 的语义无测试钉住packages/sdk-java/qwencode/src/test/java/com/alibaba/qwen/code/cli/session/SessionTest.java:537 — [review] FakeTransport 与 TestTransport 及初始化 fixture 重复packages/sdk-java/qwencode/src/test/java/com/alibaba/qwen/code/cli/session/SessionTest.java:53 — [review] 重复的 assertTrue 静态导入、导入块被拆分
中文说明
收敛姿态下延后（第 7 轮，非阻断）——已记录，本轮不要求修改：共 3 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 137 passed · 0 failed · 137 total Flakiness gate: not applicable — no runnable changed test files (0 out-of-scope file(s) noted in the log) 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：137 通过 · 0 失败 · 137 总计 抖动门：不适用 — no runnable changed test files (0 out-of-scope file(s) noted in the log) Verification reportPR #9924 Deep Verification —
|
| cell | Session.java |
oracle | result |
|---|---|---|---|
HEAD (a16dc2b, PR head 15a5fb5) |
fixed | mvn test -Dtest=SessionTest → surefire XML vs expected-all-green |
13/13 pass |
BASE (88a1363 worktree, base Session.java pristine, HEAD test copied over) |
un-fixed | same harness vs expected reds | 10/13 — the 3 predicted reds, no others |
Base reds, with the exact failing assertion each test exists to catch:
| test on base | failing assertion | mechanism |
|---|---|---|
sendPromptDrainsTurnWhenControlResponseSubtypeIsNestedError |
:297 expected: <true> but was: <false> (warning present) |
base reads top-level subtype only; nested (real CLI) errors never logged — the bug |
sendPromptUsesTopLevelSubtypeWhenNestedSubtypeIsMissing |
:317 expected: <2> but was: <1> (lines processed) |
base hits top-level error and return true — turn stops at the error line |
sendPromptUsesTopLevelSubtypeWhenResponseObjectIsMissing |
:339 expected: <2> but was: <1> |
same early-stop |
Witness: evidence/01-ab-head-vs-base-sessiontest.png (both harness runs as printed, including the [ERROR] … expected: … but was: … lines).
The PR bundles two changes — both proven load-bearing
The old code returned true (stop reading the turn) on an error subtype; the new code always returns false (drain to result). Two intermediate builds, each reverting exactly one half of the head behavior, separate the contributions:
| build | lookup | on error subtype | nested-error test | top-level tests ×2 | neutral tests ×4 |
|---|---|---|---|---|---|
| base | top-level only | return true + log.info |
✗ no warning | ✗ count=1, INFO | ✓ |
drain-reverted (nested lookup + warn kept, old return true restored) |
nested + fallback | return true |
✗ count :294 expected <3> was <1> |
✗ count | ✓ |
| lookup-reverted (top-level-only lookup, warn + drain kept) | top-level only | return false |
✗ warning :297 |
✓ | ✓ |
| head | nested + fallback | return false + log.warn |
✓ | ✓ | ✓ |
Each reverted half fails a different assertion class — the drain half is pinned by the line-count/result-count assertions, the lookup half by the warning assertions. Neither half alone reproduces head. Witness: evidence/02-variant-attribution.png.
Wire oracle — the CLI never emits top-level subtype on control_response
Verified from four independent sources (scripted grep assertions, 5/5 pass):
BaseJsonOutputAdapter.emitControlError/emitControlResponse—{type:'control_response', response:{subtype:'error'|'success', request_id, …}}ControlDispatcher.sendErrorResponse/sendSuccessResponse— same nested shape- Java SDK's own protocol spec (
protocol.ts):CLIControlResponse { type, response: ControlResponse | ControlErrorResponse }—subtypelives inside the wrapper - TS SDK
Query.sendControlResponse— nested; and a repo-wide sweep found zero emitters withsubtypeat top level
Corollaries (both verified, both fine): the top-level fallback is purely defensive, and the old "error branch unreachable for real responses" claim in the PR description is confirmed by the base cell (nested error produced no log line at all on base). The Java SDK's other control_response readers (start(), processControlRequest) already used the typed nested model — the raw-JSON path in sendPrompt was the single outlier (sibling sweep complete).
Mutation matrix (vacuity + guard coverage)
Single-point mutants of head Session.java, full suite re-run per mutant, expected outcomes encoded per mutant (evidence/03-mutation-matrix.png):
| mutant | change | killed by | status |
|---|---|---|---|
| M1 nested-bypass | helper always returns top-level | nested-error warning assertion (1 red) | killed |
| M2 no-fallback | helper returns null when no nested subtype |
both top-level tests' warning assertions (2 reds) | killed |
| M3 exact→prefix | "error".equals(x) → x.startsWith("error") |
error_extra no-warning assertion (1 red) |
killed |
| M4 warn→info | log.warn → log.info |
all 3 warning-positive tests via Level.WARN check (3 reds) |
killed — positive control |
| M5 null-guard removed | drop responseObject != null && |
ResponseObjectIsMissing test errors with NPE (1 red) |
killed |
5/5 killed, 0 survivors; unmutated head control is green (13/13). M4 is the positive control proving the harness can turn the suite red, and it lands in the same file as the mutated code. Every guard the PR introduces is pinned by a test whose name matches its fixture (fixture/name audit done: e.g. …NotExactError really feeds error_extra; …ResponseObjectIsMissing really omits response).
Findings (informational only — none blocking)
- Behavioral change for hypothetical legacy producers of top-level
subtype:"error"(intended, strictly better). Old code stopped the turn at the error line (return true) and logged at INFO; head drains toresultand logs at WARN. The old stop was a real hazard: unconsumedassistant/resultlines would stay on the stream and leak into the nextsendPromptturn, so the drain change is an alignment fix, not just cleanup. No current producer emits the top-level shape (wire oracle above), so real-world behavior change is confined to the nested case: previously silent, now a WARN log. - Top-level fallback is defensive-only surface. Verified no emitter produces it. Keeping it costs one branch and buys legacy tolerance; a maintainer preferring KISS could drop it — either choice is defensible, tests pin current behavior (M2).
- Type-boundary probe — no new failure surface from reading
response. Five malformed/edge shapes (responseas string, array, number,null, numeric nestedsubtype) driven throughsendPrompton both arms: string/number abort the turn with a loudSessionSendPromptException(fastjson2) on head and base identically — the typedjsonObject.to(CLIControlResponse…)parse that precedes the subtype check already rejects them on base; array/null/numeric-subtype complete the turn on both. 5/5 shapes identical (evidence/04-boundary-probe-head-vs-base.png).
Gates
| gate | command | result |
|---|---|---|
| CI-exact unit suite @ head | mvn --batch-mode --no-transfer-progress clean test |
13/13 pass, BUILD SUCCESS |
| CI checkstyle gate @ head | mvn --batch-mode --no-transfer-progress checkstyle:check |
BUILD SUCCESS; liveness proven: planted trailing-whitespace violation in a scratch copy → [ERROR] … Line has trailing whitespace, BUILD FAILURE, then reverted |
| Reviewer Test Plan, verbatim | the 7 exact mvn test -Dtest=SessionTest#<name> commands from the PR body |
7/7 exit 0, each ran exactly 1 test (Tests run: 1, Failures: 0) |
Not covered
- Per-commit attribution: depth-2 checkout — only
15a5fb5of the 7 metadata commits is locally reachable (git rev-list HEAD^1..HEAD^2= 1 at the shallow boundary). The aggregateHEAD^1..HEADdiff (exactly the 2 files above) is what was verified. - JDK 11/17 runtime cells: CI's matrix runs 11/17/21 across 3 OSes; this round ran on Temurin JDK 21 only. Code compiles with
maven.compiler.release=11; the tests are pure in-memory transport + logback with no JDK-version-sensitive surface. macOS/Windows cells likewise not reproducible here. daemon-e2eCI job: inspected —DaemonServeE2ETestdrives the daemon HTTP API withjava.net.httpand never touchesSession.sendPrompt, so it does not exercise this diff; not run.integration-tagged SessionTest tests: excluded by surefire config (excludedGroups=integration) — require a real CLI process/auth; same exclusion applies in CI.- Live CLI traffic capture: the sandbox has no model credentials, so the wire format was established from all emitter sources + protocol spec (shape-level oracle), not from captured live bytes.
FakeTransportsemantics were checked against the realProcessTransport.iterateOutput(callbacktrue= stop reading) — they match. - Repo-wide npm gates: untouched by a Java-only diff (no
package.json/lockfile changes), not re-run. $QWEN_VERIFY_CONTEXTalso containedscreen.diff— verified to be a byte-copy of the same PR diff, no additional information.
Methodology
Environment: CI verify container (node:22-bookworm), no root/apt access — verification tooling installed userspace (Temurin JDK 21.0.12 + Maven 3.9.16 tarballs; dependencies resolved from the image-seeded /home/node/.m2/repository). Working tree = refs/pull/9924/merge (depth 2). Harnesses drive Session.sendPrompt through an in-package FakeTransport whose callback contract (true stops the turn) was verified to match production ProcessTransport.iterateOutput; warnings observed via a logback ListAppender attached to the Session logger. Base A/B ran in a tmp/base-tree git worktree at HEAD^1 with HEAD's SessionTest.java copied over (base Session.java verified byte-identical to HEAD^1 before running); intermediate variants and mutants were single-file edits in scratch worktrees, reverted/removed after capture. Every expectation (including the base arm's reds) was encoded in JSON and checked against surefire XML by harness/check-results.mjs, so an expected base failure counts as a passing assertion; raw Maven logs per cell are in logs/, harness scripts in harness/. All 4 scratch worktrees removed after the cells were captured.
Flakiness gate log
verdict: n/a
summary: no runnable changed test files (0 out-of-scope file(s) noted in the log)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Maintainer verification — real local environmentI ran this PR against a real bundled CLI, not fixtures. The author could not run Maven locally, and CI never exercises the code path this PR fixes, so I rebuilt the whole stack here. Verdict: the premise is confirmed, the fix is correct, and I found no blocking defect. Recommending merge. One non-blocking coverage gap (M8) is listed at the end. Environment
1. The premise holds: the CLI never puts
|
base (main) |
head (this PR) | |
|---|---|---|
onControlResponse dispatched |
1 | 1 |
| assistant / result messages | 1 / 1 | 1 / 1 |
control_response error warning logged |
false | true |
| turn wall time | 12280 ms | 12257 ms |
On main the failure is completely silent: setModel returns Optional.empty, no WARN, no INFO. With the PR the reason is on the log.
Why the log is load-bearing, not decorative. CLIControlResponse.Response has only requestId / subtype / response — no error field. I dumped what the typed object actually carries to a consumer:
CONSUMER typedSubtype=error typedRequestId=183a5f77-… typedPayload=null
CONSUMER reserialized={"type":"control_response","response":{"request_id":"183a5f77-…","subtype":"error"}}
fastjson2 drops error on the floor. log.warn("control_response error: {}", jsonObject.toJSONString()) is therefore the only place the CLI's failure text reaches a Java SDK operator. That also settles the earlier reviewer question about the fixture shape — the PR's fixture keys match the live envelope exactly.
3. The tests discriminate (revert-hunk A/B)
- Head:
mvn --batch-mode clean test→Tests run: 141, Failures: 0, Errors: 0, Skipped: 5, BUILD SUCCESS.mvn checkstyle:check→ 0 violations. - Revert-hunk (this PR's
SessionTest.java+main'sSession.java) → 3 failures, all in the new tests, with the right diagnostics: the nested error never warns, and the top-level shapes stop the turn one line early. - The other 4 new tests pass on base too — they are boundary pins rather than regression proofs, and they earn their keep in the mutation matrix below (M7 / M9 / M10).
4. Mutation matrix — 10 of 11 mutants killed
Every semantic property the diff introduces is pinned: nested lookup (M1), the legacy fallback (M2), the containsKey guard (M3), the WARN level (M4), the drain behaviour (M5, M10), the warning itself (M6), exact-match semantics (M7), consumer dispatch (M9), and the warning payload (M11).
5. The 9 standing review threads are all addressed at this head
All nine unresolved threads were filed on older commits (c457e1114 / 091ba0a54 / 1cee64889) and are marked outdated. I re-checked each against 15a5fb53b5:
The blocking [Critical] is the important one: it described the round-1 behaviour where the branch returned true and desynchronized ProcessTransport's shared BufferedReader. The current head returns false unconditionally — the real-stack run drains cleanly to result in both builds, and mutant M5 (restoring the early abort) is killed by three tests. That finding no longer applies to the code in this PR.
6. Notes for the record
- Non-blocking gap (M8). Flipping the precedence in
getControlResponseSubtypeso the top-level wins over the nested subtype survives the whole suite — no fixture carries both. Unreachable in production (no producer emits a top-level subtype), so this is a hygiene item; one extra fixture with both keys set would close it. - The two top-level-fallback tests describe a shape nothing emits today. That is fine as defensive coverage, but they should not be read as regression proof of a live path.
log.info→log.warnis a deliberate, observable change (the tests assert the level). Worth a line in the release notes for anyone filtering Java SDK logs.- CI blind spot, not introduced by this PR.
mvn testexcludes@Tag("integration"), which is every real-CLISessionTestcase, and theReal daemon E2E / Java 11job runs-Dgroups=daemon-integration(DaemonServeE2ETest) — the daemon transport, notSession/ProcessTransport. So no CI job has ever run the fixed path against a real CLI. The probe above is the first one; worth considering as a follow-up job.
Reproduce
The harness, raw wire transcripts, Maven logs and mutation matrix are on wenshao/qwen-code@assets-pr9924 — rig/RealCliControlResponseProbe.java (real-CLI probe), rig/fake-openai.ts, rig/mutate.py, logs/wire-{head,base}.txt.
中文说明
维护者本地真实环境验证
我用真实打包 CLI（不是 mock）跑了这个 PR。作者本机没有 Maven，而 CI 也从未覆盖这个 PR 修复的代码路径，所以我在本地重建了整条链路。
结论：前提成立，修复正确，未发现阻塞性缺陷，建议合入。 末尾列了一个非阻塞的覆盖缺口（M8）。
环境
base origin/main@442951afee；PR head 15a5fb53b5（合并结果 370a6a15af，相对 main 只有 PR 的两个文件不同）；被测 CLI 是从合并树构建的 dist/cli.js 0.22.3（sha256 65f2d08b650d89b9…）；Zulu JDK 26.0.2 + release=11 + Maven 3.9.16；macOS 26.6.2 arm64 / Node 24.18.1；模型侧用仓库自带的 integration-tests/fake-openai-server.ts，HOME/QWEN_HOME 全隔离。
1. 前提成立：CLI 从不把 subtype放在顶层
审计了 packages/*/src 里全部 control_response 生产者（ControlDispatcher.sendSuccessResponse / sendErrorResponse、BaseJsonOutputAdapter.emitControlResponse / emitControlError）：四个全部把 subtype 嵌在 response 下，没有一个放在顶层；wire 类型 CLIControlResponse 根本没有顶层 subtype 成员。所以 main 上 error 分支确实不可达，与 PR 描述一致。真实抓包也证实了这一点（见上文 RECV 行）。
2. 真实链路 A/B —— 同一个 CLI、同样的字节、两个 SDK 构建
我写了个探针：通过 SDK 自己的 ProcessTransport 驱动打包 CLI，把每一行都 tee 下来，并在 prompt turn 进行中通过 SDK 公开 API 调用 session.setModel("")——这正是真实调用路径（processControlRequest 走 transport.isReading() → inputNoWaitResponse 分支）。CLI 拒绝空模型，于是在 turn 中途回了一个真实的嵌套 error control_response。
结果：两侧 onControlResponse 都收到 1 次、assistant/result 都是 1/1、耗时基本一致（12280ms vs 12257ms），唯一差别是 base 没有任何日志（WARN 和 INFO 都没有），head 打出了 WARN。也就是说 main 上这个失败是完全静默的：setModel 返回 Optional.empty，其余无声。
为什么这条日志是承重件，而不是装饰。 CLIControlResponse.Response 只有 requestId/subtype/response 三个字段，没有 error 字段。我把消费者实际拿到的类型化对象打了出来：typedPayload=null，重新序列化后只剩 {"request_id":…,"subtype":"error"}——fastjson2 把 error 直接丢了。所以 log.warn(..., jsonObject.toJSONString()) 是 Java SDK 使用者能看到 CLI 失败原因的唯一位置。这同时也回答了之前评审关于 fixture 形状的疑问：PR 的 fixture 键集与真实抓到的信封完全一致。
3. 测试有辨别力（revert-hunk A/B）
- head：
mvn clean test→Tests run: 141, Failures: 0，BUILD SUCCESS；checkstyle:check0 violations。 - revert-hunk（PR 的
SessionTest.java+ main 的Session.java）→ 3 个失败，且失败信息正确：嵌套 error 不再告警、顶层形状会提前一行结束 turn。 - 另外 4 个新测试在 base 上也能通过——它们是边界钉子而非回归证明，其价值体现在下面的变异矩阵（M7/M9/M10）。
4. 变异矩阵：11 个变异体杀掉 10 个
该 diff 引入的每个语义性质都被钉住了：嵌套读取（M1）、旧顶层兜底（M2）、containsKey 守卫（M3）、WARN 级别（M4）、drain 行为（M5、M10）、告警语句本身（M6）、精确匹配语义（M7）、消费者分发（M9）、告警 payload（M11）。
5. 9 条未解决评审线程在当前 head 上全部已闭合
九条线程都是在旧提交（c457e1114 / 091ba0a54 / 1cee64889）上提的，均已标记 outdated。逐条对 15a5fb53b5 复核后确认全部已解决。
其中阻塞性的 [Critical] 最关键：它描述的是第一轮实现里"分支返回 true、把 ProcessTransport 共享 BufferedReader 打乱"的行为。当前 head 无条件返回 false——真实链路两侧都干净地 drain 到 result，且变异体 M5（恢复提前中止）被三个测试杀掉。该发现已不适用于本 PR 的代码。
6. 备注
- 非阻塞缺口（M8）：把
getControlResponseSubtype里的优先级翻转（顶层压过嵌套）能存活整个测试套件——没有任何 fixture 同时带顶层和嵌套 subtype。生产上不可达（没有生产者发顶层 subtype），属卫生问题；补一个两个键都带的 fixture 即可闭合。 - 两个"顶层兜底"测试描述的是当前没有任何生产者会发的形状。作为防御性覆盖没问题，但不应被当作真实路径的回归证明。
log.info→log.warn是有意的可观察变更（测试断言了级别）。对按级别过滤 Java SDK 日志的用户，值得在 release notes 里提一句。- CI 盲区（非本 PR 引入）：
mvn test排除了@Tag("integration")，而SessionTest里所有真实 CLI 用例都带这个标签；Real daemon E2E / Java 11跑的是-Dgroups=daemon-integration（DaemonServeE2ETest），属 daemon 传输而非Session/ProcessTransport。所以从来没有任何 CI job 用真实 CLI 跑过这条被修复的路径。上面的探针是第一次；建议作为后续项考虑补一个 job。
复现
探针、原始 wire 抓包、Maven 日志与变异矩阵都在 wenshao/qwen-code@assets-pr9924：rig/RealCliControlResponseProbe.java（真实 CLI 探针）、rig/fake-openai.ts、rig/mutate.py、logs/wire-{head,base}.txt。
3 participants
What this PR does
Updates the Java SDK session response handling to read
control_responsestatus from the nestedresponse.subtypefield, matching the CLI wire format. It keeps the legacy top-levelsubtypelookup as a fallback, drains the rest of the current turn, and logs nested or top-level error control responses.Why it's needed
The current code checks the top-level
subtype, but CLI control responses carry the status underresponse.subtype. That makes the error branch unreachable for real responses, so Java sessions miss the warning signal for control errors in normal daemon responses. The regression coverage also pins the drain-to-result behavior, non-error dispatch, and no-warning behavior for neutral or missing subtypes.Reviewer Test Plan
How to verify
Run these tests in
packages/sdk-java/qwencode:mvn test -Dtest=SessionTest#sendPromptDrainsTurnWhenControlResponseSubtypeIsNestedErrormvn test -Dtest=SessionTest#sendPromptUsesTopLevelSubtypeWhenNestedSubtypeIsMissingmvn test -Dtest=SessionTest#sendPromptUsesTopLevelSubtypeWhenResponseObjectIsMissingmvn test -Dtest=SessionTest#sendPromptContinuesAfterNestedControlResponseSuccessmvn test -Dtest=SessionTest#sendPromptDoesNotWarnWhenControlResponseSubtypeIsNotExactErrormvn test -Dtest=SessionTest#sendPromptDoesNotWarnWhenControlResponseSubtypeIsProgressmvn test -Dtest=SessionTest#sendPromptDoesNotWarnWhenControlResponseSubtypeIsMissingThe regression tests feed nested, legacy top-level, and response-less
errorcontrol responses; verify prompt turns stop at theresultline instead of consuming canary output; assert error responses emit thecontrol_response errorwarning; assert success/progress/missing subtypes do not emit that warning; and assert non-error control responses still reachonControlResponse.Evidence (Before & After)
N/A for UI. Static local checks:
git diff --checkpassed. Local Java test execution was not available because this machine has no Maven (mvn: command not found); the GitHub Java SDK matrix should provide runtime verification.Tested on
git diff --checkonlyEnvironment (optional)
Local machine lacks Maven, so Java runtime verification is delegated to CI.
Risk & Scope
control_responsesubtype detection for warning logs and preserves the old top-level subtype fallback.Linked Issues
References #8835 (
java-session-control-response-subtype-wrong-level).中文说明
这个 PR 做了什么
更新 Java SDK 的 session 响应处理逻辑，从嵌套的
response.subtype字段读取control_response状态，以匹配 CLI wire format。代码保留旧的顶层subtype读取作为兜底，继续 drain 当前 turn，并在嵌套或顶层 error control response 出现时记录 warning 日志。为什么需要
当前代码检查顶层
subtype，但 CLI control response 的状态实际在response.subtype下。真实响应会导致 error 分支不可达，因此 Java session 会错过普通 daemon 响应里的 control error warning 信号。回归测试也钉住了 drain 到 result 的行为、非 error 控制响应分发，以及中性或缺失 subtype 不应误报警告的行为。Reviewer Test Plan
如何验证
在
packages/sdk-java/qwencode下运行：mvn test -Dtest=SessionTest#sendPromptDrainsTurnWhenControlResponseSubtypeIsNestedErrormvn test -Dtest=SessionTest#sendPromptUsesTopLevelSubtypeWhenNestedSubtypeIsMissingmvn test -Dtest=SessionTest#sendPromptUsesTopLevelSubtypeWhenResponseObjectIsMissingmvn test -Dtest=SessionTest#sendPromptContinuesAfterNestedControlResponseSuccessmvn test -Dtest=SessionTest#sendPromptDoesNotWarnWhenControlResponseSubtypeIsNotExactErrormvn test -Dtest=SessionTest#sendPromptDoesNotWarnWhenControlResponseSubtypeIsProgressmvn test -Dtest=SessionTest#sendPromptDoesNotWarnWhenControlResponseSubtypeIsMissing回归测试会喂入嵌套、旧顶层和缺少 response 对象的
errorcontrol response；验证 prompt turn 在result行结束而不会消费 canary 输出；断言 error response 会输出control_response errorwarning；断言 success/progress/缺失 subtype 不会误报该 warning；并断言非 error control response 仍会分发给onControlResponse。证据（Before & After）
非 UI 改动，N/A。本地静态检查：
git diff --check通过。本机缺少 Maven（mvn: command not found），无法执行本地 Java 测试；运行时验证依赖 GitHub 的 Java SDK matrix。测试环境
git diff --check环境（可选）
本机缺少 Maven，因此 Java 运行时验证交给 CI。
风险和范围
control_responsesubtype 检测和 warning 日志，并保留旧顶层 subtype 兜底。关联 Issue
关联 #8835（
java-session-control-response-subtype-wrong-level）。