Summary
Reflector.TryPatch (the JSON Merge Patch path) cannot apply a value whose JSON encoding is an object handled atomically by a registered converter — e.g. a Unity object reference {"instanceID":"…"}. It structurally descends into the node as if it were a bag of members to navigate, instead of delegating to the converter registered for the target member type.
Reported via the Unity-MCP tool gameobject-component-modify:
jsonPatch = { "sharedMaterial": { "instanceID": "568105589213746962" } }
=> "Segment 'instanceID' not found on type 'Material'"
Affects both the object and stringified forms of jsonPatch equally. (Distinct from #82, which fixed the string-vs-object schema/validation; this is the apply-time layer.)
Root cause
src/Reflector/Reflector.Patch.cs — TryPatchInternal branches purely on JSON ValueKind:
- A non-object leaf (string/number/bool/null) is delegated to
TryModify → the converter chain → resolves correctly (Reflector.Patch.cs:109-114).
- An object node is always treated as navigable members and recursed via
TryPatchMember (Reflector.Patch.cs:116+, 194-206). For {"instanceID":"…"} against a Material property it enumerates key instanceID, finds no such member on Material, and throws Segment '<key>' not found on type '<T>' (Reflector.Patch.cs:318).
The converter registry is never consulted for a ValueKind == Object node, so a reference encoded as a JSON object bypasses the converter that knows how to resolve it.
Why the other surfaces work
TryModify (componentDiff/SerializedMember) and TryModifyAt (pathPatches) both funnel the {instanceID:…} object into Converters.GetConverter(type) → the converter's SetValue (Reflector.Modify.cs:70-92). For Unity types that converter (UnityEngine_Object_ReflectionConverter, registered by Unity-MCP) resolves the ref via ToAssetObjectRef().FindAssetObject(type). The merge-patch path never reaches that resolver.
Proposed fix (ReflectorNet)
Add a converter capability flag, e.g. bool TreatJsonObjectAsAtomicValue(Type type) on IReflectionConverter (default false in BaseReflectionConverter). In TryPatchInternal, before the structural key-enumeration block and after the leaf checks (~Reflector.Patch.cs:114): when patch.ValueKind == Object, the target type has a converter, that flag is true, and the node has no $type key → route the node through TryModify (the same path the other two surfaces use). Plain POCO sub-objects keep the flag false and descend exactly as today. Keep $type polymorphic replacement flowing through the existing structural path.
The Unity UnityEngine_Object_ReflectionConverter overrides the new flag to true (one line, in Unity-MCP).
Blast radius / tests
- Callers of
TryPatch: 5 Unity-MCP modify tools (gameobject-component-modify, object-modify, gameobject-modify, assets-modify); ReflectorNet AtomicModifyTests; Unity-MCP TryPatchTests + TestToolGameObject.JsonPatchObjectOrString (the latter deliberately punted on the object-ref case). No direct callers in MCP-Plugin-dotnet/extensions.
- Risk to plain-POCO patching: low when gated on a default-false capability flag — existing
TryPatch_* tests (multi-field, deep, null, $type) unaffected.
- Add a ReflectorNet test using a stub converter whose flag is true +
SetValue consumes a {ref:…} object (proves routing with no Unity dependency).
- Foundational lib: a fix here implies a NuGet bump + downstream consumption (Unity-MCP, MCP-Plugin-dotnet) — a deliberate release cascade.
Acceptance criteria
Discovered while verifying #82 (PR #83) end-to-end against the live gameobject-component-modify tool.
Summary
Reflector.TryPatch(the JSON Merge Patch path) cannot apply a value whose JSON encoding is an object handled atomically by a registered converter — e.g. a Unity object reference{"instanceID":"…"}. It structurally descends into the node as if it were a bag of members to navigate, instead of delegating to the converter registered for the target member type.Reported via the Unity-MCP tool
gameobject-component-modify:Affects both the object and stringified forms of
jsonPatchequally. (Distinct from #82, which fixed the string-vs-object schema/validation; this is the apply-time layer.)Root cause
src/Reflector/Reflector.Patch.cs—TryPatchInternalbranches purely on JSONValueKind:TryModify→ the converter chain → resolves correctly (Reflector.Patch.cs:109-114).TryPatchMember(Reflector.Patch.cs:116+,194-206). For{"instanceID":"…"}against aMaterialproperty it enumerates keyinstanceID, finds no such member onMaterial, and throwsSegment '<key>' not found on type '<T>'(Reflector.Patch.cs:318).The converter registry is never consulted for a
ValueKind == Objectnode, so a reference encoded as a JSON object bypasses the converter that knows how to resolve it.Why the other surfaces work
TryModify(componentDiff/SerializedMember) andTryModifyAt(pathPatches) both funnel the{instanceID:…}object intoConverters.GetConverter(type)→ the converter'sSetValue(Reflector.Modify.cs:70-92). For Unity types that converter (UnityEngine_Object_ReflectionConverter, registered by Unity-MCP) resolves the ref viaToAssetObjectRef().FindAssetObject(type). The merge-patch path never reaches that resolver.Proposed fix (ReflectorNet)
Add a converter capability flag, e.g.
bool TreatJsonObjectAsAtomicValue(Type type)onIReflectionConverter(defaultfalseinBaseReflectionConverter). InTryPatchInternal, before the structural key-enumeration block and after the leaf checks (~Reflector.Patch.cs:114): whenpatch.ValueKind == Object, the target type has a converter, that flag is true, and the node has no$typekey → route the node throughTryModify(the same path the other two surfaces use). Plain POCO sub-objects keep the flagfalseand descend exactly as today. Keep$typepolymorphic replacement flowing through the existing structural path.The Unity
UnityEngine_Object_ReflectionConverteroverrides the new flag totrue(one line, in Unity-MCP).Blast radius / tests
TryPatch: 5 Unity-MCP modify tools (gameobject-component-modify,object-modify,gameobject-modify,assets-modify); ReflectorNetAtomicModifyTests; Unity-MCPTryPatchTests+TestToolGameObject.JsonPatchObjectOrString(the latter deliberately punted on the object-ref case). No direct callers in MCP-Plugin-dotnet/extensions.TryPatch_*tests (multi-field, deep, null,$type) unaffected.SetValueconsumes a{ref:…}object (proves routing with no Unity dependency).Acceptance criteria
TryPatchroutes converter-atomic JSON-object nodes (e.g. object-refs) through the registered converter instead of descending structurally.$typereplacement unchanged; all existingTryPatch_*tests green (net8.0 + net9.0).gameobject-component-modifywithjsonPatch={"sharedMaterial":{"instanceID":…}}resolves the Material (tracked in the Unity-MCP cross-ref issue).Discovered while verifying #82 (PR #83) end-to-end against the live
gameobject-component-modifytool.