feat: Pause points are easier to drive: unified naming, embedded logs, diagnosis hints, and frame stepping#1309
Merged
hatayama merged 31 commits intoJun 11, 2026
Conversation
AI-agent usability feedback from three independent E2E sessions showed the mixed naming between the "pause point" concept and the "debug-break" command family caused real failures (agents guessed clear-pause-point and got Unknown tool). Unify every user-facing name on pause-point with no backward compatibility: - CLI commands: enable/clear/wait-for-pause-point, pause-point-status, and the internal bridge commands get/clear-pause-point-status - Error codes: PAUSE_POINT_NOT_ENABLED / WAIT_TIMEOUT / EXPIRED / CLEARED - Response fields: InterruptedByPausePoint, PausePointId, PausePointHitCount - Runtime marker API: UnityCliLoopDebug.Break(id) -> UloopPausePoint.Pause(id) - Skill renamed to uloop-wait-for-pause-point; regenerated copies updated - Bump cli contract and MINIMUM_REQUIRED_CLI_VERSION to 3.0.0-beta.29 because the Unity package now depends on the renamed bridge commands
Agents reported that clearing an already-hit one-shot marker returned ClearedCount: 0 with the message "cleared.", which read as a contradiction. Clear now resolves expiry first and reports a state-specific message for already-hit, expired, and already-cleared markers, and bulk clear says "No active pause points to clear." when nothing was armed.
Agents found timeout root-causing hard: the same error covers a missed code path, a stopped PlayMode, and an already-paused Editor. The timeout error now carries details.hint with a deterministic diagnosis for those three states so the next action is obvious without extra status probing.
After a hit, agents always followed up with get-logs --search-text <marker-id> to read the values logged next to the marker, and that extra call could race a busy paused Editor. With --include-matching-logs (and an optional --matching-logs-max-count, default 10), the wait response now embeds the matching log entries directly; timeouts attach them best-effort under details.matchingLogs. Log fetch failures are ignored so a hit never turns into an error.
…skill Fold the new --include-matching-logs flag into the quick check template so the standard loop needs one less paused-Editor call, and point timeout debugging at error.details.hint and error.details.matchingLogs first.
A marker whose enable-pause-point timeout window ends before the wait deadline surfaces as PAUSE_POINT_EXPIRED instead of a wait timeout, and that error carried no hint at all. An AI-agent E2E session hit exactly this: the agent saw repeated expired errors with no guidance and could not tell an id mismatch from an exhausted enable window. Reuse the PlayMode/paused diagnoses and add an expired-specific hint that points at the enable-side --timeout-seconds.
Two AI agents independently asked for a clear-all command that already exists, so the discoverability gap is in the skill doc, not the CLI. Also explain that PAUSE_POINT_EXPIRED now carries the same diagnosis hint and what an expired marker means for the enable-side timeout.
In two supervised AI-agent E2E sessions, agents attached --include-matching-logs to nearly every wait call (44 of ~57 waits) and one explicitly asked for it to be the default, so the opt-in flag only added friction. Retire the flag, keep --matching-logs-max-count, and make the empty/absent distinction explicit: an empty MatchingLogs array means no matching log exists, while an absent field means the best-effort fetch itself failed.
The timeout error detail used camelCase matchingLogs while the hit response used PascalCase MatchingLogs for the same data, which an AI-agent E2E session flagged as confusing. One spelling now covers both surfaces. Also state explicitly in the skill that log embedding is always on and the old opt-in flag no longer exists, because all three agents tripped over stale flag docs before self-recovering.
Codex-sandboxed agents cannot exec /bin/ps, so launch died at process detection even though Unity was running and every other command worked. All three supervised agents had to discover by hand that the Editor was reachable. When the scan errors on a plain launch, probe the project IPC instead: a response proves Unity is running, so report AlreadyRunning with an unfocused-window note. Restart and quit still fail because killing requires a process id.
Supervised AI agents reported that shell-quoting long multi-line C# snippets for --code is error-prone. --code-file reads the source from a file CLI-side and forwards it as the Code parameter, so no Unity schema change is needed. Combining it with --code is rejected to avoid one silently shadowing the other.
Agents running back-to-back uloop commands hit UNITY_SERVER_BUSY and had to hand-roll retries even though the error is marked SafeToRetry: a busy response means Unity never executed the request. Reuse the existing 10s retry window to poll until the running tool frees the slot, and surface the original busy envelope unchanged if it never does.
Agents saw Press/KeyDown return Success while gameplay never observed wasPressedThisFrame, and had to rewrite game code to hand-rolled edge detection to cope. The response now carries PressEdgeObserved: a monitor on InputSystem.onAfterUpdate checks the edge inside gameplay input updates (editor updates excluded, since presses consumed there are invisible to gameplay polling). Editor-tick polling was tried first and reproduced the exact miss, which confirms the monitor placement.
…dates Generated copies under .claude/ and .agents/ follow the source skill edits: launch IPC-probe fallback, execute-dynamic-code --code-file, PressEdgeObserved, and the MatchingLogs naming note.
The IPC-probe fallback pushed launch.go past the 500-line architecture limit, so the cohesive focus-logging block moves to launch_focus_log.go.
Supervised agents repeatedly ran uloop commands in parallel without knowing the CLI is single-flight by design, and per-skill documentation would be redundant and easily skipped. The busy message is the one guaranteed touchpoint, so it now states the contract and that the CLI already retried. Also prefer reporting a busy response over a transport error cut short by the expiring retry window, because busy is the truer diagnosis of why the request never ran.
Block-breaker E2E sessions showed fast-progressing games can leave the marker code path behind (state returns to Ready/GameOver) before the wait starts, so the never-hit timeout hint now tells agents to re-trigger the code path instead of only re-checking the marker id.
An agent terminated a whole session after misreading this timeout as an Editor freeze, even though Unity answered the next command instantly. The message now states the Editor is not necessarily frozen, and the next actions walk through a responsiveness probe (get-logs) before retrying compile, with launch -r only as the last resort.
Two experiment runs in a row hit the same failure mode: .codex/skills was installed once, later installs only passed --claude/--agents, and Codex agents kept reading retired flags from the stale copy. Install now detects any known target that already holds a uloop skill and refreshes it even when its flag is omitted, and the install help documents this behavior.
Graphics.Blit (and the internal window-capture call) leave the destination assigned to RenderTexture.active, so saving the "previous" target after them captured the temporary itself. Restoring that value and then releasing the temporary emitted "Releasing render texture that is set to be RenderTexture.active\!" warnings, which polluted the zero-warning check agents run at the end of E2E sessions. All three capture paths now save before the call and restore before release.
Pause-point interruption is the most common path in marker-driven E2E sessions, yet the interrupted result dropped PressEdgeObserved and agents saw null exactly where they wanted the edge evidence. Press and KeyDown now carry their observation into the interrupted result, and the skill doc spells out when the field is null (KeyUp and timeouts only).
One key press or mouse action can hit several markers in the same
frame, but the response only named the latest one, so agents had to
issue extra status calls to find the rest. The registry now keeps all
hit snapshots (cleared alongside the existing latest-hit bookkeeping),
and keyboard/mouse interruption responses expose them as a
PausePointHits array of {Id, HitCount} in hit order.
Block-breaker style games advance state during E2E setup, so agents asked for a way to arrange scenarios before the marker is overrun. The pause point skill now shows the execute-dynamic-code Time.timeScale freeze/restore pattern as a setup aid, with an explicit caveat that it is not paused-frame proof.
Propagates the PausePointHits field docs, the PressEdgeObserved null conditions, and the fast-progressing-game time-freeze pattern into the generated .claude/.agents copies via uloop skills install.
Time.timeScale = 0 is unreliable for E2E setup: projects that read unscaled time keep advancing, and the value silently leaks into the next PlayMode session (an agent hit exactly this). control-play-mode now exposes Step - the Editor's Next Frame button (EditorApplication.Step) - which advances one frame and stays paused independent of timeScale. The pause point skill now teaches Pause/Step/Play for fast-progressing games instead of freezing time.
Contributor
|
Warning Review limit reached
More reviews will be available in 5 minutes. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughMigrates debug-break workflows to pause-point semantics across runtime, editor, CLI, telemetry, tests, and documentation; adds CLI pause-point wait/status, dynamic --code-file support, PlayMode Step action, launch IPC fallback, server_busy retry logic, and related tests and docs. ChangesPause-point and CLI support
Sequence Diagram(s)sequenceDiagram
participant RunProjectLocal
participant PausePointWait
participant UnityIPC
participant UloopPausePointRegistry
RunProjectLocal->>PausePointWait: run wait-for-pause-point --id <id>
PausePointWait->>UnityIPC: query pause-point status (poll)
UnityIPC-->>PausePointWait: status (Enabled / Hit / Expired / Cleared)
PausePointWait->>UloopPausePointRegistry: best-effort clear on timeout
PausePointWait-->>RunProjectLocal: JSON hit payload (MatchingLogs) or classified error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 79 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Packages/src/Editor/FirstPartyTools/ControlPlayMode/Skill/SKILL.md (1)
9-9:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the one-line capability summary to include
Step.The short description still says
(play/stop/pause)althoughStepis now a first-class action. Keeping this line aligned avoids mixed guidance in quick scans.Also applies to: 21-21
🤖 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/Skill/SKILL.md` at line 9, The one-line capability summary in SKILL.md still lists "(play/stop/pause)" but the Skill now supports the Step action; update that short description to include "step" (for example "(play/stop/pause/step)") so quick scans are accurate, and make the same change where the summary appears a second time in the file to keep both occurrences consistent.Packages/src/Runtime/PausePoints/UloopPausePointRegistry.cs (1)
61-78:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
ClearAll()should check for expiry before clearing to matchClear(id)behavior.
Clear(string id)at line 48 callsExpireIfNeeded(now)before determining the appropriate message, allowing it to distinguish between Enabled, Expired, and Hit entries. However,ClearAll()does not callExpireIfNeeded()on any entries before marking them as cleared.This creates an inconsistency: an entry that was enabled with a 30-second timeout and hasn't been hit after 35 seconds will be in the
Enabledstate whenClearAll()is called (becauseExpireIfNeeded()hasn't been invoked).ClearAll()will mark it asClearedwith the default message, when it should first transition toExpiredand potentially provide a different treatment.Suggested fix
public static UloopPausePointClearAllResult ClearAll() { DateTime now = NowUtc(); int clearedCount = 0; foreach (UloopPausePointEntry entry in Entries.Values) { + entry.ExpireIfNeeded(now); if (entry.Status == UloopPausePointStatus.Cleared) { continue; } entry.MarkCleared(); clearedCount++; }🤖 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/Runtime/PausePoints/UloopPausePointRegistry.cs` around lines 61 - 78, ClearAll() currently marks every UloopPausePointEntry as cleared without first evaluating time-based expiry; update ClearAll() to call entry.ExpireIfNeeded(now) (using the same DateTime now = NowUtc()) for each entry before checking entry.Status and calling entry.MarkCleared(), so entries that should transition to UloopPausePointStatus.Expired are handled the same way as Clear(string id) does; keep clearedCount and ClearLatestHitSnapshot() behavior unchanged.
🤖 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 `@Assets/Tests/PlayMode/SimulateKeyboardTests.cs`:
- Around line 228-231: The test configures a global pause provider with
InputSystemUpdateHelper.ConfigurePauseProviderForTests(...) but doesn't
guarantee ResetPauseProviderForTests() runs if an assertion or yield fails; wrap
the section that sets the provider and waits (the
UloopPausePoint.Pause("space-press"); ConfigurePauseProviderForTests(() =>
true); yield return WaitForTask(task); sequence) in a try/finally so that
InputSystemUpdateHelper.ResetPauseProviderForTests() is always called in the
finally block; apply the same try/finally pattern to the similar block around
lines 267-271.
In `@cli/internal/cli/execution_errors.go`:
- Line 7: The message in the error struct contains a hardcoded "180000ms"
duration; change it to derive from the existing constant by using
compileWaitTimeout.Milliseconds() (or toolReadinessTimeout.Milliseconds()) so
the string is built from the constant instead of a magic number; locate the
Message assignment in execution_errors.go for the relevant error (the entry
using compileWaitTimeout) and format the message to include the computed
milliseconds value.
In `@Packages/src/Editor/FirstPartyTools/SimulateMouseUi/Skill/SKILL.md`:
- Around line 81-85: The paragraph mixes CLI invocation styles (e.g.,
uloop-wait-for-pause-point, uloop-get-logs vs. bare get-logs/get-hierarchy and
simulate-mouse-ui); normalize all CLI commands to a single canonical prefix (use
the "uloop-" prefix) and update occurrences in this block to
uloop-wait-for-pause-point, uloop-get-logs, uloop-get-hierarchy,
uloop-find-game-objects, uloop-execute-dynamic-code, and uloop-simulate-mouse-ui
(and any other bare CLI names) so examples and references are consistent and
copy/paste-safe; ensure surrounding prose reflects the renamed commands and
update any inline examples or lists that mention those commands.
---
Outside diff comments:
In `@Packages/src/Editor/FirstPartyTools/ControlPlayMode/Skill/SKILL.md`:
- Line 9: The one-line capability summary in SKILL.md still lists
"(play/stop/pause)" but the Skill now supports the Step action; update that
short description to include "step" (for example "(play/stop/pause/step)") so
quick scans are accurate, and make the same change where the summary appears a
second time in the file to keep both occurrences consistent.
In `@Packages/src/Runtime/PausePoints/UloopPausePointRegistry.cs`:
- Around line 61-78: ClearAll() currently marks every UloopPausePointEntry as
cleared without first evaluating time-based expiry; update ClearAll() to call
entry.ExpireIfNeeded(now) (using the same DateTime now = NowUtc()) for each
entry before checking entry.Status and calling entry.MarkCleared(), so entries
that should transition to UloopPausePointStatus.Expired are handled the same way
as Clear(string id) does; keep clearedCount and ClearLatestHitSnapshot()
behavior unchanged.
🪄 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: 549f7baf-c75b-4808-af6a-6fd23bff3ac0
⛔ Files ignored due to path filters (1)
Packages/src/Runtime/PausePoints/UloopPausePoint.cs.metais excluded by none and included by none
📒 Files selected for processing (78)
.agents/skills/uloop-control-play-mode/SKILL.md.agents/skills/uloop-execute-dynamic-code/SKILL.md.agents/skills/uloop-get-logs/SKILL.md.agents/skills/uloop-launch/SKILL.md.agents/skills/uloop-simulate-keyboard/SKILL.md.agents/skills/uloop-simulate-mouse-input/SKILL.md.agents/skills/uloop-simulate-mouse-ui/SKILL.md.agents/skills/uloop-wait-for-debug-break/SKILL.md.agents/skills/uloop-wait-for-pause-point/SKILL.md.claude/skills/uloop-control-play-mode/SKILL.md.claude/skills/uloop-execute-dynamic-code/SKILL.md.claude/skills/uloop-get-logs/SKILL.md.claude/skills/uloop-launch/SKILL.md.claude/skills/uloop-simulate-keyboard/SKILL.md.claude/skills/uloop-simulate-mouse-input/SKILL.md.claude/skills/uloop-simulate-mouse-ui/SKILL.md.claude/skills/uloop-wait-for-debug-break/SKILL.md.claude/skills/uloop-wait-for-pause-point/SKILL.mdAssets/Tests/Editor/ControlPlayModeUseCaseTests.csAssets/Tests/Editor/PausePointTests.csAssets/Tests/Editor/ToolSettingsUseCaseTests.csAssets/Tests/Editor/UnityCliLoopToolRegistryTests.csAssets/Tests/PlayMode/SimulateKeyboardTests.csAssets/Tests/PlayMode/SimulateMouseInputTests.csPackages/src/Editor/Application/UseCases/ToolSettingsUseCase.csPackages/src/Editor/CliOnlyTools~/Launch/Skill/SKILL.mdPackages/src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.mdPackages/src/Editor/Domain/CliConstants.csPackages/src/Editor/FirstPartyTools/Common/InputSimulation/UnityCliLoopInputSimulationTypes.csPackages/src/Editor/FirstPartyTools/ControlPlayMode/ControlPlayModeSchema.csPackages/src/Editor/FirstPartyTools/ControlPlayMode/ControlPlayModeUseCase.csPackages/src/Editor/FirstPartyTools/ControlPlayMode/Skill/SKILL.mdPackages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Skill/SKILL.mdPackages/src/Editor/FirstPartyTools/GetLogs/Skill/SKILL.mdPackages/src/Editor/FirstPartyTools/PausePoint/PausePointTools.csPackages/src/Editor/FirstPartyTools/Screenshot/EditorWindowCaptureUtility.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/FirstPartyTools/SimulateMouseUi/Skill/SKILL.mdPackages/src/Editor/Infrastructure/Api/InternalBridgeCommandRouter.csPackages/src/Editor/Infrastructure/Api/PausePointStatusBridgeCommand.csPackages/src/Editor/ToolContracts/UnityCliLoopConstants.csPackages/src/Runtime/PausePoints/UloopPausePoint.csPackages/src/Runtime/PausePoints/UloopPausePointRegistry.cscli/contract.jsoncli/internal/cli/busy_status.gocli/internal/cli/command_help.gocli/internal/cli/command_registry.gocli/internal/cli/completion_options.gocli/internal/cli/connection_retry.gocli/internal/cli/connection_retry_test.gocli/internal/cli/debug_break_wait.gocli/internal/cli/debug_break_wait_test.gocli/internal/cli/dynamic_code_file.gocli/internal/cli/dynamic_code_file_test.gocli/internal/cli/error_envelope.gocli/internal/cli/error_envelope_test.gocli/internal/cli/execution_errors.gocli/internal/cli/launch.gocli/internal/cli/launch_focus_log.gocli/internal/cli/launch_ready.gocli/internal/cli/launch_test.gocli/internal/cli/native_tool_settings.gocli/internal/cli/pause_point_errors.gocli/internal/cli/pause_point_logs.gocli/internal/cli/pause_point_wait.gocli/internal/cli/pause_point_wait_test.gocli/internal/cli/run.gocli/internal/cli/skills.gocli/internal/cli/skills_display.gocli/internal/cli/skills_test.gocli/internal/tools/default-tools.json
💤 Files with no reviewable changes (4)
- .agents/skills/uloop-wait-for-debug-break/SKILL.md
- .claude/skills/uloop-wait-for-debug-break/SKILL.md
- cli/internal/cli/debug_break_wait.go
- cli/internal/cli/debug_break_wait_test.go
A failure during capture, blit, or pixel read previously leaked the temporary RenderTexture and left RenderTexture.active pointing at it. try/finally is the repository-approved pattern for resource release, so all three capture paths now restore the caller's active target and release the temporary even when the capture work throws.
The expired-window busy masking applied to every final error, so a request that actually reached Unity and failed with a real RPC error could be overwritten by a stale server_busy seen earlier in the retry window. The mask now requires the final attempt to be undispatched, which matches its intent of explaining window-expiry transport errors. Also derive the compile wait timeout message from compileWaitTimeout instead of hardcoding 180000ms; the envelope test keeps the literal expectation as a regression pin.
The pause point inspection sections mixed skill-style names (uloop-get-logs) with bare command names (get-logs), which invited copy/paste mistakes. All command references now use the runnable form (uloop get-logs), matching the examples sections. Installed copies regenerated.
Contributor
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/internal/cli/connection_retry_test.go`:
- Around line 518-560: The test uses tight sleeps to exercise the retry window
(serverConnectionRetryTimeout/serverConnectionRetryPoll) and can be flaky;
replace the implicit timing with explicit synchronization: add a channel (e.g.,
acceptedCh) that the connection-handling goroutine closes/signals immediately
after unityipc.Write(conn, []byte(accepted)) succeeds, then in the caller wait
on that channel before proceeding to the failure/timeout path instead of the
hard-coded time.Sleep(80 * time.Millisecond); compute any additional delay
relative to serverConnectionRetryTimeout (e.g., wait accepted signal then sleep
serverConnectionRetryTimeout*2 or a fixed larger margin) so the second request
is guaranteed accepted before the retry deadline is evaluated—update the
goroutine handling listener.Accept, the place that writes the accepted JSON, and
the subsequent sleep to use the channel-based coordination (referencing
serverConnectionRetryTimeout, serverConnectionRetryPoll, unityipc.Read/Write,
busy, failure, and the accepted JSON variable).
🪄 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: 9a470f10-83eb-4649-8b17-537e3af2e102
📒 Files selected for processing (13)
.agents/skills/uloop-simulate-keyboard/SKILL.md.agents/skills/uloop-simulate-mouse-input/SKILL.md.agents/skills/uloop-simulate-mouse-ui/SKILL.md.claude/skills/uloop-simulate-keyboard/SKILL.md.claude/skills/uloop-simulate-mouse-input/SKILL.md.claude/skills/uloop-simulate-mouse-ui/SKILL.mdPackages/src/Editor/FirstPartyTools/Screenshot/EditorWindowCaptureUtility.csPackages/src/Editor/FirstPartyTools/SimulateKeyboard/Skill/SKILL.mdPackages/src/Editor/FirstPartyTools/SimulateMouseInput/Skill/SKILL.mdPackages/src/Editor/FirstPartyTools/SimulateMouseUi/Skill/SKILL.mdcli/internal/cli/connection_retry.gocli/internal/cli/connection_retry_test.gocli/internal/cli/execution_errors.go
✅ Files skipped from review due to trivial changes (2)
- .claude/skills/uloop-simulate-mouse-ui/SKILL.md
- .agents/skills/uloop-simulate-keyboard/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (10)
- Packages/src/Editor/FirstPartyTools/SimulateMouseInput/Skill/SKILL.md
- Packages/src/Editor/FirstPartyTools/SimulateMouseUi/Skill/SKILL.md
- cli/internal/cli/execution_errors.go
- Packages/src/Editor/FirstPartyTools/Screenshot/EditorWindowCaptureUtility.cs
- .agents/skills/uloop-simulate-mouse-input/SKILL.md
- .claude/skills/uloop-simulate-keyboard/SKILL.md
- Packages/src/Editor/FirstPartyTools/SimulateKeyboard/Skill/SKILL.md
- .agents/skills/uloop-simulate-mouse-ui/SKILL.md
- cli/internal/cli/connection_retry.go
- .claude/skills/uloop-simulate-mouse-input/SKILL.md
There was a problem hiding this comment.
1 issue found across 13 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
The 40ms retry window left only ~30ms for the second request to dial and receive its accepted ack, which could flake under CI scheduler jitter. The window is now 150ms, and the failure response is held for twice the window starting only after the accepted ack is written, so the window has always expired when the failure arrives.
CI exposed a race in the expired-window busy masking: the connection read/write deadline shares its wall-clock instant with the retry context, and the i/o timeout can fire microseconds before the context reports expiry, so the raw transport error leaked through both the terminal branch and the no-process probe branch. The mask no longer compares against the window deadline: after a busy response in the window, any terminal non-RPC transport error reports busy (parent cancellation still wins). Real RPC answers are recognized with errors.As instead of the dispatch flag, which also fixes the dispatched read-timeout case the flag-based guard regressed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
User Impact
enable-pause-point,wait-for-pause-point,UloopPausePoint.Pause("id"), …).wait-for-pause-pointalways embeds the marker-matching log entries (MatchingLogs) in hit responses and timeout details, so a separateget-logscall while paused is unnecessary.clear-pause-pointexplains why nothing was cleared (already hit, expired, already cleared) instead of a bareClearedCount: 0.simulate-keyboard/simulate-mouse-inputresponses interrupted by a pause point now includePressEdgeObserved(did the game actually see the press edge?) and aPausePointHitslist covering every marker hit during the simulation.control-play-mode --action Stepadvances exactly one frame and stays paused (the Editor's Next Frame button), independent ofTime.timeScale. The pause point skill now teaches a Pause → arrange → Step → Play recipe for fast-progressing games instead of the timeScale freeze pattern, which silently leaks into the next PlayMode session and does not stop unscaled-time projects.uloop skills installauto-refreshes every target that already has uloop skills installed, even when its flag is omitted — stale generated skill copies can no longer drift out of sync with the CLI.COMPILE_WAIT_TIMEOUTand the busy envelope no longer read as a frozen Editor; they explain the single-flight contract and suggest probing with a light command before restarting Unity.execute-dynamic-code --code-fileaccepts long code from a file instead of a shell string.Changes
--code-file.PlayModeAction.StepviaEditorApplication.Step()with a PlayMode guard, pause point hit snapshot registry exposed asPausePointHits,PressEdgeObservedcarried through pause-point interruptions, state-aware clear messages, screenshot RenderTexture.active handling..claude/and.agents/regenerated.get-pause-point-status,clear-pause-point-status), soMINIMUM_REQUIRED_CLI_VERSIONwas raised.Verification
scripts/check-go-cli.shandscripts/build-go-cli.shpass; Go unit tests cover the rename, hints, embedded logs, and auto-refresh.uloop compile: 0 errors / 0 warnings. EditMode and PlayMode test suites pass (4 EditMode failures verified pre-existing on origin/v3-beta and untouched).