🤖 feat: share caller desktops with delegated desktop agents - #4081
🤖 feat: share caller desktops with delegated desktop agents#4081ThomasK33 wants to merge 27 commits into
Conversation
Task schema and persistence tests pass; desktop coordinator and lifecycle implementations follow in scoped integration commits. --- _Generated with [`mux`](https://github.com/coder/mux) • Model: `coder:openai/gpt-6-astra` • Thinking: `high`_
Resolve persisted desktop owners from current ancestry and serialize durable admission with input. Keep sessions owner-keyed and tokens requester-bound. Validation: 49 desktop tests and touched-file formatting pass. Typecheck and typed lint await the parent-owned taskDesktopOwnerWorkspaceId schema; typecheck also exposes the existing workspaceService.test.ts Config stub error.
Acquire distinct owner gates in sorted order and validate the full batch before its durable admission callback. Reject conflicting borrowers within one batch and delegate singleton reservations to the same path. Validation with parent schema dependencies: 51 desktop tests, touched-file ESLint/formatting, and main TypeScript config pass. Full project typecheck has only the existing workspaceService.test.ts Config stub error.
Keep shared-target metadata tied to the caller bootstrap, clear it on connection teardown, and remount viewers on workspace switches. Add full-app responsive stories, binding lifecycle tests, and bound-desktop agent guidance.
Regenerate bundled task and agent guidance. Focused desktop lifecycle tests, lint, formatting and main-process typecheck pass. Full typecheck remains blocked by the existing getSessionDir Config test fixture. --- _Generated with [`mux`](https://github.com/coder/mux) • Model: `coder:openai/gpt-6-astra` • Thinking: `high`_
Replace the action/screenshot timeout race with execFileAsync process-tree termination and its close-backed result. A SIGTERM-resistant input command can no longer outlive action completion and the desktop input gate. The deterministic regression failed with a live PID after action completion before the fix and passes after it. All 52 desktop tests, scoped lint and formatting, and main typecheck pass with parent schema dependencies applied.
The fixture already uses a real HistoryService and never reads this removed method. Remove the stale stub to restore the local typecheck gate. --- _Generated with [`mux`](https://github.com/coder/mux) • Model: `coder:openai/gpt-6-astra` • Thinking: `high`_
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
xum/src/node/services/taskService.ts
Line 10462 in 170dffc
Restore the stable status after a reported child continuation
When a reported shared-desktop child is reawakened through task_send_message, this branch changes its stable taskStatus from reported to running, even though the workspace-turn execution mirror already reserves the desktop. Normal workspace-turn settlement only transitions taskExecutionStatus to a terminal state; it does not restore taskStatus. Consequently DesktopInputCoordinator.isActive() continues treating the child as the active borrower after the continuation completes, permanently blocking owner input and subsequent shared children until an explicit stop or restart. Preserve reported for correlated workspace turns or restore it when that execution settles.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Avoid eager rejected Promises escaping before asynchronous record scanning reaches the mocked recovery calls. Both per-record recovery tests now exercise the existing production catches and pass without changing recovery behavior. --- _Generated with [`mux`](https://github.com/coder/mux) • Model: `coder:openai/gpt-6-astra` • Thinking: `high`_
|
@codex review Latest head: Generated with |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…wedge the owner desktop
This comment has been minimized.
This comment has been minimized.
Pixel can prepend non-layout nodes to its story canvas, so firstElementChild is not necessarily the 390px wrapper. Keep the responsive width and overflow assertions but select the actual frame. Both desktop story plays pass locally; static-check is green. --- _Generated with [`mux`](https://github.com/coder/mux) • Model: `coder:openai/gpt-6-astra` • Thinking: `high`_
|
@codex review Please review current head Generated with |
This comment has been minimized.
This comment has been minimized.
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Resolve duplicate sidebar fixture keys introduced by the merge with main. Dispose the monitor reconciler before removing each test session so accepted wakes cannot leak into the next test’s fixtures. Validation: static-check and static-check-full passed; 752 desktop/lifecycle tests and 8 workspace desktop tests passed. The reproduced monitor failure passed 3,000 stress tests on CI Bun 1.3.5 after teardown cleanup. --- _Generated with `xum` • Model: `coder:openai/gpt-6-astra` • Thinking: `high` • Cost: `$298.06`_ <!-- mux-attribution: model=coder:openai/gpt-6-astra thinking=high costs=298.06 -->
Bring in the startup lifecycle change that landed while validating the prior merge. The combined branch passes make static-check and all 84 startup/desktop tests. --- _Generated with `xum` • Model: `coder:openai/gpt-6-astra` • Thinking: `high` • Cost: `$298.06`_ <!-- mux-attribution: model=coder:openai/gpt-6-astra thinking=high costs=298.06 -->
|
@codex review Please review current head Generated with |
|
@codex security review Please review current head Generated with |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66f454eeb2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Revalidate task creation, reactivation, and plan handoff against the locked config snapshot so independent backends cannot persist competing desktop borrowers. Keep process-local input gates and use the existing cross-process config lock. Validation: deterministic two-backend task creation reproduced two successful reservations before the fix and one afterward; 600 task, execution, and coordinator tests passed; make static-check passed. --- _Generated with `xum` • Model: `coder:openai/gpt-6-astra` • Thinking: `high` • Cost: `$308.58`_ <!-- mux-attribution: model=coder:openai/gpt-6-astra thinking=high costs=308.58 -->
Watch the config parent directory while viewers are pending or connected, revalidate their persisted requester/owner and session bindings on changes, and fail closed if watching fails. Retain immediate local revocation and close the validation-to-watch-install race before starting TCP connection. Real cross-backend filesystem tests run the backend under Node, matching production, to avoid Bun idle fs.watch event-loss artifacts. All 72 desktop tests, scoped lint/format checks and both TypeScript configurations pass.
|
@codex review Please review current head Generated with |
|
@codex security review Please review current head Generated with |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb334a2401
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Extend the owner gate with the existing transient process-file-lock primitive so another backend cannot admit a new controller before an in-flight input finishes. Keep config as the ownership ledger and retain sorted multi-owner acquisition, automatic release, and unlocked screenshots/viewers. Validation: both cross-backend input/admission regressions failed before the change and pass afterward; 678 combined regressions, 420 coordinator stress cases, make static-check, and independent review passed. --- _Generated with `xum` • Model: `coder:openai/gpt-6-astra` • Thinking: `high` • Cost: `$336.29`_ <!-- mux-attribution: model=coder:openai/gpt-6-astra thinking=high costs=336.29 -->
|
@codex review Please review final head Generated with |
|
@codex security review Please review final head Generated with |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
This comment has been minimized.
This comment has been minimized.
1 participant
Summary
Delegating to a desktop specialist now changes the operator, not the computer. New
desktopagents share the caller's effective desktop by default;desktop: "isolated"opts into a separate desktop independently of checkout isolation. Other agents and legacy saved children retain isolated behavior unless explicitly bound.Implementation
Config.getSessionDirmock in an existing pin-order fixture so the local typecheck gate can run.Validation
633c64a78:make static-checkand 678 combined desktop/bridge/task/recovery/viewer tests passed. New regressions cover competing admissions from two backend instances, external archive/removal over real Node-hosted WebSocket/TCP bridges, and orphan-mirror recovery. The owner gate additionally passed 420 stress cases, including input held open against a second backend's admission. Earlier merge integration also passedmake static-check-fulland 3,000 monitor-test stress repetitions.Scope and risks
The original process-local gate was extended during review with the existing transient process-lock primitive: global handoff must wait for input running in another backend. Locks are released after each operation; config remains the only controller ledger.
This coordinates Mux desktop-tool input, not human noVNC input, shell commands, or external CDP clients. Desktop-process/browser-state persistence across backend restarts is not promised. No retargeting/migration of legacy children, profile copying, lease service, or changes to checkout-isolation semantics.
Dogfood evidence
Shared child and parent operate the same browser state:
Parent remains usable after recovery and child UI Stop:
Shared/isolated comparison walkthrough:
review-walkthrough.webm
Reawakened UI Stop followed by restored parent control (two sequential short clips):
ui-stop-review.webm
📋 Implementation Plan
Share the current desktop with delegated desktop agents
Goal and recommendation
Delegating computer use should change who operates the computer, not which computer is operated. Newly created desktop specialists should use the caller’s existing desktop by default; independent GUI testing should request an isolated desktop explicitly.
Implement one shared desktop-target resolver and a short-lived input gate, using existing task status as the source of control ownership. Keep repository isolation independent. Estimated net product-code change: +400–650 LoC, excluding tests, stories, agent instructions, and documentation. No new lease daemon, heartbeat, or PortableDesktop fork is needed.
Verified starting point
Investigation used clean HEAD
2a1b5b267c0133f3d89d023a55d1c2b725d70386.taskService.tscreates a distinct child workspace ID even withisolation: "none"; that flag shares only the checkout.desktopTools.tspasses the caller’s workspace ID toDesktopSessionManager, which keys sessions/startup promises by that ID.PortableDesktopSessioncreates a separate state file and virtual display for each session.DesktopPanel→useDesktopConnection→desktop.getBootstrap. Tool-only redirection would leave the viewer wrong.close(workspaceId)participates in archive/removal cleanup. Aliasing it to the parent would destroy the borrowed desktop.taskStatus; reawakened executions also usetaskExecutionIdandtaskExecutionStatusthroughWorkspaceTurnManager.Behavior contract
task(agentId: "desktop")task(..., desktop: "shared")task(..., desktop: "isolated")desktop: z.enum(["shared", "isolated"]).nullish()for sub-agent tasks. Reject this option on fullkind: "workspace"launches rather than ignoring it.desktopsupplies the default; do not infer it from the presence of individual tools or broaden agent-inheritance machinery. Custom agents opt in explicitly.taskDesktopOwnerWorkspaceIdon shared children. Absence means self-owned/isolated, avoiding redundant mode state. Store the flattened actual owner, derived server-side from the caller—not a model-supplied workspace ID.isolation: "fork"andisolation: "none"retain their current filesystem meanings in either desktop mode.n > 1with shared targeting before creating any child. Parallel GUI work must explicitly use isolated desktops.Phase 1 — Target identity, routing, and lifetime
Estimated net product code: +150–230 LoC.
src/common/utils/tools/toolDefinitions.tsandsrc/node/services/tools/task.ts; thread the option intoTaskService.create.src/common/schemas/project.tsand necessary workspace type/metadata mappings. Preserve it through config reads/writes, queued materialization, and reawakening. Keep legacy entries unmodified.ToolConfiguration. Use current persisted metadata to validate requester, owner, and ancestor relationship; reject missing/archived owners, cycles, unrelated targets, and unsupported runtimes without provisioning a fallback session. Treat invalid persisted data as an operation error, not a startup crash.has/closeowner-keyed. Closing/removing a shared child must never close the owner’s session. Recheck requester and owner lifecycle state across asynchronous startup/admission, retaining existing archive-vs-startup protection.desktopOperations.ts,DesktopTokenManager.ts/DesktopBridgeServer.tsonly as necessary: retain requester identity in bootstrap tokens, bind them to the resolved session, and validate the current requester→owner relationship at connection time. Do not let a stored owner ID become arbitrary cross-workspace authority.Gate: regression tests prove one PortableDesktop startup for parent + shared child, correct child viewer targeting, explicit isolation, legacy stability, and owner-only teardown. Do not proceed with tool-only sharing.
Phase 2 — Safe agent-input handoff
Estimated net product code: +210–350 LoC.
Add a small
DesktopInputCoordinatorinsrc/node/services/desktop/, shared by task admission andDesktopSessionManager.action. It needs only config and per-owner scoped mutexes—not a second task lifecycle state machine. Provide it early in DI (src/node/services/di/layers/core.ts/desktop.ts) toWorkspaceService,TaskService,WorkspaceTurnManager, andDesktopSessionManager; do not introduce a dependency from task creation back into the manager’sWorkspaceServicedependency.taskStatusis queued/starting/running/awaiting_report ortaskExecutionStatusis queued/starting/running. A terminal old execution mirror must not mask an active direct-message turn. Reuse existing status predicates where appropriate and test this combination explicitly. Multiple apparent controllers fail closed.TaskService, active mirrors inWorkspaceTurnManager.updateAgentTaskExecutionState, direct childWorkspaceService.sendMessagetransitions, andTaskService.recoverInterruptedTasks. Retrying an already-reserved child is idempotent, not a self-conflict. Release the gate before nested service calls such assendMessagethat also use admission; never recursively acquire it.updateAgentTaskExecutionState. Its active-state branch currently replaces the mirror, so admission must also reject superseded handles rather than reviving them. Test stale active updates and stale terminal callbacks against a newer continuation; an old in-flight input must finish before another controller can begin.recoverInterruptedTaskscan resume/requeue tasks, not merely mark them interrupted. Reconcile reservations before admitting input: a resumed child retains control, a terminal child releases it, and malformed competing reservations fail closed with an actionable stop/recovery path. No silent desktop recreation when the owner is gone.Gate: deterministic concurrency tests hold an input command open while admission races it. Prove no overlap, no partial grouped launch, correct reawakening/stop behavior, and no deadlock or permanent lockout after failure/restart recovery.
Phase 3 — Make the target visible
Estimated net product code: +40–70 LoC.
DesktopPanel.tsx/useDesktopConnection.ts, connect through the caller’s bootstrap and show a compact “Shared desktop · ” label when applicable. Both workspace viewers must show the same live desktop. Reuse existing UI patterns; no new settings page or controls.src/node/builtinAgents/desktop.mdand task parameter guidance: operate the bound desktop; explicitly request isolation for independent tests; perform dependent screenshot/action steps sequentially; report which desktop changed. Direct tools remain valid for small actions in the parent.Gate: add/update a full-app desktop story and UI coverage for the shared label and correct viewer target. Check desktop and ~375–390 px widths, with a pinned Pixel viewport where needed. No overflow or misleading isolated child screen.
Acceptance criteria and automated validation
Extend behavior-based tests in:
src/node/services/desktop/DesktopSessionManager.test.ts,DesktopTokenManager.test.ts, andDesktopBridgeServer.test.ts; colocate pure resolver/input-coordinator tests if extracted.src/node/services/tools/desktopTools.test.tsand task tool tests.src/node/services/taskService.test.ts,workspaceTurnManager.test.ts, and targeted desktop cleanup cases inworkspaceService.test.ts.Run focused tests after each phase, then a final pass after all edits:
Also run the added task-tool/UI tests and relevant Storybook interaction test using the repo’s harness. Report unrelated baseline failures separately; do not call validation successful if required gates are blocked.
Dedicated dogfooding and evidence
Use a separate sandbox, never the currently running Xum instance or another worktree.
make dev-server-sandbox DEV_SERVER_SANDBOX_ARGS="--clean-projects"as a bounded background task; use its freshXUM_ROOTand printed free ports. Add a disposable local project and enableportable-desktop. Keep provider credentials private. Confirm the real PortableDesktop binary is available; a shim test is not a substitute.agent-browser’s current core/recording instructions, connect to the sandbox app, and begin a video recording. Open the parent Desktop viewer and establish recognizable browser state (coder.com plus a harmless local test page with an editable field).desktopwithout a desktop option. Have it modify the local test page. Capture parent-before, child-during, parent-after screenshots, and show both viewers displaying the same state. Repeat with the other checkout-isolation choice.$HOME/.cache/xum-dogfood/general-9bks/, attach them with a short reproduction log, and record the validated revision. Evidence must show the parent screen, not only the child’s self-report.No PR is requested. If GitHub publication is later authorized, upload screenshots/video with native
gh ... --attachand include the implementation plan.Deliberate exclusions
No workspace-wide sharing default, automatic merging of desktop state, live retargeting of old children, browser-profile copying, shell/CDP environment changes, new lease service, or broad task lifecycle refactor. The changes share an existing desktop session; they do not merge agent reasoning history or turn separate desktops into one.
Generated with
xum• Model:coder:openai/gpt-6-astra• Thinking:high• Cost:$336.29