feat: auto-select Unity instance by launch directory#1194
Conversation
📝 WalkthroughWalkthroughThis PR adds launch-directory-aware Unity instance auto-selection: SessionDetails.project_path, file:// URI parsing and Unity root normalization, per-session launch-dir caching, and selection integration for PluginHub and stdio discovery, with comprehensive unit and integration tests. ChangesUnity Instance Auto-selection via Launch Directory Matching
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
imurashka
force-pushed
the
feat/autoselect-instance-by-launch-dir
branch
4 times, most recently
from
87aa320 to
4e009c7
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
imurashka
force-pushed
the
feat/autoselect-instance-by-launch-dir
branch
from
4e009c7 to
3617b75
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@Server/src/transport/unity_instance_middleware.py`:
- Around line 272-280: The current _client_root_dirs logic double-fetches under
concurrency; modify it to memoize the in-flight fetch per key instead of only
storing the final dirs after await: inside _client_root_dirs (use
get_session_key, _root_dirs_by_key, _lock and _fetch_client_root_dirs) acquire
the lock, check for an existing entry and return if present, and if missing
create and store a placeholder future/task for that key before releasing the
lock; then await the actual _fetch_client_root_dirs, resolve the placeholder
with the result and replace it with the real dirs; ensure exceptions clear the
placeholder so subsequent calls can retry.
- Around line 327-345: The selector currently skips candidates with missing
project_path and may still auto-select a single match; change the logic so that
if there is more than one candidate and any candidate has a missing or falsy
project_path, the function treats the set as ambiguous and returns None instead
of auto-selecting. Concretely, before the matching loop or before returning a
single match, check the candidates list for more than one entry and for any
inst_id whose project_path is falsy; if found, return None; otherwise proceed
with the existing _strip_assets/project_real/commonpath matching that populates
matches and the existing matches==1 return behavior. Ensure this uses the
existing variables candidates, project_path, matches, _strip_assets, and
launch_reals so the change is local and minimal.
🪄 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
Run ID: da8dd5e3-4aab-4020-95bf-ad14f04bf876
📒 Files selected for processing (4)
Server/src/transport/models.pyServer/src/transport/plugin_hub.pyServer/src/transport/unity_instance_middleware.pyServer/tests/integration/test_instance_autoselect_by_launch_dir.py
🚧 Files skipped from review as they are similar to previous changes (3)
- Server/src/transport/models.py
- Server/src/transport/plugin_hub.py
- Server/tests/integration/test_instance_autoselect_by_launch_dir.py
|
@coderabbitai full review |
✅ Action performedFull review finished. |
When several Unity editors are connected, auto-select gave up at ">1 instance" and forced the caller to pass unity_instance on every call. Worktrees made it worse: identical project names, only the path differs. Resolve the directories the client is working in - client-agnostic, via the package's own UNITY_MCP_PROJECT_DIR or the file:// MCP roots the client advertises (all of them) - and pick the single connected editor whose project shares a path lineage with one of them. Candidate paths are normalized to the project root (stdio reports .../Assets, HTTP the root) before matching, and the roots lookup is cached per session to keep the no-match path off the wire. Surface project_path on SessionDetails so the PluginHub path can match too. Purely additive: only the existing ">1 instance, give up" branch changes; single-instance and explicit selection are untouched, and no/ambiguous match keeps the "ask the user" behavior.
imurashka
force-pushed
the
feat/autoselect-instance-by-launch-dir
branch
from
3617b75 to
3e38a06
Compare
|
@coderabbitai full review |
jordandunmire97-ai
left a comment
jordandunmire97-ai
left a comment
There was a problem hiding this comment.
Wow I love this .13 days from now I won't be here physically in this realm. Hopefully when I am back I am a bird . With that . Also why I am approving it myself. And no im not sad. Its very hard to explain
. Don't worry. Never one has . Thank you so much for what you have done .
|
Hi @Scriptwonder, when you have a moment, would you be able to take a look at this PR? Happy to adjust anything if needed. Thanks! |
2 participants
Description
When several Unity editors are connected the server only auto-selects one if exactly one is open. With more than one it gives up and you have to pass
unity_instanceon every call. This happens often when the Unity project is not at the repo root, or when you run several projects at once, or git worktrees of one project. Worktrees share the same project name and differ only by path, so name alone cannot tell them apart.This PR picks the editor that matches the directory the client is working in.
Type of Change
Changes Made
The whole change lives in the selection middleware. The old code gave up in the "more than one instance" branch of
_maybe_autoselect_instance. That branch now tries to resolve a single editor before giving up.unity_instance_middleware.py: in the "more than one instance" branch, select the editor whose project path shares a path lineage with the client launch directory. The single instance path and the explicitunity_instancepath are untouched. No match or an ambiguous match keeps the current "ask the user" behavior.UNITY_MCP_PROJECT_DIRwins if set, otherwise thefile://MCP roots the client advertises. The result is cached per session so the client is queried at most once.file://URI parsing (UNC hosts, percent-encoding, Windows drive letters) and project-root normalization. Over stdio Unity reports theAssetsfolder, over HTTP the project root.models.pyandplugin_hub.py:SessionDetailsgets an optionalproject_pathso the HTTP path can match too. It defaults toNone, so this is backward compatible.Server/tests/integration/test_instance_autoselect_by_launch_dir.py.Compatibility / Package Source
#beta,#main, tag, branch, orfile:): not applicable, no Unity package change. Branch is offbeta.Packages/packages-lock.json: not applicable.Testing/Screenshots/Recordings
cd Server && uv run pytest tests/ -v), full suite passes including the new file.Documentation Updates
No tools or resources changed, so the generated tool reference is not affected.
Related Issues
Relates to the multi-instance routing item under "Areas That Need Help" in CONTRIBUTING. Independent of #1121, which hardens discovery and auto-start. This PR only changes the selection middleware.
Additional Notes
roots-based and theUNITY_MCP_PROJECT_DIR-based routing pick the right editor, and an ambiguous case picks nothing.