fix(tools): reset the description when the declared-file pin is lost

Bugbot caught a hole in the previous commit's own fix. Guarding the declared
-file derivation clears `_declared_realpath` and `_declared_label`, but
`description` is a serialized *field* — on a `model_validate` rebuild it
arrives already saying "The default file is notes.txt, which is read when
'file_path' is omitted".

So a rebuilt reader that lost its pin still advertised a default it could not
read. The LLM would take the tool at its word, call it with no arguments, and
get "No file path provided" — the tool lying about its own contract, which is
worse than the missing default it was meant to degrade to.

The failure path now restores the class-default description alongside clearing
the pin, so what the tool says and what it does stay matched.

105 tests across the file tools; the new one asserts a real dumped description
advertises the default, and that after a rebuild losing the pin it does not,
matches the class default, and reports "No file path provided" on a bare call.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UNumDnNbiyw3pv1WakAe6t
This commit is contained in:
Joao Moura
2026-08-03 19:01:37 -07:00
parent 953e1b94ab
commit 73d16ac6ba
2 changed files with 33 additions and 0 deletions

View File

@@ -173,6 +173,16 @@ class FileReadTool(BaseTool):
)
self._declared_realpath = None
self._declared_label = None
# The description has to come back too. On a rebuild it arrives
# from the serialized data already advertising a default file,
# so leaving it would promise the LLM something omitting
# 'file_path' can no longer deliver — it would call the tool
# with no arguments and get "No file path provided". Restoring
# the class default keeps what the tool says matched to what it
# does.
default = type(self).model_fields["description"].default
if isinstance(default, str):
self.description = default
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."

View File

@@ -434,6 +434,29 @@ def test_a_store_failing_on_the_declared_file_still_builds_the_tool(
assert "No file path provided" in tool._run()
def test_a_rebuild_that_loses_the_pin_stops_advertising_a_default(
store, failing_store, tmp_path, monkeypatch
):
"""What the tool says must match what it does.
The description is serialized, so on a rebuild it arrives already naming a
default file. If the pin is then lost, leaving that text would tell the LLM
it can omit `file_path` — and it would get "No file path provided" back.
"""
monkeypatch.chdir(tmp_path)
dumped = FileReadTool(file_path="notes.txt").model_dump()
assert "The default file is" in dumped["description"]
# Same tool, rebuilt against a store that can no longer label the path.
failing_store("display")
rebuilt = FileReadTool.model_validate(dumped)
assert rebuilt._declared_label is None
assert "The default file is" not in rebuilt.description
assert rebuilt.description == FileReadTool.model_fields["description"].default
assert "No file path provided" in rebuilt._run()
def test_a_store_failing_on_base_dir_is_not_swallowed(
failing_store, tmp_path, monkeypatch
):