feat(nix): add Nix package and NixOS integration test for cua-driver - #1746
Merged
Conversation
Adds Nix infrastructure to build and test the cua-driver Rust binary on Linux: - flake.nix: Top-level flake with packages, nixosModules, and checks - nix/cua-driver/package.nix: Rust build via rustPlatform.buildRustPackage - nix/cua-driver/module.nix: NixOS module (services.cua-driver.enable) - nix/cua-driver/tests/integration.nix: VM integration test covering CLI subcommands, MCP protocol handshake, and tool invocation with Xvfb Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR introduces a complete Nix flake infrastructure for building and deploying cua-driver, a Rust-based MCP server. It defines package derivation, NixOS service options, flake exports, and end-to-end integration tests covering CLI behavior, X11 display interaction, and JSON-RPC protocol handshake. ChangesNix Flake Build and Service Setup
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
flake.nix (1)
57-60: ⚡ Quick winExport a ready-to-use module with package prewired.
Line 57 currently exports the raw module, while
services.cua-driver.packagehas no default innix/cua-driver/module.nix(Lines 24-31). Consider exporting a wrapped module that sets a default package fromself.packages.${pkgs.system}.cua-driverto avoid easy consumer misconfiguration.♻️ Suggested flake export tweak
- # NixOS module — consumers must set services.cua-driver.package - # (or use the per-system package from self.packages) - nixosModules.cua-driver = ./nix/cua-driver/module.nix; + # NixOS module with default package wiring + nixosModules.cua-driver = { pkgs, lib, ... }: { + imports = [ ./nix/cua-driver/module.nix ]; + services.cua-driver.package = lib.mkDefault self.packages.${pkgs.system}.cua-driver; + };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@flake.nix` around lines 57 - 60, Export a wrapped NixOS module instead of the raw one so services.cua-driver.package gets a sensible default; specifically, change the export at nixosModules.cua-driver to call/overlay the module (from ./nix/cua-driver/module.nix) with a default for services.cua-driver.package pointing to self.packages.${pkgs.system}.cua-driver (use a wrapper or mkForce/defaults mechanism in the flake evaluation to inject that default while preserving the original module API).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@nix/cua-driver/tests/integration.nix`:
- Around line 156-161: The package list in environment.systemPackages only
includes xorg.xorgserver but the test calls xdpyinfo and will fail if its
package isn't present; update the environment.systemPackages array (where
xorg.xorgserver is listed) to also include xorg.xdpyinfo so xdpyinfo is
available during the integration test run.
- Around line 113-121: The current check treats any response with a "result" key
as a pass even if that result contains an error payload; modify the logic so
that when "result" in resp you also verify the result is not an error and
contains valid content: check that resp["result"] does not include an "error"
key (e.g., if resp["result"].get("error") is truthy, treat it like the "error"
branch) and require non-empty content before printing the PASS message for
get_screen_size; otherwise raise an AssertionError or route to the existing
error handling branch. Ensure you update the block that reads resp, content, and
text to implement these extra checks.
---
Nitpick comments:
In `@flake.nix`:
- Around line 57-60: Export a wrapped NixOS module instead of the raw one so
services.cua-driver.package gets a sensible default; specifically, change the
export at nixosModules.cua-driver to call/overlay the module (from
./nix/cua-driver/module.nix) with a default for services.cua-driver.package
pointing to self.packages.${pkgs.system}.cua-driver (use a wrapper or
mkForce/defaults mechanism in the flake evaluation to inject that default while
preserving the original module API).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ee0af4bc-e9bd-44d6-bc4c-e75c0d33d249
⛔ Files ignored due to path filters (1)
flake.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
flake.nixnix/cua-driver/module.nixnix/cua-driver/package.nixnix/cua-driver/tests/integration.nix
Comment on lines
+113
to
+121
| if "result" in resp: | ||
| content = resp["result"].get("content", []) | ||
| text = content[0].get("text", "") if content else "" | ||
| print(f"PASS: get_screen_size returned: {text[:200]}", flush=True) | ||
| elif "error" in resp: | ||
| err_msg = resp.get("error", {}).get("message", "unknown") | ||
| print(f"PASS: get_screen_size returned error (acceptable in headless): {err_msg}", flush=True) | ||
| else: | ||
| raise AssertionError(f"Unexpected response: {resp}") |
Contributor
There was a problem hiding this comment.
Strengthen get_screen_size assertions to prevent false-positive passes.
Any response containing "result" currently passes, even if the tool reports an error payload. This weakens the integration contract for tools/call.
Suggested patch
# get_screen_size should return display dimensions from Xvfb
if "result" in resp:
- content = resp["result"].get("content", [])
- text = content[0].get("text", "") if content else ""
- print(f"PASS: get_screen_size returned: {text[:200]}", flush=True)
+ is_error = resp["result"].get("isError", False)
+ assert not is_error, f"get_screen_size returned isError=true: {resp}"
+ sc = resp["result"].get("structuredContent", {})
+ assert sc.get("width", 0) > 0, f"Invalid width: {resp}"
+ assert sc.get("height", 0) > 0, f"Invalid height: {resp}"
+ print(f"PASS: get_screen_size returned {sc}", flush=True)
elif "error" in resp:
err_msg = resp.get("error", {}).get("message", "unknown")
print(f"PASS: get_screen_size returned error (acceptable in headless): {err_msg}", flush=True)
else:
raise AssertionError(f"Unexpected response: {resp}")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@nix/cua-driver/tests/integration.nix` around lines 113 - 121, The current
check treats any response with a "result" key as a pass even if that result
contains an error payload; modify the logic so that when "result" in resp you
also verify the result is not an error and contains valid content: check that
resp["result"] does not include an "error" key (e.g., if
resp["result"].get("error") is truthy, treat it like the "error" branch) and
require non-empty content before printing the PASS message for get_screen_size;
otherwise raise an AssertionError or route to the existing error handling
branch. Ensure you update the block that reads resp, content, and text to
implement these extra checks.
Comment on lines
+156
to
+161
| environment.systemPackages = with pkgs; [ | ||
| xorg.xorgserver # Xvfb for headless X11 | ||
| python3 # MCP client test script | ||
| jq | ||
| procps # pgrep/pkill | ||
| ]; |
Contributor
There was a problem hiding this comment.
Add the package that provides xdpyinfo to avoid command-not-found failures.
The test executes xdpyinfo (Line 186), but the package list only adds xorg.xorgserver. Please add xorg.xdpyinfo explicitly.
Suggested patch
environment.systemPackages = with pkgs; [
xorg.xorgserver # Xvfb for headless X11
+ xorg.xdpyinfo # xdpyinfo used to verify DISPLAY
python3 # MCP client test script
jq
procps # pgrep/pkill
];Also applies to: 186-186
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@nix/cua-driver/tests/integration.nix` around lines 156 - 161, The package
list in environment.systemPackages only includes xorg.xorgserver but the test
calls xdpyinfo and will fail if its package isn't present; update the
environment.systemPackages array (where xorg.xorgserver is listed) to also
include xorg.xdpyinfo so xdpyinfo is available during the integration test run.
Runs on PRs and pushes to main when nix/**, flake.nix, flake.lock, or libs/cua-driver/rust/** change. Uses OIDC to access the shared S3 nix binary cache. Signs and pushes artifacts on main branch merges. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The integration test already builds the package as a dependency, making the separate build and smoke test steps redundant. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove unused `import json` from testScript (ruff F401 lint failure) - Always sign+upload to S3 cache, even on failed runs, so partial build artifacts are cached for faster retries - Add want-mass-query=true to skip uploading paths that already exist Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
doctor's AT-SPI probe can hang without a proper D-Bus session bus. Wrap doctor with timeout 15s and the MCP test with timeout 60s. Remove xdpyinfo check that may not be available. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
machine.succeed blocks even with &. Use machine.execute which fires-and-forgets, then wait_until_succeeds for the X11 socket. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced May 28, 2026
This was referenced Jun 8, 2026
This was referenced Jun 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
flake.nix) to the cua repo withpackages.{x86_64,aarch64}-linux.cua-driverandchecks.x86_64-linux.cua-driver-integrationnix/cua-driver/package.nix: Builds the Rust cua-driver binary viarustPlatform.buildRustPackage(pure Rust deps, no system libraries needed)nix/cua-driver/module.nix: NixOS module (services.cua-driver.enable) that installs the binary + runtime deps (ImageMagick, AT-SPI, D-Bus)nix/cua-driver/tests/integration.nix: NixOS VM integration test that:list-tools,describe,doctor)click,type_text,get_screen_size,get_window_stateBuild verified locally
Note
The
.github/workflows/nix-build.ymlCI workflow was prepared but could not be pushed due to OAuth scope restrictions. It needs to be added separately (requiresworkflowscope). The workflow depends on trycua/cloud#TBD being merged first (OIDC trust for nix cache).Test plan
nix build .#cua-driverproduces working binary./result/bin/cua-driver list-toolslists 34 toolsnix flake showshows correct outputsnix build .#checks.x86_64-linux.cua-driver-integrationpasses on x86_64-linux🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests