fix(cua-driver-rs)(linux): set_config {key,value} now persists (Closes #1923)

.github/workflows/nix-build.yml

@@ -43,6 +43,12 @@ jobs:
             visual: false
             result_link: result-cua-driver-integration
             artifact_name: ""
+          - name: NixOS set_config persistence test
+            check_attr: cua-driver-set-config
+            timeout_minutes: 8
+            visual: false
+            result_link: result-cua-driver-set-config
+            artifact_name: ""
           - name: Linux cursor click GIF test
             check_attr: cua-driver-linux-cursor-click-gif
             timeout_minutes: 12

flake.nix

@@ -51,6 +51,18 @@
                 };
               };
 
+              # set_config persistence test — regression for #1923 (fixed in
+              # #1928): the {key, value} write shape must persist and read back
+              # via get_config (it was silently dropped on Linux before).
+              cua-driver-set-config = import ./nix/cua-driver/tests/set-config.nix {
+                inherit pkgs;
+                inherit (pkgs) lib;
+                cuaDriverModule = {
+                  imports = [ ./nix/cua-driver/module.nix ];
+                  services.cua-driver.package = cuaDriverPackage;
+                };
+              };
+
               # Screenshot test — uses cua-driver's own get_window_state tool
               # to capture a screenshot via MCP, proving the driver can see the display
               cua-driver-screenshot = import ./nix/cua-driver/tests/screenshot.nix {

libs/cua-driver/rust/crates/platform-linux/src/tools/impl_.rs

@@ -2855,13 +2855,19 @@ impl Tool for SetConfigTool {
         SCFG_DEF.get_or_init(|| ToolDef {
             name: "set_config".into(),
             description: "Update cua-driver-rs configuration. capture_mode / \
-                max_image_dimension take effect immediately. The experimental_pip \
-                keys persist to ~/.cua-driver/config.json and apply on next \
+                max_image_dimension take effect immediately.\n\n\
+                Two input shapes (both accepted, matching Windows/Swift):\n\
+                - **{key, value}** (preferred): `{\"key\": \"max_image_dimension\", \"value\": 800}` \
+                  — single leaf write.\n\
+                - **Legacy per-field**: `{\"capture_mode\": \"som\", \"max_image_dimension\": 0}`.\n\n\
+                The experimental_pip keys persist to ~/.cua-driver/config.json and apply on next \
                 daemon restart (the PiP backend is initialised once at startup; \
                 Linux ships only the trait stub today — see issue #1729).".into(),
             input_schema: json!({"type":"object","properties":{
-                "capture_mode":{"type":"string","enum":["som","vision","ax"],"description":"Default capture mode for get_window_state."},
-                "max_image_dimension":{"type":"integer","description":"Max dimension for screenshot resizing (0 = no limit)."},
+                "key":{"type":"string","description":"Name of a single config field to write ({key, value} shape). Pair with `value`."},
+                "value":{"description":"New value for `key`. JSON type depends on the key."},
+                "capture_mode":{"type":"string","enum":["som","vision","ax"],"description":"Legacy per-field shape. Default capture mode for get_window_state."},
+                "max_image_dimension":{"type":"integer","description":"Legacy per-field shape. Max dimension for screenshot resizing (0 = no limit)."},
                 "experimental_pip":{"type":"boolean","description":"Enable the experimental PiP preview window (applies next restart; Linux backend stubbed)."},
                 "experimental_pip_geometry":{"type":"string","description":"PiP window size + optional position in `WxH` or `WxH+X+Y` form."}
             },"additionalProperties":false}),
@@ -2872,6 +2878,52 @@ impl Tool for SetConfigTool {
         use cua_driver_core::tool_args::ArgsExt;
         let mut cfg = self.state.config.write().unwrap();
         let mut parts = Vec::new();
+        // {key, value} shape (what the Swift/macOS and Windows callers send).
+        // Linux previously read only the legacy per-field keys below, so a
+        // `{"key":"max_image_dimension","value":800}` write was silently
+        // dropped (issue #1923). Dispatch on `key` to the same fields.
+        if let (Some(key), Some(val)) = (
+            args.get("key").and_then(|v| v.as_str()),
+            args.get("value"),
+        ) {
+            match key {
+                "capture_mode" => match val.as_str() {
+                    Some(s) => { cfg.capture_mode = s.to_owned(); parts.push(format!("capture_mode={s}")); }
+                    None => return ToolResult::error(format!("`capture_mode` must be a string, got {val}.")),
+                },
+                "max_image_dimension" => match val.as_u64() {
+                    Some(n) => { cfg.max_image_dimension = n as u32; parts.push(format!("max_image_dimension={n}")); }
+                    None => return ToolResult::error(format!("`max_image_dimension` must be an integer, got {val}.")),
+                },
+                "experimental_pip" => match val.as_bool() {
+                    Some(b) => {
+                        if let Err(e) = pip_preview::write_config_key("experimental_pip", Value::Bool(b)) {
+                            return ToolResult::error(format!("failed to persist experimental_pip: {e}"));
+                        }
+                        parts.push(format!("experimental_pip={b} (next restart)"));
+                    }
+                    None => return ToolResult::error(format!("`experimental_pip` must be a boolean, got {val}.")),
+                },
+                "experimental_pip_geometry" => match val.as_str() {
+                    Some(s) => {
+                        if pip_preview::PipGeometry::parse(s).is_none() {
+                            return ToolResult::error(format!(
+                                "experimental_pip_geometry `{s}` is not a valid WxH or WxH+X+Y string"
+                            ));
+                        }
+                        if let Err(e) = pip_preview::write_config_key("experimental_pip_geometry", Value::String(s.to_owned())) {
+                            return ToolResult::error(format!("failed to persist experimental_pip_geometry: {e}"));
+                        }
+                        parts.push(format!("experimental_pip_geometry={s} (next restart)"));
+                    }
+                    None => return ToolResult::error(format!("`experimental_pip_geometry` must be a string, got {val}.")),
+                },
+                other => return ToolResult::error(format!(
+                    "Unknown config key `{other}`. Known: capture_mode, max_image_dimension, experimental_pip, experimental_pip_geometry."
+                )),
+            }
+        }
+        // Legacy per-field shape.
         if let Some(mode) = args.opt_str("capture_mode") {
             parts.push(format!("capture_mode={mode}"));
             cfg.capture_mode = mode;

nix/cua-driver/tests/set-config.nix

@@ -0,0 +1,186 @@
+# CUA Driver set_config Persistence Test
+#
+# Regression test for #1923 (fixed in PR #1928): on Linux the `set_config`
+# tool only read the legacy per-field keys, so a `{"key":..., "value":...}`
+# write (the shape the Swift/macOS and Windows callers send) was silently
+# dropped. This boots a NixOS VM, drives the driver over MCP stdio, writes
+# config via the `{key, value}` shape, then reads it back with `get_config`
+# and asserts the new value persisted.
+#
+# To run: nix build .#checks.x86_64-linux.cua-driver-set-config
+#
+{
+  pkgs,
+  lib ? pkgs.lib,
+  cuaDriverModule,
+  ...
+}:
+
+let
+  # Python MCP client that drives cua-driver over stdio.
+  # Writes config via the {key, value} shape, reads it back, asserts.
+  setConfigTest = pkgs.writeText "set-config-test.py" ''
+    import subprocess
+    import json
+    import sys
+    import os
+    import threading
+    import time
+
+    DRIVER_BIN = os.environ.get("CUA_DRIVER_BIN", "cua-driver")
+
+    def main():
+        print("=== CUA Driver set_config Persistence Test ===", flush=True)
+
+        proc = subprocess.Popen(
+            [DRIVER_BIN, "mcp", "--no-daemon-relaunch"],
+            stdin=subprocess.PIPE,
+            stdout=subprocess.PIPE,
+            stderr=subprocess.PIPE,
+            env={**os.environ},
+        )
+
+        # Drain stderr in background to prevent blocking
+        def drain_stderr():
+            for line in proc.stderr:
+                sys.stderr.buffer.write(line)
+                sys.stderr.buffer.flush()
+        t = threading.Thread(target=drain_stderr, daemon=True)
+        t.start()
+
+        def send_request(method, params=None, req_id=None):
+            msg = {"jsonrpc": "2.0", "method": method}
+            if params is not None:
+                msg["params"] = params
+            if req_id is not None:
+                msg["id"] = req_id
+            line = json.dumps(msg) + "\n"
+            print(f"[send] {line.strip()}", flush=True)
+            proc.stdin.write(line.encode())
+            proc.stdin.flush()
+
+        def read_response(timeout=30):
+            # Simple blocking read with timeout via thread
+            result = [None]
+            def reader():
+                result[0] = proc.stdout.readline()
+            rt = threading.Thread(target=reader)
+            rt.start()
+            rt.join(timeout)
+            if rt.is_alive():
+                raise TimeoutError("No response within timeout")
+            line = result[0].decode().strip()
+            print(f"[recv] {line}", flush=True)
+            return json.loads(line)
+
+        def call_tool(name, arguments, req_id):
+            send_request("tools/call", {"name": name, "arguments": arguments}, req_id=req_id)
+            resp = read_response()
+            assert resp.get("id") == req_id, f"Expected id={req_id}, got {resp.get('id')}"
+            assert "result" in resp, f"Expected result in response: {resp}"
+            return resp["result"]
+
+        def get_config(req_id):
+            result = call_tool("get_config", {}, req_id)
+            sc = result.get("structuredContent", {})
+            assert sc, f"get_config returned no structuredContent: {result}"
+            return sc
+
+        try:
+            # Initialize
+            send_request("initialize", {
+                "protocolVersion": "2024-11-05",
+                "capabilities": {},
+                "clientInfo": {"name": "nixos-set-config-test", "version": "1.0.0"},
+            }, req_id=1)
+            resp = read_response()
+            assert resp.get("id") == 1 and "result" in resp, f"initialize failed: {resp}"
+            send_request("notifications/initialized", {})
+            time.sleep(0.5)
+
+            # Baseline: confirm the value we will write differs from the default
+            # (max_image_dimension default is 1568), so a passing read-back can
+            # only mean the write took effect — not a coincidental default.
+            print("\n--- Baseline get_config ---", flush=True)
+            before = get_config(2)
+            print(f"before: max_image_dimension={before.get('max_image_dimension')} "
+                  f"capture_mode={before.get('capture_mode')}", flush=True)
+            assert before.get("max_image_dimension") != 800, \
+                f"baseline already 800; cannot prove persistence: {before}"
+
+            # The regression: write via the {key, value} shape (#1923). This is
+            # the exact path that was silently dropped on Linux before #1928.
+            print("\n--- set_config {key, value} ---", flush=True)
+            call_tool("set_config", {"key": "max_image_dimension", "value": 800}, 3)
+            call_tool("set_config", {"key": "capture_mode", "value": "ax"}, 4)
+
+            # Read back: get_config must reflect the {key, value} writes.
+            print("\n--- get_config read-back ---", flush=True)
+            after = get_config(5)
+            print(f"after: max_image_dimension={after.get('max_image_dimension')} "
+                  f"capture_mode={after.get('capture_mode')}", flush=True)
+            assert after.get("max_image_dimension") == 800, \
+                f"set_config {{key,value}} did not persist max_image_dimension: {after}"
+            assert after.get("capture_mode") == "ax", \
+                f"set_config {{key,value}} did not persist capture_mode: {after}"
+
+            print("\n=== set_config persistence test passed! ===", flush=True)
+
+        finally:
+            proc.stdin.close()
+            proc.terminate()
+            proc.wait(timeout=5)
+
+    if __name__ == "__main__":
+        main()
+  '';
+
+in
+
+pkgs.testers.nixosTest {
+  name = "cua-driver-set-config-test";
+  meta = {
+    maintainers = [ ];
+  };
+
+  nodes.machine =
+    {
+      config,
+      pkgs,
+      lib,
+      ...
+    }:
+    {
+      imports = [ cuaDriverModule ];
+      virtualisation = {
+        cores = 2;
+        memorySize = 2048;
+      };
+      services.cua-driver.enable = true;
+      environment.systemPackages = with pkgs; [
+        xorg.xorgserver # Xvfb for headless X11
+        python3 # MCP client test script
+      ];
+    };
+
+  testScript = ''
+    machine.start()
+    machine.wait_for_unit("multi-user.target")
+
+    with subtest("Binary exists and runs"):
+        machine.succeed("cua-driver --help")
+
+    with subtest("Start Xvfb"):
+        machine.execute("Xvfb :99 -screen 0 1280x1024x24 >/dev/null 2>&1 &")
+        machine.wait_until_succeeds("test -e /tmp/.X11-unix/X99", timeout=10)
+
+    with subtest("set_config {key, value} persists and reads back via get_config"):
+        machine.copy_from_host("${setConfigTest}", "/tmp/set-config-test.py")
+        result = machine.succeed(
+            "timeout 60 env DISPLAY=:99 "
+            "python3 /tmp/set-config-test.py 2>&1"
+        )
+        machine.log(result)
+        assert "set_config persistence test passed" in result, f"set_config test failed: {result}"
+  '';
+}