feat(mcp): add list_files and file_exists tools - #1329
Conversation
The MCP server exposed read/write/exec but not directory listing or existence checks, although both already exist on the Python SDK's filesystem interface (AsyncFilesystem.list/.exists) and both are exposed by the Go SDK. An agent could therefore write and read a file but could not discover what was in a directory, or test for a path without provoking a read error. Add two tools wrapping the existing SDK calls: - list_files: returns each entry's name, size, type (file/directory) and mod_time. An empty directory is an empty list, not an error. - file_exists: returns a boolean. A missing path is a successful "exists: false" answer rather than a failure. Both go through get_sandbox(), so they inherit the per-session ownership check unchanged; the tests assert the sandbox is never touched when that check rejects, so the tools fail closed. Timeouts reuse TOOL_DEFAULT_TIMEOUT/TOOL_MAX_TIMEOUT for consistency with the other file tools.
✅ Deploy Preview for agent-sandbox canceled.
|
kubernetes-prow
Bot
added
cncf-cla: yes
|
Hi @yuzhiquan. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: yuzhiquan The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
kubernetes-prow
Bot
added
the
size/L
|
Important Review skippedAuto reviews are limited based on label configuration. 🚫 Review skipped — only excluded labels are configured. (3)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe MCP server adds ChangesFilesystem MCP tools
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant MCP Client
participant MCP Server
participant Sandbox
participant Sandbox Filesystem API
MCP Client->>MCP Server: invoke list_files or file_exists
MCP Server->>Sandbox: retrieve sandbox
Sandbox->>Sandbox Filesystem API: list directory or check path
Sandbox Filesystem API-->>MCP Server: entries or exists boolean
MCP Server-->>MCP Client: typed tool response
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
clients/integrations/mcp-server/tests/unit/test_tools/test_file_exists.py (1)
71-94: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest both timeout rejection boundaries.
Both tools enforce
gt=0andle=TOOL_MAX_TIMEOUT, but the tests do not cover the full contract.
clients/integrations/mcp-server/tests/unit/test_tools/test_file_exists.py#L71-L94: Add MCP-level tests fortimeout=0andtimeout=TOOL_MAX_TIMEOUT + 1. Assert thatmock_sandbox.files.existsis not called.clients/integrations/mcp-server/tests/unit/test_tools/test_list_files.py#L136-L153: Add a test fortimeout=TOOL_MAX_TIMEOUT + 1. Assert thatmock_sandbox.files.listis not called.As per coding guidelines, focus findings on substantive issues, including missing tests.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@clients/integrations/mcp-server/tests/unit/test_tools/test_file_exists.py` around lines 71 - 94, Add MCP-level timeout boundary tests in clients/integrations/mcp-server/tests/unit/test_tools/test_file_exists.py:71-94 for timeout=0 and timeout=TOOL_MAX_TIMEOUT + 1, asserting validation rejects both and mock_sandbox.files.exists is not called. Add the upper-bound rejection test in clients/integrations/mcp-server/tests/unit/test_tools/test_list_files.py:136-153, asserting mock_sandbox.files.list is not called.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@clients/integrations/mcp-server/k8s_agent_sandbox_mcp_server/tools/list_files.py`:
- Around line 58-72: Limit the entries processed by list_files before
constructing ListFilesOutputSchema, using a defined maximum count or pagination
strategy so large directories cannot exhaust memory. Document the selected
response-size contract near list_files and preserve the existing FileEntrySchema
mapping for returned entries.
---
Nitpick comments:
In `@clients/integrations/mcp-server/tests/unit/test_tools/test_file_exists.py`:
- Around line 71-94: Add MCP-level timeout boundary tests in
clients/integrations/mcp-server/tests/unit/test_tools/test_file_exists.py:71-94
for timeout=0 and timeout=TOOL_MAX_TIMEOUT + 1, asserting validation rejects
both and mock_sandbox.files.exists is not called. Add the upper-bound rejection
test in
clients/integrations/mcp-server/tests/unit/test_tools/test_list_files.py:136-153,
asserting mock_sandbox.files.list is not called.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ebe184a4-9d00-45f1-acfc-87bb73720e23
📒 Files selected for processing (7)
clients/integrations/mcp-server/README.mdclients/integrations/mcp-server/k8s_agent_sandbox_mcp_server/server.pyclients/integrations/mcp-server/k8s_agent_sandbox_mcp_server/tools/__init__.pyclients/integrations/mcp-server/k8s_agent_sandbox_mcp_server/tools/file_exists.pyclients/integrations/mcp-server/k8s_agent_sandbox_mcp_server/tools/list_files.pyclients/integrations/mcp-server/tests/unit/test_tools/test_file_exists.pyclients/integrations/mcp-server/tests/unit/test_tools/test_list_files.py
Addresses the CodeRabbit review. Cap the number of entries list_files returns (max_entries, default 1000, hard ceiling 10000) and report total_entries plus truncated so a caller can tell a complete listing from a partial one. One correction to the review's rationale: a cap here cannot prevent memory exhaustion. The SDK's files.list() calls response.json() and builds the whole list[FileEntry] before this tool is reached, so peak allocation has already happened by the time we could truncate; that bound belongs in the SDK or the runtime. The cap is still worth having for a different reason -- a directory with tens of thousands of entries would otherwise be serialized straight into an LLM's context window and exhaust its token budget. total_entries exists so truncation is never silent, since a model shown a partial listing as if it were complete is worse than one told to narrow its query. Also extend the timeout coverage the review asked for: both tools now test the lower (0) and upper (TOOL_MAX_TIMEOUT + 1) bounds, and max_entries is tested at 0 and above its ceiling. Every rejection case asserts the sandbox was never touched, so validation failures cannot reach the backing call. Suite: 30 -> 37 tests.
kubernetes-prow
Bot
added
size/XL
|
Thanks — both findings addressed in a1503a4. Applied locally rather than via "Commit suggestion", per the contributor notice. Directory listing size — capped, but I want to flag that the stated rationale doesn't hold, in case it matters for where a real fix belongs. A cap in this tool cannot prevent memory exhaustion. entries = response.json() # whole body already parsed
file_entries = [FileEntry(**e) for e in entries] # whole list already builtBy the time That said, the cap is worth having for a different reason, so I've implemented it: a directory with tens of thousands of entries would otherwise be serialized straight into an LLM's context window and exhaust its token budget. So:
Timeout boundaries — agreed, coverage was incomplete. Both tools now parametrize over Suite is 30 → 37 tests; |
There was a problem hiding this comment.
Pull request overview
Adds two new filesystem-oriented tools to the Agent Sandbox MCP server to close a cross-SDK parity gap (Python/Go SDKs already support directory listing and existence checks).
Changes:
- Introduces
list_filestool to list directory entries (with response truncation viamax_entries, plustotal_entriesandtruncated). - Introduces
file_existstool to check path existence with a boolean result. - Adds unit tests and updates MCP server README to document both tools.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| clients/integrations/mcp-server/k8s_agent_sandbox_mcp_server/tools/list_files.py | New MCP tool that calls sandbox.files.list() and returns structured directory entries with truncation metadata. |
| clients/integrations/mcp-server/k8s_agent_sandbox_mcp_server/tools/file_exists.py | New MCP tool that calls sandbox.files.exists() and returns {exists: bool}. |
| clients/integrations/mcp-server/k8s_agent_sandbox_mcp_server/tools/init.py | Exports the two new tools from the tools package. |
| clients/integrations/mcp-server/k8s_agent_sandbox_mcp_server/server.py | Registers list_files and file_exists with the FastMCP server. |
| clients/integrations/mcp-server/tests/unit/test_tools/test_list_files.py | Unit coverage for default/non-default args, truncation behavior, validation, error surfacing, and fail-closed session ownership checks. |
| clients/integrations/mcp-server/tests/unit/test_tools/test_file_exists.py | Unit coverage for present/absent paths, validation, error surfacing, and fail-closed session ownership checks. |
| clients/integrations/mcp-server/README.md | Documents the new tools, arguments, and return contracts. |
|
/ok-to-test |
kubernetes-prow
Bot
added
ok-to-test
|
/lgtm |
kubernetes-prow
Bot
added
the
lgtm
What this PR does / why we need it:
The MCP server exposes
upload_fileanddownload_file, but there is no way tolist a directory or test whether a path exists. An agent can write a file and
read it back, yet cannot discover what is in a directory, and can only probe for
a path by attempting a read and interpreting the failure.
Both operations already exist on the Python SDK's filesystem interface —
AsyncFilesystem.list()and.exists()inclients/python/agentic-sandbox-client/k8s_agent_sandbox/files/async_filesystem.py— and both are already exposed by the Go SDK (
Sandbox.List,Sandbox.Exists).So this is a cross-SDK parity gap rather than new functionality: the
capability is present and simply unexposed over MCP.
This PR adds two thin tools over those calls, following
download_file.pylinefor line:
list_files— returns each entry'sname,size,type(
file/directory) andmod_time(POSIX timestamp), mapped from the SDK'sFileEntry. An empty directory returns an empty list, not an error.file_exists— returns a boolean. A missing path is a successfulexists: falseanswer rather than a failure, which is a more useful contractfor an LLM caller than an exception.
Notes for reviewers:
existing
get_sandbox()helper, so they inherit the per-session ownershipcheck unchanged — no new authorization path. Each tool's
test_session_id_not_foundadditionally asserts the sandbox is nevertouched when that check rejects, so the tools provably fail closed.
TOOL_DEFAULT_TIMEOUT/TOOL_MAX_TIMEOUT, so bounds andvalidation match the other file tools.
README.mddocuments both tools in the established per-tool format.Which issue(s) this PR is related to:
Follow-up to the MCP server added in #1141
Release Note
Summary by CodeRabbit
New Features
Tests