From 8f01103661ad1ac54625f9fa03948e0bfe3a0724 Mon Sep 17 00:00:00 2001
From: leyoonafr <codeacme17@gmail.com>
Date: Fri, 26 Jun 2026 15:58:48 +0800
Subject: [PATCH 1/7] feat: Extend concurrency-safe coverage for workspace file
 tools and enhance mutation path handling

---
 .../adapters/vibe/workspace_file_tool.py      |  17 ++
 .../core/tools/core/workspace_file_tool.py    | 158 +++++++++-------
 src/xagent/core/workspace.py                  | 131 +++++++++-----
 .../agent/test_react_real_tool_concurrency.py | 111 +++++++++++-
 .../vibe/test_tool_metadata_concurrency.py    |  34 ++++
 .../tools/test_workspace_file_concurrency.py  | 171 ++++++++++++++++++
 6 files changed, 517 insertions(+), 105 deletions(-)
 create mode 100644 tests/core/tools/test_workspace_file_concurrency.py

diff --git a/src/xagent/core/tools/adapters/vibe/workspace_file_tool.py b/src/xagent/core/tools/adapters/vibe/workspace_file_tool.py
index b900089de..393e0f750 100644
--- a/src/xagent/core/tools/adapters/vibe/workspace_file_tool.py
+++ b/src/xagent/core/tools/adapters/vibe/workspace_file_tool.py
@@ -174,86 +174,103 @@ def get_tools(self) -> List[FunctionTool]:
                 self.read_file,
                 name="read_file",
                 description="Read file content in workspace. Accepts either file paths (e.g., 'filename.txt') or file_ids (e.g., 'abc-123-def'). Automatically detects input type. For large files, results may be truncated in model context; use start_line/end_line to inspect a specific 1-based inclusive line range instead of repeating the same full-file read.",
+                read_only=True,
             ),
             FileTool(
                 self.write_file,
                 name="write_file",
                 description="Write file content in workspace. Use relative paths (e.g., 'filename.txt'), not absolute paths. Returns a FileRef with file_id, preview_url, download_url, and markdown_link.\n\nImportant: For HTML files, do not guess paths to uploaded files or files from other tasks. First call prepare_html_asset(file_id, html_path, alias) for every external image/CSS/JS asset, then use the returned html_src in the HTML.",
+                concurrency_safe=True,
             ),
             FileTool(
                 self.prepare_html_asset,
                 name="prepare_html_asset",
                 description="Prepare an uploaded or registered file for use inside an HTML artifact. Pass the source file_id, the target HTML output path such as 'index.html' or 'reports/index.html', and an optional alias such as 'logo.png'. The tool copies the asset next to that HTML file under assets_subdir and returns html_src relative to the HTML file. Use html_src in <img src>, <link href>, <script src>, or CSS url(). Do not compute ../ paths yourself.",
+                concurrency_safe=True,
             ),
             FileTool(
                 self.append_file,
                 name="append_file",
                 description="Append content to file in workspace. Use relative paths (e.g., 'filename.txt'), not absolute paths.",
+                concurrency_safe=True,
             ),
             FileTool(
                 self.delete_file,
                 name="delete_file",
                 description="Delete file in workspace. Use relative paths (e.g., 'filename.txt'), not absolute paths.",
+                concurrency_safe=True,
             ),
             FileTool(
                 self.list_files,
                 name="list_files",
                 description="List files in workspace directory (defaults to all directories including input, output, temp. Can also specify specific directory like list_files('input'))",
+                read_only=True,
             ),
             FileTool(
                 self.create_directory,
                 name="create_directory",
                 description="Create directory in workspace",
+                concurrency_safe=True,
             ),
             FileTool(
                 self.file_exists,
                 name="file_exists",
                 description="Check if file exists in workspace",
+                read_only=True,
             ),
             FileTool(
                 self.get_file_info,
                 name="get_file_info",
                 description="Get detailed file information in workspace. Accepts either file paths (e.g., 'filename.txt') or file_ids (e.g., 'abc-123-def'). Automatically detects input type.",
+                read_only=True,
             ),
             FileTool(
                 self.read_json_file,
                 name="read_json_file",
                 description="Read JSON file in workspace. Accepts either file paths (e.g., 'filename.txt') or file_ids (e.g., 'abc-123-def'). Automatically detects input type.",
+                read_only=True,
             ),
             FileTool(
                 self.write_json_file,
                 name="write_json_file",
                 description="Write JSON file in workspace. Use relative paths (e.g., 'data.json'), not absolute paths. Returns a FileRef with file_id, preview_url, download_url, and markdown_link.",
+                concurrency_safe=True,
             ),
             FileTool(
                 self.read_csv_file,
                 name="read_csv_file",
                 description="Read CSV file in workspace. Accepts either file paths (e.g., 'filename.txt') or file_ids (e.g., 'abc-123-def'). Automatically detects input type.",
+                read_only=True,
             ),
             FileTool(
                 self.write_csv_file,
                 name="write_csv_file",
                 description="Write CSV file in workspace. Use relative paths (e.g., 'data.csv'), not absolute paths. Returns a FileRef with file_id, preview_url, download_url, and markdown_link.",
+                concurrency_safe=True,
             ),
             FileTool(
                 self.get_workspace_output_files,
                 name="get_workspace_output_files",
                 description="Get output file list from current workspace",
+                read_only=True,
             ),
             FileTool(
                 self.list_all_user_files,
                 name="list_all_user_files",
                 description="List all user files across all workspaces and uploaded files. Supports pagination and filtering. Returns file_id, filename, storage_path, size, mime_type, and whether files are in current workspace.",
+                read_only=True,
             ),
             FileTool(
                 self.edit_file,
                 name="edit_file",
                 description="Precisely edit file content in workspace, supporting multiple edit operations based on line numbers and pattern matching. Use relative paths (e.g., 'filename.txt'), not absolute paths.",
+                concurrency_safe=True,
             ),
             FileTool(
                 self.find_and_replace,
                 name="find_and_replace",
                 description="Convenience function to find and replace text content in workspace. Use relative paths (e.g., 'filename.txt'), not absolute paths.",
+                concurrency_safe=True,
             ),
         ]
 
