Repository navigation
Keep mobile chat GUI cached during reconnect - #7064
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds chat-session snapshot caching, refresh-outcome handling, descriptor reconciliation, and extracted iOS workspace chat rendering and lifecycle logic. ChangesiOS Agent Chat session management & detail view refactor
Sequence Diagram(s)sequenceDiagram
participant WorkspaceDetailView
participant ChatEventSource
participant MobileShellComposite
participant ChatConversationStore
WorkspaceDetailView->>ChatEventSource: refreshChatSessions()
ChatEventSource-->>WorkspaceDetailView: authoritative sessions or unavailable
WorkspaceDetailView->>MobileShellComposite: rememberChatSessions(workspaceID:)
ChatEventSource-->>WorkspaceDetailView: streamed session frames
WorkspaceDetailView->>WorkspaceDetailView: reconcileChatSessionSnapshot(...)
WorkspaceDetailView->>ChatConversationStore: applyDescriptorSnapshot(_:)
WorkspaceDetailView->>ChatConversationStore: replaceSource(_, descriptor:)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (5 errors, 2 warnings)
✅ Passed checks (18 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
2df39e9 to
39e667a
Compare
39e667a to
b6c9cfb
Compare
b6c9cfb to
6a9a45b
Compare
This reverts commit c903d7f.
# Conflicts: # .github/swift-file-length-budget.tsv
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView`+AgentChat.swift:
- Around line 76-85: The warm-task identity in chatConversationWarmKey is too
granular because it includes session.version, which causes
ChatConversationStore.run() to restart on descriptor-only refreshes. Update the
key in WorkspaceDetailView+AgentChat to identify the task by workspace, session
id, and connectivity/foreground state only, so the long-lived conversation
runner stays subscribed while reconcileChatSessionSnapshot() and
ensureChatConversationStore(for:) handle metadata reconciliation.
- Around line 17-22: The visibleChatSessions logic currently prefers any
non-empty local chatSessions snapshot without verifying it belongs to the active
workspace, which can let stale sessions from another workspace drive
chatToggleSession and chosenChatSession. Update WorkspaceDetailView+AgentChat so
the local snapshot is scoped by workspace id (or ignored when it does not match
workspace.id.rawValue), and make the cache fallback in visibleChatSessions and
the related chat session selection flow use a single workspace-consistent source
of truth. Apply the same workspace identity check to the chat session state
handling referenced by the other affected section so stale local/session state
cannot substitute for the current workspace.
In
`@Packages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatConversationStore.swift`:
- Around line 388-393: Prevent equal-version descriptor snapshots from
overwriting newer live state in applyDescriptorSnapshot(_:). The current version
guard allows descriptor.version == self.descriptor.version, which can replace
agentState with stale cached state. Tighten the check in ChatConversationStore
so only strictly newer versions update self.descriptor and agentState, or
explicitly preserve the existing agentState when versions are equal. Keep the
fix localized to applyDescriptorSnapshot(_:) and align it with the versioning
behavior used by apply(_:) and .stateChanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 435f87e9-1bda-4c02-95ed-5472db7ca732
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (10)
Packages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatConversationStore.swiftPackages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Screen/ChatScreen.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+AgentChat.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftPackages/iOS/CmuxMobileShellUI/Package.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceChatPane.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceChatSessionRefreshOutcome.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView+AgentChat.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swiftPackages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/WorkspaceChatSessionRefreshOutcomeTests.swift
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift (1)
3627-3630: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winOnly clear chat snapshots for workspaces that truly disappeared.
Lines 3627-3630 purge every snapshot that belonged to
previousKey, but this helper also runs during promotion / anonymous→real reconnect where the same workspace can immediately survive under a differentworkspacesByMackey. In that case we delete the cache for a still-live workspace and the mobile GUI toggle/history disappears until a later refresh rebuilds it.Proposed fix
let removedWorkspaceIDs = Set(workspacesByMac[previousKey]?.workspaces.map { $0.id.rawValue } ?? []) workspacesByMac[previousKey] = nil - for workspaceID in removedWorkspaceIDs { + let survivingWorkspaceIDs = Set( + workspacesByMac.values.flatMap { state in + state.workspaces.map { $0.id.rawValue } + } + ) + for workspaceID in removedWorkspaceIDs.subtracting(survivingWorkspaceIDs) { chatSessionSnapshotsByWorkspaceID[workspaceID] = nil }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift` around lines 3627 - 3630, The snapshot purge in MobileShellComposite’s workspace migration flow is too aggressive because it deletes all entries from chatSessionSnapshotsByWorkspaceID for every workspace ID under previousKey, even when a workspace survives the reconnect under a new workspacesByMac key. Update the cleanup logic around the removedWorkspaceIDs handling so it only clears snapshots for workspace IDs that are no longer present anywhere in workspacesByMac, and keep the existing workspacesByMac/previousKey transition logic intact. Use the surrounding workspace promotion / anonymous-to-real reconnect path in MobileShellComposite to locate the fix.Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView+AgentChat.swift (1)
123-127: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRetry chat-store creation when the source reconnects.
If
ensureChatConversationStore(for:)returnsnilwhile the Mac is unavailable, this branch stays keyed only bysession.id; reconnect/source-identity changes will not rerun the task, leaving cached chat mode as a blankColor.clear.Suggested fix
} else { Color.clear - .task(id: session.id) { + .task(id: chatConversationWarmKey) { _ = ensureChatConversationStore(for: session) } + .task(id: chatRefreshKey) { await refreshChatSessions() } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView`+AgentChat.swift around lines 123 - 127, The fallback branch in WorkspaceDetailView+AgentChat is only keyed by session.id, so the .task that calls ensureChatConversationStore(for:) won’t rerun when the source reconnects and the Mac becomes available again. Update the task trigger in this Color.clear branch to also depend on a reconnect/source-identity signal from the relevant chat/session state, so a nil store result can be retried instead of leaving the view stuck in blank mode.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Packages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatConversationStore.swift`:
- Around line 397-400: `ChatConversationStore.replaceSource(_:descriptor:)` only
swaps the `source` and reapplies the snapshot, but it leaves an already parked
`run()` loop sleeping in the old reconnect backoff. Update `replaceSource` to
immediately wake the store by cancelling/restarting the current run or otherwise
breaking the pending backoff so it resubscribes to the new source right away.
Keep the fix in `ChatConversationStore` and make sure
`ensureChatConversationStore(...)` rebinding cannot leave the store serving
stale cached state.
---
Outside diff comments:
In
`@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift`:
- Around line 3627-3630: The snapshot purge in MobileShellComposite’s workspace
migration flow is too aggressive because it deletes all entries from
chatSessionSnapshotsByWorkspaceID for every workspace ID under previousKey, even
when a workspace survives the reconnect under a new workspacesByMac key. Update
the cleanup logic around the removedWorkspaceIDs handling so it only clears
snapshots for workspace IDs that are no longer present anywhere in
workspacesByMac, and keep the existing workspacesByMac/previousKey transition
logic intact. Use the surrounding workspace promotion / anonymous-to-real
reconnect path in MobileShellComposite to locate the fix.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView`+AgentChat.swift:
- Around line 123-127: The fallback branch in WorkspaceDetailView+AgentChat is
only keyed by session.id, so the .task that calls
ensureChatConversationStore(for:) won’t rerun when the source reconnects and the
Mac becomes available again. Update the task trigger in this Color.clear branch
to also depend on a reconnect/source-identity signal from the relevant
chat/session state, so a nil store result can be retried instead of leaving the
view stuck in blank mode.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 426b27ad-0c10-43b6-96df-81fe2dac6421
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (4)
Packages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatConversationStore.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView+AgentChat.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swift
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView+AgentChat.swift (1)
171-194: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReturn on cancelled refreshes before mutating chat-session state.
A previous
.task(id: chatRefreshKey)can still resume aftersessionEvents()orsessions(workspaceID:)when the connection/foreground epoch changes. The currentcatchalso treats cancellation as.unavailable, then writeschatSessions,chatSessionsWorkspaceID, andstore.rememberChatSessions(...), so an older refresh can overwrite a newer authoritative snapshot for the same workspace.Suggested fix
let reducer = ChatSessionListReducer(workspaceID: workspaceID) let stream = await source.sessionEvents() + guard !Task.isCancelled else { return } let seedOutcome: WorkspaceChatSessionRefreshOutcome do { - seedOutcome = .authoritative(try await source.sessions(workspaceID: workspaceID)) + let seededSessions = try await source.sessions(workspaceID: workspaceID) + guard !Task.isCancelled else { return } + seedOutcome = .authoritative(seededSessions) + } catch is CancellationError { + return } catch { seedOutcome = .unavailable } + guard !Task.isCancelled else { return } let nextSessions = seedOutcome.applying(to: visibleChatSessions) withAnimation(.snappy(duration: 0.25)) { chatSessionsWorkspaceID = workspaceID chatSessions = nextSessions @@ reconcileChatSessionSnapshot(seedOutcomeCanInvalidateSelection: seedOutcome.canInvalidateSelection) for await frame in stream { + guard !Task.isCancelled else { return } let next = reducer.applying(frame, to: visibleChatSessions) withAnimation(.snappy(duration: 0.25)) { chatSessionsWorkspaceID = workspaceIDAs per path instructions, correctness-critical cached GUI state must not become visibly stale or use competing cached/live truths.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView`+AgentChat.swift around lines 171 - 194, The refresh flow in WorkspaceDetailView+AgentChat should stop immediately when the async task is cancelled so an older .task(id: chatRefreshKey) cannot overwrite newer state. In the session-refresh logic that awaits source.sessionEvents() and source.sessions(workspaceID:), handle CancellationError separately and return before setting chatSessionsWorkspaceID, chatSessions, or calling store.rememberChatSessions(...). Keep the existing authoritative/unavailable handling for non-cancellation failures, and ensure reconcileChatSessionSnapshot only runs for non-cancelled refreshes.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Packages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatConversationStore.swift`:
- Around line 390-394: The reconciliation in ChatConversationStore’s descriptor
update path is letting unversioned snapshots overwrite newer live agent state.
Update the logic around the descriptor.version check and state assignment so
version == 0 snapshots do not replace a currently active agentState when recency
cannot be proven; either preserve the existing state in this case or ensure the
snapshot gets a real monotonic version before reaching this branch. Keep the fix
localized to the descriptor/state reconciliation code that sets self.descriptor,
agentState, and didFlushThisIdleWindow.
---
Outside diff comments:
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView`+AgentChat.swift:
- Around line 171-194: The refresh flow in WorkspaceDetailView+AgentChat should
stop immediately when the async task is cancelled so an older .task(id:
chatRefreshKey) cannot overwrite newer state. In the session-refresh logic that
awaits source.sessionEvents() and source.sessions(workspaceID:), handle
CancellationError separately and return before setting chatSessionsWorkspaceID,
chatSessions, or calling store.rememberChatSessions(...). Keep the existing
authoritative/unavailable handling for non-cancellation failures, and ensure
reconcileChatSessionSnapshot only runs for non-cancelled refreshes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: dfbaca3a-fece-43eb-aa59-ad232668111a
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (2)
Packages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatConversationStore.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView+AgentChat.swift
Greptile SummaryThis PR preserves the mobile chat GUI during Mac reconnects by moving chat-session snapshots into the shell store, warming conversation stores while in terminal mode, and replacing a per-view conversation construction with a parent-managed store that is passed into
Confidence Score: 4/5Safe to merge with attention to the unsupported-Mac detection path. The generation-based staleness guards in ChatConversationStore, the backoff-wake mechanism, and the unavailable/authoritative outcome enum are all correctly implemented. The one concrete defect is in chatSessionListFailureMeansUnsupported: when the Mac returns an RPC error whose code does not match the known set, the function falls back to scanning the free-text error message for substrings like "unknown method". A transient error whose message incidentally contains one of those phrases will be misclassified as "Mac does not support chat", causing the session list to be authoritatively cleared and potentially exiting chat mode. MobileShellComposite+AgentChat.swift — the chatSessionListFailureMeansUnsupported message-string fallback should be removed in favour of relying solely on the structured error code. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant SV as WorkspaceDetailView
participant WDC as WorkspaceDetailView+AgentChat
participant SS as MobileShellComposite
participant CCS as ChatConversationStore
participant Mac as Mac RPC Source
SV->>WDC: body → chatConversationWarmKey task
WDC->>WDC: runWarmChatConversation()
WDC->>SS: makeChatEventSource()
SS-->>WDC: MobileChatEventSource
WDC->>CCS: ensureChatConversationStore(for:)
CCS-->>WDC: ChatConversationStore (new or existing)
WDC->>CCS: run()
CCS->>Mac: source.events(sessionID:)
Mac-->>CCS: AsyncStream
note over SV,Mac: Connection drops
Mac-->>CCS: stream finishes
CCS->>CCS: waitForBackoffOrSourceReplacement()
note over SV,Mac: Mac reconnects → agentChatEventSourceIdentity changes
SV->>WDC: chatConversationWarmKey changes → task restarts
WDC->>SS: makeChatEventSource() → new source
WDC->>CCS: replaceSource(_:descriptor:sourceIdentity:)
CCS->>CCS: "sourceGeneration += 1, wakeBackoff()"
CCS->>Mac: source.events(sessionID:) [new source]
note over SV,Mac: Parallel: session list refresh
SV->>WDC: chatRefreshKey task → refreshChatSessions()
WDC->>Mac: source.sessions(workspaceID:)
Mac-->>WDC: [ChatSessionDescriptor] (authoritative)
WDC->>SS: rememberChatSessions(_, workspaceID:)
WDC->>WDC: reconcileChatSessionSnapshot(seedOutcomeCanInvalidateSelection: true)
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant SV as WorkspaceDetailView
participant WDC as WorkspaceDetailView+AgentChat
participant SS as MobileShellComposite
participant CCS as ChatConversationStore
participant Mac as Mac RPC Source
SV->>WDC: body → chatConversationWarmKey task
WDC->>WDC: runWarmChatConversation()
WDC->>SS: makeChatEventSource()
SS-->>WDC: MobileChatEventSource
WDC->>CCS: ensureChatConversationStore(for:)
CCS-->>WDC: ChatConversationStore (new or existing)
WDC->>CCS: run()
CCS->>Mac: source.events(sessionID:)
Mac-->>CCS: AsyncStream
note over SV,Mac: Connection drops
Mac-->>CCS: stream finishes
CCS->>CCS: waitForBackoffOrSourceReplacement()
note over SV,Mac: Mac reconnects → agentChatEventSourceIdentity changes
SV->>WDC: chatConversationWarmKey changes → task restarts
WDC->>SS: makeChatEventSource() → new source
WDC->>CCS: replaceSource(_:descriptor:sourceIdentity:)
CCS->>CCS: "sourceGeneration += 1, wakeBackoff()"
CCS->>Mac: source.events(sessionID:) [new source]
note over SV,Mac: Parallel: session list refresh
SV->>WDC: chatRefreshKey task → refreshChatSessions()
WDC->>Mac: source.sessions(workspaceID:)
Mac-->>WDC: [ChatSessionDescriptor] (authoritative)
WDC->>SS: rememberChatSessions(_, workspaceID:)
WDC->>WDC: reconcileChatSessionSnapshot(seedOutcomeCanInvalidateSelection: true)
Reviews (12): Last reviewed commit: "Ignore stale chat session refreshes" | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Packages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatConversationStore.swift`:
- Around line 416-419: The version check in ChatConversationStore’s descriptor
update path is too permissive and can let older cached snapshots overwrite newer
live state. Update the guard around the descriptor assignment so versioned
snapshots are only accepted when their version is strictly greater than the
current descriptor version, while still allowing the unversioned case handled by
isUnversioned. Keep the actual state replacement in the same update block that
assigns self.descriptor and agentState so stale snapshots fail closed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: cd3f6d27-6209-47fc-9672-03fb115a40cb
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (3)
Packages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatConversationStore.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView+AgentChat.swift
💤 Files with no reviewable changes (1)
- Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView+AgentChat.swift
| do { | ||
| seedOutcome = .authoritative(try await source.sessions(workspaceID: workspaceID)) | ||
| } catch { | ||
| seedOutcome = store.chatSessionListFailureMeansUnsupported(error) | ||
| ? .authoritative([]) |
There was a problem hiding this comment.
RPC failure in seed path incorrectly clears the session cache
When source.sessions() throws (e.g. a timeout while the Mac is technically connected), seedOutcome is set to .authoritative([]). Because canInvalidateSelection is true, this path (1) persists an empty list to the shell store via store.rememberChatSessions([], ...) and (2) allows applyChatModeFallback to exit chat mode. The whole point of WorkspaceChatSessionRefreshOutcome.unavailable was to prevent exactly this: transport or RPC failures should not clobber the cached snapshot. Using .unavailable here preserves the cached sessions and prevents spurious chat-mode exit on a transient RPC failure.
| do { | |
| seedOutcome = .authoritative(try await source.sessions(workspaceID: workspaceID)) | |
| } catch { | |
| seedOutcome = store.chatSessionListFailureMeansUnsupported(error) | |
| ? .authoritative([]) | |
| do { | |
| seedOutcome = .authoritative(try await source.sessions(workspaceID: workspaceID)) | |
| } catch { | |
| seedOutcome = .unavailable | |
| } |
# Conflicts: # Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceChatPane.swift # Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swift
e5bef85 to
b185e49
Compare
| public func chatSessionListFailureMeansUnsupported(_ error: any Error) -> Bool { | ||
| guard case let MobileShellConnectionError.rpcError(code, message) = error else { | ||
| return false | ||
| } | ||
| let normalizedCode = code?.trimmingCharacters(in: .whitespacesAndNewlines).lowercased() | ||
| let normalizedMessage = message.trimmingCharacters(in: .whitespacesAndNewlines).lowercased() | ||
| if let normalizedCode, | ||
| ["method_not_found", "not_found", "unknown_method", "unsupported_method"].contains(normalizedCode) { | ||
| return true | ||
| } | ||
| return normalizedMessage.contains("unknown method") | ||
| || normalizedMessage.contains("method not found") | ||
| || normalizedMessage.contains("unsupported method") | ||
| } |
There was a problem hiding this comment.
Unreliable message-string fallback drives a correctness-critical classification
When the Mac returns an rpcError whose code does not match the known set, the function falls back to scanning the free-text message for substrings like "unknown method". Any localized, reformatted, or vendor-extended error message that happens to contain one of those phrases will be misclassified as "unsupported", causing refreshChatSessions to use .authoritative([]) — which clears the cached session list and allows applyChatModeFallback to exit chat mode. Conversely, a genuinely unsupported Mac without a matching message string is silently treated as a transient failure, preserving stale sessions and leaving a chat toggle that can never open a session.
The code-based branch (normalizedCode) is a reliable structured signal; it should be the only detection path. If the Mac omits an error code for unsupported methods, the right fix is to add one on the Mac side and remove the message fallback here rather than guess on opaque prose.
Summary
Verification
Notes
Summary by CodeRabbit