fix(cmux-tui-core): set-layout-sizes takes a value only as exactly one pair and refuses a non-finite ratio; tests and docs (#34928 review follow-ups)

cmux-tui/crates/cmux-tui-core/src/mux/layout_sizes.rs

@@ -32,13 +32,17 @@ pub enum LayoutSizesError {
     Width { index: usize, error: ViewportWidthError },
     /// The value at `index` is on another screen than the first value.
     OtherScreen { index: usize },
+    /// The ratio at `index` is not a number (in-process callers; JSON cannot carry one). An infinite ratio is not
+    /// refused: it clamps, as `set-split-ratio` clamps it.
+    NanRatio { index: usize },
     /// The commit failed (the store refused it); nothing changed.
     NotPersisted { reason: String },
 }
 
 impl LayoutSizesError {
     pub const BAD_COUNT_CODE: &'static str = "invalid-argument";
     pub const OTHER_SCREEN_CODE: &'static str = "layout-sizes-other-screen";
+    pub const NAN_RATIO_CODE: &'static str = "invalid-argument";
     pub const NOT_PERSISTED_CODE: &'static str = "layout-sizes-not-persisted";
 
     /// The single command's error code for a refused value, else the batch's own.
@@ -48,6 +52,7 @@ impl LayoutSizesError {
             Self::Ratio { error, .. } => error.code(),
             Self::Width { error, .. } => error.code(),
             Self::OtherScreen { .. } => Self::OTHER_SCREEN_CODE,
+            Self::NanRatio { .. } => Self::NAN_RATIO_CODE,
             Self::NotPersisted { .. } => Self::NOT_PERSISTED_CODE,
         }
     }
@@ -56,9 +61,10 @@ impl LayoutSizesError {
     pub fn index(&self) -> Option<usize> {
         match self {
             Self::BadCount { .. } | Self::NotPersisted { .. } => None,
-            Self::Ratio { index, .. } | Self::Width { index, .. } | Self::OtherScreen { index } => {
-                Some(*index)
-            }
+            Self::Ratio { index, .. }
+            | Self::Width { index, .. }
+            | Self::OtherScreen { index }
+            | Self::NanRatio { index } => Some(*index),
         }
     }
 }
@@ -75,6 +81,9 @@ impl fmt::Display for LayoutSizesError {
             Self::OtherScreen { index } => {
                 write!(formatter, "value {index} is on another screen than value 0")
             }
+            Self::NanRatio { index } => {
+                write!(formatter, "value {index}: the split ratio is not a number")
+            }
             Self::NotPersisted { reason } => {
                 write!(formatter, "could not persist layout sizes: {reason}")
             }

cmux-tui/crates/cmux-tui-core/src/mux/resource_topology/layout_ops.rs

@@ -436,6 +436,11 @@ fn apply_layout_size_value(
     match value {
         LayoutSizeValue::SplitRatio { split, ratio } => {
             let refused = |error| LayoutSizesError::Ratio { index, error };
+            // The clamp keeps a NaN (in-process callers; JSON cannot carry one), and the store cannot persist one.
+            // An infinite ratio (JSON `1e39` as f32) clamps below, as `set-split-ratio` clamps it.
+            if ratio.is_nan() {
+                return Err(LayoutSizesError::NanRatio { index });
+            }
             let ratio = clamp_split_ratio(ratio);
             let Some((split_workspace, split_screen, owner)) =
                 state.split_screens.get(&split).copied()

cmux-tui/crates/cmux-tui-core/src/mux/tests/layout_sizes.rs

@@ -127,3 +127,186 @@ fn an_empty_or_oversized_batch_is_refused() {
     }
     assert_eq!(widths_revision_and_undo(&mux), before);
 }
+
+/// Three columns (the first pane and two panes right of it), the panes left to right.
+fn three_columns(mux: &Arc<Mux>) -> [PaneId; 3] {
+    let (first_pane, second_pane) = two_columns(mux);
+    let third = mux.new_pane_right(second_pane, 0.5, Some((18, 22))).unwrap();
+    let third_pane = mux.with_state(|state| state.pane_of(third.id).unwrap());
+    [first_pane, second_pane, third_pane]
+}
+
+/// The split id of the column at `index` on the first screen.
+fn column_split(mux: &Arc<Mux>, index: usize) -> SplitId {
+    mux.with_state(|state| state.workspaces[0].screens[0].layout_columns[index].id)
+}
+
+/// #34928 review C: a ratio value sets its column's width from the widths the same batch set before it (a column
+/// split's ratio is the share of the columns left of it), and the batch is still one change and one undo step.
+#[test]
+fn a_ratio_value_uses_the_widths_the_batch_set_before_it() {
+    let mux = test_mux();
+    let [first_pane, _, _] = three_columns(&mux);
+    let third_split = column_split(&mux, 2);
+    let (_, revision, undo) = widths_revision_and_undo(&mux);
+
+    mux.set_layout_sizes_in_transaction_checked_as(
+        &Actor::Daemon,
+        &[
+            LayoutSizeValue::PaneWidth { pane: first_pane, width: 0.4 },
+            LayoutSizeValue::SplitRatio { split: third_split, ratio: 0.5 },
+        ],
+        7,
+        15,
+    )
+    .unwrap();
+    let (widths, after_revision, after_undo) = widths_revision_and_undo(&mux);
+    let left_of_third = widths[0] + widths[1];
+    assert!(
+        (widths[2] - left_of_third).abs() < 1e-5,
+        "a ratio of 0.5 must make the third column as wide as the two left of it as the batch set them: {widths:?}"
+    );
+    assert!((widths[0] - 0.4).abs() < 1e-6, "the batch's first width: {widths:?}");
+    assert_eq!(after_revision, revision + 1, "the batch made more than one layout change");
+    assert_eq!(after_undo, undo + 1, "the batch is not one undo step");
+}
+
+/// #34928 review B: a NaN ratio is refused in-process (the clamp keeps a NaN, and the store cannot persist one), and
+/// nothing changes.
+#[test]
+fn a_nan_ratio_is_refused_and_changes_nothing() {
+    let mux = test_mux();
+    let [first_pane, _, _] = three_columns(&mux);
+    let third_split = column_split(&mux, 2);
+    let before = widths_revision_and_undo(&mux);
+    let error = mux
+        .set_layout_sizes_in_transaction_checked_as(
+            &Actor::Daemon,
+            &[
+                LayoutSizeValue::PaneWidth { pane: first_pane, width: 0.4 },
+                LayoutSizeValue::SplitRatio { split: third_split, ratio: f32::NAN },
+            ],
+            7,
+            16,
+        )
+        .expect_err("a NaN ratio was taken");
+    assert!(matches!(error, LayoutSizesError::NanRatio { index: 1 }), "{error}");
+    assert_eq!(error.code(), "invalid-argument");
+    assert_eq!(widths_revision_and_undo(&mux), before, "a refused batch changed the layout");
+}
+
+/// #34928 review C: values on two screens are refused whole, with the index of the first value on another screen.
+#[test]
+fn values_on_two_screens_are_refused_whole() {
+    let mux = test_mux();
+    let (first_pane, _) = two_columns(&mux);
+    let second = mux.new_screen(None, Some((80, 22))).unwrap();
+    let second_pane = mux.with_state(|state| state.pane_of(second.id).unwrap());
+    mux.new_pane_right(second_pane, 0.5, Some((38, 22))).unwrap();
+    let before = widths_revision_and_undo(&mux);
+
+    let error = mux
+        .set_layout_sizes_in_transaction_checked_as(
+            &Actor::Daemon,
+            &[
+                LayoutSizeValue::PaneWidth { pane: first_pane, width: 0.4 },
+                LayoutSizeValue::PaneWidth { pane: second_pane, width: 0.4 },
+            ],
+            7,
+            17,
+        )
+        .unwrap_err();
+    assert!(matches!(error, LayoutSizesError::OtherScreen { index: 1 }), "{error}");
+    assert_eq!(error.code(), "layout-sizes-other-screen");
+    assert_eq!(widths_revision_and_undo(&mux), before, "a refused batch changed the layout");
+}
+
+/// #34928 review C: a batch that changes nothing commits nothing: no layout change, no undo step, no event.
+#[test]
+fn a_batch_of_the_current_sizes_commits_nothing() {
+    let mux = test_mux();
+    let (first_pane, right_pane) = two_columns(&mux);
+    let before = widths_revision_and_undo(&mux);
+    let values: Vec<_> = [first_pane, right_pane]
+        .iter()
+        .zip(&before.0)
+        .map(|(pane, width)| LayoutSizeValue::PaneWidth { pane: *pane, width: *width })
+        .collect();
+    let events = mux.subscribe();
+
+    mux.set_layout_sizes_in_transaction_checked_as(&Actor::Daemon, &values, 7, 18).unwrap();
+    assert_eq!(
+        widths_revision_and_undo(&mux),
+        before,
+        "a batch of the current sizes changed the layout"
+    );
+    while let Ok(event) = events.try_recv() {
+        assert!(
+            !matches!(event, MuxEvent::LayoutChanged(..)),
+            "a batch that changed nothing sent a layout-changed event"
+        );
+    }
+}
+
+/// #34928 review C: a batch that changes the layout sends exactly one layout-changed event.
+#[test]
+fn a_batch_sends_one_layout_changed_event() {
+    let mux = test_mux();
+    let (first_pane, right_pane) = two_columns(&mux);
+    let events = mux.subscribe();
+
+    mux.set_layout_sizes_in_transaction_checked_as(
+        &Actor::Daemon,
+        &[
+            LayoutSizeValue::PaneWidth { pane: first_pane, width: 0.4 },
+            LayoutSizeValue::PaneWidth { pane: right_pane, width: 0.6 },
+        ],
+        7,
+        19,
+    )
+    .unwrap();
+    let mut transactions = Vec::new();
+    while let Ok(event) = events.try_recv() {
+        if let MuxEvent::LayoutChanged(_, delta) = event {
+            transactions.push(delta.and_then(|delta| delta.transaction.clone()));
+        }
+    }
+    assert_eq!(
+        transactions,
+        vec![Some(Arc::<str>::from("19"))],
+        "a batch of two values must send one layout-changed event that echoes its transaction"
+    );
+}
+
+/// #34928 review B, on the path it named: a tree split (no viewport column, so no implied width to refuse it) takes
+/// the ratio as it is. A NaN ratio is refused there too; an infinite one (JSON `1e39` read as f32) clamps to 0.95, as
+/// `set-split-ratio` clamps it (#35261 review).
+#[test]
+fn a_nan_ratio_on_a_tree_split_is_refused_and_an_infinite_one_clamps() {
+    let mux = test_mux();
+    let (_, _, _, root_split, _) = seed_split_ratio_tree(&mux);
+    let root_ratio = |mux: &Arc<Mux>| {
+        mux.with_state(|state| state.workspaces[0].screens[0].root.split_ratio(root_split))
+    };
+    let before = root_ratio(&mux);
+    assert!(before.is_some(), "the seed's root is a split");
+    let error = mux
+        .set_layout_sizes_in_transaction_checked_as(
+            &Actor::Daemon,
+            &[LayoutSizeValue::SplitRatio { split: root_split, ratio: f32::NAN }],
+            7,
+            20,
+        )
+        .expect_err("a NaN ratio on a tree split was taken");
+    assert!(matches!(error, LayoutSizesError::NanRatio { index: 0 }), "{error}");
+    assert_eq!(root_ratio(&mux), before, "a refused ratio changed the split");
+
+    mux.set_layout_sizes_in_transaction_checked_as(
+        &Actor::Daemon,
+        &[LayoutSizeValue::SplitRatio { split: root_split, ratio: f32::INFINITY }],
+        7,
+        21,
+    )
+    .expect("an infinite ratio is clamped, as set-split-ratio clamps it");
+    assert_eq!(root_ratio(&mux), Some(0.95), "an infinite ratio did not clamp to 0.95");
+}

cmux-tui/crates/cmux-tui-core/src/server.rs

@@ -317,6 +317,7 @@ mod url_open;
 #[cfg(test)]
 use capabilities::advertised_capabilities;
 use capabilities::identify_capabilities;
+use cmd_panes::LayoutSizeWire;
 /// Maximum JSON payload accepted on the Unix JSON-lines control socket.
 const MAX_JSON_LINE_BYTES: usize = crate::REMOTE_CLIENT_MESSAGE_MAX_BYTES;
 const WORKSPACE_REGISTRY_CAPABILITY: &str = "workspace-registry-v1";
@@ -2404,23 +2405,6 @@ impl Command {
     }
 }
 
-/// One value of `set-layout-sizes`: what `set-split-ratio` or `set-viewport-pane-width` sets.
-#[derive(Debug, Clone, Copy, Deserialize)]
-#[serde(untagged)]
-enum LayoutSizeWire {
-    SplitRatio { split: SplitId, ratio: f32 },
-    PaneWidth { pane: PaneId, width: f32 },
-}
-
-impl From<LayoutSizeWire> for crate::mux::LayoutSizeValue {
-    fn from(value: LayoutSizeWire) -> Self {
-        match value {
-            LayoutSizeWire::SplitRatio { split, ratio } => Self::SplitRatio { split, ratio },
-            LayoutSizeWire::PaneWidth { pane, width } => Self::PaneWidth { pane, width },
-        }
-    }
-}
-
 /// A tab named by its numeric surface id or its public `tab_...` id.
 #[derive(Debug, Clone, Deserialize)]
 #[serde(untagged)]

cmux-tui/crates/cmux-tui-core/src/server/cmd_panes.rs

@@ -29,6 +29,25 @@ use serde_json::Value;
 use serde_json::json;
 use std::sync::Arc;
 
+/// One value of `set-layout-sizes`: what `set-split-ratio` or `set-viewport-pane-width` sets. Exactly one of the two
+/// objects: a value with both pairs, with any other field, or not an object (serde gives untagged struct variants no
+/// sequence form) is refused, not half applied (#34928 review A, #35261 review).
+#[derive(Debug, Clone, Copy, serde::Deserialize)]
+#[serde(untagged, deny_unknown_fields)]
+pub(super) enum LayoutSizeWire {
+    SplitRatio { split: SplitId, ratio: f32 },
+    PaneWidth { pane: PaneId, width: f32 },
+}
+
+impl From<LayoutSizeWire> for crate::mux::LayoutSizeValue {
+    fn from(value: LayoutSizeWire) -> Self {
+        match value {
+            LayoutSizeWire::SplitRatio { split, ratio } => Self::SplitRatio { split, ratio },
+            LayoutSizeWire::PaneWidth { pane, width } => Self::PaneWidth { pane, width },
+        }
+    }
+}
+
 pub(super) fn export_layout(mux: &Arc<Mux>, screen: Option<ScreenId>) -> anyhow::Result<Value> {
     mux.with_state(|state| export_layout_json(state, screen))
 }
@@ -139,7 +158,7 @@ pub(super) fn set_layout_sizes(
     mux: &Arc<Mux>,
     client: u64,
     actor: Actor,
-    values: Vec<super::LayoutSizeWire>,
+    values: Vec<LayoutSizeWire>,
     transaction: u64,
 ) -> anyhow::Result<Value> {
     let values: Vec<crate::mux::LayoutSizeValue> = values.into_iter().map(Into::into).collect();

cmux-tui/crates/cmux-tui-core/src/server/tests/layout_receipts_shutdown.rs

@@ -250,6 +250,62 @@ fn set_layout_sizes_applies_a_trade_whole_or_refuses_it_whole() {
     assert_eq!(applied["id"], 43);
     assert_eq!(applied["ok"], true, "{applied}");
     assert_eq!(widths(&mux), vec![0.4, 0.6]);
+
+    // A ratio value: the second column's split at 0.5 makes it as wide as the column left of it (0.4).
+    let split = mux.with_state(|state| state.workspaces[0].screens[0].layout_columns[1].id);
+    handle_message(
+        &mux,
+        mux.local_test_client(7),
+        &json!({
+            "id": 44,
+            "cmd": "set-layout-sizes",
+            "values": [{"split": split, "ratio": 0.5}],
+            "transaction": 3
+        })
+        .to_string(),
+        &writer,
+    );
+    let ratio: Value = serde_json::from_str(&outbound.try_pop().unwrap()).unwrap();
+    assert_eq!(ratio["id"], 44);
+    assert_eq!(ratio["ok"], true, "{ratio}");
+    let after_ratio = widths(&mux);
+    assert!(
+        after_ratio.len() == 2 && (after_ratio[1] - after_ratio[0]).abs() < 1e-5,
+        "a ratio of 0.5 must make the second column as wide as the first: {after_ratio:?}"
+    );
+
+    // #34928 review A and #35261 review: a value that is not exactly one pair (both pairs, or an array) is refused as
+    // a malformed request, not half applied.
+    for (id, value) in [
+        (45, json!({"split": split, "ratio": 0.6, "pane": right_pane, "width": 0.3})),
+        (46, json!([split, 0.6])),
+    ] {
+        handle_message(
+            &mux,
+            mux.local_test_client(7),
+            &json!({"id": id, "cmd": "set-layout-sizes", "values": [value], "transaction": 3})
+                .to_string(),
+            &writer,
+        );
+        let refused: Value = serde_json::from_str(&outbound.try_pop().unwrap()).unwrap();
+        assert_eq!(refused["id"], id);
+        assert_eq!(
+            refused["ok"], false,
+            "a value that is not exactly one pair was taken: {refused}"
+        );
+        assert!(
+            refused["error"].as_str().is_some_and(|error| error.starts_with("bad request: ")),
+            "{refused}"
+        );
+        assert!(refused.get("error_code").is_none(), "{refused}");
+        assert_eq!(widths(&mux), after_ratio, "a refused value changed a width");
+    }
+}
+
+/// #34928 review C: identify advertises the batch command.
+#[test]
+fn identify_advertises_the_layout_sizes_batch() {
+    assert!(advertised_capabilities(false).contains(&LAYOUT_SIZES_BATCH_CAPABILITY));
 }
 
 #[test]

cmux-tui/spec/commands.md

@@ -2892,7 +2892,7 @@ Params:
 
 Result: `{}`. The commit's `layout-changed` event carries `transaction`.
 
-Errors: a refused value answers with its single command's error and `error_code` (`layout-ratio-target-missing`, `layout-ratio-out-of-range`, `row-split-compat-readonly`, `viewport-width-out-of-range`, `viewport-column-not-found`), its message prefixed with `value <index>: `. A value on another screen than the first returns `error_code:"layout-sizes-other-screen"`; no value or more than 64 returns `error_code:"invalid-argument"`; a commit the store refuses returns `error_code:"layout-sizes-not-persisted"`. In every error case nothing changed.
+Errors: a refused value answers with its single command's error and `error_code` (`layout-ratio-target-missing`, `layout-ratio-out-of-range`, `row-split-compat-readonly`, `viewport-width-out-of-range`, `viewport-column-not-found`), its message prefixed with `value <index>: `. A value on another screen than the first returns `error_code:"layout-sizes-other-screen"`; no value or more than 64 returns `error_code:"invalid-argument"`; a value that is not exactly `{split, ratio}` or `{pane, width}` (both pairs, or another field) is refused as a malformed request; a commit the store refuses returns `error_code:"layout-sizes-not-persisted"`. In every error case nothing changed.
 
 Example:
 

cmux-tui/spec/events.md

@@ -1010,7 +1010,7 @@ object{event:"layout-changed",screen:Id,workspace?:Id,active_pane?:Id,zoomed_pan
 
 Meaning: A screen's pane geometry changed through split, close/collapse, ratio update, viewport column width, row heights, dock, apply-layout, swap, undo, tab drag, or zoom. The event is emitted once per settled command.
 
-A server that advertises `layout-delta-v1` includes the screen's new layout in the event: `workspace`, and the layout fields of that `Screen` exactly as `list-workspaces` reports them at the commit (`active_pane`, `zoomed_pane`, `layout`, and when present `viewport_splits`, `viewport_base_width`, `columns`). It does not include `panes` or tabs; the pane and tab deltas carry those. Like `Screen`, the SDK schema does not list `viewport_splits`, `viewport_base_width` and `columns` yet. `transaction` is the numeric `transaction` of the request that caused the change (`set-split-ratio`, `set-viewport-pane-width`, `set-column-dock`, `set-row-heights`), as a decimal string, so the frontend that sent it can recognize the echo of its own gesture. A client patches that one screen's layout. When the event carries no layout (an older server), or names a pane the client does not have, clients re-fetch `export-layout` or `list-workspaces`.
+A server that advertises `layout-delta-v1` includes the screen's new layout in the event: `workspace`, and the layout fields of that `Screen` exactly as `list-workspaces` reports them at the commit (`active_pane`, `zoomed_pane`, `layout`, and when present `viewport_splits`, `viewport_base_width`, `columns`). It does not include `panes` or tabs; the pane and tab deltas carry those. Like `Screen`, the SDK schema does not list `viewport_splits`, `viewport_base_width` and `columns` yet. `transaction` is the numeric `transaction` of the request that caused the change (`set-split-ratio`, `set-viewport-pane-width`, `set-layout-sizes`, `set-column-dock`, `set-row-heights`), as a decimal string, so the frontend that sent it can recognize the echo of its own gesture. A client patches that one screen's layout. When the event carries no layout (an older server), or names a pane the client does not have, clients re-fetch `export-layout` or `list-workspaces`.
 
 Example: