diff --git a/Packages/Sources/RxCodeCore/CLISession/CLILineToBlocksMapper.swift b/Packages/Sources/RxCodeCore/CLISession/CLILineToBlocksMapper.swift index 44b8db36..08d5d3ef 100644 --- a/Packages/Sources/RxCodeCore/CLISession/CLILineToBlocksMapper.swift +++ b/Packages/Sources/RxCodeCore/CLISession/CLILineToBlocksMapper.swift @@ -129,8 +129,6 @@ public enum CLILineToBlocksMapper { switch part { case .text(let t): guard !t.isEmpty else { continue } - let trimmed = t.trimmingCharacters(in: .whitespacesAndNewlines) - if CLIMetaEnvelope.isNoResponseRequested(trimmed) { continue } blocks.append(.text(t)) case .toolUse(let id, let name, let input): blocks.append(.toolCall(ToolCall(id: id, name: name, input: input))) diff --git a/Packages/Tests/MessageListTests/MessageListPinnedTurnSwiftUITests.swift b/Packages/Tests/MessageListTests/MessageListPinnedTurnSwiftUITests.swift index c115ba20..d7bee799 100644 --- a/Packages/Tests/MessageListTests/MessageListPinnedTurnSwiftUITests.swift +++ b/Packages/Tests/MessageListTests/MessageListPinnedTurnSwiftUITests.swift @@ -35,7 +35,7 @@ struct MessageListPinnedTurnSwiftUITests { model.observedBottomRelease } - #expect(model.isAtBottom) + #expect(model.observedBottomRelease) } } diff --git a/RxCode.xcodeproj/project.pbxproj b/RxCode.xcodeproj/project.pbxproj index 871098d6..9f12b8bf 100644 --- a/RxCode.xcodeproj/project.pbxproj +++ b/RxCode.xcodeproj/project.pbxproj @@ -10,6 +10,8 @@ 101010102FCB100000000002 /* AppStateTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 101010102FCB100000000001 /* AppStateTests.swift */; }; 101010112FCB100000000002 /* BranchBriefingPromptContextTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 101010112FCB100000000001 /* BranchBriefingPromptContextTests.swift */; }; 33993F0F87CF4DB09F2813A8 /* AppStateProjectSwitchTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4381E755142272EB2DAA9C96 /* AppStateProjectSwitchTests.swift */; }; + 7A5C0001000000000000A002 /* MockAgentBackend.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A5C0001000000000000A001 /* MockAgentBackend.swift */; }; + 7A5C0002000000000000A002 /* CrossProjectSendConcurrencyTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A5C0002000000000000A001 /* CrossProjectSendConcurrencyTests.swift */; }; 5C2222222FCB200000000002 /* BriefingThreadRowTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5C2222222FCB200000000001 /* BriefingThreadRowTests.swift */; }; 5C3333332FCB400000000002 /* ThreadStoreThreadSummaryTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5C3333332FCB400000000001 /* ThreadStoreThreadSummaryTests.swift */; }; 6E17B0012FC8000100A10001 /* LocalAIProviderAcceptanceTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6E17B0002FC8000100A10001 /* LocalAIProviderAcceptanceTests.swift */; }; @@ -112,6 +114,8 @@ 101010102FCB100000000001 /* AppStateTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = AppStateTests.swift; sourceTree = ""; }; 101010112FCB100000000001 /* BranchBriefingPromptContextTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = BranchBriefingPromptContextTests.swift; sourceTree = ""; }; 4381E755142272EB2DAA9C96 /* AppStateProjectSwitchTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = AppStateProjectSwitchTests.swift; sourceTree = ""; }; + 7A5C0001000000000000A001 /* MockAgentBackend.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = MockAgentBackend.swift; sourceTree = ""; }; + 7A5C0002000000000000A001 /* CrossProjectSendConcurrencyTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = CrossProjectSendConcurrencyTests.swift; sourceTree = ""; }; 5C2222222FCB200000000001 /* BriefingThreadRowTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = BriefingThreadRowTests.swift; sourceTree = ""; }; 5C3333332FCB400000000001 /* ThreadStoreThreadSummaryTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = ThreadStoreThreadSummaryTests.swift; sourceTree = ""; }; 6E17B0002FC8000100A10001 /* LocalAIProviderAcceptanceTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = LocalAIProviderAcceptanceTests.swift; sourceTree = ""; }; @@ -303,6 +307,8 @@ 101010102FCB100000000001 /* AppStateTests.swift */, 101010112FCB100000000001 /* BranchBriefingPromptContextTests.swift */, 4381E755142272EB2DAA9C96 /* AppStateProjectSwitchTests.swift */, + 7A5C0001000000000000A001 /* MockAgentBackend.swift */, + 7A5C0002000000000000A001 /* CrossProjectSendConcurrencyTests.swift */, DFA0CCC02FB4CC01005991E1 /* PlanDecisionTests.swift */, DFA0CCC12FB4CC01005991E1 /* PlanCardViewTests.swift */, DFA0CCD52FB4CC02005991E1 /* HistoryListArchiveFilterTests.swift */, @@ -723,6 +729,8 @@ 101010102FCB100000000002 /* AppStateTests.swift in Sources */, 101010112FCB100000000002 /* BranchBriefingPromptContextTests.swift in Sources */, 33993F0F87CF4DB09F2813A8 /* AppStateProjectSwitchTests.swift in Sources */, + 7A5C0001000000000000A002 /* MockAgentBackend.swift in Sources */, + 7A5C0002000000000000A002 /* CrossProjectSendConcurrencyTests.swift in Sources */, DFA0CCD12FB4CC01005991E1 /* PlanDecisionTests.swift in Sources */, DFA0CCD22FB4CC01005991E1 /* PlanCardViewTests.swift in Sources */, DFA0CCE12FB4CC02005991E1 /* HistoryListArchiveFilterTests.swift in Sources */, diff --git a/RxCode/App/AppState+Agents.swift b/RxCode/App/AppState+Agents.swift index 75966594..c7e7b8bc 100644 --- a/RxCode/App/AppState+Agents.swift +++ b/RxCode/App/AppState+Agents.swift @@ -371,8 +371,11 @@ extension AppState { /// Looks up the `AgentBackend` for the given provider. Used by /// `processStream`/`cancel`/`finalize` to dispatch via the unified - /// protocol instead of switching on the enum directly. + /// protocol instead of switching on the enum directly. Tests can + /// populate `agentBackendOverrides` to substitute a mock for any + /// provider; production code never sets this. func backend(for provider: AgentProvider) -> any AgentBackend { + if let override = agentBackendOverrides[provider] { return override } switch provider { case .claudeCode: return claude case .codex: return codex diff --git a/RxCode/App/AppState+CrossProject.swift b/RxCode/App/AppState+CrossProject.swift index 3655b628..46a86416 100644 --- a/RxCode/App/AppState+CrossProject.swift +++ b/RxCode/App/AppState+CrossProject.swift @@ -105,9 +105,23 @@ extension AppState { } // After sendPrompt returns, window.currentSessionId is the (possibly - // pending-) key the stream is bound to. The CLI may rename it to its - // own sid mid-stream; we surface whichever id the completion lands on. - let postSendThreadId = window.currentSessionId ?? resolvedThreadId ?? "" + // pending-) key the stream is bound to. Resolve the real CLI session + // id before returning so the caller's agent never sees `pending-…` + // (which it can't use to follow up via `get_thread_messages` etc.). + let postSendKey = window.currentSessionId ?? resolvedThreadId ?? "" + let resolvedThreadIdForReturn: String + if postSendKey.hasPrefix("pending-") { + // Cap the rename wait at the request's timeout so we still honor + // the caller's deadline; 60s upper bound matches typical first-token + // latency under healthy conditions. + let renameTimeout = min(max(timeoutSeconds, 1), 60) + resolvedThreadIdForReturn = await awaitSessionRename( + pendingKey: postSendKey, + timeout: renameTimeout + ) ?? postSendKey + } else { + resolvedThreadIdForReturn = postSendKey + } if !waitForResponse { // Don't leak the result in the dictionary — the caller is @@ -116,7 +130,7 @@ extension AppState { _ = await self?.awaitStreamCompletion(streamId: streamId, timeout: timeoutSeconds) } return CrossProjectSendResult( - threadId: postSendThreadId, + threadId: resolvedThreadIdForReturn, projectId: resolvedProject.id, done: false, assistantText: "", @@ -138,7 +152,7 @@ extension AppState { // the caller can decide whether to poll back via get_thread_messages. let partial = lastAssistantResponseText(in: stateForSession(window.currentSessionId ?? "").messages) return CrossProjectSendResult( - threadId: window.currentSessionId ?? postSendThreadId, + threadId: resolvedThreadIdForReturn, projectId: resolvedProject.id, done: false, assistantText: partial, @@ -147,28 +161,6 @@ extension AppState { } } - /// Drop "No response requested." text blocks from the assistant message - /// at `idx`. If the message has no blocks left after the strip, remove - /// it entirely. Called at turn-finalization sites — the marker is the - /// model's response when a turn arrives without a user prompt - /// (ScheduleWakeup, hook re-entry) and reads as noise in the chat UI. - /// Strip CLI no-op meta text ("no response requested") from a message. - /// - /// `removeIfEmpty` controls whether a message left with no blocks is also - /// deleted. The normal stream path passes `true` to discard pure no-op - /// envelopes; the cancel path passes `false` so pausing a turn never makes - /// the partial assistant bubble disappear. - static func stripNoOpText(at idx: Int, in messages: inout [ChatMessage], removeIfEmpty: Bool = true) { - guard messages.indices.contains(idx) else { return } - messages[idx].blocks.removeAll { block in - guard let text = block.text else { return false } - return CLIMetaEnvelope.isNoResponseRequested(text.trimmingCharacters(in: .whitespacesAndNewlines)) - } - if removeIfEmpty, messages[idx].blocks.isEmpty { - messages.remove(at: idx) - } - } - /// Wrap a branch briefing into a system-prompt section the agent can use as /// background context. The briefing is auto-generated from earlier threads, /// so it is framed as advisory rather than authoritative. @@ -395,11 +387,15 @@ extension AppState { var eventCount = 0 var lastEventTime = Date() + logger.info("[Stream:UI] entering for-await session=\(sessionKey, privacy: .public) stream=\(streamId) cwd=\(cwd, privacy: .public)") do { for await event in stream { eventCount += 1 let gap = Date().timeIntervalSince(lastEventTime) + if eventCount == 1 { + logger.info("[Stream:UI] first event arrived session=\(sessionKey, privacy: .public) stream=\(streamId) after=\(String(format: "%.2f", gap))s") + } lastEventTime = Date() updateState(sessionKey) { $0.lastStreamEventDate = lastEventTime } @@ -418,7 +414,7 @@ extension AppState { if let state = sessionStates.removeValue(forKey: sessionKey) { sessionStates[resultEvent.sessionId] = state } - sessionIdRedirect[sessionKey] = resultEvent.sessionId + applySessionIdRedirect(from: sessionKey, to: resultEvent.sessionId) sessionKey = resultEvent.sessionId } let msgs = stateForSession(sessionKey).messages @@ -452,7 +448,7 @@ extension AppState { sessionStates[sid] = state } renameDraftState(from: previousSessionKey, to: sid, in: window) - sessionIdRedirect[previousSessionKey] = sid + applySessionIdRedirect(from: previousSessionKey, to: sid) sessionKey = sid startFlushTimer(for: sid) @@ -674,7 +670,6 @@ extension AppState { }) { state.messages[idx].isStreaming = false state.messages[idx].finalizeToolCalls() - Self.stripNoOpText(at: idx, in: &state.messages) } state.messages.append(ChatMessage(role: .assistant, isStreaming: true)) state.needsNewMessage = false @@ -716,7 +711,7 @@ extension AppState { sessionStates[resultEvent.sessionId] = state } renameDraftState(from: previousSessionKey, to: resultEvent.sessionId, in: window) - sessionIdRedirect[previousSessionKey] = resultEvent.sessionId + applySessionIdRedirect(from: previousSessionKey, to: resultEvent.sessionId) sessionKey = resultEvent.sessionId if wasForeground { window.currentSessionId = resultEvent.sessionId diff --git a/RxCode/App/AppState+Messaging.swift b/RxCode/App/AppState+Messaging.swift index ff3b6add..74fd6870 100644 --- a/RxCode/App/AppState+Messaging.swift +++ b/RxCode/App/AppState+Messaging.swift @@ -456,7 +456,6 @@ extension AppState { if let start = state.streamingStartDate { state.messages[idx].duration = Date().timeIntervalSince(start) } - Self.stripNoOpText(at: idx, in: &state.messages) } state.streamingStartDate = nil } @@ -468,42 +467,154 @@ extension AppState { // MARK: - Stream Completion (cross-project MCP) - /// Record that the stream `streamId` finished. Stored in - /// `pendingStreamCompletions` for any `awaitStreamCompletion(...)` caller - /// (currently `ide__send_to_thread`) to pick up. Latest call wins, except - /// we don't overwrite a success with an error from the fallback path. + /// Record that the stream `streamId` finished. If a caller is already + /// waiting via `awaitStreamCompletion(...)`, the result is handed to it + /// directly so it can return immediately; otherwise it's parked in + /// `pendingStreamCompletions` until someone picks it up. func recordStreamCompletion( streamId: UUID, sessionId: String, assistantText: String, error: String? ) { - pendingStreamCompletions[streamId] = StreamCompletion( + let completion = StreamCompletion( sessionId: sessionId, assistantText: assistantText, error: error ) + if let waiter = streamCompletionWaiters.removeValue(forKey: streamId) { + waiter.resume(with: completion) + return + } + pendingStreamCompletions[streamId] = completion } /// Wait up to `timeout` seconds for the stream identified by `streamId` - /// to record a completion. Polls every 100ms — MainActor serialization - /// means the recorder fires between sleeps. Returns the completion if - /// one arrived in time, otherwise `nil`. + /// to record a completion. Event-driven: `recordStreamCompletion` resumes + /// the continuation as soon as the result lands, with a parallel + /// `Task.sleep(timeout)` to surface `nil` if the deadline passes first. + /// Returns the completion if one arrived in time, otherwise `nil`. func awaitStreamCompletion(streamId: UUID, timeout: TimeInterval) async -> StreamCompletion? { - let deadline = Date().addingTimeInterval(timeout) - while Date() < deadline { + // Fast path — completion already landed before we registered. + if let completion = pendingStreamCompletions.removeValue(forKey: streamId) { + return completion + } + + let timeoutTask = Task { [weak self] in + let nanos = UInt64(max(0, timeout) * 1_000_000_000) + try? await Task.sleep(nanoseconds: nanos) + guard let self else { return } + await MainActor.run { + if let waiter = self.streamCompletionWaiters.removeValue(forKey: streamId) { + waiter.resume(with: nil) + } + } + } + + let result: StreamCompletion? = await withCheckedContinuation { cont in + let waiter = StreamCompletionWaiter(continuation: cont) + // Re-check between the fast-path read and continuation install — the + // recorder may have fired in between if any MainActor work yielded. if let completion = pendingStreamCompletions.removeValue(forKey: streamId) { - return completion + waiter.resume(with: completion) + } else { + streamCompletionWaiters[streamId] = waiter } - try? await Task.sleep(nanoseconds: 100_000_000) } - return pendingStreamCompletions.removeValue(forKey: streamId) + + timeoutTask.cancel() + return result } /// 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) { + waiter.resume(with: nil) + } + } + + // MARK: - Session-id rename handoff (cross-project MCP) + + /// Record that `pendingKey` was renamed to `realSessionId` (the CLI's own + /// `session_id`). Mirror writes to `sessionIdRedirect` already; this also + /// resumes any `awaitSessionRename` caller so the cross-project send can + /// return the real thread id instead of `pending-…`. + func applySessionIdRedirect(from pendingKey: String, to realSessionId: String) { + sessionIdRedirect[pendingKey] = realSessionId + if let waiter = sessionIdRenameWaiters.removeValue(forKey: pendingKey) { + waiter.resume(with: realSessionId) + } } + /// Wait up to `timeout` seconds for `pendingKey` to be renamed to the + /// CLI's real `session_id`. Fast-paths the answer if the rename already + /// landed. Returns `nil` on timeout. + func awaitSessionRename(pendingKey: String, timeout: TimeInterval) async -> String? { + if let real = sessionIdRedirect[pendingKey] { + return real + } + + let timeoutTask = Task { [weak self] in + let nanos = UInt64(max(0, timeout) * 1_000_000_000) + try? await Task.sleep(nanoseconds: nanos) + guard let self else { return } + await MainActor.run { + if let waiter = self.sessionIdRenameWaiters.removeValue(forKey: pendingKey) { + waiter.resume(with: nil) + } + } + } + + let result: String? = await withCheckedContinuation { cont in + let waiter = SessionRenameWaiter(continuation: cont) + if let real = sessionIdRedirect[pendingKey] { + waiter.resume(with: real) + } else { + sessionIdRenameWaiters[pendingKey] = waiter + } + } + + timeoutTask.cancel() + return result + } + +} + +/// Resumes a single waiting `awaitSessionRename` caller. Same single-shot +/// pattern as `StreamCompletionWaiter` — guards against double-resume in the +/// race between the rename notification and the timeout task. +@MainActor +final class SessionRenameWaiter { + private var continuation: CheckedContinuation? + + init(continuation: CheckedContinuation) { + self.continuation = continuation + } + + func resume(with value: String?) { + guard let cont = continuation else { return } + continuation = nil + cont.resume(returning: value) + } +} + +/// Resumes a single waiting `awaitStreamCompletion` caller. The class wrapper +/// guards against double-resume in the race between `recordStreamCompletion` +/// and the timeout task: whichever fires first wins, and the other becomes a +/// no-op when it finds the continuation already consumed. +@MainActor +final class StreamCompletionWaiter { + private var continuation: CheckedContinuation? + + init(continuation: CheckedContinuation) { + self.continuation = continuation + } + + func resume(with value: AppState.StreamCompletion?) { + guard let cont = continuation else { return } + continuation = nil + cont.resume(returning: value) + } } diff --git a/RxCode/App/AppState+Stream.swift b/RxCode/App/AppState+Stream.swift index 2ca7433f..db77646b 100644 --- a/RxCode/App/AppState+Stream.swift +++ b/RxCode/App/AppState+Stream.swift @@ -220,7 +220,6 @@ extension AppState { // New Claude turn after receiving tool result — start a new ChatMessage state.messages[idx].isStreaming = false state.messages[idx].finalizeToolCalls() - Self.stripNoOpText(at: idx, in: &state.messages) state.needsNewMessage = false state.messages.append(ChatMessage(role: .assistant, content: buffered, isStreaming: true)) } else { @@ -305,7 +304,6 @@ extension AppState { if let idx = state.messages.indices.reversed().first(where: { state.messages[$0].role == .assistant && state.messages[$0].isStreaming }) { state.messages[idx].isStreaming = false state.messages[idx].finalizeToolCalls() - Self.stripNoOpText(at: idx, in: &state.messages) } state.messages.append(ChatMessage(role: .assistant, isStreaming: true)) state.needsNewMessage = false @@ -444,13 +442,11 @@ extension AppState { // The user paused this turn — keep the partial assistant bubble // visible. markStreamInterrupted() clears the message's streaming // flag and retains in-progress tool calls (flagged as interrupted) - // instead of dropping them; the no-op strip below is told not to - // delete an emptied message. + // instead of dropping them. state.messages[idx].markStreamInterrupted() if let start = state.streamingStartDate { state.messages[idx].duration = Date().timeIntervalSince(start) } - Self.stripNoOpText(at: idx, in: &state.messages, removeIfEmpty: false) } state.streamingStartDate = nil state.inFlightUserAttachments = [] diff --git a/RxCode/App/AppState.swift b/RxCode/App/AppState.swift index 9a553115..48669434 100644 --- a/RxCode/App/AppState.swift +++ b/RxCode/App/AppState.swift @@ -172,6 +172,12 @@ final class AppState { /// id after the swap that happens mid-stream in `processStream`. var sessionIdRedirect: [String: String] = [:] + /// Callers waiting for `pending-` to be replaced by the CLI's + /// real `session_id` on the first `.system` event. The cross-project MCP + /// send (`ide__send_to_thread`) holds the JSON-RPC reply until the real + /// id is known so the sender's agent never sees a `pending-…` thread id. + var sessionIdRenameWaiters: [String: SessionRenameWaiter] = [:] + // MARK: - Stream Completion Tracking (cross-project MCP) /// Result of a finished stream. Used by `ide__send_to_thread` to surface @@ -186,6 +192,12 @@ final class AppState { /// have not yet picked up the result. Keyed by `streamId`. var pendingStreamCompletions: [UUID: StreamCompletion] = [:] + /// Active callers waiting for a stream's completion. Resumed directly by + /// `recordStreamCompletion` so the cross-project MCP handler doesn't sit + /// in a polling loop on MainActor (which starves the target project's + /// `processStream` and freezes its thread until the sender is cancelled). + var streamCompletionWaiters: [UUID: StreamCompletionWaiter] = [:] + // MARK: - Session Summaries (shared — lightweight metadata for all projects) var allSessionSummaries: [ChatSession.Summary] = [] @@ -798,6 +810,12 @@ final class AppState { let claude: ClaudeService let codex: CodexAppServer let acp: ACPService + + /// Test-only seam — populated by XCTests to substitute a mock backend for + /// the production `claude` / `codex` / `acp` services. `backend(for:)` + /// returns the override if one is registered, otherwise falls through to + /// the real service. Production code never writes to this dictionary. + var agentBackendOverrides: [AgentProvider: any AgentBackend] = [:] let acpRegistryService = ACPRegistryService() let openAISummarization = OpenAISummarizationService() let foundationModelSummarization = FoundationModelSummarizationService() diff --git a/RxCode/Services/ClaudeService+Process.swift b/RxCode/Services/ClaudeService+Process.swift index 230f99f4..6b64d7af 100644 --- a/RxCode/Services/ClaudeService+Process.swift +++ b/RxCode/Services/ClaudeService+Process.swift @@ -99,7 +99,12 @@ extension ClaudeCodeServer { return } - // Read stdout line-by-line — ends naturally at EOF + // Read stdout line-by-line via `readabilityHandler` rather than + // `FileHandle.AsyncBytes.lines`: the AsyncBytes iterator wedges + // when a second concurrent pipe reader is active (cross-project + // send spawns a second stream while the first is mid-tool-call), + // so the second CLI's events sit in the pipe and never wake the + // for-await. Dispatch's readable source delivers reliably. var parsedCount = 0 var failedCount = 0 let decoder = JSONDecoder() @@ -107,44 +112,40 @@ extension ClaudeCodeServer { var rawLineCount = 0 var capturedSessionId: String? - do { - for try await line in stdout.fileHandleForReading.bytes.lines { - guard !line.isEmpty else { continue } - guard let data = line.data(using: .utf8) else { continue } - - rawLineCount += 1 - // Diagnostic logging of raw NDJSON — full content for first 30 lines, then type field only - if let json = try? JSONSerialization.jsonObject(with: data) as? [String: Any] { - let type = (json["type"] as? String) ?? "?" - if rawLineCount <= 30 { - log.info("[Stream:RAW] #\(rawLineCount) type=\(type) line=\(line.prefix(600))") - } else if type == "stream_event" || rawLineCount % 50 == 0 { - log.info("[Stream:RAW] #\(rawLineCount) type=\(type)") - } - if capturedSessionId == nil, - let sid = (json["session_id"] as? String) ?? (json["sessionId"] as? String) { - capturedSessionId = sid - Task { await self.recordSessionId(streamId: streamId, sessionId: sid) } - } - } else if rawLineCount <= 30 { - log.info("[Stream:RAW] #\(rawLineCount) non-JSON line=\(line.prefix(600))") + for await line in Self.asyncLines(from: stdout.fileHandleForReading, log: log) { + guard !line.isEmpty else { continue } + guard let data = line.data(using: .utf8) else { continue } + + rawLineCount += 1 + // Diagnostic logging of raw NDJSON — full content for first 30 lines, then type field only + if let json = try? JSONSerialization.jsonObject(with: data) as? [String: Any] { + let type = (json["type"] as? String) ?? "?" + if rawLineCount <= 30 { + log.info("[Stream:RAW] #\(rawLineCount) type=\(type) line=\(line.prefix(600))") + } else if type == "stream_event" || rawLineCount % 50 == 0 { + log.info("[Stream:RAW] #\(rawLineCount) type=\(type)") } + if capturedSessionId == nil, + let sid = (json["session_id"] as? String) ?? (json["sessionId"] as? String) { + capturedSessionId = sid + Task { await self.recordSessionId(streamId: streamId, sessionId: sid) } + } + } else if rawLineCount <= 30 { + log.info("[Stream:RAW] #\(rawLineCount) non-JSON line=\(line.prefix(600))") + } - do { - let event = try decoder.decode(StreamEvent.self, from: data) - parsedCount += 1 - continuation.yield(event) - } catch { - failedCount += 1 - // Yield raw string so partial events still reach the UI - continuation.yield(.unknown(line)) - if failedCount <= 5 { - log.warning("[Stream] parse failed #\(failedCount): \(line.prefix(200))") - } + do { + let event = try decoder.decode(StreamEvent.self, from: data) + parsedCount += 1 + continuation.yield(event) + } catch { + failedCount += 1 + // Yield raw string so partial events still reach the UI + continuation.yield(.unknown(line)) + if failedCount <= 5 { + log.warning("[Stream] parse failed #\(failedCount): \(line.prefix(200))") } } - } catch { - log.warning("[Stream] stdout read error: \(error.localizedDescription)") } log.info("[Stream] stdout ended (parsed=\(parsedCount), failed=\(failedCount))") @@ -154,13 +155,53 @@ extension ClaudeCodeServer { continuation.onTermination = { reason in log.info("[Stream] terminated (reason=\(String(describing: reason)))") task.cancel() - // Close the pipe after the stream ends to unblock the bytes.lines read. - // onTermination is called after finish(), so there is no data loss. + // Detach the readability handler first so closing the pipe doesn't + // race a pending callback dispatch, then close to release the FD. + stdout.fileHandleForReading.readabilityHandler = nil stdout.fileHandleForReading.closeFile() } } } + /// Stream lines from `handle` using a Dispatch-backed `readabilityHandler`. + /// We use this instead of `FileHandle.AsyncBytes.lines` because the async + /// iterator can wedge when multiple concurrent pipe readers exist (the + /// cross-project send case: one CLI is mid-tool-call while another is + /// just starting). Dispatch's readable source delivers each chunk via a + /// per-handle background callback that doesn't share global async state, + /// so a second simultaneous reader is unaffected by the first's progress. + private static func asyncLines(from handle: FileHandle, log: Logger) -> AsyncStream { + AsyncStream { continuation in + // `buffer` is touched only from the readabilityHandler, which Dispatch + // serializes onto a single internal queue per FileHandle — no lock needed. + nonisolated(unsafe) var buffer = Data() + handle.readabilityHandler = { fh in + let chunk = fh.availableData + if chunk.isEmpty { + // EOF — flush any trailing non-terminated line, then finish. + if !buffer.isEmpty, let trailing = String(data: buffer, encoding: .utf8) { + continuation.yield(trailing) + buffer.removeAll(keepingCapacity: false) + } + fh.readabilityHandler = nil + continuation.finish() + return + } + buffer.append(chunk) + while let newlineIdx = buffer.firstIndex(of: 0x0A) { + let lineData = buffer[buffer.startIndex.. Self.messageMaxLength { Self.logger.error("[IDE] message exceeded limit, closing connection id=\(connectionId.uuidString, privacy: .public)") return } } catch { - Self.logger.info("[IDE] connection read ended: \(error.localizedDescription, privacy: .public)") + Self.logger.info("[IDE.runMCP] err conn=\(connectionId.uuidString.prefix(8), privacy: .public) session=\(sessionKey, privacy: .public) chunks=\(chunkCount): \(error.localizedDescription, privacy: .public)") return } } @@ -213,7 +222,8 @@ actor IDEMCPServer { data: Data.SubSequence, connection: NWConnection, sessionKey: String, - capabilities: CapabilitySet + capabilities: CapabilitySet, + connectionId: UUID ) async { let bytes = Data(data) guard @@ -229,9 +239,14 @@ actor IDEMCPServer { // Notifications (no id) — we only care about `notifications/initialized` if id == nil { + Self.logger.info("[IDE.recv] conn=\(connectionId.uuidString.prefix(8), privacy: .public) session=\(sessionKey, privacy: .public) notif=\(method ?? "", privacy: .public)") return } + let connTag = connectionId.uuidString.prefix(8) + let toolName = (params["name"] as? String) ?? "" + Self.logger.info("[IDE.recv] conn=\(connTag, privacy: .public) session=\(sessionKey, privacy: .public) method=\(method ?? "", privacy: .public) tool=\(toolName, privacy: .public)") + switch method { case "initialize": await reply( @@ -248,6 +263,7 @@ actor IDEMCPServer { ], on: connection ) + Self.logger.info("[IDE.sent] conn=\(connTag, privacy: .public) session=\(sessionKey, privacy: .public) reply=initialize") case "tools/list": let tools = await currentTools(sessionKey: sessionKey, capabilities: capabilities) @@ -256,26 +272,36 @@ actor IDEMCPServer { result: ["tools": tools.map(Self.toolDescriptor)], on: connection ) + Self.logger.info("[IDE.sent] conn=\(connTag, privacy: .public) session=\(sessionKey, privacy: .public) reply=tools/list count=\(tools.count)") case "tools/call": let name = params["name"] as? String ?? "" let arguments = params["arguments"] as? [String: Any] ?? [:] let argsValue = JSONValue.fromAny(arguments) + let callStart = Date() do { let handler = self.handler guard let handler else { throw IDEToolError.handlerFailed("IDE handler not attached") } + Self.logger.info("[IDE.call→] conn=\(connTag, privacy: .public) session=\(sessionKey, privacy: .public) tool=\(name, privacy: .public)") let result = try await handler.ideHandleToolCall( name: name, arguments: argsValue, sessionKey: sessionKey ) + let elapsed = Date().timeIntervalSince(callStart) + Self.logger.info("[IDE.call←] conn=\(connTag, privacy: .public) session=\(sessionKey, privacy: .public) tool=\(name, privacy: .public) elapsed=\(String(format: "%.1f", elapsed))s") let payload = Self.wrapToolResult(result) await reply(id: id!, result: payload, on: connection) + Self.logger.info("[IDE.sent] conn=\(connTag, privacy: .public) session=\(sessionKey, privacy: .public) reply=tools/call tool=\(name, privacy: .public)") } catch let error as IDEToolError { + let elapsed = Date().timeIntervalSince(callStart) + Self.logger.warning("[IDE.call✗] conn=\(connTag, privacy: .public) session=\(sessionKey, privacy: .public) tool=\(name, privacy: .public) elapsed=\(String(format: "%.1f", elapsed))s err=\(Self.errorMessage(for: error), privacy: .public)") await replyError(id: id!, code: Self.errorCode(for: error), message: Self.errorMessage(for: error), on: connection) } catch { + let elapsed = Date().timeIntervalSince(callStart) + Self.logger.warning("[IDE.call✗] conn=\(connTag, privacy: .public) session=\(sessionKey, privacy: .public) tool=\(name, privacy: .public) elapsed=\(String(format: "%.1f", elapsed))s err=\(error.localizedDescription, privacy: .public)") await replyError(id: id!, code: -32603, message: error.localizedDescription, on: connection) } diff --git a/RxCodeTests/CrossProjectSendConcurrencyTests.swift b/RxCodeTests/CrossProjectSendConcurrencyTests.swift new file mode 100644 index 00000000..34053533 --- /dev/null +++ b/RxCodeTests/CrossProjectSendConcurrencyTests.swift @@ -0,0 +1,251 @@ +import XCTest +import RxCodeCore +@testable import RxCode + +/// Regression tests for the cross-project `ide__send_to_thread` freeze. +/// +/// What the bug looked like: while one project's `processStream` was mid-turn +/// (mid-tool-call, mid-stream pause), a second concurrent `sendCrossProject` +/// for a different project would never see its `processStream` make progress — +/// the receiver thread sat at `isStreaming=true` with no assistant tokens until +/// the sender was cancelled. Two separate root causes were fixed: +/// +/// 1. `awaitStreamCompletion` polled MainActor every 100 ms, starving the +/// second stream's `for await event in stream` loop. (Now event-driven +/// via `StreamCompletionWaiter`.) +/// 2. The cross-project MCP reply returned `pending-` instead of +/// the CLI's real `session_id`, so the calling agent couldn't follow up +/// with `get_thread_messages`. (Now blocks on `awaitSessionRename`.) +/// +/// These tests substitute a `MockAgentBackend` for the real Claude/Codex/ACP +/// services via `appState.agentBackendOverrides`, so the production CLI spawn +/// + pipe-reader path isn't exercised — that's covered by the prod runtime +/// log artefacts and the readabilityHandler swap. The orchestration layer +/// (sendPrompt → processStream → waiters) is what these tests exercise. +@MainActor +final class CrossProjectSendConcurrencyTests: XCTestCase { + + private var appState: AppState! + private var mockBackend: MockAgentBackend! + private var defaultsSnapshot: [String: Any?] = [:] + + override func setUp() async throws { + // Snapshot the UserDefaults keys AppState seeds itself from, then pin + // a known provider so a stale value from a previous run (e.g. `.codex` + // from another test or a manual launch) doesn't route the test's + // backend dispatch around the mock and into a half-initialised real + // service that emits `.result(isError=true)`. + defaultsSnapshot = [ + "selectedAgentProvider": UserDefaults.standard.object(forKey: "selectedAgentProvider"), + "selectedModel": UserDefaults.standard.object(forKey: "selectedModel"), + ] + UserDefaults.standard.set("claudeCode", forKey: "selectedAgentProvider") + + appState = AppState(startBackgroundServices: false) + appState.selectedAgentProvider = .claudeCode + + mockBackend = MockAgentBackend(provider: .claudeCode) + // Register the mock for every provider so the test isn't sensitive to + // which provider `effectiveModelSelection` happens to resolve. + appState.agentBackendOverrides[.claudeCode] = mockBackend + appState.agentBackendOverrides[.codex] = mockBackend + appState.agentBackendOverrides[.acp] = mockBackend + } + + override func tearDown() async throws { + // Cancel anything still streaming so child Tasks don't outlive the test. + for key in appState.sessionStates.keys where appState.sessionStates[key]?.isStreaming == true { + appState.sessionStates[key]?.streamTask?.cancel() + appState.sessionStates[key]?.flushTask?.cancel() + } + mockBackend = nil + appState = nil + + for (key, value) in defaultsSnapshot { + if let value { + UserDefaults.standard.set(value, forKey: key) + } else { + UserDefaults.standard.removeObject(forKey: key) + } + } + } + + // MARK: - Real-sid resolution (PR #60 second commit) + + /// `sendCrossProject` must wait for the CLI's `.system init` event before + /// returning, so the calling agent never sees a `pending-` thread + /// id in the MCP reply. + func testCrossProjectSendReturnsRealSessionIdNotPendingPlaceholder() async throws { + let project = makeProject("alpha") + appState.projects = [project] + let realSid = "real-cli-sid-\(UUID().uuidString)" + + await mockBackend.enqueueScript( + [ + .systemInit(sessionId: realSid, delay: 0.05), + .assistantText("hello back", delay: 0.05), + .result(sessionId: realSid, delay: 0.05), + ], + forCwd: project.path + ) + + let result = try await appState.sendCrossProject( + projectId: project.id, + threadId: nil, + prompt: "ping", + agentProvider: .claudeCode, + waitForResponse: true, + timeoutSeconds: 10 + ) + + XCTAssertEqual(result.threadId, realSid, "MCP reply must carry the real session_id, not pending-…") + XCTAssertFalse(result.threadId.hasPrefix("pending-"), "threadId still starts with pending-: \(result.threadId)") + XCTAssertTrue(result.done, "wait_for_response=true should return done=true on success") + XCTAssertNil(result.error, "unexpected error: \(result.error ?? "")") + } + + // MARK: - Concurrent-stream progress (the original freeze) + + /// While project A's stream is paused mid-turn, a second `sendCrossProject` + /// for project B must still make progress and complete. Pre-fix this would + /// hang for the full duration of A's pause, because `awaitStreamCompletion`'s + /// 100 ms MainActor poll starved B's for-await. + func testSecondCrossProjectSendCompletesWhileFirstIsPausedMidStream() async throws { + let projectA = makeProject("alpha") + let projectB = makeProject("beta") + appState.projects = [projectA, projectB] + + let sidA = "sid-A-\(UUID().uuidString)" + let sidB = "sid-B-\(UUID().uuidString)" + + // A: emit init, then pause 3s, then finish. Long enough that any + // accidental serialization between streams shows up as a B timeout. + await mockBackend.enqueueScript( + [ + .systemInit(sessionId: sidA, delay: 0.05), + .assistantText("A starting", delay: 0.05), + .result(sessionId: sidA, delay: 3.0), + ], + forCwd: projectA.path + ) + + // B: completes quickly — total roughly 150 ms. + await mockBackend.enqueueScript( + [ + .systemInit(sessionId: sidB, delay: 0.05), + .assistantText("B done", delay: 0.05), + .result(sessionId: sidB, delay: 0.05), + ], + forCwd: projectB.path + ) + + // Kick off A in the background (don't await — A intentionally pauses). + let aState = appState! + let sendATask = Task { @MainActor in + _ = try? await aState.sendCrossProject( + projectId: projectA.id, + threadId: nil, + prompt: "long-running", + agentProvider: .claudeCode, + waitForResponse: false, + timeoutSeconds: 30 + ) + } + + // Wait briefly so A has entered its stream + emitted its first event. + try await Task.sleep(nanoseconds: 300_000_000) + XCTAssertTrue( + appState.sessionStates.values.contains(where: { $0.isStreaming }), + "A's stream should be active before launching B" + ) + + // Now send to B. It must complete fast even though A is paused. + let bStart = Date() + let resultB = try await appState.sendCrossProject( + projectId: projectB.id, + threadId: nil, + prompt: "should-not-be-blocked", + agentProvider: .claudeCode, + waitForResponse: true, + timeoutSeconds: 5 + ) + let bElapsed = Date().timeIntervalSince(bStart) + + XCTAssertTrue(resultB.done, "B must complete while A is paused (got error: \(resultB.error ?? "nil"))") + XCTAssertEqual(resultB.threadId, sidB) + XCTAssertLessThan( + bElapsed, 2.0, + "B took \(bElapsed)s — should be ~0.15s if not serialized behind A's 3s pause" + ) + + // Let A finish so the test tears down cleanly. + _ = await sendATask.value + } + + // MARK: - Event-driven completion handoff + + /// `recordStreamCompletion` must immediately resume a pending + /// `awaitStreamCompletion` waiter — no polling sleep in between. + func testRecordStreamCompletionResumesAwaiterImmediately() async throws { + let streamId = UUID() + let sessionId = "session-xyz" + let expectedText = "done" + let appStateRef = appState! + + let waiterTask = Task { @MainActor in + await appStateRef.awaitStreamCompletion(streamId: streamId, timeout: 5) + } + + // Wait a beat so the waiter is parked on its continuation, then record. + try await Task.sleep(nanoseconds: 50_000_000) + let startedAt = Date() + appState.recordStreamCompletion( + streamId: streamId, + sessionId: sessionId, + assistantText: expectedText, + error: nil + ) + + let completionOpt = await waiterTask.value + let completion = try XCTUnwrap(completionOpt) + let elapsed = Date().timeIntervalSince(startedAt) + + XCTAssertEqual(completion.sessionId, sessionId) + XCTAssertEqual(completion.assistantText, expectedText) + XCTAssertNil(completion.error) + XCTAssertLessThan( + elapsed, 0.05, + "awaitStreamCompletion took \(elapsed)s after record() — should be near-instant via continuation handoff" + ) + } + + /// Symmetric test for the session-id rename waiter: `applySessionIdRedirect` + /// must wake a pending `awaitSessionRename` caller immediately, not on a + /// poll cycle. + func testApplySessionIdRedirectResumesRenameWaiterImmediately() async throws { + let pendingKey = "pending-\(UUID().uuidString)" + let realSid = "real-\(UUID().uuidString)" + let appStateRef = appState! + + let waiterTask = Task { @MainActor in + await appStateRef.awaitSessionRename(pendingKey: pendingKey, timeout: 5) + } + + try await Task.sleep(nanoseconds: 50_000_000) + let startedAt = Date() + appState.applySessionIdRedirect(from: pendingKey, to: realSid) + + let resolvedOpt = await waiterTask.value + let resolved = try XCTUnwrap(resolvedOpt) + let elapsed = Date().timeIntervalSince(startedAt) + + XCTAssertEqual(resolved, realSid) + XCTAssertLessThan(elapsed, 0.05, "session-rename waiter took \(elapsed)s — should be near-instant") + } + + // MARK: - Helpers + + private func makeProject(_ name: String) -> Project { + Project(name: name, path: "/tmp/rxcode-test-\(name)-\(UUID().uuidString)", gitHubRepo: nil) + } +} diff --git a/RxCodeTests/MockAgentBackend.swift b/RxCodeTests/MockAgentBackend.swift new file mode 100644 index 00000000..bb8e8453 --- /dev/null +++ b/RxCodeTests/MockAgentBackend.swift @@ -0,0 +1,126 @@ +import Foundation +import RxCodeCore + +/// Test-only `AgentBackend` implementation that emits a scripted sequence of +/// `StreamEvent`s for each `send(_:)` call instead of spawning a real CLI. +/// +/// Scripts are keyed by the request's `cwd` because the `streamId` is generated +/// inside `sendPrompt` and isn't visible to the test ahead of time. Each +/// enqueued script is consumed once (FIFO per cwd); if no script is registered +/// for a cwd, the mock emits a minimal init+result pair so `processStream` +/// completes cleanly without hanging the test. +actor MockAgentBackend: AgentBackend { + + // MARK: - Step + + struct Step: Sendable { + /// Pause before yielding `event`. Use a long delay to simulate a + /// stream that's "stuck mid-turn" (the cross-project freeze case). + let delay: TimeInterval + let event: StreamEvent + + static func systemInit(sessionId: String, delay: TimeInterval = 0.01) -> Step { + Step(delay: delay, event: .system(SystemEvent( + subtype: "init", + sessionId: sessionId, + tools: nil, + model: nil, + claudeCodeVersion: nil + ))) + } + + static func assistantText(_ text: String, delay: TimeInterval = 0.01) -> Step { + Step(delay: delay, event: .assistant(AssistantMessage( + id: UUID().uuidString, + role: "assistant", + content: [.text(text)], + usage: nil + ))) + } + + static func result(sessionId: String, isError: Bool = false, delay: TimeInterval = 0.01) -> Step { + Step(delay: delay, event: .result(ResultEvent( + durationMs: 10, + totalCostUsd: 0, + sessionId: sessionId, + isError: isError, + totalTurns: 1, + usage: nil, + contextWindow: nil + ))) + } + } + + // MARK: - State + + nonisolated let provider: AgentProvider + nonisolated let staticCapabilities: CapabilitySet + + /// FIFO per-cwd script queue. Each entry is one turn's events. + private var scriptsByCwd: [String: [[Step]]] = [:] + + /// Observed `send(_:)` requests, in order. Tests can inspect to assert + /// the orchestration layer dispatched what they expected. + private(set) var receivedRequests: [BackendSendRequest] = [] + + // MARK: - Init + + init(provider: AgentProvider = .claudeCode) { + self.provider = provider + self.staticCapabilities = provider.staticCapabilities + } + + // MARK: - Test configuration + + /// Register one turn's worth of events for the next `send(_:)` whose + /// `cwd` matches. Multiple enqueues for the same cwd are consumed FIFO. + func enqueueScript(_ steps: [Step], forCwd cwd: String) { + scriptsByCwd[cwd, default: []].append(steps) + } + + // MARK: - AgentBackend + + func send(_ request: BackendSendRequest) -> AsyncStream { + receivedRequests.append(request) + + let script: [Step] + if var queue = scriptsByCwd[request.cwd], !queue.isEmpty { + script = queue.removeFirst() + scriptsByCwd[request.cwd] = queue + } else { + // No script registered — emit a harmless init+result so the + // caller's `for await event in stream` terminates instead of + // wedging the test. + let sid = request.sessionId ?? UUID().uuidString + script = [ + .systemInit(sessionId: sid), + .result(sessionId: sid), + ] + } + + return AsyncStream { continuation in + // Spawn outside the actor: the script's `Task.sleep` calls must + // not hold the mock's actor lock or the test's own assertions + // (which may also access the mock) would deadlock. + Task.detached { + for step in script { + if step.delay > 0 { + try? await Task.sleep(nanoseconds: UInt64(step.delay * 1_000_000_000)) + } + continuation.yield(step.event) + } + continuation.finish() + } + } + } + + func cancel(streamId: UUID) { + // No-op — scripts run to completion or hit `continuation.finish()` on + // their own. Cancellation in the production backend kills the CLI + // process; the mock has nothing analogous to tear down. + } + + func finalize(streamId: UUID) { + // No-op for the same reason as `cancel(streamId:)`. + } +}