Skip to content

docs: rewrite the lock file and diff sketches of ARCHITECTURE.md to match the code - #543

Merged
ThomasK33 merged 6 commits into
mainfrom
docs/architecture-lockfile-sketch
Nov 12, 2026
Merged

ThomasK33 merged 6 commits into
mainfrom
docs/architecture-lockfile-sketch

Conversation

@ThomasK33

@ThomasK33 ThomasK33 commented Nov 12, 2026 •

Copy link
Copy Markdown
Member

Why

Two sections of ARCHITECTURE.md show code sketches that do not match the code. Both read as how the plugin works today. They sit under headings that name the module, and nothing calls them examples, so a contributor reads them as fact.

  • The Lock File System sketch names a write_file helper and a vim.loop.fs_rename call that lockfile.lua does not have. It puts a VimLeavePre autocmd and an unconditional fs_unlink in lockfile.lua, which has no autocmd. docs: say stop only warns when it cannot delete the lock file #537 added one sentence after it and left the rewrite for this PR.
  • The Diff System sketch writes a temp file with write_file and sets the keymaps <leader>da and <leader>dq. The openDiff tool writes no temp file, and the plugin sets no diff keymaps.

What changed

Two sections of ARCHITECTURE.md. Each is now short prose with citations, plus one shortened excerpt whose code lines are verbatim from the source file (a script checked each line).

Lock File System (lockfile.lua), six bullets:

  • Where the file lives: <port>.lock in $CLAUDE_CONFIG_DIR/ide, or in ~/.claude/ide when that variable is unset or empty (lockfile.lua:11-18).
  • What it holds: pid, workspaceFolders, ideName, transport and authToken (lockfile.lua:129-135).
  • Directory mode: a new lock directory gets 0700, and create tries to tighten an existing one (lockfile.lua:94, :104).
  • The atomic write: a temp file in the same directory, opened with wx and mode 0600, renamed with os.rename. A failed write or rename makes create try to remove the temp file, and it ignores a failed removal (lockfile.lua:154-199).
  • Removal: remove uses os.remove and returns an error when no readable lock file exists and when os.remove fails (lockfile.lua:208-232).
  • The lifecycle, which init.lua owns: start calls create after the server binds, stop calls remove and only logs a warning when it fails, and the VimLeavePre autocmd that setup creates calls stop when the server runs (init.lua:514-522, :586, :635-648).

Diff System (diff.lua, diff_inline.lua), a paragraph and six bullets:

  • openDiff calls open_diff_blocking, which runs in the tool's coroutine and yields until the diff is resolved. A ClaudeCodeDiffOpened handler can resolve the diff during setup, and open_diff_blocking then returns the result at once without yielding. When the setup fails (for example, because the file has unsaved changes), it raises an error and does not yield (tools/open_diff.lua:67, diff.lua:1893-1987).
  • The layout: vertical (the default) and horizontal use Neovim's diff mode, with the proposal in a split to the right or below. For a new file, the old side is an empty buffer. unified is drawn inline by diff_inline.lua and is not a Neovim diff (diff.lua:279-288, :1595).
  • The proposal is an unlisted acwrite scratch buffer and not a temp file. Its name is <tab_name> (proposed) for a native diff of an existing file, <tab_name> (NEW FILE - proposed) for a new file, and <tab_name> (unified diff) for the unified layout (diff.lua:1730-1748, diff_inline.lua:292-299).
  • Tabs: a native diff prefers the current tab and goes to an editor window in another tab only when the current tab has none. It opens in a new tab when diff_opts.open_in_new_tab is set or the tab it picks, or the current tab, already has a diff window (diff.lua:125-136, :1625, :1700).
  • Accept and reject: in the proposal buffer, :w (also :wq, :x and ZZ) and :ClaudeCodeDiffAccept accept. :ClaudeCodeDiffDeny, closing the last window of the proposal, and deleting its buffer reject. The two commands read the diff from the current buffer and only warn in another buffer. The plugin sets no keymap. The README's example config maps <leader>aa and <leader>ad to the two commands (README.md:66-67).
  • A diff resolves once. Accept replies FILE_SAVED with the content, reject replies DIFF_REJECTED, and the plugin does not write the target file. Normally Claude writes it after FILE_SAVED. When that Claude session ends or drops the answer first, :ClaudeCodeCloseAllDiffs puts the accepted text into the file's buffer as unsaved changes. A diff whose buffer cannot take the text (for example, because it has unsaved changes) stays open (diff.lua:1071, :1154, :2209-2271, :2309).
  • Only the non-blocking open_diff writes a temp file, and the plugin does not call it. It goes under stdpath("cache")/claudecode_diffs/<session folder>, and vim.fn.tempname() supplies only a part of the folder name (diff.lua:522-561, :791-860).

Statements of the old sections that the code contradicts

