Repository navigation
fix(scripts): make mise run format-check write nothing - #594
Conversation
The task described itself as verifying formatting without writing, but it ran `treefmt --ci`, which formats the files in place and then fails because it changed them. On a tree with an unformatted file, a failed check rewrote that file. I ran it on a copy of main with CHANGELOG.md and DEVELOPMENT.md made unformatted: both files had new hashes after the check. The task now runs scripts/format-check.sh. It copies the files that git knows (tracked, and untracked but not ignored) into a temporary folder twice, runs treefmt on one copy, and compares the copies with diff -ru. The diff is the failure output, so it now shows what would change. A linter failure still fails the check. scripts/test-format-check.sh runs the real task on an unformatted Markdown probe, on the same probe formatted, and on a shell probe that shellcheck fails. The test asserts the exit status and that each probe is byte for byte the same afterwards. It fails on the old task (the probe was rewritten), and CI runs it after the format check. The copy is one tar pipe, because one cp for each file took four minutes on a loaded host.
…ormats, next to the git directory The copy of the tree that format-check formats gave a different verdict than `treefmt --ci` in three cases: - A symlink that does not resolve in the copy (dangling, or relative to a place outside the checkout) made `diff` fail, and the check blamed the formatter. treefmt's git walker skips symlinks, so the copy now holds regular files only. - A nested git repository or a submodule was copied whole and formatted. The git walker skips such a folder, so the copy leaves it out. - A Prettier configuration in a folder above the checkout had no effect in a copy under TMPDIR. The copy now lives in the git directory, so a formatter looks for its configuration in the same parent folders. A diff status above 1 now fails with its own message, and a tar failure exits 1 like a treefmt failure.
…where it copies the files
…ot on a copy The copy of the tree that format-check made gave a different verdict than `treefmt --ci` in layouts the copy cannot reproduce: a read-only git directory, an ignored formatter config or ignore file, an unreadable file that no formatter reads, and a git directory outside the checkout's folder chain. Each fix to the copy opened another case, so the copy is gone. treefmt runs in the real tree with its default walker, and `--ci` as before. The three formatters that write files (prettier, stylua, shfmt) now run through scripts/treefmt-formatter.sh. It picks write flags or check flags from one variable, FORMAT_CHECK, which the format-check task sets. In check mode a tool fails on a file that it would change, prints that file, and writes nothing. The includes and excludes of each formatter stay in treefmt.toml, so both modes format the same files. The linters are not wrapped. scripts/format-check.sh is deleted. scripts/test-format-check.sh now checks that the check fails, names and leaves three unformatted probes (Markdown, Lua and shell), that `mise run format` formats them, and that the check passes on the formatted probes and fails on a shellcheck error.
By default treefmt uses the root of the git repository as its tree root. A checkout with no .git of its own inside another git repository (a vendored copy) then got the outer repository as its tree root. treefmt looked for scripts/treefmt-formatter.sh there and failed before it checked a file, and it anchored the fixtures/*/lazy-lock.json exclude at the outer root, so the lock files counted. `treefmt --ci` on main failed in that layout because of the lock files. tree-root-file = "treefmt.toml" makes the folder of treefmt.toml the tree root. In a normal checkout that is the folder of the git root, as before. test-format-check.sh now runs the check in a vendored copy.
…ames that start with a dash treefmt keys its cache on the size and modification time of the command of each formatter. For stylua, prettier and shfmt that command is scripts/treefmt-formatter.sh, so a version bump of a tool did not make treefmt format the files again: `mise run format` wrote nothing, and `mise run format-check` then failed on the files that the new version formats differently. mise.toml now sets TREEFMT_NO_CACHE in [env], which `mise run`, `mise exec` and a shell with `mise activate` apply. treefmt formats every file on every run (about 20 s where a warm cache took 0.2 s). A treefmt outside mise's environment keeps its cache, and the README says to run `treefmt --no-cache` after you change a tool. The wrapper passes `--` before the file names, so a file whose name starts with a dash is a file, not an option. Before, an unformatted `-probe.md` passed the check. test-format-check.sh: - simulates a version bump of stylua (the real one, and the real one with another quote style) and fails when treefmt uses its cache after it; - has a probe whose name starts with a dash; - creates its probes with noclobber, so a second run in the checkout exits 2 at once, and no run blames the check for the writes of another; - compares git status (with untracked and ignored entries) and the hash of every changed or untracked file before and after each check, not only the bytes of the probes. scripts/README.md says that the check lints and checks, and lists the cache that the test does not write.
…, shfmt and shellcheck shellcheck is not behind the wrapper, so it still read a file whose name starts with -P as the option -P (a source path) and linted nothing of that name. Since the wrapper gives shfmt `--`, shfmt accepts such a name, and the check passed a script that shellcheck rejects (on main the check failed it only because shfmt rejected the name). The shellcheck options now end with `--`. actionlint and zizmor only get paths under .github/workflows/, which never start with a dash. test-format-check.sh now has a probe for each tool whose name starts with a dash: -Vformat-check-probe.lua (stylua reads -V as --version, exits 0 and skips its whole batch), -format-check-probe.sh (shfmt stops with an error on it) and -Pformat-check-probe.sh with an SC2086 error (shellcheck must fail it). The check must name each unformatted probe and `mise run format` must rewrite it. Names are matched as a whole, so the dash probe no longer satisfies the check for the plain probe. scripts/README.md says that an ignored file that exists already is not seen, and no longer says that the test makes a private cache folder.
…shows a missing --
…-2 message tells to do shfmt stops with an error on a name that starts with a dash only when the name is the first of its file list: Go's flag parser takes a dash name behind a plain name as a file. The README and the header of test-format-check.sh say so. The README says that step 4a writes the shell text to two probes. The exit-2 message tells the user to remove the probes of a killed run with `rm -- <name>`, because a plain rm reads a dash name as options. A comment in the test says that the `-P` step cannot show a missing `--` for shellcheck by itself, because the test fails earlier, at step 2.
|
@codex review |
|
@codex security review |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
Codex Review: Didn't find any major issues. Keep them coming! 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". |
… treefmt.toml The shared wrapper made treefmt's cache key on the wrapper, not on the tool. After a tool version bump, `mise run format` wrote nothing, and TREEFMT_NO_CACHE (the fix) made every `mise run format` format all files (about 25 s, where a warm cache took 0.2 s) and left a plain treefmt outside mise with the stale cache. The mechanism is narrower now. treefmt.toml holds the real tools with their write flags again, as on main, with `--` before the file names for prettier, stylua, shfmt and shellcheck (a tool reads a path that starts with a dash as an option). treefmt's cache keys on the real tools, with mise or without it. TREEFMT_NO_CACHE and the wrapper scripts/treefmt-formatter.sh are gone. `mise run format-check` runs `treefmt --ci` with scripts/format-check-bin/ first on PATH. It holds check-mode.sh and symlinks named prettier, stylua and shfmt. The script drops the options up to `--`, takes its own folder off PATH, and runs the real tool with its check flags (prettier --check, stylua --check, shfmt -i 2 -d), so the check writes nothing. A second run of the script in the same environment (no real tool on PATH) exits 2 instead of looping. tree-root-file stays. test-format-check.sh now runs the vendored copy with the stand-ins, and its version-bump step runs in two ways, under mise's environment and outside it (`env -i`), with the cache on: version 1, version 1 again (0 files from the cache) and version 2 (it rewrites the probe).
…he check when treefmt cannot find the stand-ins
The format-check task pasted `{{config_root}}` into its shell line, so a checkout whose path holds a quote,
`$HOME` or a backtick changed the shell text: a quote made a syntax error, and `$HOME` or a backtick changed the
folder name, so treefmt found no stand-in and the real tools rewrote the files. A colon in the path split the PATH
entry the same way.
The task now reads `$MISE_CONFIG_ROOT`, and before treefmt runs it checks that prettier, stylua and shfmt each
resolve to the stand-in. When one does not, it exits 2 and treefmt does not run, so a PATH that treefmt cannot
use stops the check instead of letting the real tool write.
scripts/test-format-check.sh runs the check in copies of the checkout whose paths hold `"`, `$HOME`, a backtick and a
colon. The check-mode.sh message for a repeated run also names an exported FORMAT_CHECK_STAND_IN.
The guard in the format-check task loops over prettier, stylua and shfmt. The odd-path copies break all three at once, so a guard that skipped one tool still passed them. Three more copies of the checkout, each without the symlink of one tool, must make the check exit 2 and name that tool.
…ingle quote in the checkout path
The guard in the format-check task said that a colon in the path of the checkout caused the failure, also when a
stand-in was missing, not executable, or not first on PATH. The message now lists those causes.
scripts/test-format-check.sh gets a checkout whose path holds a single quote. A task that built the stand-in folder
from `'{{config_root}}'` passed every other path and exited 2 with a syntax error in that one.
scripts/README.md says that an exported FORMAT_CHECK_STAND_IN makes the check fail with exit 1 (the stand-in exits 2
and treefmt reports the failed formatter), not exit 2.
|
@codex review |
|
@codex security review |
|
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. |
…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`_
Why
The goal:
mise run format-checkmust do what its description says.mise.tomlcalls it "Verify formatting without writing (used in CI)". It rantreefmt --ci, and treefmt has no dry run.--ciformats 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.mdandDEVELOPMENT.mdunformatted, ranmise run format-check, and looked at the files afterwards:CHANGELOG.md1ffecd326a8afd7e19b9e69803385db9DEVELOPMENT.md209225cc4d1b551fb6041a87646a230aThe 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 onCHANGELOG.md.What changed
mise.toml: theformat-checktask putsscripts/format-check-bin/first onPATHand runstreefmt --ciin 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 thatprettier,styluaandshfmteach 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 onPATH, for example because a colon in the checkout path splits the entry), exits 2 and does not run treefmt. So aPATHthat treefmt cannot use stops the check instead of letting the real tool write. The description stays, and it is true now.scripts/format-check-bin/(new) holdscheck-mode.shand three symlinks to it, namedprettier,styluaandshfmt. With the folder first onPATH, treefmt finds the symlinks in place of the real tools. treefmt runs a formatter with the options oftreefmt.toml, a--, and the file names. The script drops everything up to the first--, takes its own folder offPATH, and runs the real tool in check mode:prettier --check,stylua --checkorshfmt -i 2 -d, each with a--before the file names. The tool fails on a file that it would change, prints that file (styluaandshfmtprint the diff too), and writes nothing. A run of the script in an environment that its own earlier run set (no real tool onPATH, so it found itself) exits 2 instead of looping. An exportedFORMAT_CHECK_STAND_INhas the same effect on the stand-in: it exits 2, treefmt reports the failed formatter, andmise run format-checkfails with exit 1. The message names the variable, and nothing is written.treefmt.tomlholds 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 ofprettier,stylua,shfmtandshellcheck. 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.actionlintandzizmoronly get paths under.github/workflows/, which never start with a dash.treefmt.tomlalso setstree-root-file = "treefmt.toml", so the tree root is the folder of that file (see Blast Radius).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 comparesgit status --porcelain -uall --ignored=matchingand 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 thatshfmtkeeps andshellcheckrejects, and on-Pformat-check-probe.shwhen that is the only probe thatshellcheckrejects. 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 ofstylua(the realstylua, 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 withnoclobber, so a second run of the test in the same checkout exits 2 at once, and they are removed on exit..github/workflows/test.yml: theunit-testsjob runsscripts/test-format-check.shafter the format check.scripts/README.md, and the comments intreefmt.tomlandmise.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.
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.mise run formatwrote nothing. The fix (no cache at all) made everymise run formatformat all files (26 to 28 s, where main took about 1 s), and a plaintreefmtoutside mise kept the stale cache.mise run format, a plaintreefmt, the.claudeformat hook and editors run the real tools, exactly as on main, so treefmt's cache keys on the real tools. Onlymise run format-checkputs stand-ins onPATH, and the stand-ins hand the real tool its check flags.The price: the write flags live in
treefmt.tomland the check flags incheck-mode.sh(two places), the repo has three symlinks, and the stand-ins work only throughmise run format-check. A baretreefmt --cirewrites files, as on main. The task fails closed: when treefmt cannot reach the stand-ins,mise run format-checkexits 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 formatandmise run allrun the same commands as on main, with a--before the file names. No Lua file changed.Blast Radius
Developers and CI:
mise run format-checkistreefmt --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 printsDiff 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 runsmise run format. Theunit-testsjob gains the test step (about 100 to 130 s on this host: four whole-tree checks, onetreefmtrun on six files, a vendored check and a bump step in two ways).mise run formatand a plaintreefmtuse treefmt's cache as they do on main. A warmmise run formattakes 0.26 to 0.29 s (main 0.24 to 0.28 s). Thetreefmtrun 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 ofstylua,prettierorshfmtmakes treefmt format the files again, with mise's environment and without it (see Verification).Where this check and
treefmt --cion main give different verdicts, main is wrong in the first two cases below. In the last two, this check refuses to run or fails:.gitof its own that lies inside another git repository gets the outer repository as its treefmt tree root. Main's check then anchors thefixtures/*/lazy-lock.jsonexclude 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.tomlsetstree-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 oftreefmt.tomlis the root of the git repository, as before.-probe.mdor-dir/x.md, so main passes it. stylua reads-V.luaor-h.luaas--versionor--help, prints that, exits 0, and skips every Lua file of its batch, so main passes an unformattedlua/claudecode/logger.luanext to a-V.lua. stylua and shfmt fail on a formatted-ok.luaor-ok.sh, so main fails a formatted tree. The--before the file names (and theshellcheckoptions ending with--) make this check treat such a file like any other. A-Pprobe.shwith a shellcheck error fails on both: on main only because shfmt rejects the name, and here because shellcheck reports SC2086.PATHentry 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$HOMEor 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_ROOTnow.)FORMAT_CHECK_STAND_IN. The stand-ins use it to detect a second run, so with it setmise run format-checkfails 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:
chmod 000) makestreefmthang, 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.mise run format-check: the task stops with exit 2 (see above).mise run formatis not affected.actionlintorzizmor(onlyshellcheckis covered). The differential run below coversactionlint(actionlint_fail) andshellcheck, notzizmor. macOS and bash 3.2 were not run.Declined
treefmt.tomlgains 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 makeformat-checkrewrite its files again. Declined for this PR because it guards a future change, not a defect in this one; the stand-in folder andscripts/README.mdname 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 withformat-check changed the working treeand the changed hashes (exit 1). Against this head it printstest-format-check: ok(about 100 to 130 s on this host).The version bump.
mise.tomlpointsstyluaat a script for "version 1" (the real stylua), a formatting run follows,mise.tomlthen points at "version 2" (the real stylua with--quote-style ForceSingle), and the formatting runs again. Numbers are the files thattreefmtreported, and the files that it changed:b3110b2cmise run format: version 1, then version 2treefmtoutside mise (env -i, the tools onPATH): version 1, version 1 again, version 2, version 2 againOn 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 onb3110b2cstops earlier, at the vendored-copy step, because that head has noscripts/format-check-binto copy. So I ran step 6 of the test (the version bump) alone againstb3110b2c. 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 unformattedprobe.mdandprobe.luain the root, andmise run format-checkin each (the folder names areq"uote,s'quote,d$HOME,b`true`bandc: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 on749e925d:749e925dq"uotesh: Syntax error: Unterminated quoted string, probes unchangeds'quote749e925dd$HOMEb`true`bc:olonThe 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):
prettier --write(the check writes)styluastand-in without--checkshfmtstand-in without-dprettierstand-in without--check--before the file names (each of the three)PATHmise.tomlwithout thePATHprefixPATHPATHprefix names a folder that does not existprettier,styluaorshfmt(each){{config_root}}again (the first version of this PR)'{{config_root}}'(single quotes: only thes'quotecopy breaks it)prettier,styluaorshfmt(each)mise run formatgoes through the stand-instreefmt.tomlwithout--in the options ofprettier,styluaorshfmt(each)treefmt.tomlwithout--in theshellcheckoptionsshfmtwrite flags without-i 2prettierwrite options without--writetreefmt.tomlwithouttree-root-fileTREEFMT_NO_CACHE=1back in[env](the cost of the previous design)One mutant survives: the task takes the checkout folder from
$PWDinstead of$MISE_CONFIG_ROOT. It is equivalent only while mise runs the task in the folder of itsmise.toml, which is the default (I checked it from a subfolder). With[task_config] dir = "{{cwd}}"in amise.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 --cion main: 107 cases. The cases are the 101 of my run against the previous head749e925d(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 runsmise run format-checkon 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 against9be9f1f6(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).vendored_subdir, and the dash casesdash_name,dash_dir_md,dashdash_only_md,dash_V_lua,dash_V_lua_plus_lua_trackedanddash_h_lua_plus_lua_tracked(main passes, this head fails an unformatted file), anddash_lua_formattedanddash_sh_formatted(main fails a formatted file, this head passes). Two are this check refusing to run:path_colon_mdandpath_colon_clean(a colon in the checkout path: exit 2 here, and main's check runs).mode000_dir_tracked,treefmt --cihangs on both (see Limits). I killed both, so the exit status is the signal's.TMPDIR, except inmode000_dir_tracked, where I killed treefmt myself.",$HOME, backtick) give main's exit status, and the check names the same file that main's check rewrote.sh_trackedandshfmt_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.mdthat main skips, andpath_colon_md, where this check stops before it names a file.All 107 cases
treefmt --cion mainformat-checkactionlint_failcleancr_line_endingscrlf_trackedcrlf_tracked_formatted_attrdash_P_sh_lintbaddash_V_luadash_V_lua_plus_lua_trackeddash_dashdash_mddash_dir_mddash_e_sh_lintbaddash_filename_shdash_h_lua_plus_lua_trackeddash_luadash_lua_formatteddash_md_formatteddash_namedash_name_and_md_trackeddash_shdash_sh_formatteddashdash_only_mddeleted_plus_mddeleted_trackedexcluded_dir_trackedexec_bitfifo_untrackedformatter_missingformatter_tool_missinggitdir_readonlyglob_nameignored_but_trackedignored_but_tracked_luaignored_but_tracked_luacovignored_editorconfigignored_prettierignoreignored_prettierrc_rootignored_prettierrc_subdirignored_unformattedintent_to_addjson_syntax_errorlarge_treeleading_space_namelinked_worktreelinked_worktree_cleanlinked_worktree_parent_prettierrclinked_worktree_untrackedlinter_missinglua_syntax_errorlua_trackedmd_trackedmode000_dir_trackedmode000_dir_untrackedmode000_file_md_unformattedmode000_tracked_filenested_repo_untrackednested_repo_untracked_cleannewline_nameno_gitparent_prettierrcprettier_excludedprettierignore_trackedreadonly_unformattedseparate_gitdir_cleanseparate_gitdir_parent_prettierrcseparate_gitdir_prettierrc_above_gitdirsh_trackedshellcheck_trackedshellcheck_untrackedshfmt_plus_shellcheckspace_trackedspace_untrackedsparse_conesparse_cone_cleansparse_indexsparse_noconesubdir_cwdsubmodule_checkout_parent_prettierrcsubmodule_gitlinksymlink_abs_outsidesymlink_danglingsymlink_dangling_trackedsymlink_dirsymlink_in_cleansymlink_rel_outsidesymlink_to_unformattedsymlink_tracked_to_unformattedsymlinked_cwdtracked_dir_replaced_by_symlinktracked_replaced_by_dirtracked_replaced_by_fifotracked_replaced_by_symlinktracked_symlink_replaced_by_filetrailing_space_nameunicode_tab_namesunreadableuntracked_mdvendored_filesystem_no_gitvendored_subdirvendored_subdir_no_locksvendored_subdir_untracked_unformattedworkflow_unformattedpath_quote_mdpath_dollar_mdpath_backtick_mdpath_colon_mdpath_colon_cleanpath_quote_cleanGates (exit codes) at 7653e42, which merges main at d05b010. Main has since moved to bcf1f08 (#597, tests only): the merge is clean (
git merge-tree), and I did not run the other gates on it.mise run format-checkscripts/test-format-check.shtest-format-check: ok, 138 s at a high load)mise run allscripts/run_integration_tests_individually.sh, Neovim stable (0.12.3)scripts/run_integration_tests_individually.sh, Neovim v0.8.0scripts/run_integration_tests_individually.shunder aTMPDIRwith a space and a%(stable)git merge-tree --write-tree HEAD origin/main(main at bcf1f08)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• Model:anthropic:claude-sonnet-5-5• Thinking:xhigh