fix: catch ArgumentException from Path.IsPathRooted in IsLocalServerPath

[dsarno]

Summary

• Path.IsPathRooted() throws ArgumentException when the package source URL contains characters illegal in file paths (e.g. remote PyPI URLs with ://)
• This crashes IsLocalServerPath() → ShouldForceUvxRefresh() → TryBuildCommand(), preventing the MCP window from initializing
• Wrapped the call in a try/catch that returns false for non-path strings, since remote URLs are not local paths

Test plan

• Open MCP for Unity window with default (remote) package source — no exception
• Verify local path overrides (absolute path and file:// prefix) still correctly detected
• Verify dev mode force refresh still works independently

🤖 Generated with Claude Code

Summary by Sourcery

Bug Fixes:

• Guard IsLocalServerPath against ArgumentException thrown by Path.IsPathRooted when the package source URL contains characters illegal in file paths.

Summary by CodeRabbit

• Bug Fixes
  • Improved robustness when processing file paths with invalid characters, preventing potential crashes and ensuring more stable path detection.

[sourcery-ai]

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Wraps Path.IsPathRooted() in IsLocalServerPath() with a try/catch so that invalid path strings (like remote URLs) no longer throw ArgumentException and instead are treated as non-local paths, while preserving existing handling for file:// URLs.

Sequence diagram for IsLocalServerPath error handling in MCP window initialization

sequenceDiagram
    participant MCPWindow
    participant TryBuildCommand
    participant ShouldForceUvxRefresh
    participant AssetPathUtility

    MCPWindow->>TryBuildCommand: Execute()
    TryBuildCommand->>ShouldForceUvxRefresh: ShouldForceUvxRefresh()
    ShouldForceUvxRefresh->>AssetPathUtility: IsLocalServerPath()

    alt fromUrl starts with file://
        AssetPathUtility-->>ShouldForceUvxRefresh: return true
    else fromUrl is candidate local path
        AssetPathUtility->>AssetPathUtility: Path.IsPathRooted(fromUrl)
        AssetPathUtility-->>ShouldForceUvxRefresh: return result
    else fromUrl is remote URL with illegal path chars
        AssetPathUtility->>AssetPathUtility: Path.IsPathRooted(fromUrl)
        AssetPathUtility-->>AssetPathUtility: catch ArgumentException
        AssetPathUtility-->>ShouldForceUvxRefresh: return false
    end

    ShouldForceUvxRefresh-->>TryBuildCommand: bool shouldRefresh
    TryBuildCommand-->>MCPWindow: build result (no crash)

Loading

Flow diagram for updated IsLocalServerPath logic

flowchart TD
    A[IsLocalServerPath fromUrl input] --> B{fromUrl is null or empty}
    B -- yes --> C[Return false]
    B -- no --> D{fromUrl starts with file://}
    D -- yes --> E[Return true]
    D -- no --> F[Try Path.IsPathRooted fromUrl]
    F --> G{Path.IsPathRooted succeeds}
    G -- yes --> H[Return result of Path.IsPathRooted]
    G -- no (ArgumentException) --> I[Return false]

Loading

File-Level Changes

Change	Details	Files
Harden IsLocalServerPath against ArgumentException from Path.IsPathRooted when given non-path strings such as remote URLs, while keeping file:// handling explicit.	Split the local path detection into an early return for values starting with the file:// prefix. Wrapped the Path.IsPathRooted call in a try/catch block to catch ArgumentException and return false when the input is not a valid file path. Added a comment clarifying that values causing ArgumentException (e.g. remote URLs) are treated as non-local paths.	MCPForUnity/Editor/Helpers/AssetPathUtility.cs

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[coderabbitai]

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

The IsLocalServerPath method in AssetPathUtility.cs was enhanced with explicit exception handling. The path-root check is now wrapped in a try-catch block to gracefully handle ArgumentException when URLs contain illegal path characters, preventing crashes while maintaining the same public behavior.

Changes

Cohort / File(s)	Summary
Path Validation Robustness MCPForUnity/Editor/Helpers/AssetPathUtility.cs	Added try-catch exception handling around Path.IsPathRooted() call in IsLocalServerPath method to safely handle invalid path characters and prevent propagation of ArgumentException.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• Fixes Windows installation failures caused by long path issues when cloning the full repository via git URL (MAX_PATH 260 char limit exceeded by files in TestProjects/). #534: Both PRs modify AssetPathUtility.cs and update the IsLocalServerPath method; the retrieved PR refactored callers of this method during a GitURL-to-package-source migration.

Poem

🐰 A path that's rooted, safe and sound,
With try-catch guards all wrapped around,
No more crashes from bad chars—hooray!
Your URLs now survive the day! ✨

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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.}

[sourcery-ai]

Hey - I've left some high level feedback:

• Instead of relying on catching ArgumentException from Path.IsPathRooted, consider using Uri.TryCreate(fromUrl, UriKind.Absolute, out var uri) and checking uri.IsFile to distinguish local file paths from remote URLs in a more explicit and exception-free way.
• If you keep the exception-based approach, it may be worth adding a small helper method (e.g. IsRootedPathSafe(string path)) so the try/catch and its rationale are encapsulated and easier to reuse or adjust later.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- Instead of relying on catching `ArgumentException` from `Path.IsPathRooted`, consider using `Uri.TryCreate(fromUrl, UriKind.Absolute, out var uri)` and checking `uri.IsFile` to distinguish local file paths from remote URLs in a more explicit and exception-free way.
- If you keep the exception-based approach, it may be worth adding a small helper method (e.g. `IsRootedPathSafe(string path)`) so the try/catch and its rationale are encapsulated and easier to reuse or adjust later.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}