From c9345455a9712d16251b78e65b8196efd5894dae Mon Sep 17 00:00:00 2001 From: huangsen Date: Fri, 15 May 2026 16:51:08 +0800 Subject: [PATCH] feat(crud): enhance file_download with configurable local path and overwrite option MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Symmetric counterpart to file_upload: source is in the vault, target is on the local filesystem. path (source) accepts a short link or vault-relative path; resolution goes through path_resolver.resolve_to_absolute. Short-link ambiguity is surfaced as error="ambiguous" with the candidate list — the download is **not** executed in that case. local_path (target) is a plain filesystem path. **Optional** — when empty, the file lands in a session-scoped temp dir (lazy, auto-cleaned on process exit; each call gets its own subdirectory so concurrent agents don't trample each other). overwrite=True is the default since most download flows are intentional refreshes. BREAKING CHANGE: renamed step from file_download to download and updated method signature to accept local_path and overwrite parameters. refactor(list): switch from file_store index to direct filesystem walk Reads directly from the filesystem (Path.iterdir / Path.rglob), **not** the file_store index. The store may lag behind disk during indexing or after rapid mutations; for the most current view, the on-disk walk is the source of truth. BREAKING CHANGE: renamed step from file_list to list and removed dependency on file_store.graph for enumeration. feat(property): restrict property operations to markdown files only Property steps now operate only on .md files; non-markdown targets get error="not markdown" and the call is **not** executed. BREAKING CHANGE: property steps (read, update, delete) now validate file extension and reject non-markdown files. refactor(stat): rename file_stat to stat BREAKING CHANGE: step name changed from file_stat to stat. refactor(upload): rename file_upload to upload BREAKING CHANGE: step name changed from file_upload to upload. feat(traverse): support multiple seeds in graph traversal Allow path parameter to accept either a single seed (str) or a list of seeds for batch traversal operations. --- reme2/steps/crud/download.py | 59 +++++++++++++++--------- reme2/steps/crud/list.py | 43 ++++++++++-------- reme2/steps/crud/property/__init__.py | 5 ++- reme2/steps/crud/property/delete.py | 2 + reme2/steps/crud/property/read.py | 2 + reme2/steps/crud/property/update.py | 65 +++++++++++---------------- reme2/steps/crud/stat.py | 2 +- reme2/steps/crud/upload.py | 2 +- reme2/steps/graph/traverse.py | 9 ++-- 9 files changed, 104 insertions(+), 85 deletions(-) diff --git a/reme2/steps/crud/download.py b/reme2/steps/crud/download.py index 4422e041..8cb633f5 100644 --- a/reme2/steps/crud/download.py +++ b/reme2/steps/crud/download.py @@ -1,21 +1,23 @@ -"""``file_download`` — copy a vault file to a session temp dir. +"""``file_download`` — copy a vault file to a local path. -Agent flow: agent calls ``file_download(path)``, gets back a -local path under a fresh per-call temp directory, then opens / parses -the file with whatever tooling it likes. The vault copy is untouched. +Symmetric counterpart to ``file_upload``: source is in the vault, +target is on the local filesystem. -The temp root is lazy and session-scoped — created on first download, -left for the OS to clean up at process exit. Each download lands in -its own subdirectory so concurrent agents don't trample each other. +``path`` (source) accepts a short link or vault-relative path; +resolution goes through ``path_resolver.resolve_to_absolute``. +Short-link ambiguity is surfaced as ``error="ambiguous"`` with the +candidate list — the download is **not** executed in that case. -``path`` accepts a short link or a vault-relative path; resolution -goes through ``path_resolver.resolve_to_absolute``. Short-link -ambiguity is surfaced as ``error="ambiguous"`` with the candidate -list — the download is **not** executed in that case. +``local_path`` (target) is a plain filesystem path. **Optional** — +when empty, the file lands in a session-scoped temp dir (lazy, +auto-cleaned on process exit; each call gets its own subdirectory +so concurrent agents don't trample each other). ``overwrite=True`` +is the default since most download flows are intentional refreshes. """ from __future__ import annotations +import mimetypes import shutil import tempfile from pathlib import Path @@ -41,9 +43,9 @@ def _get_temp_root() -> Path: return _TEMP_ROOT -@R.register("file_download") +@R.register("download") class FileDownload(BaseStep): - """Copy a vault file to a session temp dir; return the local path.""" + """Copy a vault file to ``local_path`` (or a temp dir if omitted).""" component_type = ComponentEnum.STEP @@ -52,18 +54,26 @@ class FileDownload(BaseStep): async def execute(self): assert self.context is not None path: str = self.context.get("path", "") or "" + local_path: str = self.context.get("local_path", "") or "" + overwrite: bool = bool(self.context.get("overwrite", True)) assert path, "path is required" - payload = await self._download(path) + payload = await self._download(path, local_path, overwrite) self.context.response.success = "error" not in payload _set_answer(self.context, payload) - async def file_download(self, path: str) -> ToolResponse: - """Copy a vault file to session temp dir; return the local path.""" - payload = await self._download(path) + async def file_download( + self, path: str, local_path: str = "", overwrite: bool = True, + ) -> ToolResponse: + """Copy the vault file at ``path`` to ``local_path``. + + ``local_path`` empty → land in a fresh temp subdirectory under + the session temp root. + """ + payload = await self._download(path, local_path, overwrite) ok = "error" not in payload return _tool_response("file_download", ok, payload, audit=self.audit) - async def _download(self, path: str) -> dict: + async def _download(self, path: str, local_path: str, overwrite: bool) -> dict: try: src = await path_resolver.resolve_to_absolute(self.file_store, path) except path_resolver.PathAmbiguous as e: @@ -72,11 +82,20 @@ class FileDownload(BaseStep): return {"path": path, "error": "not found"} if not src.is_file(): return {"path": path, "error": "not found"} - dst_dir = Path(tempfile.mkdtemp(prefix="dl-", dir=_get_temp_root())) - dst = dst_dir / src.name + + if local_path: + dst = Path(local_path) + if dst.exists() and not overwrite: + return {"local_path": local_path, "error": "destination exists; pass overwrite=True"} + dst.parent.mkdir(parents=True, exist_ok=True) + else: + dst_dir = Path(tempfile.mkdtemp(prefix="dl-", dir=_get_temp_root())) + dst = dst_dir / src.name + shutil.copy2(src, dst) return { "path": path, "local_path": str(dst), "size": dst.stat().st_size, + "mime": mimetypes.guess_type(dst.name)[0] or "application/octet-stream", } diff --git a/reme2/steps/crud/list.py b/reme2/steps/crud/list.py index 499e1e6a..2b88ca5d 100644 --- a/reme2/steps/crud/list.py +++ b/reme2/steps/crud/list.py @@ -1,8 +1,9 @@ """``file_list`` — enumerate vault files under a directory. -Walks the file_graph in one shot via ``get_nodes(None)`` — no -filesystem scan, no per-file frontmatter parse, no per-file graph -round-trip. +Reads directly from the filesystem (``Path.iterdir`` / +``Path.rglob``), **not** the file_store index. The store may lag +behind disk during indexing or after rapid mutations; for the most +current view, the on-disk walk is the source of truth. Parameters: path — directory to list under (vault-relative or absolute). @@ -15,6 +16,7 @@ Parameters: from __future__ import annotations +from collections.abc import Iterator from pathlib import Path from agentscope.tool import ToolResponse @@ -27,34 +29,35 @@ from ...enumeration import ComponentEnum from ...utils import path_resolver -def _under(node_abs: Path, target_dir: Path, *, recursive: bool) -> bool: - if recursive: - return node_abs == target_dir or target_dir in node_abs.parents - return node_abs.parent == target_dir +def _walk(target_dir: Path, recursive: bool) -> Iterator[Path]: + items = target_dir.rglob("*") if recursive else target_dir.iterdir() + return (p for p in items if p.is_file()) -async def _list( +def _list( file_store, *, path: str, recursive: bool, limit: int, ) -> dict: - if not file_store.file_graph: - return {"items": [], "count": 0} target_dir = path_resolver.to_absolute(file_store, path or ".") + if not target_dir.is_dir(): + return {"items": [], "count": 0} + working_dir = Path(getattr(file_store, "working_dir", None) or ".").resolve() items: list[dict] = [] - for node in await file_store.file_graph.get_nodes(): - node_abs = path_resolver.to_absolute(file_store, node.path) - if not _under(node_abs, target_dir, recursive=recursive): - continue - items.append({"path": node.path, "metadata": node.front_matter.model_dump()}) + for entry in _walk(target_dir, recursive): + try: + rel = str(entry.relative_to(working_dir)) + except ValueError: + rel = str(entry) + items.append({"path": rel}) if len(items) >= limit: break return {"items": items, "count": len(items)} -@R.register("file_list") +@R.register("list") class FileList(BaseStep): """Enumerate vault files under a directory.""" @@ -64,7 +67,7 @@ class FileList(BaseStep): async def execute(self): assert self.context is not None - result = await _list( + result = _list( self.file_store, path=self.context.get("path") or "", recursive=bool(self.context.get("recursive", False)), @@ -80,6 +83,10 @@ class FileList(BaseStep): ) -> ToolResponse: """List vault files under ``path``. + Reads the filesystem directly (no file_store cache). Returns + ``{items: [{path}, ...], count}`` where ``path`` is + vault-relative. + Args: path: directory to list (relative to working_dir or absolute). Empty = working_dir root. @@ -87,7 +94,7 @@ class FileList(BaseStep): recursive: descend into subdirectories. Default False = direct children only. """ - result = await _list( + result = _list( self.file_store, path=path, recursive=recursive, diff --git a/reme2/steps/crud/property/__init__.py b/reme2/steps/crud/property/__init__.py index 4c5db21b..11ed8c79 100644 --- a/reme2/steps/crud/property/__init__.py +++ b/reme2/steps/crud/property/__init__.py @@ -1,4 +1,4 @@ -"""Property steps — frontmatter-only CRUD on memory files. +"""Property steps — frontmatter-only CRUD on **markdown** files. Three Steps: @@ -6,6 +6,9 @@ Three Steps: property:update — merge a patch into the frontmatter property:delete — drop the listed keys +Operates only on ``.md`` files; non-markdown targets get +``error="not markdown"`` and the call is **not** executed. + Body content stays untouched; use the ``crud`` siblings (``read``, ``edit``, ``append``, ``prepend``) for body-level operations. Each Step here is a pure disk read-modify-write — the watcher / parser diff --git a/reme2/steps/crud/property/delete.py b/reme2/steps/crud/property/delete.py index 0bcea44d..d9184fd7 100644 --- a/reme2/steps/crud/property/delete.py +++ b/reme2/steps/crud/property/delete.py @@ -33,6 +33,8 @@ async def _delete(file_store, path: str, keys: list[str]) -> dict: return {"path": path, "error": "not found"} if not target.is_file(): return {"path": path, "error": "not found"} + if target.suffix != ".md": + return {"path": path, "error": "not markdown"} if not keys: return {"path": path, "error": "keys is empty"} raw = target.read_text(encoding="utf-8") diff --git a/reme2/steps/crud/property/read.py b/reme2/steps/crud/property/read.py index 3169751b..995bfaac 100644 --- a/reme2/steps/crud/property/read.py +++ b/reme2/steps/crud/property/read.py @@ -31,6 +31,8 @@ async def _read(file_store, path: str) -> dict: return {"path": path, "exists": False} if not target.is_file(): return {"path": path, "exists": False} + if target.suffix != ".md": + return {"path": path, "error": "not markdown"} raw = target.read_text(encoding="utf-8") meta = dict(frontmatter.loads(raw).metadata) return {"path": path, "exists": True, "frontmatter": meta} diff --git a/reme2/steps/crud/property/update.py b/reme2/steps/crud/property/update.py index bf771e0b..419f616c 100644 --- a/reme2/steps/crud/property/update.py +++ b/reme2/steps/crud/property/update.py @@ -1,16 +1,16 @@ -"""``property:update`` — merge a patch into a memory file's frontmatter. +"""``property:update`` — set frontmatter keys on a markdown file. Read-modify-write the YAML frontmatter; body content is untouched. The watcher / parser pick up the change asynchronously. -Two input modes (mutually compatible — they merge in order): - * ``patch`` — explicit ``dict`` of frontmatter updates. - * ``**fields`` — free-form key/value pairs (obsidian convention - ``path=... key=val key=val``); merged on top of ``patch``. +Free-form kwargs form: ``property:update path=foo.md x=y z=w`` sets +frontmatter ``x`` to ``y`` and ``z`` to ``w``. Each keyword arg +becomes one frontmatter entry. ``path`` accepts a short link or vault-relative path. Short-link ambiguity is reported via ``error="ambiguous"`` with the candidate -list — the update is **not** executed in that case. +list — the update is **not** executed in that case. Non-markdown +targets return ``error="not markdown"``. """ from __future__ import annotations @@ -27,12 +27,12 @@ from ....utils import path_resolver # Context keys that belong to plumbing (request envelope, response -# slot, the path itself) and must never be promoted into a free-form -# frontmatter patch. -_RESERVED_CONTEXT_KEYS = {"path", "patch", "response", "request", "data"} +# slot, the path itself) and must never be promoted into a frontmatter +# update. +_RESERVED_CONTEXT_KEYS = {"path", "response", "request", "data"} -async def _update(file_store, path: str, patch: dict) -> dict: +async def _update(file_store, path: str, fields: dict) -> dict: try: target = await path_resolver.resolve_to_absolute(file_store, path) except path_resolver.PathAmbiguous as e: @@ -41,18 +41,20 @@ async def _update(file_store, path: str, patch: dict) -> dict: return {"path": path, "error": "not found"} if not target.is_file(): return {"path": path, "error": "not found"} - if not patch: - return {"path": path, "error": "patch is empty"} + if target.suffix != ".md": + return {"path": path, "error": "not markdown"} + if not fields: + return {"path": path, "error": "no fields to update"} raw = target.read_text(encoding="utf-8") post = frontmatter.loads(raw) - post.metadata.update(patch) + post.metadata.update(fields) target.write_text(frontmatter.dumps(post), encoding="utf-8") - return {"path": path, "updated": list(patch.keys())} + return {"path": path, "updated": fields} @R.register("property:update") class PropertyUpdate(BaseStep): - """Merge a patch into a memory file's frontmatter.""" + """Set frontmatter keys on a markdown file via free-form kwargs.""" component_type = ComponentEnum.STEP @@ -62,32 +64,17 @@ class PropertyUpdate(BaseStep): assert self.context is not None path: str = self.context.get("path") or "" assert path, "path is required" - patch = self.context.get("patch") - if patch is None: - # Free-form mode — every other context key is treated as a - # patch entry, matching the obsidian convention - # `path=... key=val key=val`. - patch = { - k: v for k, v in self.context.data.items() - if k not in _RESERVED_CONTEXT_KEYS - } - payload = await _update(self.file_store, path, dict(patch or {})) + fields = { + k: v for k, v in self.context.data.items() + if k not in _RESERVED_CONTEXT_KEYS + } + payload = await _update(self.file_store, path, fields) self.context.response.success = "error" not in payload _set_answer(self.context, payload) - async def property_update( - self, - path: str, - patch: dict | None = None, - **fields, - ) -> ToolResponse: - """Merge ``patch`` (or free-form ``**fields``) into the - frontmatter at ``path``.""" - merged: dict = {} - if patch: - merged.update(patch) - if fields: - merged.update(fields) - payload = await _update(self.file_store, path, merged) + async def property_update(self, path: str, **kwargs) -> ToolResponse: + """Set frontmatter entries: each ``key=value`` kwarg becomes one + frontmatter field on the markdown file at ``path``.""" + payload = await _update(self.file_store, path, dict(kwargs)) ok = "error" not in payload return _tool_response("property:update", ok, payload, audit=self.audit) diff --git a/reme2/steps/crud/stat.py b/reme2/steps/crud/stat.py index fe070c13..b8f574f4 100644 --- a/reme2/steps/crud/stat.py +++ b/reme2/steps/crud/stat.py @@ -77,7 +77,7 @@ async def _stat(file_store, path: str) -> dict: return out -@R.register("file_stat") +@R.register("stat") class FileStat(BaseStep): """Return metadata for a vault file or directory.""" diff --git a/reme2/steps/crud/upload.py b/reme2/steps/crud/upload.py index c87bc56e..b55bd4c8 100644 --- a/reme2/steps/crud/upload.py +++ b/reme2/steps/crud/upload.py @@ -32,7 +32,7 @@ from ...enumeration import ComponentEnum from ...utils import path_resolver -@R.register("file_upload") +@R.register("upload") class FileUpload(BaseStep): """Copy ``local_path`` into the vault at ``path``.""" diff --git a/reme2/steps/graph/traverse.py b/reme2/steps/graph/traverse.py index 8a680c1c..4dc6daac 100644 --- a/reme2/steps/graph/traverse.py +++ b/reme2/steps/graph/traverse.py @@ -152,7 +152,7 @@ class GraphTraverse(BaseStep): """BFS from a seed file to explore wikilink relationships. Parameters (per obsidian convention): - path — single seed (str). ``seeds`` accepted as alias for batch mode. + path — single seed (str) or a list of seeds. direction — ``forward`` / ``backward`` / ``both`` (or ``out`` / ``in`` / ``both``). depth — hop limit (default 1 = immediate neighbors). predicate — optional edge-type filter; ``None`` = no filter. @@ -164,9 +164,8 @@ class GraphTraverse(BaseStep): async def execute(self): assert self.context is not None - path = self.context.get("path") - seeds_raw = self.context.get("seeds") if path is None else path - depth = int(self.context.get("depth") or self.context.get("max_depth") or 1) + seeds_raw = self.context.get("path") + depth = int(self.context.get("depth") or 1) direction = (self.context.get("direction") or "forward").lower() predicate = self.context.get("predicate") assert direction in _VALID_DIRECTIONS, ( @@ -186,7 +185,7 @@ class GraphTraverse(BaseStep): self.context.response.success = False _set_answer(self.context, {"error": "not found", "target": e.target}) return - assert seeds, "path (or seeds) is required" + assert seeds, "path is required" outbound, inbound = await _build_indexes(self.file_store) results = _bfs(seeds, depth, direction, predicate, outbound, inbound) _set_answer(self.context, results)