fix: Wait for V2 server readiness in uloop launch - #2024
Conversation
Replace PR #1826's lockfile-only wait and "readiness was not checked" response with real readiness: UserSettings/UnityMcpSettings.json serverSessionId generation compare, TCP connect, and compile/reload lock absence. After the lockfile gate, focus the Editor once so V2's delayCall-based server start can tick (V2 has no SignalTick workaround). Co-authored-by: Cursor <cursoragent@cursor.com>
|
Warning Review limit reached
Next review available in: 30 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 (4)
📝 WalkthroughWalkthroughV2 launch handling captures prior server sessions, polls persisted settings and TCP readiness, waits for Unity to become idle, and reports structured readiness results for fresh, existing, and restarted launches. ChangesV2 launch readiness
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant Dispatcher
participant V2Settings
participant V2Server
CLI->>Dispatcher: launch V2 project
Dispatcher->>V2Settings: read previous session ID
Dispatcher->>V2Server: poll settings and TCP port
V2Server-->>Dispatcher: server ready or timeout
Dispatcher-->>CLI: structured launch-ready response
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 |
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 `@cli/dispatcher/internal/dispatcher/launch_v2_lockfile.go`:
- Around line 89-105: Use a shorter, dedicated timeout for the
waitForFreshUnityLockfile call in the V2 launch flow, matching the intended
approximately 5-second lockfile detection window instead of
launchReadinessTimeout. Keep launchReadinessTimeout for waitForV2ServerReady so
the full readiness wait begins only after the fresh lockfile is detected.
In `@cli/dispatcher/internal/dispatcher/launch_v2_server_ready.go`:
- Around line 193-204: Update waitForExistingV2LaunchReadiness to call
spinner.Stop() immediately when waitForV2ServerReady returns an error, before
invoking clierrors.WriteClassifiedError. Keep the deferred cleanup and
successful response flow unchanged.
- Around line 35-62: Update v2ServerReadyTimeoutError to store the timeout used
by waitForV2ServerReady, and populate that field when constructing the error. In
ToCLIError, derive Details["TimeoutSeconds"] from the stored timeout instead of
the global launchReadinessTimeout, while preserving the existing project-root
and error metadata.
🪄 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: 9ebd93ef-d956-40cb-9955-2a40c45fdb40
📒 Files selected for processing (7)
cli/dispatcher/internal/dispatcher/launch.gocli/dispatcher/internal/dispatcher/launch_deps.gocli/dispatcher/internal/dispatcher/launch_ready.gocli/dispatcher/internal/dispatcher/launch_test.gocli/dispatcher/internal/dispatcher/launch_v2_lockfile.gocli/dispatcher/internal/dispatcher/launch_v2_server_ready.gocli/dispatcher/internal/dispatcher/launch_v2_server_ready_test.go
Store the wait timeout on v2ServerReadyTimeoutError so Details match the call site, and stop the launch spinner before writing the existing-V2 readiness failure so stderr is not mixed with spinner frames. Co-authored-by: Cursor <cursoragent@cursor.com>
Cover domainreload.lock busy waits, keep FailNow out of test goroutines, stop the spinner on lockfile timeout, constantize the loopback host, use V2-specific focus vibe logs, and warn when previous sessionId read fails for a reason other than missing files. Co-authored-by: Cursor <cursoragent@cursor.com>
Summary
uloop launchagainst V2 projects no longer returns early after only seeing a freshUnityLockfile(or immediately for an already-running Editor). It now waits until the V2 TCP server is actually reachable.UserSettings/UnityMcpSettings.jsonserverSessionIdgeneration compare (not mtime), TCP connect to the advertisedcustomPort, and absence ofTemp/compiling.lock/Temp/domainreload.lock.-r, launch focuses the Editor once so V2'sdelayCall-based server start can tick. V2 has no equivalent of V3'sEditorApplicationTickBridge.SignalTick; without focus, a backgrounded idle Editor can sit forever with an emptyserverSessionId.ServerReady: true,ProjectIpcReady: false) instead of "V2 server readiness was not checked".Why sessionId, not mtime
Settings mtime is rewritten by many unrelated paths. Domain reload start can write
isServerRunning: truewith the oldserverSessionId, so mtime freshness falsely looks ready. A newserverSessionIdis written only when listen succeeds.Timeout
Probe timeout reuses V3 launch's 10 minutes (
launchReadinessTimeout), not the V2 CLI's 3-minute launch readiness window, so V2 and V3 launch wait budgets stay aligned.Test plan
scripts/check-go-cli.sh(format / vet / lint / test / rebuild).tmpfallback / ignore.bak), session generation, lock busy, port change, and timeout classificationwaitForV2ServerReady(fresh / existing /-rsession capture / timeout / focus-before-probe including focus failure)UnityPipelineTestwithdist/darwin-arm64/uloop:Ready/ServerReady true,ProjectIpcReady false, message containsV2 uloop server is readycompilesucceeds after readiness (first attempt hit a V2 CLI tools-cache sync race; immediate retry succeeded withSuccess: true)AlreadyRunning: truelaunch -r→Restarted: truelaunch -q→Quit: trueUNITY_STARTUP_TIMEOUTwhile Unity was alive butisServerRunning: false/ emptyserverSessionId/ no port listener — confirming the timeout path and the backgrounded-Editor diagnosisuloop launch -ron this checkout →Success/Ready/ProjectIpcReady true