Repository navigation
chore(skills): add a tools/call wire probe to the verify-claudecode-nvim skill - #517
Conversation
…vim skill The probe that proved #500 lived only in a scratch folder. It now runs with one command: it starts the plugin server of one tree in a headless Neovim with an empty environment, sends malformed and valid tools/call messages over a real WebSocket, and judges each answer against one case table. It ends with a COUNT line and exits 1 when a case fails or an answer leaks a path. _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
…cleanup on a signal The run dir came from the caller's TMPDIR, which made the dir location an input of the probe. It is now /tmp/wire-probe.XXXXXX. A second signal during the cleanup ended it before the dir was removed, so the cleanup now ignores TERM, INT and HUP while it runs. _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
|
@codex review |
|
@codex security review |
Codex Review Summary: ✅ CompletedThis 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. |
🛡️ Codex Security ReviewSecurity 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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 44bd194528
ℹ️ 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".
…ipt and refuse an empty run The leak check read only the text item of a result, so a path in a sibling field passed. It now reads every frame the server sends, after the three path rewrites, and a frame that holds the auth token is a leak whose text is not printed. After SIGKILL of the script, nvim stayed up for 3 minutes with its token and lock file. nvim now checks its parent every half second and quits, and its server stop closes the socket, which ends the checker. A run with no case, or with fewer judged cases than rows, exits 2 instead of 0. _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
…nt and check that nvim starts The probe stopped with a bare "nvim exited" message when nvim on PATH was a version-manager shim, which does not start with an empty environment. It now checks that nvim and node start that way, names the binary when one does not, and prints the last lines of nvim's output when the server does not come up. The checker starts with an empty environment, as nvim does, so no variable of the caller loads code into the process that holds the token and the real home folder comes from the password database. The leak rule also reads object keys. nvim gets a TMPDIR inside the run dir, so it leaves nothing in /tmp, and the count line prints the number of cases that the checker judged. _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
|
@codex review |
|
@codex security review |
|
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 ReviewSecurity 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. |
… and keep the token out of messages Two keys that the path rewrite made equal let normalize drop one value before the leak rule ran, so a leaking frame passed. The leak rule now reads every key and every string of the frame as it came off the wire, one by one. The rebuilt copy that the table is compared with no longer rewrites keys. Node's JSON parser message quotes the start of its input, so a frame that was not JSON could put 8 characters of the token on stderr, and a lock file that did not parse could do the same. The checker now ends such a run with exit 2 and a fixed message. Text from a frame reaches the output only when it holds no run of 8 or more characters of the token. _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
…-child nvim and count a leak once The leak rule and the token rule now read the text of every frame as the wire sent it. One step comes first: an escaped slash (backslashes and a slash, at any depth) is read as a slash. Then the path rewrite and the two rules. No parse and no rebuild comes before them, so a duplicate key and a JSON fragment inside a string cannot hide a path, and the per-string read is gone. nvim checks its parent before the plugin starts and quits with an error when the parent is not the launcher, so a wrapper without exec ends the same way every time instead of in the middle of a run. A leak of the last case is no longer counted twice. A failing mktemp or mkdir exits 2 instead of 1. _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
|
@codex review |
|
@codex security review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? 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 ReviewSecurity 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. |
… checker's own messages, name the dir on a failed cd A tree path that holds a double quote is written as \" in a frame, so the leak rule did not find it there and a leaking frame passed. The checker now exits 2, before it sends a case, when the tree path, the work folder path or the home folder path holds a double quote, a backslash or a control character. An exception that Node raised could put the token on stderr: the WebSocket constructor quotes a header value that holds a CR, LF or NUL. The checker now prints only text that it wrote itself. An exception that it did not raise ends the run with exit 2 and a line that holds only the exception's class name. A failing cd into the new temp dir named an empty path and left the dir. The message now names the dir that mktemp printed, and the cleanup removes it. _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
…and word a lost connection by its close A HUP, TERM or INT that lands while the checker has just exited could hang the launcher in its cleanup: `wait <pid>` blocked on a child that was already gone, and nvim stayed alive with its lock file until the launcher was killed. About 2 of 112 signals sent right after the COUNT line hung. The cleanup now stops each child by PID, polls for its end and sends SIGKILL, with no `wait`. The checker kept its handshake `onerror` after the connection opened, so a server that died mid-run could print "WebSocket handshake failed". It now drops that handler on open, and the close handler names the lost connection. The page and the launcher header say that a backslash in the tree path can make nvim fail first, so the exit 2 then comes with the launcher's own message.
|
@codex review |
|
@codex security review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! 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 ReviewSecurity 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. |
…tate its promises on the page (coder#555) ## Why The operator asked for a code-shape refactor of the plugin, landed on main in small PRs, fixes first. His rule for the verification skill: a scratch harness that catches a blocker goes into the repo's skill. coder#517 put the `tools/call` wire probe there. Its last check left a few small points open, and this PR closes them. It changes two of the probe's three files (the checker `proof-tools-call-wire.mjs` is unchanged) and adds one bullet to `AGENTS.md`: 31 changed lines in all (18 added, 13 deleted). What a user of the probe saw before: - A TERM, INT or HUP that landed after the exit handler started and before `trap '' TERM INT HUP` ran ended the cleanup early. nvim and its lock file then outlived the script until nvim's parent check (at most 5 seconds, while nvim is alive and its main loop runs), and the temp dir stayed. - When nvim died in the middle of a run, bash printed a `Killed` line with the script's own source text after the probe's message. When the `cd` into the new temp dir failed, bash printed its own `cd:` line before the probe's message. - The page said a version-manager shim does not start with an empty environment. Inside a sandbox a mise shim did start (the input lane of the first verify round). The page also did not state every promise of the probe: what the leak check is for, what it leaves outside, who can read the token, and the 5-second promise for an end of the script while nvim is still alive and its main loop runs. ## What changed The promise, in one mechanism: when the script ends and nvim is still alive, nvim and the checker end and the lock file is gone within 5 seconds, as long as nvim's main loop runs, because nvim checks its parent every half second and stops its server when the parent is gone. The trap cleanup is the faster path and removes the temp dir. This PR adds no second cleanup mechanism: no flag file, no lock, no second timer, no cleanup that runs twice. `scripts/proof-tools-call-wire.sh` (8 lines added, 6 deleted): - The ignore of TERM, INT and HUP is the first command of `cleanup`, and each of the three signal handlers ignores the three signals before it calls `exit`. From the first signal that the script handles, and from the first command of the cleanup, further signals are ignored. - `disown "$NVIM_PID"` after the start, so bash prints no `Killed` line when nvim dies. node stays a job, because `wait "$NODE_PID"` needs it. - `2>/dev/null` on the `cd` of the `REAL=` line, so the probe's message is the only line. - The header comment says that a version-manager shim "can fail to start" with an empty environment, as the page does (comment text only). `features/tools-call-wire.md` (9 lines added, 7 deleted): - The shim sentence says a shim "can fail to start" with an empty environment. - "The run leaves nothing outside the temp dir" now says that nvim and the checker leave nothing there. A run with a hostile caller environment showed that `mise which node`, which the script runs, can write files under the caller's home, such as `.cache` and `.local`. With mise off `PATH` nothing is written there. - The SIGKILL bullet became the 5-second promise for an end of the script while nvim is still alive, with its premise (nvim's main loop runs), the SIGKILL to the whole process group (nvim dies too, so the lock file of a dead server stays), and the ignored signals. Two other bullets now say that a signal can cut the cleanup short (the temp dir stays then, without a token) and where the home path of the leak check comes from. - New bullets: what the leak check is for (a path that the plugin puts into a text frame by accident, not a server that hides one on purpose; the plugin writes every text frame with `vim.json.encode`, and the check does not read Ping, Pong and Close frames) and that another spelling of a path is outside it; who can read the token, the `0700` temp dir from `mktemp -d`, that the probe trusts the code that the caller picks to run (the tree, the nvim binary, the node binary and the helper commands that the script finds on the caller's `PATH`, all of which run as the caller, so a copy of the token by one of them is outside the promise), that the folders that mise and `PATH` search, `HOME` included, are part of what the caller picks, and that the caller's `TMPDIR`, `HOME`, umask and `CLAUDE_CONFIG_DIR` do not move the lock file or change its mode. The failed-start sentence now says that the last five lines come from the tree under test. - The opening now says what the probe checks (every text frame that the server sends) in place of a promise about every frame of the server. The environment bullet says that no variable of the caller reaches the checker, in place of "loads code into". The PR also writes the probe's cleanup rule into `AGENTS.md` (1 line added, at the end of `## Security & Configuration Tips`), so that a later author does not add a second mechanism: nvim is a direct child of the script, checks its parent every half second and stops its server when the parent is gone, so the lock file is gone within 5 seconds after the script ends, as long as nvim is still alive and its main loop runs. The script's own cleanup is only the faster path, and the page states the limits. One window stays: a signal that lands after the exit handler started and before the first command of `cleanup` runs a signal handler inside the exit handler, and bash then leaves without the cleanup body. The 5-second promise (nvim is still alive and its main loop runs) covers it (see the proofs). Not in this PR: new rows of the case table, the valid-call and notification checks, and the index lines in `SKILL.md` and `features/README.md`. ## How I proved it Seven commits. `9443ea70` holds the code and the first page text. `cd0b4816` changes four lines of the page, `64cbbfdd` changes one comment line of the launcher (a diff that shows only lines that start with `#`), `03fc877d` adds the `AGENTS.md` bullet, `d7ab7bae` narrows two promises on the page after the Codex review (four lines), `9ed43d66` names the helper commands among the code that the probe trusts (one line), and `6d01befb` changes three more page lines after the check of those two commits: it narrows three sentences, says which binary each of `VCN_NVIM`, mise and `PATH` can pick, and adds one sentence about the folders that mise and `PATH` search. All runs are on Neovim 0.12.3 and the v0.8.0 release download. The signal trials and the checks of coder#517's proofs ran on `9443ea70`. The demo, the two bash lines, the spelling mutants, the token scan, the process-group kill and the SIGKILL runs ran again on the launcher of `64cbbfdd`. Its code lines are the same as in `9443ea70`. Red proof for the cleanup order: the round-4 demo of coder#517's last check, a `sleep 0.4` after the `local pid` line of `cleanup`, then one TERM 0.15 s after the `COUNT` line. - main's launcher with the sleep: exit 143, nvim alive at the launcher's exit, the lock file there and the temp dir left, on both versions. nvim and the lock were gone 1.5 s later. - this launcher with the same sleep at the same place: no nvim process, no lock file and no temp dir at exit, on both versions. Six runs (on each version a TERM and an INT 0.15 s after the `COUNT` line, and two HUP, 0 and 0.2 s after it): all six exited 0 (the signal came while the cleanup ran and was ignored). - The window that stays, with `trap 'sleep 0.4; cleanup' EXIT` in a copy and a TERM 0.15 s after the `COUNT` line: exit 143, nvim alive at exit, nvim and the lock gone within 1.5 s, the temp dir left, on both versions. Signal trials (a small python wrapper sets HUP, INT and QUIT back to their default action before it starts the script, and `SigIgn` in `/proc/<pid>/status` is clear in every trial): 450 trials, which is HUP, INT and TERM, 25 each, at the start of a run (when nvim has started), right after the `COUNT` line and just before it (while the checker reads for 200 ms after the last case), on each version. In my run the exit code was 129, 130 or 143 in all 450. Nothing was left at the script's exit (no nvim, no lock file, no temp dir), and no trial hung. The second verifier of the first verify round reran 450 trials and had 448 such exits. Its two other trials exited 0 and left no process and no lock file. The other two points, same runs on main and on this launcher: - nvim killed in the middle of a run (the checker holds 25 s after it connects): main's launcher prints `the server closed the connection` and a `Killed` line (2 lines), this launcher prints only the message (1 line), exit 2, on both versions. - `cd` into the new temp dir fails (a stand-in `mktemp` that prints a dir that does not exist, or one with mode 000): main prints a `cd:` line and the message (2 lines), this launcher prints the message only (1 line), and leaves no temp dir. The runs behind the new page sentences (both versions unless said): - Another spelling of a leaked path: a frame that holds the other name of a symlink to the tree, `~/secret` or a relative path passes (`cases=44 failed=0 leaks=0`), and a relative `rel/x.lua:12` gives `failed=31 leaks=31`. - `vim.json.encode` over printable ASCII writes `"` and `\` escaped on both, `/` escaped on 0.8 only, no `\uXXXX`. Every text frame that the plugin sends comes from `vim.json.encode` (three call sites in `server/init.lua`, one sender in `client.lua`). The plugin builds Ping, Pong and Close frames without it: its Close reasons (seven fixed literals) and its Ping payload ("ping") are fixed, and its Pong and the answer to a Close return the peer's own bytes. - A raw server that sends a Text frame, a Ping, a Pong, a Binary frame and a Close with a code and a reason to Node's WebSocket client (the checker's client), on Node 22.23.4 and 26.12.1: `onmessage` saw the Text and the Binary frame and none of the Ping, Pong or Close text. The close event carries the code and the reason, and the checker's close handler reads neither. - A verifier's runs on Neovim 0.12.3 with a hostile caller environment (`TMPDIR`, `HOME` and `CLAUDE_CONFIG_DIR` in folders of mode 0777, umask 000): the lock file had mode 0600 in folders of mode 0700, and the token was found 0 times in those folders, in the arguments and the environment of both children, and in stdout and stderr. Another run on 0.12.3, from a folder whose trusted `mise.toml` pins the node version, with mise's default setup and a `node` planted under the shared `HOME`: `mise which node` returned the planted binary, the probe passed, and the token was found once in the shared `HOME`. - A caller with umask 000 and `TMPDIR`, `HOME` and `CLAUDE_CONFIG_DIR` set to scratch dirs: lock file mode 0600 in a `0700` dir that the user owns, 0 matches of the token in the arguments and environments of the children (46 files read on 0.12.3 and 42 on 0.8), in stdout and stderr, and in the three caller dirs. The files that these dirs hold after the run come from `mise which node`. - A SIGKILL to the whole process group: no process left, the lock file still there and holding a token string, the temp dir left. A tree that blocks nvim's main loop for 12 s, plus SIGKILL of the script: nvim was gone after 11.0 s on both versions, outside the 5 seconds. - SIGKILL of the script, 5 runs per version: exit 137, both children and the lock file gone within 1.5 s in our runs (the promise is the 5-second bound while nvim is still alive and its main loop runs). Everything of coder#517's proofs on `9443ea70`, one run each unless said (39 scripted checks, 0 bad): `cases=44 failed=0 leaks=0` three times per version with identical output (also on a tree of main), the red proof on `f932d7b6` (`failed=30 leaks=15`), the leak mutants (mA `failed=2 leaks=2`, mB `failed=1 leaks=2`, mD `failed=2 leaks=2`, mE `failed=41 leaks=41`, mJ `failed=31 leaks=31`, mK `failed=2 leaks=2`, mL `failed=1 leaks=1`, mR `failed=2 leaks=2`, mQ `failed=31 leaks=31`), the quote-path refusals (exit 2), the token mutants (0 matches of the token, any 8-character run of it, on stdout and stderr), both wrapper cases (a wrapper that runs Neovim as a child exits 2, a wrapper with `exec` passes), the shim and fake-node messages, and the guards for an empty table and a skipped row. Gates. On `9443ea70` (the code): `mise run format-check` 352 files 0 changed, `mise run check` 0 warnings / 0 errors in 176 files, `mise run test` native exit 0 with 1655 successes / 0 failures / 0 errors / 22 pending (seed 1794438783), and `bash .agents/skills/verify-diff-lifecycle/check.sh --hash-only` prints `covered.lock matches the code`. On `03fc877d` (the fourth commit): `mise run format-check` 352 files 0 changed, `mise run check` 0 warnings / 0 errors in 176 files, the same `--hash-only` line, and one probe run per version with `cases=44 failed=0 leaks=0`. `mise run test` ran on `9443ea70` only. No code line changed after it: the later six commits change page lines (four lines each in the first and the fourth, one line in the fifth, three lines in the last), one comment line of the launcher and one line of `AGENTS.md`. On `d7ab7bae` (page only): `mise run format-check` 352 files 0 changed, and the probe on the scratch merge below. On `9ed43d66` (one page line) and on `6d01befb` (the last commit, three page lines): the format check again, 352 files 0 changed each time. The probe on a scratch merge of `d7ab7bae` with main `8b77e7e766da2561c4a8c35f6e82b70c50fac168` (never pushed; the merge is clean, the probe's three files are the same in the merge result, and it has the `AGENTS.md` bullet): ```text NVIM v0.12.3: exit 0 COUNT tools-call-wire cases=44 failed=0 leaks=0 NVIM v0.8.0: exit 0 COUNT tools-call-wire cases=44 failed=0 leaks=0 ``` ## Limits - The window above stays. It needs a signal in one command boundary, and the 5-second promise (nvim is still alive and its main loop runs) covers it. - The page says that `mise which node` can write files under the caller's home. The probe does not control that. - In one trial of the second verifier's rerun (a TERM right after the `COUNT` line, exit 0, no process and no lock file left), bash printed its own warning on stderr: `warning: run_pending_traps: bad value in trap_list[15]: 0x1`. Its cause is not established. Main's launcher has the same `trap '' TERM INT HUP` line in its cleanup. Nobody ran this case on main. - Code that the caller picks to run (the tree, the nvim binary, the node binary, a helper command on the caller's `PATH`) can copy the token to another place: the probe trusts it, and the page says that this is outside the promise. That code includes what mise finds: with mise's default setup, `mise which` looks for `node` and `nvim` under the caller's `HOME`, so a `HOME` that another user can write lets that user's binary run as the checker. No mechanism is added for it. - The leak check reads text frames only. It does not read Ping, Pong and Close frames, and no check of them is added. - No CI job runs the probe. It needs nvim and Node. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
…emp dir (coder#575) ## Why The operator's goal is to land the code-shape refactor of the plugin on main in small PRs, fixes first. The `tools/call` wire probe (coder#517) checks that no frame that the plugin server sends leaks a path. The launcher gives nvim its own `HOME`, XDG folders, `TMPDIR` and `CLAUDE_CONFIG_DIR` inside its temp dir (`/tmp/wire-probe.XXXXXX`), and the lock file is there too. The checker looked only for the home folder that the password database gives. So a frame that holds `vim.fn.expand("~")`, `vim.env.HOME`, a `stdpath()` path or the lock file path passed with `cases=44 failed=0 leaks=0` on both Neovim versions. The page said that the probe checks for "the user's home folder" and "a path under the real home folder", and for "a Lua source path" where the rule looks for a Lua position (`.lua:` and a digit). ## What changed - The checker (`proof-tools-call-wire.mjs`) rewrites each of four paths to a placeholder by plain text, as main does for the tree and the home folder: the work folder to `$CWD`, the temp dir to `$RUN`, the tree to `$TREE` and the real home folder to `$HOME`, in that order. `$RUN` joins `PLACEHOLDERS`, `PATHS` and `LEAK`, so a frame that holds a path under the temp dir is a leak. The launcher passes the temp dir as a new fourth argument, after the work folder. - The launcher makes the work folder in a second temp dir, `/tmp/wire-probe-cwd.XXXXXX` (the folder is `work` in it, as the case table names it), beside the temp dir and not in it. The two names differ after `wire-probe` (`.` against `-cwd.`), and `mktemp` fills `XXXXXX` with letters and digits, so neither path is a prefix of the other. The launcher also asserts it. So no path under the temp dir starts with the text of the work folder, and the checker needs no rule for where a path ends inside raw JSON text. Earlier versions of this PR had such a rule (a text prefix, a whole-path boundary on every path, then on the work folder only), and each drew one more case in review. - The cleanup of the launcher removes both temp dirs, in the same function and the same way. The cleanup rule of `AGENTS.md` ("Wire probe cleanup": nvim is a direct child of the script, checks its parent every half second and stops its server, so the lock file is gone within 5 seconds after the script ends) is unchanged and still true. The second dir adds no second cleanup mechanism. - The checker's refusal of a path with a double quote, a backslash, a control character or DEL covers the temp dir path by itself now, and the message names "the temp dir". With a backslash in the temp dir path, nvim fails first (`nvim wrote no lock file in 10 s`). Both end with exit 2 before a case is sent. - The page (`features/tools-call-wire.md`), lines 3, 12, 23, 31, 37, 39, 40 and 41, says that the check finds four things: a Lua position, a path under the temp dir, the tree and the home folder. It says what lives in the temp dir (`HOME`, the XDG folders, `TMPDIR`, `CLAUDE_CONFIG_DIR` and the lock file), that each rewrite is plain text, where the work folder lives and why no path under the temp dir can read as the work folder, that the script removes both temp dirs, and that a home folder of `/` switches the home check off, because the checker keeps a path only when it is longer than one character. Only line 3 said "a Lua source path"; it now says "a Lua position (`.lua:` and a digit)". The `stdpath()` paths are under the temp dir, except `config_dirs` and `data_dirs`, which are system folders. - `AGENTS.md`, the line "Wire probe cleanup": "Do not add a second cleanup mechanism to make that time shorter." becomes "Do not add a second cleanup mechanism." The old words limited the ban to one purpose, and the rule bans a second cleanup mechanism for any purpose. Nothing else in that file changes. - The checker and the launcher together change 48 lines against the base (32 added, 16 deleted). The four files of the PR change 66 lines (41 added, 25 deleted). - The first automated code review raised a P2 on `features/tools-call-wire.md` line 39: the sentence "the temp dirs can stay, and neither holds a token" is false when a SIGKILL of the launcher lands after its cleanup has killed a blocked nvim and before the run dir is removed. A verifier had found the earlier form of the sentence false in another case. The last commit (`c33e1d894d12e8a61463481d8c6550b8ca82919f`) cuts the token claim instead of refining it: line 39 says only that the temp dirs can stay, and the next sentences of the same line say when the lock file keeps its token (the 5-second bound needs nvim's main loop to run, and a SIGKILL of the whole process group leaves the lock file with its token in the `0700` temp dir). Its parent commit (`cecf52dbae3e59435e2e1f33da10d232f4c6e5fe`) tried a longer sentence with a list of cases, and the last commit replaces it: the line now equals the line of `601c4064` without `, and neither holds a token`. ## How I proved it Every run uses `PATH` with only the node dir, `/usr/bin` and `/bin`, an absolute Neovim path (`VCN_NVIM`), a scratch `HOME`, and an empty environment otherwise (`env -i`). The launcher gives nvim its own `HOME`, XDG folders and `CLAUDE_CONFIG_DIR`. No `claude` runs. The runs use the verifier's `probe.sh` and mutant list, with these changes: `probe.sh` reads the temp dir from `CLAUDE_CONFIG_DIR` (it read it from `WIRE_CWD`) and reports the work folder's temp dir too, and the builder takes the commit and the checkout of this head. The ten `r_*` mutants below are mine. The matrix has 68 runs across the two Neovim versions, and the host load was 23 to 28 (`evidence/option-b-r4/matrix.txt`). 1. The unchanged plugin passes on Neovim 0.12.3 and v0.8.0: `cases=44 failed=0 leaks=0`, exit 0. 2. Ten mutants put the temp dir into the `-32602` data of the entry check, in the spellings that earlier rounds used for the work folder: the temp dir itself, a path below it, plus `X/file.txt`, plus `-backup/file.txt`, plus `: now`, plus ` now`, plus one backslash, plus two backslashes, plus `\/sub`, and plus `"X`. All fail on both versions: `cases=44 failed=31 leaks=31`, with 31 lines that name `$RUN`. 3. The same spellings on the work folder pass on both versions: the work folder itself, a path below it, and the work folder plus `X/file.txt`, `-backup/file.txt`, `: now`, one backslash, two backslashes, `\/sub` and `"X` (`cases=44 failed=0 leaks=0`). The work folder lies beside the temp dir, so text that starts with its path is no path under the temp dir, and a frame may hold the work folder. The parent of the work folder plus `" now"` passes for the same reason. With the previous commit of this branch (`8716d76a`), the work folder plus one backslash and plus two backslashes passed although the work folder was under the temp dir there (the finding of the second verify round), and the temp dir plus one backslash failed. 4. The four mutants of the first version (`vim.fn.expand("~")`, `vim.env.HOME`, `vim.fn.stdpath("data")` and the lock directory) fail on both versions: 31 of 44 cases, 31 lines name `$RUN`. A Lua position, the tree path plus `": x"`, the tree path, the real home folder (also plus `": now"`) and the token (`<holds the auth token>` in 31 lines) fail 31 of 44 cases on both versions. 5. After each of the 68 runs, neither temp dir was left, and `/tmp` held no `wire-probe*` entry after the matrix. 6. A SIGKILL of the launcher alone, right after the lock file appeared, on both versions (4 runs, nvim was alive and stopped its server): the lock file and nvim were gone after 222 to 432 ms. Both temp dirs stayed, the first with 3 files (2 on v0.8.0) and the second with none, and no run of 8 characters of the token was in a file of either dir or in the output (`evidence/option-b-r4/kill-results.txt`). 7. Run through the launcher (a verifier's run), a temp dir path with a double quote, a newline or DEL ends with exit 2 and the message "the path of the temp dir holds ...", before a case is sent. A backslash in the temp dir path ends with exit 2 and `nvim wrote no lock file in 10 s`, because nvim fails first. My own run (`evidence/option-b-r4/refusal.txt`) started the checker alone, and there a backslash gives the temp dir message too. 8. The token: in the 51 of 68 runs where the probe read the throwaway server's token, no output line holds a run of 8 or more of its characters. In the other 17 runs it did not read the token in time, so those runs carry no token count. 9. Gates on the last commit `c33e1d894d12e8a61463481d8c6550b8ca82919f` (each file ends with its exit code): `mise run format-check`, `mise run check` (`CASES 5` for `proof-static.sh`, 0 warnings / 0 errors in 185 files) and `check.sh --hash-only` (`covered.lock matches the code`) all pass. `mise run test` is not run: no line of `lua/` or `plugin/` changes. 10. Independent checks: the first version (`d649d11b`) had one finding in the first verify round. The head `8716d76a` had one finding in the second round, which this commit removes by construction. `b64a3414` had no verify round. In the third round, one fresh verifier passed `387078bd` with one wording finding: the page said that both temp dirs stay after a SIGKILL of the launcher, but a kill during the cleanup leaves only the work folder's temp dir. The commit `601c4064` fixes that one page line. The first automated code review then raised one P2 on it (the last bullet of "What changed" says what it found). The two last commits change only that same page line, so no run above depends on them. ## What this PR does not cover - Another spelling of a path stays outside the check, as before: the other name of a symlink, a relative path, `~` as text. The page says so. This PR adds no detector for one. - A path that is neither under the temp dir, the tree nor the home folder is not found (`/usr/share/...` for example). The work folder's own temp dir (`/tmp/wire-probe-cwd.XXXXXX`, the parent of the work folder) is not looked for either. - The work folder is not looked for: a frame may hold it, with any text after it. - The launcher is the only caller of the checker, and it passes the work folder and the temp dir as its third and fourth arguments. - After a SIGKILL of the launcher, the temp dirs can stay, as the one temp dir did before. Whether the lock file keeps its token depends on nvim: the page says when, and this PR makes no general claim about it. - The base is an older main (`523ee200`). The files of this PR are the same on main now, and a text merge into main is clean. This PR and part 1 of the wire probe's second follow-up (`skills-wire-probe-table-1`) change adjacent lines of the page (the `call-arguments` to `call-no-leak` lines): one text conflict, which the PR that lands second resolves by taking part 1's three lines and this PR's `call-no-leak` line. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_ <!-- xum-attribution: model=anthropic:claude-sonnet-5-5 thinking=xhigh -->
Why
The operator asked for a code-shape refactor of the plugin, landed on main in small PRs, fixes first. His rule for the verification skill: a scratch harness that catches a blocker goes into the repo's skill, and memory points at repo paths. The wire probe that caught the blockers of #500 over four review rounds lived only in a scratch folder, so the next agent could not run it. This PR puts it in
.agents/skills/verify-claudecode-nvim.What changed
Three new files under
.agents/skills/verify-claudecode-nvim. No existing file, nolua/,plugin/,tests/,scripts/or.github/file changes (425 added lines, above the 300-line aim: the launcher, the checker and the page are all in it).scripts/proof-tools-call-wire.sh [tree]is the one command. It starts the plugin server oftree(default: this checkout) in one headless nvim and runs the checker. Exit 0 pass, 1 a case failed or a frame leaked, 2 the probe could not run or judged fewer cases than the table holds.scripts/proof-tools-call-wire.mjsholds the case table, the WebSocket client and the checker. It prints oneok:orFAIL:line per case and ends withCOUNT tools-call-wire cases=<n> failed=<n> leaks=<n>.features/tools-call-wire.mdis the feature page: the command, a pass and a fail, the red proof, the normalization.Rules in the script:
params, expected answer. A refusal names the parameter (-32602, and the data names it). A normal answer is written out whole. The tools, argument shapes and wrong values are expanded from short lists into rows, and the checker has one path for every row.$CWD, the plugin tree$TREE, the real home folder$HOME, in that order. A frame leaks when its text then holds.lua:and a digit,$TREEor$HOME, or 8 or more characters of the auth token in a row. So a path in a sibling field, an object key, a nested list, a duplicate key or a JSON fragment inside a plain string leaks. It also reads a frame that comes before the answer, or within 200 ms after the last answer, and counts a frame for one case only. A frame that arrives after its own answer is judged with the case in flight. The comparison with the table reads a rebuilt copy of the answer that rewrites strings but not keys, so two keys that the rewrite would make equal never pass as a correct answer. The leak rule finds each of the three paths in a frame unchanged, so the probe refuses to run when the tree path, the work folder path or the real home folder path holds a double quote, a backslash or a control character (the characters that JSON writes escaped). It exits 2 with a message that names the path, before it sends a case. A backslash in the tree path can make nvim fail to load the plugin first: the script then exits 2 withnvim wrote no lock file in 10 s. The workspace folder is not a leak:getWorkspaceFoldersreturns it on purpose, and its expected answer is{"folders":[{"name":"work","path":"$CWD","uri":"file://$CWD"}],"rootPath":"$CWD","success":true}.COUNTline. Thecases=count is the number of cases that the checker judged.PATH=/usr/bin:/bin,WIRE_PARENT,WIRE_TREE,WIRE_CWD, andHOME,TMPDIR,XDG_CONFIG_HOME,XDG_DATA_HOME,XDG_STATE_HOME,XDG_CACHE_HOMEandCLAUDE_CONFIG_DIRin/tmp/wire-probe.XXXXXX(frommktemp -d, mode 0700), and the checker gets no variable. A version-manager shim that does start adds variables of its own. nvim runs withterminal.provider = "none". The script exits 2 when aclaudeprogram is onPATH=/usr/bin:/bin, when nvim or node does not start with an empty environment (a version-manager shim does not), and when the temp dir cannot be made. nvim must be a direct child of the script: it checks its parent before the plugin starts, and when the parent is not the script (a wrapper that runs Neovim withoutexec) it quits with an error, so no server starts and no lock file is written. The script then exits 2 with a message that names the wrapper and says to setVCN_NVIMto the Neovim binary itself. A wrapper that usesexecpasses. When the server does not come up for another reason, the script prints the last lines of nvim's output.0600lock file in that folder. The checker sends it only in the upgrade header. The checker prints only text that it wrote itself (stop) and the text of a frame that passedshow, which withholds a text that holds 8 or more characters of the token in a row (the output says<holds the auth token>). It never prints the message or the stack of an exception that Node or a library raised, because such a message can quote its input (the WebSocket constructor quotes the header value): a frame that is not JSON and a lock file that does not parse end the run with exit 2 and a fixed message, and any other exception ends it with exit 2 and one line that holds only the exception's class name. When the server does not come up, the script prints the last lines of nvim's output. That happens only while no lock file exists, and the plugin logs only the length of its token. On a normal exit, a failed case, TERM, INT and HUP, the script stops nvim and the checker by PID (it polls for their end and sends SIGKILL, and does notwaitfor them) and removes the folder. After SIGKILL no cleanup runs: nvim, a direct child of the script, checks its parent every half second and quits, its server stop removes the lock file and closes the socket, and the checker exits on the closed socket. Both processes and the lock file are gone within 5 seconds (the bound the script holds to; 0.41 s at most in the 10 runs below). The folder stays then, with no token in it.vcn.sh launch: it starts a TUI in agent-tty (agent-tty, Chromium) and puts this checkout on the runtimepath, so it takes no tree argument, which the red proof needs. The probe keeps the skill's conventions:VCN_NVIM, node frommise which node, and a client that takes the token from the0600lock file asws-client.mjsdoes. It does not callvcn.sh, so fix(skills): check the state dir once, not each run.env, and resolve TMPDIR once #506 does not change how it runs.Cases in this PR, and the cases left out
44 cases:
paramsabsent,false,null,true, a number, a string,[],[1](all nameparams) and{}(namesname). A non-stringname(8 values). ForgetWorkspaceFoldersandopenFile:argumentsabsent,{}and each of the 7 non-object values, and foropenFileeach wrong type offilePath(8 values) plus"".Left out, all for size: the other nine tools (
getOpenEditors,getCurrentSelection,getLatestSelection,closeAllDiffTabs,getDiagnostics,checkDocumentDirty,saveDocument,close_tab,openDiff) with their string parameters and thegetDiagnosticsuri, thectl_*valid-call controls, and the two notification cases of the scratchwire3.lua. They are the next PR, which adds table rows and the silent-answer check. The index lines (SKILL.mdHelpers andfeatures/README.md) are not in this PR either, because #506 and #508 are open and change those files. They go in the next PR.How I proved it
Neovim 0.12.3 and the official v0.8.0 release, each three runs in a row, from the head of this branch (merged with main
4c1df79. Main has moved since toc9e57d13, which changes nothing in the skill folder or in these three files, so I did not merge it, and a run on a tree ofc9e57d13printed the same line on both versions): every run printedCOUNT tools-call-wire cases=44 failed=0 leaks=0and exit 0, and the six outputs are byte-identical.Red proof (
git archive f932d7b6a91c71c3728257ae6284bf03f9d09c77, main before #500), exit 1 on both versions:Round 3 (findings from the third review round, my proofs on this head):
vim.json.encodewrites"as\", so the rewrite to$TREEmisses it). The mutant that puts the tree path into an object key of an answer, in the folder/tmp/wp517-q"t/: the previous head exits 0 withcases=44 failed=0 leaks=0on both versions, and this head exits 2 with the message above and nook:line on both versions. The same mutant in a plain path stays red (failed=2 leaks=2), and the mutant with the tree path in every refusal givesfailed=31 leaks=31. The checker alone refuses a quote in the tree, in the work folder and in the home folder, on Node 22.23.4 and 26.12.1. A tree path under the real home folder is read as$HOME, so the old gap shows only outside it. A backslash in the tree path makes nvim fail first, and the script exits 2 withnvim wrote no lock file in 10 son both versions. A newline in the tree path does not: the checker refuses it with its own message (exit 2, both versions).stopped by an unexpected TypeError, 0 runs on stdout and stderr, on both versions.cdinto the new temp dir that fails (a stand-inmktempthat prints a dir that does not exist, or a dir with mode 000): this head names the dir thatmktempprinted in both cases, and leaves 0 dirs. The previous head named an empty path in both, and left 1 dir in the mode 000 case.wait <pid>blocked on a child that was already gone: 3 hangs in 118 signals sent right after theCOUNTline, all three HUP on 0.12.3). The cleanup no longer callswait. My 100 TERM trials on 0.8.0, each right after theCOUNTline, hung 0 times (my log keeps no exit code, so it does not show that the trap ran in each one, and I count none of my HUP or INT trials after the change). Two independent verifier runs on this head: 0 hangs in 130 and in 120 signals. Withwaitback in the cleanup: 1 hang in 130 and 1 in 60. I did not find the cause inside bash.WebSocket handshake failed. The checker now drops that handler when the connection opens, and printsthe server closed the connectionon both versions (SIGKILL of nvim, exit 2, no process left, no temp dir).cases=44 failed=0 leaks=0and byte-identical outputs, red prooffailed=30 leaks=15on both, wrapper withoutexecexit 2 and withexecpass (3 runs each, both versions), SIGKILL of the script (5 runs on each version, 0.41 s at most), 12 signal trials (TERM, INT and HUP, each between the lock file and the end of the run and right after theCOUNTline, both versions: exit 143, 130, 129 or 0, no process, no temp dir left).Findings of rounds 1 and 2 and their proofs (copies, not committed):
mE: one key is the work folder path and holds/virtual/probe.lua:42, one key is$CWDand holdssafe):failed=41 leaks=41on both versions, and the previous head printedfailed=0 leaks=0. A duplicate key in the text item of a correct result (the real home path in the first, so the parser keeps the last):failed=2 leaks=2, and the previous head passed on both versions. A path in a JSON fragment inside a plain string (the slashes are\/on Neovim 0.8):failed=31 leaks=31on both versions, and the previous head passed on 0.8. The sibling-field, object-key, nested-array and late-frame mutants of round 1 stay red (failed=2 leaks=2for the first three,failed=1 leaks=2for the late frame).xfollowed by the token (not JSON): exit 2 and one fixed message, and 0 matches of any 8-character run of the token on stdout and stderr, on both versions. The previous head printed 2 such matches on stderr. A lock file whose token value is not valid JSON, with a made-up value (not a token): 0 matches of any 8-character run of the value on stdout and stderr, and 3 on the previous head. The token in an error answer (mutantmT: the token as a fieldtof every-32602error object):failed=41 leaks=41and 0 matches.exec(VCN_NVIMis a script that runs the real binary as a child): three runs on each version exit 2 with the message, print nook:line, and leave no process and no temp dir. A wrapper withexec: three runs on each version pass withcases=44 failed=0 leaks=0.failed=1 leaks=1and oneFAILline, where the previous head printedfailed=2and a second line for the same frame. A failingmktempormkdir: exit 2 and a message, where the previous head exited 1.Round 1:
result.debugPath = "/virtual/probe.lua:42"):failed=2 leaks=2on both versions. Before, the probe printedleaks=0and exit 0. The tree path as an object key of an answer:failed=2 leaks=2, and the same mutant passes when the checker does not rewrite keys. An extra server frame 100 ms after each valid answer, which only the read after the last case sees:failed=1 leaks=2.failed=31 leaks=31, noFAILline printed the token, and the match count of the token in stdout, stderr and nvim's output file is 0.COUNTline. A loop that judges no case, or skips one row: exit 2, noCOUNTline.VCN_NVIM: exit 2 and a message that names it and says to setVCN_NVIMto a real binary. A node that does not start: exit 2 and a message that names it. A tree whoserequirefails: the last lines of nvim's output and exit 2. In a fresh clone of the branch on this host, which mise does not trust, the page's commands failed with exit 2 and that message untilVCN_NVIMnamed a real binary. With it, the whole proof exits 0, the red proof exits 1 withfailed=30 leaks=15, and the Neovim 0.8 run prints the same lines as the 0.12.3 run.HOME=/tmp:failed=31 leaks=31. The previous head printedfailed=0 leaks=0there. A module named in the caller'sNODE_OPTIONSran inside the checker of the previous head (1 marker line) and does not run now (0)./tmp(a private tmpfs), the previous head left/tmp/nvim.coderand this head leaves nothing, on both versions.Controls that must go red (copies, not committed): main with the entry error holding the tree path, or the real home path:
failed=31 leaks=31each. The checker without the$CWDrewrite:failed=2 leaks=0. A checker whose home folder is/tmp, so that the run dir lies under it: with the$CWDrewrite first it printsfailed=0 leaks=0, and with the$CWDrewrite after the$HOMEonefailed=2 leaks=2. A wrong expectedrootPath:failed=2. A wrong named parameter forfilePath:failed=8.Other checks:
claudeon the stripped PATH (a stand-in that writes a marker file): exit 2, the marker was never written, no temp dir was made.COUNTline, left no process and no temp dir (12 trials, see round 3). The cleanup ignores TERM, INT and HUP while it runs.lua/claudecode, a missing nvim, an nvim that exits at once, and a tree whoserequirefails: exit 2 with a message each.failed=30 leaks=15on the red tree with Node 22).eb16b3aa), docs(skills): stop telling callers to source run.env, and make the bare-tool self-check strict #508 (17252700), fix(skills): check the state dir once, not each run.env, and resolve TMPDIR once #506 (ab9d431c) and fix(diff): end diff mode in windows that show the file after a split diff closes #494 (c219b155), never pushed: no file of this PR changes, and each rancases=44 failed=0 leaks=0on both versions. fix(diff): hold diff teardowns behind the shared window gate #511 (ef610b7d) and fix(diff): close a new-tab diff without entering its tab #485 (d00e1be2) conflict with current main incovered.lockon their own, and their trees do not hold fix(tools): check a tools/call once and answer a malformed one with -32602 #500 (the probe printsfailed=30 leaks=15on them), so a merge check says nothing about these three files. Neither changes a path of this PR.failed=0 leaks=0, the red proof exits 1 withfailed=30 leaks=15, and Neovim 0.8 prints the same line. WithoutVCN_NVIMthe clone (which mise does not trust) exits 2 and names the shim.Gates on this head:
mise run format-check351 files 0 changed,mise run check0 warnings / 0 errors in 176 files,mise run testnative exit 0 with 1586 successes / 0 failures / 0 errors / 22 pending (seed 1794431103), andbash .agents/skills/verify-diff-lifecycle/check.sh --hash-onlyprintscovered.lock matches the code.Limits
getWorkspaceFoldersandopenFileare in the table. The feature page says so./tmp/wire-probe.XXXXXX. The probe ignoresTMPDIR.vcn.shhas the samebin()and thenenv -istart for nvim, so a version-manager shim fails there too. This PR does not changevcn.sh, because fix(skills): check the state dir once, not each run.env, and resolve TMPDIR once #506 rewrites it.Generated with
xum• Model:anthropic:claude-sonnet-5-5• Thinking:xhigh