feat: add component_properties to manage_prefabs modify_contents (#793)#802
Merged
dsarno merged 3 commits intoCoplayDev:betafrom Feb 21, 2026
Merged
Conversation
…v#793) Add component_properties parameter to manage_prefabs modify_contents, enabling AI agents to set serialized fields on prefab components without opening the prefab editor. Reuses existing ComponentOps.SetProperty for reflection, SerializedProperty, and object reference resolution (by GUID, path, or instanceID). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Reviewer's GuideAdds a new component_properties parameter to the manage_prefabs modify_contents action, wiring it through the Python tool layer into the Unity C# prefab modification pipeline to set serialized properties on existing components, with accompanying tests for behavior and validation. Sequence diagram for manage_prefabs modify_contents with component_propertiessequenceDiagram
actor Developer
participant PythonManagePrefabs as Python_manage_prefabs
participant UnityBridge as Unity_Tool_Bridge
participant CSharpManagePrefabs as CSharp_ManagePrefabs
participant ComponentResolver
participant GameObject
participant Component
participant ComponentOps
participant ErrorResponse
Developer->>PythonManagePrefabs: manage_prefabs(action=modify_contents, component_properties)
PythonManagePrefabs->>PythonManagePrefabs: Validate parameters
PythonManagePrefabs->>PythonManagePrefabs: params["componentProperties"] = component_properties
PythonManagePrefabs->>UnityBridge: Send request with componentProperties
UnityBridge->>CSharpManagePrefabs: ApplyModificationsToPrefabObject(@params, targetGo)
CSharpManagePrefabs->>CSharpManagePrefabs: Read @params["componentProperties"] or @params["component_properties"]
alt componentProperties present and nonempty
loop For each typeName in componentProperties
CSharpManagePrefabs->>ComponentResolver: TryResolve(typeName)
alt Resolution fails
ComponentResolver-->>CSharpManagePrefabs: resolveError
CSharpManagePrefabs->>ErrorResponse: new ErrorResponse("Component type not found")
CSharpManagePrefabs-->>UnityBridge: (modified=false, error)
UnityBridge-->>PythonManagePrefabs: ErrorResponse
PythonManagePrefabs-->>Developer: Raise error
else Resolution succeeds
ComponentResolver-->>CSharpManagePrefabs: componentType
CSharpManagePrefabs->>GameObject: GetComponent(componentType)
alt Component missing
GameObject-->>CSharpManagePrefabs: null
CSharpManagePrefabs->>ErrorResponse: new ErrorResponse("Component not found on target")
CSharpManagePrefabs-->>UnityBridge: (modified=false, error)
UnityBridge-->>PythonManagePrefabs: ErrorResponse
PythonManagePrefabs-->>Developer: Raise error
else Component found
GameObject-->>CSharpManagePrefabs: component
loop For each property in componentProperties[typeName]
CSharpManagePrefabs->>ComponentOps: SetProperty(component, propertyName, value)
alt SetProperty succeeds
ComponentOps-->>CSharpManagePrefabs: setError=null
CSharpManagePrefabs->>CSharpManagePrefabs: modified = true
else SetProperty fails
ComponentOps-->>CSharpManagePrefabs: setError
CSharpManagePrefabs->>CSharpManagePrefabs: errors.Add(typeName.propertyName)
end
end
end
end
end
alt Any errors collected
CSharpManagePrefabs->>ErrorResponse: new ErrorResponse("Failed to set some component properties")
CSharpManagePrefabs-->>UnityBridge: (modified=false, error)
else No errors
CSharpManagePrefabs-->>UnityBridge: (modified=true, error=null)
end
else No componentProperties
CSharpManagePrefabs-->>UnityBridge: (modified, error) from other modifications
end
UnityBridge-->>PythonManagePrefabs: Result
PythonManagePrefabs-->>Developer: Return modify_contents result
Updated class diagram for Unity prefab modification with component_propertiesclassDiagram
class ManagePrefabs {
<<static>>
+ApplyModificationsToPrefabObject(JObject params, GameObject targetGo) (bool modified, ErrorResponse error)
}
class ComponentResolver {
+TryResolve(string typeName, out Type componentType, out string resolveError) bool
}
class ComponentOps {
+SetProperty(Component component, string propertyName, JToken value, out string setError) bool
}
class ErrorResponse {
+string message
+ErrorResponse(string message)
}
class GameObject {
+string name
+GetComponent(Type componentType) Component
}
class Component {
}
ManagePrefabs ..> ComponentResolver : uses
ManagePrefabs ..> ComponentOps : uses
ManagePrefabs ..> ErrorResponse : creates
ManagePrefabs ..> GameObject : modifies
GameObject o--> Component : contains
ManagePrefabs ..> Component : gets and updates
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughThis PR introduces support for modifying serialized fields on prefab components by adding a new Changes
Sequence DiagramsequenceDiagram
participant Agent as Client/Agent
participant ServerTool as Server Tool<br/>(manage_prefabs)
participant EditorHandler as Editor Handler<br/>(ModifyContents)
participant ComponentOps as ComponentOps
participant UnityAPI as Unity API
Agent->>ServerTool: modify_contents with component_properties
ServerTool->>EditorHandler: dispatch with componentProperties param
EditorHandler->>EditorHandler: iterate component_properties dict
EditorHandler->>EditorHandler: resolve component type by name
EditorHandler->>EditorHandler: verify component exists on target
EditorHandler->>ComponentOps: SetProperty(component, prop_name, value)
ComponentOps->>UnityAPI: apply property value
UnityAPI-->>ComponentOps: success/error
ComponentOps-->>EditorHandler: property result
EditorHandler->>EditorHandler: accumulate errors
alt All properties succeed
EditorHandler->>UnityAPI: mark prefab modified
UnityAPI-->>EditorHandler: modified
EditorHandler-->>ServerTool: success response
else Any property fails
EditorHandler->>EditorHandler: consolidate error details
EditorHandler-->>ServerTool: error with failure summary
end
ServerTool-->>Agent: result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The change to TestProjects/UnityMCPTests/Packages/manifest.json hardcodes a user-specific absolute path for com.coplaydev.unity-mcp; please revert this to a project-relative path so the test project remains portable for other developers and CI.
- In ManagePrefabs.ApplyModificationsToPrefabObject, missing/invalid component types short-circuit and return immediately while property-set failures are aggregated; consider aggregating component-level errors as well (or allowing partial success) for more consistent error handling when multiple component types are specified.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The change to TestProjects/UnityMCPTests/Packages/manifest.json hardcodes a user-specific absolute path for com.coplaydev.unity-mcp; please revert this to a project-relative path so the test project remains portable for other developers and CI.
- In ManagePrefabs.ApplyModificationsToPrefabObject, missing/invalid component types short-circuit and return immediately while property-set failures are aggregated; consider aggregating component-level errors as well (or allowing partial success) for more consistent error handling when multiple component types are specified.
## Individual Comments
### Comment 1
<location> `TestProjects/UnityMCPTests/Packages/manifest.json:3` </location>
<code_context>
{
"dependencies": {
- "com.coplaydev.unity-mcp": "file:../../../MCPForUnity",
+ "com.coplaydev.unity-mcp": "file:/Users/davidsarno/unity-mcp/MCPForUnity",
"com.unity.ai.navigation": "1.1.4",
"com.unity.collab-proxy": "2.5.2",
</code_context>
<issue_to_address>
**issue (bug_risk):** Using a user-specific absolute file path in the manifest will break on other machines/CI.
This absolute path will only work on your machine. Please revert to a relative `file:` reference or switch to a versioned package source so the project remains portable across machines and CI.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| { | ||
| "dependencies": { | ||
| "com.coplaydev.unity-mcp": "file:../../../MCPForUnity", | ||
| "com.coplaydev.unity-mcp": "file:/Users/davidsarno/unity-mcp/MCPForUnity", |
Contributor
There was a problem hiding this comment.
issue (bug_risk): Using a user-specific absolute file path in the manifest will break on other machines/CI.
This absolute path will only work on your machine. Please revert to a relative file: reference or switch to a versioned package source so the project remains portable across machines and CI.
- Revert manifest.json from absolute path back to relative file:../../../MCPForUnity - Change componentProperties error handling to aggregate all errors (missing types, missing components, property failures) instead of short-circuiting on the first component-level error Addresses PR CoplayDev#802 review feedback from Sourcery. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManagePrefabsCrudTests.cs (1)
725-921: LGTM — good coverage of success, multi-component, child-target, and both error cases.One nit: Line 798 calls
EnsureFolder(TempDirectory)insideSetsOnChildTarget, butSetUp()already guarantees the folder exists. Harmless, but slightly inconsistent with the other new tests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManagePrefabsCrudTests.cs` around lines 725 - 921, The test ModifyContents_ComponentProperties_SetsOnChildTarget redundantly calls EnsureFolder(TempDirectory) even though SetUp() already ensures the folder exists; remove the EnsureFolder(TempDirectory) call from the ModifyContents_ComponentProperties_SetsOnChildTarget test method so it matches the other tests' setup and avoids the unnecessary folder creation step, leaving the rest of the method (creation of root/child GameObjects, saving prefab, and assertions) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@MCPForUnity/Editor/Tools/Prefabs/ManagePrefabs.cs`:
- Around line 761-803: The current ApplyModificationsToPrefabObject flow returns
early on type-resolution or missing-component errors
(ComponentResolver.TryResolve, targetGo.GetComponent) which discards previously
accumulated property errors and also returns before SaveAsPrefabAsset so no
changes are persisted; change the behavior to always accumulate all errors into
the existing errors list (instead of returning early) for failures from
ComponentResolver.TryResolve and missing component checks, and only return once
after processing all component entries; update the final error message to state
that "no changes were saved" (or similar) instead of "some", and only proceed to
call SaveAsPrefabAsset when errors.Count == 0 so modifications are persisted
only on success.
In `@Server/tests/test_manage_prefabs.py`:
- Around line 22-32: The test
test_tool_description_mentions_component_properties can raise a KeyError when
prefab_tool lacks a "kwargs" key; change the description lookup to use safe .get
calls so desc = prefab_tool.get("description") or prefab_tool.get("kwargs",
{}).get("description", "") — i.e., use prefab_tool.get("kwargs", {}) instead of
prefab_tool["kwargs"] when computing desc to avoid KeyError on prefab_tool.
In `@TestProjects/UnityMCPTests/Packages/manifest.json`:
- Line 3: The manifest.json currently points "com.coplaydev.unity-mcp" at an
absolute, single-developer path; change its value to a repository-relative file
URL (e.g., a relative "file:..." path inside the repo) or to a published package
version so other contributors/CI can resolve it; update the entry for
"com.coplaydev.unity-mcp" accordingly (use forward slashes, correct relative
traversal like ../ if the package lives alongside the project) and then verify
by restoring packages in Unity/CI.
---
Nitpick comments:
In
`@TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManagePrefabsCrudTests.cs`:
- Around line 725-921: The test
ModifyContents_ComponentProperties_SetsOnChildTarget redundantly calls
EnsureFolder(TempDirectory) even though SetUp() already ensures the folder
exists; remove the EnsureFolder(TempDirectory) call from the
ModifyContents_ComponentProperties_SetsOnChildTarget test method so it matches
the other tests' setup and avoids the unnecessary folder creation step, leaving
the rest of the method (creation of root/child GameObjects, saving prefab, and
assertions) unchanged.
- Error message now says "no changes saved" instead of misleading "some"
- Use .get("kwargs", {}) to avoid KeyError if registry entry lacks kwargs
Addresses CodeRabbit review feedback on PR CoplayDev#802.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
5 tasks
dsarno
added a commit
that referenced
this pull request
Feb 21, 2026
…udge) (#804) * fix: remove broken root rename assertion from prefab test LoadAssetAtPath returns the asset filename as .name for prefab roots, not the internally renamed root object name. Remove the rename + assert that was causing CombinesWithOtherModifications to fail. The test still verifies that componentProperties works alongside position changes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: remove yield frames causing StdioBridge reconnect test flakiness The 5-frame yield between client2 handshake and ping created a window where the MCP Python server could reconnect and close our test client as stale. Stale-client cleanup runs synchronously in HandleClientAsync before the read loop, so no yield is needed after reading the handshake. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: ensure focus nudge fires and logs correctly during test polling - Add file handler to root logger so __name__-based loggers (focus_nudge, run_tests) write to the log file instead of only stderr - Add fire-and-forget nudge check on non-wait_timeout get_test_job calls so stalls are detected regardless of client polling style - Add 13 unit tests for should_nudge logic, backoff reset, and gating Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address PR #804 review feedback - Store asyncio.create_task reference in module-level set to prevent GC (CodeRabbit) - Add test for _get_frontmost_app() returning None (CodeRabbit nitpick) - Add comment explaining why prefab root rename is not tested (Sourcery) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs: add Claude Code to MCP client examples in README Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #793. Adds
component_propertiesparameter tomanage_prefabs modify_contents, enabling users to set serialized fields on existing components within prefab assets.componentPropertiesinto the prefab editing flow using existingComponentOps.SetProperty()andComponentResolver.TryResolve()— no new abstractions neededcomponent_propertiesparameter with type annotation and forwards it to Unity ascomponentPropertiesUsage
Supports object references via
{"guid": "..."},{"path": "Assets/..."}, or{"instanceID": 123}.Test plan
Generated with Claude Code
Summary by Sourcery
Add support for setting serialized properties on existing components when modifying prefab contents via the manage_prefabs tool.
New Features:
Tests:
Chores:
Summary by CodeRabbit
New Features
component_propertiesparameter. Supports mapping component types to property dictionaries and object references using guid, path, or instance ID.