Skip to content

fix: execute-dynamic-code returns retryable guidance when runtime is disposed mid-reset#1762

Merged
hatayama merged 1 commit into
feature/dynamic-code-slot-recoveryfrom
fix/ode-disposed-scheduler-retryable
Jul 13, 2026
Merged

fix: execute-dynamic-code returns retryable guidance when runtime is disposed mid-reset#1762
hatayama merged 1 commit into
feature/dynamic-code-slot-recoveryfrom
fix/ode-disposed-scheduler-retryable

Conversation

@hatayama

@hatayama hatayama commented Jul 13, 2026

Copy link
Copy Markdown
Owner

Summary

  • After a server reset or domain reload, execute-dynamic-code that hits a disposed runtime now returns Success=false with clear retry guidance instead of a raw ObjectDisposedException.
  • Foreground warmup treats the same dispose as a quiet incomplete warm so CLI callers are not failed by best-effort warmup.

User Impact

  • Before: Warm could succeed, then Reset disposed the scheduler; the next Execute could surface a raw ODE / opaque failure while the bridge was simply restarting.
  • After: Execute maps that case to a retryable “server restarting” style response (same tone as compile-wait NextActions: retry shortly; uloop launch -r is not first-line). Warm remains silent on ODE.

Design (agreed plan A)

  • Reset means “server restart in progress” → return an explicit tool error so Go’s existing retry/reconnect guidance can apply. Do not re-acquire a facade and retry inside the UseCase (reload race).
  • Do not add a new RPC error type (protocol / CLI classification change) for this PR.
  • Scheduler stays pure C# and may keep throwing ODE; mapping is at the UseCase boundary only.

try-catch exception (documented)

  • catch (ObjectDisposedException) is used under the allowed pattern “catch at a boundary and return an explicit error value.”
  • Catch scope is minimal: only the runtime ExecuteAsync / TryExecuteIfIdleAsync awaits (and the Warm sequence’s runtime work). Not the whole UseCase method.
  • On catch, emit one vibe log: dynamic_code_runtime_restarting.

Warm asymmetry (fixed by tests)

  • Warm ODE → silent no-op / incomplete warm (entered=false style).
  • Execute ODE → Success=false + restarting ErrorMessage + NextActions.

Repro context from PR 6-1

Confirmed pattern: Warm succeeds → Reset → Execute hits disposed facade/scheduler and throws ODE. This PR maps that Execute-path ODE so agents see a recoverable restart signal.

Changes

  • Constants for error message + NextActions in UnityCliLoopConstants.
  • ExecuteDynamicCodeResponse.NextActions + factory helpers for the restarting response.
  • UseCase boundary mapping for Execute / TryExecuteIfIdle / Warm.
  • Two EditMode UseCase tests with an IDynamicCodeExecutionRuntime double (Execute → retryable; Warm → no-op then success).

Verification

  • dist/darwin-arm64/uloop compile --project-path <PROJECT_ROOT> → Success
  • uloop run-tests (regex) for the two new UseCase tests → Passed (2/2)

Test plan

  • Confirm Execute-path ODE returns ERROR_MESSAGE_DYNAMIC_CODE_RUNTIME_RESTARTING and NextActions mentioning retry / launch -r not needed
  • Confirm Warm-path ODE does not fail the subsequent user Execute
  • After umbrella merge, optional Editor gate: warm → reset → execute should be retryable guidance, not raw ODE

Made with Cursor

Review in cubic

Reset/reload can dispose the scheduler while Execute is in flight. Catch
ObjectDisposedException only around runtime awaits so CLI gets Success=false
with compile-wait-style NextActions instead of a raw ODE, while Warm treats
ODE as a silent incomplete warm.

Co-authored-by: Cursor <cursoragent@cursor.com>
@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Important

Review skipped

No new commits to review since the last review.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e2566462-ba65-494b-b0cf-3fdf2c0f288f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Dynamic code execution now handles disposed runtimes as controlled restarting failures, exposes retry guidance through responses, treats warmup disposal as recoverable, and adds tests for retry and successful follow-up execution.

Changes

Dynamic runtime restart handling

Layer / File(s) Summary
Restart response contracts
Packages/src/Editor/ToolContracts/UnityCliLoopConstants.cs, Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/ExecuteDynamicCodeResponse.cs, Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeExecutionResponseFactory.cs
Adds the restarting error, recovery actions, conditional NextActions serialization, and factory methods for restarting results and responses.
Execution and warmup handling
Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/ExecuteDynamicCodeUseCase.cs
Catches runtime disposal during execution and warmup, logs the restart, returns controlled results, and resets incomplete warmup state.
Restart behavior tests
Assets/Tests/Editor/DynamicCodeToolTests/ExecuteDynamicCodeUseCaseTests.cs
Covers retryable restarting responses, silent warmup disposal, later success, and the disposal-aware runtime test double.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ExecuteDynamicCodeUseCase
  participant DynamicCodeExecutionRuntime
  participant DynamicCodeExecutionResponseFactory
  ExecuteDynamicCodeUseCase->>DynamicCodeExecutionRuntime: Execute dynamic code
  DynamicCodeExecutionRuntime-->>ExecuteDynamicCodeUseCase: ObjectDisposedException
  ExecuteDynamicCodeUseCase->>DynamicCodeExecutionResponseFactory: Create restarting result
  DynamicCodeExecutionResponseFactory-->>ExecuteDynamicCodeUseCase: Failure and recovery actions
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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 matches the main change: disposed runtime handling now returns retryable guidance during reset.
Description check ✅ Passed The description directly summarizes the execute and warm dispose-handling changes covered by the diff.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/ode-disposed-scheduler-retryable

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.

@hatayama

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@hatayama
hatayama merged commit f16dd0c into feature/dynamic-code-slot-recovery Jul 13, 2026
2 checks passed
@hatayama
hatayama deleted the fix/ode-disposed-scheduler-retryable branch July 13, 2026 15:54
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