Shell Scripting

gh api --paginate and jq

gh api --paginate applies the --jq expression independently to each page of results, not to the combined output. This is a documented gh CLI behavior and a common source of bugs.

Do not use aggregating jq filters directly in --jq with --paginate:

# WRONG — `length` runs per-page; produces one number per page, not a total
count=$(gh api --paginate /repos/{owner}/{repo}/issues/comments --jq 'length')

Do collect all pages first, then pipe to a separate jq -s (slurp) call. jq -s slurps the input into an array; use add to flatten before aggregating:

# CORRECT — slurp all pages, flatten with add, then aggregate
count=$(gh api --paginate /repos/{owner}/{repo}/issues/comments | jq -s 'add | length')

Without --jq, gh api --paginate merges all page arrays into a single flat JSON array before writing to stdout. jq -s then wraps that into an array-of-one; add unwraps it back to the flat array, and the aggregating filter runs once over all items. This pattern is defensive — it works correctly whether the upstream emits one merged array or (as when --jq is present) one array per page.

This applies to any aggregating filter: length, sort_by, group_by, add, min_by, max_by, etc. If the filter only selects or transforms individual items (e.g., .[] | .id), per-page application is fine — but pipe the result through a final jq -s step before any cross-page aggregation.

When reviewing shell scripts: Flag --paginate --jq '... | length' (or any other aggregating filter in --jq) as a medium-severity finding. The fix is always to move the aggregation to a separate | jq -s 'add | ...' pipe.

Alternative — --slurp flag: When no inline --jq transform is needed, gh api --paginate --slurp combines pages into a single array directly. However, --slurp is mutually exclusive with --jq (errors with "the --slurp option is not supported with --jq or --template"), so the | jq -s 'add | ...' pipe pattern is required whenever you also need per-item filtering.

yq/jq pitfalls

yq and jq share similar syntax but have different built-in function sets. Using a jq-only function in a yq expression causes a parse error (non-zero exit, stderr message). It becomes a silent failure only when combined with error suppression (see "Fail-open error suppression" below).

Common mistakes:

• downcase is the correct yq function for lowercasing strings. ascii_downcase (from jq) does not exist in yq and causes a parse error.
• ascii_upcase (jq) vs upcase (yq) — same pattern, different function names.
• ltrimstr/rtrimstr (jq) have no yq equivalent — yq only has trim (whitespace-only). split("str") (single argument, literal string) works the same in both. jq's 2-arg split(regex; flags) form is the one case in this list that does not produce a parse error: yq accepts a second argument to split without erroring (exit 0, no stderr), but its behavior does not match jq's — verified output diverges unpredictably depending on the pattern and second argument, rather than failing loudly. Don't assume 2-arg split behaves like jq; test the actual output for your specific case, or restrict yourself to the single-argument literal form.

When generating yq expressions in workflow YAML, verify the function exists in yq's built-in set, not jq's. The yq built-in operators documentation is the reference.

When reviewing PRs: Flag unrecognized yq function names (e.g., jq-only functions used in yq expressions) as a medium-severity finding. The fix is to replace the function with its yq equivalent.

Fail-open error suppression

2>/dev/null || echo "" (or || true, || :) suppresses errors and provides a fallback value. Whether this pattern is acceptable depends on what the step does:

• Non-critical steps (logging, metrics, optional annotations): fail-open is acceptable. A silent failure in a logging step does not affect correctness.
• Gate/guard steps (dispatch routing, feature flags, conditional execution, skip/include logic): fail-open is dangerous. Silent failure in a gate means the gate never fires — the guarded behavior either always executes or never executes, depending on how the fallback value is interpreted downstream.

# DANGEROUS — if yq fails (e.g., invalid function), the gate silently
# passes through with an empty string, making the conditional meaningless
label=$(echo "$yaml" | yq '.metadata.labels.env // ""' 2>/dev/null || echo "")

