Repository navigation
Sleep stops a workspace's native chat agents - #28445
Conversation
98b1618 to
224d130
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ➖ Normal Merge Risk: 🔵 Low · up to
|
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
66d46363-cb90-45fc-8db5-71e2733ee1b3
⛔ Files ignored due to path filters: 1
src/shared/rpc-contract/rpc-params-catalog.generated.tsis excluded by!**/*.generated.*
📒 Files selected for processing: 15
src/main/runtime/rpc/methods/structured-agent-session-gate-classification.test-fixture.tssrc/main/runtime/rpc/methods/structured-agent-session-pi-dialog-capability.test.tssrc/main/runtime/rpc/methods/structured-agent-session.test.tssrc/main/runtime/rpc/methods/structured-agent-session.tssrc/renderer/src/components/sidebar/sleep-workspace-chats.tssrc/renderer/src/components/sidebar/sleep-worktree-activation-race.test.tssrc/renderer/src/components/sidebar/sleep-worktree-flow.test.tssrc/renderer/src/components/sidebar/sleep-worktree-flow.tssrc/renderer/src/components/sidebar/use-worktree-context-menu-model.tsxsrc/renderer/src/components/sidebar/workspace-lineage-menu-actions.test.tssrc/renderer/src/components/sidebar/workspace-lineage-menu-actions.tssrc/renderer/src/components/sidebar/worktree-context-menu-policy.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/runtime/structured-agent-session-close.tssrc/shared/structured-agent-session-surface-capabilities.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
fff368ec-bc04-4d95-b920-9cc76924c31e
⛔ Files ignored due to path filters: 1
src/shared/rpc-contract/rpc-params-catalog.generated.tsis excluded by!**/*.generated.*
📒 Files selected for processing: 65
mobile/src/host-screen/host-screen-overlays.tsxsrc/main/acp/acp-structured-session-adapter.tssrc/main/claude/claude-structured-session-acquisition.tssrc/main/claude/claude-structured-session-adapter.tssrc/main/codex/codex-app-server-connection.tssrc/main/codex/codex-structured-session-adapter.tssrc/main/ipc/dashboard-payload-validation.tssrc/main/ipc/ephemeral-vm-runtime-handler-cleanup.test.tssrc/main/ipc/ephemeral-vm-runtime-handlers.tssrc/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.tssrc/main/native-chat/agent-session-wire/structured-agent-session-adapter.tssrc/main/native-chat/agent-session-wire/structured-agent-session-attach-orchestration.tssrc/main/native-chat/agent-session-wire/structured-agent-session-conversation-lifetime.tssrc/main/native-chat/agent-session-wire/structured-agent-session-create.tssrc/main/native-chat/agent-session-wire/structured-agent-session-host-delivery.tssrc/main/native-chat/agent-session-wire/structured-agent-session-host-mutation-wiring.tssrc/main/native-chat/agent-session-wire/structured-agent-session-host.tssrc/main/native-chat/agent-session-wire/structured-agent-session-surface-lifetime.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-task-queue.tssrc/main/native-chat/agent-session-wire/structured-agent-session-workspace-sleep.tssrc/main/pi/rpc-session-adapter.tssrc/main/provider-process/provider-process-teardown.test.tssrc/main/provider-process/provider-process-teardown.tssrc/main/runtime/agent-session-record-founding.tssrc/main/runtime/orca-runtime-list-managed-worktrees.tssrc/main/runtime/rpc/methods/structured-agent-session-rpc.test-fixture.tssrc/main/runtime/rpc/methods/structured-agent-session-schemas.tssrc/main/runtime/rpc/methods/structured-agent-session.test.tssrc/main/runtime/rpc/methods/structured-agent-session.tssrc/main/runtime/rpc/methods/worktree.tssrc/main/runtime/runtime-notifier-contract.tssrc/main/runtime/structured-worker-authority.tssrc/main/window/runtime-window-lifecycle.tssrc/preload/api/ephemeral-vm-api.tssrc/preload/api/ui-bridge-clipboard-and-window-controls.tssrc/preload/api/ui-command-event-api.tssrc/renderer/src/components/dashboard/useDashboardPopoutBridge.tssrc/renderer/src/components/sidebar/sleep-worktree-activation-race.test.tssrc/renderer/src/components/sidebar/sleep-worktree-flow.test.tssrc/renderer/src/components/sidebar/sleep-worktree-flow.tssrc/renderer/src/components/sidebar/sleep-worktree-sidebar-position.tssrc/renderer/src/components/sidebar/use-worktree-context-menu-commands.tssrc/renderer/src/components/sidebar/use-worktree-context-menu-model.tsxsrc/renderer/src/components/sidebar/workspace-lineage-menu-actions.test.tssrc/renderer/src/components/sidebar/workspace-lineage-menu-actions.tssrc/renderer/src/components/terminal-pane/pty-connection-deliberate-sleep-guard.test.tssrc/renderer/src/components/terminal-pane/pty-connection/run-deferred-connect.tssrc/renderer/src/hooks/ipc-events/mobile-terminal-close-ipc-bridge.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/lib/ephemeral-vm-runtime-cleanup.tssrc/renderer/src/lib/sidebar-worktree-activation.test.tssrc/renderer/src/lib/sidebar-worktree-activation.tssrc/renderer/src/lib/worktree-sleep-intent.tssrc/renderer/src/runtime/structured-agent-session-client.tssrc/renderer/src/store/slices/worktrees/session/set-active-worktree.tssrc/renderer/src/store/terminals/terminal-pty-bindings.tssrc/renderer/src/store/terminals/terminal-pty-release.tssrc/renderer/src/store/terminals/terminal-tab-creation.tssrc/renderer/src/store/terminals/terminal-worktree-sleep-owner.test.tssrc/renderer/src/store/terminals/terminal-worktree-sleep-owner.tssrc/shared/dashboard-snapshot.tssrc/shared/ephemeral-vm-runtime-host-match.tssrc/shared/rpc-contract/structured-agent-session-params.tssrc/shared/rpc-contract/worktree-params.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.
4f5bd1f to
9d4018c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
41ddb650-64f5-446d-9f92-0b61548d700b
⛔ Files ignored due to path filters: 1
src/shared/rpc-contract/rpc-params-catalog.generated.tsis excluded by!**/*.generated.*
📒 Files selected for processing: 10
mobile/src/host-screen/host-server-workspaces-wiring.test.tsmobile/src/host-screen/use-host-worktree-actions.tssrc/main/native-chat/agent-session-wire/structured-agent-session-create.tssrc/main/native-chat/agent-session-wire/structured-agent-session-host-delivery.tssrc/main/native-chat/agent-session-wire/structured-agent-session-host.tssrc/main/runtime/agent-session-record-founding.tssrc/main/runtime/rpc/methods/worktree-sleep-method.tssrc/main/runtime/rpc/methods/worktree.tssrc/renderer/src/i18n/locales/en.jsonsrc/shared/rpc-contract/structured-agent-session-params.ts
🚧 Files skipped from review as they are similar to previous changes: 1
- src/renderer/src/i18n/locales/en.json
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Preserve active teardown counts when forgetting a worktree. · worktree-sleep-intent.ts:87-114
src/renderer/src/lib/worktree-sleep-intent.ts:87-114
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winPreserve active teardown counts when forgetting a worktree.
withWorktreeSleepTeardownkeeps a finalizer pending during terminal shutdown. If deletion forgets the worktree, then the samerepo::pathis recreated and starts another teardown, the old finalizer decrements the recreated worktree's count and removes it. A later PTY bind can then clear the new sleep intent while terminal shutdown is still active.Suggested fix
intents.delete(key) - teardownCounts.delete(key) listeners.delete(key)
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
1631169b-b705-4add-866f-dd4885f7d62d
📒 Files selected for processing: 2
src/renderer/src/lib/worktree-sleep-intent.tssrc/renderer/src/store/slices/workspace-activation-preparation.test.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.
|
Status (head This head is Why the PR changed since round 9: main merged #30047, which makes Sleep act on the whole workspace. When the same workspace is open on two hosts, Sleep on either copy sleeps both, after a confirmation. This PR had made Sleep act on one host's copy, which broke that confirmation flow. It was re-planned and rebuilt onto main's model. Sleep now asks every host that has a copy to stop its chats. Main's terminal-owner check, confirmation and teardown order stay as they are on main. The rebuild removed about 300 production lines of per-host handling. Rounds 10–11 reviewed the rebuild, and every finding is fixed and re-verified:
Three-model design panels settled the ordering and retirement calls, and independent host and flow reviewers agreed on each fix. The loop's fixer finished with CI green. The wrap-up (a final main sync and QA) was run by the lane coordinator after the loop session went idle. Live QA on this head: a hidden test copy on the M4Air, real signed-in Claude, Codex driving the UI, with main 70d00aa as the comparison on the same input.
Screenshots are in the PR description. The QA rig left out CI on this head: the full PR workflow passes. An earlier floating-terminal test failure came from main and was fixed there (#30568). Pre-existing, not from this PR:
Disclosed limitation: a workspace open on two hosts with no terminal bound in this window keeps main's "can't tell which host" refusal. That includes one with only chats. |
06217f7 to
1d79a56
Compare
Sleeping a workspace stopped terminals and browsers but never its structured (native) chats, so their claude/codex process kept running, and Sleep was disabled for a workspace whose only content was a chat. Sleep now stops each chat's agent through a new agentSession.sleep (the host's close, keeping the tab), routed to the host recorded on the chat's tab; an older host without the capability fails Sleep with a clear message. The Sleep menu counts a chat.
Main split runSleepWorktrees into sleepWorktreesReportingFailures (no toast) and a toasting wrapper. Move the native chat stop, host routing and typed failures into the shared core so the WSL move's Sleep request also stops chats, and report every workspace not verifiably slept (an unconfirmed chat stop included) in failedWorktreeIds. Fold one ACP adapter field onto a line so main's file plus this PR's ownership check stays within the file length limit.
685ac16 to
ba9385a
Compare
Main grouped the PTY connection suites (#29193) and moved this file to a .spec.ts registered from pty-connection-spawn-and-startup.test.ts. Taking main's copy here keeps the following main merge free of a modify/delete conflict; this PR's test changes are reapplied after the merge.
…n under its line limit The deliberate-sleep spec moved in #29193; this carries the PR's changes into it: a pane mounted after wake waits for issued teardown, an unqualified pane waits for every same-id host's teardown, and a queued startup waits too. The merge left claude-structured-session-acquisition.ts over 300 lines. resolveClaudeAcquisitionError, which already builds the exit-unproven error, now decides whether to release the acquisition.
…ops-native-chat # Conflicts: # src/main/native-chat/agent-session-wire/structured-agent-session-host.ts # src/main/runtime/rpc/methods/structured-agent-session.ts # tests/e2e/cross-version-wire/structured-agent-session-surface-manifest.ts
…ops-native-chat # Conflicts: # src/main/runtime/rpc/methods/structured-agent-session.ts
…ops-native-chat # Conflicts: # src/main/native-chat/agent-session-wire/structured-agent-session-host.ts # src/main/runtime/revisioned-state.ts # src/main/runtime/workspaces/workspace-selectors.ts # src/renderer/src/components/native-chat/StructuredAgentSessionStatusBridge.test.tsx # src/renderer/src/components/sidebar/sleep-worktree-flow.ts # src/renderer/src/components/sidebar/use-worktree-context-menu-commands.ts
…ain's teardown owner check as is
…ops-native-chat # Conflicts: # src/main/native-chat/agent-session-wire/structured-agent-session-host.ts
…er pages everywhere, headless sleep stops terminals (#30715) * fix(runtime): one owned-session teardown policy; removal closes host browser pages everywhere, headless sleep stops terminals A declared table says what each entry point (worktree, folder and project removal, sleep, disconnect) does to each owned kind (PTY, native chat, host browser page, launch record), so a new kind is a compile error until decided. Removal over desktop IPC and repo.rm now close the host's browser pages and launch records like the runtime removal; worktree.sleep with no renderer sleeps the host's terminals instead of doing nothing. Sleep->chat is declared pending #28445; disconnect stops nothing. * fix(runtime): project removal finds headless pages and failed launches without a client page; stub the new runtime method * fix(runtime): headless sleep rejects an unproven stop; read headless page owners through the bridge, not Electron
…ity, held-send revert Sleep (#28445) closes a workspace's chats through the same abort a close gives a registered start, re-aborts a start registered while it waited for the lane, and judges its receipt once the start it aborted has settled, so only a newer start reads as later work. Each adapter's owned-acquisition check covers the child it owns between acquire and open. Main's Sleep cases from the deleted surface lifetime suite move to the workspace Sleep suite, plus one for a starting agent. Codex (#30406): the split open sends the chat's command env overrides when it opens the thread; the adapter test keeps that assertion on the open's request. Revert of #30134 (#30572): the event sink, exit observation, Pi held-send wrapper and adapter heldSends overrides go with it; Pi dispatch submits through its turns again and its idle report is wired back.
ELI5
The problem. Sleeping a workspace stopped its terminals and browsers but left its native chat agents running: the claude or codex process behind each chat tab. A workspace that only had chats couldn't be slept at all, because Sleep was greyed out.
What's different for you. Sleep now also stops the workspace's chat agents. This applies to Sleep from the sidebar menu, bulk and descendant Sleep, the phone and the command line. The chat tabs and their history stay. Your next message in a slept chat starts the agent again in the same conversation. Chat-only workspaces can now be slept.
Marking the slept workspace as asleep and hiding it under "Hide sleeping workspaces" is the stacked follow-up, #28457.
What Changed
The host stops the chats. Sleep asks each execution host to stop the workspace's chats, through a new
agentSession.sleepWorkspacecall. The execution host is the machine running the agents: this desktop, a paired server, or an SSH machine reached through a paired server. The host finds every chat it runs in that workspace, including hidden chats and starts still in progress. It closes them through the existing ordered close path, which ends the process's input, then signals it, then forces an exit if needed. It answers with the chats it can prove are stopped, plus a typed failure for each of the rest.What counts as stopped. A chat counts as stopped only with proof:
A release without proof stays unverified, and that's what produces the one toast.
Only real newer work overrules a stop. A stop result counts as superseded only on facts: the chat's session restarted, or newer work exists (a running child, a delivery, a pending submission, a create or attach in flight, or an open dispatch). The provider's own "I've ended" report doesn't count. A message you send after the stop still runs and resumes the chat.
Sleep acts on the workspace, as main does. The sidebar flow is main's id-based flow from [hardening 4/5] fix(sidebar): confirm before sleeping a workspace that another host's copy shares #30047. It adds one step: when terminal teardown is admitted, it asks every host that publishes the workspace to stop its chats, from main's own list of a workspace's copies. If main's terminal-owner check refuses (it can't tell which copy runs the terminals), no chat is stopped. A workspace's VM is suspended only after its chats have stopped.
A failed Claude start is retired after a proven exit. If a Claude chat's start failed and Sleep later proves its process gone, the failed attempt is dropped, so your next message starts a fresh agent instead of being refused.
Status rows have one writer. Chat status rows stay owned by the host's status feed. Sleep no longer deletes them from the app window, so there's no race between the two writers. A chat makes a workspace sleepable only while the host reports that it holds the chat's process.
Project moves. The shared Sleep routine is also used when a project moves into WSL (feat(wsl-mover): pause a moving project's terminals through Sleep and prove they exited (D1b-5d) #28820), so that move now stops the project's chats too.
Why
The host is the only side that knows which agent processes it owns. The visible tab list misses hidden chats and starts still in progress. Reusing the host's ownership and its existing close path avoids a second way of stopping processes. Taking main's whole-workspace Sleep model keeps one meaning of what Sleep acts on. An earlier version of this PR slept one host's copy at a time; that contradicted main's #30047 decision and would have broken its confirmation flow.
Mixed versions. The new call is checked against the host's advertised capabilities. An older host that lacks it returns an ordinary failure, with the one toast, and is fixed by updating the host. The
failuresfield in the answer is optional, so older peers read the answer unchanged. Nothing else on the wire changes.Differences from the common pattern:
Known limitation: main's terminal-owner check also refuses a workspace open on two hosts that has no terminal bound in this window, so such a workspace (including one with only chats) still can't be slept. Main shows "can't tell which host runs this workspace's terminals". A chat-only workspace on a single host can be slept.
Linked Issue
Reported in the workspace Sleep thread ("Sleeping a workspace with a native chat should really sleep it ... should be properly marked as slept and hidden"). The marking and hiding half is #28457.
Visual Proof
The same input on each build: a workspace with only a Claude chat (ws-chat), and one with a terminal plus a Claude chat (ws-mixed). Both chats were idle after replying.
Before, on main: a chat-only workspace can't be slept (Sleep is greyed out).
Before, on main: after Sleep, ws-mixed shows Inactive and drops its chat row, but that chat's claude is still running.
After: Sleep is offered for the chat-only workspace.
After Sleep on ws-chat: its claude exited, there's no toast, and the workspace is still listed.
Reopening keeps the history, and a new message resumes claude and gets a reply.
After Sleep on ws-mixed: the chat's claude and the terminal's processes all stopped, with no toast.
Testing
Final head (5b829e9), live QA: a hidden test copy of Orca on the M4Air, with the real signed-in Claude and Codex driving the UI. Main (70d00aa) got the same input as the comparison.
Earlier rounds also covered:
Automated:
The full PR workflow is green on 5b829e9.
Focused tests cover: the host's Sleep answer, Sleep across the chat's lifetime, the RPC surface, the sidebar Sleep flow, main's twin-Sleep tests, and the failed-start retirement.
Each regression from review has a test that failed before its fix: a Sleep stopping a newer turn, a second Sleep after a failed start, and the next send after a retired start.
Final build manually verified through the changed native-chat flow
Existing automated contracts exercised and targeted changes covered
Not verified:
Seen but not caused by this PR:
Review
This went through eleven review rounds with independent reviewers, plus design panels on the hard calls. Defects found and fixed here:
The branch was synced with main at the end (main at 70d00aa) and has no conflicts. The pre-commit formatter also reformatted 13 of main's files that arrived unformatted in an earlier merge (formatting only).
Agent skill upstream boundary