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
25 changes: 18 additions & 7 deletions src/config/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,18 +255,29 @@ impl ProjectConfig {
repo: &crate::git::Repository,
write_hints: bool,
) -> Result<Option<Self>, ConfigError> {
let config_path = match repo
let (contents, config_path) = match repo
.project_config_path()
.map_err(|e| ConfigError(format!("Failed to get config path: {}", e)))?
{
Some(path) if path.exists() => path,
_ => return Ok(None),
Some(path) if path.exists() => {
// Load directly with toml crate to preserve insertion order
// (with preserve_order feature).
let contents = std::fs::read_to_string(&path)
.map_err(|e| ConfigError(format!("Failed to read config file: {}", e)))?;
(contents, path)
}
// No config resolved on disk. In a bare layout with the default
// branch checked out in no worktree, the parked primary worktree's
// files aren't the default branch's config — read the committed
// copy from the object store rather than silently dropping project
// config and every project hook (#3461). `config_path` is then a
// display-only revision spec, used only for diagnostics.
_ => match repo.default_branch_project_config_content() {
Some((contents, display_path)) => (contents, display_path),
None => return Ok(None),
},
};

// Load directly with toml crate to preserve insertion order (with preserve_order feature)
let contents = std::fs::read_to_string(&config_path)
.map_err(|e| ConfigError(format!("Failed to read config file: {}", e)))?;

// Check for deprecated template variables and create migration file if needed
// Only write migration file in main worktree, not linked worktrees
// emit_inline_warnings=true: print per-kind warnings inline during config load
Expand Down
79 changes: 76 additions & 3 deletions src/git/repository/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -636,8 +636,13 @@ impl Repository {
///
/// Otherwise: uses the current worktree when inside one (both normal and
/// bare repos). For bare repos at the bare root (outside any worktree),
/// falls back to the primary worktree. Returns `None` when no worktree can
/// be determined (bare repo with no linked worktrees).
/// falls back to the primary worktree. When the default branch is checked
/// out in no worktree (so `primary_worktree()` is `None`), there is no
/// on-disk path to return here — `ProjectConfig::load` reads the committed
/// default-branch config from the object store via
/// [`default_branch_project_config_content`](Self::default_branch_project_config_content)
/// so project config (and every project hook) isn't silently dropped while
/// the primary is parked on another branch (#3461).
///
/// "The current worktree" is whatever this `Repository` was rooted at, so
/// the answer to "which `.config/wt.toml` does a hook read" is decided by
Expand All @@ -662,7 +667,12 @@ impl Repository {
}

if self.is_bare().unwrap_or(false) {
// At bare repo root — use primary worktree
// At bare repo root — use the primary worktree (the one holding the
// default branch). When the default branch is checked out in no
// worktree, `primary_worktree()` is `None` and there is no on-disk
// path; `ProjectConfig::load` then reads the committed
// default-branch config from the object store via
// `default_branch_project_config_content` (#3461).
return Ok(self
.primary_worktree()?
.map(|p| p.join(".config").join("wt.toml")));
Expand All @@ -671,6 +681,69 @@ impl Repository {
Ok(None)
}

/// Content of the default branch's committed `.config/wt.toml`, read from
/// the object store via `git show`, for the one state where the on-disk
/// path can't supply it: a bare repo whose default branch is checked out in
/// no worktree.
///
/// In a bare layout the primary worktree normally holds the default branch,
/// so its on-disk `.config/wt.toml` *is* the default branch's project
/// config and [`project_config_path`](Self::project_config_path) resolves
/// it directly. When the primary worktree is transiently parked on another
/// branch (a common agent-driven workflow), no worktree exposes the default
/// branch's config on disk; returning nothing there would silently drop the
/// entire project config and every project hook (#3461). Reading the
/// committed copy from the object store restores it without depending on
/// which branch is checked out where — and, unlike scanning worktrees for
/// any `.config/wt.toml`, always reads the *default branch's* config rather
/// than whatever branch a worktree happens to be parked on.
///
/// Returns `None` cheaply — before touching the object store — for every
/// other repo shape (non-bare repos, and bare repos whose default branch is
/// checked out somewhere), so the common load path never pays for the extra
/// `git show`. Also returns `None` when the default branch ships no project
/// config (`git show` exits non-zero for a path absent from the tree),
/// matching the no-config case.
///
/// This is a best-effort resolver: the `is_bare` / `primary_worktree`
/// checks re-run calls that `project_config_path` already made
/// successfully on this path, and a `git show` failure degrades to "no
/// project config" (no hooks) rather than surfacing — the same
/// error-swallowing shape `alias.rs` uses for config resolution, and safe
/// because the fallback only ever adds hooks, never risks data.
///
/// The read uses `HEAD` rather than a resolved branch name: at the bare
/// root the repo's own `HEAD` is a symbolic ref to the default branch (it
/// is what local default-branch inference reads), so `HEAD:.config/wt.toml`
/// reads the default branch's committed config, always resolves in a bare
/// repo, and needs no separate branch lookup. The returned `PathBuf` is a
/// display-only label of the form `HEAD:.config/wt.toml` — a git revision
/// spec, not a filesystem path. Nothing is read from or written to it; it
/// only annotates diagnostics (e.g. a parse error) with the source.
pub fn default_branch_project_config_content(&self) -> Option<(String, PathBuf)> {
if !self.is_bare().unwrap_or(false) {
return None;
}
// Only the "default branch checked out nowhere" state needs this; when
// the default branch is checked out somewhere, its on-disk path already
// resolved (and stays authoritative — e.g. a deletion there wins).
if self.primary_worktree().ok().flatten().is_some() {
return None;
}

let spec = "HEAD:.config/wt.toml";
match self.run_command_output(&["show", spec]) {
Ok(output) if output.status.success() => Some((
String::from_utf8_lossy(&output.stdout).into_owned(),
PathBuf::from(spec),
)),
// A non-zero exit (typically 128, path absent from the tree) or a
// rare spawn failure: treat as "no project config", the same result
// as an absent file on disk.
_ => None,
}
}

/// Load the project configuration (.config/wt.toml) if it exists.
///
/// Result is cached in the repository's shared cache (same for all clones).
Expand Down
166 changes: 166 additions & 0 deletions tests/integration_tests/bare_repository.rs
Original file line number Diff line number Diff line change
Expand Up @@ -802,6 +802,172 @@ fn test_bare_repo_project_config_found_from_bare_root() {
);
}

#[test]
fn test_bare_repo_project_config_found_when_primary_on_non_default_branch() {
// Regression test for #3461: the project config must still be found from the
// bare root when the primary worktree is temporarily checked out to a
// *non-default* branch. This is the gap left by #1691's fix —
// `project_config_path()` locates the primary worktree via
// `primary_worktree()`, which looks it up by "which worktree holds the
// default branch". When an agent-driven workflow briefly checks out a PR
// branch in the primary worktree, no worktree holds the default branch, so
// the project source was dropped silently and NO project hooks fired.
//
// The fix reads the committed default-branch config from the object store
// (`git show <default>:.config/wt.toml`), so this test also pins that it
// reads the *default branch's* config, not whatever the parked worktree
// happens to have on disk: after parking the primary off-branch, the
// working-tree config is overwritten with a divergent hook that must NOT
// run.
let test = BareRepoTest::new();

// Create main worktree (the primary worktree for bare repos)
let main_worktree = test.create_worktree("main", "main");
test.commit_in(&main_worktree, "Initial commit");

// Place project config in the primary worktree's .config/wt.toml
let config_dir = main_worktree.join(".config");
fs::create_dir_all(&config_dir).unwrap();

// Marker written by the default branch's committed hook (the one that
// must run), plus a marker for a divergent on-disk hook that must not.
let marker_path = test.bare_repo_path().join("hook-ran-off-branch.marker");
let marker_str = marker_path.to_str().unwrap().replace('\\', "/");
let stale_marker_path = test.bare_repo_path().join("hook-ran-stale.marker");
let stale_marker_str = stale_marker_path.to_str().unwrap().replace('\\', "/");
fs::write(
config_dir.join("wt.toml"),
format!("post-start = \"echo hook-executed > '{}'\"\n", marker_str),
)
.unwrap();

// Commit the config so it's part of the worktree
let output = test
.git_command(&main_worktree)
.args(["add", ".config/wt.toml"])
.run()
.unwrap();
assert!(output.status.success());
test.commit_in(&main_worktree, "Add project config");

// Move the primary worktree off the default branch, so no worktree holds
// `main`. This is the exact state that triggers the regression.
let output = test
.git_command(&main_worktree)
.args(["checkout", "-b", "feature-x"])
.run()
.unwrap();
assert!(
output.status.success(),
"checkout -b feature-x failed: {}",
String::from_utf8_lossy(&output.stderr)
);

// Overwrite the parked worktree's on-disk config with a divergent hook,
// uncommitted. If resolution read the parked worktree's files instead of
// the default branch's committed tree, this stale hook would run.
fs::write(
config_dir.join("wt.toml"),
format!("post-start = \"echo stale > '{}'\"\n", stale_marker_str),
)
.unwrap();

// Now run `wt switch --create test-repro` from the bare repo root.
let (cd_path, exec_path, _guard) = directive_files();
let mut cmd = wt_command();
test.configure_wt_cmd(&mut cmd);
configure_directive_files(&mut cmd, &cd_path, &exec_path);
cmd.args(["switch", "--create", "test-repro", "--yes"])
.current_dir(test.bare_repo_path());

let output = cmd.output().unwrap();

if !output.status.success() {
panic!(
"wt switch failed:\nstdout: {}\nstderr: {}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
}

// The default branch's committed hook must run even though the primary
// worktree is parked on a non-default branch.
wait_for_file_content(&marker_path);
let content = fs::read_to_string(&marker_path).unwrap();
assert!(
content.contains("hook-executed"),
"Project hook must run from the bare root even when the primary worktree \
is on a non-default branch (#3461). Marker file content: {:?}",
content
);

// The parked worktree's divergent on-disk hook must NOT run — resolution
// reads the default branch's committed config, not the checked-out files.
assert!(
!stale_marker_path.exists(),
"Resolution must read the default branch's committed config via `git \
show`, not the parked worktree's on-disk file"
);
}

#[test]
fn test_bare_repo_no_project_config_when_primary_off_branch_and_none_present() {
// Companion to the #3461 fix: the object-store fallback that reads
// `git show <default>:.config/wt.toml` when the default branch is checked
// out nowhere must not conjure a config that doesn't exist. With the primary
// off the default branch and no config committed on the default branch,
// `git show` exits non-zero and resolution stays `None` — the command
// succeeds and no project hook runs.
let test = BareRepoTest::new();

// Create main worktree (the primary worktree for bare repos) — no config
let main_worktree = test.create_worktree("main", "main");
test.commit_in(&main_worktree, "Initial commit");

// Move the primary worktree off the default branch.
let output = test
.git_command(&main_worktree)
.args(["checkout", "-b", "feature-x"])
.run()
.unwrap();
assert!(
output.status.success(),
"checkout -b feature-x failed: {}",
String::from_utf8_lossy(&output.stderr)
);

// Run `wt switch --create foo` from the bare repo root. With no project
// config anywhere, it should still succeed.
let (cd_path, exec_path, _guard) = directive_files();
let mut cmd = wt_command();
test.configure_wt_cmd(&mut cmd);
configure_directive_files(&mut cmd, &cd_path, &exec_path);
cmd.args(["switch", "--create", "foo", "--yes"])
.current_dir(test.bare_repo_path());

let output = cmd.output().unwrap();
assert!(
output.status.success(),
"wt switch should succeed with no project config:\nstdout: {}\nstderr: {}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);

// No config exists, so no worktree should be reported as carrying one and
// no project hook can run. `wt config show` from the bare root confirms the
// fallback found nothing rather than resolving a phantom config.
let mut show = wt_command();
test.configure_wt_cmd(&mut show);
show.args(["config", "show"])
.current_dir(test.bare_repo_path());
let show_out = show.output().unwrap();
let stdout = String::from_utf8_lossy(&show_out.stdout);
assert!(
!stdout.contains("[pre-start]") && !stdout.contains("[post-start]"),
"no project hooks should be resolved when no config exists:\n{stdout}"
);
}

#[test]
fn test_bare_repo_project_config_found_with_dash_c_flag() {
// Regression test for #1691 (comment): project config in the primary worktree
Expand Down
Loading