[yiliang114]

What this PR does

One item from #15700's deferred list: the scanner's bounded failure summary now names the exception class when a non-first row failure carries no message. Previously such a row rendered as failure=null, with no class and no other record anywhere, because the summary is the only place an additional row failure is reported.

Why it's needed

#15632 replaced unbounded per-row stacks with one bounded summary. The sample it keeps is the only record of every failure except the first — so a message-less exception was recorded as the literal string failure=null, which identifies neither the row's cause nor its type. That is the one item in the deferred list that loses information rather than merely formatting it differently.

Reviewer Test Plan

How to verify

Non-UI. The change is confined to the sample string construction:

• With the previous wording, a tick where the second row throws a message-less IllegalStateException logs failure=null; with this change it logs failure=java.lang.IllegalStateException.
• Failures that do carry a message keep their existing rendering, so the summary format for the common case is unchanged.

aMessageLessRowFailureKeepsItsIdentityInTheSummary pins both halves, and I confirmed it fails against the unpatched construction (the assertion on failure= sees null) and passes after; commonClaimFailuresProduceOneBoundedSummary still passes unchanged.

Evidence (Before & After)

N/A — non-UI. Before (reverted hunk, same test): sample=[… failure=claim store down, … failure=null]; after: failure=java.lang.IllegalStateException. Local: both summary tests green, Checkstyle 0 violations.

Tested on

OS	Status
🍏 macOS	✅
🪟 Windows	N/A
🐧 Linux	N/A

Environment (optional)

OpenJDK 21, Maven 3.10.

Risk & Scope

• Main risk or tradeoff: only a diagnostics string changes; the counting and severity semantics of the summary are untouched, and the other eight deferred items in fix(managed-agent): close the deferred scanner summary-diagnostics suggestions from #15632 #15700 stay deferred.
• Not validated / out of scope: no MySQL or Hosted profile involvement; no behavior change for any non-logging path.
• Breaking changes / migration notes: none. Log text for message-carrying failures is unchanged.

Linked Issues

Refs #15700 — item 4 only; the other eight deferred suggestions stay deferred.

中文说明

这个 PR 做了什么

处理 #15700 延后清单中的一项：扫描器的有界失败汇总，在非首个行失败没有 message 时改为记录异常类名。此前这种情况会渲染成 failure=null，既没有类名、也没有任何其它记录——因为汇总正是额外行失败唯一的记录位置。

为什么需要

#15632 用一条有界汇总替换了逐行无界堆栈。它保留的样本是除第一个之外每个失败的唯一记录，因此无 message 的异常被记成字面量 failure=null，既不能说明原因、也不能说明类型。这是延后清单中唯一「丢信息」而非「格式不同」的一项。

评审者验证计划

如何验证

非 UI。改动仅限样本字符串构造：

• 用旧写法时，第二个行抛无 message 的 IllegalStateException，日志为 failure=null；改动后为 failure=java.lang.IllegalStateException。
• 带 message 的失败保持原有渲染，常见情形格式不变。

aMessageLessRowFailureKeepsItsIdentityInTheSummary 同时钉住这两半；我验证过它在未修复的写法下失败（failure= 处为 null）、修复后通过；commonClaimFailuresProduceOneBoundedSummary 不受影响仍通过。

证据（前后对比）

N/A —— 非 UI。修复前（回退该 hunk，同一测试）：sample=[… failure=claim store down, … failure=null]；修复后：failure=java.lang.IllegalStateException。本地两个汇总测试通过，Checkstyle 0 违规。

测试平台

macOS ✅；Windows / Linux：N/A。

环境（可选）

OpenJDK 21、Maven 3.10。

风险与范围

• 主要风险或权衡：只改诊断字符串；汇总的计数与严重度语义未动，fix(managed-agent): close the deferred scanner summary-diagnostics suggestions from #15632 #15700 其余八项仍保持延后。
• 未验证 / 不在范围：不涉及 MySQL 或 Hosted profile；任何非日志路径无行为变化。
• 破坏性变更 / 迁移说明：无。带 message 失败的日志文本不变。

关联 Issue

Refs #15700 —— 仅第 4 项；其余八条延后建议继续保持延后。

[qwen-code-review-bot]

⚠️ Deferred approval withheld — 3 PR CI workflow run(s) on f8d70f2 did not finish green; see the updated table in the Stage 2 comment. Re-run @qwen-code /triage after fixes. finalize run

⚠️ 延迟审批已搁置 —— f8d70f2 有 3 个 PR CI workflow 未以绿色完成，详见 Stage 2 评论中已更新的表格。修复后可重新运行 @qwen-code /triage。查看 finalize 运行

