Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 19 additions & 15 deletions libs/deepagents/deepagents/middleware/filesystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -1330,16 +1330,20 @@ def __init__(
)
self._glob_slots = threading.BoundedSemaphore(_SYNC_GLOB_WORKERS)

self.tools = [
self._create_ls_tool(),
self._create_read_file_tool(),
self._create_write_file_tool(),
self._create_edit_file_tool(),
self._create_delete_tool(),
self._create_glob_tool(),
self._create_grep_tool(),
self._create_execute_tool(),
]
tool_factories: tuple[tuple[str, Callable[[], BaseTool]], ...] = (
("ls", self._create_ls_tool),
("read_file", self._create_read_file_tool),
("write_file", self._create_write_file_tool),
("edit_file", self._create_edit_file_tool),
("delete", self._create_delete_tool),
("glob", self._create_glob_tool),
("grep", self._create_grep_tool),
("execute", self._create_execute_tool),
)
# Excluded tools are omitted here entirely, not just hidden from the
# model's schema, so a tool name outside `tools=` never reaches the
# dispatchable tool node
self.tools = [factory() for name, factory in tool_factories if self._enabled_tools is None or name in self._enabled_tools]

def _build_dynamic_system_prompt(self, *, include_execution: bool) -> str:
"""Build (and memoize) the dynamic system prompt.
Expand Down Expand Up @@ -2289,11 +2293,11 @@ def _unsupported_tools_and_execution_state(
runtime: Runtime[ContextT],
) -> tuple[set[str | None], bool, BackendProtocol | None]:
"""Return unsupported filesystem tools and whether execute remains active."""
unsupported: set[str | None] = (
{name for name in tool_names if name in _ALL_FS_TOOL_NAMES and name not in self._enabled_tools}
if self._enabled_tools is not None
else set()
)
# `tools=` exclusions are enforced at `__init__` (absent from
# `self.tools` entirely), so only backend-capability gating
# `execute`/`delete` on a backend that doesn't support them is
# computed here.
unsupported: set[str | None] = set()
execution_active = False
backend = None
has_execute_tool = "execute" in tool_names
Expand Down
40 changes: 40 additions & 0 deletions libs/deepagents/tests/unit_tests/test_end_to_end.py
Original file line number Diff line number Diff line change
Expand Up @@ -4661,3 +4661,43 @@ def test_allowlist_removes_tools_from_request_and_system_prompt(self) -> None:
assert "`ls`" in prompt
for disabled in ("write_file", "edit_file", "delete", "glob"):
assert f"`{disabled}`" not in prompt, f"`{disabled}` should not appear in system prompt tool list"

def test_excluded_tool_call_fails_instead_of_executing(self) -> None:
"""An excluded tool referenced in a `ToolCall` errors instead of executing.

