Skip to content

fix(fleet-sync): skip non-repo directories nested in enclosing repo#2

Merged
Freudator86 merged 2 commits into
mainfrom
fm/fm-fleet-sync-orphan-dir-fix
Jul 19, 2026
Merged

fix(fleet-sync): skip non-repo directories nested in enclosing repo#2
Freudator86 merged 2 commits into
mainfrom
fm/fm-fleet-sync-orphan-dir-fix

Conversation

@Freudator86

Copy link
Copy Markdown
Owner

Intent

Fix bin/fm-fleet-sync.sh so orphan container directories under a firstmate home's projects directory, such as -gnhf-worktrees, are recognized as non-project directories even when projects is nested inside firstmate's enclosing Git repository. Repository detection must require that the candidate directory itself is the Git worktree root, skip non-repositories cleanly as 'skipped: not a git repo' without bogus diverged or STUCK reports, preserve behavior for genuine project clones, add a colocated regression test that simulates a nested non-repo container, and keep bin/fm-lint.sh passing.

What Changed

  • sync_project() in bin/fm-fleet-sync.sh now determines whether a candidate project directory is a genuine git repository by comparing its resolved real path against git rev-parse --show-toplevel's resolved real path, instead of relying on --is-inside-work-tree. This correctly identifies orphan container directories (e.g. <repo>-gnhf-worktrees) nested inside the firstmate home's enclosing git repository as non-projects, reporting skipped: not a git repo instead of producing bogus diverged/STUCK output.
  • The cd "$PROJ" used to resolve the project's real path is guarded so a failed cd (e.g. permission denied or a directory removed mid-run) falls back to an empty project_root and is treated as a clean skip rather than aborting the whole sync run under set -eu.
  • Adds a colocated regression test, test_non_repo_directory_inside_enclosing_repo_skipped, simulating a non-repo container directory nested inside an enclosing git repo and asserting it is skipped cleanly with no STUCK or diverged output.

Risk Assessment

✅ Low: The only change since the last review is the requested fix: guarding the cd "$PROJ" failure so an inaccessible/raced directory now resolves to a clean "not a git repo" skip instead of tripping set -e and aborting the whole fleet-sync run, matching the existing guarded pattern used elsewhere in the same file.

Testing

Ran the full fm-fleet-sync.test.sh suite (23 assertions) at the target commit; all pass, including the new regression test that simulates an enclosing repo with a nested non-repo container (fake-gnhf-worktrees) and asserts it is skipped as "not a git repo" with no STUCK/diverged output, while pre-existing tests confirm genuine clones (recovered, STUCK, unchanged, no-origin, local-only) still behave as before. bin/fm-lint.sh also passes cleanly (ShellCheck 0.11.0, exit 0) as the intent requires. No UI surface is involved, so no visual artifacts were produced — CLI test output is the appropriate evidence here.

Evidence: fm-fleet-sync.test.sh full run
ok - non-repo container inside an enclosing repository is skipped cleanly
(23/23 assertions passed, including all pre-existing STUCK/recovered/skip cases)
Evidence: fm-lint.sh run
fm-lint.sh: ShellCheck 0.11.0 (pinned 0.11.0)
EXIT: 0

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

🔧 **Review** - 1 issue found → auto-fixed ✅
  • ⚠️ bin/fm-fleet-sync.sh:302 - project_root=$(cd &#34;$PROJ&#34; &amp;&amp; pwd -P) is a plain (non-conditional) statement under set -eu. If cd &#34;$PROJ&#34; fails after the preceding [ -d &#34;$PROJ&#34; ] check succeeded — e.g. a permission-denied directory, or a directory removed/renamed by a concurrent process (teardown/spawn race) between the -d test and the cd — the assignment's non-zero exit status trips set -e and aborts the entire fm-fleet-sync.sh run, not just this one project. In the whole-fleet form (for proj in &#34;$PROJECTS&#34;/*; do sync_project &#34;$proj&#34;; done), that means every remaining clone silently stops getting synced instead of the offending directory being reported as a benign skip. The sibling line 301 (git_root=$(...) || git_root=) and an existing precedent in the same file (packed_refs_lock_path at line 145: abs=$(cd &#34;$PROJ&#34; &amp;&amp; pwd -P) || return 1) both guard this exact failure mode, so this looks like an oversight rather than an intentional choice. Since the whole point of this change is to make sync robust against odd/orphan directories under projects/, an odd directory that also happens to be inaccessible or mid-deletion is a plausible trigger.

🔧 Fix: Guard cd failure in fleet-sync project-root check
✅ Re-checked - no issues remain.

✅ **Test** - passed

✅ No issues found.

  • command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"
  • bash tests/fm-fleet-sync.test.sh (full suite, 23 cases) — all pass, including new test_non_repo_directory_inside_enclosing_repo_skipped
  • Manually reviewed sync_project() logic in bin/fm-fleet-sync.sh:291-306 confirming git_root == project_root comparison and the guarded 'cd "$PROJ" && pwd -P || project_root=' fallback for cd failure
  • bash bin/fm-lint.sh — ShellCheck 0.11.0 clean, exit 0
  • git status --porcelain in the worktree post-test — clean, no leftover artifacts
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

@Freudator86
Freudator86 merged commit b384c1b into main Jul 19, 2026
5 checks passed
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.

2 participants