diff --git a/Packages/Sources/RxCodeChatKit/ChatBridge.swift b/Packages/Sources/RxCodeChatKit/ChatBridge.swift index 1c0e70df..d57a978e 100644 --- a/Packages/Sources/RxCodeChatKit/ChatBridge.swift +++ b/Packages/Sources/RxCodeChatKit/ChatBridge.swift @@ -31,6 +31,13 @@ public final class ChatBridge { public var claudeVersion: String? public var codexVersion: String? + /// User decision summaries for `ExitPlanMode` tool calls in the current session, + /// keyed by `toolCallId`. Pushed by AppState from per-session state and persisted + /// SwiftData rows. Read by `PlanCardView` and `pendingPlans` so the chip surfaces + /// the user's decision even after the CLI's own follow-up tool_result has replaced + /// `ToolCall.result` (which happens on every reload of a CLI-backed session). + public var planDecisionSummaries: [String: String] = [:] + // MARK: - Action Handlers (set up by the app target) public var sendHandler: (() async -> Void)? @@ -120,15 +127,14 @@ public final class ChatBridge { guard let toolCall = message.blocks[blockIdx].toolCall, PlanCardView.isExitPlanMode(toolCall) else { continue } + if planDecisionSummaries[toolCall.id] != nil { return false } if PlanCardView.isPlanDecided(toolCall) { return false } // If anything came after this ExitPlanMode — later blocks in the // same message, or any later message — the chat continued past - // it, so the user must have already acted on the plan. This - // covers CLI-backed sessions reloaded from disk: the CLI's - // recorded tool_result for ExitPlanMode does not match the UI's - // in-memory decision summary, so the result string alone is not - // a reliable signal once the session has been persisted. + // it, so the user must have already acted on the plan. Covers + // edge cases where neither the persisted decision nor the + // in-memory result has caught up yet (e.g. mid-reconcile). let hasLaterBlock = blockIdx < message.blocks.count - 1 let hasLaterMessage = messageIdx < messages.count - 1 if hasLaterBlock || hasLaterMessage { return false } @@ -138,4 +144,44 @@ public final class ChatBridge { } return false } + + /// All `ExitPlanMode` tool calls in the current session, mapped to a + /// presentation-ready `PendingPlan`. Includes both undecided plans (banner + /// surfaces these) and decided plans (inline chip renders the outcome). + /// Superseded plans are excluded so a re-emitted plan replaces the prior one. + public var pendingPlans: [PendingPlan] { + var collected: [PendingPlan] = [] + for message in messages { + for block in message.blocks { + guard let toolCall = block.toolCall, + PlanCardView.isExitPlanMode(toolCall) else { continue } + if PlanCardView.isSupersededExitPlanMode( + toolCall: toolCall, + in: message, + allMessages: messages + ) { continue } + + let inlineMd = toolCall.input["plan"]?.stringValue ?? "" + let markdown: String = inlineMd.isEmpty + ? (PlanCardView.fallbackPlanMarkdown(in: message) + ?? PlanCardView.latestPriorPlanMarkdown(before: message, in: messages) + ?? "") + : inlineMd + + let persistedSummary = planDecisionSummaries[toolCall.id] + let decided = persistedSummary != nil || PlanCardView.isPlanDecided(toolCall) + let summary: String? = persistedSummary + ?? (PlanCardView.isPlanDecided(toolCall) ? toolCall.result : nil) + let isStreaming = message.isStreaming && markdown.isEmpty + collected.append(PendingPlan( + toolCallId: toolCall.id, + markdown: markdown, + isStreaming: isStreaming, + isDecided: decided, + decisionSummary: summary + )) + } + } + return collected + } } diff --git a/Packages/Sources/RxCodeChatKit/InputBarView.swift b/Packages/Sources/RxCodeChatKit/InputBarView.swift index d62756e6..636a6aac 100644 --- a/Packages/Sources/RxCodeChatKit/InputBarView.swift +++ b/Packages/Sources/RxCodeChatKit/InputBarView.swift @@ -3,13 +3,15 @@ import UniformTypeIdentifiers import RxCodeCore struct InputBarView: View { - @Environment(ChatBridge.self) private var chatBridge - @Environment(WindowState.self) private var windowState + @Environment(ChatBridge.self) private var environmentChatBridge: ChatBridge? + @Environment(WindowState.self) private var environmentWindowState: WindowState? @State private var isInputFocused: Bool = false @State private var inputFocusTrigger: UUID? = nil private let accessory: Accessory private let topAccessory: TopAccessory + private let injectedChatBridge: ChatBridge? + private let injectedWindowState: WindowState? @State private var showFilePicker = false @State private var showSlashPopup = false @@ -28,6 +30,28 @@ struct InputBarView: View { init(accessory: Accessory, @ViewBuilder topAccessory: () -> TopAccessory) { self.accessory = accessory self.topAccessory = topAccessory() + self.injectedChatBridge = nil + self.injectedWindowState = nil + } + + init( + windowState: WindowState, + chatBridge: ChatBridge, + accessory: Accessory, + @ViewBuilder topAccessory: () -> TopAccessory + ) { + self.accessory = accessory + self.topAccessory = topAccessory() + self.injectedChatBridge = chatBridge + self.injectedWindowState = windowState + } + + private var chatBridge: ChatBridge { + injectedChatBridge ?? environmentChatBridge! + } + + private var windowState: WindowState { + injectedWindowState ?? environmentWindowState! } var body: some View { @@ -805,9 +829,20 @@ struct InputBarView: View { private func processNextQueued() { guard let next = chatBridge.dequeueNextForFlush() else { return } + let draftText = windowState.inputText + let draftAttachments = windowState.attachments + let shouldRestoreDraft = !draftText.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty + || !draftAttachments.isEmpty + windowState.inputText = next.text windowState.attachments = next.attachments - Task { await chatBridge.send() } + Task { + await chatBridge.send() + if shouldRestoreDraft { + windowState.inputText = draftText + windowState.attachments = draftAttachments + } + } } private func handleReturnKey() { diff --git a/Packages/Sources/RxCodeChatKit/MessageBubble.swift b/Packages/Sources/RxCodeChatKit/MessageBubble.swift index c13fb758..8e0e1589 100644 --- a/Packages/Sources/RxCodeChatKit/MessageBubble.swift +++ b/Packages/Sources/RxCodeChatKit/MessageBubble.swift @@ -68,6 +68,12 @@ struct MessageBubble: View { } else { // Assistant message: render blocks in order let renderBlocks = assistantRenderBlocks() + // While the model is paused on an undecided ExitPlanMode in this + // same message, sibling tools without results are effectively + // suspended — not running. Drop the streaming flag for those so + // their spinner doesn't keep ticking until the user approves. + let siblingsArePaused = messageHasPendingExitPlanMode + let siblingStreaming = message.isStreaming && !siblingsArePaused ForEach(renderBlocks) { block in Group { @@ -90,7 +96,7 @@ struct MessageBubble: View { externalPlanMarkdown: external ) } else { - ToolResultView(toolCall: toolCall, isMessageStreaming: message.isStreaming) + ToolResultView(toolCall: toolCall, isMessageStreaming: siblingStreaming) } case .transientTools(let id, let tools): transientToolSummary(groupId: id, tools: tools) @@ -353,6 +359,24 @@ struct MessageBubble: View { return .asymmetric(insertion: insertion, removal: .identity) } + // MARK: - Plan Pause Helpers + + /// True when this assistant message contains an `ExitPlanMode` tool call that + /// the user has not yet decided on. While true, the CLI is suspended on the + /// PreToolUse hook — sibling tool calls in the same message will never get + /// their `tool_result` delivered until the plan is approved/rejected, so the + /// "running" spinner on those siblings would otherwise tick forever. + private var messageHasPendingExitPlanMode: Bool { + for block in message.blocks { + guard let toolCall = block.toolCall, + PlanCardView.isExitPlanMode(toolCall) else { continue } + if chatBridge.planDecisionSummaries[toolCall.id] != nil { continue } + if PlanCardView.isPlanDecided(toolCall) { continue } + return true + } + return false + } + // MARK: - Transient Tool Helpers /// Read, Grep, Glob, Bash etc. are collapsed into a summary after streaming completes @@ -393,7 +417,19 @@ struct MessageBubble: View { pendingTransientGroupStartId = nil } - for block in message.blocks { + // When the model emits an ExitPlanMode tool call together with a trailing + // narration text ("Plan written with 'hi'. Awaiting approval."), the API + // stream orders them tool-then-text. Lift sibling text blocks ahead of the + // plan chip so the user sees the description with the plan rather than + // dangling after a chip that's already been decided. + let orderedBlocks: [MessageBlock] = { + guard PlanCardView.containsExitPlanMode(message) else { return message.blocks } + let textBlocks = message.blocks.filter { $0.isText } + let nonTextBlocks = message.blocks.filter { !$0.isText } + return textBlocks + nonTextBlocks + }() + + for block in orderedBlocks { if PlanCardView.shouldHideBlock(block, in: message, allMessages: chatBridge.messages) { continue } if block.isText { flushTransientTools() diff --git a/Packages/Sources/RxCodeChatKit/MessageListView.swift b/Packages/Sources/RxCodeChatKit/MessageListView.swift index 6f136adf..e1761e7f 100644 --- a/Packages/Sources/RxCodeChatKit/MessageListView.swift +++ b/Packages/Sources/RxCodeChatKit/MessageListView.swift @@ -78,7 +78,10 @@ struct MessageListView: View { } } - if chatBridge.isStreaming { + if chatBridge.isStreaming && !chatBridge.hasPendingPlanDecision { + // Hide the spinner/dots while the CLI is paused waiting on the + // user's plan decision — the model isn't actually generating + // tokens, so showing "in progress" is misleading. HStack(alignment: .top, spacing: 0) { StreamingIndicatorView( isThinking: chatBridge.isThinking, diff --git a/Packages/Sources/RxCodeChatKit/PlanCardView.swift b/Packages/Sources/RxCodeChatKit/PlanCardView.swift index f46a8230..64e39fbf 100644 --- a/Packages/Sources/RxCodeChatKit/PlanCardView.swift +++ b/Packages/Sources/RxCodeChatKit/PlanCardView.swift @@ -2,10 +2,16 @@ import SwiftUI import RxCodeCore import AppKit -/// Interactive UI for a Claude `ExitPlanMode` tool call, plus a fallback for plan -/// markdown files written via `Write` to `~/.claude/plans/*.md`. Renders the plan as -/// rich markdown and (when pending) shows action buttons for the user to accept -/// or reject the plan. +/// Compact inline status chip for a Claude `ExitPlanMode` tool call. Replaces the +/// full markdown card that used to render here — the markdown body and decision +/// buttons now live in `PlanSheetView`, opened by tapping the chip or the +/// pending-plan banner above the input bar. The chip itself only communicates +/// the basic status: "Plan ready" while pending, the decision summary once +/// resolved, or a streaming placeholder while the call is still arriving. +/// +/// The static helpers on this type are reused by `MessageBubble`, +/// `ChatBridge.pendingPlans`, and `AppState` to identify and de-duplicate plan +/// blocks across the chat history. They are unchanged from the original card. struct PlanCardView: View { let toolCall: ToolCall let planMarkdown: String @@ -15,45 +21,41 @@ struct PlanCardView: View { /// the plan to `~/.claude/plans/*.md` in a previous turn before an /// `AskUserQuestion` split the assistant run). var externalPlanMarkdown: String? = nil + /// Optional override for the chip's tap behavior. When nil (production + /// default), the chip writes `toolCall.id` to `windowState.presentedPlanToolCallId` + /// to open the plan sheet. Tests inject a closure to capture the tap without + /// needing the SwiftUI environment. + var onOpen: ((String) -> Void)? = nil @Environment(WindowState.self) private var windowState + /// Optional so isolated test mounts (without a ChatBridge ancestor) don't + /// fatal on env lookup. Production hosts always inject one. + @Environment(ChatBridge.self) private var chatBridge: ChatBridge? + @State private var isHovered: Bool = false // Match the summary strings written by AppState.respondToPlanDecision. Any other // non-nil result (e.g., CLI-side "Exit plan mode?" responses) must not be treated - // as a user decision, otherwise the accept/reject buttons get hidden before the - // user has actually clicked one. - static let userDecisionPrefixes: [String] = [ - "Accepted with Ask", - "Accepted with Edits", - "Accepted with Auto-approve", - "Rejected", - ] + // as a user decision, otherwise the chip flips to "decided" before the user has + // actually clicked anything. + static let userDecisionPrefixes: [String] = PlanDecisionAction.userDecisionResultPrefixes static func isPlanDecided(_ toolCall: ToolCall) -> Bool { guard let result = toolCall.result else { return false } - return userDecisionPrefixes.contains { result.hasPrefix($0) } + return PlanDecisionAction.isUserDecisionResult(result) } - @State private var isExpanded: Bool - @State private var showFullSheet: Bool = false - @State private var feedbackText: String = "" - @State private var isComposingFeedback: Bool = false - @State private var isResolving: Bool = false - init( toolCall: ToolCall, planMarkdown: String, isMessageStreaming: Bool, - externalPlanMarkdown: String? = nil + externalPlanMarkdown: String? = nil, + onOpen: ((String) -> Void)? = nil ) { self.toolCall = toolCall self.planMarkdown = planMarkdown self.isMessageStreaming = isMessageStreaming self.externalPlanMarkdown = externalPlanMarkdown - // Start collapsed when the plan is already decided so reloaded history - // doesn't re-display the full plan body. The .onChange below handles - // collapsing for live decisions. - self._isExpanded = State(initialValue: !Self.isPlanDecided(toolCall)) + self.onOpen = onOpen } /// Extract the plan markdown for a tool call, or nil if this isn't a plan-bearing call. @@ -194,7 +196,7 @@ struct PlanCardView: View { /// True if this `ExitPlanMode` tool call is followed by another `ExitPlanMode` /// in the same assistant run (no user message between). Used to hide stale plan - /// cards when the model re-emits a fresh plan — only the latest is actionable. + /// chips when the model re-emits a fresh plan — only the latest is actionable. static func isSupersededExitPlanMode( toolCall: ToolCall, in message: ChatMessage, @@ -220,20 +222,19 @@ struct PlanCardView: View { Self.isExitPlanMode(toolCall) } - private var isDecided: Bool { - Self.isPlanDecided(toolCall) + /// Decision summary sourced first from the persisted sidecar dict (survives + /// CLI-backed reloads), then falling back to `toolCall.result` for the + /// brief in-flight window before the persisted value has been read back + /// (and for non-CLI sessions where the dict is empty). + private var persistedDecisionSummary: String? { + if let s = chatBridge?.planDecisionSummaries[toolCall.id] { return s } + guard let result = toolCall.result, + PlanDecisionAction.isUserDecisionResult(result) else { return nil } + return result } - private var isStreaming: Bool { - // Spinner only while the parent assistant message is still streaming AND no - // plan content has arrived yet (including a prior-turn fallback). Once the - // message stops streaming, drop the spinner even if planMarkdown is still - // empty — otherwise the card stays stuck when input_json_delta never arrives - // or the parsed input lacks a `plan` key. - guard isMessageStreaming else { return false } - if !planMarkdown.isEmpty { return false } - if let external = externalPlanMarkdown, !external.isEmpty { return false } - return true + private var isDecided: Bool { + persistedDecisionSummary != nil } private var resolvedMarkdown: String { @@ -241,379 +242,107 @@ struct PlanCardView: View { return externalPlanMarkdown ?? "" } - var body: some View { - VStack(alignment: .leading, spacing: 12) { - header - - if isExpanded { - planBody + private var isStreaming: Bool { + guard isMessageStreaming else { return false } + return resolvedMarkdown.isEmpty + } - if isExitPlanMode { - decisionArea + var body: some View { + Button(action: openSheet) { + HStack(spacing: 8) { + statusIcon + statusText + Spacer(minLength: 8) + if !isStreaming { + Text(actionLabel) + .font(.system(size: ClaudeTheme.messageSize(11), weight: .semibold)) + .foregroundStyle(.white) + .padding(.horizontal, 10) + .padding(.vertical, 4) + .background(ClaudeTheme.accent, in: Capsule()) } - } else { - collapsedPreview - } - } - .bubbleStyle(.tool) - .sheet(isPresented: $showFullSheet) { - PlanFullSheet(markdown: resolvedMarkdown) - } - .onChange(of: isDecided) { _, newValue in - if newValue { - withAnimation(.easeInOut(duration: 0.2)) { isExpanded = false } } + .padding(.horizontal, 12) + .padding(.vertical, 8) + .background( + RoundedRectangle(cornerRadius: 10) + .fill(isDecided ? ClaudeTheme.surfaceSecondary : ClaudeTheme.accentSubtle) + ) + .overlay( + RoundedRectangle(cornerRadius: 10) + .strokeBorder( + isDecided + ? ClaudeTheme.borderSubtle + : ClaudeTheme.accent.opacity(isHovered ? 0.55 : 0.35), + lineWidth: 1 + ) + ) + .contentShape(RoundedRectangle(cornerRadius: 10)) } + .buttonStyle(.plain) + .pointerCursorOnHover() + .onHover { isHovered = $0 } + .disabled(isStreaming) + .help(helpText) + .animation(.easeInOut(duration: 0.12), value: isHovered) } - // MARK: - Header - @ViewBuilder - private var header: some View { - HStack(spacing: 8) { + private var statusIcon: some View { + if isStreaming { + ProgressView().controlSize(.small) + } else if isDecided { + Image(systemName: "checkmark.seal.fill") + .font(.system(size: ClaudeTheme.messageSize(13), weight: .medium)) + .foregroundStyle(ClaudeTheme.accent) + } else { Image(systemName: "doc.text.fill") .font(.system(size: ClaudeTheme.messageSize(13), weight: .medium)) .foregroundStyle(ClaudeTheme.accent) - .frame(width: 16, height: 16) - - Text("Plan", bundle: .module) - .font(.system(size: ClaudeTheme.messageSize(13), weight: .semibold)) - .foregroundStyle(ClaudeTheme.textPrimary) - - Spacer() - - if isDecided { - Image(systemName: "checkmark.circle.fill") - .foregroundStyle(ClaudeTheme.statusSuccess) - .font(.caption) - } - - headerIconButton( - systemName: "doc.on.doc", - help: String(localized: "Copy plan", bundle: .module) - ) { - copyMarkdown() - } - - headerIconButton( - systemName: "arrow.up.left.and.arrow.down.right", - help: String(localized: "Open in full window", bundle: .module) - ) { - showFullSheet = true - } - - headerIconButton( - systemName: isExpanded ? "chevron.up" : "chevron.down", - help: String(localized: isExpanded ? "Collapse" : "Expand", bundle: .module) - ) { - withAnimation(.easeInOut(duration: 0.18)) { isExpanded.toggle() } - } } } @ViewBuilder - private func headerIconButton(systemName: String, help: String, action: @escaping () -> Void) -> some View { - Button(action: action) { - Image(systemName: systemName) - .font(.system(size: 11, weight: .medium)) - .foregroundStyle(ClaudeTheme.textSecondary) - .frame(width: 22, height: 22) - .contentShape(Rectangle()) - } - .buttonStyle(.plain) - .help(help) - } - - // MARK: - Body - - @ViewBuilder - private var planBody: some View { + private var statusText: some View { if isStreaming { - // Avoid markdown flicker while input_json_delta is still streaming — show - // a lightweight placeholder until the tool call completes. - HStack(spacing: 6) { - ProgressView().controlSize(.small) - Text("Drafting plan…", bundle: .module) - .font(.system(size: ClaudeTheme.messageSize(12))) - .foregroundStyle(ClaudeTheme.textSecondary) - } - .padding(.vertical, 4) - } else if resolvedMarkdown.isEmpty { - Text("Plan content unavailable.", bundle: .module) - .font(.system(size: ClaudeTheme.messageSize(12))) - .foregroundStyle(ClaudeTheme.textSecondary) - .padding(.vertical, 4) - } else { - MarkdownContentView(text: resolvedMarkdown) - .frame(maxWidth: .infinity, alignment: .leading) - .padding(.vertical, 2) - } - } - - @ViewBuilder - private var collapsedPreview: some View { - let firstLine = resolvedMarkdown - .split(whereSeparator: \.isNewline) - .first - .map(String.init)? - .replacingOccurrences(of: #"^#+\s*"#, with: "", options: .regularExpression) - ?? "(empty plan)" - VStack(alignment: .leading, spacing: 4) { - Text(firstLine) + Text("Drafting plan…", bundle: .module) .font(.system(size: ClaudeTheme.messageSize(12))) .foregroundStyle(ClaudeTheme.textSecondary) + } else if let summary = persistedDecisionSummary, !summary.isEmpty { + Text(summary) + .font(.system(size: ClaudeTheme.messageSize(12), weight: .medium)) + .foregroundStyle(ClaudeTheme.textPrimary) .lineLimit(1) .truncationMode(.tail) - - if isDecided, let summary = toolCall.result, !summary.isEmpty { - HStack(spacing: 4) { - Image(systemName: "checkmark.seal.fill") - .foregroundStyle(ClaudeTheme.accent) - .font(.system(size: 10, weight: .medium)) - Text(summary) - .font(.system(size: ClaudeTheme.messageSize(11), weight: .medium)) - .foregroundStyle(ClaudeTheme.textSecondary) - .lineLimit(1) - .truncationMode(.tail) - } - } - } - } - - // MARK: - Decision Area - - @ViewBuilder - private var decisionArea: some View { - if isDecided { - decidedRow - } else if isComposingFeedback { - feedbackComposer - } else if !isStreaming { - decisionButtons - } - } - - @ViewBuilder - private var decidedRow: some View { - HStack(spacing: 6) { - Image(systemName: "checkmark.seal.fill") - .foregroundStyle(ClaudeTheme.accent) - .font(.system(size: 11, weight: .medium)) - Text(toolCall.result ?? "Decided") + } else { + Text("Plan ready", bundle: .module) .font(.system(size: ClaudeTheme.messageSize(12), weight: .medium)) - .foregroundStyle(ClaudeTheme.textSecondary) - .lineLimit(2) + .foregroundStyle(ClaudeTheme.textPrimary) } - .padding(.top, 4) } - @ViewBuilder - private var decisionButtons: some View { - VStack(spacing: 8) { - planButton( - title: String(localized: "Accept + Auto", bundle: .module), - systemImage: "wand.and.sparkles", - style: .primary, - fullWidth: true - ) { - submit(.acceptAutoApprove) - } - - planButton( - title: String(localized: "Accept Edits", bundle: .module), - systemImage: "pencil.tip.crop.circle.badge.checkmark", - style: .secondary, - fullWidth: true - ) { - submit(.acceptWithEdits) - } - - planButton( - title: String(localized: "Accept Ask", bundle: .module), - systemImage: "bolt.shield", - style: .secondary, - fullWidth: true - ) { - submit(.acceptAsk) - } - - planButton( - title: String(localized: "Reject with Reason", bundle: .module), - systemImage: "text.bubble", - style: .secondary, - fullWidth: true - ) { - feedbackText = "" - withAnimation(.easeInOut(duration: 0.18)) { isComposingFeedback = true } - } - - planButton( - title: String(localized: "Reject", bundle: .module), - systemImage: "xmark.circle", - style: .secondary, - fullWidth: true - ) { - submit(.reject) - } - } - .padding(.top, 4) - .disabled(isResolving) - .opacity(isResolving ? 0.6 : 1.0) + private var actionLabel: String { + isDecided + ? String(localized: "View", bundle: .module) + : String(localized: "Review", bundle: .module) } - @ViewBuilder - private var feedbackComposer: some View { - VStack(alignment: .leading, spacing: 8) { - Text("Tell Claude what to change", bundle: .module) - .font(.system(size: ClaudeTheme.messageSize(12), weight: .medium)) - .foregroundStyle(ClaudeTheme.textSecondary) - - TextField( - String(localized: "What would you like changed?", bundle: .module), - text: $feedbackText, - axis: .vertical - ) - .textFieldStyle(.plain) - .lineLimit(2...5) - .font(.system(size: ClaudeTheme.messageSize(13))) - .padding(.horizontal, 10) - .padding(.vertical, 8) - .background(ClaudeTheme.surfaceSecondary, in: RoundedRectangle(cornerRadius: 8)) - .overlay( - RoundedRectangle(cornerRadius: 8) - .strokeBorder(ClaudeTheme.borderSubtle, lineWidth: 1) - ) - .onSubmit { submitFeedback() } - - HStack(spacing: 8) { - planButton( - title: String(localized: "Cancel", bundle: .module), - systemImage: "arrow.uturn.backward", - style: .secondary - ) { - withAnimation(.easeInOut(duration: 0.18)) { isComposingFeedback = false } - } - Spacer() - planButton( - title: String(localized: "Send feedback", bundle: .module), - systemImage: "paperplane.fill", - style: .primary, - isDisabled: feedbackText.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty - ) { - submitFeedback() - } - } + private var helpText: String { + if isStreaming { + return String(localized: "Plan is still drafting…", bundle: .module) } - .disabled(isResolving) - .opacity(isResolving ? 0.6 : 1.0) - } - - private func submitFeedback() { - let trimmed = feedbackText.trimmingCharacters(in: .whitespacesAndNewlines) - guard !trimmed.isEmpty else { return } - submit(.rejectWithFeedback(reason: trimmed)) - } - - private func submit(_ action: PlanDecisionAction) { - guard !isResolving else { return } - isResolving = true - windowState.planDecisionHandler?(toolCall.id, action) - } - - private func copyMarkdown() { - let pasteboard = NSPasteboard.general - pasteboard.clearContents() - pasteboard.setString(resolvedMarkdown, forType: .string) - } - - // MARK: - Button helper - - private enum PlanButtonStyle { case primary, secondary } - - @ViewBuilder - private func planButton( - title: String, - systemImage: String, - style: PlanButtonStyle, - isDisabled: Bool = false, - fullWidth: Bool = false, - action: @escaping () -> Void - ) -> some View { - Button(action: action) { - HStack(spacing: 4) { - Image(systemName: systemImage) - .font(.system(size: 11, weight: .medium)) - Text(title) - .font(.system(size: ClaudeTheme.messageSize(12), weight: .medium)) - .lineLimit(1) - } - .frame(maxWidth: fullWidth ? .infinity : nil) - .foregroundStyle(style == .primary ? Color.white : ClaudeTheme.textPrimary) - .padding(.horizontal, 10) - .padding(.vertical, 8) - .background( - style == .primary - ? ClaudeTheme.accent - : ClaudeTheme.surfaceSecondary - ) - .clipShape(RoundedRectangle(cornerRadius: 8)) - .overlay( - RoundedRectangle(cornerRadius: 8) - .strokeBorder( - style == .primary ? Color.clear : ClaudeTheme.borderSubtle, - lineWidth: 1 - ) - ) - .opacity(isDisabled ? 0.4 : 1.0) + if isDecided { + return String(localized: "Open the plan to review the decision", bundle: .module) } - .buttonStyle(.plain) - .disabled(isDisabled) + return String(localized: "Open the plan to accept or reject", bundle: .module) } -} -// MARK: - Full Sheet - -private struct PlanFullSheet: View { - let markdown: String - @Environment(\.dismiss) private var dismiss - - var body: some View { - VStack(alignment: .leading, spacing: 0) { - HStack(spacing: 8) { - Image(systemName: "doc.text.fill") - .foregroundStyle(ClaudeTheme.accent) - Text("Plan", bundle: .module) - .font(.system(size: 14, weight: .semibold)) - Spacer() - Button { - let pb = NSPasteboard.general - pb.clearContents() - pb.setString(markdown, forType: .string) - } label: { - Image(systemName: "doc.on.doc") - } - .buttonStyle(.plain) - .help(String(localized: "Copy", bundle: .module)) - - Button { dismiss() } label: { - Image(systemName: "xmark") - } - .buttonStyle(.plain) - .help(String(localized: "Close", bundle: .module)) - .keyboardShortcut(.escape, modifiers: []) - } - .padding(.horizontal, 16) - .padding(.vertical, 12) - .background(ClaudeTheme.surfaceSecondary) - - Divider() - - ScrollView(.vertical) { - MarkdownContentView(text: markdown) - .frame(maxWidth: .infinity, alignment: .leading) - .padding(20) - } + private func openSheet() { + guard !isStreaming else { return } + if let onOpen { + onOpen(toolCall.id) + } else { + windowState.presentedPlanToolCallId = toolCall.id } - .frame(minWidth: 640, idealWidth: 760, minHeight: 480, idealHeight: 640) } } diff --git a/Packages/Sources/RxCodeChatKit/PlanSheetView.swift b/Packages/Sources/RxCodeChatKit/PlanSheetView.swift new file mode 100644 index 00000000..563aa20e --- /dev/null +++ b/Packages/Sources/RxCodeChatKit/PlanSheetView.swift @@ -0,0 +1,357 @@ +import SwiftUI +import RxCodeCore +import AppKit +import Combine + +/// Test-only inspection emissary used by `PlanSheetView` so ViewInspector tests +/// can observe @State transitions (composer reveal, feedback text). At runtime +/// `notice` is never published, so `visit` is a no-op and the hook costs nothing. +public final class PlanSheetInspection: @unchecked Sendable { + public let notice = PassthroughSubject() + public var callbacks: [UInt: (PlanSheetView) -> Void] = [:] + + public init() {} + + public func visit(_ view: PlanSheetView, _ line: UInt) { + if let callback = callbacks.removeValue(forKey: line) { + callback(view) + } + } +} + +/// Sheet UI for a Claude `ExitPlanMode` tool call. Shows the full plan markdown, +/// the five accept/reject decision buttons, and an inline feedback composer when +/// the user picks "Reject with Reason". +/// +/// Closing the sheet via the X button, Esc, or clicking outside calls `onClose`, +/// which dismisses the UI but leaves the underlying `PendingPlan` in +/// `ChatBridge.pendingPlans`. The user can re-open the sheet from the +/// pending-plan banner above the input bar or by tapping the inline status +/// chip in the chat. Only the explicit decision buttons resolve the CLI hook. +public struct PlanSheetView: View { + let plan: PendingPlan + /// Number of additional pending plans queued behind this one (e.g. when the + /// model emits multiple plans in close succession). Mirrors the + /// "+N more pending" badge on `QuestionSheetView`. + let remainingCount: Int + let onSubmit: (_ toolCallId: String, _ action: PlanDecisionAction) -> Void + /// Dismisses the sheet but leaves the plan in the pending queue so the user + /// can re-open it from the banner later. Does NOT resolve the CLI hook. + let onClose: () -> Void + + @State private var feedbackText: String + @State private var isComposingFeedback: Bool + @State private var isResolving: Bool = false + + // Test-only hook used by ViewInspector tests to observe @State transitions + // (e.g. feedback composer reveal). Does nothing at runtime. + internal let inspection = PlanSheetInspection() + + public init( + plan: PendingPlan, + remainingCount: Int, + onSubmit: @escaping (_ toolCallId: String, _ action: PlanDecisionAction) -> Void, + onClose: @escaping () -> Void + ) { + self.init( + plan: plan, + remainingCount: remainingCount, + initialFeedbackText: "", + initialIsComposingFeedback: false, + onSubmit: onSubmit, + onClose: onClose + ) + } + + /// Designated init with seams for `feedbackText`/`isComposingFeedback`. Tests + /// use this to mount the sheet with the composer already revealed, which + /// sidesteps a SwiftUI/ViewInspector double-free that triggers when the + /// `decisionButtons → feedbackComposer` view swap happens inside a hosted + /// view after a previous test already hosted+expelled a `PlanSheetView`. + internal init( + plan: PendingPlan, + remainingCount: Int, + initialFeedbackText: String, + initialIsComposingFeedback: Bool, + onSubmit: @escaping (_ toolCallId: String, _ action: PlanDecisionAction) -> Void, + onClose: @escaping () -> Void + ) { + self.plan = plan + self.remainingCount = remainingCount + self.onSubmit = onSubmit + self.onClose = onClose + self._feedbackText = State(initialValue: initialFeedbackText) + self._isComposingFeedback = State(initialValue: initialIsComposingFeedback) + } + + public var body: some View { + VStack(spacing: 0) { + topBar + Divider() + ScrollView(.vertical) { + planBody + .frame(maxWidth: .infinity, alignment: .leading) + .padding(20) + } + Divider() + footer + } + .frame(minWidth: 640, idealWidth: 760, minHeight: 480, idealHeight: 640) + .background(ClaudeTheme.background) + .onReceive(inspection.notice) { inspection.visit(self, $0) } + } + + // MARK: - Top bar + + private var topBar: some View { + HStack(alignment: .top, spacing: 8) { + Image(systemName: "doc.text.fill") + .foregroundStyle(ClaudeTheme.accent) + .padding(.top, 2) + + VStack(alignment: .leading, spacing: 2) { + Text("Plan", bundle: .module) + .font(.system(size: ClaudeTheme.size(15), weight: .semibold)) + .foregroundStyle(ClaudeTheme.textPrimary) + if remainingCount > 0 { + Text(String(format: String(localized: "+%d more pending", bundle: .module), remainingCount)) + .font(.system(size: ClaudeTheme.size(11))) + .foregroundStyle(ClaudeTheme.textTertiary) + } + } + Spacer() + + Button(action: copyMarkdown) { + Image(systemName: "doc.on.doc") + .font(.system(size: 12, weight: .medium)) + .foregroundStyle(ClaudeTheme.textSecondary) + .frame(width: 26, height: 26) + .background(Circle().fill(ClaudeTheme.surfaceSecondary)) + } + .buttonStyle(.plain) + .help(String(localized: "Copy plan", bundle: .module)) + + Button(action: onClose) { + Image(systemName: "xmark") + .font(.system(size: 12, weight: .semibold)) + .foregroundStyle(ClaudeTheme.textSecondary) + .frame(width: 26, height: 26) + .background(Circle().fill(ClaudeTheme.surfaceSecondary)) + } + .buttonStyle(.plain) + .disabled(isResolving) + .help(String(localized: "Close — you can review later from the banner", bundle: .module)) + .keyboardShortcut(.escape, modifiers: []) + } + .padding(.horizontal, 20) + .padding(.top, 18) + .padding(.bottom, 12) + } + + // MARK: - Body + + @ViewBuilder + private var planBody: some View { + if plan.markdown.isEmpty { + Text("Plan content unavailable.", bundle: .module) + .font(.system(size: ClaudeTheme.messageSize(13))) + .foregroundStyle(ClaudeTheme.textSecondary) + } else { + MarkdownContentView(text: plan.markdown) + } + } + + // MARK: - Footer + + @ViewBuilder + private var footer: some View { + if plan.isDecided { + decidedRow + .padding(.horizontal, 20) + .padding(.vertical, 14) + } else if isComposingFeedback { + feedbackComposer + .padding(.horizontal, 20) + .padding(.vertical, 14) + } else { + decisionButtons + .padding(.horizontal, 20) + .padding(.vertical, 14) + } + } + + private var decidedRow: some View { + HStack(spacing: 8) { + Image(systemName: "checkmark.seal.fill") + .foregroundStyle(ClaudeTheme.accent) + .font(.system(size: 13, weight: .medium)) + Text(plan.decisionSummary ?? String(localized: "Decided", bundle: .module)) + .font(.system(size: ClaudeTheme.messageSize(13), weight: .medium)) + .foregroundStyle(ClaudeTheme.textSecondary) + .lineLimit(2) + Spacer() + } + } + + private var decisionButtons: some View { + VStack(spacing: 8) { + planButton( + title: String(localized: "Accept + Auto", bundle: .module), + systemImage: "wand.and.sparkles", + style: .primary, + fullWidth: true + ) { + submit(.acceptAutoApprove) + } + + planButton( + title: String(localized: "Accept Edits", bundle: .module), + systemImage: "pencil.tip.crop.circle.badge.checkmark", + style: .secondary, + fullWidth: true + ) { + submit(.acceptWithEdits) + } + + planButton( + title: String(localized: "Accept Ask", bundle: .module), + systemImage: "bolt.shield", + style: .secondary, + fullWidth: true + ) { + submit(.acceptAsk) + } + + planButton( + title: String(localized: "Reject with Reason", bundle: .module), + systemImage: "text.bubble", + style: .secondary, + fullWidth: true + ) { + feedbackText = "" + withAnimation(.easeInOut(duration: 0.18)) { isComposingFeedback = true } + } + + planButton( + title: String(localized: "Reject", bundle: .module), + systemImage: "xmark.circle", + style: .secondary, + fullWidth: true + ) { + submit(.reject) + } + } + .disabled(isResolving) + .opacity(isResolving ? 0.6 : 1.0) + } + + private var feedbackComposer: some View { + VStack(alignment: .leading, spacing: 8) { + Text("Tell Claude what to change", bundle: .module) + .font(.system(size: ClaudeTheme.messageSize(12), weight: .medium)) + .foregroundStyle(ClaudeTheme.textSecondary) + + TextField( + String(localized: "What would you like changed?", bundle: .module), + text: $feedbackText, + axis: .vertical + ) + .textFieldStyle(.plain) + .lineLimit(2...5) + .font(.system(size: ClaudeTheme.messageSize(13))) + .padding(.horizontal, 10) + .padding(.vertical, 8) + .background(ClaudeTheme.surfaceSecondary, in: RoundedRectangle(cornerRadius: 8)) + .overlay( + RoundedRectangle(cornerRadius: 8) + .strokeBorder(ClaudeTheme.borderSubtle, lineWidth: 1) + ) + .onSubmit { submitFeedback() } + + HStack(spacing: 8) { + planButton( + title: String(localized: "Cancel", bundle: .module), + systemImage: "arrow.uturn.backward", + style: .secondary + ) { + withAnimation(.easeInOut(duration: 0.18)) { isComposingFeedback = false } + } + Spacer() + planButton( + title: String(localized: "Send feedback", bundle: .module), + systemImage: "paperplane.fill", + style: .primary, + isDisabled: feedbackText.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty + ) { + submitFeedback() + } + } + } + .disabled(isResolving) + .opacity(isResolving ? 0.6 : 1.0) + } + + // MARK: - Actions + + private func submitFeedback() { + let trimmed = feedbackText.trimmingCharacters(in: .whitespacesAndNewlines) + guard !trimmed.isEmpty else { return } + submit(.rejectWithFeedback(reason: trimmed)) + } + + private func submit(_ action: PlanDecisionAction) { + guard !isResolving else { return } + isResolving = true + onSubmit(plan.toolCallId, action) + } + + private func copyMarkdown() { + let pasteboard = NSPasteboard.general + pasteboard.clearContents() + pasteboard.setString(plan.markdown, forType: .string) + } + + // MARK: - Button helper + + private enum PlanButtonStyle { case primary, secondary } + + @ViewBuilder + private func planButton( + title: String, + systemImage: String, + style: PlanButtonStyle, + isDisabled: Bool = false, + fullWidth: Bool = false, + action: @escaping () -> Void + ) -> some View { + Button(action: action) { + HStack(spacing: 4) { + Image(systemName: systemImage) + .font(.system(size: 11, weight: .medium)) + Text(title) + .font(.system(size: ClaudeTheme.messageSize(12), weight: .medium)) + .lineLimit(1) + } + .frame(maxWidth: fullWidth ? .infinity : nil) + .foregroundStyle(style == .primary ? Color.white : ClaudeTheme.textPrimary) + .padding(.horizontal, 10) + .padding(.vertical, 8) + .background( + style == .primary + ? ClaudeTheme.accent + : ClaudeTheme.surfaceSecondary + ) + .clipShape(RoundedRectangle(cornerRadius: 8)) + .overlay( + RoundedRectangle(cornerRadius: 8) + .strokeBorder( + style == .primary ? Color.clear : ClaudeTheme.borderSubtle, + lineWidth: 1 + ) + ) + .opacity(isDisabled ? 0.4 : 1.0) + } + .buttonStyle(.plain) + .disabled(isDisabled) + } +} diff --git a/Packages/Sources/RxCodeCore/CLISession/CLILineToBlocksMapper.swift b/Packages/Sources/RxCodeCore/CLISession/CLILineToBlocksMapper.swift index 677988a4..44b8db36 100644 --- a/Packages/Sources/RxCodeCore/CLISession/CLILineToBlocksMapper.swift +++ b/Packages/Sources/RxCodeCore/CLISession/CLILineToBlocksMapper.swift @@ -48,6 +48,7 @@ public enum CLILineToBlocksMapper { let trimmed = s.trimmingCharacters(in: .whitespacesAndNewlines) guard !trimmed.isEmpty else { return } if CLIMetaEnvelope.isEnvelope(trimmed) { return } + if PlanDecisionAction.isContinuationPrompt(s) { return } messages.append(ChatMessage( role: .user, blocks: [.text(s)], @@ -63,6 +64,7 @@ public enum CLILineToBlocksMapper { let trimmed = t.trimmingCharacters(in: .whitespacesAndNewlines) guard !trimmed.isEmpty else { continue } if CLIMetaEnvelope.isEnvelope(trimmed) { continue } + if PlanDecisionAction.isContinuationPrompt(t) { continue } textsForNewMessage.append(t) case .toolResult(let id, let content, let isError): foldToolResult(id: id, result: content, isError: isError, into: &messages) @@ -91,10 +93,26 @@ public enum CLILineToBlocksMapper { // Walk backwards — tool_result almost always pairs with the most recent // assistant tool_use, but a single assistant turn can have multiple. for i in messages.indices.reversed() { - if messages[i].toolCallIndex(id: id) != nil { - messages[i].setToolResult(id: id, result: result, isError: isError) + guard let blockIdx = messages[i].toolCallIndex(id: id), + let call = messages[i].blocks[blockIdx].toolCall else { continue } + + // Preserve user-authored ExitPlanMode decision summaries. The CLI + // writes its own tool_result for ExitPlanMode after the user + // approves ("User has approved your plan…"), and folding that back + // would erase the "Accepted with …" / "Rejected" summary that the + // plan chip and pending-plan banner rely on. + let isExitPlan: Bool = { + let n = call.name.lowercased() + return n == "exitplanmode" || n == "exit_plan_mode" + }() + if isExitPlan, + let existing = call.result, + PlanDecisionAction.isUserDecisionResult(existing) { return } + + messages[i].setToolResult(id: id, result: result, isError: isError) + return } // Orphan tool_result (assistant line missing) — drop it silently. } diff --git a/Packages/Sources/RxCodeCore/Models/PermissionRequest.swift b/Packages/Sources/RxCodeCore/Models/PermissionRequest.swift index a4a9e1aa..aea8fbc7 100644 --- a/Packages/Sources/RxCodeCore/Models/PermissionRequest.swift +++ b/Packages/Sources/RxCodeCore/Models/PermissionRequest.swift @@ -90,8 +90,8 @@ public enum PermissionDecision: Sendable, Equatable { // MARK: - Plan Decision -/// User's choice on a Claude `ExitPlanMode` tool call. Drives the plan card buttons on -/// `PlanCardView` and is delivered to `AppState.respondToPlanDecision(...)` by the chat UI. +/// User's choice on a Claude `ExitPlanMode` tool call. Drives the plan sheet buttons on +/// `PlanSheetView` and is delivered to `AppState.respondToPlanDecision(...)` by the chat UI. public enum PlanDecisionAction: Sendable, Equatable { /// Allow the plan and switch the session to `.default` for the rest of the conversation. case acceptAsk @@ -103,4 +103,78 @@ public enum PlanDecisionAction: Sendable, Equatable { case rejectWithFeedback(reason: String) /// Deny the plan without user-authored feedback. case reject + + /// Prefixes of result strings written by `AppState.respondToPlanDecision`. + /// Anything starting with one of these is a user-authored decision and must + /// be preserved across CLI tool_result events and CLI-session reloads — + /// otherwise the plan chip would flip back to "Plan ready" once the CLI + /// records its own follow-up text (e.g., "User has approved your plan…"). + public static let userDecisionResultPrefixes: [String] = [ + "Accepted with Ask", + "Accepted with Edits", + "Accepted with Auto-approve", + "Rejected", + ] + + /// True if `result` is a user-authored plan decision (vs. CLI-side text). + public static func isUserDecisionResult(_ result: String) -> Bool { + userDecisionResultPrefixes.contains { result.hasPrefix($0) } + } + + /// Hidden follow-up user prompts that RxCode injects after a plan decision. + /// Kept here so the chat-bubble suppressor (live + reloaded sessions) and the + /// `AppState.continuationPrompt` producer share one source of truth — otherwise + /// the prompt re-appears as a user bubble when the CLI jsonl is re-parsed. + public static let continuationPromptPrefixes: [String] = [ + "Proceed with the plan.", + "Revise the plan based on this feedback: ", + ] + + /// True if `text` is an RxCode-injected plan-decision continuation prompt. + /// The CLI logs these to its jsonl just like a user-authored prompt, so the + /// reload path needs an explicit allow-list to drop them from the chat UI. + public static func isContinuationPrompt(_ text: String) -> Bool { + let trimmed = text.trimmingCharacters(in: .whitespacesAndNewlines) + return continuationPromptPrefixes.contains { trimmed.hasPrefix($0) } + } +} + +// MARK: - Pending Plan + +/// Denormalized view of an `ExitPlanMode` tool call for the input-bar banner and +/// plan sheet. Sourced from `ChatBridge.pendingPlans` (computed from the current +/// session's messages). Kept in `RxCodeCore` so both the banner (RxCode target) +/// and the sheet (RxCodeChatKit) can consume it without depending on chat-view types. +public struct PendingPlan: Identifiable, Equatable, Sendable { + /// The `ToolCall.id` of the underlying `ExitPlanMode` call. Used as the sheet's + /// presentation id and as the key for `windowState.planDecisionHandler`. + public let toolCallId: String + /// Plan markdown extracted from the tool-call input or fallback `Write` block. + public let markdown: String + /// True while the parent assistant message is still streaming AND no plan + /// markdown has arrived yet — banner suppresses these to avoid surfacing + /// an empty plan to "review". + public let isStreaming: Bool + /// True once `respondToPlanDecision` has written a decision summary. Decided + /// plans are excluded from the banner count but kept around so the inline + /// status chip can render the outcome. + public let isDecided: Bool + /// Decision summary string ("Accepted with Edits", etc.) when decided. + public let decisionSummary: String? + + public init( + toolCallId: String, + markdown: String, + isStreaming: Bool, + isDecided: Bool, + decisionSummary: String? + ) { + self.toolCallId = toolCallId + self.markdown = markdown + self.isStreaming = isStreaming + self.isDecided = isDecided + self.decisionSummary = decisionSummary + } + + public var id: String { toolCallId } } diff --git a/Packages/Sources/RxCodeCore/Models/PlanDecisionRecord.swift b/Packages/Sources/RxCodeCore/Models/PlanDecisionRecord.swift new file mode 100644 index 00000000..74980137 --- /dev/null +++ b/Packages/Sources/RxCodeCore/Models/PlanDecisionRecord.swift @@ -0,0 +1,22 @@ +import Foundation +import SwiftData + +/// Persisted user decision for a single `ExitPlanMode` tool call. One row per +/// (sessionId, toolCallId). Sidecar storage so the decision survives CLI-backed +/// session reloads — the CLI's own tool_result for ExitPlanMode ("User has +/// approved your plan, you may now start coding…") would otherwise overwrite +/// the in-memory `ToolCall.result` we previously used to record the decision. +@Model +public final class PlanDecisionRecord { + public var sessionId: String + public var toolCallId: String + public var summary: String + public var updatedAt: Date + + public init(sessionId: String, toolCallId: String, summary: String, updatedAt: Date = .now) { + self.sessionId = sessionId + self.toolCallId = toolCallId + self.summary = summary + self.updatedAt = updatedAt + } +} diff --git a/Packages/Sources/RxCodeCore/Theme/ClaudeTheme.swift b/Packages/Sources/RxCodeCore/Theme/ClaudeTheme.swift index 369e0e04..c18ef58e 100644 --- a/Packages/Sources/RxCodeCore/Theme/ClaudeTheme.swift +++ b/Packages/Sources/RxCodeCore/Theme/ClaudeTheme.swift @@ -211,6 +211,8 @@ public struct ClaudeSendButton: View { } .buttonStyle(.plain) .disabled(!isEnabled) + .accessibilityLabel("Send message") + .accessibilityIdentifier("chat-send-button") } } @@ -236,6 +238,8 @@ public struct ClaudeStopButton: View { } .buttonStyle(.plain) .help("Stop generating") + .accessibilityLabel("Stop generating") + .accessibilityIdentifier("chat-stop-button") } } diff --git a/Packages/Sources/RxCodeCore/WindowState.swift b/Packages/Sources/RxCodeCore/WindowState.swift index a8f9a59c..83c5f726 100644 --- a/Packages/Sources/RxCodeCore/WindowState.swift +++ b/Packages/Sources/RxCodeCore/WindowState.swift @@ -111,10 +111,17 @@ public final class WindowState { // MARK: - Plan Decision Handler - /// Invoked by the plan card when the user picks Accept / Accept-with-edits / - /// Accept / reject decisions from the inline plan card. Set by `AppState` at window init. + /// Invoked by the plan sheet when the user picks Accept / Accept-with-edits / + /// Accept / reject decisions. Set by `AppState` at window init. public var planDecisionHandler: (@MainActor @Sendable (String, PlanDecisionAction) -> Void)? + /// `ToolCall.id` of the plan whose sheet is currently presented. Nil means the + /// sheet is closed (banner / inline chip may still be visible). Set by the + /// banner's tap action and the inline chip; cleared by the sheet's close + /// button / Esc / outside-tap. Closing the sheet is NOT a decline — the plan + /// remains pending until a decision is recorded via `planDecisionHandler`. + public var presentedPlanToolCallId: String? + // MARK: - UI State public var interactiveTerminal: InteractiveTerminalState? diff --git a/Packages/Tests/RxCodeChatKitTests/InputBarQueueTests.swift b/Packages/Tests/RxCodeChatKitTests/InputBarQueueTests.swift new file mode 100644 index 00000000..019a2396 --- /dev/null +++ b/Packages/Tests/RxCodeChatKitTests/InputBarQueueTests.swift @@ -0,0 +1,68 @@ +import Testing +import SwiftUI +import ViewInspector +import RxCodeCore +@testable import RxCodeChatKit + +@MainActor +@Suite("InputBar queue interactions") +struct InputBarQueueTests { + + @Test("Draft typed while queued message auto-flushes is preserved") + func draftTypedWhileQueuedMessageAutoFlushesIsPreserved() async throws { + let window = WindowState() + let bridge = ChatBridge() + bridge.isStreaming = true + + var sentPrompts: [String] = [] + bridge.enqueueMessageHandler = { text, attachments in + window.enqueueMessage(text: text, attachments: attachments) + } + bridge.dequeueNextForFlushHandler = { + window.dequeueNext() + } + bridge.sendHandler = { + sentPrompts.append(window.inputText) + window.inputText = "" + window.attachments = [] + bridge.isStreaming = true + } + + let sut = InputBarView( + windowState: window, + chatBridge: bridge, + accessory: EmptyView() + ) { + EmptyView() + } + + ViewHosting.host(view: sut) + defer { ViewHosting.expel() } + + try type("queued turn", in: sut) + try sut.inspect() + .find(viewWithAccessibilityIdentifier: "chat-send-button") + .button() + .tap() + + #expect(window.messageQueue.map(\.text) == ["queued turn"]) + #expect(window.inputText.isEmpty) + + try type("draft typed during queued processing", in: sut) + bridge.isStreaming = false + try await Task.sleep(for: .milliseconds(100)) + + #expect(sentPrompts == ["queued turn"]) + #expect(window.messageQueue.isEmpty) + #expect(window.inputText == "draft typed during queued processing") + try await Task.sleep(for: .milliseconds(350)) + } + + private func type(_ text: String, in view: V) throws { + let input = try view.inspect() + .find(IMETextView.self) + .actualView() + input.text = text + } + +} diff --git a/Packages/Tests/RxCodeChatKitTests/InputBarStopRestoreTests.swift b/Packages/Tests/RxCodeChatKitTests/InputBarStopRestoreTests.swift new file mode 100644 index 00000000..24e6550a --- /dev/null +++ b/Packages/Tests/RxCodeChatKitTests/InputBarStopRestoreTests.swift @@ -0,0 +1,105 @@ +import Testing +import SwiftUI +import ViewInspector +import RxCodeCore +@testable import RxCodeChatKit + +@MainActor +@Suite("InputBar stop button") +struct InputBarStopRestoreTests { + + @Test("Stop button restores both text and attachments via cancel handler") + func stopButtonRestoresTextAndAttachments() async throws { + let window = WindowState() + let bridge = ChatBridge() + + // The chat has already sent a message with an image attachment and is now streaming. + // The composer has been cleared, mirroring what `AppState.send` does right after + // it dispatches the prompt to the CLI. + bridge.isStreaming = true + window.inputText = "" + window.attachments = [] + + let pendingUserText = "describe this image" + let imageAttachment = Attachment( + type: .image, + name: "rxcode-img-02AF8CDE.png", + path: "/tmp/rxcode-img-02AF8CDE.png", + fileSize: 1024 + ) + + // Stand-in for `AppState.cancelStreaming` after the fix — restore the in-flight + // user message into the composer, text AND attachments together. + bridge.cancelStreamingHandler = { + bridge.isStreaming = false + window.inputText = pendingUserText + window.attachments = [imageAttachment] + } + + let sut = InputBarView( + windowState: window, + chatBridge: bridge, + accessory: EmptyView() + ) { + EmptyView() + } + + ViewHosting.host(view: sut) + defer { ViewHosting.expel() } + + // Drive the real UI: locate the stop button via its accessibility id and tap it. + try sut.inspect() + .find(viewWithAccessibilityIdentifier: "chat-stop-button") + .button() + .tap() + + // `stopGeneration` dispatches via a Task; let it run. + try await Task.sleep(for: .milliseconds(50)) + + #expect(window.inputText == pendingUserText) + #expect(window.attachments.count == 1) + #expect(window.attachments.first?.name == "rxcode-img-02AF8CDE.png") + #expect(window.attachments.first?.type == .image) + + // The attachment preview row should now be back on screen, rendering the + // attachment's name through `AttachmentPreviewItem`. + #expect((try? sut.inspect().find(text: "rxcode-img-02AF8CDE.png")) != nil) + } + + @Test("Cancel handler that only restores text loses image attachments (regression guard)") + func cancelHandlerThatOnlyRestoresTextLosesAttachments() async throws { + // This test pins the bug we just fixed: if a future change reverts to only + // restoring `inputText` on cancel, the attachments silently disappear. + // It documents that behavior so anyone editing the cancel path has to + // think about attachments too. + let window = WindowState() + let bridge = ChatBridge() + bridge.isStreaming = true + + bridge.cancelStreamingHandler = { + bridge.isStreaming = false + window.inputText = "buggy restore" + // Intentionally NOT restoring `window.attachments`. + } + + let sut = InputBarView( + windowState: window, + chatBridge: bridge, + accessory: EmptyView() + ) { + EmptyView() + } + + ViewHosting.host(view: sut) + defer { ViewHosting.expel() } + + try sut.inspect() + .find(viewWithAccessibilityIdentifier: "chat-stop-button") + .button() + .tap() + try await Task.sleep(for: .milliseconds(50)) + + #expect(window.inputText == "buggy restore") + #expect(window.attachments.isEmpty) + } +} diff --git a/RxCode.xcodeproj/project.pbxproj b/RxCode.xcodeproj/project.pbxproj index 673f2c68..c420486b 100644 --- a/RxCode.xcodeproj/project.pbxproj +++ b/RxCode.xcodeproj/project.pbxproj @@ -263,6 +263,7 @@ isa = XCBuildConfiguration; buildSettings = { BUNDLE_LOADER = "$(TEST_HOST)"; + DEVELOPMENT_TEAM = P9KK452K8P; GENERATE_INFOPLIST_FILE = YES; MACOSX_DEPLOYMENT_TARGET = 26.2; PRODUCT_NAME = RxCodeTests; @@ -276,6 +277,7 @@ isa = XCBuildConfiguration; buildSettings = { BUNDLE_LOADER = "$(TEST_HOST)"; + DEVELOPMENT_TEAM = P9KK452K8P; GENERATE_INFOPLIST_FILE = YES; MACOSX_DEPLOYMENT_TARGET = 26.2; PRODUCT_NAME = RxCodeTests; diff --git a/RxCode/App/AppState.swift b/RxCode/App/AppState.swift index 09ea8dc9..c3cec223 100644 --- a/RxCode/App/AppState.swift +++ b/RxCode/App/AppState.swift @@ -12,6 +12,13 @@ struct SessionStreamState { // Messages var messages: [ChatMessage] = [] + /// Full `Attachment` objects (including image data, text content, etc.) + /// for the most recent user turn. `ChatMessage.attachmentPaths` is lossy — + /// it only stores name/path/type, so we keep the originals here so + /// `cancelStreaming` can restore them into the input bar alongside the + /// rolled-back user text. + var inFlightUserAttachments: [Attachment] = [] + /// True while persisted messages are being loaded from disk into this session. /// MessageListView keeps the ScrollView hidden while this is true to avoid the /// empty → populated "blink" when switching to a session that isn't already in memory. @@ -43,6 +50,13 @@ struct SessionStreamState { /// Per-session plan-mode toggle. When true, the CLI is launched with `--permission-mode plan` /// regardless of the user-selected permission mode. Cleared on session switch. var planMode: Bool = false + /// User decisions for `ExitPlanMode` tool calls, keyed by `toolCallId`. Sidecar + /// to `ToolCall.result` so the decision survives CLI-backed session reloads — + /// the CLI emits its own follow-up tool_result ("User has approved your plan…") + /// that we cannot prevent from overwriting `ToolCall.result` when the session + /// jsonl is parsed fresh from disk. Loaded from `ThreadStore` on session + /// activation, written through on every decision. + var planDecisionSummaries: [String: String] = [:] /// Override cwd for this session: when set, CLI runs in this Git worktree path /// instead of `project.path`. Persisted on `ChatSession.worktreePath`. var worktreePath: String? @@ -1352,6 +1366,7 @@ final class AppState { durationMs: state.durationMs, turns: state.turns ) + bridge.planDecisionSummaries = state.planDecisionSummaries } onChange: { Task { @MainActor in observeStream() } } @@ -1617,6 +1632,7 @@ final class AppState { content: displayText ?? prompt, attachments: attachments )) + state.inFlightUserAttachments = attachments } } @@ -2544,14 +2560,18 @@ final class AppState { !state.messages[lastIndex].isCompactBoundary { state.messages.remove(at: lastIndex) } - // Restore the user message that triggered this stream into the input field. + // Restore the user message that triggered this stream into the input field — + // both its text and the original attachment objects so images / pasted text + // aren't silently dropped when the user stops a turn. if let lastIndex = state.messages.indices.last, state.messages[lastIndex].role == .user, !state.messages[lastIndex].isCompactBoundary { let userText = state.messages[lastIndex].blocks.compactMap(\.text).joined() state.messages.remove(at: lastIndex) window.inputText = userText + window.attachments = state.inFlightUserAttachments } + state.inFlightUserAttachments = [] } window.showError = false @@ -2685,6 +2705,10 @@ final class AppState { // Record the outcome on the tool block so `PlanCardView` flips from buttons to // a "decided" status row. This mirrors how AskUserQuestionView reads `toolCall.result`. + // Also write into a sidecar dict that survives CLI-backed session reloads — + // the CLI emits its own follow-up tool_result ("User has approved your plan…") + // that overwrites `ToolCall.result` once the session jsonl is parsed fresh + // from disk, so the in-memory result alone is not reliable. let key = window.currentSessionId ?? window.newSessionKey updateState(key) { state in for i in state.messages.indices.reversed() { @@ -2693,7 +2717,9 @@ final class AppState { break } } + state.planDecisionSummaries[toolUseId] = summary } + threadStore.setPlanDecision(sessionId: key, toolCallId: toolUseId, summary: summary) // Plan-mode is one-shot — clear the pill so the next user turn isn't in plan mode. // This also triggers a permission re-register (no-op if there's no live CLI sid). @@ -2715,16 +2741,21 @@ final class AppState { await permission.respond(toolUseId: toolUseId, decision: decision) - // In `--print` stream-json mode, ExitPlanMode is the model's last action of the - // plan-mode turn, so the CLI emits `.result` and exits. Without a follow-up - // prompt the chat just stops. Mirror the interactive CLI by sending a hidden - // continuation message that nudges the model to execute (or revise) the plan. + // When the CLI honors `allowAndSetMode` it continues the same turn — the model + // executes (or revises) the plan inline and the turn ends naturally. In that + // case sending a follow-up prompt would spawn a redundant second turn that + // reports "the work is already done". Only inject a continuation message when + // the turn actually ended without producing any post-plan content, mirroring + // the older CLI behavior where ExitPlanMode terminated the turn outright. let continuationPrompt = Self.continuationPrompt(for: action) if let continuationPrompt { if let task = sessionStates[key]?.streamTask { _ = await task.value } + if turnContinuedAfterPlan(toolUseId: toolUseId, sessionKey: key) { + return + } await sendPrompt( continuationPrompt, skipAppendingUserMessage: true, @@ -2733,15 +2764,29 @@ final class AppState { } } - /// Prefixes of result strings written by `respondToPlanDecision`. Must stay in - /// sync with `PlanCardView.userDecisionPrefixes` — duplicated here so that - /// `AppState` can detect a decided plan without depending on a SwiftUI view type. - static let planDecisionResultPrefixes: [String] = [ - "Accepted with Ask", - "Accepted with Edits", - "Accepted with Auto-approve", - "Rejected", - ] + /// True when the assistant produced any content (text or tool calls) after the + /// given ExitPlanMode tool call in the current session — either as later blocks + /// in the same message or as subsequent messages. Used to suppress the hidden + /// "Proceed with the plan." continuation prompt when the CLI has already carried + /// the turn through to completion on its own. + private func turnContinuedAfterPlan(toolUseId: String, sessionKey: String) -> Bool { + guard let messages = sessionStates[sessionKey]?.messages else { return false } + for messageIdx in messages.indices.reversed() { + guard let blockIdx = messages[messageIdx].toolCallIndex(id: toolUseId) else { + continue + } + if blockIdx < messages[messageIdx].blocks.count - 1 { return true } + if messageIdx < messages.count - 1 { return true } + return false + } + return false + } + + /// Prefixes of result strings written by `respondToPlanDecision`. Sourced from + /// `PlanDecisionAction.userDecisionResultPrefixes` so the chip in chat, the + /// CLI-session reload guard, and the live-stream guard all share one source + /// of truth. + static let planDecisionResultPrefixes: [String] = PlanDecisionAction.userDecisionResultPrefixes static func isExitPlanModeCall(_ call: ToolCall) -> Bool { let n = call.name.lowercased() @@ -2889,6 +2934,7 @@ final class AppState { state.permissionMode = session.permissionMode if let msgs = loadedMessages { state.messages = cleanLoadedMessages(msgs) + state.planDecisionSummaries = threadStore.loadPlanDecisions(sessionId: session.id) sessionStates[session.id] = state logger.info("[SwitchToSession] applied preloaded messages sid=\(session.id, privacy: .public) cleaned=\(state.messages.count)") } else { @@ -3669,10 +3715,11 @@ final class AppState { return } state.messages = cleaned + state.planDecisionSummaries = self.threadStore.loadPlanDecisions(sessionId: sessionId) if state.model == nil { state.model = full.model } if state.effort == nil { state.effort = full.effort } if state.permissionMode == nil { state.permissionMode = full.permissionMode } - self.logger.info("[LoadMessages] applied sid=\(sessionId, privacy: .public) messages=\(state.messages.count)") + self.logger.info("[LoadMessages] applied sid=\(sessionId, privacy: .public) messages=\(state.messages.count) planDecisions=\(state.planDecisionSummaries.count)") } } } @@ -3809,6 +3856,10 @@ final class AppState { // Take a snapshot of remaining queue items so we can restore them after // `cancelStreaming` clobbers `window.inputText`/`window.attachments`. let remaining = window.messageQueue.filter { $0.id != id } + let draftText = window.inputText + let draftAttachments = window.attachments + let shouldRestoreDraft = !draftText.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty + || !draftAttachments.isEmpty // Clear the queue from memory + disk first, so the cancellation path // doesn't see the queued item we're about to send. @@ -3835,6 +3886,10 @@ final class AppState { window.inputText = target.text window.attachments = target.attachments await send(in: window) + if shouldRestoreDraft { + window.inputText = draftText + window.attachments = draftAttachments + } } /// Concatenates every queued message (texts joined with a blank line, @@ -3843,6 +3898,10 @@ final class AppState { func sendAllQueuedAsOne(in window: WindowState) async { guard !window.messageQueue.isEmpty else { return } let snapshot = window.messageQueue + let draftText = window.inputText + let draftAttachments = window.attachments + let shouldRestoreDraft = !draftText.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty + || !draftAttachments.isEmpty let key = queueKey(for: window) window.messageQueue.removeAll() @@ -3862,6 +3921,10 @@ final class AppState { window.inputText = combinedText window.attachments = combinedAttachments await send(in: window) + if shouldRestoreDraft { + window.inputText = draftText + window.attachments = draftAttachments + } } /// Sends the next queued message for a background session (one the window is not currently displaying). @@ -3886,6 +3949,7 @@ final class AppState { updateState(sessionKey) { state in state.messages.append(ChatMessage(role: .user, content: displayText, attachments: resolvedAttachments)) + state.inFlightUserAttachments = resolvedAttachments state.isStreaming = true state.hasUncheckedCompletion = false state.activeStreamId = streamId diff --git a/RxCode/Services/ThreadStore.swift b/RxCode/Services/ThreadStore.swift index 50ff2ce7..aa7bf7b6 100644 --- a/RxCode/Services/ThreadStore.swift +++ b/RxCode/Services/ThreadStore.swift @@ -17,7 +17,7 @@ final class ThreadStore { /// Convenience initializer creating its own `ModelContainer` rooted at the /// app's Application Support directory. static func make() -> ThreadStore { - let schema = Schema([ChatThread.self, TodoSnapshot.self, ThreadFileEdit.self, QueuedMessageRecord.self]) + let schema = Schema([ChatThread.self, TodoSnapshot.self, ThreadFileEdit.self, QueuedMessageRecord.self, PlanDecisionRecord.self]) let url = Self.storeURL() let config = ModelConfiguration(schema: schema, url: url) do { @@ -107,6 +107,7 @@ final class ThreadStore { renameTodoSnapshot(from: oldId, to: newId) renameFileEdits(from: oldId, to: newId) renameQueueKey(from: oldId, to: newId) + renamePlanDecisions(from: oldId, to: newId) save() } @@ -145,6 +146,7 @@ final class ThreadStore { deleteTodoSnapshotRow(sessionId: id) deleteFileEditRows(sessionId: id) deleteQueueRows(sessionKey: id) + deletePlanDecisionRows(sessionId: id) save() } @@ -163,6 +165,7 @@ final class ThreadStore { deleteTodoSnapshotRow(sessionId: id) deleteFileEditRows(sessionId: id) deleteQueueRows(sessionKey: id) + deletePlanDecisionRows(sessionId: id) } if projectId == nil { let allTodos = (try? context.fetch(FetchDescriptor())) ?? [] @@ -171,6 +174,8 @@ final class ThreadStore { for row in allEdits { context.delete(row) } let allQueues = (try? context.fetch(FetchDescriptor())) ?? [] for row in allQueues { context.delete(row) } + let allPlans = (try? context.fetch(FetchDescriptor())) ?? [] + for row in allPlans { context.delete(row) } } save() } catch { @@ -218,6 +223,62 @@ final class ThreadStore { context.delete(row) } + // MARK: - Plan Decisions + + private func fetchPlanDecision(sessionId: String, toolCallId: String) -> PlanDecisionRecord? { + var descriptor = FetchDescriptor( + predicate: #Predicate { $0.sessionId == sessionId && $0.toolCallId == toolCallId } + ) + descriptor.fetchLimit = 1 + return (try? context.fetch(descriptor))?.first + } + + func loadPlanDecisions(sessionId: String) -> [String: String] { + let descriptor = FetchDescriptor( + predicate: #Predicate { $0.sessionId == sessionId } + ) + let rows = (try? context.fetch(descriptor)) ?? [] + var map: [String: String] = [:] + for row in rows { map[row.toolCallId] = row.summary } + return map + } + + func setPlanDecision(sessionId: String, toolCallId: String, summary: String) { + if let existing = fetchPlanDecision(sessionId: sessionId, toolCallId: toolCallId) { + existing.summary = summary + existing.updatedAt = .now + } else { + context.insert(PlanDecisionRecord( + sessionId: sessionId, + toolCallId: toolCallId, + summary: summary + )) + } + save() + } + + private func planDecisions(sessionId: String) -> [PlanDecisionRecord] { + let descriptor = FetchDescriptor( + predicate: #Predicate { $0.sessionId == sessionId } + ) + return (try? context.fetch(descriptor)) ?? [] + } + + func renamePlanDecisions(from oldId: String, to newId: String) { + guard oldId != newId else { return } + for row in planDecisions(sessionId: oldId) { + if fetchPlanDecision(sessionId: newId, toolCallId: row.toolCallId) != nil { + context.delete(row) + } else { + row.sessionId = newId + } + } + } + + private func deletePlanDecisionRows(sessionId: String) { + for row in planDecisions(sessionId: sessionId) { context.delete(row) } + } + // MARK: - Thread File Edits func fetchFileEdits(sessionId: String) -> [ThreadFileEdit] { diff --git a/RxCode/Views/MainView.swift b/RxCode/Views/MainView.swift index 589c17d8..81480e42 100644 --- a/RxCode/Views/MainView.swift +++ b/RxCode/Views/MainView.swift @@ -636,6 +636,7 @@ struct ComposerControlLabel: View { struct ChatDetailModifiers: ViewModifier { @Environment(AppState.self) private var appState @Environment(WindowState.self) private var windowState + @Environment(ChatBridge.self) private var chatBridge private var presentedRequest: PermissionRequest? { guard let id = windowState.presentedPermissionId else { return nil } @@ -660,6 +661,22 @@ struct ChatDetailModifiers: ViewModifier { ) } + private var presentedPlan: PendingPlan? { + guard let id = windowState.presentedPlanToolCallId else { return nil } + return chatBridge.pendingPlans.first { $0.toolCallId == id } + } + + private var planSheetBinding: Binding { + Binding( + get: { presentedPlan != nil }, + set: { isPresented in + if !isPresented { + windowState.presentedPlanToolCallId = nil + } + } + ) + } + func body(content: Content) -> some View { content .animation(.spring(response: 0.3, dampingFraction: 0.85), value: windowState.pendingPermissions.count) @@ -702,6 +719,33 @@ struct ChatDetailModifiers: ViewModifier { windowState.presentedPermissionId = nil } } + .sheet(isPresented: planSheetBinding) { + if let plan = presentedPlan { + PlanSheetView( + plan: plan, + remainingCount: max(0, chatBridge.pendingPlans.filter { !$0.isDecided }.count - 1), + onSubmit: { toolCallId, action in + windowState.planDecisionHandler?(toolCallId, action) + // Decision recorded — close the sheet. The chip in chat + // will reflect the new status once the result lands. + windowState.presentedPlanToolCallId = nil + }, + onClose: { + // Just dismiss — the plan stays in the queue so the user + // can re-open it from the banner or inline chip later. + windowState.presentedPlanToolCallId = nil + } + ) + .environment(appState) + .environment(windowState) + .environment(chatBridge) + } + } + .onChange(of: chatBridge.pendingPlans.map(\.toolCallId)) { _, newIds in + if let id = windowState.presentedPlanToolCallId, !newIds.contains(id) { + windowState.presentedPlanToolCallId = nil + } + } .sheet(isPresented: Bindable(windowState).showModelPicker) { ModelPickerSheet() .environment(appState) diff --git a/RxCode/Views/Permission/PermissionQueueBanner.swift b/RxCode/Views/Permission/PermissionQueueBanner.swift index 94dad0da..4bed3d45 100644 --- a/RxCode/Views/Permission/PermissionQueueBanner.swift +++ b/RxCode/Views/Permission/PermissionQueueBanner.swift @@ -1,12 +1,15 @@ import SwiftUI import RxCodeCore +import RxCodeChatKit /// Compact pill displayed directly above the chat input bar whenever the CLI -/// has pending permission requests. Tapping it surfaces the full -/// `PermissionModal` for the first queued request — streaming/typing is never +/// has pending permission requests, pending `AskUserQuestion` calls, or +/// undecided `ExitPlanMode` plans. Tapping it surfaces the corresponding +/// modal/sheet for the first queued item — streaming/typing is never /// interrupted until the user explicitly opens it. struct PermissionQueueBanner: View { @Environment(WindowState.self) private var windowState + @Environment(ChatBridge.self) private var chatBridge @State private var isHovered: Bool = false private var pendingRequests: [PermissionRequest] { @@ -21,20 +24,44 @@ struct PermissionQueueBanner: View { pendingRequests.count - questionCount } + /// Plans awaiting a user decision. Decided plans are excluded (they only show + /// as the inline status chip in chat) and so are still-streaming plans whose + /// markdown hasn't arrived yet — surfacing those would let the user "Review" + /// an empty plan. + private var pendingPlans: [PendingPlan] { + chatBridge.pendingPlans.filter { !$0.isDecided && !$0.isStreaming } + } + + private var planCount: Int { pendingPlans.count } + + private var totalCount: Int { + pendingRequests.count + planCount + } + + private var isEmpty: Bool { totalCount == 0 } + private var bannerIcon: String { - permissionCount == 0 ? "questionmark.circle.fill" : "exclamationmark.shield.fill" + if planCount > 0 && permissionCount == 0 && questionCount == 0 { + return "doc.text.fill" + } + if permissionCount > 0 { + return "exclamationmark.shield.fill" + } + return "questionmark.circle.fill" } private var bannerText: String { - if questionCount > 0, permissionCount == 0 { + // Single-category messaging when only one kind is pending. + if planCount > 0, permissionCount == 0, questionCount == 0 { + return countText(planCount, singular: "plan pending", plural: "plans pending") + } + if questionCount > 0, permissionCount == 0, planCount == 0 { return countText(questionCount, singular: "question pending", plural: "questions pending") } - - if permissionCount > 0, questionCount == 0 { + if permissionCount > 0, questionCount == 0, planCount == 0 { return countText(permissionCount, singular: "permission request pending", plural: "permission requests pending") } - - return countText(pendingRequests.count, singular: "request pending", plural: "requests pending") + return countText(totalCount, singular: "request pending", plural: "requests pending") } private var actionText: String { @@ -42,7 +69,7 @@ struct PermissionQueueBanner: View { } var body: some View { - if !pendingRequests.isEmpty { + if !isEmpty { content .transition(.move(edge: .bottom).combined(with: .opacity)) } @@ -89,6 +116,13 @@ struct PermissionQueueBanner: View { } private func open() { + // Prefer surfacing pending plans first — they typically block forward + // progress on the conversation while questions/permissions are usually + // about a single in-flight tool call. + if let plan = pendingPlans.first { + windowState.presentedPlanToolCallId = plan.toolCallId + return + } windowState.presentedPermissionId = pendingRequests.first?.id } diff --git a/RxCodeTests/PlanCardViewTests.swift b/RxCodeTests/PlanCardViewTests.swift index 72c9bed7..7a6b49d6 100644 --- a/RxCodeTests/PlanCardViewTests.swift +++ b/RxCodeTests/PlanCardViewTests.swift @@ -4,99 +4,244 @@ import ViewInspector import RxCodeCore @testable import RxCodeChatKit +// Bridge the inspection helper inside `PlanSheetView` to ViewInspector's +// emissary protocol so tests can wait on @State transitions (e.g. composer reveal). +extension PlanSheetInspection: InspectionEmissary {} + @MainActor final class PlanCardViewTests: XCTestCase { - // MARK: - Pending plan: decision buttons render + // MARK: - PlanCardView chip: tapping "Review" calls onOpen with tool call id + + func testChip_tappingReview_callsOnOpenWithToolCallId() throws { + var openedId: String? + let chip = makeChip(result: nil, onOpen: { openedId = $0 }) + + try chip.inspect().find(button: "Review").tap() - func testDecisionButtons_visibleWhenNotDecided() throws { - let card = makeCard(result: nil) - let host = hosting(card) + XCTAssertEqual( + openedId, + "tool-1", + "Tapping the chip's Review button must invoke onOpen with the chip's tool call id" + ) + } - let labels = try host.inspect() + // MARK: - PlanCardView chip: decided plan shows summary + 'View' label + + func testChip_decidedPlan_showsSummaryAndViewLabel() throws { + var openedId: String? + let chip = makeChip(result: "Accepted with Edits", onOpen: { openedId = $0 }) + + let labels = try chip.inspect() .findAll(ViewType.Text.self) .compactMap { try? $0.string() } - for expected in [ - "Accept + Auto", - "Accept Edits", - "Accept Ask", - "Reject with Reason", - "Reject", - ] { - XCTAssertTrue( - labels.contains(expected), - "Expected decision button \"\(expected)\" to be present. Found: \(labels)" - ) - } + XCTAssertTrue( + labels.contains("Accepted with Edits"), + "Decided chip should display the decision summary verbatim. Labels: \(labels)" + ) + XCTAssertTrue( + labels.contains("View"), + "Decided chip should swap the action label from 'Review' to 'View'. Labels: \(labels)" + ) - // The decided-row status text should NOT appear. - XCTAssertFalse( - labels.contains(where: { $0.hasPrefix("Accepted ") || $0.hasPrefix("Rejected") }), - "Decided status text leaked into pending state. Labels: \(labels)" + // Decided chip's button should still be tappable and re-open the sheet. + try chip.inspect().find(button: "View").tap() + XCTAssertEqual( + openedId, + "tool-1", + "Decided chip should still open the read-only sheet on tap" ) } - // MARK: - Accepted plan: buttons gone, status row shown + // MARK: - PlanCardView chip: CLI noise must not flip to "decided" state - func testAcceptedPlan_hidesButtons_showsStatus() throws { - let card = makeCard(result: "Accepted with Edits") - let host = hosting(card) + func testChip_cliNoiseInResult_stillShowsReview() throws { + // The CLI itself can write non-user-decision text to `result` before the + // user has clicked anything — that must NOT make the chip render as decided. + let chip = makeChip(result: "Exit plan mode?", onOpen: { _ in }) - let labels = try host.inspect() + let labels = try chip.inspect() .findAll(ViewType.Text.self) .compactMap { try? $0.string() } - XCTAssertFalse(labels.contains("Accept + Auto"), - "Decision buttons must disappear once a plan is decided. Labels: \(labels)") - XCTAssertFalse(labels.contains("Accept Edits"), - "Decision buttons must disappear once a plan is decided. Labels: \(labels)") - XCTAssertFalse(labels.contains("Reject"), - "Decision buttons must disappear once a plan is decided. Labels: \(labels)") - XCTAssertTrue(labels.contains("Accepted with Edits"), - "Decided status row must contain the action summary. Labels: \(labels)") + XCTAssertTrue( + labels.contains("Plan ready"), + "Chip should show 'Plan ready' when result is unrelated CLI text. Labels: \(labels)" + ) + XCTAssertTrue( + labels.contains("Review"), + "Chip should show the 'Review' action label when not decided. Labels: \(labels)" + ) + } + + // MARK: - PlanSheetView: tapping "Accept Edits" calls onSubmit with the right action + + func testSheet_tappingAcceptEdits_callsOnSubmitWithAcceptWithEdits() throws { + var captured: (String, PlanDecisionAction)? + let sheet = makeSheet( + decided: false, + onSubmit: { id, action in captured = (id, action) } + ) + ViewHosting.host(view: sheet) + defer { ViewHosting.expel() } + + try sheet.inspect().find(button: "Accept Edits").tap() + + XCTAssertNotNil(captured, "onSubmit must fire when 'Accept Edits' is tapped") + XCTAssertEqual(captured?.0, "tool-1", "onSubmit should pass through the plan's tool call id") + XCTAssertEqual( + captured?.1, + .acceptWithEdits, + "Tapping 'Accept Edits' must dispatch .acceptWithEdits" + ) + } + + // MARK: - PlanSheetView: tapping "Accept + Auto" dispatches .acceptAutoApprove + + func testSheet_tappingAcceptPlusAuto_callsOnSubmitWithAcceptAutoApprove() throws { + var captured: (String, PlanDecisionAction)? + let sheet = makeSheet( + decided: false, + onSubmit: { id, action in captured = (id, action) } + ) + ViewHosting.host(view: sheet) + defer { ViewHosting.expel() } + + try sheet.inspect().find(button: "Accept + Auto").tap() + + XCTAssertEqual(captured?.1, .acceptAutoApprove) + } + + // MARK: - PlanSheetView: plain Reject dispatches .reject + + func testSheet_tappingReject_callsOnSubmitWithReject() throws { + var captured: (String, PlanDecisionAction)? + let sheet = makeSheet( + decided: false, + onSubmit: { id, action in captured = (id, action) } + ) + ViewHosting.host(view: sheet) + defer { ViewHosting.expel() } + + try sheet.inspect().find(button: "Reject").tap() + + XCTAssertEqual(captured?.1, .reject) } - // MARK: - Rejected with feedback: status row shows feedback + // MARK: - PlanSheetView: Reject with Reason → composer → submit feedback - func testRejectedWithFeedback_showsFeedbackInStatus() throws { - let summary = "Rejected: please use SQLite" - let card = makeCard(result: summary) - let host = hosting(card) + // The full flow (tap "Reject with Reason" → @State swap → type → send) is + // split across two tests because driving the `decisionButtons → feedbackComposer` + // view swap on a hosted view double-frees memory when another `PlanSheetView` + // was hosted+expelled earlier in the same xctest run. The two halves below + // exercise the same behavior without forcing the swap inside the hosted tree. + + func testSheet_tappingRejectWithReason_doesNotSubmit() throws { + var submitCalled = false + let sheet = makeSheet( + decided: false, + onSubmit: { _, _ in submitCalled = true } + ) + ViewHosting.host(view: sheet) + defer { ViewHosting.expel() } - let labels = try host.inspect() + try sheet.inspect().find(button: "Reject with Reason").tap() + + XCTAssertFalse( + submitCalled, + "Tapping 'Reject with Reason' must reveal the composer, NOT resolve the CLI hook" + ) + } + + func testSheet_sendFeedbackFromComposer_dispatchesRejectWithFeedback() throws { + var captured: (String, PlanDecisionAction)? + // Pre-populate the feedback so the "Send feedback" button is enabled. + // The text-entry path is covered separately via SwiftUI's own Binding — + // here we verify the submit dispatch passes the typed reason through. + let sheet = makeSheetWithComposer( + initialFeedback: "use SQLite instead", + onSubmit: { id, action in captured = (id, action) } + ) + ViewHosting.host(view: sheet) + defer { ViewHosting.expel() } + + let labels = try sheet.inspect() .findAll(ViewType.Text.self) .compactMap { try? $0.string() } - XCTAssertTrue( - labels.contains(where: { $0.contains("please use SQLite") }), - "Decided status row should embed the user's rejection feedback. Labels: \(labels)" + labels.contains("Tell Claude what to change"), + "Composer header must be visible when initialIsComposingFeedback is true. Labels: \(labels)" + ) + + try sheet.inspect().find(button: "Send feedback").tap() + + XCTAssertEqual(captured?.0, "tool-1") + XCTAssertEqual( + captured?.1, + .rejectWithFeedback(reason: "use SQLite instead"), + "Submitting the feedback composer must dispatch .rejectWithFeedback with the typed reason" ) } - // MARK: - CLI noise must not be treated as decided + // MARK: - PlanSheetView: closing the sheet calls onClose, NOT onSubmit - func testCliNoiseInResult_keepsButtonsVisible() throws { - // The CLI itself can write non-user-decision text to `result` before the - // user has clicked anything — that must NOT hide the accept/reject buttons. - let card = makeCard(result: "Exit plan mode?") - let host = hosting(card) + func testSheet_close_callsOnCloseAndNotOnSubmit() throws { + var submitCalled = false + var closeCalled = false + let sheet = makeSheet( + decided: false, + onSubmit: { _, _ in submitCalled = true }, + onClose: { closeCalled = true } + ) + ViewHosting.host(view: sheet) + defer { ViewHosting.expel() } + + // The X button has no text label — find it by help string. + let closeButton = try sheet.inspect().find(ViewType.Button.self) { btn in + (try? btn.help().string()) == "Close — you can review later from the banner" + } + try closeButton.tap() + + XCTAssertTrue(closeCalled, "Tapping X must call onClose") + XCTAssertFalse(submitCalled, "Closing the sheet must NOT count as decline — onSubmit must remain unfired") + } - let labels = try host.inspect() + // MARK: - PlanSheetView: decided plan shows summary + hides decision buttons + + func testSheet_decidedPlan_showsSummaryAndHidesButtons() throws { + let sheet = makeSheet(decided: true, summary: "Accepted with Edits") + ViewHosting.host(view: sheet) + defer { ViewHosting.expel() } + + let labels = try sheet.inspect() .findAll(ViewType.Text.self) .compactMap { try? $0.string() } XCTAssertTrue( - labels.contains("Accept + Auto"), - "Buttons should still render when result is unrelated CLI text. Labels: \(labels)" + labels.contains("Accepted with Edits"), + "Decided footer must contain the action summary. Labels: \(labels)" + ) + + // Buttons must be gone. + XCTAssertThrowsError( + try sheet.inspect().find(button: "Accept + Auto"), + "Decided sheet must hide the Accept + Auto button" + ) + XCTAssertThrowsError( + try sheet.inspect().find(button: "Reject"), + "Decided sheet must hide the Reject button" ) } // MARK: - Helpers - private func makeCard( + private let planMd = "# My plan\n- step 1\n- step 2" + + private func makeChip( result: String?, - planMarkdown: String = "# My plan\n- step 1\n- step 2" + planMarkdown: String = "# My plan\n- step 1\n- step 2", + onOpen: ((String) -> Void)? = nil ) -> PlanCardView { let toolCall = ToolCall( id: "tool-1", @@ -108,11 +253,51 @@ final class PlanCardViewTests: XCTestCase { return PlanCardView( toolCall: toolCall, planMarkdown: planMarkdown, - isMessageStreaming: false + isMessageStreaming: false, + onOpen: onOpen + ) + } + + private func makeSheet( + decided: Bool, + summary: String? = nil, + onSubmit: @escaping (String, PlanDecisionAction) -> Void = { _, _ in }, + onClose: @escaping () -> Void = { } + ) -> PlanSheetView { + let plan = PendingPlan( + toolCallId: "tool-1", + markdown: planMd, + isStreaming: false, + isDecided: decided, + decisionSummary: decided ? (summary ?? "Accepted with Edits") : nil + ) + return PlanSheetView( + plan: plan, + remainingCount: 0, + onSubmit: onSubmit, + onClose: onClose ) } - private func hosting(_ view: V) -> some View { - view.environment(WindowState()) + private func makeSheetWithComposer( + initialFeedback: String, + onSubmit: @escaping (String, PlanDecisionAction) -> Void = { _, _ in }, + onClose: @escaping () -> Void = { } + ) -> PlanSheetView { + let plan = PendingPlan( + toolCallId: "tool-1", + markdown: planMd, + isStreaming: false, + isDecided: false, + decisionSummary: nil + ) + return PlanSheetView( + plan: plan, + remainingCount: 0, + initialFeedbackText: initialFeedback, + initialIsComposingFeedback: true, + onSubmit: onSubmit, + onClose: onClose + ) } }