From 23dbcc8f2ef70a14a50f3ec4b8e8c7027a861afc Mon Sep 17 00:00:00 2001 From: Claude Code Bot Date: Fri, 11 Sep 2026 17:53:08 -0700 Subject: [PATCH] fix(dependabot): report why a PR read failed, not just that it did MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first real run failed on nightowlstudiollc/kebab-tax-netlify#280 with "could not read PR detail" and nothing else. The reason was unrecoverable: `2>/dev/null` discarded GraphQL's stderr, and the body's `errors` array was never inspected. That turned a diagnosable permission failure into guesswork about token grants — none of which was testable locally, because a local login reads the repo fine. Both channels now surface, because GraphQL uses both: transport failures land on stderr, while query-level errors (FORBIDDEN, NOT_FOUND, RATE_LIMITED) come back in the response body with HTTP 200 and exit 0. The failure itself was correct behaviour — collect.sh refused to emit a partial survey, and run-digest.sh refused to publish one. The digest's whole premise is that nothing goes unseen, so a silent omission is the one outcome it must not produce. This change does not alter that; it only makes the refusal explain itself. Claude-Session: https://claude.ai/code/session_01ESsw699T54JHARkQXrdL3o --- scripts/dependabot-digest/collect.sh | 24 +++++++++++++++++-- .../dependabot-digest/tests/test-render.sh | 20 ++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/scripts/dependabot-digest/collect.sh b/scripts/dependabot-digest/collect.sh index 3b328d6..fd2b002 100755 --- a/scripts/dependabot-digest/collect.sh +++ b/scripts/dependabot-digest/collect.sh @@ -140,17 +140,37 @@ while IFS=$'\t' read -r nwo number; do [[ -z "${nwo}" ]] && continue repo="${nwo#*/}" base_red="$(base_red_for "${nwo}")" + # Keep stderr: GraphQL reports the reason a read failed (a permission the + # token lacks, a rate limit, a repo it cannot see), and that reason is the + # only thing that makes this failure diagnosable. Discarding it once cost an + # hour of guessing at token grants on 2026-09-11. + detail_err="$(mktemp)" # Capture the exit status on its own line: testing $? after a [[ ]] would # read the test's status, not gh's. detail="$(gh api graphql \ -f query="${pr_detail_query}" \ - -F owner="${owner}" -F repo="${repo}" -F number="${number}" 2>/dev/null)" + -F owner="${owner}" -F repo="${repo}" -F number="${number}" 2>"${detail_err}")" detail_rc=$? if [[ "${detail_rc}" -ne 0 ]] \ || ! jq -e '.data.repository.pullRequest' >/dev/null 2>&1 <<<"${detail}"; then - echo "collect.sh: ${nwo}#${number}: could not read PR detail" >&2 + echo "collect.sh: ${nwo}#${number}: could not read PR detail (exit ${detail_rc})" >&2 + # GraphQL returns errors in the body with HTTP 200, so both streams matter. + if [[ -s "${detail_err}" ]]; then + detail_err_text="$(tr '\n' ' ' <"${detail_err}")" + echo "collect.sh: stderr: ${detail_err_text}" >&2 + fi + # Parenthesize each alternative: `+` binds tighter than `//`, so + # `.type // "?" + ": " + .message` parses as `.type // ("?: " + .message)` + # and yields a bare "FORBIDDEN" with the message dropped — exactly the + # detail this block exists to print. Verified against a sample error body. + gql_errors="$(jq -r '.errors // [] | map((.type // "?") + ": " + (.message // "?")) | join("; ")' <<<"${detail}" 2>/dev/null)" + if [[ -n "${gql_errors}" && "${gql_errors}" != "null" ]]; then + echo "collect.sh: graphql: ${gql_errors}" >&2 + fi + rm -f "${detail_err}" exit 1 fi + rm -f "${detail_err}" jq -c --arg nwo "${nwo}" --argjson base_red "${base_red}" ' .data.repository.pullRequest as $pr | ([$pr.commits.nodes[0].commit.statusCheckRollup.contexts.nodes[]? diff --git a/scripts/dependabot-digest/tests/test-render.sh b/scripts/dependabot-digest/tests/test-render.sh index 1e9e322..327d25f 100755 --- a/scripts/dependabot-digest/tests/test-render.sh +++ b/scripts/dependabot-digest/tests/test-render.sh @@ -31,6 +31,26 @@ for f in "${DIR}"/*.sh "${HERE}"/*.sh; do fi done +# Every jq program embedded in these scripts must parse, and the ones that +# format diagnostics must also say what they mean. `+` binds tighter than `//` +# in jq, so `.type // "?" + ": " + .message` silently parses as +# `.type // ("?: " + .message)` and prints a bare "FORBIDDEN" with the message +# dropped — the failure is invisible until the moment you need the message. +# Extract the error formatter from collect.sh and run it against a real error +# body rather than eyeballing the source. +gql_fmt="$(grep -o "jq -r '\.errors[^']*'" "${DIR}/collect.sh" | head -1 | sed "s/^jq -r '//; s/'$//")" +if [[ -z "${gql_fmt}" ]]; then + _fail "could not find the GraphQL error formatter in collect.sh" +else + sample='{"errors":[{"type":"FORBIDDEN","message":"Resource not accessible by integration"}]}' + formatted="$(jq -r "${gql_fmt}" <<<"${sample}" 2>/dev/null)" + if [[ "${formatted}" == *"FORBIDDEN"* && "${formatted}" == *"not accessible"* ]]; then + _pass "the GraphQL error formatter keeps both the type and the message" + else + _fail "the error formatter dropped part of the error: '${formatted}'" + fi +fi + body="${WORK}/body.md" cat "${FIX}/synthetic.ndjson" "${FIX}/live-2026-09-11.ndjson" \ | bash "${DIR}/classify.sh" \