fix(terminal): exclude loopback from no_proxy so Claude's IDE socket isn't proxied (#70)#268
Conversation
…isn't proxied (#70) When http_proxy/all_proxy is set without a localhost exclusion, the launched Claude CLI tunnels even its ws://127.0.0.1:<port> IDE connection through the proxy (proxy-from-env semantics), so the WebSocket never reaches the plugin's server and queued @ mentions clear with the error "[ClaudeCode] [queue] [ERROR] Connection timeout - clearing N queued @ mentions". get_claude_command_and_env now guarantees localhost/127.0.0.1/::1 are present in both no_proxy and NO_PROXY for the Claude terminal. The injection runs after the `env` config merge and folds in every existing source -- the inherited shell no_proxy/NO_PROXY and any user-set `env` config value -- so existing exclusions are preserved and the loopback exclusion always holds. Change-Id: If7ae20158b75038692cde095c9d58c6834a731e0 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas Kosiewski <tk@coder.com>
fixtures/issue-70/ drives the real plugin under a proxy to reproduce the exact "Connection timeout - clearing N queued @ mentions" symptom (:Issue70Send, with notifications teed to ISSUE70_LOG). scripts/repro_issue_70.sh + _probe.lua reproduce the proxy root cause against the real Claude CLI via agent-tty (baseline connects / proxy fails / no_proxy fix connects), isolating the bug to a single fact: whether Claude opens its WebSocket back to the plugin's server. Change-Id: Ie78856d7b7abcbc8567f5cd8dd15164474640bd1 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas Kosiewski <tk@coder.com>
|
@codex review |
|
@claude review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4fe8a399a1
ℹ️ 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".
The interactive fixture loads the plugin from the repo, so with the fix present the proxy scenario now connects instead of timing out. Document the pre-fix vs with-fix outcomes and note the automated harness is unaffected (it bypasses the plugin's env injection). Change-Id: I4b9d1a171b1b25c3e74677760e33491add7115fa Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas Kosiewski <tk@coder.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! ℹ️ 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". |
|
@claude review |
Summary
Addresses the reproducible, plugin-side root cause of #70 — sending files/buffer/lines failing with
[ClaudeCode] [queue] [ERROR] Connection timeout - clearing N queued @ mentions.That error is always one fact: the Claude CLI the plugin launches never opens its IDE WebSocket back to the plugin's server, so queued
@mentions hitconnection_timeoutand clear. Whenhttp_proxy/all_proxyis set in the environment without a localhost exclusion, Claude (proxy-from-env semantics) tunnels even itsws://127.0.0.1:<port>IDE connection through the proxy, which can't reach a loopback server — so the handshake never arrives. This matches theno_proxy=localhost,127.0.0.1,::1workaround reported on the issue.The fix
get_claude_command_and_env(lua/claudecode/terminal.lua) now guarantees the loopback hostslocalhost,127.0.0.1,::1are present in bothno_proxyandNO_PROXYfor the Claude terminal, so the loopback IDE socket is never proxied.Key design points:
termopenlayersenv_tableover the parent env, so the inherited shellno_proxy/NO_PROXYare read viaos.getenvand merged (order-preserving, de-duplicated) rather than overwritten — a corporateno_proxy=corp.internalis preserved.envconfig merge. The injection is the last word and also folds in anyno_proxya user set via the plugin'senvconfig option, so a user-suppliedno_proxycan't silently drop the loopback exclusion and re-break the bug.Scope / limitation
This covers providers where the env table reaches the launched Claude (
native,snacks, and the string form ofexternal). Forprovider = "none"and a function-formexternal_terminal_cmdthat ignore the env table, Claude runs outside the plugin's control, so those users still setno_proxythemselves (documented in the fixture README and the issue triage).The multi-instance / discovery facets described in the #70 triage (the env var not reaching Claude, ambiguous lock files) are largely Claude-CLI discovery behaviour rather than plugin bugs and are not auto-closed by this PR — left for the maintainer to decide.
Testing
mise run check— luacheck 0 warnings / 0 errors (101 files);mise run test— 568/568 pass (3 newterminal_speccases: loopback added, both-vars merge, and theenv-config regression).env-config regression test was confirmed red-green: it fails on the pre-fix ordering (loopback dropped) and passes after.claudeCLI driven through a PTY: under the exact dead-proxy scenario that previously timed out, the fixed plugin now delivers the@mention (Claude connects).Reproduction (added in this PR)
fixtures/issue-70/— drives the real plugin under a proxy (:Issue70Send) to surface the exact error; see itsREADME.md.scripts/repro_issue_70.sh+scripts/repro_issue_70_probe.lua— reproduce the proxy root cause against the real Claude CLI (baseline connects / proxy fails /no_proxyfix connects).Refs #70
🤖 Generated with Claude Code