-
Notifications
You must be signed in to change notification settings - Fork 3k
fix(web-shell): label usage-limited goal cards correctly #10553
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
yu-xin-c
wants to merge
4
commits into
QwenLM:main
Choose a base branch
from
yu-xin-c:codex/web-shell-usage-limited-goal
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+191
−1
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
74a13d8
fix(web-shell): distinguish usage-limited goal cards
yu-xin-c 3e34f6a
test(web-shell): cover usage-limited goal visuals
yu-xin-c 3349893
fix: resolve merge conflicts with main
qwen-code-dev-bot ee70e01
Merge branch 'main' into codex/web-shell-usage-limited-goal
wenshao File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Suggestion] R1-1: The pass-through branches of this restore guard have no test witness: nothing pins that a legacy
kind: 'aborted'card staysabortedwhen no canonicalgoalStateis 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';inpackages/core/src/goals/goal-legacy-projection.ts), so if a future edit weakens the guard — dropping thegoal['status'] !== 'usage_limited'comparison, or keying the restore ongoalStatepresence alone — every blocked goal's transcript card silently re-labels to "Goal usage limited" and the suite stays green: everykind: 'aborted'fixture inpackages/web-shell/clienttoday is paired with canonicalstatus: 'usage_limited', and the only restore test asserts the positive direction.Witness:
Add a negative case beside
restores usage-limited semantics from canonical goal state metadatainDaemonSessionProvider.test.tsx: emit_meta.goalStatuswithkind: 'aborted'paired (a) withgoalState.goal.status: 'blocked'and (b) with nogoalStateat all, and assert the transcript block keepsdata.kind: 'aborted':The fixture must include a non-empty
condition—normalizeGoalStatusrejects any goal status without one (if (!condition) return null;,DaemonSessionProvider.tsx:4876), or the card silently disappears — and it must distinguish by canonicalgoalState.goal.status, becauseblockedgoals 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 withkind: '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)