fix(external_websocket_sync): force-reset claude-agent-acp on wedged Query - #63
Merged
Conversation
…Query
The Anthropic Claude Code SDK Query (re-exposed through
@zed-industries/claude-agent-acp) gets permanently stuck after a
mid-stream cancellation, returning
Internal error: [ede_diagnostic] result_type=user
last_content_type=n/a stop_reason=null
on every subsequent prompt. The previous in-place retry (4x750ms)
spun on the same wedged handle and never recovered, deterministically
breaking Phase 9 of the WebSocket E2E suite (rapid 3-turn cancel) on
the claude agent.
Recovery path:
- Bypass Zed's ref-counted close_session via a new
AgentConnection::force_close_session trait method that
unconditionally drops sessions/pending bookkeeping and sends a raw
CloseSessionRequest. This drives the wrapper through its
teardownSession path, disposing the wedged Query.
- After teardown, drop the THREAD_REGISTRY entry, call the existing
load_thread_from_agent to materialise a fresh AcpThread (the
wrapper's loadSession hits the empty-map branch and spawns a new
Query that resumes from the on-disk transcript), and retry the send
on the fresh entity.
Strict triple-match on the error string keeps the recovery narrow;
any other prompt failure follows the existing error-event path.
In-place retry trimmed from 4x750ms to 2x500ms: one retry catches a
genuine sub-second drain race, more is just spinning when the Query
is truly wedged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…force-reset
Review follow-ups for the force-reset recovery path:
1. force_close_agent_session was only calling unregister_thread, which
leaves THREAD_KEEP_ALIVE intact. Between force-close and successful
reload, a concurrent handler (cancel-task loop, parallel follow-up)
that calls get_thread would fall through to the keep-alive map and
resurrect the wedged entity — then prompt-send into a session whose
wrapper-side state is being torn down underneath us. Now clear both
THREAD_REGISTRY and THREAD_KEEP_ALIVE before kicking off the close
RPC. Also clear EXTERNAL_ORIGINATED_ENTRIES (its indices are keyed
on the OLD entity's entries vector and are stale after reload).
Keep THREAD_AGENT_SESSION_MAP — the wrapper's loadSession-with-resume
reuses the same agent session id to rehydrate from disk transcript.
2. On the force-close-succeeded-but-reload-failed path we were sending
the ORIGINAL drain-race error to Helix instead of the load error.
The load error is the actionable signal at that point ("we unstuck
the wrapper but couldn't rehydrate the transcript"). Same for the
force-close-itself-failed path. Both now surface the recovery-stage
error with the original drain-race error preserved as context.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chocobar
added a commit
to helixml/helix
that referenced
this pull request
Jun 15, 2026
…x-ups Pulls in helixml/zed@f8e9d85d42 which addresses the two MUST FIX items from review of helixml/zed#63: clear THREAD_KEEP_ALIVE alongside THREAD_REGISTRY in force-reset, and surface reload/close errors to Helix instead of swallowing them behind the original drain-race error. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…h path
Phase 9 still hits the trait default error message in CI ("Force-closing
sessions is not supported", no "by this agent." suffix), but AcpConnection
overrides force_close_session at line 1922. Adding eprintln + log::warn
at the entry of both the AcpConnection override and the trait default
to figure out which one actually runs at runtime for the wedged claude
session.
This commit is diagnostic and will be reverted/squashed once the
dispatch issue is understood.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…e CI grep filter
The e2e harness's cleanup grep at run_e2e.sh:53 dumps only lines matching
'panic|error|fatal'. My previous DIAG eprintlns ('🔎 [DIAG]', '🚨 [DIAG]',
'🧹 [THREAD_SERVICE] Drain race signature', etc) didn't contain any of
those words, so they were silently dropped from the CI output even though
they fired at runtime. That's why build 2185's log showed only the final
"Force-close failed: ..." marker — the upstream diagnostic eprintlns were
filtered out before printing.
Renamed to include 'DIAG-ERROR' so they get captured. This is purely a
log-formatting change; the recovery logic is unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ecovery Build 2187's DIAG eprintlns finally surfaced (once they were renamed to contain "ERROR" so they survived the e2e harness's panic|error|fatal grep filter) and showed the real bug: when the wrapper-side wedge recovery fires on a follow-up message, `request.agent_name` is missing. Helix's `SendChatMessage` path used for follow-up messages on existing threads doesn't propagate `agent_name` from the originating create — only the first chat_message carries it. So the wedge recovery sees agent_name=None (or "zed-agent" default) for a thread that was actually created with agent_name="claude". My force_close_agent_session routes None/"" → ExternalAgent::NativeAgent → NativeAgentConnection, which does NOT override force_close_session → trait default fires → "Force-closing sessions is not supported" → recovery returns Err to Helix → Phase 9 times out. Fix: record agent_name in a new THREAD_AGENT_NAME_MAP at thread create / load time (mirroring how THREAD_AGENT_SESSION_MAP records the agent's session id), and have force_close_agent_session + the post-reset reload look up the recorded agent_name in preference to the request's. The recorded value matches the agent actually backing the session, so force_close_session dispatches correctly to AcpConnection's override. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…very signal The DIAG-ERROR eprintlns/log lines served their debugging purpose (confirmed end-to-end on builds 2191 + 2193: wedge detected, recovery routed correctly to AcpConnection::force_close_session via the recorded agent_name, wrapper teardown succeeded, Phase 9 PASSED). Stripping the verbose dispatch-trace markers and the "cleared X" status lines; keeping the higher-signal "wedge detected — attempting recovery" entry and the final "wrapper teardown complete" so production traces still tell us when the recovery path fires. Also tightens the trait-default `force_close_session` error message to clarify when it would legitimately fire (non-ACP backends without override). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Summary
Phase 9 of the WebSocket E2E suite (
rapid 3-turn cancel regression test) fails deterministically on theclaudeagent because the Anthropic Claude Code SDKQuery(re-exposed through@zed-industries/claude-agent-acp) wedges after a mid-stream cancellation. Every follow-up prompt returns:The previous in-place retry (4×750ms) spun on the same wedged handle and never recovered. This has blocked every helix release tag since 2.11.14 (most recently 2.11.17 → https://drone.lukemarsden.net/helixml/helix/2162/5/5).
Approach
AgentConnection::force_close_session(crates/acp_thread/src/connection.rs) — default-errors impl, bypasses ref counting.AcpConnection::force_close_session(crates/agent_servers/src/acp.rs) — dropssessions/pending_sessionsunconditionally and sends rawCloseSessionRequest. This drives the wrapper throughteardownSession, disposing the wedgedQuery.external_websocket_sync::thread_service— strict triple-match on the error string (ede_diagnostic+result_type=user+stop_reason=null):THREAD_REGISTRYentryload_thread_from_agent(wrapper'sloadSessionhits the empty-map branch, spawns a freshQueryresuming from the on-disk transcript)Strict triple-match keeps the recovery narrow — any other prompt failure follows the existing error-event path unchanged.
Risk
Resume-from-transcript should preserve enough context for Phase 9's success criterion (2 completions on the thread). If it doesn't, Phase 9 will still fail but with a different signature (
Retry after force-reset still failed: ...) — that would tell us we need to file upstream against@anthropic-ai/claude-codeandxfailPhase 9 for claude.Companion bump in helix: helixml/helix#2616
Test plan
build-sandbox-amd64 / zed-e2e-testpasses consistently (the actual gate; runs the E2E against this exact commit via the helix PR)zed-agentround of the E2E🤖 Generated with Claude Code