Show iOS terminal send progress and failures - #9723
Merged
Merged
Conversation
📝 WalkthroughWalkthroughTerminal composer and Return-key submissions now track per-terminal sending, success, and failure states. The iOS interface displays these states, preserves failed drafts, and validates delayed and rejected submissions. ChangesTerminal send status
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant TerminalComposerView
participant MobileShellComposite
participant TerminalRPC
User->>TerminalComposerView: Submit terminal content
TerminalComposerView->>MobileShellComposite: submitComposer()
MobileShellComposite->>TerminalComposerView: Set status to sending
MobileShellComposite->>TerminalRPC: Send terminal paste or input
TerminalRPC-->>MobileShellComposite: Return success or failure
MobileShellComposite->>TerminalComposerView: Set status to sent or failed
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (23 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ios/cmuxUITests/cmuxUITests.swift`:
- Around line 7533-7549: Replace the terminalSendResponseDelay-based
DispatchQueue.asyncAfter flow in ios/cmuxUITests/cmuxUITests.swift:7533-7549
with explicit request/release synchronization: wait until the request reaches
the expected state, assert it is Sending, then signal the release gate before
sending the response. Apply the same synchronization changes to the related
sites at ios/cmuxUITests/cmuxUITests.swift:2791-2813 and 7219-7294, removing
wall-clock timing dependencies while preserving the existing sendResponse
behavior.
In
`@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift`:
- Around line 7702-7713: The raw input queue must retain each submission UUID
alongside its queued chunk instead of using the mutable per-terminal entry in
prepareTerminalSendStatusForRawInput. Update the queue and
drainRawTerminalInputBuffer so batching preserves the newest submission UUID,
then pass that UUID to sendRemoteTerminalInput; add a regression test covering a
held first request, a queued second Return submission, and rejection of the
second request.
In
`@Packages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/TerminalRawInputOrderingTests.swift`:
- Around line 522-535: Update waitForTerminalSendStatus to use a ContinuousClock
deadline rather than a fixed 200-iteration Task.yield loop. Keep
terminalSendStatus(forTerminalID:) == expected as the success condition,
continue yielding between checks, and return false only when the deadline
expires.
🪄 Autofix
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 Plus
Run ID: 12b2a14e-08fa-49dc-995d-691ce33b394c
📒 Files selected for processing (9)
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftPackages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/ComposerSubmitRoutingTests.swiftPackages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/TerminalRawInputOrderingTests.swiftPackages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/MobileTerminalSendStatus.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalComposerView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalSendStatusPill.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swiftios/cmux/Resources/Localizable.xcstringsios/cmuxUITests/cmuxUITests.swift
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ios/cmuxUITests/cmuxUITests.swift`:
- Around line 2840-2841: In the test flow after
awaitTerminalPasteRequestReached(), wait for the send XCUIElement’s label to
become “Sending” using an XCTNSPredicateExpectation before asserting it. Keep
the server response blocked until this UI expectation completes, then retain the
equality assertion.
🪄 Autofix
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 Plus
Run ID: 31cc47aa-0f4b-4887-b324-ecbb02408010
📒 Files selected for processing (4)
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalComposerView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalSendStatusPill.swiftios/cmux/Resources/Localizable.xcstringsios/cmuxUITests/cmuxUITests.swift
💤 Files with no reviewable changes (1)
- ios/cmux/Resources/Localizable.xcstrings
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 46aad06. 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/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift (1)
518-523: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winClear settled status when the user deletes the draft.
Line 519makes status clearing depend on the new text being non-empty. If a user deletes a failed draft in one edit, thedidSetbelow still persists the empty draft, but this branch does not callclearSettledTerminalSendStatus. The model can keep.failedwith no draft to retry.Track whether the mutation is a user edit or programmatic reconciliation at the mutation boundary. Clear settled status for every user edit, including a transition to an empty draft. Add a regression test for this transition.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift` around lines 518 - 523, The terminalInputText mutation logic in MobileShellComposite currently skips clearSettledTerminalSendStatus when a user deletes the draft; distinguish user edits from programmatic reconciliation at the mutation boundary, then clear the settled status for every user edit, including empty-text transitions, while preserving reconciliation behavior. Add a regression test covering deletion of a failed draft and verifying its status is cleared.
🤖 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/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift`:
- Around line 518-523: The terminalInputText mutation logic in
MobileShellComposite currently skips clearSettledTerminalSendStatus when a user
deletes the draft; distinguish user edits from programmatic reconciliation at
the mutation boundary, then clear the settled status for every user edit,
including empty-text transitions, while preserving reconciliation behavior. Add
a regression test covering deletion of a failed draft and verifying its status
is cleared.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 267d21f5-3f6f-4330-b9f9-fa624f509786
📒 Files selected for processing (4)
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftPackages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/ComposerSubmitRoutingStoreBuilders.swiftPackages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/ComposerSubmitRoutingTests.swiftios/cmuxUITests/cmuxUITests.swift
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.
Summary
Verification
swift test --package-path Packages/iOS/CmuxMobileShellModelSendsndiosinstalled and launched on Aziz at commit46aad061a6The selected hosted iOS workflow could not execute its UI test because unrelated existing package tests and a repository-wide pairing lint fail to compile or pass on this branch. The focused send-status tests pass, and the device archive compiles successfully.
Note
Medium Risk
Changes the terminal input send pipeline and composer UX across store, buffer, and RPC settlement paths, but behavior is scoped to user-visible status and preserves existing retry/staging semantics with added race guards.
Overview
Adds per-terminal send settlement for iOS composer submits and Return-terminated raw commands, so users see in-flight progress and explicit failures instead of silent hangs or lost drafts.
The shell store tracks
MobileTerminalSendStatus(sending/sent/failed) with operation IDs so a late RPC from an older send cannot overwrite a newer retry. ComposersubmitComposerwraps the full image+text run; raw input only enters this path when the buffered text contains a line break or carriage return. Non-submit keystrokes do not flash chrome; editing the draft clears a settled failure state. Focus/connection teardown fails pending raw sends without disturbing an in-flight composer paste on the same terminal.UI: The composer Send button shows a spinner while sending, disables double-tap, surfaces a failure caption and red styling on error, and returns to the normal Send affordance after success (no persistent “sent” checkmark). When the composer is hidden, a status pill overlays the terminal surface for sending/failed.
The raw input send buffer carries the newest operation ID through coalesced chunks. Tests cover routing, queued Return races, draft restore after failure, and UI tests hold/release
terminal.pasteon the mock host.Reviewed by Cursor Bugbot for commit 9a887b9. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Bug Fixes
Tests