From f42881aa345f12018f2bc17887c720effe703b5c Mon Sep 17 00:00:00 2001 From: Claude Code Bot Date: Fri, 11 Sep 2026 18:20:05 -0700 Subject: [PATCH 1/3] fix(dependabot): probe the access the survey needs, and print the failing field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects found while diagnosing a FORBIDDEN that took four runs to localize. The private-repo probe tested the wrong thing. It read `repos//`, which answers under the Metadata permission alone — so it passed for a token that could not read a single commit, while every PR detail read failed. The survey reads each PR's checks through `pullRequest.commits`, so the probe now reads `commits` on the probe repo. A probe that cannot fail the way the real work fails is not a probe. The error formatter dropped the GraphQL `path`. The message alone — "Resource not accessible by personal access token" — names no field, so the CI log could not say which read was refused. The path (`repository.pullRequest.commits.nodes.0`) is what finally identified it, and it was available in the response body the whole time. Both are covered by assertions validated against the known-bad form: removing the path fails with "the error formatter dropped the path", and the probe assertion now matches the commits-based message. Advances #120. Claude-Session: https://claude.ai/code/session_01ESsw699T54JHARkQXrdL3o --- scripts/dependabot-digest/collect.sh | 14 ++++++++++---- scripts/dependabot-digest/tests/test-render.sh | 12 +++++++++++- scripts/dependabot-digest/tests/test-run-digest.sh | 2 +- 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/scripts/dependabot-digest/collect.sh b/scripts/dependabot-digest/collect.sh index fd2b002..dd605ef 100755 --- a/scripts/dependabot-digest/collect.sh +++ b/scripts/dependabot-digest/collect.sh @@ -50,9 +50,15 @@ GRAPHQL verify_private_visibility() { local probe="$1" [[ -z "${probe}" ]] && return 0 - if ! gh api "repos/${probe}" --jq '.name' >/dev/null 2>&1; then - echo "collect.sh: ${owner}: cannot read private probe repo ${probe};" \ - "token lacks private-repo access, so results would be silently incomplete" >&2 + # Probe `commits`, not `repos/`. Repository metadata answers under the + # Metadata permission alone, so a metadata probe passes for a token that + # cannot read a single commit — and the survey reads every PR's checks + # through `pullRequest.commits`. Measured 2026-09-11: this probe passed on + # nightowlstudiollc while every PR detail read returned FORBIDDEN, which is + # exactly the silent incompleteness it exists to prevent. + if ! gh api "repos/${probe}/commits?per_page=1" --jq '.[0].sha' >/dev/null 2>&1; then + echo "collect.sh: ${owner}: cannot read commits on private probe repo ${probe};" \ + "token lacks the access this survey needs, so results would be silently incomplete" >&2 return 1 fi return 0 @@ -163,7 +169,7 @@ while IFS=$'\t' read -r nwo number; do # `.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)" + gql_errors="$(jq -r '.errors // [] | map((.type // "?") + " at " + ((.path // []) | map(tostring) | join(".")) + ": " + (.message // "?")) | join("; ")' <<<"${detail}" 2>/dev/null)" if [[ -n "${gql_errors}" && "${gql_errors}" != "null" ]]; then echo "collect.sh: graphql: ${gql_errors}" >&2 fi diff --git a/scripts/dependabot-digest/tests/test-render.sh b/scripts/dependabot-digest/tests/test-render.sh index 327d25f..256d5ed 100755 --- a/scripts/dependabot-digest/tests/test-render.sh +++ b/scripts/dependabot-digest/tests/test-render.sh @@ -42,13 +42,23 @@ gql_fmt="$(grep -o "jq -r '\.errors[^']*'" "${DIR}/collect.sh" | head -1 | sed " 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"}]}' + # The `path` matters as much as the message. It is what identified the + # failing field as `commits` on 2026-09-11 — the message alone + # ("Resource not accessible by personal access token") names no field, and + # the CI log could not show which read was refused because collect.sh was + # dropping the path. + sample='{"errors":[{"type":"FORBIDDEN","path":["repository","pullRequest","commits","nodes",0],"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 + if [[ "${formatted}" == *"commits"* ]]; then + _pass "the GraphQL error formatter keeps the field path" + else + _fail "the error formatter dropped the path, which names the failing field: '${formatted}'" + fi fi body="${WORK}/body.md" diff --git a/scripts/dependabot-digest/tests/test-run-digest.sh b/scripts/dependabot-digest/tests/test-run-digest.sh index 840f25d..c14a6f4 100755 --- a/scripts/dependabot-digest/tests/test-run-digest.sh +++ b/scripts/dependabot-digest/tests/test-run-digest.sh @@ -88,7 +88,7 @@ PATH="${BIN}:${PATH}" DIGEST_OWNERS="one" DIGEST_TOKEN_ONE="t" \ DIGEST_PROBE_ONE="one/private" \ bash "${DIR}/run-digest.sh" --dry-run >/dev/null 2>"${WORK}/probe.err" probe_rc=$? -if [[ "${probe_rc}" -ne 0 ]] && grep -q "cannot read private probe repo" "${WORK}/probe.err"; then +if [[ "${probe_rc}" -ne 0 ]] && grep -q "cannot read commits on private probe repo" "${WORK}/probe.err"; then _pass "an unreadable private probe fails the run" else _fail "an unreadable private probe did not fail the run (exit ${probe_rc})" From e77d75070cb9cda49f6c6736889a480a5769608e Mon Sep 17 00:00:00 2001 From: Claude Code Bot Date: Fri, 11 Sep 2026 19:08:00 -0700 Subject: [PATCH 2/3] fix(dependabot): refuse a check rollup the token could not fully read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A fine-grained token cannot be granted Checks: read at all (github.com/orgs/community/discussions/129512). GitHub does not refuse the query for it. It returns statusCheckRollup with HTTP 200 and the correct totalCount, then nulls every CheckRun the token may not see. Measured on kebab-tax-netlify#280: 12 contexts returned, 11 of them null, the one survivor a Netlify StatusContext. Mapped as written, that PR's seven failing builds and one green required check became "1 failing check, 0 required checks". The consequence, reproduced against the classifier: a PR whose only checks are nulled CheckRuns classified as ready-to-merge and was issued a paste-ready `merge-lock authorize` line. Most of the fleet has no Netlify status to survive the nulling, so that was the common path, not the edge case. A digest that recommends merging PRs with failing builds is worse than no digest — it is the exact failure the classifier was written to prevent. Three changes, in depth order: - collect.sh refuses any response containing a nameless context, naming the count and the discussion. An unreadable check is not an absent one. - classify.sh adds a `checks-unreadable` bucket that outranks every other signal, because nothing else can be trusted once the check data is known to be partial. This is the second line of defence, in the component that decides what is safe to merge. - render.sh renders that bucket first, and says plainly that nothing in it is green. Also: GraphQL errors are now grouped rather than printed once per context (one refused permission produced eleven identical lines), and the test that extracts the error formatter now reads a multi-line jq program — a line-based grep found nothing after the reformat and failed for the wrong reason. The regression test was validated by running it against the unguarded classifier, where it fails with "a PR whose checks are all null classified as ready-to-merge". It initially passed vacuously against an undefined path variable; that is now caught explicitly. Advances #120. Claude-Session: https://claude.ai/code/session_01ESsw699T54JHARkQXrdL3o --- scripts/dependabot-digest/classify.sh | 13 +++++++ scripts/dependabot-digest/collect.sh | 37 ++++++++++++++++++- scripts/dependabot-digest/render.sh | 7 +++- .../dependabot-digest/tests/test-classify.sh | 24 +++++++++++- .../dependabot-digest/tests/test-render.sh | 7 +++- 5 files changed, 83 insertions(+), 5 deletions(-) diff --git a/scripts/dependabot-digest/classify.sh b/scripts/dependabot-digest/classify.sh index 739ba5e..c909073 100755 --- a/scripts/dependabot-digest/classify.sh +++ b/scripts/dependabot-digest/classify.sh @@ -94,6 +94,15 @@ output="$(jq -c --argjson hold "${hold_labels}" --argjson neutral_ok "${neutral_ and (.name as $n | any($neutral_ok[]; . as $p | $n | startswith($p)) | not); . as $pr + # A check with no name is one the token could not read, not one that passed. + # GitHub serves statusCheckRollup with HTTP 200 and the right totalCount, + # then nulls every CheckRun a fine-grained token lacks Checks: read for — + # which cannot be granted to one at all + # (github.com/orgs/community/discussions/129512). Counting those as absent + # turns seven red builds into "nothing failing" and puts the PR in + # ready-to-merge. collect.sh refuses such a response outright; this is the + # second line of defence, in the component that decides what is safe. + | ([.checks[] | select((.name // null) == null)]) as $unreadable | ([.checks[] | select(.required)]) as $req | ([$req[] | select(failed)]) as $req_failed | ([$req[] | select(pending)]) as $req_pending @@ -128,8 +137,12 @@ output="$(jq -c --argjson hold "${hold_labels}" --argjson neutral_ok "${neutral_ [$neutral_blocking[] | "NEUTRAL: " + .name] + (if $changes_requested then ["review: CHANGES_REQUESTED"] else [] end)) + | .unreadableChecks = ($unreadable | length) | .bucket = ( if ($held_by | length) > 0 or .isDraft then "held" + # Unreadable checks outrank every other signal: nothing below can be + # trusted when the check data is known to be incomplete. + elif ($unreadable | length) > 0 then "checks-unreadable" elif ($req_failed | length) > 0 then "needs-work" elif .mergeStateStatus == "DIRTY" or .mergeable == "CONFLICTING" then "conflicted" elif ($req_pending | length) > 0 then "waiting-on-ci" diff --git a/scripts/dependabot-digest/collect.sh b/scripts/dependabot-digest/collect.sh index dd605ef..ce6fe23 100755 --- a/scripts/dependabot-digest/collect.sh +++ b/scripts/dependabot-digest/collect.sh @@ -162,14 +162,27 @@ while IFS=$'\t' read -r nwo number; do 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}")" + # gh concatenates one copy of the message per GraphQL error, so a single + # refused permission prints the same sentence eleven times on one line. + # The graphql line below already reports the count, so truncate here + # rather than repeat it. + detail_err_text="$(tr '\n' ' ' <"${detail_err}" | cut -c1-200)" 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 // "?") + " at " + ((.path // []) | map(tostring) | join(".")) + ": " + (.message // "?")) | join("; ")' <<<"${detail}" 2>/dev/null)" + # Collapse repeats: one refused permission yields one error per context, + # and eleven identical lines bury the one fact that matters. Group by + # type+message, report the first path and how many followed it. + gql_errors="$(jq -r '.errors // [] + | group_by((.type // "?") + "" + (.message // "?")) + | map((.[0].type // "?") + " at " + + (((.[0].path // []) | map(tostring) | join("."))) + + (if length > 1 then " (and \(length - 1) more)" else "" end) + + ": " + (.[0].message // "?")) + | join("; ")' <<<"${detail}" 2>/dev/null)" if [[ -n "${gql_errors}" && "${gql_errors}" != "null" ]]; then echo "collect.sh: graphql: ${gql_errors}" >&2 fi @@ -177,6 +190,26 @@ while IFS=$'\t' read -r nwo number; do exit 1 fi rm -f "${detail_err}" + # A context that comes back as an empty object is an access failure wearing + # the shape of a result. GitHub returns the rollup with HTTP 200 and the + # right totalCount, then nulls every CheckRun the token may not read — + # measured 2026-09-11, where 11 of 12 contexts were null while only the + # StatusContext survived. Mapped naively that becomes "no failing checks and + # no required checks", which classifies a PR with seven red builds as + # ready-to-merge and offers a merge-lock line for it. Refuse instead: an + # unreadable check is not an absent one. + # A readable context always carries a name (CheckRun) or a context + # (StatusContext). A nulled one carries neither, and arrives as `{}`. + nulled="$(jq '[.data.repository.pullRequest.commits.nodes[0].commit.statusCheckRollup.contexts.nodes[]? + | select((.name // .context) == null)] | length' <<<"${detail}" 2>/dev/null)" + if [[ -n "${nulled}" && "${nulled}" != "null" && "${nulled}" -gt 0 ]]; then + echo "collect.sh: ${nwo}#${number}: ${nulled} check(s) returned null —" \ + "the token can see that checks exist but not what they say." \ + "A fine-grained token cannot grant Checks: read" \ + "(github.com/orgs/community/discussions/129512), so this survey would" \ + "under-report failures rather than fail. Refusing to continue." >&2 + exit 1 + fi 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/render.sh b/scripts/dependabot-digest/render.sh index 595e88e..e8f5df1 100755 --- a/scripts/dependabot-digest/render.sh +++ b/scripts/dependabot-digest/render.sh @@ -52,7 +52,10 @@ else # The one fact that explains this row. Naming the specific check beats a # generic status: "build" sends someone to a file, "UNSTABLE" does not. def reason: - if .bucket == "needs-work" then "required check failed: " + (.blockingFailures | join(", ")) + if .bucket == "checks-unreadable" then + "CHECK DATA INCOMPLETE: " + (.unreadableChecks | tostring) + + " check(s) unreadable — treat nothing here as green" + elif .bucket == "needs-work" then "required check failed: " + (.blockingFailures | join(", ")) elif .bucket == "hook-blocked" then (.hookBlockers | join("; ")) elif .bucket == "advisory-red" then (if (.ownFailures | length) > 0 @@ -74,6 +77,8 @@ else echo } + render_bucket checks-unreadable "Check results could not be read" \ + "The token could not read these PRs' check results, so their status is unknown — not green. GitHub returns the rollup with the right count and nulls the checks it will not show, which reads as \"nothing failing\" unless caught. A fine-grained token cannot be granted Checks: read at all (github.com/orgs/community/discussions/129512). Do not merge on the strength of this section." render_bucket ready-to-merge "Ready for a merge-lock" \ "Every required check passed and the pre-merge hook has no mechanical objection. The hook still runs its AI review on merge, which can block on content — this list means nothing stands in the way yet, not that the merge will succeed." render_bucket update-branch "Needs a branch update" \ diff --git a/scripts/dependabot-digest/tests/test-classify.sh b/scripts/dependabot-digest/tests/test-classify.sh index 2da6771..84cbc2a 100755 --- a/scripts/dependabot-digest/tests/test-classify.sh +++ b/scripts/dependabot-digest/tests/test-classify.sh @@ -165,9 +165,31 @@ else _fail "classify.sh failed on the real fixture" fi + +# A check whose fields came back null is an access failure, not a passing +# check. GitHub returns statusCheckRollup with HTTP 200 and the correct +# totalCount, then nulls every CheckRun a fine-grained token may not read — +# Checks: read cannot be granted to one at all +# (github.com/orgs/community/discussions/129512). Measured 2026-09-11: 11 of 12 +# contexts null, and the one survivor was a Netlify StatusContext. +# +# collect.sh refuses that response outright. This asserts the consequence if it +# ever stops doing so: nulled checks must never read as "nothing is failing". +nulled='{"repo":"o/r","number":9,"title":"chore: bump foo from 1.0.0 to 2.0.0","createdAt":"2026-09-01T00:00:00Z","updatedAt":"2026-09-01T00:00:00Z","isDraft":false,"mergeable":"MERGEABLE","mergeStateStatus":"UNSTABLE","reviewDecision":null,"autoMerge":false,"labels":[],"checks":[{"name":null,"conclusion":null,"status":null,"required":null}],"baseRed":[]}' +nulled_bucket="$(bash "${CLASSIFY}" <<<"${nulled}" | jq -r '.bucket')" +if [[ -z "${nulled_bucket}" ]]; then + _fail "the nulled-checks assertion produced no bucket — it would pass vacuously" +fi +if [[ "${nulled_bucket}" == "ready-to-merge" ]]; then + _fail "a PR whose checks are all null classified as ready-to-merge — a token that cannot read checks would recommend merging failing PRs" +else + _pass "nulled checks do not classify as ready-to-merge (got ${nulled_bucket})" +fi + + if [[ "${fail}" -eq 0 ]]; then echo "test-classify: all assertions passed" else echo "test-classify: FAILURES" fi -exit "${fail}" +exit "${fail}" \ No newline at end of file diff --git a/scripts/dependabot-digest/tests/test-render.sh b/scripts/dependabot-digest/tests/test-render.sh index 256d5ed..c8c7c82 100755 --- a/scripts/dependabot-digest/tests/test-render.sh +++ b/scripts/dependabot-digest/tests/test-render.sh @@ -38,7 +38,12 @@ done # 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/'$//")" +# The formatter spans several lines, so extract from `jq -r '.errors` to the +# closing quote rather than matching a single line. A line-based grep silently +# found nothing once the program was reformatted, which failed the assertion +# for the wrong reason. +gql_fmt="$(sed -n "/jq -r '\.errors/,/'[[:space:]]*<< Date: Fri, 11 Sep 2026 19:08:42 -0700 Subject: [PATCH 3/3] style(dependabot): add the missing trailing newline Repo convention; flagged by the commit reviewer. Claude-Session: https://claude.ai/code/session_01ESsw699T54JHARkQXrdL3o --- scripts/dependabot-digest/tests/test-classify.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/dependabot-digest/tests/test-classify.sh b/scripts/dependabot-digest/tests/test-classify.sh index 84cbc2a..bb22c5c 100755 --- a/scripts/dependabot-digest/tests/test-classify.sh +++ b/scripts/dependabot-digest/tests/test-classify.sh @@ -192,4 +192,4 @@ if [[ "${fail}" -eq 0 ]]; then else echo "test-classify: FAILURES" fi -exit "${fail}" \ No newline at end of file +exit "${fail}"