vdavid
diff --git a/‎apps/desktop/src-tauri/src/mcp/CLAUDE.md‎
Lines changed: 10 additions & 7 deletions b/‎apps/desktop/src-tauri/src/mcp/CLAUDE.md‎
Lines changed: 10 additions & 7 deletions
diff --git a/‎apps/desktop/src-tauri/src/mcp/executor/ack.rs‎
Lines changed: 77 additions & 30 deletions b/‎apps/desktop/src-tauri/src/mcp/executor/ack.rs‎
Lines changed: 77 additions & 30 deletions
@@ -69,20 +69,23 @@ The mechanism lives in `executor/ack.rs`. Each tool:
 | `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)                                           |
+| `WindowDisappeared(label)` | The matching `webview_windows()` entry is gone                        | `dialog close settings` (single window family)                                                                                   |
+| `WindowCountBelow {prefix, threshold}` | Count of matching windows is `< threshold`                | `dialog close file-viewer` — snapshot the count, ack when one closes (don't wait for *all* viewers to vanish)                    |
+| `Any([...])`             | Logical OR — any inner signal fires                                     | Reserved for future multi-mode tools                                                                                              |
 
 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.
+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. The navigation family (`nav_to_parent`, `nav_back`, `nav_forward`, `open_under_cursor`) uses `NAV_ACK_TIMEOUT` (5 s) because a parent/back navigation can land on an SMB or MTP path whose directory listing takes a few seconds even on success. `nav_to_path` and `select_volume` use higher round-trip budgets via `mcp_round_trip_with_timeout` and aren't part of the ack-contract family.
 
-**Note on `update_pane_tabs`.** Tab changes flow through this command (not `set_left`/`set_right`), so it also bumps the generation counter. Without that, the `tab` tool's ack would time out on every call.
+**Caveat: `GenerationAdvanced` isn't a per-action proof.** The snapshot-dispatch-wait sequence proves the FE pushed pane state *recently after* dispatch — not that the FE specifically handled our event. An unrelated push between snapshot and dispatch (the other pane's watcher, a tab refresh) will satisfy the signal as a false positive. The window is microseconds wide and the FE was clearly running (something pushed), so this is a much weaker false-positive class than the original "always OK" bug. If a real false positive ever surfaces, the fix is to switch the affected tool to `mcp_round_trip` with a request id. Tagged `TODO(mcp-ack):` in `ack.rs`.
+
+**Note on `update_pane_tabs`.** Tab changes flow through this command (not `set_left`/`set_right`). It delegates to `PaneStateStore::set_tabs`, which is the single place tab mutation + generation bump live. Add any new tab-mutating path through that helper, or the `tab` MCP tool's ack will time out.
 
 **Known TODO: `refresh` is still fire-and-forget.** The FE skips the `update_*_pane_state` push when the new listing is byte-identical to the cached state (correct behavior for state sync but no signal for the ack helper). Switching `refresh` to `mcp_round_trip` is the right follow-up, but requires a FE change to emit `mcp-response` after every re-list. The original "FE silently dropping the action" bug is less acute for refresh than for destructive tools, so this stays open. Search the codebase for `TODO(mcp-ack):` to find this and any future similar cases.
 
-Tools where the backend can't fully validate preconditions use `mcp_round_trip`: emit an event with a `requestId`, wait for the frontend to respond via `mcp-response` with `{ requestId, ok, error? }`, and return the actual outcome. Used by `move_cursor` and `set_setting` (5s timeout). `nav_to_path` uses `mcp_round_trip_with_timeout` with a 30s timeout because it waits for the directory listing to complete (the frontend delays the response until `handleListingComplete` fires in FilePane). Resources that need frontend data use `resource_round_trip` (same pattern but returns `data` from the response). Used by `cmdr://settings`. Use these patterns for any new tool/resource where the backend would otherwise need to replicate frontend knowledge.
+Tools where the backend can't fully validate preconditions use `mcp_round_trip`: emit an event with a `requestId`, wait for the frontend to respond via `mcp-response` with `{ requestId, ok, error? }`, and return the actual outcome. Used by `move_cursor` and `set_setting` (5 s timeout). `nav_to_path` uses `mcp_round_trip_with_timeout` with a 30 s timeout because it waits for the directory listing to complete (the frontend delays the response until `handleListingComplete` fires in FilePane). `open_under_cursor` also uses `mcp_round_trip_with_timeout` (5 s) because Enter on a non-directory file delegates to the OS default app (no pane state push, no viewer window), so neither `GenerationAdvanced` nor `WindowAppeared` would fire — the FE explicitly awaits `handleNavigate(entry)` and signals completion. Resources that need frontend data use `resource_round_trip` (same pattern but returns `data` from the response). Used by `cmdr://settings`. Use these patterns for any new tool/resource where the backend would otherwise need to replicate frontend knowledge.
 
 ### Configuration (`config.rs`)
 
