fix: add structured error code to SessionNotFoundError for session-closing retry - #8884
Conversation
PR QwenLM#8864 retried session switches while the target session is closing, but relied on fragile string matching against the daemon's error message. This commit: 1. Adds a `code` property to `SessionNotFoundError` — automatically set to `'session_closing'` when the extra message mentions "closing", otherwise `'session_not_found'`. 2. Includes `code` in the HTTP JSON response body so clients can distinguish closing (transient) from genuinely missing sessions without depending on error message text. 3. Updates the WebUI retry check in `DaemonSessionProvider` to use `errorBody.code === 'session_closing'` instead of matching `endsWith('The session is closing; retry after close completes')`. 4. Fixes an inconsistent error message in `rewindSession` that used the short `'The session is closing'` without the retry suffix. Closes: QwenLM#8864 (follow-up)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Thanks for the PR, @yiliang114 — this is the planned follow-up from the #8864 review thread (the structured-code suggestion deferred there), and the direction is uncontroversial, but the PR body doesn't follow the PR template, so I have to pause it here. This is a formatting gate, not a code concern.
None of the template's required sections are present (the body currently uses a custom ## Summary / ## Changes structure):
- What this PR does — prose description of the change; note the template asks for prose, not the per-component rundown the current
## Changeslist is - Why it's needed — the motivation is already in your summary (the fragile string matching landed by #8864); it helps to also note this was deferred from that review thread
- Reviewer Test Plan, with its three subsections:
- How to verify — the behaviors a reviewer should confirm and what to expect: e.g. the closing-retry path now keys off the structured
codefield instead of the message suffix, the rewind path returns the consistent retry message, and which suites pin that (the updatedDaemonSessionProvider.test.tsxfixtures) - Evidence (Before & After) — this is an internal daemon/Web Shell protocol change rather than a user-visible one, so
N/Ais fine here per the template, with commands and output under How to verify - Tested on — the OS matrix (🍏/🪟/🐧 with ✅/
⚠️ /N/A); right now it's unclear where your verification ran
- How to verify — the behaviors a reviewer should confirm and what to expect: e.g. the closing-retry path now keys off the structured
- Risk & Scope — the three bullets: main risk or tradeoff / not validated / breaking changes
- Linked Issues — reference #8864 without a closing keyword, since this is a follow-up
- The
<details>Chinese translation of the body
Could you restructure the body to follow the template? The content you already wrote is good — most of it can be moved into the right sections as-is. Please keep each paragraph or list item as one long line (the template notes that GitHub renders single newlines as <br>, so hard-wrapped text displays as a narrow column).
Once the body is updated, a maintainer can re-run triage with @qwen-code /triage to continue.
中文说明
感谢提交 PR，@yiliang114 —— 这是 #8864 评审线程中约定的后续 PR（当时搁置的结构化 code 建议），方向没有争议，但 PR 正文没有遵循 PR 模板，所以需要先停在这里。这是一次格式上的拦截，而不是对代码的质疑。
模板要求的章节全部缺失（目前正文使用了自定义的 ## Summary / ## Changes 结构）：
- What this PR does —— 用散文描述改动；注意模板要求散文描述，而不是目前
## Changes那样按组件罗列 - Why it's needed —— 动机在你的摘要里已经写了（#8864 引入的脆弱字符串匹配）；最好同时说明这是从那个评审线程搁置下来的后续工作
- Reviewer Test Plan，包含三个子章节：
- How to verify —— 评审者应确认的行为和预期结果：例如 closing 重试路径现在依赖结构化
code字段而不是消息后缀、rewind 路径返回一致的重试消息，以及哪些测试套件固定了这些行为（更新的DaemonSessionProvider.test.tsxfixtures） - Evidence (Before & After) —— 这是 daemon/Web Shell 内部协议改动，而非用户可见改动，按模板写
N/A即可，命令与输出放在 How to verify 下 - Tested on —— 操作系统矩阵（🍏/🪟/🐧 加 ✅/
⚠️ /N/A）；目前无法判断你的验证是在哪个平台上进行的
- How to verify —— 评审者应确认的行为和预期结果：例如 closing 重试路径现在依赖结构化
- Risk & Scope —— 三个要点：主要风险或权衡 / 未验证项 / 破坏性变更
- Linked Issues —— 引用 #8864（不使用关闭关键字，因为这是后续 PR）
- 正文的
<details>中文翻译
能否按模板重构正文？你已经写好的内容大部分是好的 —— 多数可以直接挪到对应章节。请保持每个段落或列表项为一长行（模板注明 GitHub 会把单个换行渲染成 <br>，硬换行的文字会显示成窄列）。
正文更新后，维护者可以用 @qwen-code /triage 重新触发 triage 继续流程。
— Qwen Code · qwen3.8-max
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterℹ️ No screenshot changed against the PR base — but this PR edits 1 render-shaping file:
Either the change has no visual effect (logic, plumbing, a state the scenarios never reach), or no scenario renders this UI — in which case the preview cannot see it, and an empty result is a coverage gap rather than a clean bill of health. To make it visible, add a scenario to Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 4 scenario(s). — Qwen Code · serve A/B |
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): PR #8884 (QwenLM/qwen-code) adds a structured code fiel...: did not trace every call path that can surface inside the WebUI event-loop catch (e.g., whether a closeSessionImpl double-close 404 can ever reach that catch …; PR #8884 (QwenLM/qwen-code) adds a structured code fiel...: none — all checks above completed within budget.; PR #8884 (QwenLM/qwen-code) adds a structured code fiel...: none — all checks I started were completed within budget.; PR #8884 (QwenLM/qwen-code) adds a structured code fiel...: none — all checks I started were completed within budget..
Not reviewed: reverse audit — stopped before round 2 by the review time budget.
— qwen3.8-max via Qwen Code /review (v0.21.8)
|
Closeout: replaced message-derived closing classification with explicit producer codes, preserved Java DELETE idempotence for 中文摘要已改为显式 closing 错误码、保留 Java DELETE 幂等语义、补充测试与协议文档并修复 PR 模板；204 个 WebUI 和 7 个 CLI 测试通过，本地无 JRE/JDK，等待 CI。 |
|
Verified this end-to-end against the installed Desktop 0.1.1 / Qwen Code 0.21.9 candidate. I rapidly switched
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
Not explored to full depth (tool budget reached): This change adds a structured session_closing error cod...: none — all checks above completed within budget.; This change adds a structured session_closing error cod...: none — all checks above completed within budget.; This change adds a structured session_closing error cod...: Java unit test not executed — no JVM tooling ( java / javac / mvn ) exists on this machine; the Java change was verified by close reading only..
中文说明
已审查。 建议见行内评论。 1 条建议级发现无法锚定到改动行，已丢弃；此处无需进一步处理。
未探索到全部深度（达到工具调用预算）：This change adds a structured session_closing error cod...：none — all checks above completed within budget.；This change adds a structured session_closing error cod...：none — all checks above completed within budget.；This change adds a structured session_closing error cod...：Java unit test not executed — no JVM tooling ( java / javac / mvn ) exists on this machine; the Java change was verified by close reading only.。
— qwen3.8-max via Qwen Code /review (v0.21.9)
Packaged Desktop regression resultThe final local Desktop Two concurrent sessions were both admitted with
|
|
Changed: corrected the canonical protocol error shape to include Verified: Prettier and Intentionally not changed: additional producer tests and helper extraction. Pending: exact-head CI and automatic review. 中文摘要已统一协议文档中的标准错误结构并补充 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): Change summary: This PR adds a structured code field ('...: did not dynamically confirm how often the child's sessionClose RPC actually returns a definitive refusal in production traffic (mechanism verified from source…; Change summary: This PR adds a structured code field ('...: none — all planned checks completed (~20 tool calls).; You are review agent reverse-audit — Reverse audit agen...: none — I finished within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget..
中文说明
已审查。 建议见行内评论。 1 条建议级发现无法锚定到改动行，已丢弃；此处无需进一步处理。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：Change summary: This PR adds a structured code field ('...：did not dynamically confirm how often the child's sessionClose RPC actually returns a definitive refusal in production traffic (mechanism verified from source…；Change summary: This PR adds a structured code field ('...：none — all planned checks completed (~20 tool calls).；You are review agent reverse-audit — Reverse audit agen...：none — I finished within budget.；You are review agent reverse-audit — Reverse audit agen...：none — all checks above completed within budget.。
— qwen3.8-max via Qwen Code /review (v0.21.9)
|
Resolved the current review feedback on |
|
@qwen-code /triage |
|
Sandboxed verification: Skipped because the PR has merge conflicts, so refs/pull/8884/merge is unavailable — resolve conflicts and re-run. 中文 — 判定：
|
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed exact head afd183369070ec0ea79b8859dd4044b8b3f544e5. The structured closing code is explicit at every closing-state producer, including the post-await restore re-check; it is serialized on REST 404s, drives only the pending WebUI load retry, and preserves Java DELETE idempotence while validating the session id. Unknown sessions retain session_not_found. Exact-head Node, Web Shell, Serve A/B, Java matrix, and real-daemon E2E checks pass, and the prior Critical findings are fixed. No blocking findings.
|
Re-run against
The branch state that blocked the last two runs is cleared: the rebase onto Moving on to code review. 🔍 中文说明针对恢复后的 head
阻挡前两轮的分支状态已清除:到 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewEverything the last run asked for is verifiably on this head — I re-checked each item against the diff and current main, not the closeout:
Test evidence — PR's own CI at
|
| Check | Conclusion |
|---|---|
| Test (ubuntu-latest, Node 22.x) | ⏳ in progress |
| Serve A/B (ubuntu-latest, Node 22.x) | ⏳ in progress |
| Capture web-shell visuals (ubuntu-latest, Node 22.x) | ⏳ in progress |
| ubuntu-latest / Java 11, Java 17, Java 21 | ✅ success |
| macos-latest / Java 21, windows-latest / Java 21 | ✅ success |
| Real daemon E2E / Java 11 | ✅ success |
| Desktop Shell (ubuntu-22.04), Desktop Shell (windows-2022) | ✅ success |
| Test (macos-latest / windows-latest, Node 22.x) | ⏭️ skipped (merge-queue-only jobs, by design) |
| Integration Tests (CLI, No Sandbox) | ⏭️ skipped |
The ubuntu suite — which covers every TS package this PR touches (acp-bridge, cli, webui) — is still running, with nothing red so far. Two notes on the shape of this table: the macOS/Windows Node tests are merge-queue-only in ci.yml (event_name == 'merge_group'), so their skip is CI policy, not a coverage gap; and the Java suites settle the one gap the author flagged (no local JDK) — the new destroyAcceptsAlreadyClosingForCurrentSession regression test ran green across the full matrix plus the real-daemon E2E.
Sandboxed verification would settle the last claim static review can't: @qwen-code /verify — that a real daemon under a genuine closing race emits code: "session_closing" on the wire and the WebUI retries it, and that the suites fail with the code check removed. A verify run for this head is already in flight per the lifecycle comment. Real-scenario testing: not driven — unattended CI run; the packaged-Desktop results earlier in the thread remain the author's claim, not independently re-run here.
中文说明
代码审查
上一轮要求的每一项都已确认落在当前 head 上 —— 逐项对照 diff 与当前 main 核验,不是照抄 closeout:
- 合并已找回。 到
32812e9的 rebase 带有 fix(webui): Make cross-session switching transactional #8882 的解决方案:isClosingSessionLoadError()优先检查body?.['code'] === 'session_closing',WebUI 冲突的解决方式与被丢弃的2d9e9d1当时所做完全一致。 - 夹具机制钉已恢复。 正向 closing 重试夹具的 body 文本已截短(
"...The session is closing"),旧的endsWith匹配器无法命中 —— 该测试只能靠code通过。新增负向测试也已落地:session_not_found404 不重试。 - 类钉 + 分类表行:均在,原样保留。
- 抛出点覆盖完整。 对照 main 审计了
bridge.ts中全部SessionNotFoundError构造点:六处 closing 消息抛出点,加上restoreSession的 post-await复查(此前是裸 not-found,现在正确拆分为 not-found 与 closing),全部带session_closing;其余均为真正的未知 session 默认值,包括两处形似但并非 closing 状态的 agent 崩溃抛出。没有遗漏的 closing 抛出点。 - 本轮新增部分是旧回退的门槛:
isClosingSessionLoadError(error, allowLegacyMessage)仅在守护进程未声明client_identity时保留文本匹配。两个调用点都核过:SSE 恢复循环从守护进程能力快照读取门槛(undefined 时向旧匹配敞开);跨 session 切换点保持仅 code 的默认值 —— 结构上正确,因为该路径只对声明client_identity的守护进程存在(fix(webui): Make cross-session switching transactional #8882 把更旧的守护进程路由到旧流程)。[webshell]retry session switch while closing #8864 自带的retains legacy closing-session retries without client identity测试也因此在构造上仍然通过。一个非阻塞保留意见:client_identity自 5 月起已随版本发布(feat(serve): add daemon-stamped client identity #4231),而code随本 PR 才发布,因此新版 WebUI 对接已发布的守护进程(v0.21.9 及以前)时,closing 竞态会显示为错误而不是自动重试。该窗口随本次发布关闭 —— 带client_identity与code字段的守护进程将同时发出 —— 且失败方向良性(手动重试可用,无损失),故记录在案、不作为阻塞。 - Java 范围确认不变且使用正确:
isCurrentSessionNotFound只被destroySession()的 DELETE 使用,code == null仍兼容旧守护进程。更早两条延期建议(Java 谓词共享、organization 路由加code)均有书面理由 —— 维持不动。
测试证据 —— 54f9fbc 自身 CI,经 API 一次性获取
(机器可读表格见英文部分。)ubuntu 套件 —— 覆盖本 PR 触及的全部 TS 包(acp-bridge、cli、webui)—— 仍在运行,目前无红。两点说明:macOS/Windows Node 测试在 ci.yml 中仅限 merge queue(event_name == 'merge_group'),跳过是 CI 策略而非覆盖缺口;Java 套件解决了作者标注的唯一缺口(本地无 JDK)—— 新增的 destroyAcceptsAlreadyClosingForCurrentSession 回归测试在完整矩阵加真实守护进程 E2E 中全绿。
沙箱验证可以了结静态审查无法证明的最后一项:@qwen-code /verify —— 真实守护进程在真实关闭竞态下是否在线上发出 code: "session_closing" 并被 WebUI 重试,以及移除 code 检查后套件是否失败。针对该 head 的 verify 运行已按生命周期评论在途。真实场景测试:未驱动 —— 无人值守 CI 运行;线程前段的打包 Desktop 结果仍是作者自述,未在此独立复跑。
— Qwen Code · qwen3.8-max
Reviewed at 54f9fbced23c8dd6ce9ea6c763865a50535ef5be · re-run with @qwen-code /triage
|
Confidence: 4/5 — every prior-round demand is verifiably landed and the change is exactly the minimal follow-up the merged parent deferred; the last fifth is CI finishing on this head. Stepping back: my independent take on this problem — one additive wire field on the existing error class, default preserved, every closing producer tagged, consumers switched to the code with a compatibility path for older daemons — is what this PR does, almost line for line. The one place it goes beyond the naive version is the The branch saga from the last two runs is over: the merge that was pushed and then lost in a force-push is back, resolved the same way, and this time it is the head itself. Recovery steps 1–4 from my last comment are all verifiable in the diff. The Java tests the author couldn't run locally ran green in CI across the full matrix plus the real-daemon E2E. A maintainer approval already stands on this exact commit; the bot's own is the one still missing, and CI is the only thing in front of it. CI: two (Housekeeping: the bot's two Aug-10 template-gate 中文说明Confidence: 4/5 —— 前几轮提出的每一项要求都已可核验地落地,改动正是已合并父 PR 延后的那个最小后续;最后五分留给当前 head 的 CI 跑完。 整体看:我对这个问题的独立方案 —— 在既有错误类上加一个可加成的 wire 字段、默认值保留、所有 closing 抛出点打标、消费方切换到 code 并为旧守护进程保留兼容路径 —— 与本 PR 几乎逐行一致。唯一超出朴素方案之处是旧文本回退的 前两轮的分支波折到此结束:那次推上去又在 force-push 中丢失的合并回来了、以同样方式解决、且这次就是 head 本身。上条评论的恢复步骤 1–4 均可在 diff 中核验。作者本地无法运行的 Java 测试已在 CI 完整矩阵加真实守护进程 E2E 中全绿。一位维护者的批准已落在同一提交上;缺的只是机器人自己的那张票,而 CI 是它前面唯一的障碍。 CI: (事务性说明:机器人 8 月 10 日针对最初 head 的两条模板门禁 — Qwen Code · qwen3.8-max Reviewed at |
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): PR #8884 (QwenLM/qwen-code) adds a structured code fiel...: none — all planned checks completed. Note verification was by reading and tracing, not by executing the test suites.; PR #8884 (QwenLM/qwen-code) adds a structured code fiel...: none — all checks above completed within budget.; PR #8884 (QwenLM/qwen-code) adds a structured code fiel...: none — all checks above completed within budget..
中文说明
已审查。 建议见行内评论。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：PR #8884 (QwenLM/qwen-code) adds a structured code fiel...：none — all planned checks completed. Note verification was by reading and tracing, not by executing the test suites.；PR #8884 (QwenLM/qwen-code) adds a structured code fiel...：none — all checks above completed within budget.；PR #8884 (QwenLM/qwen-code) adds a structured code fiel...：none — all checks above completed within budget.。
— qwen3.8-max via Qwen Code /review (v0.21.9)
|
Resolved all three current review suggestions in
I also merged the latest Verification: core and ACP builds; WebUI provider tests (232); ACP bridge tests (544); focused ESLint; Prettier; |
|
@qwen-code /review --timeout=360 |
|
Qwen Code review request accepted. Review is queued in workflow run. |
- Add toBeInstanceOf(SessionNotFoundError) alongside toMatchObject to preserve the envelope type assertion - Document session_closing code in 18-error-taxonomy.md
2d9e9d1 to
c6f5da5
Compare
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push，因为这会使已有的评审评论失效。另外，供日后参考：作为集成流程的一部分，机器人始终会自动将所有改动压缩（squash）为单个提交。 |
|
@qwen-code /triage |
|
Sandboxed verification: Skipped because the PR has merge conflicts, so refs/pull/8884/merge is unavailable — resolve conflicts and re-run. 中文 — 判定：
|
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Reviewed exact head c6f5da57b4e9e023dc08cd5a6c47495353b9e6ce. I did not find a new code issue in the structured-code flow; the focused WebUI (204) and ACP bridge (522) suites pass. The remaining blocker is the current merge conflict with main in DaemonSessionProvider.tsx; please resolve that shared-helper overlap and rerun exact-head CI before merge.
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
|
Resolved the conflict against the latest main (32812e9). The WebUI merge keeps structured session_closing matching for daemons with client_identity, while preserving the legacy message fallback only for older daemons without that capability. Focused verification passed: WebUI DaemonSessionProvider (232 tests), ACP bridge (544 tests), CLI error response (7 tests), ESLint, Prettier, and the isolated npm ci prepare/build. Maven is unavailable in this environment, so the Java test remains deferred to CI. The two remaining review threads were outdated and are now resolved. @qwen-code /review |
|
@qwen-code /triage |
ytahdn
left a comment
ytahdn
left a comment
There was a problem hiding this comment.
Approve. The structured code field on SessionNotFoundError is the right direction — replacing fragile English-text matching with a stable protocol field. Backward compatibility is preserved (code == null branch in Java client covers old servers). The split of the combined if condition into separate session_not_found vs session_closing checks is semantically clearer.
Note: This PR overlaps significantly with #8896 (same files: bridge.ts, bridgeErrors.ts, error-response.ts, Java SDK). Recommend merging this one first (smaller and more focused), then rebasing #8896 on top.
|
Sandboxed verification: ❌ not passed — findings reported (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: 61 passed · 0 failed · 61 total 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：61 通过 · 0 失败 · 61 总计 Verification reportPR #8884 Deep Verification — structured
|
| Cell | Head | Base (control) |
|---|---|---|
| Load unknown session id | 404 {error, code:"session_not_found", sessionId} |
404 {error, sessionId} — no code |
| Prompt burst while a REST close is in progress | 409 session_archiving (see finding 2) |
identical 409 |
| 16 concurrent DELETE racers | 1×204 + 15×409 session_archiving |
identical histogram |
| Load after close (session persisted by warmup prompt) | 200 restore | 200 restore |
| First DELETE completes | 204 | 204 |
Wire A/B — closing-error serialization (express oracle)
The closing branch of the A/B cannot be raced over REST-vs-REST (the session-archive coordinator rejects contenders with 409 before they reach the bridge's closing check — pre-existing on both arms, see finding 2), so the closing serialization is proven one level down with no mocks of the unit under test: ab-express.mjs imports each tree's compiled error-response.js + bridgeErrors.js (module identity across the instanceof boundary verified behaviorally — a wrong-tree class would fall through to 500, observed 404), wires them into a real express server on loopback, and constructs errors exactly as bridge.ts closing-state producers do. Witness: 03-ab-express-serialization-flip.png.
| Cell | Head | Base (control) |
|---|---|---|
Closing error → sendBridgeError |
404 {"error":"…The session is closing; retry after close completes","code":"session_closing","sessionId":"…"} |
404 same body minus code |
| Unknown-session error | 404 …"code":"session_not_found"… |
404 minus code |
The flip: 0/2 bodies carry code on base, 2/2 on head. Combined with the daemon cells, every SessionNotFoundError 404 gains its structured field end to end.
WebUI retry gate A/B
webui-retry-ab.mjs extracts isClosingSessionLoadError verbatim from each tree's source (TS annotations stripped mechanically, source echoed in the log), and drives 7 fixtures × the capability axis the call site passes (allowLegacyMessage = daemon lacks client_identity). 14/14 head, 7/7 base. Witness: 04-webui-retry-matrix-head-vs-base.png.
| Fixture | Base decision | Head decision |
|---|---|---|
F1 code:"session_closing" + short message (no retry suffix) |
no retry (text never matched) | retry (code-driven, message-independent) |
| F2 code + full legacy message | retry | retry |
F3 legacy body, daemon without client_identity |
retry | retry (legacy fallback allowed) |
F3 legacy body, daemon with client_identity |
retry | no retry (see finding 1) |
| F4/F5 not-found bodies, F6 non-HTTP, F7 wrong status | no retry | no retry |
F1 is the load-bearing flip: the retry decision no longer depends on the daemon's exact English phrasing (which the rewindSession fix in this PR demonstrably violated before).
Mutation matrix (vacuity)
Positive controls: every unmutated gate is green (cli error-response.test.ts 7/7, bridge.test.ts -t closing 2/2, DaemonSessionProvider.test.tsx -t closing 5/5 and -t retry 7/7 — two tests match both filters). Witness for row 4: 05-bridge-mutation-kills-closing-test.png.
| # | Mutation (scratch, restored after) | Suite | Outcome |
|---|---|---|---|
| 1 | sendBridgeError: drop code: err.code from the 404 json |
cli error-response.test.ts | killed — serializes the structured session-closing code fails on expected-vs-actual json (6 others green) |
| 2 | WebUI predicate + call site reverted to base text-match | webui -t closing |
killed — retries a session switch while the target session is closing fails (phase: "failed" instead of connected) |
| 3 | WebUI call site only: capability arg removed (code logic kept) | webui -t closing |
killed — retains legacy closing-session retries without client identity fails (expected "spy" to be called 2 times, but got 1 times) |
| 4 | bridgeErrors.ts: this.code = 'session_not_found' (interface-preserving) |
bridge -t closing |
killed — rejects a bounded refresh when the session starts closing fails on toMatchObject({code:'session_closing'}) |
No mutant regressed any other test. Survivors (closing-state producers with no pinning test, verified by grep for session_closing across all test files): pre-await attach check (bridge.ts ~5100), close-coalescer (~5622), closeSession "already closing" (~5861), spawn-attach during close (~6271), sendPrompt during close (~6539), rewindSession (~9439). All are coverage gaps, not dead code — each site is reachable in its own flow (the prompt/close paths were exercised live in the daemon harness and answered 409 upstream of the bridge), and each passes a literal 'session_closing' to the now-verified constructor. Completeness reporting, not merge conditions.
Sibling sweep
- All 133
new SessionNotFoundError(sites enumerated: the 7 closing-state producers all pass the code; the two other multi-arg sites (bridge.ts ~5213, ~6372) are agent-crash "retry to restore/spawn" errors and correctly keep the defaultsession_not_found. - The Java client's second predicate
isCurrentPermissionNotFoundstill excludessession_closing— verified safe: the permission routes never throw a closing code (their 404s are hand-built or bareSessionNotFoundError), so no behavior change there. - Web-shell's
WorkspaceSessionProviderusesclient_identityfor an unrelated purpose and has no closing-text matching — no sibling consumer needed updating. code: 'session_not_found'already existed at several route-level sites pre-PR (session.ts, sse-events.ts, session-runtime.ts); this PR closes thesendBridgeErrorgap and makes the field total onSessionNotFoundError.
Findings (ordered by severity; all minor)
1. Legacy-retry fallback is gated on client_identity, which every daemon already advertises — cross-version retry narrows vs base
isClosingSessionLoadError(error, !capabilities?.features.includes(CLIENT_IDENTITY_FEATURE)) allows the legacy text match only when the daemon lacks client_identity. Measured live against a head daemon: /capabilities advertises 109 features including client_identity, and the registry entry is since: 'v1' on base as well — i.e. every daemon release, including all pre-PR ones, advertises it, while the code field exists only from this PR on. Consequence (fixture F3 in the matrix): a head WebUI paired with any pre-PR daemon gets allowLegacyMessage=false and no code in the 404 body → closing retry is lost, whereas base retried on text. Same-version deployments (the daemon serves its own WebUI) are unaffected. The repo's established pattern for exactly this situation is a dedicated capability tag ("older daemons without it leave them on the legacy client-fallback behavior" — see the session_mid_turn_message_query registry comment).
Repro: node tmp/pr8884-verify-20260811-131625/webui-retry-ab.mjs --tree <head> --arm head — F3 row flips true → false between the two capability axes.
Suggested direction (not applied)
Either advertise a new capability tag for the structured code (e.g. session_closing_code) and gate the fallback on its absence, or document the cross-version narrowing as an accepted tradeoff in the protocol doc. No behavior change is needed for same-version pairings.
2. Reviewer Test Plan steps 1 and 3 are not reachable through pure REST contention
Step 1 ("load while the session is closing ⇒ 404 session_closing") and step 3 ("concurrent Java close requests ⇒ matching session_closing accepted") presuppose a 404 during a REST-originated close. Measured on both arms: while a REST close holds the archive coordinator's exclusive lease, every racing REST mutation (prompt, load, second DELETE) gets 409 session_archiving with Retry-After: 5 (pre-existing mapping, present on base; racer histogram 1×204 + 15×409 on both arms). The closing 404 branch is reachable only via close originators that bypass the coordinator — another transport's close (ACP-HTTP), agent-death teardown, or the intra-bridge re-checks — so the Java/WebUI acceptance logic is defensive-but-correct rather than dead, and step 2 (unknown session ⇒ session_not_found, no retry) verifies exactly as written. This is a plan-reachability note, not a code defect; the doc wording ("a concurrent close may return code: "session_closing"") is accurate.
3. Only one of seven closing producers is pinned by a test
See the mutation matrix survivors. Sharp edges: the rewindSession change is a real message change ('The session is closing' → full retry suffix + code) with no test, and the post-restore re-check split (the fix(acp) commit) is pinned only by the one added assertion. If a follow-up touches those sites, nothing currently fails.
4. Java change verified statically only
No JRE/JDK/Maven in this container (same constraint the author reported). Static review: the new destroyAcceptsAlreadyClosingForCurrentSession test and the widened predicate keep the sessionId equality guard (a 404 for a different session is still not treated as already-deleted, consistent with the existing destroyDoesNotTreatGenericNotFoundAsAlreadyDeleted), and the daemon-side mapping the test assumes is the one proven by the express oracle. Compilation and test execution remain unverified here.
Not covered
- Java runtime execution — no JDK in container (finding 4); the new test never ran anywhere yet.
- Per-commit attribution — depth-2 checkout: metadata lists 8 commits, only merge/base/head are reachable locally (
git rev-list HEAD^1..HEAD^2= 1). Verified the aggregateHEAD^1..HEADdiff. - Live closing-window sampling over the wire — REST-vs-REST is coordinator-masked (finding 2); I did not drive the ACP-HTTP transport or agent-kill paths to observe a live
session_closing404 from the full daemon. The serialization is proven by the express oracle instead; call this the wire shape proof, not an end-to-end trigger reproduction. - Full workspace suites (only the four affected suites/files, filtered), repo-wide lint/typecheck (CI-covered), docs lint, WebUI rendering.
- A harness-level caveat for the record: an early
-t "retry"vitest filter silently skipped theretries…tests ("retries" does not contain "retry"); the first webui mutation run was redone under-t "closing"and the counts above are from the corrected runs.
Methodology
Environment: CI verify container (node:22-bookworm), merge-ref checkout at depth 2; npm ci + npm run build pre-run at head. Head daemon driven from packages/cli/dist/index.js serve --port 0 --workspace <scratch> --no-web with an isolated HOME; base control rebuilt in tmp/base-tree with all @qwen-code workspace links re-pointed into the base tree (realpath-asserted) and all other deps symlinked from the root install (lockfile untouched by the PR, so the shared third-party deps are a clean control). Harnesses (ab-daemon.mjs, ab-express.mjs, webui-retry-ab.mjs) live in this artifact dir with raw logs under logs/; every assertion is expectation-encoded per arm (a base cell showing the old code-less shape is a PASSING control). Vitest gates ran from within each package per repo convention. Mutations were applied to scratch copies via scripted replace-with-assert, run, and restored; git status clean after every cycle. Evidence captures rendered by scripts/verify-capture.mjs.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-review-bot
left a comment
qwen-code-review-bot
left a comment
There was a problem hiding this comment.
LGTM. Reviewed exact head 54f9fbced23c8dd6ce9ea6c763865a50535ef5be.
The structured code field on SessionNotFoundError cleanly replaces the fragile English-text matching from #8864. All 7 closing-state throw sites in bridge.ts explicitly pass 'session_closing', the post-await re-check is correctly separated (byId.get !== existing vs isClosingOrAuthorizingClose), and error-response.ts serializes code in the 404 JSON body. The WebUI retry logic uses the structured code with a legacy fallback gated on daemon capability, and the Java client treats session_closing as idempotent success for DELETE. Tests cover the new serialization, session_not_found non-retry path, and Java idempotent close.
No blocking findings. The prior 4 rounds of automated review S-level findings have been addressed through subsequent commits, and both human reviewers (doudouOUC, yiliang114) confirmed no blocking issues on the latest head.
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
Independent Deep Review — PR #8884
Reviewed blind first (metadata + diff + full-file context, existing reviews deliberately withheld), then cross-validated against the 4 rounds of qwen-code-ci-bot findings and the two approvals.
Note on timing: this review was produced against head
54f9fbce, which the PR reached only via the final merge-resolution commits. The PR merged (8970856) while the review was in flight. Everything below still applies tomainand is filed as post-merge follow-up rather than as a merge gate.
Verdict
The core mechanism is sound and well-executed: SessionNotFoundError now carries a 'session_not_found' | 'session_closing' discriminator with a safe default, all 7 closing throw sites in bridge.ts opt in explicitly, sendBridgeError serializes it, and the Java SDK's destroySession idempotence is preserved. Replacing English-substring matching with a stable code is the right call and the docs were updated to match.
However — the change that landed is not the change that was reviewed. The allowLegacyMessage back-compat path in DaemonSessionProvider.tsx was introduced in the merge commit 85a8c36b, is present in no reviewed commit (15f09a04..c6f5da57) and in no main commit (86a474ba had the unconditional message matcher). It carries the one substantive defect below and has zero test coverage.
Findings: 1 Major, 3 Minor, 1 Nit (2 filed inline).
1. [Major] The allowLegacyMessage gate is unreachable for exactly the daemons it targets
packages/webui/src/daemon/session/DaemonSessionProvider.tsx:2769
isClosingSessionLoadError(
error,
!capabilities?.features.includes(CLIENT_IDENTITY_FEATURE),
)client_identity is a baseline, unconditional capability: packages/cli/src/serve/capabilities.ts:78 registers it as { since: 'v1' } and it is not listed in CONDITIONAL_SERVE_FEATURES, so getAdvertisedServeFeatures() emits it for every daemon. It was introduced by 4d9cbe49c (feat(serve): add daemon-stamped client identity, #4231) on 2026-05-17.
The daemons that actually need the legacy fallback are those built from PR #8864 (merged 2026-08-10) up to this PR: they emit '...retry after close completes' but no code. Every one of them advertises client_identity, so the flag evaluates to false and the message branch never executes.
Consequences:
- The branch is dead code in every realistic deployment, while reading as deliberate back-compat.
- Against an 8864-era daemon, a WebUI built from this commit loses the closing-session retry that worked before this PR — the
codeis absent and the message match is suppressed. That is a narrow but real regression window, not a graceful degradation. capabilitiesis always populated before this catch block runs (assigned at:1293, awaited during connect), so the?.short-circuit doesn't rescue the intended case either.- Even if the flag were
true, the fallback would still be incomplete:closeSessionImpl(bridge.ts:5861) throws'The session is already closing', which.endsWith('The session is closing; retry after close completes')does not match. Message matching was never a complete predicate — further reason not to keep a half-working version of it.
Options, roughly in order of preference:
- Drop the parameter, match
codeonly. The skew window is one day and the daemon/WebUI ship together; this is the Simplicity-First answer and matches what was actually reviewed atc6f5da57. - Match
code === 'session_closing' || message.endsWith(...)unconditionally — no capability probe. Costs one extra string compare on an error path and is correct for every daemon. - If a probe is genuinely wanted, gate on a real signal: add a
session_closing_codetag toSERVE_CAPABILITY_REGISTRY. Piggybacking on an unrelated three-month-old tag as a version proxy will silently rot again.
2. [Minor] The legacy branch has no test coverage, so CI cannot see finding #1
All four WebUI cases that exercise this predicate (DaemonSessionProvider.test.tsx:7409, :7570, :7585, :7647) put code: 'session_closing' in the body. Every one of them passes unchanged if the entire allowLegacyMessage branch is deleted — classic mutation-survivor. There is no case constructing { error: '...retry after close completes' } without code, which is the only input shape the branch exists to handle.
Test (ubuntu-latest, Node 22.x) is green at 54f9fbce, but that green says nothing about this code. Worth noting that Serve A/B and web-shell E2E Smoke were still pending at merge time.
3. [Minor] The two retry sites now disagree about daemon compatibility
DaemonSessionProvider.tsx:3501 — the cross-session transition pump — calls isClosingSessionLoadError(error) with the default allowLegacyMessage = false, i.e. code-only, no legacy tolerance whatsoever. :2769 (the SSE restore loop) tries to pass a flag. Two call sites detecting the same daemon condition now have divergent compatibility semantics, with no comment explaining the asymmetry. Whichever resolution is chosen for #1, these two should agree.
4. [Minor] ACP transports drop the structured code entirely
packages/cli/src/serve/acp-http/dispatch.ts:859
case 'SessionNotFoundError':
case 'InvalidSessionScopeError':
case 'WorkspaceMismatchError':
case 'InvalidClientIdError':
return { code: RPC.INVALID_PARAMS, message: errMsg(err) };No data — unlike its immediate neighbours SessionConflictError and SessionArchivingError, which both carry data.errorKind precisely so the client can recover the classification. jsonRpcErrorToHttpStatusWithData (packages/sdk-typescript/src/daemon/acpTransportUtils.ts:73) then maps -32602 → 400.
So over AcpHttpTransport / AcpWsTransport a closing session surfaces as HTTP 400 with no code, and isClosingSessionLoadError bails at its error.status !== 404 guard before it ever looks at the body. The feature is REST-only.
This is pre-existing, not introduced here — but it directly qualifies the PR's premise that a stable code keeps clients correct, and DaemonClient takes a pluggable transport. One-line fix in the spirit of the sibling cases:
case 'SessionNotFoundError':
return {
code: RPC.INVALID_PARAMS,
message: errMsg(err),
data: {
errorKind: (err as { code?: string }).code ?? 'session_not_found',
sessionId: (err as { sessionId?: unknown }).sessionId,
httpStatus: 404,
},
};5. [Nit] The protocol doc now over-promises the envelope
docs/developers/qwen-serve-protocol.md:67 presents code as part of the canonical SessionNotFoundError envelope. packages/cli/src/serve/routes/session.ts:3987 (handleSessionOrganizationUpdate) still returns { error, sessionId } with no code. The other three hand-written 404 sites (:2041, :2401, :2453) were updated, so this one is now the odd one out.
This is the gap the bot raised as R3-3 and you declined — reasonable at the time, but the doc edit converts an internal inconsistency into a documented-contract mismatch. Either add the field or scope the doc sentence to sendBridgeError-backed routes. (Note the test pin at multi-workspace-sessions.test.ts:4920 uses toEqual, so adding the field requires touching that assertion.)
Cross-Validation Matrix
| # | Finding | Status | Assessment |
|---|---|---|---|
| R1-1 | Java isCurrentSessionNotFound would flip closing-DELETE to a hard failure |
Overlap | Correctly identified as Critical; resolved in this PR by accepting session_closing, pinned by destroyAcceptsAlreadyClosingForCurrentSession. Verified the predicate is reachable only from the destroySession 404 path (:224) — no blast radius elsewhere. Agree. |
| R3-1 | restoreSession post-await re-check conflated "replaced" with "closing" |
Overlap | Fixed in afd18336; verified the split at bridge.ts:5118-5127. A replaced entry now yields a plain session_not_found and only a genuinely closing entry yields session_closing. Correct — retrying a replaced id would have been wrong. |
| R4-2 / R4-3 | Error-taxonomy doc row + bridge test class pin | Overlap | Both landed in c6f5da57. The toMatchObject({code:'session_closing'}) addition at bridge.test.ts:4489 is the right shape — it pins the code without loosening the existing toBeInstanceOf assertion. |
| R1-5 | Export route hardcodes session_not_found |
Overlap → I agree with your decline | session.ts:873 resolves from persisted storage where no live entry exists, so session_closing is not a reachable state. Hardcoding is correct, not an oversight. |
| R2-1 | Extract a factory for the closing error | Overlap → I agree with your decline | Seven call sites each already read unambiguously; a factory would add indirection for no invariant. Consistent with AGENTS.md Simplicity First. |
| R3-2 | isCurrentPermissionNotFound left asymmetric |
Overlap → I agree with your decline | Deliberate and correct: a closing session must not make a permission lookup resolve as idempotent success. The asymmetry encodes a real semantic difference. A short comment there would prevent the next reviewer re-raising it. |
| R2-3 | Test pinning the default code value |
Overlap → mild disagree | You declined as low-value and that is defensible, but the default is the one thing that keeps ~40 unmodified SessionNotFoundError call sites on the old contract. One assertion would make that a pinned invariant rather than an implicit one. Non-blocking. |
| R3-3 | Organization route 404 has no code |
Overlap → mild disagree, escalated | See Nit #5 — the protocol-doc edit in this PR changed the calculus after you declined it. |
| — | allowLegacyMessage gate unreachable (Major) |
Unique — mine | Landed in merge commit 85a8c36b, after every bot round. No reviewer saw it. |
| — | Legacy branch untested / mutation-survivor | Unique — mine | Same cause. |
| — | Two retry sites with divergent compat semantics | Unique — mine | :2769 vs :3501. |
| — | ACP transport loses the code | Unique — mine | Pre-existing gap, but it bounds the PR's stated guarantee. |
| ytahdn | Overlaps with #8896, land 8884 first | Unique — theirs | Verified: #8896's transactional switching landed in main and its retry site (:3501) is the one flagged in Minor #3. The ordering advice was right; the interaction between the two is what produced the asymmetry. |
| — | CONFLICTING mergeable state |
Overlap (author-acknowledged) | Resolved via 85a8c36b/1b9f37a9. The resolution itself is where the Major came in — worth a habit of re-requesting review after a non-trivial conflict resolution in shared client logic. |
Additional Audit Coverage (no findings)
- All 7 closing throw sites in
bridge.ts(:5103,:5122,:5625,:5864,:6274,:6542,:9439) verified to pass'session_closing'; no closing path left on the default. Message at:9439was correctly normalized to the canonical string. - All hand-written
res.status(404)bodies acrosspackages/cli/src/serveaudited forcodepresence — onlysession.ts:3987is missing it (Nit #5). - TypeScript SDK:
DaemonClient.closeSessiontreats 204 and any 404 as success, so it is unaffected by the code change in either direction. No parity gap. - Python SDK: no
session_not_foundstring coupling; unaffected. - Default-parameter safety: every other
new SessionNotFoundError(...)call site relies on the'session_not_found'default and none of them represent a closing session — the default is correctly chosen. - WebUI transport:
DaemonClientis constructed with the default REST transport (workspaceClientRef.current ?? new DaemonClient({baseUrl, token})), so the retry does work in the shipped configuration — which is why Minor #4 is scoped as a bound on the guarantee rather than a live break. - Java:
destroySession/detach404 paths traced; onlydestroySessionconsumes the widened predicate. - CI at
54f9fbce:Test (ubuntu-latest), Java 11/17/21 matrix, Desktop Shell, Real daemon E2E all green;Serve A/Bandweb-shell E2E Smokestill pending at merge.
Suggested follow-up
A small follow-up PR resolving #1 (preferably by deleting the branch) plus #3, with one test that feeds a code-less closing body, would close this out. #4 and #5 are independent and can be batched with other protocol cleanup.
This review was generated by QoderWork AI
| isClosingSessionLoadError(error) | ||
| isClosingSessionLoadError( | ||
| error, | ||
| !capabilities?.features.includes(CLIENT_IDENTITY_FEATURE), |
There was a problem hiding this comment.
[Major] This gate is false for every daemon that needs the fallback.
client_identity is a baseline capability — capabilities.ts:78 declares it { since: 'v1' } and it is absent from CONDITIONAL_SERVE_FEATURES, so getAdvertisedServeFeatures() returns it unconditionally. It shipped in 4d9cbe49c (#4231) on 2026-05-17.
The daemons this fallback exists for are the ones built between #8864 (merged 2026-08-10) and this PR: they emit the closing message but not the code. All of them advertise client_identity, so allowLegacyMessage resolves to false and the legacy branch never runs.
Two consequences: the branch is dead code that reads as intentional back-compat, and against an 8864-era daemon this WebUI actually loses a retry that worked before this PR (no code in the body, message match suppressed).
capabilities is already populated by the time this catch block runs — it's assigned at :1293 during connect — so the ?. short-circuit doesn't cover the intended case either.
Simplest correct fix is to drop the second argument and match on code alone; if back-compat is genuinely wanted, match code-or-message unconditionally, or introduce a real session_closing_code capability tag rather than reusing an unrelated three-month-old one as a version proxy.
This review was generated by QoderWork AI
| function isClosingSessionLoadError(error: unknown): boolean { | ||
| function isClosingSessionLoadError( | ||
| error: unknown, | ||
| allowLegacyMessage = false, |
There was a problem hiding this comment.
[Minor] This parameter is untested and was never reviewed.
allowLegacyMessage first appears in the merge commit 85a8c36b. It is in none of the reviewed commits (15f09a04..c6f5da57) and not in main either — 86a474ba had the unconditional message matcher. It entered as conflict-resolution logic and went straight to merge.
No test covers it. All four cases touching this predicate (DaemonSessionProvider.test.tsx:7409, :7570, :7585, :7647) supply code: 'session_closing', so every one of them still passes if this entire branch is deleted. There is no case with the message but no code — the only input shape the branch handles.
Separately, the other caller at :3501 (the cross-session transition pump from #8896) uses the default false, so the two retry paths for the same daemon condition now have different compatibility semantics with nothing explaining why.
Also worth noting the message predicate was never complete: closeSessionImpl (bridge.ts:5861) throws 'The session is already closing', which .endsWith('The session is closing; retry after close completes') does not match. That is an argument for removing the branch rather than fixing its gate.
This review was generated by QoderWork AI
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
|
Released in v0.21.10. |
6 participants
What this PR does
Adds an explicit structured error code for the daemon's session-closing state, updates WebUI retry handling to consume that code, and makes Java session close treat an already-closing session as an idempotent terminal result.
Why it's needed
The follow-up retry behavior from #8864 depended on matching English error text. A stable error code keeps clients correct when messages change and distinguishes an unknown session from a concurrent close without changing existing unknown-session behavior.
Reviewer Test Plan
How to verify
code: "session_closing"; confirm WebUI retries this response.code: "session_not_found"and WebUI does not retry it.session_closingresponse is accepted as successful completion.Evidence (Before & After)
N/A — protocol and client behavior only; no user-visible UI change.
Tested on
Environment (optional)
Focused WebUI and CLI Vitest suites passed on macOS. The Java regression test was added but not executed locally because no JRE/JDK is installed.
Risk & Scope
SessionNotFoundErrorcallers keep thesession_not_founddefault.Linked Issues
Follow-up to #8864.
中文说明
本 PR 做了什么
为守护进程的 session closing 状态增加显式结构化错误码，更新 WebUI 的重试判断以使用该错误码，并让 Java 客户端在关闭 session 时把“已经在关闭中”视为幂等终态。
为什么需要
#8864 的后续重试行为依赖匹配英文错误文本。稳定的错误码可在错误消息变化时保持客户端行为正确，并区分“session 不存在”和“并发关闭”，同时不改变原有的未知 session 行为。
Reviewer 测试计划
验证方式
code: "session_closing"，并确认 WebUI 会重试该响应。code: "session_not_found"，且 WebUI 不会重试。session_closing响应会被视为成功完成。前后证据
不适用——仅涉及协议和客户端行为，没有用户可见 UI 变化。
测试平台
环境
已在 macOS 通过聚焦的 WebUI 和 CLI Vitest 测试。已增加 Java 回归测试，但本地未安装 JRE/JDK，因此未执行。
风险与范围
SessionNotFoundError调用方继续使用session_not_found默认值。关联问题
#8864 的后续修复。