fix: Interrupted PlayMode tests no longer leave domain reload disabled

[hatayama]

Summary

• Interrupted PlayMode test runs now restore the user's Enter Play Mode settings even if the disable scope is abandoned.
• Recovery state is stored in a dedicated UserSettings marker file, not SessionState or UnityMcpSettings.json, so it survives editor reloads while staying separate from editor session config.

User Impact

• Before this change, an interrupted PlayMode test run could leave domain reload disabled in Unity.
• After this change, Unity restores the original Enter Play Mode settings on editor load, before the next run, or when the final active scope exits.

Changes

• Persist the original Enter Play Mode settings to UserSettings/uloop-domain-reload-recovery.json through a temporary sidecar file.
• Restore and delete pending recovery markers on editor load and before starting a new disable scope.
• Track nested disable scopes so settings are restored only after the last active scope exits.
• Add focused EditMode coverage for normal disposal, nested scopes, abandoned scopes, stale markers, and marker cleanup.

Verification

• node Packages/src/Cli~/dist/cli.bundle.cjs compile --force-recompile true --wait-for-domain-reload true --project-path <PROJECT_ROOT> -> ErrorCount 0, WarningCount 0
• node Packages/src/Cli~/dist/cli.bundle.cjs run-tests --test-mode EditMode --filter-type regex --filter-value io.github.hatayama.uLoopMCP.DomainReloadDisableScope --save-before-run false --project-path <PROJECT_ROOT> -> 5 passed
• ~/.codex/skills/codex-review/scripts/codex-review --parallel-tests "node Packages/src/Cli~/dist/cli.bundle.cjs run-tests --test-mode EditMode --filter-type regex --filter-value io.github.hatayama.uLoopMCP.DomainReloadDisableScope --save-before-run false --project-path <PROJECT_ROOT>" main -> no accepted/actionable findings

Fix: Interrupted PlayMode Tests No Longer Leave Domain Reload Disabled

Overview

This PR resolves an issue where interrupted PlayMode test runs could leave domain reload disabled in Unity. The fix introduces a static reference-counted scope model with persistent recovery state, allowing the editor to restore the user's original Enter Play Mode settings even if a DomainReloadDisableScope is abandoned or the editor is reloaded.

Architecture

Key Components

classDiagram
    class DomainReloadDisableScope {
        -_activeScopeCount: static int
        -_disposed: bool
        +DomainReloadDisableScope()
        +Dispose() void
        +ResetActiveScopeCountForTests() internal static void
    }
    
    class DomainReloadDisableScopeRecovery {
        +SaveCurrentSettings() internal static void
        +RestoreIfPending() internal static void
        +ClearPendingRestoreForTests() internal static void
        +HasPendingRestoreForTests() internal static bool
        +ReadMarkerDataForTests() internal static DomainReloadDisableScopeRecoveryData
        -RestorePendingSettingsOnEditorLoad() private static void
    }
    
    class DomainReloadDisableScopeRecoveryData {
        +originalOptionsEnabled: bool
        +originalOptions: int
    }
    
    class DomainReloadDisableScopeRecoveryConstants {
        +DirectoryPath: const string
        +MarkerFileName: const string
        +TempFileName: const string
        +MarkerFilePath: readonly string
        +TempFilePath: readonly string
    }
    
    DomainReloadDisableScope --> DomainReloadDisableScopeRecovery
    DomainReloadDisableScopeRecovery --> DomainReloadDisableScopeRecoveryData
    DomainReloadDisableScopeRecovery --> DomainReloadDisableScopeRecoveryConstants

Loading

Changes Summary

1. DomainReloadDisableScope.cs (Refactored)

