Skip to content

Commit 2da78fb

Browse files
committed
feat: Improve ab dbg remove with absolute path support and worktree cleanup
Accept absolute source paths in `ab dbg remove` (e.g., `ab dbg remove /home/user/repos/myproject`), bypassing discovery so workspaces can be managed even when `base_repo_dir` is unset. Paths are canonicalized with fallback to `std::path::absolute` for deleted repos. Clean up git worktree metadata on removal via `git worktree remove --force` before deleting workspace directories. When the source repo is missing, print a note suggesting `git worktree prune`. Extract shared `cleanup_git_worktrees` helper used by both `remove_repo` and the `--unresolved` path. Also: use `WorkspaceType` for type-safe dispatch in `remove_repo` instead of string comparison, update migration docs to mention absolute path support, and add tests for malformed `.git` files and short gitdir paths.
1 parent ab4d373 commit 2da78fb

4 files changed

Lines changed: 260 additions & 12 deletions

File tree

ab/src/main.rs

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,10 @@ use agent_box_common::config::{
33
validate_config_or_err,
44
};
55
use agent_box_common::display::info;
6-
use agent_box_common::path::{WorkspaceStatus, WorkspaceType, scan_workspaces};
6+
use agent_box_common::path::{RepoIdentifier, WorkspaceStatus, WorkspaceType, scan_workspaces};
77
use agent_box_common::repo::{
8-
find_git_workdir, locate_repo, new_workspace, remove_repo, resolve_repo_id,
8+
cleanup_git_worktrees, find_git_workdir, locate_repo, new_workspace, remove_repo,
9+
resolve_repo_id,
910
};
1011
use clap::{Parser, Subcommand};
1112
use eyre::Result;
@@ -163,7 +164,7 @@ enum DbgCommands {
163164
},
164165
/// Remove all workspaces for a given repo ID
165166
Remove {
166-
/// Repository identifier (e.g., "fr/agent-box" or "agent-box")
167+
/// Repository identifier or absolute source path (e.g., "agent-box", "fr/agent-box", or "/home/user/repos/myproject")
167168
#[arg(required_unless_present = "unresolved")]
168169
repo: Option<String>,
169170
/// Show what would be deleted without actually deleting
@@ -481,6 +482,12 @@ fn run() -> eyre::Result<()> {
481482
let dir = ws.workspace_dir(&config);
482483
if dir.exists() {
483484
println!("Removing: {}", dir.display());
485+
// Attempt git worktree cleanup before deleting the directory.
486+
// For unresolved workspaces the source repo may still exist
487+
// (e.g., workspace is "unresolved" due to a base_repo_dir change).
488+
if ws.workspace_type == WorkspaceType::Git {
489+
cleanup_git_worktrees(&dir);
490+
}
484491
if let Err(e) = std::fs::remove_dir_all(&dir) {
485492
eprintln!(" Failed to remove {}: {e}", dir.display());
486493
return Err(e.into());
@@ -495,7 +502,20 @@ fn run() -> eyre::Result<()> {
495502
// clap's required_unless_present guarantees repo is Some
496503
// here, but unwrap with a message for safety.
497504
let repo = repo.expect("repo is required by clap unless --unresolved is set");
498-
let repo_id = locate_repo(&config, Some(&repo))?;
505+
let repo_id = if repo.starts_with('/') {
506+
// Absolute path: resolve directly without discovery.
507+
// This allows removing workspaces even when discovery
508+
// is not configured (e.g., base_repo_dir defaults to "/").
509+
// Canonicalize first to resolve symlinks and ".." components;
510+
// fall back to std::path::absolute if the path doesn't exist
511+
// (the repo may have been deleted).
512+
let path = std::path::Path::new(&repo);
513+
let clean_path =
514+
path.canonicalize().or_else(|_| std::path::absolute(path))?;
515+
RepoIdentifier::from_repo_path(&config, &clean_path)?
516+
} else {
517+
locate_repo(&config, Some(&repo))?
518+
};
499519

500520
// Show what will be removed (always, even if --force is used)
501521
remove_repo(&config, &repo_id, true)?;

common/src/repo.rs

Lines changed: 234 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -355,17 +355,114 @@ fn get_session_name(session_name: Option<&str>) -> Result<String> {
355355
}
356356
}
357357

358-
/// Remove all workspaces for a given repo ID
358+
/// Remove a git worktree by running `git worktree remove --force`.
359+
///
360+
/// Parses the session's `.git` file to locate the source repo, then
361+
/// delegates to `git worktree remove`. Returns:
362+
/// - `Ok(true)`: worktree was successfully removed.
363+
/// - `Ok(false)`: not applicable (no `.git` file, unparseable gitdir path, or source repo missing).
364+
/// - `Err`: I/O failure, malformed `.git` file, or `git worktree remove` command failed.
365+
pub fn remove_git_worktree(session_path: &Path) -> Result<bool> {
366+
// Parse the .git file to find the source repo's git dir.
367+
let dot_git = session_path.join(".git");
368+
if !dot_git.is_file() {
369+
return Ok(false);
370+
}
371+
let content = std::fs::read_to_string(&dot_git)?;
372+
// Format: "gitdir: /path/to/repo/.git/worktrees/session-name"
373+
let gitdir = content
374+
.strip_prefix("gitdir: ")
375+
.map(|s| s.trim())
376+
.ok_or_else(|| eyre!("unexpected .git file format in {}", dot_git.display()))?;
377+
378+
// The source repo's .git dir is two levels up from the worktrees entry:
379+
// /repo/.git/worktrees/session -> /repo/.git
380+
let git_dir = Path::new(gitdir)
381+
.parent() // /repo/.git/worktrees
382+
.and_then(|p| p.parent()); // /repo/.git
383+
384+
let Some(git_dir) = git_dir else {
385+
return Ok(false);
386+
};
387+
388+
if !git_dir.exists() {
389+
// Source repo is gone; cannot clean up worktree metadata.
390+
return Ok(false);
391+
}
392+
393+
// Run git worktree remove. Use --force since the worktree may
394+
// have uncommitted changes (we are removing it regardless).
395+
let output = std::process::Command::new("git")
396+
.args(["worktree", "remove", "--force"])
397+
.arg(session_path)
398+
.current_dir(git_dir.parent().unwrap_or(git_dir))
399+
.output()?;
400+
401+
if output.status.success() {
402+
Ok(true)
403+
} else {
404+
let stderr = String::from_utf8_lossy(&output.stderr);
405+
Err(eyre!("git worktree remove failed: {}", stderr.trim()))
406+
}
407+
}
408+
409+
/// Attempt to clean up git worktree metadata for all sessions in a directory.
410+
///
411+
/// Iterates subdirectories, checks for `.git` files (indicating linked worktrees),
412+
/// and calls `remove_git_worktree` on each. Returns `true` if all sessions were
413+
/// cleaned up successfully, `false` if any failed (source repo missing, git error, etc.).
414+
/// Prints per-session status messages.
415+
pub fn cleanup_git_worktrees(workspace_dir: &Path) -> bool {
416+
let Ok(entries) = std::fs::read_dir(workspace_dir) else {
417+
return true; // Nothing to clean up if we can't read the directory.
418+
};
419+
let mut all_ok = true;
420+
for entry in entries.flatten() {
421+
let session_path = entry.path();
422+
if session_path.join(".git").is_file() {
423+
match remove_git_worktree(&session_path) {
424+
Ok(true) => println!(" Pruned git worktree: {}", session_path.display()),
425+
Ok(false) => {
426+
println!(
427+
" Note: could not clean up worktree metadata for {}",
428+
session_path.display()
429+
);
430+
println!(
431+
" If the source repo still exists, run `git worktree prune` in the source repo."
432+
);
433+
all_ok = false;
434+
}
435+
Err(e) => {
436+
eprintln!(
437+
" Warning: failed to prune worktree {}: {e}",
438+
session_path.display()
439+
);
440+
all_ok = false;
441+
}
442+
}
443+
}
444+
}
445+
all_ok
446+
}
447+
448+
/// Remove all workspaces for a given repo ID.
449+
///
450+
/// For git workspaces, attempts to clean up worktree metadata via
451+
/// `git worktree remove --force` for each session before deleting the
452+
/// directory. If the source repo is missing, prints a note suggesting
453+
/// `git worktree prune`.
359454
pub fn remove_repo(config: &Config, repo_id: &RepoIdentifier, dry_run: bool) -> Result<()> {
360-
let paths_to_remove: Vec<(&str, PathBuf)> = vec![
455+
let paths_to_remove: Vec<(WorkspaceType, &str, PathBuf)> = vec![
361456
(
457+
WorkspaceType::Git,
362458
"Git worktrees",
363459
config
364460
.workspace_dir
365461
.join(WorkspaceType::Git.as_str())
366462
.join(repo_id.relative_path()),
367463
),
368464
(
465+
WorkspaceType::Jj,
369466
"JJ workspaces",
370467
config
371468
.workspace_dir
@@ -378,10 +475,10 @@ pub fn remove_repo(config: &Config, repo_id: &RepoIdentifier, dry_run: bool) ->
378475
println!("\nThe following directories will be removed:");
379476

380477
let mut found_any = false;
381-
for (label, path) in &paths_to_remove {
478+
for (_, label, path) in &paths_to_remove {
382479
if path.exists() {
383480
found_any = true;
384-
println!(" [{}] {}", label, path.display());
481+
println!(" [{label}] {}", path.display());
385482
}
386483
}
387484

@@ -396,9 +493,15 @@ pub fn remove_repo(config: &Config, repo_id: &RepoIdentifier, dry_run: bool) ->
396493
}
397494

398495
// Remove all existing directories
399-
for (label, path) in &paths_to_remove {
496+
for (wtype, label, path) in &paths_to_remove {
400497
if path.exists() {
401-
println!("\nRemoving {}: {}", label, path.display());
498+
println!("\nRemoving {label}: {}", path.display());
499+
500+
// Clean up git worktree metadata for each session before deleting.
501+
if *wtype == WorkspaceType::Git {
502+
cleanup_git_worktrees(path);
503+
}
504+
402505
std::fs::remove_dir_all(path)?;
403506
println!(" ✓ Removed");
404507
}
@@ -689,4 +792,129 @@ mod tests {
689792
// Cleanup
690793
std::fs::remove_dir_all(&tmp).ok();
691794
}
795+
796+
#[test]
797+
fn test_remove_git_worktree_cleans_metadata() {
798+
// Create a real git repo, add a worktree, then call
799+
// remove_git_worktree and verify the worktree entry is gone.
800+
let tmp = temp_test_dir("rm-worktree");
801+
let repo_dir = tmp.join("my-repo");
802+
let worktree_dir = tmp.join("my-worktree");
803+
git_init_with_commit(&repo_dir);
804+
805+
// Create a linked worktree
806+
let output = Command::new("git")
807+
.args([
808+
"worktree",
809+
"add",
810+
worktree_dir.to_str().unwrap(),
811+
"-b",
812+
"rm-test-branch",
813+
])
814+
.current_dir(&repo_dir)
815+
.output()
816+
.unwrap();
817+
assert!(
818+
output.status.success(),
819+
"git worktree add failed: {}",
820+
String::from_utf8_lossy(&output.stderr)
821+
);
822+
823+
// Verify the worktree metadata exists before removal
824+
let worktree_meta = repo_dir.join(".git").join("worktrees").join("my-worktree");
825+
assert!(
826+
worktree_meta.exists(),
827+
"worktree metadata should exist before removal"
828+
);
829+
830+
// Call remove_git_worktree
831+
let result = remove_git_worktree(&worktree_dir).unwrap();
832+
assert!(result, "remove_git_worktree should return true on success");
833+
834+
// The worktree directory should be removed by git worktree remove
835+
assert!(
836+
!worktree_dir.exists(),
837+
"worktree directory should be removed"
838+
);
839+
840+
// The .git/worktrees entry should also be gone
841+
assert!(
842+
!worktree_meta.exists(),
843+
"worktree metadata should be removed from .git/worktrees/"
844+
);
845+
846+
// Cleanup
847+
std::fs::remove_dir_all(&tmp).ok();
848+
}
849+
850+
#[test]
851+
fn test_remove_git_worktree_missing_source() {
852+
// Create a fake .git file pointing to a nonexistent gitdir.
853+
// remove_git_worktree should return Ok(false) without erroring.
854+
let tmp = temp_test_dir("rm-worktree-missing");
855+
let session_dir = tmp.join("fake-session");
856+
std::fs::create_dir_all(&session_dir).unwrap();
857+
std::fs::write(
858+
session_dir.join(".git"),
859+
"gitdir: /nonexistent/repo/.git/worktrees/fake-session",
860+
)
861+
.unwrap();
862+
863+
let result = remove_git_worktree(&session_dir).unwrap();
864+
assert!(!result, "should return false when source repo is missing");
865+
866+
// Cleanup
867+
std::fs::remove_dir_all(&tmp).ok();
868+
}
869+
870+
#[test]
871+
fn test_remove_git_worktree_not_a_worktree() {
872+
// Call on a directory without a .git file.
873+
// remove_git_worktree should return Ok(false).
874+
let tmp = temp_test_dir("rm-worktree-none");
875+
let plain_dir = tmp.join("plain-dir");
876+
std::fs::create_dir_all(&plain_dir).unwrap();
877+
878+
let result = remove_git_worktree(&plain_dir).unwrap();
879+
assert!(!result, "should return false for non-worktree directory");
880+
881+
// Cleanup
882+
std::fs::remove_dir_all(&tmp).ok();
883+
}
884+
885+
#[test]
886+
fn test_remove_git_worktree_malformed_git_file() {
887+
// A .git file without the "gitdir: " prefix should return Err.
888+
let tmp = temp_test_dir("rm-worktree-malformed");
889+
let session_dir = tmp.join("bad-session");
890+
std::fs::create_dir_all(&session_dir).unwrap();
891+
std::fs::write(session_dir.join(".git"), "garbage content").unwrap();
892+
893+
let result = remove_git_worktree(&session_dir);
894+
assert!(result.is_err(), "should return Err for malformed .git file");
895+
let err_msg = format!("{}", result.unwrap_err());
896+
assert!(
897+
err_msg.contains("unexpected .git file format"),
898+
"error should mention unexpected format, got: {err_msg}"
899+
);
900+
901+
// Cleanup
902+
std::fs::remove_dir_all(&tmp).ok();
903+
}
904+
905+
#[test]
906+
fn test_remove_git_worktree_short_gitdir_path() {
907+
// A .git file with a gitdir that has fewer than 2 parent components
908+
// should return Ok(false) since we can't determine the source repo.
909+
let tmp = temp_test_dir("rm-worktree-short");
910+
let session_dir = tmp.join("short-session");
911+
std::fs::create_dir_all(&session_dir).unwrap();
912+
std::fs::write(session_dir.join(".git"), "gitdir: /foo").unwrap();
913+
914+
let result = remove_git_worktree(&session_dir).unwrap();
915+
assert!(!result, "should return false for short gitdir path");
916+
917+
// Cleanup
918+
std::fs::remove_dir_all(&tmp).ok();
919+
}
692920
}

docs/src/reference/agent-box/cli.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ Remove all workspaces for a given repo ID
176176
Usage: ab dbg remove [OPTIONS] [REPO]
177177
178178
Arguments:
179-
[REPO] Repository identifier (e.g., "fr/agent-box" or "agent-box")
179+
[REPO] Repository identifier or absolute source path (e.g., "agent-box", "fr/agent-box", or "/home/user/repos/myproject")
180180
181181
Options:
182182
--dry-run Show what would be deleted without actually deleting

docs/src/reference/agent-box/config.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ If you remove `base_repo_dir` from your configuration (or change its value), exi
224224
To inspect and clean up:
225225

226226
1. Run `ab dbg list --unresolved` to see which workspaces no longer resolve to a source repository.
227-
2. Run `ab dbg remove --unresolved` to delete the orphaned workspace directories.
227+
2. Run `ab dbg remove --unresolved` to delete all orphaned workspace directories, or `ab dbg remove /absolute/path/to/repo` to remove a specific one by its source path.
228228

229229
Workspaces created via symlinked paths before canonicalization was introduced may also appear unresolved, since the canonical path differs from the original symlink path.
230230

0 commit comments

Comments
 (0)