Old statement What the code does Evidence
Lock: local temp_path = lock_path .. ".tmp" The temp path is <port>.lock.tmp.<pid>.<hrtime>. lockfile.lua:154
Lock: write_file(temp_path, json_data) No write_file exists. The code opens the temp file with fs_open "wx" and mode 0600, loops fs_write until every byte is written, then closes it. grep -c write_file lockfile.lua is 0. lockfile.lua:156-188
Lock: vim.loop.fs_rename(temp_path, lock_path) The code calls os.rename. grep -c fs_rename lockfile.lua is 0. lockfile.lua:195
Lock: a VimLeavePre autocmd in lockfile.lua calls fs_unlink(lock_path) lockfile.lua has no autocmd. Its fs_unlink calls remove only the temp file. The lock file goes through remove with os.remove, which stop calls. The exit autocmd is in init.lua. grep -c VimLeavePre lockfile.lua is 0. lockfile.lua:191, :197, :221. init.lua:514-522, :642
Diff: "Create temp file with proposed changes" with vim.fn.tempname() and write_file(temp_file, new_content) openDiff writes no temp file. The proposal is a scratch buffer. write_file appears nowhere under lua/. The only file write in diff.lua is io.open(tmp_file, "w") in create_temp_file, which the legacy open_diff calls. tempname() seeds part of the session folder name. Probe: no claudecode_diffs* folder after an openDiff diff. diff.lua:523-524, :553, :1730-1748
Diff: edit original_file, diffthis, vsplit temp_file, diffthis The old file goes in a window with diffthis. create_split makes a split to the right (vertical) or below (horizontal). The proposal buffer goes there with diffthis. Probe: right for vertical, below for horizontal. diff.lua:279-288, :715-730
Diff: "Open diff in current tab to reduce clutter" The current tab is preferred and not guaranteed. A new tab opens when open_in_new_tab is set or the picked tab or the current tab already has a diff window. With no usable editor window in the current tab, the diff uses one in another tab. Probe: 1 tab by default, 2 tabs for each of the two cases. diff.lua:125-136, :1625, :1700. tests/integration/diff_current_tab_spec.lua:136
Diff: vim.keymap.set("n", "<leader>da", accept_all_changes) and <leader>dq with exit_diff_mode The plugin sets no diff keymap. accept_all_changes and exit_diff_mode do not exist. Accept and reject go through :w, the two commands, and closing the proposal. grep keymap diff.lua diff_inline.lua finds nothing. init.lua:1209-1219. Probe of all five accept paths and three reject paths
Diff: "Native Neovim diff implementation" True for vertical and horizontal. It is incomplete: unified is an inline renderer and not a Neovim diff. Probe: 0 windows with 'diff' set for unified. diff.lua:1595

Scope

Only these two sections of ARCHITECTURE.md. I did not audit the rest of the file.

The line numbers in this description refer to main at 5acc1002. ARCHITECTURE.md cites symbols and no line numbers, so later commits to diff.lua do not make it stale. #542 added three lines to diff.lua after this branch left main.

Blast Radius

One documentation file. Nothing loads it. A wrong sentence here misleads a contributor and breaks no behavior.

Verification

I ran the real modules in headless Neovim 0.12.3 with an isolated home (env -i, private HOME, XDG_* and CLAUDE_CONFIG_DIR, the stub claude first on PATH).

Lock file section:

  • A new lock directory has mode 0700 and the lock file has mode 0600.
  • The file holds the keys authToken, ideName, pid, transport and workspaceFolders, with ideName Neovim and transport ws.
  • No temp file stays after create.
  • fs_open "wx" fails with EEXIST for an existing file and for a dangling symlink.
  • remove returns Lock file does not exist for a missing file and Invalid port number for a bad port.
  • With CLAUDE_CONFIG_DIR empty or unset, the lock directory is ~/.claude/ide.

Diff System section:

  • vertical: the proposal is right of the old file. horizontal: below it. Both windows have 'diff' set, there is 1 tab, and the proposal buffer is acwrite and unlisted. No claudecode_diffs folder exists.
  • :w, :wq, :x, ZZ and :ClaudeCodeDiffAccept each reply FILE_SAVED with the new text, and the target file is unchanged.
  • :ClaudeCodeDiffDeny, closing the last window of the proposal, and bwipeout of its buffer each reply DIFF_REJECTED.
  • A ClaudeCodeDiffOpened handler that rejects the diff makes open_diff_blocking return DIFF_REJECTED at once: the coroutine is dead after its first resume and never yielded.
  • :ClaudeCodeDiffDeny and :ClaudeCodeDiffAccept in the old-file buffer only warn No active diff found in current buffer, and the diff stays pending.
  • The proposal buffers are named n-exist (proposed), n-new (NEW FILE - proposed) and u1 (unified diff) for an existing file, a new file and the unified layout.
  • After :w and close_saved_diffs, the target file is unchanged and its buffer holds the accepted text as unsaved changes. When the buffer already has unsaved changes, the diff stays open (kept says its buffer has unsaved changes) and the plugin does not touch the buffer.
  • A dirty target buffer makes open_diff_blocking raise Diff setup failed. The coroutine is dead after its first resume.
  • A new-file diff started from an empty buffer reuses that buffer as its old side. Started from a file buffer, it shows an empty nofile placeholder named <path> (NEW FILE).
  • open_in_new_tab = true gives 2 tabs. A diff already in the current tab gives 2 tabs. unified gives 1 tab, 0 windows with 'diff' set, and an inline diff buffer.
  • The legacy open_diff writes target.txt.new under stdpath("cache")/claudecode_diffs/<session folder>.

