Repository navigation
test: pin the termopen wrapper's default bindings and correct its notes - #597
Merged
Merged
Conversation
Review of #593 found that a wrapper whose defaults were broken still passed the spec when a TermOpen autocmd took 50 ms, and that the default clock was not pinned at all. Two new cases fix that without timing. One builds the wrapper with stand-ins for the clock and the pid only, and gives it Neovim's own pid, so the wait must end at the deadline of the fake clock: it reads /proc and binds readlink for real. The other replaces vim.fn.jobpid, vim.loop.hrtime and vim.loop.sleep and builds the wrapper with no stand-ins, so the wrapper must take its defaults from them. The helper comment and DEVELOPMENT.md now say that a job with the test Neovim's exact command line blocks termopen until it exits and raises at 10 s, and that a poll inside exec can return before exec has ended. SKILL.md and the proof script name tests/helpers/termopen_wait.lua as the wrapper's home. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
ThomasK33
marked this pull request as ready for review
November 13, 2026 03:24
Member
Author
|
@codex review |
Member
Author
|
@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 Review: Didn't find any major issues. Nice work! 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. |
This was referenced Nov 13, 2026
hareki
pushed a commit
to hareki/claudecode.nvim
that referenced
this pull request
Nov 14, 2026
## Why
The goal: `mise run format-check` must do what its description says.
`mise.toml` calls it "Verify formatting without writing (used in CI)".
It ran `treefmt --ci`, and treefmt has no dry run. `--ci` formats the
files in place and then fails because it changed them. So a failed check
rewrote the unformatted files.
I measured it on a clean copy of main. I made `CHANGELOG.md` and
`DEVELOPMENT.md` unformatted, ran `mise run format-check`, and looked at
the files afterwards:
| File | Hash before | Hash after the check |
|---|---|---|
| `CHANGELOG.md` | `1ffecd326a8afd7e` | `19b9e69803385db9` |
| `DEVELOPMENT.md` | `209225cc4d1b551f` | `b6041a87646a230a` |
The check exited 1 with "unexpected changes detected". It names each
file it changed (`ERRO file has changed path=CHANGELOG.md`), but it
shows no diff, and it has already rewritten the file. A developer who
ran it to look at a problem lost the unformatted text. The round b
verifier of the release runner work found the same thing on
`CHANGELOG.md`.
## What changed
1. `mise.toml`: the `format-check` task puts `scripts/format-check-bin/`
first on `PATH` and runs `treefmt --ci` in the real tree, with treefmt's
default walker. It reads the checkout path from `$MISE_CONFIG_ROOT`, not
from a `{{config_root}}` template: a template pastes the path into the
shell text, so a quote, `$` or backtick in the path changes what the
shell runs. Before treefmt runs, the task checks that `prettier`,
`stylua` and `shfmt` each resolve (`command -v`) to the stand-in. When
one does not, the task prints a message that names the tool and the
causes (a stand-in missing or not executable, or the folder not first on
`PATH`, for example because a colon in the checkout path splits the
entry), exits 2 and does not run treefmt. So a `PATH` that treefmt
cannot use stops the check instead of letting the real tool write. The
description stays, and it is true now.
2. `scripts/format-check-bin/` (new) holds `check-mode.sh` and three
symlinks to it, named `prettier`, `stylua` and `shfmt`. With the folder
first on `PATH`, treefmt finds the symlinks in place of the real tools.
treefmt runs a formatter with the options of `treefmt.toml`, a `--`, and
the file names. The script drops everything up to the first `--`, takes
its own folder off `PATH`, and runs the real tool in check mode:
`prettier --check`, `stylua --check` or `shfmt -i 2 -d`, each with a
`--` before the file names. The tool fails on a file that it would
change, prints that file (`stylua` and `shfmt` print the diff too), and
writes nothing. A run of the script in an environment that its own
earlier run set (no real tool on `PATH`, so it found itself) exits 2
instead of looping. An exported `FORMAT_CHECK_STAND_IN` has the same
effect on the stand-in: it exits 2, treefmt reports the failed
formatter, and `mise run format-check` fails with exit 1. The message
names the variable, and nothing is written.
3. `treefmt.toml` holds the real tools with their write flags, as on
main (`prettier --write`, `stylua`, `shfmt -i 2 -w`), and a `--` at the
end of the options of `prettier`, `stylua`, `shfmt` and `shellcheck`.
treefmt puts the paths after the options, and a tool reads a path that
starts with a dash as an option. The linters (`shellcheck`,
`actionlint`, `zizmor`) run as before. `actionlint` and `zizmor` only
get paths under `.github/workflows/`, which never start with a dash.
`treefmt.toml` also sets `tree-root-file = "treefmt.toml"`, so the tree
root is the folder of that file (see Blast Radius).
4. `scripts/test-format-check.sh` (new) runs the real task. It adds
untracked probes to the repo root: unformatted Markdown, Lua and shell,
a copy of each whose name starts with a dash (`-format-check-probe.md`,
`-Vformat-check-probe.lua`, `-format-check-probe.sh`), and
`-Pformat-check-probe.sh`. The check must fail, name each unformatted
probe, and change nothing in the working tree: the test compares `git
status --porcelain -uall --ignored=matching` and the hash of every
changed or untracked file before and after (it sees a new ignored file
or folder, and not a change to an ignored file or folder that exists
already). `mise run format -- <the probes>` must write the formatted
text. The check must pass on the formatted probes, and it must fail on a
shell probe that `shfmt` keeps and `shellcheck` rejects, and on
`-Pformat-check-probe.sh` when that is the only probe that `shellcheck`
rejects. In a vendored copy of the repo in a temporary folder, the check
must pass on a formatted probe and fail and name an unformatted one.
Then the test simulates a version bump of `stylua` (the real `stylua`,
and the real one with another quote style), in two ways: under mise's
environment and outside it, with treefmt's cache on. It runs version 1,
version 1 again (treefmt must format 0 files: the cache has the probe),
and version 2 (treefmt must rewrite the probe). Last, it makes eight
small copies of the checkout (the task, the tool settings and the
stand-ins, with one Markdown probe). In five of them the path holds a
`"`, a `'`, a `$HOME`, a backtick or a colon: the check must exit 0 on a
formatted probe, and exit 1 on an unformatted one without changing a
file (in the colon copy it must exit 2 both times and change nothing).
In three of them the symlink of one tool is missing: the check must exit
2 and name that tool. The probes are created with `noclobber`, so a
second run of the test in the same checkout exits 2 at once, and they
are removed on exit.
5. `.github/workflows/test.yml`: the `unit-tests` job runs
`scripts/test-format-check.sh` after the format check.
6. `scripts/README.md`, and the comments in `treefmt.toml` and
`mise.toml`, describe the stand-ins and the test.
### Why this shape
This is the fourth design of the check, and each earlier one drew
findings in the same place.
- The first two formatted a copy of the tree and compared it with the
tree. The verifiers found seven layouts where the copy gave another
verdict than `treefmt --ci` (a symlink that does not resolve in the
copy, a nested git repository, a Prettier configuration above the
checkout, a read-only `.git`, an ignored formatter config, an unreadable
file that no formatter reads, a git directory outside the checkout's
folder chain). Every one came from the copy.
- The third put the three writing formatters behind one wrapper script
that picked write flags or check flags. treefmt keys its cache on the
size and modification time of a formatter's command, and that command
was the wrapper, not the tool. After a version bump of a tool, `mise run
format` wrote nothing. The fix (no cache at all) made every `mise run
format` format all files (26 to 28 s, where main took about 1 s), and a
plain `treefmt` outside mise kept the stale cache.
- This one touches the normal path as little as possible. `mise run
format`, a plain `treefmt`, the `.claude` format hook and editors run
the real tools, exactly as on main, so treefmt's cache keys on the real
tools. Only `mise run format-check` puts stand-ins on `PATH`, and the
stand-ins hand the real tool its check flags.
The price: the write flags live in `treefmt.toml` and the check flags in
`check-mode.sh` (two places), the repo has three symlinks, and the
stand-ins work only through `mise run format-check`. A bare `treefmt
--ci` rewrites files, as on main. The task fails closed: when treefmt
cannot reach the stand-ins, `mise run format-check` exits 2 and writes
nothing.
## Scope
Only the check task, the stand-ins, the `--` in four formatter options,
the test and the CI step. `mise run format` and `mise run all` run the
same commands as on main, with a `--` before the file names. No Lua file
changed.
## Blast Radius
Developers and CI: `mise run format-check` is `treefmt --ci`, so it
takes as long as it did before (about 27 s on this host for 443 files).
When it fails, the output is the formatters' own: Prettier prints
`[warn] <file>`, stylua prints `Diff in <file>:` and a diff, and shfmt
prints a diff. The old output named the files too (`ERRO file has
changed path=...`), but only after it had rewritten them. A developer
who wants the formatted files runs `mise run format`. The `unit-tests`
job gains the test step (about 100 to 130 s on this host: four
whole-tree checks, one `treefmt` run on six files, a vendored check and
a bump step in two ways).
`mise run format` and a plain `treefmt` use treefmt's cache as they do
on main. A warm `mise run format` takes 0.26 to 0.29 s (main 0.24 to
0.28 s). The `treefmt` run inside it takes 24 to 48 ms here and 34 to 48
ms on main. A cold run takes 23 s (main 26 s). A version bump of
`stylua`, `prettier` or `shfmt` makes treefmt format the files again,
with mise's environment and without it (see Verification).
Where this check and `treefmt --ci` on main give different verdicts,
main is wrong in the first two cases below. In the last two, this check
refuses to run or fails:
- A vendored copy. A checkout with no `.git` of its own that lies inside
another git repository gets the outer repository as its treefmt tree
root. Main's check then anchors the `fixtures/*/lazy-lock.json` exclude
at the outer root, so the exclude does not match, and treefmt formats
the seven lock files (they are not formatted the way Prettier writes
them) and fails. That failure is a false positive. `treefmt.toml` sets
`tree-root-file = "treefmt.toml"`, so the tree root is the folder of
that file. The vendored copy with the lock files passes here and fails
on main. The vendored copy without the lock files passes on both. In
every other layout the folder of `treefmt.toml` is the root of the git
repository, as before.
- A file whose name starts with a dash. Main hands such a name to the
tools as an option. Prettier skips an unformatted `-probe.md` or
`-dir/x.md`, so main passes it. stylua reads `-V.lua` or `-h.lua` as
`--version` or `--help`, prints that, exits 0, and skips every Lua file
of its batch, so main passes an unformatted `lua/claudecode/logger.lua`
next to a `-V.lua`. stylua and shfmt fail on a formatted `-ok.lua` or
`-ok.sh`, so main fails a formatted tree. The `--` before the file names
(and the `shellcheck` options ending with `--`) make this check treat
such a file like any other. A `-Pprobe.sh` with a shellcheck error fails
on both: on main only because shfmt rejects the name, and here because
shellcheck reports SC2086.
- A checkout path that contains a colon. The colon splits the `PATH`
entry of the stand-ins, so the task exits 2 before treefmt runs, and
main's check runs and gives a verdict. A path with a `"`, a `$HOME` or a
backtick works as on main. (The first version of this PR pasted the path
into the shell text: a quote made it exit 2 on a formatted tree, and `$`
or a backtick made treefmt find the real tools, so the check wrote
files. An independent verifier found that. The task reads
`$MISE_CONFIG_ROOT` now.)
- An exported `FORMAT_CHECK_STAND_IN`. The stand-ins use it to detect a
second run, so with it set `mise run format-check` fails with exit 1
(the stand-in exits 2, and treefmt reports the failed formatter), on a
clean tree too. The message names the variable, and nothing is written.
Nothing sets it outside the stand-ins.
Limits:
- An unreadable tracked folder (`chmod 000`) makes `treefmt` hang, on
main and here, with or without the stand-ins, and treefmt ignores
SIGTERM. That is treefmt's own behavior, and this PR does not work
around it.
- A checkout path that contains a colon cannot run `mise run
format-check`: the task stops with exit 2 (see above). `mise run format`
is not affected.
- Not covered by the test: a failure of `actionlint` or `zizmor` (only
`shellcheck` is covered). The differential run below covers `actionlint`
(`actionlint_fail`) and `shellcheck`, not `zizmor`. macOS and bash 3.2
were not run.
## Declined
- A test that fails when `treefmt.toml` gains a formatter that writes
and has no check-mode stand-in (an advisor's hardening idea). Only
prettier, stylua and shfmt write today, and each has a stand-in. A new
writing formatter would make `format-check` rewrite its files again.
Declined for this PR because it guards a future change, not a defect in
this one; the stand-in folder and `scripts/README.md` name the three
tools, and a change that adds a formatter goes through the same review.
## Verification
**Red on the old task, green here.** The new test, run against main's
`mise run format-check` (`treefmt --ci`), fails with `format-check
changed the working tree` and the changed hashes (exit 1). Against this
head it prints `test-format-check: ok` (about 100 to 130 s on this
host).
**The version bump.** `mise.toml` points `stylua` at a script for
"version 1" (the real stylua), a formatting run follows, `mise.toml`
then points at "version 2" (the real stylua with `--quote-style
ForceSingle`), and the formatting runs again. Numbers are the files that
`treefmt` reported, and the files that it changed:
| How treefmt runs | main | the previous head `b3110b2c` | this head |
|---|---|---|---|
| `mise run format`: version 1, then version 2 | 441 files, then 326
(323 changed), then 0 | 443 files, then 443 (323 changed), then 443 (no
cache) | 443 files, then 326 (323 changed), then 0 |
| a plain `treefmt` outside mise (`env -i`, the tools on `PATH`):
version 1, version 1 again, version 2, version 2 again | 441, 0, 326
(323 changed), 0 | 443, 0, **0** (the stale cache), 0 | 443, 0, 326 (323
changed), 0 |
On this head the numbers equal main's. (I measured the numbers at
`749e925d`. The later commits changed only the shell line of the task
and the test, and step 6 of the test repeats the bump on this head.) A
straight run of the new test on `b3110b2c` stops earlier, at the
vendored-copy step, because that head has no `scripts/format-check-bin`
to copy. So I ran step 6 of the test (the version bump) alone against
`b3110b2c`. Inside mise it fails with "treefmt did not find the probe in
its cache on a second run (inside mise)", and outside mise it fails with
"the second stylua version writes (outside mise: treefmt used its cache
after a version bump)".
**Odd checkout paths.** Full clones of the head at paths with a `"`, a
`'`, a `$HOME`, a backtick and a colon, each with an unformatted
`probe.md` and `probe.lua` in the root, and `mise run format-check` in
each (the folder names are `q"uote`, `s'quote`, `d$HOME`, ``b`true`b``
and `c:olon`). The first version of the task pasted `{{config_root}}`
into the shell text. An independent verifier found that this broke it,
and I reproduced it on `749e925d`:
| Checkout path | `749e925d` | this head |
|---|---|---|
| `q"uote` | exit 2, `sh: Syntax error: Unterminated quoted string`,
probes unchanged | exit 1, probes unchanged |
| `s'quote` | not run on `749e925d` | exit 1, probes unchanged |
| `d$HOME` | exit 1, **both probes rewritten** | exit 1, probes
unchanged |
| ``b`true`b`` | exit 1, **both probes rewritten** | exit 1, probes
unchanged |
| `c:olon` | exit 1, **both probes rewritten** | **exit 2**, "prettier
does not resolve to the stand-in in .../c:olon/scripts/format-check-bin,
so treefmt would run another prettier and write files. A stand-in is
missing or not executable, or the folder is not first on PATH (a colon
in the path of the checkout splits the entry).", probes unchanged |
| a plain path | exit 1, probes unchanged | exit 1, probes unchanged |
The test covers the same five paths in small copies of the checkout, so
it does not format the whole tree five more times, and three copies that
miss one stand-in.
**Mutants against the test**, each in its own copy of this head (the
first message that the test prints):
| Mutant | Test result | First failing check |
|---|---|---|
| a stand-in runs `prettier --write` (the check writes) | red |
"format-check changed the working tree" |
| the `stylua` stand-in without `--check` | red | "format-check changed
the working tree" |
| the `shfmt` stand-in without `-d` | red | "format-check does not name
format-check-probe.sh" |
| the `prettier` stand-in without `--check` | red | "format-check does
not name format-check-probe.md" |
| a stand-in without `--` before the file names (each of the three) |
red | "format-check does not name ..." the probe of that tool |
| the stand-in does not drop treefmt's options | red | "format-check
failed the formatted probes" |
| the stand-in does not take its folder off `PATH` | red | "format-check
does not name format-check-probe.md" (it exits 2, it does not loop) |
| `mise.toml` without the `PATH` prefix | red | "format-check does not
name format-check-probe.md" (the guard exits 2) |
| the stand-ins last on `PATH` | red | "format-check does not name
format-check-probe.md" (the guard exits 2) |
| the `PATH` prefix names a folder that does not exist | red |
"format-check does not name format-check-probe.md" (the guard exits 2) |
| no symlink for `prettier`, `stylua` or `shfmt` (each) | red |
"format-check does not name format-check-probe.md" (the guard exits 2) |
| the task builds the stand-in folder from `{{config_root}}` again (the
first version of this PR) | red | "format-check exited 2 on a formatted
probe in .../q"uote" |
| the task builds the stand-in folder from `'{{config_root}}'` (single
quotes: only the `s'quote` copy breaks it) | red | "format-check exited
2 on a formatted probe in .../s'quote" |
| the task has no guard | red | "format-check exited 0 on a formatted
probe in .../c:olon (expected 2)" |
| the guard prints its message and does not exit | red | "format-check
exited 0 on a formatted probe in .../c:olon (expected 2)" |
| the guard exits 1 | red | "format-check exited 1 on a formatted probe
in .../c:olon (expected 2)" |
| the guard checks only that the tool exists, not that it is the
stand-in | red | "format-check exited 0 on a formatted probe in
.../c:olon (expected 2)" |
| the guard skips `prettier`, `stylua` or `shfmt` (each) | red |
"format-check exited 0 in a copy without the stand-in of <tool>
(expected 2)" |
| `mise run format` goes through the stand-ins | red | "mise run format
failed" |
| `treefmt.toml` without `--` in the options of `prettier`, `stylua` or
`shfmt` (each) | red | "format-check does not name ..." the probe of
that tool |
| `treefmt.toml` without `--` in the `shellcheck` options | red | "mise
run format failed" |
| `shfmt` write flags without `-i 2` | red | "format-check-probe.sh does
not hold the text that shfmt -i 2 writes" |
| `prettier` write options without `--write` | red |
"format-check-probe.md does not hold the text that prettier writes" |
| `treefmt.toml` without `tree-root-file` | red | "format-check failed
in a vendored copy" |
| `TREEFMT_NO_CACHE=1` back in `[env]` (the cost of the previous design)
| red | "treefmt did not find the probe in its cache on a second run
(inside mise)" |
One mutant survives: the task takes the checkout folder from `$PWD`
instead of `$MISE_CONFIG_ROOT`. It is equivalent only while mise runs
the task in the folder of its `mise.toml`, which is the default (I
checked it from a subfolder). With `[task_config] dir = "{{cwd}}"` in a
`mise.local.toml`, mise runs the task in the caller's folder, and this
mutant then exits 2 from a subfolder (the guard fails closed, so it
writes nothing) while the head passes. The test does not set that
option.
Two runs of the test at once in one checkout: the second exits 2 with
"format-check-probe.md exists already (another run of this test, or a
run that was killed)", and the first passes.
**Differential run against `treefmt --ci` on main: 107 cases.** The
cases are the 101 of my run against the previous head `749e925d` (the 90
of the round c verifier and the 11 dash cases of the round d verifier)
and six new cases for checkout paths with a `"`, a `$HOME`, a backtick
and a colon. For each case a harness copies a clone of this branch (and
of main for the old check), makes the change, and runs `mise run
format-check` on both. It snapshots every path (type, mode, size, times
and sha256 for the tree), the git directory and the common directory,
and every file that a case links to outside the tree, before and after.
I ran the 107 cases against `9be9f1f6` (the head with the path fix and
the guard) and ran main's side of the six new cases. The other 101 main
results come from the earlier run (main did not change in the files of
this PR).
- 96 cases give main's exit status. 11 give a different verdict. Nine
are main's mistakes (see Blast Radius): `vendored_subdir`, and the dash
cases `dash_name`, `dash_dir_md`, `dashdash_only_md`, `dash_V_lua`,
`dash_V_lua_plus_lua_tracked` and `dash_h_lua_plus_lua_tracked` (main
passes, this head fails an unformatted file), and `dash_lua_formatted`
and `dash_sh_formatted` (main fails a formatted file, this head passes).
Two are this check refusing to run: `path_colon_md` and
`path_colon_clean` (a colon in the checkout path: exit 2 here, and
main's check runs).
- In `mode000_dir_tracked`, `treefmt --ci` hangs on both (see Limits). I
killed both, so the exit status is the signal's.
- The new check writes nothing to the tree, the git directory, or
outside the tree in any of the 107 cases, and it leaves no treefmt cache
file. It leaves no folder in `TMPDIR`, except in `mode000_dir_tracked`,
where I killed treefmt myself.
- The three path cases without a colon (`"`, `$HOME`, backtick) give
main's exit status, and the check names the same file that main's check
rewrote.
- In the 42 cases where main's check rewrote files, this check names the
same files in 34. The other eight: four harness parsing artifacts (a
newline or a tab in a name, a directory's modification time in
`sh_tracked` and `shfmt_plus_shellcheck`), the two vendored cases where
main also rewrote the lock files, `dash_name_and_md_tracked`, where this
check also names the unformatted `-probe.md` that main skips, and
`path_colon_md`, where this check stops before it names a file.
<details>
<summary>All 107 cases</summary>
| Case | `treefmt --ci` on main | `format-check` | New check writes |
Old check rewrote |
|---|---|---|---|---|
| `actionlint_fail` | exit 1 | exit 1 | nothing | no |
| `clean` | exit 0 | exit 0 | nothing | no |
| `cr_line_endings` | exit 1 | exit 1 | nothing | yes |
| `crlf_tracked` | exit 1 | exit 1 | nothing | yes |
| `crlf_tracked_formatted_attr` | exit 1 | exit 1 | nothing | yes |
| `dash_P_sh_lintbad` | exit 1 | exit 1 | nothing | no |
| `dash_V_lua` | exit 0 | exit 1 **(different verdict)** | nothing | no
|
| `dash_V_lua_plus_lua_tracked` | exit 0 | exit 1 **(different
verdict)** | nothing | no |
| `dash_dashdash_md` | exit 1 | exit 1 | nothing | no |
| `dash_dir_md` | exit 0 | exit 1 **(different verdict)** | nothing | no
|
| `dash_e_sh_lintbad` | exit 1 | exit 1 | nothing | no |
| `dash_filename_sh` | exit 1 | exit 1 | nothing | no |
| `dash_h_lua_plus_lua_tracked` | exit 0 | exit 1 **(different
verdict)** | nothing | no |
| `dash_lua` | exit 1 | exit 1 | nothing | no |
| `dash_lua_formatted` | exit 1 | exit 0 **(different verdict)** |
nothing | no |
| `dash_md_formatted` | exit 0 | exit 0 | nothing | no |
| `dash_name` | exit 0 | exit 1 **(different verdict)** | nothing | no |
| `dash_name_and_md_tracked` | exit 1 | exit 1 | nothing | yes |
| `dash_sh` | exit 1 | exit 1 | nothing | no |
| `dash_sh_formatted` | exit 1 | exit 0 **(different verdict)** |
nothing | no |
| `dashdash_only_md` | exit 0 | exit 1 **(different verdict)** | nothing
| no |
| `deleted_plus_md` | exit 1 | exit 1 | nothing | yes |
| `deleted_tracked` | exit 0 | exit 0 | nothing | no |
| `excluded_dir_tracked` | exit 0 | exit 0 | nothing | no |
| `exec_bit` | exit 0 | exit 0 | nothing | no |
| `fifo_untracked` | exit 0 | exit 0 | nothing | no |
| `formatter_missing` | exit 1 | exit 1 | nothing | no |
| `formatter_tool_missing` | exit 1 | exit 1 | nothing | no |
| `gitdir_readonly` | exit 0 | exit 0 | nothing | no |
| `glob_name` | exit 1 | exit 1 | nothing | yes |
| `ignored_but_tracked` | exit 0 | exit 0 | nothing | no |
| `ignored_but_tracked_lua` | exit 1 | exit 1 | nothing | yes |
| `ignored_but_tracked_luacov` | exit 1 | exit 1 | nothing | yes |
| `ignored_editorconfig` | exit 1 | exit 1 | nothing | yes |
| `ignored_prettierignore` | exit 0 | exit 0 | nothing | no |
| `ignored_prettierrc_root` | exit 1 | exit 1 | nothing | yes |
| `ignored_prettierrc_subdir` | exit 1 | exit 1 | nothing | yes |
| `ignored_unformatted` | exit 0 | exit 0 | nothing | no |
| `intent_to_add` | exit 1 | exit 1 | nothing | yes |
| `json_syntax_error` | exit 1 | exit 1 | nothing | no |
| `large_tree` | exit 1 | exit 1 | nothing | yes |
| `leading_space_name` | exit 1 | exit 1 | nothing | yes |
| `linked_worktree` | exit 1 | exit 1 | nothing | yes |
| `linked_worktree_clean` | exit 0 | exit 0 | nothing | no |
| `linked_worktree_parent_prettierrc` | exit 1 | exit 1 | nothing | yes
|
| `linked_worktree_untracked` | exit 1 | exit 1 | nothing | yes |
| `linter_missing` | exit 1 | exit 1 | nothing | no |
| `lua_syntax_error` | exit 1 | exit 1 | nothing | no |
| `lua_tracked` | exit 1 | exit 1 | nothing | yes |
| `md_tracked` | exit 1 | exit 1 | nothing | yes |
| `mode000_dir_tracked` | exit 137 | exit 137 | nothing | no |
| `mode000_dir_untracked` | exit 0 | exit 0 | nothing | no |
| `mode000_file_md_unformatted` | exit 1 | exit 1 | nothing | no |
| `mode000_tracked_file` | exit 0 | exit 0 | nothing | no |
| `nested_repo_untracked` | exit 0 | exit 0 | nothing | no |
| `nested_repo_untracked_clean` | exit 0 | exit 0 | nothing | no |
| `newline_name` | exit 1 | exit 1 | nothing | yes |
| `no_git` | exit 0 | exit 0 | nothing | no |
| `parent_prettierrc` | exit 1 | exit 1 | nothing | yes |
| `prettier_excluded` | exit 0 | exit 0 | nothing | no |
| `prettierignore_tracked` | exit 0 | exit 0 | nothing | no |
| `readonly_unformatted` | exit 1 | exit 1 | nothing | no |
| `separate_gitdir_clean` | exit 0 | exit 0 | nothing | no |
| `separate_gitdir_parent_prettierrc` | exit 1 | exit 1 | nothing | yes
|
| `separate_gitdir_prettierrc_above_gitdir` | exit 0 | exit 0 | nothing
| no |
| `sh_tracked` | exit 1 | exit 1 | nothing | yes |
| `shellcheck_tracked` | exit 1 | exit 1 | nothing | no |
| `shellcheck_untracked` | exit 1 | exit 1 | nothing | no |
| `shfmt_plus_shellcheck` | exit 1 | exit 1 | nothing | yes |
| `space_tracked` | exit 1 | exit 1 | nothing | yes |
| `space_untracked` | exit 1 | exit 1 | nothing | yes |
| `sparse_cone` | exit 1 | exit 1 | nothing | yes |
| `sparse_cone_clean` | exit 0 | exit 0 | nothing | no |
| `sparse_index` | exit 0 | exit 0 | nothing | no |
| `sparse_nocone` | exit 1 | exit 1 | nothing | yes |
| `subdir_cwd` | exit 1 | exit 1 | nothing | yes |
| `submodule_checkout_parent_prettierrc` | exit 1 | exit 1 | nothing |
yes |
| `submodule_gitlink` | exit 0 | exit 0 | nothing | no |
| `symlink_abs_outside` | exit 0 | exit 0 | nothing | no |
| `symlink_dangling` | exit 0 | exit 0 | nothing | no |
| `symlink_dangling_tracked` | exit 0 | exit 0 | nothing | no |
| `symlink_dir` | exit 0 | exit 0 | nothing | no |
| `symlink_in_clean` | exit 0 | exit 0 | nothing | no |
| `symlink_rel_outside` | exit 0 | exit 0 | nothing | no |
| `symlink_to_unformatted` | exit 0 | exit 0 | nothing | no |
| `symlink_tracked_to_unformatted` | exit 0 | exit 0 | nothing | no |
| `symlinked_cwd` | exit 1 | exit 1 | nothing | yes |
| `tracked_dir_replaced_by_symlink` | exit 1 | exit 1 | nothing | no |
| `tracked_replaced_by_dir` | exit 0 | exit 0 | nothing | no |
| `tracked_replaced_by_fifo` | exit 0 | exit 0 | nothing | no |
| `tracked_replaced_by_symlink` | exit 0 | exit 0 | nothing | no |
| `tracked_symlink_replaced_by_file` | exit 1 | exit 1 | nothing | yes |
| `trailing_space_name` | exit 0 | exit 0 | nothing | no |
| `unicode_tab_names` | exit 1 | exit 1 | nothing | yes |
| `unreadable` | exit 1 | exit 1 | nothing | no |
| `untracked_md` | exit 1 | exit 1 | nothing | yes |
| `vendored_filesystem_no_git` | exit 1 | exit 1 | nothing | yes |
| `vendored_subdir` | exit 1 | exit 0 **(different verdict)** | nothing
| yes |
| `vendored_subdir_no_locks` | exit 0 | exit 0 | nothing | no |
| `vendored_subdir_untracked_unformatted` | exit 1 | exit 1 | nothing |
yes |
| `workflow_unformatted` | exit 1 | exit 1 | nothing | yes |
| `path_quote_md` | exit 1 | exit 1 | nothing | yes |
| `path_dollar_md` | exit 1 | exit 1 | nothing | yes |
| `path_backtick_md` | exit 1 | exit 1 | nothing | yes |
| `path_colon_md` | exit 1 | exit 2 **(different verdict)** | nothing |
yes |
| `path_colon_clean` | exit 0 | exit 2 **(different verdict)** | nothing
| no |
| `path_quote_clean` | exit 0 | exit 0 | nothing | no |
</details>
Gates (exit codes) at 7653e42, which merges main at d05b010. Main has
since moved to bcf1f08 (coder#597, tests only): the merge is clean (`git
merge-tree`), and I did not run the other gates on it.
| Gate | Exit |
|---|---|
| `mise run format-check` | 0 (446 files, 0 changed) |
| `scripts/test-format-check.sh` | 0 (`test-format-check: ok`, 138 s at
a high load) |
| `mise run all` | 0 (0 warnings and 0 errors in 200 files, 1904
successes, 0 failures, 0 errors, 36 pending) |
| `scripts/run_integration_tests_individually.sh`, Neovim stable
(0.12.3) | 0 (854 successes, 0 failures, 0 errors) |
| `scripts/run_integration_tests_individually.sh`, Neovim v0.8.0 | 0
(850 successes, 0 failures, 0 errors) |
| `scripts/run_integration_tests_individually.sh` under a `TMPDIR` with
a space and a `%` (stable) | 0 (854 successes) |
| `git merge-tree --write-tree HEAD origin/main` (main at bcf1f08) | 0,
clean |
The commits after 9be9f1f change only the text of the guard's message,
the docs, the test (one more path) and the merge of main. So the
differential run and the version-bump numbers above are from earlier
heads, and the test repeats the bump step on this head.
---
_Generated with [`xum`](https://github.com/coder/xum) • Model:
`anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
hareki
pushed a commit
to hareki/claudecode.nvim
that referenced
this pull request
Nov 14, 2026
…the owned stop (coder#602) ## Why This is a follow-up to coder#593. It fixes the Snacks close and the stop in `owned.lua`, the two close paths that coder#593 left open. A job that was just started can still be a fork of Neovim until its child has run `exec`. A `jobstop` on it then ends Neovim with `Caught deadly signal 'SIGHUP'`, or the buffer delete after it hangs. coder#593 documents the race, and coder#544 made the native close wait for the exec of a young job. The Snacks close and the stop in `owned.lua` still called `jobstop` at once, so a close within milliseconds of the start could kill Neovim. ## Cause `pty_process_spawn` forks the child, and `init_child()` runs `setsid()` and resets the signal handlers only after that. Until `exec` the child has Neovim's libuv handlers. When the pty master closes in that gap (a `jobstop`), the kernel sends SIGHUP to the child, libuv's handler in the child writes into the signal pipe that it shares with Neovim, and Neovim reads a SIGHUP of its own and exits. With the child held in the gap (see Evidence), a close right after the start killed Neovim in every run for the Snacks provider and for the stop in `owned.lua`, on 0.8.0 and 0.12.3. The native provider survived in every run. ## Fix The wait moves out of `native.lua` into `owned.wait_for_exec(jobid)`, with `owned.is_young(started_at)` for the 10 s rule. The code is the same: it reads the names of Neovim and of the child from `/proc/<pid>/comm` (or `nvim_get_proc` where `/proc` is missing) and waits up to 1 s until the child has another name. - `native.close` calls it for a job younger than 10 s, as before. - `snacks.M.open` records `term._cc.job_started_at` where it already patches the instance. `snacks.close` reads it and calls the wait for a job younger than 10 s. A terminal that a reload adopted has no start time and is not waited for, as in native. Without the age rule, every Snacks close on macOS would pay about 200 ms for the two `ps` calls of the `nvim_get_proc` fallback. - `owned.lua` `stop_buffer` has no start time, so it calls it for every job. That path runs only after a reload or a provider switch, and the wait ends at the first look for a job that has run `exec`. The wait runs scheduled callbacks while the job still runs. Two Codex findings on `ea0fbfd6` were about that, and `12f1d45f` fixes both: - `snacks.close` now clears the owned mark before the wait for every terminal, not only for an adopted one. An `open()` among the callbacks can no longer adopt the terminal that is closing by its mark. - `stop_buffer` reads the channel before the wait and checks `nvim_buf_is_valid(buf)` before `jobstop`. A callback that deleted the buffer during the wait no longer makes `jobstop` raise `Invalid buffer id`. `lua/claudecode/terminal/README.md` and the `verify-terminal-lifecycle` skill describe the three places. The skill changed as follows: - The fact `OwnedStopOtherStops` reads `M.wait_for_exec(channel)` and the buffer check before `jobstop`. - The new fact `SnacksCloseClearsMark` reads the mark clear before the wait. The model has a switch of the same name in `CloseBegin`. The model counts a closing terminal as ended, so the fact breaks a rule only together with `OwnedLiveSkipsEndedJob`. `check.sh --mutants` has a row for the pair. - `OwnedLiveSkipsEndedJob` alone now breaks `OpenShowsLiveClaude`, not `CloseNeverReuses`, because the model clears the mark in every Snacks close. - The model still passes with 279492 distinct states. ## CI on `ea0fbfd6` and the fixes Two checks failed on `ea0fbfd6`. 1. The integration spec compiled its shim into a TMPDIR folder. CI's second pass runs with `TMPDIR="$RUNNER_TEMP/odd tmp 50%"`. `LD_PRELOAD` splits its value at a space or a colon, so the shim was ignored, the child was never held, and the probe timed out. `f910015b` puts the shim into `mktemp -d /tmp/claudecode.XXXXXX` when the TMPDIR path holds a space or a colon. 2. `verify-diff-lifecycle` also hashes `terminal/{native,owned,snacks}.lua`, so its `covered.lock` was stale. `3a1961a0` re-records the locks of `verify-terminal-lifecycle` and `verify-diff-lifecycle`. ## Tests - `tests/integration/terminal_close_young_job_spec.lua` runs one Neovim per case with `-u NONE` (so `minimal_init.lua` does not wait for the exec and hide the race) and `tests/helpers/hold_pty_child.c` preloaded. The shim holds the child inside `init_child()` and creates a file when it does. The probe (`tests/helpers/close_young_job_probe.lua`) starts a Claude terminal job, waits for that file, and closes the job. Cases: a plain `jobstop` as the control (it must end Neovim with SIGHUP, so the other cases can fail), the native close, the Snacks close (with a stand-in for Snacks) and the `owned.lua` stop. The spec is pending without Linux, a C compiler and `timeout`. It puts the shim into a `/tmp` folder when TMPDIR holds a space or a colon, and `tests/helpers/ld_preload_path.lua` makes that choice. - `tests/unit/ld_preload_path_spec.lua` has three cases: a plain path, a path with a space and a path with a colon. Before it, a spec pattern without the colon half passed in every CI configuration. - `tests/unit/terminal/owned_wait_for_exec_spec.lua` covers the wait (names from `/proc`, from `nvim_get_proc`, the 1 s limit, a job with no pid, unreadable names), `is_young`, the order of the stop in `owned.lua` (wait, then `jobstop`, then delete) and "leaves a buffer alone that a callback deleted during the wait". - `tests/unit/terminal/snacks_close_spec.lua` covers the Snacks close: it waits before `jobstop` for a job that just started, not for a job older than 10 s, and not for a job that exited. The case "does not let an open during the wait adopt the terminal that is closing" covers the mark clear. - Five Snacks unit specs gain `vim.loop.hrtime` in their vim mock, and one gains `vim.fn.getpid`, because the provider now reads a clock and the wait reads the pid. - `proof-pty-child-hangup.sh` compiles the same shim instead of a copy of it. Under a TMPDIR with a percent sign it failed all three cases, on `main` too. With only a space or only a colon it failed one of three, and its wrapper case passed only because the shim was not loaded. Now `d6019d13` puts its work folder into `/tmp` for such a path. - The header of `hold_pty_child.c` now says that the shim holds every fork of Neovim that calls `signal(SIGCHLD, SIG_DFL)`, including the `ps` children that `nvim_get_proc` starts, not only the pty child. ## Evidence Builds: the official `v0.8.0` release binary (`NVIM v0.8.0-1210-gd367ed9b2`, as in CI) and Neovim 0.12.3, scratch HOME and XDG dirs, on a shared host. | Run | main plus the new tests | this branch | |---|---|---| | integration spec, 10 runs per build | 10 of 10 fail on both builds (the Snacks and `owned.lua` cases fail, the control and native pass) | 0 of 10 fail on both builds | | integration spec, 20 runs per build, plain TMPDIR | not run | 0 of 20 fail on 0.8.0 and on 0.12.3 | | integration spec, 20 runs per build, `TMPDIR="odd tmp 50%"` | the spec of `ea0fbfd6` times out in this TMPDIR, as in CI | 0 of 20 fail on 0.8.0 and on 0.12.3 | | the unit specs of `tests/unit/terminal` plus `native_terminal_toggle_spec`, 20 runs | not run | 0 of 20 fail (374 successes each) | Mutants of the Lua files, one run each, on `89e74e31`. The merge of main changed no terminal file and no terminal spec, so I did not rerun them on the merge commit. "Unit" counts the failing tests among 374 in `tests/unit/terminal` plus `native_terminal_toggle_spec`. "Integration" is the new spec on 0.8.0 and 0.12.3, with the same result on both. "Fact" is the `verify-terminal-lifecycle` fact that turns FALSE in a facts-only run of `check.sh`. | Mutant | Unit | Integration | Fact | |---|---|---|---| | Snacks close does not wait | 2 failures | red | `SnacksCloseClearsMark` | | Snacks close always waits (no age rule) | 1 failure | green (the wait is harmless but slow) | none | | Snacks close never waits (age rule always false) | 2 failures | red | none | | Snacks does not record the start time | 2 failures | red | none | | Snacks clears the mark only for an adopted terminal (the form on `ea0fbfd6`) | 1 error (the open adopts the closing terminal) | green | `SnacksCloseClearsMark` | | Snacks clears the mark after the wait | 1 error (same case) | green | `SnacksCloseClearsMark` | | `stop_buffer` does not wait | 2 failures | red | `OwnedStopOtherStops` | | `stop_buffer` has no `nvim_buf_is_valid(buf)` check before `jobstop` | 1 failure | green | `OwnedStopOtherStops` | | `stop_buffer` re-reads the channel with no check (the form on `ea0fbfd6`) | 1 failure | green | `OwnedStopOtherStops` | | native close does not wait | 3 failures | red | none | | native close never counts a job as young | 3 failures | red | none | | `wait_for_exec` does not call `vim.wait` | 6 failures | red | none | | `wait_for_exec` stops while the names are equal | 4 failures | red | none | | `wait_for_exec` waits 0 ms | 4 failures | red | none | | `wait_for_exec` has no guard for an unreadable name of Neovim | 4 failures | green | none | | `wait_for_exec` calls `jobpid` without `pcall` (no guard for a job without a pid) | 1 failure and 2 errors | green | none | | `is_young` uses `<=` | 2 failures | green | none | | `is_young` reads a `nil` start time as young (no `nil` guard) | 1 failure | green | none | The control mutant (no change) is green in every column, with 374 successes. I ran the facts-only `check.sh` on every row, and "none" means that every fact stayed TRUE. A mutant of `tests/helpers/ld_preload_path.lua` that drops the colon, and one that drops the space, each fail one case of `ld_preload_path_spec.lua`. The model has its own mutants. `check.sh --mutants` removes each guard of the model in turn, and it ends with "every guard is caught" on the merge commit `191a3e24`. The pair `SnacksCloseClearsMark` with `OwnedLiveSkipsEndedJob` is caught by `CloseNeverReuses` and `OpenShowsLiveClaude`. - Gate: `MISE_LOCKFILE=false mise run clean && mise run all` with GNU timeout first on PATH exits 0 with 1925 successes, 0 failures, 0 errors and 38 pending, and luacheck reports 0 warnings in 206 files. The extra pending case is the new integration spec of coder#536, which needs a real Neovim. - `mise run format-check`: 464 files, 0 changed. - Integration runner, with a plain TMPDIR and with `TMPDIR="odd tmp 50%"`: 865 successes, 0 failures, 0 errors on 0.8.0, and 869, 0, 0 on 0.12.3. - `proof-pty-child-hangup.sh` passes 3 of 3 cases on 0.8.0 and on 0.12.3, with a plain TMPDIR, with `TMPDIR="odd tmp 50%"` and (on 0.12.3) with a TMPDIR that holds a colon. On `d6019d13`'s parent, with the odd TMPDIR, it failed 3 of 3 on both builds. - The five `check.sh` scripts (`verify-diff-lifecycle`, `verify-mention-queue`, `verify-selection-tracking`, `verify-terminal-lifecycle` and `verify-ws-connection`) all print PASS. The branch merges `origin/main` (coder#597, coder#594, coder#474, coder#596 and coder#536). coder#536 changed `diff.lua`, its README, `verify-diff-lifecycle` (SKILL.md, `check.sh` and `covered.lock`) and added `diff_answer_every_call_spec.lua`. Git merged the two edits of `verify-diff-lifecycle/covered.lock` with no conflict, and `check.sh` of all five skills prints PASS on the merge result, so no lock needed a new record after `3a1961a0`. - On the merge commit `191a3e24`, the specs this PR adds or changes pass 20 of 20 runs in a plain TMPDIR and in `TMPDIR="odd tmp 50%"` (24 successes for `owned_wait_for_exec_spec`, `snacks_close_spec` and `ld_preload_path_spec`, and 374 for the terminal unit specs plus `native_terminal_toggle_spec`). The integration spec of this PR passes 20 of 20 runs per build in both TMPDIRs. `diff_answer_every_call_spec` from coder#536 passes 10 of 10 runs per build in both TMPDIRs (9 successes each run). ## Not covered - The Snacks case uses a stand-in for Snacks, because CI installs no `snacks.nvim`. The provider code under test is the real one, and the stand-in starts a real pty job with `termopen`. - The wait reads `/proc` or `ps`. Where neither gives a name, the close goes on to `jobstop` and the race stays open, as in native before this PR. A `ps` that hangs would hang the close for as long as it hangs. I did not reproduce that. - I tested the `ps` fallback only on Linux, with `/proc` hidden from Lua. I did not run it on macOS or BSD. - A `jobstop` that other code issues, for example `:bwipeout` of the terminal buffer by the user right after the start, is not covered. - Another PR that edits `native.lua`, `snacks.lua`, `owned.lua` or a `covered.lock` must be merged one at a time with this one, and re-record the lock after its merge. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The verifier of #593 passed it and left four minor findings. This PR closes them. All four sit in the test wrapper that makes
termopenreturn after the job's child ran exec (tests/helpers/termopen_wait.lua), its spec, and the notes about them.What changed
termopenblocks until that job exits, and raises when the job lives 10 s. The verifier saw a 2 s job block for 2032 ms, and a 15 s job raise 13 of 16 times. A poll can also land inside exec, where the command line reads empty, and return before exec has ended (3 of 16). The helper comment andDEVELOPMENT.mdsay so.SKILL.mdand the proof script namedtests/minimal_init.luaas the wrapper's home. It lives intests/helpers/termopen_wait.lua, andminimal_init.luainstalls it.TermOpenautocmd, a wrapper that read nothing from/proc(the verifier's m09) or whose defaultjobpidraised (m10) passed the spec 5 of 5, because the child ran exec beforetermopenreturned. Two new cases do not depend on timing. One builds the wrapper with stand-ins for the clock and the pid only, and gives it Neovim's own pid, so the wait must end at the deadline of the fake clock. That reads/procand the defaultreadlinkfor real. The other replacesvim.fn.jobpid,vim.loop.hrtimeandvim.loop.sleep, and builds the wrapper with no stand-ins, so the wrapper must take its defaults from them.hrtimebinding, which gives the 10 s deadline of the installed wrapper. The verifier's m11 (a clock that always reads 0) passed the spec in 3 of 3 runs before (red 0 of 3).No file under
lua/orplugin/changes.Evidence
Builds: the official
v0.8.0release binary (NVIM v0.8.0-1210-gd367ed9b2, as in CI) and Neovim 0.12.3. Each mutant oftermopen_wait.luaran 3 times per build against the spec. "Slow" means a 50 msTermOpenautocmd added totests/minimal_init.lua.read_procreturns niljobpidraiseshrtimealways 0sleepdoes nothingreadlinkreturns nilreadreturns nilThe table has the same counts on 0.8.0 and 0.12.3. The 12 mutants of #593 (wrapper not installed, no wait, exe test inverted, a single 1 ms poll, no deadline assertion, a 1 ms deadline, no
pcallaroundjobpid, and the others) are still red 3 of 3 on both builds against this spec.MISE_LOCKFILE=false mise run allwith GNU timeout first on PATH exits 0 with 1904 successes, 0 failures, 0 errors and 36 pending, and luacheck reports 0 warnings in 200 files.mise run format-check: 444 files, 0 changed.pty_child_exec_spec.lua, 20 runs per build: 0 failures on 0.8.0 and 0.12.3.check.sh --hash-onlyis fresh for the four verify skills.Not covered
The case that reads
/procuses Neovim's own pid, so it needs a Linux/proclike the rest of the spec (the spec is pending without it). The defaultsleepis pinned only through the case that replacesvim.loop.sleep. Nothing here changes how the wrapper behaves.Generated with
xum• Model:anthropic:claude-sonnet-5-5• Thinking:xhigh