Skip to content

feat(nix): add label-triggered cua-driver screenshot test - #1748

Merged
r33drichards merged 4 commits into
mainfrom
feat/cua-driver-screenshot-label
May 28, 2026
Merged

feat(nix): add label-triggered cua-driver screenshot test#1748
r33drichards merged 4 commits into
mainfrom
feat/cua-driver-screenshot-label

Conversation

@r33drichards

@r33drichards r33drichards commented May 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Adds a cua-driver-screenshot label that triggers a NixOS VM integration test with screenshot capture
  • When the label is applied to any PR, the workflow:
    1. Runs the full cua-driver integration test (same subtests as the existing one)
    2. Opens xterm on Xvfb showing test results, captures VM framebuffer via machine.screenshot()
    3. Uploads the screenshot as a GitHub artifact
    4. Comments on the PR with a link to the artifact
    5. Removes the cua-driver-screenshot label automatically
  • Also registers checks.x86_64-linux.cua-driver-screenshot in the flake for local use

New files

File Purpose
nix/cua-driver/tests/screenshot.nix Integration test variant with machine.screenshot()
.github/workflows/nix-screenshot.yml Label-triggered workflow

Usage

  1. Add the cua-driver-screenshot label to any PR
  2. The workflow runs the test and posts the screenshot as a comment
  3. The label is automatically removed after completion

Test plan

  • Apply cua-driver-screenshot label to this PR to trigger the workflow
  • Verify screenshot artifact is uploaded
  • Verify PR comment is posted with artifact link
  • Verify label is removed after completion

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Tests
    • Added automated screenshot testing for the CUA Driver with MCP protocol validation and integration testing.
  • Chores
    • Added GitHub Actions workflow to run screenshot tests on labeled pull requests and upload artifacts for review.

Review Change Stack

Adds a `cua-driver-screenshot` label trigger that:
1. Runs the full NixOS VM integration test with screenshot capture
2. Captures a QEMU framebuffer screenshot showing xterm with test results
3. Uploads the screenshot as a GitHub artifact
4. Comments on the PR with a link to the screenshot
5. Removes the `cua-driver-screenshot` label

New files:
- nix/cua-driver/tests/screenshot.nix: integration test variant with
  machine.screenshot() at the end
- .github/workflows/nix-screenshot.yml: label-triggered workflow

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Ignored Ignored Preview May 28, 2026 6:35pm

Request Review

@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5db8bb83-07b3-4aad-8b40-7fb0da967f98

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR introduces screenshot-based integration testing for the CUA driver. It adds a new NixOS VM test that validates the driver's MCP protocol implementation and renders display output, integrates this test into the flake's checks, and provides a GitHub Actions workflow that automates test execution with AWS credential management and artifact handling.

Changes

Screenshot Testing Infrastructure

Layer / File(s) Summary
Nix integration test definition
nix/cua-driver/tests/screenshot.nix
Defines an embedded Python MCP client that validates JSON-RPC protocol, tool discovery, and tool calls; generates a shell script test page; configures NixOS VM with X display, CUA driver, and required packages; orchestrates VM boot, CLI checks, MCP testing under display, page rendering, and screenshot capture.
Flake check registration
flake.nix
Adds cua-driver-screenshot check to flake's x86_64-linux outputs, importing the new test definition and wiring in pkgs, lib, and cuaDriverModule configuration.
GitHub Actions CI workflow
.github/workflows/nix-screenshot.yml
Implements workflow triggered by cua-driver-screenshot label: authenticates via AWS OIDC, retrieves Nix signing key from Secrets Manager, installs Nix, runs the check, extracts PNG artifacts, posts PR comment with results, removes label, signs and uploads artifacts to S3 cache, and cleans up credentials.

Sequence Diagram(s)

The PR introduces a coordinated flow across Nix test infrastructure and CI automation. The primary interaction occurs between the NixOS VM test runner, CUA driver process, and MCP client protocol validator, followed by screenshot capture and CI artifact management:

sequenceDiagram
  participant GitHub as GitHub Actions
  participant AWS as AWS Secrets Manager
  participant Nix as Nix Build
  participant VM as NixOS VM
  participant Driver as CUA Driver
  participant MCP as MCP Client
  participant Cache as S3 Cache
  GitHub->>AWS: Assume OIDC Role
  AWS->>GitHub: Return Credentials
  GitHub->>AWS: Fetch Signing Key
  AWS->>GitHub: Return Key Material
  GitHub->>Nix: Install + Configure
  Nix->>VM: Spawn Test VM
  VM->>Driver: Boot with cua-driver enabled
  VM->>Driver: CLI validation (--help, list-tools)
  MCP->>Driver: Send JSON-RPC init + tool requests
  Driver->>MCP: Respond with capabilities/tools
  VM->>VM: Start Xvfb Display
  VM->>VM: Render xterm Test Page
  VM->>VM: Capture Screenshot
  Nix->>GitHub: Return PNG Artifacts
  GitHub->>Cache: Sign & Upload to S3
  GitHub->>GitHub: Post PR Comment
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • trycua/cua#1746: Extends the existing flake.nix checks pattern by adding a new cua-driver-screenshot check that reuses the same ./nix/cua-driver/module.nix and cuaDriverPackage wiring pattern.

Poem

A rabbit's screenshot test takes flight,
Through Nix VMs rendering x-ray sight,
MCP protocol checks, PNG in the wild,
CI caches signed—automation's styled! 📸✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the main change: adding a label-triggered screenshot test for the cua-driver in Nix, which is the core objective of this PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/cua-driver-screenshot-label

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@r33drichards r33drichards added the cua-driver-screenshot Trigger cua-driver NixOS screenshot test label May 28, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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 @.github/workflows/nix-screenshot.yml:
- Around line 25-27: The workflow step using actions/checkout (uses:
actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5) leaves a writable
token in the local git config; update that checkout step to set
persist-credentials: false so the action does not persist the credentials to the
repo's git config during the job, ensuring the checkout step includes the
persist-credentials: false option.
- Around line 141-147: The "Sign and upload to Nix cache" step (which runs nix
store sign --key-file ... and nix copy --to ...) must not execute in untrusted
pull_request workflows; remove or guard this step so it does not run for PR
events and instead move signing/upload to a separate trusted workflow that
triggers only on protected branch pushes or a release (e.g., create a new
workflow that runs the nix store sign and nix copy commands on push to main with
required protections). Specifically, delete or conditionally disable the
existing step (the "Sign and upload to Nix cache" job/step) from the PR workflow
and implement a new workflow that performs nix store sign and nix copy only in
the trusted context.
🪄 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: f97e3ef8-33ab-4b51-a4f1-ad7e4709655d

📥 Commits

Reviewing files that changed from the base of the PR and between a145f20 and ca1f036.

📒 Files selected for processing (3)
  • .github/workflows/nix-screenshot.yml
  • flake.nix
  • nix/cua-driver/tests/screenshot.nix

Comment on lines +25 to +27
- name: Checkout
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Disable checkout credential persistence.

actions/checkout should set persist-credentials: false to avoid leaving a writable token in local git config during the job.

Suggested fix
       - name: Checkout
         uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
+        with:
+          persist-credentials: false
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Checkout
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
- name: Checkout
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
with:
persist-credentials: false
🧰 Tools
🪛 zizmor (1.25.2)

[warning] 25-26: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)

🤖 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 @.github/workflows/nix-screenshot.yml around lines 25 - 27, The workflow step
using actions/checkout (uses:
actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5) leaves a writable
token in the local git config; update that checkout step to set
persist-credentials: false so the action does not persist the credentials to the
repo's git config during the job, ensuring the checkout step includes the
persist-credentials: false option.

