diff --git a/CHANGELOG.md b/CHANGELOG.md index d8760aa..7932638 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ so entries say what changed, cite the issue, and stop. ## Unreleased - BUILDER.md — the handed-off PR is the parked claim's fourth shape, its handoff is its declaration, and shape 2 covers the round awaiting its first verdicts (#109). +- `labels-reconcile` — a degraded mergeability/checks read now logs gh's actual stderr (collapsed, bounded) beside the byte-identical counted line, and the blind-sweep warning leads with the observed reason instead of asserting the permissions cause (#101). - Changelog publication — count entries instead of bytes, refuse dangling grouped headings, and seed grouped re-arms with Added/Changed/Fixed (#98). - `labels-reconcile` — grant callers private-repo check reads and warn when an entire PR sweep is blind (#95). - `labels-reconcile` — the bootstrap now retires the six GitHub defaults `LABELS.md` publishes as deleted, tolerating both an already-absent label and a refused delete (#93). diff --git a/actions/labels-reconcile/labels-reconcile.sh b/actions/labels-reconcile/labels-reconcile.sh index f84ed2d..87c301e 100755 --- a/actions/labels-reconcile/labels-reconcile.sh +++ b/actions/labels-reconcile/labels-reconcile.sh @@ -60,9 +60,39 @@ run() { # every mutation goes through here — DRY_RUN=1 logs instead of doing if [ -n "${DRY_RUN:-}" ]; then log "DRY_RUN: $*"; else "$@"; fi } -blind_sweep_warning() { # $1 = unreadable PRs, $2 = all open PRs +blind_sweep_warning() { # $1 = unreadable PRs, $2 = all open PRs, $3 = sampled read-failure reason + # Report, do not diagnose (#101 D5). The old text asserted the caller's + # checks:/statuses: grants as THE cause — an inference #95 made from a + # control case, and the merged consumer-side fix (incubator#48/PR #49) + # left the symptom standing while the run emitting this warning held the + # evidence that would have said so. Lead with what gh actually said this + # sweep; the permissions hint stays, demoted to one named candidate. if [ "$2" -gt 0 ] && [ "$1" -eq "$2" ]; then - echo "::warning::labels: every open PR was unreadable; grant checks: read and statuses: read in the caller (private repos do not imply them)" + local reason="${3:-}" + if [ -n "$reason" ]; then + echo "::warning::labels: every open PR was unreadable; sampled reason: $reason — one candidate is missing checks: read and statuses: read in the caller (private repos do not imply them)" + else + echo "::warning::labels: every open PR was unreadable; no reason was captured — one candidate is missing checks: read and statuses: read in the caller (private repos do not imply them)" + fi + fi +} + +read_failure_reason() { # $1 = captured stderr → one bounded line; pure (#101) + # Verbatim, collapsed, bounded (D3): gh emits multi-line errors and GraphQL + # blobs. Collapsed so the reason is exactly one log line — a raw newline + # inside the captured per-PR output block could collide with a matched + # string — and truncated because an unbounded paste per PR per sweep is + # noise, and annotations are capped anyway. + local reason + reason="$(printf '%s' "${1-}" | tr '\n' ' ')" + if [ -z "$reason" ]; then + # Empty stderr is itself a fact (D4): a read that failed silently is a + # different observation from a denial, and must not read as one. + echo "no error output" + elif [ "${#reason}" -gt 300 ]; then + printf '%s…\n' "${reason:0:300}" + else + printf '%s\n' "$reason" fi } @@ -598,7 +628,7 @@ main() { REPO_LABELS="$(gh label list -R "$REPO" --limit 200 --json name --jq '.[].name' 2>/dev/null || echo "")" [ -z "$REPO_LABELS" ] && log "WARNING: could not read the label set — applying labels unfiltered" - local n output status total=0 unreadable=0 + local n output status total=0 unreadable=0 sampled_reason="" while IFS= read -r n; do [ -n "$n" ] || continue total=$((total + 1)) @@ -622,14 +652,28 @@ main() { # Failure to read them is NOT fatal and NOT treated as broken — an API # hiccup must never flap every PR into needs-rebase, so both degrade to # the "do not know" value that triggers nothing. - GH_VIEW="$(gh pr view "$n" -R "$REPO" --json mergeable,statusCheckRollup 2>/dev/null || echo '{}')" + # The WHY goes to gh's stderr, and 2>/dev/null threw it away — a + # permanent denial and a network hiccup left byte-identical evidence, + # and #95 had to infer a cause from a control case instead of reading + # it off a run (wrongly, it turned out). Captured into a file (#101 + # D2), never left to interleave raw into the per-PR output block, + # where an unlucky line could collide with a matched string. + GH_VIEW_ERR_FILE="$(mktemp)" + GH_VIEW="$(gh pr view "$n" -R "$REPO" --json mergeable,statusCheckRollup 2>"$GH_VIEW_ERR_FILE" || echo '{}')" + GH_VIEW_ERR="$(cat "$GH_VIEW_ERR_FILE")" + rm -f "$GH_VIEW_ERR_FILE" MERGEABLE="$(jq -r '.mergeable // "UNKNOWN"' <<<"$GH_VIEW")" CHECKS="$(checks_state <<<"$GH_VIEW")" # Read failed: leave this PR exactly as it is. Recomputing on facts we # did not read is how an API hiccup turns into a false "merge me" — # and the next tick is 15 minutes away, not 15 hours. if [ "$CHECKS" = UNREADABLE ]; then + # Two lines on purpose (#101 D1): the sweep detects a wholly blind + # pass by whole-line-matching the counted line below, so the reason + # rides its OWN line — folding it in would silently break the + # `unreadable` counter and the wholly-blind warning #96 landed. log "#$n: could not read mergeability/checks — left alone this pass" + log "#$n: read failed: $(read_failure_reason "$GH_VIEW_ERR")" exit 0 fi reconcile_pr "$n" @@ -638,11 +682,15 @@ main() { [ -n "$output" ] && printf '%s\n' "$output" if grep -qxF "labels: #$n: could not read mergeability/checks — left alone this pass" <<<"$output"; then unreadable=$((unreadable + 1)) + # the first observed reason stands in for the sweep in the blind warning + if [ -z "$sampled_reason" ]; then + sampled_reason="$(sed -n "s/^labels: #$n: read failed: //p" <<<"$output" | head -n1)" + fi elif [ "$status" -ne 0 ]; then log "#$n: reconcile failed — continuing with the remaining PRs" fi done < <(gh pr list -R "$REPO" --state open --limit 100 --json number --jq '.[].number') - blind_sweep_warning "$unreadable" "$total" + blind_sweep_warning "$unreadable" "$total" "$sampled_reason" log "reconciled." } diff --git a/test/labels-reconcile.test.sh b/test/labels-reconcile.test.sh index 724c800..8b1849f 100755 --- a/test/labels-reconcile.test.sh +++ b/test/labels-reconcile.test.sh @@ -39,16 +39,40 @@ rev() { # $1=login $2=state $3=commit $4=body $5=submitted_at → one review obj reviews() { jq -s '.' <<<"$*"; } # collect review objects into an array # -- a sweep-wide read failure is visible without changing any PR ------------ -warning="$(blind_sweep_warning 3 3)" -expect "a wholly blind sweep warns" \ - "::warning::labels: every open PR was unreadable; grant checks: read and statuses: read in the caller (private repos do not imply them)" \ +warning="$(blind_sweep_warning 3 3 "HTTP 403: Resource not accessible by integration")" +expect "a wholly blind sweep warns, leading with the observed reason" \ + "::warning::labels: every open PR was unreadable; sampled reason: HTTP 403: Resource not accessible by integration — one candidate is missing checks: read and statuses: read in the caller (private repos do not imply them)" \ "$warning" expect "the blind warning names checks: read" named \ "$(grep -qF "checks: read" <<<"$warning" && echo named || echo missing)" expect "the blind warning names statuses: read" named \ "$(grep -qF "statuses: read" <<<"$warning" && echo named || echo missing)" -expect "a partially blind sweep does not warn" "" "$(blind_sweep_warning 1 3)" -expect "a sweep with no open PRs does not warn" "" "$(blind_sweep_warning 0 0)" +# must-fail (#101 D5): the #95 inference — disproven on incubator while the +# run held the evidence — must never again be stated as the cause +expect "the warning no longer asserts the permissions diagnosis as fact" no \ + "$(grep -qF "grant checks: read and statuses: read" <<<"$warning" && echo yes || echo no)" +warning="$(blind_sweep_warning 3 3 "")" +expect "with no reason captured the warning says exactly that" yes \ + "$(grep -qF "no reason was captured" <<<"$warning" && echo yes || echo no)" +expect "...and keeps the permissions candidate" named \ + "$(grep -qF "checks: read" <<<"$warning" && echo named || echo missing)" +expect "a partially blind sweep does not warn" "" "$(blind_sweep_warning 1 3 "x")" +expect "a sweep with no open PRs does not warn" "" "$(blind_sweep_warning 0 0 "")" + +# -- the reason helper: facts in, one bounded line out (#101 D3/D4) ---------- +expect "empty stderr is reported as its own fact" "no error output" \ + "$(read_failure_reason "")" +expect "multi-line stderr collapses to one line" \ + "GraphQL: Resource not accessible by integration (repository.pullRequest.mergeable) Resource not accessible by integration (repository.pullRequest.statusCheckRollup)" \ + "$(read_failure_reason $'GraphQL: Resource not accessible by integration (repository.pullRequest.mergeable)\nResource not accessible by integration (repository.pullRequest.statusCheckRollup)')" +long_reason="$(printf 'e%.0s' {1..400})" +short_reason="$(read_failure_reason "$long_reason")" +expect "400 chars of stderr truncate to 300 plus an ellipsis, one line" \ + "$(printf 'e%.0s' {1..300})…" "$short_reason" +expect "...within the 304-byte bound" yes \ + "$([ "${#short_reason}" -le 304 ] && echo yes || echo no)" +exact_reason="$(read_failure_reason "$(printf 'e%.0s' {1..300})")" +expect "a 300-char reason passes through whole" 300 "${#exact_reason}" # -- drafts are building, whoever is requested -------------------------------- DRAFT=true HEAD_SHA=head1 REQUESTED="" REVIEWS_JSON='[]' @@ -616,7 +640,9 @@ blind_main_probe() { elif [ "$1" = pr ] && [ "$2" = list ]; then printf '101\n102\n' elif [ "$1" = pr ] && [ "$2" = view ]; then - printf '{}\n' + # a denial with its reason on stderr, the way real gh fails (#101) + printf 'GraphQL: Resource not accessible by integration (repository.pullRequest.statusCheckRollup)\n' >&2 + return 1 elif [ "$1" = api ] && [[ "$*" = *"/reviews"* ]]; then return 0 elif [ "$1" = api ]; then @@ -631,12 +657,28 @@ blind_main_probe() { } blind_main="$(blind_main_probe)" -expect "a wholly blind main sweep emits one actionable annotation" 1 \ +expect "a wholly blind main sweep emits exactly one annotation" 1 \ + "$(grep -c '^::warning::' <<<"$blind_main")" +expect "...leading with the reason the sweep actually observed" 1 \ + "$(grep -c '^::warning::.*Resource not accessible by integration' <<<"$blind_main")" +expect "...still naming the permissions candidate" 1 \ "$(grep -c '^::warning::.*checks: read.*statuses: read' <<<"$blind_main")" +# must-fail (#101 D5): red if the disproven diagnosis is re-asserted as fact +expect "...never as a stated cause" 0 \ + "$(grep -c 'grant checks: read and statuses: read' <<<"$blind_main" || true)" expect "a wholly blind main sweep leaves every PR untouched" no \ "$(grep -q '^MUTATION:' <<<"$blind_main" && echo yes || echo no)" -expect "the existing per-PR skip still runs for every blind PR" 2 \ - "$(grep -c 'could not read mergeability/checks — left alone this pass' <<<"$blind_main")" +expect "each blind PR keeps its counted line, matched by the sweep's own grep -qxF" yes \ + "$(grep -qxF 'labels: #101: could not read mergeability/checks — left alone this pass' <<<"$blind_main" \ + && grep -qxF 'labels: #102: could not read mergeability/checks — left alone this pass' <<<"$blind_main" \ + && echo yes || echo no)" +expect "each blind PR logs its reason as its own line beside the counted one" 2 \ + "$(grep -c '^labels: #10[12]: read failed: GraphQL: Resource not accessible by integration' <<<"$blind_main")" +# must-fail (#101 D1): red if a reason line whole-line-matches the counted +# string (the counter would double-count) or the counted line changed (the +# counter would miss it and the warning never fire) +expect "exactly the blind PRs match the counted shape whole-line — no more, no less" 2 \ + "$(grep -c '^labels: #[0-9]*: could not read mergeability/checks — left alone this pass$' <<<"$blind_main")" # --------------------------------------------------------------------------- # bootstrap_labels retires the GitHub defaults (#93). LABELS.md published