Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions app/src/ai/skills/file_watchers/skill_watcher_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ fn test_handle_repository_update_single_skill_added() {
moved: HashMap::new(),
commit_updated: false,
index_lock_detected: false,
remote_ref_updated: false,
};

skill_watcher_handle.update(&mut app, |skill_watcher, ctx| {
Expand Down Expand Up @@ -105,6 +106,7 @@ fn test_handle_repository_update_skill_modified() {
moved: HashMap::new(),
commit_updated: false,
index_lock_detected: false,
remote_ref_updated: false,
};

skill_watcher_handle.update(&mut app, |skill_watcher, ctx| {
Expand Down Expand Up @@ -141,6 +143,7 @@ fn test_handle_repository_update_skill_deleted() {
moved: HashMap::new(),
commit_updated: false,
index_lock_detected: false,
remote_ref_updated: false,
};

skill_watcher_handle.update(&mut app, |skill_watcher, ctx| {
Expand Down Expand Up @@ -181,6 +184,7 @@ fn test_handle_repository_update_multiple_skills_deleted() {
moved: HashMap::new(),
commit_updated: false,
index_lock_detected: false,
remote_ref_updated: false,
};

skill_watcher_handle.update(&mut app, |skill_watcher, ctx| {
Expand Down Expand Up @@ -223,6 +227,7 @@ fn test_handle_repository_update_skill_moved() {
)]),
commit_updated: false,
index_lock_detected: false,
remote_ref_updated: false,
};

skill_watcher_handle.update(&mut app, |skill_watcher, ctx| {
Expand Down Expand Up @@ -290,6 +295,7 @@ fn test_handle_repository_update_non_skill_directory_added_queues_project_direct
moved: HashMap::new(),
commit_updated: false,
index_lock_detected: false,
remote_ref_updated: false,
};

skill_watcher_handle.update(&mut app, |skill_watcher, ctx| {
Expand Down Expand Up @@ -337,6 +343,7 @@ fn test_handle_repository_update_non_skill_file_modified_in_repo_does_not_queue_
moved: HashMap::new(),
commit_updated: false,
index_lock_detected: false,
remote_ref_updated: false,
};

skill_watcher_handle.update(&mut app, |skill_watcher, ctx| {
Expand Down Expand Up @@ -380,6 +387,7 @@ fn test_handle_repository_update_non_skill_file_added_does_not_queue_project_dir
moved: HashMap::new(),
commit_updated: false,
index_lock_detected: false,
remote_ref_updated: false,
};

skill_watcher_handle.update(&mut app, |skill_watcher, ctx| {
Expand Down
6 changes: 4 additions & 2 deletions app/src/code_review/diff_state/local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,9 @@ use crate::features::FeatureFlag;
use crate::util::git::get_pr_for_branch;
use crate::util::git::{
detect_current_branch, detect_main_branch, get_unpushed_commits, parse_unified_diff_header,
run_git_command, Commit, PrInfo,
Commit, PrInfo,
};
use warp_util::git::run_git_command;

use crate::code_review::diff_size_limits::compute_diff_size;
use crate::code_review::is_file_autogenerated;
Expand Down Expand Up @@ -1233,9 +1234,10 @@ impl LocalDiffStateModel {
moved,
commit_updated,
index_lock_detected,
remote_ref_updated,
} = update;

if commit_updated {
if commit_updated || remote_ref_updated {
self.load_diffs_for_current_repo(false, ctx);
// Don't emit MetadataRefreshed here — metadata hasn't been
// recomputed yet. NewDiffsComputed handles the immediate UI
Expand Down
2 changes: 1 addition & 1 deletion app/src/code_review/git_status_update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ impl GitRepoStatusModel {
if update.is_empty() {
return false;
}
if update.commit_updated || update.index_lock_detected {
if update.commit_updated || update.index_lock_detected || update.remote_ref_updated {
return true;
}
// Check if any non-ignored file was touched.
Expand Down
71 changes: 3 additions & 68 deletions app/src/util/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,79 +3,14 @@ use std::path::Path;

use anyhow::{anyhow, Result};
use warp_core::safe_warn;
use warp_util::git::run_git_command;
#[cfg(feature = "local_fs")]
use warp_util::git::run_git_command_with_env;

#[cfg(test)]
#[path = "git_tests.rs"]
mod tests;

/// Runs a git command and returns the output as a string.
/// Thin wrapper over [`run_git_command_with_env`] with no `PATH` override.
#[cfg(feature = "local_fs")]
pub async fn run_git_command(repo_path: &Path, args: &[&str]) -> Result<String> {
run_git_command_with_env(repo_path, args, None).await
}

/// Like [`run_git_command`] but sets `PATH` on the child when `path_env` is
/// `Some`. Used by callers whose hooks need user-installed binaries (e.g.
/// the LFS `pre-push` hook → `git-lfs`). See `specs/APP-4188/TECH.md`.
#[cfg(feature = "local_fs")]
pub async fn run_git_command_with_env(
repo_path: &Path,
args: &[&str],
path_env: Option<&str>,
) -> Result<String> {
use command::r#async::Command;
use command::Stdio;

log::debug!(
"[GIT OPERATION] git.rs run_git_command git {}",
args.join(" ")
);
let mut cmd = Command::new("git");
cmd.arg("-c")
.arg("diff.autoRefreshIndex=false")
.args(args)
.current_dir(repo_path)
.stdout(Stdio::piped())
.stderr(Stdio::piped())
.env("GIT_OPTIONAL_LOCKS", "0")
.kill_on_drop(true);
if let Some(path_env) = path_env {
cmd.env("PATH", path_env);
}
let output = cmd
.output()
.await
.map_err(|e| anyhow!("Failed to execute git command: {}", e))?;

let stdout = String::from_utf8_lossy(&output.stdout).to_string();
let stderr = String::from_utf8_lossy(&output.stderr);

// Handle git diff specific behavior:
// - Exit code 0: no differences
// - Exit code 1: differences found (this is normal for diff commands)
// - Exit code > 1: actual error
if output.status.success() || (output.status.code() == Some(1) && !stdout.is_empty()) {
Ok(stdout)
} else {
Err(anyhow!("Git command failed: {}, {}", stderr, stdout))
}
}

#[cfg(not(feature = "local_fs"))]
pub async fn run_git_command(_repo_path: &Path, _args: &[&str]) -> Result<String> {
Err(anyhow!("Not supported on wasm"))
}

#[cfg(not(feature = "local_fs"))]
pub async fn run_git_command_with_env(
_repo_path: &Path,
_args: &[&str],
_path_env: Option<&str>,
) -> Result<String> {
Err(anyhow!("Not supported on wasm"))
}

/// Returns the set of local branch names for the repo at `repo_path`.
/// Uses a synchronous subprocess call — suitable for call sites in
/// synchronous view handlers where the result is needed immediately.
Expand Down
4 changes: 4 additions & 0 deletions app/src/warp_managed_paths_watcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ fn filter_repository_update(
let mut filtered = RepositoryUpdate {
commit_updated: update.commit_updated,
index_lock_detected: update.index_lock_detected,
remote_ref_updated: update.remote_ref_updated,
..Default::default()
};

Expand Down Expand Up @@ -195,6 +196,7 @@ fn filesystem_event_to_repository_update(event: &BulkFilesystemWatcherEvent) ->
.collect(),
commit_updated: false,
index_lock_detected: false,
remote_ref_updated: false,
}
}

Expand Down Expand Up @@ -411,6 +413,7 @@ mod tests {
moved: HashMap::new(),
commit_updated: false,
index_lock_detected: false,
remote_ref_updated: false,
};

let filtered =
Expand All @@ -436,6 +439,7 @@ mod tests {
)]),
commit_updated: false,
index_lock_detected: false,
remote_ref_updated: false,
};

let filtered =
Expand Down
53 changes: 49 additions & 4 deletions crates/repo_metadata/src/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,48 @@ pub(crate) fn is_shared_git_ref(path: &Path) -> bool {
.unwrap_or(false)
}

/// Returns `true` for loose remote-tracking refs under the shared `.git`
/// directory, e.g. `.git/refs/remotes/origin/main`.
pub(crate) fn is_remote_tracking_ref(path: &Path) -> bool {
if extract_worktree_git_dir(path).is_some() {
return false;
}
let components: Vec<_> = path.components().collect();
let Some(git_index) = components.iter().position(|c| c.as_os_str() == ".git") else {
return false;
};
let after_git = &components[git_index + 1..];
after_git.len() >= 4
&& after_git[0].as_os_str() == "refs"
&& after_git[1].as_os_str() == "remotes"
}

/// Returns true for Git files that can change the current branch's tracked
/// upstream ref.
pub(crate) fn is_tracking_state_git_file(path: &Path) -> bool {
let Some(suffix) = git_suffix_components(path) else {
return false;
};
suffix.len() == 1
&& matches!(
suffix[0].as_os_str().to_str(),
Some("HEAD" | "config" | "config.worktree")
)
}

/// Returns true for `.git/config` in the shared Git directory.
pub(crate) fn is_common_git_config(path: &Path) -> bool {
if extract_worktree_git_dir(path).is_some() {
return false;
}
let components: Vec<_> = path.components().collect();
let Some(git_index) = components.iter().position(|c| c.as_os_str() == ".git") else {
return false;
};
let after_git = &components[git_index + 1..];
after_git.len() == 1 && after_git[0].as_os_str() == "config"
}

/// Returns true for `.git/HEAD` and `.git/refs/heads/*`
/// (and their worktree equivalents `.git/worktrees/*/HEAD`, etc.).
pub(crate) fn is_commit_related_git_file(path: &Path) -> bool {
Expand All @@ -452,15 +494,18 @@ pub(crate) fn is_index_lock_file(path: &Path) -> bool {

/// Determines if a git-related path should be ignored by the filesystem watcher.
///
/// Uses an allowlist approach: only commit-related files (HEAD, refs/heads/*)
/// and the index lock file are allowed through. Everything else inside `.git/`
/// is ignored.
/// Uses an allowlist approach: only commit-related files (HEAD, refs/heads/*),
/// loose remote-tracking refs, tracked-upstream state files, and the index lock
/// file are allowed through. Everything else inside `.git/` is ignored.
pub fn should_ignore_git_path(path: &Path) -> bool {
if !is_git_internal_path(path) {
return false; // Not a git path, don't ignore
}
// Ignore everything inside .git/ except the allowlisted patterns.
!is_commit_related_git_file(path) && !is_index_lock_file(path)
!is_commit_related_git_file(path)
&& !is_index_lock_file(path)
&& !is_remote_tracking_ref(path)
&& !is_tracking_state_git_file(path)
}

pub fn path_passes_filters(path: &Path, gitignores: &[Gitignore]) -> bool {
Expand Down
55 changes: 50 additions & 5 deletions crates/repo_metadata/src/entry_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,10 @@ fn test_path_passes_filters_windows() {

#[test]
fn test_git_path_filtering_allowlist() {
use super::{is_commit_related_git_file, is_index_lock_file, should_ignore_git_path};
use super::{
is_commit_related_git_file, is_common_git_config, is_index_lock_file,
is_remote_tracking_ref, is_tracking_state_git_file, should_ignore_git_path,
};
use std::path::Path;

// Non-git paths should not be ignored
Expand Down Expand Up @@ -250,14 +253,20 @@ fn test_git_path_filtering_allowlist() {
assert!(!should_ignore_git_path(Path::new(
"/home/user/project/.git/index.lock"
)));
assert!(!should_ignore_git_path(Path::new(
"/home/user/project/.git/config"
)));
assert!(!should_ignore_git_path(Path::new(
"/home/user/project/.git/refs/remotes/origin/main"
)));
assert!(!should_ignore_git_path(Path::new(
"/home/user/project/.git/refs/remotes/origin/feature/nested"
)));

// Everything else in .git/ IS ignored
assert!(should_ignore_git_path(Path::new(
"/home/user/project/.git/index"
)));
assert!(should_ignore_git_path(Path::new(
"/home/user/project/.git/config"
)));
assert!(should_ignore_git_path(Path::new(
"/home/user/project/.git/COMMIT_EDITMSG"
)));
Expand All @@ -271,7 +280,7 @@ fn test_git_path_filtering_allowlist() {
"/home/user/project/.git/refs/tags/v1.0"
)));
assert!(should_ignore_git_path(Path::new(
"/home/user/project/.git/refs/remotes/origin/main"
"/home/user/project/.git/refs/remotes/origin"
)));
assert!(should_ignore_git_path(Path::new(
"/home/user/project/.git/objects/abc123"
Expand All @@ -290,6 +299,9 @@ fn test_git_path_filtering_allowlist() {
assert!(!should_ignore_git_path(Path::new(
"/home/user/project/.git/worktrees/my-wt/index.lock"
)));
assert!(!should_ignore_git_path(Path::new(
"/home/user/project/.git/worktrees/my-wt/config.worktree"
)));
// Non-allowlisted worktree paths are still ignored
assert!(should_ignore_git_path(Path::new(
"/home/user/project/.git/worktrees/my-wt/index"
Expand Down Expand Up @@ -328,6 +340,39 @@ fn test_git_path_filtering_allowlist() {
assert!(!is_index_lock_file(Path::new("/repo/.git/HEAD")));
assert!(!is_index_lock_file(Path::new("/repo/.git/index")));

// Remote-tracking refs
assert!(is_remote_tracking_ref(Path::new(
"/repo/.git/refs/remotes/origin/main"
)));
assert!(is_remote_tracking_ref(Path::new(
"/repo/.git/refs/remotes/origin/feature/nested"
)));
assert!(!is_remote_tracking_ref(Path::new(
"/repo/.git/refs/remotes/origin"
)));
assert!(!is_remote_tracking_ref(Path::new(
"/repo/.git/worktrees/wt/refs/remotes/origin/main"
)));
assert!(!is_remote_tracking_ref(Path::new(
"/repo/.git/refs/heads/main"
)));

// Tracking-state files
assert!(is_tracking_state_git_file(Path::new("/repo/.git/HEAD")));
assert!(is_tracking_state_git_file(Path::new("/repo/.git/config")));
assert!(is_tracking_state_git_file(Path::new(
"/repo/.git/worktrees/wt/config.worktree"
)));
assert!(!is_tracking_state_git_file(Path::new(
"/repo/.git/refs/remotes/origin/main"
)));

// Common config
assert!(is_common_git_config(Path::new("/repo/.git/config")));
assert!(!is_common_git_config(Path::new(
"/repo/.git/worktrees/wt/config.worktree"
)));

// Test Windows-style paths (only on Windows, as path parsing is platform-specific)
#[cfg(windows)]
{
Expand Down
Loading
Loading