Skip to content

fix(external_websocket_sync): defer UserCreatedThread until first user message - #56

Merged
lukemarsden merged 4 commits into
mainfrom
fix/lazy-draft-thread
May 13, 2026
Merged

fix(external_websocket_sync): defer UserCreatedThread until first user message#56
lukemarsden merged 4 commits into
mainfrom
fix/lazy-draft-thread

Conversation

@lukemarsden

@lukemarsden lukemarsden commented May 13, 2026 •
edited
Loading

Copy link
Copy Markdown

Summary

Two fixes (1a + 1b) for the duplicate-Claude-spawn pattern that was producing 180s chrome-devtools/github context server failed to start: Context server request timeout errors in long-running Helix spec-task containers, plus an agent-agnostic rewrite of the Phase 15 streaming-cadence assertion that was false-failing for Claude Code.

Full diagnosis: helix/design/2026-05-13-mcp-cache-contention-and-duplicate-claude-spawn.md.

Background

The agent panel's activate_draft (called from Panel::set_active(true), which fires every time Zed restores its workspace at container restart) creates a non-resume ConversationView for the empty input editor — and that view eagerly calls connection.new_session() which spawns a Claude ACP child with its full MCP server tree. On long-running Helix spec-tasks each restart therefore produced an extra Claude the user never typed in, and those extras contended with the user's real Claudes for the npm _npx/<hash> cache, causing 180s context_server timeouts. The user's example task spt_01kqc4ev5rt9rknk6g8dbkzj9a had accumulated 10 phantom helix_session rows (8 with zero interactions) from this pattern.

Fix 1a: defer UserCreatedThread until first user message

crates/external_websocket_sync/src/thread_service.rs — new defer_user_created_thread() API + PENDING_USER_CREATED_EMITS map. The existing ensure_thread_subscription NewEntry handler flushes pending emits on the first user-role entry. Drafts the user never types in are never announced to Helix.

crates/agent_ui/src/conversation_view.rs:1336 and crates/agent_ui/src/acp/thread_view.rs:1004 — switch from immediate send_websocket_event(SyncEvent::UserCreatedThread {…}) to defer_user_created_thread(…). Resume threads (is_resume=true) are unaffected.

Fix 1b: suppress speculative draft activation under external_websocket_sync

crates/agent_ui/src/agent_panel.rs::ensure_thread_initialized — under the external_websocket_sync cargo feature, skip the activate_draft call. Helix drives all real conversations through the chat_message WebSocket path (create_new_thread_sync → register_thread, NOT through activate_draft), so spec-task functionality is unaffected. Upstream Zed (without the feature) keeps the existing UX where the panel auto-creates a draft on show.

The originally-considered alternative was a placeholder ServerState::PendingDraftSession variant with a deferred-init MessageEditor — that would have required substantial refactoring of ConversationView and every agent-panel surface that reads active_thread() (model selector, mode toggle, tool-permission panel, agent capabilities query, etc.). The feature-gated suppression achieves the same end result with no UX regression in either configuration.

Phase 15 streaming-cadence assertion rewrite

