cua mcp - #31
Merged
Merged
Conversation
Design for a `cua` MCP server exposing desktop and browser control, shipped as a single image built on trycua/cua-xfce with our Go binary as the stdio entrypoint and the desktop viewable over noVNC. Tool implementations are imported from github.com/mudler/nib rather than reimplemented; our binary handles config, tool aggregation over one stdio server, and a readiness gate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Seven tasks: config, tool aggregator, readiness gate, main wiring, container image, integration tests, docs. The aggregator code and its image-passthrough test were prototyped and verified against go-sdk v1.4.0 before being written into the plan. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…server Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The aggregator unmarshalled CallToolRequest.Params.Arguments into an `any` before re-marshalling it onto the upstream call. That round trip decodes every JSON number as a float64, so integers beyond 2^53 were silently rounded (9007199254740993 arrived upstream as 9007199254740992). Arguments is a json.RawMessage, which implements json.Marshaler and emits its bytes untouched, so it can be assigned straight to CallToolParams.Arguments. This makes the request direction as lossless as the response direction and drops the now-unused encoding/json import. Adds a spec that calls a tool with 2^53+1 and asserts the upstream observes the literal integer; it fails against the old code with the rounded value. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Task 1's go mod tidy raised the module's go directive to 1.26, so Task 5's Dockerfile must not consume the Makefile's GO_VERSION=1.25.1 build arg. Task 2's argument round-trip through `any` is replaced by direct json.RawMessage pass-through, per review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
axReportHasCapability returned true for any report shape it did not recognise, so every unanticipated shape claimed accessibility works -- the claim that makes computer_use attempt element-index addressing. Parse the ax_capability field with a regex instead and treat anything unparseable as no capability, so the server degrades to pixel-only rather than over-claiming. Also add an errDriverUnavailable sentinel wrapping only the spawn/connect failure, so Task 4 can tell "driver could not start" (fatal) from "driver ran, capability undetermined" (degrade). The health_report CallTool failure intentionally does not carry it. Cover the previously untested waitForDisplay remote-display branch and the context-cancellation branch of waitForDisplayAt. The new capability specs pin the unknown-shape policy only -- no spec asserts a guessed driver spelling back at itself. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
axReportHasCapability defaulted to false when the regex found no match, but
when it did match it used a denylist of negative values and returned true for
everything else. An unanticipated negative spelling ("unsupported", "no",
"error") therefore read as a working AT-SPI capability, so probeDriverAX would
report success, the degradation warning would never print, and computer_use
would attempt element-index addressing that cannot work.
Invert the switch to an allowlist so unknown falls to the safe side for both
shape and value, and correct the doc comment, which already claimed this
behaviour. Widen the regex capture class to [a-z0-9_]+ so a versioned backend
name like "atspi2" is captured accurately rather than truncated to "atspi".
Add a spec pinning the policy for an unrecognised value. The positive branch
stays unpinned until a real driver fixture is captured. Also split the
waitForDisplay Describe block, which named only one of its four specs.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rypoint Adds cua/main.go with the process entrypoint and startUpstreams, which launches the enabled nib MCP servers on in-memory transports and connects a client session to each. The version declaration moves here from aggregate.go so there is exactly one. Driver error handling is deliberately asymmetric: a driver that cannot be spawned is fatal, since computer_use cannot function without it, while a driver that spawns but fails the health_report call degrades to hasAX = false with a warning. The health_report tool name and response shape are the least certain part of this integration and a version drift there must not take down a working desktop. Also redirects nib's logger to stderr. nib logs via mudler/xlog, whose default sink is os.Stdout; for a stdio MCP server stdout is the JSON-RPC channel, so nib's startup line landed in the middle of the protocol stream. Only the destination is overridden, so COGITO_LOG_LEVEL keeps working as before. go.mod/go.sum gain nib/mcp's transitive dependencies, which were not needed until this first import of the package. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Four reviewer-approved fixes to the cua MCP server: - main: mcp.Server.Run returns ctx.Err() on a clean shutdown, so a SIGTERM hit log.Fatalf and exited 1, which any supervisor reads as a crash. Treat context.Canceled as a normal exit. - startUpstreams: nib's entrypoints can return an error before they begin serving (a driver that fails to connect). The server end of the in-memory transport pair is then never connected and never closed, so connectUpstream blocked on the initialize round-trip until the process was signalled. New startUpstream helper races the serve error against the connect under cfg.ReadyTimeout, turning the hang into a reported failure. - startUpstreams: close the already-established computer session when the browser upstream fails, so the session and its goroutine do not leak for callers other than main. - redirectNibLogsToStderr: honour LOG_FORMAT=json with a JSONHandler, matching xlog.NewLogger, instead of silently forcing text. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rtup time.ParseDuration accepts "0", "0s" and "-5s", so CUA_READY_TIMEOUT=0 -- "0 means unlimited" being a common operator convention -- overwrote the 60s default and made every upstream handshake deadline already-expired. Reject non-positive values in LoadConfig and clamp again in startUpstream, which is now directly callable with a bare duration. The browser spec no longer needs an explicit ReadyTimeout and builds a zero-value Config again. Also harden startUpstream: - give serve its own cancelable context so a failed connect stops the server, and with it the computer upstream's cua-driver subprocess, instead of leaving both alive until the parent ctx ends - drain and close a session that connects in the same instant the serve branch wins, rather than stranding it unread in the buffered channel - document that the established session outlives connCtx only because jsonrpc2 wraps the connection ctx in notDone Covering the never-attaches branch showed the timeout was unenforceable: the in-memory pair is a net.Pipe and jsonrpc2's writer polls ctx once before a blocking, deadline-free write, so cancelling connCtx cannot unblock Connect. Enforce the budget in startUpstream's own select instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
connCtx cancellation alone cannot unblock Connect: the in-memory pair is a net.Pipe and jsonrpc2's writer polls ctx once then writes without a deadline. The budget has to be enforced by an explicit select arm. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Builds FROM trycua/cua-xfce, adding cua-driver 0.9.1, Google Chrome with a
--no-sandbox wrapper on the paths nib's discoverChrome probes, and the stdio
entrypoint.
The base image turned out to be Ubuntu 22.04 rather than Debian Bookworm, which
moved several things:
* at-spi2-core and dbus-x11 already ship in it; jammy has no usable chromium
package (`chromium` absent, `chromium-browser` a snap stub), so the browser
comes from Google's signed apt repository instead.
* supervisord needs root to drop to the `cua` user per program, so the image
stays root rather than switching to USER cua.
* The entrypoint must NOT launch a session bus. The desktop's xstartup.sh
launches one as `cua` and XFCE brings up at-spi under it; a root-owned bus
started first is one the desktop user cannot connect to, which silently
empties the accessibility tree. Root reaches the a11y bus via the
AT_SPI_BUS X property and accessibility.conf's `<allow user="root"/>`.
* `cua-driver mcp` is only a client of a `cua-driver serve` daemon, which has
to run inside the desktop session to inherit that bus -- so xstartup.sh is
replaced with a copy that starts it.
Also corrects the stale GO_VERSION default, which asserted an unbuildable
1.25.1 against a go.mod requiring 1.26.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rtup Five reviewer-approved fixes to the cua container image. Stop vendoring the base image's xstartup.sh. cua/xstartup.sh was a byte copy of trycua/cua-xfce's /usr/local/bin/xstartup.sh with one line added, and the Dockerfile overwrote the original unconditionally -- so any future change trycua makes to their desktop startup would have been silently discarded, with no build or runtime error, only a desktop drifted from what the base expects. That risk compounds with the unpinned FROM ...:latest. The Dockerfile now greps for the base's bare `wait` line and sed-injects into it, failing the build with a diagnostic if that anchor ever disappears. Verified: a stage that mangles the anchor first fails with exit code 1. Supervise `cua-driver serve`. Every computer_use and browser_* call routes through `cua-driver mcp`, which is only a client of that daemon; when it died the container stayed healthy-looking while every tool call failed for the rest of its life, unlogged. It now runs under a respawn loop in cua-driver-supervise.sh, invoked from xstartup.sh so it keeps the session bus that the AT-SPI tree depends on -- a supervisord program would not. Its output goes to /tmp/cua-driver-serve.log, never stdout, which is the MCP JSON-RPC channel. Verified: SIGKILL of the daemon is followed by a restart within a second, recorded in the log, with AT-SPI intact afterwards. Make the entrypoint's driver-socket wait fail visibly. It ran 120 silent iterations and then fell through to a generic `initialize: EOF` that points nowhere near the cause; it now says so on stderr. Derive the driver socket path from HOME instead of hardcoding /home/cua, so a base-image HOME change surfaces as a real error rather than a silent 120s startup penalty. Point /usr/bin/google-chrome-stable at the wrapper too. nib does not probe that name, so nothing was broken, but it is the name the deb advertises and leaving it unwrapped meant no --no-sandbox and an opaque launch failure for anyone who reached for it. Also record the shipped Chrome version in /etc/cua-chrome-version, since the Google apt repo is unpinned. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
probeDriverAX decides whether cua-driver reports a working AT-SPI capability, and a false verdict warns the operator that computer_use will degrade to pixel coordinates with no element indices. That verdict was produced by a regex written from reading cua-driver's source rather than from a live driver, and ground truth captured from 0.9.1 shows it was not merely wrong on one case but non-discriminating: every Linux ax_capability message leads with the token X11, so the capture group took "x11" in all three scenarios and the allowlist rejected it every time. The probe returned false unconditionally against this driver, and adding "x11" to the allowlist would have inverted the bug rather than fixed it. Parse the health_report structuredContent instead, keying on checks[name == "ax_capability"].status == "pass". The status vocabulary is closed and small (pass | fail | skip), so an allowlist over it is genuinely exhaustive, unlike one over prose. Deliberately not the envelope's "overall" field: it aggregates every check, so an unrelated failure drags it to "degraded" while AT-SPI is fine, and because ax_capability is non-core it never reaches "failed" on AT-SPI's account -- the no-DISPLAY capture still reports only "degraded". The tool declares an inputSchema but no outputSchema, so structuredContent is guaranteed only by its prose description. A nil, unparseable, or wrong-shaped payload returns false rather than erroring, preserving the safe direction: unknown degrades to pixel-only with a warning. isError was false in all three captures including the failures, so a failing check is not treated as a call failure. probeDriverAX keeps its signature and its errDriverUnavailable behaviour; only its interpretation of the response changed. The parser stays pure and separately testable so a future driver change is a one-function fix, and cites the fixture that documents the shape and how to re-capture it. Tests carry the three captured payloads as testdata and assert the real verdicts -- working AT-SPI true, AT-SPI unreachable false, no DISPLAY false -- plus policy specs for the degenerate cases. The old specs encoding the prose shape are removed; they asserted a contract that does not exist. Verified end to end against ghcr.io/mudler/mcps/cua:latest: the baseline image logs the warning while the driver reports ax_capability status pass, and the same image with the fixed binary reaches "computer_use MCP server ready" with no warning at all. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Task 5's section is rewritten to record what was actually built: the base is Ubuntu not Debian, Chrome replaces the absent chromium, the entrypoint must NOT own the D-Bus session, and cua-driver mcp needs a cua-driver serve daemon running inside the desktop session. Task 3's probe now keys on structuredContent.checks[ax_capability].status, captured from a live 0.9.1 driver. The previous regex was non-discriminating: every Linux ax_capability message leads with "X11", so it returned false in every scenario including the working one. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
sed substitutes globally, so a future base image with two bare `wait` lines would inject two supervisors racing two cua-driver daemons over one socket -- silently, through the guard meant to prevent exactly that. Assert the count is 1 on both sides of the substitution, and inject an absolute path so the injection does not depend on the base's PATH. healthReport's schema_version/overall and the item's message were modelled but never read; overall in particular sat one keystroke from the aggregate the parser must not use. Drop them and record why in a comment. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…t tree, and stdout purity Drives the real ghcr.io/mudler/mcps/cua:latest image over stdio MCP the way a client would, behind a //go:build integration tag so the normal suite is unaffected. Run with: go test -tags integration ./cua/ The load-bearing spec keys on cua-driver's own health_report structuredContent — checks[name==ax_capability].status == "pass" — asked through a live `cua-driver mcp` client inside the running container, which is the same authority probeDriverAX consults. Its consequence is asserted separately: a capture of a real window must return a non-empty, addressable element tree, which is what a broken AT-SPI setup would empty out. stdout is teed out of the transport so the protocol channel can be audited line by line afterwards, with stderr checked for supervisord output so a silent container cannot pass that spec vacuously. Missing Docker or a missing image skips with a stated reason rather than failing for environmental causes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds a README section for the container-only cua server: the 8 aggregated tools, the CUA_* and inherited VNC_* environment variables verified against cua/config.go and the image, the docker run and LocalAI stdio wiring, and the accepted trade-offs -- root Chrome with --no-sandbox, the unpinned ~6.4 GB base, unauthenticated VNC without VNC_PW, and XSendEvent input without /dev/uinput. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
cogito b618670 stops a boolean JSON Schema (2020-12 allows `true`/`false` wherever a schema is allowed) from failing the conversion into langchaingo Definitions, which silently dropped the ENTIRE tool from the model's tool list. google/jsonschema-go marshals an empty schema as `true`, so any Go `any` / [][]any field in a tool's input produces "items": true and loses the tool. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
No description provided.