Simulates an out-of-schema `write_file` call despite
`tools=["ls", "read_file"]`. Before the fix, `write_file` was still
registered on `ToolNode` and would execute. After the fix, it's no longer
registered, so `ToolNode` returns an error `ToolMessage` instead.
"""
model = FixedGenericFakeChatModel(
messages=iter(
[
AIMessage(
content="",
tool_calls=[
{
"name": "write_file",
"args": {"file_path": "/pwned.txt", "content": "hi"},
"id": "call_1",
"type": "tool_call",
}
],
),
AIMessage(content="done"),
]
)
)
agent = create_deep_agent(
model=model,
middleware=[FilesystemMiddleware(backend=StateBackend(), tools=["ls", "read_file"])],
)

result = agent.invoke({"messages": [HumanMessage(content="hi")]})

tool_messages = [m for m in result["messages"] if isinstance(m, ToolMessage)]
assert len(tool_messages) == 1
assert tool_messages[0].status == "error"
assert "write_file" in tool_messages[0].content
# The excluded tool must not have actually run.
assert "/pwned.txt" not in result.get("files", {})
140 changes: 50 additions & 90 deletions libs/deepagents/tests/unit_tests/test_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -2122,117 +2122,53 @@ def test_enabled_tools_raises_when_read_file_excluded(self):
with pytest.raises(ValueError, match="read_file must be included in tools"):
FilesystemMiddleware(backend=StateBackend(), tools=["write_file"])

def test_enabled_tools_filters_unlisted_tool(self):
"""A tool not in tools is filtered out of the model request."""
middleware = FilesystemMiddleware(
backend=StateBackend(),
system_prompt="",
tools=["read_file", "ls"],
)
write_tool = MagicMock()
write_tool.name = "write_file"
ls_tool = MagicMock()
ls_tool.name = "ls"
request = MagicMock()
request.tools = [ls_tool, write_tool]
request.override.return_value = request

middleware._filter_unsupported_tools_and_apply_prompt(request)

filtered_names = {tool.name for tool in request.override.call_args.kwargs["tools"]}
assert "write_file" not in filtered_names
assert "ls" in filtered_names

def test_enabled_tools_filters_multiple_unlisted(self):
"""Only tools in the allowlist survive; the rest are filtered."""
"""Only tools in the allowlist are registered; the rest are absent from `self.tools`."""
middleware = FilesystemMiddleware(
backend=StateBackend(),
system_prompt="",
tools=["read_file", "ls", "grep"],
)
tools = [MagicMock(name=n) for n in ("ls", "write_file", "delete", "grep")]
for t in tools:
t.name = t._mock_name
request = MagicMock()
request.tools = tools
request.override.return_value = request

middleware._filter_unsupported_tools_and_apply_prompt(request)
names = {tool.name for tool in middleware.tools}
assert "write_file" not in names
assert "delete" not in names
assert "ls" in names
assert "grep" in names

filtered_names = {tool.name for tool in request.override.call_args.kwargs["tools"]}
assert "write_file" not in filtered_names
assert "delete" not in filtered_names
assert "ls" in filtered_names
assert "grep" in filtered_names
def test_enabled_tools_does_not_double_filter_user_provided_tools(self):
"""`wrap_model_call` doesn't re-filter or touch non-filesystem tools.

def test_enabled_tools_does_not_filter_user_provided_tools(self):
"""User-provided (non-filesystem) tools are never removed by the allowlist."""
`request.tools` here mirrors what `create_agent` actually assembles:
the middleware's own (already-restricted) `self.tools`, plus a
separate user-provided tool it never owns.
"""
middleware = FilesystemMiddleware(
backend=StateBackend(),
system_prompt="",
tools=["read_file", "ls"],
)
ls_tool = MagicMock()
ls_tool.name = "ls"
write_tool = MagicMock()
write_tool.name = "write_file"
custom_tool = MagicMock()
custom_tool.name = "search"
request = MagicMock()
request.tools = [ls_tool, write_tool, custom_tool]
request.tools = [*middleware.tools, custom_tool]
request.override.return_value = request

middleware._filter_unsupported_tools_and_apply_prompt(request)

filtered_names = {tool.name for tool in request.override.call_args.kwargs["tools"]}
assert "ls" in filtered_names
assert "search" in filtered_names # user tool untouched
assert "write_file" not in filtered_names # FS tool not in allowlist

def test_enabled_tools_passes_listed_tools_through(self):
"""Tools in the allowlist survive; an unlisted tool is dropped."""
middleware = FilesystemMiddleware(
backend=StateBackend(),
system_prompt="",
tools=["read_file", "ls", "grep"],
)
ls_tool = MagicMock()
ls_tool.name = "ls"
grep_tool = MagicMock()
grep_tool.name = "grep"
edit_tool = MagicMock()
edit_tool.name = "edit_file"
request = MagicMock()
request.tools = [ls_tool, grep_tool, edit_tool]
request.override.return_value = request

middleware._filter_unsupported_tools_and_apply_prompt(request)

filtered_names = {tool.name for tool in request.override.call_args.kwargs["tools"]}
assert "ls" in filtered_names
assert "grep" in filtered_names
assert "edit_file" not in filtered_names
# Nothing left to filter as the allowlist was already applied at
# construction, so no `tools=...` override should occur.
tools_overrides = [c for c in request.override.call_args_list if "tools" in c.kwargs]
assert tools_overrides == []

def test_enabled_tools_read_file_only_filters_everything_else(self):
"""tools=["read_file"] hides all other tools."""
all_other_tools = {"ls", "write_file", "edit_file", "delete", "glob", "grep", "execute"}
"""tools=["read_file"] leaves only read_file registered on `self.tools`."""
middleware = FilesystemMiddleware(
backend=StateBackend(),
system_prompt="",
tools=["read_file"],
)
tools = [MagicMock() for _ in range(len(all_other_tools) + 1)]
for t, name in zip(tools, [*all_other_tools, "read_file"], strict=True):
t.name = name
request = MagicMock()
request.tools = tools
request.override.return_value = request

middleware._filter_unsupported_tools_and_apply_prompt(request)

filtered_names = {tool.name for tool in request.override.call_args.kwargs["tools"]}
assert "read_file" in filtered_names
assert filtered_names == {"read_file"}
names = {tool.name for tool in middleware.tools}
assert names == {"read_file"}

def test_enabled_tools_none_default_passes_all_tools(self):
"""tools=None (default) does not filter any tools."""
Expand All @@ -2253,6 +2189,34 @@ def test_enabled_tools_none_default_passes_all_tools(self):
tools_overrides = [c for c in request.override.call_args_list if "tools" in c.kwargs]
assert tools_overrides == []

def test_enabled_tools_excluded_from_self_tools_not_just_request(self):
"""Excluded tools are absent from `self.tools` itself, not just `request.tools`.

