feat: drop python dependency; make gha-timer opt-out (#62)

* feat: drop python dependency; make gha-timer opt-out

Port src/scripts/git_ungraft.py to a bash equivalent
(src/scripts/git_ungraft.sh) and drop the actions/setup-python step
from action.yml.  The composite action now has no interpreter-setup
prerequisites beyond bash itself.

The port uses `git cat-file -p` to discover parent commits, not
shallow-aware commands such as `git show --format=%P` (which returns
an empty parent list for commits listed in .git/shallow — exactly
the subset the ungraft logic needs to re-evaluate).  The awk parser
reads only the header block (up to the first blank line), so
commit-message body contents cannot be misparsed as "parent <sha>"
lines.

Add an `enable-timing` input (default: true) that controls whether
fulcrumgenomics/gha-timer is installed.  A shared shell wrapper
(src/scripts/gha_timer_wrapper.sh) transparently falls back to
emitting `::group::` / `::endgroup::` workflow commands when
gha-timer is absent, so the action log remains collapsible in the
GH Actions UI for users who opt out.

Add a matrix test case exercising the enable-timing=false path.

* fix: git_fetch_parents was a no-op on shallow-boundary commits; add shellcheck

Extract `git_fetch_parents` from `src/scripts/fetch_through_merge_base.sh`
into its own sourceable helper `src/scripts/git_fetch_parents.sh`, add
a regression test (`tests/test_git_fetch_parents.sh`), and fix the
shallow-aware-parent bug it surfaced.

Before this change the function used `git show --no-patch --format='%P'`
to discover parents.  That command (like every other shallow-aware git
porcelain: `git log --pretty=%P`, `git rev-list --parents`, etc.) returns
an EMPTY parent list for commits listed in `.git/shallow` — which is
exactly the subset for which the function is meant to do useful work
(pre-deepen parent fetching on PR merge commits that sit at the shallow
boundary).  Consequence: the optimization was a silent no-op whenever
it mattered; the deepen loop masked the miss, so CI stayed green.

The fix reads parent SHAs from the raw commit object via
`git cat-file -p`, parsed by an awk filter that emits only header-block
`parent <sha>` lines (up to the first blank line), so commit-message
body contents cannot be misparsed — preserving the protection added in
e4d95ef for the parent-in-body regression test.

CI changes:

- Add a `shellcheck` job that runs `shellcheck -x --severity=style
  --source-path=SCRIPTDIR` on every shell script in the repo.  Severity
  is the strictest setting; no rules are excluded.
- Add a `shell-tests` matrix job (ubuntu/macos) that runs the new unit
  test.  The step installs `fulcrumgenomics/gha-timer` and sources
  `src/scripts/gha_timer_wrapper.sh` so it dogfoods the action's own
  default-on timing path.

Cleanup surfaced by shellcheck:

- Switch `fetch_through_merge_base.sh` shebang from `/bin/sh` to
  `/usr/bin/env bash` (the script has always used `[[`, `BASH_SOURCE`,
  arrays, etc.).
- Quote variables in `git rev-parse --verify` arguments and in
  `bash ${SCRIPT_DIR}/...` invocations.
- Replace `let FAIL_AFTER="FAIL_AFTER-1"` (which trips `set -e` when
  the result is zero, hence the surrounding `set +e`/`set -e` dance)
  with `FAIL_AFTER=$((FAIL_AFTER - 1))`.

* docs: flesh out enable-timing description in the testing wrapper

Match the convention of the other inputs in .github/actions/testing/
action.yml, which carry the same description as the corresponding
input on the action under test instead of pointing at external docs.

* refactor: apply review feedback

- fetch_through_merge_base.sh: normalize `set -eou pipefail` to the
  idiomatic `set -euo pipefail`; fix `Deepend` typo.
- git_ungraft.sh: use `git rev-parse --absolute-git-dir` instead of
  the cd/cd/pwd dance so a GIT_DIR pointing outside the work tree is
  handled correctly; add an arity check for -C/--git-dir so `set -u`
  doesn't produce an unhelpful "$2: unbound variable"; add a comment
  noting that the rewrite predicate is equivalent to the Python
  original's `set(grafted) != set(remaining)`.
- gha_timer_wrapper.sh: document that the fallback is best-effort
  log grouping only — it emits no timing data and silently drops
  `gha_timer stop`.
- action.yml (both): quote `default: 'true'` for enable-timing to
  make the string contract explicit and document accepted values;
  fix a double-space in the description.

* test: add unit tests for git_ungraft.sh and gha_timer_wrapper.sh

test_git_ungraft.sh builds a hermetic shallow clone from a local
bare repo (no network) and asserts:

- No-candidate case leaves .git/shallow untouched (mtime preserved).
- --dry-run prints "Would ungraft <sha>" without rewriting the file.
- Default run rewrites .git/shallow, removing the ungraftable entry.

test_gha_timer_wrapper.sh exercises both branches of the wrapper:

- When gha-timer is on PATH, argv is forwarded verbatim to a shim
  binary and no ::group::/::endgroup:: markers are emitted.
- When gha-timer is absent, the fallback emits the expected
  ::group::/::endgroup:: workflow commands and drops `stop`.

Both tests are picked up by the existing `tests/test_*.sh` loop in
the shell-tests job, so the wrapper fallback is now covered in CI
without needing a separate matrix entry.

* test: pin bare-repo default branch to main in test_git_ungraft

On ubuntu-22.04 runners, `git init --bare` without an explicit
initial-branch flag creates a repo whose HEAD points to `master`.
The test then pushes `main`, so the subsequent shallow clone hits
an unborn HEAD and lands as an empty clone — no `.git/shallow`
file — failing the first assertion. Pass `-b main` to the bare
init so HEAD tracks the branch we actually push.

* test: use inode identity instead of mtime to detect shallow rewrite

The test's case 1 and case 2 want to verify that git_ungraft.sh does
not rewrite `.git/shallow` when there are no candidates (case 1) or
when run with `--dry-run` (case 2). The previous implementation
compared `stat %Y` (seconds-precision mtime) before and after, which
proved flaky on CI: git's shallow-handling appears to touch the file
in place during read-only sub-commands run by the script, leaving
the inode unchanged but bumping mtime into a new second.

Switch to inode identity. The script's rewrite path is a tempfile
+ `mv`, which atomically replaces the file and therefore changes
its inode. The inode check precisely targets that rewrite and
ignores incidental in-place touches by unrelated git machinery.

Author: nh13

.github/actions/testing/action.yml

@@ -49,8 +49,17 @@ inputs:
     default: false
     required: false
     description: >
-      True to fetch all commits if number of attempts to deepen reaches 
+      True to fetch all commits if number of attempts to deepen reaches
       its limit, false to fail the action.
+  enable-timing:
+    default: 'true'
+    required: false
+    description: >
+      True to emit per-step timings via fulcrumgenomics/gha-timer,
+      false to skip the gha-timer setup step entirely. Set this to
+      false in environments where installing gha-timer is undesirable
+      (e.g. self-hosted runners with restricted egress).  Accepted
+      values are the strings 'true' and 'false'.
   ancestor-ref:
     default: ""
     required: false
@@ -79,6 +88,7 @@ runs:
         fail-after: ${{ inputs.fail-after }}
         working-directory: ./__testing_dir__
         fallback-fetch-all: ${{ inputs.fallback-fetch-all }}
+        enable-timing: ${{ inputs.enable-timing }}
     - name: cleanup the testing directory
       shell: bash
       run: rm -rf __testing_dir__

.github/workflows/tests.yml

@@ -38,6 +38,49 @@ jobs:
             echo "::error::Unstaged changes detected. Locally try running: git clean -ffdx && npm ci && npm run format && npm run build"
             exit 1
           fi
+  shellcheck:
+    runs-on: ubuntu-24.04
+    steps:
+    - name: Checkout
+      uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1
+    - name: Install shellcheck
+      run: sudo apt-get update && sudo apt-get install -y shellcheck
+    - name: Run shellcheck
+      run: |
+        shellcheck \
+          -x \
+          --severity=style \
+          --source-path=SCRIPTDIR \
+          src/scripts/*.sh \
+          tests/*.sh
+  shell-tests:
+    strategy:
+      matrix:
+        os: [ubuntu-24.04, ubuntu-22.04, macos-15, macos-14]
+    runs-on: ${{ matrix.os }}
+    steps:
+    - name: Checkout
+      uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1
+    - name: Setup gha-timer
+      uses: fulcrumgenomics/gha-timer@d0d886fa587c7f4f714e64d84aa6e274e818d48b # v1.1.1
+      with:
+        skip-banner: true
+    - name: Run shell unit tests
+      shell: bash
+      run: |
+        set -euo pipefail
+        # shellcheck disable=SC1091
+        source src/scripts/gha_timer_wrapper.sh
+        for t in tests/test_*.sh; do
+          gha_timer start --name "${t}"
+          if bash "${t}"; then
+            gha_timer elapsed --outcome success --name "${t}"
+          else
+            status=$?
+            gha_timer elapsed --outcome failure --name "${t}"
+            exit "${status}"
+          fi
+        done
   testing:
     strategy:
       matrix:
@@ -115,3 +158,13 @@ jobs:
         base-ref: "main"
         head-ref: "test-0002-parent-in-body"
         ancestor-ref: "6eb785672d2c6770af9e985d36ddcd1b3a285f8e"
+    # Exercises the enable-timing=false opt-out: the setup-gha-timer step is
+    # skipped and the shared wrapper falls back to ::group::/::endgroup::
+    # workflow commands.  The merge-base result must be unchanged.
+    - name: Test enable-timing opt-out
+      uses: ./.github/actions/testing
+      with:
+        head-ref: "test-0001-head-ref"
+        base-ref: "test-0001-base-ref"
+        enable-timing: false
+        ancestor-ref: "6eb785672d2c6770af9e985d36ddcd1b3a285f8e"

README.md

@@ -76,6 +76,13 @@ A GitHub Action for fetching PR commits through the merge-base
     # false to fail the action.
     # Default: false
     fallback-fetch-all: ''
+
+    # True to emit per-step timings via fulcrumgenomics/gha-timer, false to skip the
+    # gha-timer setup step entirely. Set this to false in environments where
+    # installing gha-timer is undesirable (e.g. self-hosted runners with restricted
+    # egress). Accepted values are the strings 'true' and 'false'.
+    # Default: true
+    enable-timing: ''
 ```
 
 ### Outputs

action.yml

@@ -54,8 +54,17 @@ inputs:
     default: false
     required: false
     description: >
-      True to fetch all commits if number of attempts to deepen reaches 
+      True to fetch all commits if number of attempts to deepen reaches
       its limit, false to fail the action.
+  enable-timing:
+    default: 'true'
+    required: false
+    description: >
+      True to emit per-step timings via fulcrumgenomics/gha-timer,
+      false to skip the gha-timer setup step entirely. Set this to
+      false in environments where installing gha-timer is undesirable
+      (e.g. self-hosted runners with restricted egress).  Accepted
+      values are the strings 'true' and 'false'.
 outputs:
   base-ref:
     description: >
@@ -82,12 +91,16 @@ runs:
       shell: bash
       run: |
         cat ${GITHUB_ACTION_PATH}/src/misc/banner.txt
-    - id: setup-python
-      uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0
-      with:
-        python-version: "3.12"
-        update-environment: false
+    - id: validate-enable-timing
+      if: ${{ inputs.enable-timing != 'true' && inputs.enable-timing != 'false' }}
+      shell: bash
+      env:
+        ENABLE_TIMING: ${{ inputs.enable-timing }}
+      run: |
+        echo "enable-timing must be 'true' or 'false'; got '${ENABLE_TIMING}'" >&2
+        exit 2
     - id: setup-gha-timer
+      if: ${{ inputs.enable-timing == 'true' }}
       uses: fulcrumgenomics/gha-timer@d0d886fa587c7f4f714e64d84aa6e274e818d48b # v1.1.1
       with:
         skip-banner: true
@@ -104,7 +117,8 @@ runs:
         WORKING_DIRECTORY: ${{ inputs.working-directory }}
         FALLBACK_FETCH_ALL: ${{ inputs.fallback-fetch-all }}
       run: |
-        gha-timer start --name "Setting up environment variables...🚦"
+        source "${{ github.action_path }}/src/scripts/gha_timer_wrapper.sh"
+        gha_timer start --name "Setting up environment variables...🚦"
         cd $WORKING_DIRECTORY;
         if [ -z "$BASE_REF" ] ; then
             export GITHUB_BASE_REF="$FALLBACK_BASE_REF"
@@ -124,9 +138,9 @@ runs:
         echo "GITHUB_HEAD_REF=$GITHUB_HEAD_REF"
         export GITHUB_BASE_REF
         export GITHUB_HEAD_REF
-        gha-timer elapsed --outcome success --name "Setting up environment variables...🚦"
+        gha_timer elapsed --outcome success --name "Setting up environment variables...🚦"
         set -euo pipefail
-        bash ${{ github.action_path }}/src/scripts/fetch_through_merge_base.sh        
+        bash "${{ github.action_path }}/src/scripts/fetch_through_merge_base.sh"
     # Note: `outcome` does not consider `continue-on-error`
     # Note: fallback-fetch-all is a string, not a boolean, since type is not supported
     # for inputs on composite actions (only workflows).
@@ -146,12 +160,13 @@ runs:
         HEAD_REF: ${{ steps.fetch_through_merge_base.outputs.head-ref }}
         WORKING_DIRECTORY: ${{ inputs.working-directory }}
       run: |
-        gha-timer start --name "Falling back to fetching all...🚦"
+        source "${{ github.action_path }}/src/scripts/gha_timer_wrapper.sh"
+        gha_timer start --name "Falling back to fetching all...🚦"
         cd $WORKING_DIRECTORY;
         git fetch --all --unshallow --verbose;
-        "${{ steps.setup-python.outputs.python-path }}" ${{ github.action_path }}/src/scripts/git_ungraft.py;
+        bash "${{ github.action_path }}/src/scripts/git_ungraft.sh";
         echo -n "Verifying merge-base: "
-        gha-timer elapsed --outcome success --name "Falling back to fetching all...🚦"
+        gha_timer elapsed --outcome success --name "Falling back to fetching all...🚦"
         git merge-base $BASE_REF $HEAD_REF
     - id: finalize
       if: always()
@@ -161,12 +176,13 @@ runs:
         HEAD_REF: ${{ steps.fetch_through_merge_base.outputs.head-ref }}
         WORKING_DIRECTORY: ${{ inputs.working-directory }}
       run: |
-        gha-timer start --name "Getting merge-base (ancestor-ref)...🚦"
+        source "${{ github.action_path }}/src/scripts/gha_timer_wrapper.sh"
+        gha_timer start --name "Getting merge-base (ancestor-ref)...🚦"
         cd $WORKING_DIRECTORY;
         ancestor_ref="$(git merge-base $BASE_REF $HEAD_REF)";
         echo "ancestor-ref=${ancestor_ref}" >> $GITHUB_OUTPUT
-        gha-timer elapsed --outcome success --name "Getting merge-base (ancestor-ref)...🚦"
-        gha-timer stop
+        gha_timer elapsed --outcome success --name "Getting merge-base (ancestor-ref)...🚦"
+        gha_timer stop
         echo "Merge base is:"
         echo ${ancestor_ref}
         echo "Action succeeded! ✅"

src/scripts/fetch_through_merge_base.sh

@@ -1,18 +1,24 @@
-#!/bin/sh
+#!/usr/bin/env bash
 
+set -euo pipefail
 
 DEEPEN_LENGTH=${DEEPEN_LENGTH:-10}
 FAIL_AFTER=${FAIL_AFTER:-1000}
 
-SCRIPT_DIR=$(dirname ${BASH_SOURCE[0]});
+SCRIPT_DIR=$(dirname "${BASH_SOURCE[0]}");
 
-gha-timer start --name "Attempts remaining: ${FAIL_AFTER} 🚦"
+# shellcheck source=./gha_timer_wrapper.sh
+. "${SCRIPT_DIR}/gha_timer_wrapper.sh"
+# shellcheck source=./git_fetch_parents.sh
+. "${SCRIPT_DIR}/git_fetch_parents.sh"
 
-if [ -z "${GITHUB_HEAD_REF}" ]; then
+gha_timer start --name "Attempts remaining: ${FAIL_AFTER} 🚦"
+
+if [ -z "${GITHUB_HEAD_REF:-}" ]; then
   echo "Empty GITHUB_HEAD_REF!";
   exit 1;
 fi
-if [ -z "${GITHUB_BASE_REF}" ]; then
+if [ -z "${GITHUB_BASE_REF:-}" ]; then
   echo "Empty GITHUB_BASE_REF!";
   exit 1;
 fi
@@ -25,21 +31,11 @@ if [ -z "${FAIL_AFTER}" ]; then
   exit 1;
 fi
 
-set -eou pipefail
-
-function git_fetch_parents() {
-    local sha1=${1};
-    for parent_sha1 in $(git show --no-patch --format='%P' "${sha1}"); do
-        git fetch --update-head-ok --update-shallow --progress --quiet --depth=1 origin "${parent_sha1}:__github_parent__";
-        git branch -D __github_parent__;
-    done
-}
-
 # Fetch a branch or tag, and track it.  Do not fetch if a commit (yet).
-if [[ "${GITHUB_BASE_REF}" != "$(git rev-parse --verify ${GITHUB_BASE_REF})" ]]; then
+if [[ "${GITHUB_BASE_REF}" != "$(git rev-parse --verify "${GITHUB_BASE_REF}")" ]]; then
     git fetch --update-head-ok --update-shallow --progress --quiet --depth=1 origin "$GITHUB_BASE_REF:$GITHUB_BASE_REF";
 fi
-if [[ "${GITHUB_HEAD_REF}" != "$(git rev-parse --verify ${GITHUB_HEAD_REF})" ]]; then
+if [[ "${GITHUB_HEAD_REF}" != "$(git rev-parse --verify "${GITHUB_HEAD_REF}")" ]]; then
     git fetch --update-head-ok --update-shallow --progress --quiet --depth=1 origin "$GITHUB_HEAD_REF:$GITHUB_HEAD_REF";
 fi
 
@@ -52,31 +48,29 @@ GITHUB_HEAD_REF=$(git rev-parse "__github_head_ref__");
 # For merge commits we need to fetch both parents (e.g. from GitHub PRs)
 git_fetch_parents "${GITHUB_BASE_REF}";
 git_fetch_parents "${GITHUB_HEAD_REF}";
-python ${SCRIPT_DIR}/git_ungraft.py;
+bash "${SCRIPT_DIR}/git_ungraft.sh";
 
 # keep fetching deeper until we find the common ancestor reference
 while [ -z "$( git merge-base "__github_base_ref__" "__github_head_ref__" )" ]; do
  # check if we are done iterating
-  set +e;
-  let FAIL_AFTER="FAIL_AFTER-1";
-  set -e;
+  FAIL_AFTER=$((FAIL_AFTER - 1))
   if [ "$FAIL_AFTER" -le 0 ]; then
-    gha-timer elapsed --outcome failure
+    gha_timer elapsed --outcome failure
     echo "Failed to find the common ancestors of GITHUB_BASE_REF=${GITHUB_BASE_REF} and GITHUB_HEAD_REF=${GITHUB_HEAD_REF}";
     echo "Failure! ❌"
     exit 1;
   fi
   # fetch deeper
   git fetch --quiet --update-shallow --deepen="$DEEPEN_LENGTH" origin "$GITHUB_HEAD_REF";
   git fetch --quiet --update-shallow --deepen="$DEEPEN_LENGTH" origin "$GITHUB_BASE_REF";
-  python ${SCRIPT_DIR}/git_ungraft.py;
-  echo "Deepend search by ${DEEPEN_LENGTH}‼️";
-  gha-timer elapsed --outcome skipped
-  gha-timer start --name "Attempts remaining: ${FAIL_AFTER} 🚦"
+  bash "${SCRIPT_DIR}/git_ungraft.sh";
+  echo "Deepened search by ${DEEPEN_LENGTH}‼️";
+  gha_timer elapsed --outcome skipped
+  gha_timer start --name "Attempts remaining: ${FAIL_AFTER} 🚦"
 done
 
 
 # cleanup
 git branch -D __github_base_ref__ __github_head_ref__;
-gha-timer elapsed --outcome success
+gha_timer elapsed --outcome success
 echo "Success! ✅"

src/scripts/gha_timer_wrapper.sh

@@ -0,0 +1,47 @@
+# shellcheck shell=bash
+#
+# Defines a `gha_timer` shell function that forwards to the
+# fulcrumgenomics/gha-timer binary when it is present on PATH, and
+# otherwise falls back to emitting GitHub Actions workflow commands
+# (`::group::` / `::endgroup::`) so the log remains collapsible in the
+# workflow UI.
+#
+# The fallback is strictly best-effort log grouping: it emits no
+# timing data and silently drops `gha_timer stop` (the cumulative
+# summary the real binary prints has no analogue in workflow
+# commands).  Users who need per-step timings must leave
+# `enable-timing` on so the real binary is on PATH.
+#
+# This file is intended to be `source`d from bash scripts and from
+# inline `run:` blocks in action.yml.  It is NOT a standalone script.
+
+gha_timer() {
+    if command -v gha-timer >/dev/null 2>&1; then
+        gha-timer "$@"
+        return
+    fi
+    local subcommand="${1:-}"
+    [ $# -gt 0 ] && shift
+    local name=""
+    while [ $# -gt 0 ]; do
+        case "$1" in
+            --name)
+                name="${2:-}"
+                shift
+                [ $# -gt 0 ] && shift
+                ;;
+            --outcome)
+                shift
+                [ $# -gt 0 ] && shift
+                ;;
+            *)
+                shift
+                ;;
+        esac
+    done
+    case "$subcommand" in
+        start)   echo "::group::${name}" ;;
+        elapsed) echo "::endgroup::" ;;
+        *)       : ;;
+    esac
+}

src/scripts/git_fetch_parents.sh

@@ -0,0 +1,47 @@
+# shellcheck shell=bash
+#
+# Defines a `git_fetch_parents` shell function.
+#
+# Given a commit SHA (typically one at the shallow boundary), this
+# function fetches each of the commit's parents into the local
+# repository at `--depth=1`, so that the merge-commit case of a PR
+# has both sides represented locally.  The fetched tip is landed
+# under the private ref namespace
+# `refs/fetch-through-merge-base/__github_parent__` (rather than a
+# branch name that could collide with a user's branch) and is
+# deleted immediately afterward via `git update-ref -d`.
+#
+# Parent discovery reads the raw commit object via `git cat-file -p`
+# rather than a shallow-aware command such as `git show --format=%P`.
+# Shallow-aware commands treat commits recorded in `.git/shallow` as
+# if they had no parents — which is exactly the subset for which this
+# function is meant to do useful work.  The awk filter emits only
+# header-block `parent <sha>` lines (up to the first blank line),
+# so commit-message body contents cannot be misparsed.
+#
+# This file is intended to be `source`d from other shell scripts;
+# running it standalone does nothing.
+
+git_fetch_parents() {
+    local sha1="${1}"
+    local parent_sha1
+    local tmp_ref="refs/fetch-through-merge-base/__github_parent__"
+    # awk sets `in_body` at the first blank line terminating the commit
+    # header and then consumes the rest of the stream without printing.
+    # Do NOT use `awk '/^$/ { exit }'`: under `set -o pipefail`, an
+    # early-exit awk closes the read end of the pipe and causes
+    # `git cat-file` to die of SIGPIPE when the commit body exceeds the
+    # pipe buffer (~64 KB on Linux), aborting the script.
+    for parent_sha1 in $(
+        git cat-file -p "${sha1}" \
+            | awk '
+                in_body { next }
+                /^$/ { in_body = 1; next }
+                $1 == "parent" { print $2 }
+            '
+    ); do
+        git fetch --update-head-ok --update-shallow --progress --quiet \
+            --depth=1 origin "${parent_sha1}:${tmp_ref}"
+        git update-ref -d "${tmp_ref}"
+    done
+}