The previous "30% of final content visible by midpoint" assertion in crates/external_websocket_sync/e2e-test/helix-ws-test-server/main.go was failing on main (build zed-industries#1384/5/5, zed-industries#1389, zed-industries#1391) for the claude agent — false positive. Claude Code emits <thinking> blocks before the prose entry; by midpoint only thinking content has streamed, then the prose entry rapidly streams in the second half. The bug pattern the assertion was meant to catch is "everything arrives in the final Stopped re-emit", which is different.

Replaced with: assert NO MORE than 90% of final content arrives in the LAST 20% of stream time. Agent-agnostic, catches the actual regression, tolerates legitimate non-linear streaming.

Tests

Unit tests (crates/external_websocket_sync/src/thread_service.rs)

6 tests for the defer/flush/drop API:

  • defer_registers_in_pending_map_without_sending — defer must populate the pending map; assertion message points to the design doc if it ever fails.
  • flush_clears_the_pending_entry_and_returns_true — flush is the documented consumption mechanism.
  • flush_is_a_noop_when_nothing_is_pending — protects against spurious emits.
  • drop_removes_pending_entry_without_flushing — covers the disposal path.
  • flush_only_fires_once_per_pending_entry — the NewEntry handler fires on EVERY user-role entry; flush MUST self-cleanup or every follow-up message would re-emit.
  • defer_overwrites_existing_pending_entry_with_new_title — defensive against stale title metadata.

E2E tests (crates/external_websocket_sync/e2e-test/helix-ws-test-server/main.go)

  • Phase 16 (Fix 1a regression test): asserts spontaneousUserCreatedThreadCount == 0 per round. To verify regression power, revert defer_user_created_thread() calls in conversation_view.rs / acp/thread_view.rs to immediate send_websocket_event(SyncEvent::UserCreatedThread {…}) and re-run; this assertion fails with the draft thread's UUID in the diagnostic.
  • Phase 17 (Fix 1b regression test): counts claude --output-format processes via ps -eo args inside the test container and asserts the count equals real threads created in the round. Catches future regressions if anything reintroduces speculative Claude spawning.
  • Phase 15 rewrite (above): catches the actual streaming-burst regression without false-failing on Claude.

Verification

  • cargo check -p external_websocket_sync -p agent_ui clean (with and without the feature flag).
  • cargo test -p external_websocket_sync pending_user_created_emit_tests — all 6 tests pass.
  • ./stack build-zed dev produces a working binary; ./stack build-ubuntu produces a desktop image with the new Zed.
  • Fresh spec-task in inner-Helix with the new image:
    • Zed log shows 📌 Deferring UserCreatedThread for <draft_uuid> (Fix 1a working).
    • 1 Claude process alive in the container (down from 2 pre-fix). Fix 1b working.
    • 1 row in spec_task_zed_threads for the test task (down from 2). Phantom draft suppression working.
    • mcp__chrome-devtools__list_pages and mcp__github__list_pull_requests both work end-to-end.
  • CI on Helix PR fix(mcp): pre-install MCP servers globally to skip npx cache contention helix#2418 (which pins ZED_COMMIT to this commit + ships the Helix-side dedup safety net + npx cache fixes).

Related

🤖 Generated with Claude Code

lukemarsden and others added 4 commits May 13, 2026 10:45
@lukemarsden @claude
fix(external_websocket_sync): defer UserCreatedThread until first use…
32a1e3b 
…r message

The agent panel speculatively creates a "draft" ConversationView (via
agent_panel::activate_draft → ConversationView::new with
resume_session_id=None) every time the panel is shown, to back the empty
input editor. That draft's load_task previously fired UserCreatedThread
to the external sync system unconditionally for the !is_resume branch.

For Helix spec-tasks, every container restart of a long-running task
therefore registered a phantom empty `helix_session` row in
spec_task_zed_threads — the user never typed anything in the new draft
but Helix recorded it as a real conversation, complete with a duplicate
Claude ACP spawn whose npm exec children raced against the existing
ones for the npm `_npx/<hash>` cache, surfacing as 180s
`chrome-devtools/github context server failed to start: Context server
request timeout` errors. Long-running spec_tasks accumulated up to one
empty "New Chat" row per restart (10 rows on the user's own task,
8 with zero interactions).

Fix: register the (acp_thread_id, title) tuple in a pending map via
`defer_user_created_thread()` instead of emitting immediately. The
existing `ensure_thread_subscription` NewEntry handler flushes the
pending emit on the first user-role entry. Drafts the user never types
in are never announced to Helix and never produce a phantom session.

Resume threads (is_resume=true) are unaffected — they don't go through
this path.

Full diagnosis and design discussion in:
https://github.com/helixml/helix/blob/fix/mcp-cache-contention/design/2026-05-13-mcp-cache-contention-and-duplicate-claude-spawn.md

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@lukemarsden @claude
test(external_websocket_sync): unit tests for deferred UserCreatedThr…
455c095 
…ead emit

Six regression tests covering the new defer/flush/drop API surface in
thread_service.rs:

- defer_registers_in_pending_map_without_sending: defer must populate the
  pending map; the assertion message points reviewers at the design doc
  if it ever fails.
- flush_clears_the_pending_entry_and_returns_true: flush is the
  documented mechanism to consume a pending emit.
- flush_is_a_noop_when_nothing_is_pending: protects against spurious
  emits when a NewEntry arrives on a thread that wasn't deferred.
- drop_removes_pending_entry_without_flushing: covers the disposal path
  for drafts the user dismisses.
- flush_only_fires_once_per_pending_entry: the NewEntry handler in
  ensure_thread_subscription fires on every user-role entry, so flush
  MUST self-cleanup or every follow-up message would re-emit
  UserCreatedThread and create duplicate Helix sessions.
- defer_overwrites_existing_pending_entry_with_new_title: defensive
  against stale title metadata from re-registration.

These tests fail if a future change reverts the deferred-emit machinery
back to immediate `send_websocket_event(SyncEvent::UserCreatedThread …)`
at the conversation_view.rs / acp/thread_view.rs sites — the pending
map would never populate and the first assertion would catch it.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@lukemarsden @claude
test(e2e): Phase 16 — assert no spontaneous UserCreatedThread emits
056fe07 
Adds an end-of-round assertion that
roundState.spontaneousUserCreatedThreadCount == 0.

Pre-Fix-1a, every Zed startup emitted SyncEvent::UserCreatedThread
unconditionally for the agent panel's draft (non-resume) ConversationView,
which Helix recorded as a phantom helix_session row on every container
restart of a long-running spec_task. The phantom row also brought a
duplicate Claude ACP child whose npm exec children raced for the
`_npx/<hash>` cache → 180s `chrome-devtools/github context server failed
to start: Context server request timeout`.

Post-Fix-1a (this branch / 32a1e3b), defer_user_created_thread
registers the (acp_thread_id, title) tuple in a pending map; the emit
is flushed only when the NewEntry handler observes the first user-role
entry on that thread. The e2e test drives all phases via chat_message
(which goes through create_new_thread_sync, NOT through the panel's
activate_draft path), so the panel's draft never sees a user message
and its emit stays pending forever — meaning zero spontaneous
user_created_thread events should reach the test driver.

To verify the regression power: revert defer_user_created_thread()
calls in conversation_view.rs:1336 and acp/thread_view.rs:1004 back to
immediate send_websocket_event(SyncEvent::UserCreatedThread {…}) and
re-run the e2e test; this assertion will fail with the draft thread's
UUID in the diagnostic.

Phase 10 injects user_created_thread directly via ProcessSyncEvent
(bypasses WebSocket entirely), so it does not increment this counter.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@lukemarsden @claude
fix(agent_panel): suppress speculative draft activation under externa…
769a463 
…l_websocket_sync

Fix 1b for the duplicate-Claude-spawn bug documented in
helix/design/2026-05-13-mcp-cache-contention-and-duplicate-claude-spawn.md.

When the agent panel becomes active (Panel::set_active(true)), it would
call ensure_thread_initialized → activate_draft → ConversationView::new
→ load_task → connection.new_session(), spawning a Claude ACP child
with its full MCP server tree. On every Helix container restart the
panel auto-activates as part of workspace restoration, so each restart
produced one extra Claude process the user never typed in. Those extra
Claudes contended with the user's real ones for the npm `_npx/<hash>`
cache, surfacing as 180s `chrome-devtools/github context server failed
to start: Context server request timeout` errors.

Fix: under the `external_websocket_sync` cargo feature, skip the
speculative activate_draft call. Helix drives all real conversations
through the chat_message WebSocket path (which goes via
create_new_thread_sync → register_thread, NOT through activate_draft),
so spec-task functionality is unaffected. Upstream Zed (without the
feature) keeps the existing behavior — the panel auto-creates a draft
when shown — to preserve UX for users who expect to type immediately.

E2E verified: a fresh spec-task container now has exactly 1 Claude
process alive (the user's real conversation), down from 2 (real + draft).
The e2e Phase 17 assertion this commit also adds catches future
regressions: it counts `ps -eo args | grep "claude --output-format"`
inside the test container and asserts the count equals the number of
real threads created in the test round.

Phase 15 streaming-cadence assertion rewritten to be agent-agnostic.
The previous "30% of content by midpoint" check false-failed for
Claude Code's "thinking-then-burst" streaming pattern (where the
prose entry only starts streaming after the thinking entry finishes).
The new assertion checks that NO MORE THAN 90% of final content
arrives in the LAST 20% of stream time, which is the actual regression
signal for "everything-arrives-in-the-final-Stopped-burst" while
tolerating legitimate non-linear streaming.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@lukemarsden lukemarsden mentioned this pull request May 13, 2026
Merged
6 tasks
@lukemarsden
lukemarsden merged commit 62cd60a into main May 13, 2026
30 checks passed
This was referenced May 13, 2026
Merged
Merged
Merged
@lukemarsden lukemarsden mentioned this pull request May 25, 2026
Merged
This was referenced Jun 12, 2026
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@lukemarsden