[qwen-code-review-bot]

Thanks for the PR!

Template looks good ✓ — all required sections present, Risk & Scope filled in, and the 中文说明 is a complete parallel translation rather than a summary.

Problem: real, and I verified it in the code rather than taking the description's word for it. In AutomationScanner.tick(), the first row failure is kept in failure and rethrown, so it still carries its stack. Every additional failure only bumps additionalFailures — the bounded 3-entry failureSample is genuinely the only place its identity is recorded, and that WARN is throwable-free (commonClaimFailuresProduceOneBoundedSummary already pins getThrowableProxy() as null). The sample was built from error.getCause().getMessage(), so a cause with no message rendered the literal failure=null: no message, no class, no stack, nowhere else. That is deterministic behaviour, not a hypothetical. It is also an agreed follow-up rather than a drive-by idea — it is item 4 (R2-4) of #15700, still open, which explicitly asks for "a small scoped fix with the pinning test that proves it".

Direction: aligned. This is diagnostics quality on a failure summary that exists precisely to illuminate a sustained outage — the case where a failure=null row costs the most. It touches no public contract, no schema, no auth or tenant boundary, and no API surface. Taking one item from a nine-item deferred list and leaving the other eight deferred is the right instinct under the repo's "don't let review rounds balloon the PR" rule. No CHANGELOG reference applies — this is a server-side log string in an unreleased module, not a user-facing CLI behaviour.

