[teamleaderleo]

stash.restore reopens members one at a time and only then commits their removal from the stash. When member N failed, members 1..N-1 stayed open and also stayed in the stash, so a retry opened them a second time. A restore is now all or nothing. If any member can't reopen, or the commit fails, every surface this call created is closed again and every member stays in the stash. The restore returns the original error, and a retry opens each page once.

• Tracking what was opened. Surfaces are recorded the moment a reopen creates them (closed_history::recording_created/note_created). That catches a tab whose name or pin step then fails, and the starter tab of any workspace made for the pages (target new_workspace, or a here restore with no workspace at all).
• How they close. The rollback closes them through the normal batch close as a session end, so nothing lands in Recently Closed, under a fresh key. It closes only surfaces that are still open. If another client closed one in the meantime and the batch refuses, it retries once without that surface. If the close still fails, it logs.
• Starter close key. The new_workspace starter close now takes a fresh key per attempt. It used to derive the key from the request's key, so a retry after a failed attempt hit an idempotency conflict with the first attempt's receipt. That was the documented "known limitation". A replay of a committed restore returns before this point, so it's unaffected.
• Spec. The stash.restore constraints and resource-api-v2.md now state the all-or-nothing contract. The SDK descriptors are regenerated.

Decision

Choice: roll back. The default rule is to roll back unless callers already depend on partial results, and none do.

