Skip to content

feat: Default base_repo_dir to "/", fix linked worktree resolution#23

Open
nothingnesses wants to merge 5 commits into0xferrous:mainfrom
nothingnesses:#16
Open

feat: Default base_repo_dir to "/", fix linked worktree resolution#23
nothingnesses wants to merge 5 commits into0xferrous:mainfrom
nothingnesses:#16

Conversation

@nothingnesses
Copy link
Copy Markdown
Contributor

@nothingnesses nothingnesses commented Mar 20, 2026

Implement Phase 1 of the base_repo_dir removal plan (fixes #16):

  • Default base_repo_dir to "/" so session mode works for repos anywhere on the filesystem without upfront configuration.
  • Track whether base_repo_dir was explicitly set via figment.find_value to distinguish user config from the serde default.
  • Rewrite discover_repo_ids to only scan when safe (explicit non-"/" base_repo_dir), returning an error with config guidance otherwise.
  • Fix linked worktree resolution: find_git_root_from resolves linked worktrees to the main repo root via gix::open on common_dir.
  • Add find_git_workdir for local mode (no worktree resolution, no canonicalization).
  • Fix display.rs to use find_git_root_from, add repo identity header.
  • Add ab dbg list to scan and display workspace groups with status.
  • Add ab dbg remove --unresolved with dry-run, force, and confirmation warning about base_repo_dir changes.
  • Add ab new bare name hint when positional arg has no path separator.
  • Canonicalize repo paths in find_git_root_from and discover_repos_in_dir with warn-and-skip on failure.
  • Add WorkspaceType::as_str, WorkspaceStatus::as_str, and WorkspaceInfo::workspace_dir to reduce duplication.
  • Regenerate CLI reference docs including ab dbg subcommands.
  • Filter tracing log lines from CLI doc generator output.
  • Add migration documentation to config.md.
  • Update first-run, overview, and workflow docs.
  • Update .gitignore to include .direnv/, .claude/ and .playwright-mcp/.

Implement [Phase 1](https://github.com/nothingnesses/agent-box/blob/e5c0696569ef64f0b25702b124cd515018dd13dd/plan-issue-16.md) of the `base_repo_dir` removal plan (issue 0xferrous#16):

- Default `base_repo_dir` to `"/"` so session mode works for repos
  anywhere on the filesystem without upfront configuration.
- Track whether `base_repo_dir` was explicitly set via
  `figment.find_value` to distinguish user config from the serde
  default.
- Rewrite `discover_repo_ids` to only scan when safe (explicit non-`"/"`
  `base_repo_dir`), returning an error with config guidance otherwise.
- Fix linked worktree resolution: `find_git_root_from` resolves linked
  worktrees to the main repo root via `gix::open` on `common_dir`.
- Add `find_git_workdir` for local mode (no worktree resolution, no
  canonicalization).
- Fix `display.rs` to use `find_git_root_from`, add repo identity
  header.
- Add `ab dbg list` to scan and display workspace groups with status.
- Add `ab dbg remove --unresolved` with dry-run, force, and
  confirmation warning about `base_repo_dir` changes.
- Add `ab new` bare name hint when positional arg has no path separator.
- Canonicalize repo paths in `find_git_root_from` and
  `discover_repos_in_dir` with warn-and-skip on failure.
- Add `WorkspaceType::as_str`, `WorkspaceStatus::as_str`, and
  `WorkspaceInfo::workspace_dir` to reduce duplication.
- Regenerate CLI reference docs including `ab dbg` subcommands.
- Filter tracing log lines from CLI doc generator output.
- Add migration documentation to `config.md`.
- Update first-run, overview, and workflow docs.
- Update `.gitignore` to include `.direnv/`, `.claude/` and
  `.playwright-mcp/`.
@nothingnesses
Copy link
Copy Markdown
Contributor Author

nothingnesses commented Mar 20, 2026

Manual test findings for ab4d373

Date: 2026-03-20

Tests are detailed here.

Test 3: Remove base_repo_dir, create workspace

Result: PASS (workspace creation), FAIL (cleanup)

Workspace created correctly at {workspace_dir}/git/home/jessea/Documents/projects/agent-box/manual-test/. However, ab dbg remove agent-box fails with "no repository discovery directories configured" because the remove subcommand routes the repo argument through locate_repo -> discover_repo_ids, which requires configured discovery directories.

Passing a full relative path (ab dbg remove home/jessea/Documents/projects/agent-box) also fails because it still goes through locate_repo.

Finding: When base_repo_dir is unset (defaulting to /), there is no way to manage workspaces by name using ab dbg remove <name>. The only workarounds are:

  1. Restore base_repo_dir in config, then remove.
  2. Use ab dbg remove --unresolved (only works if the workspace appears unresolved).
  3. Manually delete the workspace directory.

Recommendation: ab dbg remove should accept a direct filesystem path to the workspace group directory as an alternative to a repo name that requires discovery. This could be a follow-up issue.

Test 4: Explicit base_repo_dir, existing behavior

Result: PASS

Workspace created at ~/.local/agent-box/workspaces/git/agent-box/explicit-test/ with the short relative path, matching existing behavior.

Test 5: ab info from linked worktree

Result: PASS

From inside the linked worktree at ~/.local/agent-box/workspaces/git/agent-box/explicit-test, ab info correctly shows Repository: /home/jessea/Documents/projects/agent-box (the main repo root, not the worktree).

Finding: A stale worktree from test 3 (manual-test) still appears in the worktree list even though its workspace directory was removed by ab dbg remove --unresolved. This is expected: ab dbg remove deletes the workspace directory but does not run git worktree remove to clean up the git worktree metadata. The stale entry would need git worktree prune to clean up.

Recommendation: Consider having ab dbg remove also run git worktree remove or git worktree prune as part of cleanup. This could be a follow-up issue.

Test 6: ab new from linked worktree

Result: PASS

From inside the linked worktree, ab new -s from-worktree --git correctly resolved to the source repo agent-box and created the workspace at git/agent-box/from-worktree.

Test 7: ab spawn --local from linked worktree

Result: PASS (mount path correct), FAIL (container startup)

The working directory and mount are correctly set to the worktree path (/home/jessea/.local/agent-box/workspaces/git/agent-box/explicit-test), NOT the main repo root. This confirms find_git_workdir is used in local mode.

The container itself failed with EACCES: permission denied, mkdir '/home/jessea/.local/share' from the entrypoint (Bun). This is a pre-existing container image issue (the container user lacks a writable home directory), not related to the Phase 1 changes.

Test 8: ab dbg list

Result: PASS

Output correctly shows agent-box with 2 sessions and healthy status. Also shows a pre-existing agent-images workspace with 1 session.

Test 9: ab dbg list --unresolved

Result: PASS

Shows only the fake deleted-repo entry with status unresolved. Healthy workspaces are correctly filtered out. Footer note is displayed with cleanup guidance.

Test 10: ab dbg remove --unresolved --dry-run

Result: PASS

Lists the unresolved workspace, prints [DRY RUN] No files were actually deleted. Nothing was deleted.

Test 11: ab dbg remove --unresolved

Result: PASS

Prompted for confirmation with the base_repo_dir warning. After confirming, removed the workspace and reported Done. Removed 1 unresolved workspace group(s). Verified with ab dbg list --unresolved showing No unresolved workspaces found.

Test 12: ab info shows repo identity header

Result: PASS

First line shows Repository: /home/jessea/Documents/projects/agent-box.

Finding: The stale manual-test worktree from test 3 still appears in the git worktree list (at git/home/jessea/Documents/projects/agent-box/manual-test), confirming the finding from test 5: ab dbg remove does not clean up git worktree metadata. A git worktree prune would remove it.

Test 13: ab new bare name hint

Result: PASS

Error correctly shows could not find a git repository at 'my-project' with the hint suggesting cd my-project && ab new or a full path.


Summary

Test Description Result
1 cargo build PASS
2 cargo test PASS (146 tests)
3 Remove base_repo_dir, create workspace PASS (creation), cleanup requires workaround
4 Explicit base_repo_dir unchanged PASS
5 ab info from linked worktree PASS
6 ab new from linked worktree PASS
7 ab spawn --local from worktree PASS (mount path correct)
8 ab dbg list PASS
9 ab dbg list --unresolved PASS
10 ab dbg remove --unresolved --dry-run PASS
11 ab dbg remove --unresolved PASS
12 ab info repo identity header PASS
13 ab new bare name hint PASS

Follow-up issues identified

  1. ab dbg remove cannot manage workspaces when discovery is not configured. The repo argument always goes through locate_repo -> discover_repo_ids. When base_repo_dir defaults to / and no discovery dirs are set, all name-based removal fails. Should accept a direct path as an alternative.

  2. ab dbg remove does not clean up git worktree metadata. After removing a workspace directory, the git worktree entry persists in the source repo's .git/worktrees/ directory. Stale entries show up in ab info and git worktree list until git worktree prune is run. Consider running git worktree remove or git worktree prune as part of cleanup.

… 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.
@nothingnesses
Copy link
Copy Markdown
Contributor Author

nothingnesses commented Mar 20, 2026

Manual test findings for ab dbg remove improvements 2da78fb

Date: 2026-03-20

Tests are detailed here.

Fix 1: Absolute path support

Test F1.1: Create workspace without base_repo_dir

Result: PASS

Workspace created at ~/.local/agent-box/workspaces/git/home/jessea/Documents/projects/agent-box/path-test/ as expected.

Test F1.2: ab dbg list shows full source path

Result: PASS

Shows /home/jessea/Documents/projects/agent-box with 1 session and healthy as expected. Also shows a pre-existing agent-images workspace as unresolved at /agent-images (this workspace was created with base_repo_dir = "~/Documents/projects" but is now being resolved against / since base_repo_dir was removed).

Test F1.3: ab dbg remove with absolute path, dry run

Result: PASS

Shows the workspace at the correct path, prints [DRY RUN] No files were actually deleted.

Test F1.4: ab dbg remove with absolute path, actual removal

Result: PASS

Prompted for confirmation, pruned the git worktree metadata (Pruned git worktree: .../path-test), removed the workspace directory. Worktree cleanup (Fix 2) also worked here.

Finding: The output shows [DRY RUN] No files were actually deleted. before the confirmation prompt, because remove_repo is called twice: once with dry_run=true to show the preview, then again with dry_run=false after confirmation. The [DRY RUN] line in the preview is slightly confusing since the user hasn't asked for --dry-run. Consider suppressing the [DRY RUN] message when it's just a preview before the confirmation prompt.

Recommendation: In main.rs, the preview call remove_repo(&config, &repo_id, true) should either not print [DRY RUN], or use a separate "preview" mode. This is a pre-existing pattern, not introduced by this diff. Could be a follow-up.

Test F1.5: Restore config, verify existing behavior

Result: PASS

With base_repo_dir restored, ab new -s name-test --git created workspace at the short path (git/agent-box/name-test). ab dbg remove agent-box found it by name, prompted for confirmation, pruned worktree metadata, and removed the workspace.

Fix 2: Git worktree metadata cleanup

Test F2.1: Worktree metadata cleaned up on removal

Result: PASS (already verified in F1.4 and F1.5)

Both test F1.4 and F1.5 showed Pruned git worktree: ... in the output, confirming that remove_git_worktree is called and git worktree metadata is cleaned up before directory deletion. The git worktree list verification was not run separately, but the Pruned git worktree message confirms git worktree remove --force succeeded (the function returns Err on failure).


Summary

Test Description Result
F1.1 Create workspace without base_repo_dir PASS
F1.2 ab dbg list shows full source path PASS
F1.3 ab dbg remove absolute path, dry run PASS
F1.4 ab dbg remove absolute path, actual removal PASS
F1.5 Restore config, name-based removal still works PASS
F2.1 Git worktree metadata cleaned up on removal PASS

Follow-up issues identified

  1. [DRY RUN] shown in confirmation preview. When ab dbg remove is run without --dry-run, the preview (before the confirmation prompt) still prints [DRY RUN] No files were actually deleted. because it calls remove_repo(config, repo_id, true). This is confusing. The preview call should suppress the [DRY RUN] message, or use a separate display mode. This is pre-existing behavior, not introduced by this diff.

The confirmation preview for `ab dbg remove` was showing
`[DRY RUN] No files were actually deleted.` even when the user
did not pass `--dry-run`, because `remove_repo(dry_run=true)` was
used for both the preview and the actual dry-run path. Move the
`[DRY RUN]` message out of `remove_repo` into `main.rs` so it
only prints when `--dry-run` was explicitly requested.
@nothingnesses nothingnesses marked this pull request as ready for review March 21, 2026 00:10
@0xferrous
Copy link
Copy Markdown
Owner

Codex PR Review

Reviewed branch: nothingnesses/#16 (514ea12)

Findings

1. Git worktree cleanup still breaks for relative gitdir: entries

  • File: common/src/repo.rs:371-399
  • remove_git_worktree() parses .git files and does Path::new(gitdir) directly. That works for absolute paths, but Git can write relative entries when worktree.useRelativePaths=true.
  • Example worktree .git content:
    gitdir: ../repo/.git/worktrees/wt
    
  • In that case, the code resolves the path relative to the process cwd instead of session_path, so git_dir.exists() is false and cleanup is skipped, leaving stale worktree metadata behind.
  • This matters because the PR explicitly improves ab dbg remove cleanup behavior.
  • Suggested fix: resolve relative gitdir paths against session_path (or the .git file’s parent directory) before walking up to the repo .git dir.

2. ab dbg list can report a workspace as “healthy” even when the repo is gone

  • File: common/src/path.rs:540-547
  • scan_workspaces() marks a workspace healthy if source_path.is_dir().
  • That is too weak: any plain directory at the reconstructed path is treated as a valid source repo.
  • For git workspaces, this should verify a git repo marker (.git dir/file); for jj, a .jj dir.
  • As written, a deleted repo replaced by an unrelated directory will no longer show up as unresolved, so ab dbg remove --unresolved can miss it.
  • Suggested fix: make the health check type-specific.

3. New repo tests are not hermetic with respect to global Git signing config

  • File: common/src/repo.rs:559-568
  • The new helper creates commits with plain git commit --allow-empty -m init.
  • On machines with commit.gpgsign=true, these tests fail unless GPG/pinentry is available.
  • I reproduced this locally: cargo test fails in several new repo tests with gpg failed to sign the data.
  • Suggested fix: disable signing in the test repo (git config commit.gpgsign false) or invoke git -c commit.gpgsign=false commit ....

…, isolate tests

Resolve relative `gitdir:` entries against session_path in
remove_git_worktree so cleanup works when worktree.useRelativePaths
is enabled. Make scan_workspaces check for VCS markers (.git/.jj)
instead of just testing is_dir(), preventing plain directories from
being reported as healthy. Isolate all test git commands from global
config via GIT_CONFIG_NOSYSTEM and HOME env vars. Escape brackets
and angle brackets in doc comments to fix cargo doc warnings.
@nothingnesses
Copy link
Copy Markdown
Contributor Author

nothingnesses commented Mar 21, 2026

Thanks @0xferrous. I've addressed the feedback with commit 38a324f according to this plan. Let me know if there's any other issues.

@0xferrous
Copy link
Copy Markdown
Owner

i will need some more time to review this, its just too huge

@nothingnesses
Copy link
Copy Markdown
Contributor Author

Thanks, I appreciate you doing things properly. Let me know if you find anything else.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enable users to spawn containers from anywhere on the fs

2 participants