Support kiro #221
Support kiro #221
Conversation
WalkthroughAdds a Unity EditMode test project with packages, settings, sample files, and tests; introduces a GitHub Actions workflow to run Unity tests; updates ignores; and extends the UnityMcpBridge editor to support a new MCP type “Kiro” with client entry, enum value, and manual config UI instructions. Empties package.json. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ManualConfigEditorWindow
participant McpClients
participant FileSystem
User->>ManualConfigEditorWindow: Open Manual Config (select Kiro)
ManualConfigEditorWindow->>McpClients: Resolve Kiro client config path
McpClients-->>ManualConfigEditorWindow: ~/.kiro/settings/mcp.json
ManualConfigEditorWindow-->>User: Display Kiro setup instructions
User->>FileSystem: Open/Edit ~/.kiro/settings/mcp.json
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Made a mistake and based it off the branch with the test. We can always merge this in after that's refined |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
TestProjects/UnityMCPTests/Packages/packages-lock.json (1)
71-79: Reflect git dependency pin in manifest (consistency with lock).Lockfile pins
com.unity.ide.windsurfto commit6161acc.... Consider pinning the manifest (as suggested) to keep resolution stable across fresh clones.
🧹 Nitpick comments (15)
.gitignore (1)
25-37: Consider ignoring Unity test project’s volatile directoriesTo avoid noise from Unity caches/artifacts generated by the nested test project, ignore them explicitly.
Apply this diff:
# Unity Editor *.unitypackage *.asset UnityMcpBridge.meta LICENSE.meta README.md.meta CONTRIBUTING.md.meta +# Unity test project caches (avoid accidental check-ins) +TestProjects/UnityMCPTests/Library/ +TestProjects/UnityMCPTests/Temp/ +TestProjects/UnityMCPTests/Logs/ +TestProjects/UnityMCPTests/obj/TestProjects/UnityMCPTests/Assets/Scripts.meta (1)
2-2: Optional: Verify GUID stability across referenced assetsIf any assets (e.g., assembly definitions or scenes) implicitly depend on this folder path, GUID stability helps avoid reference churn. No action required if this is a new folder.
TestProjects/UnityMCPTests/Assets/Scenes/SampleScene.unity.meta (1)
2-2: Optional: Cross-check script GUID referencesIf this scene attaches any MonoBehaviours from scripts added in this PR, consider verifying the referenced script GUIDs match the .meta of those scripts to prevent missing script warnings.
TestProjects/UnityMCPTests/Packages/manifest.json (2)
9-9: Pin git package to a commit for reproducible builds.
com.unity.ide.windsurfis referenced by a floating Git URL. Pinning to the exact commit used in the lockfile avoids accidental updates and CI drift.Apply this change to pin to the hash already resolved in the lockfile:
- "com.unity.ide.windsurf": "https://github.com/Asuta/com.unity.ide.windsurf.git", + "com.unity.ide.windsurf": "https://github.com/Asuta/com.unity.ide.windsurf.git#6161accf3e7beab96341813913e714c7e2fb5c5d",
4-14: Trim unnecessary IDE integrations for CI speed (optional).If this project exists solely for EditMode tests in CI, consider dropping extra IDE packages (
collab-proxy,feature.development, Rider/VS/VSCode IDEs) to reduce install time and lockfile noise. They aren't required to run tests.If you want, I can propose a minimal test manifest that keeps only com.unity.test-framework, TMP/UGUI (if needed), and the local package.
TestProjects/UnityMCPTests/.gitignore (1)
1-106: Solid Unity .gitignore; minor optional additions.Rules look good and cover typical Unity/IDE artifacts. Two optional niceties:
- Ignore OS thumbnail caches on Windows:
Thumbs.db(tiny).- If you don’t want Rider’s solution cache project-wide: add
/.idea/(you already ignore JetBrains plugin under Assets).Proposed additions:
+.idea/ +Thumbs.db.github/workflows/unity-tests.yml (4)
3-9: Run on pull requests too for pre-merge feedback.Currently triggers only on pushes to main. Add pull_request and workflow_dispatch to get CI signal on PRs and allow manual runs.
on: - push: - branches: [ main ] - paths: - - TestProjects/UnityMCPTests/** - - UnityMcpBridge/Editor/** - - .github/workflows/unity-tests.yml + push: + branches: [ main ] + paths: + - TestProjects/UnityMCPTests/** + - UnityMcpBridge/** + - .github/workflows/unity-tests.yml + pull_request: + branches: [ main ] + paths: + - TestProjects/UnityMCPTests/** + - UnityMcpBridge/** + - .github/workflows/unity-tests.yml + workflow_dispatch:Note: I also broadened paths from UnityMcpBridge/Editor/** to UnityMcpBridge/** so runtime changes also trigger tests.
24-24: Add concurrency and minimal permissions for better CI hygiene.Helps prevent overlapping runs on same branch and follows least-privilege practice.
jobs: + # Avoid parallel runs per ref; cancel older ones when new commits arrive + concurrency: + group: unity-tests-${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true testAllModes:Add at top-level (sibling to
on:):+permissions: + contents: read
27-33: Pin actions to commit SHAs (security best practice).actions/checkout@v4 and actions/cache@v4 should be pinned to specific SHAs to mitigate supply-chain risk.
Example:
- uses: actions/checkout@v4 + uses: actions/checkout@3df4ab11eba7bda6032a0b82a6bb43b11571feac # v4.1.7 - - uses: actions/cache@v4 + - uses: actions/cache@0c45773b623bea8c8e75f6c82b208c3cf94ea4f9 # v4.0.2 - uses: game-ci/unity-test-runner@v4 + uses: game-ci/unity-test-runner@4a5a496a1dd2c0406b5d06da9b2d39d1d5b94574 # v4Replace SHAs with the exact versions you intend to trust.
41-52: Optional: collect coverage or logs explicitly.game-ci/unity-test-runner can output coverage and logs. If desired, enable coverage or upload Editor.log separately for easier triage.
Example to enable coverage:
with: projectPath: ${{ matrix.projectPath }} unityVersion: ${{ matrix.unityVersion }} testMode: ${{ matrix.testMode }} + coverageOptions: 'generateAdditionalMetrics;generateHtmlReport'And attach logs:
- uses: actions/upload-artifact@v4 if: always() with: name: Test results for ${{ matrix.testMode }} path: ${{ steps.tests.outputs.artifactsPath }} + - uses: actions/upload-artifact@v4 + if: always() + with: + name: Editor log for ${{ matrix.testMode }} + path: ${{ matrix.projectPath }}/Library/Logs/Unity/Editor.logTestProjects/UnityMCPTests/Packages/packages-lock.json (1)
3-10: Newtonsoft.Json version mismatch between dependency declaration and resolved version.The local package
com.coplaydev.unity-mcpdeclares"com.unity.nuget.newtonsoft-json": "3.0.2", but the project resolved to3.2.1. UPM typically resolves a single version; if your package strictly requires 3.0.2 APIs, this may cause inconsistencies across environments.
- Prefer updating UnityMcpBridge/package.json to depend on 3.2.1 if compatible.
- Alternatively, remove the explicit dependency if it’s redundant and consumers already bring it in.
I can open a follow-up PR to bump UnityMcpBridge’s dependency to 3.2.1 after verifying API compatibility.
UnityMcpBridge/Editor/Data/McpClients.cs (1)
101-101: Fix comment numbering for Kiro entryThe comment label is out of sequence compared to the preceding numbered items.
Apply this diff:
- // 3) Kiro + // 6) KiroUnityMcpBridge/Editor/Windows/ManualConfigEditorWindow.cs (1)
97-103: Correct instruction string: duplicate “Settings > Settings”The Kiro instruction likely has a typo: “File > Settings > Settings …”. Suggest “File > Settings …”.
Apply this diff:
- EditorGUILayout.LabelField( - " a) Going to File > Settings > Settings > Search for \"MCP\" > Open Workspace MCP Config", - instructionStyle - ); + EditorGUILayout.LabelField( + " a) Going to File > Settings > Search for \"MCP\" > Open Workspace MCP Config", + instructionStyle + );Optional: Given the growing number of editor-specific instruction strings, consider centralizing them in a map keyed by McpTypes to reduce the if-else chain. I can draft a small helper to map types to instructions if you want.
TestProjects/UnityMCPTests/Assets/Tests/EditMode/CommandRegistryTests.cs (2)
18-28: Broaden coverage with a parameterized test for all registered commandsInstead of a single positive test, parameterize to validate every mapping in CommandRegistry. This catches future regressions when adding/removing handlers.
Apply this diff to replace the specific ManageGameObject test with a generic parameterized one:
- [Test] - public void GetHandler_ReturnsManageGameObjectHandler() - { - var handler = CommandRegistry.GetHandler("HandleManageGameObject"); - Assert.IsNotNull(handler, "Expected a handler for HandleManageGameObject."); - - var methodInfo = handler.Method; - Assert.AreEqual("HandleCommand", methodInfo.Name, "Handler method name should be HandleCommand."); - Assert.AreEqual(typeof(ManageGameObject), methodInfo.DeclaringType, "Handler should be declared on ManageGameObject."); - Assert.IsNull(handler.Target, "Handler should be a static method (no target instance)."); - } + [TestCase("HandleManageScript", typeof(ManageScript))] + [TestCase("HandleManageScene", typeof(ManageScene))] + [TestCase("HandleManageEditor", typeof(ManageEditor))] + [TestCase("HandleManageGameObject", typeof(ManageGameObject))] + [TestCase("HandleManageAsset", typeof(ManageAsset))] + [TestCase("HandleReadConsole", typeof(ReadConsole))] + [TestCase("HandleExecuteMenuItem", typeof(ExecuteMenuItem))] + [TestCase("HandleManageShader", typeof(ManageShader))] + public void GetHandler_ReturnsExpectedHandler(string commandName, Type expectedDeclaringType) + { + var handler = CommandRegistry.GetHandler(commandName); + Assert.IsNotNull(handler, $"Expected a handler for {commandName}."); + + var methodInfo = handler.Method; + Assert.AreEqual("HandleCommand", methodInfo.Name, "Handler method name should be HandleCommand."); + Assert.AreEqual(expectedDeclaringType, methodInfo.DeclaringType, $"Handler should be declared on {expectedDeclaringType.Name}."); + Assert.IsNull(handler.Target, "Handler should be a static method (no target instance)."); + }
21-21: Nit: Avoid hardcoded handler nameDerive the handler key from the type to reduce string literal drift.
Apply this minimal change:
- var handler = CommandRegistry.GetHandler("HandleManageGameObject"); + var handler = CommandRegistry.GetHandler("Handle" + nameof(ManageGameObject));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
TestProjects/UnityMCPTests/Assets/Scenes/SampleScene.unityis excluded by!**/*.unitypackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (22)
.github/workflows/unity-tests.yml(1 hunks).gitignore(1 hunks)TestProjects/UnityMCPTests/.gitignore(1 hunks)TestProjects/UnityMCPTests/Assets/Scenes.meta(1 hunks)TestProjects/UnityMCPTests/Assets/Scenes/SampleScene.unity.meta(1 hunks)TestProjects/UnityMCPTests/Assets/Scripts.meta(1 hunks)TestProjects/UnityMCPTests/Assets/Scripts/Hello.cs(1 hunks)TestProjects/UnityMCPTests/Assets/Scripts/Hello.cs.meta(1 hunks)TestProjects/UnityMCPTests/Assets/Tests.meta(1 hunks)TestProjects/UnityMCPTests/Assets/Tests/EditMode.meta(1 hunks)TestProjects/UnityMCPTests/Assets/Tests/EditMode/CommandRegistryTests.cs(1 hunks)TestProjects/UnityMCPTests/Assets/Tests/EditMode/CommandRegistryTests.cs.meta(1 hunks)TestProjects/UnityMCPTests/Assets/Tests/EditMode/UnityMCPTests.EditMode.asmdef(1 hunks)TestProjects/UnityMCPTests/Assets/Tests/EditMode/UnityMCPTests.EditMode.asmdef.meta(1 hunks)TestProjects/UnityMCPTests/Packages/manifest.json(1 hunks)TestProjects/UnityMCPTests/Packages/packages-lock.json(1 hunks)TestProjects/UnityMCPTests/ProjectSettings/Packages/com.unity.testtools.codecoverage/Settings.json(1 hunks)TestProjects/UnityMCPTests/ProjectSettings/ProjectVersion.txt(1 hunks)UnityMcpBridge/Editor/Data/McpClients.cs(1 hunks)UnityMcpBridge/Editor/Models/McpTypes.cs(1 hunks)UnityMcpBridge/Editor/Windows/ManualConfigEditorWindow.cs(1 hunks)package.json(0 hunks)
💤 Files with no reviewable changes (1)
- package.json
🧰 Additional context used
🧬 Code Graph Analysis (2)
TestProjects/UnityMCPTests/Assets/Scripts/Hello.cs (1)
UnityMcpBridge/Editor/Tools/ManageScript.cs (1)
System(759-825)
TestProjects/UnityMCPTests/Assets/Tests/EditMode/CommandRegistryTests.cs (2)
UnityMcpBridge/Editor/Tools/CommandRegistry.cs (1)
CommandRegistry(10-45)UnityMcpBridge/Editor/Tools/ManageGameObject.cs (1)
ManageGameObject(20-2153)
🔇 Additional comments (19)
TestProjects/UnityMCPTests/ProjectSettings/ProjectVersion.txt (1)
1-2: Ensure CI uses Unity 2021.3.45f1
We didn’t find any GitHub Actions workflows in .github/workflows. Please confirm that whatever CI you’re using—GitHub Actions, Azure Pipelines, Jenkins, etc.—is installing and running Unity 2021.3.45f1 to match ProjectSettings/ProjectVersion.txt. Pinning the editor version in your CI will prevent import/compile mismatches and avoid flaky test runs.TestProjects/UnityMCPTests/Assets/Scripts/Hello.cs (1)
5-11: Simple Hello World MonoBehaviour — LGTMNo issues. This is fine for sanity checks in the sample scene.
UnityMcpBridge/Editor/Models/McpTypes.cs (1)
10-10: Verify McpTypes.Kiro coverage across branching
I couldn’t find any switch statements or case labels explicitly handlingMcpTypes.Kiro. Please manually review all logic that branches onMcpTypes(switches, if/else chains, default/else handlers) and ensure the newKiromember is covered to avoid falling into unintended default paths.TestProjects/UnityMCPTests/Assets/Tests/EditMode.meta (1)
1-8: Unity meta for EditMode tests — LGTMFolder metadata looks standard; no issues.
.gitignore (1)
6-6: Ignore .kiro — LGTMThis will prevent accidental check-ins of local Kiro config directories if present within the repo.
TestProjects/UnityMCPTests/Assets/Scripts.meta (1)
1-8: LGTM: Standard folder .meta committedThe folder meta looks correct (fileFormatVersion, folderAsset: yes, DefaultImporter). Good to have it tracked for GUID stability across machines.
TestProjects/UnityMCPTests/Assets/Scenes/SampleScene.unity.meta (1)
1-7: LGTM: Scene .meta is correctStandard DefaultImporter block and GUID present. Nothing to change.
TestProjects/UnityMCPTests/Assets/Tests.meta (1)
1-8: LGTM: Tests folder .meta committedFolder meta is well-formed; good for stable references in VCS.
TestProjects/UnityMCPTests/Assets/Scripts/Hello.cs.meta (2)
1-11: LGTM: Script .meta uses MonoImporter with default settingsSerialized version, empty defaultReferences, and executionOrder 0 are standard for a simple script.
2-2: Verify Hello.cs Asset Usage and Scene/Prefab ReferencesI ran a search for the GUID bebdf68a6876b425494ee770d20f70ef across all
.unity,.prefab, and relevant.assetfiles and found no occurrences.• If Hello.cs is intended to be attached to any GameObjects, please add it to your scenes or prefabs to avoid “Missing (Mono Script)” warnings.
• If this script isn’t yet used, consider removing the.cs.meta(and the script itself) to keep the project clean.TestProjects/UnityMCPTests/Packages/manifest.json (1)
3-3: Verify local package path correctness in CI.
"file:../../../UnityMcpBridge"looks correct relative to Packages/, resolving to repo-root/UnityMcpBridge. Just flagging to ensure CI runner’s working directory is the repo root (actions/checkout default), which your job satisfies.If you’ve seen any resolution issues locally/CI, I can help generate a quick script to confirm the relative path and package.json validity in UnityMcpBridge.
TestProjects/UnityMCPTests/Assets/Scenes.meta (1)
1-8: LGTM — standard folder meta.Meta looks canonical for a Scenes folder. No issues.
.github/workflows/unity-tests.yml (1)
23-23: Ensure Unity version matches project settings.You set 2021.3.45f1, which should match ProjectSettings/ProjectVersion.txt. Just a heads-up to keep these aligned to avoid editor reimport churn and flaky caches.
I can add a guard step that reads ProjectVersion.txt and fails if it diverges from matrix.unityVersion.
TestProjects/UnityMCPTests/Packages/packages-lock.json (1)
1-416: Committing the lockfile is good; keep it updated with manifest changes.packages-lock.json looks consistent with the manifest and Unity 2021 LTS. No actionable issues.
TestProjects/UnityMCPTests/Assets/Tests/EditMode/UnityMCPTests.EditMode.asmdef.meta (1)
1-8: LGTM: Standard asmdef.meta contentThis looks like a standard Unity meta file with a stable GUID. No issues.
TestProjects/UnityMCPTests/Assets/Tests/EditMode/CommandRegistryTests.cs.meta (1)
1-12: LGTM: Standard .meta for C# scriptConventional Unity metadata; nothing to change here.
UnityMcpBridge/Editor/Data/McpClients.cs (2)
117-118: LGTM: New enum reference and default status
mcpType = McpTypes.Kiroand initializingconfigStatusto "Not Configured" align with the constructor logic that assignsMcpStatus.NotConfigured. Good consistency.
105-116: User-level MCP config path is correct on Windows, macOS, and Linux
According to the official Kiro documentation, the global MCP configuration file lives at~/.kiro/settings/mcp.jsonon all platforms. In .NET,Environment.GetFolderPath(Environment.SpecialFolder.UserProfile)correctly resolves to:
%USERPROFILE%on Windows/Users/<you>on macOS/home/<you>on LinuxThe current implementation in
UnityMcpBridge/Editor/Data/McpClients.csalready matches this behavior. No changes required.TestProjects/UnityMCPTests/Assets/Tests/EditMode/CommandRegistryTests.cs (1)
10-16: LGTM: Unknown command returns null as expectedGood negative test validating null return for unknown command names.
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Clarify case sensitivity and (optionally) add a test once finalized
CommandRegistry.GetHandler currently uses a default Dictionary comparer, which is case-sensitive, but the in-code comment suggests case-insensitive lookup “for flexibility.” Decide on the contract:
- If case-sensitive is intended: remove/adjust the comment and keep tests as-is.
- If case-insensitive is intended: change the registry to use StringComparer.OrdinalIgnoreCase and add a test like below.
To verify the current implementation details, run:
If you choose case-insensitive behavior, update the registry (in CommandRegistry.cs) roughly like:
private static readonly Dictionary<string, Func<JObject, object>> _handlers =
new Dictionary<string, Func<JObject, object>>(StringComparer.OrdinalIgnoreCase)
{
// existing mappings...
};Then add this test in this file:
public class CommandRegistryTests
{
+ [Test]
+ public void GetHandler_IsCaseInsensitive_WhenEnabled()
+ {
+ // This test is valid only if CommandRegistry uses a case-insensitive comparer.
+ var handler = CommandRegistry.GetHandler("handlemanagegameobject");
+ Assert.IsNotNull(handler, "Expected a handler for case-variant command name when case-insensitivity is enabled.");
+ }🏁 Script executed:
#!/bin/bash
# Show the handlers dictionary declaration and GetHandler implementation context
rg -n -C3 'class\s+CommandRegistry\b' UnityMcpBridge/Editor/Tools/CommandRegistry.cs
rg -n -C3 '_handlers\s*=\s*new\s*\(' UnityMcpBridge/Editor/Tools/CommandRegistry.cs
rg -n -C2 'GetHandler\s*\(' UnityMcpBridge/Editor/Tools/CommandRegistry.csLength of output: 1465
Clarify and enforce CommandRegistry case-sensitivity
The _handlers dictionary is currently instantiated with the default comparer (case-sensitive), but the in-code comment in GetHandler (line 33) states “Use case-insensitive comparison for flexibility.” Please choose one of the two behaviors and align the implementation, comments, and tests accordingly:
• If case-sensitive lookup is intended:
- Update or remove the comment on line 33 in
UnityMcpBridge/Editor/Tools/CommandRegistry.csso it no longer refers to case-insensitivity. - No additional tests required.
• If case-insensitive lookup is intended:
- Change the dictionary instantiation in
UnityMcpBridge/Editor/Tools/CommandRegistry.cs(around line 14) to:private static readonly Dictionary<string, Func<JObject, object>> _handlers = new Dictionary<string, Func<JObject, object>>(StringComparer.OrdinalIgnoreCase) { // existing mappings… };
- Add a unit test in
TestProjects/UnityMCPTests/Assets/Tests/EditMode/CommandRegistryTests.csto verify case-insensitive behavior:public class CommandRegistryTests { + [Test] + public void GetHandler_IsCaseInsensitive() + { + var handler = CommandRegistry.GetHandler("handlemanagegameobject"); + Assert.IsNotNull(handler, "Expected a handler for case-variant command names when case-insensitivity is enabled."); + } }
🤖 Prompt for AI Agents
In TestProjects/UnityMCPTests/Assets/Tests/EditMode/CommandRegistryTests.cs
around lines 28 to 30, the review notes a mismatch between the comment in
UnityMcpBridge/Editor/Tools/CommandRegistry.cs (saying case-insensitive) and the
dictionary instantiation (default case-sensitive); make lookups
case-insensitive: update UnityMcpBridge/Editor/Tools/CommandRegistry.cs near
line 14 to instantiate _handlers with StringComparer.OrdinalIgnoreCase so keys
are compared case-insensitively, and add a unit test in
TestProjects/UnityMCPTests/Assets/Tests/EditMode/CommandRegistryTests.cs that
verifies a registered handler can be retrieved using a different-case command
name (assert non-null and expected behavior).
| { | ||
| "name": "UnityMCPTests.EditMode", | ||
| "rootNamespace": "", | ||
| "references": [ | ||
| "UnityMcpBridge.Editor", | ||
| "UnityEngine.TestRunner", | ||
| "UnityEditor.TestRunner" | ||
| ], | ||
| "includePlatforms": [ | ||
| "Editor" | ||
| ], | ||
| "excludePlatforms": [], | ||
| "allowUnsafeCode": false, | ||
| "overrideReferences": true, | ||
| "precompiledReferences": [ | ||
| "nunit.framework.dll", | ||
| "Newtonsoft.Json.dll" | ||
| ], | ||
| "autoReferenced": false, | ||
| "defineConstraints": [ | ||
| "UNITY_INCLUDE_TESTS" | ||
| ], | ||
| "versionDefines": [], | ||
| "noEngineReferences": false | ||
| } No newline at end of file |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Reassess precompiledReferences and overrideReferences
Specifying “nunit.framework.dll” and “Newtonsoft.Json.dll” as precompiledReferences usually requires those DLLs to be present under Assets. Since you already reference Unity’s Test Runners, NUnit is provided via modules, and Newtonsoft.Json is typically provided via the com.unity.nuget.newtonsoft-json package (not a precompiled DLL). With overrideReferences=true, Unity uses only your explicit references, which can break if those DLLs aren’t present.
- If those DLLs are not actually in the project, remove precompiledReferences and set overrideReferences to false. Unity will resolve NUnit via the TestRunner modules and Newtonsoft via its package.
Suggested diff:
"allowUnsafeCode": false,
- "overrideReferences": true,
- "precompiledReferences": [
- "nunit.framework.dll",
- "Newtonsoft.Json.dll"
- ],
+ "overrideReferences": false,
"autoReferenced": false,To verify whether the precompiled DLLs exist locally:
Expected:
- If no results: apply the suggested diff.
- If present: keep as-is, but ensure correct versions and no duplicates with package-provided assemblies.
🏁 Script executed:
#!/bin/bash
# Search for the DLLs in the test project
fd -a -H -i 'nunit.framework.dll|Newtonsoft.Json.dll' TestProjects/UnityMCPTestsLength of output: 82
🏁 Script executed:
#!/bin/bash
# Robust search for precompiled DLLs across the TestProjects directory
find TestProjects -type f \( -iname 'nunit.framework.dll' -o -iname 'Newtonsoft.Json.dll' \)Length of output: 92
🏁 Script executed:
#!/bin/bash
# Verify presence of the Newtonsoft.Json package in manifest.json
grep -R '"com.unity.nuget.newtonsoft-json"' -n Packages/manifest.json || echo "Newtonsoft.Json package not referenced"Length of output: 214
🏁 Script executed:
#!/bin/bash
# Search for precompiled DLLs across the entire repo
echo "Searching for nunit.framework.dll and Newtonsoft.Json.dll..."
find . -type f \( -iname "nunit.framework.dll" -o -iname "Newtonsoft.Json.dll" \) -print || echo "No precompiled DLLs found"
# Locate manifest.json files and check for com.unity.nuget.newtonsoft-json package references
echo "Locating manifest.json and checking for com.unity.nuget.newtonsoft-json..."
manifest_files=$(find . -type f -iname 'manifest.json')
if [ -z "$manifest_files" ]; then
echo "No manifest.json found"
else
for mf in $manifest_files; do
echo "Checking $mf:"
grep -Hn '"com.unity.nuget.newtonsoft-json"' "$mf" || echo "Not referenced in $mf"
done
fiLength of output: 960
Update UnityMCPTests.EditMode.asmdef to remove hard-coded DLLs and restore default references
The scripts confirm that neither nunit.framework.dll nor Newtonsoft.Json.dll exist in the repo, and the local manifest doesn’t reference the Newtonsoft JSON package. With overrideReferences: true, Unity will ignore its built-in Test Runner and JSON assemblies, leading to missing-assembly errors.
Apply the following changes in TestProjects/UnityMCPTests/Assets/Tests/EditMode/UnityMCPTests.EditMode.asmdef:
"allowUnsafeCode": false,
- "overrideReferences": true,
- "precompiledReferences": [
- "nunit.framework.dll",
- "Newtonsoft.Json.dll"
- ],
+ "overrideReferences": false,
"autoReferenced": false,After this:
- Unity will automatically include the Test Runner’s NUnit assemblies.
- Unity will pick up Newtonsoft.Json from the project’s package graph (ensure you’ve added
com.unity.nuget.newtonsoft-jsonto your rootPackages/manifest.jsonif it isn’t already).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { | |
| "name": "UnityMCPTests.EditMode", | |
| "rootNamespace": "", | |
| "references": [ | |
| "UnityMcpBridge.Editor", | |
| "UnityEngine.TestRunner", | |
| "UnityEditor.TestRunner" | |
| ], | |
| "includePlatforms": [ | |
| "Editor" | |
| ], | |
| "excludePlatforms": [], | |
| "allowUnsafeCode": false, | |
| "overrideReferences": true, | |
| "precompiledReferences": [ | |
| "nunit.framework.dll", | |
| "Newtonsoft.Json.dll" | |
| ], | |
| "autoReferenced": false, | |
| "defineConstraints": [ | |
| "UNITY_INCLUDE_TESTS" | |
| ], | |
| "versionDefines": [], | |
| "noEngineReferences": false | |
| } | |
| { | |
| "name": "UnityMCPTests.EditMode", | |
| "rootNamespace": "", | |
| "references": [ | |
| "UnityMcpBridge.Editor", | |
| "UnityEngine.TestRunner", | |
| "UnityEditor.TestRunner" | |
| ], | |
| "includePlatforms": [ | |
| "Editor" | |
| ], | |
| "excludePlatforms": [], | |
| "allowUnsafeCode": false, | |
| "overrideReferences": false, | |
| "autoReferenced": false, | |
| "defineConstraints": [ | |
| "UNITY_INCLUDE_TESTS" | |
| ], | |
| "versionDefines": [], | |
| "noEngineReferences": false | |
| } |
🤖 Prompt for AI Agents
In
TestProjects/UnityMCPTests/Assets/Tests/EditMode/UnityMCPTests.EditMode.asmdef
(lines 1–25), remove the hard-coded precompiledReferences and restore Unity's
default assembly resolution by deleting the "precompiledReferences" entry (the
array containing "nunit.framework.dll" and "Newtonsoft.Json.dll") and set
"overrideReferences" to false (or remove that property entirely); this lets
Unity automatically include the Test Runner/NUnit assemblies and allow
Newtonsoft.Json to be resolved from the package manifest (ensure
com.unity.nuget.newtonsoft-json is in Packages/manifest.json if needed).
| { | ||
| "m_Name": "Settings", | ||
| "m_Path": "ProjectSettings/Packages/com.unity.testtools.codecoverage/Settings.json", | ||
| "m_Dictionary": { | ||
| "m_DictionaryValues": [] | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Enable/align code coverage configuration with CI workflow
This settings file is currently an empty dictionary. If the CI is expected to collect coverage, ensure either:
- The workflow runs Unity with -enableCodeCoverage and proper coverage options, or
- You populate this Settings.json with the desired include/exclude patterns and output path.
Recommend verifying the workflow configuration to avoid “coverage enabled but nothing collected” scenarios.
You can run this script to check whether your workflows enable coverage from the CLI:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for Unity coverage flags in workflows..."
fd -t f -E node_modules -E Library -e yml -e yaml .github 2>/dev/null | xargs -r rg -n -C2 -i -e 'enableCodeCoverage' -e 'codecoverage' -e 'coverage' || true
echo
echo "Searching for game-ci/unity-test-runner usage..."
fd -t f -E node_modules -E Library -e yml -e yaml .github 2>/dev/null | xargs -r rg -n -C2 -i 'game-ci/unity-test-runner' || trueLength of output: 489
CI workflows currently don’t enable Unity code coverage
Our search in .github/workflows found no occurrences of -enableCodeCoverage, codecoverage, or game-ci/unity-test-runner. Since your Settings.json at ProjectSettings/Packages/com.unity.testtools.codecoverage/Settings.json is an empty dictionary, no coverage data will be produced. To fix this:
- In your workflow YAML (e.g. under
.github/workflows/), add the Unity CLI flags:- name: Run tests with coverage run: | unity-editor \ -runTests \ -projectPath . \ -enableCodeCoverage \ -coverageResultsPath TestResults/coverage.xml \ -coverageOptions <include/exclude patterns>
- Or populate
Settings.jsonwith your include/exclude patterns and output path, for example:{ "m_Name": "Settings", "m_Path": "ProjectSettings/Packages/com.unity.testtools.codecoverage/Settings.json", "m_Dictionary": { "m_DictionaryValues": [ { "first": "IncludeAssemblies", "second": ["Assembly-CSharp", "Assembly-CSharp-Editor"] }, { "first": "ExcludePaths", "second": ["**/Tests/**"] }, { "first": "ResultsPath", "second": "TestResults/coverage.xml" } ] } }
Without one of these changes, “coverage enabled but nothing collected” is expected. Please update your CI configuration accordingly.
🤖 Prompt for AI Agents
In
TestProjects/UnityMCPTests/ProjectSettings/Packages/com.unity.testtools.codecoverage/Settings.json
(lines 1-7) the coverage settings dictionary is empty so Unity will not produce
coverage; fix by either updating your CI workflow under .github/workflows to run
Unity with the test-runner and enable code coverage flags (include -runTests,
-enableCodeCoverage and set -coverageResultsPath and -coverageOptions) or by
populating this Settings.json m_DictionaryValues with entries for
IncludeAssemblies, ExcludePaths and ResultsPath (e.g., set assemblies to
include, exclude test folders, and ResultsPath to TestResults/coverage.xml) so
coverage data is actually collected.
|
@msanatan I had to get on a waitlist for Kiro -- so couldn't test. But if it works for you, good by me to merge it and ask the guy from the issue to test. |
The waitlist was ridiculously long, but now they have pricing for different tiers 😅. I'll ping the person who raised it, thanks! |
2 participants
Since it was asked for. I don't like the IDE but it's likely going to be popular for some time
Summary by CodeRabbit
New Features
Tests
Chores