Repository navigation
Conversation
Claude reviews each internal PR on a Mac CI agent with full shell access, so it can build, run tests and use simulators. The step fails only when Claude proves a bug caused by the PR. Claude runs with an allowlisted environment (the Anthropic key and the test site sign-in) and has no GitHub or Buildkite agent token. The script fetches the PR data for it and posts its result as a comment-only review.
Headless Xcode would ask before Claude can use it, and nobody can answer on CI, so the script enables it for every agent on the throwaway VM. Claude falls back to xcodebuild and simctl when that fails.
Claude builds the app itself when it needs to. Also pins the effort level to medium, the current default, so an update can't change it silently.
- Any unexpected failure now exits 2 (retried once), so exit 1 only ever means a fail verdict. Annotating the build is best-effort. - A review thread only counts as Claude's if wpmobilebot started it, not just if it carries the hidden marker, which anyone can type. - sudo is used to set up headless Xcode, then denied for the build user, so Claude doesn't get root. Claude doesn't run if sudo still works.
One comment per PR, updated in place on every run: the verdict, the summary, each new finding in full, and the status of earlier findings. Screenshots become links to the Buildkite build, since GitHub can't show its artifacts.
When a Spacefast key and Space are configured, the script publishes the review's screenshots additively under wordpress-ios/pr-<n>/build-<n>/, checks each URL loads, and embeds them in the PR comments. Otherwise, or if anything fails, it links to the Buildkite build as before. Claude never sees the key.
As the build user, Claude could read the job's secrets: the AWS keys and bot SSH key mounted into every Mac VM, files in the build user's home, and the Buildkite job token in the environment of the build user's processes. The script now creates a non-admin claude-reviewer user with its own group and a copy of the checkout, and runs Claude as that user over SSH to the VM (or with launchctl asuser). Before Claude starts, the reviewer checks that it can't read those files or process environments, signal the build user's processes, or use sudo. If any check fails, Claude doesn't run. It also reports whether simulators and Xcode MCP work for the second user.
The host mounts its AWS keys and the bot's SSH key into every Mac VM, and the mount is readable by every user, so the separate reviewer user could read them. Nothing in this step needs the mount after setup, so unmount it before the reviewer starts. The reviewer's checks still stop Claude if it's there.
The reviewer controls its evidence folder, so a symlink there made root's ditto, and then the artifact upload, copy files the reviewer can't read. The reviewer now archives what it can read, and the build user unpacks it into a fresh folder and deletes anything that isn't a plain file or folder. The step also tells the artifact upload not to follow symlinks. Also enables headless Xcode MCP for the reviewer user: the setting is per user, and mcp-server takes the user from the SUDO_* variables.
mcp-server takes the user from the SUDO_* variables but the settings folder from $HOME, so it tried to write into the build user's home and failed.
Setting HOME and the SUDO_* variables wasn't enough: mcp-server still wrote into the build user's settings. The reviewer now runs the two mcp-server commands itself, through a sudo rule that allows only those commands and is deleted again before Claude starts.
mcp-server enable stores an integrity hash in the user's keychain. The new reviewer user had none, so it failed with -25308 (user interaction is not allowed).
The permission check passed, but the bridge couldn't launch Xcode Service from the reviewer's SSH-only session, so Claude was told to use tools that never loaded. The reviewer script now sends an MCP initialize request and only offers the tools if it gets a reply.
- Drops Xcode MCP: it needs a desktop session, which the separate reviewer user doesn't have. Claude taps around with AXe instead (pinned, checksum verified, installed into the reviewer's home), not WebDriverAgent. - Adds .buildkite/claude/wp-test-site.sh, based on wordpress-rs's native test server (Automattic/wordpress-rs#1665): Homebrew PHP 8.4, MariaDB 11.4 and Apache, one fresh WordPress 7.1.4 site per call on http://localhost:<port>, with an admin user and an application password. The build user installs the Homebrew packages; the reviewer creates and runs the sites. - The reviewer checks AXe and creates one site before Claude starts, and reports both in isolation-checks.txt. - The reviewer no longer gets the shared test site's credentials.
- wp-test-site.sh: a start that fails partway now removes the site's folder, database and database users, so the name can be used again. - The reviewer's AXe check keeps AXe's errors in axe-check.log, retries while the simulator finishes starting, and no longer stops the review if the download fails. - The CI notes give Claude its time budget and the Jetpack build time, so it waits for a build before finishing. - The build failure analysis ignores the review step, which explains its own failures.
Adds .buildkite/claude/validate-setup.sh, which the reviewer runs before Claude when CLAUDE_REVIEW_VALIDATE_SETUP is true. As claude-reviewer, it builds the Jetpack app, installs and launches it with a fresh test site's credentials, taps through the sign-in screen with AXe (with screenshots), confirms the sign-in from the site's access log, and builds and runs one unit test from the WordPress test plan. Each result is a line in isolation-checks.txt. The flag is on for this PR only, and must be removed before merging.
Mac CI VMs only get the job's BUILDKITE_* variables (hostmgr copies only those), so the step's CLAUDE_REVIEW_VALIDATE_SETUP setting never reached the script.
… env file - Spacefast: the Space, team and URL are now set in the script, so only SPACEFAST_TOKEN comes from the pipeline's secrets. sf installs into ~/.local/bin. Screenshots one folder down (the setup validation's) are published too. - Adds a temporary step that finds why the S3 secrets hook logs "failed to parse env file" for this pipeline. While that parse fails, the hook doesn't redact the file's values from build logs. It streams the file from S3 into a checker using the hook's own parser (godotenv v1.5.1), which prints only line numbers, variable names and line shapes, never a value.
The Claude review proved that the checker could print text from lines that are part of a multi-line secret (base64 key bodies, a password on its own line), mistaking them for names or words. It now tracks multi-line quoted values and heredocs, names a variable only on a plain NAME= line whose name is upper case with an underscore, and otherwise prints only a line's length, the kinds of characters in it, and shell words from a fixed list. The real run in build 35521 printed only two variable names and line lengths.
It passed end to end in builds 35517 and 35521. The flag stays available for re-checking the setup after Xcode or VM image updates.
The upload script now writes multiline secrets in a form the S3 secrets hook can parse, and wordpress-ios/env parses (build 35533), so its values are redacted from build logs again.
In the control test (build 35535) Claude started the Jetpack build in the background, because a Bash command times out after 10 minutes by default, and returned its verdict before the build finished, so it never used the simulator. Single commands may now run for an hour, background tasks are off, and the CI notes say to run builds in the foreground and finish every check before returning a result. The log shows each result segment's cost, and the script takes the last structured output.
Each run posted a review that said "verdict: fail" or "pass", and those piled up, so a fixed PR still looked failing. A review can't be deleted once submitted, so new reviews no longer state a verdict, and each run rewrites the first line of the earlier runs' reviews to say they were superseded. Findings and their threads are kept. The summary comment, updated on every run, is the one place with the current verdict.
A GitHub Action on review_requested retries the Claude PR Review step on the PR's latest Buildkite build, using the same reusable workflow that retries Danger when labels change. The step allows retrying after a pass, and the review script removes the request when it finishes, so it can be requested again.
The reviewer user's activity is logged from places it can't change: - Its transcript streams back to the build user instead of a file it owns. - Managed-settings hooks send every tool call to a listener run by the build user. A tool call that can't be logged is blocked. - eslogger (as root) records the reviewer's process launches, file opens, changes and deletes. - A pf rule logs the reviewer's outgoing connections, with DNS traffic to name the addresses. A known command run as the reviewer checks that each log records it. The prompt, PR data and versions are uploaded, installer output is kept, and Claude Code and the Spacefast CLI are pinned to checksummed binaries.
The test site's database and web server inherited the file descriptor the reviewer script writes Claude's transcript to. They kept the SSH session open after Claude finished, so the job hung until it timed out. Only Claude's process now gets it.
The transcript only reached the log at the end, so a stuck or slow run looked the same from outside. Each tool call and the final result are now printed while the transcript is saved.
Streaming it back over SSH left the session open after Claude finished (build 35633), so the job hung until it timed out. Claude now appends to a root-owned file with the system append-only flag, which the reviewer can add to but not change or delete. The reviewer's checkout no longer includes the audit folder.
Build 35640 hung while collecting the audit logs. Each logger now gets a SIGINT and, after 10 seconds, a SIGKILL, and the wait for the Endpoint Security log is capped. The log shows how long stopping and summarizing took and how big each file is.
eslogger's exec events include each process's whole environment, so the Endpoint Security log carried the reviewer's ANTHROPIC_API_KEY and was uploaded as an artifact (found by the Claude review in build 35643). The environment is now removed before the log is written, and any audit or evidence file that still contains one of the job's secrets is deleted before upload. The audit folder is no longer readable by the reviewer. Unchanged files' close events are dropped, halving the log.
A cancelled or timed-out job lost every audit log, because the VM shuts down before artifacts are uploaded. The logs are now uploaded in parts every two minutes to build/claude-audit-live/, each checked for the job's secrets first. The Endpoint Security log was 266 MB after six minutes, mostly reads of system files, Xcode and the reviewer's own folders. Those are now counted instead of logged; reads anywhere else are still listed. eslogger ignores SIGINT, so it now gets SIGTERM next, and the signal that stopped each logger is recorded.
Lists of paths compress well, and files-read.txt alone was 120 MB. Summaries over 1 MB are gzipped after the checks; the verdict is read from the original transcript.
… log Claude was started with env -i ... ANTHROPIC_API_KEY=... claude, so the key was in a command's arguments, visible to ps and recorded by the Endpoint Security log (found by the Claude review in build 35644). Claude now gets an exported, filtered environment instead. The secret check before upload missed matches in large compressed parts: grep -q exits at the first match, gzip gets SIGPIPE, and under pipefail the pipeline failed, which read as no secret. The ES filter also now redacts the job's secrets anywhere in an event.
The test sites, the simulator and anything Claude started outlive the reviewer's session, and the reviewer can read other users' command lines. Spacefast's token was then passed as an argument (raised by the Claude review in build 35645). The reviewer's launchd domains are now taken down and its processes killed before posting, and the run stops if any survive. sf reads the token from SPACEFAST_TOKEN instead of --token.
build/audit-log.tar.gz holds every audit result and the log parts uploaded during the run, so an audit needs one download. It's built last, from the EXIT trap, after the posting steps add their logs, and checked for the job's secrets first. The parts are still uploaded during the run, for jobs that are cancelled or time out.
The request was only removed after posting, so a run that exited early left wpmobilebot as a pending reviewer, and asking again couldn't start a new run. It's now cleared from the EXIT trap, and a failure to clear it is logged.
Its first Xcode build otherwise resolves them again, including 6.5 GB of binaries, which took 187 seconds locally. They're now resolved into DerivedData/SourcePackages before the checkout is copied. A copied folder needed no fetching in a build from a different path. The prompt's test command now uses the same DerivedData.
This reverts commit b4dec67.
…kers-pr-prototype
trunk now caches the packages' binaries, so restore them instead of resolving everything from the network (suggested by the Claude review in build 35673). Resolving stays, to fill in anything the cache is missing.
A cancelled job never reaches its EXIT trap, so the request stayed pending (build 35678). It's now also cleared at the start. The request has done its job by then, and the EXIT trap still clears one made during the run. (cherry picked from commit ec66514)
jkmassel
requested review from
wpmobilebot
and removed request for
wpmobilebot
October 30, 2026 19:49
wpmobilebot
removed their request for review
October 30, 2026 19:50
Contributor
Author
|
Both tests are done: see the test plan in #26272. |
19 tasks done
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
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.
Throwaway PR to test two paths of #26272 on real CI: the review request being cleared after an early exit, and a cancelled job. It runs only the Claude review step, and will be closed without merging.