Repository navigation
iOS: Telegram-style chat surface for agent sessions - #5984
Merged
Merged
Conversation
Message model (typed kinds with fail-open wire coding), session descriptors and agent state, ChatEventSource seam, transcript row projection, @observable conversation store, and a fixture source for previews and tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…pairing Pure JSONL-to-ChatMessage parsers validated against real Claude Code 2.1.175 and Codex CLI 0.139 session files; carry-over parse state pairs tool results across tail batches; typed JSON tree keeps payloads strongly typed; truncation budgets centralized. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
MobileChatEventSource adapts MobileCoreRPCClient to the ChatEventSource seam (mobile.chat.sessions/history/send/interrupt/answer RPCs plus the chat.message event topic); ChatWireCoding pins ISO 8601 dates on both ends of the wire. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Multi-block transcript lines share a seq; the equality check could insert the separator repeatedly. Track insertion and compare with >= so a windowed-out anchor still yields one separator. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Transcript list with bottom-anchored auto-follow and scroll pill, prose bubbles with markdown and embedded code blocks, near-full-width terminal/diff cards with head-tail collapse, actionable permission and question cards with frozen receipts, thought/tool/status rows, pending bubbles with delivery ticks, keyboard composer with accessory chips and photo attachments. Adaptive light/dark theme; terminal cards stay dark in both schemes with fixed light text. en+ja localization in the package catalog. Demo entry: Settings > Developer > Agent Chat Demo (DEBUG) on a scripted fixture conversation, verified on simulator in both color schemes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Mac side: AgentChatTranscriptService tracks agent sessions from hook events and the on-disk hook session stores, resolves and tails their JSONL transcripts (bounded backfill, incremental parse, truncation recovery), serves mobile.chat.sessions/history, pushes chat.message frames, and routes send/interrupt/answer through the existing mobile terminal injection paths so chat input behaves exactly like composer input. iOS side: workspace toolbar gains an Agent Chat button when the connected Mac reports chat-capable sessions, opening the conversation full-screen over MobileChatEventSource. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
.task on a chain resolving to EmptyView never fires; render a zero-sized clear view in the hidden state so the session poll always runs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Store: subscribe before fetching history so no event falls in the gap (buffered stream + id dedup), reconcile pending echoes during resync and tolerate budget-truncated echoes, carry in-place tool-result updates missed while disconnected, stop paging on an empty page and surface head truncation honestly. Event source: a failed subscribe handshake finishes the stream (was a silent connected-but-deaf wedge) and unsubscribes server-side on termination. Mac service: stop and evict tailers for ended sessions, memoize failed transcript resolutions, pid liveness sweep marks crashed sessions ended (no more eternal typing indicator), subagentStop no longer flips a working session idle, attachment-only sends now submit. Parser: skip isSidechain lines. UI: composer shows a session-ended bar and disables send while disconnected, transcript gains loading/empty/truncated-head states, permission and question cards disarm after the first tap with a progress spinner and haptic, equatable rows at the ForEach call site, cached date formatter, localized elapsed label, accessibility identifiers on the send/composer/decision controls. New regression tests: truncated-echo reconcile, cache-head paging stop, sidechain skip (62 total). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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 a complete agent-chat subsystem: shared models and wire types, JSONL transcript parsers (Claude/Codex) and helpers, a MainActor conversation store and projector, SwiftUI chat UI with renderer/sanitizer/cache, mobile RPC event source and host RPC handlers, mac transcript tailer/service/registry, and extensive tests. ChangesAgent chat feature
Sequence Diagram(s)sequenceDiagram
participant TerminalController
participant AgentChatTranscriptService
participant AgentChatTranscriptTailer
participant MobileChatEventSource
participant ChatConversationStore
TerminalController->>AgentChatTranscriptService: noteHookEvent(event)
AgentChatTranscriptService->>AgentChatTranscriptTailer: ensureTailer / start
AgentChatTranscriptTailer->>AgentChatTranscriptService: onBatch(appended, updated)
AgentChatTranscriptService->>MobileChatEventSource: emit chat.message frame
MobileChatEventSource->>ChatConversationStore: events(sessionID) stream item
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Collaborator
Author
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
Collapse the five mobile.chat dispatch cases into one prefix-routed case (handler switch lives in the extension file), single-line the hook ingestion task, condense the client accessor doc. Refresh the budget TSV for the three irreducible integration one-liners. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 24
🤖 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/CmuxAgentChat/Sources/CmuxAgentChat/Model/ChatQuestion.swift`:
- Around line 8-11: The Option struct's identity is currently computed from
label which is not stable or unique; change Option.id from a computed property
to an explicit stored property (e.g., public var id: String) and ensure options
are constructed with an index-based id when building a question (use the option
index as the id value); update any initializers/builders that create Option
instances to set this id, and keep Equatable/Codable/Identifiable conformance
intact by removing the computed id and relying on the stored id field in Option.
In
`@Packages/CmuxAgentChat/Sources/CmuxAgentChat/Parsing/ClaudeTranscriptParser.swift`:
- Around line 275-277: The current logic only sets pendingKey for the first
emitted message (assembler.append(message, pendingKey: index == 0 ? callID :
nil)), leaving later AskUserQuestion cards unreconciled; change it to register
all messages produced by the same tool call so they can be matched to the
incoming tool_result. Update the assembler.append call to pass the same callID
as pendingKey for every message emitted by the AskUserQuestion tool (or whenever
the message originates from that call) instead of only when index == 0, ensuring
all emitted question messages are tracked for reconciliation with tool_result.
In
`@Packages/CmuxAgentChat/Sources/CmuxAgentChat/Parsing/TranscriptDiffBuilder.swift`:
- Around line 62-65: The lines(_:) helper currently uses
text.components(separatedBy: "\n") which leaves a terminal empty string when
text ends with a newline and inflates diff hunks; update lines(_:) to remove a
single trailing empty segment for newline-terminated text (e.g. compute parts =
text.components(separatedBy: "\n"), if parts.last == "" { parts.removeLast() },
then return parts) so the function returns actual lines without a spurious
trailing empty line.
In
`@Packages/CmuxAgentChat/Sources/CmuxAgentChat/Parsing/TranscriptToolCompletion.swift`:
- Around line 83-90: answer(forPrompt:) fails when prompts contain quotes
because the needle is built from raw prompt but transcript output escapes
quotes; update the matching to account for escaped characters by escaping quotes
and backslashes in the prompt before building the needle (e.g., replace
backslashes then replace " with \") or alternatively unescape the transcript
output first and then perform the range search on the unescaped string; adjust
the logic around output.range(of: needle) and subsequent slicing so it uses the
escaped prompt variant (or unescaped output) to reliably locate and extract the
answer.
In
`@Packages/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatConversationStore.swift`:
- Around line 326-327: The dedup logic uses knownIDs =
Set(messages.suffix(64).map(\.id)) which only checks the last 64 messages and
allows older duplicates to be reappended; change it to build the Set from all
existing messages (e.g., Set(messages.map(\.id))) so fresh = newMessages.filter
{ !knownIDs.contains($0.id) } correctly filters duplicates across the whole
conversation (referencing knownIDs, messages.suffix(64), fresh, newMessages, and
id).
- Around line 265-283: resyncTail() currently fetches only one page from
source.history(...) and can miss older messages if the outage produced >pageSize
new messages; change resyncTail() to page backwards repeatedly (calling
source.history(sessionID:descriptor.id, beforeSeq: nextBefore, limit: pageSize))
until either you include the existing newestKnown seq or there are no more
pages, for each page call reconcilePending(against: page.messages) and
appendToWindow(page.messages) (or prepend if needed based on window ordering),
update hasMoreHistory = page.hasMore appropriately, and call reproject() after
finishing; ensure this loop stops when newestKnown is seen so loadOlder() can
continue paging from the correct oldest window edge.
In
`@Packages/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatTranscriptProjector.swift`:
- Around line 64-76: When inserting the unread separator mid-group (triggered by
firstUnreadSeq / insertedUnreadSeparator and rows.append(.unreadSeparator)), the
subsequent ChatMessageRowSnapshot is still computed using the original
groupLength and offset which leaves bubble metadata incorrect; update the logic
in the projection loop so that after inserting the unread separator you treat
the remainder of the group as a new group segment (adjust groupLength and offset
used for groupPosition(...) and showsTimestamp) or compute groupPosition and
showsTimestamp based on the post-separator segment (e.g., recalc remainingCount
= groupLength - offset and use that for members after the separator) so rows
before and after rows.append(.unreadSeparator) get correct .first/.last
positioning and timestamp visibility.
In `@Packages/CmuxAgentChat/Sources/CmuxAgentChat/Wire/ChatSessionEvent.swift`:
- Around line 30-49: The decoder currently throws for unknown EventName values
inside ChatSessionEvent.init(from:) which will break older clients when new
server events arrive; update the code and related types so unknown events are
handled gracefully by adding an explicit fallback (e.g., a ChatSessionEvent.case
like .unsupported(String) or .unknown(eventName: String, rawPayload: Any?)) and
change the switch in init(from:) to assign that fallback instead of throwing
(optionally log the raw event string using your logger). Ensure you also update
any encoder/consumer code that pattern-matches ChatSessionEvent to handle the
new fallback case (refer to EventName, ChatSessionEvent.init(from:), and any
switch sites that exhaustively handle ChatSessionEvent).
In
`@Packages/CmuxAgentChat/Tests/CmuxAgentChatTests/ClaudeTranscriptParserTests.swift`:
- Around line 59-62: The helper function json(_:) currently uses a forced try
(try!) when calling JSONSerialization.data(withJSONObject:), so replace the
forced try with a do/catch around JSONSerialization.data(withJSONObject:) in the
json(_:) function; on error call XCTFail (or preconditionFailure) with the
caught error message and return an empty/fallback string or abort, ensuring
tests fail gracefully instead of crashing.
In
`@Packages/CmuxAgentChat/Tests/CmuxAgentChatTests/CodexTranscriptParserTests.swift`:
- Around line 17-20: Replace the forced try! around
JSONSerialization.data(withJSONObject:) in the fixture serialization helper (the
code that builds a JSON string from "timestamp", "type", and "payload") with
explicit do-catch error handling: call JSONSerialization.data(withJSONObject:)
inside a do block, on success return String(decoding: data, as: UTF8.self), and
in the catch return a deterministic fallback string (e.g., "{}" or a fixed
"invalid-fixture" string) instead of crashing; ensure you reference the same
variables (timestamp, type, payload) and avoid propagating the thrown error to
tests.
In
`@Packages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Composer/ChatComposerView.swift`:
- Around line 315-320: The removal uses the attachments array index to also
remove from pickedItems, which is unsafe because attachments is built from a
filtered pass and indices can diverge; update removeAttachment(id:) to remove
the corresponding pickedItems entry by matching a stable identifier (e.g., an
attachment.pickerId/assetIdentifier or the original source item's unique id)
rather than by index. Locate removeAttachment and the code that constructs
attachments from pickedItems (references to attachments and pickedItems in
ChatComposerView), ensure each Attachment stores the source/picker identifier
when created, then find the pickedItems element with the same identifier
(firstIndex(where: { pickedItem.identifier == attachment.pickerId })) and remove
that element if found.
In
`@Packages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/ChatTranscriptListView.swift`:
- Around line 110-115: The top sentinel currently calls onReachTop on every
.onAppear which can re-enter and overlap fetches; add a local loading gate
(e.g., a `@State` Bool like isLoadingMore) in ChatTranscriptListView and check it
alongside hasMoreHistory/historyTruncatedAtHead before invoking onReachTop, set
isLoadingMore = true immediately before calling onReachTop and clear it when the
fetch completes (or provide a completion callback to clear it) so simultaneous
appearances won't trigger duplicate pagination.
In
`@Packages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/Rows/ChatFileEditCardView.swift`:
- Around line 96-100: The diffBlock(diff:) view currently calls
split(...).map(String.init) which materializes the entire diff on every render;
change this so the row only receives or computes the capped lines once: move the
projection/capping upstream (e.g., into the model/store or a ViewModel) or add a
cached pre-sliced property (e.g., cachedVisibleLines or preparedLines) and have
diffBlock use that plus isExpanded/expandedLineCap/collapsedLineCap to decide
whether to show more; alternatively implement a lazy prefix extractor in the
data layer so diffBlock no longer performs full split/map per render.
In
`@Packages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/Rows/ChatSessionHeaderView.swift`:
- Around line 103-110: ChatStateDotView's repeat animation uses the local state
pulsing but only sets it in .onAppear, so when pulses (derived from
isWorking/agentState) flips while the view remains mounted the animation won't
start; update the view to also react to pulses changes (e.g., add .onChange(of:
pulses) { pulsing = $0 } or set pulsing = pulses inside that handler, and
initialize pulsing from pulses in .onAppear) so pulsing follows the pulses flag
whenever ChatStateDotView is mounted.
In
`@Packages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/Rows/ChatTerminalCardView.swift`:
- Line 113: Replace hardcoded user-facing strings with localized catalog
entries: for the duration Text(verbatim: String(format: "%.1fs", duration))
create a localized key (e.g., "duration_seconds_short") and display it using
locale-aware formatting (e.g., String(format:
NSLocalizedString("duration_seconds_short", comment: ""), locale:
Locale.current, duration) or Number/Measurement formatters) so the trailing "s"
is provided by localization; for the hidden-lines message replace the inline "⋯
\(hiddenCount) more lines" with a pluralized stringsdict key (e.g.,
"hidden_lines") with .one/.other variants and render it via
String.localizedStringWithFormat(NSLocalizedString("hidden_lines", comment: ""),
hiddenCount) or LocalizedStringKey interpolation, then add the corresponding
entries to Localizable.strings and Localizable.stringsdict to cover supported
locales; update usages in ChatTerminalCardView to reference these keys
(hiddenCount and the duration display).
In
`@Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite`+AgentChat.swift:
- Around line 20-23: The chatSessions implementation swallows all RPC failures
by returning an empty array, conflating real errors with "no sessions"; change
the API to surface errors instead—either make public func
chatSessions(workspaceID: String?) async throws -> [ChatSessionDescriptor] and
propagate errors from makeChatEventSource() and try await
source.sessions(workspaceID:), or change the return to async ->
Result<[ChatSessionDescriptor], Error> and return .failure(...) on
RPC/connection errors; update callers accordingly and stop using the catch-all
(try?) that maps failures to [] in MobileShellComposite+AgentChat, referencing
chatSessions, makeChatEventSource(), and source.sessions(workspaceID:).
In `@scripts/lint-ios-package-conventions.sh`:
- Around line 22-23: The global lint scan still hardcodes only
Packages/CMUXMobile* and Packages/CmuxMobile* so Packages/CmuxAgentChat* is
never checked; update the global-scan invocation that currently lists
"Packages/CMUXMobile* Packages/CmuxMobile*" (the same place that performs the
'global' scan around the existing scan/grep logic) to also include
"Packages/CmuxAgentChat*" so it matches the SCOPES population (SCOPES array and
the initial for d in ... loop) and therefore lints Packages/CmuxAgentChat* for
UserDefaults/FileManager/Bundle usages.
In `@Sources/AppDelegate.swift`:
- Line 1960: AgentChatTranscriptService.shared.start() is performing synchronous
disk I/O on the main actor via AgentChatSessionRegistry.seedFromHookStores()
(which calls AgentChatHookSessionStore.entries(...), using Data(contentsOf:) +
JSONSerialization and kill(pid_t, 0)), so move the seeding off the main actor:
change start() (or wherever start() is invoked in AppDelegate.configure(...)) to
perform the file read/parse/killing checks inside a background task (e.g.,
Task.detached or Task { await withCheckedContinuation/... } ) and then hop back
to the MainActor only to update the in-memory records collection; alternatively
defer calling start() until after launch completes—ensure
AgentChatHookSessionStore.entries(...) does not perform blocking
Data(contentsOf:) on the main thread but returns parsed entries from a
background queue before updating AgentChatSessionRegistry.records on the
MainActor.
In `@Sources/Mobile/AgentChat/AgentChatHookSessionStore.swift`:
- Around line 45-50: The entries(agentSource:) method constructs a file path
using agentSource and can be abused with path traversal (e.g., "../"); validate
or sanitize agentSource before building the filename: ensure it only contains
allowed characters (e.g., alphanumerics, underscore, dash) or percent-encode it,
reject or normalize any input containing path separators or "..", then use the
sanitized value when appending the path component (homeDirectory and file
variables) so the hook-store filename cannot escape .cmuxterm.
In `@Sources/Mobile/AgentChat/AgentChatSessionRegistry.swift`:
- Around line 40-42: Guard against non-positive PIDs before calling kill by only
calling kill(..., 0) when the stored PID is > 0; in the checks that currently do
guard record.state != .ended, let pid = record.pid else { ... } (and the
duplicate probe later), change the guard to require pid > 0 (e.g., guard
record.state != .ended, let pid = record.pid, pid > 0) so you don't invoke kill
with 0/negative PIDs, and keep the existing update(sessionID:stateChanged:){
$0.state = .ended } logic intact.
- Line 9: The records dictionary currently keys sessions only by sessionID
(private var records: [String: AgentChatSessionRecord]) which allows collisions
across different agent sources; change the keying to a namespaced composite key
(e.g., include the agent/source identifier together with sessionID) everywhere
session keys are created or looked up: update where records are inserted/updated
(the code that adds into records), where duplicates are detected/removed (the
logic that drops duplicates), and where source prefixes are stripped (the logic
at/around Session ID normalization) so all lookups, deletions, and merges use
the composite key consistently and AgentChatSessionRecord retains both raw
sessionID and source fields for display and comparison. Ensure the composite-key
scheme is used in any methods that read/write records so sessions from different
runtimes cannot collide.
In `@Sources/Mobile/AgentChat/AgentChatTranscriptResolver.swift`:
- Around line 58-74: codexFallbackPath currently does a full recursive scan of
~/.codex/sessions on every call which is expensive; change this to build and
reuse an in-memory index (e.g., a [String: String] map from lowercased sessionID
needle to file path) in the AgentChatTranscriptResolver instance and have
codexFallbackPath consult the cached index instead of re-enumerating; add a
private method (e.g., rebuildCodexIndex) that performs the
fileManager.enumerator work once (on init, first access, or on demand) and a
simple invalidation strategy (watch the directory, expose a refresh call, or use
a TTL) so updates are picked up without rescanning on every resolution;
reference codexFallbackPath, homeDirectory, fileManager and the new
rebuild/invalidate methods when implementing.
In `@Sources/Mobile/AgentChat/AgentChatTranscriptTailer.swift`:
- Around line 109-114: Guard against non-positive limit before computing start:
if limit <= 0 return an empty page (or clamp limit to at least 1) prior to
calculating start = max(eligible.startIndex, eligible.endIndex - limit) and
before the while that accesses cache[start] and cache[start-1]; this prevents
start from equaling eligible.endIndex and avoids out-of-bounds access when
reading cache[start]. Update the method in AgentChatTranscriptTailer (variables:
limit, start, eligible, cache, seq) to perform the check/early-return or clamp
immediately and only then run the boundary-extension loop.
🪄 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: ca68571c-dbfa-4a43-9e3d-c108a06fa0d1
📒 Files selected for processing (104)
Packages/CmuxAgentChat/Package.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Model/ChatAgentKind.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Model/ChatAgentState.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Model/ChatAttachment.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Model/ChatFileEdit.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Model/ChatMessage.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Model/ChatMessageKind.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Model/ChatOutboundAttachment.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Model/ChatPermissionRequest.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Model/ChatProse.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Model/ChatQuestion.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Model/ChatRole.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Model/ChatSessionDescriptor.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Model/ChatStatusTransition.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Model/ChatTerminalCapture.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Model/ChatThought.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Model/ChatToolUse.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Model/ChatUnsupportedPayload.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Parsing/ChatTranscriptParseResult.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Parsing/ChatTranscriptParseState.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Parsing/ClaudeTranscriptParser.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Parsing/CodexTranscriptParser.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Parsing/TranscriptBatchAssembler.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Parsing/TranscriptDiffBuilder.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Parsing/TranscriptJSONValue.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Parsing/TranscriptTextBudget.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Parsing/TranscriptTimestampParser.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Parsing/TranscriptToolCompletion.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Source/ChatEventSource.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Source/FixtureChatEventSource.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatConversationStore.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatDeliveryState.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatGroupPosition.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatMessageRowSnapshot.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatPendingOutbound.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatTranscriptProjector.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatTranscriptRow.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Wire/ChatHistoryPage.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Wire/ChatSessionEvent.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Wire/ChatSessionEventFrame.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Wire/ChatWireCoding.swiftPackages/CmuxAgentChat/Tests/CmuxAgentChatTests/ChatConversationStoreTests.swiftPackages/CmuxAgentChat/Tests/CmuxAgentChatTests/ChatMessageCodableTests.swiftPackages/CmuxAgentChat/Tests/CmuxAgentChatTests/ChatTranscriptProjectorTests.swiftPackages/CmuxAgentChat/Tests/CmuxAgentChatTests/ClaudeTranscriptParserTests.swiftPackages/CmuxAgentChat/Tests/CmuxAgentChatTests/CodexTranscriptParserTests.swiftPackages/CmuxAgentChatUI/Package.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Composer/ChatAccessoryChipRow.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Composer/ChatComposerAttachment.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Composer/ChatComposerView.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Markdown/ChatANSISanitizer.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Markdown/ChatMarkdownRenderer.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Markdown/ChatProseSegment.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Markdown/ChatProseSegmenter.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Previews/ChatFixtureConversation.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Previews/ChatScreenPreviews.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Resources/Localizable.xcstringsPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Screen/ChatScreen.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Theme/ChatTheme.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Theme/Color+ChatAdaptive.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/ChatMessageRowView.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/ChatRowActions.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/ChatScrollToBottomButton.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/ChatTranscriptListView.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/ChatTranscriptRowView.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/ChatTypingIndicatorView.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/Rows/ChatAttachmentBubbleView.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/Rows/ChatDateHeaderView.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/Rows/ChatFileEditCardView.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/Rows/ChatPendingBubbleView.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/Rows/ChatPermissionCardView.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/Rows/ChatProseBubbleView.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/Rows/ChatQuestionCardView.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/Rows/ChatSessionHeaderView.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/Rows/ChatStatusRowView.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/Rows/ChatTerminalCardView.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/Rows/ChatThoughtRowView.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/Rows/ChatToolUseRowView.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/Rows/ChatUnreadSeparatorView.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/Rows/ChatUnsupportedRowView.swiftPackages/CmuxAgentChatUI/Tests/CmuxAgentChatUITests/ChatANSISanitizerTests.swiftPackages/CmuxAgentChatUI/Tests/CmuxAgentChatUITests/ChatProseSegmenterTests.swiftPackages/CmuxMobileShell/Package.swiftPackages/CmuxMobileShell/Sources/CmuxMobileShell/MobileChatEventSource.swiftPackages/CmuxMobileShell/Sources/CmuxMobileShell/MobileChatSessionsResponse.swiftPackages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+AgentChat.swiftPackages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftPackages/CmuxMobileShellUI/Package.swiftPackages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/AgentChatDemoScreen.swiftPackages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileSettingsView.swiftPackages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceAgentChatButton.swiftPackages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swiftSources/AppDelegate.swiftSources/Mobile/AgentChat/AgentChatHookSessionStore.swiftSources/Mobile/AgentChat/AgentChatSessionRecord.swiftSources/Mobile/AgentChat/AgentChatSessionRegistry.swiftSources/Mobile/AgentChat/AgentChatTranscriptResolver.swiftSources/Mobile/AgentChat/AgentChatTranscriptService.swiftSources/Mobile/AgentChat/AgentChatTranscriptTailer.swiftSources/TerminalController+MobileChat.swiftSources/TerminalController.swiftcmux.xcodeproj/project.pbxprojios/cmux/Resources/Localizable.xcstringsscripts/lint-ios-package-conventions.sh
Comment on lines
+8
to
+11
| public struct Option: Sendable, Equatable, Codable, Identifiable { | ||
| /// Stable identity within the question (the option's index as text). | ||
| public var id: String { label } | ||
|
|
There was a problem hiding this comment.
Option.id is not stable/unique as implemented.
id is derived from label, so duplicate labels collapse identity and can misroute SwiftUI row state/actions. It also contradicts the doc that says identity is index-based. Make id an explicit stored field (index-based when building options) instead of computed-from-label.
🤖 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/CmuxAgentChat/Sources/CmuxAgentChat/Model/ChatQuestion.swift` around
lines 8 - 11, The Option struct's identity is currently computed from label
which is not stable or unique; change Option.id from a computed property to an
explicit stored property (e.g., public var id: String) and ensure options are
constructed with an index-based id when building a question (use the option
index as the id value); update any initializers/builders that create Option
instances to set this id, and keep Equatable/Codable/Identifiable conformance
intact by removing the computed id and relying on the stored id field in Option.
Comment on lines
+62
to
+65
| private func lines(_ text: String) -> [String] { | ||
| guard !text.isEmpty else { return [] } | ||
| return text.components(separatedBy: "\n") | ||
| } |
There was a problem hiding this comment.
Fix trailing-newline line counting in diff rendering.
components(separatedBy: "\n") keeps a terminal empty segment for newline-terminated text, which inflates additions/deletions and can render a spurious blank +/- line.
Suggested fix
private func lines(_ text: String) -> [String] {
guard !text.isEmpty else { return [] }
- return text.components(separatedBy: "\n")
+ var parts = text.split(separator: "\n", omittingEmptySubsequences: false).map(String.init)
+ if text.hasSuffix("\n"), parts.last == "" {
+ parts.removeLast()
+ }
+ return parts
}🤖 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/CmuxAgentChat/Sources/CmuxAgentChat/Parsing/TranscriptDiffBuilder.swift`
around lines 62 - 65, The lines(_:) helper currently uses
text.components(separatedBy: "\n") which leaves a terminal empty string when
text ends with a newline and inflates diff hunks; update lines(_:) to remove a
single trailing empty segment for newline-terminated text (e.g. compute parts =
text.components(separatedBy: "\n"), if parts.last == "" { parts.removeLast() },
then return parts) so the function returns actual lines without a spurious
trailing empty line.
Comment on lines
+83
to
+90
| private func answer(forPrompt prompt: String) -> String? { | ||
| guard let output else { return nil } | ||
| let needle = "\"\(prompt)\"=\"" | ||
| guard let start = output.range(of: needle) else { return nil } | ||
| let tail = output[start.upperBound...] | ||
| guard let end = tail.range(of: "\"") else { return nil } | ||
| let answer = String(tail[..<end.lowerBound]) | ||
| return answer.isEmpty ? nil : answer |
There was a problem hiding this comment.
Handle escaped quotes when extracting answered question text.
answer(forPrompt:) builds a raw needle from prompt, so prompts containing quotes/escapes won’t match transcript-escaped output and the question stays unresolved.
Suggested fix
private func answer(forPrompt prompt: String) -> String? {
guard let output else { return nil }
- let needle = "\"\(prompt)\"=\""
- guard let start = output.range(of: needle) else { return nil }
- let tail = output[start.upperBound...]
- guard let end = tail.range(of: "\"") else { return nil }
- let answer = String(tail[..<end.lowerBound])
- return answer.isEmpty ? nil : answer
+ let escapedPrompt = NSRegularExpression.escapedPattern(for: prompt)
+ let pattern = #""\#(escapedPrompt)"="((?:\\.|[^"])*)""#
+ guard
+ let regex = try? NSRegularExpression(pattern: pattern),
+ let match = regex.firstMatch(
+ in: output,
+ range: NSRange(output.startIndex..<output.endIndex, in: output)
+ ),
+ let range = Range(match.range(at: 1), in: output)
+ else { return nil }
+
+ let raw = String(output[range])
+ let unescaped = raw.replacingOccurrences(of: #"\""#, with: #"""#)
+ return unescaped.isEmpty ? nil : unescaped
}🤖 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/CmuxAgentChat/Sources/CmuxAgentChat/Parsing/TranscriptToolCompletion.swift`
around lines 83 - 90, answer(forPrompt:) fails when prompts contain quotes
because the needle is built from raw prompt but transcript output escapes
quotes; update the matching to account for escaped characters by escaping quotes
and backslashes in the prompt before building the needle (e.g., replace
backslashes then replace " with \") or alternatively unescape the transcript
output first and then perform the range search on the unescaped string; adjust
the logic around output.range(of: needle) and subsequent slicing so it uses the
escaped prompt variant (or unescaped output) to reliably locate and extract the
answer.
Comment on lines
+75
to
+77
| func entry(agentSource: String, sessionID: String) -> Entry? { | ||
| entries(agentSource: agentSource).first { $0.sessionID == sessionID } | ||
| } |
There was a problem hiding this comment.
entry(...) does a full synchronous file parse on every lookup.
Line 76 re-reads and re-parses the entire store via entries(...) for each sessionID lookup. This is expensive and can stall the main actor when called from live event ingestion.
Suggested direction
- func entry(agentSource: String, sessionID: String) -> Entry? {
- entries(agentSource: agentSource).first { $0.sessionID == sessionID }
- }
+ // Keep an in-memory per-agent index refreshed once per store read, then lookup by key.
+ // (or pass preloaded entries/index from registry for event bursts)| /// hook events and the on-disk hook session stores. | ||
| @MainActor | ||
| final class AgentChatSessionRegistry { | ||
| private var records: [String: AgentChatSessionRecord] = [:] |
There was a problem hiding this comment.
Session identity is not namespaced by agent source, so records can collide.
Line 9 keys records only by sessionID, while Line 81 drops duplicates and Line 145-150 strips source prefixes. If two agent runtimes produce the same raw ID, one session can overwrite/hide the other and state updates can cross-wire.
Suggested fix direction
- private var records: [String: AgentChatSessionRecord] = [:]
+ private var records: [String: AgentChatSessionRecord] = [:] // key = "\(source):\(sessionID)"
- guard records[entry.sessionID] == nil else { continue }
+ let key = "\(source):\(entry.sessionID)"
+ guard records[key] == nil else { continue }
- let sessionID = Self.normalizedSessionID(event.sessionId, source: event.source)
+ let sessionID = Self.normalizedSessionID(event.sessionId, source: event.source)
+ let key = "\(event.source):\(sessionID)"Also applies to: 81-83, 106-110, 145-150
🤖 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 `@Sources/Mobile/AgentChat/AgentChatSessionRegistry.swift` at line 9, The
records dictionary currently keys sessions only by sessionID (private var
records: [String: AgentChatSessionRecord]) which allows collisions across
different agent sources; change the keying to a namespaced composite key (e.g.,
include the agent/source identifier together with sessionID) everywhere session
keys are created or looked up: update where records are inserted/updated (the
code that adds into records), where duplicates are detected/removed (the logic
that drops duplicates), and where source prefixes are stripped (the logic
at/around Session ID normalization) so all lookups, deletions, and merges use
the composite key consistently and AgentChatSessionRecord retains both raw
sessionID and source fields for display and comparison. Ensure the composite-key
scheme is used in any methods that read/write records so sessions from different
runtimes cannot collide.
Comment on lines
+40
to
+42
| guard record.state != .ended, let pid = record.pid else { continue } | ||
| if kill(pid_t(pid), 0) != 0 { | ||
| update(sessionID: sessionID, stateChanged: true) { $0.state = .ended } |
There was a problem hiding this comment.
Guard PID probes to positive values before calling kill(..., 0).
Lines 40-42 and 82 probe liveness for any integer PID. 0/negative values have special semantics and can incorrectly mark sessions alive/ended.
Suggested guard
- guard record.state != .ended, let pid = record.pid else { continue }
+ guard record.state != .ended, let pid = record.pid, pid > 0 else { continue }
- let alive = entry.pid.map { kill(pid_t($0), 0) == 0 } ?? false
+ let alive = entry.pid.map { $0 > 0 && kill(pid_t($0), 0) == 0 } ?? falseAlso applies to: 82-82
🤖 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 `@Sources/Mobile/AgentChat/AgentChatSessionRegistry.swift` around lines 40 -
42, Guard against non-positive PIDs before calling kill by only calling
kill(..., 0) when the stored PID is > 0; in the checks that currently do guard
record.state != .ended, let pid = record.pid else { ... } (and the duplicate
probe later), change the guard to require pid > 0 (e.g., guard record.state !=
.ended, let pid = record.pid, pid > 0) so you don't invoke kill with 0/negative
PIDs, and keep the existing update(sessionID:stateChanged:){ $0.state = .ended }
logic intact.
Comment on lines
+58
to
+74
| private func codexFallbackPath(sessionID: String) -> String? { | ||
| let root = homeDirectory | ||
| .appendingPathComponent(".codex", isDirectory: true) | ||
| .appendingPathComponent("sessions", isDirectory: true) | ||
| guard let enumerator = fileManager.enumerator( | ||
| at: root, | ||
| includingPropertiesForKeys: nil, | ||
| options: [.skipsHiddenFiles] | ||
| ) else { return nil } | ||
| let needle = sessionID.lowercased() | ||
| for case let url as URL in enumerator { | ||
| guard url.pathExtension == "jsonl" else { continue } | ||
| if url.lastPathComponent.lowercased().contains(needle) { | ||
| return url.path | ||
| } | ||
| } | ||
| return nil |
There was a problem hiding this comment.
Codex fallback does a full recursive filesystem scan per resolution.
Line 58-74 rescans all ~/.codex/sessions entries on each call. With many sessions/files, this becomes a repeated full-collection scan in a runtime path and can degrade chat loading and live updates.
As per coding guidelines, production paths over scalable user data should avoid per-target rescans of full collections (.github/review-bot-rules/algorithmic-complexity.md).
🤖 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 `@Sources/Mobile/AgentChat/AgentChatTranscriptResolver.swift` around lines 58 -
74, codexFallbackPath currently does a full recursive scan of ~/.codex/sessions
on every call which is expensive; change this to build and reuse an in-memory
index (e.g., a [String: String] map from lowercased sessionID needle to file
path) in the AgentChatTranscriptResolver instance and have codexFallbackPath
consult the cached index instead of re-enumerating; add a private method (e.g.,
rebuildCodexIndex) that performs the fileManager.enumerator work once (on init,
first access, or on demand) and a simple invalidation strategy (watch the
directory, expose a refresh call, or use a TTL) so updates are picked up without
rescanning on every resolution; reference codexFallbackPath, homeDirectory,
fileManager and the new rebuild/invalidate methods when implementing.
Source: Coding guidelines
Comment on lines
+109
to
+114
| var start = max(eligible.startIndex, eligible.endIndex - limit) | ||
| // Never split an equal-seq group across the boundary: extend back to | ||
| // include every message sharing the boundary line's seq. | ||
| while start > eligible.startIndex, cache[start - 1].seq == cache[start].seq { | ||
| start -= 1 | ||
| } |
There was a problem hiding this comment.
Guard non-positive limit to prevent out-of-bounds access.
If limit <= 0, start can become eligible.endIndex, and the comparison at Line 112 reads cache[start], which traps. Clamp limit inside this method (or early-return an empty page) before boundary expansion.
Suggested fix
func history(beforeSeq: Int?, limit: Int) -> ChatHistoryPage {
+ let safeLimit = max(limit, 1)
let eligible: ArraySlice<ChatMessage>
if let beforeSeq {
let end = cache.firstIndex { $0.seq >= beforeSeq } ?? cache.endIndex
eligible = cache[..<end]
} else {
eligible = cache[...]
}
- var start = max(eligible.startIndex, eligible.endIndex - limit)
+ var start = max(eligible.startIndex, eligible.endIndex - safeLimit)
// Never split an equal-seq group across the boundary: extend back to
// include every message sharing the boundary line's seq.
while start > eligible.startIndex, cache[start - 1].seq == cache[start].seq {
start -= 1
}🤖 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 `@Sources/Mobile/AgentChat/AgentChatTranscriptTailer.swift` around lines 109 -
114, Guard against non-positive limit before computing start: if limit <= 0
return an empty page (or clamp limit to at least 1) prior to calculating start =
max(eligible.startIndex, eligible.endIndex - limit) and before the while that
accesses cache[start] and cache[start-1]; this prevents start from equaling
eligible.endIndex and avoids out-of-bounds access when reading cache[start].
Update the method in AgentChatTranscriptTailer (variables: limit, start,
eligible, cache, seq) to perform the check/early-return or clamp immediately and
only then run the boundary-extension loop.
Adversarial re-check proved two round-1 fixes broken; both fixed with regression tests: window dedup now spans the full window (suffix(64) double-inserted when a tailer drain exceeded it), and ended-session tailer eviction no longer resurrects (eviction on state transition only, ended guard before eager tailing, failedResolutions retried on SessionStart/UserPromptSubmit instead of starving live push). Echo reconcile now matches Claude Code's bracketed-paste placeholder (the common multi-line send) and attachment-only echoes. Reconnects back off exponentially (0.5s..16s, injected cancellable sleep) instead of spinning a dead connection, and client unsubscribe is skipped when the connection itself died. Wire forward-compat fails open end to end: unknown nested enum values degrade one message to the unsupported row instead of throwing the page away, unknown roles/timestamps default, unknown event names decode to an explicit ignorable case. Golden-fixture tests pin all three. Perf: hook-store file reads throttled to one per session per 30s (was every hook event on the main actor), descriptorChanged pushed only when the descriptor changed beyond lastActivityAt (was every pre/postToolUse to every phone), registry computes state deltas by comparing previous records instead of hand-maintained flags, pid sweep treats EPERM as alive. Product: long-press Copy on prose bubbles, bubble width capped at the theme fraction via one list-level geometry read, tail-follow keys on row count so a pinned failed-send row can't freeze auto-scroll, composer drafts survive cover dismissal (host-owned binding), and the terminal escape hatch now selects the session's actual terminal surface instead of just dismissing. 67+14 package tests green. Codex independently verified the live pipeline end to end (append-to-emit 255ms). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ChatPendingOutbound carries the attachment payloads so a failed send's retry resends images instead of silently dropping them. New bounded ChatContentCache (sibling to the markdown renderer, environment- injected) caches ANSI-sanitized terminal lines and prose segment splits per message, removing per-materialization sanitize/split work from the scroll path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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)
Sources/TerminalController+MobileChat.swift (1)
125-128: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueConsider splitting validation for clearer error diagnostics.
The guard combines missing
session_idand out-of-rangeoption_indexinto a single error message "Missing session_id or option_index". Whenoption_indexis provided but outside 0–8, the "Missing" wording is misleading for API consumers debugging integration issues.💡 Suggested improvement
func v2MobileChatAnswer(params: [String: Any]) -> V2CallResult { - guard let sessionID = v2RawString(params, "session_id"), - let optionIndex = v2Int(params, "option_index"), optionIndex >= 0, optionIndex < 9 else { - return .err(code: "invalid_params", message: "Missing session_id or option_index", data: nil) + guard let sessionID = v2RawString(params, "session_id") else { + return .err(code: "invalid_params", message: "Missing session_id", data: nil) + } + guard let optionIndex = v2Int(params, "option_index"), optionIndex >= 0, optionIndex < 9 else { + return .err(code: "invalid_params", message: "Missing or invalid option_index (0-8)", data: 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 `@Sources/TerminalController`+MobileChat.swift around lines 125 - 128, In v2MobileChatAnswer, split the combined guard into separate validations so consumers get precise errors: first use v2RawString to validate session_id and return .err with a "missing_session_id" message if absent, then use v2Int to validate option_index and separately check its range (0–8) and return .err with an "invalid_option_index" or "option_index_out_of_range" message if it's missing or outside range; update the error messages to clearly reference the offending field (session_id or option_index) so downstream callers can diagnose issues.
🤖 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 `@Sources/TerminalController.swift`:
- Line 5133: The fire-and-forget Task calling
AgentChatTranscriptService.shared.noteHookEvent(event) lacks error handling;
change the Task block to await the throwing call inside a do/catch on the
`@MainActor` and log or report any caught errors (e.g., via a logger or OSLog) so
failures in noteHookEvent are visible; ensure you still run the call on
`@MainActor` and preserve the original fire-and-forget behavior while adding the
do/catch around the await to surface diagnostics.
---
Outside diff comments:
In `@Sources/TerminalController`+MobileChat.swift:
- Around line 125-128: In v2MobileChatAnswer, split the combined guard into
separate validations so consumers get precise errors: first use v2RawString to
validate session_id and return .err with a "missing_session_id" message if
absent, then use v2Int to validate option_index and separately check its range
(0–8) and return .err with an "invalid_option_index" or
"option_index_out_of_range" message if it's missing or outside range; update the
error messages to clearly reference the offending field (session_id or
option_index) so downstream callers can diagnose issues.
🪄 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: 08cd531d-30f2-4310-ab37-877a165c74c9
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (3)
Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftSources/TerminalController+MobileChat.swiftSources/TerminalController.swift
|
|
||
| CmuxEventBus.shared.publishWorkstreamEvent(event, phase: "received") | ||
| v2ApplyIMessageModeSideEffects(for: event) | ||
| Task { @MainActor in AgentChatTranscriptService.shared.noteHookEvent(event) } |
There was a problem hiding this comment.
Add error handling to the fire-and-forget Task.
The Task spawned to record hook events has no error handling. If noteHookEvent throws or the Task crashes, the failure will be silent, potentially causing transcript data loss without any diagnostic signal.
🛡️ Suggested fix to add error handling
-Task { `@MainActor` in AgentChatTranscriptService.shared.noteHookEvent(event) }
+Task { `@MainActor` in
+ do {
+ AgentChatTranscriptService.shared.noteHookEvent(event)
+ } catch {
+ logger.error("Failed to record hook event for agent chat: \(error)")
+ }
+}🤖 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 `@Sources/TerminalController.swift` at line 5133, The fire-and-forget Task
calling AgentChatTranscriptService.shared.noteHookEvent(event) lacks error
handling; change the Task block to await the throwing call inside a do/catch on
the `@MainActor` and log or report any caught errors (e.g., via a logger or OSLog)
so failures in noteHookEvent are visible; ensure you still run the call on
`@MainActor` and preserve the original fire-and-forget behavior while adding the
do/catch around the await to surface diagnostics.
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)
Sources/Mobile/AgentChat/AgentChatTranscriptService.swift (1)
99-110:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid recreating persistent tailers for already-ended sessions.
Lines 99-105 recreate and start a tailer from
history(), but Lines 156-163 only evict on the transition to.ended. Once a user opens an ended chat, that new watcher/cache stays resident indefinitely because there is no second.endedtransition to clean it up.💡 Minimal fix direction
func history(sessionID: String, beforeSeq: Int?, limit: Int) async -> ChatHistoryPage? { guard let record = registry.record(sessionID: sessionID) else { return nil } failedResolutions.remove(sessionID) guard let tailer = ensureTailer(for: record) else { return nil } await tailer.start() let page = await tailer.history(beforeSeq: beforeSeq, limit: limit) if record.title == nil, let title = await tailer.title { registry.update(sessionID: sessionID) { $0.title = title } } + if record.state == .ended { + tailers.removeValue(forKey: sessionID) + Task { await tailer.stop() } + } return page }Also applies to: 116-140, 154-163
🤖 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 `@Sources/Mobile/AgentChat/AgentChatTranscriptService.swift` around lines 99 - 110, history(sessionID:) currently calls ensureTailer(for:) and starts a tailer even when the session is already in .ended state, causing a persistent watcher that never gets evicted (eviction only happens on state transitions). Change history(sessionID:) to first check the session record's state and avoid creating/starting a tailer for sessions whose record.state == .ended (or if a tailer already exists, do not create a new one); instead return the stored history/title directly. Use registry.record(sessionID:), ensureTailer(for:), and tailer.start() as anchor points to implement this guard so ended sessions never spawn new persistent tailers and existing eviction logic remains correct.
♻️ Duplicate comments (1)
Packages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Composer/ChatComposerView.swift (1)
317-323:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove attachments by stable picker identity, not shared index.
Line 320 removes from
pickedItemsusing theattachmentsindex, but Lines 329-333 buildattachmentsthrough a filtered pass. Indices diverge when any picker item fails decoding, so the wrong picker item can be removed and the deleted attachment can reappear.Suggested fix
private func removeAttachment(id: String) { - guard let index = attachments.firstIndex(where: { $0.id == id }) else { return } - attachments.remove(at: index) - if pickedItems.indices.contains(index) { - pickedItems.remove(at: index) - } + guard let attachmentIndex = attachments.firstIndex(where: { $0.id == id }) else { return } + let removed = attachments.remove(at: attachmentIndex) + + if let pickedIndex = pickedItems.firstIndex(where: { $0.itemIdentifier == removed.id }) { + pickedItems.remove(at: pickedIndex) + } else if removed.id.hasPrefix("picked-"), + let rawIndex = Int(removed.id.dropFirst("picked-".count)), + pickedItems.indices.contains(rawIndex) { + pickedItems.remove(at: rawIndex) + } }Also applies to: 329-337
🤖 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/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Composer/ChatComposerView.swift` around lines 317 - 323, The removeAttachment method (and the similar block at lines ~329-337) currently removes from pickedItems by reusing the attachments index which can diverge when attachments were built via a filtered pass; instead locate and remove the corresponding picker entry by a stable identifier—e.g., call firstIndex(where:) on pickedItems matching the same stable id/identifier used for attachments (match on .id or the pickerItem.identifier), then remove that index if found; update both removeAttachment(id:) and the other deletion path to use this lookup rather than assuming shared indices.
🤖 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/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Markdown/ChatContentCache.swift`:
- Around line 35-56: The cache key currently uses messageID + content length
(the local variable key in both the lines block that uses output and in
proseSegments that uses text), which can return stale results when content
changes but length doesn't; change the key to uniquely represent the actual
content (e.g. include a content hash of output/text or the full content string
instead of count) for both the lines cache (lines, lineOrder, key using output)
and the proseSegments cache (segments, key using text); implement a stable hash
(SHA256 or similar) of the content and concatenate with messageID to form the
cache key so updates produce new entries and avoid stale render output.
---
Outside diff comments:
In `@Sources/Mobile/AgentChat/AgentChatTranscriptService.swift`:
- Around line 99-110: history(sessionID:) currently calls ensureTailer(for:) and
starts a tailer even when the session is already in .ended state, causing a
persistent watcher that never gets evicted (eviction only happens on state
transitions). Change history(sessionID:) to first check the session record's
state and avoid creating/starting a tailer for sessions whose record.state ==
.ended (or if a tailer already exists, do not create a new one); instead return
the stored history/title directly. Use registry.record(sessionID:),
ensureTailer(for:), and tailer.start() as anchor points to implement this guard
so ended sessions never spawn new persistent tailers and existing eviction logic
remains correct.
---
Duplicate comments:
In
`@Packages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Composer/ChatComposerView.swift`:
- Around line 317-323: The removeAttachment method (and the similar block at
lines ~329-337) currently removes from pickedItems by reusing the attachments
index which can diverge when attachments were built via a filtered pass; instead
locate and remove the corresponding picker entry by a stable identifier—e.g.,
call firstIndex(where:) on pickedItems matching the same stable id/identifier
used for attachments (match on .id or the pickerItem.identifier), then remove
that index if found; update both removeAttachment(id:) and the other deletion
path to use this lookup rather than assuming shared indices.
🪄 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: d508b2b4-cef3-40d1-adeb-e11b391bd0ab
📒 Files selected for processing (21)
Packages/CmuxAgentChat/Sources/CmuxAgentChat/Model/ChatMessage.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Model/ChatMessageKind.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Source/FixtureChatEventSource.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatConversationStore.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatPendingOutbound.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Wire/ChatSessionEvent.swiftPackages/CmuxAgentChat/Tests/CmuxAgentChatTests/ChatConversationStoreTests.swiftPackages/CmuxAgentChat/Tests/CmuxAgentChatTests/ChatMessageCodableTests.swiftPackages/CmuxAgentChat/Tests/CmuxAgentChatTests/ChatTranscriptProjectorTests.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Composer/ChatComposerView.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Markdown/ChatContentCache.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Previews/ChatScreenPreviews.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Screen/ChatScreen.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Theme/ChatBubbleMaxWidthKey.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/ChatTranscriptListView.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/Rows/ChatProseBubbleView.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/Rows/ChatTerminalCardView.swiftPackages/CmuxMobileShell/Sources/CmuxMobileShell/MobileChatEventSource.swiftPackages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceAgentChatButton.swiftSources/Mobile/AgentChat/AgentChatSessionRegistry.swiftSources/Mobile/AgentChat/AgentChatTranscriptService.swift
The round-3 adversarial pass proved the attachment reconcile was fixture-only dead code (real echoes are pasted image paths in prose) and that a resumed session's stale pid made the liveness sweep re-end a live session. Fixes: reconcile matches the real echo shapes (lone path line for attachment-only sends, path-prefixed suffix for text+image, paste placeholder also covers long single-line prompts) and never consumes a failed send's retry row; SessionStart clears the recorded pid and consult throttle so a claude --resume re-reads the new process; a live append whose seq regresses below the window tail re-anchors the window (transcript truncated and rewritten) instead of corrupting order; a resync whose entire newest page is beyond the window tail replaces the window instead of leaving a silent gap; reconnect backoff only resets after a stream that was genuinely healthy (event plus five seconds alive), so one pushed frame per flap can't sustain a spin; ChatContentCache keys by content hash (length collided at the 16,385-char budget clamp). Three new regression tests (70 total). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collaborator
Author
|
Iteration log since the PR opened (multi-agent review loop, each round = specialist reviews → fixes → adversarial re-verification → live E2E re-check): Round 1 (5a39f3e): conventions, correctness, and UX reviews. Fixed the silent event-loss gap between history fetch and subscribe, eternal typing indicator from dead agent processes, session-ended composer state, double-tap hazards on permission cards, tailer leaks, sidechain transcript pollution, and attachment-only sends never submitting. Round 2 (39ec981): an adversarial pass proved two round-1 fixes broken and both were refixed with regression tests (window dedup spanned only a 64-message suffix and double-inserted under tool storms; ended-session tailer eviction was instantly resurrected by the same hook event). Also: wire forward-compat now fails open end to end with golden-fixture tests, reconnects back off exponentially, hook-store file reads throttled off the per-event path, descriptor pushes coalesced, drafts survive cover dismissal, the terminal escape hatch lands on the session's actual surface. Round 3 (728cc61): a second adversarial pass found the attachment reconcile matched only fixture-shaped echoes (real echoes are pasted image paths in prose) and that claude --resume's reused session id kept a stale pid that flapped live sessions back to ended. Both fixed, plus seq-regression re-anchoring after transcript rewrites, gap-safe resync after long disconnects, and failed sends' retry rows are never consumed by other echoes. Functionality was independently re-verified each round by a codex agent driving the tagged mac build over its debug socket: package suites green (now 70+14), live append-to-push latency ~250ms, subscription handshake present, no emit storms. 🤖 Generated with Claude Code |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
Sources/Mobile/AgentChat/AgentChatSessionRegistry.swift (2)
45-45:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGuard PID probes to positive values before calling
kill(..., 0).
kill(pid_t(0), 0)sends signal 0 to all processes in the caller's process group, and negative PIDs have special semantics. If a record or entry somehow haspid = 0or negative, the liveness check could incorrectly report sessions as alive.Suggested guard
private func sweepDeadProcesses() { for (sessionID, record) in records { - guard record.state != .ended, let pid = record.pid else { continue } + guard record.state != .ended, let pid = record.pid, pid > 0 else { continue } if kill(pid_t(pid), 0) != 0, errno == ESRCH { // And in seedFromHookStores: -let alive = entry.pid.map { kill(pid_t($0), 0) == 0 } ?? false +let alive = entry.pid.map { $0 > 0 && kill(pid_t($0), 0) == 0 } ?? falseAlso applies to: 89-89
🤖 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 `@Sources/Mobile/AgentChat/AgentChatSessionRegistry.swift` at line 45, The liveness check currently calls kill(..., 0) without ensuring the pid is positive; update the guard in the loop that uses record.pid (the line with "guard record.state != .ended, let pid = record.pid else { continue }") to only proceed if pid > 0 before calling kill(pid, 0), and make the same change at the other occurrence around the code handling pid checks (the second occurrence noted near line 89); this ensures you skip zero or negative PIDs and only perform the signal-0 probe on valid positive pid_t values.
9-9:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftSession identity is not namespaced by agent source, so records can collide.
The
recordsdictionary keys sessions only bysessionID, butnormalizedSessionIDstrips the"<source>-"prefix. If two different agent runtimes (e.g., Claude and Codex) produce the same raw session ID, they would collide and overwrite each other.🤖 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 `@Sources/Mobile/AgentChat/AgentChatSessionRegistry.swift` at line 9, The records dictionary in AgentChatSessionRegistry uses only sessionID (via normalizedSessionID) which allows collisions across agent runtimes; change the storage key to include the agent source (e.g., composite key "<source>|<normalizedSessionID>" or use the original raw sessionID that preserves the "<source>-" prefix) and update all accesses (lookups, inserts, removals) that reference records, normalizedSessionID, or sessionID to build and use that namespaced key consistently; ensure methods like the initializer/creator, get/update/remove functions, and any uses of AgentChatSessionRecord construction reference the new key convention so sessions from different agent runtimes cannot overwrite each other.
🤖 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/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Markdown/ChatContentCache.swift`:
- Line 35: The cache key currently uses String.hashValue (let key =
"\(messageID)-\(output.hashValue)"), which can collide; replace this with a
stable cryptographic content hash: add an import of CryptoKit and implement a
helper like contentKey(messageID:content:) that computes a SHA256 digest of the
content, take a truncated hex prefix (e.g. first 8 bytes -> 16 hex chars) and
return "\(messageID)-\(prefix)"; use this helper wherever the current key is
built (references: the key generation site and the other occurrence at the same
file) to eliminate hashValue collisions while preserving the messageID prefix.
---
Duplicate comments:
In `@Sources/Mobile/AgentChat/AgentChatSessionRegistry.swift`:
- Line 45: The liveness check currently calls kill(..., 0) without ensuring the
pid is positive; update the guard in the loop that uses record.pid (the line
with "guard record.state != .ended, let pid = record.pid else { continue }") to
only proceed if pid > 0 before calling kill(pid, 0), and make the same change at
the other occurrence around the code handling pid checks (the second occurrence
noted near line 89); this ensures you skip zero or negative PIDs and only
perform the signal-0 probe on valid positive pid_t values.
- Line 9: The records dictionary in AgentChatSessionRegistry uses only sessionID
(via normalizedSessionID) which allows collisions across agent runtimes; change
the storage key to include the agent source (e.g., composite key
"<source>|<normalizedSessionID>" or use the original raw sessionID that
preserves the "<source>-" prefix) and update all accesses (lookups, inserts,
removals) that reference records, normalizedSessionID, or sessionID to build and
use that namespaced key consistently; ensure methods like the
initializer/creator, get/update/remove functions, and any uses of
AgentChatSessionRecord construction reference the new key convention so sessions
from different agent runtimes cannot overwrite each other.
🪄 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: 89166920-b873-4966-bfa1-62816d31b8ba
📒 Files selected for processing (4)
Packages/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatConversationStore.swiftPackages/CmuxAgentChat/Tests/CmuxAgentChatTests/ChatConversationStoreTests.swiftPackages/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Markdown/ChatContentCache.swiftSources/Mobile/AgentChat/AgentChatSessionRegistry.swift
| /// - output: The raw captured output. | ||
| /// - Returns: Display lines. | ||
| public func sanitizedLines(messageID: String, output: String) -> [String] { | ||
| let key = "\(messageID)-\(output.hashValue)" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Hash collisions can still return stale content for different messages.
The switch from .count to .hashValue improves uniqueness but String.hashValue can collide—two different strings may produce the same hash. Combined with messageID, a collision requires both the same message ID and hash collision, which is rare but possible during streaming updates where the same message ID receives different content.
A content-addressing approach using a cryptographic hash (e.g., SHA256 truncated to 16 hex chars) would eliminate this theoretical risk, though the practical impact is low given the bounded cache size and short entry lifetimes.
Alternative using stable content hash
import CryptoKit
private func contentKey(messageID: String, content: String) -> String {
let digest = SHA256.hash(data: Data(content.utf8))
let prefix = digest.prefix(8).map { String(format: "%02x", $0) }.joined()
return "\(messageID)-\(prefix)"
}Also applies to: 55-55
🤖 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/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Markdown/ChatContentCache.swift`
at line 35, The cache key currently uses String.hashValue (let key =
"\(messageID)-\(output.hashValue)"), which can collide; replace this with a
stable cryptographic content hash: add an import of CryptoKit and implement a
helper like contentKey(messageID:content:) that computes a SHA256 digest of the
content, take a truncated hex prefix (e.g. first 8 bytes -> 16 hex chars) and
return "\(messageID)-\(prefix)"; use this helper wherever the current key is
built (references: the key generation site and the other occurrence at the same
file) to eliminate hashValue collisions while preserving the messageID prefix.
The round-4 adversarial pass proved the round-3 pid fix self-defeating (the hooks CLI posts the SessionStart event before rewriting the hook store, so the same-event consult re-adopted the dead pid permanently): the registry now takes the new pid from the event's own ppid (hooks are spawned by the agent process) and suppresses that event's store consult. Transcript truncation now pushes an explicit reset frame from the tailer (codex transcripts reuse line-N ids after rewrites, which defeated the client-side seq/id heuristic); the store clears its window on reset and re-anchors from history, keeping the heuristic as a fallback for older Macs. Reconcile became two-pass (exact text match across all pendings beats shape heuristics on older ones, so a slash-command echo can't eat an attachment-only pending) with tightened shapes (clipboard-path for attachment-only, path prefix required for suffix matches). Reconnect health is stream lifetime alone, so an idle session's quiet stream no longer ratchets to a permanent 16s backoff. The failed-pending regression test was proven vacuous and rewritten to be load-bearing (mutation-checked: deleting the guard fails it); slash-command and reset tests added (72 total). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
Sources/Mobile/AgentChat/AgentChatSessionRegistry.swift (2)
45-49:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winOnly probe positive PIDs for liveness.
kill(pid, 0)on0or negative values has special process-group semantics. A bad stored PID can make unrelated processes look alive/dead here, so these probes need apid > 0guard before callingkill.Suggested fix
- guard record.state != .ended, let pid = record.pid else { continue } + guard record.state != .ended, let pid = record.pid, pid > 0 else { continue } ... - let alive = entry.pid.map { kill(pid_t($0), 0) == 0 } ?? false + let alive = entry.pid.map { $0 > 0 && kill(pid_t($0), 0) == 0 } ?? falseAlso applies to: 89-90
🤖 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 `@Sources/Mobile/AgentChat/AgentChatSessionRegistry.swift` around lines 45 - 49, The PID liveness probe calls kill(pid_t(pid), 0) without ensuring pid is positive; add a pid > 0 check to avoid special semantics for 0/negative PIDs. Update the guard that extracts record.pid (used with record.state and the kill(pid_t(pid), 0) call) to require pid > 0 (e.g., guard record.state != .ended, let pid = record.pid, pid > 0 else { continue }) and apply the same change to the other probe site referenced around the kill call (the similar code at lines 89-90) so only positive PIDs are probed before calling kill.
9-19:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftNamespace chat session identity by agent source before storing or routing it.
recordsandhookStoreConsultedAtare still keyed only by normalizedsessionID. If Claude and Codex ever emit the same raw id, one record overwrites the other, and downstream consumers will cross-wire history/pushes for the wrong conversation. This needs one composite identity carried through the registry, transcript service, and wire/session lookup path.Also applies to: 84-90, 112-166
🤖 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 `@Sources/Mobile/AgentChat/AgentChatSessionRegistry.swift` around lines 9 - 19, records and hookStoreConsultedAt are currently keyed only by normalized sessionID which allows collisions across different agent sources; change their keys (and any lookups/insertions in the registry and related methods referenced in the diff region around the registry, transcript service, and wire/session lookup code) to use a composite identity that includes agent source + normalized sessionID (e.g., a small CompositeSessionID value or a concatenated string "source|sessionID"), update all places that index, fetch, remove or consult hookStoreConsultedAt and records (including session creation/route, transcript storage, and any session-lookup helpers) to build and use this composite key, and ensure serialization/persistence and interop points pass and accept the composite identifier so records cannot be overwritten across agents (preserve existing APIs like onRecordChanged but use composite keys internally).Sources/Mobile/AgentChat/AgentChatTranscriptTailer.swift (1)
104-116:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClamp or reject non-positive history limits before touching
cache[start].
AgentChatTranscriptService.history(...)forwardslimitstraight through. Whenlimit <= 0,startlands at/aftereligible.endIndex, and the equal-seq boundary loop readscache[start], which traps.Suggested fix
func history(beforeSeq: Int?, limit: Int) -> ChatHistoryPage { + let safeLimit = max(limit, 1) let eligible: ArraySlice<ChatMessage> if let beforeSeq { let end = cache.firstIndex { $0.seq >= beforeSeq } ?? cache.endIndex eligible = cache[..<end] } else { eligible = cache[...] } - var start = max(eligible.startIndex, eligible.endIndex - limit) + var start = max(eligible.startIndex, eligible.endIndex - safeLimit)🤖 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 `@Sources/Mobile/AgentChat/AgentChatTranscriptTailer.swift` around lines 104 - 116, The history(beforeSeq:limit:) function can index cache[start] when limit <= 0; validate the incoming limit near the top of history (in AgentChatTranscriptTailer.history or any callering AgentChatTranscriptService forwarding it) and either clamp it to a minimum of 1 or return an empty ChatHistoryPage immediately; then compute eligible, start and run the equal-seq adjustment loop safely. Ensure you reference history(beforeSeq:limit:) and the start/indexing logic so the guard happens before any use of cache[start] or cache[endIndex].
🤖 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/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatConversationStore.swift`:
- Around line 359-370: In the .reset branch clear the cache-head truncation flag
so stale UI state isn't preserved: inside the reset handling (where messages =
[], pending... , hasMoreHistory = false, reproject(), and Task { await
resyncTail() }) set historyTruncatedAtHead = false before calling
reproject()/resyncTail() so that after resyncTail() reloads the transcript the
store no longer reports that history was truncated at the Mac cache head.
In `@Sources/Mobile/AgentChat/AgentChatTranscriptTailer.swift`:
- Around line 183-195: The reset branch in AgentChatTranscriptTailer clears
offsets and cache but forgets to clear the title-discovery state, leaving
reportedTitle stuck; update this branch to reset reportedTitle (and any related
discovery state if present) before calling loadInitialTail() and before emitting
the reset batch via onBatch so a new discoveredTitle can be produced and
published after the transcript rewrite.
---
Duplicate comments:
In `@Sources/Mobile/AgentChat/AgentChatSessionRegistry.swift`:
- Around line 45-49: The PID liveness probe calls kill(pid_t(pid), 0) without
ensuring pid is positive; add a pid > 0 check to avoid special semantics for
0/negative PIDs. Update the guard that extracts record.pid (used with
record.state and the kill(pid_t(pid), 0) call) to require pid > 0 (e.g., guard
record.state != .ended, let pid = record.pid, pid > 0 else { continue }) and
apply the same change to the other probe site referenced around the kill call
(the similar code at lines 89-90) so only positive PIDs are probed before
calling kill.
- Around line 9-19: records and hookStoreConsultedAt are currently keyed only by
normalized sessionID which allows collisions across different agent sources;
change their keys (and any lookups/insertions in the registry and related
methods referenced in the diff region around the registry, transcript service,
and wire/session lookup code) to use a composite identity that includes agent
source + normalized sessionID (e.g., a small CompositeSessionID value or a
concatenated string "source|sessionID"), update all places that index, fetch,
remove or consult hookStoreConsultedAt and records (including session
creation/route, transcript storage, and any session-lookup helpers) to build and
use this composite key, and ensure serialization/persistence and interop points
pass and accept the composite identifier so records cannot be overwritten across
agents (preserve existing APIs like onRecordChanged but use composite keys
internally).
In `@Sources/Mobile/AgentChat/AgentChatTranscriptTailer.swift`:
- Around line 104-116: The history(beforeSeq:limit:) function can index
cache[start] when limit <= 0; validate the incoming limit near the top of
history (in AgentChatTranscriptTailer.history or any callering
AgentChatTranscriptService forwarding it) and either clamp it to a minimum of 1
or return an empty ChatHistoryPage immediately; then compute eligible, start and
run the equal-seq adjustment loop safely. Ensure you reference
history(beforeSeq:limit:) and the start/indexing logic so the guard happens
before any use of cache[start] or cache[endIndex].
🪄 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: 9d6fd398-312a-4c16-baa0-1cb0768a4f52
📒 Files selected for processing (8)
Packages/CmuxAgentChat/Sources/CmuxAgentChat/Source/FixtureChatEventSource.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatConversationStore.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatPendingOutbound.swiftPackages/CmuxAgentChat/Sources/CmuxAgentChat/Wire/ChatSessionEvent.swiftPackages/CmuxAgentChat/Tests/CmuxAgentChatTests/ChatConversationStoreTests.swiftSources/Mobile/AgentChat/AgentChatSessionRegistry.swiftSources/Mobile/AgentChat/AgentChatTranscriptService.swiftSources/Mobile/AgentChat/AgentChatTranscriptTailer.swift
# Conflicts: # .github/swift-file-length-budget.tsv # Resources/Localizable.xcstrings # Sources/TerminalController.swift # Sources/Workspace.swift # cmux.xcodeproj/project.pbxproj # ios/cmux/Resources/Localizable.xcstrings # scripts/lint-ios-package-conventions.sh
# Conflicts: # .github/swift-file-length-budget.tsv # Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swift
austinywang
added a commit
that referenced
this pull request
Jun 16, 2026
Reverts the diagnostic step added earlier to investigate the activation-session socket timeout. That check is a pre-existing, main-wide failure (since #5984), not introduced by this PR; the diagnostic captured stale runner logs and is not needed here. Keeps this PR's workflow identical to main. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Merged
This was referenced Jun 26, 2026
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A native chat UI for AI agent sessions on iOS, rendered from the agent's transcript JSONL instead of the terminal grid. Workspaces with a chat-capable session get an Agent Chat toolbar button that opens the conversation as Telegram-style bubbles: user prompts outgoing, agent prose incoming with markdown, terminal/diff cards, collapsed thought and tool rows, actionable permission and question cards with frozen receipts, a typing indicator driven by hook state, and a composer with photo attachments, Esc/Ctrl-C chips, and a stop button.
Architecture, mac side: AgentChatTranscriptService (new, Sources/Mobile/AgentChat/) tracks sessions from agent hook events plus the ~/.cmuxterm hook session stores, resolves and tails each session's transcript with CmuxFileWatch (bounded backfill, incremental parse, truncation recovery, pid liveness sweeps), serves mobile.chat.sessions/history RPCs, and pushes chat.message frames to subscribed phones. Send, interrupt, and answer reuse the existing mobile terminal injection paths (bracketed paste plus submit key), so chat input behaves exactly like composer input.
Architecture, iOS side: two new packages. CmuxAgentChat is platform-free domain: wire DTOs, claude and codex JSONL parsers (fail-open: unknown line types become an unsupported row, never dropped), ChatConversationStore (seq-windowed transcript, optimistic pending sends reconciled against transcript echoes, subscribe-before-fetch so no event falls in the handshake gap), and a scripted FixtureChatEventSource. CmuxAgentChatUI is the SwiftUI surface, dark-first with adaptive light mode (terminal cards stay dark in both schemes). 99 package tests cover parsers, projector, store, ANSI sanitizing, and wire coding.
Verified end to end on simulator against a tagged mac build with a live claude session: pairing, session listing, history, live push (transcript append renders on the phone in about 200 ms), and phone-to-terminal send. A DEBUG-only demo (Settings > Developer > Agent Chat Demo) renders the full taxonomy from fixtures.
Known gaps, deliberate for v1: history is bounded to the newest 2000 transcript lines (the UI shows "Earlier history is on your Mac" past that); codex sessions need hook-launched terminals for the surface binding; answer() injects the option's number key, which matches claude's TUI prompts; permission cards are fixture-only until the hook wire carries permission requests. Follow-ups filed from review that are not in this PR: wire enum forward-compat (unknown raw values currently drop a frame), Dynamic Type and VoiceOver sweeps, scroll-anchor preservation on history prepend, per-event hook ordering via AsyncStream, retry-with-attachments.
🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Adds a Telegram‑style chat on iOS for coding‑agent sessions with push‑driven transcripts, block‑level markdown, and a photo‑aware composer. Scope is agent‑only; terminal/diff output renders as cards inside agent transcripts, and chat toggles in place on the current workspace tab (session list updates via push, no polling).
New Features
CmuxAgentChat(typed message model with fail‑open wire coding, Claude/Codex JSONL parsers, OSC‑133 terminal blocks, ISO‑8601 viaChatWireCoding,ChatConversationStorewith optimistic/queued/cancelable sends that flush one per idle turn,ChatSessionListReducer) andCmuxAgentChatUI(bubble UI with block‑level markdown and code‑language labels, terminal/diff cards, actions, typing indicator, composer with photo attachments and real pending thumbnails, bounded markdown/content/diff caches, en+ja localization, DEBUG demo and scroll‑geometry overlay).MobileChatEventSource(no polling). The opened session is pinned while chat is on; the in‑place toggle binds to the visible workspace tab. Ended sessions are read‑only. Header shows workspace/tab with a leading state symbol.Bug Fixes
/tmp↔/private/tmpmatching; same‑directory agents disambiguated (skip transcripts already claimed);$HOMEskipped; inode‑based rotation detection with explicit reset frames; strict pin prevents wrong‑session switches; spinner‑forever on initial load replaced by a retry path.Written for commit 75274f5. Summary will update on new commits.
Summary by CodeRabbit
New Features
UI
Parsing & Reliability
Tests
Chores