feat(web-shell): add a workspace Goals page, and stop losing /goal on daemon resume - #6561
Conversation
In daemon mode a `/goal` was silently lost whenever its session was reloaded or `qwen serve` restarted: the goal card vanished from the transcript and the Stop hook was never re-registered, so the loop simply stopped advancing. The TUI does neither of these things wrong; the ACP path was missing both halves. Goal cards were only ever emitted as live SSE `_meta` (MessageEmitter's emitGoalStatus / emitGoalTerminal) and never written to the transcript, so the one durable store — the ChatRecord JSONL — had nothing to restore from. Record them from Session.emitGoalStatus, the single choke point for `set` and `cleared` (the sessionGoalClear ext method routes through it too), and from the goal terminal observer for `achieved` / `failed` / `aborted`. Persisting `cleared` matters on its own: without it the last stored card stays `set`, and a later resume would revive a goal the user explicitly dropped. HistoryReplayer dropped those records on the way back out — it reads only `item['text']`, and a goal card has no `text` field — so re-emit them as `_meta.goalStatus`. Per-iteration `checking` cards are skipped: a TUI transcript stores one per stop-hook turn and clients suppress them as noise. That costs no fidelity, because restore reads the records directly rather than the replay output. With the transcript carrying the goal again, add #restoreGoalOnResume to loadSession and unstable_resumeSession, alongside #restoreWorktreeOnResume. It rebuilds the goal cards from the resumed ChatRecords (they live inside system/slash_command records' outputHistoryItems) and reuses the existing findGoalToRestore / findLastTerminalGoal / registerGoalHook logic, trust and hook-policy gates included.
`/goal` had no visual surface in the web shell. You could set and clear one from the composer, but the only feedback was a status-bar pill and a transcript card, and there was no way to see every goal running in the workspace at once. Add a full-pane Goals page alongside Scheduled Tasks. Each row shows the condition, the session driving it, whether the loop is mid-turn, the judge's turn count and last verdict, and how long the goal has been running. A row opens its session — the transcript IS the goal's history — or clears the goal. A form starts a new goal in a fresh session, so the loop doesn't take over a conversation already in progress. Reading the goals needs a round trip. They live in the owning `qwen --acp` child's in-memory store, and serve runs in a separate process holding only a bridge, so there is nothing local to read. Add a `sessionGoalGet` ext method that reports one session's goal state, wrap it in bridge.getSessionGoal (mirroring clearSessionGoal), and have `GET /goals` fan out over the workspace's live sessions concurrently — one timeout for a wedged child rather than one per session. A session whose probe rejects is dropped rather than failing the whole list. Clearing reuses `POST /session/:id/goal/clear`, so the page and a `/goal clear` typed in chat take the same path through the daemon. Only loaded sessions appear, which is the honest answer rather than a limitation: a goal advances only while its session is resident. Three entry points: a sidebar button, the status-bar goal pill (now a button), and a bare `/goal`, which opens the page instead of asking the daemon to print its status as text — matching how `/schedule` behaves. It sends no prompt and touches no session, so it works mid-turn too. `/goal <condition>` and `/goal clear` are unchanged. The integration test exercises the whole chain against a real daemon: `GET /goals` -> bridge -> ext method in a spawned `qwen --acp` child.
|
Thanks for the PR — this is a re-run on the same commit. Template: the body uses custom headings ( Problem: observed bug, well-documented. Goal cards were emitted only as live SSE Direction: aligned. Goals are a core Qwen Code feature ( Size: ~2,750 production lines, ~2,600 test lines. The PR is large — worth considering whether the bug fix (commit 1) and the Goals page (commit 2) could ship independently in the future. Both are reviewable independently as the author notes. Core paths ( Approach: well-reasoned. The resume fix correctly persists at the choke point ( Moving on to code review. 🔍 中文说明感谢贡献——这是对同一 commit 的重新审查。 模板:正文使用了自定义标题( 问题:已观测到的 bug,文档详尽。goal 卡片只作为 live SSE 方向:对齐。Goals 是 Qwen Code 的核心功能( 规模:约 2,750 行生产代码,约 2,600 行测试。PR 较大——未来值得考虑将 bug 修复(commit 1)和 Goals 页面(commit 2)独立提交。作者指出两者可独立 review。核心路径( 方案:设计合理。恢复修复正确地在收口点( 进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
Code ReviewIndependent proposal: given the problem (goal cards lost on daemon resume), I would have: (1) persisted goal cards at the Correctness: No bugs found. The code treats transcripts as untrusted data (correct — they're files on disk), validates every field before reading it, and uses Security: No concerns. The Conventions: Clean. ESM throughout, no Reuse: Good. No blockers found. TestingUnit Tests (856 tests, all passing)
Typecheck & LintReal-Scenario TestingThe Goals page requires (The The CI's web-shell visual preview and serve A/B bot results (both posted on this PR) provide additional coverage — the visual diff shows the Goals page rendering correctly in both dark and light themes, and the serve A/B found no response regressions against the base. 中文说明代码审查独立方案: 针对问题(daemon 恢复时 goal 卡片丢失),我会:(1) 在 正确性: 未发现 bug。代码将 transcript 视为不可信数据(正确——它们是磁盘文件),在读取每个字段前验证,诊断信息使用 安全: 无问题。 规范: 整洁。全程 ESM,无 复用: 良好。 未发现阻塞问题。 测试单元测试(856 个测试,全部通过)
类型检查与 Lint类型检查 ( 真实场景测试Goals 页面需要 — Qwen Code · qwen3.7-max Reviewed at |
|
Confidence: 4/5 — solid implementation that fixes a real bug and adds a useful feature; only nit is three cosmetic-only changes that don't belong in a feature PR. This PR does two things and does both well. The bug fix is the load-bearing half: goal cards were emitted as live SSE events and never persisted, so any daemon restart or session resume silently killed the goal loop. The fix persists at the correct choke point ( The Goals page is the natural companion. The fan-out architecture (daemon probes each child process via the bridge) is the only option given the process boundary, and the 856 tests pass across 8 suites, covering the persistence, replay, restore, route, dialog, and orchestration layers. Typecheck and lint are clean. The author caught and fixed their own export-path regression during the review rounds, which is the kind of self-review that builds confidence. The only nit: three formatting-only changes (a blank line in 中文说明信心度: 4/5 — 扎实的实现,修复了真实 bug 并添加了实用功能;唯一的瑕疵是三个不属于功能 PR 的纯格式化改动。 这个 PR 做了两件事,都做得很好。bug 修复是承重的一半:goal 卡片只作为 live SSE 事件发出,从未持久化,因此任何 daemon 重启或会话恢复都会静默杀死 goal 循环。修复在正确的收口点( Goals 页面是自然的配套。扇出架构(daemon 通过 bridge 探测每个子进程)是鉴于进程边界的唯一选择, 856 个测试跨 8 个套件全部通过,覆盖持久化、回放、恢复、路由、对话框和编排层。类型检查和 lint 均干净。作者在 review 轮次中自己发现并修复了导出路径回归,这种自我 review 增强了信心。 唯一的瑕疵:三个纯格式化改动( — Qwen Code · qwen3.7-max Reviewed at |
There was a problem hiding this comment.
Pull request overview
Adds durable /goal restoration in daemon/ACP resume paths and introduces a Web Shell workspace-wide Goals page backed by a new daemon route and ACP bridge ext-method for per-session goal state.
Changes:
- Persist goal status/terminal cards to transcript in ACP sessions and restore/re-register goal hooks on resume; update history replay to re-emit goal cards as
_meta.goalStatus. - Add daemon-side
GET /goalsfan-out route plus ACP bridgesessionGoalGetext-method to read live per-session goal state. - Add Web Shell Goals UI (sidebar entry, status-bar goal pill button, bare
/goalopens page) with polling + create/clear flows and tests.
Reviewed changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/webui/src/daemon/workspace/types.ts | Add DaemonGoal type and workspace actions for listing/clearing goals. |
| packages/webui/src/daemon/workspace/index.ts | Re-export DaemonGoal. |
| packages/webui/src/daemon/workspace/actions.ts | Implement REST-backed listGoals() and clearGoal() workspace actions. |
| packages/webui/src/daemon/index.ts | Re-export DaemonGoal at daemon package boundary. |
| packages/webui/src/daemon-react-sdk.ts | Re-export DaemonGoal in the React SDK types. |
| packages/web-shell/client/i18n.tsx | Add EN/ZH strings for Goals page and sidebar entry. |
| packages/web-shell/client/components/StatusBar.tsx | Make goal pill optionally clickable to open Goals page. |
| packages/web-shell/client/components/StatusBar.module.css | Style the new clickable goal pill button. |
| packages/web-shell/client/components/sidebar/WebShellSidebar.tsx | Add Goals icon/button entry point in sidebar. |
| packages/web-shell/client/components/sidebar/WebShellSidebar.test.tsx | Update sidebar test harness for new onOpenGoals prop. |
| packages/web-shell/client/components/dialogs/GoalsDialog.tsx | New Goals page UI: polling, list rendering, clear flow, new-goal form. |
| packages/web-shell/client/components/dialogs/GoalsDialog.test.tsx | Unit tests for GoalsDialog UI states and interactions. |
| packages/web-shell/client/components/dialogs/GoalsDialog.module.css | Styles for the GoalsDialog list/cards/form. |
| packages/web-shell/client/App.tsx | Add goals mainView, route bare /goal to Goals page, wire entry points and orchestration for “new goal in fresh session”. |
| packages/web-shell/client/App.test.tsx | Add coverage for bare /goal behavior and Goals page goal creation orchestration. |
| packages/cli/src/ui/utils/restoreGoal.ts | Add transcript parsing/collection helpers and stronger typing for restore paths. |
| packages/cli/src/ui/utils/restoreGoal.test.ts | Add tests for goal-card parsing and record collection behavior. |
| packages/cli/src/ui/types.ts | Add GOAL_STATUS_KINDS + isGoalStatusKind for validating persisted goal kinds. |
| packages/cli/src/serve/server.ts | Register the new goals routes in qwen serve. |
| packages/cli/src/serve/routes/goals.ts | New GET /goals route: concurrent per-session goal probes and projection/sorting. |
| packages/cli/src/serve/routes/goals.test.ts | Unit tests for /goals projection, sorting, partial failures, and 500 path. |
| packages/cli/src/acp-integration/session/Session.ts | Persist goal status + terminal cards to transcript in ACP sessions. |
| packages/cli/src/acp-integration/session/Session.test.ts | Tests ensuring ACP Session persists set/cleared/terminal goal cards. |
| packages/cli/src/acp-integration/session/HistoryReplayer.ts | Replay persisted goal cards as _meta.goalStatus and skip checking. |
| packages/cli/src/acp-integration/session/HistoryReplayer.test.ts | Tests for goal-card replay behavior and checking suppression. |
| packages/cli/src/acp-integration/acpAgent.ts | Restore goal hooks on resume; add sessionGoalGet ext-method implementation. |
| packages/cli/src/acp-integration/acpAgent.test.ts | Tests for goal restore on resume and new goal get ext-method behavior. |
| packages/acp-bridge/src/status.ts | Add sessionGoalGet to SERVE_CONTROL_EXT_METHODS. |
| packages/acp-bridge/src/bridgeTypes.ts | Define BridgeSessionGoal and add getSessionGoal() to the bridge interface. |
| packages/acp-bridge/src/bridge.ts | Implement getSessionGoal() bridge call via ext-method. |
| integration-tests/cli/qwen-serve-routes.test.ts | E2E coverage for GET /goals and auth requirement. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
wenshao
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.
— qwen3.7-max via Qwen Code /review
Suggestions — commit
|
| File | Issue | Suggested fix |
|---|---|---|
restoreGoal.ts:232-240 |
Trust/hook gate early returns in restoreGoalFromHistory are silent — no log when isTrustedFolder(), getDisableAllHooks(), or getHookSystem() blocks restoration. The condition-length check (line 244) correctly logs via writeStderrLine, but the two gates above it produce no trace. |
Add writeStderrLine to each gate, mirroring the length-check pattern. |
Session.ts:~1070 |
Comment says recording happens "here (rather than at each call site)" implying a single point, but #installGoalTerminalObserver also calls recordGoalStatusItem for terminal cards. The two paths are complementary (non-terminal vs terminal). |
Rewrite to state the partition explicitly: non-terminal cards recorded from slash commands, terminal cards recorded by the observer. |
restoreGoal.ts (recordGoalStatusItem) |
The catch branch (now writeStderrLine) is still untested. A regression test mocking getChatRecordingService to throw would protect this load-bearing diagnostic. |
Add a test in restoreGoal.test.ts that mocks the recording service to throw and asserts the stderr output. |
GoalsDialog.tsx:~120 |
1-second setInterval triggers full component re-render every tick. Only the elapsed-time <span> consumes the now state. |
Extract an ElapsedTime sub-component with its own timer so only timestamp text re-renders. |
goals.ts:~73 |
GET /goals fans out to every live session without pre-filtering. droppedCount helps the client, but each poll still probes all sessions. |
Add a lightweight hasGoal flag on BridgeSessionSummary so the route can skip sessions without goals. |
App.tsx:~2935 |
Bare /goal calls openGoals() unconditionally, ignoring sendToDaemon. Currently safe (all callers default true), but a future sendToDaemon: false caller would lose composer text. |
Gate on sendToDaemon or remove the parameter. |
App.tsx:~4882 |
onCreateGoal defined inline in JSX creates a new function reference each render, defeating GoalsDialog memoization. |
Wrap in useCallback with explicit deps. |
restoreGoal.ts:~88 |
goalTerminalEventToHistoryItem collapses lastReason ?? systemMessage — documented as "known lossy collapse" in tests, but systemMessage is silently dropped when both are present. |
Persist both fields when present, or add a comment in the source explaining the trade-off. |
— qwen3.7-max via Qwen Code /review
`GET /goals` fans out one ext-method probe per live session, and a wedged child holds it for the bridge's 10s `initTimeoutMs` — the same order as the 10s poll interval. `withActionTimeout` rejects the wait at 30s but never aborts the underlying fetch, so a fixed `setInterval` could stack several fan-outs against an already-struggling daemon. `reloadSeqRef` only keeps a stale response from overwriting state; it does nothing about the pile-up. Replace the interval with a single self-chaining loop that owns both the initial load and the polling, scheduling each fetch only once the previous one has settled. Folding the mount load into the chain matters: left in its own effect, the first timer would still fire while it was in flight. Reported by Copilot on QwenLM#6561.
Suggestions — commit
|
| File | Issue | Suggested fix |
|---|---|---|
packages/web-shell/client/App.tsx:3053 |
Dead code: the bare-/goal fallthrough in handleGoalSlashCommand is unreachable — the caller at line 3147 intercepts bare /goal before this function is invoked. A future maintainer reading the function in isolation would believe it owns bare-/goal routing. |
Remove the unreachable fallthrough block, or add a comment noting callers intercept bare /goal before reaching here. |
packages/cli/src/ui/utils/restoreGoal.ts:176 |
collectGoalStatusItemsFromRecords accesses record.type without a null-guard on record. A corrupted transcript entry that is null or a primitive throws TypeError, aborting the entire loop and skipping all valid goal cards after the bad entry. |
Add if (typeof record !== 'object' || record === null) continue; at the top of the loop. |
— qwen3.7-max via Qwen Code /review
…, theme vars From the /review suggestions on QwenLM#6561. Applied the ones that held up under verification; the rest are answered in the PR thread with evidence. - The New goal form accepted a clear keyword as a condition. It travels as `/goal <condition>`, so "clear" (or stop/off/reset/none/cancel) reached the daemon as a clear command: the fresh session dropped its own goal the instant it was set, with nothing to show for it. Reject it in the form. The keyword list and `/goal` arg parsing move to `utils/goalCondition.ts` so the page and App share one definition instead of the page reaching into App. - Starting a goal failed silently. `onCreateGoal` switches to the chat view first, which unmounts the Goals page, so the inline form error that `sendPrompt` rejection produced was dropped by the page's own unmount guard. Surface it as a toast instead. - `GoalsDialog.module.css` used `var(--destructive, #dc2626)`, but nothing defines `--destructive`; the hardcoded fallback stayed the same red in both themes. Use `--error-color` and match ScheduledTasksDialog's focus outline. - `recordGoalStatusItem` swallowed recording failures with a bare `catch {}`. Silently losing that write is precisely the failure this recording exists to prevent, so log it. - `GET /goals` dropped failed probes silently — an empty page and a page whose probes all failed look identical to the client. Log the dropped sessions and their reasons. Tests: clear-keyword and MAX_GOAL_LENGTH form validation, goalCondition unit tests, `sessionGoalGet` argument validation, session load surviving a throwing goal restore, `/goals` drop logging, and a regression test showing `/goal clear` sent as a prompt does persist its cleared card (a reviewer flagged this as missing; it is not).
|
Went through all 14. Applied 9 in Applied
Declined#2 — #4 — restore should emit a live status event. This would emit a duplicate. #5 — #6 — no per-call timeout on the fan-out. There already is one. That said, this suggestion is adjacent to a real bug Copilot caught separately: the 10s ceiling was the same order as the page's 10s poll interval, so polls could overlap. Fixed in #11 — 🤖 Reviewed and applied with Claude Code · Opus 4.8 (1M context) |
wenshao
left a comment
There was a problem hiding this comment.
Second round of review on QwenLM#6561. - `restoreGoalFromHistory` re-registered whatever condition the transcript held, skipping the 4000-char cap `/goal` enforces at set time. A transcript is a file: a corrupted or hand-edited `condition` would ride along in every judge call and continuation prompt for the rest of the session. Gate it alongside the existing trust and hook-policy gates. `MAX_GOAL_LENGTH` moves to `restoreGoal.ts` and `goalCommand.ts` imports it — the reverse direction would be a cycle, since goalCommand already depends on this module. - Starting a goal switched to the chat view before awaiting `sendPrompt`, which unmounted the Goals page. The previous commit routed the rejection to a toast, but the better fix is not to leave: switch views only once the prompt is admitted, so the error lands in the form the user is looking at. `GoalsDialog` keeps a toast fallback for the case where the page is closed while the prompt is still in flight. - Move the `debugLogger` declaration below the imports in `restoreGoal.ts`. Imports are hoisted so this compiled, but a statement wedged between two import blocks is not something to leave behind.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 34 out of 34 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
packages/cli/src/ui/utils/restoreGoal.ts:260
- On resume, restoreGoalFromHistory re-registers the hook via registerGoalHook, but that function sets the in-memory goal’s setAt to Date.now(). Since findGoalToRestore drops any persisted setAt, a resumed goal’s elapsed time (Goals page) and any later terminal durationMs (computed as Date.now() - goal.setAt) will be measured from the resume moment rather than from when the goal was originally set. The transcript already persists setAt on the initial 'set' card, so consider carrying it through restore and priming the active goal store with the persisted timestamp to keep elapsed/duration correct across daemon restarts.
registerGoalHook({
config,
sessionId,
condition: restorable.condition,
tokensAtStart: 0,
// Resume the iteration count so MAX_GOAL_ITERATIONS is a cross-resume cap,
// not a per-resume one.
initialIterations: restorable.iterations,
});
Third round of review on QwenLM#6561. - `debugLogger.warn` no-ops unless a debug session is active (`debugLogger.ts:216`), so a failed goal restore and a failed goal-card write were both invisible in production — the two failure modes this PR exists to fix. Promote them to `writeStderrLine`, which both `ui/App.tsx` and `session/Session.ts` already use. - `GET /goals` now returns `droppedCount`. A brownout in which every probe fails returned `{ goals: [] }`, indistinguishable from a workspace with no goals — so the user re-creates goals that are already running. The Goals page shows a notice when the list is incomplete. - `running` on the wire is really "the owning session is mid-turn", which a manual prompt in that session also sets. Renamed to `hasActivePrompt` so the field reports what the daemon actually knows. The UI still maps it to Working/Waiting. - Fix the stale "keep in sync" pointer in `goalCommand.ts`: the clear keywords moved from `App.tsx` to `utils/goalCondition.ts` in the previous commit. Tests for the four coverage gaps the review named: the `systemMessage` fallback in `goalTerminalEventToHistoryItem` (including the known lossy collapse when both fields are set), `#restoreGoalOnResume` on an empty transcript, `listGoals`/`clearGoal` in `actions.ts`, and the `sendPrompt`-after- `createNewSession` failure path (added last commit). Plus `droppedCount` projection and the degradation notice.
🖼️ 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 |
| setMainView('chat'); | ||
| loadSidebarSession(sessionId).catch( |
There was a problem hiding this comment.
[Suggestion] setMainView('chat') fires synchronously before loadSidebarSession resolves. If the session load fails, the user lands on the chat view with a stale or empty transcript and only a toast for recovery — no way back to the Goals page.
Concrete cost: a deleted or inaccessible session leaves the user disoriented in the wrong view.
| setMainView('chat'); | |
| loadSidebarSession(sessionId).catch( | |
| onOpenSession={(sessionId) => { | |
| // The goal's session transcript IS its history. | |
| loadSidebarSession(sessionId) | |
| .then(() => setMainView('chat')) | |
| .catch((error: unknown) => { | |
| reportError(error, 'Failed to open session'); | |
| }); | |
| }} |
— qwen3.7-max via Qwen Code /review
There was a problem hiding this comment.
Not taking this one — with reasons, since the failure you describe is real.
Two things make deferring the wrong trade here:
- The sibling does the same thing.
handleOpenSessionFromOverview— the Session Overview panel's equivalent — issetMainView('chat'); void loadSidebarSession(id).catch(reportError), the identical shape. Changing only the Goals handler would make two adjacent "open this session" affordances behave differently for no reason a user could infer. - The switch is the feedback.
loadSidebarSessionclears the transcript and shows a loading skeleton for the selected session — that's what tells the user their click registered. Deferring until the promise resolves means the common path (a session that loads fine) looks dead for the duration, and only then jumps.
So it trades a visible, correct success path for a tidier failure path that already reports itself via reportError. If we do want the deferred behaviour, it should change both handlers as a deliberate call — happy to do that as a follow-up if you'd rather have it; it just shouldn't land as a silent divergence in the Goals page alone.
🤖 Reviewed & applied with Claude Code · Opus 4.8 (1M context)
…page # Conflicts: # packages/cli/src/acp-integration/acpAgent.ts # packages/cli/src/acp-integration/session/Session.ts # packages/web-shell/client/components/sidebar/WebShellSidebar.workspace-removal.test.tsx
main's `createNewSession` gained a `setMainView('chat')` of its own, fired
synchronously before any await. That silently defeated the Goals handler's
deferred switch: by the time `sendPrompt` rejected, the page — and the form
that renders the error — was already gone, dropping the user into an empty
chat with no explanation. This is the exact failure the deferred switch was
written to prevent; the two changes only had to meet for it to come back.
`createNewSession` takes a `keepView` opt-out, and the Goals handler uses it,
so the page survives until the prompt is admitted. Saving and restoring
`mainView` around the call would also work but flips the view to chat and back,
which the user would see. A test pins the page staying mounted across a failed
submit; it fails if `keepView` stops being honoured.
Also from the same round:
- `registerGoalHook`'s `initialSetAt` guards are now tested — a future
timestamp, NaN, Infinity, 0 and a negative all fall back to now, and a usable
value survives. The future case is the one with teeth: `Date.now() - setAt`
renders a negative elapsed time rather than failing loudly, and nothing
covered it.
- The goals list carries `role="list"` / `role="listitem"`. They are divs, and
even a real `<ul>` loses its implicit role under `display: flex` in Safari.
- The open-session button names the action *and* the session. Its visible text
is only the session name, which says nothing about what activating it does;
the name stays in the accessible name so it still contains the visible label.
- `.fieldLabel` matches ScheduledTasksDialog's `--muted-foreground`. The two
dialogs sit side by side and had drifted.
Not taken: deferring `setMainView` in `onOpenSession` until the load resolves.
The sibling `handleOpenSessionFromOverview` switches first by the same pattern,
and `loadSidebarSession` clears the transcript and shows a loading skeleton —
which is the feedback for the common success path. Deferring would leave a
click looking dead until the load lands, and would make Goals diverge from the
Session Overview panel. If we want that behaviour it should change both.
🩺 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. Suggestions are inline. 1 Suggestion-level finding(s) could not be anchored to the diff; see the terminal output. Not reviewed: reverse audit — its prompt was built, but no agent was launched with it that opened its brief, so the reverse-audit pass did not run. Not reviewed: verification — the review posts findings, but no verifier ran (Step 4 builds its prompt with agent-prompt --role verify; none was recorded, so the findings were not verified).
— qwen3.7-max via Qwen Code /review
| registerGoalsRoutes(app, { | ||
| boundWorkspace: primaryBoundWorkspace, | ||
| bridge: primaryBridge, | ||
| }); |
There was a problem hiding this comment.
[Suggestion] GET /goals is registered only for the primary workspace via primaryBoundWorkspace and primaryBridge. Unlike scheduled tasks, which also registers registerWorkspaceQualifiedScheduledTasksRoutes (line 1564), there is no workspace-qualified counterpart for goals.
Failure scenario: in a multi-workspace Web Shell, the Goals page always queries the primary workspace's sessions. A user working in a secondary workspace sees goals from the primary workspace — or an empty list with no indication of the mismatch — rather than goals belonging to the workspace they are working in.
Concrete cost: if the daemon supports multiple workspaces, listGoals() in the workspace actions has no workspaceId parameter (unlike listScheduledTasks), so the client cannot scope the query even if a per-workspace route were added.
| registerGoalsRoutes(app, { | |
| boundWorkspace: primaryBoundWorkspace, | |
| bridge: primaryBridge, | |
| }); | |
| registerGoalsRoutes(app, { | |
| boundWorkspace: primaryBoundWorkspace, | |
| bridge: primaryBridge, | |
| }); | |
| registerWorkspaceQualifiedGoalsRoutes(app, { | |
| workspaceRegistry: primaryWorkspaceRegistry, | |
| bridge: primaryBridge, | |
| }); |
— qwen3.7-max via Qwen Code /review
There was a problem hiding this comment.
The gap is real and I've verified every part of it: registerGoalsRoutes is bound to primaryBoundWorkspace/primaryBridge only, registerWorkspaceQualifiedScheduledTasksRoutes does exist beside it (scheduled-tasks.ts:919), and listGoals() has no workspaceId where listScheduledTasks(workspaceId?) does (webui/src/daemon/workspace/types.ts:419 vs :438). In a multi-workspace shell the Goals page reads the primary's sessions regardless of where you are.
Deferring it to a follow-up rather than doing it here, deliberately:
The suggested patch can't be applied as written — registerWorkspaceQualifiedGoalsRoutes doesn't exist (and the registry variable is workspaceRegistry, not primaryWorkspaceRegistry). Writing it means doing for goals what scheduled tasks got in its own change: factor registerGoalsRoutes to take a prefix + resolveTarget (that's how registerScheduledTaskCrudRoutes supports both surfaces), add the qualified registration with its workspace resolve + trust check, add workspaceId? through listGoals in the SDK, fan the page out over workspaces in the UI, and test all four. Server-side alone would be dead code — the client has no way to call it.
That's a feature extension, not a defect in what this PR ships: multi-workspace landed on main after this branch opened, and the Goals page is scoped to the primary workspace exactly as the route comment says. It's also a change I'd want reviewed on its own rather than as a rider on a PR that's already been through six rounds.
Happy to open the follow-up — say the word and I'll do it as a separate PR against the same shape scheduled-tasks uses.
…emoved
The "Capture web-shell visuals" job fails on this PR at
`screenshots.spec.ts:395`, asserting the sidebar's "Primary" badge is visible:
Error: expect(locator).toBeVisible() failed
Error: element(s) not found
Not from this branch. The chain is on main:
- 2026-07-15 QwenLM#6880 adds the visuals spec, asserting the "Primary" badge —
correct at the time.
- 2026-07-17 QwenLM#7035 drops that badge as redundant (the workspace selector's
checkmark already conveys the default target), removing the `primaryLabel`
prop and its `<span className={styles.badge}>` render, and updates the *unit*
test to assert its absence — but leaves this spec asserting it is visible.
The capture job only runs on pull requests (it needs a PR head and a
merge-base), so main never went red for it and the breakage surfaces on the
next PR to merge main — this one.
Assert the badge's absence instead of deleting the check, mirroring the unit
test QwenLM#7035 added, so a regression re-adding it still fails here.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
…page # Conflicts: # packages/web-shell/client/e2e/visuals/screenshots.spec.ts # packages/webui/src/daemon/workspace/types.ts
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
— qwen3.7-max via Qwen Code /review
ytahdn
left a comment
There was a problem hiding this comment.
— qwen3.7-max via Qwen Code /review
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅




















Summary
Adds a workspace Goals page to the Web Shell, alongside the existing Scheduled Tasks page, so
/goalhas a visual surface instead of only a status-bar pill and a transcript card.Building it surfaced a prerequisite bug: in daemon mode a
/goalwas silently lost whenever its session was reloaded orqwen serverestarted. The first commit fixes that; the second builds the page on top. They are reviewable independently.The bug (commit 1)
Goal cards were emitted only as live SSE
_meta(MessageEmitter.emitGoalStatus/emitGoalTerminal) and never written to the transcript.recordGoalStatusItem— the function that would persist them — is called only from the TUI. And the ACP path never calledregisterGoalHookon resume, onlyunregisterGoalHook. So the one durable store, the ChatRecord JSONL, had nothing to restore from, and nothing tried to restore it.Net effect: after a
qwen serverestart or asession/resume, the goal card was gone from the transcript and the Stop hook was gone, so the loop stopped advancing without telling anyone. (A browser refresh while the session was still cached in the bridge'sbyIdmap appeared to work — that was in-memory event state, not persistence.)The fix:
Session.emitGoalStatus, which is the single choke point forsetandcleared(thesessionGoalClearext method routes through it too), and from the goal terminal observer forachieved/failed/aborted.clearedmatters on its own: without it the last stored card staysset, and a later resume would revive a goal the user explicitly dropped.HistoryReplayerto re-emit a persisted goal card as_meta.goalStatus. It previously read onlyitem['text'], and a goal card has notextfield, so it dropped them. Per-iterationcheckingcards are not replayed: a TUI transcript stores one per stop-hook turn and clients already suppress them as noise. No fidelity is lost, because restore reads the records directly rather than the replay output.#restoreGoalOnResumetoloadSessionandunstable_resumeSession, alongside the existing#restoreWorktreeOnResume. It rebuilds the goal cards from the resumedChatRecords (they live insidesystem/slash_commandrecords'outputHistoryItems) and reuses the existingfindGoalToRestore/findLastTerminalGoal/registerGoalHooklogic, trust and hook-policy gates included.The page (commit 2)
Each row shows the condition, the session driving it, whether the loop is mid-turn, the judge's turn count and last verdict, and elapsed time. A row opens its session — the transcript is the goal's history — or clears the goal. A form starts a new goal in a fresh session, so the loop doesn't take over a conversation already in progress.
Reading the goals needs a round trip. They live in the owning
qwen --acpchild's in-memory store, andqwen serveruns in a separate process holding only a bridge, so there is nothing local to read:qwen/control/session/goal/getext method reports one session's goal state.bridge.getSessionGoalwraps it, mirroringclearSessionGoal.GET /goalsfans out over the workspace's live sessions concurrently, so a wedged child costs one timeout rather than one per session. A session whose probe rejects is dropped rather than failing the whole list.POST /session/:id/goal/clear, so the page and a/goal cleartyped in chat take the same path through the daemon.Only loaded sessions appear. That's the honest answer rather than a limitation: a goal advances only while its session is resident.
Three entry points: a sidebar button, the status-bar goal pill (now a button), and a bare
/goal. Bare/goalnow opens the page instead of asking the daemon to print its status as text, matching how/schedulebehaves; it sends no prompt and touches no session, so it works mid-turn too./goal <condition>and/goal clearare unchanged.Screenshots
The Goals page, listing every goal running in the workspace. Each row shows the condition, the judge's last verdict, whether the loop is mid-turn, the turn count, elapsed time, and the session driving it.
Light theme:
"New goal" starts a goal in a fresh session, so the loop doesn't take over a conversation already in progress:
The status-bar pill is now a button, and is one of the three entry points (sidebar icon, bare
/goal, and this):These are real captures, not mockups: a live
qwen serveagainst a temporary trusted workspace, with two goals set through the normal/goal <condition>prompt path and a stub model standing in for the judge, driven headlessly with Playwright.Known gap
The daemon does not persist per-iteration
checkingcards (the client deliberately suppresses them to avoid one card per stop-hook turn). So on resume the iteration count restores from the lastsetcard, i.e. back to 0, which resetsMAX_GOAL_ITERATIONSas a cross-resume cap. The TUI does not have this problem because it persistschecking. Fixing it properly means persisting a lightweight iteration counter; left out of this PR to keep the scope contained.Reviewer Test Plan
How to verify
qwen serve, open the Web Shell. Click the target icon in the sidebar, or type a bare/goal, or click the goal pill in the status bar — all three open the Goals page.qwen serve(or reload the session). Before this PR the goal card is gone and the loop is dead. After it, the card is back in the transcript, the status pill returns, and the goal keeps working./goal <condition>and/goal clearfrom the composer behave exactly as before./goal clearstill works mid-turn.Automated
packages/cli:restoreGoal.test.ts(record parsing + restore),HistoryReplayer.test.ts(goal-card replay,checkingskipped),Session.test.ts(set/cleared/ terminal cards persisted),acpAgent.test.ts(sessionGoalGet; resume re-registers, does not revive an achieved goal, respects hook policy),serve/routes/goals.test.ts(projection, sorting, dropped probes, 500 path).packages/web-shell:GoalsDialog.test.tsx, plusApp.test.tsxcoverage for the bare-/goalbehaviour and goal creation in a fresh session.integration-tests/cli/qwen-serve-routes.test.ts:GET /goalsend to end against a real daemon — serve → bridge →sessionGoalGetin a spawnedqwen --acpchild.Before/After
/goalis composer-only in the Web Shell; there is no way to see the goals running across the workspace; and a goal is silently dropped on session reload or daemon restart.Relationship to #6535
Both touch
packages/acp-bridge/src/status.ts. This PR addssessionGoalGetnext tosessionGoalClearrather than appending to the end ofSERVE_CONTROL_EXT_METHODS, specifically so it does not collide with #6535'screateSubSession. The remaining overlaps (i18n.tsx,App.tsx,webui/.../workspace/types.ts) are in different regions of the same files and should merge cleanly.中文说明
概述
在 Web Shell 中新增工作区级的 目标(Goals) 页面,与现有的定时任务页面并列,让
/goal有一个可视化的管理界面,而不是只有状态栏的 pill 和 transcript 里的卡片。在实现过程中暴露出一个前置 bug:daemon 模式下,只要会话被重新加载或
qwen serve重启,/goal就会被静默丢弃。 第一个 commit 修复它,第二个 commit 在其之上构建页面。两者可以独立 review。这个 bug(commit 1)
goal 卡片只作为 live SSE 的
_meta发出(MessageEmitter.emitGoalStatus/emitGoalTerminal),从不写入 transcript。真正会持久化它们的recordGoalStatusItem只有 TUI 在调用。而 ACP 路径在 resume 时从不调用registerGoalHook,只调用unregisterGoalHook。于是唯一的持久化存储 —— ChatRecord JSONL —— 里没有任何可恢复的内容,也没有任何代码去恢复它。最终结果:
qwen serve重启或session/resume之后,goal 卡片从 transcript 中消失,同时 Stop hook 也消失了,循环就此停止推进,且不会给出任何提示。(如果只是刷新浏览器、而 daemon 里的会话还缓存在 bridge 的byId中,看起来是正常的 —— 那是内存中的事件状态,不是持久化。)修复方式:
Session.emitGoalStatus中持久化 goal 卡片。它是set和cleared的唯一收口(sessionGoalClear扩展方法也经过这里);terminal 观察者负责持久化achieved/failed/aborted。cleared本身就很关键:否则最后存下的卡片仍是set,之后的 resume 会复活一个用户已经明确清除的目标。HistoryReplayer把持久化的 goal 卡片重新以_meta.goalStatus发出。它之前只读item['text'],而 goal 卡片没有text字段,因此被直接丢弃。逐轮的checking卡片不会被 replay:TUI 的 transcript 每个 stop-hook 轮次都会存一张,客户端本来就把它们当作噪音抑制掉了。这不会损失任何保真度,因为恢复逻辑直接读取记录,而非 replay 的输出。#restoreGoalOnResume,在loadSession和unstable_resumeSession中调用,与已有的#restoreWorktreeOnResume并列。它从 resume 的ChatRecord中重建 goal 卡片(它们存放在system/slash_command记录的outputHistoryItems里),并复用已有的findGoalToRestore/findLastTerminalGoal/registerGoalHook逻辑,包括信任检查和 hook 策略门禁。这个页面(commit 2)
每一行展示目标条件、驱动它的会话、循环是否正在执行、判定轮次与上次判定原因,以及已运行时长。点击行可以打开对应会话 —— 该会话的 transcript 就是 这个目标的历史 —— 或者就地清除目标。表单会在一个全新的会话中启动新目标,这样循环不会接管用户正在进行的对话。
读取目标需要一次跨进程往返。它们存在于所属
qwen --acp子进程的内存中,而qwen serve运行在另一个进程里,只持有一个 bridge,本地无从读取:qwen/control/session/goal/get扩展方法,返回单个会话的目标状态。bridge.getSessionGoal对其封装,镜像clearSessionGoal的写法。GET /goals并发扇出到工作区内所有 live 会话,因此一个卡死的子进程只花费一次超时,而不是每个会话一次。探测失败的会话会被丢弃,而不是让整个列表失败。POST /session/:id/goal/clear,所以页面上的清除和在聊天里输入/goal clear走的是 daemon 中的同一条路径。只有已加载的会话会出现在列表中。这不是限制,而是诚实的答案:目标只在其会话驻留时才会推进。
三个入口:侧边栏按钮、状态栏的 goal pill(现在是一个按钮),以及无参的
/goal。无参/goal现在打开页面,而不是让 daemon 以文本形式打印状态,这与/schedule的行为一致;它不发送 prompt、也不触碰会话,因此在回合运行中也可以使用。/goal <条件>和/goal clear的行为完全不变。截图
目标页面,列出工作区内所有正在运行的目标。每行显示条件、判定器的上次结论、循环是否正在执行、轮次、已运行时长,以及驱动它的会话。
浅色主题:
"新建目标"会在一个全新会话中启动目标,因此循环不会接管用户正在进行的对话:
状态栏的 goal pill 现在是一个按钮,也是三个入口之一(侧边栏图标、无参
/goal、以及它):以上均为真实截图,而非设计稿:运行一个真实的
qwen serve,工作区为临时的受信任目录,两个目标通过正常的/goal <条件>prompt 路径设置,判定器由一个 stub 模型承担,最后用 Playwright 无头驱动截取。已知缺口
daemon 侧不持久化逐轮的
checking卡片(客户端有意抑制它们,以免每个 stop-hook 轮次刷出一张卡)。因此 resume 之后迭代计数会从最后一张set卡片恢复,也就是回到 0,这会让MAX_GOAL_ITERATIONS失去跨 resume 的上限语义。TUI 没有这个问题,因为它持久化了checking。要彻底修复需要持久化一个轻量的迭代计数器;为控制本 PR 的范围,暂未纳入。验证方法
如何验证
qwen serve,打开 Web Shell。点击侧边栏的靶心图标、或输入无参/goal、或点击状态栏的 goal pill —— 三者都会打开目标页面。qwen serve(或重新加载该会话)。本 PR 之前,goal 卡片消失且循环已死。本 PR 之后,卡片回到 transcript,状态栏 pill 恢复,目标继续工作。/goal <条件>和/goal clear,行为与之前完全一致。/goal clear在回合运行中依然可用。自动化测试
packages/cli:restoreGoal.test.ts(记录解析与恢复)、HistoryReplayer.test.ts(goal 卡片 replay,checking被跳过)、Session.test.ts(set/cleared/ terminal 卡片被持久化)、acpAgent.test.ts(sessionGoalGet;resume 重新注册、不会复活已达成的目标、遵守 hook 策略)、serve/routes/goals.test.ts(投影、排序、探测失败被丢弃、500 分支)。packages/web-shell:GoalsDialog.test.tsx,以及App.test.tsx中对无参/goal行为和在新会话中创建目标的覆盖。integration-tests/cli/qwen-serve-routes.test.ts:针对真实 daemon 的GET /goals端到端测试 —— serve → bridge → 真实 spawn 的qwen --acp子进程中的sessionGoalGet。对比
/goal只能通过输入框使用;无法查看工作区内正在运行的所有目标;且会话重新加载或 daemon 重启后目标会被静默丢弃。与 #6535 的关系
两者都改动了
packages/acp-bridge/src/status.ts。本 PR 把sessionGoalGet放在sessionGoalClear旁边,而不是追加到SERVE_CONTROL_EXT_METHODS末尾,正是为了避免与 #6535 的createSubSession撞行。其余重叠文件(i18n.tsx、App.tsx、webui/.../workspace/types.ts)改动位于同一文件的不同区域,应该可以干净合并。