# SAFER — capture stderr, emit a warning on failure, fail closed
if ! label=$(echo "$yaml" | yq '.metadata.labels.env // ""' 2>&1); then
  echo "::warning::yq failed: $label"
  exit 1
fi

When writing workflow steps: If a step gates dispatch, skips execution, or controls conditional logic, ensure its error path surfaces the failure (e.g., via ::warning::) rather than swallowing it silently — capture stderr to a variable and emit a warning on failure, instead of 2>/dev/null. Whether to then fail closed (exit 1) or deliberately proceed depends on the gate's purpose: authorization/security gates must fail closed (see scripts/check-e2e-authorization.sh, which traps errors and sets authorized=false), while feature/agent-enablement gates may deliberately warn-and-proceed (availability over strictness — an operator would rather a stage run than block on a transient parse error). The DANGEROUS/SAFER examples above illustrate the difference between silent and surfaced failure, not a universal mandate for exit 1.

When reviewing PRs: Flag 2>/dev/null || echo "" (or || true) in workflow steps that gate dispatch or skip execution as a medium-severity or higher finding — unless the fallback value is verified to be restrictive (narrows what's allowed) rather than permissive (widens what's allowed) downstream. Silent failure in a permissive-fallback gate means the gate is non-functional; the same pattern feeding a restrictive fallback is comparatively safe (it fails toward "allow nothing" rather than "allow everything"). Judge severity by what the fallback value actually does downstream, not by the presence of 2>/dev/null || echo ""/|| true alone.

Stdout contamination in command substitution

When a shell function is called inside $(...) command substitution, everything the function writes to stdout becomes part of the captured variable. If the function also writes diagnostic output (log messages, retry warnings, ::error::/::warning:: annotations) to stdout, that output is concatenated with the intended return value, silently corrupting it.

This is a common source of bugs in shell scripts that use helper functions with retry logic or error reporting. The captured variable may contain a mix of JSON and diagnostic text, causing downstream jq/yq parsing to fail silently or produce wrong results.

# WRONG — echo writes to stdout, contaminating the captured variable
retry_curl() {
  local attempt=1 max=3 delay=5
  while true; do
    if curl "$@"; then return 0; fi
    if (( attempt >= max )); then
      echo "::error::Request failed after ${max} attempts"
      return 1
    fi
    echo "::warning::Attempt ${attempt}/${max} failed, retrying in ${delay}s..."
    sleep "${delay}"
    (( attempt++ ))
  done
}

# resp now contains "::warning::Attempt 1/3 failed, retrying in 5s...\n{json}"
# jq parsing will fail or produce wrong results
resp=$(retry_curl -fsSL "https://api.example.com/data")
tag=$(echo "${resp}" | jq -r '.tag_name')

Direct all non-value output to stderr using >&2:

# CORRECT — diagnostic output goes to stderr, only curl's stdout is captured
retry_curl() {
  local attempt=1 max=3 delay=5
  while true; do
    if curl "$@"; then return 0; fi
    if (( attempt >= max )); then
      echo "::error::Request failed after ${max} attempts" >&2
      return 1
    fi
    echo "::warning::Attempt ${attempt}/${max} failed, retrying in ${delay}s..." >&2
    sleep "${delay}"
    (( attempt++ ))
  done
}

# resp contains only the curl response — jq parsing works correctly
resp=$(retry_curl -fsSL "https://api.example.com/data")
tag=$(echo "${resp}" | jq -r '.tag_name')

The same rule applies to any function output that is not the intended return value: progress messages, debug logging, echo "Retrying...", and GitHub Actions annotations (::warning::, ::error::, ::notice::). If the function might be called inside $(...), all of these must use >&2.

When reviewing PRs: If a shell function is used inside $(...) or backtick command substitution, verify that all non-value output (log messages, warnings, ::error::/::warning:: annotations, retry diagnostics) is directed to stderr. Flag stdout contamination as medium-severity when the captured value is used in conditional logic or parsed by jq/yq, and as high-severity when it feeds into security-sensitive operations (token handling, authorization checks, cryptographic verification).