Repository navigation
feat: uloop status reports whether Unity can take a command now, without side effects - #3163
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
⛔ Files ignored due to path filters (6)
📒 Files selected for processing (22)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 1 remain after this review. 📝 WalkthroughWalkthroughThe change adds ChangesEditor status bridge
CLI readiness report
Command registration and skill guidance
Priority: ⬇️ Low Estimated code review effort: 4 (Complex) | ~45 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant StatusCommand
participant UnityIPC
participant UnityCliLoopExecutionRouter
participant EditorStatusBridgeCommand
StatusCommand->>UnityIPC: request get-editor-status
UnityIPC->>UnityCliLoopExecutionRouter: deliver status command
UnityCliLoopExecutionRouter->>EditorStatusBridgeCommand: read status and tick timing
EditorStatusBridgeCommand-->>UnityCliLoopExecutionRouter: return status response
UnityCliLoopExecutionRouter-->>UnityIPC: return response
UnityIPC-->>StatusCommand: provide Editor status
Merge Risk: ⚪ Minimal · up to This adds a read-only Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The command reports readiness without executing a tool or gaining additional permissions. No new attack path was found in the inspected flow, but concurrent polling and mixed-version operation have not been validated. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 59.69% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 129 functions across 27 files. (5 skipped: 5 unsupported.)
✨ Finishing Touches 💡 1📝 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 |
uloop status needs to report which command holds the single-flight slot without entering it. TryEnter cannot be reused for that: it takes the slot, and its revocation pass starts the grace timer of a cancelled execute-dynamic-code lease, so merely asking for the status would change when that lease is taken back. - ToolExecutionSession.GetSnapshot reads the holder's name, elapsed seconds, and phase under the session lock and nothing else. - The application layer converts the snapshot into UnityCliLoopExecutionStatus, because Infrastructure cannot see the Domain types; the phase leaves as its wire name like the busy error.
uloop status needs one Editor round trip that reports whether a command can run now, and it must answer even while the Editor main thread is blocked, so it can tell a frozen Editor from a busy one. - The router handles get-editor-status before its main-thread switch and builds the answer only from thread-safe values: the execution slot snapshot, the cached play and compile state, and the main-thread liveness counter. It is not listed as an internal command, because those are switched to the main thread first. - Before answering it signals an Editor tick. An idle Editor's main loop can sleep until something wakes it, so the stall counter grows while nothing is wrong; waking it lets the CLI ask again a second later and tell a sleeping loop from a blocked one. - The play state cache now also records isCompiling and isUpdating, refreshed on the same Editor update. HasEditorState is true only when both are recorded, so defaults are never reported as readings.
uloop status wakes the Editor loop with SignalTick before it reads the stall counter, and SignalTick wakes tick. The counter was recorded only on update, so a loop woken that way could still read as stalled, and uloop status would report a sleeping Editor as blocked. EditorMainThreadDispatcher already runs uloop's main-thread work on both update and tick, so recording both keeps the counter about whether uloop work can run now. The heartbeat's mainThreadStallSeconds and the busy error's secondsSinceLastMainThreadTick now read lower while only tick runs; uloop work does run then, so not calling it stalled is the intended direction. The 5, 30, and 300 second thresholds are unchanged.
uloop status reports whether the Editor can take a command now, and it needs the project's IPC endpoint, so it belongs to the project runner. Registering it in the shared command registry also gives it help, completion, and a line in list --names, and makes the dispatcher hand it to the runner instead of treating it as an unknown tool. The fixtures that enumerate every native command are updated to match.
Agents had no side-effect-free way to ask whether the Editor can take a command: launch must not be used as a health check, sending a tool takes the single-flight slot, execute-dynamic-code runs code, and the connection retry can bring the Editor window to the front. status sends one get-editor-status request with a 5 second timeout and prints one JSON report whose State is Busy, Starting, MainThreadBlocked, Compiling, ImportingAssets, Ready, NotResponding, ServerUnavailable, NotRunning, or Unreachable. It exits 0 only for Ready, and it never retries the connection, focuses Unity, or takes the execution slot. - The order of the Editor states is the contract: a running command explains a busy main thread, and the compile values are refreshed by the main thread, so they are stale while it is blocked. - An idle Editor loop can sleep until something wakes it. When the first answer reads as blocked, status asks once more a second later, after the Editor woke its loop, and judges from that answer alone. - A refused or dropped connection is told apart by whether this project's Unity process runs. A sandbox denial stays an error, so a sandboxed agent is never told that Unity is closed, and an Editor error is checked before the disconnect check, which also matches message text such as "EOF". - A busy answer without elapsed seconds is rejected as unexpected, because the Editor always sends them and the Busy message needs them.
--help can only list the usage and a one-line summary, so an agent needs the skill to learn the states, their exit codes, which fields each state prints, and why status is safe to poll. The skill also says that a sandbox denial is an error rather than NotRunning, and that a ServerUnavailable which does not clear is the only Safe Mode signal. - status --help now ends with the instruction to load uloop-status. - The generated .claude and .agents copies are regenerated with skills install, and the README skill lists count 21 skills.
…read The test ran on the main thread, where a switch to the main thread completes at once, so it still passed when the get-editor-status branch switched to the main thread first. That switch is the contract uloop status depends on, and only a manual mutation in a real Editor showed it. - The test now registers a dispatcher that reports a background thread and holds its queue, so a switch leaves the task unfinished and the status assertion fails before the await instead of hanging. - The queueing dispatcher and the step that puts the Editor dispatcher back move out of MainThreadSwitcherTests into shared test doubles, so both fixtures use one copy.
…router The glossary said internal bridge commands are routed by InternalBridgeCommandRouter. get-editor-status is one, but the execution router answers it before switching to the Editor main thread, because uloop status must answer while that thread is blocked.
Both main and this branch changed shared CLI inputs, so their stamp hashes conflicted. Two hashes cannot be merged by hand: the rebase kept main's stamps and they are recomputed here from the rebased tree with scripts/stamp-release-inputs.sh.
hatayama
force-pushed
the
feat/uloop-status
branch
from
October 5, 2026 23:45
9f9b24d to
4cea548
Compare
The Windows CI job hung for ten minutes in this test. It started a fake server that status never contacts, so the server's Accept was still waiting when the test closed the listener, and go-winio's Close can then wait forever: the pending Accept takes the close signal, and when its connect ends with any error other than the closed-listener one, the listener loop goes back to waiting while Close waits for it to finish. The test now points status at the endpoint the other argument-error tests use, which is never contacted. A probe sent before the argument check would still fail the test, because it would fail to connect, look up the Unity process, and print a state report instead of the argument error.
This was referenced Oct 5, 2026
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
uloop statuscommand reports whether the Unity Editor for this project can take a uloop command right now, as one JSON report with aState, and exits 0 only when that state isReady.User Impact
uloop launchmust not be used as a health check (it brings the window to the front), sending any tool takes the single-flight slot, probing withexecute-dynamic-coderuns code, and the connection retry can focus the Editor.uloop statusanswers in a fraction of a second whether a command is running (with its name, elapsed time, and phase), Unity is compiling or importing, the main thread is blocked, Unity is idle, the server is not accepting connections while Unity runs, or Unity is not running at all.NotRunning, so an agent is not told that Unity is closed when only its sandbox blocked the connection.States
States are checked top to bottom, and the first match is reported.
BusyStartingMainThreadBlockedCompilingEditorApplication.isCompilingis trueImportingAssetsEditorApplication.isUpdatingis trueReadyNotRespondingServerUnavailableNotRunningUnreachableBusycomes beforeMainThreadBlockedbecause a running command such asrun-testslegitimately keeps the main thread busy.MainThreadBlockedcomes beforeCompilingandImportingAssetsbecause those values are refreshed on the main thread, so they are stale while it is blocked.Ready, withIsPlayingandIsPausedset.get-editor-status), an unreadable answer, and an unknown option.Why Safe Mode is not reported as such
In Safe Mode, Unity does not load uloop's Editor code (a native whitelist decides what loads), and Safe Mode leaves nothing the CLI could read:
EditorUtility.isInSafeModeis only a native call and no marker file is written. From the CLI, Safe Mode cannot be told apart from "the process runs but its server is not there", sostatusreportsServerUnavailable. Its message and the skill say that aServerUnavailablewhich does not clear is the Safe Mode signal.Why status wakes the Editor and asks again
An idle Editor's main loop can sleep until something wakes it, so the "seconds since the main thread last ran" counter can grow while nothing is wrong. uloop's own main-thread work always wakes the loop first, and the existing BUSY guidance trusts the stall value only after that wake-up.
get-editor-statushands no work to the main thread, so without a wake-up it could call a sleeping Editor blocked, and an agent would then restart a healthy Editor withuloop launch -r.So the Editor signals a tick before it answers, and the CLI asks once more about a second later, only when the first answer reads as
MainThreadBlocked, and decides from the second answer alone. The second probe reads a fresh answer; it is not a connection retry, and any failure ends the probe.Changes
Editor (package): a new internal bridge command,
get-editor-status, is handled in the router before the main-thread switch, so it answers while the main thread is blocked. It reads only thread-safe values:It is deliberately not in the internal bridge command list, because commands in that list switch to the main thread.
Main-thread liveness is now recorded on
EditorApplication.tickas well asupdate. uloop runs its main-thread work on both, and the wake-up signal drivestick, so the counter now means "can uloop work run now".mainThreadStallSecondsand the BUSY responsesecondsSinceLastMainThreadTicknow stay small while onlytickruns. uloop's main-thread work does run during that time, so no longer calling it stalled is the correct direction.CLI (project runner):
statusis a runner-owned native command.Skill and docs:
uloop-statusskill covers the states, output fields, and notes, andstatus --helpnow ends by pointing at it..claudeand.agentscopies are regenerated withskills install.get-editor-statusas the one that is answered before the main-thread switch, outsideInternalBridgeCommandRouter, and says why.Release inputs:
cli/commonchanged, so bothshared-inputs-stamp.jsonfiles are restamped. The protocol version is not bumped, because this adds a command without changing the wire format.The C# and Go sides ship in one PR, so the automatically merged Unity package release cannot publish the Editor side without the CLI side.
Known behavior
execute-dynamic-codewhose grace period has passed still reads asBusyuntil the next command reclaims the slot, because reading never revokes a lease.Startingis defensive: the state cache is filled before the server starts, so it should rarely appear.Verification
Unity EditMode (local Editor, filtered to the changed classes)
ToolExecutionSessionTests: 33/33. Observing cancellation in the read path makes the grace-timer test fail (checked by mutation).UnityCliLoopEditorStateSnapshotTests: 3/3.EditorStatusBridgeCommandTests|UnityCliLoopToolRegistryTests|SetCodeOptimizationBridgeCommandTests: 47/47. Turning&&into||inHasEditorStatefails exactly the two tests where only one of the two states is cached.UnityCliLoopToolRegistryTests|MainThreadSwitcherTests: 45/45.get-editor-statusrouter test registers a dispatcher that reports a background thread and holds its queue. On the main thread, where the test runs, a switch to the main thread would otherwise complete at once.Expected: RanToCompletion / But was: WaitingForActivation) without hanging. Reverted, the class passes 38/38.JsonRpcHeartbeatTests: 10/10.uloop compile: 0 errors and 0 warnings. CA1502 code complexity: no finding above 15.uloop compilereports 0 errors. Its one warning (CS0414) is in an unchanged test fixture from main, reported because main's changes recompiled the test assembly.Go (local, macOS)
statuscommand tests: one test per row of the input table (28), plus--helpand routing.common,dispatcher,release-automation, andproject-runnereach passgolangci-lint fmt --diff,go vet,golangci-lint run(0 issues), andgo test.mkdirunder/tmp, and a Unix socketbind) were skipped locally. CI runs them.Acceptstill waited when the test closed it once hung the job for ten minutes.cli/.golangci-complexity.yml) onproject-runnerandcommon: 0 issues.scripts/check-file-length.sh: no file over 500 SLOC.project-runneris at 95.3 (baseline 95.2).dispatcher(94.2) andrelease-automation(96.4) equal their baselines.commonvalue measured on macOS cannot be compared with the Linux baseline, so it was confirmed with the Linuxbuild-clicoverage in CI after rebasing onto main:common94.8 (baseline 94.8),dispatcher94.3 (baseline 94.2),project-runner95.3 (baseline 95.2), andrelease-automation96.4 (baseline 96.4). Measured the same way on macOS,origin/mainand this branch also have identical coverage in everycommonpackage.scripts/build-go-cli.shcannot fetchgo-winresin the local sandbox, so the darwin-arm64 binaries were built directly withgo build. CI builds every platform.check-skill-sizeandscripts/sync-tool-docs.sh --check: pass.go vetandgo testpass incommon,dispatcher, andproject-runner, with the same two sandbox skips.scripts/sync-tool-docs.shleaves the catalog unchanged, andcheck-skill-sizeandcheck-release-triggers --base origin/mainpass.Real Editor (darwin-arm64 dev binaries, Editor in the background)
Readywith exit 0.SecondsSinceLastMainThreadTickwas 0.0006–0.082 s, and no sample took over 1 s, so the second probe never ran.execute-dynamic-codesleeping 20 s on the main threadBusy(execute-dynamic-code,Executing, exit 1). 10 s later it was stillBusy, with a 13.0 s stall.MainThreadBlocked(8.1 s stall, exit 1). The call took 1.06 s including the second probe. 15 s later,Ready.statuswaited 5.05 s and reportedNotResponding(context deadline exceeded), so the 0.04 s answer in #2 comes from not waiting for the main thread. Reverted.Ready(max stall 0.098 s). In this environment the idle Editor kept updating about ten times a second, so a sleeping loop could not be reproduced. Reverted.NotRunning,UnityProcessRunning: false,ConnectionErrorset, exit 1.NotRunning.uloop status --helpuloop-statusskill.git status --porcelainand the listings of.uloopandTempwere identical before and after the idle samples.In #3, this Editor had not run
delayCallsince startup, so the main thread was blocked from a self-removing one-shotupdatehandler instead ofdelayCall.