Skip to content

fix: re-route AgentConnectionStore through AgentConnectionCache (redux) - #46

Merged
lukemarsden merged 1 commit into
mainfrom
fix/agent-connection-store-cache-redux
Apr 26, 2026
Merged

fix: re-route AgentConnectionStore through AgentConnectionCache (redux)#46
lukemarsden merged 1 commit into
mainfrom
fix/agent-connection-store-cache-redux

Conversation

@lukemarsden

Copy link
Copy Markdown

Summary

Re-applies the AgentConnectionStore portion of ba7e97aea6 that was reverted in 350de991de. The revert commit explicitly flagged this as a known-unfixed race that "can be addressed separately if it ever surfaces" — it surfaced.

Problem

Two claude-agent-acp wrappers spawn for the same resumed spec-task session, each with their own claude --resume and their own chrome-devtools-mcp. One of the chrome-devtools-mcp instances loses the npx race and never registers, so the model talking to that wrapper sees Error: No such tool available: mcp__chrome-devtools__*.

Reproduced on ubuntu-external-01kq54vkjmb...:

  • pstree: two sh -c "claude-agent-acp" parents, only one with chrome-devtools-mcp under it
  • Zed.log.old at 16:02:47: two [ACP_SPAWN] About to spawn ACP wrapper agent_id=AgentId("claude-acp") lines, but only one [ACP_DEDUP] No cached connection line — confirming the second spawn skipped the cache

The race: agent panel restoration on a resumed session calls AgentConnectionStore::request_connection for claude-acp at the same instant external_websocket_sync::thread_service::open_existing_thread_sync does. Both paths independently call server.connect(). The earlier fix only routed thread_service through the cache, leaving the cross-path race open.

Fix

Route AgentConnectionStore::start_connection through AgentConnectionCache::request_connection so UI-driven connects share a single AgentConnection with external_websocket_sync::thread_service for the same (project, agent_id).

Why the original revert reason no longer applies

The revert (350de991de) said E2E_AGENTS="zed-agent,claude" ./run_docker_e2e.sh round 2 (claude) phase 1 timed out with 0 events. With current main (post the stale-flag and other thread_service fixes from #44 and #45 on this fork), that E2E now passes both rounds with this change re-applied.

##################################################
  FINAL RESULTS
##################################################
  [zed-agent] PASSED
  [claude] PASSED
  [store] PASSED

Test plan

  • E2E_AGENTS="zed-agent,claude" ./run_docker_e2e.sh passes (verified locally)
  • In a fresh spec task: pstree shows only one claude-agent-acp parent per session
  • Zed.log shows [ACP_DEDUP] Reusing connection for the second caller
  • mcp__chrome-devtools__* tools work consistently

🤖 Generated with Claude Code

Re-applies the AgentConnectionStore portion of ba7e97a that was reverted
in 350de99. Without it, the cross-cache race the revert commit explicitly
left open ("UI-driven connect simultaneous with external_websocket_sync
open_thread for the same agent isn't deduped") surfaces in production: the
agent panel restoration on a resumed spec-task session calls
`request_connection` for `claude-acp` at the same instant
`thread_service::open_existing_thread_sync` does, each spawning a fresh
`claude-agent-acp` wrapper, a fresh `claude --resume`, and a fresh
`chrome-devtools-mcp`. Two ACP_SPAWN log lines for the same agent_id in the
same second; only one of the chrome-devtools-mcp instances wins the npx race
and registers, so the model talking to the other one sees
"No such tool available: mcp__chrome-devtools__*".

Verified against the reproducing spec task (ubuntu-external-01kq54vkjmb...):
two `sh -c "claude-agent-acp"` parents, only one with chrome-devtools-mcp
under it. Zed.log.old shows two ACP_SPAWN entries at 16:02:47 but only one
ACP_DEDUP "No cached connection" line, confirming the second spawn skipped
the cache.

The original revert reason was that routing AgentConnectionStore through the
cache made `E2E_AGENTS="zed-agent,claude" ./run_docker_e2e.sh` round 2
(claude) phase 1 time out with 0 events. With the current main (post the
stale-flag and other thread_service fixes from #44 / #45), that E2E now
passes both rounds with this change re-applied — verified locally.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@lukemarsden
lukemarsden merged commit fbb4da9 into main Apr 26, 2026
23 of 24 checks passed
lukemarsden added a commit to helixml/helix that referenced this pull request Apr 26, 2026
Replaces the unmerged branch commit with the merge commit on Zed main
after helixml/zed#46 landed.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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