`self.tools` is what `create_agent` registers on the dispatchable
`ToolNode` — filtering only `request.tools` in `wrap_model_call` would
leave an excluded tool callable if a `ToolCall` for it ever appeared
(e.g. a stray/hallucinated tool call), since `ToolNode` dispatches by
name lookup independent of what was bound for a given request.
"""
middleware = FilesystemMiddleware(
backend=StateBackend(),
tools=["read_file", "ls"],
)
names = {tool.name for tool in middleware.tools}
assert names == {"ls", "read_file"}

def test_enabled_tools_none_default_keeps_all_self_tools(self):
"""tools=None (default) still registers every filesystem tool."""
middleware = FilesystemMiddleware(backend=StateBackend())
names = {tool.name for tool in middleware.tools}
assert names == {"ls", "read_file", "write_file", "edit_file", "delete", "glob", "grep", "execute"}

def test_enabled_tools_all_keeps_all_self_tools(self):
"""tools="all" registers every filesystem tool, same as the default."""
middleware = FilesystemMiddleware(backend=StateBackend(), tools="all")
names = {tool.name for tool in middleware.tools}
assert names == {"ls", "read_file", "write_file", "edit_file", "delete", "glob", "grep", "execute"}

def test_enabled_tools_execute_listed_but_backend_unsupported_is_noop(self):
"""Execute in tools list is still filtered when the backend doesn't support execution."""
middleware = FilesystemMiddleware(
Expand Down Expand Up @@ -2374,17 +2338,13 @@ def test_grep_description_swap_copies_dict_tool_specs(self):
assert "LITERAL text pattern" in rewritten_grep["description"]

def test_enabled_tools_system_prompt_lists_only_enabled_tools(self):
"""Dynamic system prompt only mentions the tools that survived filtering."""
"""Dynamic system prompt only mentions tools registered on `self.tools`."""
middleware = FilesystemMiddleware(
backend=StateBackend(),
tools=["read_file", "ls"],
)
ls_tool = MagicMock()
ls_tool.name = "ls"
write_tool = MagicMock()
write_tool.name = "write_file"
request = MagicMock()
request.tools = [ls_tool, write_tool]
request.tools = middleware.tools
request.system_message = None
request.override.return_value = request

Expand Down