fix: unfreeze cross-project send target via event-driven completion#60
Merged
Merged
Conversation
awaitStreamCompletion was a 100ms polling loop on MainActor. When the sending project's agent invoked ide__send_to_thread, the MainActor-bound MCP handler entered that loop and starved the target project's processStream `for await event in stream` running on the same actor — the receiving thread stayed at "1 message, isStreaming=true" with no assistant tokens until the sender's tool call was cancelled. Replace the poll with an event-driven handoff: recordStreamCompletion resumes a CheckedContinuation directly the moment the result lands, with a parallel Task.sleep(timeout) as the deadline. A small @mainactor class guards against double-resume in the race between recorder and timeout. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR updates cross-project MCP stream completion handling in AppState to avoid MainActor starvation caused by a polling-based awaitStreamCompletion loop, replacing it with an event-driven continuation that resumes immediately on completion (or returns nil on timeout).
Changes:
- Replaces the 100ms polling loop in
awaitStreamCompletionwith awithCheckedContinuation+ timeout task. - Adds
streamCompletionWaitersto allowrecordStreamCompletionto resume waiters directly instead of relying on polling. - Introduces a small
@MainActorStreamCompletionWaiterwrapper to guard against double-resume races.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| RxCode/App/AppState+Messaging.swift | Implements event-driven stream completion waiting/resumption and adds StreamCompletionWaiter. |
| RxCode/App/AppState.swift | Adds streamCompletionWaiters state to support direct resumption of waiters. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+486
to
+490
| if let waiter = streamCompletionWaiters.removeValue(forKey: streamId) { | ||
| waiter.resume(with: completion) | ||
| return | ||
| } | ||
| pendingStreamCompletions[streamId] = completion |
Comment on lines
530
to
+534
| /// Discard a recorded completion. Called by long-running `wait_for_response=false` | ||
| /// MCP sends so the dictionary doesn't grow unbounded with abandoned results. | ||
| func discardStreamCompletion(streamId: UUID) { | ||
| pendingStreamCompletions.removeValue(forKey: streamId) | ||
| if let waiter = streamCompletionWaiters.removeValue(forKey: streamId) { |
Logs the full receiver-side handshake so we can see exactly where it hangs: every MCP method received per connection, every reply sent, tools/call latency, NWConnection state transitions, runMCP chunk count on EOF, and the moment processStream enters its for-await loop and receives its first event. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…stdout The CLI stdout reader used `for try await line in handle.bytes.lines`, whose underlying AsyncBytes iterator wedges when a second concurrent pipe reader is active — exactly the cross-project send case where the sender is mid-`ide__send_to_thread` while the target spawns its own CLI. Log evidence: the second CLI completed MCP handshake (initialize + tools/list reply observed) but emitted zero stdout to our reader, so processStream's `for await event in stream` never received the `.system init` event and the UI sat at "isStreaming=true, messages=1" indefinitely. Cancelling the sender released the AsyncBytes lock and the buffered events flushed in one burst — matching the symptom. Switch to a Dispatch-backed `readabilityHandler` pump (already used for stderr and known-good for multiple concurrent pipes). Lines are split out of an accumulating buffer; EOF flushes any trailing non-terminated line, then finishes the stream. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…holder For a new-thread cross-project send, sendPrompt assigns a transient "pending-<streamId>" key that gets replaced by the CLI's real session_id on the first `.system` event. Previously the MCP tool reply could carry the placeholder back to the caller's agent when wait_for_response=false (and on the wait_for_response=true timeout fallback path), and the caller had no usable id to follow up with later. Add a SessionRenameWaiter handoff: every site that writes to sessionIdRedirect goes through applySessionIdRedirect(from:to:), which also resumes any awaitSessionRename caller. sendCrossProject now blocks on awaitSessionRename (capped at min(timeout, 60s)) before returning the result, so the MCP reply always carries the real session_id once the CLI has produced one. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
stripNoOpText was wiping the assistant message entirely whenever the model replied with the no-op marker, on the assumption that the marker only appears for hook/ScheduleWakeup re-entries with no real prompt. That assumption breaks for ide__send_to_thread: the sender often passes context-only text (e.g. "For context, this project is RxAuthSwift…"), and the receiver model legitimately answers "No response requested." — which we then deleted, leaving the receiver thread visually empty and making the cross-project send look like the stream silently stopped. Surface the marker instead of hiding it. Removed all four live-stream strip call sites (Messaging.swift finalizeStreamSession, Stream.swift text-delta + tool-start + cancel paths, CrossProject.swift mid-stream new-message path) and the disk-replay filter in CLILineToBlocksMapper. stripNoOpText itself is deleted; isNoResponseRequested stays in case something downstream still needs the detector. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 tasks
sirily11
added a commit
that referenced
this pull request
May 26, 2026
Adds four XCTests against the orchestration layer of `sendCrossProject` to prevent regression of the freeze fixed in PR #60: - `testSecondCrossProjectSendCompletesWhileFirstIsPausedMidStream`: while project A's mock stream is paused 3s mid-turn, a second `sendCrossProject` for project B must still complete (~0.5s in practice). Pre-fix the polling `awaitStreamCompletion` would have starved B's for-await loop and B would block for the full 3s. - `testCrossProjectSendReturnsRealSessionIdNotPendingPlaceholder`: the MCP reply must carry the CLI's real `session_id`, not the transient `pending-<streamId>` placeholder. Covers the `awaitSessionRename` handoff. - `testRecordStreamCompletionResumesAwaiterImmediately` and `testApplySessionIdRedirectResumesRenameWaiterImmediately`: unit-level assertions that `recordStreamCompletion` / `applySessionIdRedirect` resume their waiters within 50ms of firing — i.e. event-driven, not polled. Supporting infrastructure: - `MockAgentBackend`: actor conforming to `AgentBackend`, scripts scripted by `cwd` (FIFO per cwd) since `streamId` is generated inside `sendPrompt` and isn't visible to the test ahead of time. - Test-only seam on `AppState`: `agentBackendOverrides` dict that `backend(for:)` consults first. Production code never writes to it; the dispatch fall-through is unchanged when the dict is empty. - `setUp` snapshots/pins `selectedAgentProvider` in UserDefaults so a stale `.codex` value from a previous app run can't route past the mock into a half-initialised real backend. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sirily11
added a commit
that referenced
this pull request
May 26, 2026
## Summary - Adds 4 XCTests that lock in the cross-project send fixes from #60 so they can't silently regress. - Includes a `MockAgentBackend` actor (scriptable per-cwd timeline of `StreamEvent`s) and a tiny test-only injection seam on `AppState` (`agentBackendOverrides`). ## Tests added - **`testSecondCrossProjectSendCompletesWhileFirstIsPausedMidStream`** — project A's mock stream pauses 3 s mid-turn; project B's `sendCrossProject` must still complete. Asserts ~0.5 s. Pre-#60 the polling `awaitStreamCompletion` would starve B for the full pause. - **`testCrossProjectSendReturnsRealSessionIdNotPendingPlaceholder`** — the MCP reply must carry the CLI's real `session_id`, not the `pending-<streamId>` placeholder. Covers the `awaitSessionRename` handoff. - **`testRecordStreamCompletionResumesAwaiterImmediately`** / **`testApplySessionIdRedirectResumesRenameWaiterImmediately`** — assert the continuation-based waiters resume within 50 ms (event-driven, not polled). ## Notes on the seam `AppState.agentBackendOverrides` is empty in production; `backend(for:)` consults it first and falls through to the real services. The mock registers for all three providers in `setUp`, and the test also pins `selectedAgentProvider = .claudeCode` in `UserDefaults` to defeat stale values from prior app runs (which previously caused the test to hit a real-but-uninitialised `CodexAppServer`). ## Test plan - [x] All four tests pass via `xcodebuild test -scheme RxCode -testPlan UnitTestPlan -only-testing:RxCodeTests/CrossProjectSendConcurrencyTests` - [x] Concurrent-progress test completes in 0.5 s (well below 2 s threshold) - [ ] Reviewer to confirm the `agentBackendOverrides` seam is acceptable for the test target 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
|
🎉 This PR is included in version 1.12.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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
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
awaitStreamCompletionwas a 100ms polling loop on MainActor. When project A's agent invokedide__send_to_thread, the MainActor-bound MCP handler entered that loop and starved project B'sprocessStream'sfor await event in streamrunning on the same actor — the receiving thread stayed at "1 message, isStreaming=true" with no assistant tokens until the sender's tool call was cancelled.recordStreamCompletionresumes aCheckedContinuationdirectly the moment the result lands, with a parallelTask.sleep(timeout)as the deadline.StreamCompletionWaiterguards against double-resume in the race between recorder and timeout.Test plan
🤖 Generated with Claude Code