MCP: ack dialog-close on mcp-dialog-closed, not pane gen

Real-MCP testing of the ack contract caught two false negatives the
unit + Playwright coverage missed:

- `dialog close <confirmation>` (mkdir / mkfile / transfer / delete):
  the close path waited on `GenerationAdvanced`, but cancelling a
  confirmation dialog doesn't touch pane state, so the bump never
  arrives. Every cancel returned `ActionNotAcknowledged` even though
  the dialog actually closed.
- `dialog close settings`: the Rust side emitted `mcp-settings-close`
  to the settings webview but `routes/settings/+page.svelte` had no
  listener. Tool returned `ActionNotAcknowledged` AND the window
  stayed open — straight-up broken, not a false negative.

Add a new `AckSignal::SoftDialogDisappeared(id)` that waits for the
`SoftDialogTracker` membership to drop (the `notifyDialogClosed` IPC
from `ModalDialog.svelte` is the trustworthy signal). Wire the
soft-dialog-close path in `dialogs.rs` to it. Add the missing
`mcp-settings-close` listener with the same two-rAF defer as the
production Escape handler, so the close behaves consistently regardless
of trigger.

Verified end-to-end via the real MCP transport: settings open/close
in ~150 ms; mkdir/mkfile/transfer/delete confirmation cancels in
~130–170 ms; all 176 MCP unit tests + clippy + rustfmt + oxfmt +
svelte-check green.

Author: vdavid

apps/desktop/src-tauri/src/mcp/CLAUDE.md

@@ -67,11 +67,14 @@ The mechanism lives in `executor/ack.rs`. Each tool:
 | ------------------------ | ----------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------- |
 | `GenerationAdvanced`     | `PaneStateStore.generation` is strictly greater than the captured value | Anything that mutates pane state (`select`, `set_view_mode`, `sort`, `toggle_hidden`, `tab`, `nav_*`, auto-confirmed `copy`/`move`/`delete`, `dialog confirm`). NOT `refresh` — see TODO note below.                                  |
 | `SoftDialogAppeared(id)` | A soft dialog with that ID is in `SoftDialogTracker`                    | Confirmation dialogs from `copy`/`move`/`delete` (autoConfirm: false), `mkdir`, `mkfile`; `dialog open about`                    |
+| `SoftDialogDisappeared(id)` | A soft dialog with that ID is no longer in `SoftDialogTracker`        | `dialog close <confirmation>` — the FE `ModalDialog` fires `notifyDialogClosed` on unmount, so the tracker reflects the close even when cancel doesn't bump pane generation |
 | `WindowAppeared(label)`  | A `webview_windows()` entry matches the label (exact, or `viewer-*`)    | `dialog open settings|file-viewer`, `dialog focus`                                                                               |
 | `WindowDisappeared`      | The matching `webview_windows()` entry is gone                          | `dialog close settings|file-viewer|about`                                                                                        |
 | `Any([...])`             | Logical OR — any inner signal fires                                     | `open_under_cursor` (directory case bumps generation, file case opens a viewer window)                                           |
 
-The polling cadence is 250 ms for state-driven signals (matching the existing `await` tool) and 100 ms for window/dialog appearance signals (windows show up faster than full pane state pushes).
+The polling cadence is 250 ms for state-driven signals (matching the existing `await` tool) and 100 ms for window/soft-dialog signals (both react faster than a full pane state push).
+
+**Gotcha/Why**: `dialog close <settings>` requires the settings window to listen for the `mcp-settings-close` event and close itself (`apps/desktop/src/routes/settings/+page.svelte`). Without that listener the backend keeps polling for `WindowDisappeared("settings")` and times out at 1500 ms while the window stays put. Same shape applies if you add new window-based dialogs: the FE side has to opt in.
 
 The 1500 ms budget is a backend-side latency budget, not a client-facing knob: MCP clients shouldn't have to tune ack timeouts. Bump it per-call via the `Duration` argument to `wait_for_ack` if a specific operation has a known higher latency floor; don't expose it as a tool parameter.
 

apps/desktop/src-tauri/src/mcp/executor/ack.rs

