fix(web-shell): label usage-limited goal cards correctly - #10553
fix(web-shell): label usage-limited goal cards correctly#10553yu-xin-c wants to merge 4 commits into
Conversation
|
⏳ Approval still deferred — 1 PR CI workflow run(s) still in progress for ⏳ 审批仍在延迟中 —— |
|
Thanks for the PR! Template looks good ✓
Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓
进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewNo blockers. Before reading the diff, my independent take on "transcript card says What I verified against the code on
Notes, not blockers: Testing evidenceUnattended CI run — I did not build or execute any PR code; the evidence below is the PR's own CI on the reviewed commit, fetched via the API at review time. The macOS/Windows test matrix jobs were skipped by the repo's PR classification profile (not failures); the ubuntu jobs carry the suite signal. Nothing red so far — three checks were still running at review time, and the table is updated by the finalize job once CI settles. CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 The author's reported local numbers (5,241 web-shell tests passing) are their claim, not independently re-run here — the CI suite above is the evidence. Sandboxed verification would settle what is still unproven: 中文说明代码审查无阻塞问题。在读 diff 之前，我对"历史消息卡显示 对照 main 上的代码核实：
非阻塞说明： 测试证据无人值守 CI 运行——未构建或执行任何 PR 代码；以上证据是审查时刻通过 API 获取的该提交自身 CI 结果。macOS/Windows 测试矩阵作业被仓库的 PR 分类配置跳过（并非失败）；ubuntu 作业承载测试信号。目前没有红色项——审查时还有三项在运行，表格会由收尾任务在 CI 结束后更新。作者自报的本地数字（5,241 项 web-shell 测试通过）是其声明，不作为证据。沙箱验证一行见英文部分：实时会话端到端渲染仍可通过维护者发起的赞助版 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean, minimal fix that lands a verified deferred finding exactly as prescribed; the only nit is that the referenced before/after screenshots never actually made it into the thread. Stepping back: the PR chose the right side of the fix. Reconciling on the Web Shell consumer keeps the legacy wire projection untouched for older clients, which is the conservative call — my independent proposal arrived at the same shape without seeing the diff. Every hunk earns its place (normalizer hook, render gate, title case, reason line, two i18n keys, two pinning tests); there is no drive-by churn, and The verdict is approve, but CI has not settled yet: the ubuntu unit suite, the no-AK integration run, and the web-shell visuals capture were still in flight at review time (two pending 中文说明回顾整体：PR 选择了正确的修复侧。在 Web Shell 消费端做协调，旧客户端看到的兼容 wire 投影保持不变，这是保守而正确的取舍——我在看 diff 之前独立得出的方案也是同样的形态。每一处改动都有必要（归一化钩子、渲染白名单、标题分支、原因行、两个 i18n 键、两个锁定行为的测试），没有顺手改动； 结论是批准，但 CI 尚未结束：审查时 ubuntu 单元测试、无 AK 集成测试和 web-shell 视觉采集仍在运行（两个待完成的 pull_request 工作流）。批准将推迟到所审提交全部变绿后执行——收尾任务会在所有检查变绿后提交绑定该提交的批准；如有检查变红或提交变动，则会改为标记提醒。 — Qwen Code · qwen3.8-max Reviewed at |
🖼️ 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 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent 1a": none — no check was cut short..
Test Plan (not a blocker): 241 tests pass — this review observed 5067, 481 passed.
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）："agent 1a"：none — no check was cut short.。
Test Plan（非阻断）：241 tests pass — this review observed 5067, 481 passed。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| if (status['kind'] !== 'aborted' || !isRecord(goalState)) return status; | ||
| const goal = goalState['goal']; | ||
| if (!isRecord(goal) || goal['status'] !== 'usage_limited') return status; |
There was a problem hiding this comment.
[Suggestion] R1-1: The pass-through branches of this restore guard have no test witness: nothing pins that a legacy kind: 'aborted' card stays aborted when no canonical goalState is co-emitted, or when the canonical status is not 'usage_limited'. Core's legacy projection maps 'blocked' goals onto this same legacy 'aborted' kind (case 'blocked': case 'usage_limited': return 'aborted'; in packages/core/src/goals/goal-legacy-projection.ts), so if a future edit weakens the guard — dropping the goal['status'] !== 'usage_limited' comparison, or keying the restore on goalState presence alone — every blocked goal's transcript card silently re-labels to "Goal usage limited" and the suite stays green: every kind: 'aborted' fixture in packages/web-shell/client today is paired with canonical status: 'usage_limited', and the only restore test asserts the positive direction.
Witness:
guard mutated (goal['status'] !== 'usage_limited' removed):
DaemonSessionProvider.test.tsx + SystemMessage.test.tsx — Tests 306 passed (306) ← suite stays green
negative probe added beside the restore test:
× keeps aborted label when canonical state is blocked
Expected: data ObjectContaining { "kind": "aborted" }
Received: data { "kind": "usage_limited", "condition": "finish the evaluation", ... }
Add a negative case beside restores usage-limited semantics from canonical goal state metadata in DaemonSessionProvider.test.tsx: emit _meta.goalStatus with kind: 'aborted' paired (a) with goalState.goal.status: 'blocked' and (b) with no goalState at all, and assert the transcript block keeps data.kind: 'aborted':
expect(blocks).toContainEqual(
expect.objectContaining({
kind: 'status',
source: 'goal',
data: expect.objectContaining({ kind: 'aborted' }),
}),
);The fixture must include a non-empty condition — normalizeGoalStatus rejects any goal status without one (if (!condition) return null;, DaemonSessionProvider.tsx:4876), or the card silently disappears — and it must distinguish by canonical goalState.goal.status, because blocked goals arrive on the wire as the identical legacy 'aborted' kind, the only differentiator available to the client.
The new test must go red if the guard is removed: delete goal['status'] !== 'usage_limited' from the condition and run it — the blocked case should fail with kind: 'usage_limited'.
中文说明
这个恢复逻辑的直通分支没有测试锁定：当没有同发的规范 goalState，或者规范状态不是 'usage_limited' 时，旧的 kind: 'aborted' 卡片应保持 aborted，但目前没有任何测试钉住这一点。core 的旧版投影会把 'blocked' 目标映射成同一个旧类型 'aborted'（packages/core/src/goals/goal-legacy-projection.ts 中的 case 'blocked': case 'usage_limited': return 'aborted';），因此如果未来有人放宽这个守卫——删掉 goal['status'] !== 'usage_limited' 比较，或改成只要存在 goalState 就恢复——所有被阻断目标的卡片都会被悄悄改标为 "Goal usage limited"，而整个测试套件依然全绿：目前 packages/web-shell/client 里所有 kind: 'aborted' 的测试数据都与规范状态 'usage_limited' 成对出现，唯一的恢复测试也只断言了正向转换。
证据（变异验证）：删掉守卫中的 goal['status'] !== 'usage_limited' 后，DaemonSessionProvider.test.tsx + SystemMessage.test.tsx 仍为 306/306 全部通过；在恢复测试旁加入负向探针测试后，blocked 场景立即失败：期望 kind: "aborted"，实际收到 kind: "usage_limited"。
建议在 DaemonSessionProvider.test.tsx 的新恢复测试旁补一个负向用例：_meta.goalStatus 为 kind: 'aborted'，分别（a）与 goalState.goal.status: 'blocked' 成对、（b）完全不带 goalState，断言历史消息块的 data.kind 保持 'aborted'。
约束：测试数据必须包含非空 condition——normalizeGoalStatus 会拒绝缺少 condition 的状态（if (!condition) return null;，DaemonSessionProvider.tsx:4876），否则卡片会静默消失；同时只能通过规范的 goalState.goal.status 来区分场景，因为 blocked 目标在传输层就是以相同的旧类型 'aborted' 到达的，这是客户端唯一可用的区分依据。
新测试必须在移除守卫时变红：删除条件中的 goal['status'] !== 'usage_limited' 后运行该测试，blocked 场景应以 kind: 'usage_limited' 失败。
— qwen3.8-max via Qwen Code /review (v0.22.3)
Verification report — head
|
|
@qwen-code /resolve |
|
Qwen Code resolved the merge conflicts and pushed the branch update. Root causeMain's
All goal-logic files auto-merged cleanly: main's goal rework (#10171) does not touch the goal-status normalization region, Textual onlyBoth sides added entirely new, independent blocks; no shared logic was modified by both. The resolution keeps both, PR block first: test(`usage-limited goal status`, async ({ page }, testInfo) => { ... });
test(`terminal turn error`, async ({ browser, page }, testInfo) => { ... });Verified the result is a strict union: diffing each resolved file against its stage-2 and stage-3 blobs shows zero dropped lines from either side (the only replaced line is the auto-merged import, which gains Load-bearing
Could not verifyNo build/tests were run (conflict resolution only; PR CI covers correctness). One note: main added 中文说明根因：main 上的 纯文本冲突：双方都是新增互不相关的测试块，没有修改同一逻辑。解决方式为两者都保留（PR 的块在前）。已逐一核对：解决结果相对双方暂存版本零行丢失；合并后相对 origin/main 的差异与原 PR 差异逐字节一致（6 个文件，+191/−1）。 关键点：两个冲突块共用标记后的闭合 未能验证：本次仅解决冲突，未运行构建或测试（由 PR 自身 CI 覆盖）。另注：main 在 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-1 restore guard pass-through branches lack a test witness — already reported (comment 3889623685)
Not explored to full depth (tool budget reached): "agent 3b": none — no check was cut short..
Test Plan (not a blocker): 241 tests pass — this review observed 5324, 503 passed.
中文说明
已审查。
本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未探索到全部深度（达到工具调用预算）："agent 3b"：none — no check was cut short.。
Test Plan（非阻断）：241 tests pass — this review observed 5324, 503 passed。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with |
5 participants
What this PR does
Web Shell now labels a Goal transcript card as
Goal usage limitedwhen the canonical V2 Goal state saysusage_limited, instead of displaying the legacy projection's broaderGoal abortedlabel. The daemon adapter reconciles only the specific pairgoalStatus.kind: abortedplusgoalState.goal.status: usage_limited; the legacy wire value remains unchanged for older clients. The message component accepts the restored kind, renders it with the existing warning treatment, keeps the last-check reason visible, and provides matching English and Chinese copy.Regression coverage pins both layers: daemon event normalization restores the canonical kind, and the rendered system message is distinct from the aborted label in English and Chinese.
Why it's needed
V2 Goal updates carry both a canonical
goalStatesnapshot and a compatibilitygoalStatuscard. The compatibility projection intentionally collapsesblockedandusage_limitedintoaborted, but Web Shell was rendering only that collapsed value. As a result, the transcript saidGoal abortedwhile the status strip correctly saidUsage limited, obscuring a recoverable resource boundary as a generic abort.Reviewer Test Plan
How to verify
agent_message_chunkwhose metadata containsgoalStatus.kind: abortedand a matching V2goalState.goal.status: usage_limited.data.kind: usage_limitedwhile the incoming compatibility card remains unchanged.Goal usage limited, preserves turns, duration, objective, and last-check reason, and does not sayGoal aborted.npm test --workspace=packages/web-shell -- --reporter=default: 233 files and 5,241 tests pass.npm run typecheck --workspace=packages/web-shellandnpm run lint --workspace=packages/web-shell: both pass.Evidence (Before & After)
A deterministic mock-daemon visual scenario now renders this exact paired status in dark and light themes. Both local Playwright captures pass and show
Goal usage limitedwith turns, duration, objective, and reason preserved; the repository visual workflow publishes the generated evidence on each push.Tested on
Environment (optional)
macOS, Node.js 22.22.2. The repository
npm ciprepare build, Web Shell build, typecheck, lint, changed-file formatting check, targeted tests (306/306), the dark/light Playwright visual scenario (2/2), and full Web Shell tests (5,241/5,241) passed.The package-wide formatting check still reports nine pre-existing files outside this diff; all six changed files pass Prettier.
Risk & Scope
abortedwhen the co-emitted canonical status is exactlyusage_limited.blockedprojects for legacy clients.Linked Issues
Addresses the first Web Shell transcript-card finding in #10046 without closing the multi-finding issue.
中文说明
这个 PR 做了什么
当规范的 V2 Goal 状态为
usage_limited时，Web Shell 现在会把 Goal 历史消息卡标记为Goal usage limited，不再显示旧兼容投影中更宽泛的Goal aborted。daemon 适配层只协调这一组特定组合：goalStatus.kind: aborted与goalState.goal.status: usage_limited；面向旧客户端的 wire 值保持不变。消息组件接受恢复后的类型，沿用现有警告色，继续显示上次检查原因，并提供对应的英文和中文文案。回归测试覆盖两个层次：daemon 事件归一化会恢复规范类型，渲染后的系统消息在英文和中文下都与 aborted 标签明确区分。
为什么需要
V2 Goal 更新同时携带规范的
goalState快照和兼容用的goalStatus卡片。兼容投影会有意把blocked和usage_limited都折叠为aborted，但 Web Shell 之前只渲染这个已经折叠的值。因此历史消息显示Goal aborted，状态条却正确显示Usage limited，把一个可以恢复的资源边界误写成了通用中止。Reviewer Test Plan
如何验证
agent_message_chunk，其 metadata 同时包含goalStatus.kind: aborted和匹配的 V2goalState.goal.status: usage_limited。data.kind: usage_limited，而输入的兼容卡片本身没有改变。Goal usage limited，保留轮次、时长、目标和上次检查原因，并且不再显示Goal aborted。npm test --workspace=packages/web-shell -- --reporter=default：233 个文件、5,241 条测试全部通过。npm run typecheck --workspace=packages/web-shell和npm run lint --workspace=packages/web-shell：均通过。证据（修改前后）
现在新增了一个确定性的 mock-daemon 视觉场景，在暗色和亮色主题下渲染这一精确的成对状态。本地两套 Playwright 截图均通过，显示
Goal usage limited，同时保留轮次、时长、目标和原因；仓库视觉工作流会在每次推送后发布生成的证据。测试平台
环境（可选）
macOS，Node.js 22.22.2。仓库
npm ci的 prepare 构建、Web Shell 构建、类型检查、lint、改动文件格式检查、针对性测试（306/306）、暗色/亮色 Playwright 视觉场景（2/2）和完整 Web Shell 测试（5,241/5,241）均通过。包级格式检查仍会报告本 diff 之外的九个主线既有文件；本次修改的六个文件全部通过 Prettier。
风险与范围
aborted且同一事件中的规范状态恰好为usage_limited时修改显示类型。blocked对旧客户端的投影方式。关联 Issue
处理 #10046 中第一条 Web Shell 历史消息卡发现，但不会关闭这个包含多项发现的 issue。