Scope HTTP endpoint and server ownership per project - #1285
Conversation
📝 WalkthroughWalkthroughThis change adds project-scoped editor preferences with coordinated legacy migration, routes HTTP transport and PID tracking through scoped storage, supports ChangesProject-scoped transport configuration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant EditorConfigurationCache
participant ProjectScopedEditorPrefs
participant HttpEndpointUtility
participant PidFileManager
EditorConfigurationCache->>ProjectScopedEditorPrefs: load or persist scoped transport settings
HttpEndpointUtility->>HttpEndpointUtility: resolve UNITY_MCP_HTTP_URL override
HttpEndpointUtility->>ProjectScopedEditorPrefs: read or save local base URL
PidFileManager->>ProjectScopedEditorPrefs: store or retrieve PID tracking data
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
47573a4 to
ce057c9
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
MCPForUnity/Editor/Helpers/HttpEndpointUtility.cs (1)
58-76: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winEnv override silently masks saved/remote settings.
With
UNITY_MCP_HTTP_URLset,SaveLocalBaseUrlstill writes the scoped pref butGetLocalBaseUrlnever returns it, and an explicitly configuredremotescope is forced to local. That's a defensible override policy, but it's invisible: a user editing the URL in the connection UI sees no effect, and a remote-configured project silently talks to a local endpoint. Consider surfacing it (one-timeMcpLogline and/or a read-only hint in the connection section) so the override is discoverable.Also applies to: 143-152
🤖 Prompt for 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. In `@MCPForUnity/Editor/Helpers/HttpEndpointUtility.cs` around lines 58 - 76, Make the UNITY_MCP_HTTP_URL override discoverable instead of silently masking saved local or remote settings: in GetLocalBaseUrl and the corresponding remote URL getter, emit a one-time McpLog message when TryGetLocalEnvironmentOverride succeeds, stating that the environment URL is active and UI/persisted values are ignored. Keep SaveLocalBaseUrl behavior unchanged.TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/EditorConfigurationCacheTests.cs (1)
463-485: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate
RestoreBool/RestoreStringhelpers across test files.These are identical to helpers used in
PidFileManagerTests.cs. Consider extracting to a shared EditMode test utility to avoid drift as project-scoped test patterns spread further.🤖 Prompt for 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. In `@TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/EditorConfigurationCacheTests.cs` around lines 463 - 485, Extract the duplicate RestoreBool and RestoreString helpers from EditorConfigurationCacheTests and PidFileManagerTests into a shared EditMode test utility. Update both test classes to use the shared utility and remove their local helper definitions, preserving the existing EditorPrefs restoration behavior.
🤖 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 `@MCPForUnity/Editor/Helpers/ProjectIdentityUtility.cs`:
- Around line 297-305: Update ResolveStorageKey and all storage operations,
including HasKey, typed Get*/Set* accessors, and DeleteKey, to use one
consistent key-resolution path. Since the current operations call GetKey
unconditionally, make ResolveStorageKey resolve every key the same way and reuse
it as the single source of truth; remove KnownProjectScopedKeys only if it is no
longer needed.
In `@MCPForUnity/Editor/Windows/EditorPrefs/EditorPrefsWindow.cs`:
- Around line 245-248: Update CreateEditorPrefItem so the UI-bound key value
displays the resolved storageKey returned by
ProjectScopedEditorPrefs.ResolveStorageKey(key), while preserving raw-key
semantics wherever lookups or filtering depend on the original key. Adjust
GetAllMcpKeys or search comparisons accordingly, or introduce a separate display
field if EditorPrefItem.Key must remain the base key.
In
`@TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/WriteToConfigTests.cs`:
- Around line 37-44: Disable legacy fallback when capturing original preference
values through ProjectScopedEditorPrefs: update the GetBool/GetString calls for
UseHttpTransport, HttpUrl, and HttpTransportScope in
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/WriteToConfigTests.cs:37-44,
and the _savedUseHttpTransport reads in
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/HttpAutoStartHandlerTests.cs:36-37
and
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/HttpBridgeReloadHandlerTests.cs:36-37,
by passing allowLegacyFallback: false while preserving their existing defaults.
In
`@TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Characterization/ServerManagementServiceCharacterizationTests.cs`:
- Around line 42-48: Update the scoped preference snapshots in
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Characterization/ServerManagementServiceCharacterizationTests.cs
lines 42-48 and
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Server/ServerCommandBuilderTests.cs
lines 27-30 to pass allowLegacyFallback: false to all specified
ProjectScopedEditorPrefs.Get* calls, while retaining the existing HasKey guards
and snapshot behavior.
In
`@TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/EditorConfigurationCacheTests.cs`:
- Around line 39-54: Update the SetUp cleanup in the test fixture to delete
_scopedHttpTransportScopeKey alongside _scopedUseHttpTransportKey,
_scopedHttpBaseUrlKey, and _migrationOwnerKey before Refresh() runs, ensuring
each test starts with a clean transport-scope preference.
In
`@TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Server/PidFileManagerTests.cs`:
- Around line 42-47: Update the SetUp and TearDown fixture logic in
PidFileManagerTests to preserve, rather than delete, the active project’s scoped
PID and handshake EditorPrefs keys. Snapshot each key’s existing value and
presence before tests, then restore the original state afterward, including
deleting only keys that were initially absent.
In
`@TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Server/ServerCommandBuilderTests.cs`:
- Around line 22-50: Update the ServerCommandBuilderTests fixture setup and
teardown to isolate UNITY_MCP_HTTP_URL: save its existing value, clear it before
endpoint tests run, and restore it afterward, including preserving whether it
was originally unset. Keep the change scoped to SetUp and TearDown so
TryBuildCommand_RemoteUrl_ReturnsFalse and the local-URL scenario always use
configured preferences.
---
Nitpick comments:
In `@MCPForUnity/Editor/Helpers/HttpEndpointUtility.cs`:
- Around line 58-76: Make the UNITY_MCP_HTTP_URL override discoverable instead
of silently masking saved local or remote settings: in GetLocalBaseUrl and the
corresponding remote URL getter, emit a one-time McpLog message when
TryGetLocalEnvironmentOverride succeeds, stating that the environment URL is
active and UI/persisted values are ignored. Keep SaveLocalBaseUrl behavior
unchanged.
In
`@TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/EditorConfigurationCacheTests.cs`:
- Around line 463-485: Extract the duplicate RestoreBool and RestoreString
helpers from EditorConfigurationCacheTests and PidFileManagerTests into a shared
EditMode test utility. Update both test classes to use the shared utility and
remove their local helper definitions, preserving the existing EditorPrefs
restoration behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9419d175-b893-4bca-9c88-9300af487142
📒 Files selected for processing (19)
MCPForUnity/Editor/Helpers/CodexConfigHelper.csMCPForUnity/Editor/Helpers/HttpEndpointUtility.csMCPForUnity/Editor/Helpers/ProjectIdentityUtility.csMCPForUnity/Editor/McpCiBoot.csMCPForUnity/Editor/Services/EditorConfigurationCache.csMCPForUnity/Editor/Services/Server/IPidFileManager.csMCPForUnity/Editor/Services/Server/PidFileManager.csMCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.csMCPForUnity/Editor/Windows/EditorPrefs/EditorPrefsWindow.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/ClientConfigFormatTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/CodexConfigHelperTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/WriteToConfigTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Characterization/ServerManagementServiceCharacterizationTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Characterization/Services_Characterization.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/EditorConfigurationCacheTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/HttpAutoStartHandlerTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/HttpBridgeReloadHandlerTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Server/PidFileManagerTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Server/ServerCommandBuilderTests.cs
ce057c9 to
eb2b8aa
Compare
There was a problem hiding this comment.
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 `@MCPForUnity/Editor/Helpers/HttpEndpointUtility.cs`:
- Around line 369-381: Update TryGetLocalEnvironmentOverride to validate the
environment override with IsHttpLocalUrlAllowedForLaunch before accepting it as
local; reject values that are not allowed loopback HTTP(S) URLs by returning
false, or classify them as remote and apply IsRemoteUrlAllowed instead. Preserve
normalization only after validation so IsRemoteScope cannot force arbitrary
external endpoints through the local transport path.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8f66e49e-4e48-43b9-823a-ba3799c9bcfa
📒 Files selected for processing (19)
MCPForUnity/Editor/Helpers/CodexConfigHelper.csMCPForUnity/Editor/Helpers/HttpEndpointUtility.csMCPForUnity/Editor/Helpers/ProjectIdentityUtility.csMCPForUnity/Editor/McpCiBoot.csMCPForUnity/Editor/Services/EditorConfigurationCache.csMCPForUnity/Editor/Services/Server/IPidFileManager.csMCPForUnity/Editor/Services/Server/PidFileManager.csMCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.csMCPForUnity/Editor/Windows/EditorPrefs/EditorPrefsWindow.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/ClientConfigFormatTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/CodexConfigHelperTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/WriteToConfigTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Characterization/ServerManagementServiceCharacterizationTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Characterization/Services_Characterization.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/EditorConfigurationCacheTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/HttpAutoStartHandlerTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/HttpBridgeReloadHandlerTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Server/PidFileManagerTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Server/ServerCommandBuilderTests.cs
🚧 Files skipped from review as they are similar to previous changes (17)
- TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Characterization/Services_Characterization.cs
- MCPForUnity/Editor/Services/Server/IPidFileManager.cs
- MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs
- MCPForUnity/Editor/Helpers/CodexConfigHelper.cs
- TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/HttpBridgeReloadHandlerTests.cs
- MCPForUnity/Editor/McpCiBoot.cs
- TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Server/ServerCommandBuilderTests.cs
- MCPForUnity/Editor/Windows/EditorPrefs/EditorPrefsWindow.cs
- TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/ClientConfigFormatTests.cs
- TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/WriteToConfigTests.cs
- MCPForUnity/Editor/Helpers/ProjectIdentityUtility.cs
- TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/HttpAutoStartHandlerTests.cs
- TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/EditorConfigurationCacheTests.cs
- MCPForUnity/Editor/Services/Server/PidFileManager.cs
- TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Server/PidFileManagerTests.cs
- TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Characterization/ServerManagementServiceCharacterizationTests.cs
- TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/CodexConfigHelperTests.cs
eb2b8aa to
f95a3f9
Compare
There was a problem hiding this comment.
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
`@TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Server/ServerCommandBuilderTests.cs`:
- Around line 47-59: Update the teardown logic around
EditorConfigurationCache.Instance.Refresh() so an originally absent scoped key
is not recreated from its legacy global fallback. Refresh before deleting the
missing scoped keys, or invoke a fallback-disabled refresh during teardown,
while preserving restoration of existing scoped values and the saved
UNITY_MCP_HTTP_URL environment variable.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9bc9fde7-2f2b-4177-9660-953cf1049496
📒 Files selected for processing (19)
MCPForUnity/Editor/Helpers/CodexConfigHelper.csMCPForUnity/Editor/Helpers/HttpEndpointUtility.csMCPForUnity/Editor/Helpers/ProjectIdentityUtility.csMCPForUnity/Editor/McpCiBoot.csMCPForUnity/Editor/Services/EditorConfigurationCache.csMCPForUnity/Editor/Services/Server/IPidFileManager.csMCPForUnity/Editor/Services/Server/PidFileManager.csMCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.csMCPForUnity/Editor/Windows/EditorPrefs/EditorPrefsWindow.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/ClientConfigFormatTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/CodexConfigHelperTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/WriteToConfigTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Characterization/ServerManagementServiceCharacterizationTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Characterization/Services_Characterization.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/EditorConfigurationCacheTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/HttpAutoStartHandlerTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/HttpBridgeReloadHandlerTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Server/PidFileManagerTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Server/ServerCommandBuilderTests.cs
🚧 Files skipped from review as they are similar to previous changes (18)
- MCPForUnity/Editor/Services/Server/IPidFileManager.cs
- TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Characterization/Services_Characterization.cs
- MCPForUnity/Editor/McpCiBoot.cs
- MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs
- MCPForUnity/Editor/Services/Server/PidFileManager.cs
- MCPForUnity/Editor/Helpers/CodexConfigHelper.cs
- MCPForUnity/Editor/Helpers/ProjectIdentityUtility.cs
- TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/HttpAutoStartHandlerTests.cs
- MCPForUnity/Editor/Helpers/HttpEndpointUtility.cs
- TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/HttpBridgeReloadHandlerTests.cs
- TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/ClientConfigFormatTests.cs
- TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Server/PidFileManagerTests.cs
- MCPForUnity/Editor/Windows/EditorPrefs/EditorPrefsWindow.cs
- TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/WriteToConfigTests.cs
- TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/CodexConfigHelperTests.cs
- MCPForUnity/Editor/Services/EditorConfigurationCache.cs
- TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/EditorConfigurationCacheTests.cs
- TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Characterization/ServerManagementServiceCharacterizationTests.cs
f95a3f9 to
8cf41f2
Compare
Description
Fix machine-global
EditorPrefscross-contamination that can make one Unity project inherit another project's local HTTP endpoint or server ownership metadata.Type of Change
Changes Made
UNITY_MCP_HTTP_URLoverride only the current Unity process and reject non-local or unsupported schemes.Compatibility / Package Source
#beta,#main, tag, branch, orfile:): branch derived directly from officialbeta(bd72241ab91c664176091789c9408d676783abec); localfile:source for validation onlyPackages/packages-lock.json(if using a Git package URL): N/ASingle-project defaults remain unchanged. Configuration remains machine-local and is not written into the Unity project.
Testing/Screenshots/Recordings
cd Server && uv run pytest tests/ -v)Unity 2022.3.62f3 targeted suite: 155 total, 154 passed, 0 failed, 1 intentional stress-only skip. Two isolated Unity processes also validated independent endpoints and project-scoped PID tracking.
Documentation Updates
tools/UPDATE_DOCS_PROMPT.md(recommended)Related Issues
Relates to #1164.
Additional Notes
A rebuilt stable branch containing this PR plus #1286 and #1287 passed 162 targeted EditMode tests: 161 passed, 0 failed, 1 intentional stress-only skip.
git diff --checkpasses.Summary by CodeRabbit
New Features
UNITY_MCP_HTTP_URLenvironment variable.Improvements
Tests