• Static Reference Tracking: Replaced per-instance captured original-settings with a static collection of active scope instances (via weak references) to track nested scopes and abandoned scopes.
• Pruning/Recovery: Constructor prunes inactive/abandoned weak references, and on first active scope creation it calls RestoreIfPending() and SaveCurrentSettings() before forcing disable-domain-reload EditorSettings.
• Disposal Idempotency: Dispose() is idempotent per instance (guarded by a _disposed flag), removes the instance from the static collection, and only when the last active scope is removed does it call RestoreIfPending().
• Test Support: Added ResetActiveScopeCountForTests() internal helper to clear static tracking for tests.

2. DomainReloadDisableScopeRecovery.cs (New)

• Automatic Recovery: Registers an [InitializeOnLoadMethod] to restore pending settings on editor load (skips Asset Import worker processes).
• Persistence Strategy:
  • Saves original EditorSettings.enterPlayModeOptionsEnabled and EditorSettings.enterPlayModeOptions to UserSettings/uloop-domain-reload-recovery.json with an atomic temp-file-then-rename strategy.
  • Restores settings and deletes both marker and temp files after successful restoration.
  • Uses a dedicated UserSettings marker file so recovery survives editor reloads and remains separate from session config.
• Test Helpers:
  • HasPendingRestoreForTests(), ReadMarkerDataForTests(), ClearPendingRestoreForTests(), and test path properties for inspecting and cleaning marker/temp files.

3. DomainReloadDisableScopeRecoveryData.cs (New)

• Serializable data class storing original Enter Play Mode settings persisted as JSON in the recovery marker file.

4. DomainReloadDisableScopeRecoveryConstants.cs (New)

• Centralizes file paths: UserSettings/uloop-domain-reload-recovery.json and .tmp variant.

5. DomainReloadDisableScopeTests.cs (New Test Suite)

Comprehensive test coverage with lifecycle handling and recovery verification:

• SetUp/TearDown snapshot and restore EditorSettings and existing marker/temp files.
• Tests verify:
  • Normal disposal restores original settings and removes recovery marker.
  • Nested scopes keep domain reload disabled until the last nested scope disposes, then restore settings and clear pending state.
  • RestoreIfPending() restores original settings when a scope is abandoned and clears pending state.
  • Constructor consumes stale markers from previous runs before saving a new baseline (including when a previous scope was GC-collected in the same session).
  • Successful RestoreIfPending() removes both marker and temp files (no residual state).

Recovery Flow

Editor Load
    ↓
[InitializeOnLoadMethod] triggers
    ↓
RestoreIfPending() checks for marker file
    ├─ If found: Read JSON → Restore settings → Delete marker/temp files
    └─ If not found: No-op

User Creates DomainReloadDisableScope
    ↓
First scope activation:
    ├─ Call RestoreIfPending() (handles stale markers)
    ├─ Call SaveCurrentSettings() (write to marker file)
    └─ Force DisableDomainReload settings
    ↓
Nested scopes: Register themselves in static collection only

Scope Disposal
    ├─ Last active scope exits → Call RestoreIfPending()
    └─ Settings restored, marker deleted

User Impact

• Before: Interrupted PlayMode test runs could leave EditorSettings.enterPlayModeOptions set to disable domain reload, affecting subsequent runs.
• After:
  • Recovery marker persisted on first scope activation.
  • Editor automatically restores original settings on load.
  • Settings restored when scope exits normally, when final nested scope exits, or when a stale marker is encountered.
  • Recovery state survives editor reload and is isolated from session configuration.

Verification

• Compilation: ErrorCount 0, WarningCount 0
• EditMode tests for DomainReloadDisableScope: 5 passed
• Parallel test review: no accepted/actionable findings

[coderabbitai]

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: b7e9e232-c830-4ada-b129-9a63fb9de401

📥 Commits

Reviewing files that changed from the base of the PR and between 346b787 and 1833bdc.

📒 Files selected for processing (2)

• Assets/Tests/Editor/DomainReloadDisableScopeTests.cs
• Packages/src/Editor/Core/CoreTools/Util/DomainReloadDisableScope.cs

🚧 Files skipped from review as they are similar to previous changes (2)

