Skip to content

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

Merged
ThomasK33 merged 13 commits into
mainfrom
fix/cwd-git-fallback-path-chars
Nov 12, 2026
Merged

ThomasK33 merged 13 commits into
mainfrom
fix/cwd-git-fallback-path-chars

Conversation

@ThomasK33

@ThomasK33 ThomasK33 commented Nov 12, 2026 •

Copy link
Copy Markdown
Member

Why

cwd.git_root must find the repository of a folder whatever its name is. A repository in a folder with a space or a comma in its path was not found when git could not run: the fallback gave nil, or the repository of Neovim's current folder. A folder named [x] {y} or $HOME-like was lost or changed even when git answered. A verifier of F12 found the first case (finding F12-P1, a space or comma in the path makes the .git fallback return nil).

What changed

Four problems in lua/claudecode/cwd.lua. The first two took a path as a pattern. The third stepped one folder down at a trailing ... The fourth is a flaw in my first version of the second step.

  1. The .git fallback called vim.fn.finddir(".git", start_dir .. ";"). finddir reads its path as a list in 'path' option syntax. A space or a comma split the path, and a ; ended it. The fallback now walks up from the full path of start_dir and tests each folder for a .git folder with vim.fn.isdirectory. A relative start_dir now reaches the folders above the current one too. Before, a relative start gave a wrong answer or nil in some cases (for example git_root("sub/deeper") in the repository root).

  2. normalize_dir passed every path through vim.fn.expand. expand reads [, {, *, ?, $ and \ in a name as patterns or variables. A folder named [x] {y} or it's came back with a trailing slash. A folder named $HOME-like or back\slash came back nil. This also happened when git rev-parse answered. normalize_dir now returns a directory that exists as written, and expands any other string (~, $VAR). See step 4 for text that starts with ~ or $.

  3. .. before Neovim 0.11. Before 0.11, fnamemodify(path, ":p") keeps a last .., and :h of a/b/.. is a/b. So a walk from a start folder that ends in .. stepped one folder down and returned the wrong root. The walk now calls simplify() on the full path first. A verifier found this.

  4. Text that starts with ~ or $. The first version of step 2 took a path that names an existing folder as written, on the raw text. vim.fn.isdirectory does not expand, so with cwd = "~" or "$HOME" and a stray folder of that literal name in Neovim's current folder (for example one that an unexpanded mkdir "~/x" made), git_root and resolve returned the stray folder where main returned the home folder. normalize_dir now sends text that starts with ~ or $ through expand first, as a shell and :cd do. A verifier of the terminal change found this.

lua/claudecode/README.md gets one new bullet that states this, with the rule for text that starts with ~ or $. It sits after the "Terminal cwd precedence" bullet. It names git_root and resolve only, and it says that the terminal still passes a static cwd and a cwd_provider result through vim.fn.expand itself.

Scope

  • Only cwd.lua changes in code. Steps 1 and 2 are one change of idea: a path is a literal, not a pattern. Step 3 is a quirk of fnamemodify before Neovim 0.11. Step 4 closes a hole that step 2 opened.
  • normalize_dir serves cwd.git_root and cwd.resolve. Nothing in lua/ calls cwd.resolve. The terminal calls cwd.git_root when git_repo_cwd is set, so this PR fixes folder names with $ and \ on that path: when git does not answer, the terminal now gets $HOME-like, back\slash, [x] {y}, a space and a comma right, where main gave nil or the repository of Neovim's current folder. A static cwd and a cwd_provider result do not go through normalize_dir: terminal.lua:build_config passes them through vim.fn.expand itself, so this PR does not change what the terminal does with those. An absolute start path that ends in .. is fixed here, through the simplify() call above.
  • Two gaps stay for separate follow-ups. Main already has both, and this PR does not reach them. First, the fallback finds a .git folder only, not a .git file in a worktree or submodule. Second, the walk is lexical, so a link into another repository answers with the repository of the link's own folder, where git answers with the link target's repository. The terminal's expand of a static cwd and of a provider result is a third follow-up, and it has its own change.
  • ~ and $VAR still expand in cwd.resolve (the spec checks both). The spec's cases for cwd.resolve are named as cwd.resolve cases, because they do not test the terminal.
  • git rev-parse still runs first. Its result for a plain path is the same string as before.

Blast Radius

  • User-visible: a repository in a folder with a space, a comma, a quote or another path character is now found when git cannot run. With git, git_root returns a folder with [, {, *, ?, $ or \ in its name exactly. It does not reach the terminal through a cwd_provider: build_config still expands the provider's result, so the root README's example still loses $ and \ there (follow-up). A plain path gives the same result as before.
  • The fallback result is an absolute path with no .. part. Before it could be relative. The one exception is a relative start whose name begins with ~: Vim reads ~ as the home folder and ~name as a user's home folder, so :p leaves ~name relative and the result can be the relative ~name. That is a pathological case, and no caller in lua/ passes a relative path (the terminal passes the file's folder from expand("%:p") or getcwd()).
  • A string that names an existing folder is taken as written, except text that starts with ~ or $. That text always goes through expand first, so a folder named ~ or $HOME in Neovim's current folder does not take the place of the home folder, as on main. The cost: a relative folder name that starts with $ and reads as a variable ($HOME-like, $odd/sub) is expanded and not found, as on main. A lone relative $odd that names no variable is left unchanged by Vim and is still found. An absolute path never starts with ~ or $, so every folder name this PR fixes keeps working. The change applies to git_root's start_dir and to cwd.resolve. The terminal's own expand of a static cwd and a provider result is a separate change.
  • Neovim 0.8: the code uses vim.fn.isdirectory, vim.fn.fnamemodify and vim.fn.simplify, which exist there. I ran the new spec on Neovim 0.8.0, 0.10.0 and 0.12.3. The .. case fails on 0.8.0 and 0.10.0 without the simplify() call, and passes on 0.12.3 either way.
  • covered.lock: cwd.lua is the terminal's working-directory lookup. check.sh printed STALE for the file hash of cwd.lua only, with no changed block or call site. The change cannot reach diff state, an openDiff answer, a disconnect or a diff command. The model passed (main: 89205 distinct states, live-DisconnectedCallsAnswered: 5269). check.sh --record wrote the new lock and check.sh --hash-only reports that it matches.

Merge with #539 (F12)

#539 landed first. This PR's merge commit with origin/main resolves the interplay: it re-records covered.lock and changes the one stale test in #539's cwd_spec.lua (see Verification). F11 (#535) merges with this PR without a conflict.

Verification

Red before the fix, green after (same spec, only lua/claudecode/cwd.lua swapped):

Spec origin/main with the fix
tests/integration/cwd_path_characters_spec.lua (37 cases, real Neovim, 0.8.0 and 0.12.3) 20 successes, 17 failures 37 successes, 0 failures
the new case "goes up from a start folder that ends in .." at ede2f733 (before the simplify() call) fails on 0.8.0 and 0.10.0, passes on 0.12.3 passes on 0.8.0, 0.10.0 and 0.12.3 (35 successes when added, 37 at the final head, 0 failures on each)
the two cases "expands a leading ~ / $ even when Neovim's cwd holds a folder named ~ / $HOME", with cwd.lua from ca9e61c0 (before the guard) 35 successes, 2 failures on 0.8.0 and 0.12.3 37 successes, 0 failures on each

The spec makes real folders and runs cwd.git_root on them, in two modes. In the first mode git is off PATH (the shell keeps its absolute path), so git ends with "not found" and a list command raises E475, as on a host without git. In the second mode a real git init makes the repository. The folder names are: a space, a comma, a semicolon, single and double quotes, brackets and braces, parentheses, non-ASCII letters, a star and a question mark, a percent sign and a hash, a backslash, a dollar sign, a leading tilde, and a plain name. Each name runs from the repository folder and from a folder two levels below it, and the answer must equal the repository path with no trailing slash. A folder that is not a repository returns nil, whatever its name. Seven more cases cover a relative start folder, a start folder that ends in .., cwd.resolve with git_repo_cwd, a static cwd with brackets and a star in cwd.resolve, ~ and $VAR in a setting of cwd.resolve, and two cases where Neovim's current folder holds a folder named ~ or $HOME and the setting must still expand. The 17 red cases are 10 of the no-git names, 4 of the git names, the relative start, resolve, and the static cwd.

Gates on the final head f12368bd, which contains a merge of origin/main (05b9785, with #551, #535, #494, #545, #553, #555 and #557) (private XDG folders). An independent verifier passed c73ab044. Since then the branch has three changes of its own and two merges with main. The changes are a docs commit (the README sentence about where the walk starts), the fix for text that starts with ~ or $ (step 4, with two spec cases and the README clause), and the covered.lock record for it. For the lock I ran the full check.sh: the model passed (89205 distinct states, 5269 live states, every fact TRUE), and only the file hash of cwd.lua was stale, with no changed block or call site.

  • mise run all: native exit 0. luacheck: 0 warnings / 0 errors in 186 files. busted: 1756 successes / 0 failures / 0 errors / 29 pending. treefmt: 0 files changed. Since fix(skills): read run values through vcn.sh get, check the agent-tty socket dir, run the static proofs in CI #545, mise run all also runs the static proofs of the verify-claudecode-nvim skill, and they pass.
  • mise run format-check: native exit 0 (428 files, 0 changed).
  • scripts/run_integration_tests_individually.sh on Neovim 0.12.3: 564 successes, 0 failed, 0 errors, native exit 0. On Neovim 0.8.0: 427 successes, 0 failed, 0 errors, native exit 0 (the 0.8.0 run skips the specs that need 0.9). The new spec gives 37 successes on both. It starts about 100 processes (git and a shell), so it takes about 6 s on a quiet host and can pass the runner's 40 s limit at a load average above 100. A TIMEOUT line there means the host was slow, not that the spec hung.
  • All four .agents/skills/verify-*/check.sh --hash-only match their covered.lock.

The merge with origin/main had two parts to resolve in an earlier commit. covered.lock conflicted on the hash line of cwd.lua, so I ran the full check.sh and check.sh --record. #539's tests/unit/cwd_spec.lua had a test that stubbed vim.fn.finddir, which the fallback no longer calls, so that test now walks up real folders. It does not depend on where TMPDIR is: it passes with TMPDIR inside a git checkout too. cwd.lua itself merged without a text conflict. The two latest merges needed no covered.lock change: #551 conflicted only in tests/mocks/vim.lua, where the branch keeps both its simplify entry and #551's wordcount, and #535 and #494 merged without a conflict.

An independent verifier found the .. case, the README wording and the other points that this body now states. The terminal.lua expand and .git files in worktrees are gaps that main already has. They are follow-ups, not part of this PR.


Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh

… other path characters

cwd.git_root has two steps that took a path as a pattern:

- The fallback for a host where git cannot run called vim.fn.finddir with
  `start_dir .. ";"`. finddir reads that as a list in 'path' syntax, so a
  space or a comma split it and a `;` ended it, and the lookup returned
  nil for a repository in such a folder.
- normalize_dir passed every path through vim.fn.expand, which reads `[`,
  `{`, `*`, `?`, `$` and `\` as patterns or variables. A folder named
  `[x] {y}` came back with a trailing slash, and one named `$HOME-like` or
  `back\slash` came back nil, also when git answered.

normalize_dir now takes a directory that exists as written and expands
only a string that is not a directory (`~`, `$VAR`). The fallback walks up
from the full path of the start folder and tests each folder for a `.git`
folder, so no path goes through a pattern. A relative start folder now
reaches the folders above the current one too.
# Conflicts:
#	lua/claudecode/README.md
main now has #539 (git_root runs git once) and the README refresh. In
this merge:

- cwd.lua merges without a text conflict: #539's single `systemlist` call
  and this branch's literal-path fallback both stay.
- tests/unit/cwd_spec.lua (from #539): the test "runs git rev-parse once
  for a folder that is not in a repository, then searches for .git"
  stubbed vim.fn.finddir, which the fallback no longer calls. It now walks
  up real folders and finds a `.git` folder.
- covered.lock is re-recorded with check.sh --record. Only the cwd.lua file
  hash differs from main's lock.
…s to what changed

Fixes from the verification of this branch:

- Before Neovim 0.11, fnamemodify(path, ":p") keeps a last `..`, and `:h` of
  `a/b/..` is `a/b`. So with git off PATH, git_root("..") from a folder with
  its own `.git` returned that folder, not the parent's repository. The
  fallback now calls simplify() on the full path before the walk. The
  mock gets simplify(), and the integration spec gets a case for a start
  folder that ends in `..`. It fails on Neovim 0.8.0 and 0.10.0 without the
  fix.
- The README bullet said the cwd lookups keep a folder name with `$` or a
  backslash as written. The terminal does not use cwd.resolve: build_config
  passes a static cwd and a cwd_provider result through vim.fn.expand. The
  bullet now names git_root and resolve and says what the terminal does.
- The unit test of the walk found a `.git` folder above TMPDIR when TMPDIR
  sat inside a checkout, and asserted nil. It now asserts that no
  repository inside the test folder answers.
- The spec's cwd.resolve cases are named and commented as cwd.resolve
  cases, not terminal cases.
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex security review

@chatgpt-codex-connector

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

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

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

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-12T13:18:04.212013Z f12368b Manual request
🔒 Security Review ✅ Completed 2026-11-12T13:23:33.182477Z f12368b Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review

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

Reviewed commit: ca9e61c001

View security finding report

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

ℹ️ About Codex security reviews in GitHub

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

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

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

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Delightful!

Reviewed commit: ca9e61c001

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

…lder

normalize_dir took a path that names an existing folder as written, and only
expanded other text. isdirectory does not expand, so with `cwd = "~"` or
`"$HOME"` and a stray folder of that literal name in Neovim's current folder
(for example one that an unexpanded `mkdir "~/x"` made), git_root and resolve
returned the stray folder where main returned the home folder.

Text that starts with `~` or `$` now always goes through expand first, as in a
shell and in `:cd`. Two spec cases are red without the guard: a stray `~`
folder and a stray `$HOME` folder. The README bullet states the rule.
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex security review

@chatgpt-codex-connector

Copy link
Copy Markdown

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

Reviewed commit: f12368bd7d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review

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

Reviewed commit: f12368bd7d

View security finding report

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

ℹ️ About Codex security reviews in GitHub

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

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

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

@ThomasK33
ThomasK33 added this pull request to the merge queue Nov 12, 2026
Merged via the queue into main with commit ab6417f Nov 12, 2026
8 checks passed
@ThomasK33
ThomasK33 deleted the fix/cwd-git-fallback-path-chars branch November 12, 2026 16:32
hareki pushed a commit to hareki/claudecode.nvim that referenced this pull request Nov 13, 2026
…oder#569)

## Why

A Claude terminal did not start, or started in another folder, when the
folder name held `$` or `\`. `build_config` in
`lua/claudecode/terminal.lua` ran a static `cwd` and every
`cwd_provider` result through `vim.fn.expand`. `expand` reads `[`, `{`,
`*`, `?`, `$` and `\` in a name as patterns or variables. So a folder
named `$HOME-like` became `<home folder>-like`, a folder named
`back\slash` became `backslash`, and `[x] {y}` and `it's` came back with
a trailing slash. When the changed path did not exist, the terminal job
did not start (a real process run at base got `Vim:E475: Invalid
argument: expected valid directory`: the `native` provider reported
"Failed to open native terminal." and the `snacks` provider "Failed to
open Claude terminal using Snacks."). When it existed, the terminal
started in that other folder. coder#561 fixed `cwd.git_root` for such
folders, so the root README's provider example (`git_root(...)`) now
returns the repository root there. This change makes the terminal keep
that result. An independent verifier of coder#561 (finding P1) found the
terminal problem.

## What changed

1. **`build_config` takes a cwd that names an existing folder as
written.** It picks the provider result first, then the static `cwd`, as
before. It then checks `vim.fn.isdirectory` on that text. A folder that
exists is the cwd. Any other text still goes through `vim.fn.expand`, so
`~/projects/my-app` and `$HOME/my-app` keep working. Since coder#561,
`normalize_dir` in `cwd.lua` follows the same rule for `cwd.git_root`
and `cwd.resolve`. Text that starts with `~` or `$` always goes through
`expand` first, because `isdirectory` does not expand and a stray folder
named `~` in Neovim's current folder must not take the place of the home
folder. A verifier found that hole in my first version.
2. **Docs.** The root README ("Working Directory Control") and the
"Terminal cwd precedence" bullet in `lua/claudecode/README.md` state the
rule.
3. **A real-Neovim spec**, `tests/integration/terminal_cwd_spec.lua`,
with 31 cases. It stubs `vim.fn.termopen`, runs `:ClaudeCodeOpen` and
reads the cwd that the terminal job got.

## Scope

- Only `build_config` changes in code. The order stays: provider, then
static `cwd`, then the git root with `git_repo_cwd`. A provider that
returns an empty string or raises an error still falls through to the
static `cwd` (the spec checks both).
- `cwd.lua` does not change.
- Text that starts with `~` or `$`, and text that names no folder,
expands as before, so a provider result `~/sub` and a `cwd` of
`$VAR/sub` work (the spec checks both for each setting). With a stray
folder of that name in Neovim's current folder, it checks `~/sub` for
each setting, `$HOME` as a static `cwd`, and `$HOME/sub` as a provider
result.

## Blast Radius

- User-visible: a folder with `$`, `\`, `[` or `{` in its name is now
the terminal's cwd as written (a folder with `*` or `?` already worked
on main when the pattern matched no other folder; `a*` with a sibling
`aXY` now works too). A plain path gives the same result as before.
- Text that starts with `~` or `$` always goes through `expand` first,
so `cwd = "~"` or `"$HOME"` reaches the home folder even when Neovim's
current folder holds a folder with that literal name, as on main. The
cost is a relative folder name that starts with `$` and is meant
literally: it is expanded, as on main. An absolute path never starts
with `~` or `$`, so every folder name this PR fixes keeps working.
- The terminal passes this cwd to the `native` provider (`termopen`) and
the `snacks` provider (`cwd` in the Snacks window options), so both get
the corrected path. The `external` and `none` providers do not read it.
- `covered.lock`: `terminal.lua` is covered by the diff lifecycle model
and the terminal lifecycle model. `check.sh` printed STALE for the file
hash of `terminal.lua` only, in both, with no changed block or call
site. The change only decides which cwd string the terminal gets. It
cannot reach the terminal's start, show, hide, close, exit handling,
adoption or provider choice, and it cannot reach diff state, an openDiff
answer, a disconnect or a diff command. Both models pass (diff lifecycle
`main`: 89205 distinct states and `live-DisconnectedCallsAnswered`:
5269, every fact TRUE. Terminal lifecycle `main`: 263060 distinct
states, every fact TRUE). `check.sh --record` wrote both locks.
- coder#561 landed first. This branch merges `origin/main` after it. The only
conflict was in `lua/claudecode/README.md`. coder#561 added a bullet that
said: "The terminal does not use `cwd.resolve`:
`terminal.lua:build_config` passes a static `cwd` and a `cwd_provider`
result through `vim.fn.expand` itself, so there a folder name with `$`
or `\` still changes." I reworded that clause. It now says: "The
terminal does not call `cwd.resolve`, but `terminal.lua:build_config`
applies the same as-written rule to a static `cwd` and a `cwd_provider`
result, so an existing folder whose text does not start with `~` or `$`
keeps `$` or `\` there too. Unlike `normalize_dir`, it passes expanded
text that names no folder on to the terminal instead of dropping it."

## Verification

| | main | this branch |
| --- | --- | --- |
| `tests/integration/terminal_cwd_spec.lua` (31 cases, real Neovim 0.8.0
and 0.12.3) | 23 successes, 8 failures | 31 successes, 0 failures |
| the four cases "expands a leading ~ / $ ... even when Neovim's cwd
holds a folder named ~ / $HOME", with `terminal.lua` from `f643b755`
(before the guard) | 27 successes, 4 failures | 31 successes, 0 failures
|

The 8 failures on main are the folders with a single quote, brackets and
braces, a backslash and a dollar sign, each for a static `cwd` and for a
`cwd_provider` result. The 23 successes on main include a plain folder,
a space, a comma, a star and a question mark, a percent sign and a hash,
a leading tilde, the `~` and `$VAR` expansions, the four stray-folder
cases, and the precedence cases.

Gates on `8cf07210`, whose parent `7ec5fbb8` merges `origin/main`
ab6417f (with coder#561) into the branch (private XDG folders). The last
commit changes one sentence of `lua/claudecode/README.md` and no code,
so `mise run all`, the integration runners and the full model checks ran
on its parent, `7ec5fbb8`. For the last commit I re-ran `format-check`
and `check.sh --hash-only` on all four skills (both pass):

- `mise run all`: native exit 0. luacheck: 0 warnings / 0 errors in 187
files. busted: 1818 successes / 0 failures / 0 errors / 30 pending.
- `mise run format-check`: native exit 0 (429 files, 0 changed).
- `scripts/run_integration_tests_individually.sh` on Neovim 0.12.3: 603
successes, 0 failed, 0 errors, native exit 0. On Neovim 0.8.0: 599
successes, 0 failed, 0 errors, native exit 0.
- Full `check.sh` of `verify-diff-lifecycle` and
`verify-terminal-lifecycle`, (both models pass with exit 0 and every
fact TRUE: diff lifecycle 21, terminal lifecycle 34. No record was
needed after the merge), then all four `.agents/skills/verify-*/check.sh
--hash-only`: every one matches its `covered.lock`.

---
_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 13, 2026
…status (coder#570)

## Why

`assert(os.execute(...))` never fails under LuaJIT. Neovim, busted and
the test rocks all run on LuaJIT. Its `os.execute` returns the raw
status: 0 for success and 256 for exit status 1. Every number is truthy
in Lua, so the assert passes whatever the command did. A `mkdir`, `ln`
or `touch` that failed left the test to run on a setup that did not
exist, and the test could pass for the wrong reason. A verifier of coder#561
found this.

I checked it on busted's LuaJIT (2.1.1744318430) and on Neovim 0.8.0 and
0.12.3 (both `Lua 5.1` with LuaJIT): `os.execute("exit 1")` returns 256
and `os.execute("true")` returns 0 on each.

## What changed

1. `tests/lockfile_test.lua` gets a local `shell(command)` helper that
asserts `status == 0 or status == true`, and names the command when it
fails. The `or status == true` part keeps it right on Lua 5.2 and later,
where `os.execute` returns `true` for success. It replaces nine
`assert(os.execute(...))` calls, two of them spread over three lines.
2. `tests/unit/temp_dir_spec.lua` checks its one `mkdir -p` the same
way, inline.
3. A second commit, from the Opus verdict: the `chmod` that the
folder-swap test runs ("should not delete a folder that takes the place
of the lock file after the check", `lockfile_test.lua` line 669) now
goes through `shell` too. That test has no probe, so a failed `chmod
000` left its "mode 000" pass running on a readable file, and the test
stayed green. With `chmod` forced to fail, the file gave 33 successes
and 0 failures before, and gives 1 failure (`failed: false && chmod 000
...`) now. Normally it gives 33 successes.

Only two files had the bug, so there is no new shared helper. Every
other place in `tests/` that reads an `os.execute` result already tests
`== 0 or == true` (`diff_spec.lua`, `integration_runner_spec.lua`,
`test_order_helper_spec.lua`, `tmpdir_suite_spec.lua` and the status
checks inside `lockfile_test.lua`). `tests/unit/cwd_spec.lua` has two
more of the broken calls, from coder#561. They are not in this PR. A forced
failure there still turns the test red, at a later line. They go to a
tracking issue (F27) that this run files after it lands.

## Verification

I made each of the ten commands fail on purpose (`false && <command>`)
and ran its file on `origin/main` and on this branch. The full table is
in the details below.

- Two sites stayed green before the change and go red now:
`lockfile_test.lua` 958 and 972 on main, which are 964 and 977 after the
change (two `authentication token` tests: a folder named like a lock
file, and a link that points nowhere).
- Seven sites were already red before, but only at a later assertion
that hides the cause. Now each fails at the setup, with the command in
the message (`failed: false && mkdir ...`).
- The `temp_dir_spec.lua` site passed its setup before, and three later
tests failed at `temp_dir_spec.lua:47`. It now fails at once with
`cannot create ...`.

<details>
<summary>Failure demonstration, each site on <code>origin/main</code>
and on this branch</summary>

Each site was made to fail by putting `false && ` in front of its
command. Each row runs the whole file once.

| file and site (line before -> after) | before the change | after the
change |
| --- | --- | --- |
| `lockfile_test.lua` 610 -> 618 | 32 successes / 1 failure / 0 errors /
0 pending - a later assertion fails: lockfile_test.lua:618: the folder
is gone | 32 successes / 1 failure / 0 errors / 0 pending - failed:
false && mkdir '/tmp/claudecode.CuDPK8/.claud |
| `lockfile_test.lua` 670 -> 678 | 32 successes / 1 failure / 0 errors /
0 pending - a later assertion fails: lockfile_test.lua:685: mode 000:
remove returned true: nil | 32 successes / 1 failure / 0 errors / 0
pending - remove raised: tests/lockfile_test.lua:347: failed: fa |
| `lockfile_test.lua` 709 -> 716 | 32 successes / 1 failure / 0 errors /
0 pending - a later assertion fails: lockfile_test.lua:701: remove
returned false: Lock file does not exist: /tmp/cla | 32 successes / 1
failure / 0 errors / 0 pending - failed: false && ln -s
'/tmp/claudecode.ybk2Ov/.claud |
| `lockfile_test.lua` 721 -> 727 | 32 successes / 1 failure / 0 errors /
0 pending - a later assertion fails: lockfile_test.lua:701: remove
returned false: Lock file does not exist: /tmp/cla | 32 successes / 1
failure / 0 errors / 0 pending - failed: false && ln -s
'/tmp/claudecode.YPFHbg/.claud |
| `lockfile_test.lua` 735 -> 741 | 32 successes / 1 failure / 0 errors /
0 pending - a later assertion fails: lockfile_test.lua:701: remove
returned false: Lock file does not exist: /tmp/cla | 32 successes / 1
failure / 0 errors / 0 pending - failed: false && ln
'/tmp/claudecode.sIlyBK/.claude/i |
| `lockfile_test.lua` 747 -> 753 | 32 successes / 1 failure / 0 errors /
0 pending - a later assertion fails: lockfile_test.lua:703: the target
is gone | 32 successes / 1 failure / 0 errors / 0 pending - failed:
false && mkdir '/tmp/claudecode.gPmemX/.claud |
| `lockfile_test.lua` 748 -> 754 | 32 successes / 1 failure / 0 errors /
0 pending - a later assertion fails: lockfile_test.lua:701: remove
returned false: Lock file does not exist: /tmp/cla | 32 successes / 1
failure / 0 errors / 0 pending - failed: false && ln -s
'/tmp/claudecode.rDzhLZ/.claud |
| `lockfile_test.lua` 958 -> 964 | 33 successes / 0 failures / 0 errors
/ 0 pending - the test stays green | 32 successes / 1 failure / 0 errors
/ 0 pending - failed: false && mkdir '/tmp/claudecode.ro |
| `lockfile_test.lua` 972 -> 977 | 33 successes / 0 failures / 0 errors
/ 0 pending - the test stays green | 32 successes / 1 failure / 0 errors
/ 0 pending - failed: false && ln -s '/tmp/claudecode.K94nha/.cl |
| `temp_dir_spec.lua` 15 -> 16 | the setup passes and three later tests
fail: temp_dir_spec.lua:47: /tmp/claudecode.3gRUQA/dir
it's/fixture_spec.lua: No | the setup fails at once, 1 error - cannot
create /tmp/claudecode.YmMEJ1/dir it's/back\sla |

</details>

## Left as it is

Three other `chmod 000` and `chmod 555` calls in `lockfile_test.lua`
(lines 583, 633 and 904) have no check either, and I left them. Each is
followed by a probe: if the file is still readable or the folder is
still writable, the test returns `pending("... (root)")`. So a `chmod`
that failed shows as a pending test in the run output, but the run stays
green and the shown reason wrongly says "(root)". The same tracking
issue (F27) covers these three calls and the `mkdir -p` at line 416,
together with moving `shell` to `tests/helpers/`. The `chmod 600` and
`chmod 755` calls only restore a mode in cleanup. `lockfile_test.lua`
and `integration_runner_spec.lua` now each hold a local `shell` helper.
A change that fixes `tests/unit/cwd_spec.lua` can move it to
`tests/helpers/`.

Gates on `c71fe8e9`, which merges `origin/main` 48ca93b into the branch
(private XDG folders). A first round of Codex review on `55bc57be` found
nothing. The chmod commit and the merge came after it:

- `mise run all`: native exit 0. luacheck: 0 warnings / 0 errors in 190
files. busted: 1832 successes / 0 failures / 0 errors / 30 pending.
- `mise run format-check`: native exit 0 (432 files, 0 changed).
`check.sh --hash-only` matches on all four skills..
- `scripts/run_integration_tests_individually.sh` on Neovim 0.12.3: 600
successes, 0 failed, 0 errors, native exit 0. On Neovim 0.8.0: 582
successes, 0 failed, 0 errors, native exit 0.

---
_Generated with [`xum`](https://github.com/coder/xum) • Model:
`anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant