feat(desktop): goal chip shows paused state, elapsed, tokens, pause/resume - #3027
Conversation
…esume The goal chip rendered a paused goal exactly like a running one — same pulsing accent dot, same counter — and the only control was clear, even though the host protocol fully supports goal.control pause|resume and resumeFromControl restarts a paused goal without spending a model turn. - Wire goal:pause / goal:resume through preload and the session-domain IPC bridge, via a generalized client controlGoalWithRetry (the clearGoal optimistic-revision retry, now shared by all three actions). - Status conflicts stop being retried away: the host folds invalid transitions into operation_conflict, and every accepted transition bumps the revision, so a conflict at an unchanged revision rethrows the host's reason instead of burning attempts and throwing a misleading retry-exhaustion error (this also fixes clearGoal). - The chip is status-aware: a paused goal shows a warning dot without pulse and its own aria label; elapsed wall-clock (frozen at pausedAt) and tokensSpent / tokenBudget render when available; a pause button appears for running goals and a resume button for paused goals next to the existing stop (clear) kill switch, with matching overflow-menu entries and failure toasts. No runtime or protocol changes — surface wiring only. Ref apache#3024 Generated-by: Maka
5c0c935 to
a014abe
Compare
📝 WalkthroughProblem solvedThe desktop goal chip did not distinguish running, waiting, and paused goals. It also lacked pause/resume controls and did not show available elapsed-time or token-usage data. This PR:
Source of truthThe PR extends the existing host projection and canonical Scope and complexityThe solution is the smallest coherent path described by the diff. Shared retry logic avoids separate pause, resume, and clear implementations. Shared The added complexity is required for:
No deletion is apparent without weakening behavior or coverage. The existing clear path was simplified by delegating to Complexity delta
Total maintenance complexity increases in UI branches and public contracts, but the increase is justified by the required behavior and shared retry implementation. ValidationThe PR reports passing unit tests, TypeScript checks, Storybook build, visual smoke tests, and Review-relevant risksThe diff changes user-visible goal controls, status labels, timing, token display, and localized error messages. These UI changes require independent human review under repository policy. The diff changes public preload and session-domain contracts. These contract changes require independent human review under repository policy. No security, licensing, release, or governance effect was identified in the current diff. The person performing the merge must review the final diff. A maintainer makes the final determination. WalkthroughThe desktop goal flow now supports pause and resume through revision-aware IPC operations. The renderer exposes live goal metadata, localized controls, elapsed time, token usage, and status-specific active, waiting, and paused presentations. ChangesGoal pause and resume controls
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: 🟡 Moderate · up to The new goal controls and status display can temporarily show an outdated pause/resume state, fail to keep paused elapsed time frozen in some cases, or show an erroneous failure toast after rapid repeated activation. The PR should receive follow-up on these bounded correctness issues before merge. Sequence Diagram(s)sequenceDiagram
participant GoalIndicator
participant MakaBridge
participant IPCHandlers
participant DesktopRuntimeHostClient
GoalIndicator->>MakaBridge: pause(sessionId) or resume(sessionId)
MakaBridge->>IPCHandlers: invoke goal:pause or goal:resume
IPCHandlers->>DesktopRuntimeHostClient: controlGoalWithRetry(sessionId, action)
DesktopRuntimeHostClient->>DesktopRuntimeHostClient: refresh revision and retry conflicts
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Astro-Han
left a comment
There was a problem hiding this comment.
I reviewed exact head a014abe7b85065d829b4549de6aa554e3d9b9c9f, including the full diff, current CI, Runtime Host goal lifecycle, IPC/preload boundary, and available automated feedback.
The primary architecture is correct. Runtime Host remains the sole goal lifecycle authority; Desktop reuses the existing goal.control contract and canonical projection rather than creating a second state machine. Consolidating clear/pause/resume through the same optimistic-control seam is also the right direction.
One P2 remains in the user-visible state contract.
SessionContextLayer only distinguishes paused; waiting still receives the running aria label, accent styling, and a pulsing dot. Runtime Host uses waiting for evaluator-directed backoff, when the goal is not currently consuming a model turn, and the issue explicitly requires active, waiting, and paused to remain distinguishable. The current chip therefore tells both sighted users and assistive technology that a waiting goal is running.
Please add an explicit waiting presentation branch and a focused rendering regression. The exact color can remain a design choice, but the label and motion must not reuse running semantics.
A non-blocking P3 remains in the final optimistic-conflict path: after repeated conflicts, a final state rejection can be collapsed into generic revision_conflict, hiding that another client already reached the requested state. A final query or idempotent-target check would improve that behavior.
No PR split is needed; the lifecycle authority and IPC structure should remain as implemented.
Disclosure: Codex performed the read-only source, lifecycle, accessibility, test, CI, and feedback analysis. The human contributor remains responsible for verifying the findings and deciding whether to merge.
中文
Goal authority 和 IPC 方向正确。唯一 P2 是 waiting 仍显示为 running 并持续 pulse,违反 active/waiting/paused 三态契约。补齐 waiting 文案、样式和测试后即可继续。
|
Thanks — the three gaps from #3024 are real (paused looked identical to running, no pause/resume control, no elapsed/tokens), and the fix is the right shape: the chip renders purely from the host projection (single authority — Conclusion: PASS — all P3, nothing blocking. P3 (optional): in AI-assisted review disclosure: this review was produced with AI assistance (pi review subagent on 中文摘要(AI 辅助审查)结论:PASS,全 P3。问题真实(#3024 三缺口:paused 与 running 视觉无差别、无 pause/resume 控制、无 elapsed/tokens;host 协议本就支持 goal.control,projection 本就携带 setAt/pausedAt/tokensSpent/tokenBudget——纯 surface 缺口),方案正确:数据权威单一(chip 纯渲染 host projection,goal-change 广播触发 refetch,30s ticker 只是本地渲染时钟),controlGoalWithRetry 泛化既有 clearGoal 走同一 seam 无并行路径;状态拒绝检测 sound(仅被接受的转移 bump revision——commit 恒 bump、非法 pause/resume/clear 返回 undefined 不 commit,故 "conflict 且 revision 未变=拒绝" 推断成立无双误判)。与 #3025/#3026 零文件重叠无冲突。pause 边界时间计算正确(running=Date.now()-setAt、paused 冻结于 pausedAt-setAt;resume 后含暂停墙钟时间已文档化且与 TUI 语义一致)。CI 全绿。P3(可选):runtime-host-client.ts:1168 末次尝试 break 跳过 status-refusal/null-goal 检测(末次遇 refusal 仍抛 revisionConflict、goal 恰在末次窗口被移除时从静默变多余报错 toast——需连续 3 次竞态才触达,实际不可达);stories 只加 setAt 未加 onPause/onResume,新按钮无视觉回归覆盖;无 goal e2e spec(pause/resume 属 genuinely new behavior,按仓库标准应补 e2e);formatCompactTokenCount 对 ≥1,000,000 输出 "1000k"(纯外观)。 |
|
Before this can merge: this is a UI/UX change (goal chip now shows paused state, elapsed time, token count, and pause/resume controls), so per the project's review gate it needs before/after screenshots — e.g. the goal chip as it was (running vs paused indistinguishable) and as it is now (paused badge, elapsed, tokens, pause/resume button). The review itself is PASS (all P3s), but the merge is gated on the screenshots. Please add them to the PR description and re-request review. AI-assisted review disclosure: this comment was written by a human maintainer following the review-gate rule; no AI contributed to this content. |
Astro-Han
left a comment
There was a problem hiding this comment.
Blocking merge pending before/after screenshots of the UI/UX change (see comment above).
Review superseded by the main review comment; screenshot request communicated via comment.
|
@Astro-Han The waiting state now has a distinct non-pulsing presentation and accessible label in d8a0d2c, with focused regression coverage. The before/after screenshots are also in the updated PR description and both image links are live. Could you please re-review the current head when convenient? |
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for making the goal chip reflect the lifecycle users actually need to reason about. I reviewed the current head with an independent @reviewer pass and a read-only ollama-cloud/deepseek-v4-flash:high pass.
The problem is correctly defined: a goal is not merely active or finished; paused and waiting states must remain distinguishable, and pause/resume must go through the Runtime Host authority rather than renderer-owned state. The implementation follows that seam, carries the operations through the existing Host/IPC/preload boundary, and keeps the UI projection focused on elapsed time, token usage, and the permitted action.
I found no concrete P0–P2 issue on this head. The current CI checks are green. The remaining observations are non-blocking: the final conflict query/direct IPC path and a narrower end-to-end assertion could be pinned more explicitly, but they do not establish a production defect.
No local test suite was run during this review; conclusions are based on source, test, and current CI inspection. Codex coordinated the independent passes and performed the final adjudication; external-model output was treated as unverified until checked against the code.
中文摘要
感谢这次让 goal chip 真正反映用户需要理解的生命周期。问题定义正确:goal 不只是活跃或结束,paused 与 waiting 必须可区分,pause/resume 也应经过 Runtime Host 权威,而不是由 renderer 自建状态。
当前实现沿用了现有 Host/IPC/preload 边界,UI 只投影 elapsed time、token usage 和允许的操作。当前 head 未发现具体的 P0–P2 问题,CI 也已全绿。
剩余仅是非阻断测试建议:可以更明确地固定最终冲突查询/直接 IPC 路径,以及增加更窄的端到端断言,但目前没有可复现的生产缺陷。
本次未在本地运行测试套件;结论来自源码、测试与当前 CI 检查。Codex 协调了独立审查并完成最终判断,外部模型输出在核对代码前均视为未验证输入。
Astro-Han
left a comment
There was a problem hiding this comment.
Non-blocking design suggestion
The Runtime Host, IPC, and state-management implementation looks solid, so I do not consider this a merge blocker.
From a product UI hierarchy perspective, however, an active Goal feels less like session metadata and more like a live execution control: it can continue consuming a budget and may need to be paused, resumed, or cleared while the user is working in the composer.
I suggest a follow-up that moves the Goal presentation out of SessionContextLayer and into Astryx ChatComposer.headerContext. Keeping the status and kill switch next to the primary input should make the relationship clearer and produce a calmer, more cohesive layout.
A compact composition could keep StatusDot, progress/budget text, and pause/resume visible, while placing Clear in MoreMenu. I would avoid ChatComposerDrawer, because it represents collapsible staged input context, and Banner, because it would give an ordinary running state too much visual weight.
This is a design refinement rather than a correctness concern; the current implementation is functionally sound and can land as-is.
Disclosure: Codex assisted by inspecting the PR structure and the available Astryx component seams. Astro-Han independently reviewed the code and agrees with this non-blocking design recommendation.
中文
后端、Runtime Host、IPC 和状态管理实现没有问题,这条建议不阻塞合并。
从产品界面的信息层级看,Goal 更像持续运行的执行控制器,而不是会话元数据。建议后续将它从 SessionContextLayer 移到 Astryx ChatComposer.headerContext,让状态、预算和暂停入口靠近用户的主要操作区域。
可以始终展示 StatusDot、进度/预算以及暂停或恢复按钮,将 Clear 收进 MoreMenu。不建议使用可折叠的 ChatComposerDrawer,也不建议使用视觉权重过高的 Banner。
Astro-Han
left a comment
There was a problem hiding this comment.
One provenance clarification remains before merge. The PR body says OpenAI Codex (Maka) assisted implementation and review remediation, and commit a014abe7b carries Generated-by: Maka, but the follow-up substantive fix commit d8a0d2c90 does not.
If that waiting-state remediation used substantive AI assistance, please amend it with the matching Generated-by: Maka trailer and preserve the trailer in the final squash commit. If it was entirely human-authored, please clarify that exception in the PR body instead. The technical approval and accepted before/after screenshots otherwise stand.
中文说明
PR body 说明 Maka 参与了实现和 review remediation,首个 commit 也有对应 trailer,但后续 waiting-state 修复 commit d8a0d2c90 没有。
如果该修复使用了实质 AI 辅助,请补 Generated-by: Maka;如果完全由人工完成,请在 PR body 明确这个例外。技术审查和 before/after 截图结论不变。
f2753e6 to
91b50fc
Compare
|
Correction to my previous provenance reply: the two commits used different tools. |
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 374eadb1-5335-46c9-b664-185951aa43a8
📒 Files selected for processing (14)
apps/desktop/src/main/__tests__/runtime-host-client-operations.test.tsapps/desktop/src/main/runtime-host-client.tsapps/desktop/src/main/runtime-host-session-domains-ipc-main.tsapps/desktop/src/preload/bridge-contract.d.tsapps/desktop/src/preload/preload.tsapps/desktop/src/renderer/app-shell.tsxapps/desktop/src/renderer/locales/shell-copy.tsapps/desktop/src/renderer/use-session-goal.tsapps/desktop/stories/app-shell.stories.tsxpackages/ui/src/__tests__/session-context-layer-goal.test.tsxpackages/ui/src/chat-view.tsxpackages/ui/src/conversation-copy.tspackages/ui/src/icons.tsxpackages/ui/src/session-context-layer.tsx
Included review availability: Your plan provides up to 3 included reviews per hour; 2 remain after this review.
91b50fc to
731c8fe
Compare
|
The failed Runtime Host lane was an unrelated |
|
Latest bot-review remediation is on |
Generated-by: Codex
Generated-by: Codex
Astro-Han
left a comment
There was a problem hiding this comment.
Reviewed exact head af751796328c27aeca0613c704e25223a8123e41.
The latest delta correctly projects the waiting state, sequences goal reads so stale responses cannot overwrite newer state, and serializes pause/resume per session. Keeping clear outside that guard is appropriate for the existing kill-switch semantics: it remains revision-safe and available while another control is pending. All checks are green, all review threads are resolved, the before/after screenshots demonstrate the paused-goal controls, and the Maka/Codex disclosure plus all three commit trailers are complete. I found no new P0–P3 issue.
Please preserve both declared provenance trailers in the final squash commit.
AI-assisted review disclosure: Codex reviewed the exact-head diff, control/projection concurrency, current-main integration, tests, live CI and threads, screenshots, and provenance metadata. No external model was used. Astro-Han authorized this review campaign.
中文说明
最新提交正确补齐了 waiting 状态、避免旧 goal 查询覆盖新状态,并按 session 串行化 pause/resume。clear 保持独立符合既有 kill switch 语义:它仍有 revision 安全控制,也能在其他操作 pending 时立即清除。CI、线程、before/after 截图以及三个提交的 Maka/Codex trailer 均已核实,没有新的 P0–P3。最终 squash 请保留两种 provenance trailer。
Summary
The desktop goal chip rendered a paused goal exactly like a running one — same pulsing accent dot, same counter — and the only control was clear, even though the host protocol fully supports
goal.controlpause | resumeandresumeFromControlrestarts a paused goal without spending a model turn.goal:pause/goal:resumeare wired through preload (window.maka.goal.pause/resume) and the session-domain IPC bridge via the shared optimistic-revisioncontrolGoalWithRetrypath.tokensSpent / tokenBudgetrender when available; paused elapsed time freezes atpausedAt.operation_conflictat an unchanged revision is rethrown as the host's transition refusal instead of being retried as a race.Runtime Host remains the lifecycle authority. The renderer reads
goal.get, refetches on the canonicalgoal-changebroadcast, and projects only the three live states (active | waiting | paused). Closes #3024.Before / after
Before — a paused goal was still announced and rendered as running, with only iteration count and clear:
After — paused has a distinct still warning state, frozen elapsed time, token usage, resume, and clear:
Review process
Two independent read-only review passes (first-principles/Occam and
simplify-audit) found no P0-P2 issues. The propagation chain remains single-authority end to end: control → accepted host commit/revision bump → canonicalgoal-change→ renderer refetch → chip projection.The remaining P3 is an existing extreme retry edge: the final optimistic-conflict attempt does not re-query to distinguish a last-window status refusal or removed goal. It requires three consecutive revision races and does not block this UI fix.
Test plan
packages/ui: 170/170 tests pass, including active, waiting, and paused goal-chip rendering.git diff --checkpasses.AI use
Tool(s) and scope: Maka authored the initial goal-chip implementation in
a014abe7b; OpenAI Codex authored the waiting-state and goal-control race remediations inaf7517963and assisted with its tests, review remediation, and PR documentation. The author reviewed and verified both contributions.Final squash trailers:
Generated-by: MakaandGenerated-by: Codex