fix(tools): make construction-time store failures non-fatal where they can be

Copilot flagged that `model_post_init` calls `store.normalize` / `store.display`
unguarded, so a store raising there stops a serialized crew from loading —
against this PR's own claim that a broken integration degrades rather than
taking file I/O down.

It suggested falling back to the local store. Not doing that: silently
redirecting a crew configured for durable storage onto a disk that will be
discarded trades a loud failure for quiet data loss, which is the exact bug
this PR exists to fix.

Three changes instead.

The protocol now *states* the invariant it only implied. `normalize` and
`display` must be pure string computation — no I/O, no raising — because the
tools call them while a tool is being constructed. Both real stores already
comply (`CdoFileStore` does `posixpath` arithmetic and never touches its
client); this makes it a contract a new store is held to rather than a
coincidence.

The reader's declared-file derivation is now guarded, because a crew that
cannot load over a *convenience default filename* is indefensible. It comes
back without a default file and logs; any real problem resurfaces on the
first read, where `_run` already reports it.

`base_dir` anchoring stays unguarded, deliberately, and now says why:
it is a containment guarantee, not a convenience. Leaving the root relative
because a store hiccuped would let a later chdir move the sandbox — handing
back a weaker sandbox than the caller asked for, silently. That failure
should surface.

104 tests across the file tools. Two new, one per half of the asymmetry.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UNumDnNbiyw3pv1WakAe6t

lib/crewai-tools/src/crewai_tools/file_storage/base.py

@@ -53,6 +53,14 @@ class FileStore(Protocol):
         construction, so it can recognize that path again later even if the
         working directory has since moved. Unlike :meth:`resolve` it never
         rejects: a path outside the sandbox still has a canonical form.
+
+        Must be a pure computation on the string: no I/O, no network, and no
+        raising. The tools call it while a tool is being constructed —
+        including when pydantic rebuilds a serialized crew — so a store that
+        reaches its backing service here turns a transient outage into a crew
+        that cannot load at all. Defer anything that can fail to
+        :meth:`resolve`, :meth:`open_text` or :meth:`write_text`, where the
+        tools report failures instead of propagating them.
         """
 
     def resolve_within(self, directory: str, filename: str) -> str:
@@ -71,7 +79,9 @@ class FileStore(Protocol):
         """Return a label for *resolved* that is safe to show an LLM.
 
         Must not leak absolute directory prefixes; the tools put the result
-        straight into agent-visible output.
+        straight into agent-visible output. Pure and non-raising for the same
+        reason as :meth:`normalize` — the reader labels its declared file at
+        construction time.
         """
 
     def exists(self, resolved: str) -> bool:

lib/crewai-tools/src/crewai_tools/tools/file_read_tool/file_read_tool.py

@@ -1,4 +1,5 @@
 from itertools import islice
+import logging
 from typing import Any
 
 from crewai.tools import BaseTool
@@ -11,6 +12,9 @@ from crewai_tools.security.safe_path import (
 )
 
 
+logger = logging.getLogger(__name__)
+
+
 class FileReadToolSchema(BaseModel):
     """Input for FileReadTool."""
 
@@ -141,13 +145,36 @@ class FileReadTool(BaseTool):
 
         # Anchor base_dir once, so the sandbox root cannot move under a later
         # chdir while the declared file stays pinned to its original location.
+        # Deliberately not guarded: anchoring is a containment guarantee, and
+        # quietly leaving the root relative would let a later chdir move the
+        # sandbox. A store that cannot normalize should fail loudly here rather
+        # than hand back a weaker sandbox than the caller asked for.
         if self.base_dir is not None:
             self.base_dir = store.normalize(self.base_dir)
 
         if self.file_path is not None:
-            self._declared_realpath = store.normalize(self.file_path, self.base_dir)
-            self._declared_label = store.display(self._declared_realpath, self.base_dir)
-            self.description = f"A tool that reads file content. The default file is {self._declared_label}, which is read when 'file_path' is omitted. You can also provide a different 'file_path' parameter to read another file, though reads are confined to the tool's allowed directory and a path that resolves outside it is rejected. Specify 'start_line' and 'line_count' to read specific parts of the file."
+            # Guarded, unlike base_dir above: the declared default is a
+            # convenience, so a store hiccuping while canonicalizing a filename
+            # should not stop a serialized crew from loading. The reader simply
+            # comes back without a default file, and any real problem resurfaces
+            # on the first read, where _run reports it instead of raising.
+            try:
+                self._declared_realpath = store.normalize(self.file_path, self.base_dir)
+                self._declared_label = store.display(
+                    self._declared_realpath, self.base_dir
+                )
+            except (FileStoreError, OSError):
+                logger.warning(
+                    "the %s store could not resolve the declared file %r; the "
+                    "tool will have no default file",
+                    getattr(store, "label", "configured"),
+                    self.file_path,
+                    exc_info=True,
+                )
+                self._declared_realpath = None
+                self._declared_label = None
+            else:
+                self.description = f"A tool that reads file content. The default file is {self._declared_label}, which is read when 'file_path' is omitted. You can also provide a different 'file_path' parameter to read another file, though reads are confined to the tool's allowed directory and a path that resolves outside it is rejected. Specify 'start_line' and 'line_count' to read specific parts of the file."
 
     def _resolve_path(self, file_path: str) -> str:
         """Resolve *file_path* and confirm the tool is allowed to read it.

lib/crewai-tools/tests/file_storage/test_file_store_seam.py

@@ -404,6 +404,55 @@ def test_reader_round_trips_through_model_dump(store, tmp_path, monkeypatch):
     assert rebuilt._run() == "dumped\n"
 
 
+# --- a store that fails while a tool is being built --------------------------
+#
+# `normalize` and `display` are specified as pure and non-raising, precisely
+# because the tools call them during construction — including when pydantic
+# rebuilds a serialized crew. A store that breaks that contract anyway must not
+# take a whole crew down with it, but the two derivations are not equal: the
+# declared default file is a convenience, `base_dir` is a containment
+# guarantee. So one degrades and one does not.
+
+
+def test_a_store_failing_on_the_declared_file_still_builds_the_tool(
+    failing_store, tmp_path, monkeypatch, caplog
+):
+    """Convenience: no default file rather than no crew."""
+    monkeypatch.chdir(tmp_path)
+    store = failing_store("display")
+
+    tool = FileReadTool(file_path="notes.txt")
+
+    assert tool._declared_realpath is None
+    assert tool._declared_label is None
+    assert "no default file" in caplog.text
+    # Still a working tool — explicit paths go through the store as usual.
+    store.failing = ""
+    store.files["/ws/other.txt"] = "still working\n"
+    assert tool._run(file_path="other.txt") == "still working\n"
+    # And omitting the path reports the missing default rather than raising.
+    assert "No file path provided" in tool._run()
+
+
+def test_a_store_failing_on_base_dir_is_not_swallowed(
+    failing_store, tmp_path, monkeypatch
+):
+    """Containment: a sandbox root that cannot be anchored must not be faked.
+
+    Degrading here would leave `base_dir` relative, so a later chdir could move
+    the sandbox — a weaker guarantee than the caller asked for, arrived at
+    silently. Failing loudly is the point.
+    """
+    monkeypatch.chdir(tmp_path)
+    failing_store("normalize")
+
+    with pytest.raises((FileStoreError, OSError)):
+        FileReadTool(base_dir="scoped")
+
+    with pytest.raises((FileStoreError, OSError)):
+        FileWriterTool(base_dir="scoped")
+
+
 # --- what the declared-path pin means across a rebuild -----------------------
 #
 # The pin is derived from what was *declared*, so whether it survives a rebuild