diff --git a/docs/implementation-plan.md b/docs/implementation-plan.md index 63becdba..2924bbba 100644 --- a/docs/implementation-plan.md +++ b/docs/implementation-plan.md @@ -57,9 +57,9 @@ Historical bootstrap material, architecture detail, packaging mechanics, go/no-g ### High-Leverage Authoring - [x] `batch.execute` with stop-on-first-error semantics and optional grouped undo -- [ ] `node.rename` with UID/reference awareness where feasible -- [ ] complex `node.set_property` (`Resource`, `NodePath`, `Array`, `Dictionary`) -- [ ] `script.patch` shipped or explicitly ruled out after a focused spike +- [x] `node.rename` with sibling-collision validation and char-safety checks (NodePath/script references in OTHER nodes are not auto-updated — documented in the tool) +- [x] complex `node.set_property` (`Resource` via res:// path, `NodePath`, `Array`, `Dictionary`, `StringName`) +- [x] `script.patch` shipped — anchor-based `old_text` → `new_text` replace with ambiguity detection and optional `replace_all` **Why this matters:** These are workflow multipliers. They matter more for real project iteration than adding another narrow read tool. @@ -77,8 +77,8 @@ Historical bootstrap material, architecture detail, packaging mechanics, go/no-g - [x] run/stop cycle is reliable - [x] batch execution is shipped with a clear contract - [ ] multi-instance routing works in practice -- [ ] `script.patch` decision is made -- [x] test coverage and smoke coverage increase where the new runtime loop needs it (277 Python + 191 GDScript = 468 total) +- [x] `script.patch` decision is made (shipped: anchor-based replace) +- [x] test coverage and smoke coverage increase where the new runtime loop needs it (282 Python + 216 GDScript = 498 total) --- diff --git a/plugin/addons/godot_ai/handlers/node_handler.gd b/plugin/addons/godot_ai/handlers/node_handler.gd index 0f2ad7cc..8ea51083 100644 --- a/plugin/addons/godot_ai/handlers/node_handler.gd +++ b/plugin/addons/godot_ai/handlers/node_handler.gd @@ -157,17 +157,31 @@ func set_property(params: Dictionary) -> Dictionary: var value = params.get("value") - # Verify property exists on the node var found := false + var prop_type: int = TYPE_NIL for prop in node.get_property_list(): if prop.name == property: found = true + prop_type = prop.get("type", TYPE_NIL) break if not found: return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "Property '%s' not found on %s" % [property, node.get_class()]) var old_value = node.get(property) - value = _coerce_value(value, typeof(old_value)) + # Prefer declared property type; fall back to runtime type for dynamic props + # (scripted @export vars can report TYPE_NIL in the property list). + var target_type: int = prop_type if prop_type != TYPE_NIL else typeof(old_value) + + if target_type == TYPE_OBJECT and value is String: + if value == "": + value = null + else: + var loaded := ResourceLoader.load(value) + if loaded == null: + return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "Resource not found: %s" % value) + value = loaded + else: + value = _coerce_value(value, target_type) _undo_redo.create_action("MCP: Set %s.%s" % [node.name, property]) _undo_redo.add_do_property(node, property, value) @@ -185,6 +199,58 @@ func set_property(params: Dictionary) -> Dictionary: } +func rename_node(params: Dictionary) -> Dictionary: + var resolved := _resolve_node(params) + if resolved.has("error"): + return resolved + var node: Node = resolved.node + var node_path: String = resolved.path + var scene_root: Node = resolved.scene_root + + var new_name: String = params.get("new_name", "") + if new_name.is_empty(): + return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "Missing required param: new_name") + + if new_name.validate_node_name() != new_name: + return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "Invalid characters in name: %s" % new_name) + + if node == scene_root: + return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "Cannot rename the scene root") + + var old_name := String(node.name) + if old_name == new_name: + return { + "data": { + "path": node_path, + "name": new_name, + "old_name": old_name, + "unchanged": true, + "undoable": false, + "reason": "Name unchanged", + } + } + + var parent := node.get_parent() + for sibling in parent.get_children(): + if sibling != node and String(sibling.name) == new_name: + return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "A sibling already has the name '%s'" % new_name) + + _undo_redo.create_action("MCP: Rename %s to %s" % [old_name, new_name]) + _undo_redo.add_do_property(node, "name", new_name) + _undo_redo.add_undo_property(node, "name", old_name) + _undo_redo.commit_action() + + return { + "data": { + "path": ScenePath.from_node(node, scene_root), + "old_path": node_path, + "name": String(node.name), + "old_name": old_name, + "undoable": true, + } + } + + func duplicate_node(params: Dictionary) -> Dictionary: var resolved := _resolve_node(params) if resolved.has("error"): @@ -380,6 +446,25 @@ static func _coerce_value(value: Variant, target_type: int) -> Variant: TYPE_FLOAT: if value is int: return float(value) + TYPE_STRING_NAME: + if value is String: + return StringName(value) + TYPE_NODE_PATH: + if value is String: + return NodePath(value) + if value == null: + return NodePath() + TYPE_OBJECT: + # Resource loading is handled in set_property so we can return a + # typed error; here we only pass through cleared values. + if value == null: + return null + TYPE_ARRAY: + if value is Array: + return value + TYPE_DICTIONARY: + if value is Dictionary: + return value return value @@ -484,6 +569,8 @@ static func _serialize_value(value: Variant) -> Variant: match typeof(value): TYPE_BOOL, TYPE_INT, TYPE_FLOAT, TYPE_STRING: return value + TYPE_STRING_NAME: + return str(value) TYPE_VECTOR2: return {"x": value.x, "y": value.y} TYPE_VECTOR3: @@ -496,6 +583,16 @@ static func _serialize_value(value: Variant) -> Variant: return str(value) TYPE_NODE_PATH: return str(value) + TYPE_ARRAY: + var arr: Array = [] + for item in value: + arr.append(_serialize_value(item)) + return arr + TYPE_DICTIONARY: + var out := {} + for k in value: + out[str(k)] = _serialize_value(value[k]) + return out TYPE_OBJECT: if value is Resource and value.resource_path: return value.resource_path diff --git a/plugin/addons/godot_ai/handlers/script_handler.gd b/plugin/addons/godot_ai/handlers/script_handler.gd index c7f5d79e..1be6b291 100644 --- a/plugin/addons/godot_ai/handlers/script_handler.gd +++ b/plugin/addons/godot_ai/handlers/script_handler.gd @@ -80,6 +80,70 @@ func read_script(params: Dictionary) -> Dictionary: } +func patch_script(params: Dictionary) -> Dictionary: + var path: String = params.get("path", "") + var old_text: String = params.get("old_text", "") + var new_text: String = params.get("new_text", "") + var replace_all: bool = params.get("replace_all", false) + + if path.is_empty(): + return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "Missing required param: path") + if not "old_text" in params: + return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "Missing required param: old_text") + if not "new_text" in params: + return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "Missing required param: new_text") + if not path.begins_with("res://"): + return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "Path must start with res://") + if not path.ends_with(".gd"): + return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "Path must end with .gd (use filesystem_write_text for other text files)") + if old_text.is_empty(): + return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "old_text must not be empty") + + var read := FileAccess.open(path, FileAccess.READ) + if read == null: + return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "File not found or unreadable: %s" % path) + var content := read.get_as_text() + read.close() + + var match_count := content.count(old_text) + if match_count == 0: + return McpErrorCodes.make(McpErrorCodes.INVALID_PARAMS, "old_text not found in %s" % path) + if match_count > 1 and not replace_all: + return McpErrorCodes.make( + McpErrorCodes.INVALID_PARAMS, + "old_text matches %d times; pass replace_all=true or provide a more specific snippet" % match_count, + ) + + var new_content: String + var replacements: int + if replace_all: + new_content = content.replace(old_text, new_text) + replacements = match_count + else: + var idx := content.find(old_text) + new_content = content.substr(0, idx) + new_text + content.substr(idx + old_text.length()) + replacements = 1 + + var write := FileAccess.open(path, FileAccess.WRITE) + if write == null: + return McpErrorCodes.make(McpErrorCodes.INTERNAL_ERROR, "Failed to open file for writing: %s" % path) + write.store_string(new_content) + write.close() + + EditorInterface.get_resource_filesystem().scan() + + return { + "data": { + "path": path, + "replacements": replacements, + "size": new_content.length(), + "old_size": content.length(), + "undoable": false, + "reason": "File system operations cannot be undone via editor undo", + } + } + + func attach_script(params: Dictionary) -> Dictionary: var node_path: String = params.get("path", "") var script_path: String = params.get("script_path", "") diff --git a/plugin/addons/godot_ai/plugin.gd b/plugin/addons/godot_ai/plugin.gd index f4a63535..812e81bd 100644 --- a/plugin/addons/godot_ai/plugin.gd +++ b/plugin/addons/godot_ai/plugin.gd @@ -46,6 +46,7 @@ func _enter_tree() -> void: _dispatcher.register("delete_node", node_handler.delete_node) _dispatcher.register("reparent_node", node_handler.reparent_node) _dispatcher.register("set_property", node_handler.set_property) + _dispatcher.register("rename_node", node_handler.rename_node) _dispatcher.register("duplicate_node", node_handler.duplicate_node) _dispatcher.register("move_node", node_handler.move_node) _dispatcher.register("add_to_group", node_handler.add_to_group) @@ -68,6 +69,7 @@ func _enter_tree() -> void: _dispatcher.register("configure_client", client_handler.configure_client) _dispatcher.register("check_client_status", client_handler.check_client_status) _dispatcher.register("create_script", script_handler.create_script) + _dispatcher.register("patch_script", script_handler.patch_script) _dispatcher.register("read_script", script_handler.read_script) _dispatcher.register("attach_script", script_handler.attach_script) _dispatcher.register("detach_script", script_handler.detach_script) diff --git a/src/godot_ai/handlers/node.py b/src/godot_ai/handlers/node.py index 024c7e09..8927767b 100644 --- a/src/godot_ai/handlers/node.py +++ b/src/godot_ai/handlers/node.py @@ -63,6 +63,14 @@ async def node_set_property(runtime: Runtime, path: str, property: str, value) - ) +async def node_rename(runtime: Runtime, path: str, new_name: str) -> dict: + require_writable(runtime) + return await runtime.send_command( + "rename_node", + {"path": path, "new_name": new_name}, + ) + + async def node_duplicate(runtime: Runtime, path: str, name: str = "") -> dict: require_writable(runtime) return await runtime.send_command( diff --git a/src/godot_ai/handlers/script.py b/src/godot_ai/handlers/script.py index c3a373bc..78fd4d8a 100644 --- a/src/godot_ai/handlers/script.py +++ b/src/godot_ai/handlers/script.py @@ -14,6 +14,25 @@ async def script_create(runtime: Runtime, path: str, content: str = "") -> dict: ) +async def script_patch( + runtime: Runtime, + path: str, + old_text: str, + new_text: str, + replace_all: bool = False, +) -> dict: + require_writable(runtime) + return await runtime.send_command( + "patch_script", + { + "path": path, + "old_text": old_text, + "new_text": new_text, + "replace_all": replace_all, + }, + ) + + async def script_read(runtime: Runtime, path: str) -> dict: return await runtime.send_command("read_script", {"path": path}) diff --git a/src/godot_ai/tools/node.py b/src/godot_ai/tools/node.py index a8c1ecc4..b3af9ddf 100644 --- a/src/godot_ai/tools/node.py +++ b/src/godot_ai/tools/node.py @@ -141,24 +141,54 @@ async def node_set_property( ctx: Context, path: str, property: str, - value: str | int | float | bool | dict | list, + value: str | int | float | bool | dict | list | None, ) -> dict: """Set a property on a node. - Sets a simple property value. For Vector2/Vector3, pass a dict - with x/y/z keys. For Color, pass a dict with r/g/b/a keys or - a hex string like "#ff0000". + Coerces `value` to match the property's declared type: + + - Vector2/Vector3: dict with x/y/z keys + - Color: dict with r/g/b/a keys, or hex string ("#ff0000") + - NodePath: string ("../Other/Node") + - Resource: res:// path string (loads and assigns); pass null or "" to clear + - StringName: plain string + - Array/Dictionary: pass a JSON list/object + - bool/int/float: JSON primitives Args: path: Scene path of the node (e.g. "/Main/Camera3D"). - property: Property name (e.g. "fov", "position", "visible"). - value: New value for the property. + property: Property name (e.g. "fov", "position", "visible", "mesh", "remote_path"). + value: New value for the property. Pass null (or "" for Resource properties) to clear. """ runtime = DirectRuntime.from_context(ctx) return await node_handlers.node_set_property( runtime, path=path, property=property, value=value, ) + @mcp.tool(meta=DEFER_META) + async def node_rename( + ctx: Context, + path: str, + new_name: str, + ) -> dict: + """Rename a node in the scene tree. + + Changes the node's `name`. Fails if a sibling already has that name, + or if the name contains `/`, `:`, or `@`. Cannot rename the scene root. + + Note: `NodePath` properties on OTHER nodes that pointed at this node + (e.g. a camera's `remote_path`) will not be auto-updated. Scripts that + reference this node by name (`$OldName`, `get_node("OldName")`) also + need manual fixes. Children of the renamed node keep working because + their paths are relative. + + Args: + path: Scene path of the node to rename (e.g. "/Main/Player"). + new_name: New name for the node (e.g. "Hero"). + """ + runtime = DirectRuntime.from_context(ctx) + return await node_handlers.node_rename(runtime, path=path, new_name=new_name) + @mcp.tool(meta=DEFER_META) async def node_duplicate( ctx: Context, diff --git a/src/godot_ai/tools/script.py b/src/godot_ai/tools/script.py index 8e03c2ed..7a8a93fa 100644 --- a/src/godot_ai/tools/script.py +++ b/src/godot_ai/tools/script.py @@ -29,6 +29,43 @@ async def script_create( runtime = DirectRuntime.from_context(ctx) return await script_handlers.script_create(runtime, path=path, content=content) + @mcp.tool(meta=DEFER_META) + async def script_patch( + ctx: Context, + path: str, + old_text: str, + new_text: str, + replace_all: bool = False, + ) -> dict: + """Patch (partial edit / string-replace) a GDScript file in place. + + Anchor-based edit: finds an exact occurrence of `old_text` in the file + and replaces it with `new_text`. Use this instead of `script_create` + when you only need to change a function, add a signal, or fix a line — + it avoids rewriting (and possibly losing) the rest of the file. + + If `old_text` matches multiple places, the call fails unless + `replace_all=true` is passed. If it matches zero places, the call + fails. Exact byte match — whitespace is significant. + + Triggers a filesystem scan so the editor picks up the edit. Not + undoable via Ctrl+Z (filesystem edits bypass editor undo). + + Args: + path: File path starting with res:// and ending with .gd (e.g. "res://scripts/player.gd"). + old_text: Exact substring to find. Must be unique unless replace_all=true. + new_text: Replacement text. Can be empty to delete. + replace_all: If true, replace every occurrence. Default false. + """ + runtime = DirectRuntime.from_context(ctx) + return await script_handlers.script_patch( + runtime, + path=path, + old_text=old_text, + new_text=new_text, + replace_all=replace_all, + ) + @mcp.tool(meta=DEFER_META) async def script_read(ctx: Context, path: str) -> dict: """Read the contents of a GDScript file. diff --git a/test_project/tests/_mcp_test_script.gd b/test_project/tests/_mcp_test_script.gd new file mode 100644 index 00000000..a00eaae4 --- /dev/null +++ b/test_project/tests/_mcp_test_script.gd @@ -0,0 +1,4 @@ +@tool +extends Node + +## Placeholder script used by main.tscn's _McpTestAttach fixture node. diff --git a/test_project/tests/test_node.gd b/test_project/tests/test_node.gd index a97dcfb6..0134065a 100644 --- a/test_project/tests/test_node.gd +++ b/test_project/tests/test_node.gd @@ -6,6 +6,8 @@ extends McpTestSuite var _handler: NodeHandler var _undo_redo: EditorUndoRedoManager +const TEST_MATERIAL_PATH := "res://tests/_mcp_test_material.tres" + func suite_name() -> String: return "node" @@ -14,6 +16,13 @@ func suite_name() -> String: func suite_setup(ctx: Dictionary) -> void: _undo_redo = ctx.get("undo_redo") _handler = NodeHandler.new(_undo_redo) + var mat := StandardMaterial3D.new() + ResourceSaver.save(mat, TEST_MATERIAL_PATH) + + +func suite_teardown() -> void: + if FileAccess.file_exists(TEST_MATERIAL_PATH): + DirAccess.remove_absolute(TEST_MATERIAL_PATH) # ----- get_children ----- @@ -209,6 +218,74 @@ func test_set_property_missing_value() -> void: assert_is_error(result, McpErrorCodes.INVALID_PARAMS) +func test_set_property_resource_path() -> void: + ## Use a fresh MeshInstance3D for a clean material_override slot. + _handler.create_node({ + "type": "MeshInstance3D", + "name": "_McpTestMat", + "parent_path": "/Main", + }) + var result := _handler.set_property({ + "path": "/Main/_McpTestMat", + "property": "material_override", + "value": TEST_MATERIAL_PATH, + }) + assert_has_key(result, "data") + assert_eq(result.data.value, TEST_MATERIAL_PATH) + assert_true(result.data.undoable) + _undo_redo.undo() # undo assign + _undo_redo.undo() # undo create + + +func test_set_property_resource_not_found() -> void: + var result := _handler.set_property({ + "path": "/Main/Camera3D", + "property": "environment", + "value": "res://does/not/exist.tres", + }) + assert_is_error(result, McpErrorCodes.INVALID_PARAMS) + + +func test_set_property_resource_null_clears() -> void: + _handler.create_node({ + "type": "MeshInstance3D", + "name": "_McpTestClear", + "parent_path": "/Main", + }) + _handler.set_property({ + "path": "/Main/_McpTestClear", + "property": "material_override", + "value": TEST_MATERIAL_PATH, + }) + var result := _handler.set_property({ + "path": "/Main/_McpTestClear", + "property": "material_override", + "value": null, + }) + assert_has_key(result, "data") + assert_eq(result.data.value, null) + _undo_redo.undo() + _undo_redo.undo() + _undo_redo.undo() + + +func test_set_property_node_path() -> void: + _handler.create_node({ + "type": "RemoteTransform3D", + "name": "_McpTestRemote", + "parent_path": "/Main", + }) + var result := _handler.set_property({ + "path": "/Main/_McpTestRemote", + "property": "remote_path", + "value": "../Camera3D", + }) + assert_has_key(result, "data") + assert_eq(result.data.value, "../Camera3D") + _undo_redo.undo() + _undo_redo.undo() + + func test_set_property_nonexistent_property() -> void: var result := _handler.set_property({ "path": "/Main/Camera3D", @@ -218,6 +295,114 @@ func test_set_property_nonexistent_property() -> void: assert_is_error(result, McpErrorCodes.INVALID_PARAMS) +# ----- _coerce_value / _serialize_value unit coverage ----- + +func test_coerce_array_passthrough() -> void: + var coerced = NodeHandler._coerce_value([1, 2, 3], TYPE_ARRAY) + assert_true(coerced is Array) + assert_eq(coerced.size(), 3) + + +func test_coerce_dictionary_passthrough() -> void: + var coerced = NodeHandler._coerce_value({"a": 1, "b": 2}, TYPE_DICTIONARY) + assert_true(coerced is Dictionary) + assert_eq(coerced["a"], 1) + + +func test_coerce_node_path_from_string() -> void: + var coerced = NodeHandler._coerce_value("../Sibling", TYPE_NODE_PATH) + assert_true(coerced is NodePath) + assert_eq(str(coerced), "../Sibling") + + +func test_coerce_string_name_from_string() -> void: + var coerced = NodeHandler._coerce_value("my_name", TYPE_STRING_NAME) + assert_true(coerced is StringName) + + +func test_serialize_array_recursive() -> void: + var result = NodeHandler._serialize_value([Vector2(1, 2), "hello", 3]) + assert_true(result is Array) + assert_eq(result[0]["x"], 1.0) + assert_eq(result[1], "hello") + + +func test_serialize_dictionary_recursive() -> void: + var result = NodeHandler._serialize_value({"pos": Vector3(1, 2, 3), "name": "x"}) + assert_true(result is Dictionary) + assert_eq(result["pos"]["z"], 3.0) + assert_eq(result["name"], "x") + + +# ----- rename_node ----- + +func test_rename_node_basic() -> void: + var suffix := str(Time.get_ticks_usec()) + var created := _handler.create_node({ + "type": "Node3D", + "name": "_McpRenameSrc%s" % suffix, + "parent_path": "/Main", + }) + assert_has_key(created, "data") + var created_path: String = created.data.path + var created_name: String = created.data.name + var target_name := "_McpRenameDst%s" % suffix + var result := _handler.rename_node({ + "path": created_path, + "new_name": target_name, + }) + assert_has_key(result, "data") + assert_eq(result.data.name, target_name) + assert_eq(result.data.old_name, created_name) + assert_true(result.data.undoable) + _undo_redo.undo() + _undo_redo.undo() + + +func test_rename_node_scene_root() -> void: + var result := _handler.rename_node({"path": "/Main", "new_name": "NewMain"}) + assert_is_error(result, McpErrorCodes.INVALID_PARAMS) + + +func test_rename_node_missing_name() -> void: + var result := _handler.rename_node({"path": "/Main/Camera3D"}) + assert_is_error(result, McpErrorCodes.INVALID_PARAMS) + + +func test_rename_node_invalid_characters() -> void: + var result := _handler.rename_node({ + "path": "/Main/Camera3D", + "new_name": "foo/bar", + }) + assert_is_error(result, McpErrorCodes.INVALID_PARAMS) + + +func test_rename_node_sibling_collision() -> void: + var result := _handler.rename_node({ + "path": "/Main/Camera3D", + "new_name": "World", + }) + assert_is_error(result, McpErrorCodes.INVALID_PARAMS) + + +func test_rename_node_unchanged() -> void: + var result := _handler.rename_node({ + "path": "/Main/Camera3D", + "new_name": "Camera3D", + }) + assert_has_key(result, "data") + assert_true(result.data.unchanged, "Should flag unchanged rename") + assert_false(result.data.undoable) + + +func test_rename_node_invalid_path() -> void: + var result := _handler.rename_node({ + "path": "/Main/Nope", + "new_name": "NewName", + }) + assert_is_error(result, McpErrorCodes.INVALID_PARAMS) + + # ----- duplicate_node ----- func test_duplicate_node_basic() -> void: diff --git a/test_project/tests/test_script.gd b/test_project/tests/test_script.gd index 63bb9487..f5d47208 100644 --- a/test_project/tests/test_script.gd +++ b/test_project/tests/test_script.gd @@ -80,6 +80,122 @@ func test_create_script_wrong_extension() -> void: assert_is_error(result, McpErrorCodes.INVALID_PARAMS) +# ----- patch_script ----- + +func test_patch_script_basic() -> void: + var path := "res://tests/_mcp_test_patch.gd" + var original := "extends Node\n\nvar speed = 5\n" + var file := FileAccess.open(path, FileAccess.WRITE) + file.store_string(original) + file.close() + + var result := _handler.patch_script({ + "path": path, + "old_text": "speed = 5", + "new_text": "speed = 10", + }) + assert_has_key(result, "data") + assert_eq(result.data.replacements, 1) + assert_false(result.data.undoable) + + var read := FileAccess.open(path, FileAccess.READ) + var new_content := read.get_as_text() + read.close() + assert_contains(new_content, "speed = 10") + DirAccess.remove_absolute(path) + + +func test_patch_script_no_match() -> void: + var result := _handler.patch_script({ + "path": TEST_SCRIPT_PATH, + "old_text": "this_does_not_exist_anywhere", + "new_text": "whatever", + }) + assert_is_error(result, McpErrorCodes.INVALID_PARAMS) + + +func test_patch_script_ambiguous_match_without_replace_all() -> void: + var path := "res://tests/_mcp_test_patch_ambig.gd" + var original := "var x = 1\nvar y = 1\n" + var file := FileAccess.open(path, FileAccess.WRITE) + file.store_string(original) + file.close() + + var result := _handler.patch_script({ + "path": path, + "old_text": "= 1", + "new_text": "= 2", + }) + assert_is_error(result, McpErrorCodes.INVALID_PARAMS) + DirAccess.remove_absolute(path) + + +func test_patch_script_replace_all() -> void: + var path := "res://tests/_mcp_test_patch_all.gd" + var original := "foo()\nfoo()\nfoo()\n" + var file := FileAccess.open(path, FileAccess.WRITE) + file.store_string(original) + file.close() + + var result := _handler.patch_script({ + "path": path, + "old_text": "foo", + "new_text": "bar", + "replace_all": true, + }) + assert_has_key(result, "data") + assert_eq(result.data.replacements, 3) + + var read := FileAccess.open(path, FileAccess.READ) + var new_content := read.get_as_text() + read.close() + assert_eq(new_content, "bar()\nbar()\nbar()\n") + DirAccess.remove_absolute(path) + + +func test_patch_script_missing_old_text() -> void: + var result := _handler.patch_script({ + "path": TEST_SCRIPT_PATH, + "new_text": "x", + }) + assert_is_error(result, McpErrorCodes.INVALID_PARAMS) + + +func test_patch_script_non_gd_extension_rejected() -> void: + var result := _handler.patch_script({ + "path": "res://main.tscn", + "old_text": "x", + "new_text": "y", + }) + assert_is_error(result, McpErrorCodes.INVALID_PARAMS) + + +func test_patch_script_missing_new_text() -> void: + var result := _handler.patch_script({ + "path": TEST_SCRIPT_PATH, + "old_text": "speed", + }) + assert_is_error(result, McpErrorCodes.INVALID_PARAMS) + + +func test_patch_script_file_not_found() -> void: + var result := _handler.patch_script({ + "path": "res://does/not/exist.gd", + "old_text": "x", + "new_text": "y", + }) + assert_is_error(result, McpErrorCodes.INVALID_PARAMS) + + +func test_patch_script_invalid_prefix() -> void: + var result := _handler.patch_script({ + "path": "/tmp/bad.gd", + "old_text": "x", + "new_text": "y", + }) + assert_is_error(result, McpErrorCodes.INVALID_PARAMS) + + # ----- read_script ----- func test_read_script_basic() -> void: diff --git a/tests/integration/test_mcp_tools.py b/tests/integration/test_mcp_tools.py index af52a91e..8bec31bf 100644 --- a/tests/integration/test_mcp_tools.py +++ b/tests/integration/test_mcp_tools.py @@ -501,6 +501,42 @@ async def respond(): assert result.data["undoable"] is True +# --------------------------------------------------------------------------- +# node_rename +# --------------------------------------------------------------------------- + + +class TestNodeRenameTool: + async def test_rename_node(self, mcp_stack): + client, plugin = mcp_stack + + async def respond(): + cmd = await plugin.recv_command() + assert cmd["command"] == "rename_node" + assert cmd["params"] == {"path": "/Main/Player", "new_name": "Hero"} + await plugin.send_response( + cmd["request_id"], + { + "path": "/Main/Hero", + "old_path": "/Main/Player", + "name": "Hero", + "old_name": "Player", + "undoable": True, + }, + ) + + task = asyncio.create_task(respond()) + result = await client.call_tool( + "node_rename", {"path": "/Main/Player", "new_name": "Hero"} + ) + await task + + assert result.data["name"] == "Hero" + assert result.data["old_name"] == "Player" + assert result.data["path"] == "/Main/Hero" + assert result.data["undoable"] is True + + # --------------------------------------------------------------------------- # node_duplicate # --------------------------------------------------------------------------- @@ -1053,6 +1089,50 @@ async def respond(): assert result.data["undoable"] is False +# --------------------------------------------------------------------------- +# script_patch +# --------------------------------------------------------------------------- + + +class TestScriptPatchTool: + async def test_patch_script(self, mcp_stack): + client, plugin = mcp_stack + + async def respond(): + cmd = await plugin.recv_command() + assert cmd["command"] == "patch_script" + assert cmd["params"] == { + "path": "res://scripts/player.gd", + "old_text": "speed = 5", + "new_text": "speed = 10", + "replace_all": False, + } + await plugin.send_response( + cmd["request_id"], + { + "path": "res://scripts/player.gd", + "replacements": 1, + "size": 120, + "old_size": 119, + "undoable": False, + }, + ) + + task = asyncio.create_task(respond()) + result = await client.call_tool( + "script_patch", + { + "path": "res://scripts/player.gd", + "old_text": "speed = 5", + "new_text": "speed = 10", + }, + ) + await task + + assert result.data["replacements"] == 1 + assert result.data["undoable"] is False + + # --------------------------------------------------------------------------- # script_read # --------------------------------------------------------------------------- diff --git a/tests/unit/test_runtime_handlers.py b/tests/unit/test_runtime_handlers.py index 588bb428..e8e382f0 100644 --- a/tests/unit/test_runtime_handlers.py +++ b/tests/unit/test_runtime_handlers.py @@ -109,6 +109,17 @@ async def send( "type": "Node3D", "undoable": True, } + if command == "rename_node": + path = params.get("path", "") + new_name = params.get("new_name", "") + parent = "/".join(path.split("/")[:-1]) if "/" in path else "" + return { + "path": f"{parent}/{new_name}" if parent else f"/{new_name}", + "old_path": path, + "name": new_name, + "old_name": path.split("/")[-1], + "undoable": True, + } if command == "move_node": return { "path": params.get("path", ""), @@ -168,6 +179,14 @@ async def send( return {"status": "ok", "client": params.get("client", "")} if command == "check_client_status": return {"claude_code": "configured", "codex": "not_configured"} + if command == "patch_script": + return { + "path": params.get("path", ""), + "replacements": 1, + "size": 100, + "old_size": 90, + "undoable": False, + } if command == "create_script": return { "path": params.get("path", ""), @@ -799,6 +818,22 @@ async def test_node_set_property_handler(): assert result["undoable"] is True +async def test_node_rename_handler(): + client = StubClient() + runtime = DirectRuntime(registry=SessionRegistry(), client=client) + result = await node_handlers.node_rename( + runtime, + path="/Main/Player", + new_name="Hero", + ) + assert result["name"] == "Hero" + assert result["old_name"] == "Player" + assert result["old_path"] == "/Main/Player" + assert result["path"] == "/Main/Hero" + assert result["undoable"] is True + assert client.calls[-1]["params"] == {"path": "/Main/Player", "new_name": "Hero"} + + async def test_node_duplicate_handler(): client = StubClient() runtime = DirectRuntime(registry=SessionRegistry(), client=client) @@ -1032,6 +1067,38 @@ async def test_script_create_handler(): } +async def test_script_patch_handler(): + client = StubClient() + runtime = DirectRuntime(registry=SessionRegistry(), client=client) + result = await script_handlers.script_patch( + runtime, + path="res://scripts/player.gd", + old_text="var speed = 5", + new_text="var speed = 10", + ) + assert result["replacements"] == 1 + assert result["undoable"] is False + assert client.calls[-1]["params"] == { + "path": "res://scripts/player.gd", + "old_text": "var speed = 5", + "new_text": "var speed = 10", + "replace_all": False, + } + + +async def test_script_patch_handler_replace_all(): + client = StubClient() + runtime = DirectRuntime(registry=SessionRegistry(), client=client) + await script_handlers.script_patch( + runtime, + path="res://scripts/player.gd", + old_text="foo", + new_text="bar", + replace_all=True, + ) + assert client.calls[-1]["params"]["replace_all"] is True + + async def test_script_read_handler(): client = StubClient() runtime = DirectRuntime(registry=SessionRegistry(), client=client)