-
Notifications
You must be signed in to change notification settings - Fork 3k
feat(cli): adopt Goal v3 in ACP sessions #8732
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
Changes from all commits
09b3d07
b1689bb
e6a3735
b0f78ad
69a27b3
086406f
5de2229
feb90d7
6d25cbb
c5c90ff
8dcbe4b
985d0b1
d178b96
ea64631
e69b5d2
8218067
56d9d4f
347f30c
d1bb047
3c8c848
8c5dc82
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |||||
|
|
||||||
| import type { | ||||||
| ApprovalMode, | ||||||
| GoalSnapshotV2, | ||||||
| SessionGroupPresetColor, | ||||||
| } from '@qwen-code/qwen-code-core'; | ||||||
| import type { | ||||||
|
|
@@ -601,16 +602,14 @@ export interface BridgeSessionSummary { | |||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * A session's live `/goal` state, as reported by the `qwen --acp` child. | ||||||
| * | ||||||
| * Only the active goal crosses the bridge. The child also caches the most | ||||||
| * recent goal that ended on its own, but nothing on this side reads it, so it | ||||||
| * is not part of the wire shape — add it back alongside the first consumer. | ||||||
| * A session's live canonical Goal state, as reported by the `qwen --acp` | ||||||
| * child. `active` remains as a compatibility projection for existing hosts. | ||||||
| */ | ||||||
| export interface BridgeSessionGoal { | ||||||
| snapshot: GoalSnapshotV2; | ||||||
| active: { | ||||||
| condition: string; | ||||||
| /** Judge turns completed so far; 0 before the first stop-hook evaluation. */ | ||||||
| /** Canonical Goal turns completed so far. */ | ||||||
| iterations: number; | ||||||
| setAt: number; | ||||||
| /** The judge's verdict on the most recent turn, when it has run. */ | ||||||
|
|
@@ -1450,9 +1449,8 @@ export interface AcpSessionBridge { | |||||
| ): Promise<{ cleared: boolean; condition?: string }>; | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] R1-7: The clear-goal route now returns
Suggested change
(If the snapshot is deliberately not part of the clear contract, strip it from the handler response and the test instead.) 中文说明clear-goal 路由现在会在线上返回 — qwen3.8-max via Qwen Code /review (v0.21.7) |
||||||
|
|
||||||
| /** | ||||||
| * Read a live session's goal state. Throws `SessionNotFoundError` when the | ||||||
| * session is not resident — goals live in the child's memory, so a | ||||||
| * non-resident session has no goal to report. | ||||||
| * Read a live session's Goal state. Throws `SessionNotFoundError` when the | ||||||
| * session is not resident because this route addresses the selected runtime. | ||||||
| */ | ||||||
| getSessionGoal(sessionId: string): Promise<BridgeSessionGoal>; | ||||||
|
Comment on lines
1449
to
1455
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] R1-7 (still standing at this commit): the child's Suggested fix: either declare 中文说明R1-7(在本 commit 仍然存在):child 的 — qwen3.8-max via Qwen Code /review (v0.21.7) |
||||||
|
|
||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -155,6 +155,12 @@ const TRANSCRIPT_GOAL_STATUS_KINDS = new Set([ | |
| 'cleared', | ||
| 'failed', | ||
| 'aborted', | ||
| // A paused goal is not running, and dropping the card here is not neutral: | ||
| // the replay stream is what feeds the goal renderer, so the older `set` card | ||
| // stays newest and every surface keeps claiming autonomous work is under way. | ||
| // Kept in step with `GOAL_STATUS_KINDS`, which the daemon-side reader | ||
| // (`parseGoalStatusItem`) validates the same on-disk cards against. | ||
| 'paused', | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] R7-12: The legacy goal-card kind allowlist is maintained as four unlinked manual copies — cli 中文说明[Suggestion] R7-12:legacy Goal 卡片类型允许列表以四份互不关联的手工副本维护——cli — qwen3.8-max via Qwen Code /review (v0.21.9) |
||
| 'checking', | ||
| ]); | ||
|
|
||
|
|
||
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] R2-11: The new required
snapshotfield onBridgeSessionGoalis produced by everysessionGoalGetpath but read by no consumer — per the AGENTS.md rule, every added field's read sites were grepped, including outside the diff. The only reader (GET /goals) projectsactiveonly, so paused/blocked goals — a state this PR newly makes reachable in ACP — vanish from the Goals page. — Failure scenario: the sole consumer ofBridgeSessionGoalispackages/cli/src/serve/routes/goals.ts, which reads onlygoal.active.*and doesif (!goal.active) continue;; webui/web-shell consume the mappedGoalView, and the SDK/webui clear path reads only{cleared}— the field crosses the wire and is dropped. Concretely:/goal pauseis now valid in ACP mode, andsessionGoalGetprojectsactiveonly whensnapshot.goal?.status === 'active'— so a paused goal returnsactive: nulland is silently filtered out ofGET /goals. The user pauses a goal from chat and it disappears from the Web Shell Goals page, with no way to see or resume it from there.Suggested fix: either consume the new field (have
GET /goalsemit paused/blocked goals fromsnapshot, presumably why it was added to the bridge shape) or dropsnapshotfromBridgeSessionGoaluntil a reader exists — and decide explicitly whether paused goals belong on the Goals page.中文说明
BridgeSessionGoal新增的必需snapshot字段被每个sessionGoalGet路径产出,却没有任何 consumer 读取——按 AGENTS.md 规则已 grep 其所有读取点(含 diff 之外)。唯一读取方(GET /goals)只投影active,因此 paused/blocked goal——本 PR 新近让 ACP 可达的状态——会从 Goals 页面消失。失败场景:BridgeSessionGoal的唯一 consumer 是packages/cli/src/serve/routes/goals.ts,它只读goal.active.*并执行if (!goal.active) continue;;webui/web-shell 消费映射后的GoalView,SDK/webui 的 clear 路径只读{cleared}——字段过线即被丢弃。具体地:/goal pause现在在 ACP 模式有效,而sessionGoalGet只在snapshot.goal?.status === 'active'时投影active——于是 paused goal 返回active: null并被GET /goals静默过滤。用户从聊天暂停一个 Goal,它就从 Web Shell Goals 页面消失,且没有任何途径查看或恢复。修复建议:要么消费新字段(让GET /goals从snapshot发出 paused/blocked goal——这大概正是把它加入 bridge 形状的原因),要么在 reader 存在之前从BridgeSessionGoal移除snapshot——并明确决定 paused goal 是否属于 Goals 页面。— qwen3.8-max via Qwen Code /review (v0.21.7)