Skip to content
Merged
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
50 changes: 49 additions & 1 deletion crates/flare-git-core/src/worktree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,21 @@ pub fn already_isolated_for(branch: &str, repo_root: &Path) -> bool {
}
}

/// `true` if `worktree_path` already exists on disk and is itself a git
/// worktree checked out to `branch` -- the on-disk counterpart to
/// `already_isolated_for` above, for callers running from outside the
/// worktree (the daemon's normal case) rather than from inside it.
#[must_use]
fn worktree_already_checked_out(worktree_path: &Path, branch: &str) -> bool {
if !worktree_path.is_dir() {
return false;
}
match run_git_in(worktree_path, &["branch", "--show-current"]) {
Ok(b) => b == branch,
Err(_) => false,
}
}

Comment on lines +69 to +83

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT

git -C "$tmp" init -q
git -C "$tmp" config user.email test@example.com
git -C "$tmp" config user.name Test
printf root > "$tmp/root.txt"
git -C "$tmp" add root.txt
git -C "$tmp" commit -qm init

candidate="$tmp/.worktrees/task/1"
mkdir -p "$candidate"
git -C "$candidate" init -q
git -C "$candidate" config user.email test@example.com
git -C "$candidate" config user.name Test
printf nested > "$candidate/nested.txt"
git -C "$candidate" add nested.txt
git -C "$candidate" commit -qm init
git -C "$candidate" branch -M task/1

test "$(git -C "$candidate" branch --show-current)" = "task/1"
! git -C "$tmp" worktree list --porcelain | grep -Fq "$candidate"

Repository: getappz/agentflare

Length of output: 156


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- helper and caller ---'
sed -n '1,110p' crates/flare-git-core/src/worktree.rs
sed -n '220,270p' crates/flare-git-core/src/worktree.rs
printf '%s\n' '--- run_git_in and ownership-related calls ---'
rg -n -C 4 'fn run_git_in|worktree_already_checked_out|worktree list|worktree add|already_isolated_for' crates/flare-git-core/src/worktree.rs

Repository: getappz/agentflare

Length of output: 11950


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- shell module ---'
fd -i -t f 'shell' .
rg -n -C 8 'pub fn run_in|fn run_in|run_in_ok' crates
printf '%s\n' '--- relevant tests ---'
sed -n '1035,1140p' crates/flare-git-core/src/worktree.rs

Repository: getappz/agentflare

Length of output: 30758


Validate worktree ownership before reusing the path.

git branch --show-current only proves that Git reports the requested branch from worktree_path. It does not prove that worktree_path is registered under repo_root. An unrelated repository at that path can cause the caller to skip worktree creation and modify the wrong checkout.

Match the path and branch against git worktree list --porcelain from repo_root. Add a negative test for an unrelated repository at the target path.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/flare-git-core/src/worktree.rs` around lines 69 - 83, Update
worktree_already_checked_out to validate ownership using git worktree list
--porcelain executed from repo_root, matching both the canonical worktree_path
and branch before returning true; do not rely solely on git branch
--show-current. Adjust callers and related symbols as needed to provide
repo_root, and add a negative test covering an unrelated repository at the
target path.

/// Adds `.worktrees/` and `.cargo/` to this repo's LOCAL, untracked ignore
/// rules (`.git/info/exclude`) rather than the tracked `.gitignore` — a
/// claim should never create a commit in the caller's repository (would
Expand Down Expand Up @@ -222,7 +237,15 @@ pub fn create_worktree(
.join(".worktrees")
.join("task")
.join(item.sequence_id.to_string());
if already_isolated_for(&branch, repo_root) {
// Two ways a re-claim can find its own worktree already in place:
// `already_isolated_for` catches the recursive case (the calling
// process is itself already running from inside it), but the daemon's
// normal dispatch always calls this from the main repo root, so that
// check never fires there -- it needs the on-disk check too, or
// `git worktree add` below fails ("already exists") on a re-claim.
if already_isolated_for(&branch, repo_root)
|| worktree_already_checked_out(&worktree_path, &branch)
{
// Re-claiming an existing worktree: nothing to create, but still
// ensure its target dir is isolated (idempotent, no-op if present),
// and re-warn since the ambient env can still be shadowing it.
Expand Down Expand Up @@ -1081,6 +1104,31 @@ mod tests {
assert_eq!(checked_out, "task/1");
}

#[test]
fn create_worktree_reuses_an_existing_worktree_when_called_from_the_repo_root() {
// The daemon always calls create_worktree from the main repo root,
// never from inside the worktree itself, so already_isolated_for's
// "am I currently inside that worktree?" check (compares --git-dir
// vs --git-common-dir on repo_root) can never fire for a real
// re-claim -- it only ever returns true for the recursive case
// where a caller happens to already be cd'd into the worktree.
// Without an on-disk check, the second call falls through to
// `git worktree add`, which refuses to re-add a branch that's
// already checked out elsewhere ("bad git state?" in item #43/#30).
let repo = init_repo();
let item = test_item(1);
let target = resolve_default_branch(&repo.path);
let worktree_path = create_worktree(&item, &repo.path, &target, None).unwrap();

let result = create_worktree(&item, &repo.path, &target, None);

assert!(result.is_ok(), "{:?}", result.err());
assert_eq!(result.unwrap(), worktree_path);
assert!(worktree_path.exists());
let checked_out = run_git_in(&worktree_path, &["branch", "--show-current"]).unwrap();
assert_eq!(checked_out, "task/1");
}

#[test]
fn create_worktree_soft_fails_on_bad_git() {
let tmp = TempDir::new().unwrap();
Expand Down
Loading