Repository navigation
Improve iOS New Task recovery and naming - #8554
Conversation
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
|
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:
📝 WalkthroughWalkthroughThe task composer now supports optional workspace names across drafts, snapshots, editing, submission, and recovery. It also adds structured failure banners, directory-permission guidance, directory precedence tests, and related UI regression coverage. ChangesTask composer enhancements
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant TaskComposerWorkspaceNameField
participant TaskComposerSheet
participant MobileTaskSubmissionSnapshot
participant WorkspaceCreation
User->>TaskComposerWorkspaceNameField: enters optional workspace name
TaskComposerWorkspaceNameField->>TaskComposerSheet: updates editable submission state
TaskComposerSheet->>MobileTaskSubmissionSnapshot: captures workspaceName
MobileTaskSubmissionSnapshot->>WorkspaceCreation: supplies workspaceTitle
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 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 |
Greptile SummaryThis PR improves the iOS New Task flow and its recovery behavior. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (18): Last reviewed commit: "fix(ios): refine task composer leading s..." | Re-trigger Greptile |
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
ios/cmux/Resources/Localizable.xcstrings (1)
12538-12544: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve the authorization/rejection meaning in Japanese.
These translations omit or change the request actor:
認証されませんでしたdescribes authentication failure, while the English says the Mac did not authorize/rejected the request. Use explicit subjects and requests, for example:Proposed localization fix
- "value": "%@ で認証されませんでした。" + "value": "%@ はリクエストを許可しませんでした。" - "value": "Mac で認証されませんでした。" + "value": "Mac はリクエストを許可しませんでした。" - "value": "%@ に拒否されました。" + "value": "%@ がリクエストを拒否しました。" - "value": "Mac に拒否されました。" + "value": "Mac がリクエストを拒否しました。"Also applies to: 12555-12561, 12606-12612, 12623-12629
🤖 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 `@ios/cmux/Resources/Localizable.xcstrings` around lines 12538 - 12544, Update the Japanese string values for the affected authorization entries, including the entry containing "%@ didn't authorize the request." and the ranges at 12555-12561, 12606-12612, and 12623-12629, to explicitly state that the Mac rejected or did not authorize the request. Preserve the request actor and rejection meaning; do not use wording that only describes authentication failure.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@ios/cmux/Resources/Localizable.xcstrings`:
- Around line 12538-12544: Update the Japanese string values for the affected
authorization entries, including the entry containing "%@ didn't authorize the
request." and the ranges at 12555-12561, 12606-12612, and 12623-12629, to
explicitly state that the Mac rejected or did not authorize the request.
Preserve the request actor and rejection meaning; do not use wording that only
describes authentication failure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1f008858-0159-406f-bd13-4632d1d24cb1
📒 Files selected for processing (1)
ios/cmux/Resources/Localizable.xcstrings
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
# Conflicts: # Packages/macOS/CmuxGit/Tests/CmuxGitTests/PullRequestProbeServiceFetchTests.swift # ios/cmux/Resources/Localizable.xcstrings
…ios-task-errors-final # Conflicts: # Packages/macOS/CmuxGit/Tests/CmuxGitTests/PullRequestProbeServiceFetchTests.swift # ios/cmux/Resources/Localizable.xcstrings
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
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 (2)
Packages/macOS/CmuxGit/Tests/CmuxGitTests/PullRequestProbeServiceFetchTests.swift (2)
43-45: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReplace scheduler-dependent waits with completion signals.
The fixed
Task.yield()counts and attempts-onlywaitUntilcan make assertions depend on task scheduling rather than mount completion. Await the coordinator/store lifecycle signal, or use a deadline-bounded poll of the real predicate.As per path instructions, tests must use real completion signals or deadline-bounded predicate polling instead of arbitrary waits.
Also applies to: 89-91, 121-123, 144-153
🤖 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/macOS/CmuxGit/Tests/CmuxGitTests/PullRequestProbeServiceFetchTests.swift` around lines 43 - 45, Update PullRequestProbeServiceFetchTests to remove fixed Task.yield() counts and attempts-only waitUntil usage in all affected cases. Await the coordinator/store lifecycle completion signal, or poll the actual mount/completion predicate with an explicit deadline, so assertions run only after the relevant state is ready.Source: Path instructions
54-60: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winInject deterministic test time.
now: Date = Date()makes cache-age behavior depend on wall-clock time and test execution timing. Remove the default and pass one fixedDate(timeIntervalSince1970:)value to each fetch.As per path instructions, tests under
Packages/**/Testsmust avoid real wall-clock dependencies and use injected virtual or fixed time.Suggested change
- now: Date = Date(), + now: Date,🤖 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/macOS/CmuxGit/Tests/CmuxGitTests/PullRequestProbeServiceFetchTests.swift` around lines 54 - 60, Update the test helper method fetch in PullRequestProbeServiceFetchTests so now has no Date() default value, then pass one fixed Date(timeIntervalSince1970:) value to every fetch invocation to make cache-age assertions deterministic.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerWorkspaceNameField.swift`:
- Line 47: Extract the shared focus-loss behavior from
TaskComposerWorkspaceNameField and TaskComposerPromptCard into a reusable
ViewModifier or equivalent helper, including both onSubmit(endEditing) and
isFocused change handling. Apply that shared modifier in both views and remove
their duplicated wiring while preserving the existing focus and editing
behavior.
- Line 1: Extract the duplicated submit-and-focus-loss behavior from
TaskComposerWorkspaceNameField and TaskComposerPromptCard into a shared View
extension, such as endsEditingOnSubmitOrBlur(isFocused:action:), preserving the
existing FocusState transition check and action invocation. Replace each field’s
local onSubmit and onChange wiring with the shared modifier, passing its
existing focus binding and endEditing action.
---
Outside diff comments:
In
`@Packages/macOS/CmuxGit/Tests/CmuxGitTests/PullRequestProbeServiceFetchTests.swift`:
- Around line 43-45: Update PullRequestProbeServiceFetchTests to remove fixed
Task.yield() counts and attempts-only waitUntil usage in all affected cases.
Await the coordinator/store lifecycle completion signal, or poll the actual
mount/completion predicate with an explicit deadline, so assertions run only
after the relevant state is ready.
- Around line 54-60: Update the test helper method fetch in
PullRequestProbeServiceFetchTests so now has no Date() default value, then pass
one fixed Date(timeIntervalSince1970:) value to every fetch invocation to make
cache-age assertions deterministic.
🪄 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: 6635c91a-0302-4b8c-800a-155b9cbb13a7
📒 Files selected for processing (18)
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/Debug/TaskComposer/TaskComposerAccessibilityPreviewView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerButton.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerCompletedOperationRecovery.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerCompletedOperationRecoveryPhase.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerCompletedOperationRequestRelation.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerContextSection.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerFailureTitleStyle.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerPromptCard.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerRoutePicker.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet+CompletedOperationRecovery.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet+DirectorySelection.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet+DraftState.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet+Policies.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerWorkspaceNameField.swiftPackages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/TaskComposerFailureMessageTests.swiftPackages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/TerminalSurfaceMountOwnershipTests.swiftPackages/macOS/CmuxGit/Tests/CmuxGitTests/PullRequestProbeServiceFetchTests.swift
💤 Files with no reviewable changes (1)
- Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet+Policies.swift
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
Summary
Verification
swift test --package-path Packages/iOS/CmuxMobileShellModel(171 tests passed)b9652492ad)custom_title: "Release checklist"; the test workspace was then closedntnmfromb9652492adwith isolated web port4662ntnmoncmux-ntfail-cdx-0721-49jq empty ios/cmux/Resources/Localizable.xcstringsThe package-convention script reports only pre-existing violations outside the touched files.
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Improves the iOS New Task flow with an optional workspace name, clearer failure banners, smarter directory defaults, and deterministic recovery that blocks only equivalent requests and offers Start Again when refresh is still missing. Also polishes the UI: adds a compact route picker, stabilizes the agent menu layout, and blends directory shortcuts into the directory picker.
New Features
Bug Fixes
Written for commit 3b43d63. Summary will update on new commits.
Summary by CodeRabbit