Evidence (origin/feat-cmux-next after #21670):

• The only references to stash.restore are the resource registry (resource.rs, wire_name.rs, page_access.rs, resource_router/owner.rs), the SDK descriptors (operation class only) and the app scope tables.
• The generated app global types the result as StashRestoreResult = {stash_id, workspace_id, workspace_ids, screen_ids, tab_ids, remaining}, which has no per-member status.
• MCP and the CLI deliberately leave stash ops out (cli/mcp/v2_tools.rs).
• The macOS client and UI slices haven't landed, and no webview calls it.
• The spec already promised a single commit: "Restored members leave the stash in the commit that stores the result."

Counterargument. A review subagent argued against rollback. Its points and what I did:

1. Tabs created before a later step of their reopen fails would escape the rollback. Fixed: surfaces are recorded at creation, not when the reopen finishes.
2. A batch close refuses the whole batch if one surface is already gone. Fixed: only surfaces still open are closed, with one retry.
3. A workspace made for the pages, and its starter tab, would survive. Fixed: starter tabs are recorded too. A split pane made by split_closed_pane_again closes with its last tab, but the neighbor's split ratio may not come back exactly. That's cosmetic, and I accepted it.
4. The derived starter-close key would make a failed new_workspace restore fail forever on that key. Fixed: the key is fresh per attempt.
5. A retry loads pages 1..N-1 a second time, which can break one-time URLs. Not fixed. Stashed pages are ordinary saved URLs; one-time OAuth codes are already stripped by closed-history privacy. A partial result loads each page once, but it leaves the stash out of step with what's open unless every client handles per-member status.
6. A member that always fails blocks "restore all", which is the strongest argument for a partial result. Not fixed. The client can restore the other members by id. A partial result would need a new result shape and would break the single-commit contract above, for callers that don't exist yet.

None of these makes a partial result better on correctness. Points 1 to 4 were real bugs in the first rollback version and are fixed here; 5 and 6 are tradeoffs, and I accept them.

Testing

• cdff2a44 adds stash_restore_that_fails_partway_opens_nothing_and_keeps_every_member. It stashes three pages and breaks the middle member's record so it can't reopen. Then:
  • with targets here and new_workspace, the restore has to fail with tab and workspace counts unchanged and all 3 members still in the stash;
  • after the member is fixed, a retry has to open exactly 3 tabs and empty the stash.
• The fix is 2312fe94 (rollback, starter key, spec) plus 233d0f2d (from the review: record surfaces at creation, skip ones already gone).
• Red: at cdff2a44, hosted verify-cmux-tui-hosted.sh --filter stash_restore_that (run 39201789671) failed the test on Linux and macOS. Counts were (2, 1), expected (1, 1): the first page stayed open.
• Green: at 233d0f2d, hosted --filter stash_ (run 39202184409) passed test (linux) and macOS lint and tests, 12/12 stash tests on each. The earlier run at 2312fe94 passed on Linux. Its macOS job never ran a test, because the runner hook refused while a fleet build waited for the host.
• Final head: the hosted --filter stash_ run is in progress; I'll record the result here.
• This Linux host can't build cmux-tui-core, so it compiles only in the hosted runs.
• Merging current feat-cmux-next (eae2935) regenerated the SDK descriptors. It also regenerated cmux-app-host/generated/cmux-app.d.ts: base's copy lacks tab.update/pane fields (pinned, allow_tabs, marked_unread) that base's catalog already has, so gen-cmux-global.ts --check fails on base.
• Locally: check-resource-api-boundary.py, check-spec-inventory.py, check-sdk-schema.py, sync-resource-api.py and gen-cmux-global.ts --check pass, and rustfmt --check is clean.

Freeze WINDOW areas: CORE (touches cmux-tui-core and the resource spec). No Cargo file changes, so not LOCK. The Air Blue coordinator merges this PR.

Changelog

Fixed: restoring a stash that fails partway no longer leaves pages open that a retry opens again; the restore is all or nothing.

🤖 Generated with Claude Code

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

• Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 1e4f3bf2-3cf8-49c0-a1e6-6c4bd62f771c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Receipt for 1f67cf6 (this PR's head; a new push replaces it)

• Visual review: HQ feed card · gallery before/after runs
• Checks: this head
• App of this head (app.zip, kept 1 day): gh workflow run reload-build.yml -R manaflow-ai/cmux --ref feat-cmux-next -f tag=pr21943 -f ref=split/20706-stash-restore-atomic -f platform=macos -f backend_mode=local

[teamleaderleo]

Bead cx-pe3mi created for this PR by the cmux coordinator.

[cursor]

Bugbot needs on-demand usage enabled

Bugbot uses usage-based billing for this team and requires on-demand usage to be enabled.

A team admin can enable on-demand usage in the Cursor dashboard.

[cursor]

Bugbot needs on-demand usage enabled

Bugbot uses usage-based billing for this team and requires on-demand usage to be enabled.

A team admin can enable on-demand usage in the Cursor dashboard.

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable defect was established.

Summary

stash.restore now tracks pages and workspace starters, then closes them if reopening or the final commit fails.

• Failed stash restores close opened pages and keep every member saved.

Acknowledged choices from teamleaderleo: closing a restored split may not recover the exact neighboring split ratio; retrying may load a URL again; one failing member blocks restoring the whole stash, while callers can select other member IDs. The description also explicitly states that a rollback close that still fails is logged.

Verification here was by code inspection; no tests were run.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Restore request] --> B{Committed result exists?}
  B -->|Yes| C[Return saved result]
  B -->|No| D[Track created pages and starters]
  D --> E[Reopen selected members]
  E -->|Success| F[Commit stash removal and result]
  F -->|Success| G[Return result]
  E -->|Failure| H[Close created pages still open]
  F -->|Failure| H
  H --> I[Return original error]

Loading

_{Reviews (1) · Last reviewed commit: "Merge remote-tracking branch 'origin/fea..." · Reviewed by Greptile}

[teamleaderleo]

Merging at 1f67cf6. A correctness review found nothing blocking, and the stash tests are green. The three reds don't come from this PR:

• cmux-next checks: they fail on Swift sources this PR doesn't touch. For example, check-motion flags HoverCardPanel.swift:192 and TerminalSizedForChip.swift:98. cmux-next: finish the strings-tables move #22042 and pre-commit registers only the commit's own tests #22056 fail the same 7 checks the same way.
• Windows GNU build: crate::apps::routing is configured out in server/cloud_host_replica.rs, which came from base commit 28c1170 (object-sync).
• release-notary: red on feat-cmux-next since 4bc289e.

[github-actions]

Shipped in:

• cmux NEXT nightly 1.0.0-nightly.3932896939801 (d9cd7134d3)
• cmux NEXT nightly 1.0.0-nightly.3932981759301 (1ab289a4c2)