Skip to content

Conversation

@msanatan
Copy link
Member

@msanatan msanatan commented Jan 27, 2026

Description

Type of Change

Save your change type

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Refactoring (no functional changes)
  • Test update

Changes Made

Testing/Screenshots/Recordings

Documentation Updates

  • I have added/removed/modified tools or resources
  • If yes, I have updated all documentation files using:
    • The LLM prompt at tools/UPDATE_DOCS_PROMPT.md (recommended)
    • Manual updates following the guide at tools/UPDATE_DOCS.md

Related Issues

Additional Notes

Summary by Sourcery

Add configurable support for using TestPyPI as the package source for the MCP server and persist this preference in editor settings.

New Features:

  • Introduce an editor toggle to enable using TestPyPI instead of PyPI when fetching the MCP server package.
  • Add a persistent editor preference key to store the TestPyPI usage setting and apply it when constructing uvx arguments.

Enhancements:

  • Update advanced settings UI to expose and document the TestPyPI option, including tooltips for clarity.

Summary by CodeRabbit

  • New Features
    • Added TestPyPI support: A new toggle in the Advanced settings section allows users to switch to the test Python package repository. When enabled, the system sources packages from TestPyPI, which contains pre-release and experimental versions, providing flexibility for testing and development workflows.

✏️ Tip: You can customize this high-level summary in your review settings.

- Add UseTestPyPI editor preference key
- Add TestPyPI toggle to Advanced settings UI with tooltip
- Configure uvx to use test.pypi.org when TestPyPI mode enabled
- Skip version pinning in TestPyPI mode to get latest pre-release
- Update ConfigJsonBuilder to handle TestPyPI index URL
@msanatan msanatan requested a review from dsarno January 27, 2026 06:07
@msanatan msanatan self-assigned this Jan 27, 2026
@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented Jan 27, 2026

Reviewer's Guide

Adds an editor preference and advanced panel toggle to optionally use TestPyPI as the default index for the MCP server package, and wires this flag into the uvx argument construction so server installs can target TestPyPI instead of PyPI.

Sequence diagram for TestPyPI toggle affecting uvx argument construction

sequenceDiagram
    actor UnityDeveloper
    participant McpAdvancedSection
    participant EditorPrefs
    participant ConfigJsonBuilder

    UnityDeveloper->>McpAdvancedSection: Toggle useTestPyPIToggle
    McpAdvancedSection->>EditorPrefs: SetBool(EditorPrefKeys.UseTestPyPI, value)
    McpAdvancedSection->>McpAdvancedSection: OnHttpServerCommandUpdateRequested

    UnityDeveloper->>ConfigJsonBuilder: BuildUvxArgs(fromUrl, packageName)
    ConfigJsonBuilder->>EditorPrefs: GetBool(EditorPrefKeys.UseTestPyPI, false)
    EditorPrefs-->>ConfigJsonBuilder: useTestPyPI
    alt useTestPyPI is true
        ConfigJsonBuilder->>ConfigJsonBuilder: Add --default-index https://test.pypi.org/simple/
        ConfigJsonBuilder->>ConfigJsonBuilder: Add --from mcpforunityserver
    else useTestPyPI is false
        ConfigJsonBuilder->>ConfigJsonBuilder: Use fromUrl and packageName
    end
Loading

Updated class diagram for TestPyPI support in MCP editor components

classDiagram
    class McpAdvancedSection {
        - Toggle debugLogsToggle
        - Toggle devModeForceRefreshToggle
        - Toggle useTestPyPIToggle
        - void CacheUIElements()
        - void InitializeUI()
        - void RegisterCallbacks()
        + void UpdatePathOverrides()
    }

    class ConfigJsonBuilder {
        - static IList~string~ BuildUvxArgs(string fromUrl, string packageName)
    }

    class EditorPrefKeys {
        <<static>>
        + const string UseTestPyPI
    }

    class EditorPrefs {
        <<external>>
        + static void SetBool(string key, bool value)
        + static bool GetBool(string key, bool defaultValue)
    }

    McpAdvancedSection ..> EditorPrefKeys : uses
    McpAdvancedSection ..> EditorPrefs : reads_writes
    ConfigJsonBuilder ..> EditorPrefKeys : uses
    ConfigJsonBuilder ..> EditorPrefs : reads
Loading

File-Level Changes