diff --git a/src/xagent/core/tools/core/workspace_file_tool.py b/src/xagent/core/tools/core/workspace_file_tool.py
index af947170c..9a1beccf2 100644
--- a/src/xagent/core/tools/core/workspace_file_tool.py
+++ b/src/xagent/core/tools/core/workspace_file_tool.py
@@ -327,20 +327,25 @@ def write_file(
         resolved_path = self._resolve_path(file_path, "output")
         logger.debug("Resolved path: %s", resolved_path)
 
-        if create_dirs:
-            logger.debug("Creating parent directories for: %s", resolved_path.parent)
-            resolved_path.parent.mkdir(parents=True, exist_ok=True)
+        with self.workspace.guard_workspace_mutation_path(
+            resolved_path
+        ) as resolved_path:
+            if create_dirs:
+                logger.debug(
+                    "Creating parent directories for: %s", resolved_path.parent
+                )
+                resolved_path.parent.mkdir(parents=True, exist_ok=True)
 
-        # Use auto_register context to automatically register files
-        with self.workspace.auto_register_files():
-            logger.debug("Writing %d bytes to: %s", len(content), resolved_path)
-            with open(resolved_path, "w", encoding=encoding) as f:
-                f.write(content)
+            # Use auto_register context to automatically register files
+            with self.workspace.auto_register_files():
+                logger.debug("Writing %d bytes to: %s", len(content), resolved_path)
+                with open(resolved_path, "w", encoding=encoding) as f:
+                    f.write(content)
 
-        file_ref = build_workspace_file_ref(
-            workspace=self.workspace,
-            file_path=resolved_path,
-        )
+            file_ref = build_workspace_file_ref(
+                workspace=self.workspace,
+                file_path=resolved_path,
+            )
 
         logger.debug(
             "Successfully wrote file: %s with file_id: %s",
@@ -392,27 +397,31 @@ def prepare_html_asset(
         ):
             raise ValueError("assets_subdir must resolve inside output")
 
-        target_dir.mkdir(parents=True, exist_ok=True)
-        target_path = self._build_unique_asset_path(target_dir / asset_name)
+        base_target_path = target_dir / asset_name
+        with self.workspace.guard_workspace_mutation_path(target_dir) as guarded_dir:
+            guarded_dir.mkdir(parents=True, exist_ok=True)
+            target_path = self._build_unique_asset_path(
+                guarded_dir / base_target_path.name
+            )
 
-        with self.workspace.auto_register_files():
-            shutil.copy2(source_path, target_path)
+            with self.workspace.auto_register_files():
+                shutil.copy2(source_path, target_path)
 
-        html_src = Path(
-            os.path.relpath(target_path.resolve(), start=html_output_path.parent)
-        ).as_posix()
-        file_ref = build_workspace_file_ref(
-            workspace=self.workspace,
-            file_path=target_path,
-        )
-        return {
-            "success": True,
-            "source_file_id": source_ref,
-            "asset_file_id": file_ref["file_id"],
-            "html_src": html_src,
-            **file_ref,
-            "file_ref": file_ref,
-        }
+            html_src = Path(
+                os.path.relpath(target_path.resolve(), start=html_output_path.parent)
+            ).as_posix()
+            file_ref = build_workspace_file_ref(
+                workspace=self.workspace,
+                file_path=target_path,
+            )
+            return {
+                "success": True,
+                "source_file_id": source_ref,
+                "asset_file_id": file_ref["file_id"],
+                "html_src": html_src,
+                **file_ref,
+                "file_ref": file_ref,
+            }
 
     def _resolve_html_output_path(self, html_path: str) -> Path:
         path = Path(str(html_path).strip())
@@ -465,21 +474,27 @@ def append_file(
         """Append content to file in workspace"""
         resolved_path = self.workspace.resolve_path_with_search(file_path)
 
-        if create_dirs:
-            resolved_path.parent.mkdir(parents=True, exist_ok=True)
+        with self.workspace.guard_workspace_mutation_path(
+            resolved_path
+        ) as resolved_path:
+            if create_dirs:
+                resolved_path.parent.mkdir(parents=True, exist_ok=True)
 
-        with open(resolved_path, "a", encoding=encoding) as f:
-            f.write(content)
+            with open(resolved_path, "a", encoding=encoding) as f:
+                f.write(content)
         return True
 
     def delete_file(self, file_path: str) -> bool:
         """Delete file in workspace"""
         resolved_path = self.workspace.resolve_path_with_search(file_path)
 
-        if not resolved_path.exists():
-            raise FileNotFoundError(f"File not found: {file_path}")
+        with self.workspace.guard_workspace_mutation_path(
+            resolved_path
+        ) as resolved_path:
+            if not resolved_path.exists():
+                raise FileNotFoundError(f"File not found: {file_path}")
 
-        resolved_path.unlink()
+            resolved_path.unlink()
         return True
 
     def file_exists(self, file_path: str) -> bool:
@@ -546,7 +561,10 @@ def scan_directory(current_path: Path, is_root: bool = True) -> None:
     def create_directory(self, directory_path: str, parents: bool = True) -> bool:
         """Create directory in workspace"""
         resolved_path = self._resolve_path(directory_path)
-        resolved_path.mkdir(parents=parents, exist_ok=True)
+        with self.workspace.guard_workspace_mutation_path(
+            resolved_path
+        ) as resolved_path:
+            resolved_path.mkdir(parents=parents, exist_ok=True)
         return True
 
     def get_file_info(self, file_path_or_id: str) -> FileInfo:
@@ -595,12 +613,15 @@ def write_json_file(
         from .file_tool import write_json_file as basic_write_json_file
 
         resolved_path = self._resolve_path(file_path, "output")
-        resolved_path.parent.mkdir(parents=True, exist_ok=True)
+        with self.workspace.guard_workspace_mutation_path(
+            resolved_path
+        ) as resolved_path:
+            resolved_path.parent.mkdir(parents=True, exist_ok=True)
 
-        with self.workspace.auto_register_files():
-            basic_write_json_file(str(resolved_path), data, encoding, indent)
+            with self.workspace.auto_register_files():
+                basic_write_json_file(str(resolved_path), data, encoding, indent)
 
-        return self._registered_write_result(resolved_path)
+            return self._registered_write_result(resolved_path)
 
     def read_csv_file(
         self,
@@ -637,15 +658,18 @@ def write_csv_file(
         from .file_tool import write_csv_file as basic_write_csv_file
 
         resolved_path = self._resolve_path(file_path, "output")
-        resolved_path.parent.mkdir(parents=True, exist_ok=True)
+        with self.workspace.guard_workspace_mutation_path(
+            resolved_path
+        ) as resolved_path:
+            resolved_path.parent.mkdir(parents=True, exist_ok=True)
 
-        with self.workspace.auto_register_files():
-            if data:
-                basic_write_csv_file(str(resolved_path), data, encoding, delimiter)
-            else:
-                resolved_path.write_text("", encoding=encoding)
+            with self.workspace.auto_register_files():
+                if data:
+                    basic_write_csv_file(str(resolved_path), data, encoding, delimiter)
+                else:
+                    resolved_path.write_text("", encoding=encoding)
 
-        return self._registered_write_result(resolved_path)
+            return self._registered_write_result(resolved_path)
 
     def get_workspace_output_files(self) -> Dict[str, Any]:
         """Get output file list from current workspace"""
@@ -706,11 +730,14 @@ def edit_file(
         resolved_path = self._resolve_path_with_search(file_path)
         logger.debug("Resolved path: %s", resolved_path)
 
-        # Convert to string path for the basic edit_file function
-        str_path = str(resolved_path)
+        with self.workspace.guard_workspace_mutation_path(
+            resolved_path
+        ) as resolved_path:
+            # Convert to string path for the basic edit_file function
+            str_path = str(resolved_path)
 
-        # Call the basic edit_file function with the resolved path
-        result = basic_edit_file(str_path, operations, encoding, backup)
+            # Call the basic edit_file function with the resolved path
+            result = basic_edit_file(str_path, operations, encoding, backup)
 
         logger.debug("edit_file result: %s", result)
         return result
@@ -740,13 +767,22 @@ def find_and_replace(
         resolved_path = self._resolve_path_with_search(file_path)
         logger.debug("Resolved path: %s", resolved_path)
 
-        # Convert to string path for the basic find_and_replace function
-        str_path = str(resolved_path)
-
-        # Call the basic find_and_replace function with the resolved path
-        result = basic_find_and_replace(
-            str_path, pattern, replacement, encoding, use_regex, case_sensitive, backup
-        )
+        with self.workspace.guard_workspace_mutation_path(
+            resolved_path
+        ) as resolved_path:
+            # Convert to string path for the basic find_and_replace function
+            str_path = str(resolved_path)
+
+            # Call the basic find_and_replace function with the resolved path
+            result = basic_find_and_replace(
+                str_path,
+                pattern,
+                replacement,
+                encoding,
+                use_regex,
+                case_sensitive,
+                backup,
+            )
 
         logger.debug("find_and_replace result: %s", result)
         return result
diff --git a/src/xagent/core/workspace.py b/src/xagent/core/workspace.py
index b7d1d520a..46c3bcee2 100644
--- a/src/xagent/core/workspace.py
+++ b/src/xagent/core/workspace.py
@@ -11,6 +11,7 @@
 import os
 import re
 import shutil
+import threading
 import unicodedata
 from contextlib import contextmanager
 from dataclasses import dataclass
@@ -81,6 +82,9 @@ def __init__(
         self.db_session = None  # Optional database session for file registration
         self._recently_registered_files: Dict[str, str] = {}  # path -> file_id mapping
         self._file_id_to_path: Dict[str, Path] = {}  # file_id -> path reverse mapping
+        self._file_registration_lock = threading.RLock()
+        self._mutation_locks_guard = threading.Lock()
+        self._mutation_locks: Dict[str, threading.RLock] = {}
         self.owner_user_id: Optional[int] = None
         self.current_task_id: Optional[int] = (
             db_task_id
@@ -111,6 +115,12 @@ def __init__(
 
     def register_file(
         self, file_path: str, file_id: Optional[str] = None, db_session: Any = None
+    ) -> str:
+        with self._file_registration_lock:
+            return self._register_file_unlocked(file_path, file_id, db_session)
+
+    def _register_file_unlocked(
+        self, file_path: str, file_id: Optional[str] = None, db_session: Any = None
     ) -> str:
         resolved_path = self.resolve_path(file_path, default_dir="output")
         if not resolved_path.exists() or not resolved_path.is_file():
@@ -640,6 +650,44 @@ def get_allowed_dirs(self) -> List[str]:
         dirs.extend([str(d) for d in self.allowed_external_dirs])
         return dirs
 
+    def normalize_workspace_mutation_path(
+        self, file_path: str | Path, default_dir: str = "output"
+    ) -> Path:
+        """Resolve a mutation target to a stable absolute path inside workspace."""
+        path = Path(file_path)
+        if path.is_absolute():
+            resolved_path = self._resolve_allowed_absolute_path(path)
+        else:
+            resolved_path = self.resolve_path(str(path), default_dir=default_dir)
+
+        workspace_abs = self.workspace_dir.resolve()
+        if resolved_path != workspace_abs and not resolved_path.is_relative_to(
+            workspace_abs
+        ):
+            raise ValueError(f"Mutation path {file_path} is outside workspace")
+        return resolved_path
+
+    def _mutation_lock_for_path(self, normalized_path: Path) -> threading.RLock:
+        key = str(normalized_path.resolve())
+        with self._mutation_locks_guard:
+            lock = self._mutation_locks.get(key)
+            if lock is None:
+                lock = threading.RLock()
+                self._mutation_locks[key] = lock
+            return lock
+
+    @contextmanager
+    def guard_workspace_mutation_path(
+        self, file_path: str | Path, default_dir: str = "output"
+    ) -> Iterator[Path]:
+        """Serialize mutations to the same normalized workspace path."""
+        normalized_path = self.normalize_workspace_mutation_path(
+            file_path, default_dir=default_dir
+        )
+        lock = self._mutation_lock_for_path(normalized_path)
+        with lock:
+            yield normalized_path
+
     def _resolve_allowed_absolute_path(self, path: Path) -> Path:
         """Resolve an absolute path after checking workspace allowlists."""
         abs_path = path.resolve()
@@ -984,37 +1032,33 @@ def auto_register_files(self) -> "Iterator[TaskWorkspace]":
         This is safer than relying on manual register_file() calls.
         """
         # Scan files before operation
-        files_before = self._scan_all_files()
+        with self._file_registration_lock:
+            files_before = self._scan_all_files()
 
         try:
             yield self
         finally:
             # Scan files after operation and register new/modified files.
-            files_after = self._scan_all_files()
-            changed_files = files_after - files_before
-            changed_files.update(
-                file_path
-                for file_path in files_after & files_before
-                if self._get_file_id_from_db(file_path, self.db_session) is not None
-            )
+            with self._file_registration_lock:
+                files_after = self._scan_all_files()
+                changed_files = files_after - files_before
+                changed_files.update(
+                    file_path
+                    for file_path in files_after & files_before
+                    if self._get_file_id_from_db(file_path, self.db_session) is not None
+                )
 
-            for file_path in changed_files:
-                try:
-                    file_id = self.register_file(str(file_path))
-                    # Store path -> file_id mapping
-                    path_str = str(file_path)
-                    resolved_str = str(file_path.resolve())
-                    self._recently_registered_files[path_str] = file_id
-                    self._recently_registered_files[resolved_str] = file_id
-                    # Store file_id -> path reverse mapping
-                    self._file_id_to_path[file_id] = file_path
-                    logger.debug(f"Auto-registered file: {file_path} -> {file_id}")
-                except Exception as e:
-                    # Don't generate fake file_id - file will need to be backfilled later
-                    logger.error(
-                        f"Failed to auto-register file {file_path}: {e}. "
-                        f"File exists on disk but is not in database - will require backfill."
-                    )
+                for file_path in changed_files:
+                    try:
+                        file_id = self._register_file_unlocked(str(file_path))
+                        self._remember_file_registration(file_id, file_path)
+                        logger.debug(f"Auto-registered file: {file_path} -> {file_id}")
+                    except Exception as e:
+                        # Don't generate fake file_id - file will need to be backfilled later
+                        logger.error(
+                            f"Failed to auto-register file {file_path}: {e}. "
+                            f"File exists on disk but is not in database - will require backfill."
+                        )
 
     def _scan_all_files(self) -> set[Path]:
         """Scan all files in workspace and return as set."""
@@ -1041,28 +1085,29 @@ def get_file_id_from_path(self, file_path: str) -> Optional[str]:
             resolved_path = Path(file_path).resolve()
             resolved_str = str(resolved_path)
 
-            # Check in-memory cache first (for files just registered)
-            logger.debug(f"get_file_id_from_path: Looking for {resolved_str}")
-            logger.debug(
-                f"get_file_id_from_path: Cache has {len(self._recently_registered_files)} entries: {list(self._recently_registered_files.keys())}"
-            )
-
-            if resolved_str in self._recently_registered_files:
+            with self._file_registration_lock:
+                # Check in-memory cache first (for files just registered)
+                logger.debug(f"get_file_id_from_path: Looking for {resolved_str}")
                 logger.debug(
-                    f"get_file_id_from_path: Found in cache: {self._recently_registered_files[resolved_str]}"
+                    f"get_file_id_from_path: Cache has {len(self._recently_registered_files)} entries: {list(self._recently_registered_files.keys())}"
                 )
-                return self._recently_registered_files[resolved_str]
 
-            # Also try the original path (not resolved)
-            if file_path in self._recently_registered_files:
-                logger.debug(
-                    f"get_file_id_from_path: Found in cache with original path: {self._recently_registered_files[file_path]}"
-                )
-                return self._recently_registered_files[file_path]
+                if resolved_str in self._recently_registered_files:
+                    logger.debug(
+                        f"get_file_id_from_path: Found in cache: {self._recently_registered_files[resolved_str]}"
+                    )
+                    return self._recently_registered_files[resolved_str]
 
-            logger.debug("get_file_id_from_path: Not found in cache, checking DB")
-            # Fall back to database query
-            return self._get_file_id_from_db(resolved_path)
+                # Also try the original path (not resolved)
+                if file_path in self._recently_registered_files:
+                    logger.debug(
+                        f"get_file_id_from_path: Found in cache with original path: {self._recently_registered_files[file_path]}"
+                    )
+                    return self._recently_registered_files[file_path]
+
+                logger.debug("get_file_id_from_path: Not found in cache, checking DB")
+                # Fall back to database query
+                return self._get_file_id_from_db(resolved_path)
         except Exception as e:
             logger.warning(f"get_file_id_from_path: Exception: {e}")
             return None
diff --git a/tests/core/agent/test_react_real_tool_concurrency.py b/tests/core/agent/test_react_real_tool_concurrency.py
index 7f174e2c6..60d72fb67 100644
--- a/tests/core/agent/test_react_real_tool_concurrency.py
+++ b/tests/core/agent/test_react_real_tool_concurrency.py
@@ -24,8 +24,11 @@
 
 import asyncio
 import inspect
+import time
+from contextlib import contextmanager
 from dataclasses import dataclass
-from typing import Any, Callable
+from threading import Lock
+from typing import Any, Callable, Iterator
 
 import pytest
 
@@ -40,7 +43,9 @@
 from xagent.core.tools.adapters.vibe.fetch_web_content import FetchWebContentTool
 from xagent.core.tools.adapters.vibe.tavily_web_search import TavilyWebSearchTool
 from xagent.core.tools.adapters.vibe.web_search import WebSearchTool
+from xagent.core.tools.adapters.vibe.workspace_file_tool import WorkspaceFileTools
 from xagent.core.tools.adapters.vibe.zhipu_web_search import ZhipuWebSearchTool
+from xagent.core.workspace import TaskWorkspace
 
 # How long a batched I/O seam waits to rendezvous with its sibling before giving
 # up. A serialized batch never rendezvouses, so it trips this instead of hanging.
@@ -54,6 +59,26 @@ def as_dict(self) -> dict[str, Any]:
         return {"success": True, "url": "https://example.com", "content": "ok"}
 
 
+class _WorkspaceRegisterTracker:
+    def __init__(self, workspace: TaskWorkspace) -> None:
+        self.workspace = workspace
+        self._lock = Lock()
+        self.active = 0
+        self.peak = 0
+
+    @contextmanager
+    def auto_register_files(self) -> Iterator[TaskWorkspace]:
+        with self._lock:
+            self.active += 1
+            self.peak = max(self.peak, self.active)
+        try:
+            time.sleep(0.05)
+            yield self.workspace
+        finally:
+            with self._lock:
+                self.active -= 1
+
+
 @dataclass
 class _RealToolCase:
     label: str
@@ -174,3 +199,87 @@ async def _seam(*_args: Any, **_kwargs: Any) -> Any:
     assert [r["tool_call_id"] for r in context.tool_results] == [
         tc["id"] for tc in batch
     ]
+
+
+def _workspace_write_tool(
+    tmp_path: Any, workspace_id: str
+) -> tuple[TaskWorkspace, _WorkspaceRegisterTracker, Any]:
+    workspace = TaskWorkspace(workspace_id, str(tmp_path))
+    tracker = _WorkspaceRegisterTracker(workspace)
+    workspace.auto_register_files = tracker.auto_register_files  # type: ignore[method-assign]
+    tool = next(
+        tool
+        for tool in WorkspaceFileTools(workspace).get_tools()
+        if tool.name == "write_file"
+    )
+    return workspace, tracker, tool
+
+
+def test_real_workspace_write_tool_is_batched_as_concurrent(tmp_path: Any) -> None:
+    """The real workspace write tool opts into the ReAct concurrent segment."""
+    _workspace, _tracker, tool = _workspace_write_tool(
+        tmp_path, "react_workspace_segment"
+    )
+    pattern = make_react(parallel=True, max_concurrency=2)
+    batch = [
+        make_tool_call(tool.name, {"file_path": "one.txt", "content": "one"}),
+        make_tool_call(tool.name, {"file_path": "two.txt", "content": "two"}),
+    ]
+
+    segment, kind = pattern._next_segment(batch, [tool])
+
+    assert kind == "concurrent"
+    assert segment == batch
+
+
+async def test_real_workspace_write_tool_serializes_same_path_in_batch(
+    tmp_path: Any,
+) -> None:
+    """Same normalized workspace path stays serial inside a ReAct batch."""
+    workspace, tracker, tool = _workspace_write_tool(
+        tmp_path, "react_workspace_same_path"
+    )
+    pattern = make_react(parallel=True, max_concurrency=2)
+    context = RecordingContext()
+    batch = [
+        make_tool_call(tool.name, {"file_path": "same.txt", "content": "first"}),
+        make_tool_call(
+            tool.name, {"file_path": "output/same.txt", "content": "second"}
+        ),
+    ]
+
+    results = await asyncio.wait_for(
+        pattern._run_concurrent_batch(batch, [tool], FakeRuntime(), context),
+        timeout=_RENDEZVOUS_TIMEOUT * 2,
+    )
+
+    assert tracker.peak == 1
+    assert len(results) == 2
+    assert all(pattern._tool_result_success(result) for result in results)
+    assert (workspace.output_dir / "same.txt").exists()
+
+
+async def test_real_workspace_write_tool_overlaps_different_paths_in_batch(
+    tmp_path: Any,
+) -> None:
+    """Different normalized workspace paths can overlap in a ReAct batch."""
+    workspace, tracker, tool = _workspace_write_tool(
+        tmp_path, "react_workspace_different_paths"
+    )
+    pattern = make_react(parallel=True, max_concurrency=2)
+    context = RecordingContext()
+    batch = [
+        make_tool_call(tool.name, {"file_path": "one.txt", "content": "one"}),
+        make_tool_call(tool.name, {"file_path": "two.txt", "content": "two"}),
+    ]
+
+    results = await asyncio.wait_for(
+        pattern._run_concurrent_batch(batch, [tool], FakeRuntime(), context),
+        timeout=_RENDEZVOUS_TIMEOUT * 2,
+    )
+
+    assert tracker.peak == 2
+    assert len(results) == 2
+    assert all(pattern._tool_result_success(result) for result in results)
+    assert (workspace.output_dir / "one.txt").read_text(encoding="utf-8") == "one"
+    assert (workspace.output_dir / "two.txt").read_text(encoding="utf-8") == "two"
diff --git a/tests/core/tools/adapters/vibe/test_tool_metadata_concurrency.py b/tests/core/tools/adapters/vibe/test_tool_metadata_concurrency.py
index 18d43136c..07928466e 100644
--- a/tests/core/tools/adapters/vibe/test_tool_metadata_concurrency.py
+++ b/tests/core/tools/adapters/vibe/test_tool_metadata_concurrency.py
@@ -40,6 +40,8 @@
     SandboxedToolWrapper,
 )
 from xagent.core.tools.adapters.vibe.web_search import WebSearchTool
+from xagent.core.tools.adapters.vibe.workspace_file_tool import WorkspaceFileTools
+from xagent.core.workspace import TaskWorkspace
 
 
 def _noop(x: int = 0) -> dict[str, Any]:
@@ -127,6 +129,38 @@ def test_builtin_writing_or_stateful_tools_are_not_concurrency_safe() -> None:
     assert PythonExecutorTool().metadata.concurrency_safe is False
 
 
+def test_workspace_file_tool_metadata_matches_guarded_concurrency(tmp_path) -> None:
+    workspace = TaskWorkspace("metadata_workspace_files", str(tmp_path))
+    tools = {tool.name: tool for tool in WorkspaceFileTools(workspace).get_tools()}
+
+    for name in {
+        "read_file",
+        "list_files",
+        "file_exists",
+        "get_file_info",
+        "read_json_file",
+        "read_csv_file",
+        "get_workspace_output_files",
+        "list_all_user_files",
+    }:
+        assert tools[name].metadata.read_only is True
+        assert tools[name].metadata.concurrency_safe is True
+
+    for name in {
+        "write_file",
+        "prepare_html_asset",
+        "append_file",
+        "delete_file",
+        "create_directory",
+        "write_json_file",
+        "write_csv_file",
+        "edit_file",
+        "find_and_replace",
+    }:
+        assert tools[name].metadata.read_only is False
+        assert tools[name].metadata.concurrency_safe is True
+
+
 def test_abstract_base_tool_metadata_exposes_concurrency_fields() -> None:
     # The metadata object must always carry the fields (even when False) so the
     # scheduler can read them uniformly across every tool.
diff --git a/tests/core/tools/test_workspace_file_concurrency.py b/tests/core/tools/test_workspace_file_concurrency.py
new file mode 100644
index 000000000..9709f0db1
--- /dev/null
+++ b/tests/core/tools/test_workspace_file_concurrency.py
@@ -0,0 +1,171 @@
+from __future__ import annotations
+
+import time
+from concurrent.futures import ThreadPoolExecutor
+from contextlib import contextmanager
+from threading import Lock
+from typing import Iterator
+
+import pytest
+
+from xagent.core.tools.core.workspace_file_tool import WorkspaceFileOperations
+from xagent.core.workspace import TaskWorkspace
+
+
+class _RegisterTracker:
+    def __init__(self) -> None:
+        self._lock = Lock()
+        self.active = 0
+        self.peak = 0
+
+    @contextmanager
+    def auto_register_files(self) -> Iterator[TaskWorkspace]:
+        with self._lock:
+            self.active += 1
+            self.peak = max(self.peak, self.active)
+        try:
+            time.sleep(0.05)
+            yield self.workspace
+        finally:
+            with self._lock:
+                self.active -= 1
+
+    def bind(self, workspace: TaskWorkspace) -> "_RegisterTracker":
+        self.workspace = workspace
+        return self
+
+
+class _CallTracker:
+    def __init__(self) -> None:
+        self._lock = Lock()
+        self.active = 0
+        self.peak = 0
+
+    def enter(self) -> None:
+        with self._lock:
+            self.active += 1
+            self.peak = max(self.peak, self.active)
+
+    def leave(self) -> None:
+        with self._lock:
+            self.active -= 1
+
+
+def _install_register_tracker(workspace: TaskWorkspace) -> _RegisterTracker:
+    tracker = _RegisterTracker().bind(workspace)
+    workspace.auto_register_files = tracker.auto_register_files  # type: ignore[method-assign]
+    return tracker
+
+
+def test_same_normalized_workspace_write_path_serializes_registration(
+    tmp_path,
+) -> None:
+    workspace = TaskWorkspace("same_path_serial", str(tmp_path))
+    ops = WorkspaceFileOperations(workspace)
+    tracker = _install_register_tracker(workspace)
+
+    with ThreadPoolExecutor(max_workers=2) as pool:
+        futures = [
+            pool.submit(ops.write_file, "same.txt", "first"),
+            pool.submit(ops.write_file, "output/same.txt", "second"),
+        ]
+        results = [future.result() for future in futures]
+
+    assert tracker.peak == 1
+    assert all(result["success"] for result in results)
+    assert (workspace.output_dir / "same.txt").exists()
+
+
+def test_different_workspace_write_paths_can_overlap_registration(tmp_path) -> None:
+    workspace = TaskWorkspace("different_path_overlap", str(tmp_path))
+    ops = WorkspaceFileOperations(workspace)
+    tracker = _install_register_tracker(workspace)
+
+    with ThreadPoolExecutor(max_workers=2) as pool:
+        futures = [
+            pool.submit(ops.write_file, "one.txt", "one"),
+            pool.submit(ops.write_file, "two.txt", "two"),
+        ]
+        results = [future.result() for future in futures]
+
+    assert tracker.peak == 2
+    assert all(result["success"] for result in results)
+    assert (workspace.output_dir / "one.txt").read_text(encoding="utf-8") == "one"
+    assert (workspace.output_dir / "two.txt").read_text(encoding="utf-8") == "two"
+
+
+def test_concurrent_workspace_writes_return_valid_file_refs(tmp_path) -> None:
+    workspace = TaskWorkspace("file_refs", str(tmp_path))
+    ops = WorkspaceFileOperations(workspace)
+
+    with ThreadPoolExecutor(max_workers=2) as pool:
+        futures = [
+            pool.submit(ops.write_file, "reports/one.txt", "one"),
+            pool.submit(ops.write_file, "reports/two.txt", "two"),
+        ]
+        results = [future.result() for future in futures]
+
+    for result in results:
+        assert result["success"] is True
+        assert result["file_id"]
+        assert result["preview_url"].endswith(result["file_id"])
+        assert result["download_url"].endswith(result["file_id"])
+        assert result["markdown_link"] == (
+            f"[{result['filename']}](file:{result['file_id']})"
+        )
+        assert result["file_ref"]["file_id"] == result["file_id"]
+
+
+def test_workspace_mutation_rejects_paths_outside_workspace(tmp_path) -> None:
+    workspace = TaskWorkspace("outside_boundary", str(tmp_path))
+    ops = WorkspaceFileOperations(workspace)
+
+    with pytest.raises(ValueError, match="outside"):
+        ops.write_file("../../outside.txt", "outside")
+
+    assert not (tmp_path / "outside.txt").exists()
+
+
+def test_prepare_html_asset_serializes_unique_names_in_same_asset_dir(
+    tmp_path,
+) -> None:
+    workspace = TaskWorkspace("html_asset_names", str(tmp_path))
+    ops = WorkspaceFileOperations(workspace)
+    source = ops.write_file("input/logo.png", "fake image")
+
+    existing_asset = workspace.output_dir / "assets" / "logo.png"
+    existing_asset.parent.mkdir(parents=True, exist_ok=True)
+    existing_asset.write_text("existing", encoding="utf-8")
+
+    tracker = _CallTracker()
+    original_build_unique_asset_path = ops._build_unique_asset_path
+
+    def tracked_build_unique_asset_path(path):
+        tracker.enter()
+        try:
+            time.sleep(0.05)
+            return original_build_unique_asset_path(path)
+        finally:
+            tracker.leave()
+
+    ops._build_unique_asset_path = tracked_build_unique_asset_path  # type: ignore[method-assign]
+
+    with ThreadPoolExecutor(max_workers=2) as pool:
+        futures = [
+            pool.submit(
+                ops.prepare_html_asset,
+                source["file_id"],
+                "index.html",
+                "logo.png",
+            ),
+            pool.submit(
+                ops.prepare_html_asset,
+                source["file_id"],
+                "index.html",
+                "logo_1.png",
+            ),
+        ]
+        results = [future.result() for future in futures]
+
+    assert tracker.peak == 1
+    assert len({result["html_src"] for result in results}) == 2

From 8922f4900c07a813dee0d7df0aa9f1a472767a54 Mon Sep 17 00:00:00 2001
From: leyoonafr <codeacme17@gmail.com>
Date: Fri, 26 Jun 2026 16:28:41 +0800
Subject: [PATCH 2/7] feat: Implement concurrency-safe file mutation tools and
 enhance workspace path locking

---
 src/xagent/core/path_locks.py                 |  50 +++++
 .../core/tools/adapters/vibe/file_tool.py     |  34 ++-
 src/xagent/core/tools/core/file_tool.py       | 109 +++++++---
 src/xagent/core/workspace.py                  |  18 +-
 .../vibe/test_tool_metadata_concurrency.py    |  25 ++-
 tests/core/tools/test_file_tool.py            | 196 ++++++++++++++++++
 6 files changed, 382 insertions(+), 50 deletions(-)
 create mode 100644 src/xagent/core/path_locks.py

diff --git a/src/xagent/core/path_locks.py b/src/xagent/core/path_locks.py
new file mode 100644
index 000000000..c58a8c037
--- /dev/null
+++ b/src/xagent/core/path_locks.py
@@ -0,0 +1,50 @@
+"""Shared normalized-path mutation locks."""
+
+from __future__ import annotations
+
+import threading
+from contextlib import ExitStack, contextmanager
+from pathlib import Path
+from typing import Iterator
+
+
+class PathMutationLockRegistry:
+    """Provide re-entrant locks keyed by normalized absolute filesystem path."""
+
+    def __init__(self) -> None:
+        self._guard = threading.Lock()
+        self._locks: dict[str, threading.RLock] = {}
+
+    @staticmethod
+    def normalize_path(path: str | Path) -> Path:
+        return Path(path).expanduser().resolve()
+
+    def _lock_for_normalized_path(self, normalized_path: Path) -> threading.RLock:
+        key = str(normalized_path)
+        with self._guard:
+            lock = self._locks.get(key)
+            if lock is None:
+                lock = threading.RLock()
+                self._locks[key] = lock
+            return lock
+
+    @contextmanager
+    def guard_path(self, path: str | Path) -> Iterator[Path]:
+        normalized_path = self.normalize_path(path)
+        lock = self._lock_for_normalized_path(normalized_path)
+        with lock:
+            yield normalized_path
+
+    @contextmanager
+    def guard_paths(self, paths: list[str | Path]) -> Iterator[tuple[Path, ...]]:
+        normalized_paths = tuple(self.normalize_path(path) for path in paths)
+        unique_paths = {str(path): path for path in normalized_paths}
+        ordered_paths = [unique_paths[key] for key in sorted(unique_paths)]
+
+        with ExitStack() as stack:
+            for normalized_path in ordered_paths:
+                stack.enter_context(self._lock_for_normalized_path(normalized_path))
+            yield normalized_paths
+
+
+GLOBAL_PATH_MUTATION_LOCKS = PathMutationLockRegistry()
diff --git a/src/xagent/core/tools/adapters/vibe/file_tool.py b/src/xagent/core/tools/adapters/vibe/file_tool.py
index 80b243348..33b065715 100644
--- a/src/xagent/core/tools/adapters/vibe/file_tool.py
+++ b/src/xagent/core/tools/adapters/vibe/file_tool.py
@@ -45,12 +45,23 @@ class FileTool(FunctionTool):
     read_only=True,
 )
 write_file_tool = FileTool(
-    write_file, name="write_file", description="Write content to file"
+    write_file,
+    name="write_file",
+    description="Write content to file",
+    concurrency_safe=True,
 )
 append_file_tool = FileTool(
-    append_file, name="append_file", description="Append content to file"
+    append_file,
+    name="append_file",
+    description="Append content to file",
+    concurrency_safe=True,
+)
+delete_file_tool = FileTool(
+    delete_file,
+    name="delete_file",
+    description="Delete file",
+    concurrency_safe=True,
 )
-delete_file_tool = FileTool(delete_file, name="delete_file", description="Delete file")
 list_files_tool = FileTool(
     list_files,
     name="list_files",
@@ -58,7 +69,10 @@ class FileTool(FunctionTool):
     read_only=True,
 )
 create_directory_tool = FileTool(
-    create_directory, name="create_directory", description="Create directory"
+    create_directory,
+    name="create_directory",
+    description="Create directory",
+    concurrency_safe=True,
 )
 file_exists_tool = FileTool(
     file_exists,
@@ -79,7 +93,10 @@ class FileTool(FunctionTool):
     read_only=True,
 )
 write_json_file_tool = FileTool(
-    write_json_file, name="write_json_file", description="Write JSON file"
+    write_json_file,
+    name="write_json_file",
+    description="Write JSON file",
+    concurrency_safe=True,
 )
 read_csv_file_tool = FileTool(
     read_csv_file,
@@ -88,17 +105,22 @@ class FileTool(FunctionTool):
     read_only=True,
 )
 write_csv_file_tool = FileTool(
-    write_csv_file, name="write_csv_file", description="Write CSV file"
+    write_csv_file,
+    name="write_csv_file",
+    description="Write CSV file",
+    concurrency_safe=True,
 )
 edit_file_tool = FileTool(
     edit_file,
     name="edit_file",
     description="Precisely edit file content, supporting various editing operations based on line numbers and pattern matching",
+    concurrency_safe=True,
 )
 find_and_replace_tool = FileTool(
     find_and_replace,
     name="find_and_replace",
     description="Convenient function to find and replace text content",
+    concurrency_safe=True,
 )
 
 
diff --git a/src/xagent/core/tools/core/file_tool.py b/src/xagent/core/tools/core/file_tool.py
index f900ca589..e90ff3a02 100644
--- a/src/xagent/core/tools/core/file_tool.py
+++ b/src/xagent/core/tools/core/file_tool.py
@@ -15,6 +15,8 @@
 
 from pydantic import BaseModel
 
+from ...path_locks import GLOBAL_PATH_MUTATION_LOCKS
+
 try:
     from PIL import Image
 
@@ -80,6 +82,14 @@ class EditResult(BaseModel):
     preview: Optional[str] = None
 
 
+def _guard_basic_file_mutation_path(file_path: str | Path):
+    return GLOBAL_PATH_MUTATION_LOCKS.guard_path(file_path)
+
+
+def _guard_basic_file_mutation_paths(*file_paths: str | Path):
+    return GLOBAL_PATH_MUTATION_LOCKS.guard_paths(list(file_paths))
+
+
 def read_file(
     file_path: str,
     encoding: str = "utf-8",
@@ -180,14 +190,15 @@ def write_file(
         raise ValueError("file_path is required")
     if not content:
         raise ValueError("content is required")
-    if create_dirs:
-        dir_path = os.path.dirname(file_path)
-        if dir_path:  # Only create directory if directory path is not empty
-            os.makedirs(dir_path, exist_ok=True)
-        # If dir_path is empty (e.g., 'hello_world.html'), file is in current directory, no need to create directory
-
-    with open(file_path, "w", encoding=encoding) as f:
-        f.write(content)
+    with _guard_basic_file_mutation_path(file_path):
+        if create_dirs:
+            dir_path = os.path.dirname(file_path)
+            if dir_path:  # Only create directory if directory path is not empty
+                os.makedirs(dir_path, exist_ok=True)
+            # If dir_path is empty (e.g., 'hello_world.html'), file is in current directory, no need to create directory
+
+        with open(file_path, "w", encoding=encoding) as f:
+            f.write(content)
     return True
 
 
@@ -209,15 +220,16 @@ def append_file(
     Raises:
         OSError: File operation error
     """
-    # Handle relative paths, use current directory if file path has no directory part
-    if create_dirs:
-        dir_path = os.path.dirname(file_path)
-        if dir_path:  # Only create directory if directory path is not empty
-            os.makedirs(dir_path, exist_ok=True)
-        # If dir_path is empty (e.g., 'hello_world.html'), file is in current directory, no need to create directory
-
-    with open(file_path, "a", encoding=encoding) as f:
-        f.write(content)
+    with _guard_basic_file_mutation_path(file_path):
+        # Handle relative paths, use current directory if file path has no directory part
+        if create_dirs:
+            dir_path = os.path.dirname(file_path)
+            if dir_path:  # Only create directory if directory path is not empty
+                os.makedirs(dir_path, exist_ok=True)
+            # If dir_path is empty (e.g., 'hello_world.html'), file is in current directory, no need to create directory
+
+        with open(file_path, "a", encoding=encoding) as f:
+            f.write(content)
     return True
 
 
@@ -235,7 +247,8 @@ def delete_file(file_path: str) -> bool:
         FileNotFoundError: File doesn't exist
         OSError: Deletion failed
     """
-    os.remove(file_path)
+    with _guard_basic_file_mutation_path(file_path):
+        os.remove(file_path)
     return True
 
 
@@ -303,7 +316,8 @@ def create_directory(directory_path: str, parents: bool = True) -> bool:
     Raises:
         OSError: Creation failed
     """
-    os.makedirs(directory_path, exist_ok=parents)
+    with _guard_basic_file_mutation_path(directory_path):
+        os.makedirs(directory_path, exist_ok=parents)
     return True
 
 
@@ -425,8 +439,9 @@ def write_json_file(
     Raises:
         OSError: File operation error
     """
-    with open(file_path, "w", encoding=encoding) as f:
-        json.dump(data, f, ensure_ascii=False, indent=indent)
+    with _guard_basic_file_mutation_path(file_path):
+        with open(file_path, "w", encoding=encoding) as f:
+            json.dump(data, f, ensure_ascii=False, indent=indent)
     return True
 
 
@@ -475,13 +490,14 @@ def write_csv_file(
         OSError: File operation error
         csv.Error: CSV write error
     """
-    if not data:
-        return True
-
-    with open(file_path, "w", encoding=encoding, newline="") as f:
-        writer = csv.DictWriter(f, fieldnames=data[0].keys(), delimiter=delimiter)
-        writer.writeheader()
-        writer.writerows(data)
+    with _guard_basic_file_mutation_path(file_path):
+        if not data:
+            return True
+
+        with open(file_path, "w", encoding=encoding, newline="") as f:
+            writer = csv.DictWriter(f, fieldnames=data[0].keys(), delimiter=delimiter)
+            writer.writeheader()
+            writer.writerows(data)
     return True
 
 
@@ -490,6 +506,19 @@ def edit_file(
     operations: List[Union[Dict[str, Any], EditOperation]],
     encoding: str = "utf-8",
     backup: bool = False,
+) -> EditResult:
+    guard_paths = [file_path]
+    if backup:
+        guard_paths.append(f"{file_path}.backup")
+    with _guard_basic_file_mutation_paths(*guard_paths):
+        return _edit_file_unlocked(file_path, operations, encoding, backup)
+
+
+def _edit_file_unlocked(
+    file_path: str,
+    operations: List[Union[Dict[str, Any], EditOperation]],
+    encoding: str = "utf-8",
+    backup: bool = False,
 ) -> EditResult:
     """
     Precisely edit file content, supporting multiple edit operations based on line numbers and pattern matching
@@ -792,6 +821,30 @@ def find_and_replace(
     use_regex: bool = True,
     case_sensitive: bool = True,
     backup: bool = False,
+) -> EditResult:
+    guard_paths = [file_path]
+    if backup:
+        guard_paths.append(f"{file_path}.backup")
+    with _guard_basic_file_mutation_paths(*guard_paths):
+        return _find_and_replace_unlocked(
+            file_path,
+            search_pattern,
+            replacement,
+            encoding,
+            use_regex,
+            case_sensitive,
+            backup,
+        )
+
+
+def _find_and_replace_unlocked(
+    file_path: str,
+    search_pattern: str,
+    replacement: str,
+    encoding: str = "utf-8",
+    use_regex: bool = True,
+    case_sensitive: bool = True,
+    backup: bool = False,
 ) -> EditResult:
     """
     Convenience function to find and replace text content
diff --git a/src/xagent/core/workspace.py b/src/xagent/core/workspace.py
index 46c3bcee2..b0185f188 100644
--- a/src/xagent/core/workspace.py
+++ b/src/xagent/core/workspace.py
@@ -20,6 +20,7 @@
 from uuid import uuid4
 
 from ..config import get_uploads_dir
+from .path_locks import GLOBAL_PATH_MUTATION_LOCKS
 
 logger = logging.getLogger(__name__)
 
@@ -83,8 +84,7 @@ def __init__(
         self._recently_registered_files: Dict[str, str] = {}  # path -> file_id mapping
         self._file_id_to_path: Dict[str, Path] = {}  # file_id -> path reverse mapping
         self._file_registration_lock = threading.RLock()
-        self._mutation_locks_guard = threading.Lock()
-        self._mutation_locks: Dict[str, threading.RLock] = {}
+        self._mutation_path_locks = GLOBAL_PATH_MUTATION_LOCKS
         self.owner_user_id: Optional[int] = None
         self.current_task_id: Optional[int] = (
             db_task_id
@@ -667,15 +667,6 @@ def normalize_workspace_mutation_path(
             raise ValueError(f"Mutation path {file_path} is outside workspace")
         return resolved_path
 
-    def _mutation_lock_for_path(self, normalized_path: Path) -> threading.RLock:
-        key = str(normalized_path.resolve())
-        with self._mutation_locks_guard:
-            lock = self._mutation_locks.get(key)
-            if lock is None:
-                lock = threading.RLock()
-                self._mutation_locks[key] = lock
-            return lock
-
     @contextmanager
     def guard_workspace_mutation_path(
         self, file_path: str | Path, default_dir: str = "output"
@@ -684,9 +675,8 @@ def guard_workspace_mutation_path(
         normalized_path = self.normalize_workspace_mutation_path(
             file_path, default_dir=default_dir
         )
-        lock = self._mutation_lock_for_path(normalized_path)
-        with lock:
-            yield normalized_path
+        with self._mutation_path_locks.guard_path(normalized_path) as guarded_path:
+            yield guarded_path
 
     def _resolve_allowed_absolute_path(self, path: Path) -> Path:
         """Resolve an absolute path after checking workspace allowlists."""
diff --git a/tests/core/tools/adapters/vibe/test_tool_metadata_concurrency.py b/tests/core/tools/adapters/vibe/test_tool_metadata_concurrency.py
index 07928466e..1195545aa 100644
--- a/tests/core/tools/adapters/vibe/test_tool_metadata_concurrency.py
+++ b/tests/core/tools/adapters/vibe/test_tool_metadata_concurrency.py
@@ -25,8 +25,15 @@
 from xagent.core.tools.adapters.vibe.base import ToolMetadata
 from xagent.core.tools.adapters.vibe.fetch_web_content import FetchWebContentTool
 from xagent.core.tools.adapters.vibe.file_tool import (
+    append_file_tool,
+    create_directory_tool,
+    delete_file_tool,
+    edit_file_tool,
+    find_and_replace_tool,
     read_file_tool,
+    write_csv_file_tool,
     write_file_tool,
+    write_json_file_tool,
 )
 from xagent.core.tools.adapters.vibe.function import FunctionTool
 from xagent.core.tools.adapters.vibe.output_filter_wrapper import (
@@ -124,8 +131,22 @@ def test_builtin_read_only_tools_are_concurrency_safe() -> None:
     assert read_file_tool.metadata.concurrency_safe is True
 
 
-def test_builtin_writing_or_stateful_tools_are_not_concurrency_safe() -> None:
-    assert write_file_tool.metadata.concurrency_safe is False
+def test_basic_file_mutation_tools_match_guarded_concurrency() -> None:
+    for tool in {
+        write_file_tool,
+        append_file_tool,
+        delete_file_tool,
+        create_directory_tool,
+        write_json_file_tool,
+        write_csv_file_tool,
+        edit_file_tool,
+        find_and_replace_tool,
+    }:
+        assert tool.metadata.read_only is False
+        assert tool.metadata.concurrency_safe is True
+
+
+def test_stateful_tools_without_call_isolation_are_not_concurrency_safe() -> None:
     assert PythonExecutorTool().metadata.concurrency_safe is False
 
 
diff --git a/tests/core/tools/test_file_tool.py b/tests/core/tools/test_file_tool.py
index 80c580ed7..757553042 100644
--- a/tests/core/tools/test_file_tool.py
+++ b/tests/core/tools/test_file_tool.py
@@ -1,14 +1,22 @@
 import os
 import tempfile
+import time
+from concurrent.futures import ThreadPoolExecutor
+from threading import Lock
 
 import pytest
 
+from xagent.core.tools.core import file_tool as file_tool_module
+from xagent.core.tools.core import workspace_file_tool as workspace_file_tool_module
+from xagent.core.tools.core.workspace_file_tool import WorkspaceFileOperations
 from xagent.core.tools.adapters.vibe.file_tool import (
     FILE_TOOLS,
     append_file,
     create_directory,
     delete_file,
+    edit_file,
     file_exists,
+    find_and_replace,
     get_file_info,
     list_files,
     read_csv_file,
@@ -18,6 +26,64 @@
     write_file,
     write_json_file,
 )
+from xagent.core.workspace import TaskWorkspace
+
+
+class _ConcurrencyTracker:
+    def __init__(self) -> None:
+        self._lock = Lock()
+        self.active = 0
+        self.peak = 0
+
+    def enter(self) -> None:
+        with self._lock:
+            self.active += 1
+            self.peak = max(self.peak, self.active)
+
+    def leave(self) -> None:
+        with self._lock:
+            self.active -= 1
+
+
+class _TrackedFile:
+    def __init__(self, handle, tracker: _ConcurrencyTracker) -> None:
+        self._handle = handle
+        self._tracker = tracker
+
+    def __enter__(self):
+        entered = self._handle.__enter__()
+        self._tracker.enter()
+        time.sleep(0.05)
+        return entered
+
+    def __exit__(self, *args):
+        try:
+            return self._handle.__exit__(*args)
+        finally:
+            self._tracker.leave()
+
+    def __getattr__(self, name):
+        return getattr(self._handle, name)
+
+
+def _install_tracked_open(
+    monkeypatch: pytest.MonkeyPatch,
+    tracker: _ConcurrencyTracker,
+    module=file_tool_module,
+) -> None:
+    real_open = open
+
+    def tracked_open(*args, **kwargs):
+        return _TrackedFile(real_open(*args, **kwargs), tracker)
+
+    monkeypatch.setattr(module, "open", tracked_open, raising=False)
+
+
+def _run_concurrently(*calls) -> None:
+    with ThreadPoolExecutor(max_workers=len(calls)) as pool:
+        futures = [pool.submit(call) for call in calls]
+        for future in futures:
+            future.result()
 
 
 def test_basic_file_operations():
@@ -264,6 +330,136 @@ def test_non_image_file_info():
             delete_file(temp_file)
 
 
+@pytest.mark.parametrize(
+    "case_name",
+    [
+        "write_file",
+        "append_file",
+        "write_json_file",
+        "write_csv_file",
+        "edit_file",
+        "find_and_replace",
+        "delete_file",
+        "create_directory",
+    ],
+)
+def test_same_normalized_basic_file_mutations_are_serialized(
+    monkeypatch: pytest.MonkeyPatch, tmp_path, case_name: str
+) -> None:
+    tracker = _ConcurrencyTracker()
+    path = tmp_path / "same.txt"
+    same_path_alias = tmp_path / "." / "same.txt"
+
+    if case_name in {"delete_file", "create_directory"}:
+
+        def tracked_call(*args, **kwargs):
+            tracker.enter()
+            try:
+                time.sleep(0.05)
+            finally:
+                tracker.leave()
+            return None
+
+        if case_name == "delete_file":
+            monkeypatch.setattr(file_tool_module.os, "remove", tracked_call)
+            calls = [
+                lambda: delete_file(str(path)),
+                lambda: delete_file(str(same_path_alias)),
+            ]
+        else:
+            monkeypatch.setattr(file_tool_module.os, "makedirs", tracked_call)
+            directory_path = tmp_path / "same_dir"
+            same_directory_alias = tmp_path / "." / "same_dir"
+            calls = [
+                lambda: create_directory(str(directory_path)),
+                lambda: create_directory(str(same_directory_alias)),
+            ]
+    else:
+        _install_tracked_open(monkeypatch, tracker)
+
+        if case_name in {"edit_file", "find_and_replace"}:
+            path.write_text("needle\n", encoding="utf-8")
+
+        if case_name == "write_file":
+            calls = [
+                lambda: write_file(str(path), "first"),
+                lambda: write_file(str(same_path_alias), "second"),
+            ]
+        elif case_name == "append_file":
+            calls = [
+                lambda: append_file(str(path), "first"),
+                lambda: append_file(str(same_path_alias), "second"),
+            ]
+        elif case_name == "write_json_file":
+            calls = [
+                lambda: write_json_file(str(path), {"value": "first"}),
+                lambda: write_json_file(str(same_path_alias), {"value": "second"}),
+            ]
+        elif case_name == "write_csv_file":
+            calls = [
+                lambda: write_csv_file(str(path), [{"value": "first"}]),
+                lambda: write_csv_file(str(same_path_alias), [{"value": "second"}]),
+            ]
+        elif case_name == "edit_file":
+            operations = [
+                {
+                    "operation_type": "replace",
+                    "line_number": 1,
+                    "content": "needle",
+                }
+            ]
+            calls = [
+                lambda: edit_file(str(path), operations),
+                lambda: edit_file(str(same_path_alias), operations),
+            ]
+        else:
+            calls = [
+                lambda: find_and_replace(
+                    str(path), "needle", "needle", use_regex=False
+                ),
+                lambda: find_and_replace(
+                    str(same_path_alias), "needle", "needle", use_regex=False
+                ),
+            ]
+
+    _run_concurrently(*calls)
+
+    assert tracker.peak == 1
+
+
+def test_different_basic_file_mutation_paths_can_overlap(
+    monkeypatch: pytest.MonkeyPatch, tmp_path
+) -> None:
+    tracker = _ConcurrencyTracker()
+    _install_tracked_open(monkeypatch, tracker)
+
+    _run_concurrently(
+        lambda: write_file(str(tmp_path / "one.txt"), "one"),
+        lambda: write_file(str(tmp_path / "two.txt"), "two"),
+    )
+
+    assert tracker.peak == 2
+
+
+def test_basic_and_workspace_file_mutations_share_normalized_path_lock(
+    monkeypatch: pytest.MonkeyPatch, tmp_path
+) -> None:
+    tracker = _ConcurrencyTracker()
+    _install_tracked_open(monkeypatch, tracker, file_tool_module)
+    _install_tracked_open(monkeypatch, tracker, workspace_file_tool_module)
+
+    workspace = TaskWorkspace("shared_basic_workspace_lock", str(tmp_path))
+    ops = WorkspaceFileOperations(workspace)
+    target_path = workspace.output_dir / "same.txt"
+
+    _run_concurrently(
+        lambda: ops.write_file("same.txt", "workspace"),
+        lambda: write_file(str(target_path), "basic"),
+    )
+
+    assert tracker.peak == 1
+
+
 def test_image_file_info_without_pil():
     """Test that get_file_info handles PIL unavailability gracefully."""
     from unittest.mock import patch

From 4d59a5024051d122770b2f255520f15804953a2b Mon Sep 17 00:00:00 2001
From: leyoonafr <codeacme17@gmail.com>
Date: Sat, 27 Jun 2026 17:09:48 +0800
Subject: [PATCH 3/7] Add concurrency safety tests for browser tools and
 enhance session management

- Introduced tests for concurrency-safe metadata exposure in browser tools.
- Implemented teardown tests for browser task sessions to ensure proper closure of derived sessions.
- Added async context managers to manage browser session operations and ensure serialized access for the same session.
- Enhanced browser evaluation tool tests to verify serialization of operations for the same session and allow overlapping for different sessions.
- Updated browser screenshot and PDF tools to ensure unique filenames during concurrent operations.
---
 .../core/tools/adapters/vibe/browser_use.py   | 107 ++-
 src/xagent/core/tools/core/browser_use.py     | 751 +++++++++---------
 tests/core/agent/test_browser_tools.py        | 253 ++++++
 .../agent/test_react_real_tool_concurrency.py | 146 +++-
 tests/core/tools/test_browser_use.py          | 172 ++++
 5 files changed, 1024 insertions(+), 405 deletions(-)

diff --git a/src/xagent/core/tools/adapters/vibe/browser_use.py b/src/xagent/core/tools/adapters/vibe/browser_use.py
index c1102f682..d0317c20a 100644
--- a/src/xagent/core/tools/adapters/vibe/browser_use.py
+++ b/src/xagent/core/tools/adapters/vibe/browser_use.py
@@ -7,6 +7,7 @@
 
 import logging
 import os
+import uuid
 from typing import Any, Mapping, Optional, Type
 
 from pydantic import BaseModel, Field
@@ -34,6 +35,19 @@
 _STEP_SESSION_ARG = "_xagent_step_id"
 
 
+def _browser_output_filename(
+    output_filename: Any, *, prefix: str, extension: str
+) -> str:
+    if output_filename:
+        return os.path.basename(str(output_filename))
+
+    from datetime import datetime
+
+    timestamp = datetime.now().strftime("%Y%m%d_%H%M%S_%f")
+    unique_suffix = uuid.uuid4().hex[:8]
+    return f"{prefix}_{timestamp}_{unique_suffix}.{extension}"
+
+
 class BrowserTaskSessionMixin:
     """Keeps browser tool default sessions aligned during task setup."""
 
@@ -324,6 +338,7 @@ class BrowserPdfResult(BaseModel):
 
 class BrowserNavigateTool(BrowserTaskSessionMixin, AbstractBaseTool):
     category = ToolCategory.BROWSER
+    concurrency_safe = True
     """Navigate to a URL in a browser session."""
 
     def __init__(
@@ -491,6 +506,7 @@ async def teardown(self, task_id: Optional[str] = None) -> None:
 
 class BrowserClickTool(BrowserTaskSessionMixin, AbstractBaseTool):
     category = ToolCategory.BROWSER
+    concurrency_safe = True
     """Click an element on the current page."""
 
     def __init__(self, task_id: Optional[str] = None):
@@ -539,6 +555,7 @@ async def run_json_async(self, args: Mapping[str, Any]) -> Any:
 
 class BrowserFillTool(BrowserTaskSessionMixin, AbstractBaseTool):
     category = ToolCategory.BROWSER
+    concurrency_safe = True
     """Fill an input field with text."""
 
     def __init__(self, task_id: Optional[str] = None):
@@ -587,6 +604,7 @@ async def run_json_async(self, args: Mapping[str, Any]) -> Any:
 
 class BrowserScreenshotTool(BrowserTaskSessionMixin, AbstractBaseTool):
     category = ToolCategory.BROWSER
+    concurrency_safe = True
     """Take a screenshot of the current page."""
 
     def __init__(
@@ -650,7 +668,6 @@ async def run_json_async(self, args: Mapping[str, Any]) -> Any:
         if self._workspace and result.get("success"):
             try:
                 import base64
-                from datetime import datetime
 
                 # Extract base64 data from data URI
                 screenshot_data = result.get("screenshot", "")
@@ -663,29 +680,30 @@ async def run_json_async(self, args: Mapping[str, Any]) -> Any:
                 image_bytes = base64.b64decode(base64_data)
 
                 # Determine filename - always save to output directory
-                output_filename = args.get("output_filename")
-                if output_filename:
-                    # Sanitize filename to prevent path traversal attacks
-                    filename = os.path.basename(output_filename)
-                else:
-                    timestamp = datetime.now().strftime("%Y%m%d_%H%M%S")
-                    filename = f"screenshot_{timestamp}.png"
+                filename = _browser_output_filename(
+                    args.get("output_filename"), prefix="screenshot", extension="png"
+                )
                 # Always save to output directory
                 file_path = self._workspace.output_dir / filename
 
-                # Save to file within auto_register context
-                with self._workspace.auto_register_files():
-                    with open(file_path, "wb") as f:
-                        f.write(image_bytes)
-
-                relative_path = str(
-                    file_path.relative_to(self._workspace.workspace_dir)
-                )
-                file_ref = build_workspace_file_ref(
-                    workspace=self._workspace,
-                    file_path=file_path,
-                    mime_type="image/png",
-                )
+                # Save and register under the shared path guard so concurrent
+                # explicit filenames cannot interleave writes or metadata.
+                with self._workspace.guard_workspace_mutation_path(
+                    file_path
+                ) as guarded_path:
+                    file_path = guarded_path
+                    with self._workspace.auto_register_files():
+                        with open(file_path, "wb") as f:
+                            f.write(image_bytes)
+
+                    relative_path = str(
+                        file_path.relative_to(self._workspace.workspace_dir)
+                    )
+                    file_ref = build_workspace_file_ref(
+                        workspace=self._workspace,
+                        file_path=file_path,
+                        mime_type="image/png",
+                    )
                 result["screenshot"] = relative_path
                 result["format"] = "file"
                 result["file_id"] = file_ref["file_id"]
@@ -707,6 +725,7 @@ async def run_json_async(self, args: Mapping[str, Any]) -> Any:
 
 class BrowserExtractTextTool(BrowserTaskSessionMixin, AbstractBaseTool):
     category = ToolCategory.BROWSER
+    concurrency_safe = True
     """Extract text content from the page."""
 
     def __init__(self, task_id: Optional[str] = None):
@@ -754,6 +773,7 @@ async def run_json_async(self, args: Mapping[str, Any]) -> Any:
 
 class BrowserEvaluateTool(BrowserTaskSessionMixin, AbstractBaseTool):
     category = ToolCategory.BROWSER
+    concurrency_safe = True
     """Execute JavaScript code in the browser."""
 
     def __init__(self, task_id: Optional[str] = None):
@@ -861,6 +881,7 @@ async def run_json_async(self, args: Mapping[str, Any]) -> Any:
 
 class BrowserSelectOptionTool(BrowserTaskSessionMixin, AbstractBaseTool):
     category = ToolCategory.BROWSER
+    concurrency_safe = True
     """Select an option from a dropdown."""
 
     def __init__(self, task_id: Optional[str] = None):
@@ -910,6 +931,7 @@ async def run_json_async(self, args: Mapping[str, Any]) -> Any:
 
 class BrowserWaitForSelectorTool(BrowserTaskSessionMixin, AbstractBaseTool):
     category = ToolCategory.BROWSER
+    concurrency_safe = True
     """Wait for an element to appear on the page."""
 
     def __init__(self, task_id: Optional[str] = None):
@@ -1001,6 +1023,7 @@ async def run_json_async(self, args: Mapping[str, Any]) -> Any:
 
 class BrowserPdfTool(BrowserTaskSessionMixin, AbstractBaseTool):
     category = ToolCategory.BROWSER
+    concurrency_safe = True
     """Save current page as PDF."""
 
     def __init__(
@@ -1060,7 +1083,6 @@ async def run_json_async(self, args: Mapping[str, Any]) -> Any:
         if self._workspace and result.get("success"):
             try:
                 import base64
-                from datetime import datetime
 
                 # Extract base64 data
                 pdf_data = result.get("pdf", "")
@@ -1073,30 +1095,31 @@ async def run_json_async(self, args: Mapping[str, Any]) -> Any:
                 pdf_bytes = base64.b64decode(base64_data)
 
                 # Determine filename - always save to output directory
-                output_filename = args.get("output_filename")
-                if output_filename:
-                    # Sanitize filename to prevent path traversal attacks
-                    filename = os.path.basename(output_filename)
-                else:
-                    timestamp = datetime.now().strftime("%Y%m%d_%H%M%S")
-                    filename = f"page_{timestamp}.pdf"
+                filename = _browser_output_filename(
+                    args.get("output_filename"), prefix="page", extension="pdf"
+                )
 
                 # Always save to output directory
                 file_path = self._workspace.output_dir / filename
 
-                # Save to file within auto_register context
-                with self._workspace.auto_register_files():
-                    with open(file_path, "wb") as f:
-                        f.write(pdf_bytes)
-
-                relative_path = str(
-                    file_path.relative_to(self._workspace.workspace_dir)
-                )
-                file_ref = build_workspace_file_ref(
-                    workspace=self._workspace,
-                    file_path=file_path,
-                    mime_type="application/pdf",
-                )
+                # Save and register under the shared path guard so concurrent
+                # explicit filenames cannot interleave writes or metadata.
+                with self._workspace.guard_workspace_mutation_path(
+                    file_path
+                ) as guarded_path:
+                    file_path = guarded_path
+                    with self._workspace.auto_register_files():
+                        with open(file_path, "wb") as f:
+                            f.write(pdf_bytes)
+
+                    relative_path = str(
+                        file_path.relative_to(self._workspace.workspace_dir)
+                    )
+                    file_ref = build_workspace_file_ref(
+                        workspace=self._workspace,
+                        file_path=file_path,
+                        mime_type="application/pdf",
+                    )
                 result["output_path"] = relative_path
                 result["format"] = "file"
                 result["file_id"] = file_ref["file_id"]
diff --git a/src/xagent/core/tools/core/browser_use.py b/src/xagent/core/tools/core/browser_use.py
index 43edcec25..ba6b7f0ae 100644
--- a/src/xagent/core/tools/core/browser_use.py
+++ b/src/xagent/core/tools/core/browser_use.py
@@ -7,8 +7,9 @@
 """
 
 import asyncio
+from contextlib import asynccontextmanager
 from datetime import datetime, timedelta
-from typing import TYPE_CHECKING, Any, Dict, Optional
+from typing import TYPE_CHECKING, Any, AsyncIterator, Dict, Optional
 
 if TYPE_CHECKING:
     from playwright.async_api import Browser, BrowserContext, Page, async_playwright
@@ -72,6 +73,13 @@ def __init__(self, session_id: str, headless: bool = True):
         self._created_at = datetime.now()
         self._last_used = datetime.now()
         self._initialized = False
+        self._operation_lock = asyncio.Lock()
+
+    @asynccontextmanager
+    async def operation_guard(self) -> AsyncIterator[None]:
+        """Serialize operations that target this session's page/context."""
+        async with self._operation_lock:
+            yield
 
     async def _ensure_initialized(self) -> None:
         """Lazy initialization: create browser on first use."""
@@ -162,6 +170,11 @@ async def get_page(self) -> Page:
 
     async def close(self) -> None:
         """Close the browser and cleanup resources."""
+        async with self.operation_guard():
+            await self._close_unlocked()
+
+    async def _close_unlocked(self) -> None:
+        """Close resources while the caller holds the operation guard."""
         try:
             # Close page first
             if self._page:
@@ -362,71 +375,72 @@ async def browser_navigate(**kwargs: Any) -> Dict[str, Any]:
 
     manager = get_browser_manager()
     session = await manager.get_or_create(session_id, headless)
-    page = await session.get_page()
-
-    try:
-        # For local files, use 'domcontentloaded' or 'load' instead of 'networkidle'
-        # because local files don't have network activity
-        if url.startswith("file://"):
-            # Override wait_until for local files
-            wait_until_local = (
-                wait_until if wait_until != "networkidle" else "domcontentloaded"
-            )
-        else:
-            wait_until_local = wait_until
+    async with session.operation_guard():
+        page = await session.get_page()
 
-        # Log navigation attempt for debugging
-        import logging
+        try:
+            # For local files, use 'domcontentloaded' or 'load' instead of 'networkidle'
+            # because local files don't have network activity
+            if url.startswith("file://"):
+                # Override wait_until for local files
+                wait_until_local = (
+                    wait_until if wait_until != "networkidle" else "domcontentloaded"
+                )
+            else:
+                wait_until_local = wait_until
 
-        logger = logging.getLogger(__name__)
-        logger.info(
-            f"[browser_navigate] Navigating to {url} (headless={headless}, wait_until={wait_until_local})"
-        )
+            # Log navigation attempt for debugging
+            import logging
 
-        # Add timeout to prevent hanging (default 30 seconds)
-        # Using wait_until parameter (default: 'networkidle', but can be 'domcontentloaded' or 'load')
-        # networkidle can hang on pages with continuous background requests
-        await page.goto(url, wait_until=wait_until_local, timeout=30000)  # type: ignore[arg-type]
-        # Get title (no timeout parameter available for title())
-        title = await page.title()
+            logger = logging.getLogger(__name__)
+            logger.info(
+                f"[browser_navigate] Navigating to {url} (headless={headless}, wait_until={wait_until_local})"
+            )
 
-        logger.info(
-            f"[browser_navigate] Successfully navigated to {url}, title: {title}"
-        )
+            # Add timeout to prevent hanging (default 30 seconds)
+            # Using wait_until parameter (default: 'networkidle', but can be 'domcontentloaded' or 'load')
+            # networkidle can hang on pages with continuous background requests
+            await page.goto(url, wait_until=wait_until_local, timeout=30000)  # type: ignore[arg-type]
+            # Get title (no timeout parameter available for title())
+            title = await page.title()
 
-        return {
-            "success": True,
-            "session_id": session_id,
-            "url": url,
-            "title": title,
-            "message": (
-                f"Navigation complete: opened {url}. This only confirms the page "
-                "loaded; it does not inspect content, validate visual rendering, "
-                "or create an image export. Do not call browser_navigate again for "
-                "this same URL unless navigation failed or the target URL changed. "
-                "Use browser_screenshot for visual inspection/rendered output, "
-                "or browser_extract_text for page text. "
-                f"Session ID: '{session_id}'."
-            ),
-        }
-    except Exception as e:
-        import logging
-        import traceback
+            logger.info(
+                f"[browser_navigate] Successfully navigated to {url}, title: {title}"
+            )
 
-        logger = logging.getLogger(__name__)
-        logger.error(f"[browser_navigate] Failed to navigate to {url}: {str(e)}")
-        logger.error(f"[browser_navigate] Traceback: {traceback.format_exc()}")
+            return {
+                "success": True,
+                "session_id": session_id,
+                "url": url,
+                "title": title,
+                "message": (
+                    f"Navigation complete: opened {url}. This only confirms the page "
+                    "loaded; it does not inspect content, validate visual rendering, "
+                    "or create an image export. Do not call browser_navigate again for "
+                    "this same URL unless navigation failed or the target URL changed. "
+                    "Use browser_screenshot for visual inspection/rendered output, "
+                    "or browser_extract_text for page text. "
+                    f"Session ID: '{session_id}'."
+                ),
+            }
+        except Exception as e:
+            import logging
+            import traceback
 
-        return {
-            "success": False,
-            "session_id": session_id,
-            "url": url,
-            "title": "",
-            "message": "",
-            "error": _format_error_with_traceback(
-                e, f"Navigation failed for URL '{url}'"
-            ),
-        }
+            logger = logging.getLogger(__name__)
+            logger.error(f"[browser_navigate] Failed to navigate to {url}: {str(e)}")
+            logger.error(f"[browser_navigate] Traceback: {traceback.format_exc()}")
+
+            return {
+                "success": False,
+                "session_id": session_id,
+                "url": url,
+                "title": "",
+                "message": "",
+                "error": _format_error_with_traceback(
+                    e, f"Navigation failed for URL '{url}'"
+                ),
+            }
 
 
 async def browser_click(**kwargs: Any) -> Dict[str, Any]:
@@ -466,26 +480,27 @@ async def browser_click(**kwargs: Any) -> Dict[str, Any]:
 
     manager = get_browser_manager()
     session = await manager.get_or_create(session_id)
-    page = await session.get_page()
+    async with session.operation_guard():
+        page = await session.get_page()
 
-    try:
-        await page.click(selector, timeout=timeout)
-        return {
-            "success": True,
-            "session_id": session_id,
-            "selector": selector,
-            "message": f"Successfully clicked element: {selector}. Session ID: {session_id}",
-        }
-    except Exception as e:
-        return {
-            "success": False,
-            "session_id": session_id,
-            "selector": selector,
-            "message": "",
-            "error": _format_error_with_traceback(
-                e, f"Click failed for selector '{selector}'"
-            ),
-        }
+        try:
+            await page.click(selector, timeout=timeout)
+            return {
+                "success": True,
+                "session_id": session_id,
+                "selector": selector,
+                "message": f"Successfully clicked element: {selector}. Session ID: {session_id}",
+            }
+        except Exception as e:
+            return {
+                "success": False,
+                "session_id": session_id,
+                "selector": selector,
+                "message": "",
+                "error": _format_error_with_traceback(
+                    e, f"Click failed for selector '{selector}'"
+                ),
+            }
 
 
 async def browser_fill(**kwargs: Any) -> Dict[str, Any]:
@@ -527,30 +542,31 @@ async def browser_fill(**kwargs: Any) -> Dict[str, Any]:
 
     manager = get_browser_manager()
     session = await manager.get_or_create(session_id)
-    page = await session.get_page()
+    async with session.operation_guard():
+        page = await session.get_page()
 
-    try:
-        # Add timeout to prevent hanging (default 30 seconds)
-        await page.fill(selector, value, timeout=30000)
-        preview = value[:50] + "..." if len(value) > 50 else value
-        return {
-            "success": True,
-            "session_id": session_id,
-            "selector": selector,
-            "value": preview,
-            "message": f"Filled {selector} with: {preview}. Session ID: {session_id}",
-        }
-    except Exception as e:
-        return {
-            "success": False,
-            "session_id": session_id,
-            "selector": selector,
-            "value": "",
-            "message": "",
-            "error": _format_error_with_traceback(
-                e, f"Fill failed for selector '{selector}'"
-            ),
-        }
+        try:
+            # Add timeout to prevent hanging (default 30 seconds)
+            await page.fill(selector, value, timeout=30000)
+            preview = value[:50] + "..." if len(value) > 50 else value
+            return {
+                "success": True,
+                "session_id": session_id,
+                "selector": selector,
+                "value": preview,
+                "message": f"Filled {selector} with: {preview}. Session ID: {session_id}",
+            }
+        except Exception as e:
+            return {
+                "success": False,
+                "session_id": session_id,
+                "selector": selector,
+                "value": "",
+                "message": "",
+                "error": _format_error_with_traceback(
+                    e, f"Fill failed for selector '{selector}'"
+                ),
+            }
 
 
 async def browser_screenshot(**kwargs: Any) -> Dict[str, Any]:
@@ -625,141 +641,144 @@ async def browser_screenshot(**kwargs: Any) -> Dict[str, Any]:
     session = await manager.get_or_create(session_id)
     logger.info(f"[browser_screenshot] Got/created session (session={session_id})")
 
-    page = await session.get_page()
-    logger.info(f"[browser_screenshot] Got page object (session={session_id})")
-
-    # Set viewport size if width or height is specified
-    current_viewport = page.viewport_size
-    if current_viewport is not None and (
-        current_viewport.get("width") != width
-        or current_viewport.get("height") != height
-    ):
-        logger.info(
-            f"[browser_screenshot] Setting viewport size to {width}x{height} (session={session_id})"
-        )
-        await page.set_viewport_size({"width": width, "height": height})
-
     # Track actual screenshot mode (may differ from requested if fallback occurs)
     actual_full_page = full_page
 
-    try:
-        # Screenshot returns bytes, convert to base64 with data URI prefix
-        import base64
+    async with session.operation_guard():
+        page = await session.get_page()
+        logger.info(f"[browser_screenshot] Got page object (session={session_id})")
 
-        logger = logging.getLogger(__name__)
-
-        logger.info(
-            f"[browser_screenshot] Starting screenshot (session={session_id}, full_page={full_page})"
-        )
+        # Set viewport size if width or height is specified
+        current_viewport = page.viewport_size
+        if current_viewport is not None and (
+            current_viewport.get("width") != width
+            or current_viewport.get("height") != height
+        ):
+            logger.info(
+                f"[browser_screenshot] Setting viewport size to {width}x{height} (session={session_id})"
+            )
+            await page.set_viewport_size({"width": width, "height": height})
 
-        if not full_page:
-            # Viewport-only screenshot (fast, reliable)
-            try:
-                logger.info(
-                    f"[browser_screenshot] Taking viewport screenshot (session={session_id})"
-                )
+        try:
+            # Screenshot returns bytes, convert to base64 with data URI prefix
+            import base64
 
-                screenshot_bytes = await page.screenshot(
-                    full_page=False,
-                    type="png",
-                )
+            logger = logging.getLogger(__name__)
 
-                logger.info(
-                    f"[browser_screenshot] Viewport screenshot captured (session={session_id}, size={len(screenshot_bytes)} bytes)"
-                )
-            except Exception as screenshot_error:
-                raise screenshot_error
-        else:
-            # full_page=True: Use Playwright's full_page (works fine in clean asyncio)
             logger.info(
-                f"[browser_screenshot] Using Playwright full_page (session={session_id}, wait_for_lazy_load={wait_for_lazy_load})"
+                f"[browser_screenshot] Starting screenshot (session={session_id}, full_page={full_page})"
             )
 
-            if wait_for_lazy_load and full_page:
-                # Trigger lazy loading by scrolling the page before screenshot
-                logger.info(
-                    f"[browser_screenshot] Triggering lazy loading before screenshot (session={session_id})"
-                )
+            if not full_page:
+                # Viewport-only screenshot (fast, reliable)
+                try:
+                    logger.info(
+                        f"[browser_screenshot] Taking viewport screenshot (session={session_id})"
+                    )
+
+                    screenshot_bytes = await page.screenshot(
+                        full_page=False,
+                        type="png",
+                    )
 
-                # Get page height
-                page_height = await page.evaluate("document.body.scrollHeight")
+                    logger.info(
+                        f"[browser_screenshot] Viewport screenshot captured (session={session_id}, size={len(screenshot_bytes)} bytes)"
+                    )
+                except Exception as screenshot_error:
+                    raise screenshot_error
+            else:
+                # full_page=True: Use Playwright's full_page (works fine in clean asyncio)
                 logger.info(
-                    f"[browser_screenshot] Page height: {page_height}px (session={session_id})"
+                    f"[browser_screenshot] Using Playwright full_page (session={session_id}, wait_for_lazy_load={wait_for_lazy_load})"
                 )
 
-                # Scroll through the page in chunks to trigger lazy loading
-                viewport_size = page.viewport_size
-                viewport_height = (
-                    viewport_size.get("height", 1080) if viewport_size else 1080
-                )
-                scroll_position = 0
-                scroll_count = 0
-                max_scrolls = 50  # Prevent infinite loops
+                if wait_for_lazy_load and full_page:
+                    # Trigger lazy loading by scrolling the page before screenshot
+                    logger.info(
+                        f"[browser_screenshot] Triggering lazy loading before screenshot (session={session_id})"
+                    )
 
-                while scroll_position < page_height and scroll_count < max_scrolls:
+                    # Get page height
+                    page_height = await page.evaluate("document.body.scrollHeight")
                     logger.info(
-                        f"[browser_screenshot] Scrolling to y={scroll_position}/{page_height} (session={session_id}, scroll {scroll_count + 1}/{max_scrolls})"
+                        f"[browser_screenshot] Page height: {page_height}px (session={session_id})"
                     )
-                    await page.evaluate(f"window.scrollTo(0, {scroll_position})")
-                    await page.wait_for_timeout(300)  # Wait 300ms for content to load
-                    scroll_position += viewport_height
-                    scroll_count += 1
 
-                # Scroll back to top before screenshot
-                logger.info(
-                    f"[browser_screenshot] Scrolled back to top (session={session_id})"
-                )
-                await page.evaluate("window.scrollTo(0, 0)")
-                await page.wait_for_timeout(500)  # Wait for page to stabilize
+                    # Scroll through the page in chunks to trigger lazy loading
+                    viewport_size = page.viewport_size
+                    viewport_height = (
+                        viewport_size.get("height", 1080) if viewport_size else 1080
+                    )
+                    scroll_position = 0
+                    scroll_count = 0
+                    max_scrolls = 50  # Prevent infinite loops
+
+                    while scroll_position < page_height and scroll_count < max_scrolls:
+                        logger.info(
+                            f"[browser_screenshot] Scrolling to y={scroll_position}/{page_height} (session={session_id}, scroll {scroll_count + 1}/{max_scrolls})"
+                        )
+                        await page.evaluate(f"window.scrollTo(0, {scroll_position})")
+                        await page.wait_for_timeout(
+                            300
+                        )  # Wait 300ms for content to load
+                        scroll_position += viewport_height
+                        scroll_count += 1
+
+                    # Scroll back to top before screenshot
+                    logger.info(
+                        f"[browser_screenshot] Scrolled back to top (session={session_id})"
+                    )
+                    await page.evaluate("window.scrollTo(0, 0)")
+                    await page.wait_for_timeout(500)  # Wait for page to stabilize
 
-            try:
-                logger.info(
-                    f"[browser_screenshot] Calling page.screenshot(full_page=True) (session={session_id})"
-                )
-                screenshot_bytes = await page.screenshot(
-                    full_page=True,
-                    type="png",
-                )
-                logger.info(
-                    f"[browser_screenshot] Full page screenshot successful: {len(screenshot_bytes)} bytes (session={session_id})"
-                )
-            except Exception as native_error:
-                logger.warning(
-                    f"[browser_screenshot] Full page failed: {native_error}, falling back to viewport (session={session_id})"
-                )
-                # Fallback to viewport
-                screenshot_bytes = await page.screenshot(
-                    full_page=False,
-                    type="png",
-                )
-                actual_full_page = False
+                try:
+                    logger.info(
+                        f"[browser_screenshot] Calling page.screenshot(full_page=True) (session={session_id})"
+                    )
+                    screenshot_bytes = await page.screenshot(
+                        full_page=True,
+                        type="png",
+                    )
+                    logger.info(
+                        f"[browser_screenshot] Full page screenshot successful: {len(screenshot_bytes)} bytes (session={session_id})"
+                    )
+                except Exception as native_error:
+                    logger.warning(
+                        f"[browser_screenshot] Full page failed: {native_error}, falling back to viewport (session={session_id})"
+                    )
+                    # Fallback to viewport
+                    screenshot_bytes = await page.screenshot(
+                        full_page=False,
+                        type="png",
+                    )
+                    actual_full_page = False
 
-        screenshot_b64 = base64.b64encode(screenshot_bytes).decode("utf-8")
-        # Add data URI prefix for compatibility with vision tools
-        screenshot_data_uri = f"data:image/png;base64,{screenshot_b64}"
+            screenshot_b64 = base64.b64encode(screenshot_bytes).decode("utf-8")
+            # Add data URI prefix for compatibility with vision tools
+            screenshot_data_uri = f"data:image/png;base64,{screenshot_b64}"
 
-        return {
-            "success": True,
-            "session_id": session_id,
-            "screenshot": screenshot_data_uri,
-            "format": "base64",
-            "full_page": actual_full_page,  # Will be False if we fell back
-            "wait_for_lazy_load": wait_for_lazy_load,
-            "message": f"Screenshot captured successfully (full_page={actual_full_page}). Session ID: {session_id}",
-        }
-    except Exception as e:
-        return {
-            "success": False,
-            "session_id": session_id,
-            "screenshot": "",
-            "format": "base64",
-            "full_page": actual_full_page,
-            "wait_for_lazy_load": wait_for_lazy_load,
-            "message": "",
-            "error": _format_error_with_traceback(
-                e, f"Screenshot failed for session '{session_id}'"
-            ),
-        }
+            return {
+                "success": True,
+                "session_id": session_id,
+                "screenshot": screenshot_data_uri,
+                "format": "base64",
+                "full_page": actual_full_page,  # Will be False if we fell back
+                "wait_for_lazy_load": wait_for_lazy_load,
+                "message": f"Screenshot captured successfully (full_page={actual_full_page}). Session ID: {session_id}",
+            }
+        except Exception as e:
+            return {
+                "success": False,
+                "session_id": session_id,
+                "screenshot": "",
+                "format": "base64",
+                "full_page": actual_full_page,
+                "wait_for_lazy_load": wait_for_lazy_load,
+                "message": "",
+                "error": _format_error_with_traceback(
+                    e, f"Screenshot failed for session '{session_id}'"
+                ),
+            }
 
 
 async def browser_extract_text(**kwargs: Any) -> Dict[str, Any]:
@@ -801,49 +820,50 @@ async def browser_extract_text(**kwargs: Any) -> Dict[str, Any]:
 
     manager = get_browser_manager()
     session = await manager.get_or_create(session_id)
-    page = await session.get_page()
+    async with session.operation_guard():
+        page = await session.get_page()
 
-    try:
-        # Try inner_text() first with timeout
-        element = page.locator(selector)
         try:
-            # Wait for element to be attached with short timeout
-            await element.wait_for(state="attached", timeout=5000)
-            text = await element.inner_text(timeout=10000)
-        except Exception as inner_error:
-            # Fallback: use JavaScript textContent which is more reliable
-            import json
-
-            escaped_selector = json.dumps(selector)
+            # Try inner_text() first with timeout
+            element = page.locator(selector)
             try:
-                text = await page.evaluate(
-                    f'document.querySelector({escaped_selector})?.textContent || ""'
-                )
-                if not text or not isinstance(text, str):
+                # Wait for element to be attached with short timeout
+                await element.wait_for(state="attached", timeout=5000)
+                text = await element.inner_text(timeout=10000)
+            except Exception as inner_error:
+                # Fallback: use JavaScript textContent which is more reliable
+                import json
+
+                escaped_selector = json.dumps(selector)
+                try:
+                    text = await page.evaluate(
+                        f'document.querySelector({escaped_selector})?.textContent || ""'
+                    )
+                    if not text or not isinstance(text, str):
+                        raise inner_error  # Re-raise original error if JS also fails
+                except Exception:
                     raise inner_error  # Re-raise original error if JS also fails
-            except Exception:
-                raise inner_error  # Re-raise original error
 
-        return {
-            "success": True,
-            "session_id": session_id,
-            "selector": selector,
-            "text": text,
-            "length": len(text),
-            "message": f"Extracted {len(text)} characters from {selector}. Session ID: {session_id}",
-        }
-    except Exception as e:
-        return {
-            "success": False,
-            "session_id": session_id,
-            "selector": selector,
-            "text": "",
-            "length": 0,
-            "message": "",
-            "error": _format_error_with_traceback(
-                e, f"Text extraction failed for selector '{selector}'"
-            ),
-        }
+            return {
+                "success": True,
+                "session_id": session_id,
+                "selector": selector,
+                "text": text,
+                "length": len(text),
+                "message": f"Extracted {len(text)} characters from {selector}. Session ID: {session_id}",
+            }
+        except Exception as e:
+            return {
+                "success": False,
+                "session_id": session_id,
+                "selector": selector,
+                "text": "",
+                "length": 0,
+                "message": "",
+                "error": _format_error_with_traceback(
+                    e, f"Text extraction failed for selector '{selector}'"
+                ),
+            }
 
 
 async def browser_evaluate(**kwargs: Any) -> Dict[str, Any]:
@@ -881,25 +901,29 @@ async def browser_evaluate(**kwargs: Any) -> Dict[str, Any]:
 
     manager = get_browser_manager()
     session = await manager.get_or_create(session_id)
-    page = await session.get_page()
 
-    try:
-        result = await page.evaluate(javascript)
+    async with session.operation_guard():
+        page = await session.get_page()
 
-        return {
-            "success": True,
-            "session_id": session_id,
-            "result": result,
-            "message": f"JavaScript executed successfully. Session ID: {session_id}",
-        }
-    except Exception as e:
-        return {
-            "success": False,
-            "session_id": session_id,
-            "result": None,
-            "message": "",
-            "error": _format_error_with_traceback(e, "JavaScript evaluation failed"),
-        }
+        try:
+            result = await page.evaluate(javascript)
+
+            return {
+                "success": True,
+                "session_id": session_id,
+                "result": result,
+                "message": f"JavaScript executed successfully. Session ID: {session_id}",
+            }
+        except Exception as e:
+            return {
+                "success": False,
+                "session_id": session_id,
+                "result": None,
+                "message": "",
+                "error": _format_error_with_traceback(
+                    e, "JavaScript evaluation failed"
+                ),
+            }
 
 
 async def browser_select_option(**kwargs: Any) -> Dict[str, Any]:
@@ -955,30 +979,41 @@ async def browser_select_option(**kwargs: Any) -> Dict[str, Any]:
 
     manager = get_browser_manager()
     session = await manager.get_or_create(session_id)
-    page = await session.get_page()
+    async with session.operation_guard():
+        page = await session.get_page()
 
-    try:
-        if value is not None:
-            # Add timeout to prevent hanging (default 30 seconds)
-            await page.select_option(selector, value=value, timeout=30000)
-            return {
-                "success": True,
-                "session_id": session_id,
-                "selector": selector,
-                "selected_value": value,
-                "message": f"Selected option with value: {value}. Session ID: {session_id}",
-            }
-        elif index is not None:
-            # Add timeout to prevent hanging (default 30 seconds)
-            await page.select_option(selector, index=index, timeout=30000)
-            return {
-                "success": True,
-                "session_id": session_id,
-                "selector": selector,
-                "selected_index": index,
-                "message": f"Selected option at index: {index}. Session ID: {session_id}",
-            }
-        else:
+        try:
+            if value is not None:
+                # Add timeout to prevent hanging (default 30 seconds)
+                await page.select_option(selector, value=value, timeout=30000)
+                return {
+                    "success": True,
+                    "session_id": session_id,
+                    "selector": selector,
+                    "selected_value": value,
+                    "message": f"Selected option with value: {value}. Session ID: {session_id}",
+                }
+            elif index is not None:
+                # Add timeout to prevent hanging (default 30 seconds)
+                await page.select_option(selector, index=index, timeout=30000)
+                return {
+                    "success": True,
+                    "session_id": session_id,
+                    "selector": selector,
+                    "selected_index": index,
+                    "message": f"Selected option at index: {index}. Session ID: {session_id}",
+                }
+            else:
+                return {
+                    "success": False,
+                    "session_id": session_id,
+                    "selector": selector,
+                    "selected_value": "",
+                    "selected_index": None,
+                    "message": "",
+                    "error": "Either value or index must be provided",
+                }
+        except Exception as e:
             return {
                 "success": False,
                 "session_id": session_id,
@@ -986,20 +1021,10 @@ async def browser_select_option(**kwargs: Any) -> Dict[str, Any]:
                 "selected_value": "",
                 "selected_index": None,
                 "message": "",
-                "error": "Either value or index must be provided",
+                "error": _format_error_with_traceback(
+                    e, f"Select option failed for selector '{selector}'"
+                ),
             }
-    except Exception as e:
-        return {
-            "success": False,
-            "session_id": session_id,
-            "selector": selector,
-            "selected_value": "",
-            "selected_index": None,
-            "message": "",
-            "error": _format_error_with_traceback(
-                e, f"Select option failed for selector '{selector}'"
-            ),
-        }
 
 
 async def browser_wait_for_selector(**kwargs: Any) -> Dict[str, Any]:
@@ -1039,26 +1064,27 @@ async def browser_wait_for_selector(**kwargs: Any) -> Dict[str, Any]:
 
     manager = get_browser_manager()
     session = await manager.get_or_create(session_id)
-    page = await session.get_page()
+    async with session.operation_guard():
+        page = await session.get_page()
 
-    try:
-        await page.wait_for_selector(selector, timeout=timeout)
-        return {
-            "success": True,
-            "session_id": session_id,
-            "selector": selector,
-            "message": f"Element found: {selector}. Session ID: {session_id}",
-        }
-    except Exception as e:
-        return {
-            "success": False,
-            "session_id": session_id,
-            "selector": selector,
-            "message": "",
-            "error": _format_error_with_traceback(
-                e, f"Timeout waiting for selector '{selector}'"
-            ),
-        }
+        try:
+            await page.wait_for_selector(selector, timeout=timeout)
+            return {
+                "success": True,
+                "session_id": session_id,
+                "selector": selector,
+                "message": f"Element found: {selector}. Session ID: {session_id}",
+            }
+        except Exception as e:
+            return {
+                "success": False,
+                "session_id": session_id,
+                "selector": selector,
+                "message": "",
+                "error": _format_error_with_traceback(
+                    e, f"Timeout waiting for selector '{selector}'"
+                ),
+            }
 
 
 async def browser_close(session_id: str) -> Dict[str, Any]:
@@ -1148,47 +1174,48 @@ async def browser_pdf(**kwargs: Any) -> Dict[str, Any]:
 
     manager = get_browser_manager()
     session = await manager.get_or_create(session_id)
-    page = await session.get_page()
+    async with session.operation_guard():
+        page = await session.get_page()
 
-    try:
-        import logging
+        try:
+            import logging
 
-        logger = logging.getLogger(__name__)
-        logger.info(
-            f"[browser_pdf] Generating PDF (session={session_id}, format={format}, landscape={landscape})"
-        )
+            logger = logging.getLogger(__name__)
+            logger.info(
+                f"[browser_pdf] Generating PDF (session={session_id}, format={format}, landscape={landscape})"
+            )
 
-        # Generate PDF from current page
-        pdf_bytes = await page.pdf(
-            landscape=landscape,
-            format=format,
-            print_background=print_background,
-        )
+            # Generate PDF from current page
+            pdf_bytes = await page.pdf(
+                landscape=landscape,
+                format=format,
+                print_background=print_background,
+            )
 
-        # Encode to base64
-        import base64
+            # Encode to base64
+            import base64
 
-        pdf_base64 = base64.b64encode(pdf_bytes).decode("utf-8")
-        file_size = len(pdf_bytes)
+            pdf_base64 = base64.b64encode(pdf_bytes).decode("utf-8")
+            file_size = len(pdf_bytes)
 
-        logger.info(f"[browser_pdf] PDF generated successfully ({file_size} bytes)")
+            logger.info(f"[browser_pdf] PDF generated successfully ({file_size} bytes)")
 
-        return {
-            "success": True,
-            "session_id": session_id,
-            "pdf": pdf_base64,
-            "format": "base64",
-            "size": file_size,
-            "message": f"PDF generated successfully ({file_size} bytes). Session ID: {session_id}",
-        }
-    except Exception as e:
-        return {
-            "success": False,
-            "session_id": session_id,
-            "pdf": "",
-            "size": 0,
-            "message": "",
-            "error": _format_error_with_traceback(
-                e, f"PDF generation failed for session '{session_id}'"
-            ),
-        }
+            return {
+                "success": True,
+                "session_id": session_id,
+                "pdf": pdf_base64,
+                "format": "base64",
+                "size": file_size,
+                "message": f"PDF generated successfully ({file_size} bytes). Session ID: {session_id}",
+            }
+        except Exception as e:
+            return {
+                "success": False,
+                "session_id": session_id,
+                "pdf": "",
+                "size": 0,
+                "message": "",
+                "error": _format_error_with_traceback(
+                    e, f"PDF generation failed for session '{session_id}'"
+                ),
+            }
diff --git a/tests/core/agent/test_browser_tools.py b/tests/core/agent/test_browser_tools.py
index 246a005ee..3328fb6fe 100644
--- a/tests/core/agent/test_browser_tools.py
+++ b/tests/core/agent/test_browser_tools.py
@@ -1,10 +1,13 @@
 from __future__ import annotations
 
+import asyncio
+from contextlib import contextmanager
 import inspect
 
 import pytest
 
 from xagent.core.tools.adapters.vibe.browser_use import (
+    BrowserPdfTool,
     BrowserScreenshotTool,
     BrowserTaskSessionMixin,
     create_browser_tools,
@@ -74,6 +77,59 @@ def test_browser_task_session_mixin_keeps_explicit_session() -> None:
     assert "_xagent_step_id" not in args
 
 
+def test_browser_session_tools_expose_concurrency_safe_metadata() -> None:
+    tools = create_browser_tools(task_id="task-412")
+    safe_tool_names = {
+        "browser_navigate",
+        "browser_click",
+        "browser_fill",
+        "browser_screenshot",
+        "browser_extract_text",
+        "browser_pdf",
+        "browser_evaluate",
+        "browser_select_option",
+        "browser_wait_for_selector",
+    }
+
+    metadata_by_name = {tool.name: tool.metadata for tool in tools}
+
+    assert {
+        name for name, metadata in metadata_by_name.items() if metadata.concurrency_safe
+    } >= safe_tool_names
+    assert metadata_by_name["browser_close"].concurrency_safe is False
+    assert metadata_by_name["browser_list_sessions"].concurrency_safe is False
+
+
+@pytest.mark.asyncio
+async def test_browser_task_session_teardown_closes_derived_task_sessions(
+    monkeypatch,
+) -> None:
+    closed_session_ids = []
+    manager = _FakeBrowserManager(
+        {
+            "task-412": _ClosableBrowserSession("task-412", closed_session_ids),
+            "task-412:render_english": _ClosableBrowserSession(
+                "task-412:render_english", closed_session_ids
+            ),
+            "task-412-preview": _ClosableBrowserSession(
+                "task-412-preview", closed_session_ids
+            ),
+            "other-task": _ClosableBrowserSession("other-task", closed_session_ids),
+        }
+    )
+    monkeypatch.setattr(
+        "xagent.core.tools.adapters.vibe.browser_use.get_browser_manager",
+        lambda: manager,
+    )
+    tool = BrowserTaskSessionMixin()
+    tool._task_id = "task-412"
+
+    await tool.teardown()
+
+    assert closed_session_ids == ["task-412", "task-412:render_english"]
+    assert set(manager._sessions) == {"task-412-preview", "other-task"}
+
+
 @pytest.mark.asyncio
 async def test_browser_screenshot_returns_registered_file_ref(
     tmp_path, monkeypatch
@@ -125,3 +181,200 @@ async def fake_browser_screenshot(**kwargs):
     assert result["file_ref"]["file_id"] == result["file_id"]
     assert result["file_ref"]["relative_path"] == "output/poster_en.png"
     assert result["markdown_link"] == (f"[poster_en.png](file:{result['file_id']})")
+
+
+@pytest.mark.asyncio
+async def test_browser_screenshot_save_uses_workspace_path_guard(
+    tmp_path, monkeypatch
+) -> None:
+    def mock_create_record(self, file_id, file_path, db_session=None):
+        self._file_id_to_path[file_id] = file_path
+
+    monkeypatch.setattr(TaskWorkspace, "_create_file_record", mock_create_record)
+
+    async def fake_browser_screenshot(**kwargs):
+        return {
+            "success": True,
+            "session_id": kwargs["session_id"],
+            "screenshot": (
+                "data:image/png;base64,"
+                "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAA"
+                "DElEQVR42mNk+M9QDwADhgGAWjR9awAAAABJRU5ErkJggg=="
+            ),
+            "format": "png",
+            "full_page": False,
+            "wait_for_lazy_load": False,
+            "message": "ok",
+            "error": "",
+        }
+
+    monkeypatch.setattr(
+        "xagent.core.tools.adapters.vibe.browser_use.browser_screenshot",
+        fake_browser_screenshot,
+    )
+
+    workspace = TaskWorkspace("test_task", str(tmp_path))
+    guard_calls = []
+    original_guard = workspace.guard_workspace_mutation_path
+
+    @contextmanager
+    def tracked_guard(file_path, default_dir="output"):
+        guard_calls.append(file_path)
+        with original_guard(file_path, default_dir=default_dir) as guarded_path:
+            yield guarded_path
+
+    workspace.guard_workspace_mutation_path = tracked_guard
+    tool = BrowserScreenshotTool(task_id="task-412", workspace=workspace)
+
+    result = await tool.run_json_async(
+        {
+            "output_filename": "poster_en.png",
+            "_xagent_step_id": "render_english",
+        }
+    )
+
+    assert result["success"] is True
+    assert guard_calls == [workspace.output_dir / "poster_en.png"]
+
+
+@pytest.mark.asyncio
+async def test_browser_pdf_save_uses_workspace_path_guard(
+    tmp_path, monkeypatch
+) -> None:
+    def mock_create_record(self, file_id, file_path, db_session=None):
+        self._file_id_to_path[file_id] = file_path
+
+    monkeypatch.setattr(TaskWorkspace, "_create_file_record", mock_create_record)
+
+    async def fake_browser_pdf(**kwargs):
+        return {
+            "success": True,
+            "session_id": kwargs["session_id"],
+            "pdf": "JVBERi0xLjQK",
+            "format": "base64",
+            "size": 8,
+            "message": "ok",
+            "error": "",
+        }
+
+    monkeypatch.setattr(
+        "xagent.core.tools.adapters.vibe.browser_use.browser_pdf",
+        fake_browser_pdf,
+    )
+
+    workspace = TaskWorkspace("test_task", str(tmp_path))
+    guard_calls = []
+    original_guard = workspace.guard_workspace_mutation_path
+
+    @contextmanager
+    def tracked_guard(file_path, default_dir="output"):
+        guard_calls.append(file_path)
+        with original_guard(file_path, default_dir=default_dir) as guarded_path:
+            yield guarded_path
+
+    workspace.guard_workspace_mutation_path = tracked_guard
+    tool = BrowserPdfTool(task_id="task-412", workspace=workspace)
+
+    result = await tool.run_json_async(
+        {
+            "output_filename": "page.pdf",
+            "_xagent_step_id": "render_english",
+        }
+    )
+
+    assert result["success"] is True
+    assert guard_calls == [workspace.output_dir / "page.pdf"]
+
+
+@pytest.mark.asyncio
+async def test_browser_screenshot_auto_filenames_are_unique(
+    tmp_path, monkeypatch
+) -> None:
+    def mock_create_record(self, file_id, file_path, db_session=None):
+        self._file_id_to_path[file_id] = file_path
+
+    monkeypatch.setattr(TaskWorkspace, "_create_file_record", mock_create_record)
+
+    async def fake_browser_screenshot(**kwargs):
+        return {
+            "success": True,
+            "session_id": kwargs["session_id"],
+            "screenshot": (
+                "data:image/png;base64,"
+                "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAA"
+                "DElEQVR42mNk+M9QDwADhgGAWjR9awAAAABJRU5ErkJggg=="
+            ),
+            "format": "png",
+            "full_page": False,
+            "wait_for_lazy_load": False,
+            "message": "ok",
+            "error": "",
+        }
+
+    monkeypatch.setattr(
+        "xagent.core.tools.adapters.vibe.browser_use.browser_screenshot",
+        fake_browser_screenshot,
+    )
+
+    workspace = TaskWorkspace("test_task", str(tmp_path))
+    tool = BrowserScreenshotTool(task_id="task-412", workspace=workspace)
+
+    first, second = await asyncio.gather(
+        tool.run_json_async({}), tool.run_json_async({})
+    )
+
+    assert first["success"] is True
+    assert second["success"] is True
+    assert first["screenshot"] != second["screenshot"]
+
+
+@pytest.mark.asyncio
+async def test_browser_pdf_auto_filenames_are_unique(tmp_path, monkeypatch) -> None:
+    def mock_create_record(self, file_id, file_path, db_session=None):
+        self._file_id_to_path[file_id] = file_path
+
+    monkeypatch.setattr(TaskWorkspace, "_create_file_record", mock_create_record)
+
+    async def fake_browser_pdf(**kwargs):
+        return {
+            "success": True,
+            "session_id": kwargs["session_id"],
+            "pdf": "JVBERi0xLjQK",
+            "format": "base64",
+            "size": 8,
+            "message": "ok",
+            "error": "",
+        }
+
+    monkeypatch.setattr(
+        "xagent.core.tools.adapters.vibe.browser_use.browser_pdf",
+        fake_browser_pdf,
+    )
+
+    workspace = TaskWorkspace("test_task", str(tmp_path))
+    tool = BrowserPdfTool(task_id="task-412", workspace=workspace)
+
+    first, second = await asyncio.gather(
+        tool.run_json_async({}), tool.run_json_async({})
+    )
+
+    assert first["success"] is True
+    assert second["success"] is True
+    assert first["output_path"] != second["output_path"]
+
+
+class _ClosableBrowserSession:
+    def __init__(self, session_id, closed_session_ids):
+        self._session_id = session_id
+        self._closed_session_ids = closed_session_ids
+
+    async def close(self) -> None:
+        self._closed_session_ids.append(self._session_id)
+
+
+class _FakeBrowserManager:
+    def __init__(self, sessions):
+        import asyncio
+
+        self._lock = asyncio.Lock()
+        self._sessions = sessions
diff --git a/tests/core/agent/test_react_real_tool_concurrency.py b/tests/core/agent/test_react_real_tool_concurrency.py
index 60d72fb67..326aab155 100644
--- a/tests/core/agent/test_react_real_tool_concurrency.py
+++ b/tests/core/agent/test_react_real_tool_concurrency.py
@@ -25,7 +25,7 @@
 import asyncio
 import inspect
 import time
-from contextlib import contextmanager
+from contextlib import asynccontextmanager, contextmanager
 from dataclasses import dataclass
 from threading import Lock
 from typing import Any, Callable, Iterator
@@ -41,10 +41,12 @@
 )
 from xagent.core.tools.adapters.vibe.exa_web_search import ExaWebSearchTool
 from xagent.core.tools.adapters.vibe.fetch_web_content import FetchWebContentTool
+from xagent.core.tools.adapters.vibe.browser_use import BrowserEvaluateTool
 from xagent.core.tools.adapters.vibe.tavily_web_search import TavilyWebSearchTool
 from xagent.core.tools.adapters.vibe.web_search import WebSearchTool
 from xagent.core.tools.adapters.vibe.workspace_file_tool import WorkspaceFileTools
 from xagent.core.tools.adapters.vibe.zhipu_web_search import ZhipuWebSearchTool
+from xagent.core.tools.core import browser_use as browser_core
 from xagent.core.workspace import TaskWorkspace
 
 # How long a batched I/O seam waits to rendezvous with its sibling before giving
@@ -79,6 +81,70 @@ def auto_register_files(self) -> Iterator[TaskWorkspace]:
                 self.active -= 1
 
 
+class _TrackedBrowserPage:
+    def __init__(
+        self,
+        session_id: str,
+        tracker: ConcurrencyTracker,
+        barrier: asyncio.Barrier | None = None,
+    ) -> None:
+        self._session_id = session_id
+        self._tracker = tracker
+        self._barrier = barrier
+
+    async def evaluate(self, javascript: str) -> str:
+        self._tracker.enter(self._session_id)
+        try:
+            if self._barrier is not None:
+                await asyncio.wait_for(
+                    self._barrier.wait(), timeout=_RENDEZVOUS_TIMEOUT
+                )
+            else:
+                await asyncio.sleep(0.05)
+            return javascript
+        finally:
+            self._tracker.leave(self._session_id)
+
+
+class _TrackedBrowserSession:
+    def __init__(
+        self,
+        session_id: str,
+        tracker: ConcurrencyTracker,
+        barrier: asyncio.Barrier | None = None,
+    ) -> None:
+        self._page = _TrackedBrowserPage(session_id, tracker, barrier)
+        self._operation_lock = asyncio.Lock()
+
+    @asynccontextmanager
+    async def operation_guard(self) -> Iterator[None]:
+        async with self._operation_lock:
+            yield
+
+    async def get_page(self) -> _TrackedBrowserPage:
+        return self._page
+
+
+class _TrackedBrowserManager:
+    def __init__(
+        self,
+        tracker: ConcurrencyTracker,
+        barrier: asyncio.Barrier | None = None,
+    ) -> None:
+        self._tracker = tracker
+        self._barrier = barrier
+        self._sessions: dict[str, _TrackedBrowserSession] = {}
+
+    async def get_or_create(
+        self, session_id: str, headless: bool = False
+    ) -> _TrackedBrowserSession:
+        if session_id not in self._sessions:
+            self._sessions[session_id] = _TrackedBrowserSession(
+                session_id, self._tracker, self._barrier
+            )
+        return self._sessions[session_id]
+
+
 @dataclass
 class _RealToolCase:
     label: str
@@ -232,6 +298,84 @@ def test_real_workspace_write_tool_is_batched_as_concurrent(tmp_path: Any) -> No
     assert segment == batch
 
 
+def test_real_browser_evaluate_tool_is_batched_as_concurrent() -> None:
+    """Browser evaluate opts into the ReAct concurrent segment."""
+    tool = BrowserEvaluateTool()
+    pattern = make_react(parallel=True, max_concurrency=2)
+    batch = [
+        make_tool_call(tool.name, {"session_id": "session-a", "javascript": "'first'"}),
+        make_tool_call(
+            tool.name, {"session_id": "session-b", "javascript": "'second'"}
+        ),
+    ]
+
+    segment, kind = pattern._next_segment(batch, [tool])
+
+    assert kind == "concurrent"
+    assert segment == batch
+
+
+async def test_real_browser_evaluate_tool_serializes_same_session_in_batch(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """Same-session browser calls stay serial inside a ReAct batch."""
+    tracker = ConcurrencyTracker()
+    manager = _TrackedBrowserManager(tracker)
+    monkeypatch.setattr(browser_core, "PLAYWRIGHT_AVAILABLE", True)
+    monkeypatch.setattr(browser_core, "get_browser_manager", lambda: manager)
+
+    tool = BrowserEvaluateTool()
+    pattern = make_react(parallel=True, max_concurrency=2)
+    context = RecordingContext()
+    batch = [
+        make_tool_call(
+            tool.name, {"session_id": "same-session", "javascript": "'first'"}
+        ),
+        make_tool_call(
+            tool.name, {"session_id": "same-session", "javascript": "'second'"}
+        ),
+    ]
+
+    results = await asyncio.wait_for(
+        pattern._run_concurrent_batch(batch, [tool], FakeRuntime(), context),
+        timeout=_RENDEZVOUS_TIMEOUT * 2,
+    )
+
+    assert tracker.peak == 1
+    assert len(results) == 2
+    assert all(pattern._tool_result_success(result) for result in results)
+
+
+async def test_real_browser_evaluate_tool_overlaps_different_sessions_in_batch(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """Different browser sessions can overlap inside a ReAct batch."""
+    tracker = ConcurrencyTracker()
+    barrier = asyncio.Barrier(2)
+    manager = _TrackedBrowserManager(tracker, barrier)
+    monkeypatch.setattr(browser_core, "PLAYWRIGHT_AVAILABLE", True)
+    monkeypatch.setattr(browser_core, "get_browser_manager", lambda: manager)
+
+    tool = BrowserEvaluateTool()
+    pattern = make_react(parallel=True, max_concurrency=2)
+    context = RecordingContext()
+    batch = [
+        make_tool_call(tool.name, {"session_id": "session-a", "javascript": "'first'"}),
+        make_tool_call(
+            tool.name, {"session_id": "session-b", "javascript": "'second'"}
+        ),
+    ]
+
+    results = await asyncio.wait_for(
+        pattern._run_concurrent_batch(batch, [tool], FakeRuntime(), context),
+        timeout=_RENDEZVOUS_TIMEOUT * 2,
+    )
+
+    assert tracker.peak == 2
+    assert len(results) == 2
+    assert all(pattern._tool_result_success(result) for result in results)
+
+
 async def test_real_workspace_write_tool_serializes_same_path_in_batch(
     tmp_path: Any,
 ) -> None:
diff --git a/tests/core/tools/test_browser_use.py b/tests/core/tools/test_browser_use.py
index cc3779851..d59b5d12d 100644
--- a/tests/core/tools/test_browser_use.py
+++ b/tests/core/tools/test_browser_use.py
@@ -5,6 +5,8 @@
 Run with: pytest tests/core/tools/test_browser_use.py
 """
 
+import asyncio
+from contextlib import asynccontextmanager
 from unittest.mock import patch
 
 import pytest
@@ -12,6 +14,22 @@
 from xagent.core.tools.core import browser_use
 
 
+_SESSION_GUARDED_OPERATIONS = [
+    (
+        browser_use.browser_navigate,
+        {"url": "https://example.com", "wait_until": "domcontentloaded"},
+    ),
+    (browser_use.browser_click, {"selector": "button.submit"}),
+    (browser_use.browser_fill, {"selector": "input[name='email']", "value": "x"}),
+    (browser_use.browser_screenshot, {}),
+    (browser_use.browser_extract_text, {}),
+    (browser_use.browser_evaluate, {"javascript": "'ok'"}),
+    (browser_use.browser_select_option, {"selector": "select.country", "value": "US"}),
+    (browser_use.browser_wait_for_selector, {"selector": ".ready"}),
+    (browser_use.browser_pdf, {}),
+]
+
+
 @pytest.fixture
 def mock_playwright():
     """Mock playwright modules."""
@@ -35,6 +53,30 @@ def reset_manager():
     browser_use._manager = None
 
 
+@pytest.mark.parametrize(
+    ("operation", "extra_args"),
+    _SESSION_GUARDED_OPERATIONS,
+    ids=[operation.__name__ for operation, _extra_args in _SESSION_GUARDED_OPERATIONS],
+)
+async def test_browser_operation_serializes_calls_for_same_session(
+    monkeypatch, operation, extra_args
+):
+    """Every browser operation is guarded by the target session."""
+    tracker = _AsyncConcurrencyTracker()
+    manager = _FakeBrowserManager(tracker)
+
+    monkeypatch.setattr(browser_use, "PLAYWRIGHT_AVAILABLE", True)
+    monkeypatch.setattr(browser_use, "get_browser_manager", lambda: manager)
+
+    results = await asyncio.gather(
+        operation(session_id="shared-session", **extra_args),
+        operation(session_id="shared-session", **extra_args),
+    )
+
+    assert [result["success"] for result in results] == [True, True]
+    assert tracker.peak == 1
+
+
 class TestBrowserNavigate:
     """Tests for browser_navigate function."""
 
@@ -179,6 +221,44 @@ def test_evaluate_without_playwright(self):
             assert result["success"] is False
             assert "not installed" in result["error"]
 
+    async def test_evaluate_serializes_operations_for_same_session(self, monkeypatch):
+        """Concurrent calls sharing a browser session run one at a time."""
+        tracker = _AsyncConcurrencyTracker()
+        manager = _FakeBrowserManager(tracker)
+
+        monkeypatch.setattr(browser_use, "PLAYWRIGHT_AVAILABLE", True)
+        monkeypatch.setattr(browser_use, "get_browser_manager", lambda: manager)
+
+        results = await asyncio.gather(
+            browser_use.browser_evaluate(
+                session_id="shared-session", javascript="'first'"
+            ),
+            browser_use.browser_evaluate(
+                session_id="shared-session", javascript="'second'"
+            ),
+        )
+
+        assert [result["success"] for result in results] == [True, True]
+        assert tracker.peak == 1
+
+    async def test_evaluate_allows_operations_for_different_sessions_to_overlap(
+        self, monkeypatch
+    ):
+        """Concurrent calls with different sessions can overlap."""
+        tracker = _AsyncConcurrencyTracker()
+        manager = _FakeBrowserManager(tracker)
+
+        monkeypatch.setattr(browser_use, "PLAYWRIGHT_AVAILABLE", True)
+        monkeypatch.setattr(browser_use, "get_browser_manager", lambda: manager)
+
+        results = await asyncio.gather(
+            browser_use.browser_evaluate(session_id="session-a", javascript="'a'"),
+            browser_use.browser_evaluate(session_id="session-b", javascript="'b'"),
+        )
+
+        assert [result["success"] for result in results] == [True, True]
+        assert tracker.peak == 2
+
 
 class TestBrowserSelectOption:
     """Tests for browser_select_option function."""
@@ -284,3 +364,95 @@ def test_session_initialization(self):
             assert session.session_id == "test-session"
             assert session.headless is True
             assert session._initialized is False
+
+
+class _AsyncConcurrencyTracker:
+    def __init__(self):
+        self.active = 0
+        self.peak = 0
+
+    async def run(self, result):
+        self.active += 1
+        self.peak = max(self.peak, self.active)
+        try:
+            await asyncio.sleep(0.01)
+            return result
+        finally:
+            self.active -= 1
+
+
+class _FakeBrowserPage:
+    def __init__(self, tracker):
+        self._tracker = tracker
+        self.viewport_size = {"width": 1920, "height": 1080}
+
+    async def goto(self, url, **kwargs):
+        return await self._tracker.run(url)
+
+    async def title(self):
+        return "Fake title"
+
+    async def click(self, selector, **kwargs):
+        return await self._tracker.run(selector)
+
+    async def fill(self, selector, value, **kwargs):
+        return await self._tracker.run(value)
+
+    async def screenshot(self, **kwargs):
+        return await self._tracker.run(b"fake-png")
+
+    def locator(self, selector):
+        return _FakeBrowserLocator(selector, self._tracker)
+
+    async def evaluate(self, javascript):
+        return await self._tracker.run(javascript)
+
+    async def select_option(self, selector, **kwargs):
+        return await self._tracker.run(kwargs)
+
+    async def wait_for_selector(self, selector, **kwargs):
+        return await self._tracker.run(selector)
+
+    async def pdf(self, **kwargs):
+        return await self._tracker.run(b"fake-pdf")
+
+    async def set_viewport_size(self, viewport_size):
+        self.viewport_size = dict(viewport_size)
+
+
+class _FakeBrowserLocator:
+    def __init__(self, selector, tracker):
+        self._selector = selector
+        self._tracker = tracker
+
+    async def wait_for(self, **kwargs):
+        return None
+
+    async def inner_text(self, **kwargs):
+        return await self._tracker.run(f"text for {self._selector}")
+
+
+class _FakeBrowserSession:
+    def __init__(self, session_id, tracker):
+        self.session_id = session_id
+        self._page = _FakeBrowserPage(tracker)
+        self._operation_lock = asyncio.Lock()
+
+    @asynccontextmanager
+    async def operation_guard(self):
+        async with self._operation_lock:
+            yield
+
+    async def get_page(self):
+        return self._page
+
+
+class _FakeBrowserManager:
+    def __init__(self, tracker):
+        self._tracker = tracker
+        self._sessions = {}
+
+    async def get_or_create(self, session_id, headless=False):
+        if session_id not in self._sessions:
+            self._sessions[session_id] = _FakeBrowserSession(session_id, self._tracker)
+        return self._sessions[session_id]

From 060c211c07eec51f4a5828865106b2f8bd895995 Mon Sep 17 00:00:00 2001
From: leyoonafr <codeacme17@gmail.com>
Date: Sat, 27 Jun 2026 17:45:32 +0800
Subject: [PATCH 4/7] feat: Enhance PythonExecutorTool for concurrency safety
 and isolation in execution

---
 .../tools/adapters/vibe/python_executor.py    |  75 +++-
 src/xagent/core/tools/core/python_executor.py | 322 ++++++++++++------
 .../agent/test_react_real_tool_concurrency.py |  70 +++-
 .../vibe/test_tool_metadata_concurrency.py    |   8 +-
 tests/core/tools/test_python_executor.py      |  60 ++++
 5 files changed, 422 insertions(+), 113 deletions(-)

diff --git a/src/xagent/core/tools/adapters/vibe/python_executor.py b/src/xagent/core/tools/adapters/vibe/python_executor.py
index 064f5a20f..da82d3800 100644
--- a/src/xagent/core/tools/adapters/vibe/python_executor.py
+++ b/src/xagent/core/tools/adapters/vibe/python_executor.py
@@ -5,6 +5,7 @@
 
 import asyncio
 import logging
+from pathlib import Path
 from typing import Any, Dict, Mapping, Optional, Type
 
 from pydantic import BaseModel, Field
@@ -12,10 +13,8 @@
 from ....workspace import TaskWorkspace
 from ...artifacts import (
     build_generated_file_metadata,
-    changed_generated_artifact_files,
-    snapshot_generated_artifact_files,
 )
-from ...core.python_executor import PythonExecutorCore
+from ...core.python_executor import _INTERNAL_WRITTEN_FILES_KEY, PythonExecutorCore
 from .base import AbstractBaseTool, ToolCategory, ToolVisibility
 from .function import FunctionTool
 from .sandboxed_tool.sandbox_config import sandbox_config
@@ -48,6 +47,8 @@ class PythonExecutorResult(BaseModel):
 class PythonExecutorTool(AbstractBaseTool):
     """Framework wrapper for the pure Python executor tool"""
 
+    concurrency_safe = True
+
     def __init__(self, workspace: Optional[TaskWorkspace] = None) -> None:
         self._visibility = ToolVisibility.PUBLIC
         self._workspace = workspace
@@ -82,7 +83,10 @@ def run_json_sync(self, args: Mapping[str, Any]) -> Any:
         workspace_env = self._get_workspace_env()
 
         # Create core executor instance
-        executor = PythonExecutorCore(working_directory=working_directory)
+        executor = PythonExecutorCore(
+            working_directory=working_directory,
+            environment=workspace_env,
+        )
 
         # Add workspace variables to the executor's globals if available
         if workspace_env:
@@ -92,23 +96,23 @@ def run_json_sync(self, args: Mapping[str, Any]) -> Any:
         else:
             full_code = exec_args.code
 
-        # Execute code within auto_register context
+        # Execute code in an isolated subprocess. The child reports only files
+        # written by this execution, avoiding concurrent snapshot bleed-through.
         if self._workspace and working_directory:
-            files_before = snapshot_generated_artifact_files(working_directory)
-            with self._workspace.auto_register_files():
-                result = executor.execute_code(full_code, exec_args.capture_output)
+            result = executor.execute_code(full_code, exec_args.capture_output)
+            written_files = result.pop(_INTERNAL_WRITTEN_FILES_KEY, [])
             if result.get("success"):
-                files_after = snapshot_generated_artifact_files(working_directory)
+                workspace_files = self._existing_workspace_files(written_files)
+                self._register_workspace_files(workspace_files)
                 result.update(
                     build_generated_file_metadata(
                         workspace=self._workspace,
-                        file_paths=changed_generated_artifact_files(
-                            files_before, files_after
-                        ),
+                        file_paths=workspace_files,
                     )
                 )
         else:
             result = executor.execute_code(full_code, exec_args.capture_output)
+            result.pop(_INTERNAL_WRITTEN_FILES_KEY, None)
 
         return PythonExecutorResult(**result).model_dump()
 
@@ -134,6 +138,49 @@ def _get_workspace_env(self) -> Optional[Dict[str, str]]:
             "WORKSPACE_DIR": str(self._workspace.workspace_dir.resolve()),
         }
 
+    def _existing_workspace_files(self, file_paths: Any) -> list[Path]:
+        """Return existing files from this workspace for this execution only."""
+        if not self._workspace or not isinstance(file_paths, list):
+            return []
+
+        workspace_root = self._workspace.workspace_dir.resolve()
+        result: list[Path] = []
+        seen: set[str] = set()
+
+        for file_path in file_paths:
+            try:
+                resolved = Path(str(file_path)).resolve()
+                resolved.relative_to(workspace_root)
+            except (OSError, ValueError):
+                continue
+            if not resolved.exists() or not resolved.is_file():
+                continue
+            if any(
+                part.startswith(".") or part == "__pycache__"
+                for part in resolved.relative_to(workspace_root).parts
+            ):
+                continue
+            key = str(resolved)
+            if key in seen:
+                continue
+            seen.add(key)
+            result.append(resolved)
+
+        return result
+
+    def _register_workspace_files(self, file_paths: list[Path]) -> None:
+        if not self._workspace:
+            return
+        for file_path in file_paths:
+            try:
+                self._workspace.register_file(str(file_path))
+            except Exception as exc:  # noqa: BLE001
+                logger.warning(
+                    "Failed to register Python executor generated file %s: %s",
+                    file_path,
+                    exc,
+                )
+
 
 @sandbox_config(
     packages=[
@@ -181,7 +228,9 @@ def execute_python_code(code: str, capture_output: bool = True) -> Dict[str, Any
         return result
 
     return PythonExecutorFunctionTool(
-        execute_python_code, description=executor.description
+        execute_python_code,
+        description=executor.description,
+        concurrency_safe=True,
     )
 
 
diff --git a/src/xagent/core/tools/core/python_executor.py b/src/xagent/core/tools/core/python_executor.py
index a27ad7155..27aad3025 100644
--- a/src/xagent/core/tools/core/python_executor.py
+++ b/src/xagent/core/tools/core/python_executor.py
@@ -4,38 +4,65 @@
 """
 
 import ast
+import contextlib
+import importlib
 import io
+import json
 import logging
 import os
+import subprocess
 import sys
+import tempfile
 import traceback
+from pathlib import Path
 from typing import Any, Dict, Optional
 
 logger = logging.getLogger(__name__)
 
-# Optional imports
-plt: Any = None
-matplotlib_module: Any = None
+_INTERNAL_WRITTEN_FILES_KEY = "_xagent_written_files"
 
-try:
-    import matplotlib as matplotlib_module
-    import matplotlib.pyplot as plt
-    import pandas as pd
-except ImportError:
-    pass
+
+class _LazyModule:
+    """Import an optional module only when executed code first touches it."""
+
+    def __init__(self, module_name: str, *, matplotlib_agg: bool = False) -> None:
+        self._module_name = module_name
+        self._matplotlib_agg = matplotlib_agg
+        self._module: Any = None
+
+    def _load(self) -> Any:
+        if self._module is None:
+            if self._matplotlib_agg:
+                import matplotlib
+
+                matplotlib.use("Agg")
+            self._module = importlib.import_module(self._module_name)
+        return self._module
+
+    def __getattr__(self, name: str) -> Any:
+        return getattr(self._load(), name)
+
+    def __repr__(self) -> str:
+        return f"<lazy module {self._module_name}>"
 
 
 class PythonExecutorCore:
     """Pure Python executor without framework dependencies"""
 
-    def __init__(self, working_directory: Optional[str] = None):
+    def __init__(
+        self,
+        working_directory: Optional[str] = None,
+        environment: Optional[dict[str, str]] = None,
+    ):
         """
         Initialize the Python executor.
 
         Args:
             working_directory: Directory to use as working directory during execution
+            environment: Environment variables to expose to executed code
         """
         self.working_directory = working_directory
+        self.environment = environment or {}
 
     def execute_code(self, code: str, capture_output: bool = True) -> Dict[str, Any]:
         """
@@ -52,75 +79,7 @@ def execute_code(self, code: str, capture_output: bool = True) -> Dict[str, Any]
             # Validate syntax first
             ast.parse(code)
 
-            # Prepare execution environment
-            output_buffer = io.StringIO()
-            error_buffer = io.StringIO()
-
-            # Use one namespace for globals and locals so imports are visible from
-            # nested scopes such as comprehensions and lambdas.
-            exec_namespace = self._create_safe_globals()
-            initial_names = set(exec_namespace)
-
-            old_cwd = None
-            if self.working_directory:
-                old_cwd = os.getcwd()
-                logger.info(
-                    f"PythonExecutor: Changing working directory from {old_cwd} to {self.working_directory}"
-                )
-                os.chdir(self.working_directory)
-
-            if capture_output:
-                old_stdout = sys.stdout
-                old_stderr = sys.stderr
-                sys.stdout = output_buffer
-                sys.stderr = error_buffer
-
-            try:
-                # Execute the code
-                exec(code, exec_namespace, exec_namespace)
-
-                output = ""
-                if capture_output:
-                    output = output_buffer.getvalue()
-
-                # If no output but there are variables, show them
-                if not output and exec_namespace:
-                    visible_vars = {
-                        k: v
-                        for k, v in exec_namespace.items()
-                        if k not in initial_names and not k.startswith("_")
-                    }
-                    if visible_vars:
-                        output = "Variables created:\n"
-                        for name, value in visible_vars.items():
-                            output += f"{name} = {repr(value)}\n"
-
-                return {
-                    "success": True,
-                    "output": output or "Code executed successfully (no output)",
-                    "error": "",
-                }
-
-            except Exception:
-                error_msg = traceback.format_exc()
-                stderr_content = error_buffer.getvalue() if capture_output else ""
-
-                return {
-                    "success": False,
-                    "output": output_buffer.getvalue() if capture_output else "",
-                    "error": f"{error_msg}\n{stderr_content}".strip(),
-                }
-
-            finally:
-                if old_cwd is not None:
-                    logger.info(
-                        f"PythonExecutor: Restoring working directory to {old_cwd}"
-                    )
-                    os.chdir(old_cwd)
-
-                if capture_output:
-                    sys.stdout = old_stdout
-                    sys.stderr = old_stderr
+            return self._execute_code_in_subprocess(code, capture_output)
 
         except SyntaxError as e:
             return {"success": False, "output": "", "error": f"Syntax Error: {str(e)}"}
@@ -177,27 +136,189 @@ def _create_safe_globals(self) -> Dict[str, Any]:
         except ImportError:
             pass
 
-        # Add numpy if available
-        try:
-            import numpy as np
+        safe_globals["np"] = _LazyModule("numpy")
+        safe_globals["numpy"] = safe_globals["np"]
+        safe_globals["pd"] = _LazyModule("pandas")
+        safe_globals["pandas"] = safe_globals["pd"]
+        safe_globals["matplotlib"] = _LazyModule("matplotlib", matplotlib_agg=True)
+        safe_globals["plt"] = _LazyModule("matplotlib.pyplot", matplotlib_agg=True)
 
-            safe_globals["np"] = np
-            safe_globals["numpy"] = np
-        except ImportError:
-            pass
+        return safe_globals
 
-        # Add pandas if available
-        if pd is not None:
-            safe_globals["pd"] = pd
-            safe_globals["pandas"] = pd
+    def _execute_code_in_subprocess(
+        self, code: str, capture_output: bool
+    ) -> Dict[str, Any]:
+        """Execute code in a child process to isolate process-global state."""
+        env = os.environ.copy()
+        env.update({str(key): str(value) for key, value in self.environment.items()})
+
+        with tempfile.TemporaryDirectory() as temp_dir:
+            result_path = Path(temp_dir) / "result.json"
+            payload = {
+                "code": code,
+                "capture_output": capture_output,
+                "result_path": str(result_path),
+            }
 
-        # Add matplotlib with safe configuration
-        if matplotlib_module is not None and plt is not None:
-            matplotlib_module.use("Agg")
-            safe_globals["matplotlib"] = matplotlib_module
-            safe_globals["plt"] = plt
+            try:
+                process = subprocess.run(
+                    [
+                        sys.executable,
+                        str(Path(__file__).resolve()),
+                        "--xagent-python-executor-child",
+                    ],
+                    input=json.dumps(payload),
+                    text=True,
+                    capture_output=True,
+                    cwd=self.working_directory,
+                    env=env,
+                )
+            except Exception as exc:
+                return {
+                    "success": False,
+                    "output": "",
+                    "error": f"Error: {str(exc)}",
+                    _INTERNAL_WRITTEN_FILES_KEY: [],
+                }
 
-        return safe_globals
+            if result_path.exists():
+                try:
+                    result = json.loads(result_path.read_text(encoding="utf-8"))
+                    if isinstance(result, dict):
+                        return result
+                except Exception as exc:
+                    logger.warning(
+                        "Failed to read Python executor child result: %s", exc
+                    )
+
+            error = process.stderr.strip() or process.stdout.strip()
+            if not error:
+                error = f"Python executor child failed with code {process.returncode}"
+            return {
+                "success": False,
+                "output": process.stdout if capture_output else "",
+                "error": error,
+                _INTERNAL_WRITTEN_FILES_KEY: [],
+            }
+
+
+def _recorded_path(path: Any) -> str | None:
+    if isinstance(path, int):
+        return None
+    try:
+        raw_path = Path(os.fsdecode(path)).expanduser()
+    except (TypeError, ValueError):
+        return None
+    if not raw_path.is_absolute():
+        raw_path = Path.cwd() / raw_path
+    return str(raw_path.resolve(strict=False))
+
+
+def _open_event_is_write(mode: Any, flags: Any) -> bool:
+    if isinstance(mode, str):
+        return any(marker in mode for marker in ("w", "a", "x", "+"))
+    if isinstance(flags, int):
+        return bool(
+            flags & (os.O_WRONLY | os.O_RDWR | os.O_APPEND | os.O_CREAT | os.O_TRUNC)
+        )
+    return False
+
+
+def _execute_child_payload(payload: dict[str, Any]) -> dict[str, Any]:
+    code = str(payload.get("code") or "")
+    capture_output = bool(payload.get("capture_output", True))
+
+    output_buffer = io.StringIO()
+    error_buffer = io.StringIO()
+    written_files: set[str] = set()
+
+    def audit_hook(event: str, args: tuple[Any, ...]) -> None:
+        try:
+            if event == "open" and len(args) >= 3:
+                path, mode, flags = args[:3]
+                if _open_event_is_write(mode, flags):
+                    recorded = _recorded_path(path)
+                    if recorded:
+                        written_files.add(recorded)
+            elif event == "os.rename" and len(args) >= 2:
+                recorded = _recorded_path(args[1])
+                if recorded:
+                    written_files.add(recorded)
+        except Exception:
+            return
+
+    sys.addaudithook(audit_hook)
+
+    exec_namespace = PythonExecutorCore()._create_safe_globals()
+    initial_names = set(exec_namespace)
+
+    try:
+        if capture_output:
+            with (
+                contextlib.redirect_stdout(output_buffer),
+                contextlib.redirect_stderr(error_buffer),
+            ):
+                exec(code, exec_namespace, exec_namespace)
+        else:
+            exec(code, exec_namespace, exec_namespace)
+
+        output = output_buffer.getvalue() if capture_output else ""
+
+        if not output and exec_namespace:
+            visible_vars = {
+                k: v
+                for k, v in exec_namespace.items()
+                if k not in initial_names and not k.startswith("_")
+            }
+            if visible_vars:
+                output = "Variables created:\n"
+                for name, value in visible_vars.items():
+                    output += f"{name} = {repr(value)}\n"
+
+        return {
+            "success": True,
+            "output": output or "Code executed successfully (no output)",
+            "error": "",
+            _INTERNAL_WRITTEN_FILES_KEY: sorted(written_files),
+        }
+
+    except Exception:
+        error_msg = traceback.format_exc()
+        stderr_content = error_buffer.getvalue() if capture_output else ""
+        return {
+            "success": False,
+            "output": output_buffer.getvalue() if capture_output else "",
+            "error": f"{error_msg}\n{stderr_content}".strip(),
+            _INTERNAL_WRITTEN_FILES_KEY: sorted(written_files),
+        }
+
+
+def _child_main() -> int:
+    try:
+        payload = json.loads(sys.stdin.read())
+        if not isinstance(payload, dict):
+            raise ValueError("Child payload must be a JSON object")
+        result_path = Path(str(payload["result_path"]))
+        result = _execute_child_payload(payload)
+        result_path.write_text(json.dumps(result), encoding="utf-8")
+        return 0
+    except Exception as exc:
+        fallback = {
+            "success": False,
+            "output": "",
+            "error": f"Python executor child error: {exc}",
+            _INTERNAL_WRITTEN_FILES_KEY: [],
+        }
+        result_path_value = locals().get("payload", {}).get("result_path")
+        if result_path_value:
+            try:
+                Path(str(result_path_value)).write_text(
+                    json.dumps(fallback), encoding="utf-8"
+                )
+            except Exception:
+                pass
+        print(fallback["error"], file=sys.stderr)
+        return 1
 
 
 # Convenience function for direct usage
@@ -216,4 +337,11 @@ def execute_python_code(
         Dictionary with execution results
     """
     executor = PythonExecutorCore(working_directory)
-    return executor.execute_code(code, capture_output)
+    result = executor.execute_code(code, capture_output)
+    result.pop(_INTERNAL_WRITTEN_FILES_KEY, None)
+    return result
+
+
+if __name__ == "__main__":
+    if len(sys.argv) > 1 and sys.argv[1] == "--xagent-python-executor-child":
+        raise SystemExit(_child_main())
diff --git a/tests/core/agent/test_react_real_tool_concurrency.py b/tests/core/agent/test_react_real_tool_concurrency.py
index 326aab155..f011664e1 100644
--- a/tests/core/agent/test_react_real_tool_concurrency.py
+++ b/tests/core/agent/test_react_real_tool_concurrency.py
@@ -40,8 +40,9 @@
     make_tool_call,
 )
 from xagent.core.tools.adapters.vibe.exa_web_search import ExaWebSearchTool
-from xagent.core.tools.adapters.vibe.fetch_web_content import FetchWebContentTool
 from xagent.core.tools.adapters.vibe.browser_use import BrowserEvaluateTool
+from xagent.core.tools.adapters.vibe.fetch_web_content import FetchWebContentTool
+from xagent.core.tools.adapters.vibe.python_executor import PythonExecutorTool
 from xagent.core.tools.adapters.vibe.tavily_web_search import TavilyWebSearchTool
 from xagent.core.tools.adapters.vibe.web_search import WebSearchTool
 from xagent.core.tools.adapters.vibe.workspace_file_tool import WorkspaceFileTools
@@ -315,6 +316,22 @@ def test_real_browser_evaluate_tool_is_batched_as_concurrent() -> None:
     assert segment == batch
 
 
+def test_real_python_executor_tool_is_batched_as_concurrent(tmp_path: Any) -> None:
+    """Python executor opts into the ReAct concurrent segment after isolation."""
+    workspace = TaskWorkspace("react_python_segment", str(tmp_path))
+    tool = PythonExecutorTool(workspace)
+    pattern = make_react(parallel=True, max_concurrency=2)
+    batch = [
+        make_tool_call(tool.name, {"code": "print('first')"}),
+        make_tool_call(tool.name, {"code": "print('second')"}),
+    ]
+
+    segment, kind = pattern._next_segment(batch, [tool])
+
+    assert kind == "concurrent"
+    assert segment == batch
+
+
 async def test_real_browser_evaluate_tool_serializes_same_session_in_batch(
     monkeypatch: pytest.MonkeyPatch,
 ) -> None:
@@ -427,3 +444,54 @@ async def test_real_workspace_write_tool_overlaps_different_paths_in_batch(
     assert all(pattern._tool_result_success(result) for result in results)
     assert (workspace.output_dir / "one.txt").read_text(encoding="utf-8") == "one"
     assert (workspace.output_dir / "two.txt").read_text(encoding="utf-8") == "two"
+
+
+async def test_real_python_executor_tool_overlaps_and_isolates_batch_results(
+    tmp_path: Any,
+) -> None:
+    """ReAct batch execution overlaps Python calls without output/artifact bleed."""
+    workspace = TaskWorkspace("react_python_overlap", str(tmp_path))
+    tool = PythonExecutorTool(workspace)
+    pattern = make_react(parallel=True, max_concurrency=2)
+    context = RecordingContext()
+
+    def code_for(label: str) -> str:
+        dirname = f"{label}_dir"
+        artifact = f"{label}.docx"
+        return f"""
+import os
+import time
+from pathlib import Path
+
+Path({dirname!r}).mkdir(exist_ok=True)
+os.chdir({dirname!r})
+print({f"{label}:entered:"!r} + Path.cwd().name)
+time.sleep(1.0)
+print({f"{label}:final:"!r} + Path.cwd().name)
+Path("..", {artifact!r}).write_bytes({label.encode()!r})
+"""
+
+    batch = [
+        make_tool_call(tool.name, {"code": code_for("alpha")}),
+        make_tool_call(tool.name, {"code": code_for("beta")}),
+    ]
+
+    started = time.perf_counter()
+    results = await asyncio.wait_for(
+        pattern._run_concurrent_batch(batch, [tool], FakeRuntime(), context),
+        timeout=_RENDEZVOUS_TIMEOUT * 2,
+    )
+    elapsed = time.perf_counter() - started
+
+    assert elapsed < 2.0
+    assert len(results) == 2
+    assert all(pattern._tool_result_success(result) for result in results)
+    assert "alpha:final:alpha_dir" in results[0]["output"]
+    assert "beta:" not in results[0]["output"]
+    assert "beta:final:beta_dir" in results[1]["output"]
+    assert "alpha:" not in results[1]["output"]
+    assert results[0]["generated_files"] == ["alpha.docx"]
+    assert results[1]["generated_files"] == ["beta.docx"]
+    assert [r["tool_call_id"] for r in context.tool_results] == [
+        tc["id"] for tc in batch
+    ]
diff --git a/tests/core/tools/adapters/vibe/test_tool_metadata_concurrency.py b/tests/core/tools/adapters/vibe/test_tool_metadata_concurrency.py
index 1195545aa..4fef4b926 100644
--- a/tests/core/tools/adapters/vibe/test_tool_metadata_concurrency.py
+++ b/tests/core/tools/adapters/vibe/test_tool_metadata_concurrency.py
@@ -40,6 +40,8 @@
     OutputFilteredToolWrapper,
 )
 from xagent.core.tools.adapters.vibe.python_executor import PythonExecutorTool
+from xagent.core.tools.adapters.vibe.python_executor import PythonExecutorToolForBasic
+from xagent.core.tools.adapters.vibe.python_executor import get_python_executor_tool
 from xagent.core.tools.adapters.vibe.sandboxed_tool.sandbox_config import (
     sandbox_config,
 )
@@ -146,8 +148,10 @@ def test_basic_file_mutation_tools_match_guarded_concurrency() -> None:
         assert tool.metadata.concurrency_safe is True
 
 
-def test_stateful_tools_without_call_isolation_are_not_concurrency_safe() -> None:
-    assert PythonExecutorTool().metadata.concurrency_safe is False
+def test_python_executor_tools_are_concurrency_safe_after_call_isolation() -> None:
+    assert PythonExecutorTool().metadata.concurrency_safe is True
+    assert PythonExecutorToolForBasic().metadata.concurrency_safe is True
+    assert get_python_executor_tool().metadata.concurrency_safe is True
 
 
 def test_workspace_file_tool_metadata_matches_guarded_concurrency(tmp_path) -> None:
diff --git a/tests/core/tools/test_python_executor.py b/tests/core/tools/test_python_executor.py
index f91121ed8..cda04176c 100644
--- a/tests/core/tools/test_python_executor.py
+++ b/tests/core/tools/test_python_executor.py
@@ -2,7 +2,11 @@
 Tests for PythonExecutor tool
 """
 
+import asyncio
+import os
 import sys
+import time
+from pathlib import Path
 
 import pytest
 
@@ -170,6 +174,62 @@ def test_overwritten_generated_file_returns_inline_artifact(self, tmp_path):
             }
         ]
 
+    @pytest.mark.asyncio
+    async def test_concurrent_workspace_calls_isolate_process_state_and_metadata(
+        self, tmp_path
+    ):
+        """Concurrent executions keep cwd, output, env, and artifacts separate."""
+        workspace = TaskWorkspace("test_python_concurrent_isolation", str(tmp_path))
+        executor = PythonExecutorTool(workspace=workspace)
+        original_cwd = Path.cwd()
+
+        def code_for(label: str) -> str:
+            dirname = f"{label}_dir"
+            artifact = f"{label}.docx"
+            return f"""
+import os
+import time
+from pathlib import Path
+
+Path({dirname!r}).mkdir(exist_ok=True)
+os.chdir({dirname!r})
+print({f"{label}:entered:"!r} + Path.cwd().name)
+time.sleep(1.0)
+print({f"{label}:final:"!r} + Path.cwd().name)
+print({f"{label}:env:"!r} + str(os.environ.get("WORKSPACE_DIR") == WORKSPACE_DIR))
+Path("..", {artifact!r}).write_bytes({label.encode()!r})
+"""
+
+        started = time.perf_counter()
+        try:
+            left_result, right_result = await asyncio.gather(
+                executor.run_json_async(
+                    {"code": code_for("left"), "capture_output": True}
+                ),
+                executor.run_json_async(
+                    {"code": code_for("right"), "capture_output": True}
+                ),
+            )
+        finally:
+            os.chdir(original_cwd)
+        elapsed = time.perf_counter() - started
+
+        assert elapsed < 1.8
+        assert left_result["success"] is True
+        assert right_result["success"] is True
+        assert "left:entered:left_dir" in left_result["output"]
+        assert "left:final:left_dir" in left_result["output"]
+        assert "left:env:True" in left_result["output"]
+        assert "right:" not in left_result["output"]
+        assert "right:entered:right_dir" in right_result["output"]
+        assert "right:final:right_dir" in right_result["output"]
+        assert "right:env:True" in right_result["output"]
+        assert "left:" not in right_result["output"]
+        assert left_result["generated_files"] == ["left.docx"]
+        assert right_result["generated_files"] == ["right.docx"]
+        assert left_result["file_refs"][0]["file_id"]
+        assert right_result["file_refs"][0]["file_id"]
+
     def test_syntax_error(self, python_executor):
         """Test handling of syntax errors"""
         result = python_executor.run_json_sync(

From 5f763b2f91f13088d898eb184f86bbb58801c447 Mon Sep 17 00:00:00 2001
From: leyoonafr <codeacme17@gmail.com>
Date: Sun, 28 Jun 2026 01:34:13 +0800
Subject: [PATCH 5/7] feat: Enhance concurrency safety for document and image
 tools, add path guards for file mutations

---
 .../tools/adapters/vibe/document_parser.py    |   4 +
 .../core/tools/adapters/vibe/image_tool.py    |   3 +
 src/xagent/core/tools/core/document_parser.py |  18 ++--
 src/xagent/core/tools/core/excel.py           |  45 ++++++--
 src/xagent/core/tools/core/image_tool.py      | 101 +++++++++++-------
 .../agent/test_react_real_tool_concurrency.py |  90 ++++++++++++++++
 .../vibe/test_tool_metadata_concurrency.py    |  34 ++++++
 tests/core/tools/core/test_excel.py           |  32 ++++++
 tests/core/tools/core/test_image_tool_core.py |  23 ++++
 tests/core/tools/test_document_tool.py        |  55 ++++++++++
 10 files changed, 351 insertions(+), 54 deletions(-)

diff --git a/src/xagent/core/tools/adapters/vibe/document_parser.py b/src/xagent/core/tools/adapters/vibe/document_parser.py
index 9bc0f896a..804606697 100644
--- a/src/xagent/core/tools/adapters/vibe/document_parser.py
+++ b/src/xagent/core/tools/adapters/vibe/document_parser.py
@@ -19,6 +19,8 @@
 
 
 class DocumentParseTool(AbstractBaseTool):
+    read_only = True
+
     def __init__(self, workspace: TaskWorkspace | None = None) -> None:
         self._visibility = ToolVisibility.PUBLIC
         self.workspace = workspace
@@ -60,6 +62,8 @@ async def run_json_async(self, args: Mapping[str, Any]) -> Any:
 
 # This alternative tool will write the result to a file
 class DocumentParseWithOutputTool(AbstractBaseTool):
+    concurrency_safe = True
+
     def __init__(self, workspace: TaskWorkspace | None = None) -> None:
         self._visibility = ToolVisibility.PUBLIC
         self.workspace = workspace
diff --git a/src/xagent/core/tools/adapters/vibe/image_tool.py b/src/xagent/core/tools/adapters/vibe/image_tool.py
index 8dd1235e1..6ba97edd4 100644
--- a/src/xagent/core/tools/adapters/vibe/image_tool.py
+++ b/src/xagent/core/tools/adapters/vibe/image_tool.py
@@ -80,16 +80,19 @@ def get_tools(self) -> list:
                 self.generate_image,
                 name="generate_image",
                 description=generate_description,
+                concurrency_safe=True,
             ),
             ImageGenerationFunctionTool(
                 self.edit_image,
                 name="edit_image",
                 description=edit_description,
+                concurrency_safe=True,
             ),
             ImageGenerationFunctionTool(
                 self.list_available_models,
                 name="list_image_models",
                 description="List all available image generation models, including model ID, availability status, and detailed description information (Note: model information is already provided in the generate_image tool description)",
+                read_only=True,
             ),
         ]
 
diff --git a/src/xagent/core/tools/core/document_parser.py b/src/xagent/core/tools/core/document_parser.py
index a5a12f6d1..fac67452a 100644
--- a/src/xagent/core/tools/core/document_parser.py
+++ b/src/xagent/core/tools/core/document_parser.py
@@ -4,6 +4,7 @@
 
 from pydantic import BaseModel, Field
 
+from ....core.path_locks import GLOBAL_PATH_MUTATION_LOCKS
 from ....providers.pdf_parser import (
     DeepDocParser,
     DocumentParser,
@@ -230,13 +231,16 @@ async def parse_document_with_output(
     resolved_output_path = tool_args.output_path
 
     try:
-        output_ext = Path(resolved_output_path).suffix.lower()
-        if tool_args.output_format == "json" or output_ext == ".json":
-            write_json_output(result, resolved_output_path)
-        elif tool_args.output_format == "txt" or output_ext == ".txt":
-            write_text_output(result, resolved_output_path)
-        else:  # Default to markdown
-            write_markdown_output(result, resolved_output_path)
+        with GLOBAL_PATH_MUTATION_LOCKS.guard_path(
+            resolved_output_path
+        ) as guarded_output_path:
+            output_ext = guarded_output_path.suffix.lower()
+            if tool_args.output_format == "json" or output_ext == ".json":
+                write_json_output(result, str(guarded_output_path))
+            elif tool_args.output_format == "txt" or output_ext == ".txt":
+                write_text_output(result, str(guarded_output_path))
+            else:  # Default to markdown
+                write_markdown_output(result, str(guarded_output_path))
 
         logger.info(f"Successfully wrote parsing result to: {resolved_output_path}")
         return DocumentParseWithOutputResult()
diff --git a/src/xagent/core/tools/core/excel.py b/src/xagent/core/tools/core/excel.py
index b615b8e94..6af1e1a9f 100644
--- a/src/xagent/core/tools/core/excel.py
+++ b/src/xagent/core/tools/core/excel.py
@@ -1,9 +1,12 @@
 from __future__ import annotations
 
 import asyncio
+import functools
 from types import ModuleType
 from typing import Any, Dict, List, Optional, Set
 
+from ...path_locks import GLOBAL_PATH_MUTATION_LOCKS
+
 # Optional import for openpyxl
 openpyxl: ModuleType | None = None
 try:
@@ -201,7 +204,39 @@ async def update_excel_cells(
     loop = asyncio.get_running_loop()
 
     try:
-        workbook = await loop.run_in_executor(None, load_workbook, file_path)
+        return await loop.run_in_executor(
+            None,
+            functools.partial(
+                _update_excel_cells_sync,
+                file_path=file_path,
+                updates=updates,
+                sheet_name=sheet_name,
+                font_size=font_size,
+                auto_size=auto_size,
+                padding=padding,
+                wrap_text=wrap_text,
+                max_width=max_width,
+                auto_size_links=auto_size_links,
+            ),
+        )
+    except Exception as e:
+        raise ValueError(f"Batch update Excel failed: {e}") from e
+
+
+def _update_excel_cells_sync(
+    *,
+    file_path: str,
+    updates: List[Dict[str, Any]],
+    sheet_name: str,
+    font_size: Optional[int],
+    auto_size: bool,
+    padding: float,
+    wrap_text: bool,
+    max_width: Optional[float],
+    auto_size_links: bool,
+) -> str:
+    with GLOBAL_PATH_MUTATION_LOCKS.guard_path(file_path) as guarded_file_path:
+        workbook = load_workbook(filename=guarded_file_path)
         sheet = workbook[sheet_name]
 
         updated_count = 0
@@ -216,12 +251,10 @@ async def update_excel_cells(
             try:
                 cell = sheet[cell_address]
 
-                # Value update
                 if "new_value" in update_item:
                     cell.value = update_item.get("new_value")
                 new_value = cell.value
 
-                # Hyperlink handling: set or clear if provided
                 hyperlink_provided = "hyperlink" in update_item
                 hyperlink_target = (
                     update_item.get("hyperlink") if hyperlink_provided else None
@@ -238,7 +271,6 @@ async def update_excel_cells(
                     _apply_basic_font(cell, font_size)
                     _apply_wrap_alignment(cell, wrap_text)
 
-                # Comment handling: set or clear if provided
                 if "comment_text" in update_item:
                     comment_text = update_item.get("comment_text")
                     if comment_text:
@@ -247,7 +279,6 @@ async def update_excel_cells(
                     else:
                         cell.comment = None
 
-                # Auto size
                 should_auto_size = auto_size and (
                     auto_size_links or not hyperlink_target
                 )
@@ -268,9 +299,7 @@ async def update_excel_cells(
             except Exception as e:
                 raise ValueError(f"Failed to update cell {cell_address}: {e}") from e
 
-        await loop.run_in_executor(None, workbook.save, file_path)
+        workbook.save(guarded_file_path)
         return (
             f"Successfully batch updated {updated_count} cells in sheet '{sheet_name}'."
         )
-    except Exception as e:
-        raise ValueError(f"Batch update Excel failed: {e}") from e
diff --git a/src/xagent/core/tools/core/image_tool.py b/src/xagent/core/tools/core/image_tool.py
index 17aafbb0f..965d40b2c 100644
--- a/src/xagent/core/tools/core/image_tool.py
+++ b/src/xagent/core/tools/core/image_tool.py
@@ -10,14 +10,16 @@
 import os
 import shutil
 import uuid
+from contextlib import contextmanager
 from pathlib import Path
-from typing import Any, Dict, Optional
+from typing import Any, Dict, Iterator, Optional
 from urllib import parse
 
 import aiohttp
 
 from ...file_ref import build_workspace_file_ref
 from ...model.image.base import BaseImageModel
+from ...path_locks import GLOBAL_PATH_MUTATION_LOCKS
 from ...workspace import TaskWorkspace
 
 logger = logging.getLogger(__name__)
@@ -371,6 +373,23 @@ def _normalize_image_inputs(self, image: str | list[str]) -> list[str]:
             return image
         raise ValueError("image must be a string or list of strings")
 
+    @contextmanager
+    def _guard_output_path(self, save_path: Path) -> Iterator[Path]:
+        """Use the workspace guard when available, otherwise the shared path lock."""
+        workspace_guard = None
+        if isinstance(self._workspace, TaskWorkspace):
+            workspace_guard = self._workspace.guard_workspace_mutation_path
+        elif self._workspace is not None:
+            workspace_guard = vars(self._workspace).get("guard_workspace_mutation_path")
+
+        if workspace_guard is not None:
+            with workspace_guard(save_path) as guarded_path:
+                yield Path(guarded_path)
+            return
+
+        with GLOBAL_PATH_MUTATION_LOCKS.guard_path(save_path) as guarded_path:
+            yield guarded_path
+
     async def _download_image(
         self, image_url: str, filename: Optional[str] = None, timeout: int = 30
     ) -> str:
@@ -420,48 +439,52 @@ async def _download_image(
         save_path = self._workspace.output_dir / filename
 
         try:
-            if is_data_url:
-                header, _, data = image_url.partition(",")
-                if ";base64" in header:
-                    content = base64.b64decode(data)
-                else:
-                    content = parse.unquote_to_bytes(data)
-                with open(save_path, "wb") as f:
-                    f.write(content)
-                logger.info(f"Saved data URL image to: {save_path}")
-                return str(save_path)
-
-            local_path = None
-            if image_url.startswith("file://"):
-                parsed = parse.urlparse(image_url)
-                if parsed.scheme == "file":
-                    local_path = Path(parse.unquote(parsed.path))
-            elif os.path.isabs(image_url) or os.path.exists(image_url):
-                local_path = Path(image_url)
-
-            if local_path is not None:
-                if not local_path.is_file():
-                    raise RuntimeError(f"Local image path is not a file: {local_path}")
-                shutil.copyfile(local_path, save_path)
-                logger.info(f"Copied local image to: {save_path}")
-                return str(save_path)
-
-            # Download the image with configurable timeout
-            timeout_obj = aiohttp.ClientTimeout(total=timeout)
-            async with aiohttp.ClientSession(timeout=timeout_obj) as session:
-                async with session.get(image_url) as response:
-                    if response.status != 200:
+            with self._guard_output_path(save_path) as guarded_save_path:
+                save_path = guarded_save_path
+                if is_data_url:
+                    header, _, data = image_url.partition(",")
+                    if ";base64" in header:
+                        content = base64.b64decode(data)
+                    else:
+                        content = parse.unquote_to_bytes(data)
+                    with open(save_path, "wb") as f:
+                        f.write(content)
+                    logger.info(f"Saved data URL image to: {save_path}")
+                    return str(save_path)
+
+                local_path = None
+                if image_url.startswith("file://"):
+                    parsed = parse.urlparse(image_url)
+                    if parsed.scheme == "file":
+                        local_path = Path(parse.unquote(parsed.path))
+                elif os.path.isabs(image_url) or os.path.exists(image_url):
+                    local_path = Path(image_url)
+
+                if local_path is not None:
+                    if not local_path.is_file():
                         raise RuntimeError(
-                            f"Failed to download image: HTTP {response.status}"
+                            f"Local image path is not a file: {local_path}"
                         )
+                    shutil.copyfile(local_path, save_path)
+                    logger.info(f"Copied local image to: {save_path}")
+                    return str(save_path)
+
+                # Download the image with configurable timeout
+                timeout_obj = aiohttp.ClientTimeout(total=timeout)
+                async with aiohttp.ClientSession(timeout=timeout_obj) as session:
+                    async with session.get(image_url) as response:
+                        if response.status != 200:
+                            raise RuntimeError(
+                                f"Failed to download image: HTTP {response.status}"
+                            )
 
-                    # Save the image
-                    with open(save_path, "wb") as f:
-                        async for chunk in response.content.iter_chunked(8192):
-                            f.write(chunk)
+                        # Save the image
+                        with open(save_path, "wb") as f:
+                            async for chunk in response.content.iter_chunked(8192):
+                                f.write(chunk)
 
-            logger.info(f"Downloaded image to: {save_path}")
-            return str(save_path)
+                logger.info(f"Downloaded image to: {save_path}")
+                return str(save_path)
 
         except Exception as e:
             logger.warning(f"Failed to download image from {image_url}: {e}")
diff --git a/tests/core/agent/test_react_real_tool_concurrency.py b/tests/core/agent/test_react_real_tool_concurrency.py
index f011664e1..bee4507f0 100644
--- a/tests/core/agent/test_react_real_tool_concurrency.py
+++ b/tests/core/agent/test_react_real_tool_concurrency.py
@@ -23,10 +23,12 @@
 from __future__ import annotations
 
 import asyncio
+import base64
 import inspect
 import time
 from contextlib import asynccontextmanager, contextmanager
 from dataclasses import dataclass
+from pathlib import Path
 from threading import Lock
 from typing import Any, Callable, Iterator
 
@@ -42,6 +44,7 @@
 from xagent.core.tools.adapters.vibe.exa_web_search import ExaWebSearchTool
 from xagent.core.tools.adapters.vibe.browser_use import BrowserEvaluateTool
 from xagent.core.tools.adapters.vibe.fetch_web_content import FetchWebContentTool
+from xagent.core.tools.adapters.vibe.image_tool import ImageGenerationTool
 from xagent.core.tools.adapters.vibe.python_executor import PythonExecutorTool
 from xagent.core.tools.adapters.vibe.tavily_web_search import TavilyWebSearchTool
 from xagent.core.tools.adapters.vibe.web_search import WebSearchTool
@@ -146,6 +149,35 @@ async def get_or_create(
         return self._sessions[session_id]
 
 
+class _TrackedImageModel:
+    model_id = "tracked-image"
+
+    def __init__(
+        self,
+        tracker: ConcurrencyTracker,
+        barrier: asyncio.Barrier,
+    ) -> None:
+        self._tracker = tracker
+        self._barrier = barrier
+
+    def has_ability(self, ability: str) -> bool:
+        return ability == "generate"
+
+    async def generate_image(self, **kwargs: Any) -> dict[str, Any]:
+        prompt = str(kwargs["prompt"])
+        self._tracker.enter(prompt)
+        try:
+            await asyncio.wait_for(self._barrier.wait(), timeout=_RENDEZVOUS_TIMEOUT)
+        finally:
+            self._tracker.leave(prompt)
+
+        encoded = base64.b64encode(f"image:{prompt}".encode()).decode()
+        return {
+            "image_url": f"data:image/png;base64,{encoded}",
+            "request_id": f"request-{prompt}",
+        }
+
+
 @dataclass
 class _RealToolCase:
     label: str
@@ -495,3 +527,61 @@ def code_for(label: str) -> str:
     assert [r["tool_call_id"] for r in context.tool_results] == [
         tc["id"] for tc in batch
     ]
+
+
+async def test_real_image_generation_tool_overlaps_and_registers_unique_artifacts(
+    tmp_path: Any,
+) -> None:
+    """Image generation runs through ReAct concurrency with isolated artifacts."""
+    tracker = ConcurrencyTracker()
+    barrier = asyncio.Barrier(2)
+    workspace = TaskWorkspace("react_image_generation", str(tmp_path))
+    tool = ImageGenerationTool(
+        {"tracked-image": _TrackedImageModel(tracker, barrier)},
+        workspace=workspace,
+    ).get_tools()[0]
+    pattern = make_react(parallel=True, max_concurrency=2)
+    context = RecordingContext()
+    batch = [
+        make_tool_call(tool.name, {"prompt": "alpha"}),
+        make_tool_call(tool.name, {"prompt": "beta"}),
+    ]
+
+    segment, kind = pattern._next_segment(batch, [tool])
+    assert kind == "concurrent"
+    assert segment == batch
+
+    results = await asyncio.wait_for(
+        pattern._run_concurrent_batch(batch, [tool], FakeRuntime(), context),
+        timeout=_RENDEZVOUS_TIMEOUT * 2,
+    )
+
+    assert tracker.peak == 2
+    assert len(results) == 2
+    assert all(pattern._tool_result_success(result) for result in results)
+    assert results[0]["request_id"] == "request-alpha"
+    assert results[1]["request_id"] == "request-beta"
+
+    image_paths = [result["image_path"] for result in results]
+    assert len(set(image_paths)) == 2
+    assert all(
+        path and (workspace.output_dir / Path(path).name).exists()
+        for path in image_paths
+    )
+
+    for result in results:
+        assert result["file_id"]
+        assert result["file_ref"]["file_id"] == result["file_id"]
+        assert result["artifacts"] == [
+            {
+                "type": "image",
+                "file_id": result["file_id"],
+                "filename": Path(result["image_path"]).name,
+                "mime_type": "image/png",
+                "display": "inline",
+            }
+        ]
+
+    assert [r["tool_call_id"] for r in context.tool_results] == [
+        tc["id"] for tc in batch
+    ]
diff --git a/tests/core/tools/adapters/vibe/test_tool_metadata_concurrency.py b/tests/core/tools/adapters/vibe/test_tool_metadata_concurrency.py
index 4fef4b926..7dd3be88e 100644
--- a/tests/core/tools/adapters/vibe/test_tool_metadata_concurrency.py
+++ b/tests/core/tools/adapters/vibe/test_tool_metadata_concurrency.py
@@ -23,6 +23,10 @@
 
 from tests.core.tools.adapters.sandboxed_tool.conftest import FakeBaseTool
 from xagent.core.tools.adapters.vibe.base import ToolMetadata
+from xagent.core.tools.adapters.vibe.document_parser import (
+    DocumentParseTool,
+    DocumentParseWithOutputTool,
+)
 from xagent.core.tools.adapters.vibe.fetch_web_content import FetchWebContentTool
 from xagent.core.tools.adapters.vibe.file_tool import (
     append_file_tool,
@@ -36,6 +40,7 @@
     write_json_file_tool,
 )
 from xagent.core.tools.adapters.vibe.function import FunctionTool
+from xagent.core.tools.adapters.vibe.image_tool import ImageGenerationTool
 from xagent.core.tools.adapters.vibe.output_filter_wrapper import (
     OutputFilteredToolWrapper,
 )
@@ -58,6 +63,12 @@ def _noop(x: int = 0) -> dict[str, Any]:
     return {"x": x}
 
 
+def _image_model() -> MagicMock:
+    model = MagicMock()
+    model.has_ability.side_effect = lambda ability: ability in {"generate", "edit"}
+    return model
+
+
 def test_tool_metadata_defaults_are_not_concurrency_safe() -> None:
     metadata = ToolMetadata(name="anything")
     assert metadata.read_only is False
@@ -186,6 +197,29 @@ def test_workspace_file_tool_metadata_matches_guarded_concurrency(tmp_path) -> N
         assert tools[name].metadata.concurrency_safe is True
 
 
+def test_image_tool_metadata_matches_artifact_safety(tmp_path) -> None:
+    workspace = TaskWorkspace("metadata_image_artifacts", str(tmp_path))
+    image_tool = ImageGenerationTool({"model": _image_model()}, workspace=workspace)
+    tools = {tool.name: tool for tool in image_tool.get_tools()}
+
+    assert tools["list_image_models"].metadata.read_only is True
+    assert tools["list_image_models"].metadata.concurrency_safe is True
+
+    for name in {"generate_image", "edit_image"}:
+        assert tools[name].metadata.read_only is False
+        assert tools[name].metadata.concurrency_safe is True
+
+
+def test_document_parser_metadata_matches_artifact_safety() -> None:
+    parse_tool = DocumentParseTool()
+    parse_with_output_tool = DocumentParseWithOutputTool()
+
+    assert parse_tool.metadata.read_only is True
+    assert parse_tool.metadata.concurrency_safe is True
+    assert parse_with_output_tool.metadata.read_only is False
+    assert parse_with_output_tool.metadata.concurrency_safe is True
+
+
 def test_abstract_base_tool_metadata_exposes_concurrency_fields() -> None:
     # The metadata object must always carry the fields (even when False) so the
     # scheduler can read them uniformly across every tool.
diff --git a/tests/core/tools/core/test_excel.py b/tests/core/tools/core/test_excel.py
index a07b4cb26..05cec5419 100644
--- a/tests/core/tools/core/test_excel.py
+++ b/tests/core/tools/core/test_excel.py
@@ -1,6 +1,11 @@
+from contextlib import contextmanager
+from pathlib import Path
+from types import SimpleNamespace
+
 import pytest
 from openpyxl import Workbook, load_workbook
 
+from xagent.core.tools.core import excel as excel_module
 from xagent.core.tools.core.excel import (
     calculate_cell_size_from_font,
     read_excel_cells,
@@ -76,6 +81,33 @@ async def test_update_excel_cells_basic(temp_excel_file):
     assert any("E5: New Data 2" in cell for cell in updated_content)
 
 
+@pytest.mark.asyncio
+async def test_update_excel_cells_guards_the_entire_file_mutation(
+    temp_excel_file, monkeypatch
+):
+    guarded_paths = []
+
+    @contextmanager
+    def guard_path(path):
+        guarded_paths.append(Path(path).resolve())
+        yield Path(path).resolve()
+
+    monkeypatch.setattr(
+        excel_module,
+        "GLOBAL_PATH_MUTATION_LOCKS",
+        SimpleNamespace(guard_path=guard_path),
+        raising=False,
+    )
+
+    result = await update_excel_cells(
+        file_path=temp_excel_file,
+        updates=[{"cell_address": "A1", "new_value": "Guarded"}],
+    )
+
+    assert "Successfully batch updated 1 cells" in result
+    assert guarded_paths == [Path(temp_excel_file).resolve()]
+
+
 def test_calculate_cell_size_from_font():
     """Test cell size calculation function"""
     # Test basic calculation
diff --git a/tests/core/tools/core/test_image_tool_core.py b/tests/core/tools/core/test_image_tool_core.py
index 972d36818..0ac87e935 100644
--- a/tests/core/tools/core/test_image_tool_core.py
+++ b/tests/core/tools/core/test_image_tool_core.py
@@ -2,6 +2,8 @@
 Tests for ImageGenerationToolCore class
 """
 
+from contextlib import contextmanager
+from pathlib import Path
 from unittest.mock import AsyncMock, Mock, patch
 
 import pytest
@@ -339,6 +341,27 @@ async def mock_iter_chunked(chunk_size):
         # Verify the call was made
         mock_get.assert_called_once()
 
+    @pytest.mark.asyncio
+    async def test_download_image_guards_workspace_output_path(self, mock_workspace):
+        """Image output writes must share the workspace path-lock contract."""
+        guarded_paths = []
+
+        @contextmanager
+        def guard_workspace_mutation_path(path, default_dir="output"):
+            guarded_paths.append(Path(path).resolve())
+            yield Path(path).resolve()
+
+        mock_workspace.guard_workspace_mutation_path = guard_workspace_mutation_path
+        tool = ImageGenerationToolCore({}, {}, mock_workspace)
+
+        result = await tool._download_image(
+            "data:image/png;base64,ZmFrZV9pbWFnZQ==",
+            filename="guarded.png",
+        )
+
+        assert Path(result).name == "guarded.png"
+        assert guarded_paths == [(mock_workspace.output_dir / "guarded.png").resolve()]
+
     def test_list_available_models(self, image_tool_core, mock_image_models):
         """Test listing available models"""
         result = image_tool_core.list_available_models()
diff --git a/tests/core/tools/test_document_tool.py b/tests/core/tools/test_document_tool.py
index 35a9d0de0..2a56082c9 100644
--- a/tests/core/tools/test_document_tool.py
+++ b/tests/core/tools/test_document_tool.py
@@ -1,3 +1,6 @@
+from contextlib import contextmanager
+from pathlib import Path
+from types import SimpleNamespace
 from unittest.mock import AsyncMock, MagicMock, patch
 
 import pytest
@@ -6,6 +9,7 @@
     DocumentParseTool,
     DocumentParseWithOutputTool,
 )
+from xagent.core.tools.core import document_parser as document_parser_core
 from xagent.core.tools.core.document_parser import (
     DocumentCapabilities,
     DocumentParseArgs,
@@ -433,6 +437,57 @@ async def test_document_parse_with_output_tool_successful_execution(
     mock_file.write.assert_any_call("parser_version: v1.0\n")
 
 
+@pytest.mark.asyncio
+@patch("builtins.open")
+async def test_document_parse_with_output_guards_output_path(
+    mock_open,
+    mock_parser_registry,
+    mock_workspace,
+    parse_result_full,
+    monkeypatch,
+):
+    parser_name = "full_local_combo"
+    mock_parser_registry["instances"][
+        parser_name
+    ].parse.return_value = parse_result_full
+
+    mock_file = MagicMock()
+    mock_open.return_value.__enter__.return_value = mock_file
+    guarded_paths = []
+
+    @contextmanager
+    def guard_path(path):
+        guarded_paths.append(Path(path).resolve())
+        yield Path(path).resolve()
+
+    monkeypatch.setattr(
+        document_parser_core,
+        "GLOBAL_PATH_MUTATION_LOCKS",
+        SimpleNamespace(guard_path=guard_path),
+        raising=False,
+    )
+
+    tool = DocumentParseWithOutputTool(workspace=mock_workspace)
+    args = {
+        "file_path": "input.pdf",
+        "parser_name": parser_name,
+        "output_path": "output.txt",
+        "output_format": "txt",
+        "capabilities": {
+            "capability_text": True,
+            "capability_figure": False,
+            "requires_full_text_result": False,
+            "requires_segmented_result": True,
+            "use_local_parser": True,
+        },
+    }
+
+    result = await tool.run_json_async(args)
+
+    assert result == DocumentParseWithOutputResult()
+    assert guarded_paths == [Path("/mock/workspace/output.txt").resolve()]
+
+
 @pytest.mark.asyncio
 @patch("builtins.open")
 async def test_document_parse_with_output_tool_error_on_file_writing(

From 32fdd88b5e956f0e04cf3f52a90a63b59832d8c5 Mon Sep 17 00:00:00 2001
From: leyoonafr <codeacme17@gmail.com>
Date: Sun, 28 Jun 2026 22:15:30 +0800
Subject: [PATCH 6/7] feat: Enhance concurrency safety in path locks and
 improve workspace file state management

---
 src/xagent/core/path_locks.py                 | 39 +++++++++++++-----
 src/xagent/core/tools/core/python_executor.py |  3 +-
 src/xagent/core/workspace.py                  | 40 ++++++++++++++-----
 tests/core/test_path_locks.py                 | 32 +++++++++++++++
 .../test_workspace_durable_registration.py    | 24 +++++++++++
 tests/core/tools/test_python_executor.py      | 23 +++++++++++
 6 files changed, 141 insertions(+), 20 deletions(-)
 create mode 100644 tests/core/test_path_locks.py

diff --git a/src/xagent/core/path_locks.py b/src/xagent/core/path_locks.py
index c58a8c037..f93b5ad32 100644
--- a/src/xagent/core/path_locks.py
+++ b/src/xagent/core/path_locks.py
@@ -13,27 +13,46 @@ class PathMutationLockRegistry:
 
     def __init__(self) -> None:
         self._guard = threading.Lock()
-        self._locks: dict[str, threading.RLock] = {}
+        self._locks: dict[str, tuple[threading.RLock, int]] = {}
 
     @staticmethod
     def normalize_path(path: str | Path) -> Path:
         return Path(path).expanduser().resolve()
 
-    def _lock_for_normalized_path(self, normalized_path: Path) -> threading.RLock:
-        key = str(normalized_path)
+    def _acquire_lock_for_key(self, key: str) -> threading.RLock:
         with self._guard:
-            lock = self._locks.get(key)
-            if lock is None:
+            lock_entry = self._locks.get(key)
+            if lock_entry is None:
                 lock = threading.RLock()
-                self._locks[key] = lock
+                self._locks[key] = (lock, 1)
+                return lock
+
+            lock, ref_count = lock_entry
+            self._locks[key] = (lock, ref_count + 1)
             return lock
 
+    def _release_lock_for_key(self, key: str) -> None:
+        with self._guard:
+            lock_entry = self._locks.get(key)
+            if lock_entry is None:
+                return
+
+            lock, ref_count = lock_entry
+            if ref_count <= 1:
+                self._locks.pop(key, None)
+            else:
+                self._locks[key] = (lock, ref_count - 1)
+
     @contextmanager
     def guard_path(self, path: str | Path) -> Iterator[Path]:
         normalized_path = self.normalize_path(path)
-        lock = self._lock_for_normalized_path(normalized_path)
-        with lock:
-            yield normalized_path
+        key = str(normalized_path)
+        lock = self._acquire_lock_for_key(key)
+        try:
+            with lock:
+                yield normalized_path
+        finally:
+            self._release_lock_for_key(key)
 
     @contextmanager
     def guard_paths(self, paths: list[str | Path]) -> Iterator[tuple[Path, ...]]:
@@ -43,7 +62,7 @@ def guard_paths(self, paths: list[str | Path]) -> Iterator[tuple[Path, ...]]:
 
         with ExitStack() as stack:
             for normalized_path in ordered_paths:
-                stack.enter_context(self._lock_for_normalized_path(normalized_path))
+                stack.enter_context(self.guard_path(normalized_path))
             yield normalized_paths
 
 
diff --git a/src/xagent/core/tools/core/python_executor.py b/src/xagent/core/tools/core/python_executor.py
index 27aad3025..b45420a13 100644
--- a/src/xagent/core/tools/core/python_executor.py
+++ b/src/xagent/core/tools/core/python_executor.py
@@ -294,10 +294,12 @@ def audit_hook(event: str, args: tuple[Any, ...]) -> None:
 
 
 def _child_main() -> int:
+    result_path_value: Any = None
     try:
         payload = json.loads(sys.stdin.read())
         if not isinstance(payload, dict):
             raise ValueError("Child payload must be a JSON object")
+        result_path_value = payload.get("result_path")
         result_path = Path(str(payload["result_path"]))
         result = _execute_child_payload(payload)
         result_path.write_text(json.dumps(result), encoding="utf-8")
@@ -309,7 +311,6 @@ def _child_main() -> int:
             "error": f"Python executor child error: {exc}",
             _INTERNAL_WRITTEN_FILES_KEY: [],
         }
-        result_path_value = locals().get("payload", {}).get("result_path")
         if result_path_value:
             try:
                 Path(str(result_path_value)).write_text(
diff --git a/src/xagent/core/workspace.py b/src/xagent/core/workspace.py
index b0185f188..5d818b396 100644
--- a/src/xagent/core/workspace.py
+++ b/src/xagent/core/workspace.py
@@ -16,7 +16,7 @@
 from contextlib import contextmanager
 from dataclasses import dataclass
 from pathlib import Path
-from typing import Any, Dict, Iterator, List, Optional
+from typing import Any, Dict, Iterator, List, Optional, cast
 from uuid import uuid4
 
 from ..config import get_uploads_dir
@@ -622,7 +622,7 @@ def resolve_file_id(self, file_id: str) -> Optional[Path]:
                         return None
                     from ..web.services.managed_file_ref import ManagedFileRef
 
-                    return ManagedFileRef(record).materialize()
+                    return cast(Path, ManagedFileRef(record).materialize())
                 return None
             finally:
                 if should_close:
@@ -1023,20 +1023,31 @@ def auto_register_files(self) -> "Iterator[TaskWorkspace]":
         """
         # Scan files before operation
         with self._file_registration_lock:
-            files_before = self._scan_all_files()
+            file_states_before = self._scan_all_file_states()
 
         try:
             yield self
         finally:
             # Scan files after operation and register new/modified files.
             with self._file_registration_lock:
-                files_after = self._scan_all_files()
+                file_states_after = self._scan_all_file_states()
+                files_before = set(file_states_before)
+                files_after = set(file_states_after)
                 changed_files = files_after - files_before
-                changed_files.update(
-                    file_path
-                    for file_path in files_after & files_before
-                    if self._get_file_id_from_db(file_path, self.db_session) is not None
-                )
+
+                for file_path in files_after & files_before:
+                    if file_states_after[file_path] == file_states_before[file_path]:
+                        continue
+
+                    resolved_str = str(file_path.resolve())
+                    cached_file_id = self._recently_registered_files.get(
+                        str(file_path)
+                    ) or self._recently_registered_files.get(resolved_str)
+                    if cached_file_id or (
+                        self._get_file_id_from_db(file_path, self.db_session)
+                        is not None
+                    ):
+                        changed_files.add(file_path)
 
                 for file_path in changed_files:
                     try:
@@ -1069,6 +1080,17 @@ def _scan_all_files(self) -> set[Path]:
                 files.add(file_path)
         return files
 
+    def _scan_all_file_states(self) -> dict[Path, tuple[int, int]]:
+        """Scan all workspace files and return path to mtime/size state."""
+        file_states: dict[Path, tuple[int, int]] = {}
+        for file_path in self._scan_all_files():
+            try:
+                stat = file_path.stat()
+            except OSError:
+                continue
+            file_states[file_path] = (stat.st_mtime_ns, stat.st_size)
+        return file_states
+
     def get_file_id_from_path(self, file_path: str) -> Optional[str]:
         """Get file_id from file path using database or in-memory cache."""
         try:
diff --git a/tests/core/test_path_locks.py b/tests/core/test_path_locks.py
new file mode 100644
index 000000000..086052bd6
--- /dev/null
+++ b/tests/core/test_path_locks.py
@@ -0,0 +1,32 @@
+from xagent.core.path_locks import PathMutationLockRegistry
+
+
+def test_guard_path_removes_unused_lock_after_context_exit(tmp_path):
+    registry = PathMutationLockRegistry()
+    path = tmp_path / "artifact.png"
+    key = str(path.resolve())
+
+    with registry.guard_path(path) as normalized_path:
+        assert normalized_path == path.resolve()
+        assert key in registry._locks
+
+    assert registry._locks == {}
+
+
+def test_guard_paths_removes_each_unique_lock_after_context_exit(tmp_path):
+    registry = PathMutationLockRegistry()
+    first_path = tmp_path / "first.txt"
+    second_path = tmp_path / "second.txt"
+
+    with registry.guard_paths([second_path, first_path, second_path]) as normalized:
+        assert normalized == (
+            second_path.resolve(),
+            first_path.resolve(),
+            second_path.resolve(),
+        )
+        assert set(registry._locks) == {
+            str(first_path.resolve()),
+            str(second_path.resolve()),
+        }
+
+    assert registry._locks == {}
diff --git a/tests/core/test_workspace_durable_registration.py b/tests/core/test_workspace_durable_registration.py
index 6a2e674d9..9e87a8d0f 100644
--- a/tests/core/test_workspace_durable_registration.py
+++ b/tests/core/test_workspace_durable_registration.py
@@ -560,6 +560,30 @@ def test_auto_register_files_resyncs_modified_existing_file(
         engine.dispose()
 
 
+def test_auto_register_files_skips_db_lookup_for_unchanged_existing_files(
+    monkeypatch, tmp_path
+):
+    workspace = TaskWorkspace(id="web_task_900", base_dir=str(tmp_path / "workspaces"))
+    for index in range(5):
+        (workspace.output_dir / f"unchanged-{index}.txt").write_text(
+            "same content", encoding="utf-8"
+        )
+
+    queried_paths = []
+
+    def record_db_lookup(file_path, db_session=None):
+        del db_session
+        queried_paths.append(file_path)
+        return None
+
+    monkeypatch.setattr(workspace, "_get_file_id_from_db", record_db_lookup)
+
+    with workspace.auto_register_files():
+        pass
+
+    assert queried_paths == []
+
+
 def test_workspace_register_file_resyncs_external_file_without_reclassifying_upload(
     monkeypatch, tmp_path, mock_workspace_db
 ):
diff --git a/tests/core/tools/test_python_executor.py b/tests/core/tools/test_python_executor.py
index cda04176c..18b38a710 100644
--- a/tests/core/tools/test_python_executor.py
+++ b/tests/core/tools/test_python_executor.py
@@ -4,6 +4,7 @@
 
 import asyncio
 import os
+import subprocess
 import sys
 import time
 from pathlib import Path
@@ -15,6 +16,7 @@
     PythonExecutorResult,
     PythonExecutorTool,
 )
+from xagent.core.tools.core import python_executor as python_executor_core
 from xagent.core.workspace import TaskWorkspace
 
 
@@ -82,6 +84,27 @@ def test_import_visible_inside_comprehension(self, python_executor):
         assert "3 True" in result["output"]
         assert result["error"] == ""
 
+    def test_child_main_handles_non_dict_payload_without_secondary_exception(self):
+        """Invalid child payloads should report the original error cleanly."""
+        process = subprocess.run(
+            [
+                sys.executable,
+                str(Path(python_executor_core.__file__).resolve()),
+                "--xagent-python-executor-child",
+            ],
+            input="[]",
+            text=True,
+            capture_output=True,
+            check=False,
+        )
+
+        assert process.returncode == 1
+        assert (
+            "Python executor child error: Child payload must be a JSON object"
+            in process.stderr
+        )
+        assert "AttributeError" not in process.stderr
+
     def test_print_output(self, python_executor):
         """Test code with print statements"""
         result = python_executor.run_json_sync(

From 87c063e6ffb3074cc62fb07761d4a6c2b00dcb86 Mon Sep 17 00:00:00 2001
From: leyoonafr <codeacme17@gmail.com>
Date: Mon, 29 Jun 2026 12:56:24 +0800
Subject: [PATCH 7/7] feat: Implement RAG ingestion locks for concurrency
 safety in document and web ingestion processes

---
 .../core/RAG_tools/kb/ingestion_locks.py      | 114 ++++++++
 .../RAG_tools/kb/pipeline_compatibility.py    | 191 ++++++-------
 .../vibe/test_tool_metadata_concurrency.py    |  17 ++
 .../kb/test_pipeline_compatibility.py         | 259 ++++++++++++++++++
 4 files changed, 489 insertions(+), 92 deletions(-)
 create mode 100644 src/xagent/core/tools/core/RAG_tools/kb/ingestion_locks.py

diff --git a/src/xagent/core/tools/core/RAG_tools/kb/ingestion_locks.py b/src/xagent/core/tools/core/RAG_tools/kb/ingestion_locks.py
new file mode 100644
index 000000000..7820adb3a
--- /dev/null
+++ b/src/xagent/core/tools/core/RAG_tools/kb/ingestion_locks.py
@@ -0,0 +1,114 @@
+"""Process-local locks for RAG ingestion write boundaries."""
+
+from __future__ import annotations
+
+import asyncio
+import threading
+from contextlib import asynccontextmanager, contextmanager
+from contextvars import ContextVar
+from typing import AsyncIterator, Iterator
+
+
+_HELD_LOCK_COUNTS: ContextVar[dict[str, int]] = ContextVar(
+    "rag_ingestion_held_lock_counts",
+    default={},
+)
+
+
+class RAGIngestionLockRegistry:
+    """Provide re-entrant ingestion locks keyed by logical RAG resources."""
+
+    def __init__(self) -> None:
+        self._guard = threading.Lock()
+        self._locks: dict[str, tuple[threading.Lock, int]] = {}
+
+    @staticmethod
+    def collection_key(collection: str) -> str:
+        normalized = str(collection or "").strip()
+        return f"collection:{normalized or '<empty>'}"
+
+    def _retain_lock(self, key: str) -> threading.Lock:
+        with self._guard:
+            entry = self._locks.get(key)
+            if entry is None:
+                lock = threading.Lock()
+                self._locks[key] = (lock, 1)
+                return lock
+
+            lock, ref_count = entry
+            self._locks[key] = (lock, ref_count + 1)
+            return lock
+
+    def _release_lock_entry(self, key: str) -> None:
+        with self._guard:
+            entry = self._locks.get(key)
+            if entry is None:
+                return
+
+            lock, ref_count = entry
+            if ref_count <= 1:
+                self._locks.pop(key, None)
+            else:
+                self._locks[key] = (lock, ref_count - 1)
+
+    @staticmethod
+    def _enter_reentrant_context(key: str) -> object:
+        held_counts = dict(_HELD_LOCK_COUNTS.get())
+        held_counts[key] = held_counts.get(key, 0) + 1
+        return _HELD_LOCK_COUNTS.set(held_counts)
+
+    @staticmethod
+    def _holds_lock(key: str) -> bool:
+        return _HELD_LOCK_COUNTS.get().get(key, 0) > 0
+
+    @contextmanager
+    def guard_collection(self, collection: str) -> Iterator[str]:
+        key = self.collection_key(collection)
+        if self._holds_lock(key):
+            token = self._enter_reentrant_context(key)
+            try:
+                yield key
+            finally:
+                _HELD_LOCK_COUNTS.reset(token)
+            return
+
+        lock = self._retain_lock(key)
+        lock.acquire()
+        token = self._enter_reentrant_context(key)
+        try:
+            yield key
+        finally:
+            _HELD_LOCK_COUNTS.reset(token)
+            lock.release()
+            self._release_lock_entry(key)
+
+    @asynccontextmanager
+    async def async_guard_collection(self, collection: str) -> AsyncIterator[str]:
+        key = self.collection_key(collection)
+        if self._holds_lock(key):
+            token = self._enter_reentrant_context(key)
+            try:
+                yield key
+            finally:
+                _HELD_LOCK_COUNTS.reset(token)
+            return
+
+        lock = self._retain_lock(key)
+        try:
+            while not lock.acquire(blocking=False):
+                await asyncio.sleep(0.01)
+            token = self._enter_reentrant_context(key)
+            try:
+                yield key
+            finally:
+                _HELD_LOCK_COUNTS.reset(token)
+                lock.release()
+        finally:
+            self._release_lock_entry(key)
+
+    def reset_for_tests(self) -> None:
+        with self._guard:
+            self._locks.clear()
+
+
+RAG_INGESTION_LOCKS = RAGIngestionLockRegistry()
diff --git a/src/xagent/core/tools/core/RAG_tools/kb/pipeline_compatibility.py b/src/xagent/core/tools/core/RAG_tools/kb/pipeline_compatibility.py
index 7d267c3b9..4e7dd018f 100644
--- a/src/xagent/core/tools/core/RAG_tools/kb/pipeline_compatibility.py
+++ b/src/xagent/core/tools/core/RAG_tools/kb/pipeline_compatibility.py
@@ -23,6 +23,7 @@
     PersistencePolicy,
     SideEffectPlane,
 )
+from .ingestion_locks import RAG_INGESTION_LOCKS
 
 if TYPE_CHECKING:
     from ..core.schemas import CollectionInfo
@@ -173,36 +174,37 @@ def process_document(
     ) -> IngestionResult:
         from ..pipelines.document_ingestion import _process_document_impl
 
-        with self._operation_context(
-            operation_type="document_ingestion",
-            collection=collection,
-            details={"source_path": source_path, "file_id": file_id},
-        ) as operation:
-            with self._storage_context():
-                result = _process_document_impl(
-                    collection=collection,
-                    source_path=source_path,
-                    config=config,
-                    progress_manager=progress_manager,
-                    user_id=user_id,
-                    is_admin=is_admin,
-                    file_id=file_id,
-                    metadata_source_path=metadata_source_path,
-                    commit_gate=commit_gate,
-                )
-                self._record_document_ingestion_side_effects(
-                    operation,
-                    result,
-                    collection=collection,
-                    source_path=source_path,
-                    file_id=file_id,
-                    user_id=user_id,
-                    is_admin=is_admin,
-                )
-                self.ensure_collection_backend_binding(collection)
-                if self._should_finish_document_ingestion_operation(operation):
-                    self._finish_document_ingestion_outcome(operation, result)
-                return result
+        with RAG_INGESTION_LOCKS.guard_collection(collection):
+            with self._operation_context(
+                operation_type="document_ingestion",
+                collection=collection,
+                details={"source_path": source_path, "file_id": file_id},
+            ) as operation:
+                with self._storage_context():
+                    result = _process_document_impl(
+                        collection=collection,
+                        source_path=source_path,
+                        config=config,
+                        progress_manager=progress_manager,
+                        user_id=user_id,
+                        is_admin=is_admin,
+                        file_id=file_id,
+                        metadata_source_path=metadata_source_path,
+                        commit_gate=commit_gate,
+                    )
+                    self._record_document_ingestion_side_effects(
+                        operation,
+                        result,
+                        collection=collection,
+                        source_path=source_path,
+                        file_id=file_id,
+                        user_id=user_id,
+                        is_admin=is_admin,
+                    )
+                    self.ensure_collection_backend_binding(collection)
+                    if self._should_finish_document_ingestion_operation(operation):
+                        self._finish_document_ingestion_outcome(operation, result)
+                    return result
 
     def run_document_ingestion(
         self,
@@ -219,44 +221,46 @@ def run_document_ingestion(
     ) -> IngestionResult:
         from ..pipelines.document_ingestion import _run_document_ingestion_impl
 
-        with self._operation_context(
-            operation_type="document_ingestion",
-            collection=collection,
-            details={"source_path": source_path, "file_id": file_id},
-        ) as operation:
-            with self._storage_context():
-                result: object = _run_document_ingestion_impl(
-                    collection=collection,
-                    source_path=source_path,
-                    ingestion_config=ingestion_config,
-                    progress_manager=progress_manager,
-                    user_id=user_id,
-                    is_admin=is_admin,
-                    file_id=file_id,
-                    metadata_source_path=metadata_source_path,
-                    commit_gate=commit_gate,
-                )
-                # Phase-1 compatibility: _run_document_ingestion_impl() still calls
-                # the public process_document symbol, which legacy tests/callers may
-                # monkeypatch. Only structured results carry rollback metadata.
-                if not isinstance(result, IngestionResult):
-                    return cast(IngestionResult, result)
-                if operation is not None and operation.outcome is None:
-                    self._record_document_ingestion_side_effects(
-                        operation,
-                        result,
+        with RAG_INGESTION_LOCKS.guard_collection(collection):
+            with self._operation_context(
+                operation_type="document_ingestion",
+                collection=collection,
+                details={"source_path": source_path, "file_id": file_id},
+            ) as operation:
+                with self._storage_context():
+                    result: object = _run_document_ingestion_impl(
                         collection=collection,
                         source_path=source_path,
-                        file_id=file_id,
+                        ingestion_config=ingestion_config,
+                        progress_manager=progress_manager,
                         user_id=user_id,
                         is_admin=is_admin,
+                        file_id=file_id,
+                        metadata_source_path=metadata_source_path,
+                        commit_gate=commit_gate,
                     )
-                    self.ensure_collection_backend_binding(collection)
-                    if self._should_finish_document_ingestion_operation(operation):
-                        self._finish_document_ingestion_outcome(operation, result)
-                elif operation is None:
-                    self.ensure_collection_backend_binding(collection)
-                return result
+                    # Phase-1 compatibility: _run_document_ingestion_impl() still
+                    # calls the public process_document symbol, which legacy
+                    # tests/callers may monkeypatch. Only structured results carry
+                    # rollback metadata.
+                    if not isinstance(result, IngestionResult):
+                        return cast(IngestionResult, result)
+                    if operation is not None and operation.outcome is None:
+                        self._record_document_ingestion_side_effects(
+                            operation,
+                            result,
+                            collection=collection,
+                            source_path=source_path,
+                            file_id=file_id,
+                            user_id=user_id,
+                            is_admin=is_admin,
+                        )
+                        self.ensure_collection_backend_binding(collection)
+                        if self._should_finish_document_ingestion_operation(operation):
+                            self._finish_document_ingestion_outcome(operation, result)
+                    elif operation is None:
+                        self.ensure_collection_backend_binding(collection)
+                    return result
 
     def search_documents(
         self,
@@ -315,36 +319,39 @@ async def run_web_ingestion(
     ) -> WebIngestionResult:
         from ..pipelines.web_ingestion import _run_web_ingestion_impl
 
-        with self._operation_context(
-            operation_type="web_ingestion",
-            collection=collection,
-            persistence_policy=PersistencePolicy.PRESERVE_SUCCESSFUL_CHILDREN,
-            details={"start_url": crawl_config.start_url},
-        ) as operation:
-            with self._storage_context():
-                result = await _run_web_ingestion_impl(
-                    collection=collection,
-                    crawl_config=crawl_config,
-                    ingestion_config=ingestion_config,
-                    progress_callback=progress_callback,
-                    user_id=user_id,
-                    is_admin=is_admin,
-                    file_handler=file_handler,
-                    pipeline_facade=self,
-                )
-                await self.ensure_collection_backend_binding_async(collection)
-                outcome = self._record_web_ingestion_outcome(operation, result)
-                if (
-                    outcome is not None
-                    and result.side_effects_may_remain
-                    != outcome.side_effects_may_remain
-                ):
-                    result = result.model_copy(
-                        update={
-                            "side_effects_may_remain": outcome.side_effects_may_remain
-                        }
+        async with RAG_INGESTION_LOCKS.async_guard_collection(collection):
+            with self._operation_context(
+                operation_type="web_ingestion",
+                collection=collection,
+                persistence_policy=PersistencePolicy.PRESERVE_SUCCESSFUL_CHILDREN,
+                details={"start_url": crawl_config.start_url},
+            ) as operation:
+                with self._storage_context():
+                    result = await _run_web_ingestion_impl(
+                        collection=collection,
+                        crawl_config=crawl_config,
+                        ingestion_config=ingestion_config,
+                        progress_callback=progress_callback,
+                        user_id=user_id,
+                        is_admin=is_admin,
+                        file_handler=file_handler,
+                        pipeline_facade=self,
                     )
-                return result
+                    await self.ensure_collection_backend_binding_async(collection)
+                    outcome = self._record_web_ingestion_outcome(operation, result)
+                    if (
+                        outcome is not None
+                        and result.side_effects_may_remain
+                        != outcome.side_effects_may_remain
+                    ):
+                        result = result.model_copy(
+                            update={
+                                "side_effects_may_remain": (
+                                    outcome.side_effects_may_remain
+                                )
+                            }
+                        )
+                    return result
 
     @contextmanager
     def web_page_operation(
diff --git a/tests/core/tools/adapters/vibe/test_tool_metadata_concurrency.py b/tests/core/tools/adapters/vibe/test_tool_metadata_concurrency.py
index 7dd3be88e..7f4891106 100644
--- a/tests/core/tools/adapters/vibe/test_tool_metadata_concurrency.py
+++ b/tests/core/tools/adapters/vibe/test_tool_metadata_concurrency.py
@@ -28,6 +28,9 @@
     DocumentParseWithOutputTool,
 )
 from xagent.core.tools.adapters.vibe.fetch_web_content import FetchWebContentTool
+from xagent.core.tools.adapters.vibe.file_ingestion_tool import (
+    CreateKnowledgeBaseFromFileTool,
+)
 from xagent.core.tools.adapters.vibe.file_tool import (
     append_file_tool,
     create_directory_tool,
@@ -53,6 +56,9 @@
 from xagent.core.tools.adapters.vibe.sandboxed_tool.sandboxed_tool_wrapper import (
     SandboxedToolWrapper,
 )
+from xagent.core.tools.adapters.vibe.web_ingestion_tool import (
+    CreateKnowledgeBaseFromUrlTool,
+)
 from xagent.core.tools.adapters.vibe.web_search import WebSearchTool
 from xagent.core.tools.adapters.vibe.workspace_file_tool import WorkspaceFileTools
 from xagent.core.workspace import TaskWorkspace
@@ -220,6 +226,17 @@ def test_document_parser_metadata_matches_artifact_safety() -> None:
     assert parse_with_output_tool.metadata.concurrency_safe is True
 
 
+def test_knowledge_base_ingestion_tools_remain_unsafe_until_real_tool_coverage() -> (
+    None
+):
+    file_ingestion_tool = CreateKnowledgeBaseFromFileTool(user_id=1)
+    url_ingestion_tool = CreateKnowledgeBaseFromUrlTool(user_id=1)
+
+    for tool in {file_ingestion_tool, url_ingestion_tool}:
+        assert tool.metadata.read_only is False
+        assert tool.metadata.concurrency_safe is False
+
+
 def test_abstract_base_tool_metadata_exposes_concurrency_fields() -> None:
     # The metadata object must always carry the fields (even when False) so the
     # scheduler can read them uniformly across every tool.
diff --git a/tests/core/tools/core/RAG_tools/kb/test_pipeline_compatibility.py b/tests/core/tools/core/RAG_tools/kb/test_pipeline_compatibility.py
index 254363b1c..74eee8f44 100644
--- a/tests/core/tools/core/RAG_tools/kb/test_pipeline_compatibility.py
+++ b/tests/core/tools/core/RAG_tools/kb/test_pipeline_compatibility.py
@@ -2,6 +2,9 @@
 
 from __future__ import annotations
 
+import asyncio
+import threading
+import time
 from datetime import datetime, timezone
 from pathlib import Path
 from typing import Any, Callable, Optional
@@ -335,6 +338,262 @@ def fail_if_called(*_: object, **__: object) -> None:
     assert result is expected_result
 
 
+def test_run_document_ingestion_serializes_same_collection_writes(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    from xagent.core.tools.core.RAG_tools.pipelines import document_ingestion
+
+    facade = KBPipelineCompatibilityFacade(
+        storage_shim=_FakeStorageShim(_FakeMetadataStore(CollectionInfo(name="demo"))),
+        operation_compatibility=KBOperationCompatibilityFacade(),
+    )
+    active_count = 0
+    max_active_count = 0
+    active_guard = threading.Lock()
+    first_entered = threading.Event()
+    release_first = threading.Event()
+    worker_errors: list[BaseException] = []
+
+    def fake_run_document_ingestion_impl(**_: object) -> IngestionResult:
+        nonlocal active_count, max_active_count
+
+        with active_guard:
+            active_count += 1
+            max_active_count = max(max_active_count, active_count)
+            is_first = not first_entered.is_set()
+            if is_first:
+                first_entered.set()
+
+        try:
+            if is_first:
+                assert release_first.wait(timeout=2)
+            return _successful_ingestion_result()
+        finally:
+            with active_guard:
+                active_count -= 1
+
+    monkeypatch.setattr(
+        document_ingestion,
+        "_run_document_ingestion_impl",
+        fake_run_document_ingestion_impl,
+    )
+
+    def run_ingestion(source_path: str) -> None:
+        try:
+            facade.run_document_ingestion("demo", source_path)
+        except BaseException as exc:  # noqa: BLE001
+            worker_errors.append(exc)
+
+    first = threading.Thread(target=lambda: run_ingestion("/tmp/first.md"))
+    second = threading.Thread(target=lambda: run_ingestion("/tmp/second.md"))
+
+    first.start()
+    assert first_entered.wait(timeout=2)
+    second.start()
+    time.sleep(0.05)
+    release_first.set()
+    first.join(timeout=2)
+    second.join(timeout=2)
+
+    assert not first.is_alive()
+    assert not second.is_alive()
+    assert worker_errors == []
+    assert max_active_count == 1
+
+
+def test_run_document_ingestion_allows_different_collections_to_overlap(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    from xagent.core.tools.core.RAG_tools.pipelines import document_ingestion
+
+    metadata_store = _FakeMetadataStore(None)
+    facade = KBPipelineCompatibilityFacade(
+        storage_shim=_FakeStorageShim(metadata_store),
+        operation_compatibility=KBOperationCompatibilityFacade(),
+    )
+    entered_barrier = threading.Barrier(2)
+    entered_collections: list[str] = []
+    entered_guard = threading.Lock()
+    worker_errors: list[BaseException] = []
+
+    def fake_run_document_ingestion_impl(
+        collection: str, **_: object
+    ) -> IngestionResult:
+        with entered_guard:
+            entered_collections.append(collection)
+        entered_barrier.wait(timeout=2)
+        return _successful_ingestion_result(doc_id=f"doc-{collection}")
+
+    monkeypatch.setattr(
+        document_ingestion,
+        "_run_document_ingestion_impl",
+        fake_run_document_ingestion_impl,
+    )
+
+    def run_ingestion(collection: str, source_path: str) -> None:
+        try:
+            facade.run_document_ingestion(collection, source_path)
+        except BaseException as exc:  # noqa: BLE001
+            worker_errors.append(exc)
+
+    first = threading.Thread(target=lambda: run_ingestion("first", "/tmp/first.md"))
+    second = threading.Thread(target=lambda: run_ingestion("second", "/tmp/second.md"))
+
+    first.start()
+    second.start()
+    first.join(timeout=2)
+    second.join(timeout=2)
+
+    assert not first.is_alive()
+    assert not second.is_alive()
+    assert worker_errors == []
+    assert sorted(entered_collections) == ["first", "second"]
+
+
+@pytest.mark.asyncio
+async def test_run_web_ingestion_serializes_same_collection_without_blocking_loop(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    from xagent.core.tools.core.RAG_tools.pipelines import web_ingestion
+
+    facade = KBPipelineCompatibilityFacade(
+        storage_shim=_FakeStorageShim(_FakeMetadataStore(CollectionInfo(name="demo"))),
+        operation_compatibility=KBOperationCompatibilityFacade(),
+    )
+    active_count = 0
+    max_active_count = 0
+    first_entered = asyncio.Event()
+    release_first = asyncio.Event()
+
+    async def fake_run_web_ingestion_impl(
+        collection: str,
+        crawl_config: WebCrawlConfig,
+        **_: object,
+    ) -> WebIngestionResult:
+        nonlocal active_count, max_active_count
+
+        active_count += 1
+        max_active_count = max(max_active_count, active_count)
+        is_first = not first_entered.is_set()
+        if is_first:
+            first_entered.set()
+
+        try:
+            if is_first:
+                await asyncio.wait_for(release_first.wait(), timeout=2)
+            return WebIngestionResult(
+                status="success",
+                collection=collection,
+                total_urls_found=1,
+                pages_crawled=1,
+                pages_failed=0,
+                documents_created=1,
+                chunks_created=1,
+                embeddings_created=1,
+                crawled_urls=[crawl_config.start_url],
+                failed_urls={},
+                message="ok",
+                elapsed_time_ms=1,
+            )
+        finally:
+            active_count -= 1
+
+    monkeypatch.setattr(
+        web_ingestion,
+        "_run_web_ingestion_impl",
+        fake_run_web_ingestion_impl,
+    )
+
+    first = asyncio.create_task(
+        facade.run_web_ingestion(
+            "demo",
+            WebCrawlConfig(start_url="https://example.com/first"),
+        )
+    )
+    await asyncio.wait_for(first_entered.wait(), timeout=2)
+    second = asyncio.create_task(
+        facade.run_web_ingestion(
+            "demo",
+            WebCrawlConfig(start_url="https://example.com/second"),
+        )
+    )
+    await asyncio.sleep(0.05)
+    assert not second.done()
+
+    release_first.set()
+    await asyncio.wait_for(asyncio.gather(first, second), timeout=2)
+
+    assert max_active_count == 1
+
+
+@pytest.mark.asyncio
+async def test_run_web_ingestion_allows_different_collections_to_overlap(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    from xagent.core.tools.core.RAG_tools.pipelines import web_ingestion
+
+    facade = KBPipelineCompatibilityFacade(
+        storage_shim=_FakeStorageShim(_FakeMetadataStore(None)),
+        operation_compatibility=KBOperationCompatibilityFacade(),
+    )
+    active_count = 0
+    max_active_count = 0
+    both_entered = asyncio.Event()
+
+    async def fake_run_web_ingestion_impl(
+        collection: str,
+        crawl_config: WebCrawlConfig,
+        **_: object,
+    ) -> WebIngestionResult:
+        nonlocal active_count, max_active_count
+
+        active_count += 1
+        max_active_count = max(max_active_count, active_count)
+        if active_count == 2:
+            both_entered.set()
+
+        try:
+            await asyncio.wait_for(both_entered.wait(), timeout=2)
+            return WebIngestionResult(
+                status="success",
+                collection=collection,
+                total_urls_found=1,
+                pages_crawled=1,
+                pages_failed=0,
+                documents_created=1,
+                chunks_created=1,
+                embeddings_created=1,
+                crawled_urls=[crawl_config.start_url],
+                failed_urls={},
+                message="ok",
+                elapsed_time_ms=1,
+            )
+        finally:
+            active_count -= 1
+
+    monkeypatch.setattr(
+        web_ingestion,
+        "_run_web_ingestion_impl",
+        fake_run_web_ingestion_impl,
+    )
+
+    await asyncio.wait_for(
+        asyncio.gather(
+            facade.run_web_ingestion(
+                "first",
+                WebCrawlConfig(start_url="https://example.com/first"),
+            ),
+            facade.run_web_ingestion(
+                "second",
+                WebCrawlConfig(start_url="https://example.com/second"),
+            ),
+        ),
+        timeout=2,
+    )
+
+    assert max_active_count == 2
+
+
 @pytest.mark.asyncio
 async def test_web_ingestion_records_page_child_outcomes_and_preserve_policy(
     monkeypatch: pytest.MonkeyPatch,