From 65448aee0b053e0d67fb7f412cff50abaee1e6c3 Mon Sep 17 00:00:00 2001 From: HumbleTurtle Date: Fri, 22 May 2026 23:19:39 -0300 Subject: [PATCH] Require identity for graph node tools --- resources/scripts/napseer_mcp_server.py | 59 +++++++++------- tests/test_mcp_memory_ergonomics_helpers.py | 77 +++++++++++++++++++++ 2 files changed, 112 insertions(+), 24 deletions(-) diff --git a/resources/scripts/napseer_mcp_server.py b/resources/scripts/napseer_mcp_server.py index 3d683fe..60ebc03 100755 --- a/resources/scripts/napseer_mcp_server.py +++ b/resources/scripts/napseer_mcp_server.py @@ -7893,11 +7893,11 @@ def delete_archived_node(args): def link_nodes(args): - reject_agent_namespace_path(args["source_path"], "source_path") - reject_agent_namespace_path(args["target_path"], "target_path") + if args.get("source_path") or args.get("target_path"): + raise RuntimeError("paths are index/routes only; use source_node_id/target_node_id or source_uri/target_uri") project_id = resolve_project_id(args) - source = read_node_by_path({"path": args["source_path"]}) - target = read_node_by_path({"path": args["target_path"]}) + source = resolve_existing_node_ref({"node_id": args.get("source_node_id"), "uri": args.get("source_uri")}) + target = resolve_existing_node_ref({"node_id": args.get("target_node_id"), "uri": args.get("target_uri")}) relation = args.get("relation", "references") links = source.get("links") or [] if not isinstance(links, list): @@ -7917,8 +7917,8 @@ def link_nodes(args): {"links": links}, project_id, f"link node {source['full_path']}", - "path", - source["full_path"], + "node", + source["id"], ) index_node(updated) else: @@ -7935,9 +7935,10 @@ def link_nodes(args): def get_backlinks_by_path(args): - reject_agent_namespace_path(args["path"]) + if args.get("path"): + raise RuntimeError("path is an index/route only; resolve it with nap_node_by_path, then use node_id or uri") project_id = resolve_project_id(args) - target = get_node_by_path({"path": args["path"]}) + target = resolve_existing_node_ref(args) backlinks = request_json("GET", f"/v1/projects/{project_id}/nodes/{target['id']}/backlinks") return {"target": {"id": target["id"], "full_path": target["full_path"]}, **backlinks} @@ -9521,14 +9522,14 @@ def inbound_reference_plan(path): def move_node(args): - old_path = args.get("path") or args.get("source_path") new_path = args.get("new_path") or args.get("target_path") - if not old_path or not new_path: - raise RuntimeError("path and new_path are required") - reject_agent_namespace_path(old_path) + if args.get("path") or args.get("source_path"): + raise RuntimeError("path is an index/route only; resolve it with nap_node_by_path, then use node_id or uri") + if not new_path: + raise RuntimeError("new_path is required") reject_agent_namespace_path(new_path) project_id = resolve_project_id(args) - existing = read_node_by_path({"path": old_path}) + existing = resolve_existing_node_ref(args) old_full_path = existing["full_path"] new_full_path = normalize_node_path(new_path) references_to_rewrite = [ @@ -9553,7 +9554,9 @@ def move_node(args): f"/v1/projects/{project_id}/nodes/{existing['id']}", payload, project_id, - f"move node {old_path} to {new_path} and rewrite inbound references", + f"move node {old_full_path} to {new_path} and rewrite inbound references", + "node", + existing["id"], ) remove_indexed_node(existing["id"]) index_node(updated) @@ -11238,11 +11241,16 @@ def raw_tools(): }, { "name": "nap_mv", - "description": "Move or rename one node by path and rewrite inbound links/metadata references in the same backend operation.", + "description": "Move or rename one existing node by node_id or nap:// URI and rewrite inbound links/metadata references in the same backend operation.", "inputSchema": { "type": "object", - "required": ["path", "new_path"], - "properties": {"path": {"type": "string"}, "new_path": {"type": "string"}, "dry_run": {"type": "boolean", "default": False}}, + "required": ["new_path"], + "properties": { + "node_id": {"type": "string", "description": "Canonical node id of the node to move."}, + "uri": {"type": "string", "description": "Canonical nap://project-name/project-id/node-id URI of the node to move."}, + "new_path": {"type": "string", "description": "New route/index path for the node."}, + "dry_run": {"type": "boolean", "default": False}, + }, "additionalProperties": False, }, }, @@ -11540,13 +11548,14 @@ def raw_tools(): }, { "name": "nap_ln", - "description": "Create a first-class graph link from source_path to target_path. Use this instead of burying references in prose or metadata.", + "description": "Create a first-class graph link between existing nodes identified by node_id or nap:// URI. Use this instead of burying references in prose or metadata.", "inputSchema": { "type": "object", - "required": ["source_path", "target_path"], "properties": { - "source_path": {"type": "string", "description": "Node that should hold the outgoing link."}, - "target_path": {"type": "string", "description": "Node being referenced."}, + "source_node_id": {"type": "string", "description": "Canonical node id for the node that should hold the outgoing link."}, + "source_uri": {"type": "string", "description": "Canonical nap:// URI for the node that should hold the outgoing link."}, + "target_node_id": {"type": "string", "description": "Canonical node id for the node being referenced."}, + "target_uri": {"type": "string", "description": "Canonical nap:// URI for the node being referenced."}, "relation": {"type": "string", "default": "references", "description": "Short relation slug, for example references, depends-on, supersedes, implements, explains."}, }, "additionalProperties": False, @@ -11554,11 +11563,13 @@ def raw_tools(): }, { "name": "nap_backlinks", - "description": "List nodes that link to or mention a target node path. Use this to recover graph context before editing or summarizing.", + "description": "List nodes that link to or mention a target node identified by node_id or nap:// URI. Use this to recover graph context before editing or summarizing.", "inputSchema": { "type": "object", - "required": ["path"], - "properties": {"path": {"type": "string", "description": "Target node full_path."}}, + "properties": { + "node_id": {"type": "string", "description": "Canonical target node id."}, + "uri": {"type": "string", "description": "Canonical target nap://project-name/project-id/node-id URI."}, + }, "additionalProperties": False, }, }, diff --git a/tests/test_mcp_memory_ergonomics_helpers.py b/tests/test_mcp_memory_ergonomics_helpers.py index 6a89099..f3b348f 100644 --- a/tests/test_mcp_memory_ergonomics_helpers.py +++ b/tests/test_mcp_memory_ergonomics_helpers.py @@ -304,6 +304,25 @@ def test_tools_include_node_by_path_descriptor(mod): assert "uri" in schema["properties"] assert "path" not in schema["properties"] + mv_schema = next(item for item in mod.tools() if item["name"] == "nap_mv")["inputSchema"] + assert mv_schema["required"] == ["new_path"] + assert "node_id" in mv_schema["properties"] + assert "uri" in mv_schema["properties"] + assert "path" not in mv_schema["properties"] + + ln_schema = next(item for item in mod.tools() if item["name"] == "nap_ln")["inputSchema"] + assert "source_node_id" in ln_schema["properties"] + assert "source_uri" in ln_schema["properties"] + assert "target_node_id" in ln_schema["properties"] + assert "target_uri" in ln_schema["properties"] + assert "source_path" not in ln_schema["properties"] + assert "target_path" not in ln_schema["properties"] + + backlinks_schema = next(item for item in mod.tools() if item["name"] == "nap_backlinks")["inputSchema"] + assert "node_id" in backlinks_schema["properties"] + assert "uri" in backlinks_schema["properties"] + assert "path" not in backlinks_schema["properties"] + def test_mutation_tools_reject_path_and_patch_by_node_id(mod): found = node("/notes/current", "active", node_type="note") @@ -341,6 +360,64 @@ def test_mutation_tools_reject_path_and_patch_by_node_id(mod): mod.update_node_by_path({"path": "/notes/current", "content_text": "bad"}) +def test_graph_tools_use_node_identity_instead_of_paths(mod): + source = node("/notes/source", "active", node_type="note") + source["id"] = "source-123" + target = node("/notes/target", "active", node_type="note") + target["id"] = "target-123" + writes = [] + reads = [] + + def fake_get_node_by_id(args): + reads.append(args) + node_id = args.get("node_id") + if node_id == "source-123": + return source + if node_id == "target-123": + return target + raise AssertionError(f"unexpected node_id {node_id}") + + def fake_write(method, path, payload, *args, **kwargs): + writes.append((method, path, payload, args)) + return {**source, "full_path": "/notes/source-renamed" if payload.get("name") == "source-renamed" else source["full_path"], "links": payload.get("links", source["links"])} + + def fake_request(method, path, payload=None, **kwargs): + assert method == "GET" + assert path == "/v1/projects/project-1/nodes/target-123/backlinks" + return {"items": [source]} + + mod.get_node_by_id = fake_get_node_by_id + mod.request_project_write = fake_write + mod.request_json = fake_request + mod.index_node = lambda node: None + mod.inbound_reference_plan = lambda path: [{"id": "other-1", "path": "/notes/other"}] + mod.remove_indexed_node = lambda node_id: None + + linked = mod.link_nodes({"source_node_id": "source-123", "target_node_id": "target-123", "relation": "references"}) + assert linked["created"] is True + assert writes[-1][0] == "PATCH" + assert writes[-1][1] == "/v1/projects/project-1/nodes/source-123" + assert writes[-1][2]["links"] == [{"path": "/notes/target", "relation": "references"}] + assert writes[-1][3][2:] == ("node", "source-123") + + backlinks = mod.get_backlinks_by_path({"node_id": "target-123"}) + assert backlinks["target"]["id"] == "target-123" + + moved = mod.move_node({"node_id": "source-123", "new_path": "/notes/source-renamed"}) + assert moved["moved"] is True + assert writes[-1][1] == "/v1/projects/project-1/nodes/source-123" + assert writes[-1][2]["folder_path"] == "/notes" + assert writes[-1][2]["name"] == "source-renamed" + assert writes[-1][3][2:] == ("node", "source-123") + + with pytest.raises(RuntimeError, match="paths are index/routes only"): + mod.link_nodes({"source_path": "/notes/source", "target_path": "/notes/target"}) + with pytest.raises(RuntimeError, match="path is an index/route only"): + mod.get_backlinks_by_path({"path": "/notes/target"}) + with pytest.raises(RuntimeError, match="path is an index/route only"): + mod.move_node({"path": "/notes/source", "new_path": "/notes/source-renamed"}) + + def test_tee_and_bulk_create_by_path_update_by_identity(mod): existing = node("/notes/current", "active", node_type="note") existing["id"] = "node-123"