Change Details Files
Add a persistent editor preference and UI toggle in the Advanced settings to control whether TestPyPI is used for server package resolution.
  • Introduce a new Toggle field for the TestPyPI option and cache it from the UXML layout.
  • Initialize the toggle value from a new EditorPrefs key during UI setup and when updating path overrides.
  • Register a value-changed callback that saves the preference and triggers an HTTP server command update when the toggle changes.
  • Add tooltip text for the TestPyPI toggle and its label to explain its behavior and intended use.
  • Update the Advanced section UXML to declare the new toggle element with the expected name/id.
MCPForUnity/Editor/Windows/Components/Advanced/McpAdvancedSection.cs
MCPForUnity/Editor/Windows/Components/Advanced/McpAdvancedSection.uxml
MCPForUnity/Editor/Constants/EditorPrefKeys.cs
Wire the TestPyPI preference into uvx argument construction so installs can target TestPyPI instead of PyPI, using an unversioned package name.
  • Read the new TestPyPI editor preference inside the uvx argument builder.
  • When TestPyPI is enabled, add uvx --default-index pointing at https://test.pypi.org/simple/ and use --from mcpforunityserver without a version pin.
  • Fall back to the existing fromUrl-based behavior when TestPyPI is not enabled.
  • Minor using directive reorder to keep namespaces organized.
MCPForUnity/Editor/Helpers/ConfigJsonBuilder.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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 27, 2026

📝 Walkthrough

Walkthrough

This pull request adds TestPyPI support to the MCP for Unity system, introducing a new EditorPref flag to allow users to optionally use the test PyPI registry. The feature includes UI controls, configuration logic to apply TestPyPI settings when building UVX arguments, and updated metadata for the OpenCodeConfigurator script.

Changes

Cohort / File(s) Summary
Metadata Configuration
MCPForUnity/Editor/Clients/Configurators/OpenCodeConfigurator.cs.meta
Replaced simple GUID with complete MonoImporter metadata block, adding externalObjects, serializedVersion, defaultReferences, executionOrder, icon, userData, assetBundleName, and assetBundleVariant fields
Constants & Settings
MCPForUnity/Editor/Constants/EditorPrefKeys.cs
Added new internal constant UseTestPyPI = "MCPForUnity.UseTestPyPI" for managing TestPyPI preference storage
Build Configuration Logic
MCPForUnity/Editor/Helpers/ConfigJsonBuilder.cs
Reordered using directives and implemented TestPyPI detection in UVX argument construction; when enabled, applies --default-index https://test.pypi.org/simple/ and --from mcpforunityserver with unversioned package name
Advanced UI Section
MCPForUnity/Editor/Windows/Components/Advanced/McpAdvancedSection.cs, MCPForUnity/Editor/Windows/Components/Advanced/McpAdvancedSection.uxml
Added new UI toggle useTestPyPIToggle with label "Use TestPyPI:" to McpAdvancedSection, including EditorPrefs persistence, value-changed callbacks, and tooltip wiring following existing toggle patterns

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

codex

Suggested reviewers

  • dsarno

Poem

