fix(web-shell): allow session refresh with daemon auth - #8445
Conversation
E2E Test ReportEnvironment: macOS, source daemon on loopback, built Web Shell assets, bearer authentication configured through Baseline
After the fix
An independent verification run reproduced the same result on a separate token daemon. Automated validation
The complete server test file was also run: 849 tests passed and two unrelated session-group tests failed with |
|
Re-run at the current head — gate summary, updated from the 2026-08-03 pass.
Moving on to code review. 🔍 中文说明在当前 head 上的重跑 —— 门禁摘要,基于 2026-08-03 那一轮更新。
进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
🩺 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
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline. Test Plan (not a blocker): 17 passed — this review observed 17016 passed.
中文说明
已审查——无阻断问题。 建议见行内评论。 Test Plan(非阻断):17 passed — this review observed 17016 passed。
— qwen3.8-max via Qwen Code /review (v0.21.4)
| app.get('/session/:id', (req: Request, res: Response, next: NextFunction) => { | ||
| if (!isDocumentNavigation(req)) return next(); | ||
| sendIndex(res); | ||
| }); |
There was a problem hiding this comment.
[Suggestion] This pre-auth route intercepts the only two request shapes that previously exercised mountWebShellSpaFallback's shell-serving branch: the existing tests falls back to the shell for SPA deep-link navigations (GET /session/abc123, Accept: text/html) and falls back to the shell on a sec-fetch navigation signal (GET /session/deep, Sec-Fetch-Mode: navigate) now match this route, so the fallback's positive branch (GET/HEAD document navigation to an unmatched non-/session path → 200 shell) has zero test coverage. Mutation-verified: with mountWebShellSpaFallback commented out at server.ts:2187, all 17 "Web Shell static serving" tests still pass. — Concrete cost: a future change that breaks or deletes the fallback's sendIndex(res) branch (guard reorder, refactor, or dropping the mount at server.ts:2187) would ship with the whole suite green, and document navigations to any deep path other than /session/:id (manually typed URLs today, any future client SPA route) would receive a 404 instead of the shell. Consider adding a direct positive test for the fallback, e.g.:
it('falls back to the shell for non-session SPA deep-link navigations', async () => {
const app = createServeApp(baseOpts, undefined, { webShellDir });
const res = await request(app)
.get('/deep/link')
.set('Host', host)
.set('Accept', 'text/html');
expect(res.status).toBe(200);
expect(res.text).toContain('<div id="root">');
});中文说明
这个免鉴权路由拦截了此前唯一会执行到 mountWebShellSpaFallback shell 服务分支的两种请求形态:现有测试 falls back to the shell for SPA deep-link navigations(GET /session/abc123,Accept: text/html)和 falls back to the shell on a sec-fetch navigation signal(GET /session/deep,Sec-Fetch-Mode: navigate)现在会命中本路由,因此 fallback 的正向分支(对未匹配的非 /session 路径发起 GET/HEAD 文档导航 → 返回 200 shell)失去了全部测试覆盖。已通过变异验证:将 server.ts:2187 处的 mountWebShellSpaFallback 注释掉后,全部 17 个 "Web Shell static serving" 测试仍然全部通过。— 具体代价:未来任何破坏或删除 fallback 中 sendIndex(res) 分支的改动(守卫顺序调整、重构、移除 server.ts:2187 处的挂载)都会在整个测试套件全绿的情况下合入,而对 /session/:id 之外任意深层路径的文档导航(目前是手动输入的 URL,未来可能是任何新的客户端 SPA 路由)将收到 404 而不是 shell。建议为 fallback 补充一个直接的正向测试,例如对未匹配的非会话路径发起带 Accept: text/html 的 GET /deep/link,并断言 200 + <div id="root">(示例代码见英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.4)
|
@qwen-code /takeover |
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。移除 |
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下: Address review summary — PR #8445 (round 1)Feedback points and decisions1.
|
|
🔀 Base updated: red check(s) [Test (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)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.21.3)
| it('falls back to the shell for non-session SPA deep-link navigations', async () => { | ||
| // The pre-auth `/session/:id` route claims the session deep-link shapes | ||
| // above, so this non-session navigation is the only direct coverage of | ||
| // mountWebShellSpaFallback's shell branch. |
There was a problem hiding this comment.
[Suggestion] The two pre-existing tests falls back to the shell for SPA deep-link navigations (~line 3023, GET /session/abc123, Accept: text/html) and falls back to the shell on a sec-fetch navigation signal (~line 3107, GET /session/deep, Accept: */*, Sec-Fetch-Mode: navigate) now resolve against the new pre-auth /session/:id route instead of mountWebShellSpaFallback, while keeping fallback-sounding names — probe-verified: both shapes are served with servedByFallback: false at this commit. The first is also now a near-duplicate of the first assertion of the token-gated test above (same path/accept shape, differing only in token config). — Concrete cost: a future change that breaks the fallback's navigation detection for non-session paths (e.g. narrowing isDocumentNavigation only inside the fallback, or a mount reorder) ships with all three fallback-named tests green; under a simulated regression of exactly that kind, a Sec-Fetch-Mode: navigate request to /deep/link returned 404 while the whole suite passed. The fallback's Sec-Fetch-Mode branch currently has no direct integration coverage. Consider retargeting the sec-fetch test to a non-session shape so it exercises the fallback again, and renaming the other test to describe the pre-auth session route it now covers:
it('falls back to the shell on a sec-fetch navigation signal', async () => {
const app = createServeApp(baseOpts, undefined, { webShellDir });
const res = await request(app)
.get('/deep/link')
.set('Host', host)
.set('Accept', '*/*')
.set('Sec-Fetch-Mode', 'navigate');
expect(res.status).toBe(200);
expect(res.text).toContain('<div id="root">');
});中文说明
两个既有测试 falls back to the shell for SPA deep-link navigations(约第 3023 行,GET /session/abc123、Accept: text/html)和 falls back to the shell on a sec-fetch navigation signal(约第 3107 行,GET /session/deep、Accept: */*、Sec-Fetch-Mode: navigate)现在命中的是新的免鉴权 /session/:id 路由,而不是 mountWebShellSpaFallback,但测试名仍在描述 fallback —— 已通过探针验证:在当前提交下,这两种请求形态都由免鉴权路由返回(servedByFallback: false)。前者现在也与上方带 token 新测试的第一个断言近乎重复(路径/Accept 形态相同,仅 token 配置不同)。— 具体代价:未来任何破坏 fallback 对非会话路径导航检测的改动(例如只在 fallback 内部收窄 isDocumentNavigation,或调整挂载顺序)都会在所有三个 fallback 命名的测试全绿的情况下合入;在模拟的同类回归下,对 /deep/link 发起 Sec-Fetch-Mode: navigate 请求返回 404,而整个测试套件全部通过。fallback 的 Sec-Fetch-Mode 分支目前没有直接的集成覆盖。建议把 sec-fetch 测试改为指向非会话形态,使其重新覆盖 fallback(示例代码见英文部分),并将另一个测试重命名为其所实际覆盖的免鉴权会话路由。
— qwen3.8-max via Qwen Code /review (v0.21.3)
…navigation Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 2/100 轮)。改动内容与我反驳保留之处如下: Round summaryFeedback: 1 inline finding from the automated reviewer (round 2, rc:3708748269 — [Suggestion] Fallback-named tests no longer exercise the SPA fallback — ADDRESSED (implemented)Verified against the code at HEAD before editing: Changes (all in
Commit: Verification
中文说明本轮摘要反馈: 自动审查器(第 2 轮)的 1 条行内发现( rc:3708748269 — [Suggestion] 名为 fallback 的测试实际上已不再覆盖 SPA fallback — 已处理(已实现)在编辑前已对照 HEAD 代码核实: 改动(全部位于
提交: 验证
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
There was a problem hiding this comment.
Reviewed. 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 qwen serve daemon suite did not run locally. Test Plan (not a blocker): 17 passed — this review observed 17060 passed.
中文说明
已审查。 1 条建议级发现无法锚定到改动行,已丢弃;此处无需进一步处理。 未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its qwen serve daemon suite did not run locally。 Test Plan(非阻断):17 passed — this review observed 17060 passed。
— qwen3.8-max via Qwen Code /review (v0.21.5)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: AutoFix review round — no action neededThis round had no actionable feedback for PR #8445 (branch Triage summary:
Outcome: no changes this round; the branch remains at 中文说明AutoFix 审查轮次 — 无需处理本轮针对 PR #8445(分支 分诊摘要:
结论: 本轮无改动;分支保持在 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
Code ReviewOverview. Adds an exact The approach is sound and minimal: exact single-segment matching means One issue below needs fixing before this lands; the rest are follow-ups/nits. 🔴 1. The deferred-runtime pre-auth mirror was not updated — the headline bug still reproduces
// Mirrors the routes `mountWebShellAssets` registers before `bearerAuth` in
// `createServeApp` (`GET /` and `GET /assets/*`). Only these skip the
// deferred-runtime auth gate; everything else keeps the bearer requirement.
function isPreAuthWebShellRequest(req: Request): boolean {
if (req.method !== 'GET' && req.method !== 'HEAD') return false;
return req.path === '/' || req.path.startsWith('/assets/');
}
Failure path with this PR applied:
So the scenario in the PR description ("refresh Suggested fix ( const SESSION_DEEP_LINK_PATH = /^\/session\/[^/]+\/?$/u;
function isPreAuthWebShellRequest(req: Request): boolean {
if (req.method !== 'GET' && req.method !== 'HEAD') return false;
if (req.path === '/' || req.path.startsWith('/assets/')) return true;
// Exact session document navigations are mounted pre-auth in the runtime
// app too; keep the doc-nav discriminator so API-shaped requests stay gated.
return SESSION_DEEP_LINK_PATH.test(req.path) && isDocumentNavigation(req);
}and extend the existing test
🟡 2. Three hand-maintained copies of the same route listThe pre-auth surface is now encoded in 🟡 3. Comments/docs that are now imprecise
🟢 4. Base-path deployments still 401
🟢 5. Pre-auth also means pre-rate-limit
Test nits
Verdict: direction and security reasoning are right; the exact-match + doc-nav discriminator is the correct way to avoid weakening the API gate. Blocking only on #1 — without it the fix doesn't hold for the default 中文摘要结论:方向正确,精确单段匹配 + 文档导航判别确实没有削弱 API 鉴权;但 #1 需要修复后才能合入。
|
…e window The deferred-runtime gate applied bearerAuth to every non-bootstrap request while the runtime was cold, so a browser refresh of /session/<id> (and / and /assets/*) 401'd on the default `qwen serve --token ...` start until something else warmed the runtime. Exempt the same surface mountWebShellAssets registers before auth, via a shared isPreAuthWebShellRequest predicate, so cold document navigations start the runtime and load the shell while JSON fetches, API subpaths, and --no-web daemons stay gated. The predicate is dynamically imported to respect the serve fast-path import-boundary guards.
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 3/100 轮)。改动内容与我反驳保留之处如下: Autofix review round summary — PR #8445Addressed the blocking finding (#1), both yellow findings (#2, #3), and all test nits from @wenshao's review. One committed fix round: Feedback points and dispositions🔴 #1 — deferred-runtime gate 401s the shell's entry points on a cold daemon: FIXED. The underlying defect is real and was verified in code: while the runtime is deferred (the default 🟡 #2 — three hand-maintained copies of the pre-auth route list: ADDRESSED. The pre-auth surface now has one canonical predicate: 🟡 #3 — imprecise comments/docs: ADDRESSED. (a) 🟢 #4 — base-path deployments behind path-preserving proxies still 401: RECORDED, no code change. Confirmed pre-existing and out of scope for this PR, as the review notes. The suggested mitigation is a line in the PR body's Risk & Scope section; this workflow does not edit PR bodies, so the limitation is recorded here instead: deployments fronting the daemon with a base-path-preserving reverse proxy (e.g. 🟢 #5 — pre-auth also means pre-rate-limit: DECLINED (accepted risk, no code change). Verified against the code: Test nits: ALL ADDRESSED.
Conflict notesNone — Verification
中文说明Autofix 评审轮次总结 — PR #8445已处理 @wenshao 评审中的阻塞项(#1)、两个黄色建议(#2、#3)以及全部测试建议。一轮提交完成修复: 各反馈点及处理结果🔴 #1 — 冷启动 deferred runtime 期间 shell 入口被 401:已修复。 底层缺陷真实存在且已在代码中核实:runtime 处于 deferred 状态时(即不带 🟡 #2 — pre-auth 路由清单存在三份手工副本:已处理。 pre-auth 表面现在有一个规范判定函数:从 🟡 #3 — 注释/文档表述不准确:已处理。 (a) 🟢 #4 — 路径保留型反代后的 base path 部署仍会 401:已记录,不改代码。 与评审所述一致,确认为既有限制、不在本 PR 范围内。评审建议在 PR 描述的 Risk & Scope 中加一行说明;本工作流不编辑 PR 描述,故在此记录该限制:以保留 base path 的反代(如 🟢 #5 — pre-auth 同时意味着绕过限流:拒绝修改(接受该风险,不改代码)。 已在代码中核实: 测试建议:全部处理。
冲突说明无 —— 验证
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/模型 |
|
🔀 Base updated: red check(s) [Test (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)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。 |
|
📊 Takeover milestone — round 10/100, in the current window. Census: 9 pushed fix(es), 1 no-change review(s), 0 timeout(s), 1 rejected attempt(s), 0 other round(s) (crash / model error / gate error / infra), 3 base update(s). This many rounds deserves a human look. Options: keep going (fine — nothing changes), split or reduce the PR if rounds keep accumulating, or release takeover (remove the 中文说明📊 接管里程碑 —— 第 10/100 轮(当前窗口)。统计:推送修复 9 次、审阅无需改动 1 次、超时 0 次、验证拒绝 1 次、其他轮次(崩溃/模型错误/门错误/infra)0 次、base 更新 3 次。 轮次到这个量值得人工看一眼。可选:继续(无需操作);若轮次持续累积,考虑拆分或缩减 PR;或释放接管(移除 |
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 bundled-CLI suite did not run locally.
中文说明
已审查。 建议见行内评论。 未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its bundled-CLI suite did not run locally。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| it('serves the shell for /session/:id document navigations (pre-auth route)', async () => { | ||
| const app = createServeApp(baseOpts, undefined, { webShellDir }); |
There was a problem hiding this comment.
[Suggestion] R10-1: This test renamed in this diff advertises (pre-auth route), but it builds the app from baseOpts, which has no token — and bearerAuth(undefined) is a pass-through (auth.ts:271-274), so the assertion passes regardless of where the route is mounted relative to auth; the test does not exercise the property its name claims. The tokened siblings added in the same hunk carry the real pre-auth assertion. — Failure scenario: if a future change moves app.get('/session/:id', ...) to after app.use(bearerAuth(opts.token)), this test stays green despite its title asserting pre-auth mounting, giving a maintainer relying on it a false sense of security.
| it('serves the shell for /session/:id document navigations (pre-auth route)', async () => { | |
| const app = createServeApp(baseOpts, undefined, { webShellDir }); | |
| it('serves the shell for /session/:id document navigations (pre-auth route)', async () => { | |
| const app = createServeApp({ ...baseOpts, token: 'secret' }, undefined, { webShellDir }); |
中文说明
[建议] 本次 diff 重命名的测试名为 (pre-auth route)(鉴权前路由),但它使用不含 token 的 baseOpts 构建应用——而 bearerAuth(undefined) 是直通中间件(auth.ts:271-274),因此无论该路由挂载在鉴权之前还是之后,断言都会通过;该测试并未验证其名称所声称的属性。同一 hunk 中新增的带 token 兄弟用例才是真正的 pre-auth 断言。失败场景:如果未来有改动将 app.get('/session/:id', ...) 移到 app.use(bearerAuth(opts.token)) 之后,此测试仍会保持绿色,尽管其标题声称验证了鉴权前挂载——依赖该测试的维护者将获得虚假的安全感。
— qwen3.8-max via Qwen Code /review (v0.21.5)
|
🤖 AutoFix ran out of time before finishing (timeout (1080000ms)) (attempt 11/100) — it will retry on the next scan. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31011714214 🧠 Handled by Qwen Code · model/模型 |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: Same-run verification repair — no code change requiredThe previous commit (
Repair performed this round (no source changes):
Since the committed artifact is already correct, there is nothing to commit: the fix is the restored build state, which lets the repair verification gate's settings-schema freshness check run the generator to completion (it runs on every path, including the no-op path). No review threads were in scope this round (Critical-only mode; the deferred non-Critical items remain open for human follow-up). Verification
中文说明同轮验证修复 — 无需改动代码上一个提交(
本轮执行的修复(不涉及源码改动):
由于已提交的产物本身就是正确的,没有可提交的内容:真正的修复是恢复后的构建状态,它使修复验证门禁的 settings-schema 新鲜度检查能够把生成器完整跑完(该检查在所有路径上都会执行,包括 noop 路径)。本轮没有可处理的评审条目(仅处理 Critical 模式;被延后的非 Critical 条目保持开放,留待人工跟进)。 验证
Deferred non-Critical feedbackCritical-only mode is active after 10 change-producing rounds. 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. (
中文说明完成 10 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈;这些条目保持开放,留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后;达到预算的作者(如有)在下方点名。(评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
Maintainer verification — real local stack (daemon + browser)I built and ran this branch end-to-end against a real Verdict: verified, no blocking findings. The reported bug reproduces on the merge-base and is gone on this branch; every other route stays behind Setup (click to expand)
Both daemons log 1. The user-visible bug: refreshing
|
control 32e2741 |
this PR 2a68383 |
|
|---|---|---|
| reload HTTP status | 401 application/json |
200 text/html |
| React root mounted after reload | no | yes |
| session transcript restored | no | yes |
2. The auth gate, probed directly
23 raw-HTTP probes against both warm daemons. No Authorization header unless the row says so.
Now served pre-auth (the fix):
| request | control | this PR |
|---|---|---|
GET /session/<id> Accept: text/html |
401 json | 200 html (shell) |
HEAD /session/<id> Accept: text/html |
401 json | 200 html |
GET /session/<id>/ trailing slash |
401 json | 200 html (shell) |
GET /Session/<id> case variant |
401 json | 200 html (shell) |
GET /session/<id>?ref=1 query string |
401 json | 200 html (shell) |
GET /session/<id> Sec-Fetch-Mode: navigate |
401 json | 200 html (shell) |
GET / and GET // |
200 html | 200 html (unchanged) |
Still gated — identical on both sides:
| request | control | this PR |
|---|---|---|
GET /session/<id> Accept: application/json |
401 | 401 |
GET /session/<id>/status Accept: text/html |
401 | 401 |
GET /session/<id>/transcript Accept: text/html |
401 | 401 |
GET /session/<id>/events Accept: text/html |
401 | 401 |
GET /session/<id>/a/b deeper subpath |
401 | 401 |
GET /capabilities Accept: text/html |
401 | 401 |
GET /daemon/status Accept: text/html |
401 | 401 |
GET /workspace/x/sessions Accept: text/html |
401 | 401 |
POST / DELETE /session/<id> Accept: text/html |
401 | 401 |
--no-web daemon, GET /session/<id> Accept: text/html |
401 | 401 |
GET /session/<id>/status with bearer |
200 | 200 |
GET /capabilities with bearer |
200 | 200 |
No response body on either side contains the bearer token.
3. The deferred-runtime (cold) window
This is the part of the PR that is hardest to reach by hand, so I probed it in a real daemon rather than only in tests: the fast path defers the runtime app, so the very first request after listen() is answered by the cold gate. I fire the request as soon as the port accepts, well inside the ~1 s window before the fallback start timer.
The daemon log is the load-bearing evidence here — on this branch the navigation itself starts the runtime (daemon workspace roots initialized at +120 ms, i.e. 880 ms before the 1000 ms fallback was due — the fallback timer fired line never appears in that run), and the request completes status=200. On the control the same navigation 401s and the runtime is never started at all. A cold Accept: application/json request still 401s and still does not start the runtime, and a --no-web daemon 401s both /session/<id> and /.
4. Build, tests, and whether the new tests actually bite
| check | result |
|---|---|
npm run build (full workspace) on both trees |
pass |
packages/cli → vitest run src/serve |
125 files, 3849 passed, 1 skipped |
server.test.ts + run-qwen-serve.test.ts + fast-path.test.ts |
1162 passed |
tsc -p packages/cli --noEmit |
clean |
eslint --max-warnings 0 on the 5 touched TS files |
clean |
prettier --check on the touched files + docs/users/qwen-serve.md |
clean |
npm run check:serve-fast-path-bundle |
Startup bundle closure checks passed. — confirms the dynamic import('./web-shell-static.js') really does keep the module out of the serve fast-path static closure |
I did not hit the two socket hang up failures mentioned in the PR description; the full src/serve run was green on this machine.
Mutation check (are the new tests real?). I cloned the PR tree, kept every test, and broke only the production code in two ways: (M1) delete the pre-auth app.get('/session/:id', …) route, (M2) make isPreAuthWebShellRequest always return false. Result: 10 tests fail, and they are precisely the new ones —
× createServeApp > Web Shell static serving > serves the shell for GET and HEAD /session/:id document navigations
× createServeApp > Web Shell static serving > serves the shell for /session/:id on a sec-fetch-only navigation signal
× runQwenServe runtime startup failures > serves Web Shell document navigations during the deferred runtime window
× runQwenServe runtime startup failures > serves the Web Shell root during the deferred runtime window
× runQwenServe runtime startup failures > serves the // root alias during the deferred window like the warm app
× runQwenServe runtime startup failures > serves Web Shell assets during the deferred runtime window
× runQwenServe runtime startup failures > answers bare /assets during the deferred window like the warm app
× runQwenServe runtime startup failures > serves trailing-slash and case-variant session deep links during the deferred window
× runQwenServe runtime startup failures > serves query-carrying session deep links during the deferred window
× runQwenServe runtime startup failures > answers pre-auth Web Shell navigations with the failure envelope when the deferred runtime fails
The 401-asserting guard tests survive both mutations, which is correct — they are regression guards, not coverage of the new branch.
5. One observation, non-blocking
Percent-encoded single-segment session paths also land on the new pre-auth route, because Express does not decode %2F when matching:
GET /session/..%2fcapabilities Accept: text/html -> 200 text/html (401 on control)
GET /session/<id>%2fstatus Accept: text/html -> 200 text/html (401 on control)
I checked whether this is an auth bypass, and it is not: both responses are byte-identical to GET / (sha256 564cd3b3…, the same public shell), no API route is reached, and no session data or token appears. So the effect is only that a token-protected daemon now returns the already-public shell on a few more URL shapes. Worth a sentence in the isPreAuthWebShellRequest doc comment if you want the invariant written down, but it doesn't change the security posture and I would not hold the merge on it.
Recommendation
LGTM for merge. The behaviour claimed in the description reproduces exactly, in a real daemon and a real browser, and the guardrails hold on every probe I could think of.
中文版本
维护者验证 —— 真实本地栈(daemon + 浏览器)
我在真实的 qwen serve daemon 和真实浏览器上端到端跑了这个分支,并与它的 merge-base 做同条件对照,用来确认修复生效、并检查鉴权是否被削弱。
结论:验证通过,无阻塞问题。 描述里的 bug 在 merge-base 上稳定复现,在本分支上消失;其余路由全部仍在 bearerAuth 之后;新增测试是有效的(非空洞)。
环境
| 主机 | macOS 25.6.0 (arm64),Node v24.18.1 |
| 被测 | 2a68383(agent/fix-web-shell-session-refresh-auth) |
| 对照 | 32e2741 —— 本 PR 与 main 的 merge-base |
| 工作树 | 两棵独立 git worktree,各自 npm install + 完整 npm run build,因此每个 daemon 提供的是它自己的 packages/web-shell/dist |
| daemon | node packages/cli/dist/index.js serve --hostname 127.0.0.1 --port <p> --token <t> --workspace <ws>,PR 在 :18441,对照在 :18442 |
| 隔离 | 每个 daemon 独立的 QWEN_HOME 与 workspace;模型 provider 指向本地 OpenAI 兼容 mock,不涉及真实凭据与网络 |
| 浏览器 | Playwright 驱动的真实 Chromium,1440×900 @2× |
两个 daemon 的日志都有 Web Shell UI served from …/packages/web-shell/dist,说明 shell 确实由被测 daemon 提供。
1. 用户可见的问题:刷新 /session/<id>
真实浏览器、真实流程:先在 /#token=… 打开 shell,发一条 prompt 让 URL 变成 /session/<id>,再执行一次普通的 location.reload() —— 也就是用户在地址栏做的刷新,无法附带 bearer 头。
对照 32e2741 |
本 PR 2a68383 |
|
|---|---|---|
| 刷新的 HTTP 状态 | 401 application/json |
200 text/html |
| 刷新后 React root 是否挂载 | 否 | 是 |
| 会话记录是否恢复 | 否 | 是 |
2. 直接探测鉴权门
对两个已热起的 daemon 做了 23 组原始 HTTP 探测,除非表格标注,否则都不带 Authorization 头。
新的免鉴权路径(即修复本身):
| 请求 | 对照 | 本 PR |
|---|---|---|
GET /session/<id> Accept: text/html |
401 json | 200 html(shell) |
HEAD /session/<id> Accept: text/html |
401 json | 200 html |
GET /session/<id>/ 尾斜杠 |
401 json | 200 html(shell) |
GET /Session/<id> 大小写变体 |
401 json | 200 html(shell) |
GET /session/<id>?ref=1 带 query |
401 json | 200 html(shell) |
GET /session/<id> Sec-Fetch-Mode: navigate |
401 json | 200 html(shell) |
GET / 与 GET // |
200 html | 200 html(无变化) |
仍然被拦截,两侧完全一致:
| 请求 | 对照 | 本 PR |
|---|---|---|
GET /session/<id> Accept: application/json |
401 | 401 |
GET /session/<id>/status Accept: text/html |
401 | 401 |
GET /session/<id>/transcript Accept: text/html |
401 | 401 |
GET /session/<id>/events Accept: text/html |
401 | 401 |
GET /session/<id>/a/b 更深子路径 |
401 | 401 |
GET /capabilities Accept: text/html |
401 | 401 |
GET /daemon/status Accept: text/html |
401 | 401 |
GET /workspace/x/sessions Accept: text/html |
401 | 401 |
POST / DELETE /session/<id> Accept: text/html |
401 | 401 |
--no-web daemon,GET /session/<id> Accept: text/html |
401 | 401 |
GET /session/<id>/status 带 bearer |
200 | 200 |
GET /capabilities 带 bearer |
200 | 200 |
两侧任何响应体中都不含 bearer token。
3. deferred runtime(冷启动)窗口
这是本 PR 里最难手工触达的部分,所以我没有只依赖单测,而是在真实 daemon 上探测:fast path 会延迟构建 runtime app,因此 listen() 之后的第一个请求由冷门(cold gate)处理。我在端口刚可连接时立刻发请求,落在 fallback 启动定时器(约 1 秒)之前。
判据主要看 daemon 日志:本分支上,是这次导航本身启动了 runtime(daemon workspace roots initialized 出现在 +120 ms,比 1000 ms 的 fallback 早 880 ms,该 run 的日志里根本没有 fallback timer fired 这一行),请求以 status=200 完成;对照分支上同样的导航返回 401,runtime 根本没有启动。冷启动窗口内的 Accept: application/json 请求仍然 401,且仍然不会启动 runtime;--no-web 的 daemon 对 /session/<id> 和 / 都返回 401。
4. 构建、测试,以及新增测试是否真的有效
| 检查项 | 结果 |
|---|---|
两棵树的 npm run build(完整 workspace) |
通过 |
packages/cli → vitest run src/serve |
125 个文件,3849 通过,1 跳过 |
server.test.ts + run-qwen-serve.test.ts + fast-path.test.ts |
1162 通过 |
tsc -p packages/cli --noEmit |
干净 |
对 5 个改动 TS 文件的 eslint --max-warnings 0 |
干净 |
改动文件 + docs/users/qwen-serve.md 的 prettier --check |
干净 |
npm run check:serve-fast-path-bundle |
Startup bundle closure checks passed. —— 证实动态 import('./web-shell-static.js') 确实把该模块挡在了 serve fast-path 的静态闭包之外 |
我没有复现出 PR 描述里提到的两个 socket hang up 失败;本机上完整的 src/serve 是全绿的。
变异测试(新增测试是不是空洞的?) 我克隆了 PR 树,保留全部测试,只破坏生产代码:(M1)删掉免鉴权的 app.get('/session/:id', …) 路由;(M2)让 isPreAuthWebShellRequest 恒返回 false。结果 10 个测试失败,且恰好都是本 PR 新增的那些(列表见英文部分)。断言 401 的守护型测试在两种变异下都仍然通过,这是正确的 —— 它们是回归护栏,不是新分支的覆盖。
5. 一处观察,不阻塞
百分号编码的单段会话路径也会落到新的免鉴权路由上,因为 Express 在路由匹配时不会解码 %2F:
GET /session/..%2fcapabilities Accept: text/html -> 200 text/html (对照为 401)
GET /session/<id>%2fstatus Accept: text/html -> 200 text/html (对照为 401)
我确认过这不是鉴权绕过:两个响应与 GET / 逐字节相同(sha256 564cd3b3…,同一个公开 shell),没有命中任何 API 路由,也没有泄露任何会话数据或 token。因此实际影响只是:配置了 token 的 daemon 现在会在更多 URL 形状上返回本来就公开的 shell。如果希望把这个不变式写进代码,可以在 isPreAuthWebShellRequest 的注释里加一句;但它不改变安全边界,我不建议因此阻塞合并。
建议
同意合并。描述中声称的行为在真实 daemon + 真实浏览器上完全复现,我能想到的各种探测下护栏也都成立。
|
🤖 Addressed the latest review feedback (round 12/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 12/100 轮)。改动内容与我反驳保留之处如下: Autofix review-response summary — PR #8445 (round: comment-only)Commit: Feedback triage
No reviews, no inline comments, no failed checks, no still-red checks, and no base-conflict ( What changed and whyOne sentence was added to the existing doc comment of Before writing it, the claim was verified against the code, not taken on trust: the warm app's Kept minimal per AGENTS.md: no test changes (a comment changes no behavior), no drive-by edits, nothing removed by this round. VerificationCommands actually run this round, on commit
中文说明Autofix 评审回应总结 — PR #8445(本轮:仅注释改动)提交: 反馈分诊
没有 review、没有行内评论、没有失败检查、没有持续红检查,也没有 base 冲突( 改动内容与理由在 写入之前,该说法是对照代码核实过的,而非照单全收:热路径的 按 AGENTS.md 保持最小改动:不改测试(注释不改变任何行为)、无顺手改动、本轮也没有删除任何内容。 验证本轮在提交
Deferred non-Critical feedbackCritical-only mode is active after 10 change-producing rounds. 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. ( 中文说明完成 10 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈;这些条目保持开放,留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后;达到预算的作者(如有)在下方点名。(评论 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/模型 |
|
🔀 Base updated: red check(s) [Test (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)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。 |
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 bundled-CLI qwen serve suite did not run locally.
中文说明
已审查。 建议见行内评论。 未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its bundled-CLI qwen serve suite did not run locally。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| const preAuthExempted = | ||
| authGate !== undefined && | ||
| (await Promise.resolve(options.isPreAuthRequest?.(req)).catch( | ||
| () => false, | ||
| )) === true; |
There was a problem hiding this comment.
[Suggestion] The fail-closed guard only catches a rejecting predicate: options.isPreAuthRequest?.(req) is evaluated as the argument to Promise.resolve(...), so a synchronously throwing predicate — a shape the declared type (req: Request) => boolean | Promise<boolean> admits — escapes .catch(() => false), unwinds dispatch(), and dispatch().catch(next) forwards it to Express's default error handler, 500ing every deferred request instead of degrading to the bearer gate as the adjacent comment promises. Today's sole production wiring (import('./web-shell-static.js').then(...)) cannot throw synchronously, so nothing breaks as shipped; the gap is between the guard's stated intent/type contract and what it covers. — Failure scenario: a future caller wires isPreAuthRequest as a sync predicate that throws on a malformed request → every request in the deferred startup window returns 500 instead of falling back to the bearer gate.
| const preAuthExempted = | |
| authGate !== undefined && | |
| (await Promise.resolve(options.isPreAuthRequest?.(req)).catch( | |
| () => false, | |
| )) === true; | |
| const preAuthExempted = | |
| authGate !== undefined && | |
| (await Promise.resolve() | |
| .then(() => options.isPreAuthRequest?.(req)) | |
| .catch(() => false)) === true; |
中文说明
[建议] fail-closed 防护目前只能捕获 reject 的谓词:options.isPreAuthRequest?.(req) 是作为 Promise.resolve(...) 的实参求值的,因此同步抛异常的谓词(声明类型 (req: Request) => boolean | Promise<boolean) 明确允许这种形态)会绕过 .catch(() => false),使 dispatch() 抛出,经 dispatch().catch(next) 交给 Express 默认错误处理器,导致 deferred 窗口的每个请求都返回 500,而不是像相邻注释承诺的那样降级到 bearer 鉴权。当前唯一的生产接线(import('./web-shell-static.js').then(...))不会同步抛异常,所以现网不受影响;缺口在于防护声明的意图/类型契约与实际覆盖范围不一致。失败场景:未来某个调用方把 isPreAuthRequest 接成一个遇到畸形请求就抛异常的同步谓词 → deferred 启动窗口内的所有请求返回 500,而不是回落到 bearer 鉴权。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| * that cannot attach the bearer header. Percent-encoded single-segment deep | ||
| * links (e.g. `/session/<id>%2fstatus`) also match — Express does not decode |
There was a problem hiding this comment.
[Suggestion] The doc comment declares percent-encoded single-segment deep links (/session/<id>%2f…) intentionally pre-auth-exempted, but no test in server.test.ts or run-qwen-serve.test.ts exercises any percent-encoded session path, so this documented cold/warm parity is unpinned — and %2F-encoded session IDs are a real client shape (sessionPath.ts builds the address-bar URL via encodeURIComponent(sessionId)). — Failure scenario: a future cleanup that decodes req.path inside isPreAuthWebShellRequest (e.g. decodeURIComponent(req.path)) or rewrites SESSION_DEEP_LINK_PATH to reject % → a cold daemon 401s a browser refresh of /session/parent%2Fchild while the warm runtime app still serves the shell — exactly the cold/warm divergence this PR exists to eliminate — shipping green, because a verification probe confirmed that mutation survives the current suite.
Suggested fix (two tests, one per layer):
// server.test.ts — warm app, tokened
const res = await request(app)
.get('/session/a%2Fb')
.set('Host', host)
.set('Accept', 'text/html');
expect(res.status).toBe(200);
expect(res.text).toContain('<div id="root">');
// run-qwen-serve.test.ts — deferred window: same request → 200 + shell,
// and the runtime starts中文说明
[建议] 文档注释声明百分号编码的单段深链接(/session/<id>%2f…)是有意豁免 pre-auth 的,但 server.test.ts 与 run-qwen-serve.test.ts 中没有任何测试覆盖百分号编码的会话路径,因此这一被文档化的冷/热一致性没有被钉住——而 %2F 编码的会话 ID 是真实的客户端形态(sessionPath.ts 通过 encodeURIComponent(sessionId) 构造地址栏 URL)。失败场景:未来某次 "清理" 在 isPreAuthWebShellRequest 内解码 req.path(如 decodeURIComponent(req.path)),或把 SESSION_DEEP_LINK_PATH 改成拒绝 % → 冷 daemon 会对 /session/parent%2Fchild 的浏览器刷新返回 401,而热的 runtime app 仍会提供 shell——这正是本 PR 要消除的冷/热分歧——并且会绿灯合入,因为验证探针确认该变异能在当前测试套件下存活。
— qwen3.8-max via Qwen Code /review (v0.21.6)
|
🤖 AutoFix ran out of time before finishing (timeout (7200000ms)) (attempt 13/100) — it will retry on the next scan. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31071603852 🧠 Handled by Qwen Code · model/模型 |
#8445) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 14/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 14/100 轮)。改动内容与我反驳保留之处如下: Autofix round summary — PR #8445 (address-review)Critical-only mode is active (round 11). The only actionable item this round was the failed required check; all non-Critical feedback stays deferred for human follow-up per the Critical-only rules. Feedback triage
DiagnosisThe failing
Root cause of that failure: the suite bound its daemon to the repository checkout ( Fix
A/B verification with the stray settings present: old code fails with the exact CI signature ( VerificationAll commands actually run this round, in order:
Not run: actionlint / shellcheck / yamllint (the PR and the fix touch no workflow, shell, or YAML files; the merged content passed main's CI with the same linter versions). The workflow's independent CI remains the final gate. 中文说明Autofix 本轮总结 — PR #8445(address-review)当前处于仅处理 Critical 的模式(第 11 轮)。本轮唯一可执行的条目是失败的必需检查;按照 Critical-only 规则,所有非 Critical 反馈保持延后,留待人工跟进。 反馈分诊
诊断失败的
该失败的根因:该套件把守护进程绑定到仓库检出目录( 修复
在残留 settings 存在的情况下做 A/B 验证:旧代码以与 CI 完全一致的签名失败( 验证本轮实际运行的全部命令(按顺序):
未运行:actionlint / shellcheck / yamllint(PR 与本次修复均未触碰 workflow、shell 或 YAML 文件;合并进来的内容已在 main 的 CI 中用相同 linter 版本通过)。工作流的独立 CI 仍是最终验证门禁。 Deferred non-Critical feedbackCritical-only mode is active after 10 change-producing rounds. 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. (
中文说明完成 10 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈;这些条目保持开放,留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后;达到预算的作者(如有)在下方点名。(评论 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
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| * `GET /` (or the startup-failure envelope). Keep in sync with the routes | ||
| * registered in `mountWebShellAssets`. | ||
| */ | ||
| export function isPreAuthWebShellRequest(req: Request): boolean { |
There was a problem hiding this comment.
[Suggestion] R12-1: The predicate's GET/HEAD method guard is exercised by no test — deleting if (req.method !== 'GET' && req.method !== 'HEAD') return false; survives the entire suite (mutation-verified: run-qwen-serve.test.ts stays 231/231 green, and a deferred-window probe flips between the mutated and clean trees). — Failure scenario: under that mutation an anonymous POST / (or POST /assets/x) during the deferred runtime window passes preAuthExempted through the navigation-less / and /assets arms, skips the bearer gate, and starts the daemon runtime before the warm app answers 401 — unauthenticated runtime startup that no existing test catches. Fix in run-qwen-serve.test.ts: extend keeps JSON and API-subpath requests gated during the deferred runtime window with a tokenless POST / probe:
const postRes = await fetch(`${handle.url}/`, { method: 'POST' });
expect(postRes.status).toBe(401);
expect(createBridge).not.toHaveBeenCalled();中文说明
[建议] R12-1:isPreAuthWebShellRequest 的 GET/HEAD 方法守卫没有任何测试覆盖——删除 if (req.method !== 'GET' && req.method !== 'HEAD') return false; 后整个套件依然全绿(已做变异验证:run-qwen-serve.test.ts 仍为 231/231 通过,deferred 窗口探针在变异前后行为翻转)。失败场景:该变异下,deferred runtime 窗口内的匿名 POST /(或 POST /assets/x)会经由不要求导航信号的 /、/assets 分支通过 preAuthExempted,跳过 bearer 鉴权并启动 daemon runtime,之后才被热 app 返回 401——现有测试均无法捕获这种未鉴权的 runtime 启动。修复:在 run-qwen-serve.test.ts 的 keeps JSON and API-subpath requests gated during the deferred runtime window 用例中补充无 token 的 POST / 探测(断言 401 且 createBridge 未被调用)。
— qwen3.8-max via Qwen Code /review (v0.21.6)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: Autofix review round — no action taken (PR #8445)This round found no actionable feedback, so no changes were made and no commit was created:
Critical-only mode is active after 10 change-producing rounds. The two items listed in the deferred non-Critical section (one review and one inline thread on The branch head is unchanged at 中文说明Autofix 审查轮次 — 未执行任何操作(PR #8445)本轮没有可处理的反馈,因此未做任何改动,也未创建提交:
在经历 10 个产生改动的轮次后,已进入仅处理 Critical 的模式。延后的非 Critical 部分中列出的两个条目(一个审查,以及 分支 HEAD 保持不变,仍为 Deferred non-Critical feedbackCritical-only mode is active after 10 change-producing rounds. 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. (
中文说明完成 10 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈;这些条目保持开放,留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后;达到预算的作者(如有)在下方点名。(评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
@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: 63 passed · 0 failed · 63 total 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:63 通过 · 0 失败 · 63 总计 Verification reportPR #8445 Deep Verification — fix(web-shell): allow session refresh with daemon authVerdict: 中文摘要
Central claim + A/BCentral claim: with a bearer token configured, an unauthenticated document navigation to the exact session deep link Secondary claims: (1) the deferred-runtime gate exempts exactly the warm app's pre-auth surface (root, Wire-oracle harness
7/7 navigation cells flip 401→200; 10/10 gated/pre-existing cells are byte-identical across arms. Both arms exited 0 → 17/17 each. Witnesses: CorrectionsNone (first round; no prior report to carry forward — FindingsF1 — Suggestion (completeness): the predicate's GET/HEAD method gate is not pinned by any testMutant M9 deleted No other findings: no bypass in the 29-cell ladder, no regression in any gated cell, and the Not covered
MethodologyEnvironment: the CI verify container ( Mutation matrix (guards introduced by this PR):
8/9 killed, 1 survivor classified; positive control (M0) proves the harness can turn the suite red. Evidence imagesHarness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
Code reviewIndependent baseline first: I'd have mounted an exact
No critical findings, no convention violations. The The cold-window flow, since it's the subtlest part of the change: sequenceDiagram
participant P1 as Browser navigation
participant P2 as Delegating app cold gate
participant P3 as isPreAuthWebShellRequest
participant P4 as Runtime app
P1->>P2: GET /session/id document nav
P2->>P3: exempt from bearer gate?
P3-->>P2: true (fails closed on error)
P2->>P4: start runtime and wait
alt runtime ready
P4-->>P1: 200 public shell html
else runtime failed
P2-->>P1: 503 daemon_runtime_failed
end
Testing evidence (this run is CI-path — no local execution of PR code)All 64 check-runs on the reviewed commit are complete; zero failures. The substantive lanes:
The ubuntu unit lane is the gating suite for this repo; the macOS/Windows unit lanes and the no-sandbox integration lane are skipped by repo gating (they were not red — just not run). All Live-behavior signal. The central claim is behavioural (refresh returns 200 + the session restores; everything else stays 401). What substantiates it at this point: the PR's own tests (mutation-checked by the maintainer) and the maintainer's real-stack A/B — real daemon + real browser against the merge-base, 23 raw-HTTP probes, cold-window probes — which @wenshao ran at Sandboxed verification is already closing the last gap: 中文说明代码审查先说独立方案:我会把精确的
无阻塞问题,无规范违规。 测试证据(本轮为 CI 路径——不本地执行 PR 代码)被审 commit 上 64 个 check-run 全部完成,零失败。实质通道:ubuntu 单测套件(本仓库的把关套件)、Serve A/B、web-shell E2E Smoke 均为 success;macOS/Windows 单测与无沙箱集成通道是被仓库门禁 skip(未运行,非变红)。所有 行为证据:核心主张是行为性的。目前的支撑是 PR 自带测试(经维护者变异测试核验)加上维护者的真实栈 A/B(真实 daemon + 真实浏览器对照 merge-base,23 组原始 HTTP 探测、冷窗口探测)——@wenshao 在 沙箱验证正在收口最后一环: — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 5/5 — observed bug with a linked reproduction, a minimal diff that matches the independent proposal, green CI on the reviewed commit, and a maintainer deep-verification plus approval standing on exactly this head. The previous run stopped at the gate on policy, not on defects: an auth-touching carve-out needs a maintainer's sign-off. That question has been answered thoroughly since — an explicit sign-off on the carve-out in review, and then a real-stack A/B (real daemon, real browser, 23 raw-HTTP probes, cold-window probes, mutation-checked tests) concluding "verified, no blocking findings", with an approval pinned to the commit this pass reviewed. Stepping back over the whole picture:
Approving, pinned to the reviewed commit. 中文说明置信度:5/5 —— 已观测的 bug 且有关联复现,diff 最小且与独立方案一致,被审 commit 上 CI 全绿,维护者做了深度验证并已在同一 head 上批准。 上一轮是出于政策(而非缺陷)在门禁止步:auth 相关的免鉴权例外需要维护者签字。此后该问题已被充分回答——评审中的明确签字,加上真实栈 A/B(真实 daemon、真实浏览器、23 组原始 HTTP 探测、冷窗口探测、变异测试核验)得出"验证通过、无阻塞问题",且批准就落在本轮审查的 commit 上。 整体回顾:问题真实存在(#8560 可具体复现,机制在基线代码中可见);实现与独立方案一致,并补足了我必然会要求的部分——deferred-runtime 冷门精确镜像热路径免鉴权表面、谓词 fail-closed、导航可达的启动失败 envelope;没有更简单的方案能覆盖冷 daemon,显见的替代方案(把 SPA fallback 移到鉴权前)在代码注释中已说明更差且未被采用。每个改动块都有必要——各轮评审带来的增长全部是响应性的,无顺手改动。测试有效:维护者变异测试在破坏修复时恰好失败十个新测试,401 守护测试如回归护栏应有的那样存活。该 head 上 CI 全绿;沙箱 予以批准,批准锚定在被审 commit。 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
Released in v0.21.7. |






What this PR does
Allows an exact Web Shell session document navigation to load the public HTML shell before bearer authentication, while keeping non-document requests and all session API subpaths behind the existing authentication gate.
Adds regression coverage for GET and HEAD document navigation, the same-path JSON request, and an authenticated session API subpath.
Why it's needed
When
qwen serveruns with a bearer token, refreshing/session/<id>currently returns401 Unauthorized. Browser address-bar navigation cannot add the bearer header, and the token stored in the URL fragment or session storage is only available after the Web Shell JavaScript loads. The root page works because it is served before authentication, but the session deep-link fallback previously ran after authentication and was never reached.This change restores refreshable session deep links without weakening API authentication.
Reviewer Test Plan
How to verify
/session/<id>, and refresh the page.Unauthorized./capabilitiesstill return 401.Evidence (Before & After)
Before: an unauthenticated document navigation to
/session/<id>returned401 application/jsonwith{"error":"Unauthorized"}.After: the same document navigation returns
200 text/html; the same-path JSON request, the session status API, and/capabilitiescontinue to return 401 without a bearer token.Tested on
Environment (optional)
Local macOS source daemon on loopback with
QWEN_SERVER_TOKEN, built Web Shell assets, and HTTP probes for document and API requests.Validation completed with the scoped Web Shell static-serving tests (17 passed), full build, full typecheck, ESLint, Prettier, and an independent token-daemon runtime verification. A full server test-file run also completed 849 tests; two unrelated session-group cases failed with
socket hang up, while the scoped group passed repeatedly.Risk & Scope
Linked Issues
Closes #8560
中文说明
本 PR 做了什么
允许精确的 Web Shell 会话文档导航在 bearer 鉴权前加载公开 HTML shell,同时保持非文档请求和所有会话 API 子路径继续经过现有鉴权。
新增回归覆盖,包括 GET 和 HEAD 文档导航、同路径 JSON 请求,以及需要鉴权的会话 API 子路径。
为什么需要
当
qwen serve配置 bearer token 后,刷新/session/<id>当前会返回401 Unauthorized。浏览器地址栏导航无法添加 bearer 请求头,而 URL fragment 或 session storage 中保存的 token 只有在 Web Shell JavaScript 加载后才能使用。根页面正常是因为它在鉴权前提供,但会话深链接 fallback 之前位于鉴权之后,因此永远无法到达。此变更恢复会话深链接刷新能力,同时不削弱 API 鉴权。
Reviewer Test Plan
如何验证
/session/<id>,然后刷新页面。Unauthorized。/capabilities发起未鉴权请求时仍返回 401。证据(修复前后)
修复前:未鉴权的
/session/<id>文档导航返回401 application/json,响应为{"error":"Unauthorized"}。修复后:相同文档导航返回
200 text/html;同路径 JSON 请求、会话状态 API 和/capabilities在没有 bearer token 时仍返回 401。测试系统
环境(可选)
macOS 本地源码 daemon,绑定 loopback,配置
QWEN_SERVER_TOKEN,使用已构建的 Web Shell 资源,并通过 HTTP 探测文档请求和 API 请求。验证包括 Web Shell 静态服务定向测试(17 项通过)、完整构建、完整类型检查、ESLint、Prettier,以及独立的 token daemon 运行时复验。完整 server 测试文件也运行了 849 项;两个无关的 session-group 用例因
socket hang up失败,而定向分组重复运行均通过。风险与范围
关联 Issue
Closes #8560