Skip to content

fix(mcp): preserve open viewer pane when the app acknowledgement is lost (SW-10) - #30752

Merged
teamleaderleo merged 3 commits into
feat-cmux-nextfrom
sw10/mcp-open-preserve-uncertain-20261101
Nov 2, 2026
Merged

teamleaderleo merged 3 commits into
feat-cmux-nextfrom
sw10/mcp-open-preserve-uncertain-20261101

Conversation

@teamleaderleo

Copy link
Copy Markdown
Collaborator

SW-10 — Preserve an MCP viewer when the app applied open but its acknowledgement was lost

Fixes the outstanding correctness review on merged #29080.

What was wrong

cmux mcp open({kind:"file", where:"below", ...}) first splits a pane, then issues the app's action.run to open a viewer. The MCP app transport can return state: "in_progress" when a reply is lost after the app accepted the action. open_tool.rs::call_open used to treat every isError:true as a definitive refusal and immediately pane_close its split, potentially deleting the viewer that successfully opened.

The same danger exists when a sticky-column update's response is uncertain.

Fix

  • Introduce failed_open, which checks the existing typed MCP failure state.
  • On in_progress, preserve the created pane; include created_pane, the original app idempotency_key (if present), and a recovery instruction in the MCP error receipt. The content text and structuredContent are generated from the same envelope.
  • On a confirmed refusal and other definite errors, preserve the existing undo_split cleanup behavior.
  • Use this handling for viewer-open and sticky-column errors, and the page-tab lookup path.

This is a deliberately bounded correctness repair. The separate larger improvement—stable whole-open IDs, guarded cleanup after user content arrives, and resumable multi-stage split+open—remains open. This patch refuses to destroy a possibly successful viewer when an acknowledgement is uncertain.

Test-first verification

The first commit (af641e542a63c7007a3ba4e83ff16285f82bfd51) adds an explicit lost acknowledgement after action.run scenario to the existing MCP open integration-style unit test. It asserts: in_progress receipt, original action key, created pane ID, recovery guidance and zero pane.close calls. The test's original definite-refusal branch continues to assert pane cleanup. Implementation follows in ce8ce541bdf09d1d89c9775293f0c70003afa13d.

Source base: feat-cmux-next@5f817fac2d9f8afa248f8be25064570c0f7677fd. Two files changed (MCP open code and MCP tests). No duplicate action registry, transport or global idempotency implementation.

Current verification: exact GitHub code/diff reviewed. This chat cannot run Rust locally; hosted PR checks and independent review are required before landing. Suggested focused test: cargo test -p cmux-tui --lib cli::mcp::tests::open_splits_and_opens_in_one_call, then the MCP test family and panic/clippy guard on the exact head.

SW-10 issue

@blacksmith-sh

blacksmith-sh Bot commented Nov 1, 2026

Copy link
Copy Markdown

Found 6 test failures on Blacksmith runners:

Failures

Test View Logs
/tmp/tmp8kahntcl/summary-missing.json (live):/summary-missing.json (live): View Logs
/tmp/tmpfn2c3k_9/summary-fail.json (live):/summary-fail.json (live): View Logs
[new_workspaces] debug.new_workspace timed 0 of 50 new workspace rows/
[new_workspaces] debug.new_workspace timed 0 of 50 new workspace rows
View Logs
[new_workspaces] the window draws no frames/[new_workspaces] the window draws no frames View Logs
[tabs] debug.tab_switch timed 0 of 40 switches/
[tabs] debug.tab_switch timed 0 of 40 switches
View Logs
[tabs] the window draws no frames/[tabs] the window draws no frames View Logs

Fix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

cmux status (one comment, edited in place; each line is updated by the job that owns it)

Review 788a5b7: HQ feed card · gallery before/after runs · checks

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=pr30752 -f ref=sw10/mcp-open-preserve-uncertain-20261101 -f platform=macos -f backend_mode=local

red: cmux-next checks (god files, concurrency, crash safety, l10n); NEW: red check: Web bundle key inputs (+4 more) is not red on the base 30b3f65; do not merge until green; see lane-rules

Shipped in:

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Owned by an agent session · session

@teamleaderleo
teamleaderleo force-pushed the sw10/mcp-open-preserve-uncertain-20261101 branch from ce8ce54 to b340078 Compare November 2, 2026 01:33
@teamleaderleo
teamleaderleo marked this pull request as ready for review November 2, 2026 02:01
@teamleaderleo
teamleaderleo merged commit 0e9e8dd into feat-cmux-next Nov 2, 2026
113 of 119 checks passed
@teamleaderleo
teamleaderleo deleted the sw10/mcp-open-preserve-uncertain-20261101 branch November 2, 2026 02:01
teamleaderleo added a commit that referenced this pull request Nov 2, 2026
…n open retries [carry of #30752 + #30766] (#31558)

* test(mcp): keep a viewer pane when action.run applies but loses its acknowledgement (red first)

* fix(mcp): retain a newly opened pane when the app outcome is uncertain

* test(mcp): identical open IDs replay identical split and viewer action keys

* feat(mcp): reuse stable stage keys for whole open retries

* test(mcp): match the open recovery hint's actual wording

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-5cc5f183969be11a815e

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: cmux-bot[bot] <348531757+cmux-bot[bot]@users.noreply.github.com>
lawrencecchen added a commit that referenced this pull request Nov 3, 2026
The in_progress error of open keeps the app stage's idempotency_key, as
#30752's open_splits_and_opens_in_one_call asserts; the retry key question
goes to its author. Clippy: drop the unneeded super:: qualification.

This branch was previously deployed

3 inactive (2 outdated) deployments
ci-routing-agent — 788a5b7b Deployed Nov 2, 2026 by teamleaderleo via cmux-tui tree preflight #98445
artifacts — ce8ce541 Deployed Nov 1, 2026 by teamleaderleo via add the Windows daemon to the tree #95706
sentry-dsyms — ce8ce541 Deployed Nov 1, 2026 by teamleaderleo via upload cmux-tui dSYMs to Sentry #95706
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:normal Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant