Skip to content

fix: Compile no longer leaves Unity busy after finishing#1237

Merged
hatayama merged 3 commits into
v3-betafrom
feature/hatayama/fix-compile-hang-recovery
May 28, 2026
Merged

fix: Compile no longer leaves Unity busy after finishing#1237
hatayama merged 3 commits into
v3-betafrom
feature/hatayama/fix-compile-hang-recovery

Conversation

@hatayama

Copy link
Copy Markdown
Owner

Summary

  • uloop compile now recovers when Unity finishes compiling but the completion signal is missed.
  • Follow-up commands such as get-logs no longer stay blocked behind a stale busy state.

User Impact

  • Before this change, uloop compile could remain in progress indefinitely even after Unity had stopped compiling, and later commands could keep returning UNITY_SERVER_BUSY until the Editor was restarted.
  • After this change, the compile request completes as indeterminate once Unity is no longer compiling and the finish signal still has not arrived, so agents can inspect logs or retry without restarting Unity.

Changes

  • Watch the full compile lifecycle instead of only waiting for compile start.
  • Preserve the existing compile-start timeout behavior while adding missed-finish recovery.
  • Add focused EditMode coverage for missed completion, normal completion, and start-timeout paths.

Verification

  • Packages/src/Cli~/dist/darwin-arm64/uloop compile --project-path "$(git rev-parse --show-toplevel)"
  • Packages/src/Cli~/dist/darwin-arm64/uloop run-tests --test-mode EditMode --filter-type regex --filter-value ".*CompileLifecycleWatchdogTests.*" --project-path "$(git rev-parse --show-toplevel)"
  • Packages/src/Cli~/dist/darwin-arm64/uloop compile --force-recompile --project-path "$(git rev-parse --show-toplevel)"
  • Packages/src/Cli~/dist/darwin-arm64/uloop get-logs --project-path "$(git rev-parse --show-toplevel)"
  • codex-review v3-beta --parallel-tests ...

Unity can leave the compile command waiting forever if EditorApplication.isCompiling drops before compilationFinished reaches the controller. Watch the compile lifecycle after start and complete the request as indeterminate so the execution slot is released and follow-up commands can run.

- Preserve the existing start-timeout behavior in the watchdog.
- Add focused EditMode tests for missed callback recovery and normal callback completion.
@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 7f03ca17-685a-4211-9cb4-19937aeef227

📥 Commits

Reviewing files that changed from the base of the PR and between cdc4c7f and 562d1b4.

📒 Files selected for processing (2)
  • Assets/Tests/Editor/CompileLifecycleWatchdogTests.cs
  • Packages/src/Editor/FirstPartyTools/Compile/CompileController.cs
🚧 Files skipped from review as they are similar to previous changes (2)
  • Assets/Tests/Editor/CompileLifecycleWatchdogTests.cs
  • Packages/src/Editor/FirstPartyTools/Compile/CompileController.cs

📝 Walkthrough

Walkthrough

Adds CompileLifecycleWatchdog (polling state machine) plus a new protocol constant, integrates it into CompileController to replace start-only polling, produces indeterminate results when finish callbacks are missed, and adds tests that validate start-timeout, missed-callback recovery, and fault propagation.

Changes

Compile Lifecycle Watchdog

Layer / File(s) Summary
Timeout constant and watchdog state machine
Packages/src/Editor/ToolContracts/UnityCliLoopConstants.cs, Packages/src/Editor/FirstPartyTools/Compile/CompileLifecycleWatchdog.cs
Adds COMPILE_FINISH_MISSED_CALLBACK_GRACE_MS = 500 and implements CompileLifecycleWatchdog with injected isEditorCompiling, isRequestCompleted, a poll-delay task, lifecycle callbacks, and WatchAsync that enforces start timeout, observes compile start, detects stopped-compiling → missed-completion after a grace window, and surfaces cancellation.
CompileController lifecycle integration
Packages/src/Editor/FirstPartyTools/Compile/CompileController.cs
TryCompileAsync now starts WatchCompileLifecycleAsync (fire-and-forget). Adds watchdog wiring and fault handling, HandleCompileStoppedWithoutFinishEvent to abort with an indeterminate CompileResult, CreateIndeterminateCompileResult helper, and updates Cleanup() to reset _isCompiling and _isForceCompile.
Watchdog behavior tests
Assets/Tests/Editor/CompileLifecycleWatchdogTests.cs
Adds CompileLifecycleWatchdogTests with async tests covering missed-callback recovery (grace window), no recovery when request completes after start, start-timeout when compile never starts, a test for polling task faults surfacing, and two private isCompiling suppliers (SequenceCompilationState, ConstantCompilationState).

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.54% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically summarizes the main change: fixing a bug where compilation no longer leaves Unity in a busy state after finishing, which aligns directly with the PR's core objective of handling missed completion callbacks.
Description check ✅ Passed The description is directly related to the changeset, clearly explaining the problem (missed completion signals leaving Unity busy), the solution (full lifecycle watching with missed-finish recovery), and user impact.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/hatayama/fix-compile-hang-recovery

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@Packages/src/Editor/FirstPartyTools/Compile/CompileController.cs`:
- Line 165: The call in CompileController.cs currently fire-and-forgets
WatchCompileLifecycleAsync(ct), which lets CompileLifecycleWatchdog.WatchAsync
fault silently and prevents watchdog callbacks (e.g., AbortCompile*,
compilationFinished) from running; instead, capture the returned Task from
WatchCompileLifecycleAsync(ct) and attach an observed continuation (or await it
on a safe background Task) that catches exceptions, logs the error, and invokes
the appropriate abort/cleanup path (e.g., call the AbortCompile* methods or
signal compilationFinished) so faults don't leave the controller waiting;
reference WatchCompileLifecycleAsync, CompileLifecycleWatchdog.WatchAsync,
AbortCompile* and compilationFinished when adding this try/catch/continuation
handling.
🪄 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: eba5330c-57ee-4ff8-803c-bdb07888880f

📥 Commits

Reviewing files that changed from the base of the PR and between ba735f1 and d775c18.

⛔ Files ignored due to path filters (2)
  • Assets/Tests/Editor/CompileLifecycleWatchdogTests.cs.meta is excluded by none and included by none
  • Packages/src/Editor/FirstPartyTools/Compile/CompileLifecycleWatchdog.cs.meta is excluded by none and included by none
📒 Files selected for processing (4)
  • Assets/Tests/Editor/CompileLifecycleWatchdogTests.cs
  • Packages/src/Editor/FirstPartyTools/Compile/CompileController.cs
  • Packages/src/Editor/FirstPartyTools/Compile/CompileLifecycleWatchdog.cs
  • Packages/src/Editor/ToolContracts/UnityCliLoopConstants.cs

Comment thread Packages/src/Editor/FirstPartyTools/Compile/CompileController.cs Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 6 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread Packages/src/Editor/FirstPartyTools/Compile/CompileController.cs Outdated
Comment thread Packages/src/Editor/FirstPartyTools/Compile/CompileController.cs Outdated
Keep the missed-callback recovery from leaving a pending compile request if the watchdog itself faults, and preserve the force-compile result contract for indeterminate recovery results.
@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Actionable comments posted: 0

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 2 files (changes from recent commits).

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread Packages/src/Editor/FirstPartyTools/Compile/CompileController.cs Outdated
Prevent a delayed watchdog fault from aborting a newer compile by carrying the original compile task through fault handling and delayCall recovery. Add focused coverage for matching and stale compile request identity.
@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Actionable comments posted: 0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant