fix: use GitHub API for diff fetching, avoid shell diff command #5

2026-04-30T22:05:19Z

rileyseaburg commented

2026-04-30 22:05:19 +00:00

(Migrated from github.com)

Problem

PR #506 in spotlessbinco (and any repo using this action) fails with:

diff: unrecognized option '--git'
diff: Try 'diff --help' for more information.

The action relied solely on git diff for gathering PR diffs. On some GitHub Actions runner images, this can interact with environment quirks that cause the system diff command to be invoked with --git flags it doesn't understand.

Fix

Primary: GitHub API diff — Uses Accept: application/vnd.github.v3.diff to fetch the diff directly from the GitHub API. No local tooling dependency, no shell diff command involved.
Secondary: git diff — Falls back to git diff only when the API is unavailable (e.g., private repos with restricted token scopes).
Safer PROMPT construction — Reads diff content into a bash variable before embedding in the prompt string, rather than using inline `` inside a double-quoted string that also contains backtick-heavy markdown fencing. This avoids bash parsing edge-cases where diff --git headers in the diff content could interact with backtick escaping.

Testing

shellcheck passes (only pre-existing SC2000 style warnings)
The API diff approach is used by many GitHub Actions and is more reliable than local git diff

Impact

Unblocks PR #506 (spotlessbinco) — fix: prevent checkout price mismatches (Issue #505)
Makes the action more reliable across all runner environments

## Problem PR #506 in spotlessbinco (and any repo using this action) fails with: ``` diff: unrecognized option '--git' diff: Try 'diff --help' for more information. ``` The action relied solely on `git diff` for gathering PR diffs. On some GitHub Actions runner images, this can interact with environment quirks that cause the system `diff` command to be invoked with `--git` flags it doesn't understand. ## Fix 1. **Primary: GitHub API diff** — Uses `Accept: application/vnd.github.v3.diff` to fetch the diff directly from the GitHub API. No local tooling dependency, no shell `diff` command involved. 2. **Secondary: git diff** — Falls back to `git diff` only when the API is unavailable (e.g., private repos with restricted token scopes). 3. **Safer PROMPT construction** — Reads diff content into a bash variable before embedding in the prompt string, rather than using inline `` inside a double-quoted string that also contains backtick-heavy markdown fencing. This avoids bash parsing edge-cases where `diff --git` headers in the diff content could interact with backtick escaping. ## Testing - shellcheck passes (only pre-existing SC2000 style warnings) - The API diff approach is used by many GitHub Actions and is more reliable than local `git diff` ## Impact - Unblocks PR #506 (spotlessbinco) — `fix: prevent checkout price mismatches (Issue #505)` - Makes the action more reliable across all runner environments

👀 1

gemini-code-assist[bot] (Migrated from github.com) reviewed

2026-04-30 22:06:25 +00:00

gemini-code-assist[bot] (Migrated from github.com) left a comment

Code Review

This pull request updates the diff gathering logic to prioritize the GitHub API over local git commands, reducing dependencies on runner environments. It also refactors prompt construction to avoid bash parsing issues when handling raw diff text. Feedback suggests extending the API-first logic to the apply_fix function for consistency and optimizing the curl implementation by streaming output directly to a file to better handle large diffs and avoid memory overhead.

## Code Review This pull request updates the diff gathering logic to prioritize the GitHub API over local git commands, reducing dependencies on runner environments. It also refactors prompt construction to avoid bash parsing issues when handling raw diff text. Feedback suggests extending the API-first logic to the `apply_fix` function for consistency and optimizing the `curl` implementation by streaming output directly to a file to better handle large diffs and avoid memory overhead.

action/lib/local.sh

					
				@ -37,0 +53,4 @@

				  fi

				  # ── Secondary: local git diff ────────────────────────────────────

				  if [ "${api_ok}" != "true" ]; then

Streaming the curl output directly to a file is more efficient than capturing it in a bash variable, especially for large diffs, as it avoids memory overhead and potential variable size limits in bash.

Additionally, note that the GitHub API returns the full PR diff, whereas the fallback git diff command (lines 58-60) filters for specific file extensions (e.g., .rs, .py, .ts). This inconsistency means that when the API is used, the prompt may include files (like lockfiles or documentation) that were previously excluded. Consider if you should apply similar filtering to the API output or if this expanded context is intentional.

    if curl -sS -f \
      -H "Authorization: token ${GITHUB_TOKEN}" \
      -H "Accept: application/vnd.github.v3.diff" \
      "https://api.github.com/repos/${REPO_FULL_NAME}/pulls/${PR_NUMBER}" \
      > "${DIFF_FILE}" 2>>"${CODETETHER_LOG_FILE}" && [ -s "${DIFF_FILE}" ]; then
      api_ok="true"
      checkpoint "local: GitHub API diff fetched ($({ wc -l < "${DIFF_FILE}"; } 2>/dev/null || echo 0) lines)"
    else
      log_warn "GitHub API diff failed or returned empty — using git diff instead"
    fi

