fix(serve): prefer a usable issuer over an expired same-subject twin - #10042
Conversation
|
Thanks for the PR! Template looks good ✓ Problem: real and demonstrated, not theoretical. Renewing a CA genuinely leaves two certificates in the bundle that share a subject and a key, so both pass the walk's "did this sign that" test; the greedy first-match then reports the expired twin as the path the handshake depends on whenever it sorts first — a false Direction: aligned. Follow-up fix to the diagnostic #9392 just introduced — a check whose entire purpose is to catch real outages was instead crying wolf on the exact configuration a renewal produces. No direct CHANGELOG reference, but the area shipped recently and is clearly relevant. Size: not applicable (no core paths; 60 production lines changed: +44/−16, plus 202 test lines). Approach: the scope feels right, and what the review rounds added since the first pass is in scope rather than bloat: (1) the walk's issuer preference and the per-member validity flags now judge one sampled clock ( Risk: no elevated risk signals (no high-risk path matches). Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：真实且可复现，不是理论性问题。CA 续期确实会在 bundle 里留下两份共享 subject 和密钥的证书，两份都能通过链上行"是不是它签的"检查；贪心首匹配在过期那份排在前面时，会把它报成握手所依赖的路径——在健康的续期 CA 配置上误报 方向：对齐。这是 #9392 刚引入的诊断的后续修复——一个本应捕捉真实故障的检查，反而在续期这一标准场景上误报。CHANGELOG 无直接引用，但该功能刚上线，领域明确相关。 规模：不适用（未触及核心路径；生产代码改动 60 行：+44/−16，另有测试 202 行）。 方案：范围合理，且首轮之后各评审轮次新增的内容属于范围内而非膨胀：(1) 链上行的签发者偏好与逐成员有效期判定现在共用同一个采样时钟（ 风险：无升级风险信号（未命中高风险路径）。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewBefore reading the diff, my independent proposal for this bug was exactly what the PR does: collect every issuer the walk's signature predicate accepts, prefer one whose validity window contains the current time, and fall back to the previous first-match when nothing is currently valid. The diff matches it, and goes one step further in a way I'd have asked for: the clock is sampled once in Walking the edge cases against the surrounding code at this head:
No critical issues; no convention violations. Two non-blocking nits, both already out of the merge path: TestingEvidence carried: the PR's own CI state for the reviewed commit via the GitHub API, with log excerpts from the cancelled job. Triage never builds or runs PR code, so nothing below was independently re-executed. The main unit job on this commit did not finish — it hit the 60-minute job limit and was cancelled mid-run (annotation: All five new tests pass. The one failure inside this file — Separately, the behavioral claim itself no longer rests on the author's word: maintainer verification at the earlier head built both arms from source and measured a real CI at
One row per check name (latest run); bot-orchestration checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略机器人编排检查，失败项排在最前。 Not verified: a completed unit suite at this head — the run was killed by the job limit before the final summary, so "green" does not exist yet for 中文说明代码审查读 diff 之前，我对这个 bug 的独立方案与 PR 完全一致：收集链上行签名判定接受的所有签发者，优先选择有效期覆盖当前时间的一份，没有可用候选时回退到原来的首匹配。diff 与之相符，并且多做了一步我也会要求的事：时钟在 对照当前提交的周边代码逐一核对边界情形：
无关键问题，无规范违规。两条不阻塞合并的小问题，且都已在合并路径之外： 测试本节证据：被审提交自身 CI 状态（经 GitHub API 获取）及被取消任务的日志摘录。triage 从不构建或运行 PR 代码，以下内容均未独立复跑。 本提交的主单测任务没有跑完——命中 60 分钟任务上限，中途被取消（标注： 另外，行为性结论本身已不再依赖作者自述：维护者在更早的提交上从源码构建了对照组与修复组，用真实 未验证：本提交上一次跑完的单测套件——任务在最终汇总前被上限终止， — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — the code itself is ready to ship (the only nits are non-blocking and recorded upstream); approval is withheld in this run solely because CI on the reviewed head timed out and never went green. Stepping back: this remains the minimal correct fix for a defect that is real and deterministic — a renewed CA leaves two same-subject, same-key certificates in the bundle, and the greedy walk reported the expired twin as the path the handshake depends on. My independent proposal was the same filter-and-prefer with a first-match fallback; I found no simpler path, and the review rounds added exactly the hardening I would have asked for: one sampled clock shared by the walk and the report, and boundary tests at the exact notAfter/notBefore instants. The maintainer's measured A/B on a real daemon closed the behavioral question at the fix commit (false alarm gone in every ordering, no true positive lost), and the delta since then is pinned by tests that passed in CI on this head. Six months from now this reads as careful, not clever. Why no approval this run: the unit job on 中文说明置信度：4/5 —— 代码本身已可合并（仅有的小问题均不阻塞且已记录在上游）；本次运行未批准，仅因被审提交上的 CI 超时、从未转绿。 退一步看：这仍是针对一个真实且确定性缺陷的最小正确修复——CA 续期会在 bundle 里留下两份同 subject、同密钥的证书，贪心链上行把过期的孪生根报成握手所依赖的路径。我的独立方案就是同样的 filter-and-prefer 加首匹配回退；没有找到更简单的路径，而各评审轮次补充的正是我会要求的加固：链上行与报告共用同一个采样时钟，以及在 notAfter/notBefore 精确时刻的边界测试。维护者在修复提交上用真实 daemon 完成的实测 A/B 已经关闭了行为性问题（各种排序下误报均消失，且不丢失任何真阳性），此后的增量由本次提交上已通过 CI 的测试钉住。六个月后再看，这份代码是细致而非炫技。 本次为何不批准： — Qwen Code · qwen3.8-max Reviewed at |
qqqys
added
the
autofix/takeover
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 12 scenario(s). — Qwen Code · serve A/B |
|
Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with |
|
🤝 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. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： No action needed this roundThis round's feedback contains no actionable findings, and the red items are transient CI cancellations, not code defects. Nothing was committed. Feedback triage
Failed checks are cancellations, not failuresAll three red checks concluded CANCELLED, not FAILURE, and none carries any test-error evidence:
A cancelled run's remedy is re-running the workflow, which the workflow/maintainers own — no code change can influence it. Local evidence on this commit (
|
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 explored to full depth (tool budget reached): "agent 3b": none — nothing was cut short..
Test Plan (not a blocker): src/serve/run-qwen-serve.test.ts — no such file or directory; Tests 328 passed — this review observed 24761 passed.
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）："agent 3b"：none — nothing was cut short.。
Test Plan（非阻断）：src/serve/run-qwen-serve.test.ts — no such file or directory; Tests 328 passed — this review observed 24761 passed。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| const issuer: X509Certificate | undefined = | ||
| issuers.find((candidate) => certValidAt(candidate, Date.now())) ?? | ||
| issuers[0]; |
There was a problem hiding this comment.
[Suggestion] The walk's validity preference and its caller's reporting gate now implement the same validity decision in two independent places, sampled from two different clocks: walkWorkerAnchorPath samples Date.now() inside the issuer selection (line 1554), while describeWorkerTlsTrustGaps captures its own const now = Date.now() later (line 1114) and flags path members with separate inline comparisons (validTo < now / validFrom > now, lines 1135–1147) instead of reusing the new certValidAt helper. Today the two sites agree only by coincidence — the boundary semantics happen to be exact negations of each other — and nothing documents or tests the invariant: every fake-clock fixture sits years from any validity boundary, so a one-site edit passes the suite silently. When that drift lands, the walk can again select an issuer the reporting loop contradicts, and the diagnostic tells an operator to renew a CA that OpenSSL actually authorizes through the renewed twin — precisely the false CERT_HAS_EXPIRED alarm this PR removes; even today, a boot that lands on the millisecond where a renewed twin's boundary sits between the two samples produces the same disagreement. Capture now once in describeWorkerTlsTrustGaps and pass it into walkWorkerAnchorPath (single caller, line 921) for the certValidAt preference, and route the per-member loop through the same helper (flag on !certValidAt(member, now), keeping the expired/not-yet-valid message split), so the one-clock/one-predicate invariant is explicit instead of coincidental.
中文说明
链上行的有效期偏好和调用方的报告关卡现在在两个独立的位置实现了同一个有效期判定，并且分别从两个不同的时钟采样：walkWorkerAnchorPath 在签发者选择内部采样 Date.now()（第 1554 行），而 describeWorkerTlsTrustGaps 在之后捕获自己的 const now = Date.now()（第 1114 行），并用独立的内联比较（validTo < now / validFrom > now，第 1135–1147 行）标记路径成员，没有复用新的 certValidAt 辅助函数。目前两处一致纯属巧合——边界语义恰好互为否定——且没有任何文档或测试固定这个不变量：所有伪造时钟的 fixture 都离有效期边界相差数年，因此只改一处的编辑会悄无声息地通过整个测试套件。当这种漂移出现时，链上行可能再次选出一个与报告循环相矛盾的签发者，诊断会告诉运维去续期一个 OpenSSL 实际上通过续期副本完成授权的 CA——正是本 PR 要消除的 CERT_HAS_EXPIRED 误报；即便在今天，一次启动若恰好落在续期孪生证书的边界处于两次采样之间的毫秒上，也会产生同样的不一致。建议在 describeWorkerTlsTrustGaps 中只捕获一次 now，并将其传入 walkWorkerAnchorPath（唯一调用点，第 921 行）用于 certValidAt 偏好，同时让逐成员循环也通过同一个辅助函数判定（以 !certValidAt(member, now) 标记，保留过期/尚未生效的消息分支），使“一个时钟、一个谓词”的不变量显式化，而不是依赖巧合。
— qwen3.8-max via Qwen Code /review (v0.22.0)
Maintainer verification — real
|
| Serving bundle (order as written in the PEM) | arm | boot diagnostic reports | measured worker handshake |
|---|---|---|---|
| A leaf-ok + EXPIRED + renewed | base | (none) | handshake OK |
| A leaf-ok + EXPIRED + renewed | head | (none) | handshake OK |
| B leaf-badSAN + EXPIRED + renewed | base | ❌ CERT_HAS_EXPIRED + ALTNAME |
ERR_TLS_CERT_ALTNAME_INVALID |
| B leaf-badSAN + EXPIRED + renewed | head | ALTNAME |
ERR_TLS_CERT_ALTNAME_INVALID |
| E leaf-badSAN + renewed + EXPIRED | base | ALTNAME |
ERR_TLS_CERT_ALTNAME_INVALID |
| E leaf-badSAN + renewed + EXPIRED | head | ALTNAME |
ERR_TLS_CERT_ALTNAME_INVALID |
| F leaf-badSAN + FUTURE + renewed | base | ❌ CERT_NOT_YET_VALID + ALTNAME |
ERR_TLS_CERT_ALTNAME_INVALID |
| F leaf-badSAN + FUTURE + renewed | head | ALTNAME |
ERR_TLS_CERT_ALTNAME_INVALID |
| C leaf-ok + EXPIRED only | base | CERT_HAS_EXPIRED |
CERT_HAS_EXPIRED |
| C leaf-ok + EXPIRED only | head | CERT_HAS_EXPIRED |
CERT_HAS_EXPIRED |
| G leaf-ok + EXPIRED CA:TRUE + valid CA:FALSE | base | ❌ CERT_HAS_EXPIRED |
INVALID_PURPOSE |
| G leaf-ok + EXPIRED CA:TRUE + valid CA:FALSE | head | NON_CA_ANCHOR |
INVALID_PURPOSE |
B vs E is the sharpest cut. Identical certificates, identical defect, only the order of the two
root copies in the PEM differs. The base arm invents a CERT_HAS_EXPIRED gap only when the expired
copy sorts first; head returns the same answer either way. That is the bug stated as an observable
property: today's verdict depends on PEM ordering, which no operator controls or thinks about.
C is the regression guard. When the only copy of the root really is expired, both arms still
report it, and the reported code matches what the handshake actually returns. The ?? issuers[0]
fallback does what it claims.
4. Two items review listed as unverified — now measured
-
Control arm (triage: “Not verified: the control-arm claim … code cannot be executed in CI triage”).
Restoring the greedy walk in the head tree and re-running the file:× describeWorkerTlsTrustGaps > prefers a usable issuer over an expired same-subject twin AssertionError: expected [ Array(1) ] to deeply equal [] Test Files 1 failed (1) Tests 1 failed | 327 passed (328)Exactly one failure, exactly the new test. The claim in the PR body is accurate.
-
macOS leg (the CI table on
32f9e83showsTest (macos-latest, Node 22.x)cancelled, and the
PR marks macOS⚠️ ). Run here on macOS:✓ src/serve/run-qwen-serve.test.ts (328 tests) 21997ms Test Files 1 passed (1) Tests 328 passed (328)macOS can be moved from
⚠️ to ✅. Windows remains unverified by me.
5. Two things the PR under-claims
-
The not-yet-valid direction is fixed too (case F).
certValidAtrequiresvalidFrom <= now, so a
twin whosenotBeforeis in the future is skipped as well. The base arm emits a false
CERT_NOT_YET_VALIDthere; head does not. The PR body only describes the expired direction. -
The edge review called theoretical resolves in this PR's favour (case G). Review noted the
preference could newly surface “a valid-but-incapable twin shadowing an expired-but-capable one
through theincapableIssuerbranch” and set it out of scope. Measured against a real handshake,
the actual failure isINVALID_PURPOSE— head names exactly that and points at the CA:FALSE cause;
base saysCERT_HAS_EXPIREDand prescribes a renewal that would change nothing. The preference is
more accurate there, not less.
6. One honest caveat about blast radius
The predicted gaps are printed only when the daemon's live handshake probe has already failed
(run-qwen-serve.ts: if (trustFailure) { for (const gap of predictedGaps) …). So on a fully healthy
renewed-CA setup nothing is logged on either arm — case A above — and the false alarm cannot appear
on its own. What it does is ride along with a genuine worker-TLS failure and misdirect the fix:
during a real outage the operator is handed a CERT_HAS_EXPIRED line and told to renew a CA they
just renewed, next to the line that actually explains the outage. That is arguably worse than crying
wolf on a green boot, but it does mean the framing “a loud red on a healthy configuration” is a
slight overstatement of what an operator can hit today. It does not change the merge decision.
Also unchanged and correctly scoped: the walk still models OpenSSL rather than executing a handshake,
so a bundle OpenSSL resolves differently for reasons other than validity is out of this PR's reach —
the PR says so.
Evidence — raw daemon logs, per-run handshake results, both vitest logs, the certificate fixtures
and every script needed to reproduce this are on
wenshao/qwen-code@assets/pr10042-verify.
中文说明
维护者验证 —— 真实 qwen serve daemon，本地构建，macOS
我从源码构建了 3f956bc，并在真实 daemon（TLS 服务 + 真实的 daemon 托管 channel worker）上端到端地验证了这个改动，而不是只跑单元测试。结果发在这里作为合并参考。
结论：缺陷真实存在、可在测试之外复现，修复正确，且没有丢失任何真阳性。建议合并。 评审留下的两个未验证项 —— 对照组结论与 macOS 腿 —— 现已被实测关闭；另一个被评审称为「理论边界」的情形，实测结果反而站在本 PR 这一边。
环境与两条腿的构建方式
- macOS 26.6.2（Darwin 25.6.0），node
v24.18.1（内置 OpenSSL 3.5.7），系统openssl3.6.3。 - PR head
3f956bc（包含修复提交32f9e83），npm ci && npm run build && npm run bundle。 - head 腿 —— 该 bundle，未做改动。
- base 腿 —— 同一棵树，只还原一处 hunk：把
walkWorkerAnchorPath里选签发者的位置恢复成贪心的chain.find(...)（git diff --stat= 1 file, +1 −18），重新 bundle。
字节级核对过：certValidAt在 head bundle 中恰好出现在一个 chunk，在 base bundle 中为零；编译后的选择点分别是issuers.find(c => certValidAt(c, Date.now())) ?? issuers[0]与chain.find(...)。
夹具是用 OpenSSL 造的真实 CA 续期场景：一把根密钥、两张自签根证书，共享该密钥和 subject CN=qwen renewed test root CA，一张已过期（2024-01-01 → 2024-06-01），一张当前有效（2026-01-01 → 2036-01-01），外加一张由同一把密钥签发的叶子证书 —— 因此两张根都能验证叶子，也都能通过链上行「是不是它签的」这一判定。过期的那份写在服务 PEM 的最前面。
1. 前提成立 —— 合并后的 bundle 确实是经由续期那份完成握手的
先立地基真值，再看诊断说了什么：
$ openssl verify -CAfile bundleA-renewed-ok.pem leaf-good.pem # 过期那份在前
leaf-good.pem: OK
$ openssl verify -CAfile root-expired.pem leaf-good.pem # 只有过期那份
error 10 at 1 depth lookup: certificate has expired
再用 daemon 自己的 WORKER_TLS_TRUST_PROBE（逐字照抄、单独运行）打活着的 daemon，NODE_EXTRA_CA_CERTS 设为 resolveWorkerCaCertPath 交给 worker 的那份 bundle：
{"ok":true,"authorized":true, ...}真实的 channel worker 也连上了（[Channel] "tlsfixture" connected.）。所以在这个配置下每一次握手都成功 —— base 腿在这种 bundle 上可能打印的那句（"every worker handshake to the daemon will fail CERT_HAS_EXPIRED. Renew that chain member"）在事实层面是错的，而它开出的处方，正是运维刚刚做过的续期。
2. 真实 daemon 启动日志的端到端 A/B
同一份服务 bundle、同一个 daemon、同一个 channel worker —— 只差那一处 hunk：
base 报两条缺口，head 报一条 —— 而 head 报的那条，正是真实 worker 形态握手实际返回的失败码（ERR_TLS_CERT_ALTNAME_INVALID）所对应的那条。
3. 完整矩阵
| 服务 bundle（顺序即 PEM 中的书写顺序） | 腿 | 启动诊断报告 | 实测 worker 握手 |
|---|---|---|---|
| A leaf-ok + EXPIRED + renewed | base | 无 | 握手成功 |
| A leaf-ok + EXPIRED + renewed | head | 无 | 握手成功 |
| B leaf-badSAN + EXPIRED + renewed | base | ❌ CERT_HAS_EXPIRED + ALTNAME |
ERR_TLS_CERT_ALTNAME_INVALID |
| B leaf-badSAN + EXPIRED + renewed | head | ALTNAME |
ERR_TLS_CERT_ALTNAME_INVALID |
| E leaf-badSAN + renewed + EXPIRED | base | ALTNAME |
ERR_TLS_CERT_ALTNAME_INVALID |
| E leaf-badSAN + renewed + EXPIRED | head | ALTNAME |
ERR_TLS_CERT_ALTNAME_INVALID |
| F leaf-badSAN + FUTURE + renewed | base | ❌ CERT_NOT_YET_VALID + ALTNAME |
ERR_TLS_CERT_ALTNAME_INVALID |
| F leaf-badSAN + FUTURE + renewed | head | ALTNAME |
ERR_TLS_CERT_ALTNAME_INVALID |
| C leaf-ok + 只有 EXPIRED | base | CERT_HAS_EXPIRED |
CERT_HAS_EXPIRED |
| C leaf-ok + 只有 EXPIRED | head | CERT_HAS_EXPIRED |
CERT_HAS_EXPIRED |
| G leaf-ok + 过期的 CA:TRUE + 有效的 CA:FALSE | base | ❌ CERT_HAS_EXPIRED |
INVALID_PURPOSE |
| G leaf-ok + 过期的 CA:TRUE + 有效的 CA:FALSE | head | NON_CA_ANCHOR |
INVALID_PURPOSE |
B 与 E 的对比是最锋利的一刀。 完全相同的证书、完全相同的缺陷，只有两张根证书在 PEM 里的先后顺序不同。base 腿只在过期那份排前面时才凭空多报一条 CERT_HAS_EXPIRED；head 两种顺序给出同样的答案。这就是把这个 bug 表述成一条可观测性质：今天的结论取决于 PEM 顺序，而这既不是运维能控制的，也不是他们会想到的。
C 是回归护栏。 当根证书只有过期的那一份时，两条腿都仍然报出来，且报出的码与握手实际返回的一致。?? issuers[0] 的回退确实做到了它声称的事。
4. 评审列为「未验证」的两项 —— 现已实测
-
对照组（triage 原话："Not verified: the control-arm claim … code cannot be executed in CI triage"）。在 head 树上还原贪心链上行并重跑该文件：
× describeWorkerTlsTrustGaps > prefers a usable issuer over an expired same-subject twin AssertionError: expected [ Array(1) ] to deeply equal [] Test Files 1 failed (1) Tests 1 failed | 327 passed (328)恰好一个失败，恰好是新增的那条测试。PR 正文的说法属实。
-
macOS 腿（
32f9e83的 CI 表格里Test (macos-latest, Node 22.x)是 cancelled，PR 中 macOS 标为⚠️ ）。本机 macOS 实跑：✓ src/serve/run-qwen-serve.test.ts (328 tests) 21997ms Test Files 1 passed (1) Tests 328 passed (328)macOS 可以从
⚠️ 改为 ✅。Windows 我这边没有验证。
5. PR 少说了的两件事
-
尚未生效（not-yet-valid）的方向也一并修好了（用例 F）。
certValidAt要求validFrom <= now，因此notBefore在未来的孪生根同样会被跳过。base 腿在这里会误报CERT_NOT_YET_VALID，head 不会。PR 正文只描述了过期这一个方向。 -
评审称为「理论边界」的那一项，实测站在本 PR 这一边（用例 G）。评审指出这个偏好可能新引入 "有效但无签发能力的孪生根，经由
incapableIssuer分支遮蔽过期但有能力的那份"，并将其列为范围外。用真实握手去量：实际失败是INVALID_PURPOSE—— head 恰好点名了它，并指出 CA:FALSE 这个成因；base 说的是CERT_HAS_EXPIRED，开出的续期处方什么也改变不了。这个偏好在该边界上更准，而不是更差。
6. 关于影响面的一点如实说明
预测出的缺口只在 daemon 的活体握手探针已经失败时才会打印（run-qwen-serve.ts：if (trustFailure) { for (const gap of predictedGaps) …）。因此在一个完全健康的续期 CA 配置上，两条腿都什么也不打印 —— 也就是上面的用例 A —— 误报无法单独出现。它真正造成的伤害是搭着一次真实的 worker TLS 故障一起出现，并把修复方向带偏：在真实故障期间，运维会在那条真正解释故障的行旁边，看到一条 CERT_HAS_EXPIRED，并被要求去续期一个刚刚续过的 CA。这可以说比在全绿启动时狼来了更糟，但也意味着「对健康配置报了红」这个说法，略微高估了运维今天实际能碰到的情形。这不改变合并结论。
同样未变、且范围界定正确的一点：链上行仍然是在模拟 OpenSSL 而非真的去握一次手，因此若某个 bundle 被 OpenSSL 以有效期之外的其他理由解析成别的路径，就不在本 PR 的射程之内 —— PR 自己也是这么写的。
证据 —— 原始 daemon 日志、每次运行的握手结果、两份 vitest 日志、证书夹具，以及复现所需的全部脚本，都在
wenshao/qwen-code@assets/pr10042-verify。
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Autofix review round summary — PR #10042Feedback points and decisions
Tests added (witnesses for the invariant)
Note: an earlier call-count-based draft of test 1 proved unreliable because Mutation probes (each new guard/branch has a witness)
Conflict notesNone ( Verification
中文说明Autofix 审查轮次总结 — PR #10042反馈点与决定
新增测试（不变量的见证）
说明：测试 1 早期基于调用次数的草稿不可靠，因为 变异探针（每个新关卡/分支都有见证）
冲突说明无（ 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 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.
Reviewed — no blockers. Suggestions are inline.
Test Plan (not a blocker): src/serve/run-qwen-serve.test.ts — no such file or directory; Tests 328 passed — this review observed 24767 passed.
中文说明
已审查——无阻断问题。 建议见行内评论。
Test Plan（非阻断）：src/serve/run-qwen-serve.test.ts — no such file or directory; Tests 328 passed — this review observed 24767 passed。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| } | ||
| }); | ||
|
|
||
| it('agrees on the exact instant the short-lived twin expires', () => { |
There was a problem hiding this comment.
[Suggestion] The expiry-boundary test does not pin the validTo edge of certValidAt: changing >= to > in certValidAt (run-qwen-serve.ts:1343) leaves all 51 describeWorkerTlsTrustGaps tests green, because at exactly notAfter the short-lived twin is judged invalid one instant early, but the walk simply re-anchors through the long-lived twin and the report is still []. The comment above ("a one-sided edit to either predicate fails instead of passing silently") therefore overclaims — only the notBefore edge is pinned, because both twins share notBefore and the issuers[0] fallback keeps the invalid twin in the path there. The blast radius is one millisecond, but a future edit flipping the validTo boundary would ship green, and the diagnostic would call certificates expired one instant early (OpenSSL accepts them through their notAfter instant) — the same false CERT_HAS_EXPIRED class this PR fixes. Verified by mutation probe: baseline 51 passed; validTo >=→> mutant 51 passed (survives); control validFrom <=→< mutant 1 failed (the comparator is live). A discriminating witness: add a boundary case where the expiring certificate is the walk's only issuer — a two-cert bundle (leaf + short-lived twin only, no long-lived twin to re-anchor through) with the clock at exactly 2026-08-22T12:12:20Z asserting []; that case passes on correct code and fails under the mutation while all 51 originals stay green. Alternatively, narrow the comment's claim to the notBefore edge.
it('still anchors at the exact notAfter when the short twin is the only issuer', () => {
vi.useFakeTimers();
vi.setSystemTime(new Date('2026-08-22T12:12:20Z'));
try {
// leaf + short-lived twin only: no long-lived twin to hide a strict `>` behind
expect(
describeWorkerTlsTrustGaps({
cert: Buffer.from(TEST_TLS_CERT_LEAF_PLUS_SHORT_ROOT_ONLY),
certPath: '/certs/fullchain.pem',
daemonUrl,
}),
).toEqual([]);
} finally {
vi.useRealTimers();
}
});中文说明
过期边界测试没有钉住 certValidAt 的 validTo 边界：把 certValidAt（run-qwen-serve.ts:1343）里的 >= 改成 >，全部 51 个 describeWorkerTlsTrustGaps 测试依然全绿——因为在恰好等于 notAfter 的瞬间，短有效期的孪生证书被提前一刻判定为无效，但链上行会直接改锚到长有效期的孪生证书，报告仍是 []。所以上方的注释（“对任一谓词的单边修改都会失败而不是悄无声息地通过”）说过头了——实际只有 notBefore 边界被钉住：两张孪生证书共享 notBefore，issuers[0] 回退会把无效的孪生证书留在路径上。影响范围只有一毫秒，但未来若有编辑翻转 validTo 边界，会一路绿灯地合入，诊断会把证书提前一刻判为过期（OpenSSL 接受到 notAfter 当刻为止）——正是本 PR 要消除的 CERT_HAS_EXPIRED 误报类别。已通过变异探针验证：基线 51 通过；validTo >=→> 变异体 51 通过（存活）；对照 validFrom <=→< 变异体 1 失败（比较器是活的）。可判定的见证：新增一个“过期证书是链上行唯一签发者”的边界用例——双证书 bundle（仅叶子 + 短有效期孪生证书，没有可供重新锚定的长有效期孪生证书），时钟恰好拨到 2026-08-22T12:12:20Z，断言 []；该用例在正确代码下通过，在上述变异下失败，而原有 51 个测试仍全绿。或者，把注释的表述收窄到仅 notBefore 边界。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| .mockImplementation(() => | ||
| new Error().stack?.includes('walkWorkerAnchorPath') === true |
There was a problem hiding this comment.
[Suggestion] The one-clock straddle test routes the two clocks by matching a stack trace against the literal name of the module-private function walkWorkerAnchorPath. If that function is renamed or extracted (a refactor no string literal follows), the stack check matches nothing, the mock returns pastWindow everywhere, and the test still passes under the exact two-clocks regression it exists to catch — traced: walk@pastWindow prefers the renewed long root, report@pastWindow finds it valid, result []. Verified by A/B probe: arm A (two-clock regression, no rename) — this test FAILS, the guard works today; arm B (same regression plus rename walkWorkerAnchorPath → walkWorkerChainPath) — this test PASSES, the guard silently no-ops. Discriminate by call order instead of by name: the fixed code calls Date.now exactly once (the caller's sample), so poisoning any second sample removes the name coupling. One measured caveat: a naive first-call counter does not work as-is — an unrelated Date.now consumer (the scanCertificateBlocks temp-file write, reached because this test omits certSourcePath while the production call site passes it) consumes the first-call slot; a working variant must suppress or account for that stray sample first (e.g. give the test the same certSourcePath production passes, then count), or keep frame-based routing anchored to something more stable than a module-private name.
// sketch: poison the SECOND sample instead of routing by stack name
let calls = 0;
const clock = vi.spyOn(Date, 'now').mockImplementation(() => {
calls += 1;
// first call may be an unrelated consumer (see caveat above) — anchor the
// counting to the caller's sample, e.g. after passing certSourcePath so
// scanCertificateBlocks reuses the source file and stops sampling.
return calls <= ALLOWED_STRAY_CALLS ? pastWindow : insideWindow;
});中文说明
单时钟跨越测试通过在栈轨迹里匹配模块私有函数 walkWorkerAnchorPath 的字面名字来区分两个时钟。如果该函数被重命名或提取（字符串常量不会跟随的重构），栈检查将匹配不到任何东西，mock 会处处返回 pastWindow，而测试在它本要捕捉的双时钟回归下依然通过——已推演：walk@pastWindow 偏好续期的长根，report@pastWindow 判定其有效，结果为 []。已通过 A/B 探针验证：臂 A（双时钟回归、不重命名）——本测试失败，守卫今天有效；臂 B（同样的回归加上重命名 walkWorkerAnchorPath → walkWorkerChainPath）——本测试通过，守卫悄无声息地失效。建议按调用次序而非名字来区分：修复后的代码只调用一次 Date.now（调用方的采样），因此对任何第二次采样投毒即可消除对名字的耦合。一个实测注意事项：朴素的首次调用计数器目前并不能直接生效——有一个无关的 Date.now 使用方（scanCertificateBlocks 的临时文件写入；由于本测试省略了 certSourcePath 而生产调用点会传，才会走到）会占用第一次调用的位置；可用的变体必须先抑制或计入这次杂散采样（例如让测试也传入和生产一致的 certSourcePath，再去计数），或者把基于栈帧的路由锚定在比模块私有名字更稳定的东西上。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Autofix review round — PR #10042 (round 2)Commit: Feedback dispositions[rc:3862529185] The expiry-boundary test does not pin the [rc:3862529196] The one-clock straddle test routes clocks by matching a stack trace against the module-private name — ADDRESSED. [rv:5030145350] Review body — no blockers; noted. The Test Plan note is informational: [ic:5423521678] Maintainer verification (real daemon A/B, macOS) — no action needed. Thank you for the end-to-end measurement; it closes the control-arm and macOS items. The not-yet-valid direction and case G it surfaces are already covered by the existing Mutation probes (each new guard witnessed)
All mutants were reverted; the committed tree contains test changes only. Verification
中文说明Autofix 审查轮次 — PR #10042（第 2 轮）提交： 反馈处置[rc:3862529185] 过期边界测试没有钉住 [rc:3862529196] 单时钟跨越测试靠栈轨迹匹配模块私有名字来区分两个时钟 —— 已处理。 [rv:5030145350] 审查正文 —— 无阻断；已知悉。 Test Plan 备注属信息性： [ic:5423521678] 维护者验证（真实 daemon A/B，macOS）—— 无需改动。 感谢这份端到端实测，它关闭了对照组与 macOS 两个未验证项。它顺带点出的尚未生效（not-yet-valid）方向与用例 G，分别已被现有的 变异探针（每个新守卫都有自己的见证）
所有变异体均已还原；提交的树只包含测试改动。 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 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.
Reviewed — no blockers. Suggestions are inline.
Test Plan (not a blocker): src/serve/run-qwen-serve.test.ts — no such file or directory; Tests 328 passed — this review observed 24768 passed.
Convergence: round 3 posted 1 inline comment(s), 1 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/serve/run-qwen-serve.test.ts (findings in round 2; 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. No Critical finding is open on this round, so merging and moving the remaining Suggestion threads to a follow-up issue is available as an ending — a merged pull request cannot diverge further. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
已审查——无阻断问题。 建议见行内评论。
Test Plan（非阻断）：src/serve/run-qwen-serve.test.ts — no such file or directory; Tests 328 passed — this review observed 24768 passed。
收敛情况：第 3 轮发布了 1 条行内评论，其中 1 条是首次提出；上一轮发布了 2 条（其中 2 条首次提出）。发现反复回到同一批文件：packages/cli/src/serve/run-qwen-serve.test.ts（第 2 轮已出过发现，本轮又有 1 条）。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。本轮没有未决的 Critical，因此"合入后把剩余 Suggestion 线程转到后续 issue"是一个可选的结束方式——已合入的 PR 不会继续发散。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.0)
| /** The renewed-root bundle minus its long-lived twin. */ | ||
| const leafPlusShortTwinOnlyPem = (): string => { | ||
| const blocks = TEST_TLS_CERT_FULLCHAIN_RENEWED_ROOT.match( |
There was a problem hiding this comment.
[Suggestion] leafPlusShortTwinOnlyPem() selects the short-lived twin by positional index only (blocks![1]), and nothing verifies that index 1 actually holds the short-lived twin — the required order is documented only in the fixture's comment. If a future edit or regeneration of TEST_TLS_CERT_FULLCHAIN_RENEWED_ROOT places the long-lived twin at index 1 (appending the renewed root last is the common convention), the helper hands the boundary test the long-lived twin as "the only issuer": at the frozen clock 2026-08-22T12:12:20Z that twin is comfortably valid, so still anchors at the exact notAfter when the short twin is the only issuer keeps passing while no longer failing under a strict > edit to certValidAt — the exact regression this PR exists to catch is silently un-pinned. Measured at this commit: reordering the fixture alone leaves all 52 describeWorkerTlsTrustGaps tests green, and combining that reorder with the strict-> mutant makes the witness pass while pinning nothing.
BASELINE (original order): witness passes
M1 strict `>` in certValidAt: witness FAILS — expected [ Array(1) ] to deeply equal []
M2 fixture reordered alone: 52 passed — the reorder is silent
M3 M1 + M2: witness PASSES — the reorder silently un-pins it
Select the twin by property instead of by index so a reordered fixture fails loudly, e.g.:
const leafPlusShortTwinOnlyPem = (): string => {
const blocks = TEST_TLS_CERT_FULLCHAIN_RENEWED_ROOT.match(
/-----BEGIN CERTIFICATE-----[^-]*-----END CERTIFICATE-----/g,
);
// Fail loudly if the fixture order changes: index 1 must be the
// short-lived twin the boundary clock sits on.
const shortTwin = new X509Certificate(blocks![1]);
if (new Date(shortTwin.validTo).getTime() !== Date.UTC(2026, 7, 22, 12, 12, 20)) {
throw new Error('renewed-root fixture order changed');
}
return `${blocks![0]}\n${blocks![1]}\n`;
};(X509Certificate from node:crypto.)
中文说明
leafPlusShortTwinOnlyPem() 仅按位置索引（blocks![1]）选取短有效期的孪生证书，没有任何检查确认索引 1 处确实是短有效期的那份——所需的顺序只写在 fixture 上方的注释里。如果未来对 TEST_TLS_CERT_FULLCHAIN_RENEWED_ROOT 的编辑或重新生成把长有效期的孪生证书放到索引 1（“续期后的根追加在最后”是常见的追加习惯），这个助手就会把长有效期的孪生证书当作“唯一签发者”交给边界测试：在冻结的时钟 2026-08-22T12:12:20Z 下那份证书远未过期，于是 still anchors at the exact notAfter when the short twin is the only issuer 依然通过，却无法在 certValidAt 被改成严格 > 时失败——本 PR 要抓的回归就这样悄无声息地失去钉子。在本提交上实测：仅重排 fixture 时全部 52 个 describeWorkerTlsTrustGaps 测试保持全绿；把该重排与严格 > 变异组合后，见证用例通过、但什么都没钉住。
建议改为按属性而非索引选取孪生证书：用 node:crypto 的 X509Certificate 校验 blocks![1] 的 validTo 恰为 2026-08-22T12:12:20Z（即 Date.UTC(2026, 7, 22, 12, 12, 20)），否则抛错，使 fixture 顺序变化时大声失败，而不是静默地解除边界测试的武装。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/100 轮）。改动内容与我反驳保留之处如下： Address-review summary — PR #10042 (round 4)Feedback points and decisions[rc:3863871370]
|
| Probe | Mutation | Result |
|---|---|---|
| Baseline | none | 52 passed |
| M1 | strict > in certValidAt (validTo side) |
witness FAILS — the pin is live |
| M2 | fixture blocks 1/2 swapped alone | 52 passed — the reorder is silent |
| M3 | M1 + M2 combined | 52 passed — the reorder silently un-pins the witness |
Fix (9 lines): the helper now asserts the selected block is the short-lived twin by property — its validTo must equal the boundary instant 2026-08-22T12:12:20Z — and throws renewed-root fixture order changed otherwise, so any reorder or regeneration of the fixture fails loudly instead of disarming the test. X509Certificate was already imported; the instant literal matches the one the boundary tests freeze the clock on.
Mutation-probing the new guard itself:
| Probe | State | Result |
|---|---|---|
| A | fixture reordered, guard present | boundary test FAILS with renewed-root fixture order changed |
| B | fixture reordered, guard removed | 52 passed — the guard is the sole witness of the reorder |
| M1 (post-fix) | strict > mutant, original fixture |
witness FAILS — the boundary pin remains live through the change |
All mutations were fully restored afterwards; the final diff is exactly the 9-line guard.
[rv:5031779257] Review body — no action needed
The review is COMMENTED with no blockers; its single actionable finding is the inline suggestion above. The Test Plan note (src/serve/run-qwen-serve.test.ts — no such file or directory, observed 24768 passed) describes the reviewer's own run invoking a relative path from the wrong directory and falling back to the full suite; the file exists at packages/cli/src/serve/run-qwen-serve.test.ts and passes (see Verification). The convergence observation is noted but is a maintainer call, not a code change.
Changes
packages/cli/src/serve/run-qwen-serve.test.ts:leafPlusShortTwinOnlyPem()now verifiesblocks[1].validTo === 2026-08-22T12:12:20ZviaX509Certificateand throws on mismatch (+9 lines). No production code touched.- Commit:
35c8512a3e—test(serve): fail loudly on a reordered renewed-root fixture (#10042).
Conflict notes
--conflict false; no merge performed.
Verification
- Reproduction & mutation probes (focused vitest,
describeWorkerTlsTrustGaps): Baseline 52 passed; M1 witness fails; M2 52 passed (silent hole confirmed); M3 52 passed (un-pin confirmed); Probe A fails loudly with the guard; Probe B green with guard removed; post-fix M1 witness fails — as tabulated above npm run build— passednpm run typecheck— passednpm run lint— passednpx prettier --check packages/cli/src/serve/run-qwen-serve.test.ts— passednpx vitest run src/serve/run-qwen-serve.test.ts(packages/cli, touched) — 332 passed- Integration tests after
npm run bundle: not applicable — the change is test-only and exercisesdescribeWorkerTlsTrustGapsthrough its direct unit import, not the bundled CLI - Settings schema: not touched, no regeneration needed
中文说明
处理评审总结 — PR #10042（第 4 轮）
反馈点与决定
[rc:3863871370] leafPlusShortTwinOnlyPem() 仅按位置索引选取短有效期孪生证书 — 已实现
该发现指出：边界见证用例 still anchors at the exact notAfter when the short twin is the only issuer 依赖 TEST_TLS_CERT_FULLCHAIN_RENEWED_ROOT 在块索引 1 处放置短有效期的孪生证书，而这一约定只写在 fixture 上方的注释里。若 fixture 被重排，助手会把长有效期的孪生证书当作"唯一签发者"交给路径搜索；在冻结的边界时钟下它远未过期，测试依旧通过，而把 certValidAt 改成严格 > 却不再失败——本 PR 要抓的回归就这样被悄无声息地拔掉了钉子。
改动前先在改动前的提交上复现，通过变异探针：
| 探针 | 变异 | 结果 |
|---|---|---|
| 基线 | 无 | 52 通过 |
| M1 | certValidAt（validTo 侧）改为严格 > |
见证用例失败——钉子有效 |
| M2 | 仅交换 fixture 块 1/2 | 52 通过——重排是静默的 |
| M3 | M1 + M2 组合 | 52 通过——重排静默地拔掉钉子 |
修复（9 行）： 助手现在按属性断言所选块就是短有效期的孪生证书——其 validTo 必须等于边界时刻 2026-08-22T12:12:20Z——否则抛出 renewed-root fixture order changed，使 fixture 的任何重排或重新生成都大声失败，而不是悄悄解除测试的武装。X509Certificate 已在文件中导入；时刻字面量与边界测试冻结时钟所用的完全一致。
对新守卫本身的变异探针：
| 探针 | 状态 | 结果 |
|---|---|---|
| A | fixture 已重排、守卫在位 | 边界测试失败，报 renewed-root fixture order changed |
| B | fixture 已重排、守卫移除 | 52 通过——守卫是重排的唯一见证 |
| M1（修复后） | 严格 > 变异、原序 fixture |
见证用例失败——改动后边界钉子依然有效 |
所有变异事后均已完全还原；最终 diff 恰为这 9 行守卫。
[rv:5031779257] 评审正文 — 无需处理
该评审为 COMMENTED、无阻断；其唯一可执行的发现即上述行内建议。Test Plan 备注（src/serve/run-qwen-serve.test.ts — no such file or directory、观察到 24768 passed）描述的是评审方自己在错误目录下以相对路径调用、回退到全量套件的运行痕迹；该文件位于 packages/cli/src/serve/run-qwen-serve.test.ts 且测试通过（见验证）。关于收敛的观察已记录，但那是维护者的决定，不是代码改动。
变更
packages/cli/src/serve/run-qwen-serve.test.ts：leafPlusShortTwinOnlyPem()现在通过X509Certificate校验blocks[1].validTo === 2026-08-22T12:12:20Z，不匹配即抛错（+9 行）。未触碰生产代码。- 提交：
35c8512a3e—test(serve): fail loudly on a reordered renewed-root fixture (#10042)。
冲突说明
--conflict false；未执行合并。
验证
- 复现与变异探针（聚焦 vitest，
describeWorkerTlsTrustGaps）：基线 52 通过；M1 见证失败；M2 52 通过（确认静默漏洞）；M3 52 通过（确认静默拔钉）；探针 A 在守卫在位时大声失败；探针 B 移除守卫后回绿；修复后 M1 见证失败——如上表 npm run build— 通过npm run typecheck— 通过npm run lint— 通过npx prettier --check packages/cli/src/serve/run-qwen-serve.test.ts— 通过npx vitest run src/serve/run-qwen-serve.test.ts（packages/cli，被触碰包）— 332 通过npm run bundle后的集成测试：不适用——本次为纯测试改动，且通过直接单元导入调用describeWorkerTlsTrustGaps，不经过打包后的 CLI- 设置 schema：未触碰，无需重新生成
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。
Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。
🧠 Handled by Qwen Code · model/模型 qwen3.8-max
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix round summary — PR #10042 (no action taken)Feedback triaged
No maintainer or automated-reviewer feedback in this window requests a code change. Diff growth is within budget (source 11 / test 114 lines vs 400/400) and no growth audit was required. ChecksThe "Failed checks" list contains six checks from one
The OutcomeNo code changes were made this round; nothing was committed. 中文说明Autofix 轮次总结 — PR #10042（未采取任何行动）已分类的反馈
本时间窗口内没有任何来自维护者或自动评审器的反馈要求进行代码修改。Diff 增长在预算之内（源码 11 行 / 测试 114 行，预算为 400/400），也不需要进行增长审计（growth audit）。 检查（Checks）“Failed checks” 列表中包含来自同一次
结果本轮未做任何代码修改；未提交任何内容。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /review |
|
Qwen Code review request accepted. Review is queued in workflow run. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: reverse audit — stopped before round 4 by the review time budget.
Test Plan (not a blocker): src/serve/run-qwen-serve.test.ts — no such file or directory.
Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:
packages/cli/src/serve/run-qwen-serve.test.ts:2535 — [probe] one-clock witness poisons only Date.now; new Date() reads escape, so a split-clock regression can pass silently
中文说明
未审查：反向审计——评审时间预算不足，未能开始第 4 轮。
Test Plan（非阻断）：src/serve/run-qwen-serve.test.ts — no such file or directory。
收敛姿态下延后（第 4 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.2)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： 🤖 No code changes this round — the round-4 review is COMMENTED with zero findings, and its sole downgrade reason does not reproduce on evidence. · 本轮无代码改动——第 4 轮评审为 COMMENTED、零发现，其唯一降级理由经证据核查不成立。 Address-review round — PR #10042 (no action)Review [rv:5047848471] is Feedback points and decisions[rv:5047848471] "CI failing: …" (six SDK Java checks) — Declined with evidence
[rv:5047848471] Test Plan note (
|
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 explored to full depth (tool budget reached): "agent 6a": run the PR's new/changed vitest tests — npm install in the worktree timed out after 10 minutes, so I verified the fixture and walk logic with a standalone no….
Test Plan (not a blocker): src/serve/run-qwen-serve.test.ts — no such file or directory.
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）："agent 6a"：run the PR's new/changed vitest tests — npm install in the worktree timed out after 10 minutes, so I verified the fixture and walk logic with a standalone no…。
Test Plan（非阻断）：src/serve/run-qwen-serve.test.ts — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| it('prefers a usable issuer over an expired same-subject twin', () => { | ||
| vi.useFakeTimers(); | ||
| vi.setSystemTime(new Date('2030-01-01T00:00:00Z')); |
There was a problem hiding this comment.
[Suggestion] R5-1: The walk's issuer preference is only tested against an expired twin; its not-yet-valid clause has no witness — a one-line mutation to expiry-only preference survives the whole suite. Both fixture twins share notBefore (2026-08-19T12:12:20Z), so no clock in the current tests ever puts one twin before its window while the other is inside it: mutating the predicate at run-qwen-serve.ts:1685 to skip only expired issuers (validTo >= now) leaves all five new tests and the two pre-existing validity tests green. A discriminating input exists and is a real renewal shape — a pre-issued replacement CA whose notBefore lies in the future, ordered before its already-valid twin in the bundle. If such a regression ever shipped, the walk would anchor through the not-yet-valid copy and report the false CERT_NOT_YET_VALID alarm this PR exists to remove, telling the operator to fix the notBefore of a CA the merged bundle already authorizes through the valid twin.
Witness:
[probe] mutant arm — issuers.find((c) => new Date(c.validTo).getTime() >= now) ?? issuers[0]:
describeWorkerTlsTrustGaps suite: Tests 52 passed (mutant survives the entire suite)
Discriminating probe (first twin not-yet-valid, notBefore 2030-01-01; second twin valid since 2020;
same subject AND key; clock 2027-01-01, expecting []):
under mutant: FAIL — …chains through "CN=qwen future twin test CA", which is not yet valid … CERT_NOT_YET_VALID…
under shipped: gaps = [], Tests 1 passed (1) — the probe flips
Suggested fix: add one test where the first same-subject/same-key twin is not yet valid and a later twin is already valid (a new fixture constant), the clock sits inside the valid twin's window, and describeWorkerTlsTrustGaps returns [] — mirroring the 2030 test's shape in the not-yet-valid direction.
The new fixture's twins must still pass certIssuedBy (same subject AND same key as the leaf's issuer), and the fixture must be a separate constant: leafPlusShortTwinOnlyPem() throws if index 1 of TEST_TLS_CERT_FULLCHAIN_RENEWED_ROOT is not the 2026-08-22T12:12:20Z short twin (run-qwen-serve.test.ts). If you add the test, prove it is load-bearing: apply the expiry-only mutant above and confirm the new test is the one that goes red.
中文说明
链上行的签发者偏好只针对过期的孪生证书做过测试；其“尚未生效”分支没有见证——一行变异成“仅排除过期”的偏好就能在整个测试套件中存活。当前夹具里的两个孪生证书共享 notBefore（2026-08-19T12:12:20Z），所以现有测试里没有任何时钟能让一个孪生证书在窗口之前而另一个在窗口之内：把 run-qwen-serve.ts:1685 的谓词变异为只跳过过期证书（validTo >= now），5 个新测试和 2 个既有有效期测试全部保持全绿。存在可判别的输入，且是真实的续期形态——一张预签发的替换 CA，其 notBefore 在未来，且排在已经生效的孪生证书之前。若这种回归上线，链上行会锚定到尚未生效的那份，并报出本 PR 正要消除的 CERT_NOT_YET_VALID 误报，让运维去修一张合并 bundle 已经通过有效那份完成授权的 CA 的 notBefore。
见证：变异臂（仅排除过期）下 describeWorkerTlsTrustGaps 套件 52 个测试全部通过（变异体存活）；可判别探针（第一个孪生证书 notBefore 2030-01-01 尚未生效、第二个自 2020 起有效、同 subject 且同密钥、时钟 2027-01-01、期望 []）在变异下失败并报出 CERT_NOT_YET_VALID 缺口，在现有实现下 gaps = [] 通过——探针可翻转。
建议修复：新增一个测试——第一个同 subject/同密钥的孪生证书尚未生效、后面的孪生证书已生效（新夹具常量），时钟拨到有效孪生证书的窗口内，断言 describeWorkerTlsTrustGaps 返回 []——即 2030 用例在“尚未生效”方向上的镜像。
新夹具的孪生证书必须仍能通过 certIssuedBy（与叶子证书的签发者同 subject 且同密钥），且必须是独立的常量：leafPlusShortTwinOnlyPem() 会在 TEST_TLS_CERT_FULLCHAIN_RENEWED_ROOT 的下标 1 不是 2026-08-22T12:12:20Z 短有效期孪生证书时抛错（run-qwen-serve.test.ts）。加上测试后请证明它是承重的：施加上面的“仅排除过期”变异，确认正是新测试变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| /** Whether `cert`'s validity window contains `now`. */ | ||
| function certValidAt(cert: X509Certificate, now: number): boolean { |
There was a problem hiding this comment.
[Suggestion] R5-2: The new certValidAt helper is an exact generalisation of the inline validity predicate still written out in runQwenServeImpl's boot validation (run-qwen-serve.ts:3533-3549) — !certValidAt(x509, now) is logically identical to the boot path's validTo < now / validFrom > now pair, boundary inclusivity included. The diff applied the helper to the member-validity loop and the walk but left this second implementation inline, so the same file now carries two definitions of "cert is within its validity window at instant t". This PR's own new tests pin the exact notAfter/notBefore instants as load-bearing, so the predicate is boundary-sensitive: a future edit that adjusts the window in only one implementation — flipping < to <= at boot, or changing how the instant is sampled — makes boot validation and describeWorkerTlsTrustGaps disagree about the same certificate at the same instant (boot accepts a leaf the diagnostic then reports as expired, or vice versa), with nothing at either site exposing the inconsistency.
Witness:
[probe] candidate consolidation applied in a scratch tree — runQwenServe TLS boot suite:
Tests 8 passed | 354 skipped (362)
(includes `rejects an expired certificate at boot`, rejects.toThrow(/expired on/))
Suggested fix — wrap the boot checks in the helper, the same shape the diff applied to the member loop:
if (!certValidAt(x509, now)) {
if (new Date(x509.validTo).getTime() < now) {
throw new Error(/* existing expired message */);
} else {
throw new Error(/* existing not-yet-valid message */);
}
}The boot path throws with two distinct messages and the expired check taking precedence (run-qwen-serve.ts:3533-3549); the consolidation must preserve both message texts and that precedence.
中文说明
新增的 certValidAt 助手是 runQwenServeImpl 启动校验（run-qwen-serve.ts:3533-3549）里仍然内联的有效期谓词的完全泛化——!certValidAt(x509, now) 与启动路径的 validTo < now / validFrom > now 组合在逻辑上完全一致，包括边界包含性。本 diff 把助手用到了成员有效期循环和链上行，却把这第二份实现留在了原处，于是同一个文件现在有两个“证书在时刻 t 处于有效期内”的定义。本 PR 新增的测试已把恰好 notAfter/notBefore 时刻钉为承重边界，说明该谓词对边界敏感：未来任何只调整其中一份实现的修改——例如把启动处的 < 改成 <=，或改变时刻的采样方式——都会让启动校验与 describeWorkerTlsTrustGaps 对同一张证书在同一时刻给出不同结论（启动放行而诊断报过期，或反之），且两处都不会暴露这一不一致。
见证：在临时树中应用候选合并后，runQwenServe TLS 启动套件 Tests 8 passed | 354 skipped (362)（包含 rejects an expired certificate at boot），合并在所有钉住的点上行为保持。
建议修复——把启动校验包进助手，形状与本 diff 在成员循环上应用的一致（代码见上）。
启动路径抛出两个不同的错误消息且过期检查优先（run-qwen-serve.ts:3533-3549）；合并时必须保留两个消息文本和该优先级。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| const issuer: X509Certificate | undefined = | ||
| issuers.find((candidate) => certValidAt(candidate, now)) ?? issuers[0]; |
There was a problem hiding this comment.
[Suggestion] R5-3: The validity-first issuer preference is greedy with no backtracking: a currently-valid issuer that cannot anchor is preferred over an expired twin that does anchor, so for that bundle shape the report names the wrong cause and the wrong remedy. A renewed CA whose replacement keeps subject and key but is cross-signed by a new root not present in the bundle — store [leaf, E, V] with E the expired self-signed twin and V the valid twin whose issuer R is absent — makes the walk pick V, find no issuer for it, and end unanchored: the operator gets "nothing in the channel workers' bundle anchors their trust … UNABLE_TO_VERIFY_LEAF_SIGNATURE … Point NODE_EXTRA_CA_CERTS at the issuing CA" even though the issuing CA is in the bundle and the real fix is renewing the expired anchoring twin E — the wrong-remedy class this PR exists to remove. Pre-diff, chain.find took E first, anchored, and the per-member loop named the expiry correctly. The handshake fails either way and the operator is still warned (the boot path runs a live handshake probe after the static prediction), so this is a misdiagnosis, not a silent outage — the trigger shape is unusual, which is why this is a Suggestion.
Witness:
[probe] A/B, openssl-constructed [leaf, expired self-signed twin, valid cross-signed twin whose issuer is absent], clock 2026-08-29
PR arm: …is issued by another CA (CN=qwen v3 renewed CA), not self-signed …
fail UNABLE_TO_VERIFY_LEAF_SIGNATURE … Point NODE_EXTRA_CA_CERTS at the issuing CA …
(misdiagnosis — the issuing CA IS in the bundle)
BASE arm: …chains through "CN=qwen v3 renewed CA", which expired on Jan 1 00:00:00 2025 GMT …
fail CERT_HAS_EXPIRED. Renew that chain member and restart. (expiry named correctly)
Suggested fix: make the preference two-pass — run the walk with the validity-first preference, and if it does not anchor (or terminates on incapableIssuer/nonCaTerminator), re-run it with plain bundle-order preference and report that walk's diagnosis, so the expired-but-anchoring twin is named when it is the chain OpenSSL would actually have attempted.
A two-pass walk must reuse the already-sampled now rather than resampling — the one-clock invariant is pinned by expect(samples).toBe(1) in 'judges the walk and the report at one sampled instant' (run-qwen-serve.test.ts). If you add the two-pass fallback, prove it is load-bearing: add a test with a [leaf, expired self-signed twin, valid non-self-signed twin whose issuer is absent] bundle asserting the gap names the expired member (CERT_HAS_EXPIRED wording) — removing the fallback must make it fail.
中文说明
有效期优先的签发者偏好是贪心且无回溯的：一个当前有效但无法锚定的签发者会优先于一个已过期但能锚定的孪生证书被选中，于是对这种 bundle 形态，报告会指错原因、给错补救办法。设续期后的 CA 保留了 subject 和密钥，但被一个不在 bundle 中的新根交叉签发——存储 [leaf, E, V]，其中 E 是过期的自签孪生证书、V 是签发者 R 缺席的有效孪生证书——链上行会选中 V，找不到它的签发者，以未锚定结束：运维收到“channel workers 的 bundle 里没有任何东西能锚定信任……UNABLE_TO_VERIFY_LEAF_SIGNATURE……请把 NODE_EXTRA_CA_CERTS 指向签发 CA”，可签发 CA 明明就在 bundle 里，真正的修复是续期那张过期的可锚定孪生证书 E——正是本 PR 要消除的“给错补救”这一类。改动前 chain.find 会先取 E、成功锚定，成员循环也会正确指出过期。两种情况下握手都会失败，且运维仍会收到告警（启动路径在静态预测之后还有真实握手探测），所以这是误诊而非静默故障——触发形态罕见，因此定级为 Suggestion。
见证：用 openssl 构造 [叶子, 过期自签孪生证书, 签发者缺席的有效交叉签名孪生证书]，时钟固定 2026-08-29 做 A/B：PR 臂报 UNABLE_TO_VERIFY_LEAF_SIGNATURE/请设置 NODE_EXTRA_CA_CERTS（误诊——签发 CA 就在 bundle 里）；改动前的 bundle 顺序臂锚定到过期孪生证书，正确报出 CERT_HAS_EXPIRED 并指明续期该成员。
建议修复：把偏好改成两段式——先按有效期优先的偏好走链，若未能锚定（或以 incapableIssuer/nonCaTerminator 终止），再按 bundle 原顺序偏好重走一次，并报告第二次行走的诊断，使得当“过期但能锚定”的孪生证书才是 OpenSSL 实际会尝试的链时，报告能正确点名它。
两段式行走必须复用已采样的 now 而不是重新采样——单时钟不变量由 'judges the walk and the report at one sampled instant' 里的 expect(samples).toBe(1) 钉住（run-qwen-serve.test.ts）。若加上两段式回退，请证明它是承重的：新增一个 [叶子, 过期自签孪生证书, 签发者缺席的有效非自签孪生证书] bundle 的测试，断言缺口点名过期成员（CERT_HAS_EXPIRED 措辞）——移除回退后该测试必须失败。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x), web-shell E2E Smoke (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Test (ubuntu-latest, Node 22.x), web-shell E2E Smoke (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R5-2 duplicate: certValidAt not folded into the boot leaf guard — already reported (comment 3886712751)
- R5-3 duplicate: greedy issuer preference without backtracking — already reported (comment 3886712754)
Not explored to full depth (tool budget reached): "agent 6b": running the five new vitest cases ( npx vitest run src/serve/run-qwen-serve.test.ts in packages/cli ) — the worktree has no node_modules and no built worksp…; "agent 5": running the new describeWorkerTlsTrustGaps tests at this commit — the worktree has no node_modules and npm ci + npm run build was not attempted in the sha…; "agent 6c": could not execute the new vitest tests in this worktree (no node_modules or built dist prerequisites; full npm ci + npm run build exceeds the review budget)…; "agent 1a": could not execute the new tests to confirm they pass — neither the worktree nor the parent checkout has node_modules , so running vitest requires a full npm c….
Test Plan (not a blocker): src/serve/run-qwen-serve.test.ts — no such file or directory.
Deferred under the convergence posture (round 6, not a blocker) — recorded, not requested in this round:
packages/cli/src/serve/run-qwen-serve.test.ts:2931 — [probe] R6-1 one-clock witness over-asserts: any Date.now read fails the testpackages/cli/src/serve/run-qwen-serve.test.ts:2879 — [review] R5-1 issuer preference has no not-yet-valid witness (still stands)packages/cli/src/serve/run-qwen-serve.ts:1462 — [review] R5-2 certValidAt not folded into the boot leaf guard (still stands)packages/cli/src/serve/run-qwen-serve.ts:1685 — [review] R5-3 greedy issuer preference, no backtracking (still stands)
中文说明
已审查。
本轮确认的 2 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未探索到全部深度（达到工具调用预算）："agent 6b"：running the five new vitest cases ( npx vitest run src/serve/run-qwen-serve.test.ts in packages/cli ) — the worktree has no node_modules and no built worksp…；"agent 5"：running the new describeWorkerTlsTrustGaps tests at this commit — the worktree has no node_modules and npm ci + npm run build was not attempted in the sha…；"agent 6c"：could not execute the new vitest tests in this worktree (no node_modules or built dist prerequisites; full npm ci + npm run build exceeds the review budget)…；"agent 1a"：could not execute the new tests to confirm they pass — neither the worktree nor the parent checkout has node_modules , so running vitest requires a full npm c…。
Test Plan（非阻断）：src/serve/run-qwen-serve.test.ts — no such file or directory。
收敛姿态下延后（第 6 轮，非阻断）——已记录，本轮不要求修改：共 4 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
🔀 Base updated: red check(s) [ubuntu-latest / Java 11] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [ubuntu-latest / Java 11] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R5-1 duplicate: issuer preference has no not-yet-valid twin witness — already reported (comment 3886712748)
- R5-2 duplicate: certValidAt not folded into the boot leaf guard — already reported (comment 3886712751)
Test Plan (not a blocker): src/serve/run-qwen-serve.test.ts — no such file or directory.
1 Suggestion(s) were drafted inline past the resolved critical posting floor; the CLI moved them into the deferral list below (floor enforcement).
Deferred under the convergence posture (round 7, not a blocker) — recorded, not requested in this round:
packages/cli/src/serve/run-qwen-serve.ts:1465 — [review] R7-1: certValidAt treats the exact notAfter instant as still valid ( validTo >= now ), but a real worker handshake already fails CERT_HAS_EXPIRED at that second — the diagnostic m…
中文说明
已审查。
本轮确认的 2 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
Test Plan（非阻断）：src/serve/run-qwen-serve.test.ts — no such file or directory。
1 条 Suggestion 在已解析的 critical 发布下限之外被起草为行内评论；CLI 已将其移入下方延后清单（下限强制执行）。
收敛姿态下延后（第 7 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
@qwen-code /triage |
|
Sandboxed verification: ❌ not passed — non-deterministic tests (flakiness gate) - 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: 39 passed · 0 failed · 39 total Flakiness gate: ❌ 1 of 1 changed test file(s) returned different results across identical re-runs (5 full round(s)) The deterministic flakiness gate re-ran the test files this PR changes and got different outcomes from identical runs (agent verdict: 中文 — 判定：❌ 不通过 · 测试结果不确定（抖动门）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：39 通过 · 0 失败 · 39 总计 抖动门：❌ 1 of 1 changed test file(s) returned different results across identical re-runs (5 full round(s)) 确定性抖动门将本 PR 改动的测试文件原样重跑了多轮，得到了不一致的结果（agent 判定： Verification reportVerification report — PR #10042
|
| arm | production code | result | sole failure |
|---|---|---|---|
head e783939 |
PR | 361 passed, 1 skipped, 0 failed | — |
base fe34a5c |
greedy first-match, two clocks | 360 passed, 1 failed | prefers a usable issuer over an expired same-subject twin (false CERT_HAS_EXPIRED reproduced) |
| v-clock | greedy + one clock | 360 passed, 1 failed | same central test → the clock change alone does not fix the alarm |
| v-pref | prefer-usable + two clocks | 360 passed, 1 failed | judges the walk and the report at one sampled instant → preference alone still emits the false alarm under the two-clock poison |
Witnesses: 03-vitest-arms-and-mutation-matrix.png, 04-base-arm-vitest-red.png. The one skip on every arm is the pre-existing writes a worker TLS trust gap to the daemon log at boot (it.skipIf(!hasIpv6Loopback)) — environmental, unrelated to this PR; the describeWorkerTlsTrustGaps gate (tls.getCACertificates) is live on this Node and all five new tests executed.
Mock-free dist A/B, real clock (no fake timers; 2026-08-29, short twin expired 2026-08-22)
Bundles generated with openssl ca -startdate/-enddate: three self-signed roots sharing subject and key (expired / long-lived / not-yet-valid) plus a leaf they all verify (gen-fixtures.sh). Harness ab-harness.mjs drives each arm's compiled dist (dist/src/serve/... on head, dist/serve/... on the archived base build). Witness: 01-ab-dist-cells.png.
| bundle (leaf + …) | base | head | meaning |
|---|---|---|---|
| expired-first (defect shape) | 1 gap CERT_HAS_EXPIRED (false) | 0 gaps | central flip |
| nyv-first (not-yet-valid twin first) | 1 gap CERT_NOT_YET_VALID (false) | 0 gaps | sibling-class flip |
| renewed-first (order control) | 0 gaps | 0 gaps | order-independent |
| short-only (only copy expired) | 1 gap | 1 gap | true positive preserved on head |
| both-expired | 1 gap | 1 gap | fallback still reports |
| long-only (positive control) | 0 gaps | 0 gaps | — |
Real-handshake oracle (ground truth, no PR code involved) — 02-handshake-oracle.png
A Node TLS server serves the expired-first fullchain; clients connect with the worker's merged-bundle shapes:
| client trust bundle | handshake | diagnostic agreement |
|---|---|---|
| expired-first (merged) | OK | base's "every handshake fails CERT_HAS_EXPIRED" is empirically false; head's silence matches reality |
| renewed-first (merged) | OK | same |
| short-only | FAIL CERT_HAS_EXPIRED | head still reports this gap — the diagnostic's surviving message is true |
| long-only | OK | control |
Mutation matrix (single-hunk mutants of head, full file each)
| mutant | killed by | failure shape |
|---|---|---|
| M1 greedy first-match restored (= v-clock) | prefers a usable issuer… |
false CERT_HAS_EXPIRED gap |
M2 certValidAt validTo >= → > |
still anchors at the exact notAfter when the short twin is the only issuer |
false NOT_YET_VALID at the edge |
M3 walk samples its own Date.now() |
judges the walk and the report at one sampled instant |
expected 2 to be 1 — the sample-count assert fires while the gap list stays [] (the quiet variant, caught loud) |
M4 certValidAt validFrom <= → < |
agrees on the exact instant the short-lived twin becomes valid |
false NOT_YET_VALID (both twins share notBefore, so the fallback picks the short twin) |
Survivors: none. Every guard the PR introduces is pinned, including both inclusive boundary edges (my pre-run prediction that M4 would survive was wrong — the fixture's shared notBefore pins it). Witness: 03-vitest-arms-and-mutation-matrix.png.
Findings
No blocking findings. Non-blocking observations:
- Two incidental flakes, unrelated to the PR. Under full-file runs with mutants in place,
keeps health responsive before starting deferred runtime work(M2 run,vi.waitForoncreateAcpSessionBridge) andkeeps the daemon log quiet when the serving cert covers the dialled host(M4 run, 15 s timeout) each failed once; both pass in isolation with the mutant still applied, and passed on every other arm. These are load-correlated timing flakes in real-server supervisor tests, not PR behavior. Recorded inlogs/mut-M2-rerun-health.log,logs/mut-M4-rerun-quiet.log. The same flake class also appeared in one intermediate base-arm archival attempt (3 failures: central + the two timing tests) under post-build load; the archived canonical base run (logs/v-base.log,base-arm-junit.xml) has exactly the one central failure. - One transient harness hiccup. Of five
ab-table.mjsruns, one printed a cell mismatch; the other four (including the captured one and three consecutive re-runs) match all 12 cells with exit 0. Non-reproducing, fail-loud, and adjudicated environmental; not counted in the assertion tally.
Corrections
None — no earlier review round exists for this PR, and the description's claims all held up under measurement (including "the new case is the only failure" when the greedy walk is restored, verified as the base arm: 1 failed | 360 passed).
Not covered
- Per-commit attribution. The checkout is shallow (depth 2): only 1 of the snapshot's 10 commits is locally reachable, so the two fix commits (
32f9e83preference,74087feone-clock) were verified as reconstructed intermediate variants (v-clock / v-pref) against the aggregateHEAD^1..HEADdiff, not as checked-out commits. - Full daemon boot path. The harnesses drive
describeWorkerTlsTrustGaps(the boot diagnostic) directly and run real handshakes; the end-to-endrunQwenServeboot with channels was exercised only by the suite's pre-existing supervisor tests. - Mixed-capability twins. A twin pair where the valid copy is CA-incapable and the expired copy is CA-capable was not constructed; the preference picks "usable now", which for such a pathological bundle would surface
incapableIssuerwhile OpenSSL would also fail on the capable-but-expired copy — both outcomes are gap-bearing, so this is at most a message-choice nuance, but it was not measured. - Whole-suite / repo gates. Only
packages/cli src/serve/run-qwen-serve.test.tswas run (the PR's only touched surface); repo-wide lint/typecheck/CI were left to the PR's own pipeline. - Base dist build caveat. The base worktree's
tsc --buildexited 1 on an unrelated UI test type error (@testing-library/reactresolution through the symlinked per-packagenode_modules); emission completed and the emitteddist/serve/run-qwen-serve.js(flat layout, vs the head CI build'sdist/src/serve/) was verified to be pure base code (0 occurrences ofcertValidAt, greedychain.findpresent) before use. The main-tree dist was CI-built at head and used as-is. A copy of the base dist is archived inbase-dist/(restructured asbase-dist/packages/cli/distwith anode_modulessymlink) so the harnesses re-run after worktree cleanup; the full base-arm suite log and junit are archived aslogs/v-base.logandbase-arm-junit.xml.
Methodology
Environment: CI node:22-bookworm container at refs/pull/10042/merge (depth 2); npm ci + npm run build pre-run at head. Suite arms ran in scratch worktrees (tmp/v-base at HEAD^1, tmp/v-{clock,pref,mut} at HEAD with single-hunk edits) wired to the root node_modules via symlinks (PR touches no lockfile; internal @qwen-code/* dists are head-built and unchanged by the PR — realpath-checked). The mock-free harness imports the compiled dist of each arm and calls describeWorkerTlsTrustGaps with certSourcePath mirroring the production call site (run-qwen-serve.ts:8428 passes certSourcePath: tlsCertPath); the handshake oracle uses real tls.createServer/tls.connect with the generated bundles. Fixtures were generated with openssl ca -startdate/-enddate (OpenSSL 3.0) and cross-verified (-no_check_time verify OK per twin; identical pubkey hashes). All scripted assertions live in ab-harness.mjs, handshake-oracle.mjs, check-results.mjs (23 suite-level checks), ab-table.mjs; raw logs in logs/; captures in evidence/ via scripts/verify-capture.mjs. Assertion tally: 12 dist cells + 4 handshake outcomes + 23 adjudication checks = 39/39.
Flakiness gate log
rounds=5 files=1 skipped=0
file packages/cli/src/serve/run-qwen-serve.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/run-qwen-serve.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/cli/src/serve/run-qwen-serve.test.ts: PPPFP
verdict: flaky
summary: 1 of 1 changed test file(s) returned different results across identical re-runs (5 full round(s))
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/cli/src/serve/run-qwen-serve.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/run-qwen-serve.test.ts: P (exit 0)
round 3 · packages/cli/src/serve/run-qwen-serve.test.ts: P (exit 0)
round 4 · packages/cli/src/serve/run-qwen-serve.test.ts: F (exit 1)
--- output tail · round 4 · packages/cli/src/serve/run-qwen-serve.test.ts ---
l worker error when ACP process shutdown also fails�[32m 21�[2mms�[22m�[39m
�[32m✓�[39m runQwenServe channel worker supervisor�[2m > �[22mkeeps the channel lease when lifecycle aggregation wraps the retryable worker error�[32m 35�[2mms�[22m�[39m
�[32m✓�[39m runQwenServe channel worker supervisor�[2m > �[22mbounds the logger flush before allowing a retryable close to reject�[32m 287�[2mms�[22m�[39m
�[32m✓�[39m runQwenServe channel worker supervisor�[2m > �[22mforce-stops the bridge while retrying a failed channel teardown�[32m 66�[2mms�[22m�[39m
�[32m✓�[39m runQwenServe channel worker supervisor�[2m > �[22mremoves serve-owned pidfile through the legacy fallback cleanup path�[32m 41�[2mms�[22m�[39m
�[32m✓�[39m runQwenServe channel worker supervisor�[2m > �[22mkeeps non-serve-owned pidfiles in the legacy fallback cleanup path�[32m 27�[2mms�[22m�[39m
�[32m✓�[39m runQwenServe channel worker supervisor�[2m > �[22mkeeps serve running when worker pidfile metadata cannot be written�[32m 20�[2mms�[22m�[39m
�[32m✓�[39m runQwenServe channel worker supervisor�[2m > �[22mupdates the serve-owned pidfile when a restarted worker becomes ready�[32m 19�[2mms�[22m�[39m
�[32m✓�[39m runQwenServe channel worker supervisor�[2m > �[22mforwards channel worker log and exit details into the daemon log�[32m 28�[2mms�[22m�[39m
�[32m✓�[39m runQwenServe channel worker supervisor�[2m > �[22mpasses a loopback daemon URL to workers when serve binds a wildcard host�[32m 20�[2mms�[22m�[39m
�[32m✓�[39m runQwenServe channel worker supervisor�[2m > �[22mdoes not write a worker pidfile after runtime startup already timed out�[32m 31�[2mms�[22m�[39m
�[32m✓�[39m runQwenServe channel worker supervisor�[2m > �[22mreports a warning when the ready channel worker exits�[32m 24�[2mms�[22m�[39m
�[32m✓�[39m runQwenServe channel worker supervisor�[2m > �[22mfails serve startup when the worker exits before ready�[32m 43�[2mms�[22m�[39m
�[32m✓�[39m runQwenServe channel worker supervisor�[2m > �[22mdrains the runtime when channel grouping rejects startup after listen�[32m 18�[2mms�[22m�[39m
�[32m✓�[39m runQwenServe channel worker supervisor�[2m > �[22mpreserves the startup reason when channel cleanup also fails�[32m 27�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m runQwenServe channel worker supervisor�[2m > �[22mkeeps the serve owner alive when failed startup cannot confirm worker exit �[33m 1093�[2mms�[22m�[39m
�[32m✓�[39m runQwenServe channel worker supervisor�[2m > �[22mrefuses to start when another channel service is already running�[32m 37�[2mms�[22m�[39m
�[32m✓�[39m runQwenServe channel worker supervisor�[2m > �[22mretries channel pidfile reservation after an EEXIST stale file cleanup�[32m 43�[2mms�[22m�[39m
�[32m✓�[39m runQwenServe channel worker supervisor�[2m > �[22mremoves the channel pidfile reservation when listener startup fails�[32m 20�[2mms�[22m�[39m
�[32m✓�[39m runQwenServe channel worker supervisor�[2m > �[22mretries the next port on EADDRINUSE and succeeds�[32m 20�[2mms�[22m�[39m
�[32m✓�[39m runQwenServe channel worker supervisor�[2m > �[22mdoes not retry on non-EADDRINUSE listen errors�[32m 10�[2mms�[22m�[39m
�[32m✓�[39m runQwenServe channel worker supervisor�[2m > �[22mrejects after exhausting all port retry attempts�[32m 9�[2mms�[22m�[39m
�[32m✓�[39m runQwenServe channel worker supervisor�[2m > �[22mdoes not retry EADDRINUSE when port is 0 (ephemeral)�[32m 9�[2mms�[22m�[39m
�[32m✓�[39m runQwenServe channel worker supervisor�[2m > �[22mdoes not remove the channel pidfile reservation for handled uncaught exceptions�[32m 18�[2mms�[22m�[39m
�[32m✓�[39m runQwenServe channel worker supervisor�[2m > �[22mpreserves the channel pidfile reservation until an unhandled-exit worker is confirmed gone�[32m 26�[2mms�[22m�[39m
�[32m✓�[39m runQwenServe startup observability�[2m > �[22mnames every pre-auth surface in the --allow-origin '*' warning�[32m 18�[2mms�[22m�[39m
�[32m✓�[39m runQwenServe startup observability�[2m > �[22mkeeps the stdout listening contract and exposes startup timing on stderr and status�[32m 51�[2mms�[22m�[39m
�[32m✓�[39m runQwenServe startup observability�[2m > �[22muses boot runtimeOutputDir for daemon logs�[32m 20�[2mms�[22m�[39m
�[32m✓�[39m runQwenServe startup observability�[2m > �[22muses explicit daemonLogBaseDir when provided by an embedder�[32m 22�[2mms�[22m�[39m
�[32m✓�[39m runQwenServe startup observability�[2m > �[22mpreserves Storage runtime base dir for default exported callers�[32m 20�[2mms�[22m�[39m
�[32m✓�[39m runQwenServe startup observability�[2m > �[22mtracks preheat running and succeeded states for an internally-created bridge�[32m 34�[2mms�[22m�[39m
�[32m✓�[39m runQwenServe startup observability�[2m > �[22mtracks preheat failed state and error message for an internally-created bridge�[32m 24�[2mms�[22m�[39m
�[31m⎯⎯⎯⎯⎯⎯⎯�[39m�[1m�[41m Failed Tests 1 �[49m�[22m�[31m⎯⎯⎯⎯⎯⎯⎯�[39m
�[41m�[1m FAIL �[22m�[49m src/serve/run-qwen-serve.test.ts�[2m > �[22mrestores the Conversations runtime for a persisted scheduled task
�[31m�[1mAssertionError�[22m: expected "startScheduledTaskKeepalive" to be called with arguments: [ ObjectContaining{…} ]�[90m
Received:
�[1m 1st startScheduledTaskKeepalive call:
�[22m�[2m [�[22m
�[32m- ObjectContaining {�[90m
�[32m- "boundWorkspace": "/__w/_temp/flake-inv-tmp/qws-live-task-keepalive-spV0F5/home/Documents/Qwen Code/Conversations",�[90m
�[31m+ {�[90m
�[31m+ "boundWorkspace": "/__w/_temp/flake-inv-tmp/qws-live-task-keepalive-spV0F5/workspace",�[90m
�[31m+ "bridge": {�[90m
�[31m+ "activePromptCount": 0,�[90m
�[31m+ "activeWork": false,�[90m
�[31m+ "activeWorkCoverage": {�[90m
�[31m+ "covered": 0,�[90m
�[31m+ "oldestCoveredReportAt": null,�[90m
�[31m+ "onNegotiatedChannel": 0,�[90m
�[31m+ "total": 0,�[90m
�[31m+ },�[90m
�[31m+ "getAllSessions": [Function spy],�[90m
�[31m+ "getDaemonStatusSnapshot": [Function spy],�[90m
�[31m+ "getEventRing": [Function spy],�[90m
�[31m+ "getSession": [Function spy],�[90m
�[31m+ "isChannelLive": [Function spy],�[90m
�[31m+ "killAllSync": [Function spy],�[90m
�[31m+ "lastActivityAt": null,�[90m
�[31m+ "pendingPermissionCount": 0,�[90m
�[31m+ "preheat": [Function spy],�[90m
�[31m+ "publishWorkspaceEvent": [Function spy],�[90m
�[31m+ "resume": [Function spy],�[90m
�[31m+ "sessionCount": 0,�[90m
�[31m+ "shutdown": [Function spy],�[90m
�[31m+ "spawnOrAttach": [Function spy],�[90m
�[31m+ },�[90m
�[31m+ "cleanupSession": [Function cleanupSession],�[90m
�[31m+ "intervalMs": 600000,�[90m
�[31m+ "onTasksRead": [Function onTasksRead],�[90m
�[31m+ "reviveTimeoutMs": 70000,�[90m
�[31m+ "runtimeBaseDir": "/__w/_temp/flake-inv-tmp/.qwen",�[90m
�[2m },�[22m
�[2m ]�[22m
�[31m�[90m
Number of calls: �[1m1�[22m
�[31m�[39m
�[36m �[2m❯�[22m src/serve/run-qwen-serve.test.ts:�[2m585:30�[22m�[39m
�[90m583| �[39m await handle.runtimeReady;
�[90m584| �[39m await vi.waitFor(() => {
�[90m585| �[39m expect(startKeepalive).toHaveBeenCalledWith(
�[90m | �[39m �[31m^�[39m
�[90m586| �[39m expect.objectContaining({
�[90m587| �[39m boundWorkspace: canonicalRoot,
�[31m�[2m⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯�[22m�[39m
�[2m Test Files �[22m �[1m�[31m1 failed�[39m�[22m�[90m (1)�[39m
�[2m Tests �[22m �[1m�[31m1 failed�[39m�[22m�[2m | �[22m�[1m�[32m360 passed�[39m�[22m�[2m | �[22m�[33m1 skipped�[39m�[90m (362)�[39m
�[2m Start at �[22m 20:25:09
�[2m Duration �[22m 124.22s�[2m (transform 26.92s, setup 289ms, collect 30.18s, tests 51.88s, environment 986ms, prepare 1.04s)�[22m
JUNIT report written to /__w/qwen-code/qwen-code/packages/cli/junit.xml
round 5 · packages/cli/src/serve/run-qwen-serve.test.ts: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
|
🤖 AutoFix updated a stale base — the fix did not pass verification, but this PR was behind Same-run verification repair stopped: gate failures are host saturation, not this PRThe previous commit The gate's 7 failures
中文说明🤖 AutoFix 更新了一个过期的 base —— 修复未通过验证，但本 PR 落后于 同轮验证修复中止：门禁失败源于宿主机 CPU 饱和，与本 PR 无关上一个提交 门禁的 7 个失败
以上取自门禁自身的 JUnit 报告（ 证据1. HEAD 上的确定性本地复现。 启动耗时 26.5–28.1 秒，而测试预算只有 30 秒。 2. origin/main 上同样慢——不是本 PR 引入。 临时把 Run log: https://github.com/QwenLM/qwen-code/actions/runs/33268794582 🧠 Handled by Qwen Code · model/模型 |
Local verification of #10042 — real PKI, real handshakes, real daemonVerdict: recommend merge. The walk change is correct when measured against a real OpenSSL oracle, its tests are non-vacuous (7/7 mutants killed), and it removes two classes of false boot warning without silencing the genuine one. One correction to the PR body is in Finding 1 — it concerns the story, not the code. Environment
The two PKIs:
1. Ground truth: what OpenSSL and Node actually doBefore judging the model, I measured the thing it models — a real
The PR's premise holds: a merged renewed bundle handshakes fine regardless of order, and only the bundle whose sole copy is expired really fails 2.
|
| check | what the log shows |
|---|---|
Test (ubuntu-latest, Node 22.x) |
##[error]The operation was canceled at 1h00m; no failing test in the log up to that point |
Serve A/B (ubuntu-latest, Node 22.x) |
canceled at 46m in the middle of a vite build step |
web-shell E2E Smoke |
Playwright failures in packages/web-shell specs; also red on #10522, and this PR touches no web-shell code |
Locally, src/serve has 14 pre-existing failures in 6 unrelated files (fast-path, capabilities-docs-contract, workspace-file-system, workspace-agents, workspace-memory, conversation-runtime-ownership) — identical on both arms, so they are environmental (running as root), not PR-caused.
Reproducing
# renewed root: one CA key, two self-signed copies sharing a subject, short one first
openssl genrsa -out ca.key.pem 2048
openssl req -x509 -new -key ca.key.pem -config root.cnf -set_serial 0x1001 \
-not_before <-40d> -not_after <-10d> -out root-short.pem
openssl req -x509 -new -key ca.key.pem -config root.cnf -set_serial 0x1002 \
-not_before <-40d> -not_after <+10y> -out root-long.pem
openssl x509 -req -in leaf.csr -CA root-long.pem -CAkey ca.key.pem -out leaf.pem …
cat leaf.pem root-short.pem root-long.pem > fullchain.pem
# ground truth
openssl verify -CAfile fullchain.pem leaf.pem # OK
# real daemon, both arms
qwen serve --port 4171 --hostname 127.0.0.1 \
--tls-cert fullchain.pem --tls-key leaf.key.pem --channel <any>中文版报告
#10042 本地验证 —— 真实 PKI、真实握手、真实 daemon
结论：建议合并。 以真实 OpenSSL 为基准衡量，链上行的这处改动是正确的；它的测试非空转（7/7 变异体全部被杀死）；它消除了两类虚假启动告警，同时没有掩盖真实的那一条。对 PR 描述有一处更正，见发现 1 —— 只涉及叙述，不涉及代码。
环境
| PR head | 33dfa5d6（fix/worker-anchor-valid-issuer） |
| Base | fe34a5cf —— merge-base；head 的目录树与它仅相差本 PR（2 个文件，+246/−16） |
| 主机 | Linux、Node v22.22.2（OpenSSL 3.5.5）、系统 openssl 3.5.6 |
| 构建 | 全新 npm ci + npm run build；A/B 只替换 packages/cli/src/serve/run-qwen-serve.ts 后重建 packages/cli |
| 夹具 | 本地用 openssl 现生成两套 PKI —— 不是 PR 里内置的夹具，让链上行去判断它从未见过的证书 |
两套 PKI：
- 续期根 —— 同一把 CA 密钥、同一 subject 签出两份：一份 11 天前已过期的短有效期证书，一份 10 年有效的续期证书，外加一张两者都能验证的叶子证书。两种排列顺序各做一个 bundle。
- 交叉签名 bridge —— ISRG X1 / DST X3 的形状：同一把 CA 密钥，既有一份有效的自签根，又有一份由 bundle 中不存在的旧根交叉签发、且已过期的副本。
1. 基准事实：OpenSSL 和 Node 实际是怎么做的
在评判模型之前，先测量它所建模的东西 —— 一个真实的 tls.createServer 加载该 bundle，再由另一个独立进程把 NODE_EXTRA_CA_CERTS 指向同一个 bundle 去连接，也就是 WORKER_TLS_TRUST_PROBE 的完全相同形态。
PR 的前提成立：合并后的续期 bundle 无论顺序如何握手都成功，只有那份"唯一副本已过期"的 bundle 才真的以 CERT_HAS_EXPIRED 失败。
2. 用这个基准检验 describeWorkerTlsTrustGaps —— base 对比 PR
base 在 A 和 E 两个 case 上与真实握手结论相反；PR 则 6/6 全部一致，而 case C 证明 ?? issuers[0] 的回退仍然会点名真正过期的链成员。
Case E 是本 PR 顺带修掉的第二类虚假告警，PR 描述里没有提到。 在交叉签名的 bundle 上，贪心的链上行会踩到那份过期的交叉签名证书，发现它的旧签发者不在 bundle 里，于是返回 anchored: false —— base 因此会在一个每次握手都成功的 bundle 上打出更吓人的那条："nothing in the channel workers' bundle anchors their trust … UNABLE_TO_VERIFY_LEAF_SIGNATURE … Point NODE_EXTRA_CA_CERTS at the issuing CA"。优先选择可用签发者同样把它修掉了。建议在 PR 描述里补一句 —— 同一个缺陷，但杀伤面更大。
3. 真实 daemon，两个 arm
在隔离的 HOME/工作区里跑 qwen serve --tls-cert <bundle> --tls-key <key> --channel tg，让启动诊断真实执行。
base：续期根场景 2 条告警、bridge 场景 3 条告警，其中分别有 1 条和 2 条是假的。PR：两个场景都只剩 1 条，且是真的那条。反向对照 —— 一份没有续期副本、确实已过期的根 —— 在修复后依然被报出来。
4. 非空转验证
PR head 上的 run-qwen-serve.test.ts：362 通过 / 0 失败。packages/cli 的 tsc --noEmit：干净。
发现
发现 1 —— PR 描述里的运维故事在生产调用点上到达不了（属于文档问题，不是代码问题）
描述里说这个诊断*"对一个健康的配置报了红，并且让运维去续期一个他们刚刚续过的 CA"*。在唯一的生产调用点上这不会发生，因为这份预测只有在实时握手探针已经失败时才会打印：
// packages/cli/src/serve/run-qwen-serve.ts:8457-8478
const trustFailure = await (deps.workerTlsTrustVerifier ?? verifyWorkerTlsTrust)({ … });
if (trustFailure) {
if (predictedGaps.length > 0) {
for (const gap of predictedGaps) daemonLog.warn(gap);
} else { … }
}在 PR 自己描述的场景里 ——"合并后的 bundle 是通过续期的那份完成授权的，每一次握手都是成功的"—— trustFailure 是 undefined，于是什么都不会打印。实测见截图 3 的第一组：对于健康的续期 bundle，base 和 PR 打印的 worker-TLS 告警都是零条，尽管 describeWorkerTlsTrustGaps 本身在 base 上确实会返回那条假 gap（截图 2 的 A 行）。
这并不削弱这个修复，只是把它的价值挪了个位置。这条假告警确实能到达运维的路径有两条，都已在上面验证过或可在同一段代码里读出：
- 混合 bundle（截图 3 的第 2 组和截图 4）：一个真实缺陷 + 一个续期根。base 会把那条假的
CERT_HAS_EXPIRED打在最前面，排在真实原因之前，把运维指向去续期一个他们刚续过的 CA。这才是真正需要这个修复的场景。 - 非信任类的探针失败（
WORKER_TLS_VERIFY_TIMEOUT、探针被 kill）：predictedGaps.length > 0会走进if分支，于是假的过期告警是取代探针自己的错误码被打印出来 —— 真实原因反而被压掉了。
建议把 Why it's needed 一节改写成描述"混合 bundle"的情形，而不是完全健康的情形，因为后者根本不会浮现。不要求改代码。
发现 2 —— 四条新增边界测试里有一条不可能失败（次要，测试卫生）
agrees on the exact instant the short-lived twin expires 是唯一一条没有任何变异能让它失败的新用例，连它名字所指的 notAfter 边界变异也不行（截图 5 第 3 行 —— 那个变异是被另一条测试抓住的）。因为长有效期的那份还在 bundle 里，链上行在边界处会直接改锚到它，断言两种情况下都成立。这正是 PR 自己写在 still anchors at the exact notAfter when the short twin is the only issuer 上方的注释所预测的，而那条配套测试确实钉住了这个边界 —— 所以这份冗余是有意为之、也无害。指出来只是为了避免后来的读者把它误当成边界覆盖。
另外检查过、未发现问题
certValidAt与它替换掉的两个判定：对所有可解析的日期完全等价，包括notAfter/notBefore的闭区间边界。唯一的行为差异是validFrom/validTo无法解析（NaN）时会新产生一条*"not yet valid"* gap —— 我构造不出能触达它的证书；Node 连99991231235959Z都会渲染成Date可解析的字符串。- 把
const now = Date.now()上提到链上行之前：新旧采样点之间没有任何Date.now()的消费者，并且judges the walk and the report at one sampled instant里的samples === 1断言在本机成立（loader oracle 走的是spawnSync，不碰时钟）。 - 这个偏好是逐跳生效的，不会把原本能锚定的链上行变成失败：在我构造的所有形状里，当前有效的签发者要么直接锚定，要么导向与 OpenSSL 相同的结论。
CI
三个红色检查都不能归因于本 PR：
| 检查项 | 日志显示 |
|---|---|
Test (ubuntu-latest, Node 22.x) |
1h00m 处 ##[error]The operation was canceled；在此之前日志里没有失败用例 |
Serve A/B (ubuntu-latest, Node 22.x) |
46m 处在一个 vite build 步骤中途被取消 |
web-shell E2E Smoke |
失败全部在 packages/web-shell 的 Playwright 用例里；#10522 上同样是红的，而本 PR 没有碰任何 web-shell 代码 |
本地跑 src/serve 有 14 条既有失败，分布在 6 个无关文件（fast-path、capabilities-docs-contract、workspace-file-system、workspace-agents、workspace-memory、conversation-runtime-ownership）—— 两个 arm 完全一致，属于环境因素（以 root 运行），不是本 PR 引入的。
复现步骤
# 续期根：同一把 CA 密钥、同一 subject 的两份自签证书，短有效期的排在前面
openssl genrsa -out ca.key.pem 2048
openssl req -x509 -new -key ca.key.pem -config root.cnf -set_serial 0x1001 \
-not_before <-40d> -not_after <-10d> -out root-short.pem
openssl req -x509 -new -key ca.key.pem -config root.cnf -set_serial 0x1002 \
-not_before <-40d> -not_after <+10y> -out root-long.pem
openssl x509 -req -in leaf.csr -CA root-long.pem -CAkey ca.key.pem -out leaf.pem …
cat leaf.pem root-short.pem root-long.pem > fullchain.pem
# 基准事实
openssl verify -CAfile fullchain.pem leaf.pem # OK
# 真实 daemon，两个 arm 各跑一次
qwen serve --port 4171 --hostname 127.0.0.1 \
--tls-cert fullchain.pem --tls-key leaf.key.pem --channel <任意频道>
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 6b": running the new tests in packages/cli/src/serve/run-qwen-serve.test.ts under vitest — the worktree has no node_modules and no workspace dist/ builds, and …; "agent 6b": npm run typecheck /lint for packages/cli — same environment blocker; the sole modified call site was instead verified by direct read..
Test Plan (not a blocker): src/serve/run-qwen-serve.test.ts — no such file or directory.
中文说明
未探索到全部深度（达到工具调用预算）："agent 6b"：running the new tests in packages/cli/src/serve/run-qwen-serve.test.ts under vitest — the worktree has no node_modules and no workspace dist/ builds, and …；"agent 6b"：npm run typecheck /lint for packages/cli — same environment blocker; the sole modified call site was instead verified by direct read.。
Test Plan（非阻断）：src/serve/run-qwen-serve.test.ts — no such file or directory。
— 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: 100 passed · 0 failed · 100 total Flakiness gate: ✅ 1 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：100 通过 · 0 失败 · 100 总计 抖动门：✅ 1 changed test file(s) x 5 identical rounds, no divergence Verification reportVerification report (round 2) — PR #10042
|
| # | round-1 finding | severity | status at the new head |
|---|---|---|---|
| 1 | Flakiness gate: restores the Conversations runtime for a persisted scheduled task returned PPPFP over 5 identical full-file runs → run reported ❌ not passed despite the merge-ready agent verdict |
gate | root-caused, stands as a pre-existing main issue, not caused by this PR. Re-measured: 10 round-2 full-file runs (head 5 all green; base 5 failing exactly the intended central cell) never failed the keepalive test; instrumented probes show the keepalive call always arrives with the correct workspace, 50–169 ms after runtimeReady, against the 1000 ms vi.waitFor window. See "Flakiness-gate failure investigation". |
| 2 | Incidental flakes keeps health responsive… / keeps the daemon log quiet… under load |
note | did not recur in any of the 10 round-2 full-file runs; consistent with load-correlated timing flakes; stands as environmental |
| 3 | Transient ab-table harness hiccup (1 of 5 runs) |
note | did not recur: ab-table re-ran 3× this round (smoke + 2 captures), 17/17 cells every time |
Central claim + A/B
Central claim: when a renewed CA leaves two same-subject/same-key roots in the serving bundle and the expired copy sorts first, the anchor walk must describe the usable copy — no false CERT_HAS_EXPIRED at boot — while a chain whose only copy is expired is still reported.
Secondary claims: (1) the walk's issuer preference and the per-member validity flags judge one sampled instant; (2) the issuers[0] fallback preserves every pre-existing outcome.
Suite A/B (head test file on every arm, packages/cli vitest, full file)
| arm | production code | result | sole failure |
|---|---|---|---|
head 8d12d264 |
PR | 362 passed, 0 failed, 0 skipped | — |
base-arm (HEAD^1 02f2101f + head test file) |
greedy first-match, two clocks | 361 passed, 1 failed | prefers a usable issuer over an expired same-subject twin (false alarm reproduced) |
| base file on base | base test file (357 tests; PR tests absent) | 357 passed | — (control: base tree is green on its own file) |
| v-clock | greedy + one clock | 361 passed, 1 failed | central test → the clock change alone does not fix the alarm |
| v-pref | prefer-usable + two clocks | 361 passed, 1 failed | judges the walk and the report at one sampled instant → preference alone still fails the two-clock poison |
Witnesses: 03-suite-arms-adjudication.png, 04-base-arm-vitest-red.png. The IPv6-loopback test that round 1 skipped is not skipped in this container — head is 362/362 with zero skips.
Mock-free dist A/B, real clock (2026-08-29; short twin expired 2026-08-22)
Fixtures regenerated with openssl ca -startdate/-enddate (OpenSSL 3.0.20): four self-signed roots sharing subject and key (identical pubkey SHA-256 a97ae7c8…) — expired / expired-2 / renewed / not-yet-valid — plus a leaf each twin verifies (-no_check_time OK ×4). ab-table.mjs imports each arm's compiled dist/src/serve/run-qwen-serve.js and calls describeWorkerTlsTrustGaps with certSourcePath mirroring production. 17/17 scripted cells hold. Witness: 01-ab-dist-cells.png.
| bundle (leaf + …) | base | head | meaning |
|---|---|---|---|
| expired-first (defect shape) | 1 gap CERT_HAS_EXPIRED (false) | 0 gaps | central flip |
| nyv-first (not-yet-valid twin first) | 1 gap CERT_NOT_YET_VALID (false) | 0 gaps | sibling-class flip |
| renewed-first (order control) | 0 gaps | 0 gaps | order-independent |
| short-only (only copy expired) | 1 gap | 1 gap | true positive preserved on head |
| both-expired | 1 gap | 1 gap | fallback still reports |
| long-only (positive control) | 0 gaps | 0 gaps | — |
Real-handshake oracle (ground truth, no PR code) — 02-handshake-oracle.png
A Node TLS server serves the expired-first fullchain; clients connect with the worker's merged-bundle shapes. 4/4 cells hold.
| client trust bundle | handshake | diagnostic agreement |
|---|---|---|
| expired-first (merged) | OK | base's "every handshake fails CERT_HAS_EXPIRED" is empirically false; head's silence matches reality |
| renewed-first (merged) | OK | same |
| short-only | FAIL CERT_HAS_EXPIRED | head still reports this gap — its surviving message is true |
| long-only | OK | control |
Mutation matrix (single-point mutants of head, full file each)
| mutant | killed by | failure shape |
|---|---|---|
| M1 greedy first-match restored (= v-clock) | prefers a usable issuer… |
false CERT_HAS_EXPIRED gap |
M2 certValidAt validTo >= → > |
still anchors at the exact notAfter when the short twin is the only issuer |
expected [ Array(1) ] to deeply equal [] — false NOT_YET_VALID at the exact notAfter edge |
M3 walk samples its own Date.now() |
judges the walk and the report at one sampled instant |
expected 3 to be 1 — the sample-count assert fires (quiet variant, caught loud) |
M4 certValidAt validFrom <= → < |
agrees on the exact instant the short-lived twin becomes valid |
false gap at the exact notBefore edge |
M0 positive control: gap message CERT_HAS_EXPIRED → CERT_EXPIRED (no guard touched) |
pre-existing names an expired chain member the signature-only walk accepts |
to contain 'CERT_HAS_EXPIRED' fails — proves the harness collects tests that exercise the mutated file |
Survivors: none. All four guards the PR introduces are pinned, both inclusive boundary edges included, and the positive control confirms the kill signal is real. Witness: 03-suite-arms-adjudication.png.
Flakiness-gate failure investigation
What the gate saw (round 1): 1 of 5 identical full-file runs failed restores the Conversations runtime for a persisted scheduled task — the keepalive spy had exactly one call (the primary runtime's, boundWorkspace=<serve workspace>), and the expected call with boundWorkspace=<…>/home/Documents/Qwen Code/Conversations never arrived within the default 1000 ms vi.waitFor window.
Round-2 measurements (all re-run at the new head):
- Reproduction: 0/10 full-file runs this round failed that test — head 5/5 all green (
05-flake-reps-head-vs-base.png), base 5/5 failing exactly the intended central A/B cell and nothing else. The flake did not reproduce under this round's load profile; round 1's gate regime (5 sequential runs on a loaded shared ECS host) caught a tail event. - Attribution to main, not this PR: the test is byte-identical between
HEAD^1andHEAD(git diff HEAD^1..HEADcontains zero lines of it), and the production keepalive path is untouched by the PR (the diff's production hunks are exclusively the TLS walk region;describeWorkerTlsTrustGaps's only call site is the TLS-configured boot branch, which this test does not exercise). The base arm shows identical behavior. - Mechanism — latency race, not silent drop:
markRuntimeReady()resolveshandle.runtimeReadybefore live-conversation discovery finishes; the conversation runtime's keepalive starts only after that runtime is built asynchronously (runtimeAdded→startKeepaliveForWorkspace). A silent drop is structurally excluded for this runtime:trustedis unconditionallytrueforlive-conversationprovenance, and theruntimeAddedhook can only fire with the app/locals absent during an await-free synchronous stretch between the registry loop and the locals install. The instrumented probe (archived asflake-probe.test.ts— the gate test withvi.waitFortimeout 1 s → 20 s plus twoappendFileSynctimestamp recorders) confirms: in 4/4 runs the conversation keepalive call arrived with the canonical Conversations workspace, at +50, +61, +87, +169 ms afterruntimeReady(06-probe-latency-margin.png). - Margin: threshold 1000 ms; natural delay 50–169 ms under this round's container load. The gate's PPPFP shows the tail crossed 1000 ms under its regime (the vitest config itself documents that ECS hosts run several jobs at once). This is a speed/load-correlated failure with a thin margin: it passes on quiet boxes and fails intermittently on loaded ones — exactly the shape the gate is designed to catch, in a test this PR did not write.
Conclusion: pre-existing main test race (assertion window vs. async runtime materialization latency), not a PR defect. It will keep failing the flakiness gate on any PR that touches this file until main fixes the test; a maintainer may want to land the fix (finding 1) and re-trigger the lane.
Findings
- (Non-blocking, pre-existing on main — not caused by this PR)
restores the Conversations runtime for a persisted scheduled taskraces the default 1 svi.waitForwindow against the asynchronous materialization of the Conversations runtime; on loaded runners the keepalive start can land past the window even though it always arrives correctly (probe delays 50–169 ms here; gate tail >1000 ms in round 1). Repro of the shape: run the file 5× back-to-back on a loaded host (round-1 gate log); the cause (tail latency) is measured, not replayed. Minimal suggested fix for main: give that wait a generous timeout — the probe file demonstrates{ timeout: 20000 }(4/4 green, call always arrives, no other behavior change). Measured as required: with the patch, 4/4 probe runs green and the conversation keepalive always arrives with the canonical workspace; without it, round-1's gate produced 1/5 red on identical code. The red cannot be forced on demand in this round's load regime, so the margin argument (50–169 ms natural vs. 1000 ms threshold vs. observed gate tail) is the evidence. This is a test-window fix, not a production change; if a maintainer prefers, waiting on an explicit signal instead of a timeout would remove the load dependence entirely. - (Note) The two round-1 incidental supervisor flakes did not recur across 10 round-2 full-file runs; left as environmental, unchanged from round 1.
Corrections
None — round 1's description-level claims all re-held at the new head (the base arm's sole failure is exactly the central test, as the PR description states).
Not covered
- Per-commit attribution — checkout is depth 2 (1 of the snapshot's 11 commits reachable); the two fix commits were verified as reconstructed intermediate variants (v-clock / v-pref) against the aggregate
HEAD^1..HEADdiff, not as checked-out commits. - Mixed-capability twins — a twin pair where the valid copy is CA-incapable and the expired copy CA-capable was not constructed (same caveat as round 1; both outcomes are gap-bearing, so at most a message-choice nuance).
- Full daemon boot path with channels — harnesses drive
describeWorkerTlsTrustGapsdirectly plus real handshakes; end-to-end boot exercised only by the suite's pre-existing supervisor tests. - Forcing the gate-regime tail — the >1 s latency tail cannot be reproduced on demand here (the load state no longer exists); the margin analysis stands on the natural samples plus the gate's own round-1 PPPFP.
- Whole-suite / repo gates — only
packages/cli src/serve/run-qwen-serve.test.tswas run; repo-wide lint/typecheck/CI were left to the PR's own pipeline. - Probe instrumentation scope — the probe file changes the keepalive test only (timeout + two timing recorders); its latency numbers come from instrumented runs, its green/red from the same file.
Methodology
Environment: CI node:22-bookworm container at refs/pull/10042/merge (depth 2); npm ci + npm run build pre-run at head. Suite arms in scratch worktrees: tmp/v-base at HEAD^1 (full node scripts/build.js; the packages/webui vite step failed — unrelated to cli tests — while cli dist emission completed and was verified pure base code: 0 × certValidAt, 0 × issuers.find, greedy chain.find( present, exports load); tmp/v-arm at HEAD for v-clock/v-pref/M0–M4 (single-hunk text substitutions enforced exactly-once by apply-arm.mjs, git checkout -- between arms). Worktrees wired to the root node_modules by symlink; the per-package node_modules dirs (nested @opentelemetry/*, @testing-library/*) likewise — without them the base core build fails TS2307. Realpath checks: node_modules/@qwen-code/qwen-code-core → head tree packages/core; the effective diff (git diff HEAD^1..HEAD --name-only = only the two serve files) proves core/acp-bridge/web-templates/channels are byte-identical between arms, so head-resolved links are clean controls; vitest's resolve.alias additionally maps those packages to each worktree's own source. Fixtures: gen-fixtures.sh (openssl ca, explicit dates), cross-verified. Harnesses: ab-table.mjs (17 cells), handshake-oracle.mjs (4), check-results.mjs (45), check-flakes.mjs (26), check-probes.mjs (8) — 100 scripted assertions total; raw logs in logs/; captures in evidence/ via scripts/verify-capture.mjs. logs/probe-timings-2.log was reconstructed verbatim from the captured run-2 output (the live file is overwritten by each subsequent run). Assertion tally: 17 + 4 + 45 + 26 + 8 = 100 passed, 0 failed.
Flakiness gate log
rounds=5 files=1 skipped=0
file packages/cli/src/serve/run-qwen-serve.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/run-qwen-serve.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/cli/src/serve/run-qwen-serve.test.ts: PPPPP
verdict: pass
summary: 1 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/cli/src/serve/run-qwen-serve.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/run-qwen-serve.test.ts: P (exit 0)
round 3 · packages/cli/src/serve/run-qwen-serve.test.ts: P (exit 0)
round 4 · packages/cli/src/serve/run-qwen-serve.test.ts: P (exit 0)
round 5 · packages/cli/src/serve/run-qwen-serve.test.ts: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
🤖 AutoFix ran out of time before finishing (timeout (3600000ms)) (attempt 5/100) — it will retry on the next scan.
See the Qwen Autofix agent step logs for model/tool output. 中文说明🤖 AutoFix 在完成前耗尽了时间（timeout (3600000ms)）（第 5/100 次尝试）—— 将在下次扫描时重试。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/33269210648 🧠 Handled by Qwen Code · model/模型 |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix round report — PR #10042 (no code changes)Critical-only mode is active (round 6). The actionable set this round is one Finding 1 — PR body's operator story is not reachable at the production call siteVerified correct. At the only production caller Disposition: acknowledged; not actionable from this round. The finding
Finding 2 — the boundary test
|
| check | runtime | configured timeout | source |
|---|---|---|---|
Test (ubuntu-latest, Node 22.x) |
20:57:01 → 21:57:25 (60m24s) | timeout-minutes: 60 |
.github/workflows/ci.yml, test job |
web-shell E2E Smoke (ubuntu-latest, Node 22.x) |
21:57:29 → 22:17:39 (20m10s) | timeout-minutes: 20 |
.github/workflows/ci.yml |
Serve A/B (ubuntu-latest, Node 22.x) |
20:56:06 → 21:41:30 (45m24s) | timeout-minutes: 45 |
.github/workflows/serve-ab.yml, ab job |
Each run ended in CANCELLED (not FAILURE) at its job-level timeout. This
PR touches only packages/cli/src/serve/run-qwen-serve.ts and its colocated
test: the web-shell smoke exercises none of it (the maintainer reports it red
on unrelated PR #10522 as well), and the maintainer's log reading shows no
failing test before the Test cancellation and the Serve A/B cancellation
mid vite build. Local evidence: the PR's own focused suite is green and fast
(see Verification). Raising CI timeout budgets would mean editing .github/,
which is outside this round's permitted footprint and is not a defect of this
PR anyway. No in-scope code action exists; the workflow's independent CI
remains the final gate, and a re-run should clear once the jobs complete
within their timeout budgets.
Verification
Commands actually run this round (no code was changed, so no commit):
npm run build— passedcd packages/cli && npx vitest run src/serve/run-qwen-serve.test.ts— 362/362 passed (~82s), matching the maintainer's local result
中文说明
Autofix 轮次报告 — PR #10042（无代码改动）
当前处于仅处理 Critical 的模式（第 6 轮）。本轮可执行集合为一份维护者验证报告（[ic:5464841489]，结论"建议合并"）中的两条非代码发现，外加三个被取消的 CI 检查。所有要点均已在当前 head（afc810b1）上对照代码核实；不需要也不做任何代码改动，分支 head 保持不变。各项处置如下。
发现 1 — PR 描述中的运维故事在生产调用点上到达不了
核实属实。 在唯一的生产调用点（packages/cli/src/serve/run-qwen-serve.ts 中的 channel worker TLS 信任检查），predictedGaps 只在 if (trustFailure) 内部打印，因此对于健康的续期 bundle —— 实时握手探针成功的情形 —— 什么日志都不会输出。修复前那条假告警确实能到达运维的两条路径也在代码中得到确认：（1）同时含有真实缺陷的混合 bundle，假的 CERT_HAS_EXPIRED 行会排在真实原因之前打印；（2）非信任类的探针失败 —— verifyWorkerTlsTrust 对任何探针进程错误都会解析出一个 truthy 的失败对象（超时/探针被 kill → stdout 无法解析 → 返回错误码对象），因此只要 predictedGaps 非空，假的过期告警就会取代探针自身的错误码被打印，把真实原因压掉。
处置：认可；但本轮无法执行。 该发现要求的是编辑 PR 描述，并明确"不要求改代码"。PR 描述是 GitHub 侧的元数据：本轮没有 GitHub 凭据，且 address-review 工作流没有更新 PR 描述的通道（pr-body.md 只在创建 PR 时被消费；不存在 gh pr edit）。留给 PR 作者/维护者处理，以下为 Why it's needed 一节的建议替换文本（可自行调整 —— 本轮看不到当前 PR 描述原文）：
当实时的 worker 握手探针失败时，启动诊断会打印预测出的信任缺口来引导运维。修复之前，链上行取第一个 subject 加签名都匹配的证书；但续期后的 CA 会在 bundle 里留下两份共享同一 subject 和同一密钥的证书，两者都能验证它们签发的内容。当已过期的那份排在前面时，诊断会把每次握手所依赖的链成员指认为它，并让运维去续期一个他们刚刚续过的 CA —— 在混合 bundle（另有一个真正损坏的成员）上它会排在真实原因之前打印；在非信任类的探针失败（如
WORKER_TLS_VERIFY_TIMEOUT或探针被 kill）上，它会取代探针自身的错误码被打印。同样的"优先可用签发者"逻辑还修复了交叉签名的情形：旧的链上行会踩到那份过期的交叉签名证书，其旧签发者不在 bundle 中，于是在一个每次握手都成功的 bundle 上报告anchored: false（UNABLE_TO_VERIFY_LEAF_SIGNATURE，"把NODE_EXTRA_CA_CERTS指向签发 CA"）。逐跳优先选择当前可用的签发者，使预测与 OpenSSL 实际使用的路径一致。
发现 2 — 边界测试 agrees on the exact instant the short-lived twin expires 不可能失败
与描述一致，已核实。 其配套测试 still anchors at the exact notAfter when the short twin is the only issuer 通过 leafPlusShortTwinOnlyPem() 移除了长有效期的那份，这才让 notAfter 边界变得可观测；该测试上方的注释也已说明：只要长有效期的那份还在 bundle 里，certValidAt 中若写成严格的 >，链上行就会改锚到它而照样报告无缺口。维护者的变异矩阵确认 notAfter 边界变异体是被配套测试杀死的，而不是这条。
处置：拒绝（不改动）。 维护者已明确这份冗余"有意为之、也无害"。该测试仍然在边界处断言了正确的 bundle 级行为，并与 notBefore 一侧的测试（agrees on the exact instant the short-lived twin becomes valid）对称；删除、改名或加注都只是纯粹的变动噪音，而且角色分工已经在关键位置（配套测试上方）写明。不做代码改动。
失败的检查 — 三项均为作业级超时取消，不是测试失败
checks.json 中的运行时长与配置的 timeout-minutes 对比如下：
| 检查项 | 运行时长 | 配置的超时 | 出处 |
|---|---|---|---|
Test (ubuntu-latest, Node 22.x) |
20:57:01 → 21:57:25（60分24秒） | timeout-minutes: 60 |
.github/workflows/ci.yml 的 test 作业 |
web-shell E2E Smoke (ubuntu-latest, Node 22.x) |
21:57:29 → 22:17:39（20分10秒） | timeout-minutes: 20 |
.github/workflows/ci.yml |
Serve A/B (ubuntu-latest, Node 22.x) |
20:56:06 → 21:41:30（45分24秒） | timeout-minutes: 45 |
.github/workflows/serve-ab.yml 的 ab 作业 |
三次运行均在作业级超时处以 CANCELLED（而非 FAILURE）结束。本 PR 只改动了 packages/cli/src/serve/run-qwen-serve.ts 及其同目录测试：web-shell 冒烟测试完全不涉及这些代码（维护者报告该检查在无关的 PR #10522 上同样是红的），且维护者的日志分析显示 Test 被取消前没有任何失败用例，Serve A/B 是在 vite build 步骤中途被取消。本地证据：本 PR 自己的聚焦测试套件是绿的且很快（见"验证"一节）。调高 CI 超时预算意味着修改 .github/，这既超出本轮允许的范围，也不是本 PR 的缺陷。不存在范围内可执行的代码动作；工作流的独立 CI 仍是最终门槛，待作业能在超时预算内跑完，重跑即应通过。
验证
本轮实际运行的命令（未改动代码，故无提交）：
npm run build— 通过cd packages/cli && npx vitest run src/serve/run-qwen-serve.test.ts— 362/362 通过（约 82 秒），与维护者本地结果一致
Deferred non-Critical feedback
Critical-only mode is active: 5 change-producing rounds are complete. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (@qwen-code /retry starts a fresh counting window.)
- Review by @qwen-code-ci-bot: fix(serve): prefer a usable issuer over an expired same-subject twin #10042 (review)
- Review by @qwen-code-ci-bot: fix(serve): prefer a usable issuer over an expired same-subject twin #10042 (review)
- Review by @qwen-code-ci-bot: fix(serve): prefer a usable issuer over an expired same-subject twin #10042 (review)
- Review by @qwen-code-ci-bot: fix(serve): prefer a usable issuer over an expired same-subject twin #10042 (review)
- Inline rc:3886712748 packages/cli/src/serve/run-qwen-serve.test.ts:2879: fix(serve): prefer a usable issuer over an expired same-subject twin #10042 (comment)
- Inline rc:3886712751 packages/cli/src/serve/run-qwen-serve.ts:1462: fix(serve): prefer a usable issuer over an expired same-subject twin #10042 (comment)
- Inline rc:3886712754 packages/cli/src/serve/run-qwen-serve.ts:1685: fix(serve): prefer a usable issuer over an expired same-subject twin #10042 (comment)
- PR comment by @qwen-code-ci-bot: fix(serve): prefer a usable issuer over an expired same-subject twin #10042 (comment)
- PR comment by @qwen-code-ci-bot: fix(serve): prefer a usable issuer over an expired same-subject twin #10042 (comment)
中文说明
已进入仅处理 Critical 的模式：已完成 5 个产生改动的轮次。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 @qwen-code /retry 可开启新的计数窗口。）
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。
🧠 Handled by Qwen Code · model/模型 qwen3.8-max
4 participants
What this PR does
When the boot-time worker TLS trust diagnostic walks a serving bundle up to its anchor, it now prefers an issuer whose validity window contains the current time over one that has expired, instead of taking whichever matching certificate happens to come first in the bundle.
Why it's needed
Renewing a CA leaves two certificates in the bundle that share a subject and a key, so both verify what they issued and both match the walk's "did this sign that" test. The walk took the first match, so a bundle whose expired copy sorts first reported that expired certificate as the path the handshake depends on —
--tls-cert "…" chain member "…" expired on …, every worker handshake to the daemon fails CERT_HAS_EXPIRED. Meanwhile the merged bundle authorizes through the renewed copy and every handshake succeeds. The diagnostic exists to catch silent-green outages; here it produced the opposite, a loud red on a healthy configuration, and told an operator to renew a CA they had just renewed. OpenSSL may use either copy, so the diagnostic should describe the one that is usable.Reviewer Test Plan
How to verify
The new fixture
TEST_TLS_CERT_FULLCHAIN_RENEWED_ROOTis a leaf plus two self-signed roots sharing a subject and key, with the short-lived copy first in the bundle; the test moves the clock to 2030 so exactly one copy is expired, and assertsdescribeWorkerTlsTrustGapsreports no gap. To see it witness the defect, restore the greedy walk inwalkWorkerAnchorPath(chain.find(...)in place of the filter-and-prefer) and re-run: the new case is the only failure.The existing expired-chain-member cases (
names an expired chain member the OPERATOR file contributed,names an expired chain member the signature-only walk accepts) still pass: a chain whose only copy of a member is expired is still reported, because the preference falls back toissuers[0]when nothing usable matches.Evidence (Before & After)
N/A — no user-visible surface changes; the difference is the content of a boot diagnostic, covered by the test above.
Tested on
Environment (optional)
Unit tests only,
npm ci+npx vitest runon Node 22.Risk & Scope
nonCaTerminator,incapableIssuer,pathLengthViolation) is decided by the same predicates as before, andissuers[0]keeps the previous behavior when no candidate is currently valid.Linked Issues
Follow-up to #9392, which introduced
walkWorkerAnchorPath.中文说明
这个 PR 做了什么
启动时的 worker TLS 信任诊断在把服务证书链向上走到信任锚时，现在会优先选择有效期覆盖当前时间的签发者，而不是取 bundle 里第一个匹配上的证书。
为什么需要
CA 续期会在 bundle 里留下两份共享 subject 和密钥的证书，因此两份都能验证它们签发的东西，也都能通过链上行"是不是它签的"这一检查。原来的实现取第一个匹配项，于是当过期的那份排在前面时，诊断就会把这份过期证书报成握手所依赖的路径 ——
--tls-cert "…" chain member "…" expired on …, every worker handshake to the daemon fails CERT_HAS_EXPIRED。而实际上合并后的 bundle 是通过续期的那份完成授权的，每一次握手都是成功的。这个诊断的存在意义是抓"表面全绿实则中断"的故障，这里却产生了相反的结果：对一个健康的配置报了红，并且让运维去续期一个他们刚刚续过的 CA。OpenSSL 用哪一份都有可能，所以诊断应该描述可用的那一份。评审验证方案
如何验证
新增的 fixture
TEST_TLS_CERT_FULLCHAIN_RENEWED_ROOT由一张叶子证书加两张共享 subject 和密钥的自签根组成，短有效期的那份排在前面；测试把时钟拨到 2030 年，使得恰好只有一份过期，然后断言describeWorkerTlsTrustGaps报告没有缺口。想看到它确实见证了这个缺陷，把walkWorkerAnchorPath里的贪心写法还原（用chain.find(...)替换 filter-and-prefer）再跑一次：新增的这条是唯一的失败项。已有的过期链成员用例（
names an expired chain member the OPERATOR file contributed、names an expired chain member the signature-only walk accepts）依然通过：如果某个链成员只有过期的那一份，仍然会被报出来，因为在没有可用候选时这个偏好会回退到issuers[0]。证据（前后对比）
N/A —— 没有用户可见的界面变化；差异体现在启动诊断的内容上，已由上面的测试覆盖。
测试平台
环境（可选）
仅单元测试，Node 22 上执行
npm ci+npx vitest run。风险与范围
nonCaTerminator、incapableIssuer、pathLengthViolation）仍由和之前完全相同的判定得出，并且当没有任何候选当前有效时，issuers[0]保持原有行为。关联 Issue
#9392 的后续，
walkWorkerAnchorPath由该 PR 引入。