@@ -61,6 +61,12 @@ pub enum AckSignal {
     GenerationAdvanced { from: u64 },
     /// A soft dialog with this ID appeared in `SoftDialogTracker`.
     SoftDialogAppeared(&'static str),
+    /// A soft dialog with this ID is no longer present in `SoftDialogTracker`.
+    /// Use this when an MCP tool dispatches a close to a soft (overlay) dialog:
+    /// the FE's `ModalDialog` runs `notifyDialogClosed` on destroy, so the
+    /// tracker reflects the close even when the surrounding pane state didn't
+    /// change (e.g. cancelling a confirmation dialog doesn't bump generation).
+    SoftDialogDisappeared(&'static str),
     /// A Tauri webview window whose label equals (or starts with, for viewers)
     /// the given pattern appeared.
     WindowAppeared(&'static str),
@@ -79,6 +85,7 @@ impl AckSignal {
                 format!("pane state generation > {from}")
             }
             AckSignal::SoftDialogAppeared(id) => format!("soft dialog '{id}' opened"),
+            AckSignal::SoftDialogDisappeared(id) => format!("soft dialog '{id}' closed"),
             AckSignal::WindowAppeared(label) => format!("window '{label}' opened"),
             AckSignal::WindowDisappeared(label) => format!("window '{label}' closed"),
             AckSignal::Any(signals) => {
@@ -140,6 +147,12 @@ fn check_signal<R: Runtime>(app: &AppHandle<R>, signal: &AckSignal) -> bool {
             .try_state::<SoftDialogTracker>()
             .map(|tracker| tracker.get_open_types().iter().any(|d| d == id))
             .unwrap_or(false),
+        AckSignal::SoftDialogDisappeared(id) => app
+            .try_state::<SoftDialogTracker>()
+            .map(|tracker| !tracker.get_open_types().iter().any(|d| d == id))
+            // If the tracker isn't registered (test contexts), treat the dialog as
+            // gone — there's nothing to wait for.
+            .unwrap_or(true),
         AckSignal::WindowAppeared(pattern) => window_matches(app, pattern),
         AckSignal::WindowDisappeared(pattern) => !window_matches(app, pattern),
         AckSignal::Any(signals) => signals.iter().any(|s| check_signal(app, s)),
@@ -159,10 +172,15 @@ fn window_matches<R: Runtime>(app: &AppHandle<R>, pattern: &str) -> bool {
     }
 }
 
-/// Whether any leaf in the signal tree references windows. Drives poll cadence.
+/// Whether any leaf in the signal tree references windows or soft dialogs.
+/// Both are FE-side mutations that don't require a full pane-state push, so
+/// they should react with the tighter cadence.
 fn signal_uses_windows(signal: &AckSignal) -> bool {
     match signal {
-        AckSignal::WindowAppeared(_) | AckSignal::WindowDisappeared(_) => true,
+        AckSignal::WindowAppeared(_)
+        | AckSignal::WindowDisappeared(_)
+        | AckSignal::SoftDialogAppeared(_)
+        | AckSignal::SoftDialogDisappeared(_) => true,
         AckSignal::Any(signals) => signals.iter().any(signal_uses_windows),
         _ => false,
     }
@@ -200,6 +218,9 @@ mod tests {
                 .describe()
                 .contains("delete-confirmation")
         );
+        let closed = AckSignal::SoftDialogDisappeared("mkdir-confirmation").describe();
+        assert!(closed.contains("mkdir-confirmation"));
+        assert!(closed.contains("closed"));
         assert!(AckSignal::WindowAppeared("settings").describe().contains("settings"));
         assert!(AckSignal::WindowDisappeared("settings").describe().contains("settings"));
         let any = AckSignal::Any(vec![
@@ -215,6 +236,9 @@ mod tests {
     fn signal_uses_windows_picks_tighter_cadence() {
         assert!(!signal_uses_windows(&AckSignal::GenerationAdvanced { from: 0 }));
         assert!(signal_uses_windows(&AckSignal::WindowAppeared("settings")));
+        assert!(signal_uses_windows(&AckSignal::SoftDialogDisappeared(
+            "mkdir-confirmation"
+        )));
         assert!(signal_uses_windows(&AckSignal::Any(vec![
             AckSignal::GenerationAdvanced { from: 0 },
             AckSignal::WindowAppeared("viewer"),

apps/desktop/src-tauri/src/mcp/executor/dialogs.rs

@@ -4,7 +4,8 @@
 //! - `open settings|file-viewer|about` → child window appears in `webview_windows()`.
 //! - `open` confirmation dialogs → not allowed (use copy/move/delete/mkdir/mkfile instead).
 //! - `close settings|file-viewer|about` → matching window disappears.
-//! - `close <confirmation>` → soft dialog is no longer in the tracker, ack via dialog gone.
+//! - `close <confirmation>` → soft dialog is no longer in `SoftDialogTracker`. Cancel
+//!   doesn't reliably bump pane generation, so we wait for the tracker entry to vanish.
 //! - `focus settings|file-viewer|about` → window is present (no-op fast path; if the
 //!   window isn't there, the wait_for_ack times out, which is the correct contract for
 //!   focusing a non-existent dialog).
@@ -197,14 +198,13 @@ async fn execute_dialog_close<R: Runtime>(app: &AppHandle<R>, dialog_type: &str,
         }
         "transfer-confirmation" | "mkdir-confirmation" | "new-file-confirmation" | "delete-confirmation" => {
             app.emit("close-confirmation", ())?;
-            // For soft confirmation dialogs we can't directly probe a "disappeared"
-            // signal — we'd need a NotSoftDialog variant. Instead, ack on a generation
-            // bump: when the dialog closes, the FE typically re-pushes pane state.
-            // Pragmatic and matches the current FE behavior.
-            let pre_gen = snapshot_generation(app);
+            // Soft confirmation dialogs unmount their `ModalDialog`, which fires
+            // `notifyDialogClosed` and updates the `SoftDialogTracker`. Wait for the
+            // tracker to lose the dialog ID. Cancel doesn't reliably bump generation
+            // (that's what we used to wait for, and it broke on every cancel).
             wait_for_ack(
                 app,
-                AckSignal::GenerationAdvanced { from: pre_gen },
+                AckSignal::SoftDialogDisappeared(soft_dialog_id(dialog_type)),
                 DEFAULT_ACK_TIMEOUT,
             )
             .await?;

apps/desktop/src-tauri/src/mcp/tests/ack_system_tests.rs

@@ -178,6 +178,26 @@ fn soft_dialog_tracker_membership_matches_ack_check() {
     assert!(!tracker.get_open_types().iter().any(|d| d == id));
 }
 
+#[test]
+fn soft_dialog_disappeared_signal_flips_after_close() {
+    // The ack contract for `dialog close <confirmation>` relies on the tracker
+    // losing the dialog ID. Pins the semantic: after `close()` the tracker reports
+    // the id as absent, which is what `AckSignal::SoftDialogDisappeared` checks.
+    let tracker = SoftDialogTracker::new();
+    let id = "mkdir-confirmation";
+
+    tracker.open(id.to_string());
+    let after_open_absent = !tracker.get_open_types().iter().any(|d| d == id);
+    assert!(!after_open_absent, "dialog must be present right after open");
+
+    tracker.close(id);
+    let after_close_absent = !tracker.get_open_types().iter().any(|d| d == id);
+    assert!(
+        after_close_absent,
+        "tracker must report the dialog as gone after close — this is what `SoftDialogDisappeared` polls"
+    );
+}
+
 #[test]
 fn soft_dialog_tracker_distinguishes_dialog_ids() {
     // Important for the ack contract: copy/move both open "transfer-confirmation",

apps/desktop/src/routes/settings/+page.svelte

@@ -22,6 +22,7 @@
     let contentElement: HTMLElement | null = $state(null)
     let unlistenFocusSelf: UnlistenFn | undefined
     let unlistenNavigate: UnlistenFn | undefined
+    let unlistenMcpClose: UnlistenFn | undefined
 
     function safeParseSectionParam(raw: string): string[] | null {
         try {
@@ -217,6 +218,19 @@
                 handleSectionSelect(event.payload.section)
             })
 
+            // MCP can request that this window close (used by `dialog close settings`).
+            // Mirror the Escape-key handler: defer the close() by two rAFs so the in-flight
+            // IPC ack can settle before webkit2gtk begins destroying the webview.
+            unlistenMcpClose = await listen('mcp-settings-close', () => {
+                log.debug('Received mcp-settings-close, closing window')
+                const win = getCurrentWindow()
+                requestAnimationFrame(() => {
+                    requestAnimationFrame(() => {
+                        void win.close()
+                    })
+                })
+            })
+
             log.debug('Settings page ready')
         } catch (error) {
             log.error('Failed to initialize settings: {error}', { error })
@@ -231,6 +245,7 @@
         // Clean up event listeners
         unlistenFocusSelf?.()
         unlistenNavigate?.()
+        unlistenMcpClose?.()
         cleanupAccentColor()
         cleanupTextSize()
     })

docs/tooling/mcp.md

@@ -52,6 +52,9 @@ What this means for automation:
 - For long-running operations, poll completion via the `await` tool just like before.
 - Timeouts surface as JSON-RPC errors with a clear message ("Action not acknowledged by backend within 1500 ms (waiting
   for: …)"). Treat them as real failures — don't retry blindly; check `cmdr://state` to triage.
+- One legit timeout source: navigating into a remote share (`open_under_cursor` on an SMB host, `nav_to_path` into
+  `/Volumes/<share>` right after a mount) can take a few seconds end-to-end. The tool will time out at 1500 ms even
+  though the navigation eventually succeeds. Use `await` with `path` or `path_contains` after dispatching to confirm.
 
 Architecture details: `apps/desktop/src-tauri/src/mcp/CLAUDE.md` § "Action-tool ack contract".