Merge pull request #106 from claude-bot-andresmgsl/build/101-degraded-read-reason
labels-reconcile: report why the degraded read degraded; blind-sweep warning stops asserting an unobserved cause
This commit is contained in:
Daniel Marin
GitHub
commit
d4e133bc1f
3 changed files with 105 additions and 14 deletions
|
|
@ -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).
|
||||
|
|
|
|||
|
|
@ -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."
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Reference in a new issue