From e465ece7cbedf8267084e09a37d0e5484d104072 Mon Sep 17 00:00:00 2001 From: chenpengfei Date: Thu, 20 Aug 2026 12:14:48 +0800 Subject: [PATCH 01/20] refactor: share vector URI rewrite rules --- openviking/storage/vector_migration.py | 93 ++++++++++++++++++-------- tests/storage/test_vector_migration.py | 88 ++++++++++++++++++++++++ 2 files changed, 154 insertions(+), 27 deletions(-) diff --git a/openviking/storage/vector_migration.py b/openviking/storage/vector_migration.py index 0e33820d23..0886ee0508 100644 --- a/openviking/storage/vector_migration.py +++ b/openviking/storage/vector_migration.py @@ -5,9 +5,13 @@ from __future__ import annotations from dataclasses import dataclass, field -from typing import Any +from typing import Any, Literal -from openviking.core.namespace import context_type_for_uri, owner_fields_for_uri +from openviking.core.namespace import ( + context_type_for_uri, + owner_fields_for_uri, + owner_space_for_uri, +) from openviking.server.identity import RequestContext, Role from openviking.storage.expr import And, Contains, Eq, Or, PathScope from openviking.storage.vector_ids import vector_record_id @@ -58,6 +62,10 @@ def _root_ctx(account_id: str) -> RequestContext: return RequestContext(user=UserIdentifier(account_id, "default"), role=Role.ROOT) +def _normalize_uri(uri: str) -> str: + return VikingURI(uri).uri.rstrip("/") + + def _vector_record_id(account_id: str, uri: str, level: Any) -> str: return vector_record_id(account_id, uri, level) @@ -70,7 +78,9 @@ def _has_vector_payload(record: dict[str, Any]) -> bool: return False -def _uri_in_scope(uri: str, scope_uri: str, *, recursive: bool) -> bool: +def uri_in_transfer_scope(uri: str, scope_uri: str, *, recursive: bool) -> bool: + """Return whether *uri* belongs to a file or recursive directory transfer.""" + scope_uri = _normalize_uri(scope_uri) return ( uri == scope_uri or uri.startswith(scope_uri + "#") @@ -78,7 +88,10 @@ def _uri_in_scope(uri: str, scope_uri: str, *, recursive: bool) -> bool: ) -def _rewrite_uri(uri: str, source_uri: str, target_uri: str) -> str: +def rewrite_transfer_uri(uri: str, source_uri: str, target_uri: str) -> str: + """Rewrite a URI from one transfer scope to another without prefix leakage.""" + source_uri = _normalize_uri(source_uri) + target_uri = _normalize_uri(target_uri) if uri == source_uri: return target_uri if uri.startswith(source_uri + "/") or uri.startswith(source_uri + "#"): @@ -86,6 +99,46 @@ def _rewrite_uri(uri: str, source_uri: str, target_uri: str) -> str: return uri +def rewrite_vector_record( + record: dict[str, Any], + *, + source_uri: str, + target_uri: str, + ctx: RequestContext, + mode: Literal["copy", "move"], + timestamp: Any, +) -> dict[str, Any]: + """Build a target vector record while retaining its existing vector payload.""" + if mode not in {"copy", "move"}: + raise ValueError(f"Unsupported vector transfer mode: {mode}") + record_uri = record.get("uri") + if not isinstance(record_uri, str): + raise ValueError("Vector record is missing a string URI") + + rewritten_uri = rewrite_transfer_uri(record_uri, source_uri, target_uri) + payload = {key: value for key, value in record.items() if key != "_score"} + owner_fields = owner_fields_for_uri(rewritten_uri) + payload.update( + { + "id": _vector_record_id(ctx.account_id, rewritten_uri, record.get("level", 2)), + "uri": rewritten_uri, + "account_id": ctx.account_id, + "owner_user_id": owner_fields.get("owner_user_id"), + "owner_space": owner_space_for_uri(rewritten_uri), + "context_type": context_type_for_uri(rewritten_uri), + } + ) + if mode == "copy": + payload.update( + { + "created_at": timestamp, + "updated_at": timestamp, + "active_count": 0, + } + ) + return payload + + async def _records_in_scope( vector_store: Any, *, @@ -114,7 +167,7 @@ async def _records_in_scope( record for record in records if isinstance(record.get("uri"), str) - and _uri_in_scope(record["uri"], uri, recursive=recursive) + and uri_in_transfer_scope(record["uri"], uri, recursive=recursive) ] @@ -164,27 +217,13 @@ async def copy_vector_records( result.skipped += 1 continue - rewritten_uri = _rewrite_uri(source_record_uri, source_uri, target_uri) - owner_fields = owner_fields_for_uri(rewritten_uri) - level = record.get("level", 2) - payload = { - key: value - for key, value in record.items() - if key not in {"id", "uri", "account_id", "owner_user_id", "owner_space", "_score"} - and value is not None - } - payload.update( - { - "id": _vector_record_id(account_id, rewritten_uri, level), - "uri": rewritten_uri, - "account_id": account_id, - "owner_user_id": owner_fields.get("owner_user_id"), - "owner_space": owner_fields.get("owner_user_id") or "", - "context_type": context_type_for_uri(rewritten_uri), - "created_at": timestamp, - "updated_at": timestamp, - "active_count": 0, - } + payload = rewrite_vector_record( + record, + source_uri=source_uri, + target_uri=target_uri, + ctx=ctx, + mode="copy", + timestamp=timestamp, ) try: await vector_store.upsert(payload, ctx=ctx) @@ -192,7 +231,7 @@ async def copy_vector_records( except Exception as exc: result.failed += 1 result.warnings.append( - f"Failed to copy vector {source_record_uri} to {rewritten_uri}: {exc}" + f"Failed to copy vector {source_record_uri} to {payload['uri']}: {exc}" ) return result diff --git a/tests/storage/test_vector_migration.py b/tests/storage/test_vector_migration.py index 3374419e72..5627e42d91 100644 --- a/tests/storage/test_vector_migration.py +++ b/tests/storage/test_vector_migration.py @@ -3,7 +3,95 @@ import pytest +from openviking.server.identity import RequestContext, Role +from openviking.storage import vector_migration from openviking.storage.vector_migration import copy_vector_records, delete_vector_records +from openviking_cli.session.user_id import UserIdentifier + + +def _ctx(account_id: str = "acct", user_id: str = "alice") -> RequestContext: + return RequestContext(user=UserIdentifier(account_id, user_id), role=Role.USER) + + +def test_transfer_scope_includes_chunks_but_not_sibling_prefixes(): + source = "viking://resources/src.md" + + assert vector_migration.uri_in_transfer_scope(source, source, recursive=False) + assert vector_migration.uri_in_transfer_scope(f"{source}#chunk_0001", source, recursive=False) + assert not vector_migration.uri_in_transfer_scope( + "viking://resources/src.md.backup", source, recursive=True + ) + assert not vector_migration.uri_in_transfer_scope( + "viking://resources/src.md/child", source, recursive=False + ) + assert vector_migration.uri_in_transfer_scope( + "viking://resources/src.md/child", source, recursive=True + ) + + +def test_rewrite_vector_record_for_copy_preserves_payload_and_resets_metadata(): + record = { + "id": "old-id", + "uri": "viking://user/alice/memories/src.md#chunk_1", + "level": 2, + "vector": [0.1, 0.2], + "sparse_vector": {"7": 0.8}, + "content": "chunk", + "tags": ["team=a"], + "created_at": 10, + "updated_at": 11, + "active_count": 9, + "account_id": "acct", + "owner_user_id": "alice", + } + + result = vector_migration.rewrite_vector_record( + record, + source_uri="viking://user/alice/memories/src.md", + target_uri="viking://user/alice/memories/dst.md", + ctx=_ctx(), + mode="copy", + timestamp=123, + ) + + assert result["uri"] == "viking://user/alice/memories/dst.md#chunk_1" + assert result["id"] != "old-id" + assert result["vector"] == [0.1, 0.2] + assert result["sparse_vector"] == {"7": 0.8} + assert result["content"] == "chunk" + assert result["tags"] == ["team=a"] + assert result["created_at"] == 123 + assert result["updated_at"] == 123 + assert result["active_count"] == 0 + assert result["owner_user_id"] == "alice" + + +@pytest.mark.parametrize("level", [0, 1, 2]) +def test_rewrite_vector_record_for_move_preserves_metadata(level): + record = { + "id": "old-id", + "uri": "viking://resources/src", + "level": level, + "vector": [0.1], + "created_at": 10, + "updated_at": 11, + "active_count": 9, + } + + result = vector_migration.rewrite_vector_record( + record, + source_uri="viking://resources/src", + target_uri="viking://resources/dst", + ctx=_ctx(), + mode="move", + timestamp=123, + ) + + assert result["uri"] == "viking://resources/dst" + assert result["id"] != "old-id" + assert result["created_at"] == 10 + assert result["updated_at"] == 11 + assert result["active_count"] == 9 class FakeVectorStore: From ba3303329de60578a1cd58b0066813a5ea7afbe9 Mon Sep 17 00:00:00 2001 From: chenpengfei Date: Thu, 20 Aug 2026 12:24:23 +0800 Subject: [PATCH 02/20] feat: add strict vector transfer transactions --- .../storage/viking_vector_index_backend.py | 345 ++++++++++++++---- openviking/storage/vikingdb_manager.py | 26 +- tests/storage/test_vector_transfer.py | 276 ++++++++++++++ 3 files changed, 562 insertions(+), 85 deletions(-) create mode 100644 tests/storage/test_vector_transfer.py diff --git a/openviking/storage/viking_vector_index_backend.py b/openviking/storage/viking_vector_index_backend.py index 23b78de8c4..d105aec7b1 100644 --- a/openviking/storage/viking_vector_index_backend.py +++ b/openviking/storage/viking_vector_index_backend.py @@ -10,7 +10,7 @@ from dataclasses import dataclass from typing import Any, AsyncIterator, Dict, List, Mapping, Optional -from openviking.core.namespace import canonical_user_root, uri_parts, visible_roots +from openviking.core.namespace import canonical_user_root, resolve_uri, uri_parts, visible_roots from openviking.server.identity import RequestContext, Role from openviking.storage.acl import ( ACL_CONTEXT_FIELDS, @@ -19,13 +19,18 @@ acl_grant_tokens, acl_principals, ) -from openviking.storage.expr import And, Eq, FilterExpr, In, Or, PathScope, RawDSL -from openviking.storage.vector_ids import vector_record_id +from openviking.storage.expr import And, Contains, Eq, FilterExpr, In, Or, PathScope, RawDSL +from openviking.storage.vector_migration import ( + rewrite_vector_record, + uri_in_transfer_scope, +) from openviking.storage.vectordb.collection.collection import Collection from openviking.storage.vectordb.collection.result import UpdateResult from openviking.storage.vectordb.utils.logging_init import init_cpp_logging from openviking.storage.vectordb_adapters import create_collection_adapter from openviking.utils.tags import merge_search_tags +from openviking.utils.time_utils import get_current_timestamp +from openviking_cli.exceptions import ConflictError from openviking_cli.utils import get_logger from openviking_cli.utils.config.vectordb_config import DEFAULT_INDEX_NAME, VectorDBBackendConfig @@ -74,6 +79,26 @@ class UpsertOptions: search_tag_mode: str = "replace" +@dataclass +class VectorTransferResult: + """Counts produced by one strict online vector URI transfer.""" + + scanned: int = 0 + written: int = 0 + deleted: int = 0 + restored: int = 0 + batches: int = 0 + + +class VectorTransferRollbackError(RuntimeError): + """Raised when a vector transfer and its compensation both fail.""" + + def __init__(self, message: str, *, phase: str, residual_count: int): + super().__init__(message) + self.phase = phase + self.residual_count = residual_count + + def normalize_upsert_options( options: UpsertOptions | Mapping[str, Any] | None = None, ) -> UpsertOptions: @@ -1435,95 +1460,255 @@ async def delete_uris(self, ctx: RequestContext, uris: List[str]) -> None: backend = self._get_backend_for_context(ctx) await backend.delete_by_filter(And(conds)) - async def update_uri_mapping( + @staticmethod + def _uri_transfer_filter(ctx: RequestContext, uri: str, *, recursive: bool) -> FilterExpr: + scopes: List[FilterExpr] = [Eq("uri", uri), Contains("uri", uri + "#")] + if recursive: + scopes.append(PathScope("uri", uri, depth=-1)) + return And([Eq("account_id", ctx.account_id), Or(scopes)]) + + async def _scan_uri_transfer_scope( self, ctx: RequestContext, uri: str, - new_uri: str, - levels: Optional[List[int]] = None, - ) -> bool: - conds: List[FilterExpr] = [Eq("uri", uri), Eq("account_id", ctx.account_id)] - if levels: - conds.append(In("level", levels)) + *, + recursive: bool, + include_full_records: bool, + batch_size: int = 100, + ) -> tuple[List[Dict[str, Any]], int]: + """Scan one URI scope without a fixed total-record limit.""" + records: List[Dict[str, Any]] = [] + cursor: Optional[str] = None + batches = 0 + seen_cursors: set[str] = set() + while True: + page, next_cursor = await self.scroll( + filter=self._uri_transfer_filter(ctx, uri, recursive=recursive), + limit=batch_size, + cursor=cursor, + output_fields=["id", "uri"], + ctx=ctx, + ) + batches += 1 + scoped = [ + record + for record in page + if isinstance(record.get("uri"), str) + and uri_in_transfer_scope(record["uri"], uri, recursive=recursive) + ] + if include_full_records and scoped: + ids = [str(record["id"]) for record in scoped if record.get("id")] + if len(ids) != len(scoped): + raise RuntimeError(f"Vector records without IDs found under {uri}") + full_records = await self.get(ids, ctx=ctx) + by_id = {str(record["id"]): record for record in full_records if record.get("id")} + if len(by_id) != len(ids): + raise RuntimeError(f"Failed to fetch complete vector records under {uri}") + records.extend(by_id[record_id] for record_id in ids) + else: + records.extend(scoped) - records = await self.filter( - filter=And(conds), - limit=100, - output_fields=["id"], - ctx=ctx, + if next_cursor is None: + break + if next_cursor in seen_cursors: + raise RuntimeError(f"Vector scroll cursor repeated under {uri}: {next_cursor}") + seen_cursors.add(next_cursor) + cursor = next_cursor + return records, batches + + async def _delete_vector_transfer_ids( + self, + ctx: RequestContext, + ids: List[str], + *, + batch_size: int = 100, + ) -> int: + deleted = 0 + for offset in range(0, len(ids), batch_size): + deleted += int(await self.delete(ids[offset : offset + batch_size], ctx=ctx) or 0) + return deleted + + async def copy_uri_mapping( + self, + ctx: RequestContext, + source_uri: str, + target_uri: str, + recursive: bool = False, + ) -> VectorTransferResult: + """Copy every vector record in a URI scope without regenerating embeddings.""" + source_uri = resolve_uri(source_uri).uri + target_uri = resolve_uri(target_uri).uri + target_records, _ = await self._scan_uri_transfer_scope( + ctx, + target_uri, + recursive=recursive, + include_full_records=False, ) - if not records: - return False - record_ids = [str(record["id"]) for record in records if record.get("id")] - if not record_ids: - logger.warning( - "update_uri_mapping found records without ids: uri=%s new_uri=%s account_id=%s", - uri, - new_uri, - ctx.account_id, + if target_records: + raise ConflictError( + f"copy target vector scope already exists: {target_uri}", + resource=target_uri, ) - return False - full_records = await self.get(record_ids, ctx=ctx) - if not full_records: - logger.warning( - "update_uri_mapping failed to fetch full records: uri=%s new_uri=%s account_id=%s ids=%s", - uri, - new_uri, - ctx.account_id, - record_ids, - ) - return False - updated_records: List[Dict[str, Any]] = [] - ids_to_delete: List[str] = [] - for record in full_records: - if "id" not in record: - continue - raw_level = record.get("level", 2) + source_records, batches = await self._scan_uri_transfer_scope( + ctx, + source_uri, + recursive=recursive, + include_full_records=True, + ) + result = VectorTransferResult(scanned=len(source_records), batches=batches) + if not source_records: + return result + + timestamp = get_current_timestamp() + target_payloads = [ + rewrite_vector_record( + record, + source_uri=source_uri, + target_uri=target_uri, + ctx=ctx, + mode="copy", + timestamp=timestamp, + ) + for record in source_records + ] + attempted_target_ids: List[str] = [] + try: + for offset in range(0, len(target_payloads), 100): + payload_batch = target_payloads[offset : offset + 100] + attempted_target_ids.extend(str(payload["id"]) for payload in payload_batch) + written_ids = await self.upsert_many(payload_batch, ctx=ctx) + if len(written_ids) != len(payload_batch): + raise RuntimeError( + f"Vector copy wrote {len(written_ids)} of {len(payload_batch)} records" + ) + result.written += len(written_ids) + except Exception as transfer_error: try: - level = int(raw_level) - except (TypeError, ValueError): - level = 2 - - new_id = vector_record_id(ctx.account_id, new_uri, level) - - updated = { - **record, - "id": new_id, - "uri": new_uri, - } - if self._acl_enabled(ctx): - updated.update( - await self.acl_manager.materialize_moved_record(record, new_uri, ctx) - ) - vector = updated.get("vector") - if not vector: - logger.warning( - "update_uri_mapping skipped record without dense vector: old_uri=%s new_uri=%s level=%s account_id=%s id=%s", - uri, - new_uri, - level, - ctx.account_id, - record.get("id"), - ) - continue - updated_records.append(updated) - old_id = record.get("id") - if old_id and old_id != new_id: - ids_to_delete.append(old_id) + await self._delete_vector_transfer_ids(ctx, attempted_target_ids) + except Exception as rollback_error: + diagnostic_suffix = "" + try: + residual_records, _ = await self._scan_uri_transfer_scope( + ctx, + target_uri, + recursive=recursive, + include_full_records=False, + ) + residual_count = len(residual_records) + except Exception as diagnostic_error: + residual_count = len(attempted_target_ids) + diagnostic_suffix = f"; residual scan failed: {diagnostic_error}" + raise VectorTransferRollbackError( + f"Vector copy failed and target cleanup failed: {rollback_error}" + f"{diagnostic_suffix}", + phase="copy_target_cleanup", + residual_count=residual_count, + ) from transfer_error + raise + return result - if not updated_records: - return False - new_ids = await self._upsert_many_raw(updated_records, ctx=ctx) - if len(new_ids) != len(updated_records): - raise RuntimeError( - f"Failed to update {len(updated_records) - len(new_ids)} URI mapping record(s)" + async def update_uri_mapping( + self, + ctx: RequestContext, + source_uri: str, + target_uri: str, + recursive: bool = False, + ) -> VectorTransferResult: + """Move every vector record in a URI scope with compensating rollback.""" + source_uri = resolve_uri(source_uri).uri + target_uri = resolve_uri(target_uri).uri + target_records, _ = await self._scan_uri_transfer_scope( + ctx, + target_uri, + recursive=recursive, + include_full_records=False, + ) + if target_records: + raise ConflictError( + f"move target vector scope already exists: {target_uri}", + resource=target_uri, ) - if ids_to_delete: - await self.delete(list(set(ids_to_delete)), ctx=ctx) + source_records, batches = await self._scan_uri_transfer_scope( + ctx, + source_uri, + recursive=recursive, + include_full_records=True, + ) + result = VectorTransferResult(scanned=len(source_records), batches=batches) + if not source_records: + return result + + timestamp = get_current_timestamp() + target_payloads = [ + rewrite_vector_record( + record, + source_uri=source_uri, + target_uri=target_uri, + ctx=ctx, + mode="move", + timestamp=timestamp, + ) + for record in source_records + ] + target_ids = [str(payload["id"]) for payload in target_payloads] + attempted_target_ids: List[str] = [] + try: + for offset in range(0, len(target_payloads), 100): + payload_batch = target_payloads[offset : offset + 100] + attempted_target_ids.extend(str(payload["id"]) for payload in payload_batch) + written_ids = await self.upsert_many(payload_batch, ctx=ctx) + if len(written_ids) != len(payload_batch): + raise RuntimeError( + f"Vector move wrote {len(written_ids)} of {len(payload_batch)} records" + ) + result.written += len(written_ids) + except Exception as transfer_error: + try: + await self._delete_vector_transfer_ids(ctx, attempted_target_ids) + except Exception as rollback_error: + raise VectorTransferRollbackError( + f"Vector move prepare failed and target cleanup failed: {rollback_error}", + phase="move_target_cleanup", + residual_count=len(attempted_target_ids), + ) from transfer_error + raise - return True + source_ids = [str(record["id"]) for record in source_records] + try: + for offset in range(0, len(source_ids), 100): + source_batch = source_ids[offset : offset + 100] + deleted = int(await self.delete(source_batch, ctx=ctx) or 0) + if deleted != len(source_batch): + raise RuntimeError( + f"Vector move deleted {deleted} of {len(source_batch)} source records" + ) + result.deleted += deleted + except Exception as transfer_error: + rollback_errors: List[Exception] = [] + try: + restored_ids = await self.upsert_many(source_records, ctx=ctx) + result.restored = len(restored_ids) + if result.restored != len(source_records): + raise RuntimeError( + f"Vector move restored {result.restored} of {len(source_records)} records" + ) + except Exception as rollback_error: + rollback_errors.append(rollback_error) + try: + await self._delete_vector_transfer_ids(ctx, target_ids) + except Exception as rollback_error: + rollback_errors.append(rollback_error) + if rollback_errors: + raise VectorTransferRollbackError( + "Vector move source deletion failed and compensation was incomplete: " + + "; ".join(str(error) for error in rollback_errors), + phase="move_source_restore", + residual_count=len(source_records), + ) from transfer_error + raise + return result async def increment_active_count(self, ctx: RequestContext, uris: List[str]) -> int: updated = 0 diff --git a/openviking/storage/vikingdb_manager.py b/openviking/storage/vikingdb_manager.py index 53f688bb66..4e099034d9 100644 --- a/openviking/storage/vikingdb_manager.py +++ b/openviking/storage/vikingdb_manager.py @@ -14,6 +14,7 @@ from openviking.storage.queuefs.queue_manager import QueueManager from openviking.storage.viking_vector_index_backend import ( UpsertOptions, + VectorTransferResult, VikingVectorIndexBackend, normalize_upsert_options, ) @@ -501,15 +502,30 @@ async def delete_account_data(self, account_id: str) -> int: async def delete_uris(self, uris: List[str]) -> None: return await self._manager.delete_uris(self._ctx, uris) + async def copy_uri_mapping( + self, + source_uri: str, + target_uri: str, + recursive: bool = False, + ) -> VectorTransferResult: + return await self._manager.copy_uri_mapping( + self._ctx, + source_uri=source_uri, + target_uri=target_uri, + recursive=recursive, + ) + async def update_uri_mapping( self, - uri: str, - new_uri: str, - ) -> bool: + source_uri: str, + target_uri: str, + recursive: bool = False, + ) -> VectorTransferResult: return await self._manager.update_uri_mapping( self._ctx, - uri=uri, - new_uri=new_uri, + source_uri=source_uri, + target_uri=target_uri, + recursive=recursive, ) async def increment_active_count(self, uris: List[str]) -> int: diff --git a/tests/storage/test_vector_transfer.py b/tests/storage/test_vector_transfer.py new file mode 100644 index 0000000000..a6e12a7a02 --- /dev/null +++ b/tests/storage/test_vector_transfer.py @@ -0,0 +1,276 @@ +# Copyright (c) 2026 Beijing Volcano Engine Technology Co., Ltd. +# SPDX-License-Identifier: AGPL-3.0 + +from __future__ import annotations + +from typing import Any + +import pytest + +from openviking.server.identity import RequestContext, Role +from openviking.storage.viking_vector_index_backend import ( + VectorTransferRollbackError, + VikingVectorIndexBackend, +) +from openviking_cli.exceptions import ConflictError +from openviking_cli.session.user_id import UserIdentifier + + +def _ctx() -> RequestContext: + return RequestContext(user=UserIdentifier("acct", "alice"), role=Role.USER) + + +def _record(record_id: str, uri: str, **overrides: Any) -> dict[str, Any]: + return { + "id": record_id, + "uri": uri, + "level": 2, + "vector": [0.1, 0.2], + "sparse_vector": {"7": 0.8}, + "content": f"content:{record_id}", + "created_at": 10, + "updated_at": 11, + "active_count": 3, + "account_id": "acct", + **overrides, + } + + +class _MemoryTransferBackend(VikingVectorIndexBackend): + """In-memory I/O boundary for exercising the real transfer methods.""" + + def __init__(self, records: list[dict[str, Any]]) -> None: + self.records = {str(record["id"]): dict(record) for record in records} + self.upsert_calls = 0 + self.fail_upsert_at: int | None = None + self.delete_calls = 0 + self.fail_delete_at: int | None = None + self.partial_delete_count = 0 + + async def scroll( + self, + filter=None, + limit: int = 100, + cursor: str | None = None, + output_fields=None, + *, + ctx: RequestContext, + ) -> tuple[list[dict[str, Any]], str | None]: + del filter, output_fields, ctx + offset = int(cursor or 0) + ordered = [dict(self.records[key]) for key in sorted(self.records)] + page = ordered[offset : offset + limit] + next_cursor = str(offset + limit) if offset + limit < len(ordered) else None + return page, next_cursor + + async def get(self, ids: list[str], *, ctx: RequestContext) -> list[dict[str, Any]]: + del ctx + return [dict(self.records[record_id]) for record_id in ids if record_id in self.records] + + async def upsert(self, data: dict[str, Any], *, ctx: RequestContext, options=None) -> str: + del ctx, options + self.upsert_calls += 1 + if self.fail_upsert_at == self.upsert_calls: + raise RuntimeError("injected vector write failure") + record = dict(data) + self.records[str(record["id"])] = record + return str(record["id"]) + + async def upsert_many( + self, data_list: list[dict[str, Any]], *, ctx: RequestContext + ) -> list[str]: + return [await self.upsert(data, ctx=ctx) for data in data_list] + + async def delete(self, ids: list[str], *, ctx: RequestContext) -> int: + del ctx + self.delete_calls += 1 + deleted = 0 + for record_id in ids: + if self.records.pop(record_id, None) is not None: + deleted += 1 + if self.fail_delete_at == self.delete_calls and deleted >= self.partial_delete_count: + self.fail_delete_at = None + raise RuntimeError("injected vector delete failure") + return deleted + + +def _records_under( + backend: _MemoryTransferBackend, uri: str, *, recursive: bool = True +) -> list[dict[str, Any]]: + return [ + record + for record in backend.records.values() + if record["uri"] == uri + or record["uri"].startswith(uri + "#") + or (recursive and record["uri"].startswith(uri + "/")) + ] + + +@pytest.mark.asyncio +async def test_copy_uri_mapping_keeps_source_and_copies_all_pages(): + source = "viking://resources/src" + records = [_record("source-root", source)] + [ + _record(f"source-{index:03d}", f"{source}/file-{index:03d}.md") for index in range(204) + ] + backend = _MemoryTransferBackend(records) + + result = await backend.copy_uri_mapping( + _ctx(), source, "viking://resources/dst", recursive=True + ) + + assert result.scanned == 205 + assert result.written == 205 + assert result.batches == 3 + assert len(_records_under(backend, source)) == 205 + assert len(_records_under(backend, "viking://resources/dst")) == 205 + + +@pytest.mark.asyncio +async def test_copy_uri_mapping_preserves_dense_sparse_and_chunk_payloads(): + source = "viking://resources/src.md" + backend = _MemoryTransferBackend( + [ + _record("source-file", source, sparse_vector={}), + _record("source-chunk", f"{source}#chunk_0001", vector=[]), + _record("outside", "viking://resources/other.md"), + ] + ) + + result = await backend.copy_uri_mapping( + _ctx(), source, "viking://resources/dst.md", recursive=False + ) + + copied = _records_under(backend, "viking://resources/dst.md") + assert result.written == 2 + assert {record["uri"] for record in copied} == { + "viking://resources/dst.md", + "viking://resources/dst.md#chunk_0001", + } + assert {tuple(record["vector"]) for record in copied} == {(), (0.1, 0.2)} + assert {tuple(record["sparse_vector"].items()) for record in copied} == { + (), + (("7", 0.8),), + } + + +@pytest.mark.asyncio +async def test_copy_uri_mapping_rejects_preexisting_target_before_writing(): + backend = _MemoryTransferBackend( + [ + _record("source", "viking://resources/src.md"), + _record("target", "viking://resources/dst.md#chunk_0001"), + ] + ) + + with pytest.raises(ConflictError, match="target vector scope already exists"): + await backend.copy_uri_mapping( + _ctx(), + "viking://resources/src.md", + "viking://resources/dst.md", + recursive=False, + ) + + assert backend.upsert_calls == 0 + assert len(_records_under(backend, "viking://resources/src.md")) == 1 + + +@pytest.mark.asyncio +async def test_copy_uri_mapping_removes_partial_target_when_write_fails(): + source = "viking://resources/src" + backend = _MemoryTransferBackend( + [ + _record("source-root", source), + _record("source-a", f"{source}/a.md"), + _record("source-b", f"{source}/b.md"), + ] + ) + backend.fail_upsert_at = 3 + + with pytest.raises(RuntimeError, match="injected vector write failure"): + await backend.copy_uri_mapping(_ctx(), source, "viking://resources/dst", recursive=True) + + assert len(_records_under(backend, source)) == 3 + assert _records_under(backend, "viking://resources/dst") == [] + + +@pytest.mark.asyncio +async def test_copy_uri_mapping_reports_actual_residual_after_cleanup_failure(): + source = "viking://resources/src" + backend = _MemoryTransferBackend( + [ + _record("source-root", source), + _record("source-a", f"{source}/a.md"), + _record("source-b", f"{source}/b.md"), + ] + ) + backend.fail_upsert_at = 3 + backend.fail_delete_at = 1 + backend.partial_delete_count = 1 + + with pytest.raises(VectorTransferRollbackError) as exc_info: + await backend.copy_uri_mapping(_ctx(), source, "viking://resources/dst", recursive=True) + + assert exc_info.value.phase == "copy_target_cleanup" + assert exc_info.value.residual_count == 1 + assert len(_records_under(backend, "viking://resources/dst")) == 1 + + +@pytest.mark.asyncio +async def test_update_uri_mapping_deletes_source_only_after_targets_exist(): + source = "viking://resources/src" + backend = _MemoryTransferBackend( + [ + _record("source-root", source, created_at=5, updated_at=6, active_count=7), + _record("source-a", f"{source}/a.md"), + _record("source-b", f"{source}/b.md"), + ] + ) + + result = await backend.update_uri_mapping( + _ctx(), source, "viking://resources/dst", recursive=True + ) + + assert result.scanned == 3 + assert result.written == 3 + assert result.deleted == 3 + assert _records_under(backend, source) == [] + targets = _records_under(backend, "viking://resources/dst") + assert len(targets) == 3 + root = next(record for record in targets if record["uri"] == "viking://resources/dst") + assert (root["created_at"], root["updated_at"], root["active_count"]) == (5, 6, 7) + + +@pytest.mark.asyncio +async def test_update_uri_mapping_restores_source_and_removes_target_after_partial_delete(): + source = "viking://resources/src" + backend = _MemoryTransferBackend( + [ + _record("source-root", source), + _record("source-a", f"{source}/a.md"), + _record("source-b", f"{source}/b.md"), + ] + ) + backend.fail_delete_at = 1 + backend.partial_delete_count = 1 + + with pytest.raises(RuntimeError, match="injected vector delete failure"): + await backend.update_uri_mapping(_ctx(), source, "viking://resources/dst", recursive=True) + + assert len(_records_under(backend, source)) == 3 + assert _records_under(backend, "viking://resources/dst") == [] + + +@pytest.mark.asyncio +async def test_update_uri_mapping_returns_empty_result_when_source_has_no_vectors(): + backend = _MemoryTransferBackend([_record("outside", "viking://resources/other.md")]) + + result = await backend.update_uri_mapping( + _ctx(), + "viking://resources/missing.md", + "viking://resources/dst.md", + recursive=False, + ) + + assert result.scanned == 0 + assert result.written == 0 + assert result.deleted == 0 From ad4e012661308777a5de8c381512ce8774a6d256 Mon Sep 17 00:00:00 2001 From: chenpengfei Date: Thu, 20 Aug 2026 13:04:40 +0800 Subject: [PATCH 03/20] feat: add transactional VikingFS copy --- openviking/pyagfs/async_client.py | 1 + openviking/storage/viking_fs/_ops.py | 462 ++++++++++++--- openviking/storage/viking_fs/_vector.py | 82 +-- tests/pyagfs/test_async_client_fs_ctx.py | 53 ++ ...test_semantic_processor_mv_vector_store.py | 50 +- tests/storage/test_viking_fs_cp.py | 540 ++++++++++++++++++ tests/storage/test_viking_fs_write_locking.py | 2 + 7 files changed, 1021 insertions(+), 169 deletions(-) create mode 100644 tests/storage/test_viking_fs_cp.py diff --git a/openviking/pyagfs/async_client.py b/openviking/pyagfs/async_client.py index 08f247dabf..7e0101286a 100644 --- a/openviking/pyagfs/async_client.py +++ b/openviking/pyagfs/async_client.py @@ -259,6 +259,7 @@ async def cp( src_path, dst_path, recursive=recursive, + stream=True, fs_ctx=_fs_ctx_with_auto_pathlock(src_path, fs_ctx, auto_pathlock), ) diff --git a/openviking/storage/viking_fs/_ops.py b/openviking/storage/viking_fs/_ops.py index c5773cdb6b..f2a85a84a8 100644 --- a/openviking/storage/viking_fs/_ops.py +++ b/openviking/storage/viking_fs/_ops.py @@ -3,6 +3,7 @@ """Core filesystem operations mixin for VikingFS.""" import asyncio +import uuid from dataclasses import replace from datetime import datetime, timezone from typing import Any, Dict, List, Optional, Union @@ -30,6 +31,7 @@ ) from openviking.utils.time_utils import format_iso8601, parse_iso_datetime from openviking_cli.exceptions import ( + ConflictError, FailedPreconditionError, InvalidArgumentError, NotFoundError, @@ -49,6 +51,15 @@ def _glob_match_uri(entry_uri: str, is_dir: Optional[bool]) -> str: return f"{entry_uri}/" +class TransferRollbackError(RuntimeError): + """A filesystem transfer failed and left a compensation failure to repair.""" + + def __init__(self, message: str, *, phase: str, residual_uri: str): + super().__init__(message) + self.phase = phase + self.residual_uri = residual_uri + + class _OpsMixin: """Core filesystem operations (read/write/mkdir/rm/mv/stat/glob/tree/ls/temp).""" @@ -298,6 +309,194 @@ async def remove_files( auto_pathlock=auto_pathlock, ) + async def cp( + self, + old_uri: str, + new_uri: str, + recursive: bool = False, + ctx: Optional[RequestContext] = None, + lease_ref: Dict[str, Any] | None = None, + ) -> Dict[str, Any]: + """Copy a file or directory together with its vector records.""" + await self._ensure_access(old_uri, ctx) + await self._ensure_access(new_uri, ctx, action=AclAction.WRITE) + old_scope = old_uri.rstrip("/") + new_scope = new_uri.rstrip("/") + if old_scope == new_scope: + raise InvalidArgumentError("cp source and target must be different") + if new_scope.startswith(old_scope + "/"): + raise InvalidArgumentError("cp target cannot be inside the source subtree") + + old_path = self._uri_to_path(old_uri, ctx=ctx) + new_path = self._uri_to_path(new_uri, ctx=ctx) + try: + stat = await self._async_agfs.stat(old_path) + except Exception as exc: + if is_not_found_error(exc): + raise FileNotFoundError(f"cp source not found: {old_uri}") from exc + mapped = map_exception(exc, resource=old_uri) + if mapped is not None: + raise mapped from exc + raise + is_dir = stat.get("isDir", False) if isinstance(stat, dict) else False + if is_dir and not recursive: + raise FailedPreconditionError( + f"Cannot copy directory without --recursive: {old_uri}", + details={"resource": old_uri, "expected_flag": "recursive"}, + ) + if not is_dir and new_uri.rstrip("/") != new_uri: + raise InvalidArgumentError( + f"cp destination for a file must include the target file name: {new_uri}" + ) + + await self._ensure_transfer_parent_directory(new_path, new_uri, operation="cp") + await self._ensure_transfer_target_missing(new_path, new_uri) + lock_kind = "tree" if is_dir else "exact" + lease = await self._async_agfs.pathlock_acquire_batch( + [ + {"path": old_path, "kind": lock_kind}, + {"path": new_path, "kind": "exact"}, + ], + owner_lease_ref=lease_ref, + ) + operation_id = uuid.uuid4().hex + try: + await self._ensure_transfer_target_missing(new_path, new_uri) + try: + files_created = await self._copy_agfs_entry( + old_path, + new_path, + is_dir=is_dir, + ctx=ctx, + lease_ref=lease, + ) + vector_result = await self._copy_vector_store_uris( + old_uri, + new_uri, + recursive=is_dir, + ctx=ctx, + ) + except Exception as transfer_error: + try: + await self._cleanup_transfer_target( + new_path, + is_dir=is_dir, + ctx=ctx, + lease_ref=lease, + ) + except Exception as rollback_error: + if not is_not_found_error(rollback_error): + raise TransferRollbackError( + f"cp failed and target cleanup failed for {new_uri}: {rollback_error}", + phase="target_cleanup", + residual_uri=new_uri, + ) from transfer_error + raise + result: Dict[str, Any] = { + "operation_id": operation_id, + "operation": "copy", + "from": old_uri, + "to": new_uri, + "recursive": is_dir, + "phase": "completed", + "files_created": files_created, + } + if vector_result is not None: + result["vectors"] = { + "scanned": vector_result.scanned, + "written": vector_result.written, + "deleted": vector_result.deleted, + "restored": vector_result.restored, + "batches": vector_result.batches, + } + logger.info( + "Filesystem transfer completed: operation_id=%s operation=copy " + "object_type=%s recursive=%s result=success", + operation_id, + "directory" if is_dir else "file", + is_dir, + ) + return result + finally: + await self._async_agfs.pathlock_release(lease) + + async def _ensure_transfer_target_missing(self, path: str, uri: str) -> None: + try: + await self._async_agfs.stat(path) + except Exception as exc: + if is_not_found_error(exc): + return + mapped = map_exception(exc, resource=uri) + if mapped is not None: + raise mapped from exc + raise + raise ConflictError(f"transfer target already exists: {uri}", resource=uri) + + async def _ensure_transfer_parent_directory( + self, path: str, uri: str, *, operation: str + ) -> None: + parent_path = path.rstrip("/").rsplit("/", 1)[0] or "/" + try: + parent_stat = await self._async_agfs.stat(parent_path) + except Exception as exc: + if is_not_found_error(exc): + raise FileNotFoundError(f"{operation} target parent not found: {uri}") from exc + mapped = map_exception(exc, resource=uri) + if mapped is not None: + raise mapped from exc + raise + if not isinstance(parent_stat, dict) or not parent_stat.get("isDir", False): + raise InvalidArgumentError(f"{operation} target parent is not a directory: {uri}") + + async def _copy_agfs_entry( + self, + old_path: str, + new_path: str, + *, + is_dir: bool, + ctx: Optional[RequestContext], + lease_ref: Dict[str, Any], + ) -> int: + if is_dir: + return await self._copy_directory_with_exact_locks( + old_path, + new_path, + ctx=ctx, + lease_ref=lease_ref, + ) + + await self._async_agfs.cp( + old_path, + new_path, + recursive=False, + fs_ctx=self._pathlock_fs_ctx(ctx, lease_ref), + ) + return 1 + + async def _cleanup_transfer_target( + self, + path: str, + *, + is_dir: bool, + ctx: Optional[RequestContext], + lease_ref: Dict[str, Any], + ) -> None: + cleanup_lease = lease_ref + if is_dir: + cleanup_lease = await self._async_agfs.pathlock_acquire_tree( + path, + owner_lease_ref=lease_ref, + ) + try: + await self._async_agfs.rm( + path, + recursive=is_dir, + fs_ctx=self._pathlock_fs_ctx(ctx, cleanup_lease), + ) + finally: + if cleanup_lease is not lease_ref: + await self._async_agfs.pathlock_release(cleanup_lease) + async def mv( self, old_uri: str, @@ -317,9 +516,16 @@ async def mv( await self._ensure_access(old_uri, guard_ctx, action=AclAction.MANAGE) await self._ensure_access(old_uri, ctx, action=AclAction.WRITE) await self._ensure_access(new_uri, ctx, action=AclAction.WRITE) + old_scope = old_uri.rstrip("/") + new_scope = new_uri.rstrip("/") + if old_scope == new_scope: + raise InvalidArgumentError("mv source and target must be different") + if new_scope.startswith(old_scope + "/"): + raise InvalidArgumentError("mv target cannot be inside the source subtree") old_path = self._uri_to_path(old_uri, ctx=ctx) new_path = self._uri_to_path(new_uri, ctx=ctx) - target_uri = self._path_to_uri(old_path, ctx=ctx) + source_vector_uri = self._path_to_uri(old_path, ctx=ctx) + target_uri = source_vector_uri new_acl_scope = acl_enabled and is_acl_uri(new_uri) # Verify source exists and determine type before locking. @@ -336,6 +542,8 @@ async def mv( if is_dir: await self._ensure_access(old_uri, ctx, action=AclAction.MANAGE) + await self._ensure_transfer_parent_directory(new_path, new_uri, operation="mv") + await self._ensure_transfer_target_missing(new_path, new_uri) if not is_dir: if new_uri.rstrip("/") != new_uri: @@ -375,6 +583,7 @@ async def mv( owner_lease_ref=lease_ref, ) + operation_id = uuid.uuid4().hex try: uris_to_move = ( await self._collect_uris( @@ -389,29 +598,52 @@ async def mv( uris_to_move.append(target_uri) if is_dir: await self._ensure_access_many(uris_to_move, ctx, action=AclAction.MANAGE) + await self._ensure_transfer_target_missing(new_path, new_uri) # Check if it's temp directory (files already encrypted) is_temp = old_uri.startswith("viking://temp/") # Copy source to destination. Source must stay intact until vector updates succeed. try: - await self._copy_for_mv( - old_uri=old_uri, - new_uri=new_uri, - old_path=old_path, - new_path=new_path, - is_dir=is_dir, - is_temp=is_temp, - ctx=ctx, - lease_ref=lease, + files_created = ( + await self._copy_for_mv( + old_uri=old_uri, + new_uri=new_uri, + old_path=old_path, + new_path=new_path, + is_dir=is_dir, + is_temp=is_temp, + ctx=ctx, + lease_ref=lease, + ) + or 0 ) - except Exception as e: - if "not found" in str(e).lower(): + except Exception as transfer_error: + try: + await self._cleanup_transfer_target( + new_path, + is_dir=is_dir, + ctx=ctx, + lease_ref=lease, + ) + except Exception as rollback_error: + if not is_not_found_error(rollback_error): + raise TransferRollbackError( + f"mv AGFS copy failed and target cleanup failed for " + f"{new_uri}: {rollback_error}", + phase="target_cleanup", + residual_uri=new_uri, + ) from transfer_error + if is_not_found_error(transfer_error): try: await self._delete_from_vector_store(uris_to_move, ctx=ctx) - except Exception: - # Orphan cleanup is best effort here; preserve the copy error. - pass + except Exception as vector_cleanup_error: + raise TransferRollbackError( + f"mv source disappeared and orphan vector cleanup failed for " + f"{old_uri}: {vector_cleanup_error}", + phase="source_vector_cleanup", + residual_uri=old_uri, + ) from transfer_error else: logger.info( f"[VikingFS] mv source not found, cleaned orphan index: {old_uri}" @@ -419,49 +651,103 @@ async def mv( raise # Update VectorDB URIs (on failure, clean up the copy) - vector_mappings: List[tuple[str, str]] = [] try: - vector_mappings = await self._update_vector_store_uris( - uris_to_move, old_uri, new_uri, ctx=ctx + vector_result = await self._update_vector_store_uris( + old_uri, + new_uri, + recursive=is_dir, + ctx=ctx, ) if acl_manager is not None and new_acl_scope: await acl_manager.refresh_context_subtree( new_uri, self._ctx_or_default(ctx), ) - except Exception: - if vector_mappings: - await self._restore_vector_store_uris(vector_mappings, ctx=ctx) + except Exception as transfer_error: try: - if is_dir: - cleanup_lease = await self._async_agfs.pathlock_acquire_tree( - new_path, - owner_lease_ref=lease, - ) - try: - await self._async_agfs.rm( - new_path, - recursive=True, - fs_ctx=self._pathlock_fs_ctx(ctx, cleanup_lease), - ) - finally: - await self._async_agfs.pathlock_release(cleanup_lease) - else: - await self._async_agfs.rm( - new_path, - fs_ctx=self._pathlock_fs_ctx(ctx, lease), - ) - except Exception: - pass + await self._cleanup_transfer_target( + new_path, + is_dir=is_dir, + ctx=ctx, + lease_ref=lease, + ) + except Exception as rollback_error: + raise TransferRollbackError( + f"mv vector transfer failed and target cleanup failed for " + f"{new_uri}: {rollback_error}", + phase="target_cleanup", + residual_uri=new_uri, + ) from transfer_error raise # Delete source - await self._async_agfs.rm( - old_path, - recursive=is_dir, - fs_ctx=self._pathlock_fs_ctx(ctx, lease), + try: + await self._async_agfs.rm( + old_path, + recursive=is_dir, + fs_ctx=self._pathlock_fs_ctx(ctx, lease), + ) + except Exception as delete_error: + try: + await self._cleanup_transfer_target( + old_path, + is_dir=is_dir, + ctx=ctx, + lease_ref=lease, + ) + await self._copy_agfs_entry( + new_path, + old_path, + is_dir=is_dir, + ctx=ctx, + lease_ref=lease, + ) + await self._update_vector_store_uris( + new_uri, + old_uri, + recursive=is_dir, + ctx=ctx, + ) + await self._cleanup_transfer_target( + new_path, + is_dir=is_dir, + ctx=ctx, + lease_ref=lease, + ) + except Exception as rollback_error: + raise TransferRollbackError( + f"mv source deletion failed and rollback was incomplete for " + f"{old_uri} -> {new_uri}: {rollback_error}", + phase="source_restore", + residual_uri=old_uri, + ) from delete_error + raise + result: Dict[str, Any] = { + "operation_id": operation_id, + "operation": "move", + "from": old_uri, + "to": new_uri, + "recursive": is_dir, + "phase": "completed", + "files_created": files_created, + "files_deleted": files_created, + } + if vector_result is not None: + result["vectors"] = { + "scanned": vector_result.scanned, + "written": vector_result.written, + "deleted": vector_result.deleted, + "restored": vector_result.restored, + "batches": vector_result.batches, + } + logger.info( + "Filesystem transfer completed: operation_id=%s operation=move " + "object_type=%s recursive=%s result=success", + operation_id, + "directory" if is_dir else "file", + is_dir, ) - return {} + return result finally: await self._async_agfs.pathlock_release(lease) @@ -475,38 +761,27 @@ async def _copy_for_mv( is_temp: bool, ctx: Optional[RequestContext] = None, lease_ref: Dict[str, Any] | None = None, - ) -> None: + ) -> int: """Copy source to destination for mv without deleting source.""" - if is_temp: - if is_dir: - await self._copy_temp_dir_with_exact_locks( - old_path, - new_path, - ctx=ctx, - lease_ref=lease_ref, - ) - else: - await self._async_agfs.cp( - old_path, - new_path, - recursive=False, - fs_ctx=self._pathlock_fs_ctx(ctx, lease_ref), - ) - return - - if is_dir: - await self._copy_dir_through_vikingfs(old_uri, new_uri, ctx=ctx, lease_ref=lease_ref) - else: - await self._copy_file_through_vikingfs(old_uri, new_uri, ctx=ctx, lease_ref=lease_ref) + del old_uri, new_uri, is_temp + if lease_ref is None: + raise ValueError("mv copy requires a pathlock lease") + return await self._copy_agfs_entry( + old_path, + new_path, + is_dir=is_dir, + ctx=ctx, + lease_ref=lease_ref, + ) - async def _copy_temp_dir_with_exact_locks( + async def _copy_directory_with_exact_locks( self, old_path: str, new_path: str, ctx: Optional[RequestContext], lease_ref: Dict[str, Any] | None, - ) -> None: - """Copy an encrypted temp directory while locking every destination entry. + ) -> int: + """Copy a directory through AGFS while locking every destination entry. Args: old_path: Source backend directory path. @@ -515,12 +790,13 @@ async def _copy_temp_dir_with_exact_locks( lease_ref: Exact lease covering the current destination directory. Returns: - None. + Number of created directories and files. """ if lease_ref is None: - raise ValueError("temp directory copy requires a pathlock lease") + raise ValueError("directory copy requires a pathlock lease") fs_ctx = self._pathlock_fs_ctx(ctx, lease_ref) await self._async_agfs.mkdir(new_path, fs_ctx=fs_ctx) + copied = 1 entries = await self._async_agfs.ls(old_path, fs_ctx=fs_ctx) for entry in entries: name = entry.get("name", "") @@ -534,7 +810,7 @@ async def _copy_temp_dir_with_exact_locks( ) try: if entry.get("isDir", False): - await self._copy_temp_dir_with_exact_locks( + copied += await self._copy_directory_with_exact_locks( old_child, new_child, ctx=ctx, @@ -547,8 +823,10 @@ async def _copy_temp_dir_with_exact_locks( recursive=False, fs_ctx=self._pathlock_fs_ctx(ctx, child_lease), ) + copied += 1 finally: await self._async_agfs.pathlock_release(child_lease) + return copied async def _copy_dir_through_vikingfs( self, @@ -960,15 +1238,17 @@ async def _tree_original( ) continue new_entry = dict(entry.get("extra", {})) - new_entry.update({ - "name": info["name"], - "size": info["size"], - "mode": info["mode"], - "modTime": info["modTime"], - "isDir": info["isDir"], - "rel_path": entry["rel_path"], - "uri": entry_uri, - }) + new_entry.update( + { + "name": info["name"], + "size": info["size"], + "mode": info["mode"], + "modTime": info["modTime"], + "isDir": info["isDir"], + "rel_path": entry["rel_path"], + "uri": entry_uri, + } + ) result.append(new_entry) return result @@ -1003,13 +1283,15 @@ async def _tree_agent( } ) continue - result.append({ - "uri": entry_uri, - "size": 0 if is_dir else info["size"], - "isDir": is_dir, - "modTime": format_iso8601(parse_iso_datetime(info["modTime"])), - "rel_path": entry["rel_path"], - }) + result.append( + { + "uri": entry_uri, + "size": 0 if is_dir else info["size"], + "isDir": is_dir, + "modTime": format_iso8601(parse_iso_datetime(info["modTime"])), + "rel_path": entry["rel_path"], + } + ) await self._batch_fetch_abstracts( [entry for entry in result if entry.get("access") != "denied"], diff --git a/openviking/storage/viking_fs/_vector.py b/openviking/storage/viking_fs/_vector.py index 79d77eed72..000d408e5a 100644 --- a/openviking/storage/viking_fs/_vector.py +++ b/openviking/storage/viking_fs/_vector.py @@ -2,7 +2,7 @@ # SPDX-License-Identifier: AGPL-3.0 """Vector store integration mixin for VikingFS.""" -from typing import TYPE_CHECKING, Any, List, Optional, Sequence +from typing import TYPE_CHECKING, Any, List, Optional from openviking.server.identity import RequestContext from openviking.storage.viking_fs._base import logger @@ -34,68 +34,46 @@ async def _delete_from_vector_store( logger.warning(f"[VikingFS] Failed to delete from vector store: {e}") raise - async def _update_vector_store_uris( + async def _copy_vector_store_uris( self, - uris: List[str], old_base: str, new_base: str, + *, + recursive: bool, ctx: Optional[RequestContext] = None, - ) -> List[tuple[str, str]]: - """Update URIs in vector store (when moving files). - - Preserves vector data and updates URI-derived identifiers without regenerating embeddings. - """ + ) -> Any: + """Copy a complete vector URI scope while preserving the source.""" vector_store = self._get_vector_store() if not vector_store: - return [] - - real_ctx = self._ctx_or_default(ctx) - mappings: List[tuple[str, str]] = [] - - try: - for uri in uris: - new_uri = new_base + uri[len(old_base) :] - updated = await vector_store.update_uri_mapping( - ctx=real_ctx, - uri=uri, - new_uri=new_uri, - ) - if updated: - mappings.append((uri, new_uri)) - logger.debug(f"[VikingFS] Updated URI: {uri} -> {new_uri}") - except Exception: - await self._restore_vector_store_uris(mappings, ctx=real_ctx) - raise - return mappings + return None + return await vector_store.copy_uri_mapping( + ctx=self._ctx_or_default(ctx), + source_uri=old_base, + target_uri=new_base, + recursive=recursive, + ) - async def _restore_vector_store_uris( + async def _update_vector_store_uris( self, - mappings: Sequence[tuple[str, str]], + old_base: str, + new_base: str, + *, + recursive: bool, ctx: Optional[RequestContext] = None, - ) -> None: + ) -> Any: + """Strictly move a complete vector URI scope. + + Preserves vector data and updates URI-derived identifiers without regenerating embeddings. + """ vector_store = self._get_vector_store() if not vector_store: - return - real_ctx = self._ctx_or_default(ctx) - for old_uri, new_uri in sorted(mappings, key=lambda item: item[0].count("/")): - try: - restored = await vector_store.update_uri_mapping( - ctx=real_ctx, - uri=new_uri, - new_uri=old_uri, - ) - if not restored: - logger.warning( - "[VikingFS] Vector URI rollback found no records: %s -> %s", - new_uri, - old_uri, - ) - except Exception: - logger.exception( - "[VikingFS] Failed to roll back vector URI: %s -> %s", - new_uri, - old_uri, - ) + return None + return await vector_store.update_uri_mapping( + ctx=self._ctx_or_default(ctx), + source_uri=old_base, + target_uri=new_base, + recursive=recursive, + ) def _get_vector_store(self) -> Optional["VikingVectorIndexBackend"]: """Get vector store instance.""" diff --git a/tests/pyagfs/test_async_client_fs_ctx.py b/tests/pyagfs/test_async_client_fs_ctx.py index 92fb0faeb1..e5765ba51e 100644 --- a/tests/pyagfs/test_async_client_fs_ctx.py +++ b/tests/pyagfs/test_async_client_fs_ctx.py @@ -90,3 +90,56 @@ def stat(self, path: str) -> dict[str, Any]: with pytest.raises(ValueError, match="cross-account"): cp(_CpClient(), "/local/a/data/file.txt", "/mem/data/file.txt") + + +@pytest.mark.asyncio +async def test_async_cp_streams_when_same_mount_fast_path_is_unavailable() -> None: + class _FallbackClient: + def __init__(self) -> None: + self.cat_calls: list[tuple[str, bool, dict[str, str] | None]] = [] + self.written_chunks: list[bytes] = [] + + def stat(self, path: str, *, ctx: dict[str, str] | None = None) -> dict[str, Any]: + del ctx + return {"isDir": path.endswith("/data")} + + def copy_within_mount( + self, source: str, target: str, *, ctx: dict[str, str] | None = None + ) -> dict[str, bool]: + del source, target, ctx + return {"performed": False} + + def cat( + self, + path: str, + *, + stream: bool = False, + ctx: dict[str, str] | None = None, + ): + self.cat_calls.append((path, stream, ctx)) + if not stream: + raise AssertionError("fallback copy loaded the complete file") + return iter((b"first", b"second")) + + def write(self, path: str, data, *, ctx: dict[str, str] | None = None) -> str: + del path, ctx + self.written_chunks = list(data) + return "written" + + client = _FallbackClient() + agfs = AsyncAGFSClient(client) + + await agfs.cp( + "/local/acct/data/source.bin", + "/local/acct/data/target.bin", + fs_ctx={"account_id": "acct", "lease_ref": "lease"}, + ) + + assert client.cat_calls == [ + ( + "/local/acct/data/source.bin", + True, + {"account_id": "acct", "lease_ref": "lease"}, + ) + ] + assert client.written_chunks == [b"first", b"second"] diff --git a/tests/storage/test_semantic_processor_mv_vector_store.py b/tests/storage/test_semantic_processor_mv_vector_store.py index d0f950918c..5c5f1a0f59 100644 --- a/tests/storage/test_semantic_processor_mv_vector_store.py +++ b/tests/storage/test_semantic_processor_mv_vector_store.py @@ -13,8 +13,21 @@ async def test_mv_preserves_canonical_user_uris_for_vector_update(monkeypatch): fs = VikingFS.__new__(VikingFS) fs.acl_manager = None fs._async_agfs = AsyncMock() - fs._async_agfs.stat.return_value = {"isDir": False} - fs._async_agfs.pathlock_acquire_batch.return_value = {"lease_ref": "lease-1"} + + async def stat(path): + if path.endswith("/peers/vaka/memories/profile.md"): + return {"isDir": False} + if path.endswith("/user/default/memories"): + return {"isDir": True} + raise FileNotFoundError(path) + + fs._async_agfs.stat.side_effect = stat + fs._async_agfs.pathlock_acquire_batch.return_value = { + "lease_ref": "operation-ref", + "owner_id": "operation-owner", + "ownership_ref": "operation-ownership", + "owned": True, + } fs._collect_uris = AsyncMock(return_value=[]) fs._copy_for_mv = AsyncMock() fs._update_vector_store_uris = AsyncMock() @@ -26,42 +39,25 @@ async def test_mv_preserves_canonical_user_uris_for_vector_update(monkeypatch): ) fs._update_vector_store_uris.assert_awaited_once_with( - ["viking://user/default/peers/vaka/memories/profile.md"], "viking://user/default/peers/vaka/memories/profile.md", "viking://user/default/memories/profile.md", + recursive=False, ctx=ctx, ) @pytest.mark.asyncio -async def test_vector_uri_batch_rolls_back_completed_updates(): +async def test_update_vector_store_uris_propagates_update_failure(): ctx = RequestContext(user=UserIdentifier("acc", "default"), role=Role.ROOT) fs = VikingFS.__new__(VikingFS) fs.vector_store = AsyncMock() - fs.vector_store.update_uri_mapping.side_effect = [True, RuntimeError("failed"), True] + fs.vector_store.update_uri_mapping.side_effect = RuntimeError("vector unavailable") - with pytest.raises(RuntimeError, match="failed"): + with pytest.raises(RuntimeError, match="vector unavailable"): await fs._update_vector_store_uris( - ["viking://resources/old/a.md", "viking://resources/old/b.md"], - "viking://resources/old", - "viking://resources/new", + "viking://user/default/source.md", + "viking://user/default/target.md", + recursive=False, ctx=ctx, ) - - assert fs.vector_store.update_uri_mapping.await_args_list == [ - call( - ctx=ctx, - uri="viking://resources/old/a.md", - new_uri="viking://resources/new/a.md", - ), - call( - ctx=ctx, - uri="viking://resources/old/b.md", - new_uri="viking://resources/new/b.md", - ), - call( - ctx=ctx, - uri="viking://resources/new/a.md", - new_uri="viking://resources/old/a.md", - ), - ] + fs.vector_store.update_uri_mapping.assert_awaited_once() diff --git a/tests/storage/test_viking_fs_cp.py b/tests/storage/test_viking_fs_cp.py new file mode 100644 index 0000000000..b11d944cb1 --- /dev/null +++ b/tests/storage/test_viking_fs_cp.py @@ -0,0 +1,540 @@ +# Copyright (c) 2026 Beijing Volcano Engine Technology Co., Ltd. +# SPDX-License-Identifier: AGPL-3.0 + +from __future__ import annotations + +from types import SimpleNamespace +from unittest.mock import AsyncMock + +import pytest + +from openviking.server.identity import RequestContext, Role +from openviking.storage.viking_fs import VikingFS +from openviking.storage.viking_fs._ops import TransferRollbackError +from openviking_cli.exceptions import ConflictError, FailedPreconditionError, InvalidArgumentError +from openviking_cli.session.user_id import UserIdentifier + + +def _ctx() -> RequestContext: + return RequestContext(user=UserIdentifier("acct", "alice"), role=Role.ROOT) + + +class _CopyAGFS: + def __init__( + self, + *, + source_is_dir: bool = False, + target_exists: bool = False, + fail_copy: bool = False, + parent_exists: bool = True, + ): + self.source_is_dir = source_is_dir + self.target_exists = target_exists + self.fail_copy = fail_copy + self.parent_exists = parent_exists + self.events: list[tuple] = [] + self.operation_lease = { + "lease_ref": "copy-operation", + "owner_id": "copy-owner", + "ownership_ref": "copy-ownership", + "owned": True, + } + + async def stat(self, path, fs_ctx=None): + self.events.append(("stat", path, fs_ctx)) + if path.endswith("/source") or path.endswith("/source.md"): + return {"isDir": self.source_is_dir} + if path == "/local/acct/resources" and self.parent_exists: + return {"isDir": True} + if (path.endswith("/target") or path.endswith("/target.md")) and self.target_exists: + return {"isDir": self.source_is_dir} + raise FileNotFoundError(path) + + async def pathlock_acquire_batch(self, requests, timeout_secs=0.0, owner_lease_ref=None): + del timeout_secs + self.events.append(("acquire-batch", requests, owner_lease_ref)) + return self.operation_lease + + async def pathlock_release(self, lease): + self.events.append(("release", lease)) + + async def cp(self, source, target, recursive=False, fs_ctx=None): + self.events.append(("cp", source, target, recursive, fs_ctx)) + self.target_exists = True + if self.fail_copy: + raise RuntimeError("injected AGFS copy failure") + + async def rm(self, path, recursive=False, fs_ctx=None): + self.events.append(("rm", path, recursive, fs_ctx)) + if path.endswith("/target") or path.endswith("/target.md"): + self.target_exists = False + + +class _DirectoryCopyAGFS(_CopyAGFS): + def __init__(self): + super().__init__(source_is_dir=True) + self.directories = { + "/local/acct/resources", + "/local/acct/resources/source", + "/local/acct/resources/source/empty", + } + self.files = { + "/local/acct/resources/source/.hidden": b"hidden", + "/local/acct/resources/source/data.bin": b"\x00\xff", + } + self.exact_leases: list[tuple[str, dict]] = [] + + async def stat(self, path, fs_ctx=None): + self.events.append(("stat", path, fs_ctx)) + if path in self.directories: + return {"isDir": True} + if path in self.files: + return {"isDir": False} + raise FileNotFoundError(path) + + async def mkdir(self, path, fs_ctx=None): + self.events.append(("mkdir", path, fs_ctx)) + self.directories.add(path) + + async def ls(self, path, fs_ctx=None): + self.events.append(("ls", path, fs_ctx)) + if path.endswith("/source"): + return [ + {"name": "empty", "isDir": True}, + {"name": ".hidden", "isDir": False}, + {"name": "data.bin", "isDir": False}, + ] + if path.endswith("/source/empty"): + return [] + raise FileNotFoundError(path) + + async def pathlock_acquire_exact(self, path, timeout_secs=0.0, owner_lease_ref=None): + del timeout_secs + lease = { + "lease_ref": f"child-{len(self.exact_leases)}", + "owner_id": owner_lease_ref["owner_id"], + "ownership_ref": f"child-owner-{len(self.exact_leases)}", + "owned": True, + } + self.exact_leases.append((path, lease)) + self.events.append(("acquire-exact", path, owner_lease_ref)) + return lease + + async def cp(self, source, target, recursive=False, fs_ctx=None): + self.events.append(("cp", source, target, recursive, fs_ctx)) + self.files[target] = self.files[source] + + +class _MoveRollbackAGFS(_CopyAGFS): + def __init__(self): + super().__init__() + self.paths = {"/local/acct/resources/source.md"} + self.fail_source_delete = True + + async def stat(self, path, fs_ctx=None): + self.events.append(("stat", path, fs_ctx)) + if path == "/local/acct/resources": + return {"isDir": True} + if path in self.paths: + return {"isDir": False} + raise FileNotFoundError(path) + + async def cp(self, source, target, recursive=False, fs_ctx=None): + self.events.append(("cp", source, target, recursive, fs_ctx)) + assert source in self.paths + self.paths.add(target) + + async def rm(self, path, recursive=False, fs_ctx=None): + self.events.append(("rm", path, recursive, fs_ctx)) + if path.endswith("/source.md") and self.fail_source_delete: + self.fail_source_delete = False + self.paths.discard(path) + raise RuntimeError("injected source delete failure") + self.paths.discard(path) + + +def _viking_fs(monkeypatch, agfs: _CopyAGFS) -> VikingFS: + fs = VikingFS.__new__(VikingFS) + fs._async_agfs = agfs + fs.vector_store = None + monkeypatch.setattr(fs, "_ensure_access", lambda *_args, **_kwargs: None) + monkeypatch.setattr(fs, "_ensure_mutable_access", lambda *_args, **_kwargs: None) + monkeypatch.setattr( + fs, + "_uri_to_path", + lambda uri, **_kwargs: ( + "/local/acct/resources/source" + (".md" if uri.endswith(".md") else "") + if "/source" in uri + else "/local/acct/resources/target" + (".md" if uri.endswith(".md") else "") + ), + ) + monkeypatch.setattr( + fs, + "_path_to_uri", + lambda path, **_kwargs: ( + "viking://resources/source.md" + if path.endswith("/source.md") + else "viking://resources/target.md" + ), + ) + return fs + + +@pytest.mark.asyncio +async def test_cp_file_uses_exact_locks_agfs_copy_then_vector_copy(monkeypatch): + agfs = _CopyAGFS() + fs = _viking_fs(monkeypatch, agfs) + vector_copy = AsyncMock() + monkeypatch.setattr(fs, "_copy_vector_store_uris", vector_copy, raising=False) + + await fs.cp( + "viking://resources/source.md", + "viking://resources/target.md", + ctx=_ctx(), + ) + + acquire = next(event for event in agfs.events if event[0] == "acquire-batch") + assert acquire[1] == [ + {"path": "/local/acct/resources/source.md", "kind": "exact"}, + {"path": "/local/acct/resources/target.md", "kind": "exact"}, + ] + copy_event = next(event for event in agfs.events if event[0] == "cp") + assert copy_event[1:4] == ( + "/local/acct/resources/source.md", + "/local/acct/resources/target.md", + False, + ) + assert copy_event[4]["lease_ref"] == "copy-operation" + vector_copy.assert_awaited_once_with( + "viking://resources/source.md", + "viking://resources/target.md", + recursive=False, + ctx=_ctx(), + ) + assert agfs.events[-1] == ("release", agfs.operation_lease) + + +@pytest.mark.asyncio +async def test_cp_returns_diagnostic_transfer_summary(monkeypatch): + agfs = _CopyAGFS() + fs = _viking_fs(monkeypatch, agfs) + monkeypatch.setattr( + fs, + "_copy_vector_store_uris", + AsyncMock( + return_value=SimpleNamespace(scanned=2, written=2, deleted=0, restored=0, batches=1) + ), + raising=False, + ) + + result = await fs.cp( + "viking://resources/source.md", + "viking://resources/target.md", + ctx=_ctx(), + ) + + assert len(result["operation_id"]) == 32 + assert result["operation"] == "copy" + assert result["phase"] == "completed" + assert result["files_created"] == 1 + assert result["vectors"] == { + "scanned": 2, + "written": 2, + "deleted": 0, + "restored": 0, + "batches": 1, + } + + +@pytest.mark.asyncio +async def test_cp_directory_requires_recursive_before_locking(monkeypatch): + agfs = _CopyAGFS(source_is_dir=True) + fs = _viking_fs(monkeypatch, agfs) + + with pytest.raises(FailedPreconditionError, match="recursive"): + await fs.cp( + "viking://resources/source", + "viking://resources/target", + recursive=False, + ctx=_ctx(), + ) + + assert not any(event[0] == "acquire-batch" for event in agfs.events) + + +@pytest.mark.asyncio +async def test_cp_directory_preserves_empty_hidden_and_binary_entries(monkeypatch): + agfs = _DirectoryCopyAGFS() + fs = _viking_fs(monkeypatch, agfs) + vector_copy = AsyncMock() + monkeypatch.setattr(fs, "_copy_vector_store_uris", vector_copy, raising=False) + + await fs.cp( + "viking://resources/source", + "viking://resources/target", + recursive=True, + ctx=_ctx(), + ) + + acquire = next(event for event in agfs.events if event[0] == "acquire-batch") + assert acquire[1] == [ + {"path": "/local/acct/resources/source", "kind": "tree"}, + {"path": "/local/acct/resources/target", "kind": "exact"}, + ] + assert "/local/acct/resources/target/empty" in agfs.directories + assert agfs.files["/local/acct/resources/target/.hidden"] == b"hidden" + assert agfs.files["/local/acct/resources/target/data.bin"] == b"\x00\xff" + assert {path for path, _lease in agfs.exact_leases} == { + "/local/acct/resources/target/empty", + "/local/acct/resources/target/.hidden", + "/local/acct/resources/target/data.bin", + } + vector_copy.assert_awaited_once_with( + "viking://resources/source", + "viking://resources/target", + recursive=True, + ctx=_ctx(), + ) + + +@pytest.mark.asyncio +async def test_cp_rejects_existing_target_without_copying(monkeypatch): + agfs = _CopyAGFS(target_exists=True) + fs = _viking_fs(monkeypatch, agfs) + monkeypatch.setattr(fs, "_copy_vector_store_uris", AsyncMock(), raising=False) + + with pytest.raises(ConflictError, match="target already exists"): + await fs.cp( + "viking://resources/source.md", + "viking://resources/target.md", + ctx=_ctx(), + ) + + assert not any(event[0] == "cp" for event in agfs.events) + + +@pytest.mark.asyncio +async def test_cp_rejects_missing_target_parent_before_locking(monkeypatch): + agfs = _CopyAGFS(parent_exists=False) + fs = _viking_fs(monkeypatch, agfs) + monkeypatch.setattr(fs, "_copy_vector_store_uris", AsyncMock(), raising=False) + + with pytest.raises(FileNotFoundError, match="target parent not found"): + await fs.cp( + "viking://resources/source.md", + "viking://resources/target.md", + ctx=_ctx(), + ) + + assert not any(event[0] == "acquire-batch" for event in agfs.events) + + +@pytest.mark.asyncio +async def test_mv_rejects_missing_target_parent_before_locking(monkeypatch): + agfs = _CopyAGFS(parent_exists=False) + fs = _viking_fs(monkeypatch, agfs) + monkeypatch.setattr(fs, "_ensure_delete_access", lambda *_args, **_kwargs: None) + monkeypatch.setattr(fs, "_update_vector_store_uris", AsyncMock()) + + with pytest.raises(FileNotFoundError, match="target parent not found"): + await fs.mv( + "viking://resources/source.md", + "viking://resources/target.md", + ctx=_ctx(), + ) + + assert not any(event[0] == "acquire-batch" for event in agfs.events) + + +@pytest.mark.asyncio +async def test_mv_removes_partial_target_when_agfs_copy_fails(monkeypatch): + agfs = _CopyAGFS(fail_copy=True) + fs = _viking_fs(monkeypatch, agfs) + monkeypatch.setattr(fs, "_ensure_delete_access", lambda *_args, **_kwargs: None) + vector_move = AsyncMock() + monkeypatch.setattr(fs, "_update_vector_store_uris", vector_move) + + with pytest.raises(RuntimeError, match="injected AGFS copy failure"): + await fs.mv( + "viking://resources/source.md", + "viking://resources/target.md", + ctx=_ctx(), + ) + + assert any( + event[0:3] == ("rm", "/local/acct/resources/target.md", False) for event in agfs.events + ) + assert not agfs.target_exists + vector_move.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_cp_removes_target_file_when_vector_copy_fails(monkeypatch): + agfs = _CopyAGFS() + fs = _viking_fs(monkeypatch, agfs) + monkeypatch.setattr( + fs, + "_copy_vector_store_uris", + AsyncMock(side_effect=RuntimeError("vector copy failed")), + raising=False, + ) + + with pytest.raises(RuntimeError, match="vector copy failed"): + await fs.cp( + "viking://resources/source.md", + "viking://resources/target.md", + ctx=_ctx(), + ) + + assert any( + event[0:3] == ("rm", "/local/acct/resources/target.md", False) for event in agfs.events + ) + assert not agfs.target_exists + + +@pytest.mark.asyncio +async def test_cp_removes_partial_target_when_agfs_copy_fails(monkeypatch): + agfs = _CopyAGFS(fail_copy=True) + fs = _viking_fs(monkeypatch, agfs) + vector_copy = AsyncMock() + monkeypatch.setattr(fs, "_copy_vector_store_uris", vector_copy, raising=False) + + with pytest.raises(RuntimeError, match="injected AGFS copy failure"): + await fs.cp( + "viking://resources/source.md", + "viking://resources/target.md", + ctx=_ctx(), + ) + + assert any( + event[0:3] == ("rm", "/local/acct/resources/target.md", False) for event in agfs.events + ) + assert not agfs.target_exists + vector_copy.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_mv_restores_source_and_removes_target_when_source_delete_fails(monkeypatch): + agfs = _MoveRollbackAGFS() + fs = _viking_fs(monkeypatch, agfs) + monkeypatch.setattr(fs, "_ensure_delete_access", lambda *_args, **_kwargs: None) + vector_move = AsyncMock() + monkeypatch.setattr(fs, "_update_vector_store_uris", vector_move) + + with pytest.raises(RuntimeError, match="injected source delete failure"): + await fs.mv( + "viking://resources/source.md", + "viking://resources/target.md", + ctx=_ctx(), + ) + + assert agfs.paths == {"/local/acct/resources/source.md"} + assert vector_move.await_args_list[0].args == ( + "viking://resources/source.md", + "viking://resources/target.md", + ) + assert vector_move.await_args_list[1].args == ( + "viking://resources/target.md", + "viking://resources/source.md", + ) + + +@pytest.mark.asyncio +async def test_mv_returns_diagnostic_transfer_summary(monkeypatch): + agfs = _CopyAGFS() + fs = _viking_fs(monkeypatch, agfs) + monkeypatch.setattr(fs, "_ensure_delete_access", lambda *_args, **_kwargs: None) + monkeypatch.setattr( + fs, + "_update_vector_store_uris", + AsyncMock( + return_value=SimpleNamespace(scanned=2, written=2, deleted=2, restored=0, batches=1) + ), + ) + + result = await fs.mv( + "viking://resources/source.md", + "viking://resources/target.md", + ctx=_ctx(), + ) + + assert len(result["operation_id"]) == 32 + assert result["operation"] == "move" + assert result["phase"] == "completed" + assert result["files_created"] == 1 + assert result["files_deleted"] == 1 + assert result["vectors"]["deleted"] == 2 + + +@pytest.mark.asyncio +async def test_mv_reports_target_residual_when_vector_failure_cleanup_fails(monkeypatch): + agfs = _CopyAGFS() + fs = _viking_fs(monkeypatch, agfs) + monkeypatch.setattr(fs, "_ensure_delete_access", lambda *_args, **_kwargs: None) + monkeypatch.setattr( + fs, + "_update_vector_store_uris", + AsyncMock(side_effect=RuntimeError("vector move failed")), + ) + monkeypatch.setattr( + fs, + "_cleanup_transfer_target", + AsyncMock(side_effect=RuntimeError("target cleanup failed")), + ) + + with pytest.raises(TransferRollbackError) as exc_info: + await fs.mv( + "viking://resources/source.md", + "viking://resources/target.md", + ctx=_ctx(), + ) + + assert exc_info.value.phase == "target_cleanup" + assert exc_info.value.residual_uri == "viking://resources/target.md" + assert isinstance(exc_info.value.__cause__, RuntimeError) + assert str(exc_info.value.__cause__) == "vector move failed" + + +@pytest.mark.asyncio +async def test_mv_reports_source_residual_when_source_restore_fails(monkeypatch): + agfs = _MoveRollbackAGFS() + fs = _viking_fs(monkeypatch, agfs) + monkeypatch.setattr(fs, "_ensure_delete_access", lambda *_args, **_kwargs: None) + monkeypatch.setattr(fs, "_update_vector_store_uris", AsyncMock()) + monkeypatch.setattr( + fs, + "_copy_agfs_entry", + AsyncMock(side_effect=[1, RuntimeError("source restore copy failed")]), + ) + + with pytest.raises(TransferRollbackError) as exc_info: + await fs.mv( + "viking://resources/source.md", + "viking://resources/target.md", + ctx=_ctx(), + ) + + assert exc_info.value.phase == "source_restore" + assert exc_info.value.residual_uri == "viking://resources/source.md" + assert isinstance(exc_info.value.__cause__, RuntimeError) + assert str(exc_info.value.__cause__) == "injected source delete failure" + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + ("source", "target", "source_is_dir"), + [ + ("viking://resources/source.md", "viking://resources/source.md", False), + ("viking://resources/source", "viking://resources/source/child", True), + ], +) +async def test_mv_rejects_same_path_and_source_subtree(monkeypatch, source, target, source_is_dir): + agfs = _CopyAGFS(source_is_dir=source_is_dir) + fs = _viking_fs(monkeypatch, agfs) + monkeypatch.setattr(fs, "_ensure_delete_access", lambda *_args, **_kwargs: None) + monkeypatch.setattr(fs, "_update_vector_store_uris", AsyncMock()) + + with pytest.raises(InvalidArgumentError): + await fs.mv(source, target, ctx=_ctx()) + + assert not any(event[0] == "acquire-batch" for event in agfs.events) diff --git a/tests/storage/test_viking_fs_write_locking.py b/tests/storage/test_viking_fs_write_locking.py index 2474e619b0..8dda8212a2 100644 --- a/tests/storage/test_viking_fs_write_locking.py +++ b/tests/storage/test_viking_fs_write_locking.py @@ -70,6 +70,8 @@ async def stat(self, path): """Return the configured source kind and a missing destination.""" if path.endswith("/source.md") or path.endswith("/source"): return {"isDir": self.source_is_dir} + if path.endswith("/resources"): + return {"isDir": True} raise FileNotFoundError(path) async def pathlock_acquire_batch( From 5a229ffb475698065bc748ce226b2f52d8596472 Mon Sep 17 00:00:00 2001 From: chenpengfei Date: Thu, 20 Aug 2026 13:08:30 +0800 Subject: [PATCH 04/20] feat: coordinate filesystem copy service --- openviking/service/fs_service.py | 102 ++++++++++++++++++++ tests/service/test_fs_service.py | 156 +++++++++++++++++++++++++++++++ 2 files changed, 258 insertions(+) diff --git a/openviking/service/fs_service.py b/openviking/service/fs_service.py index f2a6ce374b..21f745b5b1 100644 --- a/openviking/service/fs_service.py +++ b/openviking/service/fs_service.py @@ -466,6 +466,59 @@ async def _enqueue_delete_refresh( raise return decision.action + async def _enqueue_copy_refresh( + self, + *, + root_uri: str, + copied_uri: str, + context_type: str, + ctx: RequestContext, + ) -> str: + """Queue a parent-only semantic refresh after a committed copy.""" + await mark_semantic_sidecars_pending( + viking_fs=self._viking_fs, + dir_uri=root_uri, + changed_entries=1, + ctx=ctx, + ) + try: + queue_manager = get_queue_manager() + except RuntimeError as exc: + logger.warning("QueueManager not available, skipping copy refresh: %s", exc) + return "skipped" + + semantic_queue = queue_manager.get_queue(queue_manager.SEMANTIC, allow_create=True) + telemetry_id = get_current_telemetry().telemetry_id + msg = SemanticMsg( + uri=root_uri, + context_type=context_type, + recursive=False, + account_id=ctx.account_id, + user_id=ctx.user.user_id, + peer_id=ctx.user.user_id, + role=str(ctx.role), + skip_vectorization=True, + telemetry_id=telemetry_id, + coalesce_key=build_semantic_coalesce_key( + context_type=context_type, + uri=root_uri, + account_id=ctx.account_id, + user_id=ctx.user.user_id, + peer_id=ctx.user.user_id, + ), + changes={"added": [copied_uri]}, + generation_trigger="content_copy", + ) + if telemetry_id: + get_request_wait_tracker().register_semantic_root(telemetry_id, msg.id) + try: + await semantic_queue.enqueue(msg) + except Exception as exc: + if telemetry_id: + get_request_wait_tracker().mark_semantic_failed(telemetry_id, msg.id, str(exc)) + raise + return "queued" + async def _wait_for_refresh(self, *, timeout: Optional[float]) -> Dict[str, Any]: telemetry_id = get_current_telemetry().telemetry_id if telemetry_id: @@ -481,6 +534,55 @@ async def _wait_for_refresh(self, *, timeout: Optional[float]) -> Dict[str, Any] except TimeoutError as exc: raise DeadlineExceededError("queue processing", timeout) from exc + async def cp( + self, + from_uri: str, + to_uri: str, + recursive: bool, + ctx: RequestContext, + ) -> Dict[str, Any]: + """Copy a resource and queue derived parent semantics after commit.""" + from_uri = validate_viking_uri(from_uri, field_name="from_uri") + to_uri = validate_viking_uri(to_uri, field_name="to_uri") + viking_fs = self._ensure_initialized() + async with self._uri_mutation_coordinator.mutation( + ctx.account_id, + [from_uri, to_uri], + ): + transfer_result = await viking_fs.cp( + from_uri, + to_uri, + recursive=recursive, + ctx=ctx, + ) + + result = dict(transfer_result or {}) + result.setdefault("from", from_uri) + result.setdefault("to", to_uri) + result.setdefault("recursive", recursive) + context_type = context_type_for_uri(to_uri) + refresh_parent_uri = self._semantic_refresh_parent_uri(to_uri, context_type) + if not refresh_parent_uri: + return result + + result["semantic_root_uri"] = refresh_parent_uri + try: + result["semantic_status"] = await self._enqueue_copy_refresh( + root_uri=refresh_parent_uri, + copied_uri=to_uri, + context_type=context_type, + ctx=ctx, + ) + except Exception as exc: + logger.warning( + "Copy committed but parent semantic refresh failed for %s: %s", + to_uri, + exc, + ) + result["semantic_status"] = "failed" + result["semantic_error"] = str(exc) + return result + async def mv(self, from_uri: str, to_uri: str, ctx: RequestContext) -> None: """Move resource.""" viking_fs = self._ensure_initialized() diff --git a/tests/service/test_fs_service.py b/tests/service/test_fs_service.py index afe848db9f..68e37b7f1d 100644 --- a/tests/service/test_fs_service.py +++ b/tests/service/test_fs_service.py @@ -2,6 +2,7 @@ # SPDX-License-Identifier: AGPL-3.0 """Tests for file-system service coordination behavior.""" +from contextlib import asynccontextmanager from types import SimpleNamespace from unittest.mock import AsyncMock @@ -16,6 +17,7 @@ class _FakeVikingFS: def __init__(self, *, rm_error=None, events=None): self.rm_calls = [] self.mv_calls = [] + self.cp_calls = [] self.rm_error = rm_error self.events = events @@ -30,6 +32,42 @@ async def mv(self, from_uri, to_uri, ctx=None): if self.events is not None: self.events.append(("mv", from_uri, to_uri)) + async def cp(self, from_uri, to_uri, recursive=False, ctx=None): + self.cp_calls.append( + { + "from_uri": from_uri, + "to_uri": to_uri, + "recursive": recursive, + "ctx": ctx, + } + ) + if self.events is not None: + self.events.append(("cp", from_uri, to_uri)) + return { + "operation_id": "copy-1", + "operation": "copy", + "from": from_uri, + "to": to_uri, + "recursive": recursive, + } + + +class _FakeMutationCoordinator: + def __init__(self, events=None): + self.calls = [] + self.events = events + + @asynccontextmanager + async def mutation(self, account_id, uris): + self.calls.append({"account_id": account_id, "uris": list(uris)}) + if self.events is not None: + self.events.append(("mutation-enter", *uris)) + try: + yield + finally: + if self.events is not None: + self.events.append(("mutation-exit", *uris)) + class _FakeWatchManager: def __init__(self, *, events=None): @@ -405,6 +443,124 @@ async def test_resource_mv_without_watch_scheduler_moves_resource_directly(reque ] +@pytest.mark.asyncio +async def test_resource_cp_coordinates_mutation_without_copying_watch_tasks(request_context): + source = "viking://resources/codeask/wiki" + target = "viking://resources/archive/wiki" + events = [] + viking_fs = _FakeVikingFS(events=events) + coordinator = _FakeMutationCoordinator(events=events) + watch_manager = _FakeWatchManager(events=events) + service = FSService( + viking_fs=viking_fs, + watch_scheduler=_FakeWatchScheduler(watch_manager), + uri_mutation_coordinator=coordinator, + ) + + async def enqueue_refresh(**kwargs): + events.append(("refresh", kwargs["root_uri"])) + assert kwargs == { + "root_uri": "viking://resources/archive", + "copied_uri": target, + "context_type": "resource", + "ctx": request_context, + } + return "queued" + + service._enqueue_copy_refresh = enqueue_refresh + + result = await service.cp(source, target, recursive=True, ctx=request_context) + + assert coordinator.calls == [{"account_id": "default", "uris": [source, target]}] + assert viking_fs.cp_calls == [ + { + "from_uri": source, + "to_uri": target, + "recursive": True, + "ctx": request_context, + } + ] + assert [event[0] for event in events] == [ + "mutation-enter", + "cp", + "mutation-exit", + "refresh", + ] + assert watch_manager.validate_calls == [] + assert watch_manager.rewrite_calls == [] + assert result["from"] == source + assert result["to"] == target + assert result["semantic_root_uri"] == "viking://resources/archive" + assert result["semantic_status"] == "queued" + + +@pytest.mark.asyncio +async def test_resource_cp_refresh_failure_does_not_roll_back_copy(request_context): + source = "viking://resources/source.md" + target = "viking://resources/archive/copied.md" + viking_fs = _FakeVikingFS() + service = FSService(viking_fs=viking_fs) + service._enqueue_copy_refresh = AsyncMock(side_effect=RuntimeError("queue unavailable")) + + result = await service.cp(source, target, recursive=False, ctx=request_context) + + assert len(viking_fs.cp_calls) == 1 + assert viking_fs.mv_calls == [] + assert result["semantic_status"] == "failed" + assert result["semantic_error"] == "queue unavailable" + + +@pytest.mark.asyncio +async def test_non_resource_cp_skips_parent_semantic_refresh(request_context): + viking_fs = _FakeVikingFS() + service = FSService(viking_fs=viking_fs) + service._enqueue_copy_refresh = AsyncMock() + + result = await service.cp( + "viking://user/ryoma/memories/source.md", + "viking://user/ryoma/memories/copied.md", + recursive=False, + ctx=request_context, + ) + + service._enqueue_copy_refresh.assert_not_awaited() + assert "semantic_root_uri" not in result + + +@pytest.mark.asyncio +async def test_copy_refresh_message_only_rebuilds_parent_semantics( + request_context, + monkeypatch, +): + service = FSService(viking_fs=_FakeVikingFS()) + queue_manager = _FakeQueueManager() + mark_pending = AsyncMock() + monkeypatch.setattr( + "openviking.service.fs_service.get_queue_manager", + lambda: queue_manager, + ) + monkeypatch.setattr( + "openviking.service.fs_service.mark_semantic_sidecars_pending", + mark_pending, + ) + + status = await service._enqueue_copy_refresh( + root_uri="viking://resources/archive", + copied_uri="viking://resources/archive/copied.md", + context_type="resource", + ctx=request_context, + ) + + assert status == "queued" + assert len(queue_manager.messages) == 1 + msg = queue_manager.messages[0] + assert msg.uri == "viking://resources/archive" + assert msg.recursive is False + assert msg.skip_vectorization is True + assert msg.changes == {"added": ["viking://resources/archive/copied.md"]} + assert msg.generation_trigger == "content_copy" + + @pytest.mark.asyncio async def test_resource_rm_wait_registers_request_before_semantic_root( request_context, From 57b9aa374d906a1be93eef4975cda25dccd93458 Mon Sep 17 00:00:00 2001 From: chenpengfei Date: Thu, 20 Aug 2026 13:15:39 +0800 Subject: [PATCH 05/20] feat: expose filesystem copy API --- openviking/server/routers/filesystem.py | 48 +++++++++++- .../test_api_fs_content_endpoint_suite.py | 77 ++++++++++++++++++- tests/server/test_filesystem_router.py | 63 +++++++++++++++ 3 files changed, 184 insertions(+), 4 deletions(-) diff --git a/openviking/server/routers/filesystem.py b/openviking/server/routers/filesystem.py index 2b74754c58..c10b0a38c9 100644 --- a/openviking/server/routers/filesystem.py +++ b/openviking/server/routers/filesystem.py @@ -39,9 +39,7 @@ def _clean_memory_attrs(raw: str) -> dict[str, Any]: return attrs -async def _tags_attr( - service: Any, uri: str, ctx: RequestContext, *, is_dir: bool -) -> list[str]: +async def _tags_attr(service: Any, uri: str, ctx: RequestContext, *, is_dir: bool) -> list[str]: vikingdb_manager = getattr(service, "vikingdb_manager", None) if not vikingdb_manager: return [] @@ -310,6 +308,50 @@ class MvRequest(BaseModel): to_uri: str +class CpRequest(BaseModel): + """Request model for cp.""" + + from_uri: str + to_uri: str + recursive: bool = False + + +@router.post("/cp") +async def cp( + request: CpRequest, + _ctx: RequestContext = Depends(get_request_context), +): + """Copy a file or directory together with its vector records.""" + service = get_service() + from_uri = resolve_path_variables(request.from_uri) + to_uri = resolve_path_variables(request.to_uri) + try: + result = await service.fs.cp( + from_uri, + to_uri, + recursive=request.recursive, + ctx=_ctx, + ) + except AGFSNotFoundError: + raise NotFoundError(from_uri, "file") + except AGFSClientError as exc: + mapped = map_exception(exc, resource=from_uri, resource_type="file") + if mapped is not None: + raise mapped from exc + raise + except Exception as exc: + mapped = map_exception(exc, resource=from_uri) + if mapped is not None: + raise mapped from exc + raise + + response_result = dict(result or {}) + response_result.setdefault("from", from_uri) + response_result.setdefault("to", to_uri) + response_result.setdefault("recursive", request.recursive) + return Response(status="ok", result=response_result) + + @router.post("/mv") async def mv( request: MvRequest, diff --git a/tests/server/test_api_fs_content_endpoint_suite.py b/tests/server/test_api_fs_content_endpoint_suite.py index f07d390142..d332872f41 100644 --- a/tests/server/test_api_fs_content_endpoint_suite.py +++ b/tests/server/test_api_fs_content_endpoint_suite.py @@ -6,7 +6,7 @@ import httpx from openviking.pyagfs.exceptions import AGFSHTTPError -from openviking_cli.exceptions import NotFoundError +from openviking_cli.exceptions import ConflictError, FailedPreconditionError, NotFoundError def _assert_error( @@ -167,6 +167,81 @@ async def fake_mv(*args, **kwargs): _assert_error(response, status_code=404, error_code="NOT_FOUND") +async def test_cp_missing_source_returns_not_found(app, service, monkeypatch): + async def fake_cp(*args, **kwargs): + raise FileNotFoundError("cp source not found") + + monkeypatch.setattr(service.fs, "cp", fake_cp, raising=False) + response = await _request_with_handler( + app, + "POST", + "/api/v1/fs/cp", + json={ + "from_uri": "viking://resources/missing", + "to_uri": "viking://resources/target", + }, + ) + _assert_error(response, status_code=404, error_code="NOT_FOUND") + + +async def test_cp_existing_target_returns_conflict(app, service, monkeypatch): + async def fake_cp(*args, **kwargs): + raise ConflictError("copy target already exists") + + monkeypatch.setattr(service.fs, "cp", fake_cp, raising=False) + response = await _request_with_handler( + app, + "POST", + "/api/v1/fs/cp", + json={ + "from_uri": "viking://resources/source", + "to_uri": "viking://resources/target", + "recursive": True, + }, + ) + _assert_error(response, status_code=409, error_code="CONFLICT") + + +async def test_cp_directory_without_recursive_returns_precondition( + app, + service, + monkeypatch, +): + async def fake_cp(*args, **kwargs): + raise FailedPreconditionError("directory copy requires recursive") + + monkeypatch.setattr(service.fs, "cp", fake_cp, raising=False) + response = await _request_with_handler( + app, + "POST", + "/api/v1/fs/cp", + json={ + "from_uri": "viking://resources/source", + "to_uri": "viking://resources/target", + }, + ) + _assert_error(response, status_code=412, error_code="FAILED_PRECONDITION") + + +async def test_cp_internal_rollback_error_is_redacted(app, service, monkeypatch): + async def fake_cp(*args, **kwargs): + raise RuntimeError("rollback failed with private vector payload") + + monkeypatch.setattr(service.fs, "cp", fake_cp, raising=False) + response = await _request_with_handler( + app, + "POST", + "/api/v1/fs/cp", + json={ + "from_uri": "viking://resources/source", + "to_uri": "viking://resources/target", + }, + ) + + _assert_error(response, status_code=500, error_code="INTERNAL") + assert "private vector payload" not in response.text + + async def test_read_missing_uri_returns_not_found(app, service, monkeypatch): async def fake_read(*args, **kwargs): raise FileNotFoundError("read target missing") diff --git a/tests/server/test_filesystem_router.py b/tests/server/test_filesystem_router.py index 4cdf734d26..26fa29eda6 100644 --- a/tests/server/test_filesystem_router.py +++ b/tests/server/test_filesystem_router.py @@ -38,6 +38,69 @@ async def fake_rm(uri, ctx=None, recursive=False, wait=False, timeout=None): assert response.result["memory_cleanup"] == cleanup +@pytest.mark.asyncio +async def test_cp_resolves_paths_and_preserves_service_result(monkeypatch): + calls = [] + + async def fake_cp(from_uri, to_uri, recursive=False, ctx=None): + calls.append((from_uri, to_uri, recursive, ctx)) + return { + "operation_id": "copy-1", + "from": from_uri, + "to": to_uri, + "recursive": recursive, + "semantic_root_uri": "viking://resources/archive", + "semantic_status": "queued", + } + + monkeypatch.setattr( + filesystem, + "get_service", + lambda: SimpleNamespace(fs=SimpleNamespace(cp=fake_cp)), + ) + monkeypatch.setattr( + filesystem, + "resolve_path_variables", + lambda uri: uri.replace("{test:resources}", "viking://resources"), + ) + ctx = RequestContext(user=UserIdentifier("acct", "alice"), role=Role.USER) + + response = await filesystem.cp( + filesystem.CpRequest( + from_uri="{test:resources}/a.md", + to_uri="{test:resources}/archive/a.md", + recursive=True, + ), + _ctx=ctx, + ) + + assert calls == [ + ( + "viking://resources/a.md", + "viking://resources/archive/a.md", + True, + ctx, + ) + ] + assert response.result == { + "operation_id": "copy-1", + "from": "viking://resources/a.md", + "to": "viking://resources/archive/a.md", + "recursive": True, + "semantic_root_uri": "viking://resources/archive", + "semantic_status": "queued", + } + + +def test_cp_request_defaults_to_non_recursive(): + request = filesystem.CpRequest( + from_uri="viking://resources/a.md", + to_uri="viking://resources/b.md", + ) + + assert request.recursive is False + + @pytest.mark.asyncio async def test_attrs_returns_memory_fields_and_tags(monkeypatch): raw_memory = ( From 4863a7363094bdd4cc64fa973f7179486d63daa1 Mon Sep 17 00:00:00 2001 From: chenpengfei Date: Thu, 20 Aug 2026 13:21:02 +0800 Subject: [PATCH 06/20] feat: add ov cp command --- crates/ov_cli/src/client.rs | 35 ++++++++++++++++ crates/ov_cli/src/commands/filesystem.rs | 18 ++++++++ crates/ov_cli/src/handlers.rs | 18 ++++++++ crates/ov_cli/src/help_ui.rs | 33 ++++++++++++++- crates/ov_cli/src/main.rs | 53 ++++++++++++++++++++++++ 5 files changed, 156 insertions(+), 1 deletion(-) diff --git a/crates/ov_cli/src/client.rs b/crates/ov_cli/src/client.rs index 8049e3bf3d..e453b4f6b7 100644 --- a/crates/ov_cli/src/client.rs +++ b/crates/ov_cli/src/client.rs @@ -659,6 +659,20 @@ impl HttpClient { self.post("/api/v1/fs/mv", &body).await } + pub async fn cp( + &self, + from_uri: &str, + to_uri: &str, + recursive: bool, + ) -> Result { + let body = serde_json::json!({ + "from_uri": from_uri, + "to_uri": to_uri, + "recursive": recursive, + }); + self.post("/api/v1/fs/cp", &body).await + } + pub async fn stat(&self, uri: &str) -> Result { let params = vec![("uri".to_string(), uri.to_string())]; self.get("/api/v1/fs/stat", ¶ms).await @@ -2268,6 +2282,27 @@ mod tests { assert!(!request.contains("include_mod_time_iso=")); } + #[tokio::test] + async fn cp_posts_recursive_request_body() { + let (base_url, request_rx) = spawn_request_capture_server().await; + let client = HttpClient::new(base_url, None, None, None, None, 5.0, false, None); + + client + .cp( + "viking://resources/source", + "viking://resources/target", + true, + ) + .await + .expect("cp request should succeed"); + + let request = request_rx.await.expect("request should be captured"); + assert!(request.starts_with("POST /api/v1/fs/cp ")); + assert!(request.contains(r#""from_uri":"viking://resources/source""#)); + assert!(request.contains(r#""to_uri":"viking://resources/target""#)); + assert!(request.contains(r#""recursive":true"#)); + } + #[tokio::test] async fn gateway_token_is_not_sent_without_a_gateway_challenge() { let (base_url, request_rx) = spawn_request_capture_server().await; diff --git a/crates/ov_cli/src/commands/filesystem.rs b/crates/ov_cli/src/commands/filesystem.rs index d9d15f45fa..7e58769462 100644 --- a/crates/ov_cli/src/commands/filesystem.rs +++ b/crates/ov_cli/src/commands/filesystem.rs @@ -858,6 +858,24 @@ pub async fn mv( Ok(()) } +pub async fn cp( + client: &HttpClient, + from_uri: &str, + to_uri: &str, + recursive: bool, + output_format: OutputFormat, + compact: bool, +) -> Result<()> { + let result = client.cp(from_uri, to_uri, recursive).await?; + output_message_result( + result, + format!("Copied: {} -> {}", from_uri, to_uri), + output_format, + compact, + ); + Ok(()) +} + pub async fn stat( client: &HttpClient, uri: &str, diff --git a/crates/ov_cli/src/handlers.rs b/crates/ov_cli/src/handlers.rs index dee9e11122..cd5e90319d 100644 --- a/crates/ov_cli/src/handlers.rs +++ b/crates/ov_cli/src/handlers.rs @@ -1770,6 +1770,24 @@ pub async fn handle_mv(from_uri: String, to_uri: String, ctx: CliContext) -> Res commands::filesystem::mv(&client, &from_uri, &to_uri, ctx.output_format, ctx.compact).await } +pub async fn handle_cp( + from_uri: String, + to_uri: String, + recursive: bool, + ctx: CliContext, +) -> Result<()> { + let client = ctx.get_client(); + commands::filesystem::cp( + &client, + &from_uri, + &to_uri, + recursive, + ctx.output_format, + ctx.compact, + ) + .await +} + pub async fn handle_stat(uri: String, ctx: CliContext) -> Result<()> { let client = ctx.get_client(); commands::filesystem::stat(&client, &uri, ctx.output_format, ctx.compact).await diff --git a/crates/ov_cli/src/help_ui.rs b/crates/ov_cli/src/help_ui.rs index 250e797acf..5cbe8eeadb 100644 --- a/crates/ov_cli/src/help_ui.rs +++ b/crates/ov_cli/src/help_ui.rs @@ -72,7 +72,7 @@ const CORE_WORKFLOW: &[HelpCommand] = help_commands![ ]; const FILESYSTEM: &[HelpCommand] = help_commands![ - "ls", "tree", "mkdir", "rm", "mv", "stat", "attrs", "acl", "get" + "ls", "tree", "mkdir", "rm", "cp", "mv", "stat", "attrs", "acl", "get" ]; const SEARCH_CONTEXT: &[HelpCommand] = help_commands![ @@ -290,6 +290,30 @@ const COMMAND_HELP_SPECS: &[CommandHelpSpec] = &[ }, ], }, + CommandHelpSpec { + path: &["cp"], + purpose: "Copy a file or directory without reparsing or regenerating vectors.", + examples: &[ + HelpItem { + label: "ov cp viking://notes/draft.md viking://notes/draft-copy.md", + description: "Copy one file and its vector records.", + }, + HelpItem { + label: "ov cp -r viking://projects/source viking://projects/backup", + description: "Recursively copy a directory and all vector records.", + }, + ], + next_steps: &[ + HelpItem { + label: "ov stat ", + description: "Confirm the copied resource metadata.", + }, + HelpItem { + label: "ov read ", + description: "Read the copied resource.", + }, + ], + }, CommandHelpSpec { path: &["mv"], purpose: "Move or rename a resource.", @@ -2734,6 +2758,13 @@ mod tests { assert!(rendered.contains("experimental")); assert!(rendered.contains("ov --help")); assert!(!rendered.contains("Commands:\n add-resource")); + assert!(rendered.contains("cp")); + let cp = command_spec(&["cp".to_string()]).expect("cp command help"); + assert!( + cp.examples + .iter() + .any(|item| item.label.contains("ov cp -r")) + ); } #[test] diff --git a/crates/ov_cli/src/main.rs b/crates/ov_cli/src/main.rs index 2e38a30064..98fc36632d 100644 --- a/crates/ov_cli/src/main.rs +++ b/crates/ov_cli/src/main.rs @@ -608,6 +608,18 @@ enum Commands { )] timeout: Option, }, + /// [Data] Copy a file or directory + Cp { + /// Source URI + #[arg(value_name = "source")] + from_uri: String, + /// Target URI + #[arg(value_name = "target")] + to_uri: String, + /// Copy a directory recursively + #[arg(short, long, help_heading = "Common options")] + recursive: bool, + }, /// [Data] Move or rename resource #[command(alias = "rename")] Mv { @@ -3503,6 +3515,11 @@ async fn main() { wait, timeout, } => handlers::handle_rm(uri, recursive, wait, timeout, ctx).await, + Commands::Cp { + from_uri, + to_uri, + recursive, + } => handlers::handle_cp(from_uri, to_uri, recursive, ctx).await, Commands::Mv { from_uri, to_uri } => handlers::handle_mv(from_uri, to_uri, ctx).await, Commands::Stat { uri } => handlers::handle_stat(uri, ctx).await, Commands::Attrs { action } => match action { @@ -3793,6 +3810,42 @@ mod tests { assert_eq!(cli.actor_peer_id.as_deref(), Some("peer-a")); } + #[test] + fn cli_parses_copy_recursive_flag() { + let file = Cli::try_parse_from([ + "ov", + "cp", + "viking://resources/a.md", + "viking://resources/b.md", + ]) + .expect("file copy should parse"); + match file.command { + Commands::Cp { + from_uri, + to_uri, + recursive, + } => { + assert_eq!(from_uri, "viking://resources/a.md"); + assert_eq!(to_uri, "viking://resources/b.md"); + assert!(!recursive); + } + _ => panic!("expected cp command"), + } + + let directory = Cli::try_parse_from([ + "ov", + "cp", + "-r", + "viking://resources/src", + "viking://resources/dst", + ]) + .expect("recursive directory copy should parse"); + match directory.command { + Commands::Cp { recursive, .. } => assert!(recursive), + _ => panic!("expected cp command"), + } + } + #[test] fn cli_parses_snapshot_diff_refs() { let cli = Cli::try_parse_from([ From 3ae1c824c91de91802ac263a46e30502a41102ae Mon Sep 17 00:00:00 2001 From: chenpengfei Date: Thu, 20 Aug 2026 13:26:33 +0800 Subject: [PATCH 07/20] test: cover filesystem copy end to end --- tests/api_test/api/client.py | 16 ++++ tests/api_test/conftest.py | 2 + .../filesystem/slow/test_fs_mv_deep.py | 22 +++++ tests/api_test/filesystem/test_fs_cp.py | 90 +++++++++++++++++++ tests/cli/conftest.py | 10 +++ tests/cli/test_cli_filesystem.py | 42 ++++++++- 6 files changed, 181 insertions(+), 1 deletion(-) create mode 100644 tests/api_test/filesystem/test_fs_cp.py diff --git a/tests/api_test/api/client.py b/tests/api_test/api/client.py index 29aca7e38b..32eb81e79d 100644 --- a/tests/api_test/api/client.py +++ b/tests/api_test/api/client.py @@ -476,6 +476,22 @@ def fs_mv(self, from_uri: str, to_uri: str) -> requests.Response: url = self._build_url(self.server_url, endpoint) return self.session.post(url, json={"from_uri": from_uri, "to_uri": to_uri}) + def fs_cp( + self, + from_uri: str, + to_uri: str, + recursive: bool = False, + ) -> requests.Response: + endpoint = "/api/v1/fs/cp" + url = self._build_url(self.server_url, endpoint) + return self.session.post( + url, + json={ + "from_uri": from_uri, + "to_uri": to_uri, + "recursive": recursive, + }, + ) def session_used( self, session_id: str, diff --git a/tests/api_test/conftest.py b/tests/api_test/conftest.py index 0bec692898..66501c00e9 100644 --- a/tests/api_test/conftest.py +++ b/tests/api_test/conftest.py @@ -12,6 +12,7 @@ "test_wait_processed.py::TestWaitProcessed::test_wait_processed": "等待资源处理完成", "test_fs_ls.py::TestFsLs::test_fs_ls_root": "列出文件系统根目录", "test_fs_mkdir.py::TestFsMkdir::test_fs_mkdir": "创建目录", + "test_fs_cp.py::TestFsCp::test_cp_file_preserves_source_and_content": "复制文件和向量", "test_fs_mv.py::TestFsMv::test_fs_mv": "移动文件/目录", "test_fs_read_write.py::TestFsReadWrite::test_fs_read": "读取文件内容", "test_fs_rm.py::TestFsRm::test_fs_rm": "删除文件/目录", @@ -109,6 +110,7 @@ "test_wait_processed.py::TestWaitProcessed::test_wait_processed": "/api/v1/resources/wait", "test_fs_ls.py::TestFsLs::test_fs_ls_root": "/api/v1/fs/ls", "test_fs_mkdir.py::TestFsMkdir::test_fs_mkdir": "/api/v1/fs/mkdir", + "test_fs_cp.py::TestFsCp::test_cp_file_preserves_source_and_content": "/api/v1/fs/cp", "test_fs_mv.py::TestFsMv::test_fs_mv": "/api/v1/fs/mv", "test_fs_read_write.py::TestFsReadWrite::test_fs_read": "/api/v1/fs/read", "test_fs_rm.py::TestFsRm::test_fs_rm": "/api/v1/fs/rm", diff --git a/tests/api_test/filesystem/slow/test_fs_mv_deep.py b/tests/api_test/filesystem/slow/test_fs_mv_deep.py index 6cfb4aa63e..bf73086e6d 100644 --- a/tests/api_test/filesystem/slow/test_fs_mv_deep.py +++ b/tests/api_test/filesystem/slow/test_fs_mv_deep.py @@ -2,6 +2,28 @@ class TestFsMvDeep: + def test_mv_rejects_existing_target_without_changing_files(self, api_client): + suffix = uuid.uuid4().hex[:8] + src = f"viking://resources/mvdeep_conflict_src_{suffix}.md" + dst = f"viking://resources/mvdeep_conflict_dst_{suffix}.md" + try: + assert ( + api_client.fs_write(src, "source unchanged", mode="create", wait=True).status_code + == 200 + ) + assert ( + api_client.fs_write(dst, "target unchanged", mode="create", wait=True).status_code + == 200 + ) + + mv_resp = api_client.fs_mv(src, dst) + assert mv_resp.status_code == 409, mv_resp.text + assert "source unchanged" in api_client.fs_read(src).json().get("result", "") + assert "target unchanged" in api_client.fs_read(dst).json().get("result", "") + finally: + api_client.fs_rm(src) + api_client.fs_rm(dst) + def test_mv_file_preserves_content(self, api_client): src = f"viking://resources/mvdeep_src_{uuid.uuid4().hex[:8]}.md" dst = f"viking://resources/mvdeep_dst_{uuid.uuid4().hex[:8]}.md" diff --git a/tests/api_test/filesystem/test_fs_cp.py b/tests/api_test/filesystem/test_fs_cp.py new file mode 100644 index 0000000000..d2a7b7cf07 --- /dev/null +++ b/tests/api_test/filesystem/test_fs_cp.py @@ -0,0 +1,90 @@ +import uuid + + +class TestFsCp: + def test_cp_file_preserves_source_and_content(self, api_client): + suffix = uuid.uuid4().hex[:8] + source = f"viking://resources/cp-source-{suffix}.md" + target = f"viking://resources/cp-target-{suffix}.md" + content = f"copy payload {suffix}" + try: + write = api_client.fs_write(source, content, mode="create", wait=True) + assert write.status_code == 200 + + copied = api_client.fs_cp(source, target) + assert copied.status_code == 200, copied.text + result = copied.json().get("result", {}) + assert result.get("from") == source + assert result.get("to") == target + assert result.get("recursive") is False + + for uri in (source, target): + stat = api_client.fs_stat(uri) + assert stat.status_code == 200 + read = api_client.fs_read(uri) + assert read.status_code == 200 + assert content in read.json().get("result", "") + + found = api_client.find(query=suffix, target_uri=target, limit=5) + assert found.status_code == 200 + resources = found.json().get("result", {}).get("resources", []) + assert any(item.get("uri") == target for item in resources) + + assert api_client.fs_rm(source).status_code == 200 + assert api_client.fs_read(target).status_code == 200 + finally: + api_client.fs_rm(source) + api_client.fs_rm(target) + + def test_cp_directory_requires_recursive_and_copies_tree(self, api_client): + suffix = uuid.uuid4().hex[:8] + source = f"viking://resources/cp-dir-source-{suffix}" + target = f"viking://resources/cp-dir-target-{suffix}" + child = f"{source}/nested/child.md" + try: + assert api_client.fs_mkdir(source).status_code == 200 + assert api_client.fs_mkdir(f"{source}/nested").status_code == 200 + assert api_client.fs_mkdir(f"{source}/empty").status_code == 200 + assert ( + api_client.fs_write( + child, "recursive copy child", mode="create", wait=True + ).status_code + == 200 + ) + + without_recursive = api_client.fs_cp(source, target) + assert without_recursive.status_code == 412, without_recursive.text + + copied = api_client.fs_cp(source, target, recursive=True) + assert copied.status_code == 200, copied.text + assert api_client.fs_read(f"{target}/nested/child.md").status_code == 200 + tree = api_client.fs_tree(target) + assert tree.status_code == 200 + tree_text = tree.text + assert "nested/child.md" in tree_text + assert "empty" in tree_text + finally: + api_client.fs_rm(source, recursive=True) + api_client.fs_rm(target, recursive=True) + + def test_cp_rejects_existing_target_without_changing_either_file(self, api_client): + suffix = uuid.uuid4().hex[:8] + source = f"viking://resources/cp-conflict-source-{suffix}.md" + target = f"viking://resources/cp-conflict-target-{suffix}.md" + try: + assert ( + api_client.fs_write(source, "source remains", mode="create", wait=True).status_code + == 200 + ) + assert ( + api_client.fs_write(target, "target remains", mode="create", wait=True).status_code + == 200 + ) + + copied = api_client.fs_cp(source, target) + assert copied.status_code == 409, copied.text + assert "source remains" in api_client.fs_read(source).json().get("result", "") + assert "target remains" in api_client.fs_read(target).json().get("result", "") + finally: + api_client.fs_rm(source) + api_client.fs_rm(target) diff --git a/tests/cli/conftest.py b/tests/cli/conftest.py index 3fb8915b16..127052a954 100644 --- a/tests/cli/conftest.py +++ b/tests/cli/conftest.py @@ -553,6 +553,16 @@ def ov_mv(src_uri, dst_uri): return ov_retry(["mv", src_uri, dst_uri, "-o", "json"], attempts=20, interval=15) +def ov_cp(src_uri, dst_uri, *, recursive=False, output="json"): + args = ["cp"] + if recursive: + args.append("-r") + args.extend([src_uri, dst_uri]) + if output: + args.extend(["-o", output]) + return ov_retry(args, attempts=20, interval=15) + + def ov_write(uri, content, *extra_args): return ov_retry( [ diff --git a/tests/cli/test_cli_filesystem.py b/tests/cli/test_cli_filesystem.py index 688dfab182..b8eaea0396 100644 --- a/tests/cli/test_cli_filesystem.py +++ b/tests/cli/test_cli_filesystem.py @@ -7,7 +7,7 @@ import uuid import pytest -from conftest import ov, ov_add_resource, ov_mkdir, ov_mv, ov_rm +from conftest import ov, ov_add_resource, ov_cp, ov_mkdir, ov_mv, ov_rm pytestmark = pytest.mark.cli_remote @@ -135,3 +135,43 @@ def test_mv_directory(self, test_dir_uri): f"ov mv should exit 0, got {r['exit_code']}: {r['stderr'][:300]}" ) ov_rm(dst_uri) + + +class TestFsCp: + def test_cp_file_json_and_human_output(self, test_dir_uri): + suffix = uuid.uuid4().hex[:6] + source = f"{test_dir_uri}/cp_source_{suffix}.txt" + json_target = f"{test_dir_uri}/cp_json_{suffix}.txt" + human_target = f"{test_dir_uri}/cp_human_{suffix}.txt" + with tempfile.NamedTemporaryFile(suffix=".txt", delete=False, mode="w") as f: + f.write("copy me") + temp_path = f.name + try: + assert ov_add_resource(temp_path, source)["exit_code"] == 0 + json_result = ov_cp(source, json_target) + assert json_result["exit_code"] == 0, json_result["stderr"] + assert json_result["json"] and json_result["json"].get("ok") is True + + human_result = ov_cp(source, human_target, output=None) + assert human_result["exit_code"] == 0, human_result["stderr"] + assert "Copied:" in human_result["stdout"] + finally: + os.unlink(temp_path) + ov_rm(source) + ov_rm(json_target) + ov_rm(human_target) + + def test_cp_directory_requires_recursive(self, test_dir_uri): + suffix = uuid.uuid4().hex[:6] + source = f"{test_dir_uri}/cp_dir_source_{suffix}" + target = f"{test_dir_uri}/cp_dir_target_{suffix}" + try: + assert ov_mkdir(source)["exit_code"] == 0 + without_recursive = ov_cp(source, target) + assert without_recursive["exit_code"] != 0 + + recursive = ov_cp(source, target, recursive=True) + assert recursive["exit_code"] == 0, recursive["stderr"] + finally: + ov_rm(source) + ov_rm(target) From d38291c4d594f9d6d08ba0dc10ea460934f66290 Mon Sep 17 00:00:00 2001 From: chenpengfei Date: Thu, 20 Aug 2026 13:29:59 +0800 Subject: [PATCH 08/20] test: compose CLI collection hooks --- tests/cli/conftest.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/cli/conftest.py b/tests/cli/conftest.py index 127052a954..35ff6a0b78 100644 --- a/tests/cli/conftest.py +++ b/tests/cli/conftest.py @@ -333,7 +333,7 @@ def _check_cli_compatible(): CLI_COMPATIBLE = _check_cli_compatible() -def pytest_collection_modifyitems(config, items): +def _apply_cli_skip_markers(items): skip_reason = None if not CLI_COMPATIBLE: skip_reason = "openviking CLI not available" @@ -616,6 +616,10 @@ def _find_file_in_pack(pack_uri, retries=10, interval=5): return None +def pytest_collection_modifyitems(items): + _apply_cli_skip_markers(items) + + @pytest.fixture(scope="session") def ensure_resources_dir(): r = ov_mkdir("viking://resources") From 9c053c22bd663a6035351c76c52280bef92086af Mon Sep 17 00:00:00 2001 From: chenpengfei Date: Thu, 20 Aug 2026 13:33:00 +0800 Subject: [PATCH 09/20] fix: support vector copy on volcengine backend --- .../storage/viking_vector_index_backend.py | 12 ++++++-- tests/storage/test_vector_transfer.py | 28 ++++++++++++++++++- 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/openviking/storage/viking_vector_index_backend.py b/openviking/storage/viking_vector_index_backend.py index d105aec7b1..9811bfd7a3 100644 --- a/openviking/storage/viking_vector_index_backend.py +++ b/openviking/storage/viking_vector_index_backend.py @@ -33,6 +33,7 @@ from openviking_cli.exceptions import ConflictError from openviking_cli.utils import get_logger from openviking_cli.utils.config.vectordb_config import DEFAULT_INDEX_NAME, VectorDBBackendConfig +from openviking_cli.utils.uri import VikingURI logger = get_logger(__name__) @@ -1460,11 +1461,16 @@ async def delete_uris(self, ctx: RequestContext, uris: List[str]) -> None: backend = self._get_backend_for_context(ctx) await backend.delete_by_filter(And(conds)) - @staticmethod - def _uri_transfer_filter(ctx: RequestContext, uri: str, *, recursive: bool) -> FilterExpr: - scopes: List[FilterExpr] = [Eq("uri", uri), Contains("uri", uri + "#")] + def _uri_transfer_filter(self, ctx: RequestContext, uri: str, *, recursive: bool) -> FilterExpr: + scopes: List[FilterExpr] = [Eq("uri", uri)] if recursive: scopes.append(PathScope("uri", uri, depth=-1)) + if self.mode == "volcengine": + parent = VikingURI(uri).parent + if parent is not None and parent.uri != "viking://": + scopes.append(PathScope("uri", parent.uri, depth=1)) + else: + scopes.append(Contains("uri", uri + "#")) return And([Eq("account_id", ctx.account_id), Or(scopes)]) async def _scan_uri_transfer_scope( diff --git a/tests/storage/test_vector_transfer.py b/tests/storage/test_vector_transfer.py index a6e12a7a02..e6840947ff 100644 --- a/tests/storage/test_vector_transfer.py +++ b/tests/storage/test_vector_transfer.py @@ -8,6 +8,7 @@ import pytest from openviking.server.identity import RequestContext, Role +from openviking.storage.expr import And, Contains, Or, PathScope from openviking.storage.viking_vector_index_backend import ( VectorTransferRollbackError, VikingVectorIndexBackend, @@ -46,6 +47,12 @@ def __init__(self, records: list[dict[str, Any]]) -> None: self.delete_calls = 0 self.fail_delete_at: int | None = None self.partial_delete_count = 0 + self.backend_mode = "local" + self.scroll_filters = [] + + @property + def mode(self) -> str: + return self.backend_mode async def scroll( self, @@ -56,7 +63,8 @@ async def scroll( *, ctx: RequestContext, ) -> tuple[list[dict[str, Any]], str | None]: - del filter, output_fields, ctx + del output_fields, ctx + self.scroll_filters.append(filter) offset = int(cursor or 0) ordered = [dict(self.records[key]) for key in sorted(self.records)] page = ordered[offset : offset + limit] @@ -153,6 +161,24 @@ async def test_copy_uri_mapping_preserves_dense_sparse_and_chunk_payloads(): } +@pytest.mark.asyncio +async def test_volcengine_transfer_scope_avoids_unsupported_contains_filter(): + source = "viking://resources/src.md" + backend = _MemoryTransferBackend([_record("source", source)]) + backend.backend_mode = "volcengine" + + await backend.copy_uri_mapping(_ctx(), source, "viking://resources/dst.md", recursive=False) + + assert backend.scroll_filters + for filter_expr in backend.scroll_filters: + assert isinstance(filter_expr, And) + scopes = next(cond for cond in filter_expr.conds if isinstance(cond, Or)).conds + assert not any(isinstance(scope, Contains) for scope in scopes) + assert any( + isinstance(scope, PathScope) and scope.path == "viking://resources" for scope in scopes + ) + + @pytest.mark.asyncio async def test_copy_uri_mapping_rejects_preexisting_target_before_writing(): backend = _MemoryTransferBackend( From e8e2b55aa7a54010bed7e488a565ba7e2535f635 Mon Sep 17 00:00:00 2001 From: chenpengfei Date: Thu, 20 Aug 2026 14:06:06 +0800 Subject: [PATCH 10/20] fix: harden copy transaction boundaries --- openviking/service/fs_service.py | 3 + openviking/storage/queuefs/semantic_dag.py | 98 ++++++++-- openviking/storage/queuefs/semantic_msg.py | 4 + .../storage/queuefs/semantic_processor.py | 3 + openviking/storage/viking_fs/_ops.py | 66 ++++++- .../storage/viking_vector_index_backend.py | 168 +++++++++++++++++- tests/service/test_fs_service.py | 3 + .../storage/test_semantic_dag_incremental.py | 51 ++++++ ...t_semantic_processor_target_preexisting.py | 23 +++ tests/storage/test_vector_transfer.py | 97 ++++++++++ tests/storage/test_viking_fs_cp.py | 48 ++++- 11 files changed, 536 insertions(+), 28 deletions(-) diff --git a/openviking/service/fs_service.py b/openviking/service/fs_service.py index 21f745b5b1..cd57b6ef4e 100644 --- a/openviking/service/fs_service.py +++ b/openviking/service/fs_service.py @@ -470,6 +470,7 @@ async def _enqueue_copy_refresh( self, *, root_uri: str, + source_uri: str, copied_uri: str, context_type: str, ctx: RequestContext, @@ -508,6 +509,7 @@ async def _enqueue_copy_refresh( ), changes={"added": [copied_uri]}, generation_trigger="content_copy", + copy_source_uri=source_uri, ) if telemetry_id: get_request_wait_tracker().register_semantic_root(telemetry_id, msg.id) @@ -569,6 +571,7 @@ async def cp( try: result["semantic_status"] = await self._enqueue_copy_refresh( root_uri=refresh_parent_uri, + source_uri=from_uri, copied_uri=to_uri, context_type=context_type, ctx=ctx, diff --git a/openviking/storage/queuefs/semantic_dag.py b/openviking/storage/queuefs/semantic_dag.py index 0fa381e87a..e3d7ceb97c 100644 --- a/openviking/storage/queuefs/semantic_dag.py +++ b/openviking/storage/queuefs/semantic_dag.py @@ -175,6 +175,7 @@ def __init__( source: Optional[Dict[str, str]] = None, generation_trigger: str = "semantic_refresh", aggregate_directory: bool = True, + copy_source_uri: str = "", ): self._processor = processor self._context_type = context_type @@ -193,6 +194,7 @@ def __init__( self._source = dict(source) if source else None self._generation_trigger = generation_trigger self._aggregate_directory = aggregate_directory + self._copy_source_uri = copy_source_uri self._task_context = get_task_context() self._telemetry = get_current_telemetry() self._stale = False @@ -596,6 +598,29 @@ async def _read_existing_summary(self, file_path: str) -> Optional[Dict[str, str return None + async def _read_copy_source_summary(self, file_path: str) -> Optional[Dict[str, str]]: + """Reuse the copied file's source summary without parsing or invoking VLM.""" + added = self._changes.get("added", []) + if not self._copy_source_uri or len(added) != 1 or file_path != added[0]: + return await self._read_existing_summary(file_path) + source_parent = self._copy_source_uri.rsplit("/", 1)[0] + source_name = self._copy_source_uri.rsplit("/", 1)[-1] + try: + overview = await self._viking_fs.read_file( + f"{source_parent}/.overview.md", ctx=self._ctx + ) + summaries = self._processor._parse_overview_md(body_for_preview(overview)) + if source_name in summaries: + return { + "name": file_path.rsplit("/", 1)[-1], + "summary": str(summaries[source_name]), + } + except SemanticSidecarFormatError: + raise + except Exception as exc: + logger.debug("Failed to reuse copy source summary for %s: %s", file_path, exc) + return None + async def _check_dir_children_changed( self, dir_uri: str, current_files: List[str], current_dirs: List[str] ) -> bool: @@ -657,7 +682,13 @@ async def _file_summary_task(self, parent_uri: str, file_path: str) -> None: need_vectorize = True try: summary_dict = None - if self._incremental_update: + if self._generation_trigger == "content_copy": + summary_dict = await self._read_copy_source_summary(file_path) + if summary_dict is None: + summary_dict = {"name": file_name, "summary": ""} + self._file_change_status[file_path] = file_path in self._changed_paths + need_vectorize = False + elif self._incremental_update: content_changed = await self._check_file_content_changed(file_path) self._file_change_status[file_path] = content_changed node = self._nodes.get(parent_uri) @@ -817,6 +848,29 @@ def _select_direct_media_overview( return None return summary + def _build_content_copy_overview( + self, + dir_uri: str, + existing_overview: Optional[str], + file_summaries: List[Dict[str, str]], + children_abstracts: List[Dict[str, str]], + ) -> str: + """Patch copied entries into a parent overview without invoking a model.""" + overview = (existing_overview or f"# {dir_uri.rsplit('/', 1)[-1]}").rstrip() + added_names = {path.rsplit("/", 1)[-1] for path in self._changes.get("added", [])} + sections: List[str] = [] + for item in file_summaries: + name = str(item.get("name") or "") + if name in added_names: + sections.append(f"### {name}\n{str(item.get('summary') or '')}".rstrip()) + for item in children_abstracts: + name = str(item.get("name") or "") + if name in added_names: + sections.append(f"### {name}/\n{str(item.get('abstract') or '')}".rstrip()) + if not sections: + return overview + return f"{overview}\n\n" + "\n\n".join(sections) + @property def stale(self) -> bool: return self._stale @@ -896,10 +950,9 @@ async def _overview_task(self, dir_uri: str) -> None: need_vectorize = True children_changed = True should_write = True - abstract = "" + abstract: Optional[str] = None try: - overview = None - abstract = None + overview: Optional[str] = None if self._incremental_update: children_changed = await self._check_dir_children_changed( dir_uri, node.file_paths, node.children_dirs @@ -920,17 +973,30 @@ async def _overview_task(self, dir_uri: str) -> None: # Sampling happened in _dispatch_dir, before summary work was # scheduled. Only the prepared bounded inputs reach the prompt. sampled_entries = len(file_summaries) + len(children_abstracts) - overview = self._select_direct_media_overview(node, file_summaries) - if overview is None: - async with self._llm_sem: - overview = await self._processor._generate_overview( - dir_uri, - file_summaries, - children_abstracts, - total_files=len(node.file_paths), - total_children=len(node.children_dirs), - ) - overview, abstract = self._processor._normalize_overview_generation(overview) + if self._generation_trigger == "content_copy": + ( + existing_overview, + existing_abstract, + ) = await self._read_existing_overview_abstract(dir_uri) + overview = self._build_content_copy_overview( + dir_uri, + existing_overview, + file_summaries, + children_abstracts, + ) + abstract = existing_abstract or "" + else: + overview = self._select_direct_media_overview(node, file_summaries) + if overview is None: + async with self._llm_sem: + overview = await self._processor._generate_overview( + dir_uri, + file_summaries, + children_abstracts, + total_files=len(node.file_paths), + total_children=len(node.children_dirs), + ) + overview, abstract = self._processor._normalize_overview_generation(overview) if self._closed: return @@ -992,7 +1058,7 @@ async def _overview_task(self, dir_uri: str) -> None: self._root_done.set() return - await self._on_child_done(parent_uri, dir_uri, abstract) + await self._on_child_done(parent_uri, dir_uri, abstract or "") self._release_dir_node(dir_uri) def get_stats(self) -> DagStats: diff --git a/openviking/storage/queuefs/semantic_msg.py b/openviking/storage/queuefs/semantic_msg.py index 02d6fb8d4e..d61e984ec1 100644 --- a/openviking/storage/queuefs/semantic_msg.py +++ b/openviking/storage/queuefs/semantic_msg.py @@ -72,6 +72,7 @@ class SemanticMsg: aggregate_directory: bool = True use_hierarchical_aggregation: bool = False propagate_to_parent: bool = True + copy_source_uri: str = "" def __init__( self, @@ -98,6 +99,7 @@ def __init__( aggregate_directory: bool = True, use_hierarchical_aggregation: bool = False, propagate_to_parent: bool = True, + copy_source_uri: str = "", ): self.id = str(uuid4()) self.uri = uri @@ -123,6 +125,7 @@ def __init__( self.aggregate_directory = bool(aggregate_directory) self.use_hierarchical_aggregation = bool(use_hierarchical_aggregation) self.propagate_to_parent = bool(propagate_to_parent) + self.copy_source_uri = copy_source_uri def to_dict(self) -> Dict[str, Any]: """Convert object to dictionary.""" @@ -181,6 +184,7 @@ def from_dict(cls, data: Dict[str, Any]) -> "SemanticMsg": aggregate_directory=data.get("aggregate_directory", True), use_hierarchical_aggregation=data.get("use_hierarchical_aggregation", False), propagate_to_parent=data.get("propagate_to_parent", True), + copy_source_uri=data.get("copy_source_uri", ""), ) if "id" in data and data["id"]: obj.id = data["id"] diff --git a/openviking/storage/queuefs/semantic_processor.py b/openviking/storage/queuefs/semantic_processor.py index 05163b6465..73180d9759 100644 --- a/openviking/storage/queuefs/semantic_processor.py +++ b/openviking/storage/queuefs/semantic_processor.py @@ -248,6 +248,8 @@ async def _requeue_semantic_msg_after_error( async def _enqueue_parent_refresh( self, msg: SemanticMsg, uri: str, *, l0_body_changed: bool ) -> None: + if msg.generation_trigger == "content_copy": + return if msg.context_type not in {"resource", "skill"}: return if not msg.propagate_to_parent: @@ -486,6 +488,7 @@ async def on_dequeue( source=msg.source, generation_trigger=msg.generation_trigger, aggregate_directory=msg.aggregate_directory, + copy_source_uri=msg.copy_source_uri, ) await executor.run(run_uri) self._cache_dag_stats( diff --git a/openviking/storage/viking_fs/_ops.py b/openviking/storage/viking_fs/_ops.py index f2a85a84a8..a38752d931 100644 --- a/openviking/storage/viking_fs/_ops.py +++ b/openviking/storage/viking_fs/_ops.py @@ -9,15 +9,18 @@ from typing import Any, Dict, List, Optional, Union from openviking.core.namespace import ( + is_hidden_by_actor_peer_view, may_include_hidden_actor_peers, + uri_parts, ) from openviking.pyagfs.exceptions import ( AGFSClientError, AGFSDirectoryNotEmptyError, AGFSHTTPError, ) +from openviking.resource.watch_storage import is_watch_task_control_uri from openviking.server.error_mapping import is_not_found_error, map_exception -from openviking.server.identity import RequestContext +from openviking.server.identity import RequestContext, Role from openviking.storage.acl import AclAction, is_acl_uri from openviking.storage.expr import PathScope from openviking.storage.internal_names import STORAGE_INTERNAL_ENTRY_NAMES @@ -35,6 +38,7 @@ FailedPreconditionError, InvalidArgumentError, NotFoundError, + PermissionDeniedError, ) from openviking_cli.utils.uri import VikingURI @@ -318,7 +322,7 @@ async def cp( lease_ref: Dict[str, Any] | None = None, ) -> Dict[str, Any]: """Copy a file or directory together with its vector records.""" - await self._ensure_access(old_uri, ctx) + await self._ensure_copy_source_access(old_uri, recursive=recursive, ctx=ctx) await self._ensure_access(new_uri, ctx, action=AclAction.WRITE) old_scope = old_uri.rstrip("/") new_scope = new_uri.rstrip("/") @@ -366,6 +370,8 @@ async def cp( files_created = await self._copy_agfs_entry( old_path, new_path, + old_uri=old_uri, + new_uri=new_uri, is_dir=is_dir, ctx=ctx, lease_ref=lease, @@ -432,6 +438,41 @@ async def _ensure_transfer_target_missing(self, path: str, uri: str) -> None: raise raise ConflictError(f"transfer target already exists: {uri}", resource=uri) + async def _ensure_copy_source_access( + self, + uri: str, + *, + recursive: bool, + ctx: Optional[RequestContext], + ) -> None: + """Require a copy source to stay inside the caller's visible data view.""" + await self._ensure_access(uri, ctx) + real_ctx = self._ctx_or_default(ctx) + canonical_uri = uri + if is_watch_task_control_uri(canonical_uri): + raise PermissionDeniedError( + "Copying watch-task control state is not allowed", + resource=canonical_uri, + ) + if recursive and ( + is_hidden_by_actor_peer_view(canonical_uri, real_ctx) + or may_include_hidden_actor_peers(canonical_uri, real_ctx) + ): + raise PermissionDeniedError( + "Copy source may include hidden peer data", + resource=canonical_uri, + ) + if real_ctx.role != Role.ROOT and uri_parts(canonical_uri) in ( + [], + ["user"], + ["resources"], + ["temp"], + ): + raise PermissionDeniedError( + "Copying a namespace container root requires root access", + resource=canonical_uri, + ) + async def _ensure_transfer_parent_directory( self, path: str, uri: str, *, operation: str ) -> None: @@ -453,6 +494,8 @@ async def _copy_agfs_entry( old_path: str, new_path: str, *, + old_uri: str, + new_uri: str, is_dir: bool, ctx: Optional[RequestContext], lease_ref: Dict[str, Any], @@ -461,6 +504,8 @@ async def _copy_agfs_entry( return await self._copy_directory_with_exact_locks( old_path, new_path, + old_uri=old_uri, + new_uri=new_uri, ctx=ctx, lease_ref=lease_ref, ) @@ -698,6 +743,8 @@ async def mv( await self._copy_agfs_entry( new_path, old_path, + old_uri=new_uri, + new_uri=old_uri, is_dir=is_dir, ctx=ctx, lease_ref=lease, @@ -763,12 +810,14 @@ async def _copy_for_mv( lease_ref: Dict[str, Any] | None = None, ) -> int: """Copy source to destination for mv without deleting source.""" - del old_uri, new_uri, is_temp + del is_temp if lease_ref is None: raise ValueError("mv copy requires a pathlock lease") return await self._copy_agfs_entry( old_path, new_path, + old_uri=old_uri, + new_uri=new_uri, is_dir=is_dir, ctx=ctx, lease_ref=lease_ref, @@ -778,6 +827,8 @@ async def _copy_directory_with_exact_locks( self, old_path: str, new_path: str, + old_uri: str, + new_uri: str, ctx: Optional[RequestContext], lease_ref: Dict[str, Any] | None, ) -> int: @@ -804,6 +855,13 @@ async def _copy_directory_with_exact_locks( continue old_child = f"{old_path.rstrip('/')}/{name}" new_child = f"{new_path.rstrip('/')}/{name}" + old_child_uri = f"{old_uri.rstrip('/')}/{name}" + new_child_uri = f"{new_uri.rstrip('/')}/{name}" + await self._ensure_copy_source_access( + old_child_uri, + recursive=bool(entry.get("isDir", False)), + ctx=ctx, + ) child_lease = await self._async_agfs.pathlock_acquire_exact( new_child, owner_lease_ref=lease_ref, @@ -813,6 +871,8 @@ async def _copy_directory_with_exact_locks( copied += await self._copy_directory_with_exact_locks( old_child, new_child, + old_uri=old_child_uri, + new_uri=new_child_uri, ctx=ctx, lease_ref=child_lease, ) diff --git a/openviking/storage/viking_vector_index_backend.py b/openviking/storage/viking_vector_index_backend.py index 9811bfd7a3..a582938f31 100644 --- a/openviking/storage/viking_vector_index_backend.py +++ b/openviking/storage/viking_vector_index_backend.py @@ -495,12 +495,90 @@ async def get(self, ids: List[str]) -> List[Dict[str, Any]]: logger.error("Error getting records: %s", e) return [] + def _with_account_filter( + self, filter: Optional[Dict[str, Any] | FilterExpr] + ) -> Optional[FilterExpr]: + if not self._bound_account_id: + if isinstance(filter, dict): + return RawDSL(filter) + return filter + account_filter = Eq("account_id", self._bound_account_id) + if not filter: + return account_filter + if isinstance(filter, dict): + filter = RawDSL(filter) + return And([account_filter, filter]) + async def get_strict(self, ids: List[str]) -> List[Dict[str, Any]]: + """Fetch records without converting backend errors to misses.""" records = await self._async_adapter.call("get", ids) if self._bound_account_id: records = [r for r in records if r.get("account_id") == self._bound_account_id] return records + async def strict_get(self, ids: List[str]) -> List[Dict[str, Any]]: + """Transaction alias for strict record reads.""" + return await self.get_strict(ids) + + async def strict_delete(self, ids: List[str]) -> int: + """Delete transaction records and propagate every backend failure.""" + if self._bound_account_id: + records = await self.strict_get(ids) + valid_ids = [str(record["id"]) for record in records if record.get("id")] + ids = valid_ids + if not ids: + return 0 + return int(await self._async_adapter.call("delete", ids=ids) or 0) + + async def strict_query( + self, + *, + filter: Optional[Dict[str, Any] | FilterExpr] = None, + limit: int = 10, + offset: int = 0, + output_fields: Optional[List[str]] = None, + order_by: Optional[str] = None, + order_desc: bool = False, + ) -> List[Dict[str, Any]]: + """Query transaction records without fail-open exception handling.""" + return await self._async_adapter.call( + "query", + query_vector=None, + sparse_query_vector=None, + filter=self._with_account_filter(filter), + limit=limit, + offset=offset, + output_fields=output_fields, + order_by=order_by, + order_desc=order_desc, + ) + + async def strict_scroll( + self, + filter: Optional[Dict[str, Any] | FilterExpr] = None, + limit: int = 100, + cursor: Optional[str] = None, + output_fields: Optional[List[str]] = None, + ) -> tuple[List[Dict[str, Any]], Optional[str]]: + """Return a stable URI-ordered page for a transactional scan.""" + offset = int(cursor) if cursor else 0 + records = await self.strict_query( + filter=filter, + limit=limit, + offset=offset, + output_fields=output_fields, + order_by="uri", + order_desc=False, + ) + next_cursor = str(offset + len(records)) if len(records) == limit else None + return records, next_cursor + + async def strict_count(self, filter: Optional[Dict[str, Any] | FilterExpr] = None) -> int: + """Count transaction records without converting backend errors to zero.""" + return int( + await self._async_adapter.call("count", filter=self._with_account_filter(filter)) or 0 + ) + async def delete(self, ids: List[str]) -> int: try: if self._bound_account_id: @@ -635,7 +713,7 @@ async def remove_by_uri(self, uri: str) -> int: if any(r.get("level") in [0, 1] for r in target_records): total_deleted += await self._remove_descendants(parent_uri=uri) - ids = [r.get("id") for r in target_records if r.get("id")] + ids = [str(r["id"]) for r in target_records if r.get("id")] if ids: total_deleted += await self.delete(ids) return total_deleted @@ -1254,6 +1332,37 @@ async def scroll( output_fields=output_fields, ) + async def _strict_transfer_page( + self, + ctx: RequestContext, + filter: FilterExpr, + *, + limit: int, + cursor: Optional[str], + output_fields: List[str], + ) -> tuple[List[Dict[str, Any]], Optional[str]]: + backend = self._get_backend_for_context(ctx) + return await backend.strict_scroll( + filter=filter, + limit=limit, + cursor=cursor, + output_fields=output_fields, + ) + + async def _strict_transfer_count(self, ctx: RequestContext, filter: FilterExpr) -> int: + backend = self._get_backend_for_context(ctx) + return await backend.strict_count(filter=filter) + + async def _strict_transfer_get( + self, ctx: RequestContext, ids: List[str] + ) -> List[Dict[str, Any]]: + backend = self._get_backend_for_context(ctx) + return await backend.strict_get(ids) + + async def _strict_transfer_delete(self, ctx: RequestContext, ids: List[str]) -> int: + backend = self._get_backend_for_context(ctx) + return await backend.strict_delete(ids) + async def count( self, filter: Optional[Dict[str, Any] | FilterExpr] = None, @@ -1483,19 +1592,38 @@ async def _scan_uri_transfer_scope( batch_size: int = 100, ) -> tuple[List[Dict[str, Any]], int]: """Scan one URI scope without a fixed total-record limit.""" + transfer_filter = self._uri_transfer_filter(ctx, uri, recursive=recursive) + expected_count = await self._strict_transfer_count(ctx, transfer_filter) records: List[Dict[str, Any]] = [] cursor: Optional[str] = None batches = 0 seen_cursors: set[str] = set() + seen_ids: set[str] = set() + scanned_count = 0 while True: - page, next_cursor = await self.scroll( - filter=self._uri_transfer_filter(ctx, uri, recursive=recursive), + page, next_cursor = await self._strict_transfer_page( + ctx, + transfer_filter, limit=batch_size, cursor=cursor, output_fields=["id", "uri"], - ctx=ctx, ) batches += 1 + if not page and scanned_count < expected_count: + raise RuntimeError( + f"Vector scan ended after {scanned_count} of {expected_count} records under {uri}" + ) + scanned_count += len(page) + for record in page: + record_id = record.get("id") + if not record_id: + raise RuntimeError(f"Vector records without IDs found under {uri}") + normalized_id = str(record_id) + if normalized_id in seen_ids: + raise RuntimeError( + f"Vector scan returned duplicate vector record {normalized_id} under {uri}" + ) + seen_ids.add(normalized_id) scoped = [ record for record in page @@ -1506,7 +1634,7 @@ async def _scan_uri_transfer_scope( ids = [str(record["id"]) for record in scoped if record.get("id")] if len(ids) != len(scoped): raise RuntimeError(f"Vector records without IDs found under {uri}") - full_records = await self.get(ids, ctx=ctx) + full_records = await self._strict_transfer_get(ctx, ids) by_id = {str(record["id"]): record for record in full_records if record.get("id")} if len(by_id) != len(ids): raise RuntimeError(f"Failed to fetch complete vector records under {uri}") @@ -1514,8 +1642,18 @@ async def _scan_uri_transfer_scope( else: records.extend(scoped) - if next_cursor is None: + if scanned_count == expected_count: break + if scanned_count > expected_count: + raise RuntimeError( + f"Vector scan returned {scanned_count} records but count was {expected_count} " + f"under {uri}" + ) + if next_cursor is None: + raise RuntimeError( + f"Vector scan cursor ended after {scanned_count} of {expected_count} records " + f"under {uri}" + ) if next_cursor in seen_cursors: raise RuntimeError(f"Vector scroll cursor repeated under {uri}: {next_cursor}") seen_cursors.add(next_cursor) @@ -1531,7 +1669,21 @@ async def _delete_vector_transfer_ids( ) -> int: deleted = 0 for offset in range(0, len(ids), batch_size): - deleted += int(await self.delete(ids[offset : offset + batch_size], ctx=ctx) or 0) + batch = ids[offset : offset + batch_size] + batch_deleted = await self._strict_transfer_delete(ctx, batch) + residual = await self._strict_transfer_get(ctx, batch) + if residual: + raise RuntimeError( + f"Vector cleanup deleted {batch_deleted} of {len(batch)} records and left " + f"{len(residual)} records" + ) + if batch_deleted != len(batch): + logger.info( + "Vector cleanup removed %s of %s attempted IDs; remaining IDs were never written", + batch_deleted, + len(batch), + ) + deleted += batch_deleted return deleted async def copy_uri_mapping( @@ -1685,7 +1837,7 @@ async def update_uri_mapping( try: for offset in range(0, len(source_ids), 100): source_batch = source_ids[offset : offset + 100] - deleted = int(await self.delete(source_batch, ctx=ctx) or 0) + deleted = await self._strict_transfer_delete(ctx, source_batch) if deleted != len(source_batch): raise RuntimeError( f"Vector move deleted {deleted} of {len(source_batch)} source records" diff --git a/tests/service/test_fs_service.py b/tests/service/test_fs_service.py index 68e37b7f1d..6a935011b8 100644 --- a/tests/service/test_fs_service.py +++ b/tests/service/test_fs_service.py @@ -461,6 +461,7 @@ async def enqueue_refresh(**kwargs): events.append(("refresh", kwargs["root_uri"])) assert kwargs == { "root_uri": "viking://resources/archive", + "source_uri": source, "copied_uri": target, "context_type": "resource", "ctx": request_context, @@ -546,6 +547,7 @@ async def test_copy_refresh_message_only_rebuilds_parent_semantics( status = await service._enqueue_copy_refresh( root_uri="viking://resources/archive", + source_uri="viking://resources/source.md", copied_uri="viking://resources/archive/copied.md", context_type="resource", ctx=request_context, @@ -559,6 +561,7 @@ async def test_copy_refresh_message_only_rebuilds_parent_semantics( assert msg.skip_vectorization is True assert msg.changes == {"added": ["viking://resources/archive/copied.md"]} assert msg.generation_trigger == "content_copy" + assert msg.copy_source_uri == "viking://resources/source.md" @pytest.mark.asyncio diff --git a/tests/storage/test_semantic_dag_incremental.py b/tests/storage/test_semantic_dag_incremental.py index 908d721199..60b9ad2012 100644 --- a/tests/storage/test_semantic_dag_incremental.py +++ b/tests/storage/test_semantic_dag_incremental.py @@ -64,6 +64,7 @@ def __init__(self, viking_fs): self.summarized_files = [] self.sync_calls = [] self.vectorized_files = [] + self.generated_overviews = [] def _parse_overview_md(self, overview_content): results = {} @@ -79,6 +80,7 @@ async def _generate_single_file_summary(self, file_path, llm_sem=None, ctx=None) return {"name": file_path.split("/")[-1], "summary": "summary"} async def _generate_overview(self, dir_uri, file_summaries, children_abstracts, **kwargs): + self.generated_overviews.append(dir_uri) lines = ["FILES:"] for item in file_summaries: name = item.get("name", "") @@ -293,5 +295,54 @@ def make_executor(): assert vectorize_directory.await_count == 2 +@pytest.mark.asyncio +async def test_content_copy_reuses_source_summary_without_resummarizing_file(monkeypatch): + source_parent = "viking://resources/source" + root_uri = "viking://resources/archive" + copied_uri = f"{root_uri}/copied.jpg" + fake_fs = _FakeVikingFS( + tree={root_uri: [{"name": "copied.jpg", "isDir": False}]}, + file_contents={ + copied_uri: "binary-placeholder", + f"{source_parent}/.overview.md": render_abstract_overview( + ContextLevel.OVERVIEW, + source_parent, + "FILES:\n- original.jpg: already summarized", + ), + }, + ) + monkeypatch.setattr( + "openviking.storage.queuefs.semantic_dag.get_viking_fs", lambda: fake_fs + ) + monkeypatch.setattr( + "openviking.storage.queuefs.semantic_dag.get_openviking_config", + lambda: SimpleNamespace(semantic=SimpleNamespace(overview_sample_limit=32)), + ) + processor = _FakeProcessor(fake_fs) + executor = SemanticDagExecutor( + processor=processor, + context_type="resource", + max_concurrent_llm=2, + ctx=RequestContext(user=UserIdentifier("acc1", "user1"), role=Role.USER), + incremental_update=True, + target_uri=root_uri, + recursive=False, + changes={"added": [copied_uri]}, + skip_vectorization=True, + generation_trigger="content_copy", + copy_source_uri=f"{source_parent}/original.jpg", + ) + + await executor.run(root_uri) + + assert processor.summarized_files == [] + assert processor.vectorized_files == [] + assert processor.generated_overviews == [] + overview = parse_abstract_overview( + fake_fs._file_contents[f"{root_uri}/.overview.md"] + ).body + assert "### copied.jpg\nalready summarized" in overview + + if __name__ == "__main__": pytest.main([__file__]) diff --git a/tests/storage/test_semantic_processor_target_preexisting.py b/tests/storage/test_semantic_processor_target_preexisting.py index a11c180f06..8df69a6bc0 100644 --- a/tests/storage/test_semantic_processor_target_preexisting.py +++ b/tests/storage/test_semantic_processor_target_preexisting.py @@ -226,6 +226,29 @@ async def test_memory_trigger_does_not_select_hierarchical_aggregation(monkeypat processor._process_memory_directory.assert_awaited_once() +@pytest.mark.asyncio +async def test_content_copy_does_not_enqueue_ancestor_refresh(monkeypatch): + processor = SemanticProcessor() + plan_refresh = AsyncMock() + monkeypatch.setattr( + "openviking.storage.queuefs.semantic_processor.plan_abstract_overview_refresh", + plan_refresh, + ) + msg = SemanticMsg( + uri="viking://resources/archive", + context_type="resource", + generation_trigger="content_copy", + ) + + await processor._enqueue_parent_refresh( + msg, + "viking://resources/archive/copied.jpg", + l0_body_changed=True, + ) + + plan_refresh.assert_not_awaited() + + @pytest.mark.asyncio async def test_sync_wrapper_delegates_to_sync_tree_and_cleans_temp(monkeypatch): """The wrapper calls viking_fs.sync_tree and then deletes the temp tree.""" diff --git a/tests/storage/test_vector_transfer.py b/tests/storage/test_vector_transfer.py index e6840947ff..59b462d78a 100644 --- a/tests/storage/test_vector_transfer.py +++ b/tests/storage/test_vector_transfer.py @@ -3,7 +3,9 @@ from __future__ import annotations +from types import SimpleNamespace from typing import Any +from unittest.mock import AsyncMock import pytest @@ -12,6 +14,7 @@ from openviking.storage.viking_vector_index_backend import ( VectorTransferRollbackError, VikingVectorIndexBackend, + _SingleAccountBackend, ) from openviking_cli.exceptions import ConflictError from openviking_cli.session.user_id import UserIdentifier @@ -47,6 +50,7 @@ def __init__(self, records: list[dict[str, Any]]) -> None: self.delete_calls = 0 self.fail_delete_at: int | None = None self.partial_delete_count = 0 + self.drop_delete_requests = False self.backend_mode = "local" self.scroll_filters = [] @@ -92,6 +96,8 @@ async def upsert_many( async def delete(self, ids: list[str], *, ctx: RequestContext) -> int: del ctx self.delete_calls += 1 + if self.drop_delete_requests: + return 0 deleted = 0 for record_id in ids: if self.records.pop(record_id, None) is not None: @@ -101,6 +107,25 @@ async def delete(self, ids: list[str], *, ctx: RequestContext) -> int: raise RuntimeError("injected vector delete failure") return deleted + async def _strict_transfer_count(self, ctx, filter): + del ctx, filter + return len(self.records) + + async def _strict_transfer_page(self, ctx, filter, *, limit, cursor, output_fields): + return await self.scroll( + filter=filter, + limit=limit, + cursor=cursor, + output_fields=output_fields, + ctx=ctx, + ) + + async def _strict_transfer_get(self, ctx, ids): + return await self.get(ids, ctx=ctx) + + async def _strict_transfer_delete(self, ctx, ids): + return await self.delete(ids, ctx=ctx) + def _records_under( backend: _MemoryTransferBackend, uri: str, *, recursive: bool = True @@ -133,6 +158,58 @@ async def test_copy_uri_mapping_keeps_source_and_copies_all_pages(): assert len(_records_under(backend, "viking://resources/dst")) == 205 +@pytest.mark.asyncio +async def test_strict_scroll_propagates_real_adapter_query_failure(): + backend = _SingleAccountBackend.__new__(_SingleAccountBackend) + backend._bound_account_id = "acct" + backend._async_adapter = SimpleNamespace( + call=AsyncMock(side_effect=RuntimeError("injected query failure")) + ) + + with pytest.raises(RuntimeError, match="injected query failure"): + await backend.strict_scroll(limit=100, output_fields=["id", "uri"]) + + +@pytest.mark.asyncio +async def test_strict_delete_removes_existing_subset_when_attempted_ids_include_missing(): + adapter_call = AsyncMock( + side_effect=[ + [{"id": "written", "account_id": "acct"}], + 1, + ] + ) + backend = _SingleAccountBackend.__new__(_SingleAccountBackend) + backend._bound_account_id = "acct" + backend._async_adapter = SimpleNamespace(call=adapter_call) + + deleted = await backend.strict_delete(["written", "never-written"]) + + assert deleted == 1 + assert adapter_call.await_args_list[1].args == ("delete",) + assert adapter_call.await_args_list[1].kwargs == {"ids": ["written"]} + + +@pytest.mark.asyncio +async def test_transfer_scan_fails_closed_on_repeated_record_page(): + backend = _MemoryTransferBackend( + [ + _record("source-a", "viking://resources/src/a.md"), + _record("source-b", "viking://resources/src/b.md"), + ] + ) + + async def repeated_page(*_args, **_kwargs): + record = dict(backend.records["source-a"]) + return [record], "1" + + backend._strict_transfer_page = repeated_page + + with pytest.raises(RuntimeError, match="duplicate vector record"): + await backend.copy_uri_mapping( + _ctx(), "viking://resources/src", "viking://resources/dst", recursive=True + ) + + @pytest.mark.asyncio async def test_copy_uri_mapping_preserves_dense_sparse_and_chunk_payloads(): source = "viking://resources/src.md" @@ -241,6 +318,26 @@ async def test_copy_uri_mapping_reports_actual_residual_after_cleanup_failure(): assert len(_records_under(backend, "viking://resources/dst")) == 1 +@pytest.mark.asyncio +async def test_copy_uri_mapping_reports_rollback_when_delete_returns_zero(): + source = "viking://resources/src" + backend = _MemoryTransferBackend( + [ + _record("source-root", source), + _record("source-a", f"{source}/a.md"), + _record("source-b", f"{source}/b.md"), + ] + ) + backend.fail_upsert_at = 3 + backend.drop_delete_requests = True + + with pytest.raises(VectorTransferRollbackError) as exc_info: + await backend.copy_uri_mapping(_ctx(), source, "viking://resources/dst", recursive=True) + + assert exc_info.value.phase == "copy_target_cleanup" + assert exc_info.value.residual_count == 2 + + @pytest.mark.asyncio async def test_update_uri_mapping_deletes_source_only_after_targets_exist(): source = "viking://resources/src" diff --git a/tests/storage/test_viking_fs_cp.py b/tests/storage/test_viking_fs_cp.py index b11d944cb1..38118f896e 100644 --- a/tests/storage/test_viking_fs_cp.py +++ b/tests/storage/test_viking_fs_cp.py @@ -11,7 +11,12 @@ from openviking.server.identity import RequestContext, Role from openviking.storage.viking_fs import VikingFS from openviking.storage.viking_fs._ops import TransferRollbackError -from openviking_cli.exceptions import ConflictError, FailedPreconditionError, InvalidArgumentError +from openviking_cli.exceptions import ( + ConflictError, + FailedPreconditionError, + InvalidArgumentError, + PermissionDeniedError, +) from openviking_cli.session.user_id import UserIdentifier @@ -19,6 +24,14 @@ def _ctx() -> RequestContext: return RequestContext(user=UserIdentifier("acct", "alice"), role=Role.ROOT) +def _user_ctx(*, actor_peer_id: str | None = None) -> RequestContext: + return RequestContext( + user=UserIdentifier("acct", "alice"), + role=Role.USER, + actor_peer_id=actor_peer_id, + ) + + class _CopyAGFS: def __init__( self, @@ -180,6 +193,39 @@ def _viking_fs(monkeypatch, agfs: _CopyAGFS) -> VikingFS: return fs +@pytest.mark.parametrize( + "uri", + ["viking://", "viking://user", "viking://resources", "viking://temp"], +) +def test_cp_rejects_non_root_container_sources(monkeypatch, uri): + fs = _viking_fs(monkeypatch, _CopyAGFS(source_is_dir=True)) + + with pytest.raises(PermissionDeniedError, match="container root"): + fs._ensure_copy_source_access(uri, recursive=True, ctx=_user_ctx()) + + +def test_cp_rejects_watch_control_source(monkeypatch): + fs = _viking_fs(monkeypatch, _CopyAGFS()) + + with pytest.raises(PermissionDeniedError, match="watch-task control"): + fs._ensure_copy_source_access( + "viking://resources/.watch_tasks.json", + recursive=False, + ctx=_ctx(), + ) + + +def test_cp_rejects_actor_peer_scope_that_can_include_hidden_peers(monkeypatch): + fs = _viking_fs(monkeypatch, _CopyAGFS(source_is_dir=True)) + + with pytest.raises(PermissionDeniedError, match="hidden peer"): + fs._ensure_copy_source_access( + "viking://user/alice/peers", + recursive=True, + ctx=_user_ctx(actor_peer_id="peer-a"), + ) + + @pytest.mark.asyncio async def test_cp_file_uses_exact_locks_agfs_copy_then_vector_copy(monkeypatch): agfs = _CopyAGFS() From e5ae38b82e4ddee4ee82dc7e7f0f6f2ef5def29a Mon Sep 17 00:00:00 2001 From: chenpengfei Date: Thu, 20 Aug 2026 14:56:03 +0800 Subject: [PATCH 11/20] fix: retain copied entry in parent semantics --- crates/ov_cli/src/help_ui.rs | 4 ++-- openviking/storage/queuefs/semantic_dag.py | 6 +++-- .../storage/test_semantic_dag_incremental.py | 24 ++++++++++++++++++- 3 files changed, 29 insertions(+), 5 deletions(-) diff --git a/crates/ov_cli/src/help_ui.rs b/crates/ov_cli/src/help_ui.rs index 5cbe8eeadb..985d9da8ed 100644 --- a/crates/ov_cli/src/help_ui.rs +++ b/crates/ov_cli/src/help_ui.rs @@ -295,11 +295,11 @@ const COMMAND_HELP_SPECS: &[CommandHelpSpec] = &[ purpose: "Copy a file or directory without reparsing or regenerating vectors.", examples: &[ HelpItem { - label: "ov cp viking://notes/draft.md viking://notes/draft-copy.md", + label: "ov cp viking://resources/notes/draft.md viking://resources/notes/draft-copy.md", description: "Copy one file and its vector records.", }, HelpItem { - label: "ov cp -r viking://projects/source viking://projects/backup", + label: "ov cp -r viking://resources/projects/source viking://resources/projects/backup", description: "Recursively copy a directory and all vector records.", }, ], diff --git a/openviking/storage/queuefs/semantic_dag.py b/openviking/storage/queuefs/semantic_dag.py index e3d7ceb97c..c650ae8c7f 100644 --- a/openviking/storage/queuefs/semantic_dag.py +++ b/openviking/storage/queuefs/semantic_dag.py @@ -966,6 +966,8 @@ async def _overview_task(self, dir_uri: str) -> None: async with node.lock: file_summaries = self._finalize_file_summaries(node) children_abstracts = await self._finalize_children_abstracts(node) + all_file_summaries = file_summaries + all_children_abstracts = children_abstracts # Freshness describes the directory itself, including direct # entries whose summaries failed. Those entries remain visible # as unsampled coverage instead of disappearing from the count. @@ -981,8 +983,8 @@ async def _overview_task(self, dir_uri: str) -> None: overview = self._build_content_copy_overview( dir_uri, existing_overview, - file_summaries, - children_abstracts, + all_file_summaries, + all_children_abstracts, ) abstract = existing_abstract or "" else: diff --git a/tests/storage/test_semantic_dag_incremental.py b/tests/storage/test_semantic_dag_incremental.py index 60b9ad2012..f3375bf324 100644 --- a/tests/storage/test_semantic_dag_incremental.py +++ b/tests/storage/test_semantic_dag_incremental.py @@ -301,9 +301,25 @@ async def test_content_copy_reuses_source_summary_without_resummarizing_file(mon root_uri = "viking://resources/archive" copied_uri = f"{root_uri}/copied.jpg" fake_fs = _FakeVikingFS( - tree={root_uri: [{"name": "copied.jpg", "isDir": False}]}, + tree={ + root_uri: [ + {"name": "copied.jpg", "isDir": False}, + {"name": "keep.txt", "isDir": False}, + ] + }, file_contents={ copied_uri: "binary-placeholder", + f"{root_uri}/keep.txt": "keep", + f"{root_uri}/.overview.md": render_abstract_overview( + ContextLevel.OVERVIEW, + root_uri, + "FILES:\n- keep.txt: existing summary", + ), + f"{root_uri}/.abstract.md": render_abstract_overview( + ContextLevel.ABSTRACT, + root_uri, + "existing abstract", + ), f"{source_parent}/.overview.md": render_abstract_overview( ContextLevel.OVERVIEW, source_parent, @@ -318,6 +334,12 @@ async def test_content_copy_reuses_source_summary_without_resummarizing_file(mon "openviking.storage.queuefs.semantic_dag.get_openviking_config", lambda: SimpleNamespace(semantic=SimpleNamespace(overview_sample_limit=32)), ) + monkeypatch.setattr( + "openviking.storage.queuefs.semantic_dag.deterministic_sample", + lambda values, _limit: [ + tagged for tagged in values if tagged[1].endswith("/keep.txt") + ], + ) processor = _FakeProcessor(fake_fs) executor = SemanticDagExecutor( processor=processor, From 4c3d7a7d55ce5dc6c967b03742f6a3f5ba555b3a Mon Sep 17 00:00:00 2001 From: chenpengfei Date: Thu, 20 Aug 2026 15:18:42 +0800 Subject: [PATCH 12/20] fix: scan local vectors with supported sort key --- .../storage/viking_vector_index_backend.py | 5 ++- tests/storage/test_vector_transfer.py | 45 +++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/openviking/storage/viking_vector_index_backend.py b/openviking/storage/viking_vector_index_backend.py index a582938f31..0754a7ea9e 100644 --- a/openviking/storage/viking_vector_index_backend.py +++ b/openviking/storage/viking_vector_index_backend.py @@ -567,7 +567,10 @@ async def strict_scroll( limit=limit, offset=offset, output_fields=output_fields, - order_by="uri", + # The local engine's scalar sorter does not return records for + # path/string fields. ``updated_at`` is an indexed date-time field + # on every context collection and provides stable offset pages. + order_by="updated_at", order_desc=False, ) next_cursor = str(offset + len(records)) if len(records) == limit else None diff --git a/tests/storage/test_vector_transfer.py b/tests/storage/test_vector_transfer.py index 59b462d78a..66fc2acca0 100644 --- a/tests/storage/test_vector_transfer.py +++ b/tests/storage/test_vector_transfer.py @@ -10,7 +10,9 @@ import pytest from openviking.server.identity import RequestContext, Role +from openviking.storage.collection_schemas import CollectionSchemas from openviking.storage.expr import And, Contains, Or, PathScope +from openviking.storage.vectordb import engine as vectordb_engine from openviking.storage.viking_vector_index_backend import ( VectorTransferRollbackError, VikingVectorIndexBackend, @@ -18,6 +20,7 @@ ) from openviking_cli.exceptions import ConflictError from openviking_cli.session.user_id import UserIdentifier +from openviking_cli.utils.config.vectordb_config import VectorDBBackendConfig def _ctx() -> RequestContext: @@ -139,6 +142,48 @@ def _records_under( ] +@pytest.mark.asyncio +async def test_copy_uri_mapping_scans_real_local_path_records(tmp_path): + if not getattr(vectordb_engine, "PersistStore", None): + pytest.skip("local persistent vectordb engine is not available in this environment") + + source = "viking://resources/src.md" + target = "viking://resources/dst.md" + backend = VikingVectorIndexBackend( + config=VectorDBBackendConfig( + backend="local", + name="context", + dimension=4, + path=str(tmp_path), + ) + ) + try: + assert await backend.create_collection( + "context", CollectionSchemas.context_collection("context", 4) + ) + assert ( + await backend.upsert( + _record( + "source-file", + source, + vector=[0.1, 0.2, 0.3, 0.4], + created_at="2026-08-20T00:00:00Z", + updated_at="2026-08-20T00:00:00Z", + ), + ctx=_ctx(), + ) + == "source-file" + ) + + result = await backend.copy_uri_mapping(_ctx(), source, target, recursive=False) + + assert result.scanned == 1 + copied = await backend.get_context_by_uri(target, ctx=_ctx()) + assert [record["uri"] for record in copied] == [target] + finally: + await backend.close() + + @pytest.mark.asyncio async def test_copy_uri_mapping_keeps_source_and_copies_all_pages(): source = "viking://resources/src" From 3940dbafae5e8da657e3d5050acc99ecc9b3f0f2 Mon Sep 17 00:00:00 2001 From: chenpengfei Date: Thu, 20 Aug 2026 18:20:37 +0800 Subject: [PATCH 13/20] fix: refresh move target parent semantics --- openviking/service/fs_service.py | 33 +++++++++++++++++++++ tests/service/test_fs_service.py | 50 ++++++++++++++++++++++++++++++-- 2 files changed, 81 insertions(+), 2 deletions(-) diff --git a/openviking/service/fs_service.py b/openviking/service/fs_service.py index cd57b6ef4e..b5a85212ae 100644 --- a/openviking/service/fs_service.py +++ b/openviking/service/fs_service.py @@ -592,12 +592,15 @@ async def mv(self, from_uri: str, to_uri: str, ctx: RequestContext) -> None: watch_manager = self._get_watch_manager() if not watch_manager or context_type_for_uri(from_uri) != "resource": await viking_fs.mv(from_uri, to_uri, ctx=ctx) + await self._refresh_move_target_parent(from_uri=from_uri, to_uri=to_uri, ctx=ctx) return if context_type_for_uri(to_uri) != "resource": await viking_fs.mv(from_uri, to_uri, ctx=ctx) + await self._refresh_move_target_parent(from_uri=from_uri, to_uri=to_uri, ctx=ctx) return if is_watch_task_control_uri(from_uri) or is_watch_task_control_uri(to_uri): await viking_fs.mv(from_uri, to_uri, ctx=ctx) + await self._refresh_move_target_parent(from_uri=from_uri, to_uri=to_uri, ctx=ctx) return transaction_task = asyncio.create_task( @@ -629,6 +632,36 @@ async def mv(self, from_uri: str, to_uri: str, ctx: RequestContext) -> None: exc_info=True, ) raise + await self._refresh_move_target_parent(from_uri=from_uri, to_uri=to_uri, ctx=ctx) + + async def _refresh_move_target_parent( + self, + *, + from_uri: str, + to_uri: str, + ctx: RequestContext, + ) -> None: + """Queue the same target-parent semantic refresh used by cp after mv commits.""" + if is_watch_task_control_uri(from_uri) or is_watch_task_control_uri(to_uri): + return + context_type = context_type_for_uri(to_uri) + refresh_parent_uri = self._semantic_refresh_parent_uri(to_uri, context_type) + if not refresh_parent_uri: + return + try: + await self._enqueue_copy_refresh( + root_uri=refresh_parent_uri, + source_uri=from_uri, + copied_uri=to_uri, + context_type=context_type, + ctx=ctx, + ) + except Exception as exc: + logger.warning( + "Move committed but target parent semantic refresh failed for %s: %s", + to_uri, + exc, + ) async def _move_resource_with_watch_transaction( self, diff --git a/tests/service/test_fs_service.py b/tests/service/test_fs_service.py index 6a935011b8..4a29751e41 100644 --- a/tests/service/test_fs_service.py +++ b/tests/service/test_fs_service.py @@ -377,6 +377,19 @@ async def test_resource_mv_validates_then_moves_then_rewrites_watch_tasks(reques watch_scheduler=_FakeWatchScheduler(watch_manager), ) + async def enqueue_refresh(**kwargs): + events.append(("refresh", kwargs["root_uri"])) + assert kwargs == { + "root_uri": "viking://resources/codeask", + "source_uri": "viking://resources/codeask/wiki", + "copied_uri": "viking://resources/codeask/wiki-renamed", + "context_type": "resource", + "ctx": request_context, + } + return "queued" + + service._enqueue_copy_refresh = enqueue_refresh + await service.mv( "viking://resources/codeask/wiki", "viking://resources/codeask/wiki-renamed", @@ -399,7 +412,7 @@ async def test_resource_mv_validates_then_moves_then_rewrites_watch_tasks(reques "ctx": request_context, } ] - assert [event[0] for event in events] == ["validate", "mv", "rewrite"] + assert [event[0] for event in events] == ["validate", "mv", "rewrite", "refresh"] @pytest.mark.asyncio @@ -425,9 +438,23 @@ async def test_resource_mv_conflict_fails_before_resource_move(request_context): @pytest.mark.asyncio async def test_resource_mv_without_watch_scheduler_moves_resource_directly(request_context): - viking_fs = _FakeVikingFS() + events = [] + viking_fs = _FakeVikingFS(events=events) service = FSService(viking_fs=viking_fs) + async def enqueue_refresh(**kwargs): + events.append(("refresh", kwargs["root_uri"])) + assert kwargs == { + "root_uri": "viking://resources/codeask", + "source_uri": "viking://resources/codeask/wiki", + "copied_uri": "viking://resources/codeask/wiki-renamed", + "context_type": "resource", + "ctx": request_context, + } + return "queued" + + service._enqueue_copy_refresh = enqueue_refresh + await service.mv( "viking://resources/codeask/wiki", "viking://resources/codeask/wiki-renamed", @@ -441,6 +468,25 @@ async def test_resource_mv_without_watch_scheduler_moves_resource_directly(reque "ctx": request_context, } ] + assert [event[0] for event in events] == ["mv", "refresh"] + + +@pytest.mark.asyncio +async def test_resource_mv_watch_control_file_skips_parent_refresh(request_context): + viking_fs = _FakeVikingFS() + service = FSService( + viking_fs=viking_fs, + watch_scheduler=_FakeWatchScheduler(_FakeWatchManager()), + ) + service._enqueue_copy_refresh = AsyncMock() + + await service.mv( + "viking://resources/.watch_tasks.json", + "viking://resources/watch-tasks-backup.json", + ctx=request_context, + ) + + service._enqueue_copy_refresh.assert_not_awaited() @pytest.mark.asyncio From 22215a749a2909ecb71e3b7db001134244eb4abe Mon Sep 17 00:00:00 2001 From: chenpengfei Date: Thu, 20 Aug 2026 19:06:46 +0800 Subject: [PATCH 14/20] fix: report missing copy target directory --- openviking/storage/viking_fs/_ops.py | 6 +++- .../test_api_fs_content_endpoint_suite.py | 29 +++++++++++++++++++ tests/storage/test_viking_fs_cp.py | 15 ++++++++-- 3 files changed, 47 insertions(+), 3 deletions(-) diff --git a/openviking/storage/viking_fs/_ops.py b/openviking/storage/viking_fs/_ops.py index a38752d931..afb5dd2941 100644 --- a/openviking/storage/viking_fs/_ops.py +++ b/openviking/storage/viking_fs/_ops.py @@ -481,7 +481,11 @@ async def _ensure_transfer_parent_directory( parent_stat = await self._async_agfs.stat(parent_path) except Exception as exc: if is_not_found_error(exc): - raise FileNotFoundError(f"{operation} target parent not found: {uri}") from exc + parent_uri = VikingURI(uri).parent + raise NotFoundError( + parent_uri.uri if parent_uri is not None else "", + "directory", + ) from exc mapped = map_exception(exc, resource=uri) if mapped is not None: raise mapped from exc diff --git a/tests/server/test_api_fs_content_endpoint_suite.py b/tests/server/test_api_fs_content_endpoint_suite.py index d332872f41..777283a092 100644 --- a/tests/server/test_api_fs_content_endpoint_suite.py +++ b/tests/server/test_api_fs_content_endpoint_suite.py @@ -184,6 +184,35 @@ async def fake_cp(*args, **kwargs): _assert_error(response, status_code=404, error_code="NOT_FOUND") +async def test_cp_missing_target_parent_reports_directory(app, service, monkeypatch): + target_parent = "viking://resources/missing-parent" + + async def fake_cp(*args, **kwargs): + raise NotFoundError(target_parent, "directory") + + monkeypatch.setattr(service.fs, "cp", fake_cp, raising=False) + response = await _request_with_handler( + app, + "POST", + "/api/v1/fs/cp", + json={ + "from_uri": "viking://resources/source.md", + "to_uri": f"{target_parent}/target.md", + }, + ) + + _assert_error( + response, + status_code=404, + error_code="NOT_FOUND", + message_fragment=f"Directory not found: {target_parent}", + ) + assert response.json()["error"]["details"] == { + "resource": target_parent, + "type": "directory", + } + + async def test_cp_existing_target_returns_conflict(app, service, monkeypatch): async def fake_cp(*args, **kwargs): raise ConflictError("copy target already exists") diff --git a/tests/storage/test_viking_fs_cp.py b/tests/storage/test_viking_fs_cp.py index 38118f896e..5fc918c26a 100644 --- a/tests/storage/test_viking_fs_cp.py +++ b/tests/storage/test_viking_fs_cp.py @@ -15,6 +15,7 @@ ConflictError, FailedPreconditionError, InvalidArgumentError, + NotFoundError, PermissionDeniedError, ) from openviking_cli.session.user_id import UserIdentifier @@ -365,13 +366,18 @@ async def test_cp_rejects_missing_target_parent_before_locking(monkeypatch): fs = _viking_fs(monkeypatch, agfs) monkeypatch.setattr(fs, "_copy_vector_store_uris", AsyncMock(), raising=False) - with pytest.raises(FileNotFoundError, match="target parent not found"): + with pytest.raises(NotFoundError) as exc_info: await fs.cp( "viking://resources/source.md", "viking://resources/target.md", ctx=_ctx(), ) + assert exc_info.value.message == "Directory not found: viking://resources" + assert exc_info.value.details == { + "resource": "viking://resources", + "type": "directory", + } assert not any(event[0] == "acquire-batch" for event in agfs.events) @@ -382,13 +388,18 @@ async def test_mv_rejects_missing_target_parent_before_locking(monkeypatch): monkeypatch.setattr(fs, "_ensure_delete_access", lambda *_args, **_kwargs: None) monkeypatch.setattr(fs, "_update_vector_store_uris", AsyncMock()) - with pytest.raises(FileNotFoundError, match="target parent not found"): + with pytest.raises(NotFoundError) as exc_info: await fs.mv( "viking://resources/source.md", "viking://resources/target.md", ctx=_ctx(), ) + assert exc_info.value.message == "Directory not found: viking://resources" + assert exc_info.value.details == { + "resource": "viking://resources", + "type": "directory", + } assert not any(event[0] == "acquire-batch" for event in agfs.events) From 3f57eeb622f32e2704140482595d836673beec95 Mon Sep 17 00:00:00 2001 From: chenpengfei Date: Thu, 20 Aug 2026 20:47:31 +0800 Subject: [PATCH 15/20] feat: rebuild transfer parent semantics from target summaries --- openviking/service/fs_service.py | 2 +- openviking/storage/abstract_overview.py | 32 ++- openviking/storage/queuefs/semantic_dag.py | 234 +++++++++++------- .../storage/queuefs/semantic_processor.py | 15 ++ .../storage/viking_vector_index_backend.py | 40 +++ tests/service/test_fs_service.py | 2 +- .../storage/test_semantic_dag_incremental.py | 213 ++++++++++++++-- tests/storage/test_vector_transfer.py | 41 ++- 8 files changed, 461 insertions(+), 118 deletions(-) diff --git a/openviking/service/fs_service.py b/openviking/service/fs_service.py index b5a85212ae..bbb6282c2e 100644 --- a/openviking/service/fs_service.py +++ b/openviking/service/fs_service.py @@ -498,7 +498,7 @@ async def _enqueue_copy_refresh( user_id=ctx.user.user_id, peer_id=ctx.user.user_id, role=str(ctx.role), - skip_vectorization=True, + skip_vectorization=False, telemetry_id=telemetry_id, coalesce_key=build_semantic_coalesce_key( context_type=context_type, diff --git a/openviking/storage/abstract_overview.py b/openviking/storage/abstract_overview.py index f11c16f8be..c05abb98a8 100644 --- a/openviking/storage/abstract_overview.py +++ b/openviking/storage/abstract_overview.py @@ -153,6 +153,14 @@ def _normalize_metadata(metadata: Mapping[str, Any]) -> Dict[str, Any]: f"abstract overview freshness.{field} must be a non-negative integer" ) counters[field] = value + if "missing_summary_entries" in freshness: + value = freshness["missing_summary_entries"] + if isinstance(value, bool) or not isinstance(value, int) or value < 0: + raise SemanticSidecarFormatError( + "semantic sidecar freshness.missing_summary_entries " + "must be a non-negative integer" + ) + counters["missing_summary_entries"] = value if counters["sampled_entries"] + counters["unsampled_entries"] != counters["total_entries"]: raise AbstractOverviewFormatError( "abstract overview freshness sampled + unsampled must equal total" @@ -338,20 +346,22 @@ def deterministic_sample(items: Sequence[_T], limit: int) -> list[_T]: def freshness_metadata( - total_entries: int, sampled_entries: int, pending: int = 0 + total_entries: int, + sampled_entries: int, + pending: int = 0, + missing_summary_entries: Optional[int] = None, ) -> Dict[str, int]: """Create validated direct-child freshness counters.""" - return _normalize_metadata( - { - "freshness": { - "total_entries": total_entries, - "sampled_entries": sampled_entries, - "unsampled_entries": total_entries - sampled_entries, - "pending_child_changes": pending, - } - } - )["freshness"] + freshness = { + "total_entries": total_entries, + "sampled_entries": sampled_entries, + "unsampled_entries": total_entries - sampled_entries, + "pending_child_changes": pending, + } + if missing_summary_entries is not None: + freshness["missing_summary_entries"] = missing_summary_entries + return _normalize_metadata({"freshness": freshness})["freshness"] async def _read_existing_document( diff --git a/openviking/storage/queuefs/semantic_dag.py b/openviking/storage/queuefs/semantic_dag.py index c650ae8c7f..dd384ff8c0 100644 --- a/openviking/storage/queuefs/semantic_dag.py +++ b/openviking/storage/queuefs/semantic_dag.py @@ -53,6 +53,9 @@ class DirNode: pending_snapshot: int = 0 sampled_children_dirs: Optional[Set[str]] = None sampled_file_paths: Optional[Set[str]] = None + total_entries: Optional[int] = None + missing_summary_entries: Optional[int] = None + transfer_inputs_ready: bool = True dispatched: bool = False overview_scheduled: bool = False lock: asyncio.Lock = field(default_factory=asyncio.Lock) @@ -381,6 +384,12 @@ async def _dispatch_dir(self, dir_uri: str, parent_uri: Optional[str]) -> bool: try: children_dirs, file_paths = await self._list_dir(dir_uri, "_dispatch_dir") + if self._generation_trigger == "content_copy": + node = await self._prepare_transfer_node(dir_uri, children_dirs, file_paths) + self._nodes[dir_uri] = node + self._schedule_overview(dir_uri) + return False + sample_limit = getattr( get_openviking_config().semantic, "overview_sample_limit", @@ -461,12 +470,111 @@ async def _dispatch_dir(self, dir_uri: str, parent_uri: Optional[str]) -> bool: return False except Exception as e: logger.error(f"Failed to dispatch directory {dir_uri}: {e}", exc_info=True) + if self._generation_trigger == "content_copy": + raise if parent_uri: await self._on_child_done(parent_uri, dir_uri, "") elif self._root_done: self._root_done.set() return True + @staticmethod + def _transfer_summary_ready(value: str) -> bool: + summary = value.strip() + return bool(summary) and not summary.endswith( + ("[Directory abstract is not ready]", "[Directory overview is not ready]") + ) + + async def _load_transfer_candidates( + self, + candidates: List[tuple[str, str]], + ) -> Dict[tuple[str, str], Dict[str, str]]: + file_paths = [uri for kind, uri in candidates if kind == "file"] + file_summaries = await self._processor._load_transfer_file_summaries( + file_paths, ctx=self._ctx + ) + loaded: Dict[tuple[str, str], Dict[str, str]] = {} + for file_path in file_paths: + summary = str(file_summaries.get(file_path) or "").strip() + if self._transfer_summary_ready(summary): + loaded[("file", file_path)] = { + "name": file_path.rsplit("/", 1)[-1], + "summary": summary, + } + + child_uris = [uri for kind, uri in candidates if kind == "directory"] + if child_uris: + results = await asyncio.gather( + *[self._viking_fs.abstract(uri, ctx=self._ctx) for uri in child_uris], + return_exceptions=True, + ) + for child_uri, result in zip(child_uris, results, strict=True): + if isinstance(result, BaseException): + continue + abstract = str(result or "").strip() + if self._transfer_summary_ready(abstract): + loaded[("directory", child_uri)] = { + "name": child_uri.rsplit("/", 1)[-1], + "abstract": abstract, + } + return loaded + + async def _prepare_transfer_node( + self, + dir_uri: str, + children_dirs: List[str], + file_paths: List[str], + ) -> DirNode: + """Sample the target directory first, then read only existing summaries.""" + candidates = sorted( + [("file", uri) for uri in file_paths] + [("directory", uri) for uri in children_dirs], + key=lambda item: (item[1].rsplit("/", 1)[-1], item[0], item[1]), + ) + sample_limit = getattr(get_openviking_config().semantic, "overview_sample_limit", 32) + primary = deterministic_sample(candidates, sample_limit) + loaded = await self._load_transfer_candidates(primary) + selected = [candidate for candidate in primary if candidate in loaded] + missing_primary = len(primary) - len(selected) + + inspected = set(primary) + if missing_primary: + remaining = [candidate for candidate in candidates if candidate not in inspected] + fallback_loaded = await self._load_transfer_candidates(remaining) + loaded.update(fallback_loaded) + ready_remaining = [candidate for candidate in remaining if candidate in fallback_loaded] + selected.extend(deterministic_sample(ready_remaining, missing_primary)) + inspected.update(remaining) + + selected.sort(key=lambda item: (item[1].rsplit("/", 1)[-1], item[0], item[1])) + selected_files = [uri for kind, uri in selected if kind == "file"] + selected_dirs = [uri for kind, uri in selected if kind == "directory"] + missing_count = ( + sum(1 for candidate in inspected if candidate not in loaded) + if len(inspected) == len(candidates) + else None + ) + pending_snapshot = await read_abstract_overview_pending_snapshot( + viking_fs=self._viking_fs, + dir_uri=dir_uri, + ctx=self._ctx, + lock=self._lock, + ) + return DirNode( + uri=dir_uri, + children_dirs=selected_dirs, + file_paths=selected_files, + file_index={uri: idx for idx, uri in enumerate(selected_files)}, + child_index={uri: idx for idx, uri in enumerate(selected_dirs)}, + file_summaries=[loaded[("file", uri)] for uri in selected_files], + children_abstracts=[loaded[("directory", uri)] for uri in selected_dirs], + pending=0, + pending_snapshot=pending_snapshot, + total_entries=len(candidates), + missing_summary_entries=missing_count, + transfer_inputs_ready=bool(selected), + dispatched=True, + ) + async def _list_dir(self, uri: str, from_hint: str) -> tuple[list[str], list[str]]: """List directory entries and return (child_dirs, file_paths).""" try: @@ -598,29 +706,6 @@ async def _read_existing_summary(self, file_path: str) -> Optional[Dict[str, str return None - async def _read_copy_source_summary(self, file_path: str) -> Optional[Dict[str, str]]: - """Reuse the copied file's source summary without parsing or invoking VLM.""" - added = self._changes.get("added", []) - if not self._copy_source_uri or len(added) != 1 or file_path != added[0]: - return await self._read_existing_summary(file_path) - source_parent = self._copy_source_uri.rsplit("/", 1)[0] - source_name = self._copy_source_uri.rsplit("/", 1)[-1] - try: - overview = await self._viking_fs.read_file( - f"{source_parent}/.overview.md", ctx=self._ctx - ) - summaries = self._processor._parse_overview_md(body_for_preview(overview)) - if source_name in summaries: - return { - "name": file_path.rsplit("/", 1)[-1], - "summary": str(summaries[source_name]), - } - except SemanticSidecarFormatError: - raise - except Exception as exc: - logger.debug("Failed to reuse copy source summary for %s: %s", file_path, exc) - return None - async def _check_dir_children_changed( self, dir_uri: str, current_files: List[str], current_dirs: List[str] ) -> bool: @@ -682,13 +767,7 @@ async def _file_summary_task(self, parent_uri: str, file_path: str) -> None: need_vectorize = True try: summary_dict = None - if self._generation_trigger == "content_copy": - summary_dict = await self._read_copy_source_summary(file_path) - if summary_dict is None: - summary_dict = {"name": file_name, "summary": ""} - self._file_change_status[file_path] = file_path in self._changed_paths - need_vectorize = False - elif self._incremental_update: + if self._incremental_update: content_changed = await self._check_file_content_changed(file_path) self._file_change_status[file_path] = content_changed node = self._nodes.get(parent_uri) @@ -848,29 +927,6 @@ def _select_direct_media_overview( return None return summary - def _build_content_copy_overview( - self, - dir_uri: str, - existing_overview: Optional[str], - file_summaries: List[Dict[str, str]], - children_abstracts: List[Dict[str, str]], - ) -> str: - """Patch copied entries into a parent overview without invoking a model.""" - overview = (existing_overview or f"# {dir_uri.rsplit('/', 1)[-1]}").rstrip() - added_names = {path.rsplit("/", 1)[-1] for path in self._changes.get("added", [])} - sections: List[str] = [] - for item in file_summaries: - name = str(item.get("name") or "") - if name in added_names: - sections.append(f"### {name}\n{str(item.get('summary') or '')}".rstrip()) - for item in children_abstracts: - name = str(item.get("name") or "") - if name in added_names: - sections.append(f"### {name}/\n{str(item.get('abstract') or '')}".rstrip()) - if not sections: - return overview - return f"{overview}\n\n" + "\n\n".join(sections) - @property def stale(self) -> bool: return self._stale @@ -908,13 +964,18 @@ async def _write_directory_semantics( total_entries: int, sampled_entries: int, consume_pending: int, + missing_summary_entries: Optional[int] = None, ) -> AbstractOverviewWriteResult: metadata: Dict[str, Any] = { "generated_by": { "component": "SemanticProcessor", "trigger": self._generation_trigger, }, - "freshness": freshness_metadata(total_entries, sampled_entries), + "freshness": freshness_metadata( + total_entries, + sampled_entries, + missing_summary_entries=missing_summary_entries, + ), } if dir_uri == self._root_uri and self._source: metadata["source"] = self._source @@ -951,9 +1012,18 @@ async def _overview_task(self, dir_uri: str) -> None: children_changed = True should_write = True abstract: Optional[str] = None + overview: Optional[str] = None + total_entries = ( + node.total_entries + if node.total_entries is not None + else len(node.file_paths) + len(node.children_dirs) + ) + sampled_inputs: List[tuple[str, Dict[str, str]]] = [] try: - overview: Optional[str] = None - if self._incremental_update: + if self._generation_trigger == "content_copy" and not node.transfer_inputs_ready: + need_vectorize = False + should_write = False + elif self._incremental_update and self._generation_trigger != "content_copy": children_changed = await self._check_dir_children_changed( dir_uri, node.file_paths, node.children_dirs ) @@ -962,49 +1032,41 @@ async def _overview_task(self, dir_uri: str) -> None: need_vectorize = False overview, abstract = await self._read_existing_overview_abstract(dir_uri) should_write = overview is None or abstract is None - if overview is None or abstract is None: + if should_write and (overview is None or abstract is None): async with node.lock: file_summaries = self._finalize_file_summaries(node) children_abstracts = await self._finalize_children_abstracts(node) - all_file_summaries = file_summaries - all_children_abstracts = children_abstracts # Freshness describes the directory itself, including direct # entries whose summaries failed. Those entries remain visible # as unsampled coverage instead of disappearing from the count. - total_entries = len(node.file_paths) + len(node.children_dirs) # Sampling happened in _dispatch_dir, before summary work was - # scheduled. Only the prepared bounded inputs reach the prompt. - sampled_entries = len(file_summaries) + len(children_abstracts) - if self._generation_trigger == "content_copy": - ( - existing_overview, - existing_abstract, - ) = await self._read_existing_overview_abstract(dir_uri) - overview = self._build_content_copy_overview( - dir_uri, - existing_overview, - all_file_summaries, - all_children_abstracts, - ) - abstract = existing_abstract or "" - else: + # scheduled. Transfer refreshes similarly prepare only target + # entries whose existing L2 summaries are ready. + sampled_inputs = sorted( + [("file", item) for item in file_summaries] + + [("directory", item) for item in children_abstracts], + key=lambda tagged: str(tagged[1].get("name") or ""), + ) + sampled_entries = len(sampled_inputs) + if self._generation_trigger != "content_copy": overview = self._select_direct_media_overview(node, file_summaries) - if overview is None: - async with self._llm_sem: - overview = await self._processor._generate_overview( - dir_uri, - file_summaries, - children_abstracts, - total_files=len(node.file_paths), - total_children=len(node.children_dirs), - ) - overview, abstract = self._processor._normalize_overview_generation(overview) + if overview is None: + async with self._llm_sem: + overview = await self._processor._generate_overview( + dir_uri, + file_summaries, + children_abstracts, + total_files=len(node.file_paths), + total_children=len(node.children_dirs), + ) + overview, abstract = self._processor._normalize_overview_generation(overview) if self._closed: return # Write directly, protected by the outer semantic lock. if should_write: + assert overview is not None and abstract is not None try: wrote = await self._write_directory_semantics( dir_uri, @@ -1013,6 +1075,7 @@ async def _overview_task(self, dir_uri: str) -> None: total_entries=total_entries, sampled_entries=sampled_entries, consume_pending=node.pending_snapshot, + missing_summary_entries=node.missing_summary_entries, ) if dir_uri == self._root_uri: self._root_write_result = wrote @@ -1029,6 +1092,7 @@ async def _overview_task(self, dir_uri: str) -> None: logger.error(f"Failed to generate overview for {dir_uri}: {e}", exc_info=True) else: if need_vectorize and not self._skip_vectorization: + assert overview is not None and abstract is not None try: await self._processor._vectorize_directory( dir_uri, diff --git a/openviking/storage/queuefs/semantic_processor.py b/openviking/storage/queuefs/semantic_processor.py index 73180d9759..55e1a5ef6e 100644 --- a/openviking/storage/queuefs/semantic_processor.py +++ b/openviking/storage/queuefs/semantic_processor.py @@ -1569,6 +1569,21 @@ async def _vectorize_directory( creator_acl_grant=creator_acl_grant, ) + async def _load_transfer_file_summaries( + self, + file_paths: List[str], + ctx: Optional[RequestContext] = None, + ) -> Dict[str, str]: + """Load copied/moved file summaries from their existing target L2 vectors.""" + if not file_paths: + return {} + viking_fs = get_viking_fs() + vector_store = viking_fs._get_vector_store() + if vector_store is None: + return {} + active_ctx = ctx or self._default_ctx + return await vector_store.get_l2_abstracts_by_uris(file_paths, ctx=active_ctx) + async def _vectorize_single_file( self, parent_uri: str, diff --git a/openviking/storage/viking_vector_index_backend.py b/openviking/storage/viking_vector_index_backend.py index 0754a7ea9e..1e7cb3a75b 100644 --- a/openviking/storage/viking_vector_index_backend.py +++ b/openviking/storage/viking_vector_index_backend.py @@ -1543,6 +1543,46 @@ async def get_context_by_uri( output_fields=LOOKUP_OUTPUT_FIELDS, ) + async def get_l2_abstracts_by_uris( + self, + uris: List[str], + *, + ctx: RequestContext, + ) -> Dict[str, str]: + """Strictly load existing L2 abstracts for a bounded URI set.""" + requested_by_canonical: Dict[str, str] = {} + for uri in uris: + requested_by_canonical.setdefault(canonicalize_uri(uri, ctx), uri) + canonical_uris = list(requested_by_canonical) + if not canonical_uris: + return {} + + abstracts: Dict[str, str] = {} + chunk_size = 100 + for start in range(0, len(canonical_uris), chunk_size): + chunk = canonical_uris[start : start + chunk_size] + cursor: Optional[str] = None + while True: + records, cursor = await self._strict_transfer_page( + ctx, + And([In("uri", chunk), Eq("level", 2)]), + limit=100, + cursor=cursor, + output_fields=["uri", "abstract", "updated_at"], + ) + for record in records: + uri = str(record.get("uri") or "") + abstract = str(record.get("abstract") or "").strip() + if uri and abstract and uri not in abstracts: + abstracts[uri] = abstract + if cursor is None: + break + return { + requested_by_canonical[uri]: abstract + for uri, abstract in abstracts.items() + if uri in requested_by_canonical + } + async def delete_account_data(self, account_id: str, *, ctx: RequestContext) -> int: """删除指定 account 的所有数据（仅限，root 角色操作）""" self._check_root_role(ctx) diff --git a/tests/service/test_fs_service.py b/tests/service/test_fs_service.py index 4a29751e41..e20a40f561 100644 --- a/tests/service/test_fs_service.py +++ b/tests/service/test_fs_service.py @@ -604,7 +604,7 @@ async def test_copy_refresh_message_only_rebuilds_parent_semantics( msg = queue_manager.messages[0] assert msg.uri == "viking://resources/archive" assert msg.recursive is False - assert msg.skip_vectorization is True + assert msg.skip_vectorization is False assert msg.changes == {"added": ["viking://resources/archive/copied.md"]} assert msg.generation_trigger == "content_copy" assert msg.copy_source_uri == "viking://resources/source.md" diff --git a/tests/storage/test_semantic_dag_incremental.py b/tests/storage/test_semantic_dag_incremental.py index f3375bf324..42838c5203 100644 --- a/tests/storage/test_semantic_dag_incremental.py +++ b/tests/storage/test_semantic_dag_incremental.py @@ -43,6 +43,12 @@ async def stat(self, uri, ctx=None): async def read_file(self, path, ctx=None): return self._file_contents.get(self._norm(path), "") + async def abstract(self, uri, ctx=None): + return self._file_contents.get( + self._norm(f"{uri}/.abstract.md"), + f"# {uri} [Directory abstract is not ready]", + ) + async def write_file(self, path, content, ctx=None, lease_ref=None): norm_path = self._norm(path) self._file_contents[norm_path] = content @@ -59,11 +65,14 @@ def _uri_to_path(self, uri, ctx=None): class _FakeProcessor: - def __init__(self, viking_fs): + def __init__(self, viking_fs, transfer_summaries=None): self._fs = viking_fs + self.transfer_summaries = transfer_summaries or {} + self.transfer_summary_calls = [] self.summarized_files = [] self.sync_calls = [] self.vectorized_files = [] + self.vectorized_dirs = [] self.generated_overviews = [] def _parse_overview_md(self, overview_content): @@ -86,8 +95,20 @@ async def _generate_overview(self, dir_uri, file_summaries, children_abstracts, name = item.get("name", "") summary = item.get("summary", "") lines.append(f"- {name}: {summary}") + for item in children_abstracts: + name = item.get("name", "") + abstract = item.get("abstract", "") + lines.append(f"- {name}/: {abstract}") return "\n".join(lines) + async def _load_transfer_file_summaries(self, file_paths, ctx=None): + self.transfer_summary_calls.append(list(file_paths)) + return { + path: self.transfer_summaries[path] + for path in file_paths + if self.transfer_summaries.get(path) + } + def _normalize_overview_generation(self, overview): return overview, "abstract" @@ -112,7 +133,7 @@ async def _vectorize_directory( ctx=None, ingest_options=None, ): - return None + self.vectorized_dirs.append(uri) async def _sync_topdown_recursive( self, root_uri, target_uri, ctx=None, file_change_status=None, lock=None @@ -296,8 +317,7 @@ def make_executor(): @pytest.mark.asyncio -async def test_content_copy_reuses_source_summary_without_resummarizing_file(monkeypatch): - source_parent = "viking://resources/source" +async def test_content_copy_rebuilds_target_overview_from_target_l2_summaries(monkeypatch): root_uri = "viking://resources/archive" copied_uri = f"{root_uri}/copied.jpg" fake_fs = _FakeVikingFS( @@ -320,11 +340,6 @@ async def test_content_copy_reuses_source_summary_without_resummarizing_file(mon root_uri, "existing abstract", ), - f"{source_parent}/.overview.md": render_abstract_overview( - ContextLevel.OVERVIEW, - source_parent, - "FILES:\n- original.jpg: already summarized", - ), }, ) monkeypatch.setattr( @@ -334,13 +349,13 @@ async def test_content_copy_reuses_source_summary_without_resummarizing_file(mon "openviking.storage.queuefs.semantic_dag.get_openviking_config", lambda: SimpleNamespace(semantic=SimpleNamespace(overview_sample_limit=32)), ) - monkeypatch.setattr( - "openviking.storage.queuefs.semantic_dag.deterministic_sample", - lambda values, _limit: [ - tagged for tagged in values if tagged[1].endswith("/keep.txt") - ], + processor = _FakeProcessor( + fake_fs, + transfer_summaries={ + copied_uri: "copied target L2 summary", + f"{root_uri}/keep.txt": "kept target L2 summary", + }, ) - processor = _FakeProcessor(fake_fs) executor = SemanticDagExecutor( processor=processor, context_type="resource", @@ -350,20 +365,180 @@ async def test_content_copy_reuses_source_summary_without_resummarizing_file(mon target_uri=root_uri, recursive=False, changes={"added": [copied_uri]}, - skip_vectorization=True, + skip_vectorization=False, generation_trigger="content_copy", - copy_source_uri=f"{source_parent}/original.jpg", + copy_source_uri="viking://resources/source/original.jpg", ) await executor.run(root_uri) assert processor.summarized_files == [] assert processor.vectorized_files == [] - assert processor.generated_overviews == [] + assert processor.generated_overviews == [root_uri] + assert processor.vectorized_dirs == [root_uri] overview = parse_abstract_overview( fake_fs._file_contents[f"{root_uri}/.overview.md"] ).body - assert "### copied.jpg\nalready summarized" in overview + abstract = parse_abstract_overview( + fake_fs._file_contents[f"{root_uri}/.abstract.md"] + ).body + assert "- copied.jpg: copied target L2 summary" in overview + assert "- keep.txt: kept target L2 summary" in overview + assert abstract.strip() == "abstract" + + +@pytest.mark.asyncio +async def test_content_copy_samples_before_loading_summaries(monkeypatch): + root_uri = "viking://resources/archive" + file_paths = [f"{root_uri}/{name}.txt" for name in "abcde"] + fake_fs = _FakeVikingFS( + tree={root_uri: [{"name": path.rsplit("/", 1)[-1], "isDir": False} for path in file_paths]}, + file_contents={path: path for path in file_paths}, + ) + monkeypatch.setattr("openviking.storage.queuefs.semantic_dag.get_viking_fs", lambda: fake_fs) + monkeypatch.setattr( + "openviking.storage.queuefs.semantic_dag.get_openviking_config", + lambda: SimpleNamespace(semantic=SimpleNamespace(overview_sample_limit=2)), + ) + processor = _FakeProcessor( + fake_fs, + transfer_summaries={path: f"summary-{path.rsplit('/', 1)[-1]}" for path in file_paths}, + ) + executor = SemanticDagExecutor( + processor=processor, + context_type="resource", + max_concurrent_llm=2, + ctx=RequestContext(user=UserIdentifier("acc1", "user1"), role=Role.USER), + incremental_update=True, + target_uri=root_uri, + recursive=False, + changes={"added": [file_paths[2]]}, + generation_trigger="content_copy", + ) + + await executor.run(root_uri) + + assert processor.transfer_summary_calls == [[file_paths[0], file_paths[-1]]] + assert processor.summarized_files == [] + overview = parse_abstract_overview(fake_fs._file_contents[f"{root_uri}/.overview.md"]).body + assert "a.txt" in overview + assert "e.txt" in overview + assert "c.txt" not in overview + + +@pytest.mark.asyncio +async def test_content_copy_backfills_missing_sample_summary(monkeypatch): + root_uri = "viking://resources/archive" + file_paths = [f"{root_uri}/{name}.txt" for name in "abcde"] + fake_fs = _FakeVikingFS( + tree={root_uri: [{"name": path.rsplit("/", 1)[-1], "isDir": False} for path in file_paths]}, + file_contents={path: path for path in file_paths}, + ) + monkeypatch.setattr("openviking.storage.queuefs.semantic_dag.get_viking_fs", lambda: fake_fs) + monkeypatch.setattr( + "openviking.storage.queuefs.semantic_dag.get_openviking_config", + lambda: SimpleNamespace(semantic=SimpleNamespace(overview_sample_limit=2)), + ) + processor = _FakeProcessor( + fake_fs, + transfer_summaries={path: f"summary-{path.rsplit('/', 1)[-1]}" for path in file_paths[1:]}, + ) + executor = SemanticDagExecutor( + processor=processor, + context_type="resource", + max_concurrent_llm=2, + ctx=RequestContext(user=UserIdentifier("acc1", "user1"), role=Role.USER), + incremental_update=True, + target_uri=root_uri, + recursive=False, + changes={"added": [file_paths[2]]}, + generation_trigger="content_copy", + ) + + await executor.run(root_uri) + + assert processor.transfer_summary_calls[0] == [file_paths[0], file_paths[-1]] + assert processor.transfer_summary_calls[1] == file_paths[1:-1] + overview_doc = parse_abstract_overview(fake_fs._file_contents[f"{root_uri}/.overview.md"]) + assert "a.txt" not in overview_doc.body + assert "b.txt" in overview_doc.body + assert "e.txt" in overview_doc.body + assert overview_doc.metadata["freshness"]["missing_summary_entries"] == 1 + + +@pytest.mark.asyncio +async def test_content_copy_with_no_ready_summaries_preserves_existing_sidecars(monkeypatch): + root_uri = "viking://resources/archive" + file_path = f"{root_uri}/pending.txt" + old_overview = render_abstract_overview(ContextLevel.OVERVIEW, root_uri, "old overview") + old_abstract = render_abstract_overview(ContextLevel.ABSTRACT, root_uri, "old abstract") + fake_fs = _FakeVikingFS( + tree={root_uri: [{"name": "pending.txt", "isDir": False}]}, + file_contents={ + file_path: "pending", + f"{root_uri}/.overview.md": old_overview, + f"{root_uri}/.abstract.md": old_abstract, + }, + ) + monkeypatch.setattr("openviking.storage.queuefs.semantic_dag.get_viking_fs", lambda: fake_fs) + monkeypatch.setattr( + "openviking.storage.queuefs.semantic_dag.get_openviking_config", + lambda: SimpleNamespace(semantic=SimpleNamespace(overview_sample_limit=32)), + ) + processor = _FakeProcessor(fake_fs) + executor = SemanticDagExecutor( + processor=processor, + context_type="resource", + max_concurrent_llm=2, + ctx=RequestContext(user=UserIdentifier("acc1", "user1"), role=Role.USER), + incremental_update=True, + target_uri=root_uri, + recursive=False, + changes={"added": [file_path]}, + generation_trigger="content_copy", + ) + + await executor.run(root_uri) + + assert processor.generated_overviews == [] + assert processor.vectorized_dirs == [] + assert fake_fs._file_contents[f"{root_uri}/.overview.md"] == old_overview + assert fake_fs._file_contents[f"{root_uri}/.abstract.md"] == old_abstract + + +@pytest.mark.asyncio +async def test_content_copy_propagates_vector_summary_read_failure(monkeypatch): + root_uri = "viking://resources/archive" + file_path = f"{root_uri}/copied.txt" + fake_fs = _FakeVikingFS( + tree={root_uri: [{"name": "copied.txt", "isDir": False}]}, + file_contents={file_path: "copied"}, + ) + monkeypatch.setattr("openviking.storage.queuefs.semantic_dag.get_viking_fs", lambda: fake_fs) + monkeypatch.setattr( + "openviking.storage.queuefs.semantic_dag.get_openviking_config", + lambda: SimpleNamespace(semantic=SimpleNamespace(overview_sample_limit=32)), + ) + processor = _FakeProcessor(fake_fs) + + async def fail_summary_read(file_paths, ctx=None): + raise RuntimeError("vector backend unavailable") + + processor._load_transfer_file_summaries = fail_summary_read + executor = SemanticDagExecutor( + processor=processor, + context_type="resource", + max_concurrent_llm=2, + ctx=RequestContext(user=UserIdentifier("acc1", "user1"), role=Role.USER), + incremental_update=True, + target_uri=root_uri, + recursive=False, + changes={"added": [file_path]}, + generation_trigger="content_copy", + ) + + with pytest.raises(RuntimeError, match="vector backend unavailable"): + await executor.run(root_uri) if __name__ == "__main__": diff --git a/tests/storage/test_vector_transfer.py b/tests/storage/test_vector_transfer.py index 66fc2acca0..04bae4949a 100644 --- a/tests/storage/test_vector_transfer.py +++ b/tests/storage/test_vector_transfer.py @@ -11,7 +11,7 @@ from openviking.server.identity import RequestContext, Role from openviking.storage.collection_schemas import CollectionSchemas -from openviking.storage.expr import And, Contains, Or, PathScope +from openviking.storage.expr import And, Contains, Eq, In, Or, PathScope from openviking.storage.vectordb import engine as vectordb_engine from openviking.storage.viking_vector_index_backend import ( VectorTransferRollbackError, @@ -203,6 +203,45 @@ async def test_copy_uri_mapping_keeps_source_and_copies_all_pages(): assert len(_records_under(backend, "viking://resources/dst")) == 205 +@pytest.mark.asyncio +async def test_get_l2_abstracts_by_uris_uses_strict_batched_lookup(): + backend = _MemoryTransferBackend([]) + backend._strict_transfer_page = AsyncMock( + side_effect=[ + ( + [ + { + "uri": "viking://resources/a.md", + "abstract": "summary-a", + "updated_at": 1, + }, + { + "uri": "viking://resources/b.md", + "abstract": "", + "updated_at": 1, + }, + ], + None, + ) + ] + ) + + result = await backend.get_l2_abstracts_by_uris( + ["viking://resources/a.md", "viking://resources/b.md"], + ctx=_ctx(), + ) + + assert result == {"viking://resources/a.md": "summary-a"} + call = backend._strict_transfer_page.await_args + assert call.kwargs["output_fields"] == ["uri", "abstract", "updated_at"] + assert call.args[1] == And( + [ + In("uri", ["viking://resources/a.md", "viking://resources/b.md"]), + Eq("level", 2), + ] + ) + + @pytest.mark.asyncio async def test_strict_scroll_propagates_real_adapter_query_failure(): backend = _SingleAccountBackend.__new__(_SingleAccountBackend) From f1b1a35928c42a50df7fdcb07a5d17559f6021bc Mon Sep 17 00:00:00 2001 From: chenpengfei Date: Fri, 21 Aug 2026 12:11:29 +0800 Subject: [PATCH 16/20] fix: refresh both parents after move --- openviking/service/fs_service.py | 66 +++++++++++-------- openviking/storage/queuefs/semantic_dag.py | 2 +- tests/service/test_fs_service.py | 63 +++++++++++++++--- .../storage/test_semantic_dag_incremental.py | 40 +++++++++++ 4 files changed, 133 insertions(+), 38 deletions(-) diff --git a/openviking/service/fs_service.py b/openviking/service/fs_service.py index bbb6282c2e..d9a0b96f1b 100644 --- a/openviking/service/fs_service.py +++ b/openviking/service/fs_service.py @@ -7,7 +7,7 @@ """ import asyncio -from typing import TYPE_CHECKING, Any, Dict, List, Optional +from typing import TYPE_CHECKING, Any, Dict, List, Literal, Optional from openviking.core.context import ContextLevel from openviking.core.namespace import classify_uri, context_type_for_uri, uri_leaf_name @@ -474,8 +474,9 @@ async def _enqueue_copy_refresh( copied_uri: str, context_type: str, ctx: RequestContext, + change_kind: Literal["added", "deleted"] = "added", ) -> str: - """Queue a parent-only semantic refresh after a committed copy.""" + """Queue a parent-only semantic refresh after a committed transfer.""" await mark_semantic_sidecars_pending( viking_fs=self._viking_fs, dir_uri=root_uri, @@ -507,7 +508,7 @@ async def _enqueue_copy_refresh( user_id=ctx.user.user_id, peer_id=ctx.user.user_id, ), - changes={"added": [copied_uri]}, + changes={change_kind: [copied_uri]}, generation_trigger="content_copy", copy_source_uri=source_uri, ) @@ -592,15 +593,15 @@ async def mv(self, from_uri: str, to_uri: str, ctx: RequestContext) -> None: watch_manager = self._get_watch_manager() if not watch_manager or context_type_for_uri(from_uri) != "resource": await viking_fs.mv(from_uri, to_uri, ctx=ctx) - await self._refresh_move_target_parent(from_uri=from_uri, to_uri=to_uri, ctx=ctx) + await self._refresh_move_parents(from_uri=from_uri, to_uri=to_uri, ctx=ctx) return if context_type_for_uri(to_uri) != "resource": await viking_fs.mv(from_uri, to_uri, ctx=ctx) - await self._refresh_move_target_parent(from_uri=from_uri, to_uri=to_uri, ctx=ctx) + await self._refresh_move_parents(from_uri=from_uri, to_uri=to_uri, ctx=ctx) return if is_watch_task_control_uri(from_uri) or is_watch_task_control_uri(to_uri): await viking_fs.mv(from_uri, to_uri, ctx=ctx) - await self._refresh_move_target_parent(from_uri=from_uri, to_uri=to_uri, ctx=ctx) + await self._refresh_move_parents(from_uri=from_uri, to_uri=to_uri, ctx=ctx) return transaction_task = asyncio.create_task( @@ -632,36 +633,47 @@ async def mv(self, from_uri: str, to_uri: str, ctx: RequestContext) -> None: exc_info=True, ) raise - await self._refresh_move_target_parent(from_uri=from_uri, to_uri=to_uri, ctx=ctx) + await self._refresh_move_parents(from_uri=from_uri, to_uri=to_uri, ctx=ctx) - async def _refresh_move_target_parent( + async def _refresh_move_parents( self, *, from_uri: str, to_uri: str, ctx: RequestContext, ) -> None: - """Queue the same target-parent semantic refresh used by cp after mv commits.""" + """Queue transfer refreshes for both affected parents after mv commits.""" if is_watch_task_control_uri(from_uri) or is_watch_task_control_uri(to_uri): return - context_type = context_type_for_uri(to_uri) - refresh_parent_uri = self._semantic_refresh_parent_uri(to_uri, context_type) - if not refresh_parent_uri: - return - try: - await self._enqueue_copy_refresh( - root_uri=refresh_parent_uri, - source_uri=from_uri, - copied_uri=to_uri, - context_type=context_type, - ctx=ctx, - ) - except Exception as exc: - logger.warning( - "Move committed but target parent semantic refresh failed for %s: %s", - to_uri, - exc, - ) + + source_context_type = context_type_for_uri(from_uri) + target_context_type = context_type_for_uri(to_uri) + source_parent_uri = self._semantic_refresh_parent_uri(from_uri, source_context_type) + target_parent_uri = self._semantic_refresh_parent_uri(to_uri, target_context_type) + + refreshes: List[tuple[str, str, Literal["added", "deleted"], str]] = [] + if source_parent_uri and source_parent_uri != target_parent_uri: + refreshes.append((source_parent_uri, from_uri, "deleted", source_context_type)) + if target_parent_uri: + refreshes.append((target_parent_uri, to_uri, "added", target_context_type)) + + for parent_uri, changed_uri, change_kind, context_type in refreshes: + try: + await self._enqueue_copy_refresh( + root_uri=parent_uri, + source_uri=from_uri, + copied_uri=changed_uri, + change_kind=change_kind, + context_type=context_type, + ctx=ctx, + ) + except Exception as exc: + logger.warning( + "Move committed but %s parent semantic refresh failed for %s: %s", + change_kind, + parent_uri, + exc, + ) async def _move_resource_with_watch_transaction( self, diff --git a/openviking/storage/queuefs/semantic_dag.py b/openviking/storage/queuefs/semantic_dag.py index dd384ff8c0..d60781bad8 100644 --- a/openviking/storage/queuefs/semantic_dag.py +++ b/openviking/storage/queuefs/semantic_dag.py @@ -571,7 +571,7 @@ async def _prepare_transfer_node( pending_snapshot=pending_snapshot, total_entries=len(candidates), missing_summary_entries=missing_count, - transfer_inputs_ready=bool(selected), + transfer_inputs_ready=not candidates or bool(selected), dispatched=True, ) diff --git a/tests/service/test_fs_service.py b/tests/service/test_fs_service.py index e20a40f561..8b44577a43 100644 --- a/tests/service/test_fs_service.py +++ b/tests/service/test_fs_service.py @@ -383,6 +383,7 @@ async def enqueue_refresh(**kwargs): "root_uri": "viking://resources/codeask", "source_uri": "viking://resources/codeask/wiki", "copied_uri": "viking://resources/codeask/wiki-renamed", + "change_kind": "added", "context_type": "resource", "ctx": request_context, } @@ -441,34 +442,47 @@ async def test_resource_mv_without_watch_scheduler_moves_resource_directly(reque events = [] viking_fs = _FakeVikingFS(events=events) service = FSService(viking_fs=viking_fs) + refresh_calls = [] async def enqueue_refresh(**kwargs): events.append(("refresh", kwargs["root_uri"])) - assert kwargs == { - "root_uri": "viking://resources/codeask", - "source_uri": "viking://resources/codeask/wiki", - "copied_uri": "viking://resources/codeask/wiki-renamed", - "context_type": "resource", - "ctx": request_context, - } + refresh_calls.append(kwargs) return "queued" service._enqueue_copy_refresh = enqueue_refresh await service.mv( "viking://resources/codeask/wiki", - "viking://resources/codeask/wiki-renamed", + "viking://resources/archive/wiki", ctx=request_context, ) assert viking_fs.mv_calls == [ { "from_uri": "viking://resources/codeask/wiki", - "to_uri": "viking://resources/codeask/wiki-renamed", + "to_uri": "viking://resources/archive/wiki", "ctx": request_context, } ] - assert [event[0] for event in events] == ["mv", "refresh"] + assert refresh_calls == [ + { + "root_uri": "viking://resources/codeask", + "source_uri": "viking://resources/codeask/wiki", + "copied_uri": "viking://resources/codeask/wiki", + "change_kind": "deleted", + "context_type": "resource", + "ctx": request_context, + }, + { + "root_uri": "viking://resources/archive", + "source_uri": "viking://resources/codeask/wiki", + "copied_uri": "viking://resources/archive/wiki", + "change_kind": "added", + "context_type": "resource", + "ctx": request_context, + }, + ] + assert [event[0] for event in events] == ["mv", "refresh", "refresh"] @pytest.mark.asyncio @@ -610,6 +624,35 @@ async def test_copy_refresh_message_only_rebuilds_parent_semantics( assert msg.copy_source_uri == "viking://resources/source.md" +@pytest.mark.asyncio +async def test_transfer_refresh_message_records_deleted_source_entry( + request_context, + monkeypatch, +): + service = FSService(viking_fs=_FakeVikingFS()) + queue_manager = _FakeQueueManager() + monkeypatch.setattr( + "openviking.service.fs_service.get_queue_manager", + lambda: queue_manager, + ) + monkeypatch.setattr( + "openviking.service.fs_service.mark_semantic_sidecars_pending", + AsyncMock(), + ) + + status = await service._enqueue_copy_refresh( + root_uri="viking://resources/source", + source_uri="viking://resources/source/moved.md", + copied_uri="viking://resources/source/moved.md", + change_kind="deleted", + context_type="resource", + ctx=request_context, + ) + + assert status == "queued" + assert queue_manager.messages[0].changes == {"deleted": ["viking://resources/source/moved.md"]} + + @pytest.mark.asyncio async def test_resource_rm_wait_registers_request_before_semantic_root( request_context, diff --git a/tests/storage/test_semantic_dag_incremental.py b/tests/storage/test_semantic_dag_incremental.py index 42838c5203..cc4d4bf052 100644 --- a/tests/storage/test_semantic_dag_incremental.py +++ b/tests/storage/test_semantic_dag_incremental.py @@ -506,6 +506,46 @@ async def test_content_copy_with_no_ready_summaries_preserves_existing_sidecars( assert fake_fs._file_contents[f"{root_uri}/.abstract.md"] == old_abstract +@pytest.mark.asyncio +async def test_content_copy_rebuilds_semantics_when_move_leaves_source_directory_empty(monkeypatch): + root_uri = "viking://resources/source" + old_overview = render_semantic_sidecar(ContextLevel.OVERVIEW, root_uri, "old file summary") + old_abstract = render_semantic_sidecar(ContextLevel.ABSTRACT, root_uri, "old abstract") + fake_fs = _FakeVikingFS( + tree={root_uri: []}, + file_contents={ + f"{root_uri}/.overview.md": old_overview, + f"{root_uri}/.abstract.md": old_abstract, + }, + ) + monkeypatch.setattr("openviking.storage.queuefs.semantic_dag.get_viking_fs", lambda: fake_fs) + monkeypatch.setattr( + "openviking.storage.queuefs.semantic_dag.get_openviking_config", + lambda: SimpleNamespace(semantic=SimpleNamespace(sidecar_sample_size=32)), + ) + processor = _FakeProcessor(fake_fs) + executor = SemanticDagExecutor( + processor=processor, + context_type="resource", + max_concurrent_llm=2, + ctx=RequestContext(user=UserIdentifier("acc1", "user1"), role=Role.USER), + incremental_update=True, + target_uri=root_uri, + recursive=False, + changes={"deleted": [f"{root_uri}/moved.txt"]}, + generation_trigger="content_copy", + ) + + await executor.run(root_uri) + + assert processor.generated_overviews == [root_uri] + assert processor.vectorized_dirs == [root_uri] + overview = parse_semantic_sidecar(fake_fs._file_contents[f"{root_uri}/.overview.md"]).body + abstract = parse_semantic_sidecar(fake_fs._file_contents[f"{root_uri}/.abstract.md"]).body + assert overview.strip() == "FILES:" + assert abstract.strip() == "abstract" + + @pytest.mark.asyncio async def test_content_copy_propagates_vector_summary_read_failure(monkeypatch): root_uri = "viking://resources/archive" From 1ef061508acd6e13c47d14275c54a6f9f1831685 Mon Sep 17 00:00:00 2001 From: chenpengfei Date: Fri, 21 Aug 2026 13:50:50 +0800 Subject: [PATCH 17/20] docs: document filesystem copy API --- crates/ov_cli/README.md | 7 +++ crates/ov_cli/README_CN.md | 7 +++ docs/en/api/01-overview.md | 1 + docs/en/api/03-filesystem.md | 84 ++++++++++++++++++++++++++++++++++++ docs/zh/api/01-overview.md | 1 + docs/zh/api/03-filesystem.md | 84 ++++++++++++++++++++++++++++++++++++ 6 files changed, 184 insertions(+) diff --git a/crates/ov_cli/README.md b/crates/ov_cli/README.md index ce42a041ae..c236a7aad5 100644 --- a/crates/ov_cli/README.md +++ b/crates/ov_cli/README.md @@ -130,11 +130,18 @@ Run `ov --help` and `ov --help` for the exact command surface of your - `tree` - Show a hierarchical tree. - `mkdir` - Create a directory. - `rm` - Remove a resource or directory. +- `cp` - Copy a file, or copy a directory recursively with `-r`. - `mv` - Move or rename a resource. - `stat` - Show resource metadata. - `attrs` - Get logical extended attributes. - `get` - Download a file to a local path. +```bash +# The destination parent must exist and the destination itself must not exist. +ov cp viking://resources/docs/guide.md viking://resources/archive/guide-copy.md +ov cp -r viking://resources/docs viking://resources/docs-backup +``` + ### Content Access - `read` - Read L2 full content. diff --git a/crates/ov_cli/README_CN.md b/crates/ov_cli/README_CN.md index 1313c8f50a..9ad2c65046 100644 --- a/crates/ov_cli/README_CN.md +++ b/crates/ov_cli/README_CN.md @@ -130,11 +130,18 @@ ov grep "openviking" --uri viking://resources - `tree` - 显示目录树。 - `mkdir` - 创建目录。 - `rm` - 删除资源或目录。 +- `cp` - 复制文件，或使用 `-r` 递归复制目录。 - `mv` - 移动或重命名资源。 - `stat` - 查看资源元数据。 - `attrs` - 获取逻辑扩展属性。 - `get` - 下载文件到本地路径。 +```bash +# 目标父目录必须已存在，目标本身必须不存在。 +ov cp viking://resources/docs/guide.md viking://resources/archive/guide-copy.md +ov cp -r viking://resources/docs viking://resources/docs-backup +``` + ### 内容访问 - `read` - 读取 L2 全量内容。 diff --git a/docs/en/api/01-overview.md b/docs/en/api/01-overview.md index 15cd2979a2..8f398f5d07 100644 --- a/docs/en/api/01-overview.md +++ b/docs/en/api/01-overview.md @@ -366,6 +366,7 @@ This catalog follows the routes actually mounted by the server. Each group headi | POST | `/api/v1/fs/attrs/set_tags` | Set retrieval tags (compatibility alias) | | POST | `/api/v1/fs/mkdir` | Create a directory | | DELETE | `/api/v1/fs` | Delete a resource | +| POST | `/api/v1/fs/cp` | Copy a file or directory together with its vector records | | POST | `/api/v1/fs/mv` | Move or rename a resource | ### [ACL](12-acl.md) diff --git a/docs/en/api/03-filesystem.md b/docs/en/api/03-filesystem.md index 48c3f43ac2..423ba604f9 100644 --- a/docs/en/api/03-filesystem.md +++ b/docs/en/api/03-filesystem.md @@ -616,6 +616,90 @@ When deleting `viking://resources/...`, the response may include `memory_cleanup --- +### cp() + +Copy a file or directory to a new Viking URI. The source remains unchanged. Existing vector records under the source URI are copied and rewritten for the destination, so the copied content does not need to be parsed, described by a VLM, or embedded again. + +The destination parent directory must already exist, and the destination itself must not exist. Copying a directory requires `recursive=true` (or `-r` in the CLI). The destination cannot equal the source or be inside the source directory tree. + +**Parameters** + +| Parameter | Type | Required | Default | Description | +|-----------|------|----------|---------|-------------| +| from_uri | str | Yes | - | Source Viking URI | +| to_uri | str | Yes | - | Destination Viking URI, including the new file or directory name | +| recursive | bool | No | False | Required when the source is a directory | + +**HTTP API** + +``` +POST /api/v1/fs/cp +``` + +```bash +# Copy one file +curl -X POST http://localhost:1933/api/v1/fs/cp \ + -H "Content-Type: application/json" \ + -H "X-API-Key: your-key" \ + -d '{ + "from_uri": "viking://resources/docs/guide.md", + "to_uri": "viking://resources/archive/guide-copy.md", + "recursive": false + }' + +# Copy a directory recursively +curl -X POST http://localhost:1933/api/v1/fs/cp \ + -H "Content-Type: application/json" \ + -H "X-API-Key: your-key" \ + -d '{ + "from_uri": "viking://resources/docs", + "to_uri": "viking://resources/docs-backup", + "recursive": true + }' +``` + +**CLI** + +```bash +# Copy one file +ov cp viking://resources/docs/guide.md viking://resources/archive/guide-copy.md + +# Copy a directory recursively +ov cp -r viking://resources/docs viking://resources/docs-backup +``` + +**Response** + +```json +{ + "status": "ok", + "result": { + "operation_id": "61ec2a80bf5f46a28aa3497fbdcb56dd", + "operation": "copy", + "from": "viking://resources/docs/guide.md", + "to": "viking://resources/archive/guide-copy.md", + "recursive": false, + "phase": "completed", + "files_created": 1, + "vectors": { + "scanned": 3, + "written": 3, + "deleted": 0, + "restored": 0, + "batches": 1 + }, + "semantic_root_uri": "viking://resources/archive", + "semantic_status": "queued" + } +} +``` + +`semantic_status: "queued"` means the copy has already committed and the destination parent's overview and abstract will be rebuilt asynchronously from summaries available at the destination. The API does not wait for that refresh. A refresh enqueue failure may return `semantic_status: "failed"` and `semantic_error`; it does not roll back the completed file and vector copy. + +Common errors include `NOT_FOUND` when the source or destination parent is missing, `CONFLICT` when the destination already exists or a path lock is busy, `FAILED_PRECONDITION` when a directory is copied without `recursive=true`, and `INVALID_ARGUMENT` for invalid source/destination relationships. + +--- + ### mv() Move file or directory. diff --git a/docs/zh/api/01-overview.md b/docs/zh/api/01-overview.md index 2b4dc43be1..2e821db6af 100644 --- a/docs/zh/api/01-overview.md +++ b/docs/zh/api/01-overview.md @@ -361,6 +361,7 @@ JSON 输出 - 错误： | POST | `/api/v1/fs/attrs/set_tags` | 设置检索标签（兼容别名） | | POST | `/api/v1/fs/mkdir` | 创建目录 | | DELETE | `/api/v1/fs` | 删除资源 | +| POST | `/api/v1/fs/cp` | 复制文件或目录及其向量记录 | | POST | `/api/v1/fs/mv` | 移动或重命名资源 | ### [ACL](12-acl.md) diff --git a/docs/zh/api/03-filesystem.md b/docs/zh/api/03-filesystem.md index e3713367a4..3cec281bd9 100644 --- a/docs/zh/api/03-filesystem.md +++ b/docs/zh/api/03-filesystem.md @@ -608,6 +608,90 @@ openviking rm viking://resources/old.md [--recursive] --- +### cp() + +把文件或目录复制到新的 Viking URI，源内容保持不变。源 URI 下已有的向量记录会同步复制并改写为目标 URI，因此无需重新解析复制内容，也无需重新执行文件级 VLM 或 embedding。 + +目标父目录必须已经存在，目标本身必须不存在。复制目录时必须设置 `recursive=true`（CLI 中使用 `-r`）。目标不能与源相同，也不能位于源目录子树内。 + +**参数** + +| 参数 | 类型 | 必填 | 默认值 | 说明 | +|------|------|------|--------|------| +| from_uri | str | 是 | - | 源 Viking URI | +| to_uri | str | 是 | - | 目标 Viking URI，必须包含新的文件名或目录名 | +| recursive | bool | 否 | False | 源为目录时必须设为 `true` | + +**HTTP API** + +``` +POST /api/v1/fs/cp +``` + +```bash +# 复制单个文件 +curl -X POST http://localhost:1933/api/v1/fs/cp \ + -H "Content-Type: application/json" \ + -H "X-API-Key: your-key" \ + -d '{ + "from_uri": "viking://resources/docs/guide.md", + "to_uri": "viking://resources/archive/guide-copy.md", + "recursive": false + }' + +# 递归复制目录 +curl -X POST http://localhost:1933/api/v1/fs/cp \ + -H "Content-Type: application/json" \ + -H "X-API-Key: your-key" \ + -d '{ + "from_uri": "viking://resources/docs", + "to_uri": "viking://resources/docs-backup", + "recursive": true + }' +``` + +**CLI** + +```bash +# 复制单个文件 +ov cp viking://resources/docs/guide.md viking://resources/archive/guide-copy.md + +# 递归复制目录 +ov cp -r viking://resources/docs viking://resources/docs-backup +``` + +**响应** + +```json +{ + "status": "ok", + "result": { + "operation_id": "61ec2a80bf5f46a28aa3497fbdcb56dd", + "operation": "copy", + "from": "viking://resources/docs/guide.md", + "to": "viking://resources/archive/guide-copy.md", + "recursive": false, + "phase": "completed", + "files_created": 1, + "vectors": { + "scanned": 3, + "written": 3, + "deleted": 0, + "restored": 0, + "batches": 1 + }, + "semantic_root_uri": "viking://resources/archive", + "semantic_status": "queued" + } +} +``` + +`semantic_status: "queued"` 表示复制已经提交，目标父目录的 overview 和 abstract 将根据目标目录中已有的摘要异步重建，接口不会等待刷新完成。若语义刷新入队失败，响应可能包含 `semantic_status: "failed"` 和 `semantic_error`；已经完成的文件和向量复制不会因此回滚。 + +常见错误包括：源或目标父目录不存在时返回 `NOT_FOUND`；目标已存在或路径锁繁忙时返回 `CONFLICT`；复制目录但未设置 `recursive=true` 时返回 `FAILED_PRECONDITION`；源和目标关系非法时返回 `INVALID_ARGUMENT`。 + +--- + ### mv() 移动文件或目录。 From 5119be45ae1b61184c9adc6efde653dffb9f1b74 Mon Sep 17 00:00:00 2001 From: chenpengfei Date: Fri, 21 Aug 2026 14:19:06 +0800 Subject: [PATCH 18/20] fix: adapt copy flow to canonical URI boundary --- openviking/server/routers/filesystem.py | 4 +++- openviking/service/fs_service.py | 2 -- openviking/storage/viking_vector_index_backend.py | 2 +- tests/api_test/api/client.py | 1 + 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/openviking/server/routers/filesystem.py b/openviking/server/routers/filesystem.py index c10b0a38c9..073dfb1793 100644 --- a/openviking/server/routers/filesystem.py +++ b/openviking/server/routers/filesystem.py @@ -39,7 +39,9 @@ def _clean_memory_attrs(raw: str) -> dict[str, Any]: return attrs -async def _tags_attr(service: Any, uri: str, ctx: RequestContext, *, is_dir: bool) -> list[str]: +async def _tags_attr( + service: Any, uri: str, ctx: RequestContext, *, is_dir: bool +) -> list[str]: vikingdb_manager = getattr(service, "vikingdb_manager", None) if not vikingdb_manager: return [] diff --git a/openviking/service/fs_service.py b/openviking/service/fs_service.py index d9a0b96f1b..5c4d936d25 100644 --- a/openviking/service/fs_service.py +++ b/openviking/service/fs_service.py @@ -545,8 +545,6 @@ async def cp( ctx: RequestContext, ) -> Dict[str, Any]: """Copy a resource and queue derived parent semantics after commit.""" - from_uri = validate_viking_uri(from_uri, field_name="from_uri") - to_uri = validate_viking_uri(to_uri, field_name="to_uri") viking_fs = self._ensure_initialized() async with self._uri_mutation_coordinator.mutation( ctx.account_id, diff --git a/openviking/storage/viking_vector_index_backend.py b/openviking/storage/viking_vector_index_backend.py index 1e7cb3a75b..7696615561 100644 --- a/openviking/storage/viking_vector_index_backend.py +++ b/openviking/storage/viking_vector_index_backend.py @@ -1552,7 +1552,7 @@ async def get_l2_abstracts_by_uris( """Strictly load existing L2 abstracts for a bounded URI set.""" requested_by_canonical: Dict[str, str] = {} for uri in uris: - requested_by_canonical.setdefault(canonicalize_uri(uri, ctx), uri) + requested_by_canonical.setdefault(resolve_uri(uri).uri, uri) canonical_uris = list(requested_by_canonical) if not canonical_uris: return {} diff --git a/tests/api_test/api/client.py b/tests/api_test/api/client.py index 32eb81e79d..4e2e5b488a 100644 --- a/tests/api_test/api/client.py +++ b/tests/api_test/api/client.py @@ -492,6 +492,7 @@ def fs_cp( "recursive": recursive, }, ) + def session_used( self, session_id: str, From dec8c5987ceefa9a637d63cf99e5232f313a2558 Mon Sep 17 00:00:00 2001 From: chenpengfei Date: Mon, 24 Aug 2026 14:55:34 +0800 Subject: [PATCH 19/20] fix: adapt transfer semantics after upstream rebase --- openviking/service/fs_service.py | 3 +- openviking/storage/abstract_overview.py | 26 ++++++------- openviking/storage/queuefs/semantic_dag.py | 4 +- tests/service/test_fs_service.py | 4 +- .../storage/test_semantic_dag_incremental.py | 38 ++++++------------- 5 files changed, 29 insertions(+), 46 deletions(-) diff --git a/openviking/service/fs_service.py b/openviking/service/fs_service.py index 5c4d936d25..f4d07cb893 100644 --- a/openviking/service/fs_service.py +++ b/openviking/service/fs_service.py @@ -22,6 +22,7 @@ from openviking.session.memory.memory_updater import MemoryUpdater from openviking.session.memory.utils.content_visibility import visible_content from openviking.storage.abstract_overview import ( + mark_abstract_overview_pending, plan_abstract_overview_refresh, render_abstract_overview, ) @@ -477,7 +478,7 @@ async def _enqueue_copy_refresh( change_kind: Literal["added", "deleted"] = "added", ) -> str: """Queue a parent-only semantic refresh after a committed transfer.""" - await mark_semantic_sidecars_pending( + await mark_abstract_overview_pending( viking_fs=self._viking_fs, dir_uri=root_uri, changed_entries=1, diff --git a/openviking/storage/abstract_overview.py b/openviking/storage/abstract_overview.py index c05abb98a8..19e9aa4f74 100644 --- a/openviking/storage/abstract_overview.py +++ b/openviking/storage/abstract_overview.py @@ -139,7 +139,9 @@ def _normalize_metadata(metadata: Mapping[str, Any]) -> Dict[str, Any]: "pending_child_changes", } if not isinstance(freshness, Mapping) or not required.issubset(freshness): - raise AbstractOverviewFormatError("abstract overview freshness has an invalid field set") + raise AbstractOverviewFormatError( + "abstract overview freshness has an invalid field set" + ) counters: Dict[str, int] = {} for field in ( "total_entries", @@ -156,8 +158,8 @@ def _normalize_metadata(metadata: Mapping[str, Any]) -> Dict[str, Any]: if "missing_summary_entries" in freshness: value = freshness["missing_summary_entries"] if isinstance(value, bool) or not isinstance(value, int) or value < 0: - raise SemanticSidecarFormatError( - "semantic sidecar freshness.missing_summary_entries " + raise AbstractOverviewFormatError( + "abstract overview freshness.missing_summary_entries " "must be a non-negative integer" ) counters["missing_summary_entries"] = value @@ -430,19 +432,15 @@ async def write_abstract_overview( ) next_freshness = dict(requested_freshness) consumed = current_pending if consume_pending is None else max(consume_pending, 0) - next_freshness["pending_child_changes"] = max( - current_pending - consumed, 0 - ) + next_freshness["pending_child_changes"] = max(current_pending - consumed, 0) merged_metadata["freshness"] = next_freshness - overview_body_changed = ( - existing_overview is None - or semantic_body_digest(existing_overview.body) != semantic_body_digest(overview) - ) - abstract_body_changed = ( - existing_abstract is None - or semantic_body_digest(existing_abstract.body) != semantic_body_digest(abstract) - ) + overview_body_changed = existing_overview is None or semantic_body_digest( + existing_overview.body + ) != semantic_body_digest(overview) + abstract_body_changed = existing_abstract is None or semantic_body_digest( + existing_abstract.body + ) != semantic_body_digest(abstract) rendered_overview = render_abstract_overview( ContextLevel.OVERVIEW, dir_uri, overview, merged_metadata diff --git a/openviking/storage/queuefs/semantic_dag.py b/openviking/storage/queuefs/semantic_dag.py index d60781bad8..7b07aa36c7 100644 --- a/openviking/storage/queuefs/semantic_dag.py +++ b/openviking/storage/queuefs/semantic_dag.py @@ -401,9 +401,7 @@ async def _dispatch_dir(self, dir_uri: str, parent_uri: Optional[str]) -> bool: key=lambda item: item[1].rsplit("/", 1)[-1], ) sampled_entries = deterministic_sample(direct_entries, sample_limit) - sampled_children_dirs = { - uri for kind, uri in sampled_entries if kind == "directory" - } + sampled_children_dirs = {uri for kind, uri in sampled_entries if kind == "directory"} sampled_file_paths = {uri for kind, uri in sampled_entries if kind == "file"} pending_snapshot = ( await read_abstract_overview_pending_snapshot( diff --git a/tests/service/test_fs_service.py b/tests/service/test_fs_service.py index 8b44577a43..8fc2db8a55 100644 --- a/tests/service/test_fs_service.py +++ b/tests/service/test_fs_service.py @@ -601,7 +601,7 @@ async def test_copy_refresh_message_only_rebuilds_parent_semantics( lambda: queue_manager, ) monkeypatch.setattr( - "openviking.service.fs_service.mark_semantic_sidecars_pending", + "openviking.service.fs_service.mark_abstract_overview_pending", mark_pending, ) @@ -636,7 +636,7 @@ async def test_transfer_refresh_message_records_deleted_source_entry( lambda: queue_manager, ) monkeypatch.setattr( - "openviking.service.fs_service.mark_semantic_sidecars_pending", + "openviking.service.fs_service.mark_abstract_overview_pending", AsyncMock(), ) diff --git a/tests/storage/test_semantic_dag_incremental.py b/tests/storage/test_semantic_dag_incremental.py index cc4d4bf052..ef6f480e76 100644 --- a/tests/storage/test_semantic_dag_incremental.py +++ b/tests/storage/test_semantic_dag_incremental.py @@ -219,9 +219,7 @@ async def test_pending_refresh_rebuilds_every_sampled_file_summary(monkeypatch): file_paths = [f"{root_uri}/{name}" for name in file_names] sampled_paths = deterministic_sample(file_paths, 4) changed_path = f"{root_uri}/file-020.txt" - old_overview = "FILES:\n" + "\n".join( - f"- {name}: old-summary" for name in file_names - ) + old_overview = "FILES:\n" + "\n".join(f"- {name}: old-summary" for name in file_names) metadata = {"freshness": freshness_metadata(40, 4, pending=4)} fake_fs = _FakeVikingFS( tree={ @@ -237,9 +235,7 @@ async def test_pending_refresh_rebuilds_every_sampled_file_summary(monkeypatch): ), }, ) - monkeypatch.setattr( - "openviking.storage.queuefs.semantic_dag.get_viking_fs", lambda: fake_fs - ) + monkeypatch.setattr("openviking.storage.queuefs.semantic_dag.get_viking_fs", lambda: fake_fs) monkeypatch.setattr( "openviking.storage.queuefs.semantic_dag.get_openviking_config", lambda: SimpleNamespace(semantic=SimpleNamespace(overview_sample_limit=4)), @@ -262,9 +258,7 @@ async def test_pending_refresh_rebuilds_every_sampled_file_summary(monkeypatch): assert set(processor.summarized_files) == set(sampled_paths) | {changed_path} assert processor.vectorized_files == [changed_path] - overview = parse_abstract_overview( - fake_fs._file_contents[f"{root_uri}/.overview.md"] - ) + overview = parse_abstract_overview(fake_fs._file_contents[f"{root_uri}/.overview.md"]) assert overview.metadata["freshness"]["pending_child_changes"] == 0 @@ -284,9 +278,7 @@ async def test_directory_vectorization_retries_after_matching_sidecar_write(monk ), }, ) - monkeypatch.setattr( - "openviking.storage.queuefs.semantic_dag.get_viking_fs", lambda: fake_fs - ) + monkeypatch.setattr("openviking.storage.queuefs.semantic_dag.get_viking_fs", lambda: fake_fs) monkeypatch.setattr( "openviking.storage.queuefs.semantic_dag.get_openviking_config", lambda: SimpleNamespace(semantic=SimpleNamespace(overview_sample_limit=32)), @@ -342,9 +334,7 @@ async def test_content_copy_rebuilds_target_overview_from_target_l2_summaries(mo ), }, ) - monkeypatch.setattr( - "openviking.storage.queuefs.semantic_dag.get_viking_fs", lambda: fake_fs - ) + monkeypatch.setattr("openviking.storage.queuefs.semantic_dag.get_viking_fs", lambda: fake_fs) monkeypatch.setattr( "openviking.storage.queuefs.semantic_dag.get_openviking_config", lambda: SimpleNamespace(semantic=SimpleNamespace(overview_sample_limit=32)), @@ -376,12 +366,8 @@ async def test_content_copy_rebuilds_target_overview_from_target_l2_summaries(mo assert processor.vectorized_files == [] assert processor.generated_overviews == [root_uri] assert processor.vectorized_dirs == [root_uri] - overview = parse_abstract_overview( - fake_fs._file_contents[f"{root_uri}/.overview.md"] - ).body - abstract = parse_abstract_overview( - fake_fs._file_contents[f"{root_uri}/.abstract.md"] - ).body + overview = parse_abstract_overview(fake_fs._file_contents[f"{root_uri}/.overview.md"]).body + abstract = parse_abstract_overview(fake_fs._file_contents[f"{root_uri}/.abstract.md"]).body assert "- copied.jpg: copied target L2 summary" in overview assert "- keep.txt: kept target L2 summary" in overview assert abstract.strip() == "abstract" @@ -509,8 +495,8 @@ async def test_content_copy_with_no_ready_summaries_preserves_existing_sidecars( @pytest.mark.asyncio async def test_content_copy_rebuilds_semantics_when_move_leaves_source_directory_empty(monkeypatch): root_uri = "viking://resources/source" - old_overview = render_semantic_sidecar(ContextLevel.OVERVIEW, root_uri, "old file summary") - old_abstract = render_semantic_sidecar(ContextLevel.ABSTRACT, root_uri, "old abstract") + old_overview = render_abstract_overview(ContextLevel.OVERVIEW, root_uri, "old file summary") + old_abstract = render_abstract_overview(ContextLevel.ABSTRACT, root_uri, "old abstract") fake_fs = _FakeVikingFS( tree={root_uri: []}, file_contents={ @@ -521,7 +507,7 @@ async def test_content_copy_rebuilds_semantics_when_move_leaves_source_directory monkeypatch.setattr("openviking.storage.queuefs.semantic_dag.get_viking_fs", lambda: fake_fs) monkeypatch.setattr( "openviking.storage.queuefs.semantic_dag.get_openviking_config", - lambda: SimpleNamespace(semantic=SimpleNamespace(sidecar_sample_size=32)), + lambda: SimpleNamespace(semantic=SimpleNamespace(overview_sample_limit=32)), ) processor = _FakeProcessor(fake_fs) executor = SemanticDagExecutor( @@ -540,8 +526,8 @@ async def test_content_copy_rebuilds_semantics_when_move_leaves_source_directory assert processor.generated_overviews == [root_uri] assert processor.vectorized_dirs == [root_uri] - overview = parse_semantic_sidecar(fake_fs._file_contents[f"{root_uri}/.overview.md"]).body - abstract = parse_semantic_sidecar(fake_fs._file_contents[f"{root_uri}/.abstract.md"]).body + overview = parse_abstract_overview(fake_fs._file_contents[f"{root_uri}/.overview.md"]).body + abstract = parse_abstract_overview(fake_fs._file_contents[f"{root_uri}/.abstract.md"]).body assert overview.strip() == "FILES:" assert abstract.strip() == "abstract" From ecb4a3004b073e8c7ec4153d5e0fca3be7b8248e Mon Sep 17 00:00:00 2001 From: chenpengfei Date: Mon, 31 Aug 2026 22:35:03 +0800 Subject: [PATCH 20/20] fix: harden copy and move transaction boundaries --- openviking/pyagfs/async_client.py | 2 + openviking/pyagfs/helpers.py | 73 +++++-- openviking/server/routers/filesystem.py | 16 +- openviking/service/fs_service.py | 82 +++++--- openviking/storage/queuefs/semantic_dag.py | 9 - openviking/storage/viking_fs/_ops.py | 139 +++++++------ tests/agfs/test_fs_binding.py | 25 +++ tests/pyagfs/test_async_client_fs_ctx.py | 135 ++++++++++++ tests/server/test_filesystem_router.py | 73 ++++++- tests/service/test_fs_service.py | 95 +++++++++ .../storage/test_semantic_dag_incremental.py | 11 +- tests/storage/test_semantic_dag_skip_files.py | 2 + tests/storage/test_semantic_dag_stats.py | 6 +- tests/storage/test_viking_fs_cp.py | 196 ++++++++++++++---- 14 files changed, 690 insertions(+), 174 deletions(-) diff --git a/openviking/pyagfs/async_client.py b/openviking/pyagfs/async_client.py index 7e0101286a..3732a8e821 100644 --- a/openviking/pyagfs/async_client.py +++ b/openviking/pyagfs/async_client.py @@ -245,6 +245,7 @@ async def cp( *, fs_ctx: Dict[str, str] | None = None, auto_pathlock: bool = True, + allow_same_mount_fast_path: bool = False, ) -> Any: """Copy a path within AGFS while preserving the caller's FsContext. @@ -261,6 +262,7 @@ async def cp( recursive=recursive, stream=True, fs_ctx=_fs_ctx_with_auto_pathlock(src_path, fs_ctx, auto_pathlock), + allow_same_mount_fast_path=allow_same_mount_fast_path, ) async def grep(self, **kwargs: Any) -> Dict[str, Any]: diff --git a/openviking/pyagfs/helpers.py b/openviking/pyagfs/helpers.py index e3a58cf77d..f8d9b9a47b 100644 --- a/openviking/pyagfs/helpers.py +++ b/openviking/pyagfs/helpers.py @@ -55,6 +55,7 @@ def cp( recursive: bool = False, stream: bool = False, fs_ctx: dict[str, str] | None = None, + allow_same_mount_fast_path: bool = False, ) -> None: """Copy a file or directory within AGFS. @@ -64,6 +65,9 @@ def cp( dst: Destination path in AGFS recursive: If True, copy directories recursively stream: If True, use streaming for large files (memory efficient) + allow_same_mount_fast_path: If True, allow a backend verbatim copy. This + is reserved for already-encrypted internal trees whose bytes are safe + to preserve unchanged. Raises: AGFSClientError: If source doesn't exist or operation fails @@ -81,9 +85,23 @@ def cp( if is_dir: if not recursive: raise ValueError(f"Cannot copy directory '{src}' without recursive=True") - _copy_directory(client, src, dst, stream, fs_ctx=fs_ctx) + _copy_directory( + client, + src, + dst, + stream, + fs_ctx=fs_ctx, + allow_same_mount_fast_path=allow_same_mount_fast_path, + ) else: - _copy_file(client, src, dst, stream, fs_ctx=fs_ctx) + _copy_file( + client, + src, + dst, + stream, + fs_ctx=fs_ctx, + allow_same_mount_fast_path=allow_same_mount_fast_path, + ) def upload( @@ -168,6 +186,7 @@ def _copy_file( stream: bool, *, fs_ctx: dict[str, str] | None = None, + allow_same_mount_fast_path: bool = False, ) -> None: """Copy a single file within AGFS. @@ -177,18 +196,31 @@ def _copy_file( # Ensure parent directory exists _ensure_remote_parent_dir(client, dst, fs_ctx=fs_ctx) - if _try_copy_within_mount_fast_path(client, src, dst, fs_ctx=fs_ctx): + if allow_same_mount_fast_path and _try_copy_within_mount_fast_path( + client, src, dst, fs_ctx=fs_ctx + ): return if stream: - # The binding client returns bytes or an iterator of bytes, not an - # HTTP response object with iter_content(). - _call_with_optional_ctx( - client.write, - dst, - _iter_file_bytes(_call_with_optional_ctx(client.cat, src, stream=True, ctx=fs_ctx)), - ctx=fs_ctx, - ) + try: + source = _call_with_optional_ctx(client.cat, src, stream=True, ctx=fs_ctx) + except RuntimeError as exc: + if "streaming not supported" not in str(exc).lower(): + raise + # The embedded Rust binding currently accepts only complete byte + # buffers. Keep this correctness fallback separate from the raw + # same-mount path, whose offset writes are unsafe on object stores. + data = _call_with_optional_ctx(client.cat, src, ctx=fs_ctx) + _call_with_optional_ctx(client.write, dst, data, ctx=fs_ctx) + else: + # Streaming clients return bytes or an iterator of bytes, not an + # HTTP response object with iter_content(). + _call_with_optional_ctx( + client.write, + dst, + _iter_file_bytes(source), + ctx=fs_ctx, + ) else: # Read entire file and write data = _call_with_optional_ctx(client.cat, src, ctx=fs_ctx) @@ -202,6 +234,7 @@ def _copy_directory( stream: bool, *, fs_ctx: dict[str, str] | None = None, + allow_same_mount_fast_path: bool = False, ) -> None: """Recursively copy a directory within AGFS.""" # Create destination directory @@ -221,10 +254,24 @@ def _copy_directory( if item.get("isDir", False): # Recursively copy subdirectory - _copy_directory(client, src_path, dst_path, stream, fs_ctx=fs_ctx) + _copy_directory( + client, + src_path, + dst_path, + stream, + fs_ctx=fs_ctx, + allow_same_mount_fast_path=allow_same_mount_fast_path, + ) else: # Copy file - _copy_file(client, src_path, dst_path, stream, fs_ctx=fs_ctx) + _copy_file( + client, + src_path, + dst_path, + stream, + fs_ctx=fs_ctx, + allow_same_mount_fast_path=allow_same_mount_fast_path, + ) def _upload_file( diff --git a/openviking/server/routers/filesystem.py b/openviking/server/routers/filesystem.py index 073dfb1793..510362b8e2 100644 --- a/openviking/server/routers/filesystem.py +++ b/openviking/server/routers/filesystem.py @@ -39,9 +39,7 @@ def _clean_memory_attrs(raw: str) -> dict[str, Any]: return attrs -async def _tags_attr( - service: Any, uri: str, ctx: RequestContext, *, is_dir: bool -) -> list[str]: +async def _tags_attr(service: Any, uri: str, ctx: RequestContext, *, is_dir: bool) -> list[str]: vikingdb_manager = getattr(service, "vikingdb_manager", None) if not vikingdb_manager: return [] @@ -209,9 +207,7 @@ async def attrs( }, } if result["context_type"] == "memory" and not stat_result.get("isDir", False): - result["attrs"]["memory"] = _clean_memory_attrs( - await service.fs.read(uri, ctx=_ctx) - ) + result["attrs"]["memory"] = _clean_memory_attrs(await service.fs.read(uri, ctx=_ctx)) return Response(status="ok", result=result) except AGFSNotFoundError: raise NotFoundError(uri, "file") @@ -325,8 +321,12 @@ async def cp( ): """Copy a file or directory together with its vector records.""" service = get_service() - from_uri = resolve_path_variables(request.from_uri) - to_uri = resolve_path_variables(request.to_uri) + from_uri = validate_request_viking_uri( + resolve_path_variables(request.from_uri), _ctx, field_name="from_uri" + ) + to_uri = validate_request_viking_uri( + resolve_path_variables(request.to_uri), _ctx, field_name="to_uri" + ) try: result = await service.fs.cp( from_uri, diff --git a/openviking/service/fs_service.py b/openviking/service/fs_service.py index f4d07cb893..04d134195c 100644 --- a/openviking/service/fs_service.py +++ b/openviking/service/fs_service.py @@ -7,6 +7,7 @@ """ import asyncio +from collections.abc import Coroutine from typing import TYPE_CHECKING, Any, Dict, List, Literal, Optional from openviking.core.context import ContextLevel @@ -251,9 +252,7 @@ async def mkdir( overview="", context_type=context_type_for_uri(directory_uri), ctx=ctx, - creator_acl_grant=( - CreatorAclGrant.DIRECT if not directory_preexisting else None - ), + creator_acl_grant=(CreatorAclGrant.DIRECT if not directory_preexisting else None), include_overview=False, ) @@ -545,7 +544,21 @@ async def cp( recursive: bool, ctx: RequestContext, ) -> Dict[str, Any]: - """Copy a resource and queue derived parent semantics after commit.""" + """Copy a resource without exposing a cancellable partial transaction.""" + return await self._finish_transfer_after_caller_cancel( + self._cp_and_refresh(from_uri, to_uri, recursive=recursive, ctx=ctx), + operation="copy", + ) + + async def _cp_and_refresh( + self, + from_uri: str, + to_uri: str, + *, + recursive: bool, + ctx: RequestContext, + ) -> Dict[str, Any]: + """Commit copy and enqueue its parent refresh as one cancellation-safe unit.""" viking_fs = self._ensure_initialized() async with self._uri_mutation_coordinator.mutation( ctx.account_id, @@ -587,33 +600,52 @@ async def cp( return result async def mv(self, from_uri: str, to_uri: str, ctx: RequestContext) -> None: - """Move resource.""" + """Move a resource without exposing a cancellable partial transaction.""" + await self._finish_transfer_after_caller_cancel( + self._mv_and_refresh(from_uri, to_uri, ctx=ctx), + operation="move", + ) + + async def _mv_and_refresh( + self, + from_uri: str, + to_uri: str, + *, + ctx: RequestContext, + ) -> None: + """Commit move/watch state and enqueue all affected parent refreshes.""" viking_fs = self._ensure_initialized() watch_manager = self._get_watch_manager() - if not watch_manager or context_type_for_uri(from_uri) != "resource": - await viking_fs.mv(from_uri, to_uri, ctx=ctx) - await self._refresh_move_parents(from_uri=from_uri, to_uri=to_uri, ctx=ctx) - return - if context_type_for_uri(to_uri) != "resource": - await viking_fs.mv(from_uri, to_uri, ctx=ctx) - await self._refresh_move_parents(from_uri=from_uri, to_uri=to_uri, ctx=ctx) - return - if is_watch_task_control_uri(from_uri) or is_watch_task_control_uri(to_uri): + use_watch_transaction = ( + watch_manager is not None + and context_type_for_uri(from_uri) == "resource" + and context_type_for_uri(to_uri) == "resource" + and not is_watch_task_control_uri(from_uri) + and not is_watch_task_control_uri(to_uri) + ) + if not use_watch_transaction: await viking_fs.mv(from_uri, to_uri, ctx=ctx) - await self._refresh_move_parents(from_uri=from_uri, to_uri=to_uri, ctx=ctx) - return - - transaction_task = asyncio.create_task( - self._move_resource_with_watch_transaction( + else: + assert watch_manager is not None + await self._move_resource_with_watch_transaction( viking_fs, watch_manager, from_uri, to_uri, ctx, ) - ) + await self._refresh_move_parents(from_uri=from_uri, to_uri=to_uri, ctx=ctx) + + async def _finish_transfer_after_caller_cancel( + self, + transaction: Coroutine[Any, Any, Any], + *, + operation: str, + ) -> Any: + """Finish an already-started transfer before propagating caller cancellation.""" + transaction_task = asyncio.create_task(transaction) try: - await asyncio.shield(transaction_task) + return await asyncio.shield(transaction_task) except asyncio.CancelledError: while not transaction_task.done(): try: @@ -628,11 +660,11 @@ async def mv(self, from_uri: str, to_uri: str, ctx: RequestContext) -> None: pass except Exception: logger.error( - "Resource move transaction failed while caller was cancelled", + "Filesystem %s transaction failed while caller was cancelled", + operation, exc_info=True, ) raise - await self._refresh_move_parents(from_uri=from_uri, to_uri=to_uri, ctx=ctx) async def _refresh_move_parents( self, @@ -935,9 +967,7 @@ async def grant_acl( ) -> Dict[str, Any]: return await self._ensure_initialized().grant_acl(uri, principal, level, ctx=ctx) - async def revoke_acl( - self, uri: str, principal: str, ctx: RequestContext - ) -> Dict[str, Any]: + async def revoke_acl(self, uri: str, principal: str, ctx: RequestContext) -> Dict[str, Any]: return await self._ensure_initialized().revoke_acl(uri, principal, ctx=ctx) async def delete_acl(self, uri: str, ctx: RequestContext) -> Dict[str, Any]: diff --git a/openviking/storage/queuefs/semantic_dag.py b/openviking/storage/queuefs/semantic_dag.py index 7b07aa36c7..e7ee30f85f 100644 --- a/openviking/storage/queuefs/semantic_dag.py +++ b/openviking/storage/queuefs/semantic_dag.py @@ -532,17 +532,8 @@ async def _prepare_transfer_node( primary = deterministic_sample(candidates, sample_limit) loaded = await self._load_transfer_candidates(primary) selected = [candidate for candidate in primary if candidate in loaded] - missing_primary = len(primary) - len(selected) inspected = set(primary) - if missing_primary: - remaining = [candidate for candidate in candidates if candidate not in inspected] - fallback_loaded = await self._load_transfer_candidates(remaining) - loaded.update(fallback_loaded) - ready_remaining = [candidate for candidate in remaining if candidate in fallback_loaded] - selected.extend(deterministic_sample(ready_remaining, missing_primary)) - inspected.update(remaining) - selected.sort(key=lambda item: (item[1].rsplit("/", 1)[-1], item[0], item[1])) selected_files = [uri for kind, uri in selected if kind == "file"] selected_dirs = [uri for kind, uri in selected if kind == "directory"] diff --git a/openviking/storage/viking_fs/_ops.py b/openviking/storage/viking_fs/_ops.py index afb5dd2941..979f590be7 100644 --- a/openviking/storage/viking_fs/_ops.py +++ b/openviking/storage/viking_fs/_ops.py @@ -355,12 +355,16 @@ async def cp( await self._ensure_transfer_parent_directory(new_path, new_uri, operation="cp") await self._ensure_transfer_target_missing(new_path, new_uri) - lock_kind = "tree" if is_dir else "exact" - lease = await self._async_agfs.pathlock_acquire_batch( - [ - {"path": old_path, "kind": lock_kind}, + lock_requests = ( + self._directory_transfer_lock_requests(old_path, new_path) + if is_dir + else [ + {"path": old_path, "kind": "exact"}, {"path": new_path, "kind": "exact"}, - ], + ] + ) + lease = await self._async_agfs.pathlock_acquire_batch( + lock_requests, owner_lease_ref=lease_ref, ) operation_id = uuid.uuid4().hex @@ -476,7 +480,7 @@ async def _ensure_copy_source_access( async def _ensure_transfer_parent_directory( self, path: str, uri: str, *, operation: str ) -> None: - parent_path = path.rstrip("/").rsplit("/", 1)[0] or "/" + parent_path = self._transfer_parent_path(path) try: parent_stat = await self._async_agfs.stat(parent_path) except Exception as exc: @@ -493,6 +497,39 @@ async def _ensure_transfer_parent_directory( if not isinstance(parent_stat, dict) or not parent_stat.get("isDir", False): raise InvalidArgumentError(f"{operation} target parent is not a directory: {uri}") + @staticmethod + def _transfer_parent_path(path: str) -> str: + return path.rstrip("/").rsplit("/", 1)[0] or "/" + + @classmethod + def _directory_transfer_lock_requests( + cls, old_path: str, new_path: str + ) -> List[Dict[str, str]]: + """Lock stable parents so directory deletion and recreation stay covered.""" + + parents: List[str] = [] + for parent in ( + cls._transfer_parent_path(old_path), + cls._transfer_parent_path(new_path), + ): + if any(cls._tree_lock_covers(existing, parent) for existing in parents): + continue + parents = [ + existing for existing in parents if not cls._tree_lock_covers(parent, existing) + ] + parents.append(parent) + return [{"path": parent, "kind": "tree"} for parent in parents] + + @staticmethod + def _tree_lock_covers(ancestor: str, path: str) -> bool: + normalized_ancestor = ancestor.rstrip("/") or "/" + normalized_path = path.rstrip("/") or "/" + if normalized_ancestor == "/": + return True + return normalized_path == normalized_ancestor or normalized_path.startswith( + f"{normalized_ancestor}/" + ) + async def _copy_agfs_entry( self, old_path: str, @@ -505,7 +542,7 @@ async def _copy_agfs_entry( lease_ref: Dict[str, Any], ) -> int: if is_dir: - return await self._copy_directory_with_exact_locks( + return await self._copy_directory_under_parent_locks( old_path, new_path, old_uri=old_uri, @@ -530,21 +567,11 @@ async def _cleanup_transfer_target( ctx: Optional[RequestContext], lease_ref: Dict[str, Any], ) -> None: - cleanup_lease = lease_ref - if is_dir: - cleanup_lease = await self._async_agfs.pathlock_acquire_tree( - path, - owner_lease_ref=lease_ref, - ) - try: - await self._async_agfs.rm( - path, - recursive=is_dir, - fs_ctx=self._pathlock_fs_ctx(ctx, cleanup_lease), - ) - finally: - if cleanup_lease is not lease_ref: - await self._async_agfs.pathlock_release(cleanup_lease) + await self._async_agfs.rm( + path, + recursive=is_dir, + fs_ctx=self._pathlock_fs_ctx(ctx, lease_ref), + ) async def mv( self, @@ -617,10 +644,7 @@ async def mv( if is_dir: lease = await self._async_agfs.pathlock_acquire_batch( - [ - {"path": old_path, "kind": "tree"}, - {"path": new_path, "kind": "exact"}, - ], + self._directory_transfer_lock_requests(old_path, new_path), owner_lease_ref=lease_ref, ) else: @@ -827,7 +851,7 @@ async def _copy_for_mv( lease_ref=lease_ref, ) - async def _copy_directory_with_exact_locks( + async def _copy_directory_under_parent_locks( self, old_path: str, new_path: str, @@ -836,13 +860,13 @@ async def _copy_directory_with_exact_locks( ctx: Optional[RequestContext], lease_ref: Dict[str, Any] | None, ) -> int: - """Copy a directory through AGFS while locking every destination entry. + """Copy a directory under the operation's stable parent Tree leases. Args: old_path: Source backend directory path. new_path: Destination backend directory path. ctx: Request context used for filesystem operations. - lease_ref: Exact lease covering the current destination directory. + lease_ref: Batch lease covering the source and destination parents. Returns: Number of created directories and files. @@ -866,30 +890,23 @@ async def _copy_directory_with_exact_locks( recursive=bool(entry.get("isDir", False)), ctx=ctx, ) - child_lease = await self._async_agfs.pathlock_acquire_exact( - new_child, - owner_lease_ref=lease_ref, - ) - try: - if entry.get("isDir", False): - copied += await self._copy_directory_with_exact_locks( - old_child, - new_child, - old_uri=old_child_uri, - new_uri=new_child_uri, - ctx=ctx, - lease_ref=child_lease, - ) - else: - await self._async_agfs.cp( - old_child, - new_child, - recursive=False, - fs_ctx=self._pathlock_fs_ctx(ctx, child_lease), - ) - copied += 1 - finally: - await self._async_agfs.pathlock_release(child_lease) + if entry.get("isDir", False): + copied += await self._copy_directory_under_parent_locks( + old_child, + new_child, + old_uri=old_child_uri, + new_uri=new_child_uri, + ctx=ctx, + lease_ref=lease_ref, + ) + else: + await self._async_agfs.cp( + old_child, + new_child, + recursive=False, + fs_ctx=self._pathlock_fs_ctx(ctx, lease_ref), + ) + copied += 1 return copied async def _copy_dir_through_vikingfs( @@ -1261,7 +1278,9 @@ async def tree( await self._ensure_access(uri, ctx) extra_fields = extra_fields or [] if output == "original": - entries = await self._tree_original(uri, show_all_hidden, node_limit, level_limit, ctx=ctx) + entries = await self._tree_original( + uri, show_all_hidden, node_limit, level_limit, ctx=ctx + ) elif output == "agent": entries = await self._tree_agent( uri, abs_limit, show_all_hidden, node_limit, level_limit, ctx=ctx @@ -1937,12 +1956,11 @@ async def _augment_entries_extra_fields( if not may_include_hidden_actor_peers(entry_uri, real_ctx): filter_expr = PathScope("uri", entry_uri, depth=-1) entry["count"] = await vector_store.count( - filter=filter_expr, ctx=real_ctx, + filter=filter_expr, + ctx=real_ctx, ) except Exception as e: - logger.warning( - f"[VikingFS] Failed to count nodes for {entry_uri}: {e}" - ) + logger.warning(f"[VikingFS] Failed to count nodes for {entry_uri}: {e}") if need_locked and lock_paths: for i, path in lock_paths: @@ -1951,9 +1969,7 @@ async def _augment_entries_extra_fields( except Exception: entries[i]["isLocked"] = False - def _try_uri_to_path( - self, uri: str, ctx: Optional[RequestContext] = None - ) -> Optional[str]: + def _try_uri_to_path(self, uri: str, ctx: Optional[RequestContext] = None) -> Optional[str]: """Best-effort URI to path conversion; returns None on failure.""" try: return self._uri_to_path(uri, ctx=ctx) @@ -2006,6 +2022,7 @@ async def persist_temp_tree( dst_path, recursive=True, fs_ctx=fs_ctx or {"account_id": self._ctx_or_default(ctx).account_id}, + allow_same_mount_fast_path=True, ) async def delete_temp( diff --git a/tests/agfs/test_fs_binding.py b/tests/agfs/test_fs_binding.py index 22976aa2b2..a80fde0662 100644 --- a/tests/agfs/test_fs_binding.py +++ b/tests/agfs/test_fs_binding.py @@ -128,6 +128,31 @@ async def test_flat_file_lock_keeps_persisted_target_a_file( stat_info = await vfs.stat(uri) await vfs.rm(uri, recursive=bool(stat_info.get("isDir"))) + async def test_recursive_cp_uses_parent_tree_batch_lease( + self, + viking_fs_binding_instance, + ): + """A batch lease on distinct parents must cover all recursive copy operations.""" + vfs = viking_fs_binding_instance + unique = uuid.uuid4().hex + source_uri = f"viking://temp/cp_source_{unique}" + target_uri = f"viking://resources/cp_target_{unique}" + + try: + await vfs.mkdir("viking://resources/", exist_ok=True) + await vfs.mkdir(source_uri) + await vfs.mkdir(f"{source_uri}/empty") + await vfs.write(f"{source_uri}/data.bin", b"\x00\xff") + + await vfs.cp(source_uri, target_uri, recursive=True) + + assert (await vfs.stat(f"{target_uri}/empty"))["isDir"] is True + assert await vfs.read(f"{target_uri}/data.bin") == b"\x00\xff" + finally: + for uri in (source_uri, target_uri): + if await vfs.exists(uri): + await vfs.rm(uri, recursive=True) + async def test_borrowed_pathlock_cannot_release_via_raw_ref(self, viking_fs_binding_instance): """Reject borrowed lifecycle control through typed and raw lease refs.""" vfs = viking_fs_binding_instance diff --git a/tests/pyagfs/test_async_client_fs_ctx.py b/tests/pyagfs/test_async_client_fs_ctx.py index e5765ba51e..a05f184e0e 100644 --- a/tests/pyagfs/test_async_client_fs_ctx.py +++ b/tests/pyagfs/test_async_client_fs_ctx.py @@ -143,3 +143,138 @@ def write(self, path: str, data, *, ctx: dict[str, str] | None = None) -> str: ) ] assert client.written_chunks == [b"first", b"second"] + + +@pytest.mark.asyncio +async def test_async_cp_streams_by_default_when_same_mount_fast_path_is_available() -> None: + class _FastPathClient: + def __init__(self) -> None: + self.fast_path_calls: list[tuple[str, str]] = [] + self.cat_calls: list[tuple[str, bool]] = [] + self.written_chunks: list[bytes] = [] + + def stat(self, path: str, *, ctx: dict[str, str] | None = None) -> dict[str, Any]: + del ctx + return {"isDir": path.endswith("/data")} + + def copy_within_mount( + self, source: str, target: str, *, ctx: dict[str, str] | None = None + ) -> dict[str, bool]: + del ctx + self.fast_path_calls.append((source, target)) + return {"performed": True} + + def cat( + self, + path: str, + *, + stream: bool = False, + ctx: dict[str, str] | None = None, + ): + del ctx + self.cat_calls.append((path, stream)) + return iter((b"first", b"second")) + + def write(self, path: str, data, *, ctx: dict[str, str] | None = None) -> str: + del path, ctx + self.written_chunks = list(data) + return "written" + + client = _FastPathClient() + agfs = AsyncAGFSClient(client) + + await agfs.cp( + "/local/acct/data/source.bin", + "/local/acct/data/target.bin", + ) + + assert client.fast_path_calls == [] + assert client.cat_calls == [("/local/acct/data/source.bin", True)] + assert client.written_chunks == [b"first", b"second"] + + +@pytest.mark.asyncio +async def test_async_cp_can_explicitly_use_same_mount_fast_path() -> None: + class _FastPathClient: + def __init__(self) -> None: + self.fast_path_calls: list[tuple[str, str]] = [] + + def stat(self, path: str, *, ctx: dict[str, str] | None = None) -> dict[str, Any]: + del ctx + return {"isDir": path.endswith("/data")} + + def copy_within_mount( + self, source: str, target: str, *, ctx: dict[str, str] | None = None + ) -> dict[str, bool]: + del ctx + self.fast_path_calls.append((source, target)) + return {"performed": True} + + def cat(self, *args, **kwargs): + raise AssertionError("explicit fast path must not stream file bytes") + + client = _FastPathClient() + agfs = AsyncAGFSClient(client) + + await agfs.cp( + "/local/acct/data/source.bin", + "/local/acct/data/target.bin", + allow_same_mount_fast_path=True, + ) + + assert client.fast_path_calls == [ + ("/local/acct/data/source.bin", "/local/acct/data/target.bin") + ] + + +@pytest.mark.asyncio +async def test_async_cp_falls_back_to_complete_write_when_binding_cannot_stream() -> None: + class _BindingClient: + def __init__(self) -> None: + self.fast_path_calls: list[tuple[str, str]] = [] + self.cat_calls: list[tuple[str, bool]] = [] + self.written: bytes | None = None + + def stat(self, path: str, *, ctx: dict[str, str] | None = None) -> dict[str, Any]: + del ctx + return {"isDir": path.endswith("/data")} + + def copy_within_mount( + self, source: str, target: str, *, ctx: dict[str, str] | None = None + ) -> dict[str, bool]: + del ctx + self.fast_path_calls.append((source, target)) + return {"performed": True} + + def cat( + self, + path: str, + *, + stream: bool = False, + ctx: dict[str, str] | None = None, + ) -> bytes: + del ctx + self.cat_calls.append((path, stream)) + if stream: + raise RuntimeError("Streaming not supported in binding mode") + return b"complete payload" + + def write(self, path: str, data: bytes, *, ctx: dict[str, str] | None = None) -> str: + del path, ctx + self.written = data + return "written" + + client = _BindingClient() + agfs = AsyncAGFSClient(client) + + await agfs.cp( + "/local/acct/data/source.bin", + "/local/acct/data/target.bin", + ) + + assert client.fast_path_calls == [] + assert client.cat_calls == [ + ("/local/acct/data/source.bin", True), + ("/local/acct/data/source.bin", False), + ] + assert client.written == b"complete payload" diff --git a/tests/server/test_filesystem_router.py b/tests/server/test_filesystem_router.py index 26fa29eda6..137eb5910c 100644 --- a/tests/server/test_filesystem_router.py +++ b/tests/server/test_filesystem_router.py @@ -3,6 +3,7 @@ """Filesystem router tests.""" from types import SimpleNamespace +from unittest.mock import AsyncMock import httpx import pytest @@ -11,6 +12,7 @@ from openviking.server.auth import get_request_context from openviking.server.identity import RequestContext, Role from openviking.server.routers import filesystem +from openviking_cli.exceptions import InvalidURIError from openviking_cli.session.user_id import UserIdentifier @@ -101,6 +103,62 @@ def test_cp_request_defaults_to_non_recursive(): assert request.recursive is False +@pytest.mark.asyncio +async def test_cp_canonicalizes_home_aliases(monkeypatch): + calls = [] + + async def fake_cp(from_uri, to_uri, recursive=False, ctx=None): + calls.append((from_uri, to_uri, recursive, ctx)) + return {} + + monkeypatch.setattr( + filesystem, + "get_service", + lambda: SimpleNamespace(fs=SimpleNamespace(cp=fake_cp)), + ) + ctx = RequestContext(user=UserIdentifier("acct", "alice"), role=Role.USER) + + response = await filesystem.cp( + filesystem.CpRequest( + from_uri="viking://~/resources/a.md", + to_uri="viking://~/resources/b.md", + ), + _ctx=ctx, + ) + + assert calls == [ + ( + "viking://user/alice/resources/a.md", + "viking://user/alice/resources/b.md", + False, + ctx, + ) + ] + assert response.result["from"] == "viking://user/alice/resources/a.md" + assert response.result["to"] == "viking://user/alice/resources/b.md" + + +@pytest.mark.asyncio +async def test_cp_rejects_invalid_uri_before_calling_service(monkeypatch): + cp_mock = AsyncMock() + monkeypatch.setattr( + filesystem, + "get_service", + lambda: SimpleNamespace(fs=SimpleNamespace(cp=cp_mock)), + ) + + with pytest.raises(InvalidURIError, match="Invalid URI"): + await filesystem.cp( + filesystem.CpRequest( + from_uri="/local/acct/resources/a.md", + to_uri="viking://resources/b.md", + ), + _ctx=RequestContext(user=UserIdentifier("acct", "alice"), role=Role.USER), + ) + + cp_mock.assert_not_awaited() + + @pytest.mark.asyncio async def test_attrs_returns_memory_fields_and_tags(monkeypatch): raw_memory = ( @@ -121,7 +179,7 @@ class FakeVectorManager: async def filter(self, **kwargs): return [ { - "uri": kwargs["filter"]["conds"][0], + "uri": "viking://user/alice/memories/preferences/theme.md", "level": 2, "search_tags": ["team=search"], } @@ -143,6 +201,7 @@ async def filter(self, **kwargs): attrs = response.result["attrs"] assert attrs["memory"] == { + "version": 1, "tags": ["ui"], "fields": {"topic": "theme"}, "resource_refs": ["viking://resources/docs/api.md"], @@ -210,9 +269,7 @@ async def fake_mkdir(uri, ctx=None, description=None): @pytest.mark.asyncio async def test_http_stat_returns_canonical_request_uri(monkeypatch): seen = {} - request_context = RequestContext( - user=UserIdentifier("acct", "alice"), role=Role.USER - ) + request_context = RequestContext(user=UserIdentifier("acct", "alice"), role=Role.USER) async def fake_stat(uri, ctx=None): seen.update(uri=uri, ctx=ctx) @@ -235,9 +292,7 @@ async def fake_stat(uri, ctx=None): app.include_router(filesystem.router) app.dependency_overrides[get_request_context] = lambda: request_context transport = httpx.ASGITransport(app=app) - async with httpx.AsyncClient( - transport=transport, base_url="http://testserver" - ) as client: + async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: response = await client.get( "/api/v1/fs/stat", params={"uri": "viking://~/resources/notes.md"}, @@ -280,9 +335,7 @@ async def fake_stat(uri, ctx=None): user=UserIdentifier("acct", "alice"), role=Role.USER ) transport = httpx.ASGITransport(app=app) - async with httpx.AsyncClient( - transport=transport, base_url="http://testserver" - ) as client: + async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: response = await client.get("/api/v1/fs/stat", params={"uri": record_id}) assert response.status_code == 200, response.text diff --git a/tests/service/test_fs_service.py b/tests/service/test_fs_service.py index 8fc2db8a55..56cda20d70 100644 --- a/tests/service/test_fs_service.py +++ b/tests/service/test_fs_service.py @@ -2,6 +2,7 @@ # SPDX-License-Identifier: AGPL-3.0 """Tests for file-system service coordination behavior.""" +import asyncio from contextlib import asynccontextmanager from types import SimpleNamespace from unittest.mock import AsyncMock @@ -571,6 +572,100 @@ async def test_resource_cp_refresh_failure_does_not_roll_back_copy(request_conte assert result["semantic_error"] == "queue unavailable" +@pytest.mark.asyncio +async def test_resource_cp_finishes_transfer_and_refresh_after_caller_cancel(request_context): + source = "viking://resources/source.md" + target = "viking://resources/archive/copied.md" + started = asyncio.Event() + release = asyncio.Event() + completed = asyncio.Event() + viking_fs = _FakeVikingFS() + + async def blocked_cp(from_uri, to_uri, recursive=False, ctx=None): + del from_uri, to_uri, recursive, ctx + started.set() + await release.wait() + completed.set() + return {"operation_id": "copy-cancel"} + + viking_fs.cp = blocked_cp + service = FSService(viking_fs=viking_fs) + service._enqueue_copy_refresh = AsyncMock(return_value="queued") + + task = asyncio.create_task(service.cp(source, target, recursive=False, ctx=request_context)) + await started.wait() + task.cancel() + await asyncio.sleep(0) + release.set() + + with pytest.raises(asyncio.CancelledError): + await task + + assert completed.is_set() + service._enqueue_copy_refresh.assert_awaited_once() + + +@pytest.mark.asyncio +async def test_resource_cp_finishes_inflight_refresh_after_caller_cancel(request_context): + source = "viking://resources/source.md" + target = "viking://resources/archive/copied.md" + refresh_started = asyncio.Event() + release_refresh = asyncio.Event() + refresh_completed = asyncio.Event() + service = FSService(viking_fs=_FakeVikingFS()) + + async def blocked_refresh(**kwargs): + del kwargs + refresh_started.set() + await release_refresh.wait() + refresh_completed.set() + return "queued" + + service._enqueue_copy_refresh = blocked_refresh + task = asyncio.create_task(service.cp(source, target, recursive=False, ctx=request_context)) + await refresh_started.wait() + task.cancel() + await asyncio.sleep(0) + release_refresh.set() + + with pytest.raises(asyncio.CancelledError): + await task + + assert refresh_completed.is_set() + + +@pytest.mark.asyncio +async def test_resource_mv_finishes_transfer_and_refresh_after_caller_cancel(request_context): + source = "viking://resources/source.md" + target = "viking://resources/archive/moved.md" + started = asyncio.Event() + release = asyncio.Event() + completed = asyncio.Event() + viking_fs = _FakeVikingFS() + + async def blocked_mv(from_uri, to_uri, ctx=None): + del from_uri, to_uri, ctx + started.set() + await release.wait() + completed.set() + + viking_fs.mv = blocked_mv + service = FSService(viking_fs=viking_fs) + service._enqueue_copy_refresh = AsyncMock(return_value="queued") + + task = asyncio.create_task(service.mv(source, target, ctx=request_context)) + await started.wait() + task.cancel() + await asyncio.sleep(0) + release.set() + + with pytest.raises(asyncio.CancelledError): + await task + + assert completed.is_set() + assert service._enqueue_copy_refresh.await_count == 2 + + @pytest.mark.asyncio async def test_non_resource_cp_skips_parent_semantic_refresh(request_context): viking_fs = _FakeVikingFS() diff --git a/tests/storage/test_semantic_dag_incremental.py b/tests/storage/test_semantic_dag_incremental.py index ef6f480e76..fd6c3836be 100644 --- a/tests/storage/test_semantic_dag_incremental.py +++ b/tests/storage/test_semantic_dag_incremental.py @@ -121,6 +121,7 @@ async def _vectorize_single_file( ctx=None, use_summary=False, ingest_options=None, + creator_acl_grant=None, ): self.vectorized_files.append(file_path) @@ -132,6 +133,7 @@ async def _vectorize_directory( overview, ctx=None, ingest_options=None, + creator_acl_grant=None, ): self.vectorized_dirs.append(uri) @@ -413,7 +415,7 @@ async def test_content_copy_samples_before_loading_summaries(monkeypatch): @pytest.mark.asyncio -async def test_content_copy_backfills_missing_sample_summary(monkeypatch): +async def test_content_copy_does_not_backfill_missing_sample_summary(monkeypatch): root_uri = "viking://resources/archive" file_paths = [f"{root_uri}/{name}.txt" for name in "abcde"] fake_fs = _FakeVikingFS( @@ -443,13 +445,12 @@ async def test_content_copy_backfills_missing_sample_summary(monkeypatch): await executor.run(root_uri) - assert processor.transfer_summary_calls[0] == [file_paths[0], file_paths[-1]] - assert processor.transfer_summary_calls[1] == file_paths[1:-1] + assert processor.transfer_summary_calls == [[file_paths[0], file_paths[-1]]] overview_doc = parse_abstract_overview(fake_fs._file_contents[f"{root_uri}/.overview.md"]) assert "a.txt" not in overview_doc.body - assert "b.txt" in overview_doc.body + assert "b.txt" not in overview_doc.body assert "e.txt" in overview_doc.body - assert overview_doc.metadata["freshness"]["missing_summary_entries"] == 1 + assert overview_doc.metadata["freshness"].get("missing_summary_entries") is None @pytest.mark.asyncio diff --git a/tests/storage/test_semantic_dag_skip_files.py b/tests/storage/test_semantic_dag_skip_files.py index 505305dafe..9b60920634 100644 --- a/tests/storage/test_semantic_dag_skip_files.py +++ b/tests/storage/test_semantic_dag_skip_files.py @@ -72,6 +72,7 @@ async def _vectorize_directory( overview, ctx=None, ingest_options=None, + creator_acl_grant=None, ): pass @@ -87,6 +88,7 @@ async def _vectorize_single_file( ctx=None, use_summary=False, ingest_options=None, + creator_acl_grant=None, ): self.vectorized_files.append(file_path) diff --git a/tests/storage/test_semantic_dag_stats.py b/tests/storage/test_semantic_dag_stats.py index fdbe008637..f4cfc9471a 100644 --- a/tests/storage/test_semantic_dag_stats.py +++ b/tests/storage/test_semantic_dag_stats.py @@ -92,6 +92,7 @@ async def _vectorize_directory( overview, ctx=None, ingest_options=None, + creator_acl_grant=None, ): self.vectorized_dirs.append(uri) @@ -104,6 +105,7 @@ async def _vectorize_single_file( ctx=None, use_summary=False, ingest_options=None, + creator_acl_grant=None, ): if self.verify_streaming: assert summary_dict["content"] @@ -156,7 +158,9 @@ def fail(self, exc: Exception) -> None: def _patch_semantic_config(monkeypatch, *, overview_sample_limit=32): monkeypatch.setattr( "openviking.storage.queuefs.semantic_dag.get_openviking_config", - lambda: SimpleNamespace(semantic=SimpleNamespace(overview_sample_limit=overview_sample_limit)), + lambda: SimpleNamespace( + semantic=SimpleNamespace(overview_sample_limit=overview_sample_limit) + ), ) diff --git a/tests/storage/test_viking_fs_cp.py b/tests/storage/test_viking_fs_cp.py index 5fc918c26a..0f1e5bd0b8 100644 --- a/tests/storage/test_viking_fs_cp.py +++ b/tests/storage/test_viking_fs_cp.py @@ -112,15 +112,23 @@ async def mkdir(self, path, fs_ctx=None): async def ls(self, path, fs_ctx=None): self.events.append(("ls", path, fs_ctx)) - if path.endswith("/source"): - return [ - {"name": "empty", "isDir": True}, - {"name": ".hidden", "isDir": False}, - {"name": "data.bin", "isDir": False}, - ] - if path.endswith("/source/empty"): - return [] - raise FileNotFoundError(path) + if path not in self.directories: + raise FileNotFoundError(path) + prefix = f"{path.rstrip('/')}/" + entries: dict[str, bool] = {} + for directory in self.directories: + if not directory.startswith(prefix): + continue + relative = directory[len(prefix) :] + if relative and "/" not in relative: + entries[relative] = True + for file_path in self.files: + if not file_path.startswith(prefix): + continue + relative = file_path[len(prefix) :] + if relative and "/" not in relative: + entries[relative] = False + return [{"name": name, "isDir": is_dir} for name, is_dir in sorted(entries.items())] async def pathlock_acquire_exact(self, path, timeout_secs=0.0, owner_lease_ref=None): del timeout_secs @@ -138,6 +146,33 @@ async def cp(self, source, target, recursive=False, fs_ctx=None): self.events.append(("cp", source, target, recursive, fs_ctx)) self.files[target] = self.files[source] + async def rm(self, path, recursive=False, fs_ctx=None): + self.events.append(("rm", path, recursive, fs_ctx)) + prefix = f"{path.rstrip('/')}/" + self.directories = { + item for item in self.directories if item != path and not item.startswith(prefix) + } + self.files = { + item: content + for item, content in self.files.items() + if item != path and not item.startswith(prefix) + } + + +class _DirectoryMoveRollbackAGFS(_DirectoryCopyAGFS): + def __init__(self): + super().__init__() + self.fail_source_delete = True + + async def rm(self, path, recursive=False, fs_ctx=None): + if path == "/local/acct/resources/source" and self.fail_source_delete: + self.events.append(("rm", path, recursive, fs_ctx)) + self.fail_source_delete = False + self.files.pop(f"{path}/data.bin", None) + self.directories.discard(f"{path}/empty") + raise RuntimeError("injected partial source delete failure") + await super().rm(path, recursive=recursive, fs_ctx=fs_ctx) + class _MoveRollbackAGFS(_CopyAGFS): def __init__(self): @@ -171,25 +206,17 @@ def _viking_fs(monkeypatch, agfs: _CopyAGFS) -> VikingFS: fs = VikingFS.__new__(VikingFS) fs._async_agfs = agfs fs.vector_store = None - monkeypatch.setattr(fs, "_ensure_access", lambda *_args, **_kwargs: None) - monkeypatch.setattr(fs, "_ensure_mutable_access", lambda *_args, **_kwargs: None) + fs.acl_manager = None + monkeypatch.setattr(fs, "_ensure_access", AsyncMock()) monkeypatch.setattr( fs, "_uri_to_path", - lambda uri, **_kwargs: ( - "/local/acct/resources/source" + (".md" if uri.endswith(".md") else "") - if "/source" in uri - else "/local/acct/resources/target" + (".md" if uri.endswith(".md") else "") - ), + lambda uri, **_kwargs: f"/local/acct/{uri.removeprefix('viking://')}", ) monkeypatch.setattr( fs, "_path_to_uri", - lambda path, **_kwargs: ( - "viking://resources/source.md" - if path.endswith("/source.md") - else "viking://resources/target.md" - ), + lambda path, **_kwargs: f"viking://{path.removeprefix('/local/acct/')}", ) return fs @@ -198,29 +225,32 @@ def _viking_fs(monkeypatch, agfs: _CopyAGFS) -> VikingFS: "uri", ["viking://", "viking://user", "viking://resources", "viking://temp"], ) -def test_cp_rejects_non_root_container_sources(monkeypatch, uri): +@pytest.mark.asyncio +async def test_cp_rejects_non_root_container_sources(monkeypatch, uri): fs = _viking_fs(monkeypatch, _CopyAGFS(source_is_dir=True)) with pytest.raises(PermissionDeniedError, match="container root"): - fs._ensure_copy_source_access(uri, recursive=True, ctx=_user_ctx()) + await fs._ensure_copy_source_access(uri, recursive=True, ctx=_user_ctx()) -def test_cp_rejects_watch_control_source(monkeypatch): +@pytest.mark.asyncio +async def test_cp_rejects_watch_control_source(monkeypatch): fs = _viking_fs(monkeypatch, _CopyAGFS()) with pytest.raises(PermissionDeniedError, match="watch-task control"): - fs._ensure_copy_source_access( + await fs._ensure_copy_source_access( "viking://resources/.watch_tasks.json", recursive=False, ctx=_ctx(), ) -def test_cp_rejects_actor_peer_scope_that_can_include_hidden_peers(monkeypatch): +@pytest.mark.asyncio +async def test_cp_rejects_actor_peer_scope_that_can_include_hidden_peers(monkeypatch): fs = _viking_fs(monkeypatch, _CopyAGFS(source_is_dir=True)) with pytest.raises(PermissionDeniedError, match="hidden peer"): - fs._ensure_copy_source_access( + await fs._ensure_copy_source_access( "viking://user/alice/peers", recursive=True, ctx=_user_ctx(actor_peer_id="peer-a"), @@ -325,17 +355,12 @@ async def test_cp_directory_preserves_empty_hidden_and_binary_entries(monkeypatc acquire = next(event for event in agfs.events if event[0] == "acquire-batch") assert acquire[1] == [ - {"path": "/local/acct/resources/source", "kind": "tree"}, - {"path": "/local/acct/resources/target", "kind": "exact"}, + {"path": "/local/acct/resources", "kind": "tree"}, ] assert "/local/acct/resources/target/empty" in agfs.directories assert agfs.files["/local/acct/resources/target/.hidden"] == b"hidden" assert agfs.files["/local/acct/resources/target/data.bin"] == b"\x00\xff" - assert {path for path, _lease in agfs.exact_leases} == { - "/local/acct/resources/target/empty", - "/local/acct/resources/target/.hidden", - "/local/acct/resources/target/data.bin", - } + assert agfs.exact_leases == [] vector_copy.assert_awaited_once_with( "viking://resources/source", "viking://resources/target", @@ -344,6 +369,37 @@ async def test_cp_directory_preserves_empty_hidden_and_binary_entries(monkeypatc ) +@pytest.mark.asyncio +async def test_cp_directory_locks_both_distinct_parent_trees(monkeypatch): + agfs = _DirectoryCopyAGFS() + agfs.directories = { + "/local/acct/resources", + "/local/acct/resources/source-parent", + "/local/acct/resources/source-parent/source", + "/local/acct/resources/source-parent/source/empty", + "/local/acct/resources/target-parent", + } + agfs.files = { + "/local/acct/resources/source-parent/source/data.bin": b"data", + } + fs = _viking_fs(monkeypatch, agfs) + monkeypatch.setattr(fs, "_copy_vector_store_uris", AsyncMock(), raising=False) + + await fs.cp( + "viking://resources/source-parent/source", + "viking://resources/target-parent/target", + recursive=True, + ctx=_ctx(), + ) + + acquire = next(event for event in agfs.events if event[0] == "acquire-batch") + assert acquire[1] == [ + {"path": "/local/acct/resources/source-parent", "kind": "tree"}, + {"path": "/local/acct/resources/target-parent", "kind": "tree"}, + ] + assert agfs.exact_leases == [] + + @pytest.mark.asyncio async def test_cp_rejects_existing_target_without_copying(monkeypatch): agfs = _CopyAGFS(target_exists=True) @@ -385,7 +441,6 @@ async def test_cp_rejects_missing_target_parent_before_locking(monkeypatch): async def test_mv_rejects_missing_target_parent_before_locking(monkeypatch): agfs = _CopyAGFS(parent_exists=False) fs = _viking_fs(monkeypatch, agfs) - monkeypatch.setattr(fs, "_ensure_delete_access", lambda *_args, **_kwargs: None) monkeypatch.setattr(fs, "_update_vector_store_uris", AsyncMock()) with pytest.raises(NotFoundError) as exc_info: @@ -407,7 +462,6 @@ async def test_mv_rejects_missing_target_parent_before_locking(monkeypatch): async def test_mv_removes_partial_target_when_agfs_copy_fails(monkeypatch): agfs = _CopyAGFS(fail_copy=True) fs = _viking_fs(monkeypatch, agfs) - monkeypatch.setattr(fs, "_ensure_delete_access", lambda *_args, **_kwargs: None) vector_move = AsyncMock() monkeypatch.setattr(fs, "_update_vector_store_uris", vector_move) @@ -474,7 +528,6 @@ async def test_cp_removes_partial_target_when_agfs_copy_fails(monkeypatch): async def test_mv_restores_source_and_removes_target_when_source_delete_fails(monkeypatch): agfs = _MoveRollbackAGFS() fs = _viking_fs(monkeypatch, agfs) - monkeypatch.setattr(fs, "_ensure_delete_access", lambda *_args, **_kwargs: None) vector_move = AsyncMock() monkeypatch.setattr(fs, "_update_vector_store_uris", vector_move) @@ -496,11 +549,75 @@ async def test_mv_restores_source_and_removes_target_when_source_delete_fails(mo ) +@pytest.mark.asyncio +async def test_mv_directory_rolls_back_partial_source_delete_under_parent_tree(monkeypatch): + agfs = _DirectoryMoveRollbackAGFS() + fs = _viking_fs(monkeypatch, agfs) + vector_move = AsyncMock() + monkeypatch.setattr(fs, "_update_vector_store_uris", vector_move) + + with pytest.raises(RuntimeError, match="injected partial source delete failure"): + await fs.mv( + "viking://resources/source", + "viking://resources/target", + ctx=_ctx(), + ) + + assert agfs.directories == { + "/local/acct/resources", + "/local/acct/resources/source", + "/local/acct/resources/source/empty", + } + assert agfs.files == { + "/local/acct/resources/source/.hidden": b"hidden", + "/local/acct/resources/source/data.bin": b"\x00\xff", + } + acquire = next(event for event in agfs.events if event[0] == "acquire-batch") + assert acquire[1] == [{"path": "/local/acct/resources", "kind": "tree"}] + assert agfs.exact_leases == [] + assert vector_move.await_args_list[0].args[:2] == ( + "viking://resources/source", + "viking://resources/target", + ) + assert vector_move.await_args_list[1].args[:2] == ( + "viking://resources/target", + "viking://resources/source", + ) + + +@pytest.mark.asyncio +async def test_persist_temp_tree_explicitly_enables_same_mount_fast_path(monkeypatch): + fs = VikingFS.__new__(VikingFS) + fs._async_agfs = SimpleNamespace(cp=AsyncMock()) + monkeypatch.setattr(fs, "_ensure_access", AsyncMock()) + monkeypatch.setattr( + fs, + "_uri_to_path", + lambda uri, **_kwargs: f"/local/acct/{uri.removeprefix('viking://')}", + ) + monkeypatch.setattr(fs, "_pathlock_fs_ctx", lambda *_args, **_kwargs: {"lease_ref": "x"}) + monkeypatch.setattr(fs, "_ensure_parent_dirs", AsyncMock()) + + await fs.persist_temp_tree( + "viking://temp/import-1", + "viking://resources/doc-1", + ctx=_ctx(), + lease_ref={"lease_ref": "x"}, + ) + + fs._async_agfs.cp.assert_awaited_once_with( + "/local/acct/temp/import-1", + "/local/acct/resources/doc-1", + recursive=True, + fs_ctx={"lease_ref": "x"}, + allow_same_mount_fast_path=True, + ) + + @pytest.mark.asyncio async def test_mv_returns_diagnostic_transfer_summary(monkeypatch): agfs = _CopyAGFS() fs = _viking_fs(monkeypatch, agfs) - monkeypatch.setattr(fs, "_ensure_delete_access", lambda *_args, **_kwargs: None) monkeypatch.setattr( fs, "_update_vector_store_uris", @@ -527,7 +644,6 @@ async def test_mv_returns_diagnostic_transfer_summary(monkeypatch): async def test_mv_reports_target_residual_when_vector_failure_cleanup_fails(monkeypatch): agfs = _CopyAGFS() fs = _viking_fs(monkeypatch, agfs) - monkeypatch.setattr(fs, "_ensure_delete_access", lambda *_args, **_kwargs: None) monkeypatch.setattr( fs, "_update_vector_store_uris", @@ -556,7 +672,6 @@ async def test_mv_reports_target_residual_when_vector_failure_cleanup_fails(monk async def test_mv_reports_source_residual_when_source_restore_fails(monkeypatch): agfs = _MoveRollbackAGFS() fs = _viking_fs(monkeypatch, agfs) - monkeypatch.setattr(fs, "_ensure_delete_access", lambda *_args, **_kwargs: None) monkeypatch.setattr(fs, "_update_vector_store_uris", AsyncMock()) monkeypatch.setattr( fs, @@ -588,7 +703,6 @@ async def test_mv_reports_source_residual_when_source_restore_fails(monkeypatch) async def test_mv_rejects_same_path_and_source_subtree(monkeypatch, source, target, source_is_dir): agfs = _CopyAGFS(source_is_dir=source_is_dir) fs = _viking_fs(monkeypatch, agfs) - monkeypatch.setattr(fs, "_ensure_delete_access", lambda *_args, **_kwargs: None) monkeypatch.setattr(fs, "_update_vector_store_uris", AsyncMock()) with pytest.raises(InvalidArgumentError):