@@ -114,7 +117,7 @@ Directory module split by test category:
 
 ### MCP action tools wait for backend ack before returning success
 
-**Decision (May 2026):** Every fire-and-forget action tool waits for a typed ack signal (`AckSignal::GenerationAdvanced`, `SoftDialogAppeared`, `WindowAppeared`, `WindowDisappeared`, or `Any`) within a 1500 ms budget before returning `OK`. On timeout, the tool returns a `ToolError::internal` whose message names the missing signal and elapsed budget.
+**Decision (May 2026):** Every fire-and-forget action tool waits for a typed ack signal (`AckSignal::GenerationAdvanced`, `SoftDialogAppeared`/`Disappeared`, `WindowAppeared`/`Disappeared`, `WindowCountBelow`, or `Any`) within a 1500 ms budget (5 s for the nav family) before returning `OK`. On timeout, the tool returns a `ToolError::internal` whose message names the missing signal and elapsed budget.
 
 **Why.** Real QA hit a paper-cut: MCP tools were returning `OK` while the FE was stalled (modal blocking input, error pane up, race during startup), so the dispatched action was silently dropped. That made MCP unreliable as an automation surface. The ack contract makes `OK` a real promise: the FE actually processed the dispatched action.
 
@@ -201,7 +204,7 @@ The `cmdr://settings` resource and `set_setting` tool both use round-trips to th
 
 `execute_tool()` is an async function. Action tools follow the ack contract (see "Action-tool ack contract" above): dispatch the event, then `wait_for_ack` for a small backend-side signal before returning. The tool's reported "OK" thus means "the FE accepted the dispatched action," not "the underlying operation completed." For long-running operations (a copy of 10 GB), the agent still has to poll via the `await` tool to observe completion. The ack-contract change made the FE-accepted line meaningful — pre-May 2026, the tool returned `OK` even when the FE wasn't listening.
 
-Three categories of latency-sensitive tools exist beyond the ack contract: (1) `mcp_round_trip` tools (`nav_to_path`, `move_cursor`, `set_setting`) that wait up to 5–30 s for an explicit `mcp-response` event with success/failure, (2) search tools (`search`, `ai_search`) that load the search index via `spawn_blocking` and (for `ai_search`) call the LLM API, (3) `select_volume` which polls until the target pane's `volume_name` matches.
+Three categories of latency-sensitive tools exist beyond the ack contract: (1) `mcp_round_trip` tools (`nav_to_path`, `move_cursor`, `set_setting`, `open_under_cursor`) that wait up to 5–30 s for an explicit `mcp-response` event with success/failure, (2) search tools (`search`, `ai_search`) that load the search index via `spawn_blocking` and (for `ai_search`) call the LLM API, (3) `select_volume` which polls until the target pane's `volume_name` matches.
 
 ### Error codes are JSON-RPC standard
 
 
@@ -17,9 +17,29 @@
 //! - `WindowAppeared` / `WindowDisappeared`: a Tauri webview window with the given label
 //!   prefix appeared (or vanished). Use this for child windows (settings, file-viewer,
 //!   about) and for `dialog close` actions.
