Repository navigation
🤖 fix: harden verify-xum launch, cleanup and recipes (follow-up to #7895) - #7914
Merged
Merged
Conversation
A project-local verification skill: verify-xum.sh launches one mock-AI make dev-server-sandbox per checkout (env -i, setsid process group, seeded project and workspace), checks it with doctor, waits with idle and send-ready, captures evidence that survives cleanup, and a feature map covers workspaces, chat, review, command palette and settings.
Signed-off-by: Thomas Kosiewski <tk@coder.com>
… bounded probes, group identity - The newest-run link lives in the checkout's git dir next to the lock, so two checkouts that share VERIFY_XUM_HOME never load each other's run. - launch takes the checkout lock under flock, so two launches at once cannot both start. - Every curl has --max-time, and the readiness loop uses a wall-clock deadline. - doctor deletes the previous version.json before it asks /version again. - cleanup signals the group only while a process in it has VERIFY_XUM_SERVER_OF=<run id>. - SKILL.md says a canned todo_write still writes its TODO list.
…ound every launch phase - settings.md and command-palette.md wait for the option to be selected before Enter. The first palette open after a page load shows a loading input that drops Enter (LazyCommandPalette.tsx:37/60), so an Enter typed right after the text was lost. - settings.md waits until no modal dialog is open after Escape. The palette shortcuts do nothing while one is (isDialogOpen(), keybinds.ts:291, App.tsx:1133). - README and SKILL.md: run a step's commands as one && chain, so a failed wait stops it instead of typing into the composer. - env also puts the helper on PATH, so the recipes' bare verify-xum.sh resolves. - launch checks its deadline during port discovery, the readiness loop, every seed API call and the workspace-setup wait.
…unnable review recipe - load_run refuses a run that another checkout launched, so cleanup and doctor never act on it. - cleanup signals a group only while its live leader is still the launch's make dev-server-sandbox, and blanks the group id in state.env after the stop. - doctor reads /version without writing a file. SKILL.md says every check line reads ok. - review.md Restore unmarks the hunk, unchecks Uncommitted and closes the Review tab, so the recipe runs again in the same run. - settings.md drives Ctrl+, too. chat.md and workspaces.md create the evidence folder first. - SKILL.md: Linux only, the xum run snippet runs under env -i with a fake key (it refuses without one, and the old snippet used the caller's real keys), two more proof standards. - workspaces.md records that document.hasFocus() was true on agent-browser 0.33.2.
…absolute The live Xum's server URL and token are also in the env as legacy MUX_* names, which the CLI reads, so env -u of the XUM_* names alone did not keep them out. A relative VERIFY_XUM_HOME made the run link in the git dir point at a path relative to the git dir.
… exited, end recipes on the workspace - workspaces.md runs idle after each create's reply. The reply text shows before the stream ends, and archiving a streaming workspace opens a confirm dialog instead. - cleanup treats a zombie group leader (empty cmdline) like an exited one, so it still stops the group's live members. - review.md says to repeat from Open the sidebar after a cold-cache page reload. - settings.md Restore re-opens the workspace: Escape from a Settings URL lands on /project.
…ive TMPDIR absolute - README: start every recipe whose preconditions name Verify playground by opening it. The workspaces and command-palette recipes end on release-notes, and the recipes read their evidence from $VERIFY_XUM_WORKSPACE_ID. - make -C runs the sandbox from the checkout, so a relative TMPDIR resolved there while the script resolved the logged XUM_ROOT against the caller's directory.
… saved chat reply - launch keeps the flock fd open, and the server's make and dev-server-sandbox.ts inherit it, so the lock stays held while the server runs at whatever point the launcher dies. A second launch is refused at once. launch saves state, the run link and its cleanup trap before the spawn, and cleanup reads the group id and XUM_ROOT from the pidfile and server log when a killed launch never saved them. - chat.md waits until chat.jsonl has the second reply before it reads the history. The transcript shows the reply before the server saves it at stream end.
…rited lock - launch refuses while the process group recorded for this checkout's newest run is alive, checked with the same leader-cmdline and run-id test that cleanup uses. The group's leader appends its id to the run's state.env before it starts make, so the guard and cleanup find it at whatever point the launcher dies. The flock now only serializes the check and the spawn, and the server does not inherit it. - Removed the inherited lock fd, the separate pidfile and lock file, and cleanup's pidfile and server-log fallbacks. state.env is written atomically. - doctor checks that the checkout's newest-run link names the run. - review.md: after a cold-cache reload the sidebar and Review tab stay open, so the recovery continues at Include uncommitted edits. - command-palette.md: #7897 is on main, so the loading input keeps an early Enter. The wait for the selected option stays.
…deletes its root when the group stops
…ine, wait for the settings write - launch empties VERIFY_XUM_PGID, ROOT, BACKEND, URL, WORKSPACE_ID and WORKTREE before its first state write, and reads only the last recorded group id. A shell that had evaluated `env` for an earlier run exported its PGID, so the new state held two ids and the launch failed with an orphan group. - Each readiness probe checks the deadline first and uses the time left as curl's limit. - settings.md waits until config.json holds the new value before it captures or reloads, and again in Restore. - SKILL.md states the one leftover-folder limit, says that a panel stays open across a cold-cache reload, and command-palette.md says that the first open shows a textbox until the palette's code loads.
…ing on SIGPIPE Under pipefail, `curl | grep -q` fails when grep exits at the match while curl still writes: doctor printed "FAIL ... serves the Vite dev UI" on 2 of 40 runs against a healthy server, and 0 of 40 after this change. Both checks now match the captured output instead.
…and config guards
- review.md Restore waits until config.json holds "includeUncommitted": false.
- The demo seed commit runs with core.hooksPath=/dev/null, so the developer's git hooks stay out.
- workspaces.md waits for the archive and restore results before it captures.
- Without XUM_SCRATCH_DIR, VERIFY_XUM_HOME defaults to ${TMPDIR:-/tmp}/verify-xum-<uid>.
- The README expand step clicks only when the project is collapsed, so it is safe under &&.
- Launch refuses a sandbox that copied a config.json (it copies none with both --clean flags),
so every run starts from default preferences.
- When the backend records an auth token in server.lock (#7904), api() and doctor send it to
curl on stdin, and `env` exports VERIFY_XUM_OPEN_QUERY (?token=...) for the first browser open
without printing the token. Without a token nothing changes.
- Launch stops at once when the backend crashes after its rebuild, clamps curl's --max-time to at
least 1 s, and the docs no longer claim the guard holds in the instant before the group id is
written. The Cleanup run-directory sentence sits above the Limit paragraph again.
- The run home never defaults to a shared TMPDIR: $XUM_SCRATCH_DIR/verify-xum, else
$XDG_RUNTIME_DIR/verify-xum, else $HOME/.cache/verify-xum. Launch creates it and each run folder
with mode 0700.
- Before any command sources a run's state.env, it checks that the home and the run folder are
real folders (only the newest-run link is followed), owned by this user and not writable by
group or others, and refuses otherwise. A folder another account created first could otherwise
swap state.env and run commands as you.
- open-query passes the token to node on stdin, not in argv.
- review.md runs idle before Mark as read, the README expand snippet is one { ...; } group, and
SKILL.md says VERIFY_XUM_OPEN_QUERY stays in the shell's environment.
…navigations and writes - verify-xum.sh open <path> replaces `agent-browser open "...$VERIFY_XUM_OPEN_QUERY"`. node builds the URL from server.lock and pipes [["open", url]] to `agent-browser batch` on stdin, so the token is in no process's command line or environment. VERIFY_XUM_OPEN_QUERY and open-query are gone. A failed open prints agent-browser's output with the token redacted. - command-palette.md waits for the destination URL after the palette switch, and workspaces.md after each sidebar row click. - workspaces.md waits for the keyboard archive to remove the row, and review.md Restore waits until review-state.json has no "isRead": true. - Launch records its start in milliseconds, so a build stamp from earlier in the same second is stale.
A path like //host, /\host or /<tab>/host resolves to another origin, and open would have sent the auth token there. Also rewrap the features README line that the open change made too long. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
The restore click came right after Ctrl+Shift+B opened the sidebar again. A click during that slide is lost, and one no-token proof run kept the tab at Review 1/1. Run idle first, as Mark as read does. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
- The launch deadline starts before the guard, and flock waits only for the time left, so a short VERIFY_XUM_READY_TIMEOUT bounds that wait. - The spawned group holds the launch flock until it has written its PGID, and carries VERIFY_XUM_SERVER_OF from its start. A launcher killed before that record no longer lets a second launch start a second server in the same checkout. - Parents of a new run home are created with umask 077. - New helper: verify-xum.sh workspace-field NAME FIELD [RUN] prints a field of a workspace's config.json entry. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
- The workspace and command palette waits used a literal <release-notes id>. They read the id with verify-xum.sh workspace-field release-notes id. - The archive steps wait until config.json has the archivedAt time. - The palette's first open waits until Suggestions lists an option. - Mark as read waits for the hunk's button, which shows late after a reload. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
… the demo repo An exported GIT_INDEX_FILE, GIT_DIR or GIT_CONFIG_* made the demo repo's git commands act on the caller's repository: git add . replaced its index. The script now unsets the variables of git rev-parse --local-env-vars plus the discovery and config ones, and the env output unsets them too, so the recipes' git commands act on the demo repo. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
A UI change must also be checked at 375 px: set the viewport, repeat the changed recipe's captures, check that nothing runs past the right edge, then restore the viewport. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
When the app fails before it strips ?token= (a renderer error, a chunk that does not load), the browser stays on a URL with the token, and capture saved that URL into the evidence, which is meant to be shared. capture now passes the URL and the snapshot through redact_token, which replaces the token (raw and URL-encoded) and any token= value with <redacted>. Cleanup does the same in every text file of the evidence and in the server log, before it deletes server.lock. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
The pathname check and the open were separate steps, so a FIFO swapped in between made the read-only open block before the timed flock. A node helper now opens the guard with O_NOFOLLOW and O_NONBLOCK (O_EXCL and mode 0600 when it is absent), checks the open file with fstat and sets its mode to 0600. The shell opens the same file through /proc/<helper pid>/fd/<fd>, so it never opens the pathname itself. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
…ing the server Stopping the group makes the backend delete server.lock within about a second, so cleanup's redaction loaded an empty token and left a bare token in the evidence. Cleanup now reads the token first, and the redactor gets it on fd 3. The token= pattern also matches only a non-empty value, so an empty ?token= and the Makefile banner's ?token=<token> stay as they are. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
…IFO race" This reverts commit 663215c.
The launch guard and the newest-run link live in the checkout's git dir. Point fixes for a peer-writable git dir (a symlink guard, a FIFO guard, a folder at the run link) kept coming, so every command now refuses a git dir, or a worktree's common git dir, that is not a real folder of this user or that group or others can write, the rule the run home's folders follow. The checks on the guard's open fd stay. The node FIFO helper is reverted: no other account can put a FIFO there. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
…eaches the browser - agent-browser 0.33 serves each session's screen and input as a WebSocket on 127.0.0.1 that any local account can connect to, and a new daemon starts it on its own. open now runs stream disable every time, refuses to open while stream status reports it on, and closes the session if it comes on during the open. stream disable also drops clients that connected before. - SKILL.md says to run the skill only where you are the only local account: Chrome's DevTools port on 127.0.0.1 asks for no credentials, and agent-browser cannot turn it off. - Launch keeps a copy of the token in the private run folder (0600), so capture and cleanup can redact while server.lock is gone (a backend restart). Cleanup deletes the copy last. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
# Conflicts: # .xum/skills/verify-xum/scripts/verify-xum.sh
open read the token from server.lock only, so an open during a backend restart, while server.lock is gone, reached the app without it and ended at the token prompt. It now takes the token from run_token (server.lock, else the run's 0600 copy) and hands it to node on fd 3. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
The recipe pressed Ctrl+Shift+B twice and checked only the end state, so a shortcut that did nothing passed. It now waits for Expand right sidebar after the first press and Collapse right sidebar after the second. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
A run home inside the checkout publishes the token cache and evidence through Vite. File modes do not help because Vite runs as the same user. Launch now refuses that home before creating any folder. Every command that loads a run checks it too, including a run an older version left there. The check resolves symlinks and .. physically and covers both the current checkout and the main worktree of a linked checkout. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh` • Cost: `$153.98`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=xhigh costs=153.98 -->
Vite allows its detected workspace root (server.fs.allow defaults to searchForWorkspaceRoot(root)), and that root can be a parent of the checkout, for example a folder whose package.json declares workspaces. A run home under that parent passed the checkout check, and Vite served <run>/token to an unauthenticated request. Every home check now asks the checkout's own vite for that root (searchForWorkspaceRoot of the physical checkout, as vite started there computes it) and refuses a home inside it, in addition to the checkout and the main worktree. When vite cannot answer, the check refuses. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh` • Cost: `$166.57`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=xhigh costs=166.57 -->
# Conflicts: # .xum/skills/verify-xum/scripts/verify-xum.sh
…orded The new group's leader appended its id to state.env and then closed the launch flock and ran make even when that append failed (read-only file, full disk). The server then ran with no recorded group: launch_failed could not stop it, cleanup dropped the newest-run link, and the next launch could start a second server in the same checkout. The leader now exits when the append fails, before it closes the flock or starts make. The command-palette recipe now waits until the palette is gone after Escape, so the next shortcut cannot close a palette that is still open. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh` • Cost: `$169.60`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=xhigh costs=169.60 -->
CI's shellcheck (0.8.0) reports SC2015 on `A && B || die`. The check is now an if/else with the same result: vite_workspace_root prints the root only when the checkout's vite reports one absolute path, and refuses otherwise. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh` • Cost: `$171.54`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=xhigh costs=171.54 -->
# Conflicts: # .xum/skills/verify-xum/SKILL.md # .xum/skills/verify-xum/features/command-palette.md # .xum/skills/verify-xum/features/review.md # .xum/skills/verify-xum/features/workspaces.md # .xum/skills/verify-xum/scripts/verify-xum.sh
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff022b0b0b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…rs above the git dirs A folder at .git/verify-xum.current made ln -sfn put the new link inside it, so the next launch did not see the live run and started a second server. Launch now refuses anything but a symlink there before it makes anything, and the link is written with ln -T. The git-dir rule checked only the git dirs themselves. A group-writable folder above a private .git let another account rename the whole git dir away. Every command now also checks the folders above both git dirs with the run home's rule (owned by root or you, not writable by group or others unless sticky). The phone-width proof standard now allows the phone layout's own path to a control, or a capture that it is absent. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh` • Cost: `$173.69`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=xhigh costs=173.69 -->
Member
Author
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
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.
Summary
This PR follows up #7895 (the verify-xum skill). It ships every fix that a Codex review thread on #7895 deferred to "the follow-up PR in this run", plus two nits from the root verifier. Only files under
.xum/skills/verify-xum/change.Codex threads fixed
All 11 threads are on #7895 and resolved there with a planned-fix reply.
Script (
scripts/verify-xum.sh):verify-xum.currentruns while the process holds the launch guard's flock. Onlypoint_current_run_link_atanddrop_current_run_link_ifchange the link, in launch, cleanup and the failure cleanup.demo_gitseeds the demo repo withgit init --template=,core.attributesFile=/dev/null,core.hooksPath=/dev/null,core.fsmonitor=falseandGIT_ATTR_NOSYSTEM=1. Each git command runs undertimeoutwith the time left before the launch deadline.require_private_git_dirs). The checks on the guard's open fd stay as a second line. The node helper that the FIFO thread's reply planned is not needed, because no other account can put a FIFO there. This branch reverts its commit.opentakes the token fromrun_token(server.lock, else the run's 0600 copy) and passes it to node on fd 3.printf ... >>state.env || exit 1before it closes the flock or startsmake.Recipes and SKILL.md:
verify-xum.sh workspace-field release-notes idin the same bash call as theirwait --url.review.mdwaits forExpand right sidebarafter the first Ctrl+Shift+B and forCollapse right sidebarafter the second.command-palette.mdwaits until the palette element is gone before the next shortcut.Verifier nits
Fixes from this PR's review
verify-xum.current, under the guard and before it makes anything. It writes the link withln -sfnT. Before this fix, a folder there received the new link, and a second launch started a second server.Validation
features/*.mdrecipe exactly as written against a realmake dev-server-sandbox, and the result was 184 PASS and 0 FAIL at b48bdde, on top of current main (with 🤖 fix: reach the dev-server backend over a private Unix socket so a port squat cannot catch the token #7909's Unix-socket backend).state.env). Before:makeran anyway. After launch failed, 18 server processes were still running with no recorded group, and the newest-run link was gone. After:makenever runs and no process is left. The guard stays held until launch fails.git addfailed (clean filter 'boom' failed), and the template's attributes and fsmonitor applied. After: the seed commits, and neither applies.openwithserver.lockabsent (a restarting backend). Before: the page showed the auth prompt. After: the app loads. In neither case does the URL keeptoken=..gitunder a 0775 folder, and a worktree whose git dirs sit there. Before: both pass. After: both are refused. A 1777 (sticky) folder and a 0755 folder pass.make static-check. I also ran shellcheck 0.8.0 (CI's apt version) over all 46 shell scripts. Local 0.11.0 does not report SC2015, which failed CI on 🤖 feat: add verify-xum skill that drives the real Xum web UI #7895.Risks
The risk is low. Only the agent-facing verify-xum skill changes, not product code. The git-dir rule refuses a shared repository whose git dir is group-writable, and SKILL.md says to use a checkout of your own. When the PGID write fails, launch now fails at the launch deadline (default 300 s) instead of leaving a server running that cleanup cannot stop.
Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:xhigh• Cost:$173.69