Repository navigation
fix(server): never repeat a client id within one Neovim process - #383
Conversation
Reloading claudecode.server.client resets its id counter, while the diff module keeps the ids of the clients that opened its diffs. A new connection then gets an old id and passes as the owner of a diff the user accepted for a Claude that is gone. This test fails on main with "id client_1 came back after a reload". Part of #363 _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
A diff the user accepted stays tied to the id of the client that opened it. The id counter was a local of claudecode.server.client, so a reload of claudecode.server.* restarted it at 1 while the diff module kept the old ids. After a server restart, a new Claude got the id of a Claude that had exited, passed as the owner of its accepted diff, and closeAllDiffTabs dropped the accepted edits before anything wrote them. The counter now lives in _G.claudecode_clients_created, which a module reload keeps. check.sh reads the new counter for ClientIdsUnique. A Claude CLI that reconnects still gets a new id. Claude Code 2.1.312 sends no session id, and it never refers to an older diff over a new connection, so PROTOCOL.md documents this instead of matching on its ide_connected pid. Part of #363 _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
check.sh --mutants catches every guard, and check.sh passes on this code. The record also takes in the selection, open_file, utils and send_message changes from #367, #375 and #378, which main had not recorded. None of them can reach diff state, an openDiff answer, a disconnect or a diff command. Part of #363 _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
|
@codex review |
|
@codex security review |
Codex Review SummaryThis 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. Delightful! 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. |
Claude Code 2.1.312 sends clientInfo with name, title, version, description and websiteUrl, plus capabilities. None of them names a session. ClientIdsUnique in check.sh now also counts an assignment of the counter written without spaces, through _G["..."], or with rawset. Part of #363 _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
|
@codex review |
|
@codex security review |
|
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. |
The merge combines #383's ClientIdsUnique regex with this branch's UserCloseKeepsAcceptedText. check.sh --mutants on the merged tree catches every guard, and check.sh passes. Part of #363 _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
Why
The operator goal is to clear the open follow-up issues in this repository (#362, #363 and #374 now) with small reviewed PRs. This PR takes #363 items 2 and 3.
:wstays tied to the id of the client that opened it, and no other client may close it. Since fix(diff): keep accepted diffs through reentrant bulk closes and other clients #361 the ids come from a counter, but the counter was a local ofclaudecode.server.client. A reload ofclaudecode.server.*restarted it at 1, whileclaudecode.diffkept the old ids. After a server restart, the next Claude got the id of a Claude that had exited. It then passed as the owner of that Claude's accepted diff, and itscloseAllDiffTabsdropped the accepted edits before anything wrote them to disk.What changed
lua/claudecode/server/client.lua:create_clientcounts in_G.claudecode_clients_created, which a module reload keeps. Ids keep the formclient_<n>.PROTOCOL.md, section "4. Claude Connects": each connection is a client, a reconnect makes a new client, and what Claude Code 2.1.312 sends and does on a reconnect.lua/claudecode/server/README.md: the id invariant and the new global.verify-diff-lifecycle: theClientIdsUniquefact now reads the global counter. It also requires that nothing else inlua/orplugin/assigns that global, with or without spaces, through_G["..."], or withrawset.covered.lockis re-recorded.tests/unit/server/client_id_spec.lua: a test that reloads the module three times and asserts that no id comes back.What Claude Code 2.1.312 sends and does on a reconnect (read from
~/.local/share/claude/versions/2.1.312)ws-idetransport opens the socket with subprotocolmcpand two headers,User-AgentandX-Claude-Code-Ide-Authorization. The token comes from the lock file, so every Claude of this Neovim sends the same one.initializecarriesclientInfo{ name: "claude-code", title: "Claude Code", version: "2.1.312", description, websiteUrl }andcapabilities, the same for every process and with no session id.ide_connectedwith{ pid: process.pid }. The plugin has no handler for it. A pid is not unique across pid namespaces (two Claude CLIs in two containers can both report the same pid), and the OS reuses it. Matching on it could hand one live Claude's accepted diff to another, which is the harm fix(diff): keep accepted diffs through reentrant bulk closes and other clients #361 fixed.openDiffgets thetab_name✻ [Claude Code] <basename> (<first 6 chars of a random UUID>) ⧉, so a new connection does not reuse an old name.openDiffand sends that diff'sclose_tabthrough it. After a reconnect, thatclose_tabfails on the old connection. The new connection never names the old diff.So matching a reconnect to its old id would change nothing for this CLI. The leftover diff is the same as the accepted diff of a Claude that exited (#363 item 1). #376 lets
:ClaudeCodeCloseAllDiffsfinish such a diff.Scope
#363 items 2 and 3 only. Item 2 changes only docs. This PR does not touch
diff.lua, which #376 changes.Blast Radius
claudecode.server.*in a running Neovim and then restart the server. A new Claude can no longer take over an accepted diff of a Claude that exited. Nobody else sees a change: ids keep their form and still count up from 1 in a fresh Neovim._G.claudecode_clients_created.covered.lockwill conflict with fix(diff): let :ClaudeCodeCloseAllDiffs finish an accepted diff #376, because both PRs re-record it. Whichever PR merges second must re-runcheck.sh --recordafter the merge.Verification
busted tests/unit/server/client_id_spec.lua(red)id client_1 came back after a reloadmise run test: 839 successes, 0 failuresverify-claudecode-nvim(below)client_1,closeAllDiffTabsanswersCLOSED_1_DIFF_TABS, and the accepted text is goneclient_2,closeAllDiffTabsanswersCLOSED_0_DIFF_TABS, and the accepted diff staysverify-diff-lifecycle/check.shwith the fix and main's fact regexClientIdsUnique FALSE,main: BROKEN: SavedDiffKeptwith the traceClaudeOpensDiff, UserHandlerResolves, SetupEnds, ClientDisconnects, ClientIdReused, ClaudeOpensDiffcheck.shPASS.main: pass (89205 distinct states found),live: pass (5269 distinct states found)check.sh --mutantsClientIdsUnique=FALSE caught by: SavedDiffKeptcheck.shwith one extra assignment of the counter:_G.claudecode_clients_created=0,_G["claudecode_clients_created"] = 0, orrawset(_G, "claudecode_clients_created", 0)ClientIdsUnique FALSE,main: BROKEN: SavedDiffKeptThe real-Neovim script: Claude calls
openDiff, the user saves it with:w, and Claude getsFILE_SAVED. Claude exits before it writes the file. The user types:luato clearclaudecode.server.*frompackage.loaded, then:ClaudeCodeStopand:ClaudeCodeStarton the same port. A second Claude connects and callscloseAllDiffTabs.Output on main
Output on this PR
covered.lockon main was already stale forselection.lua,tools/open_file.lua,utils.luaand thesend_messagepart ofserver/client.lua(#367, #375, #378). None of those changes can reach diff state, an openDiff answer, a disconnect or a diff command, so the new record covers them too.Part of #363
Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:xhigh