![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg) Streaming the `curl` output directly to a file is more efficient than capturing it in a bash variable, especially for large diffs, as it avoids memory overhead and potential variable size limits in bash. Additionally, note that the GitHub API returns the full PR diff, whereas the fallback `git diff` command (lines 58-60) filters for specific file extensions (e.g., `.rs`, `.py`, `.ts`). This inconsistency means that when the API is used, the prompt may include files (like lockfiles or documentation) that were previously excluded. Consider if you should apply similar filtering to the API output or if this expanded context is intentional. ```suggestion if curl -sS -f \ -H "Authorization: token ${GITHUB_TOKEN}" \ -H "Accept: application/vnd.github.v3.diff" \ "https://api.github.com/repos/${REPO_FULL_NAME}/pulls/${PR_NUMBER}" \ > "${DIFF_FILE}" 2>>"${CODETETHER_LOG_FILE}" && [ -s "${DIFF_FILE}" ]; then api_ok="true" checkpoint "local: GitHub API diff fetched ($({ wc -l < "${DIFF_FILE}"; } 2>/dev/null || echo 0) lines)" else log_warn "GitHub API diff failed or returned empty — using git diff instead" fi ```

action/lib/local.sh

The apply_fix function still relies solely on git diff (line 198) to gather the current state before applying changes. Since the primary goal of this PR is to avoid issues with the local diff command on certain runner images, this function should be updated to use the same GitHub API-first logic implemented in gather_diff. Otherwise, the apply_fix flow will continue to fail in the environments this PR aims to support.

![high](https://www.gstatic.com/codereviewagent/high-priority.svg) The `apply_fix` function still relies solely on `git diff` (line 198) to gather the current state before applying changes. Since the primary goal of this PR is to avoid issues with the local `diff` command on certain runner images, this function should be updated to use the same GitHub API-first logic implemented in `gather_diff`. Otherwise, the `apply_fix` flow will continue to fail in the environments this PR aims to support.

copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed

2026-04-30 22:10:01 +00:00

copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

Pull request overview

Updates the local-mode diff collection and prompt construction in the GitHub Action to avoid runner-specific failures caused by relying on local diff tooling, by preferring GitHub’s PR .diff API with a git diff fallback.

Changes:

Fetch PR diffs primarily via GitHub API (Accept: application/vnd.github.v3.diff), falling back to git diff when unavailable.
Refactor prompt construction to avoid inline command substitution inside backtick-heavy markdown fencing.
Minor quoting/robustness tweaks around truncation and artifact copy.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

## Pull request overview Updates the local-mode diff collection and prompt construction in the GitHub Action to avoid runner-specific failures caused by relying on local diff tooling, by preferring GitHub’s PR `.diff` API with a `git diff` fallback. **Changes:** - Fetch PR diffs primarily via GitHub API (`Accept: application/vnd.github.v3.diff`), falling back to `git diff` when unavailable. - Refactor prompt construction to avoid inline command substitution inside backtick-heavy markdown fencing. - Minor quoting/robustness tweaks around truncation and artifact copy. --- 💡 <a href="/rileyseaburg/codetether-action/new/main?filename=.github/instructions/*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.

action/lib/local.sh

					
				@ -37,0 +45,4 @@

				      2>>"${CODETETHER_LOG_FILE}" || true)"

				    if [ -n "${api_diff}" ]; then

				      printf '%s' "${api_diff}" > "${DIFF_FILE}"

				      api_ok="true"

Switching to the GitHub API .diff endpoint changes behavior vs the previous git diff call: the API response includes all files in the PR, while the old implementation filtered by pathspec (e.g. *.rs, *.py, *.ts, etc.). This makes it easier for large/generated/non-code diffs (lockfiles, vendor, etc.) to consume the 3000-line budget and crowd out relevant code changes. Consider reintroducing equivalent filtering (or explicitly documenting that the API mode intentionally reviews all file types).

Switching to the GitHub API `.diff` endpoint changes behavior vs the previous `git diff` call: the API response includes *all* files in the PR, while the old implementation filtered by pathspec (e.g. `*.rs`, `*.py`, `*.ts`, etc.). This makes it easier for large/generated/non-code diffs (lockfiles, vendor, etc.) to consume the 3000-line budget and crowd out relevant code changes. Consider reintroducing equivalent filtering (or explicitly documenting that the API mode intentionally reviews all file types).

action/lib/local.sh

					
				@ -37,0 +48,4 @@

				      api_ok="true"

				      checkpoint "local: GitHub API diff fetched ($({ wc -l < "${DIFF_FILE}"; } 2>/dev/null || echo 0) lines)"

				    else

				      log_warn "GitHub API diff returned empty — using git diff instead"

The GitHub API diff is captured into the api_diff bash variable before writing to disk. For large PRs this can consume a lot of memory (and can fail) before the later 3000-line truncation happens. Prefer streaming curl output directly to DIFF_FILE (e.g., curl ... -o "$DIFF_FILE") and then checking the file is non-empty / has lines before setting api_ok=true.

The GitHub API diff is captured into the `api_diff` bash variable before writing to disk. For large PRs this can consume a lot of memory (and can fail) before the later 3000-line truncation happens. Prefer streaming curl output directly to `DIFF_FILE` (e.g., `curl ... -o "$DIFF_FILE"`) and then checking the file is non-empty / has lines before setting `api_ok=true`.

Sign in to join this conversation.

No reviewers