fix(desktop): inbox persistence order, desktop-only overdue sweep, and ungated pickup windows by waleedlatif1 · Pull Request #8685 · simstudioai/sim

waleedlatif1 · 2026-10-06T18:14:46Z

Summary

Follow-up to #8650: three findings from its final review, plus an ordering bug in the desktop inbox.

The overdue sweep only touches desktop calls. The sweep, the per-call deadline read and the inbox filter with a SQL form of isDesktopToolCall before their limits: a desktop tool by name, or a read/grep/glob under user-local/. A pending workflow call, or a VFS read of Sim's own files, on a device-bound run is never failed with the desktop's "did not pick it up" result, and such rows can't fill a sweep batch. settleOverdueDesktopToolCall also checks isDesktopToolCall on the call itself.
The cron is bounded without forgetting anything. It starts from the few unsettled (pending or running) calls on the status index, in persistence order, within its batch limit. It has no time horizon, so a call the backstop missed for days is still settled, and a long-lived run's call is reached whatever the run's age.
A device gets its calls in the order they were persisted. The inbox ordered by created_at, then tool_call_id. Calls of one turn can be persisted in the same millisecond, and then the tie broke on random ids, so a device could run a click before the type the model emitted first. Calls now carry persist_seq, a strictly increasing number assigned on insert; pre-persist writes calls in emission order. The inbox and the overdue sweep order by it.
- Migration 0399 is forward-only and rewrites nothing. It creates the sequence, adds the column with no default, then sets the default (new rows only) and ties the sequence to the column. Rows persisted before it have no position and sort first, as the oldest. check:migrations passes.
- Autopsy: the order tests relied on wall-clock spacing between inserts, which usually puts each call in its own millisecond, so the tie only showed up intermittently in CI. The new test persists three calls with the same created_at, with ids that sort in reverse, so the timestamp and the id both give the wrong order.
A decision clears only a gated call's pickup window. recordToolPermissionDecision cleared pickup_deadline_at for any call. It now clears it only when permission_requested_at is set, so a call that was never gated keeps the window it was offered with. A decision posted for an ungated call is still recorded, as before; only the deadline is left alone. This avoids refusing decisions for calls gated before the marker existed, which fix(mothership): refuse desktop claims for unapproved or stopped calls #8643's predicate also accepts.

Test plan

bound-turn.integration.ts:
- a bound run's pending run_workflow call and its Sim-files read call both survive the stale-execution cron;
- 201 overdue Sim-file reads older than an abandoned desktop call don't stop the cron settling it;
- a run that started two days ago still has its recently overdue call settled, and a call whose window lapsed two days ago is still settled;
- a decision posted for an ungated, offered call leaves its pickup deadline unchanged, and the device can still claim and complete it.
Red before the fix: removing the isDesktopToolCall check fails the Sim-files test, clearing the deadline unconditionally fails the ungated-call test, and the batch-starvation and old-run tests fail on the previous head.
executor.integration.ts: three calls persisted in the same millisecond are listed in persistence order. Red when the inbox orders by timestamp, then id.
All suites under lib/desktop, lib/mothership/tools/client and lib/mothership/async-runs pass (101 tests).
Gate: lint, type-check, check:audits, check:migrations, docs-manifest:check, block registry and drizzle-kit generate pass. In bun run test, the one failure was the route-inventory test timing out under machine load; it passes run on its own.

…ed pickup windows - The overdue listing and the per-call deadline read only consider desktop tool calls, and the settlement checks the call is one the desktop runs, so a bound run's workflow call or Sim-files VFS read is never failed with the desktop's not-started result. - The cron scans only runs inside the inbox's horizon, oldest calls first, within its batch limit. - Recording a decision clears the pickup deadline only for a call that was gated; a call that was never gated keeps the window it was offered with.

waleedlatif1 · 2026-10-06T18:14:52Z

@greptile

vercel · 2026-10-06T18:14:53Z

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

1 Skipped Deployment

Project	Deployment	Actions	Updated
docs	Skipped		Oct 6, 2026 7:29pm UTC

waleedlatif1 · 2026-10-06T18:14:54Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-10-06T18:14: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 5 files

_{Reply with feedback, questions, or to request a fix.

Fix all with cubic | Turn on auto-fix | Re-trigger cubic}

cubic-dev-ai

All reported issues were addressed across 5 files

_{Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.

Turn on auto-fix | Re-trigger cubic}

greptile-apps · 2026-10-06T18:20:21Z

Confidence Score: 5/5

[Critical risk] Database schema migration and tool call ordering logic.

The PR appears safe to merge based on the reviewed changes.

Summary

The PR narrows desktop inbox and overdue recovery to calls the desktop runs, adds a sequence to preserve tool-call persistence order, and retains pickup deadlines for ungated calls.

The previously reported sweep filtering and recovery-window issues are addressed.
No new actionable issue was identified in the changes since the previous review.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Persist tool call] --> B[Assign persist_seq]
  B --> C{Desktop tool or user-local VFS call?}
  C -->|Yes| D[Desktop inbox, ordered by persist_seq]
  C -->|Yes, overdue| E[Bounded recovery sweep]
  C -->|No| F[Non-desktop execution path]

_{Reviews (5) · Last reviewed commit: "test(mothership): give the hand-built to..."}

…und it by deadline The sweep's tool-name filter admitted read, grep and glob calls on Sim's own files, which the settlement then skipped, so enough of them could fill every batch and starve real desktop calls. Queries now use the SQL form of isDesktopToolCall (a desktop tool by name, or a VFS read of a granted local folder) before their limit. The sweep's horizon is now on the deadline that lapsed, not on the run's start: Sim does not enforce a run's wall clock, so a long-lived run's recently overdue call is still settled.

waleedlatif1 · 2026-10-06T18:45:12Z

@greptile

waleedlatif1 · 2026-10-06T18:45:13Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-10-06T18:45:25Z

@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 6 files

Confidence score: 5/5

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

_{Turn on auto-fix | Re-trigger cubic}

The inbox ordered calls by created_at, then tool_call_id. Calls of one turn can be persisted in the same millisecond, and then the tie broke on random ids: a device could run a click before the type the model emitted first. Calls now carry persist_seq, a strictly increasing number assigned on insert (pre-persist writes them in emission order), and the inbox and the overdue sweep order by it. The migration adds the column without a default and then sets the default, so existing rows are not rewritten; rows persisted before it have no position and sort first, as the oldest.

waleedlatif1 · 2026-10-06T19:01:05Z

@greptile

waleedlatif1 · 2026-10-06T19:01:06Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-10-06T19:01:12Z

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

…op tool names as const A 24 h horizon on the lapsed deadline meant a call the backstop missed for a day was never settled. The sweep starts from the few unsettled (pending or running) calls in persistence order, so it needs no horizon to stay small. The desktop tool names are a literal array declared as const, and the lookup set is derived from it.

waleedlatif1 · 2026-10-06T19:04:45Z

@greptile

waleedlatif1 · 2026-10-06T19:04:46Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-10-06T19:04:53Z

@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 12 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}

… sequence column

waleedlatif1 · 2026-10-06T19:29:19Z

@greptile

waleedlatif1 · 2026-10-06T19:29:20Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-10-06T19:29:26Z

@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 13 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}