diff --git a/lib/read.sh b/lib/read.sh index c391cb2..1aa26be 100644 --- a/lib/read.sh +++ b/lib/read.sh @@ -29,6 +29,7 @@ guarded_read() { # $1 = variable to fill, rest = the read; sets READ_FAILURE_STD shift __err="$(mktemp)" || return 1 __out="$("$@" 2>"$__err")" || __rc=$? + # shellcheck disable=SC2034 # the out-parameter: every caller reads it beside the status READ_FAILURE_STDERR="$(cat "$__err")" rm -f "$__err" printf -v "$__var" '%s' "$__out" diff --git a/test/issueflow-reconcile.test.sh b/test/issueflow-reconcile.test.sh index 678999f..3ba46c4 100644 --- a/test/issueflow-reconcile.test.sh +++ b/test/issueflow-reconcile.test.sh @@ -293,6 +293,7 @@ iso_at() { date -u -d "@$1" +%Y-%m-%dT%H:%M:%SZ; } # crew#329's job log carried, verbatim (#247), and the payload beside it. GH_STUB_STDERR="gh: We couldn't respond to your request in time. (HTTP 504)" GH_STUB_ERROR_BODY='{"message":"We could not respond to your request in time.","documentation_url":"https://docs.github.com/rest"}' +export GH_STUB_STDERR # the PATH-stubbed gh of the executable runs reads it too issue_stub_gh() { if [ "$1" = api ]; then @@ -645,6 +646,119 @@ churned="$(issue_probe 24 $'claimed\nneeds-ruling')" check "8 real-quiet days nudge through a 2-day-old label churn" 0 "" \ grep -q 'ruling nudge' <<<"$churned" +# --------------------------------------------------------------------------- +# An unreadable fact invents no verdict on the issue surface either (#247). +# `gh api` prints a 5xx body to stdout AND exits non-zero, and GitHub's 5xx +# body is a JSON object — so the payload that reached the guards was valid +# JSON, `.labels[]` came back empty, and queue_decision was handed the wrong +# input. The pure guards first, then the two decisions the fall-through +# reached. +# --------------------------------------------------------------------------- +payload_refused() { ! issue_payload_valid "$@"; } # 0 when the payload is refused + +check "a healthy issue payload is accepted" 0 "" \ + issue_payload_valid 40 <<<'{"number":40,"labels":[{"name":"ready"}]}' +check "an issue carrying no labels at all is still a valid payload" 0 "" \ + issue_payload_valid 40 <<<'{"number":40,"labels":[]}' +# The reported shape: gh renders `gh: (HTTP 504)` from a body with a +# `message` key, which proves the body was valid JSON. The status check is what +# catches this one; the shape check refuses it independently. +check "a JSON error object is not an issue payload" 0 "" \ + payload_refused 40 <<<"$GH_STUB_ERROR_BODY" +# The live path a status check alone would leave open (D3): 200, exit 0, and +# `.labels[]` empties exactly as it does on the 504. +check "an HTTP 200 whose body is null is refused" 0 "" payload_refused 40 <<<'null' +check "a payload missing .labels is refused" 0 "" \ + payload_refused 40 <<<'{"number":40}' +check "a payload whose .labels is not an array is refused" 0 "" \ + payload_refused 40 <<<'{"number":40,"labels":"ready"}' +check "a payload about a different issue is refused" 0 "" \ + payload_refused 40 <<<'{"number":41,"labels":[]}' +check "a payload that is not JSON at all is refused" 0 "" \ + payload_refused 40 <<<'not json' +check "an empty payload is refused" 0 "" payload_refused 40 "$(cfix 50)" +printf '%s\n' "$GH_STUB_ERROR_BODY" >"$(cfix 50).http-error" +jq -n --arg at "$(iso_at $((INOW - 10 * 86400)))" \ + '[{"event":"assigned","created_at":$at}]' >"$(tfix 50)" +claim_edits_before="$(wc -l <"$TMP/issue-edits")" +check "a 504 on the comments read skips the issue instead of grading its age" \ + 3 "#50: skipped this pass — could not read its activity history: $GH_STUB_STDERR" \ + issue_probe 50 claimed 1 +check "...so the live claim is not reclaimed" 1 "" \ + grep -q 'stale claim reclaimed -> ready' <<<"$(issue_probe 50 claimed 1)" +# shellcheck disable=SC2016 # positional parameters belong to bash -c +check "...no unassign, no label swap, and no reclaim comment" 0 "" \ + bash -c 'test "$1" -eq "$(wc -l <"$2")" && test ! -f "$3"' _ \ + "$claim_edits_before" "$TMP/issue-edits" "$TMP/posted-50" + +# -- the suppressed comment: a 504 on the marker read ----------------------- +# The marker is on the issue. Read as "no marker", a failed read re-posts the +# comment the marker exists to suppress — every sweep, forever. +jq -n --arg b '' \ + --arg at "$(iso_at $((INOW - 3600)))" \ + '[{"user":{"login":"sweep-bot"},"created_at":$at,"html_url":"https://x/m","body":$b}]' \ + >"$(cfix 51)" +printf '%s\n' "$GH_STUB_ERROR_BODY" >"$(cfix 51).http-error" +check "a 504 on the marker read skips rather than reading it as no marker" \ + 3 "#51: skipped this pass — could not read its comments: $GH_STUB_STDERR" \ + issue_probe 51 blocked 1 false "" "no parseable declaration here" +check "...so no duplicate comment is posted" 1 "" test -f "$TMP/posted-51" + +# -- a deliberate skip is counted; a genuine crash is still named (D4) ------- +printf '%s\n' '{"number":60,"labels":[{"name":"ready"}],"assignees":[]}' \ + >"$TMP/repos_owner_repo_issues_60.json" +printf '%s\n' '{"number":61,"labels":[{"name":"ready"}],"assignees":[]}' \ + >"$TMP/repos_owner_repo_issues_61.json" +printf '%s\n' "$GH_STUB_ERROR_BODY" >"$TMP/repos_owner_repo_issues_61.json.http-error" +pass_probe() { # $1 issue; $2 non-empty makes reconcile_issue crash + ( + REPO=owner/repo + gh() { issue_stub_gh "$@"; } + [ -z "${2:-}" ] || reconcile_issue() { return 9; } + SKIPPED_COUNT=0 + SKIPPED_ISSUES="" + reconcile_issue_pass "$1" + printf 'rc=%s count=%s issues=%s\n' "$?" "$SKIPPED_COUNT" "$SKIPPED_ISSUES" + ) +} +check "a genuine non-read crash still names the failure byte-identically" 0 \ + "issueflow: #60: reconcile failed — continuing with the remaining issues" \ + pass_probe 60 crash +check "...and the pass still returns 0, so the loop reaches the next issue" 0 \ + "rc=0" pass_probe 60 crash +check "...and a crash is not counted as a skip" 0 "count=0" pass_probe 60 crash +check "a skipped issue is counted and named" 0 "count=1 issues=#61" pass_probe 61 +check "...and is not also reported as a crash" 1 "" \ + grep -q 'reconcile failed' <<<"$(pass_probe 61)" +check "...leaving the loop free to continue" 0 "rc=0" pass_probe 61 + # --------------------------------------------------------------------------- # The arrival path, executed the way the action executes it (#91): four # triage-authored mints died silently because the stand-down `return`s in @@ -681,7 +795,15 @@ if [ "$1" = api ]; then *'states: MERGED'*) file="$GH_FIXTURES/graphql-merged.json" ;; esac fi - [ ! -f "$file.error" ] || exit 1 + # `.http-error` is the real 5xx (#247): the response body — GitHub's JSON + # error object — goes to STDOUT, the reason to stderr, and the status is + # non-zero. `.error` is the payload-free failure, which is the safe path. + if [ -f "$file.http-error" ]; then + cat "$file.http-error" + printf '%s\n' "${GH_STUB_STDERR:-}" >&2 + exit 1 + fi + [ ! -f "$file.error" ] || { printf '%s\n' "${GH_STUB_STDERR:-}" >&2; exit 1; } if [ -f "$file" ]; then payload="$(cat "$file")"; else payload='[]'; fi if [ -n "$jqexpr" ]; then jq -r "$jqexpr" <<<"$payload"; else printf '%s\n' "$payload"; fi exit 0 @@ -849,4 +971,85 @@ check "a dead API on the arrival path still fails the run (D2)" 0 "" \ check "...and the sweep does not run over a lying arrival" 1 "" \ grep -qF 'issueflow: reconciled.' <<<"$err_out" +# --------------------------------------------------------------------------- +# The whole sweep over an unreadable board (#247), executed. The sourced +# probes above drive one issue's pass; only this path exercises the loop, the +# counting and the tail — and only this path reproduces crew#329's log, which +# ended `issueflow: reconciled.` with rc=0 over a label it should never have +# written. Its own fixture directory: the arrival fixtures above are stateful +# across their cases. +# --------------------------------------------------------------------------- +SWEEP="$TMP/sweep" +mkdir -p "$SWEEP" +printf '%s\n' \ + '{"data":{"repository":{"pullRequests":{"nodes":[],"pageInfo":{"hasNextPage":false,"endCursor":null}}}}}' \ + >"$SWEEP/graphql-open.json" +cp "$SWEEP/graphql-open.json" "$SWEEP/graphql-merged.json" +# 70: the 504 with a JSON error body on the per-issue read. +printf '%s\n' "$GH_STUB_ERROR_BODY" >"$SWEEP/repos_owner_repo_issues_70.json.http-error" +# 71: healthy, and carrying no queue label — so if the sweep reaches it, it +# writes needs-triage. That write is the evidence the loop continued. +printf '%s\n' \ + '{"number":71,"user":{"login":"triage-one"},"labels":[{"name":"enhancement"}],"assignees":[]}' \ + >"$SWEEP/repos_owner_repo_issues_71.json" +# 72: HTTP 200 whose body is `null` — exit 0, and the label set empties just +# as it does on the 504. The shape check is the only thing that catches it. +printf 'null\n' >"$SWEEP/repos_owner_repo_issues_72.json" + +sweep_board() { printf '%s\n' "$1" >"$SWEEP/repos_owner_repo_issues_state_open_per_page_100.json"; } +sweep_run() { + : >"$SWEEP/edits" + env PATH="$ARRIVAL/stub:$PATH" GH_FIXTURES="$SWEEP" ISSUEFLOW_NOW="$INOW" \ + REPO=owner/repo LABELS_CONF="$ARRIVAL/labels.conf" \ + bash "$ROOT/actions/issueflow-reconcile/issueflow-reconcile.sh" 2>&1 +} + +sweep_board '[{"number":70},{"number":71}]' +sweep_out="$(sweep_run)" +sweep_rc=$? +check "an unreadable issue does not red the sweep (D7)" 0 "" test "$sweep_rc" -eq 0 +check "the 504's JSON error body is skipped, with the reason named" 0 \ + "issueflow: #70: skipped this pass — could not read the issue: $GH_STUB_STDERR" \ + printf '%s\n' "$sweep_out" +check "...and crew#329's label is never written" 1 "" \ + grep -qF '#70: needs-triage (no queue state)' <<<"$sweep_out" +check "...nor any edit at all on the unreadable issue" 1 "" \ + grep -qF 'issue edit 70' "$SWEEP/edits" +check "...while the readable issue beside it is reconciled as before" 0 "" \ + grep -qxF 'issue edit 71 -R owner/repo --add-label needs-triage' "$SWEEP/edits" +check "...and the partial pass names its count and its issue" 0 \ + 'issueflow: 1 issue skipped this pass on an unreadable fact: #70' \ + printf '%s\n' "$sweep_out" +check "...after a byte-identical reconciled. line" 0 "" \ + grep -qxF 'issueflow: reconciled.' <<<"$sweep_out" + +sweep_board '[{"number":72}]' +null_out="$(sweep_run)" +null_rc=$? +check "an HTTP 200 whose body is null exits 0 and writes nothing" 0 "" \ + test "$null_rc" -eq 0 +check "...because the shape check refuses it, on its own line" 0 \ + 'issueflow: #72: skipped this pass — the issue read answered a payload that is not issue #72 carrying a label array' \ + printf '%s\n' "$null_out" +check "...so no label is derived from an empty label set" 1 "" \ + grep -qF 'issue edit 72' "$SWEEP/edits" +check "...and the tail names it too" 0 \ + 'issueflow: 1 issue skipped this pass on an unreadable fact: #72' \ + printf '%s\n' "$null_out" + +sweep_board '[{"number":70},{"number":72}]' +both_out="$(sweep_run)" +check "two skipped issues are both named, in the plural" 0 \ + 'issueflow: 2 issues skipped this pass on unreadable facts: #70 #72' \ + printf '%s\n' "$both_out" + +sweep_board '[{"number":71}]' +whole_out="$(sweep_run)" +whole_rc=$? +check "a whole pass still exits 0" 0 "" test "$whole_rc" -eq 0 +check "...ends on the byte-identical reconciled. line, with no tail after it" 0 \ + "issueflow: reconciled." printf '%s\n' "$(tail -n1 <<<"$whole_out")" +check "...and says nothing about skipping" 1 "" \ + grep -q 'skipped this pass' <<<"$whole_out" + summary