• Packages/src/Editor/Core/CoreTools/Util/DomainReloadDisableScope.cs
• Assets/Tests/Editor/DomainReloadDisableScopeTests.cs

---

📝 Walkthrough

Walkthrough

Adds a recovery utility that persists Enter Play Mode settings to a JSON marker and restores them on editor load or when the last DomainReloadDisableScope disposes. DomainReloadDisableScope is refactored to track active scopes statically (WeakReferences) so nested scopes cooperate. New tests cover disposal, nesting, abandoned-scope recovery, stale-marker consumption, and file cleanup.

Changes

Domain Reload Scope Recovery System

Layer / File(s)	Summary
Recovery marker persistence and restoration Packages/src/Editor/Core/CoreTools/Util/DomainReloadDisableScopeRecovery.cs	Adds editor-load restore entry, SaveCurrentSettings(), RestoreIfPending(), JSON marker/temp persistence with atomic rename, read/delete helpers, test inspection/clear helpers, payload type, and centralized path constants.
Reference-counted scope with recovery integration Packages/src/Editor/Core/CoreTools/Util/DomainReloadDisableScope.cs	Refactors scope to static WeakReference-based active-scope tracking. First active scope consumes any stale marker, saves current settings, disables domain reload; Dispose() is idempotent and runs restore only when the last active scope is removed. Adds ResetActiveScopeCountForTests() for test isolation.
Comprehensive recovery and scope behavior tests Assets/Tests/Editor/DomainReloadDisableScopeTests.cs	NUnit fixture snapshots/restores EditorSettings and prior marker/temp files; tests verify disposal restore, nested-scope behavior, RestoreIfPending for abandoned scopes (including GC-abandoned), constructor consumption of stale markers, and cleanup of marker/temp files. Includes GC and file restoration helpers.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related issues

• #1243: Implements persistent marker-based recovery and refactors DomainReloadDisableScope to address leaked DisableDomainReload scenarios described in the issue.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 11.11% 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
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly addresses the main fix—preventing domain reload from remaining disabled after interrupted PlayMode tests—which matches the core functionality added across the new recovery system and scope tracking changes.
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-domain-reload-marker-v2

---

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 and usage tips.}

[coderabbitai]

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/Core/CoreTools/Util/DomainReloadDisableScope.cs`:
- Around line 17-23: Call DomainReloadDisableScopeRecovery.RestoreIfPending()
before checking or incrementing _activeScopeCount so recovery runs even if a
previous scope left _activeScopeCount > 0; only call
DomainReloadDisableScopeRecovery.SaveCurrentSettings() when _activeScopeCount ==
0 (to preserve current settings for the first active scope), then increment
_activeScopeCount as before. Update the DomainReloadDisableScope constructor
(referencing _activeScopeCount,
DomainReloadDisableScopeRecovery.RestoreIfPending(), and
DomainReloadDisableScopeRecovery.SaveCurrentSettings()) to restore pending
recovery unconditionally and save settings only for the first-entered scope.

🪄 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: 05891f51-6772-4d6a-bf63-917377353884

📥 Commits

Reviewing files that changed from the base of the PR and between 5b1b172 and 346b787.

⛔ Files ignored due to path filters (2)

• Assets/Tests/Editor/DomainReloadDisableScopeTests.cs.meta is excluded by none and included by none
• Packages/src/Editor/Core/CoreTools/Util/DomainReloadDisableScopeRecovery.cs.meta is excluded by none and included by none

📒 Files selected for processing (3)

• Assets/Tests/Editor/DomainReloadDisableScopeTests.cs
• Packages/src/Editor/Core/CoreTools/Util/DomainReloadDisableScope.cs
• Packages/src/Editor/Core/CoreTools/Util/DomainReloadDisableScopeRecovery.cs

[cubic-dev-ai]

2 issues found across 5 files

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

Re-trigger cubic</sub>