Checks:

  • Citation check over the seven READMEs and ARCHITECTURE.md (I added the file to the validator's list): 787 citations, 1 flagged. The flagged one is [plugin/claudecode.lua:pcall(require, "claudecode")], an expression that main already cites. A negative control with two wrong symbols flagged both.
  • mise run format-check: exit 0 (355 files, 0 changed).
  • I did not run mise run all. The change is docs only, and no test, script or CI file reads ARCHITECTURE.md (I checked tests, scripts, .github and mise.toml). CI runs the full suite on the pushed head.
  • git merge-tree --write-tree of the head and origin/main is clean.

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

The sketch showed a VimLeavePre autocmd and an fs_unlink in lockfile.lua, a
write_file helper, a fixed '.tmp' name and fs_rename. None of these exist.
lockfile.lua has no autocmd. create writes '<port>.lock.tmp.<pid>.<hrtime>'
with fs_open 'wx' (mode 0600) and renames it with os.rename. remove uses
os.remove. The exit cleanup is the ClaudeCodeShutdown autocmd in init.lua,
which calls stop. The section is now prose with citations and a shortened
excerpt whose code lines are verbatim.
… code

The sketch wrote a temp file with a write_file helper and set <leader>da and
<leader>dq keymaps. Neither exists. The openDiff tool calls open_diff_blocking,
which shows the proposal in an unlisted acwrite scratch buffer and writes no
temp file. The plugin sets no diff keymaps: :w or :ClaudeCodeDiffAccept
accepts, and :ClaudeCodeDiffDeny, closing the last window of the proposal or
deleting its buffer rejects. Only the legacy open_diff writes a temp file, under
stdpath('cache')/claudecode_diffs, and tempname() only seeds the session
folder name. The unified layout is drawn by diff_inline.lua and is not a
Neovim diff.
#537 added a sentence after the lock file sketch and qualified the Resource
Cleanup comment. The sentence conflicts with the rewritten lock file section.
The lifecycle bullet of this branch says the same thing with citations, so it
stays. The comment fix of #537 is kept.
@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-12T04:42:38.757089Z e87cdde Manual request
🔒 Security Review ✅ Completed 2026-11-12T04:42:20.404336Z e87cdde 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: a92d2fb826

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 chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a92d2fb826

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

Comment thread ARCHITECTURE.md Resolved
Comment thread ARCHITECTURE.md Resolved
Comment thread ARCHITECTURE.md Resolved
Comment thread ARCHITECTURE.md Resolved
Comment thread ARCHITECTURE.md Resolved
Codex round 1, five P2 findings, all valid:
- open_diff_blocking does not yield when a ClaudeCodeDiffOpened handler
  resolves the diff during setup (it returns the result at once).
- The proposal buffer name depends on the layout: '(proposed)' for a native
  diff of an existing file, '(NEW FILE - proposed)' for a new file and
  '(unified diff)' for the unified layout.
- A native diff prefers the current tab and uses an editor window in another
  tab only when the current tab has none (windows_current_tab_first).
- lockfile.create only tries to remove its temp file: the result of the
  fs_unlink call is ignored.
- Claude writing the accepted file is the normal flow. When its session ends
  first, :ClaudeCodeCloseAllDiffs puts the accepted text into the buffer.
Also: the old side of a new-file diff is a placeholder, and the two diff
commands read the diff from the current buffer, so they warn elsewhere.
@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 Security Review

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

Reviewed commit: df7e05a0a9

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 chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: df7e05a0a9

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

Comment thread ARCHITECTURE.md Resolved
Comment thread ARCHITECTURE.md Resolved
Codex round 2, two P2 findings, both valid:
- _setup_blocking_diff opens a new tab when the picked tab or the current tab
  already has a diff window (diff.lua:1700), not only the picked tab.
- :ClaudeCodeCloseAllDiffs puts the accepted text into the file's buffer only
  when that buffer can take it. A diff whose buffer has unsaved changes stays
  open (close_saved_diffs, put_accepted_text_in_buffer).
Also: a setup failure raises an error and does not yield, and the old side of
a new-file diff is an empty buffer (a placeholder, or a reused empty buffer).
@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 Security Review

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

Reviewed commit: e87cdde830

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. You're on a roll.

Reviewed commit: e87cdde830

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

@ThomasK33
ThomasK33 added this pull request to the merge queue Nov 12, 2026
Merged via the queue into main with commit 0edd653 Nov 12, 2026
8 checks passed
@ThomasK33
ThomasK33 deleted the docs/architecture-lockfile-sketch branch November 12, 2026 05:13
hareki pushed a commit to hareki/claudecode.nvim that referenced this pull request Nov 13, 2026
…oder#546)

## Why

`README.md` says "Accept: `:w` (save) or `<leader>aa`" and "Reject: `:q`
or `<leader>ad`", and tells the reader to use `<leader>as` to send a
selection or a tree file. A reader takes these as plugin behavior. The
plugin sets none of the three keys. They are mappings in the lazy.nvim
spec of the Installation section (`README.md:58`, `:60-63`, `:66-67`).

## What changed

Three places in `README.md` and one in a fixture README. The new text
names the command and says where the key comes from.

| Line | Old | New | Evidence |
|---|---|---|---|
| `README.md:214` | "use `<leader>as` to send it to Claude" | "run
`:ClaudeCodeSend` ... (the `keys` in the Installation spec map
`<leader>as` to it)" | `:ClaudeCodeSend` is defined at
`lua/claudecode/init.lua:946`. The spec maps it at `README.md:58`. |
| `README.md:215` | "press `<leader>as` on a file" | "run
`:ClaudeCodeTreeAdd` on a file ... (the `keys` ... map `<leader>as` to
it in the file types that the spec lists)" | `:ClaudeCodeTreeAdd` is
defined at `init.lua:1049`. The spec maps it for `NvimTree`, `neo-tree`,
`oil`, `minifiles`, `netrw` and `snacks_picker_list` at
`README.md:60-63`. |
| `README.md:251-252` | "Accept: `:w` (save) or `<leader>aa`" and
"Reject: `:q` or `<leader>ad`" | "Accept: `:w` (save) or
`:ClaudeCodeDiffAccept`" and "Reject: `:q` or `:ClaudeCodeDiffDeny`",
then a paragraph: run these in the proposed buffer, the plugin sets no
keymaps for the diff, and the spec maps `<leader>aa` and `<leader>ad` to
the two commands | The commands are defined at `init.lua:1209` and
`:1216`. The spec maps the keys at `README.md:66-67`. |
| `fixtures/issue-289/README.md:58` | "the README-default `<leader>as`"
| "the `<leader>as` mapping from the README's Installation spec, which
this fixture sets" | The fixture sets the key itself
(`fixtures/issue-289/init.lua:100`). |

The commands and `:w` and `:q` behave as the new text says. I ran the
real module in headless Neovim 0.12.3 (isolated home): `:w` accepts and
`:q` in the proposal window rejects, in the vertical and the unified
layout. `:ClaudeCodeDiffAccept` and `:ClaudeCodeDiffDeny` act on the
diff of the current buffer, so the new text says to run them in the
proposed buffer (they only warn elsewhere).

## Scope

I grepped every tracked Markdown file for `<leader>`, modifier keys and
the words keymap, keybinding, shortcut, mapped and bound.

- **Plugin keymaps.** The plugin sets no `<leader>` key. `grep -rn
'<leader>' lua plugin --include=*.lua` finds nothing. Its only keymap
calls are `<C-z>` in terminal mode (`lua/claudecode/utils.lua:28`) and
the Snacks instance keys (`terminal/snacks.lua:626`, `:640`). The diff
modules set no keymap. The README statements about `<C-z>` and the
Snacks keys are therefore correct and I did not touch them.
- **Example blocks.** Every other key in `README.md` (`<leader>ac`,
`<C-,>`, `<M-,>`, `<C-h>` and the rest) sits inside an explicit example
config. They claim nothing about the plugin.
- **Fixture READMEs.** I checked each `<leader>` mention in them against
the fixture's own Lua. All 25 appear there. For the diff and send keys I
also read the mapping: `fixtures/repro/init.lua:78-79`,
`fixtures/remote-diff/init.lua:96-97`,
`fixtures/issue-238/init.lua:144-145` and
`fixtures/issue-289/init.lua:100` call `vim.keymap.set`, and
`fixtures/snacks-explorer/lua/plugins/dev-claudecode.lua:18-19` uses a
lazy `keys` entry. So these READMEs describe the fixture and are
correct.
- **Not changed.**
- `ARCHITECTURE.md:111-113` shows `<leader>da` and `<leader>dq` keymaps
that nothing sets. PR coder#543 rewrites that section.
- `fixtures/issue-289/init.lua:20` has the code comment "the
README-default `<leader>as` keymap". It is code, not a README.
- `CHANGELOG.md` and `scripts/fixtures/changelog-released.md` mention
`<leader>as`. They are release notes of past versions.
- `dev-config.lua` (the local development spec, described in the Local
Interface & Invariants section of `README.md`) maps `<leader>a*` keys.
It is documented as a spec and claims no default.

## Blast Radius

Two Markdown files. Nothing loads them. A wrong sentence here misleads a
reader and breaks no behavior.

## Verification

- `mise run format-check`: exit 0 (358 files, 0 changed).
- Citation check over the seven READMEs and `ARCHITECTURE.md`: 764
citations, 1 flagged. The flagged one is
`[plugin/claudecode.lua:pcall(require, "claudecode")]`, an expression
that `main` already cites. This change adds no citation.
- I did not run `mise run all`. The change is documentation only, and no
test, script or CI file reads these files. CI runs the full suite on the
pushed head.
- `git merge-tree --write-tree` of the head and `origin/main` is clean.

---
_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
## Why

The Performance & Security list of `ARCHITECTURE.md` says "Async I/O:
Non-blocking vim.loop operations". That is too broad. Sockets and timers
use libuv callbacks, but file I/O is synchronous. Two sibling bullets
have the same problem: "Memory Efficient: Minimal footprint, no caching"
and "Resource Cleanup: Automatic on vim exit". The advisor on coder#543 also
asked for two small additions in the same file.

## What changed

Five lines of `ARCHITECTURE.md`.

| Statement | Old | New | Evidence |
|---|---|---|---|
| Async I/O | "Non-blocking vim.loop operations" | "Async and Blocking
I/O": sockets and timers use libuv callbacks, file I/O is synchronous |
The TCP server uses `listen`, `read_start` and a ping timer with
callbacks (`tcp.lua:120`, `:207`, `:534`) and `tcp_handle:write`
(`client.lua:84`, `:90`). Timers: `selection.lua:506`, `init.lua:137`.
File I/O: `lockfile.lua` calls `fs_open`, `fs_write` and `fs_close` with
no callback (`:157`, `:172`, `:184`) and uses `os.rename` and
`os.remove` (`:195`, `:221`). `diff.lua` uses `io.open` (`read_file`
`:492`, `create_temp_file` `:553`). Every `fs_*` call under `lua/` is in
`lockfile.lua` and has no callback (`fs_open`, `fs_write`, `fs_close`,
`fs_unlink`, `fs_chmod`). |
| Memory | "Minimal footprint, no caching" | "In-memory State": the
plugin keeps, among other things, the latest selection, the proposed
text of each open diff, an input buffer for each client and the
@-mentions that wait to go out, and reads `CLAUDE_CONFIG_DIR` once at
load | `selection.lua:315` (`latest_selection`), `diff.lua:1774` and
`:1026` (`new_file_contents`, `active_diffs`), `client.lua:54`
(`buffer`), `init.lua:51` (`mention_queue`), `lockfile.lua:20`
(`M.lock_dir = get_lock_dir()`). The text had no number to support
"minimal footprint", so the new bullet states what is kept. |
| Resource Cleanup | "Automatic on vim exit" | A `VimLeavePre` autocmd
runs `stop` when the server runs, `stop` stops the server and tries to
remove the lock file, and a SIGKILL skips it and leaves the lock file |
`init.lua:514-522` and `:635-648`. Probe below. |
| Lock directory bullet (coder#543 advisor) | no citation | adds
`[lua/claudecode/lockfile.lua:M.create]` | `lockfile.lua:94`, `:104` |
| Reject list (coder#543 advisor) | closing the last window or deleting the
buffer | adds switching that window to another buffer (`:edit`,
`:enew`), which rejects through `BufHidden` on the next loop turn and
only if the diff is still the same one and the proposal is still hidden
| `diff.lua:1189-1226` (`_register_reject_when_hidden`). Probe below. |

## Scope

Only the Performance & Security list, and the two bullets from the coder#543
advisor.

- **Other bullets of the list.** "Debounced Updates" holds (`debounce_ms
= 100`, `selection.lua:318`). "Localhost Only" holds (both binds use
`127.0.0.1`, `tcp.lua:86` and `:114`).
- **Thread Safety** (Key Implementation Patterns, not changed). It says
all Neovim API calls from async contexts use `vim.schedule()`. I read
every callback in `server/`: it touches Neovim only through
`vim.schedule` or the logger. The selection and mention timers wrap
their work in `vim.schedule`. I found no counterexample.
- **Not audited.** The rest of the file, including the Overview ("zero
external dependencies") and the WebSocket "Key implementation details".

## Blast Radius

One documentation file. Nothing loads it. A wrong sentence here misleads
a contributor and breaks no behavior.

## Verification

I ran the real modules in headless Neovim 0.12.3 with an isolated home.

- **Exit cleanup.** A real server started by `setup` with `auto_start`,
then `:qa!`: the lock file exists before and 0 lock files remain after.
Nothing but the `VimLeavePre` autocmd of `setup` stops the server there.
- **SIGKILL.** `scripts/repro_issue_70_probe.lua`, then `kill -KILL` of
the `nvim` that the case started: the lock file stays (1 left). A normal
stop leaves 0.
- **Switch reject.** In the proposal window, `:edit <other file>` and
`:enew` each make `open_diff_blocking` reply `DIFF_REJECTED`. This holds
in the `vertical` and the `unified` layout. The reply does not exist
right after the command and arrives on the next loop turn, as the
`vim.schedule` in the `BufHidden` handler says.

Checks:

- Citation check over the seven READMEs and `ARCHITECTURE.md`: 806
citations, 1 flagged. The flagged one is
`[plugin/claudecode.lua:pcall(require, "claudecode")]`, an expression
that `main` already cites. A negative control with two wrong symbols
flagged both (bad 3, then 1 after the restore).
- `mise run format-check`: exit 0 (358 files, 0 changed).
- I did not run `mise run all`. The change is documentation only, and
nothing in `tests`, `scripts`, `.github` or `mise.toml` reads
`ARCHITECTURE.md`. CI runs the full suite on the pushed head.
- `git merge-tree --write-tree` of the head and `origin/main` is clean.

---
_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
… missing (coder#557)

## Why

The goal: the lock file code must tell a lock file that exists from one
that is missing, whether or not the plugin can read it, and
`lockfile.remove` must never delete anything that is not a lock file.
`lockfile.remove`, `lockfile.get_auth_token` and `:checkhealth
claudecode` checked for the file with `vim.fn.filereadable`. For a lock
file with mode 000, `filereadable` is 0, so all three said that the lock
file did not exist.

1. `remove` skipped a delete that the system can do. Deleting a file
needs write permission on its folder, not read permission on the file.
`stop` only logs the error as a warning, so the lock file stayed and the
warning was false. The coder#537 advisor's probe found this (case 4: lock
file mode 000, file stays, warning "Lock file does not exist").
2. `get_auth_token` must still refuse a file that it cannot read, but
its error said that the file did not exist.
3. `:checkhealth claudecode` reported "Lock file missing" for such a
file, so a user who ran it was sent to restart the integration instead
of fixing a permission.

Making `remove` delete such a file also needed a safe delete.
`os.remove` is the C `remove`, which deletes an empty folder. Another
process of the same user can replace the lock file with an empty folder
between the check and the delete. `filereadable` did not stop that
either. It checks for a folder and for read permission at two moments,
so a folder that appears between them passes as readable, and
`os.remove` deletes it. The round b verifier ran a second process that
swaps the file and a folder in a loop: main deleted 73,836 folders in 20
s with a mode-000 file (Neovim 0.12.3). So the delete itself must refuse
a folder.

## What changed

1. `lockfile.remove` looks for the name with `vim.loop.fs_lstat` (it
works on Neovim 0.8) and deletes it with `vim.loop.fs_unlink` instead of
`os.remove`. `fs_unlink` refuses a folder (`EISDIR`), so the delete
never removes a folder, also when another process puts one there after
the check. `fs_lstat` does not follow a link, so a link at the lock
path, even one that points nowhere, is a name that `remove` deletes. It
deletes the link and never its target. "Lock file does not exist" is
returned only when `fs_lstat` finds nothing or a folder. The error
returns keep their shape: a raise or a nil result from the delete gives
`false, "Failed to remove lock file: <message>"`. The message is now
libuv's ("EACCES: permission denied: <path>") and not the C library's.
2. `lockfile.get_auth_token` tells the cases apart with
`vim.loop.fs_stat`. A missing path or a folder gives "Lock file does not
exist: <path>", as before. A file that is there and that `io.open`
cannot open gives "Lock file exists but cannot be read: <path>:
Permission denied" (the system message). It still returns `false, nil,
error`. The old "Failed to open lock file" error appeared when `io.open`
failed after `filereadable` accepted the path, including a bound Unix
socket without a race. The new error replaces it.
3. `lua/claudecode/health.lua` keeps `filereadable` for "Lock file
present". When it is 0, `vim.loop.fs_stat` tells the cases apart: a file
that is there gives the error "Lock file exists but cannot be read:
<path>" with advice to make it readable, and nothing or a folder gives
"Lock file missing" as before.
4. Tests in `tests/lockfile_test.lua`, with real files in the suite's
private temp home. For `remove`:
   - an unreadable lock file is deleted and `remove` returns success,
- a missing file gives the "does not exist" error, and a folder named
like a lock file stays,
   - a read-only folder gives the `fs_unlink` error and the file stays,
- a folder that takes the place of the file after the existence check
stays, and `remove` returns false (for a file with mode 000 and one with
mode 600),
- a link that points nowhere, a link to the lock file of another port, a
hard link and a link to a folder lose only their name, and the target
stays,
- a FIFO and a socket at the lock path are deleted, as before, and the
tests assert that `remove` returns success and that the name is gone,
- the failure injection mocks `fs_unlink`, the call that is used: a nil
result with an error, and a raise, both give `false` with "Failed to
remove lock file".

For `get_auth_token`: a missing file gives the exact "does not exist"
message, an unreadable file is refused with the "cannot be read" message
(no token, no "does not exist"), a FIFO with mode 000 and a socket give
the same message, and a folder and a link that points nowhere give the
"does not exist" message. In `tests/unit/health_spec.lua`: a lock file
that is there but unreadable gives "exists but cannot be read", and a
folder at the lock path gives "Lock file missing". The mode 000 tests,
the FIFO test and the read-only test are pending when the user is root,
who can read and write anyway. The FIFO and socket tests are pending
where `mkfifo` or `python3` is missing. The tests assert on messages and
booleans only, never on a token or on the content of a lock file.
5. Test mocks changed. The `filereadable` mock says what Neovim says
(`:h filereadable()`: 1 only for a file that exists, can be read and is
not a folder) with the shell's `test -r` and `test ! -d`, so it never
opens the file (opening a FIFO waits for a writer). The `vim.loop` mock
gets an `fs_stat` and an `fs_lstat` that need no read permission on the
file and know a file, a folder, a FIFO, a socket and a link. Its
`fs_unlink` calls `unlink(2)` through `ffi`, so it refuses a folder and
fails like libuv, where it used to call `os.remove` and always return
true.
6. Docs. `ARCHITECTURE.md` said that `remove` "returns an error when no
readable lock file exists" (added by coder#543). That is false now. It says
that `remove` deletes the file with `vim.loop.fs_unlink`, which refuses
a folder, returns an error when `fs_lstat` finds nothing or a folder and
when `fs_unlink` fails, still deletes a mode 000 lock file, and removes
a link as a link. Its line on blocking I/O names `fs_unlink`, `fs_stat`,
`fs_lstat` and the `io.open` of `get_auth_token`, and no longer
`os.remove`. `lua/claudecode/README.md` says how `lockfile.remove` finds
and deletes the name, has a line for the two errors of
`lockfile.get_auth_token`, and says that a lock file in a lock folder
that cannot be searched gives "does not exist", because `fs_lstat` and
`fs_stat` find nothing. It also names the three states of the health
check (present, missing, there but not readable). `README.md` says that
`:checkhealth claudecode` checks that the lock file exists and can be
read. The lines that say that removal can fail stay true, so I did not
touch them (`README.md` around line 1126 and 1154).
7. `covered.lock` is re-recorded for the new hashes of `lockfile.lua`
and `health.lua`. No block and no call site changed.

## Scope

Only `lockfile.remove`, `lockfile.get_auth_token` and the lock file line
of `:checkhealth claudecode`, their tests and the doc lines.
`get_auth_token` has no caller in `lua/`. It is part of the module's
API, and the tests are its only users here.

Not changed, with the reason for each:

- `get_auth_token` still opens a readable FIFO at the lock path with
`io.open`, which waits for a writer. Main does the same, and no user
flow puts a FIFO there. This head also reaches it when another process
swaps a file with mode 000 for a FIFO after the `fs_stat` call.
`get_auth_token` has no caller in `lua/`.
- `remove` deletes the name `<port>.lock` in the lock folder as it
resolves at that moment. If another process replaces the lock folder
itself with a link to another folder between `fs_lstat` and `fs_unlink`,
`remove` deletes a `<port>.lock` file there (never a folder). Main does
the same for a readable file. Only the Neovim that holds a port owns a
live lock file for it, and `stop` removes the lock file before the
server stops, so no live lock file is lost. The other process needs
write access to the config folder. A fix needs `unlinkat` with an open
folder handle.
- `:checkhealth claudecode` says "Lock file present" for a Unix socket
at the lock path, because `filereadable` is 1 for it. Main says the
same, and no user flow puts a socket there.

## Blast Radius

`lockfile.remove` runs from `stop` and from the token-mismatch path of
`start` in `init.lua`. It differs from main in these ways:
- A lock file that exists but cannot be read (mode 000) is now deleted.
Before, it stayed and `stop` logged a wrong warning.
- Any other thing that is not a folder is now handed to the delete.
`filereadable` also refused a file that it could not read, and nothing
else.
- A link at the lock path is removed as a link and never its target. A
link to an existing readable file was removed this way before. Now a
link to a file that cannot be read, a link that points nowhere and a
link to a folder are removed too, and their targets stay.
- The delete is `fs_unlink`, which refuses a folder. On main, a folder
that another process puts at the path after a readable file was checked
is deleted when it is empty (`os.remove`). Here it stays, and `remove`
returns false.
- The text of a delete failure is libuv's ("EACCES: permission denied:
<path>") and not the C library's ("<path>: Permission denied"). The
prefix "Failed to remove lock file" and the `false, error` shape are the
same.

`lockfile.get_auth_token` differs in two ways. First, unreadable regular
files now give the "cannot be read" error instead of "does not exist",
and paths that `filereadable` accepted but `io.open` rejected, such as
bound Unix sockets, now give that error instead of "Failed to open lock
file". The failure tuple remains `false, nil, error`, so no caller that
checks the first value changes. Second, a file that becomes readable
between the `fs_stat` call and `io.open` (another process runs `chmod`,
or renames a readable file over the path) now gives its token, where
main said that the file did not exist. `get_auth_token` reads what is at
the path when it opens it, as main does for a readable file, and no code
in `lua/` calls it.

`:checkhealth claudecode` differs in one way: a lock file that exists
but cannot be read gives the error "Lock file exists but cannot be read:
<path>" and not "Lock file missing". A readable lock file still gives
"Lock file present", and nothing or a folder at the path still gives
"Lock file missing".

A lock file that exists in a lock folder that cannot be searched (mode
000 on the folder) is the one case that all three still report as
missing: the stat call finds nothing there, as `filereadable` did. The
README says so.

Unchanged: a missing path gives the same error in both, a folder gives
the same error and stays, a read-only folder gives an error from the
delete and the file stays, a raise or a nil result from the delete gives
the same shape of error, and a readable lock file gives the same token.

The diff-lifecycle check does not apply: `lockfile.lua` and `health.lua`
cannot reach diff state, an openDiff answer, a disconnect or a diff
command. `check.sh` still passes (`main`: 89,205 distinct states,
`live-DisconnectedCallsAnswered`: 5,269), and `covered.lock` is
re-recorded.

## Security

The lock file holds the auth token. This change does not change who can
read it, where it lives, or what the plugin reads:
- Who can read it: unchanged. The plugin writes it with mode 0600, and
this change adds no read. The stat calls read no content.
`get_auth_token` still refuses a file that it cannot read, and it reads
only a file that `io.open` opens, as before.
- Where: unchanged. The path is still `lock_dir`, a slash, the port
number and `.lock`. A link at that path is removed as a link and its
target stays: a link to an existing readable non-directory was removed
this way before, and now a link to an unreadable target, a dangling link
and a link to a folder are removed too, with their targets left alone (a
test for each).
- How long: shorter in one case. A lock file that the plugin cannot
read, with a token for a server that has stopped, is deleted now.
Before, it stayed on disk.
- Deleting only a name: `remove` can no longer delete a folder,
including one that another process of the same user puts at the path
after the check, because `fs_unlink` refuses a folder. That process
needs write access to the private lock folder already, and this is not
an escalation. It closes a way to delete an empty folder that is not a
lock file.
- What an error shows: the new `get_auth_token` error holds the path and
the system message from `io.open` ("Permission denied"). The new health
error holds the path. Neither holds a token or any content.
- The tests and the probes print booleans and messages only. They never
print a token or any content of a lock file.

## Verification

Red on main, green here. The same test files run with earlier code and
with the fix:

| Step | Result |
|---|---|
| `busted tests/lockfile_test.lua`, `remove` tests only, main's
`lockfile.lua` | `19 successes / 1 failure`: `remove returned false:
Lock file does not exist: …/12353.lock` (the unreadable-file test) |
| `get_auth_token` tests added, the first `remove` fix only | `22
successes / 1 failure`: `Lock file does not exist: …/12361.lock` (the
unreadable-file test) |
| `busted tests/unit/health_spec.lua`, main's `health.lua` | `9
successes / 1 failure`: the test that an unreadable lock file is not
reported as missing |
| the final `tests/lockfile_test.lua` against the first version of the
fix (`os.remove`, `fs_stat`) | `27 successes / 5 failures`: the folder
that takes the place of the file, the link that points nowhere, the link
to a folder, and the two failure injections of `fs_unlink` |
| the final `tests/lockfile_test.lua` against `get_auth_token` with
`fs_lstat` instead of `fs_stat` | `32 successes / 1 failure`: the link
that points nowhere |
| this head, `busted tests/lockfile_test.lua` | `33 successes / 0
failures / 0 errors / 0 pending` |
| this head, `busted tests/unit/health_spec.lua` | `10 successes / 0
failures / 0 errors / 0 pending` |

The other new tests pass before and after, as controls (a missing file,
a folder, a read-only folder, a hard link, a link to the lock file of
another port, and the FIFO and socket tests that pin what `filereadable`
let through).

The folder race on real files (a real Neovim, not the mock). Another
process runs `rm -f` and `mkdir` on the lock path right after the first
`fs_stat`, `fs_lstat` or `filereadable` call that names it. `remove`
must return false and the folder must stay. Both a file with mode 000
and one with mode 600 are tried:

| Code | Neovim 0.12.3 | Neovim 0.8.0 |
|---|---|---|
| main (`filereadable`, `os.remove`) | mode 000: folder stays. mode 600:
`remove` returns true and deletes the folder | same |
| the first version of this PR (`fs_stat`, `os.remove`) | `remove`
returns true and deletes the folder, for both modes | same |
| this head (`fs_lstat`, `fs_unlink`) | `remove` returns false and the
folder stays, for both modes | same |

The hook acts right after the check. The main row for mode 000 holds for
that order only. Against a second process that swaps the file and a
folder in a loop, main also deletes folders for mode 000 (73,836 in 20
s, round b verifier). This head deleted none in four 20 s runs (Neovim
0.12.3 and 0.8.0, mode 600 and 000).

`vim.loop.fs_unlink` gives the same results on Neovim 0.8.0 and 0.12.3:
it deletes a file with mode 000, a link to a file, a dangling link, a
link to a folder (the folder and its content stay), a FIFO and one name
of a hard link (the other name stays), and it refuses an empty folder
with `EISDIR`, a missing path with `ENOENT`, and a read-only folder with
`EACCES`.

Mutants of `remove` and `get_auth_token` against the final test file
(all killed, the control passes 33 of 33): `remove` back to
`filereadable`, `fs_stat` instead of `fs_lstat`, `os.remove` instead of
`fs_unlink`, no folder check, no existence check, every entry that is
not a regular file treated as missing, an error for every call, a nil
result counted as success, and no `pcall`. `get_auth_token` back to
`filereadable`, `fs_lstat` instead of `fs_stat`, the old "Failed to
open" message, no folder check, the "does not exist" message for an
unreadable file, no system message, a message without the reason, and
every entry that is not a regular file treated as missing.

Real Neovim 0.12.3, real files in a probe folder (the same scripts run
against main's code and the fix, and print booleans and messages only):

| Case | main | here |
|---|---|---|
| `remove`, lock file mode 000 | `false`, "Lock file does not exist",
file stays | `true`, file deleted |
| `remove`, missing file | `false`, "Lock file does not exist" | same |
| `remove`, folder `<port>.lock` | `false`, "Lock file does not exist",
folder stays | same |
| `remove`, read-only lock folder | `false`, "Failed to remove lock
file: …: Permission denied", file stays | `false`, "Failed to remove
lock file: EACCES: permission denied: …", file stays |
| `get_auth_token`, lock file mode 000 | `false, nil`, "Lock file does
not exist" | `false, nil`, "Lock file exists but cannot be read: …:
Permission denied" |
| `get_auth_token`, missing file | `false, nil`, "Lock file does not
exist" | same |
| `get_auth_token`, folder | `false, nil`, "Lock file does not exist" |
same |
| `get_auth_token`, readable file | `true`, a token | same |
| `:checkhealth claudecode`, readable lock file | "Lock file present" |
same |
| `:checkhealth claudecode`, lock file mode 000 | "Lock file missing" |
"Lock file exists but cannot be read" |
| `:checkhealth claudecode`, lock file missing | "Lock file missing" |
same |
| `:checkhealth claudecode`, folder at the lock path | "Lock file
missing" | same |

The `:checkhealth` rows ran with a real server and a real lock file,
with `XDG_*` and `CLAUDE_CONFIG_DIR` in a probe folder.

Gates on this head (`e6e3a170`, with main at `f7f89cb5` merged):
- `mise exec -- busted -v tests/lockfile_test.lua`: `33 successes / 0
failures / 0 errors / 0 pending`. `busted -v
tests/unit/health_spec.lua`: `10 successes / 0 failures / 0 errors / 0
pending`.
- `mise run format-check`: `formatted 426 files (0 changed)`.
- `.agents/skills/verify-diff-lifecycle/check.sh` (full): `RESULT: PASS`
(`main`: 89205 states, `live-DisconnectedCallsAnswered`: 5269 states).
`check.sh --hash-only`: `covered.lock matches the code`.
- `mise run all`: `Total: 0 warnings / 0 errors in 185 files`, `1756
successes / 0 failures / 0 errors / 28 pending`.
- `scripts/run_integration_tests_individually.sh`: `Total Success: 527`,
`Total Failed: 0`, `Total Errors: 0`.
- `git merge-tree --write-tree HEAD origin/main` (main at `523ee200`):
clean. `check.sh --hash-only` on that merge: `covered.lock matches the
code`. The two commits that main has since `f7f89cb5` touch none of the
8 files of this PR.

---
_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