Size: not applicable. Neither changed path matches the core-module patterns (packages/core/src/**, or auth/providers/models/config/tools/services under a package's src/), and the change stays inside one package. For the record: 9 production lines (AutomationScanner.java, +8/−1) and 39 test lines (AutomationScannerTest.java, +39/−0), no generated or schema files.

Approach: this is what I would have written. Read from the problem statement alone, my proposal was to fall back to the exception's class name at the sample-construction site and nowhere else — which is exactly the shape here. Scope is minimal: one expression, one behaviour, one pinning test, no drive-by edits or formatting churn, and no attempt to fold in the neighbouring deferred items (R2-3's sample/count population mismatch sits three lines away and is correctly left alone). Two small non-blocking thoughts go in the code review below rather than here.

Risk: no elevated risk signals. Neither file matches the revert-correlated high-risk paths, so no extra review depth is warranted beyond the normal pass.

Moving on to code review. 🔍

中文说明

感谢贡献！

模板完整 ✓ —— 必需章节齐全，Risk & Scope 已填写，中文说明是逐段对应的完整翻译而非摘要。

问题：真实存在，并且我是读代码确认的，不是照抄 PR 描述。 AutomationScanner.tick() 中，第一个行失败会被存入 failure 并重新抛出，因此仍带堆栈；而其余每个失败只是让 additionalFailures 加一 —— 有界的 3 条 failureSample 确实是其身份唯一的记录位置，且该 WARN 不携带 throwable（commonClaimFailuresProduceOneBoundedSummary 已钉住 getThrowableProxy() 为 null）。样本原先由 error.getCause().getMessage() 构造，所以无 message 的 cause 会渲染成字面量 failure=null：没有 message、没有类名、没有堆栈，别处也没有。这是确定性行为，不是假设。它也是一个已达成共识的后续项，而非临时起意 —— 即 #15700（仍为 open）的第 4 项（R2-4），该 issue 明确要求「一个小范围修复，并配上能证明它的钉住测试」。

方向：对齐。 这是针对失败汇总的诊断质量改进，而该汇总存在的意义正是照亮持续性故障 —— 恰恰是 failure=null 代价最高的场景。改动不涉及公共契约、schema、鉴权或租户边界，也不涉及任何 API 面。从九项延后清单中取一项、其余八项继续延后，符合仓库「不要让评审轮次把 PR 撑大」的规则。CHANGELOG 无对应参考 —— 这是未发布模块中的服务端日志字符串，不是面向用户的 CLI 行为。

规模：不适用。 两个改动路径都不匹配核心模块规则（packages/core/src/**，或某个包 src/ 下的 auth/providers/models/config/tools/services），且改动限于单个包内。记录一下：生产代码 9 行（AutomationScanner.java，+8/−1），测试 39 行（AutomationScannerTest.java，+39/−0），无生成文件或 schema 文件。

方案：这就是我会写的样子。 只看问题描述时，我的方案就是「在样本构造处回退到异常类名，且仅此一处」—— 与本 PR 完全一致。范围最小化：一个表达式、一个行为、一个钉住测试，没有顺手改动或格式化噪声，也没有试图把相邻的延后项一起塞进来（R2-3 的样本/计数口径不一致就在三行之外，被正确地留着没动）。两条非阻塞的小想法放在下面的代码审查里。

风险：无升级风险信号。 两个文件都不匹配与 revert 相关的高风险路径，因此除常规审查外不需要额外的审查深度。

进入代码审查 🔍

— Qwen Code · qwen3.8-max-2026-09-02

_{Reviewed at f8d70f27e7538c0445f413cb9f6b9bd39ecf48dd · re-run with @qwen-code /triage}

[qwen-code-review-bot]

Code review

No Critical findings, and no AGENTS.md violations. The change is correct and it stays inside its own scope. Details worth recording, since a green suite on a 9-line diff proves very little on its own:

The fix is right, and it changes nothing else. The ternary's true branch is byte-for-byte the old expression, so a failure that does carry a message renders exactly as before — the counting (additionalFailures), the 3-entry bound, the WARN severity and the throwable-free payload are all untouched. That is what keeps this to item 4 and leaves item R2-3 (the sample records the first failure while the count excludes it) alone, three lines away, as intended. Using getClass().getName() rather than getSimpleName() is the better call for a log line that has to identify a cause across packages.

No new null surface. error.getCause() is now assigned to a local before being dereferenced, but the pre-existing line already dereferenced it in the same expression, and FutureTask always attaches a cause to the ExecutionException it throws — so the change adds no NPE path that was not already there.

The new test is deterministic — I checked this specifically, because it looks racy and it isn't. The production scanner pool is scanner-threads: 4, so a test that splits behaviour on claims.incrementAndGet() == 1 while asserting against join order would be a coin flip. It isn't one here: the 5-argument package-private constructor the test uses (new AutomationScanner(downAtClaim, connector, settings, clock::get, "scanner-silent-store")) defaults the executor to Runnable::run, so tasks execute serially during submission, in findArmed order — which is also the LinkedHashMap join order. Claim order and join order coincide by construction. Worth stating because the design doc already records the opposite case for aTickJoinsEveryRowBeforeItReportsOneFailure, which needs a real one-thread pool precisely because a direct executor cannot pin a join.

The test is load-bearing, not decorative. Against the unpatched construction the sample holds failure=null, so contains("failure=" + IllegalStateException.class.getName()) fails; the doesNotContain("failure=null") assertion pins the old rendering from the other side. No new imports are needed — every symbol it uses is already imported by commonClaimFailuresProduceOneBoundedSummary fifty lines above. logged.list having size 1 is safe: the test calls tick() directly, bypassing scan()'s own catch-and-warn.

Three non-blocking notes — none of them a reason to hold the PR:

• An empty message still loses the identity. getMessage() != null covers the null case item 4 describes, but an exception carrying "" renders failure= with nothing after it — the same information loss, one edge over. This module declares org.apache.commons.lang3.StringUtils for null-safe blank checks (41 files under src/main use it, none in the automation package yet), and StringUtils.defaultIfBlank(cause.getMessage(), cause.getClass().getName()) would cover both. Equally, Objects.requireNonNullElse(...) is the JDK one-call form of the exact semantics you have now, if you would rather not widen the blank case. Your call — the current form does fix the item as written.
• The "both halves" claim is slightly stronger than the test. The PR description says the test pins that message-carrying failures keep their rendering. It pins the new half, and it pins the first failure's message via hasMessage, but it never asserts that the sample still reads failure=claim store down — and no other test does either (commonClaimFailuresProduceOneBoundedSummary asserts tenant/schedule substrings and the counts, not the failure text). One extra contains("failure=claim store down") would make that half explicit and cost a line.
• The new pinning test is not in the design doc's Validation register. That register lists pinning tests by name in both language versions (commonClaimFailuresProduceOneBoundedSummary sits at en:121 / zh-CN:103). Leaving it out is consistent with your stated scope, but it is the same gap fix(managed-agent): close the deferred scanner summary-diagnostics suggestions from #15632 #15700 item 8 tracks for refactor(managed-agent): close the scanner row-pool residues (#15527) #15632's test, so this PR opens a second instance of it. Either add one line per language here, or note on item 8 that it should absorb this test too, so the register does not silently drift.

The three-line comment in tick() is justified — "the sample is the only record of an additional failure" is exactly the kind of non-obvious why that warrants a comment, and it matches the dense why-comments this method already carries.

Test evidence

This is an unattended CI run, so per the review rules I did not build, run, or execute anything from this PR — no mvn, no checkout, no PR-derived code. The evidence below is the PR's own CI, read through the API for the reviewed commit, plus the static reasoning above.

Snapshot at review time — 38 check-runs on f8d70f2, still in flight. The legs that actually validate this diff had not finished, so I am reporting them as pending rather than guessing an outcome:

Final CI results for f8d70f2 (auto-updated by the triage finalize job after CI completed):

Check	Conclusion
(needs.scope.outputs.tier == 'full' || needs.scope.outputs.touched != 'true' || (github.event_name != 'pull_request' &&	🚫 cancelled
Build and verify reference distribution	🚫 cancelled
Installed npm global and npx (${{ matrix.target }})	🚫 cancelled
Installed npm Node26 private Bun acceptance	🚫 cancelled
Publish experimental reference distribution	🚫 cancelled
SDK Java (fast) / ubuntu-latest / Java 21	🚫 cancelled
Check TUI impact	✅ success
Classify PR	✅ success
Integration Tests (no-AK, No Sandbox)	✅ success
Lint & Static (ubuntu-latest, Node 22.x)	✅ success
SDK Java (fast) / Flyway migration version uniqueness	✅ success
SDK Java (fast) / Real daemon E2E / Java 11	✅ success
Select the Managed Agent tier	✅ success
Test (ubuntu-latest, Node 22.x)	✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x)	✅ success

_{One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。}

The one red check is runner infrastructure, not this PR. web-shell E2E Smoke failed in a setup step before any test ran:

cache_dir="${HOME}/.cache/qwen-code/npm"
mkdir -p "${cache_dir}"
…
mkdir: Permission denied
##[error]Process completed with exit code 1.

The step immediately before it succeeded (Using pre-installed Node v22.23.4 / npm 10.9.9), so the runner had a working toolchain and then could not create its npm cache directory. I am classifying this from the identity of the failing step and the scope of the diff — a Java-only change under packages/managed-agent-server cannot affect a self-hosted runner's filesystem permissions on ${HOME}/.cache. It needs a maintainer's eye on the runner, not a change to this PR.

Not verified, and explicitly so: the Java leg that runs this module's unit/H2 suite, Checkstyle and SpotBugs (SDK Java (fast) / ubuntu-latest / Java 21) was still in progress at review time, so the new test's actual result and the Checkstyle claim are unconfirmed here. The PR description reports both green locally on macOS with OpenJDK 21 — that is the author's claim, not evidence I re-ran, and I did not treat it as a substitute for the pending check. Per the review rules I do not poll CI; the finalize pass rewrites the table above once these settle.

Sandboxed verification would settle one specific thing: @qwen-code /verify — that aMessageLessRowFailureKeepsItsIdentityInTheSummary genuinely fails against the unpatched construction. The author says they reverted the hunk and watched it fail, which is the right check, but it is a claim I cannot re-run from a static review, and CI only proves the test passes, never that it is load-bearing. Everything else here is settled by the pending Java leg; there is no TUI surface, so /tmux has nothing to drive.

中文说明

代码审查

无 Critical 问题，也没有违反 AGENTS.md。改动是正确的，并且严格守住自己的范围。以下几点值得记录，因为 9 行 diff 上的绿色测试本身说明不了太多：

修复本身正确，且没有顺带改变别的行为。 三元表达式的真分支与旧表达式逐字一致，所以带 message 的失败渲染完全不变 —— 计数（additionalFailures）、3 条上限、WARN 级别、不携带 throwable 的负载都未动。这正是它只解决第 4 项、而把 R2-3（样本包含首个失败而计数不含）原样留在三行之外的原因。用 getClass().getName() 而非 getSimpleName() 对一条需要跨包定位 cause 的日志来说是更好的选择。

没有引入新的空指针面。 error.getCause() 现在先赋给局部变量再解引用，但原来那一行本就在同一表达式里解引用它，且 FutureTask 抛出的 ExecutionException 一定带 cause —— 所以没有新增原本不存在的 NPE 路径。

新测试是确定性的 —— 我特意查了这点，因为它看起来像竞态，实际不是。 生产扫描器线程池是 scanner-threads: 4，因此一个用 claims.incrementAndGet() == 1 区分行为、又按 join 顺序断言的测试本该是掷硬币。这里不是：测试使用的 5 参包级构造函数（new AutomationScanner(downAtClaim, connector, settings, clock::get, "scanner-silent-store")）把 executor 默认成 Runnable::run，任务在提交过程中串行执行，顺序即 findArmed 顺序，也就是 LinkedHashMap 的 join 顺序。claim 顺序与 join 顺序由构造天然一致。之所以要说明：设计文档里恰好记录过相反的例子 —— aTickJoinsEveryRowBeforeItReportsOneFailure 必须用真实的单线程池，因为直接 executor 钉不住 join。

这个测试是有效的，不是摆设。 在未修复的构造下样本是 failure=null，contains("failure=" + IllegalStateException.class.getName()) 会失败；doesNotContain("failure=null") 从另一侧钉住旧渲染。无需新增 import —— 它用到的每个符号，上方五十行的 commonClaimFailuresProduceOneBoundedSummary 已经引入。logged.list 断言为 1 条是安全的：测试直接调用 tick()，绕过了 scan() 自己的 catch-and-warn。

三条非阻塞意见 —— 都不是压住这个 PR 的理由：

• 空 message 仍会丢掉身份。 getMessage() != null 覆盖了第 4 项描述的 null 情形，但携带 "" 的异常会渲染成 failure=，后面什么都没有 —— 差一个边界，信息丢失是同一类。本模块声明了 org.apache.commons.lang3.StringUtils 用于空值安全的 blank 检查（src/main 下 41 个文件在用，automation 包目前还没有），StringUtils.defaultIfBlank(cause.getMessage(), cause.getClass().getName()) 能同时覆盖两种情况。如果不想扩大到 blank 情形，Objects.requireNonNullElse(...) 则是当前语义的 JDK 单次调用写法。你来定 —— 现有写法确实按字面修好了这一项。
• 「同时钉住两半」的说法比测试本身略强。 PR 描述称测试钉住了带 message 失败的渲染不变。它钉住了新的一半，也通过 hasMessage 钉住了首个失败的 message，但从未断言样本里仍然是 failure=claim store down —— 其它测试也没有（commonClaimFailuresProduceOneBoundedSummary 断言的是 tenant/schedule 子串和计数，不是 failure 文本）。多加一行 contains("failure=claim store down") 就能把这半说清楚。
• 新的钉住测试没有进设计文档的 Validation register。 该 register 在两个语言版本里都按名字列出钉住测试（commonClaimFailuresProduceOneBoundedSummary 在 en:121 / zh-CN:103）。不加与你声明的范围一致，但这与 fix(managed-agent): close the deferred scanner summary-diagnostics suggestions from #15632 #15700 第 8 项所追踪的、针对 refactor(managed-agent): close the scanner row-pool residues (#15527) #15632 测试的缺口是同一个，于是本 PR 又开了第二例。要么在这里每种语言各加一行，要么在第 8 项里注明它也应吸收这个测试，以免 register 悄悄漂移。

tick() 里那三行注释是合理的 ——「样本是额外失败唯一的记录」正是那种值得写下来的非显而易见原因，也与该方法已有的密集 why 注释风格一致。

测试证据

这是无人值守的 CI 运行，因此按审查规则我没有构建、运行或执行本 PR 的任何内容 —— 没有 mvn、没有 checkout、没有执行 PR 带来的代码。下面的证据是通过 API 读到的、该 PR 自己在被审 commit 上的 CI 结果，加上上文的静态推理。

审查时点的快照 —— f8d70f2 上 38 个 check-run，仍在进行中。真正验证本 diff 的几个 leg 还没跑完，所以我如实报告为 pending，而不是猜测结果（表格见英文部分，此处不重复）。

唯一的红 check 是 runner 基础设施问题，与本 PR 无关。 web-shell E2E Smoke 在任何测试运行之前的准备步骤就失败了：mkdir -p "${HOME}/.cache/qwen-code/npm" 报 mkdir: Permission denied，进程以 exit code 1 结束。它前一步是成功的（Using pre-installed Node v22.23.4 / npm 10.9.9），说明 runner 工具链正常，只是无法创建自己的 npm 缓存目录。我这个判断依据的是失败步骤的身份和 diff 的范围 —— 一个只改 packages/managed-agent-server 下 Java 代码的 PR，不可能影响自托管 runner 上 ${HOME}/.cache 的文件系统权限。这需要维护者关注 runner，而不是改这个 PR。

未验证，并明确说明：运行本模块 unit/H2 套件、Checkstyle 与 SpotBugs 的 Java leg（SDK Java (fast) / ubuntu-latest / Java 21）在审查时仍在进行中，所以新测试的实际结果和 Checkstyle 结论在此未被确认。PR 描述称在 macOS + OpenJDK 21 上本地两项均通过 —— 那是作者的声明，不是我复跑的证据，我也没有拿它替代这个 pending 的 check。按审查规则我不轮询 CI；这些跑完后，finalize 环节会就地重写上面的表格。

沙箱验证可以确定一件具体的事：@qwen-code /verify —— 即 aMessageLessRowFailureKeepsItsIdentityInTheSummary 在未修复的构造下确实会失败。作者说回退该 hunk 后看着它失败了，这是正确的检查方式，但静态审查无法复跑；CI 只能证明测试通过，永远证明不了它是有效的。其余部分由 pending 的 Java leg 决定；没有 TUI 面，所以 /tmux 无可驱动。

— Qwen Code · qwen3.8-max-2026-09-02

_{Reviewed at f8d70f27e7538c0445f413cb9f6b9bd39ecf48dd · re-run with @qwen-code /triage}

[qwen-code-review-bot]

Confidence: 4/5 — clean across every stage; the three notes above are polish, not doubt about the change.

Stepping back: this is what a well-behaved follow-up looks like. A nine-item deferred list was recorded in an open issue instead of being folded into #15632 while that PR was still under review; one item — the only one that lost information rather than reformatting it — got picked up on its own, with the pinning test the issue asked for, and the other eight were explicitly left alone. That is the "don't let review rounds balloon the PR" rule being followed rather than quoted at.

My independent proposal, written from the problem statement before I read the diff, was to fall back to the exception class name at the sample-construction site and nowhere else. The PR does exactly that. I did not find a simpler path it missed, and I looked for one — the ternary is the whole change, and the alternatives I can name (Objects.requireNonNullElse, StringUtils.defaultIfBlank) are the same size with slightly different edge behaviour, not simplifications.

The part I would normally worry about on a diff this small is a test that passes for the wrong reason, so I went after it directly. Two things I expected to be problems turned out not to be: the claims.incrementAndGet() == 1 split is not a race, because the constructor the test uses defaults to a direct executor and so claim order and join order coincide by construction; and the assertion is genuinely load-bearing, because the unpatched expression renders failure=null and the contains on the class name cannot pass against it. Six months from now this reads as an obvious line with a test that explains itself — I would thank the author, not curse them.

What I am not confident about is the one thing I could not check: the Java leg that runs this module's unit/H2 suite, Checkstyle and SpotBugs was still in flight at review time, so the new test's real result is unconfirmed. I am not going to approve against a result that does not exist yet, and I am not going to treat the author's local macOS run as a substitute for it — hence the deferral below rather than an approval now. The red web-shell E2E Smoke check does not change that calculus: it died in a runner cache-setup step with mkdir: Permission denied before running any test, and a Java-only diff cannot cause it. It does deserve a maintainer's look at that runner, separately from this PR.

So: approve on the merits, deferred until CI lands green on the reviewed commit. The three notes — the empty-message edge, the one missing assertion for the unchanged rendering, and the absent Validation-register entry that duplicates #15700 item 8's gap — are all fine to handle here or in the item 8 follow-up. None of them should hold this up.

中文说明

信心度：4/5 —— 各阶段都干净；上面三条意见属于打磨，而不是对改动本身的怀疑。

退一步看整体：这是一个规范的后续 PR 该有的样子。九项延后清单被记录在一个 open issue 里，而不是在 #15632 还在评审时塞进那个 PR；随后单独取出一项 —— 其中唯一「丢信息」而非「换个格式」的一项 —— 并按该 issue 的要求配上钉住测试，其余八项明确保持不动。这是在真正执行「不要让评审轮次把 PR 撑大」这条规则，而不只是引用它。

在读 diff 之前，我根据问题描述写下的独立方案就是：在样本构造处回退到异常类名，且仅此一处。本 PR 正是如此。我没有找到它漏掉的更简路径，而且是认真找过的 —— 三元表达式就是全部改动，我能想到的替代写法（Objects.requireNonNullElse、StringUtils.defaultIfBlank）体量相同、只是边界行为略有差异，并不构成简化。

对这么小的 diff，我通常会担心的问题是「测试因错误的原因通过」，所以我直接去查了这一点。两个我预期会成为问题的地方最终都不是：claims.incrementAndGet() == 1 的分支不是竞态，因为测试用的构造函数默认走直接 executor，claim 顺序与 join 顺序由构造天然一致；断言也确实有效，因为未修复的表达式渲染出 failure=null，针对类名的 contains 不可能通过。六个月后再读，这是一行显而易见的代码配一个能自我说明的测试 —— 我会感谢作者，而不是骂他。

我不确定的是唯一一件查不了的事：运行本模块 unit/H2 套件、Checkstyle 与 SpotBugs 的 Java leg 在审查时仍在进行，所以新测试的真实结果尚未确认。我不会对着一个还不存在的结果去 approve，也不会拿作者本地的 macOS 运行来替代它 —— 因此下面是延后而非立即批准。红色的 web-shell E2E Smoke 不改变这个判断：它在运行任何测试之前，就在 runner 的缓存准备步骤里以 mkdir: Permission denied 失败了，而只改 Java 的 diff 不可能导致它。那个 runner 确实值得维护者单独看一眼，但与本 PR 无关。

结论：就改动本身而言批准，但延后到 CI 在被审 commit 上跑绿。三条意见 —— 空 message 的边界、缺少的那一条「渲染不变」断言、以及重复了 #15700 第 8 项缺口的 Validation register 未登记 —— 放在本 PR 处理或留给第 8 项的后续都可以，都不应成为压住它的理由。

— Qwen Code · qwen3.8-max-2026-09-02

_{Reviewed at f8d70f27e7538c0445f413cb9f6b9bd39ecf48dd · re-run with @qwen-code /triage}

Approval deferred until CI lands green on f8d70f27e7538c0445f413cb9f6b9bd39ecf48dd; the finalize pass posts the commit-pinned approval and withholds it if anything lands red or the head moves.

[qwen-code-review-bot]

Qwen Code review was cancelled before a review could be posted. Nothing failed and nothing is retried automatically: the run was cancelled — by an operator, an upstream event, or the job exceeding its execution time limit. If you still want a review of this PR, request one with @qwen-code /review. See workflow logs.

[yiliang114]

The original cache-permission failure is historical: job 122259155192 on ecs-qwen-hz4-30 failed at 2026-10-27 14:46:06Z. The current cache directory was created later, at 16:28:48Z.

At 17:24:04–05Z we directly tested all six hz hosts using each actual github-runner account: create a unique temporary directory under the existing npm cache, write/read back a fixed marker, then remove only those probe files. All six passed creation, write/readback and cleanup (exit 0), including hz4. No permissions or shared cache contents were changed by that validation.

This PR already has attempt 2 running, so no duplicate rerun was triggered. Its current smoke job on hz1-3 has passed npm-cache configuration and font preflight and is installing dependencies: https://github.com/QwenLM/qwen-code/actions/runs/40524112310/job/122348773711 . This confirms the cache prerequisite is currently usable; full E2E result remains pending.

[yiliang114]

Local verification on 2026-10-28 for head f8d70f27e7538c0445f413cb9f6b9bd39ecf48dd. The clean local worktree matches the published head.

The message-less sampled failure now logs failure=java.lang.IllegalStateException; the existing bounded-summary case also passes.

Check	Command	Result
Summary regression tests and style	JAVA_HOME=/opt/homebrew/opt/openjdk@21 mvn -q -Dtest='AutomationScannerTest#aMessageLessRowFailureKeepsItsIdentityInTheSummary+commonClaimFailuresProduceOneBoundedSummary' test checkstyle:check	exit 0; both tests passed

Java commands ran in packages/managed-agent-server; formatting ran from the repository root. UI screenshots are not applicable to these changes. This is focused local evidence, not a full-suite pass. Remote Java/acceptance/review runs were cancelled; those checks and formal approval still need completion before merge.

[chiga0]

Scope: 2 files — AutomationScanner.java (method body) and AutomationScannerTest.java (new test). Standard tier.

Findings

No blocking findings.
Approval blockers: none.

What was checked

Read

• Full context of the changed ExecutionException catch block (lines 155–210 at head). The failureSample branch is the only site in the file calling error.getCause().getMessage(); a grep for getCause and getMessage found no sibling site with the same pattern.
• Pre-existing commonClaimFailuresProduceOneBoundedSummary: uses only message-carrying failures throughout, so the new ternary (cause.getMessage() != null ? … : cause.getClass().getName()) does not alter its path. Consistent with the PR description.
• New test aMessageLessRowFailureKeepsItsIdentityInTheSummary: correctly exercises both halves — first failure has a message and is rethrown; second is message-less and lands in failureSample as failure=java.lang.IllegalStateException. Assertions contains("failure=" + IllegalStateException.class.getName()) and doesNotContain("failure=null") together pin the fix without ambiguity.

Pre-existing issue not introduced by this PR: error.getCause() itself is not null-checked before the ternary. If ExecutionException were constructed with a null cause, this would NPE as it did before. The PR does not widen this risk.

Not covered: no local working tree, so rungs 2/3 (local build / mutation probe) were not run. The change is confined to a diagnostic-string branch inside a catch block with no behavioral effect outside LOG.warn, so the coverage gap is not material.

Reviewed with AI assistance.

[qqqys]

Approving at f8d70f27e7538c0445f413cb9f6b9bd39ecf48dd.

I read the production hunk in its full context in tick() rather than from the diff alone, because the interesting questions here are whether the fallback can itself throw and whether anything beside the string moved.

No new null surface. The obvious worry with error.getCause() is a null cause turning a diagnostics path into an NPE. That dereference is not new: the pre-patch line was error.getCause().getMessage(), and failure = error.getCause() a few lines above already assigns the same value, with the rethrow chain below doing failure instanceof Error / instanceof RuntimeException. The new code hoists the identical call into a local and dereferences it exactly as before, so the assumption set is unchanged. It also holds structurally — the only source of ExecutionException here is task.getKey().get() on a FutureTask, whose run() stores a caught Throwable, so the cause is non-null on every path that reaches this catch.

Nothing but the string moved. The ternary's true branch is byte-identical to the old expression, so a failure carrying a message renders exactly as before. additionalFailures++, the failureSample.size() < 3 bound, the WARN severity and the throwable-free payload are all untouched, and the failure assignment that drives the rethrow is untouched. So the summary stays bounded in the sense #15632 established — bounded count of samples — and the only length delta is a class name replacing the literal null. getClass().getName() over getSimpleName() is right for a line whose job is identifying a cause across packages.

The pinning test is deterministic, which is not obvious from reading it. The test throws a message-carrying exception on whichever row claims first and a message-less one on the rest, then asserts the rethrown failure hasMessage("claim store down") — but the rethrown failure is the first one in join order, not execution order. Those two orders coincide only because the 5-arg test constructor (AutomationScanner.java lines 91-95) delegates with Runnable::run at line 94, so executor.execute(task) runs each task inline during the submit loop, in LinkedHashMap insertion order. Execution order therefore equals join order equals findArmed order, and row 1 gets the message-carrying throw while row 2 supplies the sampled message-less one. I confirmed this rather than assuming it, because under the 4-thread automationScannerExecutor the same assertions would be genuinely racy. logged.list having size 1 is safe for the reason the code shows: tick() is called directly, bypassing scan()'s own catch-and-warn, and additionalFailures == 1 > 0 produces exactly one WARN.

The test also fails against the unpatched construction from both directions — contains("failure=java.lang.IllegalStateException") would see null, and doesNotContain("failure=null") would trip — so it pins the change rather than merely accompanying it.

On CI: the red entries are cancellations, not failures, and the lane that validates this diff had already produced its result before being cancelled. In SDK Java (fast) / ubuntu-latest / Java 21 the log shows Running com.alibaba.qwen.code.managedagent.automation.AutomationScannerTest followed by Tests run: 71, Failures: 0, Errors: 0, Skipped: 0, and You have 0 Checkstyle violations. earlier in the same job; the job's step 11 was cancelled afterwards, with steps 15-16 skipped as a consequence. review-pr, the npm/Bun acceptance lanes and the reference-distribution lanes are likewise cancelled and have no path to a nine-line diagnostics change in a Java module. A SDK Java (full) tier run is still pending on this commit; I did not wait on it, and nothing in the completed evidence is attributable to this PR.

One adjacent item I am deliberately not raising as a blocker, since it predates this PR and is already tracked: an exception whose message is the empty string still renders failure= with nothing after it, and the sample records the first failure while the count excludes it. The first is the same rendering as before the change, so it is not a regression, and the second is #15700's item R2-3 three lines away. This PR does what it says it does — item 4 only — and leaves the rest alone.

[qwen-code-review-bot]

Released in v0.26.3.

[github-actions]

Managed Agent gate failed

Run 40628289562 · commit f8d70f27e753

• SDK Java (full) / Runtime Broker and Managed Agent MariaDB / Java 21: Install and verify Managed Agent dependencies, tests, Checkstyle, and MySQL integration
• SDK Java (full) / Hosted process fault gates / MySQL 8.4 / Java 21: Verify Hosted Java, Spring and MySQL processes
• SDK Java (full) / EventTransport multi-node gates / RocketMQ 5.5.1 + MySQL 8.4 / Java 21: Start the RocketMQ LiteTopic cluster
• Managed Agent gate: Report the tier result

Failing tests

• com.alibaba.qwen.code.managedagent.ManagedSessionLifecycleTest.aFailedRenameLogsAndChainsTheRootCause
• com.alibaba.qwen.code.managedagent.HostedAutomationSpringCrashIT.fg11c2SpringFixtureBootsAndScansOnMySql

Tracking issue