feat: Pause Unity at named debug breaks for inspection#1283
Conversation
Introduce an editor-only pause point marker API modeled on Unity's conditional call-site pattern, plus first-party arm and clear tools backed by an internal status bridge. This replaces direct Debug.Break usage with an armed marker flow that can expire or be cleared safely.
Add the native wait-for-pause-point command so agents can poll marker hits without occupying the normal Unity tool slot. Document the arm/wait/inspect flow and point input simulation skills at pause points for ambiguous gameplay verification.
Align the public marker API with the Unity CLI Loop C# naming style and keep future debug helpers under UnityCliLoopDebug.
Clear stale terminal pause point states during bulk clear, remove first-party schema description attributes, and keep the architecture dependency test aligned with the public pause point runtime assembly.
Align the command names with UnityCliLoopDebug.Break by exposing arm-debug-break, wait-for-debug-break, and clear-debug-break instead of the pause-point wording.
Use enable-debug-break for the user-facing CLI command so the action reads as enabling a UnityCliLoopDebug.Break marker. Keep the internal armed status contract while aligning help text, tool metadata, and skill guidance with the clearer command name.
Treat a pause during keyboard or mouse input observation as a successful debug-break interruption so the tool slot is released and agents can inspect the paused Unity state instead of hitting Busy forever.
Expose debug-break-status as a CLI-only marker inspection command, surface PlayMode Domain Reload risk when enabling markers before Play, and include running/requested tool plus Unity play state in Busy responses so agents can diagnose paused-editor stalls.
Clarify marker placement for keyboard and mouse verification, document debug-break-status, and sync generated agent skills so agents can discover the workflow without relying on prompt-specific instructions.
Report enabled terminology and pause/play details in debug break wait failures so agents can distinguish a missed marker path from an already-paused Editor or marker lifetime issue.
Rename the wait status elapsed field to describe that it is measured since enable-debug-break, and include marker id/count when keyboard or mouse simulation is interrupted by a UnityCliLoopDebug.Break marker. This keeps input interruption responses actionable without mixing log or scene inspection responsibilities into the wait command.
Flush keyboard and mouse cleanup when Unity is paused so debug-break inspection does not leave injected input state active after the tool returns.
Wait one runtime frame after each SmoothDelta injection so gameplay Update can hit a debug-break marker before the next input update is scheduled.
Require the CLI release that includes the new native debug-break commands so package users get an update prompt instead of unknown-command failures.
Clarify short input durations, add compile hints for test asmdef setup, and avoid reading Unity editor state from background-thread busy responses so parallel commands return server_busy instead of internal Unity thread errors.
Keep the v3-beta branch diff clean for git diff --check after generated Unity meta files were added.
Preserve unknown play state in server-busy responses and add a final status probe before clearing wait-for-debug-break timeouts.
Show wait-for-debug-break and debug-break-status alongside built-in tools, and make the native CLI dispatch respect disabled tool settings so the checkbox state matches command behavior.
📝 WalkthroughWalkthroughImplements a complete debug-break pause-point feature allowing users to place named pause markers in Unity gameplay code, enable/clear them via CLI tools, and detect interruptions in keyboard/mouse input simulations with full debug metadata (marker ID, hit count) reported in response payloads. Updates tool busy exceptions to capture editor play-state context; extends JSON-RPC error responses accordingly. ChangesDebug Break Pause-Point Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
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/Common/InputSystem/InputSystemUpdateHelper.cs (1)
36-75:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftMake
ApplyOnNextConfiguredUpdatepause-aware.
WaitForObservationFrames/WaitForPressLifetimenow returnInputSimulationWaitOutcome.Paused, but callers can still block before reaching those pause-aware waits:InputSystemUpdateHelper.ApplyOnNextConfiguredUpdatecompletes only whenInputSystem.onBeforeUpdatefires (orctis canceled). If Unity gets paused after the callback is subscribed but before the nextonBeforeUpdate, the awaited task may never complete, so flows likeSimulateMouseInputUseCase.ExecuteSmoothDeltacan get stuck holding the tool’s execution/held input state until resume.Update
ApplyOnNextConfiguredUpdateto short-circuit onEditorApplication.isPaused(or use the explicit-update fallback approach used byApplyOnExplicitUpdate) so callers always reach the existing pause-handling paths.🤖 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/Common/InputSystem/InputSystemUpdateHelper.cs` around lines 36 - 75, ApplyOnNextConfiguredUpdate can hang if the editor is paused after subscribing to InputSystem.onBeforeUpdate; after resolving targetUpdateType, detect EditorApplication.isPaused and short-circuit to the explicit-update path by returning ApplyOnExplicitUpdate(apply, targetUpdateType, ct) (same approach used when InputUpdateTypeResolver.RequiresExplicitUpdate() is true). Keep the existing onBeforeUpdate subscription logic for the non-paused case and preserve cancellation handling via ct.Register as before.
🧹 Nitpick comments (1)
cli/internal/cli/debug_break_wait_test.go (1)
303-327: ⚡ Quick winCover
debug-break-statusin the disabled-settings regression test.The shared guard in
cli/internal/cli/native_tool_settings.gocovers bothwait-for-debug-breakanddebug-break-status, but this test only proves the wait path. Adding the status command as a second table row would protect the shared settings gate from future constant or dispatch drift.🤖 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/internal/cli/debug_break_wait_test.go` around lines 303 - 327, The test TestRunProjectLocalWaitForDebugBreakRespectsToolSettings only exercises debugBreakWaitCommandName; extend it to also exercise the peer command ("debug-break-status" or the constant debugBreakStatusCommandName) so the shared guard in native_tool_settings.go is covered for both paths—either convert the test into a small table-driven loop over []string{debugBreakWaitCommandName, debugBreakStatusCommandName} (or the literal "debug-break-status") and run the same RunProjectLocal/assertions for each command, or duplicate the RunProjectLocal/assertion block for the status command; keep the same expectations (exit code 1 and error envelope fields matching errorCodeToolDisabled and the command name).
🤖 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/Common/InputSystem/InputSystemUpdateHelper.cs`:
- Around line 36-75: ApplyOnNextConfiguredUpdate can hang if the editor is
paused after subscribing to InputSystem.onBeforeUpdate; after resolving
targetUpdateType, detect EditorApplication.isPaused and short-circuit to the
explicit-update path by returning ApplyOnExplicitUpdate(apply, targetUpdateType,
ct) (same approach used when InputUpdateTypeResolver.RequiresExplicitUpdate() is
true). Keep the existing onBeforeUpdate subscription logic for the non-paused
case and preserve cancellation handling via ct.Register as before.
---
Nitpick comments:
In `@cli/internal/cli/debug_break_wait_test.go`:
- Around line 303-327: The test
TestRunProjectLocalWaitForDebugBreakRespectsToolSettings only exercises
debugBreakWaitCommandName; extend it to also exercise the peer command
("debug-break-status" or the constant debugBreakStatusCommandName) so the shared
guard in native_tool_settings.go is covered for both paths—either convert the
test into a small table-driven loop over []string{debugBreakWaitCommandName,
debugBreakStatusCommandName} (or the literal "debug-break-status") and run the
same RunProjectLocal/assertions for each command, or duplicate the
RunProjectLocal/assertion block for the status command; keep the same
expectations (exit code 1 and error envelope fields matching
errorCodeToolDisabled and the command name).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 838f0dcc-3af0-437a-ba3f-3b5a83f1f4e1
⛔ Files ignored due to path filters (25)
Assets/Tests/Editor/InputSimulationDurationFormatterTests.cs.metais excluded by none and included by noneAssets/Tests/Editor/PausePointTests.cs.metais excluded by none and included by noneAssets/Tests/Editor/ToolSettingsUseCaseTests.cs.metais excluded by none and included by noneAssets/Tests/Editor/UnityCLILoop.Tests.Editor.asmdefis excluded by none and included by noneAssets/Tests/Editor/UnityCliLoopToolExecutionServiceTests.cs.metais excluded by none and included by noneAssets/Tests/PlayMode/UnityCLILoop.Tests.PlayMode.asmdefis excluded by none and included by nonePackages/src/Editor/CliOnlyTools~/PausePoint.metais excluded by none and included by nonePackages/src/Editor/CliOnlyTools~/PausePoint/Skill.metais excluded by none and included by nonePackages/src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.md.metais excluded by none and included by nonePackages/src/Editor/FirstPartyTools/Common/InputSimulation/InputSimulationDurationFormatter.cs.metais excluded by none and included by nonePackages/src/Editor/FirstPartyTools/PausePoint.metais excluded by none and included by nonePackages/src/Editor/FirstPartyTools/PausePoint/PausePointTools.cs.metais excluded by none and included by nonePackages/src/Editor/FirstPartyTools/PausePoint/UnityCLILoop.FirstPartyTools.PausePoint.Editor.asmdefis excluded by none and included by nonePackages/src/Editor/FirstPartyTools/PausePoint/UnityCLILoop.FirstPartyTools.PausePoint.Editor.asmdef.metais excluded by none and included by nonePackages/src/Editor/FirstPartyTools/SimulateKeyboard/UnityCLILoop.FirstPartyTools.SimulateKeyboard.Editor.asmdefis excluded by none and included by nonePackages/src/Editor/FirstPartyTools/SimulateMouseInput/UnityCLILoop.FirstPartyTools.SimulateMouseInput.Editor.asmdefis excluded by none and included by nonePackages/src/Editor/FirstPartyTools/UnityCLILoop.FirstPartyTools.Editor.asmdefis excluded by none and included by nonePackages/src/Editor/Infrastructure/Api/PausePointStatusBridgeCommand.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/UnityCLILoop.Infrastructure.asmdefis excluded by none and included by nonePackages/src/Runtime/PausePoints.metais excluded by none and included by nonePackages/src/Runtime/PausePoints/AssemblyInfo.cs.metais excluded by none and included by nonePackages/src/Runtime/PausePoints/UloopPausePointRegistry.cs.metais excluded by none and included by nonePackages/src/Runtime/PausePoints/UnityCLILoop.PausePoints.Runtime.asmdefis excluded by none and included by nonePackages/src/Runtime/PausePoints/UnityCLILoop.PausePoints.Runtime.asmdef.metais excluded by none and included by nonePackages/src/Runtime/PausePoints/UnityCliLoopDebug.cs.metais excluded by none and included by none
📒 Files selected for processing (56)
.agents/skills/uloop-simulate-keyboard/SKILL.md.agents/skills/uloop-simulate-mouse-input/SKILL.md.agents/skills/uloop-wait-for-debug-break/SKILL.md.claude/skills/uloop-simulate-keyboard/SKILL.md.claude/skills/uloop-simulate-mouse-input/SKILL.md.claude/skills/uloop-wait-for-debug-break/SKILL.mdAssets/Tests/Editor/CliSetupApplicationServiceTests.csAssets/Tests/Editor/CompileSessionResultServiceTests.csAssets/Tests/Editor/InputSimulationDurationFormatterTests.csAssets/Tests/Editor/JsonRpcProcessorCliVersionGateTests.csAssets/Tests/Editor/OnionAssemblyDependencyTests.csAssets/Tests/Editor/PausePointTests.csAssets/Tests/Editor/ToolSettingsUseCaseTests.csAssets/Tests/Editor/UnityCliLoopToolExecutionServiceTests.csAssets/Tests/Editor/UnityCliLoopToolRegistryTests.csAssets/Tests/PlayMode/SimulateKeyboardTests.csAssets/Tests/PlayMode/SimulateMouseInputTests.csPackages/src/Editor/Application/UnityCliLoopEditorStateGuard.csPackages/src/Editor/Application/UnityCliLoopToolBusyException.csPackages/src/Editor/Application/UnityCliLoopToolExecutionService.csPackages/src/Editor/Application/UseCases/ToolSettingsUseCase.csPackages/src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.mdPackages/src/Editor/FirstPartyTools/Common/InputSimulation/InputSimulationDurationFormatter.csPackages/src/Editor/FirstPartyTools/Common/InputSimulation/UnityCliLoopInputSimulationTypes.csPackages/src/Editor/FirstPartyTools/Common/InputSystem/InputSystemUpdateHelper.csPackages/src/Editor/FirstPartyTools/Compile/CompileSessionResultService.csPackages/src/Editor/FirstPartyTools/PausePoint/PausePointTools.csPackages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardResponse.csPackages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardTool.csPackages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardUseCase.csPackages/src/Editor/FirstPartyTools/SimulateKeyboard/Skill/SKILL.mdPackages/src/Editor/FirstPartyTools/SimulateMouseInput/SimulateMouseInputResponse.csPackages/src/Editor/FirstPartyTools/SimulateMouseInput/SimulateMouseInputTool.csPackages/src/Editor/FirstPartyTools/SimulateMouseInput/SimulateMouseInputUseCase.csPackages/src/Editor/FirstPartyTools/SimulateMouseInput/Skill/SKILL.mdPackages/src/Editor/Infrastructure/Api/InternalBridgeCommandRouter.csPackages/src/Editor/Infrastructure/Api/JsonRpcProcessor.csPackages/src/Editor/Infrastructure/Api/PausePointStatusBridgeCommand.csPackages/src/Editor/ToolContracts/UnityCliLoopConstants.csPackages/src/Runtime/PausePoints/AssemblyInfo.csPackages/src/Runtime/PausePoints/UloopPausePointRegistry.csPackages/src/Runtime/PausePoints/UnityCliLoopDebug.cscli/internal/cli/busy_status.gocli/internal/cli/command_help.gocli/internal/cli/command_registry.gocli/internal/cli/completion.gocli/internal/cli/completion_options.gocli/internal/cli/debug_break_wait.gocli/internal/cli/debug_break_wait_test.gocli/internal/cli/error_envelope.gocli/internal/cli/error_envelope_test.gocli/internal/cli/execution_errors.gocli/internal/cli/native_tool_settings.gocli/internal/cli/run.gocli/internal/cli/skills_targets.gocli/internal/tools/default-tools.json
💤 Files with no reviewable changes (1)
- cli/internal/cli/completion.go
There was a problem hiding this comment.
3 issues found across 81 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Keep the latest play/pause state on the main thread so busy responses created before the request reaches the main thread can still include diagnostics. Also document the New Input System prerequisite for debug-break workflows that use simulated input.
Avoid prompting agents to change Player Settings while following the wait-for-debug-break workflow. Keep the generated skill copies in sync with the source skill.
Describe wait-for-debug-break as a runtime checkpoint tool so agents consider it for gameplay, UI, async, physics, and state transitions instead of only simulated input checks.
Describe wait-for-debug-break as a normal PlayMode E2E checkpoint tool instead of a fallback for failed screenshots, and sync the generated skill copies.
Include frame-specific PlayMode bugs in the wait-for-debug-break trigger so agents use it for development debugging as well as E2E transition proof.
Guide agents to use wait-for-debug-break when they would otherwise rely on sleep, repeated polling, or a second state read for transient PlayMode state.
Require CLI 3.0.0-beta.26 so the Unity package rejects older native CLIs that do not include the debug-break command contract.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Assets/Tests/Editor/CliSetupApplicationServiceTests.cs (1)
41-41: ⚡ Quick winAssert against
CliConstantsinstead of string literals.These tests duplicate version/tag literals, which can drift from the source contract on future bumps.
Proposed change
- Assert.That(service.GetMinimumRequiredCliVersion(), Is.EqualTo("3.0.0-beta.26")); + Assert.That(service.GetMinimumRequiredCliVersion(), Is.EqualTo(CliConstants.MINIMUM_REQUIRED_CLI_VERSION)); ... - Assert.That(service.GetMinimumRequiredCliReleaseTag(), Is.EqualTo("cli-v3.0.0-beta.26")); + Assert.That(service.GetMinimumRequiredCliReleaseTag(), Is.EqualTo(CliConstants.MINIMUM_REQUIRED_CLI_RELEASE_TAG));Also applies to: 52-52
🤖 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/CliSetupApplicationServiceTests.cs` at line 41, Replace the hard-coded version string in the test with the shared constant from the CLI contract: change the assertion to compare service.GetMinimumRequiredCliVersion() against CliConstants.MinimumRequiredCliVersion (and update the other occurrence mentioned) so the test uses the canonical source of truth instead of a literal.
🤖 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.
Nitpick comments:
In `@Assets/Tests/Editor/CliSetupApplicationServiceTests.cs`:
- Line 41: Replace the hard-coded version string in the test with the shared
constant from the CLI contract: change the assertion to compare
service.GetMinimumRequiredCliVersion() against
CliConstants.MinimumRequiredCliVersion (and update the other occurrence
mentioned) so the test uses the canonical source of truth instead of a literal.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 34b4b2cd-a1dc-4895-a224-5f45e05a8a6b
📒 Files selected for processing (7)
.agents/skills/uloop-wait-for-debug-break/SKILL.md.claude/skills/uloop-wait-for-debug-break/SKILL.mdAssets/Tests/Editor/CliSetupApplicationServiceTests.csPackages/src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.mdPackages/src/Editor/Domain/CliConstants.cscli/contract.jsoncli/internal/tools/default-tools.json
✅ Files skipped from review due to trivial changes (2)
- Packages/src/Editor/Domain/CliConstants.cs
- cli/contract.json
🚧 Files skipped from review as they are similar to previous changes (1)
- cli/internal/tools/default-tools.json
Summary
User Impact
Changes
Verification