Yehowshua/ZIm
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Have read_file support images (#30435)

Browse source 
This is very basic support for them. There are a number of other TODOs
before this is really a first-class supported feature, so not adding any
release notes for it; for now, this PR just makes it so that if
read_file tries to read a PNG (which has come up in practice), it at
least correctly sends it to Anthropic instead of messing up.

This also lays the groundwork for future PRs for more first-class
support for images in tool calls across more image file formats and LLM
providers.

Release Notes:

- N/A

---------

Co-authored-by: Agus Zubiaga <hi@aguz.me>
Co-authored-by: Agus Zubiaga <agus@zed.dev>
This commit is contained in:
Richard Feldman 2025-05-13 10:58:00 +02:00 • committed by GitHub
parent f01af006e1
commit 8fdf309a4a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
30 changed files with 557 additions and 194 deletions
Download patch file Download diff file Expand all files Collapse all files

99
crates/assistant_tools/src/terminal_tool.rs
View file

@ -1,5 +1,5 @@
use crate::schema::json_schema_for;
use anyhow::{Context as _, Result, anyhow, bail};
use anyhow::{Context as _, Result, anyhow};
use assistant_tool::{ActionLog, Tool, ToolCard, ToolResult, ToolUseStatus};
use futures::{FutureExt as _, future::Shared};
use gpui::{
@ -125,18 +125,24 @@ impl Tool for TerminalTool {
Err(err) => return Task::ready(Err(anyhow!(err))).into(),
};
let input_path = Path::new(&input.cd);
let working_dir = match working_dir(&input, &project, input_path, cx) {
let working_dir = match working_dir(&input, &project, cx) {
Ok(dir) => dir,
Err(err) => return Task::ready(Err(err)).into(),
};
let program = self.determine_shell.clone();
let command = if cfg!(windows) {
format!("$null | & {{{}}}", input.command.replace("\"", "'"))
} else if let Some(cwd) = working_dir
.as_ref()
.and_then(|cwd| cwd.as_os_str().to_str())
{
// Make sure once we're *inside* the shell, we cd into `cwd`
format!("(cd {cwd}; {}) </dev/null", input.command)
} else {
format!("({}) </dev/null", input.command)
};
let args = vec!["-c".into(), command];
let cwd = working_dir.clone();
let env = match &working_dir {
Some(dir) => project.update(cx, |project, cx| {
@ -319,19 +325,13 @@ fn process_content(
} else {
content
};
let is_empty = content.trim().is_empty();
let content = format!(
"```\n{}{}```",
content,
if content.ends_with('\n') { "" } else { "\n" }
);
let content = content.trim();
let is_empty = content.is_empty();
let content = format!("```\n{content}\n```");
let content = if should_truncate {
format!(
"Command output too long. The first {} bytes:\n\n{}",
"Command output too long. The first {} bytes:\n\n{content}",
content.len(),
content,
)
} else {
content
@ -371,42 +371,47 @@ fn process_content(
fn working_dir(
input: &TerminalToolInput,
project: &Entity<Project>,
input_path: &Path,
cx: &mut App,
) -> Result<Option<PathBuf>> {
let project = project.read(cx);
let cd = &input.cd;
if input.cd == "." {
// Accept "." as meaning "the one worktree" if we only have one worktree.
if cd == "." || cd == "" {
// Accept "." or "" as meaning "the one worktree" if we only have one worktree.
let mut worktrees = project.worktrees(cx);
match worktrees.next() {
Some(worktree) => {
if worktrees.next().is_some() {
bail!(
if worktrees.next().is_none() {
Ok(Some(worktree.read(cx).abs_path().to_path_buf()))
} else {
Err(anyhow!(
"'.' is ambiguous in multi-root workspaces. Please specify a root directory explicitly.",
);
))
}
Ok(Some(worktree.read(cx).abs_path().to_path_buf()))
}
None => Ok(None),
}
} else if input_path.is_absolute() {
// Absolute paths are allowed, but only if they're in one of the project's worktrees.
if !project
.worktrees(cx)
.any(|worktree| input_path.starts_with(&worktree.read(cx).abs_path()))
{
bail!("The absolute path must be within one of the project's worktrees");
} else {
let input_path = Path::new(cd);
if input_path.is_absolute() {
// Absolute paths are allowed, but only if they're in one of the project's worktrees.
if project
.worktrees(cx)
.any(|worktree| input_path.starts_with(&worktree.read(cx).abs_path()))
{
return Ok(Some(input_path.into()));
}
} else {
if let Some(worktree) = project.worktree_for_root_name(cd, cx) {
return Ok(Some(worktree.read(cx).abs_path().to_path_buf()));
}
}
Ok(Some(input_path.into()))
} else {
let Some(worktree) = project.worktree_for_root_name(&input.cd, cx) else {
bail!("`cd` directory {:?} not found in the project", input.cd);
};
Ok(Some(worktree.read(cx).abs_path().to_path_buf()))
Err(anyhow!(
"`cd` directory {cd:?} was not in any of the project's worktrees."
))
}
}
@ -727,8 +732,8 @@ mod tests {
)
});
let output = result.output.await.log_err().map(|output| output.content);
assert_eq!(output, Some("Command executed successfully.".into()));
let output = result.output.await.log_err().unwrap().content;
assert_eq!(output.as_str().unwrap(), "Command executed successfully.");
}
#[gpui::test]
@ -761,12 +766,13 @@ mod tests {
cx,
);
cx.spawn(async move |_| {
let output = headless_result
.output
.await
.log_err()
.map(|output| output.content);
assert_eq!(output, expected);
let output = headless_result.output.await.map(|output| output.content);
assert_eq!(
output
.ok()
.and_then(|content| content.as_str().map(ToString::to_string)),
expected
);
})
};
@ -774,7 +780,7 @@ mod tests {
check(
TerminalToolInput {
command: "pwd".into(),
cd: "project".into(),
cd: ".".into(),
},
Some(format!(
"```\n{}\n```",
@ -789,12 +795,9 @@ mod tests {
check(
TerminalToolInput {
command: "pwd".into(),
cd: ".".into(),
cd: "other-project".into(),
},
Some(format!(
"```\n{}\n```",
tree.path().join("project").display()
)),
None, // other-project is a dir, but *not* a worktree (yet)
cx,
)
})