🐰 A toggle hops into view,
TestPyPI now at hand—true or false, we choose our brew,
EditorPrefs store the preference dear,
UVX arguments shift without fear,
One feature complete, another test tier! ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request description template is mostly not filled out, with only the Type of Change and Related Issues sections remaining as placeholders, while critical sections like 'Changes Made' and 'Testing/Screenshots/Recordings' are empty. Fill out the required sections: explicitly state the Type of Change, detail all Changes Made, provide Testing/Screenshots/Recordings if applicable, and fill in any other relevant sections before merging.
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 (1 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Support testpypi' is concise and directly describes the main feature being added to the pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • Several places assume useTestPyPIToggle is non-null (e.g., setting .value and registering callbacks in InitializeUI, RegisterCallbacks, and UpdatePathOverrides) without null checks, which could throw if the UXML element is missing; consider mirroring the null-guard pattern used when setting tooltips.
  • The TestPyPI URL and the unversioned package name ("https://test.pypi.org/simple/" and "mcpforunityserver") are hardcoded inside BuildUvxArgs; consider extracting these into named constants to avoid duplication and make future changes easier.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Several places assume `useTestPyPIToggle` is non-null (e.g., setting `.value` and registering callbacks in `InitializeUI`, `RegisterCallbacks`, and `UpdatePathOverrides`) without null checks, which could throw if the UXML element is missing; consider mirroring the null-guard pattern used when setting tooltips.
- The TestPyPI URL and the unversioned package name (`"https://test.pypi.org/simple/"` and `"mcpforunityserver"`) are hardcoded inside `BuildUvxArgs`; consider extracting these into named constants to avoid duplication and make future changes easier.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
MCPForUnity/Editor/Helpers/ConfigJsonBuilder.cs (1)

173-190: Logic error: TestPyPI mode uses incorrect package name in uvx command.

When useTestPyPI is true, the code adds "mcpforunityserver" via --from mcpforunityserver (line 183) but then line 190 unconditionally adds "mcp-for-unity" as the package argument. This results in --from mcpforunityserver mcp-for-unity, which attempts to install the wrong package from TestPyPI.

The comment on line 181 ("Use unversioned package name to get latest from TestPyPI") indicates the intent is to use "mcpforunityserver" as the final package argument, not "mcp-for-unity". Either line 190 should be moved into the else block, or packageName should be reassigned to "mcpforunityserver" in TestPyPI mode.

🧹 Nitpick comments (1)
MCPForUnity/Editor/Clients/Configurators/OpenCodeConfigurator.cs.meta (1)

1-11: Consider verifying if this metadata update is necessary for the PR.

This file contains auto-generated Unity metadata for OpenCodeConfigurator.cs. The change from a bare GUID to a full MonoImporter block is standard Unity behavior when assets are reimported (e.g., after Unity version changes or git operations).

The metadata structure itself is correct with standard default values. However, since this PR focuses on TestPyPI support and OpenCodeConfigurator.cs doesn't appear directly related to that feature based on the PR objectives, consider whether this file needs to be included in the PR or if it was unintentionally modified during development.

@dsarno
Copy link
Collaborator

dsarno commented Jan 27, 2026

Thanks for adding this @msanatan! One concern: the Python server and C# client are tightly coupled, so if schemas diverge -- which they do anytime there's a significant server change -- things break silently. That's the state we're in right now for main vs beta -- beta users are getting fails because they're pulling down the (older) main server by default.

Since beta auto-publishes to TestPyPI and main publishes to PyPI, beta users need TestPyPI to get matching schemas. Making it opt-in means beta is kinda broken by default unless you know to flip this hidden toggle.

So we should flip the default to true on the beta branch:

bool useTestPyPI = EditorPrefs.GetBool(EditorPrefKeys.UseTestPyPI, true);

That way beta "just works" out of the box, and we flip it back to false when releasing to main.

@msanatan
Copy link
Member Author

@dsarno shouldn't it be the other way around? We default for release settings which most customers use, and do patches for beta testers?

@dsarno
Copy link
Collaborator

dsarno commented Jan 27, 2026

I thought that's what I'm suggesting? Default for most users is main with pypi -- or if you opt in to beta via the beta git link download, you get testpypi automatically (and not via a second opt in in the advanced settings, which this PR proposes). Maybe I'm not understanding something about the flow here?

@dsarno
Copy link
Collaborator

dsarno commented Jan 27, 2026

I thought that's what I'm suggesting? Default for most users is main with pypi -- or if you opt in to beta via the beta git link download, you get testpypi automatically (and not via a second opt in in the advanced settings, which this PR proposes). Maybe I'm not understanding something about the flow here?

@msanatan are we intending to have the main download link default to beta? I think that's what we're doing. But that seems to defeat the point of users opting in to beta and makes everyone a beta tester. Either way, beta needs to default to testpypi for server or there will be a breaking mismatch between c# and server.

https://github.com/CoplayDev/unity-mcp.git?path=/MCPForUnity <-- points to beta.

@dsarno
Copy link
Collaborator

dsarno commented Jan 27, 2026

@msanatan for a more concrete example:

I just tested the "way it is right now" : beta with PyPI server.

Because we updated how manage_prefabs work a few days ago (on beta) and the schema changed, manage prefabs is currently broken on beta. The PyPi (older) version bundled with beta uses a schema that doesn't match with the C#:

  • manage_prefabs(action="open_stage") [old schema] → passes Python validation, but C# rejects: "Unknown action"
  • manage_prefabs(action="modify_contents") [new schema] → fails Python Pydantic validation, never reaches C#

The schemas are already out of sync. Beta C# expects modify_contents, PyPI 9.2.0 still has open_stage in its enum. Both directions are broken.

This is why TestPyPI needs to be the default on beta, not opt-in.

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.

3 participants