fix: Worker compiler timeouts no longer block indefinitely

[hatayama]

Summary

• Worker compiler timeouts now remain bounded even when redirected output streams do not close promptly after process termination.
• Late stream-read failures are observed so timeout recovery does not leave unobserved task exceptions.

User Impact

• Dynamic code worker setup can return its existing timeout failure instead of holding the shared worker lock indefinitely in a failed compiler-process cleanup.
• Successful compiler execution and compiler diagnostic parsing are unchanged.

Changes

• Reuse one 500 ms termination cleanup limit for both the post-kill process wait and redirected stream draining.
• Replace the timeout path's unbounded stream wait with a bounded, non-throwing drain while preserving the existing worker_compiler_timeout result.
• Observe each stream-read failure independently, plus the aggregate drain failure, whether it happens before or after the bound expires.
• Cover completed and pending stream-drain contracts without launching a real child process. A real orphaned-pipe EditMode test is intentionally avoided because it could leave background work or freeze the Unity Test Runner.

Verification

• Confirmed the initial two tests were Red before implementation because the bounded drain contract did not exist.
• Confirmed the faulted-stream regression test was Red with AggregateException before the review fix.
• dist/darwin-arm64/uloop compile --project-path <PROJECT_ROOT>: 0 errors, 0 warnings
• SharedRoslynCompilerWorkerHostTests: 14 passed
• ExternalCompilerPathResolverTests: 15 passed
• ExternalCompilerMessageParserTests: 1 passed
• StaticFacadeStateGuardTests: 20 passed
• No wire-format or protocol-version change

Refs: R2-31 in the Unity CLI Loop refactoring ToDo.

[coderabbitai]

Warning

Review limit reached

@hatayama, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 15 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: fbe29e3a-4f6b-4ed8-9d9d-35e9029d7652

📥 Commits

Reviewing files that changed from the base of the PR and between f26acbc and 3527c28.

📒 Files selected for processing (2)

• Assets/Tests/Editor/DynamicCodeToolTests/SharedRoslynCompilerWorkerHostTests.cs
• Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerAssemblyBuilder.cs

📝 Walkthrough

Walkthrough

Adds a bounded drain mechanism for compiler stdout/stderr tasks used after killing a timed-out compiler process, introducing a termination timeout constant and two helper methods, plus new unit tests validating drain behavior for pending and completed tasks.

Changes

Compiler stream drain handling

Layer / File(s)	Summary
Timeout handling and drain helpers Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerAssemblyBuilder.cs	Adds WorkerCompilerTerminationTimeoutMilliseconds constant, updates the timeout path to kill the process then bound the wait via a new WaitForCompilerStreamDrain helper, and adds ObserveCompilerStreamFailure to consume faults from stream tasks.
Unit tests for stream drain Assets/Tests/Editor/DynamicCodeToolTests/SharedRoslynCompilerWorkerHostTests.cs	Adds a Threading.Tasks import and two NUnit tests verifying WaitForCompilerStreamDrain returns false for a pending task and true when both stream tasks are already completed.

Estimated code review effort: 2 (Simple) | ~15 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Title check	✅ Passed	The title clearly summarizes the main change: preventing worker compiler timeout recovery from blocking indefinitely.
Description check	✅ Passed	The description matches the changeset and explains the bounded timeout handling, stream drain behavior, and added tests.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/hatayama/bound-worker-compiler-stream-drain

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[cubic-dev-ai]

1 issue found across 2 files

_{You’re at about 93% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.}

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerAssemblyBuilder.cs">

<violation number="1" location="Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SharedRoslynCompilerWorkerAssemblyBuilder.cs:145">
P1: The bounded drain can still throw here when one of the redirected stream tasks faults before the 500 ms wait finishes. `Task.WaitAll(Task[], int)` throws `AggregateException` for completed faulted tasks, and the late-observation hook only runs after `WaitAll` returns `false`, so the timeout recovery path can still escape instead of returning `worker_compiler_timeout`.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

All reported issues were addressed across 2 files (changes from recent commits).

_{You’re at about 93% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.}

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>