Skip to content

Commit 916f823

Browse files
joaomdmouraclaude
andcommitted
docs(tools): state what the declared-path pin means across a rebuild
Bugbot flagged that `model_post_init` re-derives `_declared_realpath` from the serialized `file_path`, so a rebuild in a different working directory can repoint the declared default. The mechanism is real. Traced it to exactly one case of three: absolute file_path -> survives a rebuild anywhere relative file_path + base_dir -> survives; base_dir is anchored already relative file_path, no base_dir -> re-anchors to the rebuilding cwd Keeping the re-anchor, deliberately. A bare relative path names nothing absolute to preserve, and the alternative is pinning a directory that, for a rebuild in a fresh container, no longer exists — reading a stale absolute path would be the worse failure. It is also not a regression in any case: before this branch a rebuilt reader lost the declared file outright and answered "No file path provided". Rewriting `file_path` to its resolved form at construction would close it, but `tests/agents/test_agent.py:2311` pins that the authored string survives, so that is a public-contract change rather than a fix. A serialized pin field would too, at the cost of a schema change — noted on the thread for whoever reviews, not taken unilaterally. So: all three cases now have a test, and the class docstring says which is which, so the behavior is a decision rather than something a reader has to infer. 38 tests in the seam suite, 338 across the file tools and crewai's tool suite. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UNumDnNbiyw3pv1WakAe6t
1 parent a2f6bd0 commit 916f823

2 files changed

Lines changed: 73 additions & 0 deletions

File tree

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,15 @@ class FileReadTool(BaseTool):
5050
construction, so a later chdir cannot repoint it, and it can be addressed
5151
either by omitting ``file_path`` or by the label shown in the description.
5252
53+
That pin is anchored from what was declared, which matters when a tool is
54+
rebuilt from a serialized crew in a different working directory. An
55+
absolute ``file_path``, or a relative one with ``base_dir`` set, names the
56+
same file after the rebuild as before it. A *relative* ``file_path`` with
57+
no ``base_dir`` names nothing absolute, so it re-anchors to the working
58+
directory of the process doing the rebuilding — the same file the same
59+
arguments would have named there. Pass ``base_dir`` when a declared
60+
relative path must survive a move.
61+
5362
Args:
5463
file_path (Optional[str]): Path to the file to be read. If provided,
5564
this becomes the default file path for the tool.

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

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,70 @@ def test_reader_round_trips_through_model_dump(store, tmp_path, monkeypatch):
404404
assert rebuilt._run() == "dumped\n"
405405

406406

407+
# --- what the declared-path pin means across a rebuild -----------------------
408+
#
409+
# The pin is derived from what was *declared*, so whether it survives a rebuild
410+
# in a different working directory depends on whether the declaration named
411+
# somewhere absolute. These three cases are the whole story; they are pinned
412+
# here so the behavior is a decision rather than an accident. Note the local
413+
# store is the one that makes this observable, since its `normalize` is the one
414+
# that consults the process cwd.
415+
416+
417+
def test_an_absolute_declared_path_survives_a_rebuild_elsewhere(tmp_path, monkeypatch):
418+
"""The strongest case: an absolute declaration is cwd-independent."""
419+
here, there = tmp_path / "here", tmp_path / "there"
420+
here.mkdir(), there.mkdir()
421+
(here / "notes.txt").write_text("from here\n")
422+
(there / "notes.txt").write_text("from there\n")
423+
424+
monkeypatch.chdir(here)
425+
dumped = FileReadTool(file_path=str(here / "notes.txt")).model_dump()
426+
monkeypatch.chdir(there)
427+
rebuilt = FileReadTool.model_validate(dumped)
428+
429+
assert rebuilt._run() == "from here\n"
430+
431+
432+
def test_a_declared_base_dir_pins_a_relative_path_across_a_rebuild(tmp_path, monkeypatch):
433+
"""base_dir is anchored at construction, so it carries the pin with it."""
434+
here, there = tmp_path / "here", tmp_path / "there"
435+
here.mkdir(), there.mkdir()
436+
(here / "notes.txt").write_text("from here\n")
437+
(there / "notes.txt").write_text("from there\n")
438+
439+
monkeypatch.chdir(here)
440+
dumped = FileReadTool(file_path="notes.txt", base_dir=str(here)).model_dump()
441+
monkeypatch.chdir(there)
442+
rebuilt = FileReadTool.model_validate(dumped)
443+
444+
assert rebuilt._run() == "from here\n"
445+
446+
447+
def test_a_bare_relative_declared_path_reanchors_on_rebuild(tmp_path, monkeypatch):
448+
"""A relative path with no base_dir names nothing absolute to preserve.
449+
450+
It re-anchors to the rebuilding process's working directory — the same file
451+
the same arguments would name there. Documented rather than "fixed": the
452+
alternative is pinning a path from a working directory that, for a rebuild
453+
in a fresh container, no longer exists. Callers needing the pin to survive
454+
pass `base_dir`, which the test above covers.
455+
"""
456+
here, there = tmp_path / "here", tmp_path / "there"
457+
here.mkdir(), there.mkdir()
458+
(here / "notes.txt").write_text("from here\n")
459+
(there / "notes.txt").write_text("from there\n")
460+
461+
monkeypatch.chdir(here)
462+
dumped = FileReadTool(file_path="notes.txt").model_dump()
463+
monkeypatch.chdir(there)
464+
rebuilt = FileReadTool.model_validate(dumped)
465+
466+
assert rebuilt._run() == "from there\n"
467+
# And it is a real re-anchor, not a stale absolute path that happens to read.
468+
assert rebuilt._declared_realpath == str(there / "notes.txt")
469+
470+
407471
# --- a store that fails ------------------------------------------------------
408472
#
409473
# A store may fail where the local filesystem never could. Every exit from

0 commit comments

Comments
 (0)