From 505b6f799c73545a1d45bb8ee72f4ac19bd30b38 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Fri, 31 Jul 2026 08:42:48 -0500 Subject: [PATCH] =?UTF-8?q?fix(git):=20ci-queue-wait.sh=20=E2=80=94=20pars?= =?UTF-8?q?e=20the=20payload=20it=20was=20handed,=20not=20stdin=20(#1019)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both python sites piped a status payload into `python3 - <<'PY'`. The heredoc binds stdin to the program text, so `json.load(sys.stdin)` saw EOF, the bare `except` fired, and the parser returned "unknown" for every input — success, pending and failure alike. "unknown" then reaches a silent `exit 0` arm. Consequence: the gate-6 queue guard has never made a determination. It exits 0 on every invocation, on both the gitea and github paths (one shared parser). The pending wait loop, --require-status and the 124 timeout were all unreachable code. What it still validated was connectivity — an unresolved token, head sha, or platform could exit 1. Fix is the one already shipped in the sibling: capture the payload with `payload=$(cat)` before invoking python, pass it by environment. pr-ci-wait.sh:38 has carried a comment describing this exact bug — "yielding EOF and returning unknown every time" — along with the remedy. It was never backported to the sibling the constitution makes mandatory before every push and merge. Adds test-ci-queue-wait-parse.sh, enumerated in test:framework-shell. Every assertion is on the RETURNED STATE STRING; a suite asserting only rc=0 passes against the broken build, which is how this survived. Verified by mutation: against the pre-fix wrapper the suite fails 10 of 14, and the four that pass are the four for which passing is correct (the undecodable-input control, the unknown-vocabulary case, and both needle halves). The needle scans with heredoc semantics rather than matching text. `json.load(sys.stdin)` is correct under `python3 -c`, where the program comes from argv and stdin really is the payload — ci-queue-wait.sh uses that form legitimately in gitea_get_branch_head_sha. A flat grep flags that innocent site; the scanner names only the two defective ones. Out of scope, deliberately, and recorded on #1019: token-in-argv (ps-visible) and the hardcoded BRANCH="main". Bundling them would make a safety-critical parse fix harder to review. Refs #1019 --- .../framework/tools/git/ci-queue-wait.sh | 21 +- .../tools/git/test-ci-queue-wait-parse.sh | 205 ++++++++++++++++++ packages/mosaic/package.json | 2 +- 3 files changed, 221 insertions(+), 7 deletions(-) create mode 100644 packages/mosaic/framework/tools/git/test-ci-queue-wait-parse.sh diff --git a/packages/mosaic/framework/tools/git/ci-queue-wait.sh b/packages/mosaic/framework/tools/git/ci-queue-wait.sh index f1cd6825..48a594e6 100755 --- a/packages/mosaic/framework/tools/git/ci-queue-wait.sh +++ b/packages/mosaic/framework/tools/git/ci-queue-wait.sh @@ -34,12 +34,18 @@ EOF # get_remote_host and get_gitea_token are provided by detect-platform.sh get_state_from_status_json() { - python3 - <<'PY' + # Capture piped JSON BEFORE invoking `python3 - <&2 + exit 1 +fi + +TMPDIR_T="$(mktemp -d)" +trap 'rm -rf "$TMPDIR_T"' EXIT + +extract_fn() { + # $1 = function name, $2 = source file, $3 = destination + sed -n "/^$1()/,/^}/p" "$2" > "$3" + [[ -s "$3" ]] || { printf 'FATAL: could not extract %s from %s\n' "$1" "$2" >&2; exit 1; } +} + +extract_fn get_state_from_status_json "$WRAPPER" "$TMPDIR_T/state.sh" +extract_fn print_pending_contexts "$WRAPPER" "$TMPDIR_T/contexts.sh" + +state_of() { + # shellcheck disable=SC1091 + ( source "$TMPDIR_T/state.sh"; printf '%s' "$1" | get_state_from_status_json ) +} + +expect_state() { + local label="$1" payload="$2" want="$3" got + got="$(state_of "$payload")" + if [[ "$got" == "$want" ]]; then + pass "$label -> $want" + else + fail "$label -> got '$got', want '$want'" + fi +} + +echo "=== parser returns a state derived from its input (#1019) ===" + +expect_state "success payload" \ + '{"state":"success","statuses":[{"status":"success","context":"ci/build"}]}' \ + 'terminal-success' + +expect_state "pending payload" \ + '{"state":"pending","statuses":[{"status":"pending","context":"ci/build"}]}' \ + 'pending' + +expect_state "failure payload" \ + '{"state":"failure","statuses":[{"status":"failure","context":"ci/build"}]}' \ + 'terminal-failure' + +expect_state "mixed success+pending is pending" \ + '{"state":"pending","statuses":[{"status":"success"},{"status":"pending"}]}' \ + 'pending' + +expect_state "running counts as pending" \ + '{"state":"pending","statuses":[{"status":"running"}]}' \ + 'pending' + +expect_state "error counts as failure" \ + '{"state":"failure","statuses":[{"status":"error"}]}' \ + 'terminal-failure' + +expect_state "empty status set is no-status" \ + '{"state":"","statuses":[]}' \ + 'no-status' + +# Control. This is the ONE input for which "unknown" is correct. Without it, a +# regression that hardcoded "unknown" again would still fail the cases above but +# the suite would give no signal that "unknown" remains reachable when it should be. +expect_state "undecodable payload stays unknown" \ + 'not json at all' \ + 'unknown' + +expect_state "unrecognised status vocabulary is unknown" \ + '{"state":"weird","statuses":[{"status":"weird"}]}' \ + 'unknown' + +echo "=== pending contexts are reported to the operator ===" + +contexts_of() { + # shellcheck disable=SC1091 + ( source "$TMPDIR_T/contexts.sh"; printf '%s' "$1" | print_pending_contexts ) +} + +out="$(contexts_of '{"statuses":[{"status":"pending","context":"ci/alpha"},{"status":"pending","context":"ci/beta"}]}')" +if grep -q 'ci/alpha' <<<"$out" && grep -q 'ci/beta' <<<"$out"; then + pass "both pending contexts emitted" +else + fail "pending contexts not emitted; got: $out" +fi + +out="$(contexts_of '{"statuses":[{"status":"success","context":"ci/alpha"}]}')" +# Assert the POSITIVE message, not merely the absence of the context name. Absence +# alone is satisfied by total silence — and the pre-fix build was silent, so an +# absence-only assertion passed against the very defect this suite exists to catch. +if grep -q 'ci/alpha' <<<"$out"; then + fail "a non-pending context was emitted; got: $out" +elif grep -q 'no pending contexts' <<<"$out"; then + pass "non-pending context suppressed, and reported as 'no pending contexts'" +else + fail "expected an explicit 'no pending contexts' report; got: $out" +fi + +echo "=== needle: the broken construct is caught, not merely absent today ===" + +# Rebuild the pre-fix form and assert this suite would have failed against it. +# Without this, the suite proves the current file is correct but not that it can +# detect the defect returning. +BROKEN="$TMPDIR_T/broken.sh" +cat > "$BROKEN" <<'BROKEN_EOF' +get_state_from_status_json() { + python3 - <<'PY' +import json +import sys + +try: + payload = json.load(sys.stdin) +except Exception: + print("unknown") + raise SystemExit(0) +print("terminal-success" if (payload.get("state") or "") == "success" else "pending") +PY +} +BROKEN_EOF + +broken_got="$( ( source "$BROKEN"; printf '%s' '{"state":"success","statuses":[]}' | get_state_from_status_json ) )" +if [[ "$broken_got" == "unknown" ]]; then + pass "[NEEDLE ] pre-fix construct reproduces the defect (returns 'unknown' for a success payload)" +else + fail "[NEEDLE ] pre-fix construct did NOT reproduce the defect; got '$broken_got' — the needle no longer pins anything" +fi + +# And assert the shipped wrapper does not contain that construct. +# +# The check must have HEREDOC SEMANTICS, not merely match the text. `json.load(sys.stdin)` +# is perfectly correct under `python3 -c '...'` — there the program comes from argv, so +# stdin really is the payload, and ci-queue-wait.sh uses that form legitimately in +# gitea_get_branch_head_sha. Only `python3 - <