-//! - `Any`: succeeds when any of the inner signals fire (logical OR). Used by
-//!   `open_under_cursor` where opening a directory bumps the pane generation but opening
-//!   a file launches a viewer window.
+//! - `WindowCountBelow`: the number of windows matching a label prefix is strictly less
+//!   than a snapshotted count. Use this for close-one-of-many scenarios (closing a
+//!   specific `viewer-*` window by path) where `WindowDisappeared` would only fire when
+//!   ALL viewers are gone.
+//!
+//! Multi-mode tools that can produce different signals depending on what happened (the
+//! original `open_under_cursor` was the only such case) live outside the ack contract
+//! entirely — they use `mcp_round_trip` and let the FE explicitly signal completion. An
+//! earlier `AckSignal::Any` variant tried to OR these together but couldn't cover the
+//! OS-open branch (Enter on a non-directory file produces neither a state push nor a
+//! viewer window); round-trip is the only honest ack for that shape.
+//!
+//! ## Caveat: `GenerationAdvanced` is not a per-action ack
+//!
+//! `snapshot_generation` + dispatch + wait for `GenerationAdvanced` proves the FE pushed
+//! pane state recently after dispatch — not that it specifically handled this action. An
+//! unrelated state push (other pane's MTP watcher, a tab refresh) between the snapshot
+//! and the dispatch can satisfy the signal without the FE having processed our event.
+//! In practice this is a much weaker false-positive class than the original "always OK"
+//! bug (the FE was almost certainly running, since something pushed state), so the
+//! contract is acceptable. Stronger guarantees would require either a request-id-based
+//! `mcp-response` round-trip (see `mcp_round_trip`) or per-tool FE acks. TODO(mcp-ack):
+//! revisit if real-world false positives surface.
 //!
 //! ## Timeout
 //!
@@ -48,6 +68,13 @@ use crate::mcp::pane_state::PaneStateStore;
 /// Default ack budget. Backend-side latency budget; not a client-facing knob.
 pub const DEFAULT_ACK_TIMEOUT: Duration = Duration::from_millis(1500);
 
+/// Ack budget for navigation tools whose state push can include a directory listing
+/// against a remote backend (SMB/MTP). Local paths still ack in ~50 ms; remote paths
+/// can take a few seconds end-to-end. 5 s strikes a balance: still bounded, but no
+/// spurious timeouts on a working remote share. `nav_to_path` and `select_volume`
+/// use higher round-trip budgets via `mcp_round_trip_with_timeout`.
+pub const NAV_ACK_TIMEOUT: Duration = Duration::from_secs(5);
+
 /// Polling cadence for state-driven signals. Matches the existing `await` tool.
 const STATE_POLL_INTERVAL: Duration = Duration::from_millis(250);
 
