feat: Show live progress feedback while installing the CLI#1959
Conversation
Installer stdout/stderr must reach the editor UI while install runs. Add an onOutputLine callback on the setup command runner and thread IProgress<string> through NativeCliInstaller and CliSetupApplicationService. Co-authored-by: Cursor <cursoragent@cursor.com>
Users previously saw only a disabled Installing button with no feedback. Animate the install button label with elapsed time, keep the seconds suffix stable via transparent rich-text padding dots, and stream the latest installer output line under the button. Avoid ProgressBar percent UI because download size is unavailable at every layer. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Warning Review limit reached
Next review available in: 42 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughCLI installation now streams command output through installer and application layers into animated progress UI. Formatting helpers and tests cover status/detail text, while Setup Wizard and settings interfaces bind, display, update, and hide installation progress. ChangesCLI installation output streaming
Progress presentation
Setup Wizard integration
Settings presenter integration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant SetupWizardCliWorkflowController
participant CliSetupApplicationService
participant NativeCliInstaller
participant CliInstallProgressView
User->>SetupWizardCliWorkflowController: start CLI installation
SetupWizardCliWorkflowController->>CliInstallProgressView: show progress
SetupWizardCliWorkflowController->>CliSetupApplicationService: install with Progress<string>
CliSetupApplicationService->>NativeCliInstaller: forward progress reporter
NativeCliInstaller->>CliInstallProgressView: report installer output lines
CliInstallProgressView->>CliInstallProgressView: update status and detail text
SetupWizardCliWorkflowController->>CliInstallProgressView: hide progress on completion
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Installer process output is untrusted and can contain angle-bracket fragments. Keep rich text enabled only on the button label that uses transparent padding dots for stable elapsed-time layout. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Assets/Tests/Editor/NativeCliInstallerTests.cs (1)
328-352: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover stderr forwarding too.
The new test only validates stdout, while the feature contract includes stderr. Make the echo command emit one stderr line and assert it reaches the callback.
Suggested test adjustment
- "-c \"echo line1; echo line2\"", - "echo line1; echo line2"); + "-c \"echo line1; echo line2 >&2\"", + "echo line1; echo line2 >&2"); ... Assert.That(receivedLines, Does.Contain("line1")); Assert.That(receivedLines, Does.Contain("line2"));🤖 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 `@Assets/Tests/Editor/NativeCliInstallerTests.cs` around lines 328 - 352, Update RunInstallCommand_StreamsOutputLinesToCallback and its BuildEchoInstallCommand setup so the test command emits a distinct stderr line in addition to stdout, then assert the callback’s receivedLines contains that stderr line alongside line1 and line2.
🤖 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/src/Editor/Presentation/UIToolkit/Components/CliInstallProgressView.cs`:
- Line 40: Replace the DateTime.UtcNow-based elapsed-time tracking in the
install progress flow, including _startedAtUtc, with a Stopwatch-based monotonic
timer. Start it when installation begins and use its elapsed value for duration
calculations, retaining DateTime.UtcNow only for any separate wall-clock
timestamp requirement.
- Around line 55-65: Update the CliInstallProgressView progress-reporting flow
to associate each installation with a unique token or epoch, passing it through
Progress<string> callbacks into SetDetailLine. Have SetDetailLine ignore
callbacks whose token differs from the currently active, non-disposed
generation, while preserving existing formatting and empty-line handling.
---
Nitpick comments:
In `@Assets/Tests/Editor/NativeCliInstallerTests.cs`:
- Around line 328-352: Update RunInstallCommand_StreamsOutputLinesToCallback and
its BuildEchoInstallCommand setup so the test command emits a distinct stderr
line in addition to stdout, then assert the callback’s receivedLines contains
that stderr line alongside line1 and line2.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 4bf7fb8c-0397-4bd7-88e8-3ec53c06a270
⛔ Files ignored due to path filters (3)
Assets/Tests/Editor/CliInstallProgressFormattingTests.cs.metais excluded by none and included by nonePackages/src/Editor/Presentation/CliInstallProgressFormatting.cs.metais excluded by none and included by nonePackages/src/Editor/Presentation/UIToolkit/Components/CliInstallProgressView.cs.metais excluded by none and included by none
📒 Files selected for processing (20)
Assets/Tests/Editor/CliInstallProgressFormattingTests.csAssets/Tests/Editor/CliPathSetupFlowTests.csAssets/Tests/Editor/CliSetupApplicationServiceTests.csAssets/Tests/Editor/CliSetupSectionTests.csAssets/Tests/Editor/NativeCliInstallerTests.csPackages/src/Editor/Application/CliSetupApplicationService.csPackages/src/Editor/Infrastructure/CLI/NativeCliInstaller.csPackages/src/Editor/Infrastructure/CLI/NativeCliSetupCommandRunner.csPackages/src/Editor/Presentation/CliInstallProgressFormatting.csPackages/src/Editor/Presentation/Setup/SetupWizardCliWorkflowController.csPackages/src/Editor/Presentation/Setup/SetupWizardWindow.csPackages/src/Editor/Presentation/Setup/SetupWizardWindow.ussPackages/src/Editor/Presentation/Setup/SetupWizardWindow.uxmlPackages/src/Editor/Presentation/Setup/SetupWizardWorkflowController.csPackages/src/Editor/Presentation/UIToolkit/Components/CliInstallProgressView.csPackages/src/Editor/Presentation/UIToolkit/Components/CliSetupSection.csPackages/src/Editor/Presentation/UIToolkit/UnityCliLoopSettingsWindow.ussPackages/src/Editor/Presentation/UIToolkit/UnityCliLoopSettingsWindow.uxmlPackages/src/Editor/Presentation/UIToolkit/UnityCliLoopSettingsWindowUI.csPackages/src/Editor/Presentation/UnityCliLoopSettingsCliSetupPresenter.cs
Wall-clock DateTime.UtcNow can jump during NTP adjustments and make elapsed time negative against FormatStatusLine asserts. Drive the button timer from Stopwatch instead. Also assert stderr lines reach the installer progress callback alongside stdout. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
Summary
User Impact
Before: Pressing Install CLI only disabled the button and showed a static "Installing..." label. There was no progress signal, so a multi-second install looked like a freeze.
After:
Installing.→Installing..→Installing... (Ns)with the seconds position kept stable (transparent rich-text padding dots reserve three-dot width).Percent progress is intentionally not used: no layer knows the download size in advance, so a percentage would be fabricated.
Changes
onOutputLinecallback andIProgress<string>from the command runner up to Presentation.ProgressBarafter visual review: it looked like a looping fake percent fill. Final design uses the button label + detail line only.Verification
dist/darwin-arm64/uloop compile→ Error 0 / Warning 0CliInstallProgressFormatting(9), streaming callback test, related setup/section/application tests passed