Repository navigation
feat(desktop): run a chat's desktop tools in the background executor - #8668
waleedlatif1 wants to merge 11 commits into
Conversation
|
@cubic-dev-ai review this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
a31aaa8 to
72a5ba3
Compare
|
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
8c2b3fd to
de04c40
Compare
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
48920e2 to
0a5f255
Compare
…or its successor dispose() now reports idle once. A recovered delivery that settles after sign-out no longer reports through the shared busy callback, where it could release the sleep blocker the next session's executor was holding.
…en Terminal is switched off - A tmux run is tracked as soon as its window exists, not once its wait ends. Sign-out now reaches a command the chat view started that is still inside its wait window. Reaping skips runs whose call is still reading their files. - Switching Terminal off stops the agent's commands before the shells go, as sign-out does. - The runner's deadline is built on the shared interruptible sleep. - A hostname longer than Sim's 128-character limit is shortened, so registration does not fail on it.
f6d9127 to
6cf16e3
Compare
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 47 files
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Turn on auto-fix | Re-trigger cubic
…ach other - Sign-out waits for a restart's journal walk before clearing the journal, and a walk or delivery that starts after sign-out sends nothing, so none of the previous session's results outlive it. - An unrecognized device's stopped loops stay stopped: the reconcile timer no longer re-arms itself after them. - Switching Terminal off disposes the shells only if it is still off once the agent's commands have stopped. - A tmux run whose terminal closes mid-wait keeps its output files until its call has read them. - A terminal operation that outlives its deadline is reported as outcome unknown and not to be retried, as a browser action is.
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
1 issue found across 47 files
Confidence score: 3/5
- In
apps/desktop/src/main/terminal/index.ts, a run stopped through escalation is reported as completed with no exit code, so the agent can mistake a force-killed command for success. Preserve a non-success status for these runs.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/main/terminal/index.ts">
<violation number="1" location="apps/desktop/src/main/terminal/index.ts:1292">
P2: A `run` stopped through the escalation path is reported to the agent as `status: 'completed'` with `exitCode: null`, which makes a force-killed command indistinguishable from a successful completion. When `interruptTmuxRun` has to `closeRunWindow` (the command ignored Ctrl-C), the run's status file is never written, so `pollRun` returns `done: false` and the stop path forces `done: true`, which `runInTmux` turns into `status: 'completed'`. That result is what the executor records and the model reasons from, so the agent can conclude an escalated kill finished normally. Track that the outcome came from the stop and report it distinctly (for example `status: 'stopped'` with `exitCode: null`) instead of completing the run's normal-done semantics.</violation>
</file>
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Fix all with cubic | Turn on auto-fix | Re-trigger cubic
… assertions in tests When the app has no usable account storage (signing out, switching account, storage unavailable), a local file call now says so. It no longer tells the model a setting is off and to ask the user to switch it on.
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
No issues found across 47 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Turn on auto-fix | Re-trigger cubic
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
No issues found across 47 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Turn on auto-fix | Re-trigger cubic
Summary
Phase 2a of the desktop background executor. The Sim desktop app now picks up and runs the desktop tool calls of turns bound to it from Electron main, so a chat keeps working after the user switches chats, switches workspace, or reloads the window. It uses the device protocol from #8644 and #8650 as is, and stays dormant until Sim enables the executor for the user.
Based on
staging, now that #8650 is squashed.What the desktop app does
Identity. It keeps a stable install id in userData. It registers that id with the app partition's own session on startup, on sign-in and session change, and when the browser or terminal is switched on or off. Signing out stops every action, clears the outbox and retires the id. A
409(the id belongs to another account) mints a new one.Pickup. An SSE doorbell, plus the reconciling inbox read. The inbox is read on connect, on every ring, on wake (
powerMonitorresume), when the network returns, and on Sim's reconcile timer. A missed doorbell is normal: the timer catches it inside the pickup window. Reconnects usebackoffWithJitter, and a silent stream is replaced after two missed heartbeats.Claim, then queue. A call is claimed as soon as it is offered, then queued per chat and per surface. Its lease is renewed from the claim until Sim acknowledges the result, so a backlog never misses the pickup window. Calls in one chat run in the order they were offered, and different chats run side by side.
Execution. Each call runs from Sim's record of it, in that chat's own browser scope, terminals or granted folders. This covers
browser_*,terminal,read_local_fileand user-localread/grep/glob. A surface switched off on the machine answers "not run".Outbox. An encrypted journal (
claiming→claimed→started→result), written before the step it guards, usingsafeStoragelike the other userData stores. After a restart:Nothing runs twice. Results are retried until Sim answers. Any answer acknowledges the result: recorded, duplicate or superseded. A refused token (
404/410) is final and is dropped.Stop. An inbox
cancel, or a410on lease renewal, stops the action through the browser's own cancel. Terminal runs get a real cancel: Ctrl-C, then SIGTERM, then SIGKILL to that command's own process group, with the shell left running. It also reachesinput,killand paneclose, between keystrokes. A tmux run gets Ctrl-C in its own window, and the window is closed if that does not end it.Agent commands end with the session. Sign-out, an account change, and switching Terminal off each stop every command the agent started, including a
runwhose wait window has passed and any tmux run window, before the shells go. A command the user started is never touched: only commands arunstarted count as the agent's.Approvals. A chat in the background that waits on the user's approval raises a native notification. The notification only opens the chat at its approval card. It closes once the call is decided, and it never shows the command, since it can appear on a locked screen.
Turn binding. The preload exposes
desktopExecutor.getDevice(), so the composer from feat(desktop): bind turns to a desktop and enforce its deadlines from the row #8650 binds turns. It returns null while Sim reports the executor off.Shared projections. The browser, terminal and local-filesystem result projections move from the web app into
@sim/desktop-bridge(tool-results,local-filesystem-tools). The chat view and the executor now build identical model-facing results. The renderer behaviour is unchanged; its existing tests pass as they are. The one code path removed, aREJECTEDterminal code mapped tocancelled, was dead: nothing produces that code.Not in this PR
import_local_filesin the background is answered as "not run" here; feat(desktop): import local files from the background executor #8672 adds the upload route and the runner.Test plan
Unit tests, written first and red before the code existed:
413compaction, superseded and duplicate results, Stop of a queued call and of a running call,410revocation, crash recovery,401re-registration, the claim cap, pause on suspend, and write-ahead ofstarted;inputandkill;Guards were checked by mutation: write-ahead off fails its test, and terminal cancel or journal recovery off fails E2E scenarios E and C.
Packaged-app E2E (
apps/desktop/e2e/background-executor.spec.ts), run with Playwright_electronagainst a fixture Sim that speaks the device protocol. The JSON report goes toBACKGROUND_EXECUTOR_REPORT_PATH, wired indesktop-e2e.yml.read_local_file. Meanwhile the window navigates to chat C, then to another workspace, then fully reloads. Every call completes exactly once with its own token. The page sees exactly 10 clicks, and the commands run once each, in order. Chat B cannot see chat A's tabs. Nothing touches/api/copilot/confirmor/authorize. p95 pickup is under 1.5 s.browser_wait_forand asleepcommand. The process is gone, and both results are acknowledged as superseded.All 7 scenarios pass. With the executor not started, all 7 fail.
Full gate on the rebased branch:
lint:check,type-check,check:audits,test,docs-manifest:checkand the block registry check. Under heavy machine load, a few unrelated timing-bound tests (the route inventory, markdown perf) time out; each passes when run on its own.