feat: Delegate V2 projects through the V3 dispatcher#1807
Conversation
Use the resolved packages-lock semver before scanning PackageCache so stale cache entries cannot select the wrong legacy CLI. Preserve safe git dependency fallback and report actionable V2 guidance when multiple cached versions make selection ambiguous.
Resolve the current Unity package generation before loading project-runner pins so V3 artifacts left after a downgrade cannot suppress V2 delegation. Keep malformed or ambiguous detection fail-open to the existing pinned V3 path, and document the precedence rule.
Trust PackageCache only for git dependencies or missing lock entries because local and unknown package sources resolve outside that cache. Fail open to the pinned V3 runner for every other non-semver source, and align the detection comment with resolved-package precedence.
|
Warning Review limit reached
Next review available in: 29 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 Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe dispatcher now detects Unity V2 projects, installs matching V2 CLI versions into a versioned cache, delegates eligible commands through Node.js, preserves dispatcher-owned commands, adds structured guidance, and updates English and Japanese installation documentation. ChangesV2 Dispatcher Support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant Dispatcher
participant UnityProject
participant V2Cache
participant V2CLI
User->>Dispatcher: run project command
Dispatcher->>UnityProject: inspect manifest, lockfile, and cache
UnityProject-->>Dispatcher: resolved V2 package version
Dispatcher->>V2Cache: install or reuse versioned CLI
V2Cache-->>Dispatcher: CLI entrypoint
Dispatcher->>V2CLI: execute original arguments
V2CLI-->>User: forward stdout and stderr
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 |
![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
🤖 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/dispatcher_v2_detect.go`:
- Around line 48-72: Update detectV2DispatcherProject and
dispatcherPackageLockEntryForProject so lock-entry presence remains found=true
even when Version is missing or empty, allowing the existing non-git source
guard to run. When no lock entry exists, inspect the manifest dependency
specification and permit dispatcherPackageCacheVersions fallback only for an
explicitly compatible source; reject local, embedded, and unknown dependencies.
Add coverage for these dependency types with missing or empty lock versions.
In `@cli/dispatcher/internal/dispatcher/run_dispatcher.go`:
- Around line 188-197: Update tryRunDetectedDispatcherV2Project to pass the err
returned by deps.runV2CLI into dispatcherV2ProjectDetectedError instead of
discarding it, while preserving the existing success path. Extend
dispatcherV2ProjectDetectedError to accept the execution error and include its
details in the returned error envelope’s Details map alongside the existing
project information.
🪄 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
Run ID: a85fdb34-15c5-4a36-8976-7ec4a0d4f06b
📒 Files selected for processing (13)
README.mdREADME_ja.mdcli/common/errors/error_envelope.gocli/dispatcher/internal/dispatcher/dispatcher_v2_delegate.gocli/dispatcher/internal/dispatcher/dispatcher_v2_delegate_test.gocli/dispatcher/internal/dispatcher/dispatcher_v2_detect.gocli/dispatcher/internal/dispatcher/dispatcher_v2_detect_test.gocli/dispatcher/internal/dispatcher/dispatcher_v2_install.gocli/dispatcher/internal/dispatcher/dispatcher_v2_install_test.gocli/dispatcher/internal/dispatcher/dispatcher_v2_run.gocli/dispatcher/internal/dispatcher/run_dispatcher.gocli/dispatcher/shared-inputs-stamp.jsoncli/project-runner/shared-inputs-stamp.json
| clierrors.WriteErrorEnvelope(stderr, dispatcherV2ProjectDetectedError(projectRoot, v2Project)) | ||
| return true, 1 | ||
| } | ||
|
|
||
| code, err := deps.runV2CLI(ctx, v2Project.PackageVersion, args, stdout, stderr) | ||
| if err == nil { | ||
| return true, code | ||
| } | ||
| clierrors.WriteErrorEnvelope(stderr, dispatcherV2ProjectDetectedError(projectRoot, v2Project)) | ||
| return true, 1 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Do not swallow the underlying execution error.
When runV2CLI fails, the returned err is swallowed, and the user is only presented with the generic "requires Node.js 22 or later" message. This makes diagnosing other failures—such as network errors during npm install, read-only cache directories, or missing npm—very difficult.
Consider passing the error to dispatcherV2ProjectDetectedError and including it in the Details map so the actual failure reason is preserved in the error envelope.
🛠️ Proposed changes to preserve the execution error
Update the call sites in tryRunDetectedDispatcherV2Project:
- if v2Project.PackageVersion == "" {
- clierrors.WriteErrorEnvelope(stderr, dispatcherV2ProjectDetectedError(projectRoot, v2Project))
- return true, 1
- }
-
- code, err := deps.runV2CLI(ctx, v2Project.PackageVersion, args, stdout, stderr)
- if err == nil {
- return true, code
- }
- clierrors.WriteErrorEnvelope(stderr, dispatcherV2ProjectDetectedError(projectRoot, v2Project))
- return true, 1
+ if v2Project.PackageVersion == "" {
+ clierrors.WriteErrorEnvelope(stderr, dispatcherV2ProjectDetectedError(projectRoot, v2Project, nil))
+ return true, 1
+ }
+
+ code, err := deps.runV2CLI(ctx, v2Project.PackageVersion, args, stdout, stderr)
+ if err == nil {
+ return true, code
+ }
+ clierrors.WriteErrorEnvelope(stderr, dispatcherV2ProjectDetectedError(projectRoot, v2Project, err))
+ return true, 1And update dispatcherV2ProjectDetectedError (further down in the file) to populate the Details map:
-func dispatcherV2ProjectDetectedError(projectRoot string, v2Project dispatcherV2Project) clierrors.CLIError {
+func dispatcherV2ProjectDetectedError(projectRoot string, v2Project dispatcherV2Project, runErr error) clierrors.CLIError {
if len(v2Project.PackageVersionCandidates) > 0 {
return clierrors.CLIError{
ErrorCode: clierrors.ErrorCodeV2ProjectDetected,
Phase: clierrors.ErrorPhaseProjectResolve,
Message: "This Unity project uses uloop V2, but its package version could not be resolved unambiguously.",
Retryable: true,
SafeToRetry: true,
ProjectRoot: projectRoot,
NextActions: []string{
"Open the Unity project once so Unity Package Manager can refresh `Packages/packages-lock.json`, then retry the command.",
"As a last resort, run `npx uloop-cli@2 <command>` from this project.",
},
Details: map[string]any{
"V2PackageVersionCandidates": v2Project.PackageVersionCandidates,
},
}
}
- return clierrors.CLIError{
+ errEnvelope := clierrors.CLIError{
ErrorCode: clierrors.ErrorCodeV2ProjectDetected,
Phase: clierrors.ErrorPhaseProjectResolve,
Message: "This Unity project uses uloop V2 and requires Node.js 22 or later.",
Retryable: true,
SafeToRetry: true,
ProjectRoot: projectRoot,
NextActions: []string{
"Install Node.js 22 or later, then retry the command.",
"As a last resort, run `npx uloop-cli@" + v2Project.PackageVersion + " <command>` from this project.",
},
Details: map[string]any{
"V2PackageVersion": v2Project.PackageVersion,
},
}
+ if runErr != nil {
+ errEnvelope.Details["RunError"] = runErr.Error()
+ }
+ return errEnvelope
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| clierrors.WriteErrorEnvelope(stderr, dispatcherV2ProjectDetectedError(projectRoot, v2Project)) | |
| return true, 1 | |
| } | |
| code, err := deps.runV2CLI(ctx, v2Project.PackageVersion, args, stdout, stderr) | |
| if err == nil { | |
| return true, code | |
| } | |
| clierrors.WriteErrorEnvelope(stderr, dispatcherV2ProjectDetectedError(projectRoot, v2Project)) | |
| return true, 1 | |
| clierrors.WriteErrorEnvelope(stderr, dispatcherV2ProjectDetectedError(projectRoot, v2Project, nil)) | |
| return true, 1 | |
| } | |
| code, err := deps.runV2CLI(ctx, v2Project.PackageVersion, args, stdout, stderr) | |
| if err == nil { | |
| return true, code | |
| } | |
| clierrors.WriteErrorEnvelope(stderr, dispatcherV2ProjectDetectedError(projectRoot, v2Project, err)) | |
| return true, 1 |
🤖 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 `@cli/dispatcher/internal/dispatcher/run_dispatcher.go` around lines 188 - 197,
Update tryRunDetectedDispatcherV2Project to pass the err returned by
deps.runV2CLI into dispatcherV2ProjectDetectedError instead of discarding it,
while preserving the existing success path. Extend
dispatcherV2ProjectDetectedError to accept the execution error and include its
details in the returned error envelope’s Details map alongside the existing
project information.
Keep the dispatcher entrypoint below the repository architecture limit by colocating V2 detection, delegation, and guidance with the V2 execution path. This is a mechanical responsibility split with no behavior change.
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 `@cli/dispatcher/internal/dispatcher/dispatcher_v2_run.go`:
- Around line 26-73: Preserve the runV2CLI failure when constructing the error
envelope: update dispatcherV2ProjectDetectedError to accept the underlying
error, pass err from the failed execution path, and include its details in the
returned CLIError Details map. Keep the package-version-resolution branch
behavior unchanged and ensure the existing no-error path still returns the CLI
exit code.
🪄 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
Run ID: a45b1c20-899b-4b7e-90ef-27b069c5d609
📒 Files selected for processing (2)
cli/dispatcher/internal/dispatcher/dispatcher_v2_run.gocli/dispatcher/internal/dispatcher/run_dispatcher.go
💤 Files with no reviewable changes (1)
- cli/dispatcher/internal/dispatcher/run_dispatcher.go
Keep lock-entry presence independent from its version so non-git sources cannot fall through to stale PackageCache data. When the lock entry is missing, allow cache fallback only for explicit Git manifest dependencies. Preserve the underlying V2 execution failure in structured error details.
Addressed in 294c2c2; all review threads are resolved and the latest CodeRabbit check passed.
1 participant
Summary
Detection and safety
Resolved semver lock entries take precedence over stale project-runner pins. Git dependencies can fall back to the active project PackageCache, while local, embedded, unknown, malformed, and mixed-generation states fail open to the existing pinned-runner path. Ambiguous V2-only cache state returns V2-specific recovery guidance instead of choosing a CLI version arbitrarily.
Known limitation
A git-installed project with both V2 and V3 PackageCache entries cannot yet distinguish a downgrade with a stale V3 pin from an upgrade with a valid V3 pin using a verified cross-version signal. The dispatcher therefore keeps the pinned-runner path for mixed generations. This is the same outcome as before this feature and avoids destructive misdelegation, but automatic V2 recovery does not cover that state.
Recovery: reopen the project so Unity Package Manager refreshes its project cache. For a confirmed V2 downgrade, remove the stale
.uloop/project-runner-pin.jsonif it remains. Follow-up investigation: #1806.Validation
scripts/check-go-cli.sh--versionreturned 2.2.0 andget-logssucceededget-logssucceeded with package version 2.1.6compilereturned success with zero errors and zero warningsReview
The cumulative branch was reviewed manually, by the advisor, and with the structured Codex autoreview helper against
origin/v3-beta. Three actionable package-resolution findings were fixed with regression coverage. The final mixed-generation git-cache finding was consciously rejected as a non-regression and tracked in #1806 because correct upgrade/downgrade discrimination requires a separately validated Unity-version signal.