From 020a8aa736f3c9c176e31e2214619df32267f879 Mon Sep 17 00:00:00 2001
From: Ramesh Padmanabhaiah <codeforester@gmail.com>
Date: Fri, 19 Jun 2026 19:22:45 -0700
Subject: [PATCH] Tighten git helper skip behavior

---
 lib/bash/git/README.md          |  2 +
 lib/bash/git/lib_git.sh         |  8 ++--
 lib/bash/git/tests/lib_git.bats | 73 ++++++++++++++++++++++++++++++++-
 3 files changed, 79 insertions(+), 4 deletions(-)

diff --git a/lib/bash/git/README.md b/lib/bash/git/README.md
index 22fefe4..d2dffcd 100644
--- a/lib/bash/git/README.md
+++ b/lib/bash/git/README.md
@@ -39,6 +39,8 @@ log_info "Current branch: $branch"
 - `git_get_current_branch` uses `git -C` so it does not change the caller's
   working directory or directory stack. Missing directories and non-Git
   directories return success with an empty result variable.
+- `git_update_repo` changes into the target repository while it runs because
+  its submodule update sequence depends on repository-relative execution.
 - `check_script_up_to_date` treats missing git state, untracked scripts, or missing upstreams as skip conditions rather than hard failures.
 - `check_script_up_to_date <script>` compares `HEAD` with the local remote-tracking upstream ref. It does not fetch by default, so the result reflects the freshness of local refs.
 - `check_script_up_to_date --fetch <script>` runs `git fetch --quiet` first, then compares against the refreshed upstream ref. If fetch fails, the helper logs a warning and falls back to local remote-tracking refs.