@@ -70,11 +97,15 @@ pub enum AckSignal {
     /// A Tauri webview window whose label equals (or starts with, for viewers)
     /// the given pattern appeared.
     WindowAppeared(&'static str),
-    /// A Tauri webview window matching the pattern vanished.
+    /// A Tauri webview window matching the pattern vanished. For prefix families
+    /// like `viewer`, this fires only when zero matching windows remain; use
+    /// `WindowCountBelow` to wait for *one* viewer to close while others stay open.
     WindowDisappeared(&'static str),
-    /// Succeeds when any inner signal fires. Used for tools where the ack
-    /// kind depends on what got opened (for example `open_under_cursor`).
-    Any(Vec<AckSignal>),
+    /// The number of webview windows matching the pattern is strictly less than
+    /// `threshold`. Used to ack "close one of N viewers" cleanly: snapshot the
+    /// count, dispatch the close, wait for the count to drop. For close-all,
+    /// use `threshold: 1` (i.e., wait for count to reach 0).
+    WindowCountBelow { prefix: &'static str, threshold: usize },
 }
 
 impl AckSignal {
@@ -88,9 +119,8 @@ impl AckSignal {
             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) => {
-                let parts: Vec<String> = signals.iter().map(|s| s.describe()).collect();
-                format!("any of [{}]", parts.join(", "))
+            AckSignal::WindowCountBelow { prefix, threshold } => {
+                format!("window count for '{prefix}' < {threshold}")
             }
         }
     }
@@ -150,39 +180,56 @@ fn check_signal<R: Runtime>(app: &AppHandle<R>, signal: &AckSignal) -> bool {
         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.
+            // Asymmetry vs. SoftDialogAppeared (which returns false when the tracker
+            // isn't registered): if the tracker isn't there, no dialog is open either,
+            // so "this dialog is gone" is trivially true. The Appeared variant must
+            // wait — without a tracker it can never see the dialog. Lets unit tests
+            // drive close paths without spinning up a tracker fixture, while keeping
+            // the open path strict.
             .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)),
+        AckSignal::WindowCountBelow { prefix, threshold } => count_windows_matching(app, prefix) < *threshold,
     }
 }
 
 /// True if any Tauri webview window has a label exactly equal to `pattern`,
 /// or (for the `viewer` family) starting with `pattern-`.
 fn window_matches<R: Runtime>(app: &AppHandle<R>, pattern: &str) -> bool {
+    count_windows_matching(app, pattern) > 0
+}
+
+/// Count of Tauri webview windows matching `pattern`. For prefix families
+/// (`viewer`), counts every `viewer-*` window; for exact labels (`settings`,
+/// `file-viewer-help`, etc.), returns 0 or 1.
+fn count_windows_matching<R: Runtime>(app: &AppHandle<R>, pattern: &str) -> usize {
     let windows = app.webview_windows();
-    // `viewer-` is a label prefix family — each opened file has its own
-    // `viewer-<id>` window. Other tracked labels (settings, about) are exact.
     if pattern == "viewer" {
-        windows.keys().any(|k| k.starts_with("viewer-"))
+        windows.keys().filter(|k| k.starts_with("viewer-")).count()
+    } else if windows.contains_key(pattern) {
+        1
     } else {
-        windows.contains_key(pattern)
+        0
     }
 }
 
+/// Snapshot the current count of Tauri webview windows matching `prefix`.
+/// Use with `WindowCountBelow { threshold: snapshot }` to ack on "one closed."
+pub fn snapshot_window_count<R: Runtime>(app: &AppHandle<R>, prefix: &'static str) -> usize {
+    count_windows_matching(app, prefix)
+}
+
 /// 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(_)
+        | AckSignal::WindowCountBelow { .. }
         | AckSignal::SoftDialogAppeared(_)
         | AckSignal::SoftDialogDisappeared(_) => true,
-        AckSignal::Any(signals) => signals.iter().any(signal_uses_windows),
-        _ => false,
+        AckSignal::GenerationAdvanced { .. } => false,
     }
 }
 
@@ -223,13 +270,13 @@ mod tests {
         assert!(closed.contains("closed"));
         assert!(AckSignal::WindowAppeared("settings").describe().contains("settings"));
         assert!(AckSignal::WindowDisappeared("settings").describe().contains("settings"));
-        let any = AckSignal::Any(vec![
-            AckSignal::GenerationAdvanced { from: 1 },
-            AckSignal::WindowAppeared("viewer"),
-        ]);
-        let s = any.describe();
-        assert!(s.contains("any of"));
-        assert!(s.contains("viewer"));
+        let count = AckSignal::WindowCountBelow {
+            prefix: "viewer",
+            threshold: 3,
+        }
+        .describe();
+        assert!(count.contains("viewer"));
+        assert!(count.contains("3"));
     }
 
     #[test]
@@ -239,10 +286,10 @@ mod tests {
         assert!(signal_uses_windows(&AckSignal::SoftDialogDisappeared(
             "mkdir-confirmation"
         )));
-        assert!(signal_uses_windows(&AckSignal::Any(vec![
-            AckSignal::GenerationAdvanced { from: 0 },
-            AckSignal::WindowAppeared("viewer"),
-        ])));
+        assert!(signal_uses_windows(&AckSignal::WindowCountBelow {
+            prefix: "viewer",
+            threshold: 1,
+        }));
     }
 
     // Verifies the core promise: once the generation strictly advances past