chore(skills): ignore later signals in the wire probe's cleanup and state its promises on the page by ThomasK33 · Pull Request #555 · coder/claudecode.nvim

ThomasK33 · 2026-11-12T07:53:26Z

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. #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 #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 #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 #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):

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 • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh

…tate its promises on the page Follow-up A of the wire probe (#517). The probe's cleanup now starts with the ignore of TERM, INT and HUP, and each of the three handlers ignores them before it exits, so a further signal cannot end a cleanup that has started. A signal that lands between the start of the exit handler and the cleanup's first command can still cut the cleanup short, and the 5-second bound (nvim checks its parent every half second) holds then. nvim is disowned after the start, so bash no longer prints a `Killed` line when it dies in the middle of a run, and the `cd` into the new temp dir no longer prints bash's own line before the probe's message. The page words the shim sentence so that it is true ("may not start"), narrows "leaves nothing outside the temp dir" to nvim and the checker, and states once what the probe promises about the leak check and the auth token: what the leak check is for and what it leaves outside, who can read the token, the inputs that do not change that, the 5-second promise for every end of the script with its premise, the SIGKILL to the whole process group, and where the lines of a failed start come from.

…tten Wording fixes after the first verify round of the follow-up: - The 5-second bullet promises the bound for an end of the script while nvim is still alive and its main loop runs. The two cases outside it stay named. - The leak-check bullet says where the paths come from: the tree path as the script resolved it, the home path as the password database gives it. - The environment bullet says that a signal can cut the cleanup short, and points at the bullet that says what stays then. - The prerequisite bullet says a shim "can fail to start" with an empty environment. - "Can write to mise's own cache" became "can write files": `mise which node` wrote `.cache` and `.local` under the caller's home in a run.

… start The header comment said that a version-manager shim does not start with an empty environment. The page says that it can fail to start, and a shim did start inside a sandbox. Comment text only: no code line changes.

The operator decided that the wire probe keeps one cleanup mechanism: nvim is a direct child of the script, checks its parent every half second and stops its server when the parent is gone. This bullet in the Security & Configuration Tips section writes that rule down, so that a later change does not add a second cleanup mechanism to make the time shorter. The page of the probe states the limits. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_

ThomasK33 · 2026-11-12T07:53:37Z

@codex review

ThomasK33 · 2026-11-12T07:53:49Z

@codex security review

chatgpt-codex-connector · 2026-11-12T07:53:50Z

Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-12T09:47:13.156610Z	`6d01bef`	Manual request
🔒 Security Review	✅ Completed 2026-11-12T09:47:04.850769Z	`6d01bef`	Manual request

ℹ️ 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" or "@codex security review".

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 · 2026-11-12T07:56:18Z

🛡️ Codex Security Review

Security review completed. No security issues were found in this pull request.

Reviewed commit: 03fc877df8

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

You comment "@codex security review"
A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 03fc877df8

ℹ️ 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".

Two sentences of the page promised more than the probe keeps (Codex review of the PR, two findings): - The token bullet said that no input of the caller changes who can read the token, the tree and the binaries included. A tree, an nvim or a node that the caller picks runs as the caller in a process that holds the token and can copy it. The bullet now says that the probe trusts that code and that such a copy is outside the promise. TMPDIR, HOME, the umask and CLAUDE_CONFIG_DIR stay harmless. - The leak bullet said that the plugin writes every frame with vim.json.encode. The plugin builds Ping, Pong and Close frames without it, and the checker's WebSocket client passes no Ping or Pong to the checker, which ignores the code and reason of a Close. The bullet, the call-no-leak line and the leak-rule step now say text frame. No script and no mechanism changes. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_

…trusts The page said that no other input of the caller changes who can read the token. The script also finds env, mktemp, rm and other helper commands on the caller's PATH, and they run as the caller too. The sentence now names them with the tree and the two binaries, and it says only what holds for the caller's TMPDIR, HOME, umask and CLAUDE_CONFIG_DIR. _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-fable-5-2` • Thinking: `xhigh`_

…probe checks A check found that the page still promised too much in three places. With mise's default setup, mise looks for node and nvim under the caller's HOME. So HOME is part of what the caller picks, and the page now says so. What holds for the caller's TMPDIR, HOME, umask and CLAUDE_CONFIG_DIR is narrower: they do not move the lock file or change its mode. No variable of the caller reaches the checker. The page said "loads code into", which HOME contradicts. The opening said that no frame of the server shows a path. The server echoes a peer's Close reason, and the probe reads text frames only. The opening now says what the probe checks. _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-fable-5-2` • Thinking: `xhigh`_

ThomasK33 · 2026-11-12T09:44:04Z

@codex review

ThomasK33 · 2026-11-12T09:44:15Z

@codex security review

chatgpt-codex-connector · 2026-11-12T09:47:03Z

🛡️ Codex Security Review

Security review completed. No security issues were found in this pull request.

Reviewed commit: 6d01befb1a

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

You comment "@codex security review"
A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

chatgpt-codex-connector · 2026-11-12T09:47:11Z

Codex Review: Didn't find any major issues. 🚀

Reviewed commit: 6d01befb1a

ℹ️ 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".

…a reason (coder#554) ## Why coder#532 left four follow-ups from its last review and the readiness advisor. This PR makes them, and it fixes what the review of this PR found. ## What changed 1. **A line with a reason for every non-zero exit, by stage.** A file whose Neovim exits non-zero gets at least one line: `FAILED: <file>: no plenary summary (nvim exit N)`, the `TIMEOUT:` line, or a new `FAILED: <file>: nvim exit N`. The new line prints when neither of the other two did, for example when Neovim crashes or exits 1 after a clean summary. A file whose specs fail (plenary exits 1) now also gets `FAILED: <file>: nvim exit 1`. A file with no summary whose status is 124, 137 or 143 gets two lines, the no-summary line and the `TIMEOUT:` line, as it does on main. A crash before the summary (for example 139) gets one line, the no-summary line. An exit status above 128 names its signal: `nvim exit 139, SIGSEGV`, also in the no-summary line. A failing `tee` gets its own line, `FAILED: <file>: tee exit N`, with the signal name above 128 (`tee exit 141, SIGPIPE`). On main, a `tee` failure after a clean summary printed no reason line, and the no-summary line showed the status of `tee` as the status of Neovim (a `tee` that exited 3 gave `nvim exit 3`). The file's subshell writes the exit statuses of Neovim and of `tee` to a file in the state folder, so the runner's cleanup deletes it with the folder. A subshell that ends after the cleanup (a signal stop) usually finds the folder gone, and its write fails without a message. When both fail, both lines print. Known limit: the subshell can still open its file while the cleanup runs. Under `strace -f`, this left the state folder with `exit_statuses` in `TMPDIR` and made `rm` print `Directory not empty` in 3 of 3 runs (2 scripted runs and 1 manual run), and main was clean in the 1 run under the same setup. At normal speed it happened in 0 of 150 runs on this PR, and in 0 of 60 runs on main. A follow-up PR in this run fixes it. 2. **The `TIMEOUT:` lines are true.** Exit 137 comes from `timeout -k` and also from an out-of-memory kill: `TIMEOUT: <file> was stopped (exit 137, SIGKILL): nvim did not exit within 40s, or another process killed it`. An outside SIGTERM that reaches `timeout` gives 124 with uutils `timeout` and 143 with GNU `timeout` (a child that dies of the signal), so the lines for 124 and 143 now end with `or an outside signal stopped it`. 3. **The file's process group is killed after any non-zero exit of Neovim (a `tee` failure alone does not kill it).** Before, only a timeout status killed it. When the Neovim that the runner starts died by a signal before its summary, the Neovim that plenary started for the spec ran on. This is on main too. The cleanup after a signal to the runner is unchanged. Known limit, not changed: the kill uses the process group id that `timeout` had. A process group id that another process reused in the few milliseconds after `timeout` ended would get SIGKILL. `pid_max` is 4194304 on my host, so a reuse needs a wrap-around in those milliseconds. Main has had the same risk after a timeout status since coder#445. I declined to change it, because the chance is very small (a wrap-around of 4194304 ids inside those milliseconds) and main already takes the same risk after a timeout status. The follow-up PR records this reason. 4. **README.** `scripts/README.md` says what the early window does for a runner that leads a pty session: SIGTERM, SIGHUP, SIGINT and SIGUSR1 leave nothing and start no Neovim, SIGKILL starts no Neovim but leaves all four temp items (without a terminal the Neovim starts as well), and bash ignores SIGQUIT so the file runs to its end. It also describes the new output lines and where the statuses go. 5. **Comments and tests.** The comment on the reason lines says what is true (see 1). The `-i NONE` test also checks `--headless` and `-u` followed by `tests/minimal_init.lua`. ## Tests New or changed runner spec tests (the names are the columns of the table below): - `clean_after`: five tests. A clean summary followed by exit 139, 134, 130, 129 or 1 prints the `FAILED:` line (with `, SIGSEGV`, `, SIGABRT`, `, SIGINT`, `, SIGHUP` or no name), exactly one `FAILED:` line, the counts in the totals, and the file in the final list. - `crash_before`: no summary and exit 139 names SIGSEGV in the no-summary line, with one `FAILED:` line and no `TIMEOUT` line. - `sigkill_timeout` and `kill_after`: a stub Neovim that kills itself with SIGKILL gets the `TIMEOUT:` text, exactly once, and no extra `FAILED:` line after a clean summary. - `stuck_137`: the test of a Neovim that ignores SIGTERM expects `(exit 137, SIGKILL)`. - `tee_stage`: `tee` exits 3 after a clean summary: `FAILED: <file>: tee exit 3`, no `nvim exit` text. - `both_stages`: Neovim exits 1 and `tee` exits 141: both lines, `tee exit 141, SIGPIPE`. - `outside_term`: an outside SIGTERM reaches `timeout` while the stub Neovim sleeps. The `TIMEOUT:` line has the new text with exit 124 (uutils) or 143 (GNU), exactly once. - `p3_orphan`: exit 139 before any summary with a process left in the group: the process is killed. - `cleanup`: after a file that failed, with a failing `tee`, nothing is left in `TMPDIR`, the status file included. - `late_tee`: a `tee` that ends a second after the SIGTERM, long after the runner cleaned up: nothing is left in `TMPDIR`, and the runner prints no `No such file` message. Each test ran against each mutant of the runner, with uutils `timeout` 0.10.0 and with GNU `timeout` 9.7. The two builds gave the same table. Red means the test fails. | Variant | clean_after | crash_before | sigkill_timeout | kill_after | stuck_137 | tee_stage | both_stages | outside_term | p3_orphan | cleanup | late_tee | |---|---|---|---|---|---|---|---|---|---|---|---| | this PR | green | green | green | green | green | green | green | green | green | green | green | | main's runner | **red** | **red** | **red** | **red** | **red** | **red** | **red** | **red** | **red** | **red** | green | | first push of this PR | green | green | green | green | green | **red** | **red** | **red** | **red** | **red** | green | | signal names only above 130 | **red** | green | green | green | green | green | green | green | green | green | green | | status file ignored | green | green | green | green | green | **red** | **red** | green | green | green | green | | no `tee` line | green | green | green | green | green | **red** | **red** | green | green | green | green | | a `tee` failure does not fail the file | green | green | green | green | green | **red** | green | green | green | green | green | | `tee` line without the signal name | green | green | green | green | green | green | **red** | green | green | green | green | | 124 and 143 lines as on main | green | green | green | green | green | green | green | **red** | green | green | green | | group kill only after a timeout status | green | green | green | green | green | green | green | green | **red** | green | green | | no group kill after a non-zero exit | green | green | green | green | green | green | green | green | **red** | green | green | | state folder not deleted | green | green | green | green | green | green | green | green | green | **red** | green | | nvim line uses the status of the subshell | green | green | green | green | green | **red** | green | green | green | green | green | | status file in the temp folder (previous push) | green | green | green | green | green | green | green | green | green | green | **red** | | status write without the guard for a folder that is gone | green | green | green | green | green | green | green | green | green | green | **red** | The existing test of what a timed-out file leaves running is green for this PR and for the variant that kills the group only after a timeout status. It is red when the group kill is removed. `late_tee` is green for main and for the first push because neither has a status file. Not tested: the fallback for a status file that the runner cannot read. The runner then uses the exit status of the subshell. A variant whose subshell exits with the status of Neovim only, and so loses the status of `tee` in that fallback, passes every test. A follow-up PR in this run adds a test for it. ### Real Neovim A tree with one spec, `timeout` 0.10.0 (uutils) and 9.7 (GNU), stdin `/dev/null`: | Case | main | this PR | |---|---|---| | the runner's own Neovim dies by SIGSEGV after the spec's Neovim started | `FAILED: … (nvim exit 139)`, the spec's Neovim still running 3 s after the runner ended | `FAILED: … (nvim exit 139, SIGSEGV)`, the spec's Neovim gone | | SIGTERM to the `timeout` that runs the runner's Neovim, before the summary, uutils | exit 124, `TIMEOUT: … (exit 124): nvim did not exit within 40s` | exit 124, the same line ending `, or an outside signal stopped it` | | the same, before the summary, GNU | real Neovim exits 1 on SIGTERM: `FAILED: … no plenary summary (nvim exit 1)`, no `TIMEOUT:` line | the same output | | the same, after the summary, uutils | the `TIMEOUT:` line only | the `TIMEOUT:` line only, with the new ending | | the same, after the summary, GNU | no line | `FAILED: … nvim exit 1` | | a `tee` that exits 3 after a clean summary | runner exit 1, no reason line | `FAILED: a_spec.lua: tee exit 3` | Early window, a stub `bash` that delays the process id file, a hanging spec, the runner leading a pty session: | Signal to the runner | Neovim started | Temp items left | |---|---|---| | SIGTERM, SIGHUP, SIGINT (Ctrl-C) | no | none | | SIGKILL | no | all four | | SIGQUIT | yes, the file runs to its end (bash ignores the signal) | not applicable | Without a terminal, SIGKILL to the runner leaves all four items and the Neovim starts. ### Whole runner, main versus this PR An independent verifier ran this on this exact head, with stdin `/dev/null` and `TMPDIR` unset. The rows are identical: 123 per-file rows each (30 files with a `Running` line and three count lines, plus three totals), no difference, and no `FAILED`, `TIMEOUT`, E886, `Killed` or `No such file` line. | Neovim | main | this PR | |---|---|---| | 0.12.3 | 527 passed, 0 failed, 0 errors | 527 passed, 0 failed, 0 errors | | v0.8.0 | 390 passed, 0 failed, 0 errors | 390 passed, 0 failed, 0 errors | With `TMPDIR` set to a folder with a space and a `%` (what the CI guard uses), this PR on 0.12.3 also gives 527 passed, 0 failed, 0 errors. ### Gate The verifier ran `mise run clean && mise run all` on this exact head. The formatter changed no file (0 of 426), luacheck reports 0 warnings and 0 errors in 185 files, and busted reports 1751 successes, 0 failures, 0 errors, 28 pending. `shellcheck` on `scripts/*.sh` and `actionlint` pass. A gate run of mine earlier in this round failed one test: the window test found the status file in `TMPDIR` after a signal stop, because it had been next to the output file. I moved it into the state folder and added the `late_tee` test. Three merges of main came after the commit that moved the status file (`51ed703b`). They brought coder#556 with coder#458 (a docs change and a `repro_issue_218.sh` change), then coder#494 (a diff change with its own integration spec), then coder#555 (a wire-probe cleanup change). My last gate run covered only the first of them. I ran the spec that coder#494 added through this runner 10 times alone just now: 10 passed, in 3.8 to 5.4 seconds. One earlier run of mine hit the 40 second limit on it. I did not find the cause. ## Follow-up A follow-up PR in this run has no number yet. It holds these changes: 1. Wording. The README line on the output lines and a runner comment say "a timeout or a signal" for the two-line case. It is status 124, 137 or 143, and a crash with 139 or 134 gets only the no-summary line. The README line and a comment say an outside SIGTERM gives 143 with GNU `timeout`. That holds only when the child dies of the signal, and real Neovim gives 1. The README line says "because nvim did not exit in time", which the hedged `TIMEOUT:` text contradicts. The README sentences on the cleanup ("deletes the four" and the status file) leave out the known limit in item 1 above. Optional: no signal name prints for a status from 193 to 255, because bash has no signal above 64. 2. A fix for the known limit in item 1: no file path is opened after the cleanup starts, with a regression test. Two ways are not tried yet. The runner can open the status file on a fixed file descriptor before it starts the subshell, or the cleanup can kill and reap the subshell before `rm -rf`. 3. A test that reaches the fallback for an unreadable status file, and fails for the variant that exits with the status of Neovim only. 4. A record of the group id limit from item 3 and the reason I declined it. No code change. ## Not in this PR A 40 second limit per file (`FILE_TIMEOUT`) was reached at a high host load while specs ran. The limit is the same on main. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_

… other path characters (coder#561) ## Why `cwd.git_root` must find the repository of a folder whatever its name is. A repository in a folder with a space or a comma in its path was not found when git could not run: the fallback gave nil, or the repository of Neovim's current folder. A folder named `[x] {y}` or `$HOME-like` was lost or changed even when git answered. A verifier of F12 found the first case (finding F12-P1, a space or comma in the path makes the `.git` fallback return nil). ## What changed Four problems in `lua/claudecode/cwd.lua`. The first two took a path as a pattern. The third stepped one folder down at a trailing `..`. The fourth is a flaw in my first version of the second step. 1. **The `.git` fallback** called `vim.fn.finddir(".git", start_dir .. ";")`. `finddir` reads its path as a list in `'path'` option syntax. A space or a comma split the path, and a `;` ended it. The fallback now walks up from the full path of `start_dir` and tests each folder for a `.git` folder with `vim.fn.isdirectory`. A relative `start_dir` now reaches the folders above the current one too. Before, a relative start gave a wrong answer or nil in some cases (for example `git_root("sub/deeper")` in the repository root). 2. **`normalize_dir`** passed every path through `vim.fn.expand`. `expand` reads `[`, `{`, `*`, `?`, `$` and `\` in a name as patterns or variables. A folder named `[x] {y}` or `it's` came back with a trailing slash. A folder named `$HOME-like` or `back\slash` came back nil. This also happened when `git rev-parse` answered. `normalize_dir` now returns a directory that exists as written, and expands any other string (`~`, `$VAR`). See step 4 for text that starts with `~` or `$`. 3. **`..` before Neovim 0.11.** Before 0.11, `fnamemodify(path, ":p")` keeps a last `..`, and `:h` of `a/b/..` is `a/b`. So a walk from a start folder that ends in `..` stepped one folder down and returned the wrong root. The walk now calls `simplify()` on the full path first. A verifier found this. 4. **Text that starts with `~` or `$`.** The first version of step 2 took a path that names an existing folder as written, on the raw text. `vim.fn.isdirectory` does not expand, so with `cwd = "~"` or `"$HOME"` and a stray folder of that literal name in Neovim's current folder (for example one that an unexpanded `mkdir "~/x"` made), `git_root` and `resolve` returned the stray folder where main returned the home folder. `normalize_dir` now sends text that starts with `~` or `$` through `expand` first, as a shell and `:cd` do. A verifier of the terminal change found this. `lua/claudecode/README.md` gets one new bullet that states this, with the rule for text that starts with `~` or `$`. It sits after the "Terminal cwd precedence" bullet. It names `git_root` and `resolve` only, and it says that the terminal still passes a static `cwd` and a `cwd_provider` result through `vim.fn.expand` itself. ## Scope - Only `cwd.lua` changes in code. Steps 1 and 2 are one change of idea: a path is a literal, not a pattern. Step 3 is a quirk of `fnamemodify` before Neovim 0.11. Step 4 closes a hole that step 2 opened. - `normalize_dir` serves `cwd.git_root` and `cwd.resolve`. Nothing in `lua/` calls `cwd.resolve`. The terminal calls `cwd.git_root` when `git_repo_cwd` is set, so this PR fixes folder names with `$` and `\` on that path: when git does not answer, the terminal now gets `$HOME-like`, `back\slash`, `[x] {y}`, a space and a comma right, where main gave nil or the repository of Neovim's current folder. A static `cwd` and a `cwd_provider` result do not go through `normalize_dir`: `terminal.lua:build_config` passes them through `vim.fn.expand` itself, so this PR does not change what the terminal does with those. An absolute start path that ends in `..` is fixed here, through the `simplify()` call above. - Two gaps stay for separate follow-ups. Main already has both, and this PR does not reach them. First, the fallback finds a `.git` folder only, not a `.git` file in a worktree or submodule. Second, the walk is lexical, so a link into another repository answers with the repository of the link's own folder, where git answers with the link target's repository. The terminal's `expand` of a static `cwd` and of a provider result is a third follow-up, and it has its own change. - `~` and `$VAR` still expand in `cwd.resolve` (the spec checks both). The spec's cases for `cwd.resolve` are named as `cwd.resolve` cases, because they do not test the terminal. - `git rev-parse` still runs first. Its result for a plain path is the same string as before. ## Blast Radius - User-visible: a repository in a folder with a space, a comma, a quote or another path character is now found when git cannot run. With git, `git_root` returns a folder with `[`, `{`, `*`, `?`, `$` or `\` in its name exactly. It does not reach the terminal through a `cwd_provider`: `build_config` still expands the provider's result, so the root README's example still loses `$` and `\` there (follow-up). A plain path gives the same result as before. - The fallback result is an absolute path with no `..` part. Before it could be relative. The one exception is a relative start whose name begins with `~`: Vim reads `~` as the home folder and `~name` as a user's home folder, so `:p` leaves `~name` relative and the result can be the relative `~name`. That is a pathological case, and no caller in `lua/` passes a relative path (the terminal passes the file's folder from `expand("%:p")` or `getcwd()`). - A string that names an existing folder is taken as written, except text that starts with `~` or `$`. That text always goes through `expand` first, so a folder named `~` or `$HOME` in Neovim's current folder does not take the place of the home folder, as on main. The cost: a relative folder name that starts with `$` and reads as a variable (`$HOME-like`, `$odd/sub`) is expanded and not found, as on main. A lone relative `$odd` that names no variable is left unchanged by Vim and is still found. An absolute path never starts with `~` or `$`, so every folder name this PR fixes keeps working. The change applies to `git_root`'s `start_dir` and to `cwd.resolve`. The terminal's own `expand` of a static `cwd` and a provider result is a separate change. - Neovim 0.8: the code uses `vim.fn.isdirectory`, `vim.fn.fnamemodify` and `vim.fn.simplify`, which exist there. I ran the new spec on Neovim 0.8.0, 0.10.0 and 0.12.3. The `..` case fails on 0.8.0 and 0.10.0 without the `simplify()` call, and passes on 0.12.3 either way. - `covered.lock`: `cwd.lua` is the terminal's working-directory lookup. `check.sh` printed STALE for the file hash of `cwd.lua` only, with no changed block or call site. The change cannot reach diff state, an openDiff answer, a disconnect or a diff command. The model passed (`main`: 89205 distinct states, `live-DisconnectedCallsAnswered`: 5269). `check.sh --record` wrote the new lock and `check.sh --hash-only` reports that it matches. ### Merge with coder#539 (F12) coder#539 landed first. This PR's merge commit with `origin/main` resolves the interplay: it re-records `covered.lock` and changes the one stale test in coder#539's `cwd_spec.lua` (see Verification). F11 (coder#535) merges with this PR without a conflict. ## Verification Red before the fix, green after (same spec, only `lua/claudecode/cwd.lua` swapped): | Spec | origin/main | with the fix | | --- | --- | --- | | `tests/integration/cwd_path_characters_spec.lua` (37 cases, real Neovim, 0.8.0 and 0.12.3) | 20 successes, 17 failures | 37 successes, 0 failures | | the new case "goes up from a start folder that ends in .." at `ede2f733` (before the `simplify()` call) | fails on 0.8.0 and 0.10.0, passes on 0.12.3 | passes on 0.8.0, 0.10.0 and 0.12.3 (35 successes when added, 37 at the final head, 0 failures on each) | | the two cases "expands a leading ~ / $ even when Neovim's cwd holds a folder named ~ / $HOME", with `cwd.lua` from `ca9e61c0` (before the guard) | 35 successes, 2 failures on 0.8.0 and 0.12.3 | 37 successes, 0 failures on each | The spec makes real folders and runs `cwd.git_root` on them, in two modes. In the first mode git is off `PATH` (the shell keeps its absolute path), so `git` ends with "not found" and a list command raises E475, as on a host without git. In the second mode a real `git init` makes the repository. The folder names are: a space, a comma, a semicolon, single and double quotes, brackets and braces, parentheses, non-ASCII letters, a star and a question mark, a percent sign and a hash, a backslash, a dollar sign, a leading tilde, and a plain name. Each name runs from the repository folder and from a folder two levels below it, and the answer must equal the repository path with no trailing slash. A folder that is not a repository returns nil, whatever its name. Seven more cases cover a relative start folder, a start folder that ends in `..`, `cwd.resolve` with `git_repo_cwd`, a static `cwd` with brackets and a star in `cwd.resolve`, `~` and `$VAR` in a setting of `cwd.resolve`, and two cases where Neovim's current folder holds a folder named `~` or `$HOME` and the setting must still expand. The 17 red cases are 10 of the no-git names, 4 of the git names, the relative start, `resolve`, and the static `cwd`. Gates on the final head `f12368bd`, which contains a merge of `origin/main` (05b9785, with coder#551, coder#535, coder#494, coder#545, coder#553, coder#555 and coder#557) (private XDG folders). An independent verifier passed `c73ab044`. Since then the branch has three changes of its own and two merges with main. The changes are a docs commit (the README sentence about where the walk starts), the fix for text that starts with `~` or `$` (step 4, with two spec cases and the README clause), and the `covered.lock` record for it. For the lock I ran the full `check.sh`: the model passed (89205 distinct states, 5269 live states, every fact TRUE), and only the file hash of `cwd.lua` was stale, with no changed block or call site. - `mise run all`: native exit 0. luacheck: 0 warnings / 0 errors in 186 files. busted: 1756 successes / 0 failures / 0 errors / 29 pending. treefmt: 0 files changed. Since coder#545, `mise run all` also runs the static proofs of the `verify-claudecode-nvim` skill, and they pass. - `mise run format-check`: native exit 0 (428 files, 0 changed). - `scripts/run_integration_tests_individually.sh` on Neovim 0.12.3: 564 successes, 0 failed, 0 errors, native exit 0. On Neovim 0.8.0: 427 successes, 0 failed, 0 errors, native exit 0 (the 0.8.0 run skips the specs that need 0.9). The new spec gives 37 successes on both. It starts about 100 processes (git and a shell), so it takes about 6 s on a quiet host and can pass the runner's 40 s limit at a load average above 100. A TIMEOUT line there means the host was slow, not that the spec hung. - All four `.agents/skills/verify-*/check.sh --hash-only` match their `covered.lock`. The merge with `origin/main` had two parts to resolve in an earlier commit. `covered.lock` conflicted on the hash line of `cwd.lua`, so I ran the full `check.sh` and `check.sh --record`. coder#539's `tests/unit/cwd_spec.lua` had a test that stubbed `vim.fn.finddir`, which the fallback no longer calls, so that test now walks up real folders. It does not depend on where `TMPDIR` is: it passes with `TMPDIR` inside a git checkout too. `cwd.lua` itself merged without a text conflict. The two latest merges needed no `covered.lock` change: coder#551 conflicted only in `tests/mocks/vim.lua`, where the branch keeps both its `simplify` entry and coder#551's `wordcount`, and coder#535 and coder#494 merged without a conflict. An independent verifier found the `..` case, the README wording and the other points that this body now states. The `terminal.lua` expand and `.git` files in worktrees are gaps that main already has. They are follow-ups, not part of this PR. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_

ThomasK33 added 4 commits November 11, 2026 23:04

chatgpt-codex-connector Bot reviewed Nov 12, 2026

View reviewed changes

Comment thread .agents/skills/verify-claudecode-nvim/features/tools-call-wire.md Resolved

Comment thread .agents/skills/verify-claudecode-nvim/features/tools-call-wire.md Resolved

ThomasK33 added 3 commits November 12, 2026 08:27

ThomasK33 added this pull request to the merge queue Nov 12, 2026

Merged via the queue into main with commit f7f89cb Nov 12, 2026
7 checks passed

ThomasK33 deleted the skills-wire-probe-cleanup-order branch November 12, 2026 11:06

This was referenced Nov 12, 2026

fix(runner): name why a file failed when nvim exits non-zero without a reason #554

Merged

fix(cwd): find a repository whose folder name has a space, a comma or other path characters #561

Merged

Navigation Menu

Repository navigation

chore(skills): ignore later signals in the wire probe's cleanup and state its promises on the page - #555

chore(skills): ignore later signals in the wire probe's cleanup and state its promises on the page#555
ThomasK33 merged 7 commits into
mainfrom
skills-wire-probe-cleanup-order

ThomasK33 commented Nov 12, 2026 •

edited

Loading

Uh oh!

ThomasK33 commented Nov 12, 2026

Uh oh!

ThomasK33 commented Nov 12, 2026

Uh oh!

chatgpt-codex-connector Bot commented Nov 12, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot commented Nov 12, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

Uh oh!

ThomasK33 commented Nov 12, 2026

Uh oh!

ThomasK33 commented Nov 12, 2026

Uh oh!

chatgpt-codex-connector Bot commented Nov 12, 2026

Uh oh!

chatgpt-codex-connector Bot commented Nov 12, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

ThomasK33 commented Nov 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

What changed

How I proved it

Limits

Uh oh!

ThomasK33 commented Nov 12, 2026

Uh oh!

ThomasK33 commented Nov 12, 2026

Uh oh!

chatgpt-codex-connector Bot commented Nov 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector Bot commented Nov 12, 2026

🛡️ Codex Security Review

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

ThomasK33 commented Nov 12, 2026

Uh oh!

ThomasK33 commented Nov 12, 2026

Uh oh!

chatgpt-codex-connector Bot commented Nov 12, 2026

🛡️ Codex Security Review

Uh oh!

chatgpt-codex-connector Bot commented Nov 12, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

ThomasK33 commented Nov 12, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Nov 12, 2026 •

edited

Loading