fix(mothership): keep local file tools running when the chat view changes by waleedlatif1 · Pull Request #8666 · simstudioai/sim

waleedlatif1 · 2026-10-06T05:01:28Z

Summary

Follow-ups to #8652, all serving "work keeps running when I leave a chat":

Local file tools keep running, and each turn's Stop reaches its own desktop tools.
- Browser actions, read_local_file, imports and user-local VFS reads share one lifetime, owned by their turn (keyed by the turn's stream id in home/hooks/desktop-tool-lifetimes.ts), not by a stream reader or a view. Only that turn's Stop ends it.
- Each running tool holds a lease on its turn and releases it when it settles. A turn is kept only while one of its tools runs, so a live turn can't be evicted and settled turns leave nothing behind.
- A history reconnect, a stream recovery or leaving the chat view never aborts a running tool: it finishes and reports.
- Stop in another chat leaves it running. Stop from a view reopened on the same turn still reaches it.
One classifier. isDesktopExecutedToolCall (client-executed-tools.ts) duplicated isDesktopToolCall. It's removed, and the stream tool handler and isClientExecutedToolCall use the single classifier in lib/mothership/tools/desktop-tools.ts.
A report the server refuses as final is final.
- When the desktop app holds a call (claimed, or a report raced its claim and lost, or a not-started report arrives for a claimed call), /api/copilot/confirm now answers 409 instead of 404.
- The completion reporter treats 409 as final, including on the trimmed retry of an oversized report, instead of retrying five times with backoff and logging an error.
- 404s are unchanged, because some workflow 404s are transient and still worth retrying.
Model-actionable not-started wording. The results say what is known: nothing in the Sim desktop app picked the action up and nothing happened; don't retry it in this turn; ask the user to keep this chat open in the Sim desktop app or ask again later. For a local read whose result was lost, the message follows the stored claim: one the desktop claimed is described as started; one with no claim (an older desktop reads without claiming) as possibly never started, and reading changed nothing. The result shapes (notStarted, outcomeUnknown + doNotRetry) are unchanged.
Obsolete probe deleted. scripts/probes/mothership-request-admission.ts called removed repository APIs (stopPendingRequest, owner-token-less claims, old signatures) and built its schema from the 0317 snapshot. Nothing ran or referenced it except an explicit-any baseline entry, which shrinks. The same admission behaviour is covered against real Postgres by orphaned-runs.integration.ts, stream-recovery.integration.ts and desktop-tool-authorization.integration.ts.

Not included: persisting desktopClaimsLocalReads in the recovery config. StreamRecoveryConfigSchema is .strict(), so during a rolling deploy an older pod would reject a recovery config carrying the new field and fail to recover the turn. Doing it safely takes two deploys: readers accept the optional field first, writers send it later.

Type of Change

Bug fix

Testing

use-chat.dom.test.tsx: the in-flight desktop action suite now runs for both a browser action and a read_local_file. Recovery and unmount leave it running; Stop cancels it, including after a recovery and from a view reopened on the turn; Stop in another chat leaves it running. Before this change the local-file recovery and unmount cases fail; the post-recovery, reopened-view and other-chat cases fail for both kinds on the intermediate view-owned design.
desktop-tool-lifetimes.test.ts: Stop cancels every running tool of its turn and no other turn; a turn with a running tool stays reachable however many other turns come and go (fails under a bounded LRU); a settled turn's next tool gets a fresh lifetime; a tool settling after Stop can't release a newer lease.
completion.test.ts: a 409 settles the report at once, including on the oversized-payload retry, with no retry backoff. Both fail before.
confirm/route.test.ts: the held-call refusals answer 409. desktop-tool-pickup.integration.ts asserts the stale not-started report gets 409 against real Postgres.
executor.test.ts: a claimed local read whose result was lost is called started; an unclaimed one isn't. Both fail without their guard.
Reverted each guard on its own and watched its test go red.
bun run lint, bun run type-check, bun run check:audits, bun run test, bun run test:integration, bun run docs-manifest:check, block-registry check: all pass.

Checklist

Code follows project style guidelines
Self-reviewed my changes
Tests added/updated and passing (new tests pass the test-audit authoring gate)
No new warnings introduced
I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

…nges Local file reads and imports now take the same Stop-only lifetime as browser actions: a history reconnect, a stream recovery or leaving the chat view leaves them running so they finish and report, and only the user's Stop cancels them. The desktop-tool classifier in client-executed-tools.ts is removed in favour of the single one in tools/desktop-tools.ts. When the desktop app holds a call, confirm now answers 409 to any other reporter (a stale replay, a lost race against the claim), and the client treats 409 as final instead of retrying a 404 five times. The not-started results now tell the model what it can act on: the action never started; don't retry it in this turn; ask the user to keep the chat open in the Sim desktop app. A local read the server cannot see picked up is no longer described as started. The obsolete admission probe script is deleted. Nothing ran it, it no longer type-checked against the repository API, and the integration suites cover the same admission behaviour against real Postgres.

vercel · 2026-10-06T05:01:31Z

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated
docs	Skipped		Oct 6, 2026 6:35am UTC

waleedlatif1 · 2026-10-06T05:01:33Z

@greptile

waleedlatif1 · 2026-10-06T05:01:34Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-10-06T05:01:39Z

@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

All reported issues were addressed across 19 files

_{Fix all with cubic | Re-trigger cubic}

cubic-dev-ai

All reported issues were addressed across 19 files

_{Fix all with cubic | Re-trigger cubic}

greptile-apps · 2026-10-06T05:07:22Z

Confidence Score: 5/5

[Medium risk] Refactors how desktop tools persist across chat view changes.

The PR appears safe to merge; no outstanding finding or new issue remains.

Summary

The PR moves desktop-tool cancellation to turn-scoped leases, lets local file and browser work survive chat-view changes, and treats server refusals for claimed calls as final. Since the previous review, the test import was changed to the repository’s absolute-path convention.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Turn[Chat turn and stream ID] --> Lease[Desktop-tool lease]
  Lease --> Tool[Browser or local-file tool]
  View[Chat view or stream reader] -->|May change without aborting| Tool
  Stop[Stop for this turn] -->|Aborts| Lease
  Tool -->|Settles and releases| Lease

_{Reviews (5) · Last reviewed commit: "test(mothership): import the lease regis..."}

…ults say what is known Desktop tools now take one lifetime the view owns and only the user's Stop ends, so Stop still reaches a tool that a replaced stream reader started. A held-call 409 is final on the trimmed retry of an oversized report too. The not-started result no longer asserts why nothing picked the call up, and a local read the desktop claimed is described as started when its result is lost.

waleedlatif1 · 2026-10-06T05:25:51Z

@greptile

waleedlatif1 · 2026-10-06T05:25:52Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-10-06T05:25:59Z

@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

All reported issues were addressed across 19 files

_{Fix all with cubic | Re-trigger cubic}

A desktop tool outlives the chat view that started it, so its Stop belongs to the turn, not the view: tools are keyed by the turn's stream id, which survives reader replacement and remounts and differs between chats. Stop in another chat leaves them running, and Stop from a view reopened on the turn still reaches them.

waleedlatif1 · 2026-10-06T05:39:34Z

@greptile

waleedlatif1 · 2026-10-06T05:39:35Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-10-06T05:39:41Z

@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

All reported issues were addressed across 20 files

_{Fix all with cubic | Re-trigger cubic}

Replace the bounded LRU of turn controllers with leases: each running browser action or local file tool holds its turn's Stop until it settles, so a live turn can never be evicted and settled turns leave nothing behind.

waleedlatif1 · 2026-10-06T06:27:37Z

@greptile

waleedlatif1 · 2026-10-06T06:27:38Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-10-06T06:27:44Z

@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

No issues found across 23 files

Confidence score: 5/5

Automated review surfaced no issues in the provided summaries.
No files require special attention.

_{Re-trigger cubic}

waleedlatif1 · 2026-10-06T06:35:13Z

@greptile

waleedlatif1 · 2026-10-06T06:35:14Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-10-06T06:35:20Z

@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

No issues found across 23 files

Confidence score: 5/5

Automated review surfaced no issues in the provided summaries.
No files require special attention.

_{Re-trigger cubic}

… folders, and bound the outbox in bytes - Stop reaches `input`, `kill` and pane `close`, not just `run` and `handoff`. A Stop that lands while the session resolves means none of them starts. A batch of keys or lines stops between keystrokes and says some input may already have arrived. - A terminal call stopped while it waited for the chat's previous operation never reaches the terminal. - Sign-out stops running actions before it waits on a reconcile in flight. - The outbox budgets result data in UTF-8 bytes, so a journal of non-ASCII results still reads back after a restart. - A local read, grep, glob, list or stat answers only while its folder is still granted when it finishes. This covers the chat view's path as well. - Grep says when its results are truncated, as glob does. - A doorbell stream that keeps closing as soon as it opens backs off instead of reconnecting every second. - The not-started results follow #8666's wording: what happened, that nothing ran, and what the model should do instead of retrying.

waleedlatif1 requested a review from a team as a code owner October 6, 2026 05:01

vercel Bot deployed to Preview October 6, 2026 05:03 View deployment