🪲 BUG-#21: Fix workspace jail global mutable state

pycodeloop/core/config.py

@@ -16,8 +16,7 @@
     render_skills_index,
 )
 from pycodeloop.store.sqlite_sessions import SqliteSessions
-from pycodeloop.tools import DEFAULT_TOOLS, READ_ONLY_TOOLS, DelegateTool
-from pycodeloop.tools._workspace import set_workspace_enabled
+from pycodeloop.tools import DelegateTool, build_tools
 
 
 def _default_provider() -> Provider:
@@ -141,21 +140,23 @@ def __init__(
         self.provider = (
             provider if provider is not None else _default_provider()
         )
-        self.tools = list(tools) if tools is not None else list(DEFAULT_TOOLS)
+        self.workspace = workspace
+        if tools is not None:
+            self.tools = list(tools)
+        else:
+            default_tools, _ = build_tools(workspace)
+            self.tools = default_tools
         self.system_prompt = system_prompt
         self.max_turns = max_turns
         self.max_history_turns = max_history_turns
-        self.workspace = workspace
-        set_workspace_enabled(workspace)
         self.skills = self._discover_skills(
             skills, skill_sources, skills_refresh
         )
         if delegation:
+            _, read_only_tools = build_tools(workspace)
             self.tools = [
                 *self.tools,
-                DelegateTool(
-                    provider=self.provider, tools=list(READ_ONLY_TOOLS)
-                ),
+                DelegateTool(provider=self.provider, tools=read_only_tools),
             ]
         if memory:
             self._load_memory()

pycodeloop/tools/__init__.py

@@ -18,49 +18,66 @@
 from .sql import SqlQueryTool, SqlSchemaTool
 from .web import WebFetchTool
 
-_read_file = ReadFileTool()
-_list_dir = ListDirTool()
-_glob = GlobTool()
-_grep = GrepTool()
-_web_fetch = WebFetchTool()
-_git_status = GitStatusTool()
-_git_diff = GitDiffTool()
-_git_log = GitLogTool()
-_sql_schema = SqlSchemaTool()
-_sql_query = SqlQueryTool()
 
-DEFAULT_TOOLS: list[Tool] = [
-    _read_file,
-    WriteFileTool(),
-    EditFileTool(),
-    DeleteFileTool(),
-    _list_dir,
-    _glob,
-    _grep,
-    BashTool(),
-    _web_fetch,
-    HttpRequestTool(),
-    _git_status,
-    _git_diff,
-    _git_log,
-    GitCommitTool(),
-    EnvTool(),
-    _sql_schema,
-    _sql_query,
-]
+def build_tools(workspace: bool = True) -> tuple[list[Tool], list[Tool]]:
+    """Fresh `(default_tools, read_only_tools)` for one `Config` — never
+    shared across `Config`/`Agent` instances, so each one's `workspace`
+    jail setting stays scoped to itself instead of racing through a
+    process-wide global. Tools common to both lists (read_file, list_dir,
+    glob, grep, web_fetch, the read-only git/sql tools) are still
+    instantiated once per call and shared between the two returned
+    lists, matching the previous single-instantiation behavior — just
+    scoped per call instead of per process."""
+    read_file = ReadFileTool(workspace=workspace)
+    list_dir = ListDirTool(workspace=workspace)
+    glob_tool = GlobTool(workspace=workspace)
+    grep = GrepTool(workspace=workspace)
+    web_fetch = WebFetchTool()
+    git_status = GitStatusTool()
+    git_diff = GitDiffTool()
+    git_log = GitLogTool()
+    sql_schema = SqlSchemaTool()
+    sql_query = SqlQueryTool()
 
-READ_ONLY_TOOLS: list[Tool] = [
-    _read_file,
-    _list_dir,
-    _glob,
-    _grep,
-    _web_fetch,
-    _git_status,
-    _git_diff,
-    _git_log,
-    _sql_schema,
-    _sql_query,
-]
+    default_tools: list[Tool] = [
+        read_file,
+        WriteFileTool(workspace=workspace),
+        EditFileTool(workspace=workspace),
+        DeleteFileTool(workspace=workspace),
+        list_dir,
+        glob_tool,
+        grep,
+        BashTool(),
+        web_fetch,
+        HttpRequestTool(),
+        git_status,
+        git_diff,
+        git_log,
+        GitCommitTool(),
+        EnvTool(),
+        sql_schema,
+        sql_query,
+    ]
+
+    read_only_tools: list[Tool] = [
+        read_file,
+        list_dir,
+        glob_tool,
+        grep,
+        web_fetch,
+        git_status,
+        git_diff,
+        git_log,
+        sql_schema,
+        sql_query,
+    ]
+
+    return default_tools, read_only_tools
+
+
+DEFAULT_TOOLS: list[Tool]
+READ_ONLY_TOOLS: list[Tool]
+DEFAULT_TOOLS, READ_ONLY_TOOLS = build_tools()
 
 __all__ = [
     "Tool",
@@ -85,4 +102,5 @@
     "WebFetchTool",
     "DEFAULT_TOOLS",
     "READ_ONLY_TOOLS",
+    "build_tools",
 ]

pycodeloop/tools/_workspace.py

@@ -13,8 +13,6 @@
 
 from pathlib import Path
 
-_enabled = True
-
 
 class OutsideWorkspaceError(ValueError):
     def __init__(self, path: str, root: Path) -> None:
@@ -25,39 +23,32 @@ def __init__(self, path: str, root: Path) -> None:
         )
 
 
-def set_workspace_enabled(enabled: bool) -> None:
-    """Toggle the jail process-wide. Callers doing this mid-run (rather
-    than once at startup via `Config(workspace=...)`) should know it
-    affects every tool call from that point on, not just their own."""
-    global _enabled
-    _enabled = enabled
-
-
-def is_workspace_enabled() -> bool:
-    return _enabled
-
-
 def workspace_root() -> Path:
     return Path.cwd().resolve()
 
 
-def resolve_in_workspace(path: str, root: Path | None = None) -> Path:
+def resolve_in_workspace(
+    path: str, root: Path | None = None, enabled: bool = True
+) -> Path:
     """Resolve `path` under `root` (default: cwd). Raises
     `OutsideWorkspaceError` if the resolved path escapes the root via
-    `..` or an absolute path outside it — unless the jail was disabled
-    via `set_workspace_enabled(False)`, in which case `path` resolves
-    as-is with no restriction."""
+    `..` or an absolute path outside it — unless `enabled` is False, in
+    which case `path` resolves as-is with no restriction.
+
+    `enabled` is a plain parameter, not process-wide state — each tool
+    instance decides for itself so two `Config`s with different
+    `workspace=` settings (or tests running in the same process) can't
+    interfere with each other."""
     target = Path(path).expanduser()
+    base = (root or workspace_root()).resolve()
 
-    if not _enabled:
-        base = (root or workspace_root()).resolve()
+    if not enabled:
         return (
             target.resolve()
             if target.is_absolute()
             else (base / target).resolve()
         )
 
-    base = (root or workspace_root()).resolve()
     if not target.is_absolute():
         target = base / target
     resolved = target.resolve()

pycodeloop/tools/filesystem.py

@@ -33,9 +33,9 @@ def _looks_like_diff(text: str) -> bool:
     return bool(_HUNK_HEADER.search(text) or _DIFF_PREAMBLE.search(text))
 
 
-def _resolve_path(path: str) -> Path | ToolResult:
+def _resolve_path(path: str, *, workspace: bool = True) -> Path | ToolResult:
     try:
-        return resolve_in_workspace(path)
+        return resolve_in_workspace(path, enabled=workspace)
     except OutsideWorkspaceError as exc:
         return ToolResult(output=str(exc), is_error=True)
 
@@ -68,8 +68,13 @@ class ReadFileTool(Tool):
         "required": ["path"],
     }
 
-    def __init__(self, access_log: FileAccessLog | None = None) -> None:
+    def __init__(
+        self,
+        access_log: FileAccessLog | None = None,
+        workspace: bool = True,
+    ) -> None:
         self._log = access_log or default_log
+        self._workspace = workspace
 
     def run(
         self,
@@ -78,7 +83,7 @@ def run(
         limit: int | None = None,
         force: bool = False,
     ) -> ToolResult:
-        resolved = _resolve_path(path)
+        resolved = _resolve_path(path, workspace=self._workspace)
         if isinstance(resolved, ToolResult):
             return resolved
         target = resolved
@@ -141,8 +146,13 @@ class WriteFileTool(Tool):
     }
     dangerous = True
 
-    def __init__(self, access_log: FileAccessLog | None = None) -> None:
+    def __init__(
+        self,
+        access_log: FileAccessLog | None = None,
+        workspace: bool = True,
+    ) -> None:
         self._log = access_log or default_log
+        self._workspace = workspace
 
     def preview(self, path: str, content: str, **_) -> str:
         if _looks_like_diff(content):
@@ -153,7 +163,7 @@ def preview(self, path: str, content: str, **_) -> str:
             )
 
         try:
-            target = resolve_in_workspace(path)
+            target = resolve_in_workspace(path, enabled=self._workspace)
         except OutsideWorkspaceError as exc:
             return str(exc)
 
@@ -174,7 +184,7 @@ def run(self, path: str, content: str) -> ToolResult:
                 is_error=True,
             )
 
-        resolved = _resolve_path(path)
+        resolved = _resolve_path(path, workspace=self._workspace)
         if isinstance(resolved, ToolResult):
             return resolved
         target = resolved
@@ -212,8 +222,13 @@ class EditFileTool(Tool):
     }
     dangerous = True
 
-    def __init__(self, access_log: FileAccessLog | None = None) -> None:
+    def __init__(
+        self,
+        access_log: FileAccessLog | None = None,
+        workspace: bool = True,
+    ) -> None:
         self._log = access_log or default_log
+        self._workspace = workspace
 
     def _apply(
         self, path: str, old_string: str, new_string: str, replace_all: bool
@@ -229,7 +244,7 @@ def _apply(
                 is_error=True,
             )
 
-        resolved = _resolve_path(path)
+        resolved = _resolve_path(path, workspace=self._workspace)
         if isinstance(resolved, ToolResult):
             return resolved
         target = resolved
@@ -318,12 +333,17 @@ class DeleteFileTool(Tool):
     }
     dangerous = True
 
-    def __init__(self, access_log: FileAccessLog | None = None) -> None:
+    def __init__(
+        self,
+        access_log: FileAccessLog | None = None,
+        workspace: bool = True,
+    ) -> None:
         self._log = access_log or default_log
+        self._workspace = workspace
 
     def preview(self, path: str, **_) -> str:
         try:
-            target = resolve_in_workspace(path)
+            target = resolve_in_workspace(path, enabled=self._workspace)
         except OutsideWorkspaceError as exc:
             return str(exc)
 
@@ -334,7 +354,7 @@ def preview(self, path: str, **_) -> str:
         return _diff(path, before, "")
 
     def run(self, path: str) -> ToolResult:
-        resolved = _resolve_path(path)
+        resolved = _resolve_path(path, workspace=self._workspace)
         if isinstance(resolved, ToolResult):
             return resolved
         target = resolved
@@ -359,8 +379,11 @@ class ListDirTool(Tool):
         "properties": {"path": {"type": "string", "default": "."}},
     }
 
+    def __init__(self, workspace: bool = True) -> None:
+        self._workspace = workspace
+
     def run(self, path: str = ".") -> ToolResult:
-        resolved = _resolve_path(path)
+        resolved = _resolve_path(path, workspace=self._workspace)
         if isinstance(resolved, ToolResult):
             return resolved
         target = resolved

pycodeloop/tools/search.py

@@ -44,11 +44,14 @@ class GrepTool(Tool):
         "required": ["pattern"],
     }
 
+    def __init__(self, workspace: bool = True) -> None:
+        self._workspace = workspace
+
     def run(
         self, pattern: str, path: str = ".", max_results: int = 100
     ) -> ToolResult:
         try:
-            root = resolve_in_workspace(path)
+            root = resolve_in_workspace(path, enabled=self._workspace)
         except OutsideWorkspaceError as exc:
             return ToolResult(output=str(exc), is_error=True)
 
@@ -91,11 +94,14 @@ class GlobTool(Tool):
         "required": ["pattern"],
     }
 
+    def __init__(self, workspace: bool = True) -> None:
+        self._workspace = workspace
+
     def run(
         self, pattern: str, path: str = ".", max_results: int = 100
     ) -> ToolResult:
         try:
-            root = resolve_in_workspace(path)
+            root = resolve_in_workspace(path, enabled=self._workspace)
         except OutsideWorkspaceError as exc:
             return ToolResult(output=str(exc), is_error=True)