Require identity for graph node tools
This commit is contained in:
@@ -7893,11 +7893,11 @@ def delete_archived_node(args):
|
|||||||
|
|
||||||
|
|
||||||
def link_nodes(args):
|
def link_nodes(args):
|
||||||
reject_agent_namespace_path(args["source_path"], "source_path")
|
if args.get("source_path") or args.get("target_path"):
|
||||||
reject_agent_namespace_path(args["target_path"], "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)
|
project_id = resolve_project_id(args)
|
||||||
source = read_node_by_path({"path": args["source_path"]})
|
source = resolve_existing_node_ref({"node_id": args.get("source_node_id"), "uri": args.get("source_uri")})
|
||||||
target = read_node_by_path({"path": args["target_path"]})
|
target = resolve_existing_node_ref({"node_id": args.get("target_node_id"), "uri": args.get("target_uri")})
|
||||||
relation = args.get("relation", "references")
|
relation = args.get("relation", "references")
|
||||||
links = source.get("links") or []
|
links = source.get("links") or []
|
||||||
if not isinstance(links, list):
|
if not isinstance(links, list):
|
||||||
@@ -7917,8 +7917,8 @@ def link_nodes(args):
|
|||||||
{"links": links},
|
{"links": links},
|
||||||
project_id,
|
project_id,
|
||||||
f"link node {source['full_path']}",
|
f"link node {source['full_path']}",
|
||||||
"path",
|
"node",
|
||||||
source["full_path"],
|
source["id"],
|
||||||
)
|
)
|
||||||
index_node(updated)
|
index_node(updated)
|
||||||
else:
|
else:
|
||||||
@@ -7935,9 +7935,10 @@ def link_nodes(args):
|
|||||||
|
|
||||||
|
|
||||||
def get_backlinks_by_path(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)
|
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")
|
backlinks = request_json("GET", f"/v1/projects/{project_id}/nodes/{target['id']}/backlinks")
|
||||||
return {"target": {"id": target["id"], "full_path": target["full_path"]}, **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):
|
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")
|
new_path = args.get("new_path") or args.get("target_path")
|
||||||
if not old_path or not new_path:
|
if args.get("path") or args.get("source_path"):
|
||||||
raise RuntimeError("path and new_path are required")
|
raise RuntimeError("path is an index/route only; resolve it with nap_node_by_path, then use node_id or uri")
|
||||||
reject_agent_namespace_path(old_path)
|
if not new_path:
|
||||||
|
raise RuntimeError("new_path is required")
|
||||||
reject_agent_namespace_path(new_path)
|
reject_agent_namespace_path(new_path)
|
||||||
project_id = resolve_project_id(args)
|
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"]
|
old_full_path = existing["full_path"]
|
||||||
new_full_path = normalize_node_path(new_path)
|
new_full_path = normalize_node_path(new_path)
|
||||||
references_to_rewrite = [
|
references_to_rewrite = [
|
||||||
@@ -9553,7 +9554,9 @@ def move_node(args):
|
|||||||
f"/v1/projects/{project_id}/nodes/{existing['id']}",
|
f"/v1/projects/{project_id}/nodes/{existing['id']}",
|
||||||
payload,
|
payload,
|
||||||
project_id,
|
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"])
|
remove_indexed_node(existing["id"])
|
||||||
index_node(updated)
|
index_node(updated)
|
||||||
@@ -11238,11 +11241,16 @@ def raw_tools():
|
|||||||
},
|
},
|
||||||
{
|
{
|
||||||
"name": "nap_mv",
|
"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": {
|
"inputSchema": {
|
||||||
"type": "object",
|
"type": "object",
|
||||||
"required": ["path", "new_path"],
|
"required": ["new_path"],
|
||||||
"properties": {"path": {"type": "string"}, "new_path": {"type": "string"}, "dry_run": {"type": "boolean", "default": False}},
|
"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,
|
"additionalProperties": False,
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
@@ -11540,13 +11548,14 @@ def raw_tools():
|
|||||||
},
|
},
|
||||||
{
|
{
|
||||||
"name": "nap_ln",
|
"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": {
|
"inputSchema": {
|
||||||
"type": "object",
|
"type": "object",
|
||||||
"required": ["source_path", "target_path"],
|
|
||||||
"properties": {
|
"properties": {
|
||||||
"source_path": {"type": "string", "description": "Node that should hold the outgoing link."},
|
"source_node_id": {"type": "string", "description": "Canonical node id for the node that should hold the outgoing link."},
|
||||||
"target_path": {"type": "string", "description": "Node being referenced."},
|
"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."},
|
"relation": {"type": "string", "default": "references", "description": "Short relation slug, for example references, depends-on, supersedes, implements, explains."},
|
||||||
},
|
},
|
||||||
"additionalProperties": False,
|
"additionalProperties": False,
|
||||||
@@ -11554,11 +11563,13 @@ def raw_tools():
|
|||||||
},
|
},
|
||||||
{
|
{
|
||||||
"name": "nap_backlinks",
|
"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": {
|
"inputSchema": {
|
||||||
"type": "object",
|
"type": "object",
|
||||||
"required": ["path"],
|
"properties": {
|
||||||
"properties": {"path": {"type": "string", "description": "Target node full_path."}},
|
"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,
|
"additionalProperties": False,
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -304,6 +304,25 @@ def test_tools_include_node_by_path_descriptor(mod):
|
|||||||
assert "uri" in schema["properties"]
|
assert "uri" in schema["properties"]
|
||||||
assert "path" not 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):
|
def test_mutation_tools_reject_path_and_patch_by_node_id(mod):
|
||||||
found = node("/notes/current", "active", node_type="note")
|
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"})
|
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):
|
def test_tee_and_bulk_create_by_path_update_by_identity(mod):
|
||||||
existing = node("/notes/current", "active", node_type="note")
|
existing = node("/notes/current", "active", node_type="note")
|
||||||
existing["id"] = "node-123"
|
existing["id"] = "node-123"
|
||||||
|
|||||||
Reference in New Issue
Block a user