env: add new FileSystem-related tools such as Grep, Glob and Edit.#198
env: add new FileSystem-related tools such as Grep, Glob and Edit.#198tiborvass wants to merge 3 commits intodagger:mainfrom
Conversation
aluzzardi
left a comment
aluzzardi
left a comment
There was a problem hiding this comment.
Looks good to me!
I'll let @vito and @kpenfound do a review -- not too familiar with what other agents tools do (line based edit vs search and replace etc)
environment/filesystem.go
Outdated
|
|
||
| func (env *Environment) FileGrep(ctx context.Context, path, pattern, include string) (string, error) { | ||
| // Hack: use busybox to run `sed` since dagger doesn't have native file editing primitives. | ||
| args := []string{"/bin/grep", "-E", "--", pattern, include} |
There was a problem hiding this comment.
What about using ripgrep instead? It's smart enough to avoid grepping through node_modules and other crap
environment/filesystem.go
Outdated
| ) | ||
|
|
||
| // FIXME: See hack where it's used | ||
| const fileUtilsBaseImage = "busybox" |
There was a problem hiding this comment.
nit: I think there's another one of this pointing to alpine in the codebase, can we merge the two?
nit2: can we pin this to a digest? Otherwise AFAIK buildkit will ping the registry each time to resolve the digest and see if it changed compared to local
| } | ||
| defer dag.Close() | ||
|
|
||
| go warmCache(ctx, dag) |
environment/filesystem.go
Outdated
|
|
||
| func (env *Environment) FileGrep(ctx context.Context, path, pattern, include string) (string, error) { | ||
| // Hack: use busybox to run `sed` since dagger doesn't have native file editing primitives. | ||
| args := []string{"/bin/grep", "-E", "--", pattern, include} |
There was a problem hiding this comment.
Claude choked on this command when I gave it a go:
https://dagger.cloud/vito/traces/b72d0edd3100fb0789cb2f718eba3b0e?span=af8472a10106d33f
Looks like grep didn't like a blank include arg. Maybe it should default to .?
mcpserver/tools.go
Outdated
| mcp.Required(), | ||
| ), | ||
| mcp.WithArray("edits", | ||
| mcp.Description("Array of sed search-replace operations to perform on the contents of target_file (e.g. \"s/old/new/g\").\nUses extended regex syntax."), |
There was a problem hiding this comment.
Claude choked on this too:
● container-use - environment_file_edit (MCP)(environment_source: "/var/home/vito/src/bind", environment_id: "oriented-deer", target_file: "treesitter/binding.gyp", edits:
["s/tree_sitter_bind_binding/tree_sitter_sprout_binding/g"], explanation: "Updating binding.gyp to use sprout instead of bind")
⎿ Error: MCP error -32603: could not bind arguments
● Let me read and update the file content:
I think the description is just out of date - looks like it's no longer sed syntax (which I think makes sense, since Claude by necessity includes a bunch of context to ensure it doesn't misfire edits and that'd be awkward to squish into sed syntax).
|
@tiborvass btw -- with @grouville's new integration tests / MCP fan-out, perhaps we can now actually test those (once we're happy with the signatures)? |
|
@aluzzardi yes, that's why it was in Draft, wanted to add tests. |
tiborvass
force-pushed
the
fstools
branch
6 times, most recently
from
122e3aa to
4c0c3d0
Compare
Also modifies List to add an `ignore` argument. Signed-off-by: Tibor Vass <teabee89@gmail.com>
This prevents streaming all the contents of a file from dagger. Signed-off-by: Tibor Vass <teabee89@gmail.com>
tiborvass
force-pushed
the
fstools
branch
3 times, most recently
from
6961f0d to
6a592ed
Compare
Signed-off-by: Tibor Vass <teabee89@gmail.com>
|
@tiborvass I'm working on a couple of Dagger PRs that may be useful here:
Would have pinged earlier about it, but I just started on these on a whim the other day and it ended up going smoother than I expected. 🙏 |
|
@vito nice thank you! Feel free to take over the branch or steal from it whatever you need, i'm not attached to it! |
3 participants
Also modifies List to add an
ignoreargument.