Comment on lines +141 to +147
- name: Sign and upload to Nix cache
if: always()
run: |
echo "Signing and uploading build artifacts to Nix cache..."
nix store sign --key-file "${{ runner.temp }}/signing-key.sec" --all
nix copy --to "s3://${{ env.NIX_CACHE_BUCKET }}?region=${{ env.AWS_REGION }}&want-mass-query=true" --all -L

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Do not sign/upload cache artifacts from a pull_request workflow.

This job fetches a trusted signing key and then signs/uploads store paths for PR code. That creates a cache-poisoning/supply-chain risk in an untrusted execution context.

Suggested fix
       - name: Sign and upload to Nix cache
-        if: always()
+        if: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }}
         run: |
           echo "Signing and uploading build artifacts to Nix cache..."
           nix store sign --key-file "${{ runner.temp }}/signing-key.sec" --all
           nix copy --to "s3://${{ env.NIX_CACHE_BUCKET }}?region=${{ env.AWS_REGION }}&want-mass-query=true" --all -L

Move cache signing/publishing to a separate trusted workflow (e.g., push on protected branch) rather than this PR-label workflow.

🤖 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 @.github/workflows/nix-screenshot.yml around lines 141 - 147, The "Sign and
upload to Nix cache" step (which runs nix store sign --key-file ... and nix copy
--to ...) must not execute in untrusted pull_request workflows; remove or guard
this step so it does not run for PR events and instead move signing/upload to a
separate trusted workflow that triggers only on protected branch pushes or a
release (e.g., create a new workflow that runs the nix store sign and nix copy
commands on push to main with required protections). Specifically, delete or
conditionally disable the existing step (the "Sign and upload to Nix cache"
job/step) from the PR workflow and implement a new workflow that performs nix
store sign and nix copy only in the trusted context.

@github-actions github-actions Bot removed the cua-driver-screenshot Trigger cua-driver NixOS screenshot test label May 28, 2026
- Use `import -window root` (ImageMagick) + `copy_from_vm` to capture
  the Xvfb display, not `machine.screenshot()` which captures the
  blank QEMU VGA console and hangs
- Remove unused `glob` require that caused JS SyntaxError in comment step
- Add imagemagick to VM packages

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@r33drichards r33drichards added the cua-driver-screenshot Trigger cua-driver NixOS screenshot test label May 28, 2026
@github-actions

Copy link
Copy Markdown
Contributor

CUA Driver Screenshot Test

⚠️ No screenshots were captured. Check the workflow run for details.

@github-actions github-actions Bot removed the cua-driver-screenshot Trigger cua-driver NixOS screenshot test label May 28, 2026
ImageMagick's `import -window root` hangs in headless Xvfb. Switch to
xwd + xwdtopnm + pnmtopng pipeline which is more reliable. Also use
copy_from_machine (not deprecated copy_from_vm) and bump step timeout.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@r33drichards r33drichards added the cua-driver-screenshot Trigger cua-driver NixOS screenshot test label May 28, 2026
@github-actions

Copy link
Copy Markdown
Contributor

CUA Driver Screenshot Test

⚠️ No screenshots were captured. Check the workflow run for details.

@github-actions github-actions Bot removed the cua-driver-screenshot Trigger cua-driver NixOS screenshot test label May 28, 2026
machine.succeed("sleep N") blocks indefinitely in NixOS VM tests.
Use Python's time.sleep() in the test driver instead. Also add
timeout to xwd and suppress stderr noise from netpbm.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@r33drichards r33drichards added the cua-driver-screenshot Trigger cua-driver NixOS screenshot test label May 28, 2026
@github-actions

Copy link
Copy Markdown
Contributor

CUA Driver Screenshot Test

Screenshot captured from NixOS VM integration test.

📸 Download screenshot artifact

✅ Test passed

@github-actions github-actions Bot removed the cua-driver-screenshot Trigger cua-driver NixOS screenshot test label May 28, 2026
@r33drichards
r33drichards merged commit 8d7e546 into main May 28, 2026
9 checks passed
r33drichards pushed a commit that referenced this pull request May 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants