fix(tools): bind the reader's store in model_post_init, report store failures

Two review findings, both only reachable once a non-local store is
registered — which is why the local-filesystem tests could not see them.

The reader bound `_store` in `__init__` only. `BaseTool._resolve_tool_dict`
rebuilds a serialized tool with `model_validate`, which skips `__init__`
entirely, so a reconstructed reader came back with `_store` still None and
raised AttributeError on the first read — where before this branch it would
have called `validate_file_path`/`open` directly and worked. Binding moved
to `model_post_init`, which pydantic runs on both paths, and the declared
path, its label and the generated description are derived there too so a
rebuilt reader is indistinguishable from a fresh one. The writer already
did this correctly.

Every store call was guarded for `ValueError` alone, but the protocol
explicitly sanctions `FileStoreError` for failures the local filesystem
cannot have. Such a failure escaped `_run` and aborted the agent's step
instead of returning the error string the tools otherwise always return.
Both tools now wrap the whole operation, which also covers `exists()` and
the store's own `display()` — neither of which had any handler. The
established messages stay on their specific boundaries.

A bare `OSError("...")` still degrades to its type, because
`format_error_for_display` only passes `strerror` through: an OS-populated
OSError renders its absolute filename into `str()`, and #6692 deliberately
closed that. Stores wanting a legible message should raise `FileStoreError`.
Left that helper alone rather than widen a redaction from here.

12 new tests: reconstruction through both `model_validate` and a full
`model_dump` round-trip, a store failing at each of resolve/resolve_within/
display/exists/ensure_parent/open_text/write_text, and the redaction holding
on the new path.

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/tools/file_read_tool/file_read_tool.py

@@ -4,7 +4,7 @@ from typing import Any
 from crewai.tools import BaseTool
 from pydantic import BaseModel, Field, PrivateAttr
 
-from crewai_tools.file_storage import FileStore, resolve_file_store
+from crewai_tools.file_storage import FileStore, FileStoreError, resolve_file_store
 from crewai_tools.security.safe_path import (
     format_error_for_display,
     format_sandbox_error,
@@ -102,29 +102,43 @@ class FileReadTool(BaseTool):
             encoding (str): Text encoding used to decode the file.
             **kwargs: Additional keyword arguments passed to BaseTool.
         """
+        # Hand these to pydantic rather than assigning them afterwards, so the
+        # fields are already populated by the time model_post_init derives
+        # everything that depends on them. That hook is the only place the
+        # derivation can live: see its docstring.
+        if file_path is not None:
+            kwargs["file_path"] = file_path
+        if base_dir is not None:
+            kwargs["base_dir"] = base_dir
+        kwargs["encoding"] = encoding
+
+        super().__init__(**kwargs)
+
+    def model_post_init(self, context: object) -> None:
+        """Bind the store, then derive everything that depends on it.
+
+        This has to happen here rather than in ``__init__`` because pydantic
+        reconstructs a serialized tool through ``model_validate``, which skips
+        ``__init__`` entirely — ``BaseTool._resolve_tool_dict`` does exactly
+        that when a crew carrying this tool is rebuilt. Deriving here means a
+        reconstructed reader binds its store and pins its declared path just
+        like a freshly constructed one, instead of coming back with no store
+        and failing on the first read.
+        """
+        super().model_post_init(context)
+
         store = resolve_file_store()
+        self._store = store
 
         # Anchor base_dir once, so the sandbox root cannot move under a later
         # chdir while the declared file stays pinned to its original location.
-        if base_dir is not None:
-            base_dir = store.normalize(base_dir)
+        if self.base_dir is not None:
+            self.base_dir = store.normalize(self.base_dir)
 
-        display_path = None
-        if file_path is not None:
-            display_path = store.display(store.normalize(file_path, base_dir), base_dir)
-            kwargs["description"] = (
-                f"A tool that reads file content. The default file is {display_path}, 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."
-            )
-
-        super().__init__(**kwargs)
-        self.file_path = file_path
-        self.base_dir = base_dir
-        self.encoding = encoding
-        self._store = store
-        self._declared_realpath = (
-            store.normalize(file_path, base_dir) if file_path is not None else None
-        )
-        self._declared_label = display_path
+        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."
 
     def _resolve_path(self, file_path: str) -> str:
         """Resolve *file_path* and confirm the tool is allowed to read it.
@@ -160,6 +174,27 @@ class FileReadTool(BaseTool):
         line_count: int | None = None,
     ) -> str:
         """Read a file, or a window of its lines, as text."""
+        try:
+            return self._read(file_path, start_line, line_count)
+        except (FileStoreError, OSError) as e:
+            # A store can fail for reasons the local filesystem never had: an
+            # unreachable endpoint, a rejected request. Every other exit from
+            # this tool is an agent-visible string, and raising here would kill
+            # the agent's step rather than let it react, so this one is too.
+            # Path resolution and the store's own path labelling both live
+            # under here, which is why the message carries no path.
+            return (
+                f"Error: the {self._store.label} store failed. "
+                f"{format_error_for_display(e)}"
+            )
+
+    def _read(
+        self,
+        file_path: str | None,
+        start_line: int | None,
+        line_count: int | None,
+    ) -> str:
+        """Do the read. Wrapped by :meth:`_run`, which reports store failures."""
         start_line = start_line or 1
         line_count = line_count or None
 

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

@@ -1,7 +1,7 @@
 from crewai.tools import BaseTool
 from pydantic import BaseModel, Field, PrivateAttr
 
-from crewai_tools.file_storage import FileStore, resolve_file_store
+from crewai_tools.file_storage import FileStore, FileStoreError, resolve_file_store
 from crewai_tools.security.safe_path import (
     format_error_for_display,
     format_sandbox_error,
@@ -111,6 +111,28 @@ class FileWriterTool(BaseTool):
         overwrite: str | bool = False,
     ) -> str:
         """Write *content* to *filename*, confined to the tool's sandbox."""
+        try:
+            return self._write(filename, content, directory, overwrite)
+        except (FileStoreError, OSError) as e:
+            # A store can fail for reasons the local filesystem never had: an
+            # unreachable endpoint, a size ceiling. Every other exit from this
+            # tool is an agent-visible string, and raising here would kill the
+            # agent's step rather than let it react, so this one is too. It
+            # also covers the calls with no handler of their own — exists() and
+            # the store's own path labelling — which is why it carries no path.
+            return (
+                f"An error occurred while writing to the file: the "
+                f"{self._store.label} store failed. {format_error_for_display(e)}"
+            )
+
+    def _write(
+        self,
+        filename: str,
+        content: str,
+        directory: str | None,
+        overwrite: str | bool,
+    ) -> str:
+        """Do the write. Wrapped by :meth:`_run`, which reports store failures."""
         directory = directory or "./"
 
         try:

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

@@ -14,6 +14,7 @@ import posixpath
 from crewai_tools import FileReadTool, FileWriterTool
 from crewai_tools.file_storage import (
     FileStore,
+    FileStoreError,
     register_file_store_factory,
     reset_file_store_factory,
     resolve_file_store,
@@ -91,6 +92,58 @@ class MemoryFileStore:
         self.files[resolved] = content
 
 
+class FailingFileStore(MemoryFileStore):
+    """A store where one named operation fails, the way a remote one can.
+
+    The local filesystem cannot produce most of these — an unreachable
+    endpoint has no local equivalent — so a store that only ever fails the
+    way `open()` does would not exercise the tools' error paths at all.
+    """
+
+    def __init__(self, failing: str, exc: Exception | None = None) -> None:
+        super().__init__()
+        self.failing = failing
+        self.exc = exc or FileStoreError("workspace endpoint unreachable")
+
+    def _maybe_fail(self, name: str) -> None:
+        if name == self.failing:
+            raise self.exc
+
+    def normalize(self, path: str, base_dir: str | None = None) -> str:
+        self._maybe_fail("normalize")
+        return super().normalize(path, base_dir)
+
+    def resolve(self, path: str, base_dir: str | None = None) -> str:
+        self._maybe_fail("resolve")
+        return super().resolve(path, base_dir)
+
+    def resolve_within(self, directory: str, filename: str) -> str:
+        self._maybe_fail("resolve_within")
+        return super().resolve_within(directory, filename)
+
+    def display(self, resolved: str, base: str | None = None) -> str:
+        self._maybe_fail("display")
+        return super().display(resolved, base)
+
+    def exists(self, resolved: str) -> bool:
+        self._maybe_fail("exists")
+        return super().exists(resolved)
+
+    def ensure_parent(self, resolved: str) -> None:
+        self._maybe_fail("ensure_parent")
+        super().ensure_parent(resolved)
+
+    def open_text(self, resolved: str, encoding: str):
+        self._maybe_fail("open_text")
+        return super().open_text(resolved, encoding)
+
+    def write_text(
+        self, resolved: str, content: str, encoding: str, *, overwrite: bool
+    ) -> None:
+        self._maybe_fail("write_text")
+        super().write_text(resolved, content, encoding, overwrite=overwrite)
+
+
 @pytest.fixture
 def store():
     memory = MemoryFileStore()
@@ -99,6 +152,21 @@ def store():
     reset_file_store_factory()
 
 
+@pytest.fixture
+def failing_store():
+    """Register a store whose *named* operation fails; the test picks which."""
+    created: list[FailingFileStore] = []
+
+    def _register(failing: str, exc: Exception | None = None) -> FailingFileStore:
+        broken = FailingFileStore(failing, exc)
+        register_file_store_factory(lambda: broken)
+        created.append(broken)
+        return broken
+
+    yield _register
+    reset_file_store_factory()
+
+
 def test_default_store_is_local():
     assert isinstance(resolve_file_store(), LocalFileStore)
 
@@ -277,3 +345,178 @@ def test_os_error_message_does_not_leak_an_absolute_path(tmp_path, monkeypatch):
 
     assert "Error" in result
     assert str(tmp_path) not in result
+
+
+# --- reconstruction through pydantic -----------------------------------------
+#
+# BaseTool rebuilds a serialized tool with `model_validate` (see
+# `_resolve_tool_dict`), which skips `__init__` entirely. Anything derived only
+# in `__init__` comes back missing, and for the store that means every read
+# raising AttributeError on a None.
+
+
+def test_reconstructed_reader_reads_through_the_store(store, tmp_path, monkeypatch):
+    monkeypatch.chdir(tmp_path)
+    store.files["/ws/notes.txt"] = "reconstructed\n"
+
+    tool = FileReadTool.model_validate({"file_path": "notes.txt"})
+
+    assert tool._store is store
+    assert tool._run() == "reconstructed\n"
+
+
+def test_reconstructed_writer_writes_through_the_store(store, tmp_path, monkeypatch):
+    monkeypatch.chdir(tmp_path)
+
+    tool = FileWriterTool.model_validate({"base_dir": "scoped"})
+
+    assert tool._store is store
+    assert tool.base_dir == "/ws/scoped"
+    assert "successfully written" in tool._run(
+        filename="a.txt", content="x", overwrite=True
+    )
+    assert store.files == {"/ws/scoped/a.txt": "x"}
+
+
+def test_reconstructed_reader_keeps_its_declared_file(store, tmp_path, monkeypatch):
+    """The declared path is pinned in the hook, so a rebuild pins it too."""
+    monkeypatch.chdir(tmp_path)
+    store.files["/ws/sub/pinned.txt"] = "kept\n"
+
+    fresh = FileReadTool(file_path="sub/pinned.txt")
+    rebuilt = FileReadTool.model_validate({"file_path": "sub/pinned.txt"})
+
+    assert rebuilt._declared_realpath == fresh._declared_realpath == "/ws/sub/pinned.txt"
+    assert rebuilt._declared_label == fresh._declared_label
+    assert rebuilt.description == fresh.description
+    # Addressable by the label the description shows, same as a fresh tool.
+    assert rebuilt._run(file_path=rebuilt._declared_label) == "kept\n"
+
+
+def test_reader_round_trips_through_model_dump(store, tmp_path, monkeypatch):
+    """The full serialize/deserialize cycle, not just a hand-built dict."""
+    monkeypatch.chdir(tmp_path)
+    store.files["/ws/notes.txt"] = "dumped\n"
+
+    original = FileReadTool(file_path="notes.txt")
+    rebuilt = FileReadTool.model_validate(original.model_dump())
+
+    assert rebuilt._run() == "dumped\n"
+
+
+# --- a store that fails ------------------------------------------------------
+#
+# A store may fail where the local filesystem never could. Every exit from
+# these tools is an agent-visible string, so a store failure has to become one
+# too rather than raising into the agent's step.
+
+
+@pytest.mark.parametrize(
+    "failing", ["resolve", "resolve_within", "display", "exists", "ensure_parent"]
+)
+def test_writer_reports_a_store_failure_instead_of_raising(
+    failing, failing_store, tmp_path, monkeypatch
+):
+    monkeypatch.chdir(tmp_path)
+    failing_store(failing)
+
+    result = FileWriterTool()._run(filename="a.txt", content="x", overwrite=True)
+
+    assert "error occurred while writing" in result
+    assert "memory" in result
+    assert "workspace endpoint unreachable" in result
+
+
+def test_writer_reports_a_failing_write(failing_store, tmp_path, monkeypatch):
+    """write_text already had a handler; it must keep its own wording."""
+    monkeypatch.chdir(tmp_path)
+    failing_store("write_text")
+
+    result = FileWriterTool()._run(filename="a.txt", content="x", overwrite=True)
+
+    assert "An error occurred while writing to the file" in result
+    assert "workspace endpoint unreachable" in result
+
+
+@pytest.mark.parametrize("failing", ["resolve", "display"])
+def test_reader_reports_a_store_failure_instead_of_raising(
+    failing, failing_store, tmp_path, monkeypatch
+):
+    monkeypatch.chdir(tmp_path)
+    failing_store(failing)
+
+    result = FileReadTool()._run(file_path="notes.txt")
+
+    assert "Error" in result
+    assert "memory" in result
+    assert "workspace endpoint unreachable" in result
+
+
+def test_reader_reports_a_failing_open(failing_store, tmp_path, monkeypatch):
+    """open_text already had a handler; it must keep its own wording."""
+    monkeypatch.chdir(tmp_path)
+    failing_store("open_text")
+
+    result = FileReadTool()._run(file_path="notes.txt")
+
+    assert "Failed to read file" in result
+    assert "workspace endpoint unreachable" in result
+
+
+def _run_either(tool_cls, tool):
+    """Drive whichever tool this is with its minimal arguments."""
+    if tool_cls is FileReadTool:
+        return tool._run(file_path="notes.txt")
+    return tool._run(filename="a.txt", content="x", overwrite=True)
+
+
+@pytest.mark.parametrize("tool_cls", [FileReadTool, FileWriterTool])
+def test_an_os_error_from_the_store_is_reported_too(
+    tool_cls, failing_store, tmp_path, monkeypatch
+):
+    """Not every store failure arrives as FileStoreError.
+
+    A store wrapping a socket or a subprocess can raise OSError from resolve,
+    which is neither a ValueError nor something the read/write handlers see.
+    ``strerror`` is what survives redaction, so that is what an agent gets.
+    """
+    monkeypatch.chdir(tmp_path)
+    failing_store("resolve", OSError(104, "Connection reset by peer"))
+
+    result = _run_either(tool_cls, tool_cls())
+
+    assert "Connection reset by peer" in result
+
+
+@pytest.mark.parametrize("tool_cls", [FileReadTool, FileWriterTool])
+def test_a_bare_os_error_degrades_to_its_type_without_raising(
+    tool_cls, failing_store, tmp_path, monkeypatch
+):
+    """A single-arg OSError has no strerror, so only its type survives.
+
+    That is `format_error_for_display` holding the line it was given in #6692:
+    an OS-populated OSError renders its absolute filename into `str()`, so the
+    message is never passed through wholesale. The cost is a thin report for a
+    hand-raised `OSError("...")`; stores wanting a legible message should raise
+    `FileStoreError`, whose text is preserved. What matters here is that the
+    tool still returns rather than raising.
+    """
+    monkeypatch.chdir(tmp_path)
+    failing_store("resolve", OSError("connection reset by peer"))
+
+    result = _run_either(tool_cls, tool_cls())
+
+    assert "OSError" in result
+    assert "store failed" in result
+
+
+def test_a_store_failure_does_not_leak_an_absolute_path(
+    failing_store, tmp_path, monkeypatch
+):
+    """The store-failure path is agent-visible, so it gets the same redaction."""
+    monkeypatch.chdir(tmp_path)
+    failing_store("resolve", OSError(2, "No such file", str(tmp_path / "secret.txt")))
+
+    result = FileWriterTool()._run(filename="a.txt", content="x", overwrite=True)
+
+    assert str(tmp_path) not in result