feat: pause-point round 10 feedback improvements#1969
Conversation
--list-options is not a real command, so following the old NextActions caused a second failure and hid real flags such as --timeout-seconds. Co-authored-by: Cursor <cursoragent@cursor.com>
Package updates left the CLI project cache stale until a manual sync, so real commands could fail with UNKNOWN_COMMAND. Reuse the get-tool-details IPC serialization path so published bytes match uloop sync. Co-authored-by: Cursor <cursoragent@cursor.com>
Agents repeatedly tried `uloop version` and hit UNKNOWN_COMMAND because only --version existed. Route the new dispatcher-owned command through the shared version output helpers so text and --json stay identical. Co-authored-by: Cursor <cursoragent@cursor.com>
Pause-point workflows already say \"resume\" (--resume-play), but the control-play-mode action enum only had Play, so agents hit invalid-argument errors. Add Resume at the enum tail and document the alias in skills. Co-authored-by: Cursor <cursoragent@cursor.com>
E2E records need both which line was armed and what was captured in one response. Copy ResolvedLine/ResolvedLineText/ResolvedMethod/SnapshotTiming from the enable payload using the same enable-time carry pattern as Warning. Co-authored-by: Cursor <cursoragent@cursor.com>
P3 and P4 touched non-test cli/common sources, so refresh the dispatcher and project-runner shared-input stamps for check-release-triggers. Co-authored-by: Cursor <cursoragent@cursor.com>
The previous schema update replaced the per-action explanations with a short enum list. Restore the original wording and append Resume as an alias of Play. Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughThe change adds ChangesPlay Mode Resume action
Tool catalog cache publishing
Version command
Pause-point await response propagation
Tool option error guidance
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
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/common/tools/default-tools.json`:
- Line 329: Update the action description in the default-tools schema to
document Resume as an alias of Play that resumes a paused session or starts Play
Mode when the Editor is stopped, while preserving the descriptions of the other
actions.
In `@cli/dispatcher/internal/dispatcher/version.go`:
- Around line 28-33: Update the version argument error handling around the
dispatcher’s ArgumentError construction to identify the actual invalid argument,
including trailing arguments after a valid option such as --json, instead of
always using args[1]. Add the documented uloop version --help action to
NextActions, and add regression coverage for both the invalid trailing-argument
report and the help guidance.
In
`@Packages/src/Editor/FirstPartyTools/ControlPlayMode/ControlPlayModeUseCase.cs`:
- Around line 99-100: Update the StatusOnly compile-gate flow in
ControlPlayModeUseCase so PlayModeAction.Resume is normalized to Play before
calling ShouldBlockPlayForCompileErrors. Preserve the existing Resume execution
path through ExecutePlayModeStart and ensure both actions receive identical
compile-blocking behavior.
🪄 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: 538edaf6-080f-4074-ad83-e3d78175f618
⛔ Files ignored due to path filters (1)
Packages/src/Editor/Infrastructure/Api/ToolCatalogCachePublisher.cs.metais excluded by none and included by none
📒 Files selected for processing (25)
.agents/skills/uloop-control-play-mode/SKILL.md.claude/skills/uloop-control-play-mode/SKILL.mdPackages/src/Editor/CompositionRoot/UnityCliLoopApplicationRegistration.csPackages/src/Editor/FirstPartyTools/ControlPlayMode/ControlPlayModeSchema.csPackages/src/Editor/FirstPartyTools/ControlPlayMode/ControlPlayModeUseCase.csPackages/src/Editor/FirstPartyTools/ControlPlayMode/Skill/SKILL.mdPackages/src/Editor/Infrastructure/Api/ToolCatalogCachePublisher.csPackages/src/Editor/ToolContracts/UnityCliLoopConstants.cscli/common/clicore/command_registry.gocli/common/clicore/command_registry_test.gocli/common/tools/default-tools.jsoncli/dispatcher/internal/dispatcher/dispatcher_process.gocli/dispatcher/internal/dispatcher/dispatcher_test.gocli/dispatcher/internal/dispatcher/dispatcher_version.gocli/dispatcher/internal/dispatcher/help_test.gocli/dispatcher/internal/dispatcher/native_command_options.gocli/dispatcher/internal/dispatcher/version.gocli/dispatcher/shared-inputs-stamp.jsoncli/project-runner/internal/projectrunner/pause_point_enable.gocli/project-runner/internal/projectrunner/pause_point_enable_test.gocli/project-runner/internal/projectrunner/pause_point_types.gocli/project-runner/internal/projectrunner/runner_usage_test.gocli/project-runner/internal/projectrunner/tool_params.gocli/project-runner/internal/projectrunner/tool_params_test.gocli/project-runner/shared-inputs-stamp.json
Resume skipped state waiting and compile-block handling because wait logic only recognized Play. Map Resume once at the wait entrypoints, keep the raw RequestedAction in errors, and align StatusOnly/C# gates plus schema/version error wording from review feedback. Co-authored-by: Cursor <cursoragent@cursor.com>
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/src/Editor/FirstPartyTools/ControlPlayMode/ControlPlayModeUseCase.cs (1)
98-100: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftBump the IPC protocol generation for the new
Resumewire action.A newer runner sends
Resumeto Unity, but a previous package generation does not define that action. Without a protocol bump, the exact-match gate can admit an incompatible pair that fails when this request is dispatched. Increment bothcli/common/clicontract/contract.jsonprotocolVersionandCliConstants.REQUIRED_CLI_PROTOCOL_VERSION; ensure the runner pin targets a published runner release advertising that protocol before package release.As per path instructions, “If a change makes a CLI/package from the previous protocol generation unable to interoperate,” both protocol values must be incremented in the same PR.
🤖 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/src/Editor/FirstPartyTools/ControlPlayMode/ControlPlayModeUseCase.cs` around lines 98 - 100, Increment the protocol version in cli/common/clicontract/contract.json and update CliConstants.REQUIRED_CLI_PROTOCOL_VERSION to the same new value for the added PlayModeAction.Resume wire action. Update the runner version pin to a published release advertising that protocol before release, keeping both protocol values synchronized.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
`@Packages/src/Editor/FirstPartyTools/ControlPlayMode/ControlPlayModeUseCase.cs`:
- Around line 98-100: Increment the protocol version in
cli/common/clicontract/contract.json and update
CliConstants.REQUIRED_CLI_PROTOCOL_VERSION to the same new value for the added
PlayModeAction.Resume wire action. Update the runner version pin to a published
release advertising that protocol before release, keeping both protocol values
synchronized.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d8b897d7-bf8b-4b07-a2fc-4ecf739ea4cc
📒 Files selected for processing (7)
Assets/Tests/Editor/ControlPlayModeUseCaseTests.csPackages/src/Editor/FirstPartyTools/ControlPlayMode/ControlPlayModeUseCase.cscli/common/tools/default-tools.jsoncli/dispatcher/internal/dispatcher/dispatcher_test.gocli/dispatcher/internal/dispatcher/version.gocli/project-runner/internal/projectrunner/control_play_mode_wait.gocli/project-runner/internal/projectrunner/control_play_mode_wait_test.go
🚧 Files skipped from review as they are similar to previous changes (3)
- cli/dispatcher/internal/dispatcher/version.go
- cli/dispatcher/internal/dispatcher/dispatcher_test.go
- cli/common/tools/default-tools.json
Summary
Addresses pause-point round 10 CLI UX feedback (no pause-point core bugs; five surrounding CLI experience issues):
NextActionsnow guide touloop <tool> --helpinstead of the non-existent--list-options.uloop/tools.jsonon every domain reload so package updates no longer leave a stale tool cache (UNKNOWN_COMMAND for real tools)uloop version/uloop version --json, matching--version/--version --jsoncontrol-play-mode --action Resumeas an alias ofPlay(pause-point workflows already use "resume" vocabulary)enable-pause-point --awaithit responses now include enable-timeResolvedLine/ResolvedLineText/ResolvedMethod/SnapshotTimingAlso stamps shared release inputs after non-test
cli/commonchanges.Test plan
scripts/check-go-cli.sh(exit 0)uloop syncthen delete.uloop/tools.json,uloop compile(domain reload),cmp→ IDENTICALuloop versionmatchesuloop --version(text and--json)ResumedFromPause: trueuloop compile→ 0 errors / 0 warningsuloop run-testsfull suite: 2361 passed, 0 failed, 7 platform-guarded skips (root status Skipped is pre-existing CLI semantics)