feat: add readFile to Sandbox

[lgrammel]

Background

We introduced a Sandbox abstraction for bash execution in #14949

Another common operation is reading files.

Summary

Add basic readFile support for Sandbox.

Related Issues

Builds on #14949

[felixarntz]

@lgrammel What's the planned scope for the Sandbox abstraction, in terms of methods to require?

Fundamentally, I think a sandbox only needs to have executeCommand. Reading files usually still happens through executing a command. A readFile tool could implement the rest itself I think?

I'm not sure about introducing more methods on the main Sandbox contract. If we want filesystem methods, how about we have readFile and writeFile etc. on a separate contract?

Maybe we could have Filesystem if we want to provide these? Then sandbox implementations could choose whether they are Sandbox or Sandbox & Filesystem.

[lgrammel]

@felixarntz i agree on the separation in theory, but we are optimizing for agents/tools that use sandboxes and separating the filesystem would introduce unnecessary complexity. usually filesystems are tied to sandboxes for advanced uses cases, e.g. partial file reads and writes. especially tools like the openai patch tool and the anthropic text editor tool would need those.

the combined sandbox abstraction also aligns with the minimal sandbox spec malte had, and e.g. in open agents the bash execution approach you suggest broke for large files and a dedicated readFile tool was required. there is also an advantage in having dedicated read/write tools for approval reasons. cc @nicoalbanese

[lgrammel]

there is an issue around supporting binary files

[felixarntz]

@lgrammel That rationale makes sense, thanks. All good then from my perspective.

Just one follow-up, could you clarify what the intended scope is? Other than executeCommand and readFile, what do we want to have on the Sandbox abstraction? I assume writeFile? how about deleteFile? anything else?

[felixarntz]

I know that's the default root, but should we consider having rootDirectory as a general option on our sandboxes? I think having that makes sense on pretty much every sandbox, to control the default location where commands are executed. It's already in the demo LocalSandbox implementation like that, but here we hard-code the path.

[lgrammel]

those are all currently in user land. i agree we should have those in sandbox constructors once we add specific sandboxes in one fashion or another.

[lgrammel]

@lgrammel That rationale makes sense, thanks. All good then from my perspective.

Just one follow-up, could you clarify what the intended scope is? Other than executeCommand and readFile, what do we want to have on the Sandbox abstraction? I assume writeFile? how about deleteFile? anything else?

we are currently thinking executeCode, readFile, writeFile will be the minimal needed. you are right, maybe deleteFile as well.

put the pr on hold for more discussion next week, esp around how to handle different types of files (binary vs text)

[felixarntz]

@lgrammel For reference, here is something I built a while ago where I was thinking about a sandbox abstraction. I called it "environment" instead of "sandbox", but same foundational idea: https://github.com/felixarntz/ai-code-agents/blob/main/packages/environment-utils/src/types.ts

It's an older exploration I'm not maintaining, but there may be some useful ideas in there - whether to apply or to intentionally rule out.

To be clear, I like where we're going here, it's much simpler in how it doesn't couple tools with the sandbox by definition. But I thought it's worth comparing.