From f9895c54685db8d5ead46d333b0bf6272828f6ba Mon Sep 17 00:00:00 2001 From: Oliver Azevedo Barnes Date: Mon, 2 Mar 2026 17:10:12 +0000 Subject: [PATCH] devcontainer: Fix git output (#49230) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #48434 In Dev Containers, failed git operations were surfaced with a generic failure message, while the useful git output (stderr/stdout) was not reliably available to users. This happened because in devcontainers the git operation errors go through an RPC layer and then got wrapped with `anyhow::Context` (e.g. “sending pull request”); the toast displayed only that outer context via `to_string()`, masking the underlying git stderr message. This change ensures the full git operation output is preserved and surfaced via Zed’s “See logs” flow in Dev Containers, matching the information you get when running the same git command in a terminal. ### What you should expect in the UI - You will see a generic toast like “git pull failed” / “git push failed”. - When clicking on the toast’s “See logs”, the log tab now contains the full git error output (e.g. non-fast-forward hints, merge conflict details, “local changes would be overwritten”, etc.), which previously could be missing/too generic. --- ## Manual testing Run inside a Dev Container and ensure git auth works (SSH keys/agent or HTTPS credentials). 1. **Dirty-tree pull failure** - Make remote ahead by 1 commit (push from another clone). - Locally modify the same file without committing. - In Zed: **Pull** - **Expect:** toast “git pull failed” + **See logs** shows “local changes would be overwritten…” (or equivalent). 2. **Non-fast-forward push failure** - Ensure remote ahead. - Locally create 1 commit. - In Zed: **Push** - **Expect:** toast “git push failed” + **See logs** shows “rejected (non-fast-forward)” + hint to pull first. 3. **Merge-conflict pull failure** - Create conflicting commits on the same lines (one local commit, one remote commit). - In Zed: **Pull** - **Expect:** toast “git pull failed” + **See logs** shows conflict output (“CONFLICT…”, “Automatic merge failed…”). Release Notes: - Fixed devcontainer git failure toasts so they show the actual git error --------- Co-authored-by: KyleBarton --- Cargo.lock | 1 + crates/git_ui/Cargo.toml | 1 + crates/git_ui/src/git_panel.rs | 58 ++++++++++++++++++++++++++++++++- crates/project/src/git_store.rs | 15 +++------ crates/proto/src/error.rs | 6 ++++ 5 files changed, 70 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 97cc166c14e099b57b74585277869052de0cff87..1a192869d6fee631d129e23c275c86e51168fe2f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7310,6 +7310,7 @@ dependencies = [ "pretty_assertions", "project", "prompt_store", + "proto", "rand 0.9.2", "remote", "remote_connection", diff --git a/crates/git_ui/Cargo.toml b/crates/git_ui/Cargo.toml index 28fac0f849a487c6654e2ac5976191cd3e1a733f..a25911d65eb87d176a0a987d996e159e2c43628c 100644 --- a/crates/git_ui/Cargo.toml +++ b/crates/git_ui/Cargo.toml @@ -44,6 +44,7 @@ panel.workspace = true picker.workspace = true project.workspace = true prompt_store.workspace = true +proto.workspace = true remote_connection.workspace = true remote.workspace = true schemars.workspace = true diff --git a/crates/git_ui/src/git_panel.rs b/crates/git_ui/src/git_panel.rs index 5131e1d144e2cee0cbdbb32a062d3f9c4ea4a08b..1fabc387247e3f0889749463e3aabd89ef0bff42 100644 --- a/crates/git_ui/src/git_panel.rs +++ b/crates/git_ui/src/git_panel.rs @@ -56,6 +56,7 @@ use project::{ project_settings::{GitPathStyle, ProjectSettings}, }; use prompt_store::{BuiltInPrompt, PromptId, PromptStore, RULES_FILE_NAMES}; +use proto::RpcError; use serde::{Deserialize, Serialize}; use settings::{Settings, SettingsStore, StatusStyle}; use smallvec::SmallVec; @@ -6420,7 +6421,7 @@ pub(crate) fn show_error_toast( cx: &mut App, ) { let action = action.into(); - let message = e.to_string().trim().to_string(); + let message = format_git_error_toast_message(&e); if message .matches(git::repository::REMOTE_CANCELLED_BY_USER) .next() @@ -6446,6 +6447,20 @@ pub(crate) fn show_error_toast( } } +fn rpc_error_raw_message_from_chain(error: &anyhow::Error) -> Option<&str> { + error + .chain() + .find_map(|cause| cause.downcast_ref::().map(RpcError::raw_message)) +} + +fn format_git_error_toast_message(error: &anyhow::Error) -> String { + if let Some(message) = rpc_error_raw_message_from_chain(error) { + message.trim().to_string() + } else { + error.to_string().trim().to_string() + } +} + #[cfg(test)] mod tests { use git::{ @@ -6477,6 +6492,47 @@ mod tests { }); } + #[test] + fn test_format_git_error_toast_message_prefers_raw_rpc_message() { + let rpc_error = RpcError::from_proto( + &proto::Error { + message: + "Your local changes to the following files would be overwritten by merge\n" + .to_string(), + code: proto::ErrorCode::Internal as i32, + tags: Default::default(), + }, + "Pull", + ); + + let message = format_git_error_toast_message(&rpc_error); + assert_eq!( + message, + "Your local changes to the following files would be overwritten by merge" + ); + } + + #[test] + fn test_format_git_error_toast_message_prefers_raw_rpc_message_when_wrapped() { + let rpc_error = RpcError::from_proto( + &proto::Error { + message: + "Your local changes to the following files would be overwritten by merge\n" + .to_string(), + code: proto::ErrorCode::Internal as i32, + tags: Default::default(), + }, + "Pull", + ); + let wrapped = rpc_error.context("sending pull request"); + + let message = format_git_error_toast_message(&wrapped); + assert_eq!( + message, + "Your local changes to the following files would be overwritten by merge" + ); + } + #[gpui::test] async fn test_entry_worktree_paths(cx: &mut TestAppContext) { init_test(cx); diff --git a/crates/project/src/git_store.rs b/crates/project/src/git_store.rs index 45ba6817248929391dcc484b25879cf34e7506b9..ae776966a770ccadcffdbf9b140ed10d4871b317 100644 --- a/crates/project/src/git_store.rs +++ b/crates/project/src/git_store.rs @@ -4956,8 +4956,7 @@ impl Repository { .map(|repo_path| repo_path.to_proto()) .collect(), }) - .await - .context("sending stash request")?; + .await?; Ok(()) } } @@ -5166,8 +5165,7 @@ impl Repository { }), askpass_id, }) - .await - .context("sending commit request")?; + .await?; Ok(()) } @@ -5206,8 +5204,7 @@ impl Repository { askpass_id, remote: fetch_options.to_proto(), }) - .await - .context("sending fetch request")?; + .await?; Ok(RemoteCommandOutput { stdout: response.stdout, @@ -5308,8 +5305,7 @@ impl Repository { } as i32), }) - .await - .context("sending push request")?; + .await?; Ok(RemoteCommandOutput { stdout: response.stdout, @@ -5375,8 +5371,7 @@ impl Repository { branch_name: branch.as_ref().map(|b| b.to_string()), remote_name: remote.to_string(), }) - .await - .context("sending pull request")?; + .await?; Ok(RemoteCommandOutput { stdout: response.stdout, diff --git a/crates/proto/src/error.rs b/crates/proto/src/error.rs index d83b0fc499ba9dddb1d6417307fea9eaed9fdfd7..f551e8c3fc4d7023f5d9d43c3dc6eb51ffe2bb46 100644 --- a/crates/proto/src/error.rs +++ b/crates/proto/src/error.rs @@ -159,6 +159,12 @@ pub struct RpcError { /// in the app; however it is useful for chaining .message() and .with_tag() on /// ErrorCode. impl RpcError { + /// Returns the raw server-provided error message without any RPC framing + /// (e.g. without the "RPC request X failed: " prefix that `Display` adds). + pub fn raw_message(&self) -> &str { + &self.msg + } + /// from_proto converts a crate::Error into an anyhow::Error containing /// an RpcError. pub fn from_proto(error: &crate::Error, request: &str) -> anyhow::Error {