From 6f0b49959462d1635546d4dfcb3bd7e81b7d9138 Mon Sep 17 00:00:00 2001 From: Dylan Date: Fri, 22 Aug 2025 13:54:51 -0700 Subject: [PATCH] [config] Detect git worktrees for project trust (#2585) ## Summary When resolving our current directory as a project, we want to be a little bit more clever: 1. If we're in a sub-directory of a git repo, resolve our project against the root of the git repo 2. If we're in a git worktree, resolve the project against the root of the git repo ## Testing - [x] Added unit tests - [x] Confirmed locally with a git worktree (the one i was using for this feature) --- codex-rs/core/src/config.rs | 26 ++++- codex-rs/core/src/git_info.rs | 107 ++++++++++++++++++ .../tui/src/onboarding/trust_directory.rs | 8 +- 3 files changed, 134 insertions(+), 7 deletions(-) diff --git a/codex-rs/core/src/config.rs b/codex-rs/core/src/config.rs index 988d4767..2cde43ae 100644 --- a/codex-rs/core/src/config.rs +++ b/codex-rs/core/src/config.rs @@ -7,6 +7,7 @@ use crate::config_types::ShellEnvironmentPolicyToml; use crate::config_types::Tui; use crate::config_types::UriBasedFileOpener; use crate::config_types::Verbosity; +use crate::git_info::resolve_root_git_project_for_trust; use crate::model_family::ModelFamily; use crate::model_family::find_family_for_model; use crate::model_provider_info::ModelProviderInfo; @@ -503,10 +504,27 @@ impl ConfigToml { pub fn is_cwd_trusted(&self, resolved_cwd: &Path) -> bool { let projects = self.projects.clone().unwrap_or_default(); - projects - .get(&resolved_cwd.to_string_lossy().to_string()) - .map(|p| p.trust_level.clone().unwrap_or("".to_string()) == "trusted") - .unwrap_or(false) + let is_path_trusted = |path: &Path| { + let path_str = path.to_string_lossy().to_string(); + projects + .get(&path_str) + .map(|p| p.trust_level.as_deref() == Some("trusted")) + .unwrap_or(false) + }; + + // Fast path: exact cwd match + if is_path_trusted(resolved_cwd) { + return true; + } + + // If cwd lives inside a git worktree, check whether the root git project + // (the primary repository working directory) is trusted. This lets + // worktrees inherit trust from the main project. + if let Some(root_project) = resolve_root_git_project_for_trust(resolved_cwd) { + return is_path_trusted(&root_project); + } + + false } pub fn get_config_profile( diff --git a/codex-rs/core/src/git_info.rs b/codex-rs/core/src/git_info.rs index 5f25d8fe..91634ab9 100644 --- a/codex-rs/core/src/git_info.rs +++ b/codex-rs/core/src/git_info.rs @@ -1,5 +1,6 @@ use std::collections::HashSet; use std::path::Path; +use std::path::PathBuf; use codex_protocol::mcp_protocol::GitSha; use futures::future::join_all; @@ -425,6 +426,38 @@ async fn diff_against_sha(cwd: &Path, sha: &GitSha) -> Option { Some(diff) } +/// Resolve the path that should be used for trust checks. Similar to +/// `[utils::is_inside_git_repo]`, but resolves to the root of the main +/// repository. Handles worktrees. +pub fn resolve_root_git_project_for_trust(cwd: &Path) -> Option { + let base = if cwd.is_dir() { cwd } else { cwd.parent()? }; + + // TODO: we should make this async, but it's primarily used deep in + // callstacks of sync code, and should almost always be fast + let git_dir_out = std::process::Command::new("git") + .args(["rev-parse", "--git-common-dir"]) + .current_dir(base) + .output() + .ok()?; + if !git_dir_out.status.success() { + return None; + } + let git_dir_s = String::from_utf8(git_dir_out.stdout) + .ok()? + .trim() + .to_string(); + + let git_dir_path_raw = if Path::new(&git_dir_s).is_absolute() { + PathBuf::from(&git_dir_s) + } else { + base.join(&git_dir_s) + }; + + // Normalize to handle macOS /var vs /private/var and resolve ".." segments. + let git_dir_path = std::fs::canonicalize(&git_dir_path_raw).unwrap_or(git_dir_path_raw); + git_dir_path.parent().map(Path::to_path_buf) +} + #[cfg(test)] mod tests { use super::*; @@ -732,6 +765,80 @@ mod tests { assert_eq!(state.sha, GitSha::new(&remote_sha)); } + #[test] + fn resolve_root_git_project_for_trust_returns_none_outside_repo() { + let tmp = TempDir::new().expect("tempdir"); + assert!(resolve_root_git_project_for_trust(tmp.path()).is_none()); + } + + #[tokio::test] + async fn resolve_root_git_project_for_trust_regular_repo_returns_repo_root() { + let temp_dir = TempDir::new().expect("Failed to create temp dir"); + let repo_path = create_test_git_repo(&temp_dir).await; + let expected = std::fs::canonicalize(&repo_path).unwrap().to_path_buf(); + + assert_eq!( + resolve_root_git_project_for_trust(&repo_path), + Some(expected.clone()) + ); + let nested = repo_path.join("sub/dir"); + std::fs::create_dir_all(&nested).unwrap(); + assert_eq!( + resolve_root_git_project_for_trust(&nested), + Some(expected.clone()) + ); + } + + #[tokio::test] + async fn resolve_root_git_project_for_trust_detects_worktree_and_returns_main_root() { + let temp_dir = TempDir::new().expect("Failed to create temp dir"); + let repo_path = create_test_git_repo(&temp_dir).await; + + // Create a linked worktree + let wt_root = temp_dir.path().join("wt"); + let _ = std::process::Command::new("git") + .args([ + "worktree", + "add", + wt_root.to_str().unwrap(), + "-b", + "feature/x", + ]) + .current_dir(&repo_path) + .output() + .expect("git worktree add"); + + let expected = std::fs::canonicalize(&repo_path).ok(); + let got = resolve_root_git_project_for_trust(&wt_root) + .and_then(|p| std::fs::canonicalize(p).ok()); + assert_eq!(got, expected); + let nested = wt_root.join("nested/sub"); + std::fs::create_dir_all(&nested).unwrap(); + let got_nested = + resolve_root_git_project_for_trust(&nested).and_then(|p| std::fs::canonicalize(p).ok()); + assert_eq!(got_nested, expected); + } + + #[test] + fn resolve_root_git_project_for_trust_non_worktrees_gitdir_returns_none() { + let tmp = TempDir::new().expect("tempdir"); + let proj = tmp.path().join("proj"); + std::fs::create_dir_all(proj.join("nested")).unwrap(); + + // `.git` is a file but does not point to a worktrees path + std::fs::write( + proj.join(".git"), + format!( + "gitdir: {}\n", + tmp.path().join("some/other/location").display() + ), + ) + .unwrap(); + + assert!(resolve_root_git_project_for_trust(&proj).is_none()); + assert!(resolve_root_git_project_for_trust(&proj.join("nested")).is_none()); + } + #[tokio::test] async fn test_get_git_working_tree_state_unpushed_commit() { let temp_dir = TempDir::new().expect("Failed to create temp dir"); diff --git a/codex-rs/tui/src/onboarding/trust_directory.rs b/codex-rs/tui/src/onboarding/trust_directory.rs index 7e41ae05..e775a829 100644 --- a/codex-rs/tui/src/onboarding/trust_directory.rs +++ b/codex-rs/tui/src/onboarding/trust_directory.rs @@ -1,6 +1,7 @@ use std::path::PathBuf; use codex_core::config::set_project_trusted; +use codex_core::git_info::resolve_root_git_project_for_trust; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; use ratatui::buffer::Buffer; @@ -144,10 +145,11 @@ impl StepStateProvider for TrustDirectoryWidget { impl TrustDirectoryWidget { fn handle_trust(&mut self) { - if let Err(e) = set_project_trusted(&self.codex_home, &self.cwd) { + let target = + resolve_root_git_project_for_trust(&self.cwd).unwrap_or_else(|| self.cwd.clone()); + if let Err(e) = set_project_trusted(&self.codex_home, &target) { tracing::error!("Failed to set project trusted: {e:?}"); - self.error = Some(e.to_string()); - // self.error = Some("Failed to set project trusted".to_string()); + self.error = Some(format!("Failed to set trust for {}: {e}", target.display())); } self.selection = Some(TrustDirectorySelection::Trust);