iOS: iMessage-style terminal composer (open by default, inline send, per-terminal drafts) - #5876
Merged
Merged
Conversation
… composer view Model layer for the iMessage-style terminal composer, ported from the dogfood branch (feat-ios-dog-unified) onto main: - ComposerDockState/ComposerDockIntent: pure reducer mapping a compose-button tap onto open/reveal-and-focus/close so a hidden-or-unfocused composer is never dismissed (the draft-loss fix), with unit tests. - MobileShellComposite: isComposerPresented + composerFocusRequest tokens, submitComposerInput() via the new terminal.paste RPC (bracketed paste + single submit, cleared only on ack), per-terminal drafts behind a TerminalDraftStoring seam with an in-memory store (disk persistence lands separately), and sign-out draft wipe. - DiagnosticEventCode 18-24: composer instrumentation events (DEBUG). - InputResponderIdentity + terminal.paste marked session-scoped in MobileCoreRPCClient. - TerminalComposerView: glass composer band UI + glass field/circle styles. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds the v2MobileTerminalPaste handler the iOS composer submits through: sendText bracketed-pastes the composed block (so interior newlines stay literal) and a single named submit key commits it once, mirroring the macOS TextBox composer dispatch. Claude Code surfaces upgrade a multi-line return intent to ctrl+enter via TextBoxAgentDetection (now internal). Registered in the socket switch, v2Capabilities, mobileHostHandleRPC, and the mobile ticket authorization gate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… toggle The surface (GhosttySurfaceView) owns the whole bottom dock — terminal grid / composer band / accessory toolbar / keyboard — in one coordinate system: - Composer band: a surface-owned container the host (GhosttySurfaceRepresentable) mounts the SwiftUI compose field into via UIHostingController; the host measures the field with sizeThatFits and the surface reserves exactly that height above the toolbar, so a field-grow pushes only the terminal up (animated reflow on the keyboard curve, symmetric close that unmounts the field only after the band collapses). - Toolbar always visible: rides the keyboard top when up and the home indicator when down; never reparented, so its buttons cannot disappear. A pinned HIDE button suppresses all bottom chrome until the next terminal tap. - Compose toggle pinned in the bar container, outside the scrollable button row, so a persisted scroll offset can never carry it off-screen. - Compose-button taps resolve through the pure ComposerDockState reducer (open / reveal-and-focus / close), fixing the compose-hide-reveal-compose draft loss; reveal paths re-focus the composer field via the store's focus token. - DEBUG-only dock/store accessibility probes + diagnostic events for UI tests. - en+ja strings for the composer and HIDE controls; in-memory per-terminal draft store injected at the composition root. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…val) Four XCUITests against the DEBUG dock/store probes: repeated open/close cycles resolve to a genuine close (not a stuck reveal), the draft survives the compose-hide-reveal-compose cycle, the compose button stays hittable and on-screen after typing and after a hide-reveal reflow, and a rapid double toggle settles with surface and store in agreement. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two races found in review: - A close-then-quick-reopen could unmount the freshly remounted compose field: UIKit runs animation completions even when interrupted, and the close path unconditionally unmounted in its completion. The completion is now generation-guarded (and re-checks mounted state) so only the latest close transition unmounts. - A successful send did not clear the submitted terminal's STORED draft when the user switched terminals while the ack was in flight (the switch persists the outgoing text under the submitted terminal's key), so already-sent text resurrected on switch-back and invited a duplicate submission. The post-ack step now captures the submitted terminal id and clears that stored draft iff it still equals the sent text, preserving anything newer; extracted as reconcileComposerDraftAfterSend with unit tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Draft persistence was fire-and-forget via independent unstructured Tasks, which are not ordered relative to each other: a keystroke save that suspended inside the store actor could apply after a newer save, after the post-send clear, or after the sign-out wipe, resurrecting stale text or leaking one account's unsent draft into the next session. All draft-store operations now chain onto a single FIFO tail task (enqueueDraftOperation), so effects apply in exactly the order they were issued from the main actor. The post-send check-then-clear runs as one enqueued unit, making it atomic with respect to the terminal switch's own save. drainDraftOperationsForTesting() exposes the tail as a test seam; ordering tests use a store whose first save suspends long enough that unordered tasks would invert. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two field-ownership races in the terminal-switch draft swap: - A fast A -> B -> C switch saves the transient cleared placeholder as B's draft before B's stored draft has loaded into the field, erasing B's real draft even though the user never edited B. - Text typed and deleted during a draft load's in-flight window is resurrected when the late load applies into the (deliberately) empty field. Tests only; the fix lands in the next commit so CI proves they catch the bug. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The terminal-switch swap unconditionally saved the visible field as the outgoing terminal's draft. During a fast A -> B -> C switch the field is still the transient cleared placeholder (B's stored draft has not loaded yet), so the B -> C save wrote that placeholder over B's real draft, deleting it even though the user never edited B. swapDraft now records the incoming terminal in draftLoadPendingTerminalID and treats the field as authoritative for the outgoing terminal only when no load is pending for it; the un-applied placeholder is never persisted. A user edit clears the marker (claiming ownership for the selected terminal), so edited fields are always saved, and applyLoadedDraft now consumes the marker as its primary guard, which also stops a late load from resurrecting text the user typed and deleted during the in-flight window. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The composer band now presents by default for every terminal, like iMessage showing its input bar in every conversation. isComposerPresented becomes a derived per-terminal state over a session-only dismissed set, so the chevron still hides it (remembered per terminal) and any other terminal stays open. Presented no longer implies focused: the field appears with the keyboard DOWN. Focus is a one-shot store handshake (composerFocusRequest + a pending flag consumed on the field's appear/onChange), armed only by an explicit open after a dismissal, the reveal-and-focus path, or a terminal switch while the field held first responder (mirrored via @focusstate so the keyboard hands over in place mid-compose instead of dropping). Default-open presentations never arm it, and the existing autofocus gate keeps the terminal input proxy from popping the keyboard under the open band. The send button moves INSIDE the field's rounded glass container (trailing, riding the last line via bottom alignment) as a filled accent circle with a white up-arrow, iMessage-style; it stays disabled/dimmed while the trimmed draft is empty and keeps the bracketed-paste submit semantics untouched. Draft ownership is untouched: the FIFO draft pipeline, field-ownership marker, and swap/load guards behave exactly as before; opening by default only makes the loaded draft visible sooner. Covered by 9 new ComposerDefaultOpenTests (119 total green) and updated composer UI-test baselines for the default-open dock. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
main removed Self.localizedConnectionError(for:) in favor of applyOperationalError(_:); update the terminal.paste error path to match.
trimmingCharacters(in:)/.whitespacesAndNewlines resolved only through transitive member visibility; make the Foundation dependency explicit.
…nder hidden chrome terminal.paste: once sendText has delivered the paste, a submit-key failure no longer surfaces as an RPC error (the client would keep the draft and re-paste a duplicate block on retry). Report ok with submitted:false plus a submit_error code instead, so the client clears the delivered draft and the text sits at the prompt awaiting a manual submit. chromeHidden grid: reserve only a live keyboard, not keyboardOccupancyInBounds, whose keyboard-down fallback is the bottom safe-area inset; the HIDE state now reclaims the home-indicator strip too, matching bottomDockFrames() pinning the dock to bounds.height.
With the composer open by default, the band is presented while the terminal's hidden input proxy (a sibling of composerContainer) can hold first responder. Gating keyboard dismissal on composerActive sent endEditing into a subtree that did not contain the responder, so the keyboard toggle no-opped and HIDE left the keyboard up. Both paths now resign whichever responder actually owns the keyboard.
…ppers panel.sendText / sendNamedKeyResult run resumeForExplicitInputIfNeeded() first, waking a hibernated agent terminal the way local typing does; the raw surface calls bypassed that wake.
…ponder probe Aziz policy pass: document the new public composer symbols (draft store witnesses, terminalInputText/selectedTerminalID, delegate defaults, the composer accessory case, safeAreaInsetsDidChange); split ComposerDockIntent, DelayingDraftStore, DraftSwitchOwnershipTests, ComposerDockProbeView, and ComposerStoreProbe into their own files; replace the CurrentResponderProbe namespace enum with a file-scope helper.
The focus request was a single global token + pending bit observed by every mounted TerminalComposerView. During a mid-compose terminal switch the OUTGOING composer is still mounted when the store bumps the token, so it could consume the pending bit and focus itself while being torn down, leaving the incoming composer unfocused (the keyboard handoff failed). The request now records its target terminal (the selected terminal at issue time) and consumption is keyed: a mismatched consume returns false and leaves the request armed for the incoming mount. Any switch that does not arm a new handshake invalidates a stale unconsumed one. Two new store tests pin the outgoing-steal and stale-request behaviors.
…terminal Each edit enqueued a FIFO task capturing the full draft snapshot; behind a slow draft store a typing burst retained every intermediate snapshot (memory grew as edits x draft size). Edits now overwrite a per-terminal latest-value entry and arm at most one flush task, which reads the entry at execution time, so a burst reaches the store as a bounded number of saves while barrier operations (switch save/load, post-send clear, sign-out wipe) keep their strict FIFO ordering. Regression test pins bounded save count and final-text-wins.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Author
|
@codex review |
|
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 an iMessage-style iOS terminal composer: intent/state contracts, per-terminal async draft persistence and FIFO ordering, SwiftUI composer hosted into the UIKit surface, bottom-dock/layout orchestration, terminal.paste transport with ack-gated reconciliation, DEBUG probes, and unit + UI tests. ChangesiMessage-style Terminal Composer
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
… probe The composer feature grows GhosttySurfaceView, MobileShellComposite, TerminalInputTextView, TerminalController, and MobileHostService past their checked-in budgets; refresh the budget accepting that growth (the bottom dock and draft pipeline live where their owners live). CurrentResponderProbe becomes an injectable struct: the CI package conventions lint rejects free functions while the local Aziz policy rejects caseless namespace enums; a value type with an instance method satisfies both.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 688d25744f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let composerFrame = CGRect(x: 0, y: max(0, composerTop), width: width, height: effectiveComposerHeight) | ||
| // Toolbar's reserved bottom is the composer's top (or the bottom edge with no | ||
| // composer), and its reserved top is one button-row band above that. | ||
| let toolbarBottom = effectiveComposerHeight > 0 ? composerTop : bottomEdge |
There was a problem hiding this comment.
When the composer grows to multiple lines, composerBandHeight increases and this line moves toolbarBottom up to composerTop, placing the accessory toolbar above the composer and shifting it upward with every field-height change. The rest of the dock contract says a composer grow should push only the terminal grid while the toolbar stays docked above the keyboard/home indicator; with this geometry the toolbar floats away from the keyboard and the composer sits below it instead.
Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment.
The geometry is intentional: the dock stack is terminal / toolbar / composer / keyboard, with the compose field nearest the keyboard (iMessage's layout) and the toolbar riding the band's top edge, which is the design that was dogfooded. The toolbar moving up as the field grows is the coherent behavior in that stack. The contradiction was in three stale doc comments (composerActive, composerContainer, composerBandHeight) describing the older composer-above-toolbar order; c0c22b53f updates them to describe bottomDockFrames() as implemented.
Contributor
Greptile SummaryLands the iMessage-style iOS terminal composer: a default-open, per-terminal composer band in a single surface-owned bottom dock, with inline send, per-terminal draft persistence through a FIFO pipeline, and a new
Confidence Score: 5/5Safe to merge; no data-loss, double-submit, or draft-erasure paths remain open after the FIFO pipeline, re-entrancy guard, terminal-keyed focus handshake, and partial-success terminal.paste ack. The change is large (~4000 lines across 34 files) but methodically hardened: every invariant named in the PR description has a corresponding test suite (dock reducer, default-open, draft ordering/coalescing, switch ownership, submit reconcile), the FIFO pipeline ordering is structurally guaranteed rather than relied on by timing, the sign-out wipe is enqueued last on the same pipeline, and the generation-guarded deferred-close animation cannot unmount a freshly remounted composer. Localization covers all new user-facing strings in both supported locales. No blocking primitives, no sleeps, and no unguarded mutable shared state were introduced. No files require special attention; the key risk paths (draft erasure on fast switch, double-paste on re-send, stale focus request consuming wrong composer view) all have corresponding unit tests that were explicitly called out in the PR description and commit history. Important Files Changed
Reviews (6): Last reviewed commit: "UI tests: account for the always-visible..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+1309
to
+1400
| // is visible above the strip. | ||
| toolbar.layer.zPosition = Self.bottomChromeZPosition | ||
| toolbar.clipsToBounds = false | ||
| updateDockedToolbarVisibility() | ||
| layoutBottomDock() | ||
| } | ||
|
|
||
| /// Layer `zPosition` for the bottom chrome (toolbar + composer band), placing it | ||
| /// above the Ghostty renderer's sublayer so a lifted Liquid-Glass button is not | ||
| /// clipped by the terminal render bounds (item 6). Below the zoom HUD (1100). | ||
| private static let bottomChromeZPosition: CGFloat = 1000 | ||
|
|
||
| /// Whether the always-visible bottom chrome (the docked accessory toolbar and, | ||
| /// when open, the composer band) is currently on screen. | ||
| /// | ||
| /// Round 8 makes the toolbar ALWAYS visible — terminal mode, composer mode, | ||
| /// keyboard up AND down — so the only thing that hides it is the explicit HIDE | ||
| /// button (``chromeHidden``). The toolbar is no longer keyboard-tied. When the | ||
| /// keyboard is down the toolbar (and any open composer) ride above the bottom | ||
| /// safe area instead of disappearing; see ``bottomChromeInset``. | ||
| private var dockedToolbarShouldBeVisible: Bool { | ||
| !chromeHidden | ||
| } | ||
|
|
||
| /// True while the HIDE button has temporarily suppressed the bottom chrome | ||
| /// (toolbar + composer band). The chrome reappears on the next tap of the | ||
| /// terminal (``handleTap``). `isComposerPresented` is unchanged while hidden, so | ||
| /// the composer (and its draft) reappear intact. Item 2 of the Round 8 spec. | ||
| private var chromeHidden = false | ||
|
|
||
| /// Bottom space (points) reserved below the toolbar for the keyboard OR the home | ||
| /// indicator, whichever applies. | ||
| /// | ||
| /// When the software keyboard is up the toolbar rides its top, so this is the | ||
| /// live keyboard height. When the keyboard is down the toolbar is still visible | ||
| /// (Round 8), so it must clear the bottom safe area (home indicator) rather than | ||
| /// sit flush on the screen edge — this returns ``safeAreaInsetsBottom`` then. The | ||
| /// composer band and toolbar stack ABOVE this inset; the grid reserves it too. | ||
| /// Used by ``bottomDockFrames()`` and the grid reservation. | ||
| private var keyboardOccupancyInBounds: CGFloat { | ||
| keyboardHeight > 0 ? keyboardHeight : safeAreaInsetsBottom | ||
| } | ||
|
|
||
| /// The bottom safe-area inset (home-indicator height) in this surface's bounds. | ||
| /// | ||
| /// The surface extends under the bottom safe area (the host applies | ||
| /// `ignoresSafeArea(.container, .bottom)`), so when the keyboard is down the | ||
| /// always-visible toolbar must clear this much to avoid the home indicator. Reads | ||
| /// the view's own inset, falling back to the window's, because `safeAreaInsets` | ||
| /// can be zero before the view is on a window. | ||
| private var safeAreaInsetsBottom: CGFloat { | ||
| let own = safeAreaInsets.bottom | ||
| if own > 0 { return own } | ||
| return window?.safeAreaInsets.bottom ?? 0 | ||
| } | ||
|
|
||
| /// Reconcile the docked bar's visibility (and its reserved grid height) with | ||
| /// the current keyboard + composer state. Hiding the bar releases its reserved | ||
| /// height so the terminal grid reclaims that space; showing it reserves the | ||
| /// height again. Idempotent: a no-op when already in the target state. | ||
| private func updateDockedToolbarVisibility() { | ||
| let shouldShow = dockedToolbarShouldBeVisible | ||
| let reserved: CGFloat = shouldShow ? Self.persistentToolbarHeight : 0 | ||
| guard dockedToolbar?.isHidden != !shouldShow || reservedToolbarHeight != reserved else { return } | ||
| dockedToolbar?.isHidden = !shouldShow | ||
| // The composer band rides with the toolbar: hide it when the chrome is | ||
| // suppressed, show it again when the chrome returns and a field is mounted. | ||
| // Its frame already collapses to `.zero` while hidden (see | ||
| // ``bottomDockFrames()``); toggling `isHidden` also stops it intercepting taps. | ||
| composerContainer.isHidden = !shouldShow || composerContainer.subviews.isEmpty | ||
| reservedToolbarHeight = reserved | ||
| setNeedsGeometrySync() | ||
| setNeedsLayout() | ||
| } | ||
|
|
||
| /// Temporarily hide (or re-show) the bottom chrome — the always-visible toolbar | ||
| /// and any open composer band — via the HIDE button (item 2). | ||
| /// | ||
| /// Hiding also drops the software keyboard: with the toolbar always visible, HIDE | ||
| /// only makes sense as "clear all chrome to see the full terminal", which requires | ||
| /// resigning the keyboard too. `isComposerPresented` is left untouched, so the | ||
| /// composer (and its draft) reappear intact on the next terminal tap | ||
| /// (``handleTap``). Animated on the keyboard curve via ``animateBottomDock``. | ||
| private func setChromeHidden(_ hidden: Bool) { | ||
| guard chromeHidden != hidden else { return } | ||
| chromeHidden = hidden | ||
| if hidden, keyboardHeight > 0 { | ||
| // Drop the keyboard first; its hide notification re-seats the dock, then | ||
| // the visibility update below removes the toolbar/composer. Resign | ||
| // whichever responder actually owns the keyboard — the band can be | ||
| // presented while the terminal's hidden input proxy (a sibling of | ||
| // `composerContainer`) holds first responder, so gating on |
Contributor
There was a problem hiding this comment.
The ~700-line addition folds the entire bottom-dock coordinate system (bottomDockFrames, layoutBottomDock, animateBottomDock, setComposerBandHeight, mountComposerView, setChromeHidden, installComposerContainer) and the ComposerDockReducer integration into an already over-budget file. The dock layout math and the HIDE-chrome state machine are independently testable and have no metal/render dependency — they could be extracted into a BottomDockLayout struct or a dedicated UIKit dock view that GhosttySurfaceView delegates to, keeping the renderer itself shorter and easier to audit. The TerminalInputTextView.swift file also grew from 949 → 1,205 lines in the same PR, crossing its own budget line.
Rule Used: Flag Swift changes that add too much unrelated res... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Comment on lines
+2191
to
+2195
| /// Dismiss the iMessage-style composer if it is open. Used when the user hides | ||
| /// the keyboard while composing, so the composer can never be left presented | ||
| /// with the keyboard down. Idempotent: a no-op when the composer is already | ||
| /// closed. | ||
| public func dismissComposer() { |
Contributor
There was a problem hiding this comment.
dismissComposer() contradicts Round 8 semantics
The comment says the function is called "when the user hides the keyboard while composing, so the composer can never be left presented with the keyboard down." Round 8 deliberately reverses this: the composer now survives a keyboard-down, and handleKeyboardWillHide no longer calls dismissComposer(). The stale comment would mislead a future reader about when this method is appropriate to call.
Suggested change
| /// Dismiss the iMessage-style composer if it is open. Used when the user hides | |
| /// the keyboard while composing, so the composer can never be left presented | |
| /// with the keyboard down. Idempotent: a no-op when the composer is already | |
| /// closed. | |
| public func dismissComposer() { | |
| /// Dismiss the iMessage-style composer if it is open. Called by the composer's | |
| /// chevron and any explicit close path. Since Round 8, the keyboard collapsing | |
| /// does NOT call this — the composer survives a keyboard-down and its draft is | |
| /// preserved until the user explicitly taps the chevron. Idempotent: a no-op | |
| /// when the composer is already closed. | |
| public func dismissComposer() { |
Comment on lines
+908
to
+913
| /// Structured diagnostic log (DEBUG dogfood builds only), property-injected | ||
| /// from the shell store by ``GhosttySurfaceRepresentable`` so the | ||
| /// composer-dock probes land in the blob the diagnostic export captures. | ||
| /// `nil` in release and in hosts that do not wire it; every probe is then a | ||
| /// no-op. | ||
| public var diagnosticLog: DiagnosticLog? |
Contributor
There was a problem hiding this comment.
diagnosticLog is a public var in release builds, set only inside #if DEBUG
The property is assigned once, inside a #if DEBUG block in GhosttySurfaceRepresentable.makeUIView, so it will always be nil in release. Keeping it public leaks the DiagnosticLog type into GhosttySurfaceView's release-visible API and wastes a word of storage per surface. Wrapping it in #if DEBUG removes it entirely from non-debug builds.
Suggested change
| /// Structured diagnostic log (DEBUG dogfood builds only), property-injected | |
| /// from the shell store by ``GhosttySurfaceRepresentable`` so the | |
| /// composer-dock probes land in the blob the diagnostic export captures. | |
| /// `nil` in release and in hosts that do not wire it; every probe is then a | |
| /// no-op. | |
| public var diagnosticLog: DiagnosticLog? | |
| #if DEBUG | |
| /// Structured diagnostic log (DEBUG dogfood builds only), property-injected | |
| /// from the shell store by ``GhosttySurfaceRepresentable`` so the | |
| /// composer-dock probes land in the blob the diagnostic export captures. | |
| /// `nil` in hosts that do not wire it; every probe is then a no-op. | |
| public var diagnosticLog: DiagnosticLog? | |
| #endif |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
…BUG-gate fixes Cursor: signOut() now clears the per-terminal composer-dismissed set and the field-focus mirror, so the next account gets the default-open composer everywhere and the selection reset cannot arm a stale focus request; makeUIView seeds the surface's composerActive flag so the compose button's intent math never depends on SwiftUI's make/update ordering. Greptile: dismissComposer() doc updated to Round 8 semantics (keyboard collapse never dismisses), and GhosttySurfaceView.diagnosticLog is DEBUG-gated to match its only readers.
Contributor
Author
|
Re Greptile P2 (GhosttySurfaceView.swift growth 2,953 → 3,654): accepted as known debt for this landing. The surface owns the whole bottom dock (terminal grid / composer band / toolbar / keyboard) in one coordinate system by design, and the prior two-layout-system split is exactly what caused the round-5/6 frame fights this PR removes. The Swift file-length budget was refreshed in 688d257 accepting the growth; extracting the dock geometry into its own type within the same file's coordinate ownership is follow-up territory, not a pre-merge refactor. Other Greptile and Cursor findings are fixed in 90435a13f. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift (1)
3297-3311:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift
terminal.pastestill has an ambiguous-failure duplicate-send path.These lines fix the late-success case, but the error path still treats every thrown
sendRequestas "nothing was pasted". For a non-idempotent RPC, a timeout/disconnect after the Mac already applied the paste will preserve the draft and let the user retry, which can paste the same block twice. This needs an operation id / dedupe contract, or another authoritative post-send reconciliation path, instead of mapping all thrown requests tofalse.Based on PR context,
terminal.pasteis the write path for composed input and this change only removes the false-negative late-ack case.🤖 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/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift` around lines 3297 - 3311, The error path for terminal.paste incorrectly treats any thrown sendRequest as “nothing was pasted,” risking duplicate non-idempotent sends; modify the send/retry contract so that terminal.paste uses an authoritative operation id and reconciliation instead of mapping all thrown errors to false: attach a unique operationId to the paste request at the send point, persist the pending operationId keyed by terminalID (or connectionGeneration) before calling sendRequest, and on any thrown error consult that persisted operationId with a post-send reconciliation call (or check server-side status) before returning false; update isCurrentRemoteOperation, handleTerminalInputResponse and the error-handling sequence (disconnectForAuthorizationFailureIfNeeded, markMacConnectionUnavailableIfNeeded, applyOperationalError) to clear/preserve the persisted operationId only after authoritative confirmation so retries do not double-apply the same paste.
🤖 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/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift`:
- Around line 660-666: The MobileShellComposite is accumulating composer
lifecycle state; extract those properties and logic into a dedicated
collaborator (e.g., ComposerState or ComposerCoordinator) and have
MobileShellComposite hold and forward to that collaborator. Move
composerDismissedTerminalIDs, composerFieldIsFocused,
composerFocusRequestPending, composerFocusRequestTerminalID and any related
didSet/handlers and persistence/replay logic into the new type, expose a minimal
API (focusRequest/clear/dismiss/persist) and replace direct reads/writes in
MobileShellComposite with calls to that API; ensure existing initializers
subscribe/restore state and update unit/integration tests where
MobileShellComposite previously manipulated those properties.
In
`@Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift`:
- Around line 1562-1571: Replace the blind toggle delegate call with a
target-bearing callback: update the delegate API (currently
ghosttySurfaceViewDidRequestComposerToggle(_:)) to accept the resolved Bool/enum
(composer open vs close) and call it with intent == .openComposer instead of
delegate?.ghosttySurfaceViewDidRequestComposerToggle(self); then update
host-side code (e.g. GhosttySurfaceRepresentable) to apply the target
idempotently (call the store setter like setComposerActive(target) or an
equivalent store?.setComposerActive(target) rather than
store?.toggleComposer()), and adjust the default no-op delegate implementation
to match the new signature.
---
Outside diff comments:
In `@Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift`:
- Around line 3297-3311: The error path for terminal.paste incorrectly treats
any thrown sendRequest as “nothing was pasted,” risking duplicate non-idempotent
sends; modify the send/retry contract so that terminal.paste uses an
authoritative operation id and reconciliation instead of mapping all thrown
errors to false: attach a unique operationId to the paste request at the send
point, persist the pending operationId keyed by terminalID (or
connectionGeneration) before calling sendRequest, and on any thrown error
consult that persisted operationId with a post-send reconciliation call (or
check server-side status) before returning false; update
isCurrentRemoteOperation, handleTerminalInputResponse and the error-handling
sequence (disconnectForAuthorizationFailureIfNeeded,
markMacConnectionUnavailableIfNeeded, applyOperationalError) to clear/preserve
the persisted operationId only after authoritative confirmation so retries do
not double-apply the same paste.
🪄 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: fe685bb1-487d-4218-9dfc-1ff718b7ac22
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (2)
Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftPackages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift
Comment on lines
+660
to
+666
| // switch they trigger cannot arm a stale focus request, and drop any | ||
| // already-armed handshake (the selection reset's didSet only clears it | ||
| // when the terminal id actually changes). | ||
| composerDismissedTerminalIDs = [] | ||
| composerFieldIsFocused = false | ||
| composerFocusRequestPending = false | ||
| composerFocusRequestTerminalID = nil |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Split composer orchestration out of MobileShellComposite.
This hunk adds more composer lifecycle state into a store that already owns pairing, reconnects, draft persistence, terminal transport, replay, feedback, and analytics. At ~4k lines, this type is well past the repo’s Swift size/responsibility limits; please extract the composer/draft state machine into a dedicated collaborator instead of continuing to deepen the god object.
As per coding guidelines, "Flag Swift production files that exceed 400 lines without a clear single responsibility, or exceed 800 lines even with mostly coherent responsibility" and "Flag files that mix UI rendering, state ownership, persistence, networking, parsing, subprocess/socket protocol, and platform bridge code in one place."
🤖 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/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift`
around lines 660 - 666, The MobileShellComposite is accumulating composer
lifecycle state; extract those properties and logic into a dedicated
collaborator (e.g., ComposerState or ComposerCoordinator) and have
MobileShellComposite hold and forward to that collaborator. Move
composerDismissedTerminalIDs, composerFieldIsFocused,
composerFocusRequestPending, composerFocusRequestTerminalID and any related
didSet/handlers and persistence/replay logic into the new type, expose a minimal
API (focusRequest/clear/dismiss/persist) and replace direct reads/writes in
MobileShellComposite with calls to that API; ensure existing initializers
subscribe/restore state and update unit/integration tests where
MobileShellComposite previously manipulated those properties.
Source: Coding guidelines
Comment on lines
+1562
to
+1571
| // Optimistically flip the local mirror to the intent's outcome BEFORE | ||
| // the store round-trip. `composerActive` is otherwise synced back via | ||
| // SwiftUI's `updateUIView`, which runs a render pass after the store | ||
| // mutation — a second tap landing inside that window would read the | ||
| // stale flag, resolve `.openComposer` again, and the toggle would | ||
| // dismiss the composer the first tap just presented. The authoritative | ||
| // sync still arrives via `setComposerActive` (idempotent when the | ||
| // optimistic value already matches). | ||
| setComposerActive(intent == .openComposer) | ||
| delegate?.ghosttySurfaceViewDidRequestComposerToggle(self) |
There was a problem hiding this comment.
Pass the resolved composer target through the delegate instead of firing another blind toggle.
This hunk now computes an explicit .openComposer / .closeComposer outcome, but the host callback is still a no-arg toggle; downstream, GhosttySurfaceRepresentable turns that into store?.toggleComposer() on a later Task { @mainactor ... }. That leaves the write side non-idempotent: any intervening host-side presentation change can still flip the store the wrong way, and the documented default no-op delegate is no longer actually safe because the surface mutates composerActive before the callback runs. Please carry the target state (open vs close) through the delegate and apply it idempotently on the store side instead of re-toggling.
🤖 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/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift`
around lines 1562 - 1571, Replace the blind toggle delegate call with a
target-bearing callback: update the delegate API (currently
ghosttySurfaceViewDidRequestComposerToggle(_:)) to accept the resolved Bool/enum
(composer open vs close) and call it with intent == .openComposer instead of
delegate?.ghosttySurfaceViewDidRequestComposerToggle(self); then update
host-side code (e.g. GhosttySurfaceRepresentable) to apply the target
idempotently (call the store setter like setComposerActive(target) or an
equivalent store?.setComposerActive(target) rather than
store?.toggleComposer()), and adjust the default no-op delegate implementation
to match the new signature.
…terminal The two iphone-simulator failures (testComposerDraftSurvivesHideRevealCompose, testComposerButtonHittabilityAfterTypingNoHideReveal) showed the reveal focus request armed (composerFocusRequest=1) but never consumed: the request was keyed to store.selectedTerminalID while the composer view's id comes from WorkspaceDetailView's rendered terminal, which falls back to the workspace's first terminal and can diverge from the selection. The compose toggle and focus delegate paths now pass the surface's own terminal id through toggleComposer(forTerminalID:) / presentAndFocusComposer(forTerminalID:), so the request target and the consuming view's id come from the same coordinator and match by construction. The mid-switch arm still keys to the new selection. Unit test pins the rendered-vs-selected divergence.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 71928ab. Configure here.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift (1)
2185-2209:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThread
terminalIDthrough the presented-state mutation too.
GhosttySurfaceRepresentablenow calls these APIs with the requestingsurfaceID, but this implementation only uses that id for the focus handshake. The open/close state still goes throughisComposerPresented/setComposerPresented, which are keyed offselectedTerminalID. When the rendered terminal diverges from selection, a toggle/revive on surface B will arm focus for B but record the dismissal/open state against A instead, so the per-terminal dismissed-set state drifts.💡 Narrow fix
public func toggleComposer(forTerminalID terminalID: String? = nil) { - if isComposerPresented { - setComposerPresented(false) + if isComposerPresented(forTerminalID: terminalID) { + setComposerPresented(false, forTerminalID: terminalID) } else { - setComposerPresented(true) + setComposerPresented(true, forTerminalID: terminalID) requestComposerFieldFocus(forTerminalID: terminalID) } } public func presentAndFocusComposer(forTerminalID terminalID: String? = nil) { - setComposerPresented(true) + setComposerPresented(true, forTerminalID: terminalID) requestComposerFieldFocus(forTerminalID: terminalID) }private func isComposerPresented(forTerminalID terminalID: String?) -> Bool { guard let terminalID = terminalID ?? selectedTerminalID?.rawValue else { return false } return !composerDismissedTerminalIDs.contains(terminalID) } private func setComposerPresented(_ presented: Bool, forTerminalID terminalID: String? = nil) { guard let terminalID = terminalID ?? selectedTerminalID?.rawValue, presented != isComposerPresented(forTerminalID: terminalID) else { return } // existing mutation + diagnostics }Please also extend the rendered-vs-selected regression to assert the dismissed-set target, not just the focus token.
Also applies to: 2257-2260
🤖 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/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift` around lines 2185 - 2209, The toggle/present logic threads the focus terminalID into requestComposerFieldFocus but still reads/writes the presented state keyed only to selectedTerminalID, causing dismissed-set drift; update isComposerPresented and setComposerPresented to accept an optional terminalID (use terminalID ?? selectedTerminalID?.rawValue) and use those in toggleComposer(forTerminalID:) and presentAndFocusComposer(forTerminalID:) so the dismissal/open mutation targets the requesting terminal; ensure the mutation still short-circuits when presented == current presented and preserves existing diagnostics; finally extend the rendered-vs-selected regression test to assert the composerDismissedTerminalIDs entry for the target terminalID (not just the focus token).
🤖 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.
Outside diff comments:
In `@Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift`:
- Around line 2185-2209: The toggle/present logic threads the focus terminalID
into requestComposerFieldFocus but still reads/writes the presented state keyed
only to selectedTerminalID, causing dismissed-set drift; update
isComposerPresented and setComposerPresented to accept an optional terminalID
(use terminalID ?? selectedTerminalID?.rawValue) and use those in
toggleComposer(forTerminalID:) and presentAndFocusComposer(forTerminalID:) so
the dismissal/open mutation targets the requesting terminal; ensure the mutation
still short-circuits when presented == current presented and preserves existing
diagnostics; finally extend the rendered-vs-selected regression test to assert
the composerDismissedTerminalIDs entry for the target terminalID (not just the
focus token).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 62563929-8da2-412d-8bf1-b3f81083efb3
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (4)
Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftPackages/CmuxMobileShell/Tests/CmuxMobileShellTests/ComposerDefaultOpenTests.swiftPackages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/GhosttySurfaceRepresentable.swiftPackages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalComposerView.swift
On the reveal path (compose tap on a presented-but-unfocused composer, and the tap-to-reveal-from-HIDE path) the store handshake drives the SwiftUI field's @focusstate, but a programmatic @focusstate set inside a hosting controller frame-mounted into the band is dropped nondeterministically (iphone-sim CI failed the reveal focus while ipad passed identical code). After requesting focus from the host, the surface now also drives the band's backing UITextField/UITextView to first responder directly on the next runloop hop; SwiftUI mirrors UIKit first responder back into @focusstate, so the store mirror stays consistent. No-op when the band is unmounted or the field already holds focus.
…election wait testTerminalRendersColorBandsAcrossZoomLevels: the single-band threshold drops 16 -> 12 of 24 strip samples. The always-visible toolbar + default-open composer band legitimately shorten the terminal grid, so a clean max-zoom band on the iPhone height fills ~14 samples with row gaps between glyph lines (CI dump: strong=14 distinct=1 uniform=true) — a clean render, not a blank or torn one. Blank (strong~0) and garble (uniform=false) detection are unchanged. assertHostSelection: raise the mock-selection wait to 20s; a saturated CI runner can exceed the old 8s for the create round-trip plus the new surface/composer mount (testWorkspaceToolbarCreatesWorkspaceAndTerminal passed 4/4 on an AWS M4 Pro iPad simulator at the same commit, so the ipad CI failure is runner-load timing, not a code defect). Wait-until, so fast runs return immediately.
lawrencecchen
added a commit
that referenced
this pull request
Jun 11, 2026
Reset to origin/main (composer #5876 landed), then merge old dog HEAD a300868 to preserve every feature not yet on main: notifications dismiss-sync (#5568), multi-Mac switcher hardening (#5545), foreground repaint (#5571), image paste too-large toast (#5572), hidden native input (#5596), workspace groups (#5625), wslist round-10 snapshot, scroll-to-bottom hysteresis, DEV dogfood pane, attachments button, arrow toolbar keys, terminal.paste capability gating. Conflict policy: main's reviewed composer-land form wins for composer core (keyed focus handshake, draft FIFO coalescing, paste submit partial-success), dog wins for unlanded feature surface. ghostty pinned to dog 34cbf18 (descendant of main's e5c962a). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
lawrencecchen
added a commit
that referenced
this pull request
Jun 11, 2026
selected_tests_passed_despite_xcodebuild_status only knew XCTest failure formats. Swift Testing prints failures as '✘ Test ... failed' and the final 'Failing tests:' section is not always flushed into the tee'd log, so a run with genuinely failing Swift Testing tests (exit 65) could be misclassified as a runner cleanup failure and turn the job green. That false green let the stale manual-pairing tests merge red on #5876 and earlier. Add the Swift Testing failure markers to the negative grep. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
lawrencecchen
added a commit
that referenced
this pull request
Jun 11, 2026
…te restriction, catch Swift Testing failures in success override (#5906) * Align manual-pairing auth-contract tests with encrypted-route restriction d7ee593 restricted routeAllowsStackAuth to encrypted/loopback routes (Tailscale, iroh, loopback) as a security fix but only updated the policy unit tests. Four cmuxFeatureTests auth-contract tests still asserted the old contract (Stack token sent over plain-TCP LAN/.local manual routes) and have failed on every ios-simulator run since, masked by the workflow's success-override grep not recognizing Swift Testing failure output. Rewrite the three LAN/.local tests as rejection-contract tests: pairing fails before any RPC (and the Stack bearer token) leaves the device, and the actionable route-not-allowed error is surfaced. Repoint the probe-then-fallback test at a Tailscale host so the method_not_found to synthetic-ticket fallback path keeps its coverage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Catch Swift Testing failures in the ios-simulator success override selected_tests_passed_despite_xcodebuild_status only knew XCTest failure formats. Swift Testing prints failures as '✘ Test ... failed' and the final 'Failing tests:' section is not always flushed into the tee'd log, so a run with genuinely failing Swift Testing tests (exit 65) could be misclassified as a runner cleanup failure and turn the job green. That false green let the stale manual-pairing tests merge red on #5876 and earlier. Add the Swift Testing failure markers to the negative grep. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
lawrencecchen
added a commit
that referenced
this pull request
Jun 12, 2026
Re-applies origin/feat-ios-dog-unified (771f532, dog 11b) onto current origin/main (f2627b9). Main's reviewed forms win for landed features (composer #5876, QR #5872, presence gate #5912, CI fix #5906): seven pairing/RPC/transport files take main's private-extension structure, persistPairedMacFromTicket keeps main's serialized-write-chain lookup, QR pairing tests keep main's durable-write polls. Dog-carried unlanded work is preserved: MobileHostService+Capabilities.swift superset (notification.dismiss.v1, terminal.paste.v1, workspace.groups.v1, DEBUG dogfood verbs) replaces main's inline subset var, dismiss-sync observer and capability flag resets kept, dogfood pane model kept.
hhsw2015
pushed a commit
to hhsw2015/cmux
that referenced
this pull request
Jun 12, 2026
…te restriction, catch Swift Testing failures in success override (manaflow-ai#5906) * Align manual-pairing auth-contract tests with encrypted-route restriction d7ee593 restricted routeAllowsStackAuth to encrypted/loopback routes (Tailscale, iroh, loopback) as a security fix but only updated the policy unit tests. Four cmuxFeatureTests auth-contract tests still asserted the old contract (Stack token sent over plain-TCP LAN/.local manual routes) and have failed on every ios-simulator run since, masked by the workflow's success-override grep not recognizing Swift Testing failure output. Rewrite the three LAN/.local tests as rejection-contract tests: pairing fails before any RPC (and the Stack bearer token) leaves the device, and the actionable route-not-allowed error is surfaced. Repoint the probe-then-fallback test at a Tailscale host so the method_not_found to synthetic-ticket fallback path keeps its coverage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Catch Swift Testing failures in the ios-simulator success override selected_tests_passed_despite_xcodebuild_status only knew XCTest failure formats. Swift Testing prints failures as '✘ Test ... failed' and the final 'Failing tests:' section is not always flushed into the tee'd log, so a run with genuinely failing Swift Testing tests (exit 65) could be misclassified as a runner cleanup failure and turn the job green. That false green let the stale manual-pairing tests merge red on manaflow-ai#5876 and earlier. Add the Swift Testing failure markers to the negative grep. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
lawrencecchen
added a commit
that referenced
this pull request
Jun 13, 2026
…#5596/#5625/#5628) over current main Beta queue (#5876/#5872/#5869/#5875/#5927/#5912/#5726/#5776/#5916) is now on main; conflicts resolved by taking main as authoritative for the merged workspace-list/notifications/read-state/close surface, while preserving the carry-set: terminal.paste capability (#5572), hidden-input strings (#5596), smooth-scroll/scroll-to-bottom (#5628), and the live notifications feed (notificationsStore + mobile.notifications.list/mark_read dispatch). Dropped the superseded mute design. Capability flags unified onto main's computed supportedHostCapabilities set (added computed supportsTerminalPaste + DEBUG supportsDogfoodChecklist). xcstrings merged (HEAD-precedence union, mute keys dropped). pbxproj took HEAD consistently; budget regenerated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jun 15, 2026
This was referenced Jun 23, 2026
ShubhamPatilsd
pushed a commit
to emergent-inc/mosaic
that referenced
this pull request
Jul 9, 2026
…te restriction, catch Swift Testing failures in success override (#5906) * Align manual-pairing auth-contract tests with encrypted-route restriction 4ba9e3d restricted routeAllowsStackAuth to encrypted/loopback routes (Tailscale, iroh, loopback) as a security fix but only updated the policy unit tests. Four cmuxFeatureTests auth-contract tests still asserted the old contract (Stack token sent over plain-TCP LAN/.local manual routes) and have failed on every ios-simulator run since, masked by the workflow's success-override grep not recognizing Swift Testing failure output. Rewrite the three LAN/.local tests as rejection-contract tests: pairing fails before any RPC (and the Stack bearer token) leaves the device, and the actionable route-not-allowed error is surfaced. Repoint the probe-then-fallback test at a Tailscale host so the method_not_found to synthetic-ticket fallback path keeps its coverage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Catch Swift Testing failures in the ios-simulator success override selected_tests_passed_despite_xcodebuild_status only knew XCTest failure formats. Swift Testing prints failures as '✘ Test ... failed' and the final 'Failing tests:' section is not always flushed into the tee'd log, so a run with genuinely failing Swift Testing tests (exit 65) could be misclassified as a runner cleanup failure and turn the job green. That false green let the stale manual-pairing tests merge red on manaflow-ai/cmux#5876 and earlier. Add the Swift Testing failure markers to the negative grep. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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.
Lands the iMessage-style terminal composer for iOS. Supersedes #5511.
What ships
isComposerPresentedis derived per-terminal state over a session-only dismissed set: the chevron hides it (remembered per terminal), and dismissing one terminal leaves every other terminal open. Presented does not imply focused; the band appears with the keyboard down, and focus is a one-shot store handshake (composerFocusRequest+ a pending flag) armed only by an explicit open, the reveal-and-focus path, or a mid-compose terminal switch.terminal.pasteRPC. Multi-line composer text lands as one bracketed paste plus a single submit keypress (upgraded to ctrl+enter for Claude Code multi-line), instead ofterminal.inputCR-fragmenting each line into separate commands.Review hardening (autoreview loop, 8 rounds to clean)
terminal.pastereports partial success (submitted: false+submit_error) when the submit key fails after the text was already delivered, so the client clears the draft and a retry cannot paste a duplicate block.terminal.pasteroutes through theTerminalPanelexplicit-input wrappers, waking a hibernated agent terminal the way local typing does.endEditingon the composer container alone resigned nothing.Tests
swift test: CmuxMobileShell 79 / CMUXMobileCore 74 / CmuxMobileRPC 23, all green, including suites for the dock reducer, default-open, submit/draft reconcile, FIFO ordering + coalescing, draft-swap field ownership, and focus-request keying.cmux-ios) and macOS app, tagged derivedDataPaths.Localization
Composer strings (
mobile.composer.placeholder,mobile.composer.send,mobile.composer.close,terminal.input_accessory.composer) are localized in en + ja inios/cmux/Resources/Localizable.xcstrings; the branch diff was audited for bare English UI strings (none found).🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Large mobile UI/state-machine change across shell, surface layout, and remote terminal input (
terminal.paste); mitigated by extensive tests but still affects core typing/submit flows on iOS.Overview
Adds an iMessage-style iOS terminal composer that is open by default per terminal (session-only dismiss per terminal), with an inline send control and per-terminal drafts wired through a new
TerminalDraftStoringseam and in-memory store plus a FIFO draft pipeline (coalesced saves, sign-out wipe, post-send reconcile).Bottom dock moves into
GhosttySurfaceView: composer band + always-visible toolbar + keyboard in one layout stack (replacingsafeAreaInset/toolbar handoff).GhosttySurfaceRepresentablehostsTerminalComposerViewin the surface composer band; compose-button behavior usesComposerDockState(open / reveal-and-focus / close) so hide/reveal cycles do not clear drafts.Send path uses new
terminal.pasteRPC from the shell (ack-gated draft clear, re-entrancy guard); mobile RPC client treats paste like terminal input for ticket coverage. Focus uses a terminal-keyedcomposerFocusRequesthandshake plus UIKit assist on reveal.Adds DEBUG diagnostics (composer events, dock probes) and broad unit tests for dock reducer, default-open, draft ordering/switch ownership, and submit reconcile; minor toolbar UX (sticky-lock border, composer accessory action).
Reviewed by Cursor Bugbot for commit a2d7d0e. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Ships an iMessage-style terminal composer on iOS with a default-open band, inline send, and per-terminal drafts in a surface-owned bottom dock. Adds
terminal.pastefor bracketed multi-line sends and hardens focus/draft handling to prevent draft loss, double submits, and flaky reveal focus.New Features
terminal.paste.TerminalDraftStoringwith FIFO ordering and coalesced keystroke saves; in-memory store for this PR; composer strings localized (en, ja).Bug Fixes
terminal.pasteaccepts late acks after reconnect; partial submit failures returnsubmitted: falsewithsubmit_error.Written for commit 2b2c4d9. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests
UI