From ccbacabf668a0ce4c7787b54aa353ba425c8f9a1 Mon Sep 17 00:00:00 2001 From: Claude Code Bot Date: Thu, 15 Oct 2026 14:56:27 -0700 Subject: [PATCH 1/2] fix(repo-walker): exit 1 when a repo fails my-repos and beacon-repos printed "Failed: N" and exited 0, so a caller checking the exit status saw success. allup worked around this by parsing the summary text (dotfiles#432). repo_walker_main now returns 1 when any repo failed its status check, switch, pull or branch restore. Dirty repos are still skipped with exit 0, and report-only lookups do not change the status. The wrappers call repo_walker_main last, so the status passes through unchanged. The only caller is allup's _allup_pull_step, which already handles a non-zero exit. Two existing walk tests pull from a GitHub repo that does not exist; they now expect exit 1. Closes #6 --- README.md | 2 ++ lib/repo-walker.sh | 10 ++++++++++ tests/repo-walker.bats | 41 ++++++++++++++++++++++++++++++++++++++--- 3 files changed, 50 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 9d17669..30a2910 100644 --- a/README.md +++ b/README.md @@ -22,6 +22,8 @@ Internal repo; not intended for public consumption. sensible default: `MY_REPOS_DIR` (`my-repos.sh`), `BEACON_REPOS_DIR` (`beacon-repos.sh`). A missing directory warns and exits 0 rather than failing, so `allup` still passes on a machine that legitimately lacks one. + A repo that fails its pull makes the walker exit 1; a dirty repo is + skipped and does not. - LaunchAgent plists that invoke these scripts live in `~/Developer/LaunchAgents/`, not in this repo. The empty `LaunchAgents/` directory here is a placeholder. diff --git a/lib/repo-walker.sh b/lib/repo-walker.sh index 080b2be..c9a0a09 100644 --- a/lib/repo-walker.sh +++ b/lib/repo-walker.sh @@ -72,6 +72,10 @@ Options: Matched literally — no globbing. --quiet, -q Suppress per-repo detail; show only summary --help, -h Show this help message and exit + +Exit status: 0 when no repo failed (dirty repos are skipped, not failed), +1 when any repo failed its status check, switch, pull or branch restore, +2 on a usage error. EOF } @@ -989,4 +993,10 @@ repo_walker_main() { echo " - ${f}" done fi + + # allup reads this status. An if, not `&&`: that returns 1 on success. + if ((failed > 0)); then + return 1 + fi + return 0 } diff --git a/tests/repo-walker.bats b/tests/repo-walker.bats index 454088a..b9590d5 100755 --- a/tests/repo-walker.bats +++ b/tests/repo-walker.bats @@ -247,7 +247,9 @@ teardown() { git -C "${r}" commit -qm one git -C "${r}" remote add origin git@github.com:smartwatermelon/repo3-does-not-exist-xyz.git run bash -c "MY_REPOS_DIR='${TMPDIR_T}/walkdir' '${REPO_ROOT}/my-repos.sh'" - [ "${status}" -eq 0 ] + # The origin does not exist, so the pull fails and the walk exits 1; what + # matters here is that it reaches the summary instead of aborting early. + [ "${status}" -eq 1 ] [[ "${output}" == *"repo3"* ]] [[ "${output}" == *"Total:"* ]] } @@ -325,7 +327,7 @@ _move_upstream_tag() { _move_upstream_tag run bash -c "MY_REPOS_DIR='${TMPDIR_T}/walkdir' '${REPO_ROOT}/my-repos.sh'" - [ "${status}" -eq 0 ] + [ "${status}" -eq 1 ] [[ "${output}" == *"pull failed"* ]] # The failure must come from the divergence, not from the tag. [[ "${output}" != *"would clobber existing tag"* ]] @@ -334,6 +336,37 @@ _move_upstream_tag() { [ "${after}" = "${before}" ] } +@test "walk exits 1 when a pull fails, in both wrappers" { + # An origin whose path does not exist: the pull fails. + mkdir -p "${TMPDIR_T}/walkdir" "${TMPDIR_T}/beacondir" + for d in walkdir/brokenrepo beacondir/brokenrepo; do + _GIT_WRAPPER_ACTIVE=1 git -C "${TMPDIR_T}" init -q -b main "${d}" + done + git -C "${TMPDIR_T}/walkdir/brokenrepo" remote add origin \ + "${TMPDIR_T}/missing/smartwatermelon/brokenrepo.git" + git -C "${TMPDIR_T}/beacondir/brokenrepo" remote add origin \ + "${TMPDIR_T}/missing/beacon-biosignals/brokenrepo.git" + + run bash -c "MY_REPOS_DIR='${TMPDIR_T}/walkdir' '${REPO_ROOT}/my-repos.sh' --quiet" + [ "${status}" -eq 1 ] + [[ "${output}" == *"Failed: 1"* ]] + [[ "${output}" == *"- brokenrepo"* ]] + + run bash -c "BEACON_REPOS_DIR='${TMPDIR_T}/beacondir' '${REPO_ROOT}/beacon-repos.sh' --quiet" + [ "${status}" -eq 1 ] + [[ "${output}" == *"Failed: 1"* ]] +} + +@test "walk exits 0 when the only repo left alone is dirty" { + _mk_tag_upstream + echo scratch >"${walked}/untracked" + + run bash -c "MY_REPOS_DIR='${TMPDIR_T}/walkdir' '${REPO_ROOT}/my-repos.sh'" + [ "${status}" -eq 0 ] + [[ "${output}" == *"Skipped: 1"* ]] + [[ "${output}" == *"Failed: 0"* ]] +} + @test "parse_args accepts --show-dead-branches and --status" { run bash -c "source '${LIB}'; repo_walker_parse_args --status; \ echo \"\${REPO_WALKER_SHOW_PRS} \${REPO_WALKER_SHOW_ISSUES} \${REPO_WALKER_SHOW_DEAD_BRANCHES}\"" @@ -1013,7 +1046,9 @@ _mkrepo() { _mkrepo "${TMPDIR_T}/walk/moved" git@github.com:smartwatermelon/moved.git run bash -c "BASH_ENV='${stub}' MY_REPOS_DIR='${TMPDIR_T}/walk' \ bash '${REPO_ROOT}/my-repos.sh' --check-remotes 2>&1" - [ "${status}" -eq 0 ] + # The fake origin cannot be pulled, so the walk exits 1. A mis-pointed + # remote alone is report-only and does not affect the status. + [ "${status}" -eq 1 ] [[ "${output}" == *"Mis-pointed: 1"* ]] [[ "${output}" == *"Mis-pointed remotes:"* ]] [[ "${output}" == *"Re-run with --fix-remotes"* ]] From 7d0ddbcfff3e0dc412d837427a9b881db6e0e32f Mon Sep 17 00:00:00 2001 From: Claude Code Bot Date: Thu, 15 Oct 2026 14:57:34 -0700 Subject: [PATCH 2/2] test(repo-walker): name the cause of the report-only exit status Review follow-up. The report-only walk test exits 1 because its fake origin cannot be pulled, not because of the mis-pointed remote. Assert Failed: 1 so the cause is explicit, and record why the test cannot isolate a mis-pointed remote. README now lists every failure that sets exit 1. --- README.md | 4 ++-- tests/repo-walker.bats | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 30a2910..8d95911 100644 --- a/README.md +++ b/README.md @@ -22,8 +22,8 @@ Internal repo; not intended for public consumption. sensible default: `MY_REPOS_DIR` (`my-repos.sh`), `BEACON_REPOS_DIR` (`beacon-repos.sh`). A missing directory warns and exits 0 rather than failing, so `allup` still passes on a machine that legitimately lacks one. - A repo that fails its pull makes the walker exit 1; a dirty repo is - skipped and does not. + A repo that fails its status check, switch, pull or branch restore makes + the walker exit 1; a dirty repo is skipped and does not. - LaunchAgent plists that invoke these scripts live in `~/Developer/LaunchAgents/`, not in this repo. The empty `LaunchAgents/` directory here is a placeholder. diff --git a/tests/repo-walker.bats b/tests/repo-walker.bats index b9590d5..b1ab626 100755 --- a/tests/repo-walker.bats +++ b/tests/repo-walker.bats @@ -1046,9 +1046,11 @@ _mkrepo() { _mkrepo "${TMPDIR_T}/walk/moved" git@github.com:smartwatermelon/moved.git run bash -c "BASH_ENV='${stub}' MY_REPOS_DIR='${TMPDIR_T}/walk' \ bash '${REPO_ROOT}/my-repos.sh' --check-remotes 2>&1" - # The fake origin cannot be pulled, so the walk exits 1. A mis-pointed - # remote alone is report-only and does not affect the status. + # The fake origin cannot be pulled, so the walk exits 1 for that reason. + # This test cannot prove a mis-pointed remote alone leaves the status at 0: + # git remote get-url applies insteadOf, so a reachable fake is not cheap. [ "${status}" -eq 1 ] + [[ "${output}" == *"Failed: 1"* ]] [[ "${output}" == *"Mis-pointed: 1"* ]] [[ "${output}" == *"Mis-pointed remotes:"* ]] [[ "${output}" == *"Re-run with --fix-remotes"* ]]