diff --git a/lib/bash/git/lib_git.sh b/lib/bash/git/lib_git.sh
index 009269d..206b118 100644
--- a/lib/bash/git/lib_git.sh
+++ b/lib/bash/git/lib_git.sh
@@ -137,7 +137,7 @@ git_update_repo() {
 
     if [[ -z "$git_repo" ]]; then
         log_error "No git repository path provided."
-        log_info "Usage: update_repo /path/to/repo [allowed_dirty_path] [expected_branch]"
+        log_info "Usage: git_update_repo /path/to/repo [allowed_dirty_path] [expected_branch]"
         return 1
     fi
 
@@ -150,6 +150,8 @@ git_update_repo() {
         log_error "Unable to create temporary git log file."
         return 1
     }
+    # git_update_repo intentionally works inside the target repository because
+    # the submodule update sequence below needs the repository as its cwd.
     if ! pushd "$git_repo" > /dev/null; then
         # If cd fails, we can't proceed.
         _git_update_repo_finish "$git_log" false 1
@@ -169,7 +171,7 @@ git_update_repo() {
     current_branch=$(git rev-parse --abbrev-ref HEAD)
     if [[ "$current_branch" != "$expected_branch" ]]; then
         log_debug "Current branch of '$git_repo' is '${current_branch}', not '$expected_branch'. Skipping update."
-        _git_update_repo_finish "$git_log" true 1
+        _git_update_repo_finish "$git_log" true 0
         return $?
     fi
 
@@ -344,7 +346,7 @@ check_script_up_to_date() {
             log_warn "Unable to fetch upstream state; using local remote-tracking refs."
         fi
     else
-        log_info "Using local remote-tracking refs; pass --fetch for a live remote check."
+        log_debug "Using local remote-tracking refs; pass --fetch for a live remote check."
     fi
 
     behind=$(git -C "$repo_root" rev-list --count HEAD.."$upstream" 2>/dev/null)
diff --git a/lib/bash/git/tests/lib_git.bats b/lib/bash/git/tests/lib_git.bats
index 5a24691..8e17118 100644
--- a/lib/bash/git/tests/lib_git.bats
+++ b/lib/bash/git/tests/lib_git.bats
@@ -106,6 +106,14 @@ setup() {
     [[ "$output" != *"invalid variable name"* ]]
 }
 
+@test "git_update_repo usage names the current function" {
+    bats_run git_update_repo
+
+    [ "$status" -eq 1 ]
+    [[ "$output" == *"Usage: git_update_repo /path/to/repo [allowed_dirty_path] [expected_branch]"* ]]
+    [[ "$output" != *"Usage: update_repo"* ]]
+}
+
 @test "git_update_repo skips dirty repositories when no dirty path is allowed" {
     local repo="$TEST_TMPDIR/repo"
 
@@ -121,6 +129,20 @@ setup() {
     [[ "$output" == *"has local changes; skipping auto-update"* ]]
 }
 
+@test "git_update_repo treats branch mismatch as a skip" {
+    local repo="$TEST_TMPDIR/repo"
+
+    init_git_repo "$repo"
+    printf 'base\n' > "$repo/data.txt"
+    commit_all "$repo" "Initial commit"
+    set_log_level DEBUG
+
+    bats_run git_update_repo "$repo" "" main
+
+    [ "$status" -eq 0 ]
+    [[ "$output" == *"not 'main'. Skipping update"* ]]
+}
+
 @test "git_update_repo fails clearly when origin remote is missing" {
     local before_head
     local repo="$TEST_TMPDIR/repo"
@@ -421,6 +443,40 @@ setup() {
     [ "$(cat "$git_log")" = "pull attempt 3" ]
 }
 
+@test "_git_pull_with_retry falls back for invalid configured max attempts" {
+    local git_log="$TEST_TMPDIR/git.log"
+    local max_attempts
+    local pull_count="$TEST_TMPDIR/pull-count"
+
+    git() {
+        local count
+
+        if [[ "${1:-}" == "pull" ]]; then
+            count="$(cat "$pull_count")"
+            count=$((count + 1))
+            printf '%s\n' "$count" > "$pull_count"
+            printf 'pull attempt %s\n' "$count" >&2
+            [[ "$count" -ge 2 ]]
+            return $?
+        fi
+        command git "$@"
+    }
+
+    for max_attempts in abc 0 -1; do
+        printf '0\n' > "$pull_count"
+        : > "$git_log"
+
+        BASE_GIT_PULL_MAX_ATTEMPTS="$max_attempts" bats_run _git_pull_with_retry "$git_log"
+
+        [ "$status" -eq 0 ]
+        [ "$(cat "$pull_count")" = "2" ]
+        [[ "$output" == *"BASE_GIT_PULL_MAX_ATTEMPTS must be a positive integer; using 2."* ]]
+        [[ "$output" == *"git pull failed on attempt 1; retrying once."* ]]
+        [ "$(cat "$git_log")" = "pull attempt 2" ]
+    done
+    unset -f git
+}
+
 @test "_git_pull_with_retry fails after two pull attempts" {
     local git_log="$TEST_TMPDIR/git.log"
     local pull_count="$TEST_TMPDIR/pull-count"
@@ -479,7 +535,22 @@ setup() {
     bats_run check_script_up_to_date "$script_path"
 
     [ "$status" -eq 0 ]
-    [[ "$output" == *"local remote-tracking refs"* ]]
+    [[ "$output" != *"Using local remote-tracking refs"* ]]
+    [[ "$output" == *"Repository is up to date with origin/master."* ]]
+}
+
+@test "check_script_up_to_date reports local remote-tracking refs at debug level" {
+    local repo="$TEST_TMPDIR/repo"
+    local remote="$TEST_TMPDIR/remote.git"
+    local script_path="$repo/scripts/tool.sh"
+
+    create_tracked_repo_with_upstream "$repo" "$remote" "scripts/tool.sh" "#!/usr/bin/env bash"
+    set_log_level DEBUG
+
+    bats_run check_script_up_to_date "$script_path"
+
+    [ "$status" -eq 0 ]
+    [[ "$output" == *"Using local remote-tracking refs; pass --fetch for a live remote check."* ]]
     [[ "$output" == *"Repository is up to date with origin/master."* ]]
 }