fix(webui): hydrate Goal independently of session metadata - #9977
Merged
Conversation
Collaborator
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: observed bug, verified against the base code. Direction: aligned — this repairs broken session-switch UX rather than adding surface. No external CHANGELOG signal applies; the area is internal to qwen-code's Goal gate. Size: not applicable — the change stays inside Approach: scope feels right — the minimal fix is exactly "take Goal out of the aggregate wait and publish it when its own read settles", which is what this does. The one non-obvious companion change, the relaxed guard on the final metadata merge, is needed: with Goal now publishing early, an immediate auth/terminal failure can clear the attachment before React flushes the merge, and the pre-existing "restores the workspace reasoning preview after an auth failure" test pins that metadata must still land. Replaced-attachment protection remains intact because a replaced attachment leaves Risk: no elevated risk signals — neither changed file matches the high-risk paths from the revert history. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到的 bug，并且我们在基线代码中核实过。web-shell 的 方向：对齐——修复会话切换时的 UX 故障，而不是新增功能面。外部 CHANGELOG 无直接参考信号；该区域属于 qwen-code 内部的 Goal 门控。 规模：不适用——改动只在 方案：范围合理——最小修复正是"把 Goal 从聚合等待中拿出来，在它自己的读取落定时发布"，本 PR 正是这么做的。唯一不显眼的配套改动是最终元数据合并的守卫放宽，这是必要的：Goal 提前发布后，立即发生的 auth/终端错误可能在 React 刷新该合并之前清除 attachment，而既有的 "restores the workspace reasoning preview after an auth failure" 测试钉住了这些元数据必须落地。替换 attachment 的防护仍然有效，因为替换后的 风险：无升级风险信号——两个改动文件均未命中 revert 历史中的高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
Code review — no blockers found. My independent take on this problem was exactly what the PR does: pull
sequenceDiagram
participant P1 as loadSession
participant P2 as goal() read
participant P3 as metadata fetches
participant P4 as connection state
P1->>P2: issue goal read
P1->>P3: providers, commands, context, git
P2-->>P4: publish early, reconcile vs live frames
P3-->>P4: final merge once all settle
Test evidence — the PR's own CI, read via the API (unattended run; PR code is never executed here). The ubuntu unit suite is still running on the reviewed commit; nothing is red so far. The author reports the Provider suite passing 244/244 locally — author's claim, not independently re-run. Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Not verified here: browser-level visual automation and live-daemon E2E (the PR says neither was run), and the web-shell gate behavior on Windows/Linux. Sandboxed verification would settle the remaining gap: 中文说明代码审查——未发现阻塞问题。我对这个问题的独立方案与 PR 完全一致：把
测试证据——无人值守运行，此处不执行 PR 代码，只通过 API 读取 PR 自己的 CI。被审提交上 ubuntu 单测套件仍在运行，目前没有红色检查。作者报告 Provider 套件本地 244/244 通过——这是作者自述，未经独立复跑。CI 表格见上方标记区域，完成后由 finalize 流程更新。 此处未验证：浏览器级可视化自动化与 live daemon E2E（PR 自述均未运行），以及 Windows/Linux 上 web-shell 门控行为。 沙箱验证可以补齐剩余缺口： — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
Confidence: 4/5 — a focused, correctly-scoped race fix whose trickiest corners are pinned by tests; approval deferred only because the unit suite is still running. Stepping back: this PR does one thing and does it the way I would have. The problem is real — I confirmed in the base code that the Goal gate fails closed on The reason this is 4 rather than 5: the ordering contract in this provider is subtle enough that I'd want the ubuntu unit suite green before anyone merges (it is still running), and nobody has exercised the gate in a real browser session — the regression tests carry the weight for now. If I were maintaining this in six months, the two-phase publication plus the comments explaining the tombstone stamping would read fine. Approval is deferred until CI lands green on the reviewed commit; nothing is red at the moment. 中文说明置信度：4/5 —— 一个范围正确、聚焦的竞态修复，最棘手的部分都有测试钉住；仅因单测套件仍在运行而暂缓批准。 退一步看：这个 PR 只做一件事，而且做法与我的独立方案一致。问题是真实存在的——我在基线代码中确认了 Goal 门控在 给 4 而不是 5 的原因：这个 provider 里的顺序契约足够微妙，我希望在任何人合并之前看到 ubuntu 单测套件变绿（它仍在运行），而且还没有人在真实浏览器会话中验证过该门控——目前由回归测试承担这一职责。 批准暂缓至被审提交上的 CI 全绿；目前没有红色检查。 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterFull-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
Contributor
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. |
qwen-code-ci-bot
approved these changes
Aug 25, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
chiga0
approved these changes
Aug 25, 2026
chiga0
left a comment
chiga0
left a comment
Collaborator
There was a problem hiding this comment.
No blocking findings.
Checked:
goalPromise.thenearly setConnection guard (!goalState && goalStateAtLoadStart !== undefined) correctly preserves a live-created goal against a stale bare-null load response.selectGoalStateFromReadinmappers.tsconfirms: bare-null with noclearedGoalis rejected whencurrent.goal.goalId !== observedGoalId. Clean.- Double write path: Path 1 (early goal publish) and Path 2 (final comprehensive update) both call
selectGoalStateFromRead(current.goalState, goalState, goalStateAtLoadStart?.goal?.goalId)with the samegoalStatevalue. Path 2 re-runs the reconciliation on whatever live frames have arrived by then — this is idempotent and correct. - Test change at line 1902: batching
applyGoalSnapshot+pendingGoal.resolvein the sameactcorrectly tests the "Goal update and stale bare-null read in the same React batch" scenario. Clean. - New session-switch test: covers the stated scenario (Goal hydrates before
providers/commands/contextsettle). Clean.
One question — inline below.
CI:
Test (ubuntu-latest, Node 22.x)✅,web-shell E2E Smoke✅,Desktop Shell(ubuntu + windows) ✅Test (macos-latest, Node 22.x)SKIPPED,Test (windows-latest, Node 22.x)SKIPPED — change is webui only; no path-dependent platform code observed.
Not covered: no local toolchain — rungs 1–3 not run.
Reviewed with AI assistance.
| if ( | ||
| sessionRef.current !== activeSession || | ||
| abort.signal.aborted || | ||
| (sessionRef.current !== undefined && |
Collaborator
There was a problem hiding this comment.
The guard here changed from sessionRef.current !== activeSession to (sessionRef.current !== undefined && sessionRef.current !== activeSession).
The new form allows the final setConnection to proceed when sessionRef.current is undefined — which happens during a state resync (handleStateResync at ~line 2547 sets sessionRef.current = undefined while keeping the same sessionId). In that window, the old guard would have blocked this update; the new guard does not, so the load's final commit can set status: \'connected\' and loadingTranscript: undefined over the resync's status: \'connecting\'.
Is this intentional? If the resync overlaps with a metadata load, the metadata from the original load should still be valid (same session), so proceeding seems reasonable. I want to confirm that the abort.signal.aborted check is sufficient to catch the cases where proceeding would be wrong.
Collaborator
|
Thanks for the detailed race analysis — the diagnosis of the Goal hydration gate reads as solid. One concern about placement, though: |
yiliang114
approved these changes
Aug 25, 2026
Collaborator
|
Released in v0.22.2. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
This change hydrates Goal ownership independently from the rest of session metadata and publishes the Goal snapshot as soon as its session-scoped request settles. Provider, supported-command, context, and Git metadata continue loading in parallel without keeping Slash commands behind their slowest request.
Goal snapshots still reconcile through the existing revision and cleared-goal ordering rules. Functional state updates preserve a Goal created or updated while the load-time read is in flight, and exact session attachment checks prevent a replaced attachment from publishing stale state.
Regression coverage now exercises a session switch where unrelated metadata remains pending, a Goal update and a stale bare-null read resolving in the same React batch, immediate metadata events, Goal request failures, streamed revisions, clear tombstones, and replaced same-ID attachments.
Why it's needed
The Slash-command gate deliberately fails closed whenever a real session has no hydrated Goal state, because
undefinedmeans the client does not yet know whether a Goal owns the session. During a session switch, however, the Goal request was grouped with unrelated metadata in one aggregate wait. A slow provider, command, context, orworkspace/gitrequest therefore left Goal state unknown even after the daemon had already returned{ goal: null, activity: "idle" }.That produced the misleading persistent error “Goal is occupying the session or state is still loading” for commands such as
/compress. Waiting or sending an ordinary chat message did not repair it because neither action changed the unresolved Goal hydration state. Goal is the only state that determines this gate, so its availability should not depend on unrelated workspace metadata.The independent update also has to remain ordered with live Goal changes. A load-time bare-null response can resolve in the same React batch as a newly created active Goal; publishing a value computed from an older connection reference would overwrite that Goal and incorrectly reopen the Slash gate. This change keeps both the early Goal publication and final metadata merge functional, so reconciliation always sees the newest queued state.
Reviewer Test Plan
How to verify
/compressis accepted before the delayed metadata completes.Evidence (Before & After)
Before: the deterministic session-switch regression observed
goalState === undefinedwhile the Goal request had already completed, and the same-batch concurrency regression observed a newly created active Goal being replaced by the stale idle/null read.After: the session-switch regression observes idle/null before unrelated metadata resolves, while the same-batch regression retains the active Goal. The complete Provider suite passes 244/244, Goal action/mapper tests pass 135/135, and Goal gate tests pass 10/10.
Tested on
Environment (optional)
Local Node.js workspace with Vitest, TypeScript, Vite production build, Prettier, and ESLint pre-commit checks.
Risk & Scope
Linked Issues
N/A
中文说明
本 PR 做了什么
本次修改将 Goal 所有权状态从其他会话 metadata 中独立加载，并在会话级 Goal 请求完成后立即发布快照。Provider、支持的命令、上下文和 Git metadata 仍会并行加载，但 Slash 命令不再等待其中最慢的请求。
Goal 快照仍通过现有 revision 与 cleared-goal 顺序规则合并。functional state update 会保留 load-time 读取期间创建或更新的 Goal，精确的会话 attachment 检查则会阻止已被替换的 attachment 发布过期状态。
回归测试覆盖：无关 metadata 持续 pending 时切换会话、Goal 更新与过期 bare-null 读取在同一个 React 批次完成、立即到达的 metadata 事件、Goal 请求失败、流式 revision、清空 tombstone，以及 same-ID attachment 替换。
为什么需要修改
Slash 命令门禁在真实会话尚未完成 Goal hydration 时会故意 fail closed，因为
undefined表示客户端还不知道该会话是否正被 Goal 占用。但在切换会话时，Goal 请求此前与无关 metadata 被放在同一个聚合等待中。只要 provider、命令、上下文或workspace/git中任一请求较慢，即使 daemon 已返回{ goal: null, activity: "idle" }，Goal 状态仍会保持未知。这会让
/compress等命令持续显示误导性的“Goal 正在占用会话或状态仍在加载”。继续等待或发送普通聊天消息都无法修复，因为这两个动作都不会改变尚未发布的 Goal hydration 状态。Goal 是该门禁唯一依赖的状态，因此它的可用性不应取决于无关的 workspace metadata。独立更新还必须与实时 Goal 变化保持正确顺序。load-time 的 bare-null 响应可能与新创建的 active Goal 在同一个 React 批次内完成；如果根据旧 connection ref 发布确定值，就会覆盖 active Goal 并错误地重新开放 Slash 门禁。本次修改让提前发布 Goal 和最终合并 metadata 都使用 functional update，从而保证 reconciliation 总能看到队列中的最新状态。
Reviewer 测试计划
如何验证
/compress已可执行。Before / After 证据
修改前：确定性会话切换回归测试在 Goal 请求已经完成时仍观察到
goalState === undefined；同批次并发回归测试则观察到新创建的 active Goal 被过期 idle/null 读取覆盖。修改后：会话切换测试在无关 metadata 完成前即可观察到 idle/null，同批次测试则保留 active Goal。Provider 完整测试 244/244 通过，Goal action/mapper 测试 135/135 通过，Goal gate 测试 10/10 通过。
测试平台
环境（可选）
本地 Node.js workspace，使用 Vitest、TypeScript、Vite production build、Prettier 和 ESLint pre-commit 检查。
风险与范围
关联 Issue
N/A