Repository navigation
docs: correct the I/O and cleanup claims in ARCHITECTURE.md - #547
Merged
Merged
Conversation
The Performance and Security list said 'Async I/O: Non-blocking vim.loop operations', 'Memory Efficient: Minimal footprint, no caching' and 'Resource Cleanup: Automatic on vim exit'. Sockets and timers use libuv callbacks, but all file I/O is synchronous (lockfile.lua fs_open, fs_write and fs_close have no callback, and it uses os.rename and os.remove. diff.lua uses io.open). The plugin keeps the latest selection, the proposed text of each open diff, an input buffer for each client and a mention queue, and it reads CLAUDE_CONFIG_DIR once at load. The exit cleanup is a VimLeavePre autocmd that runs stop. A SIGKILL skips it and leaves the lock file. Two items from the review of #543: the lock directory bullet cites lockfile.lua M.create, and the reject list names switching the last proposal window to another buffer (:edit, :enew), which rejects through BufHidden in _register_reject_when_hidden.
Member
Author
|
@codex review |
Member
Author
|
@codex security review |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. 🚀 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. |
ThomasK33
added a commit
that referenced
this pull request
Nov 12, 2026
… the write path The #547 advisor named two items in ARCHITECTURE.md: - The Resource Cleanup sketch called M.stop() with no condition. The real VimLeavePre callback runs stop only when M.state.server is set, and clears the @-mention queue otherwise (init.lua:514-525). The sketch now shows the if and the else, and its lead-in no longer says 'Automatic cleanup'. - The I/O bullet said the TCP server writes with libuv's asynchronous write without a citation. It now cites client.lua write, which calls tcp_handle:write.
This was referenced Nov 12, 2026
hareki
pushed a commit
to hareki/claudecode.nvim
that referenced
this pull request
Nov 13, 2026
… I/O citations (coder#549) ## Why The READMEs name the buffer that holds Claude's proposed text in two ways. `README.md:1144` and `:1148`, `ARCHITECTURE.md` and 8 of the 16 uses in `lua/claudecode/README.md` say "proposal buffer". `lua/claudecode/README.md` and the new paragraph in `README.md:254` (from coder#546) also say "proposed buffer". The advisor of coder#546 asked for one name for one thing. The advisor of coder#547 named two small items in `ARCHITECTURE.md`. This PR takes them too, after coder#546 and coder#547 landed. - The Resource Cleanup sketch calls `M.stop()` with no condition. The real `VimLeavePre` callback runs `stop` only when `M.state.server` is set, and otherwise clears the @-mention queue (`lua/claudecode/init.lua:514-525`). - The I/O bullet says the TCP server "writes with libuv's asynchronous `write`" and cites nothing for it. ## What changed **1. The rename.** Nine words in two files. Each "proposed buffer" becomes "proposal buffer", and nothing else changes (a word-level diff shows 9 changes of "proposed" to "proposal" and no other change). | File | Lines | Count | |---|---|---| | `README.md` | 254 | 1 | | `lua/claudecode/README.md` | 181, 182, 183 (twice), 184, 187 (twice), 208 | 8 | I read each changed sentence in its new form. All nine read correctly, for example "A write of the proposal buffer (`:w`, `:wq`, `:x`, `ZZ` ...)" and "Both layouts keep the proposal buffer's `'bufhidden'` at `hide`". The other phrases in these two files already use "proposal" (`proposal window`, `proposal lines`, `proposal buffers`). `proposed changes` in `README.md` is a different thing and stays. **2. Resource Cleanup sketch** (`ARCHITECTURE.md`). The sketch now shows the `if M.state.server then M.stop() else clear_mention_queue() end` of the real callback, with its `ClaudeCodeShutdown` group. Every code line comes from `init.lua:514-525`, except two: the label `-- init.lua, M.setup (shortened)` and the trailing comment on `M.stop()`, which the old sketch had and which describes `M.stop` correctly. The other comment line is the real comment at `init.lua:520`. The lead-in "Automatic cleanup on shutdown:" had the same unconditional wording, so it now says that the autocmd that `setup` creates runs `stop` only when the server runs, and otherwise only clears the queue, with citations. This agrees with the Performance bullet ("runs `stop` when the server runs"). **3. I/O citation** (`ARCHITECTURE.md`). The write clause now cites `[lua/claudecode/server/client.lua:write]`. `write` is `local function write(client, data, callback)` at `client.lua:81`, and it calls `client.tcp_handle:write` at `:84` and `:90`. ## Scope The two README files above and the two `ARCHITECTURE.md` items. Left unchanged on purpose: - **Other bullets and sketches of `ARCHITECTURE.md`.** Not audited again here. - **Code comments and strings under `lua/`** (9 uses of "proposed buffer" in lowercase, plus `Proposed buffer ID` at `diff.lua:1187`). They sit in `diff.lua` (8) and `init.lua` (1). Both files are recorded in `covered.lock`, so a comment edit needs a new record, which is too much for a wording fix. - **`fixtures/issue-218/README.md` (2) and `fixtures/issue-238/README.md` (7).** They describe repro scripts. The sample output in `fixtures/issue-238/README.md` (`proposed buffer bufhidden = "hide"`) is what `scripts/repro_issue_238.lua` prints (`:260` and `:279`). A rename of the prose alone leaves two names in one README, and a rename of the output changes a script string. They stay as they are, because the fixture READMEs record what those scripts print. ## Blast Radius Three Markdown files. Nothing loads them. A wrong sentence here misleads a reader and breaks no behavior. ## Verification - Word-level diff of the two READMEs against `main`: exactly 9 changes of "proposed" to "proposal", no other change. - `grep` for "proposed buffer" in `README.md`, `lua/claudecode/README.md` and `ARCHITECTURE.md` finds 0 after the change. - The merge of `main` (coder#547 and coder#538) was clean and added no "proposed buffer" to a README. - `mise run format-check`: exit 0 (358 files, 0 changed). - Citation check over the seven READMEs and `ARCHITECTURE.md`: 809 entries (733 citations, 26 relative links and 50 test paths), 1 flagged. The flagged one is `[plugin/claudecode.lua:pcall(require, "claudecode")]`, an expression that `main` already cites. A negative control with three wrong symbols in the new citations flagged all three (bad 4, then 1 after the restore). The validator matches a plain name such as `write` only as a whole word anywhere in the file, so I also confirmed the definitions of `write` (`client.lua:81`) and `clear_mention_queue` (`init.lua:150`) with `grep`. - 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`_
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The Performance & Security list of
ARCHITECTURE.mdsays "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 #543 also asked for two small additions in the same file.What changed
Five lines of
ARCHITECTURE.md.listen,read_startand a ping timer with callbacks (tcp.lua:120,:207,:534) andtcp_handle:write(client.lua:84,:90). Timers:selection.lua:506,init.lua:137. File I/O:lockfile.luacallsfs_open,fs_writeandfs_closewith no callback (:157,:172,:184) and usesos.renameandos.remove(:195,:221).diff.luausesio.open(read_file:492,create_temp_file:553). Everyfs_*call underlua/is inlockfile.luaand has no callback (fs_open,fs_write,fs_close,fs_unlink,fs_chmod).CLAUDE_CONFIG_DIRonce at loadselection.lua:315(latest_selection),diff.lua:1774and: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.VimLeavePreautocmd runsstopwhen the server runs,stopstops the server and tries to remove the lock file, and a SIGKILL skips it and leaves the lock fileinit.lua:514-522and:635-648. Probe below.[lua/claudecode/lockfile.lua:M.create]lockfile.lua:94,:104:edit,:enew), which rejects throughBufHiddenon the next loop turn and only if the diff is still the same one and the proposal is still hiddendiff.lua:1189-1226(_register_reject_when_hidden). Probe below.Scope
Only the Performance & Security list, and the two bullets from the #543 advisor.
debounce_ms = 100,selection.lua:318). "Localhost Only" holds (both binds use127.0.0.1,tcp.lua:86and:114).vim.schedule(). I read every callback inserver/: it touches Neovim only throughvim.scheduleor the logger. The selection and mention timers wrap their work invim.schedule. I found no counterexample.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.
setupwithauto_start, then:qa!: the lock file exists before and 0 lock files remain after. Nothing but theVimLeavePreautocmd ofsetupstops the server there.scripts/repro_issue_70_probe.lua, thenkill -KILLof thenvimthat the case started: the lock file stays (1 left). A normal stop leaves 0.:edit <other file>and:eneweach makeopen_diff_blockingreplyDIFF_REJECTED. This holds in theverticaland theunifiedlayout. The reply does not exist right after the command and arrives on the next loop turn, as thevim.schedulein theBufHiddenhandler says.Checks:
ARCHITECTURE.md: 806 citations, 1 flagged. The flagged one is[plugin/claudecode.lua:pcall(require, "claudecode")], an expression thatmainalready 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).mise run all. The change is documentation only, and nothing intests,scripts,.githubormise.tomlreadsARCHITECTURE.md. CI runs the full suite on the pushed head.git merge-tree --write-treeof the head andorigin/mainis clean.Generated with
xum• Model:anthropic:claude-sonnet-5-5• Thinking:xhigh