fix(fleet-sync): skip non-repo directories nested in enclosing repo#2
Merged
Conversation
This was referenced Jul 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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()inbin/fm-fleet-sync.shnow determines whether a candidate project directory is a genuine git repository by comparing its resolved real path againstgit 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, reportingskipped: not a git repoinstead of producing bogus diverged/STUCK output.cd "$PROJ"used to resolve the project's real path is guarded so a failedcd(e.g. permission denied or a directory removed mid-run) falls back to an emptyproject_rootand is treated as a clean skip rather than aborting the whole sync run underset -eu.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 noSTUCKordivergedoutput.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 trippingset -eand 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
Evidence: fm-lint.sh run
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 "$PROJ" && pwd -P)is a plain (non-conditional) statement underset -eu. Ifcd "$PROJ"fails after the preceding[ -d "$PROJ" ]check succeeded — e.g. a permission-denied directory, or a directory removed/renamed by a concurrent process (teardown/spawn race) between the-dtest and thecd— the assignment's non-zero exit status tripsset -eand aborts the entire fm-fleet-sync.sh run, not just this one project. In the whole-fleet form (for proj in "$PROJECTS"/*; do sync_project "$proj"; 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 "$PROJ" && 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_skippedManually 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 failurebash bin/fm-lint.sh — ShellCheck 0.11.0 clean, exit 0git 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.