From 824dfbe7548d16b52710cdaa0814144e3044203c Mon Sep 17 00:00:00 2001 From: Martin Simon Date: Wed, 4 Nov 2026 08:47:10 +0100 Subject: [PATCH] Take a commit GitHub made despite reporting an error createCommitOnBranch can answer "Something went wrong while executing your query" for a commit it has already made. pkghaus/packages run 41738316016 hit exactly that: act 0.2.96-1 landed as 2a8060a at 23:23:21Z, the error came back at 23:23:25Z, the step failed, and the land job never dispatched the release, so master carried a changelog the archive does not serve. The error arrives in a 2xx body, so curl's --retry never saw it. On any error the script now reads the branch tip over REST and takes it as its own commit only if its parent is HEAD_OID, its tree is the staged tree (git write-tree) and GitHub reports it verified. Otherwise it retries, three attempts in all, pausing 5, 15 and 40 s so a branch that still resolves to its old tip can catch up. expectedHeadOid refuses every attempt after one has landed, so a retry cannot commit twice. The tree comparison was checked against the incident: an index built from 5553cf6 plus the two files of 2a8060a writes 7e24c8a, the tree GitHub built. Any difference GitHub introduces, file modes included, fails closed, which is the old behaviour. Tests cover the landed commit, a retry after an error that did not land, and an unsigned, wrong-tree or wrong-parent tip. Each of the three tip conditions, the lookup and the retry was removed in turn and the suite caught it. --- README.md | 8 ++++ commit-branch.sh | 108 +++++++++++++++++++++++++++++++---------------- tests/run.sh | 95 ++++++++++++++++++++++++++++++++++++++++- 3 files changed, 174 insertions(+), 37 deletions(-) diff --git a/README.md b/README.md index 1d421df..d0589a7 100644 --- a/README.md +++ b/README.md @@ -107,6 +107,14 @@ output and reading raw never unquotes them. having moved since checkout, which is the behaviour you want: a mismatch means something genuinely unexpected happened and failing is correct. +**An error does not mean nothing was committed.** GitHub can answer the +mutation with `Something went wrong while executing your query` after it has +made the commit. On any error the action reads the branch tip and takes it as +its own commit only if the tip's parent is the checked-out `HEAD`, its tree is +the staged tree and GitHub reports it verified; otherwise it tries again, three +attempts in all. `expectedHeadOid` refuses every attempt after one has landed, +so a retry cannot commit twice. + **File contents go base64 in the request body**, so payload size is the practical ceiling on what one commit can carry. diff --git a/commit-branch.sh b/commit-branch.sh index 0ba6ffd..07e1f74 100755 --- a/commit-branch.sh +++ b/commit-branch.sh @@ -83,11 +83,14 @@ fi export BRANCH MESSAGE GITHUB_REPOSITORY HEAD_OID="$(git rev-parse HEAD)" -export HEAD_OID +# The tree the mutation should build: HEAD plus what was staged above. +EXPECTED_TREE="$(git write-tree)" +export HEAD_OID EXPECTED_TREE payload="$(mktemp)" response="$(mktemp)" -trap 'rm -f "$payload" "$response"' EXIT +tip="$(mktemp)" +trap 'rm -f "$payload" "$response" "$tip"' EXIT # -z output and NUL parsing: a path may contain anything but NUL, and git # quotes unusual ones in its default format. Reading raw never unquotes. @@ -132,42 +135,38 @@ sys.stderr.write(f"{len(additions)} addition(s), {len(deletions)} deletion(s)\n" log "committing to $BRANCH, $(wc -c < "$payload") byte payload" -# --fail-with-body writes the body and exits non-zero, and under set -e that -# exit skipped the reporting below and the trap deleted the file: an auth -# failure or a 502 showed curl's generic "returned error: NNN" and nothing -# GitHub actually said. The body is printed here instead, where it is still on -# disk. -# -# --retry covers the transient half. This runs after the archive has already -# been published to R2, so failing here leaves the bucket ahead of the state -# branches until the next ingest repairs it. A retry is safe rather than -# merely convenient: expectedHeadOid makes the mutation conditional, so if the -# first attempt did land, the retry is refused for the right reason instead of -# committing twice. -if ! curl -sS --fail-with-body -X POST "$API/graphql" \ - --max-time 120 --retry 3 --retry-connrefused --retry-all-errors \ - -H "Authorization: bearer $GITHUB_TOKEN" \ - -H 'Content-Type: application/json' \ - --data @"$payload" > "$response"; then - log "FATAL: the GraphQL request to $API failed. What it returned:" - cat "$response" >&2 - exit 1 -fi - -# stdout carries the oid and nothing else, so the substitution below captures -# it; every diagnostic goes to stderr as before. -# -# What keeps an unsigned commit from reaching the caller is the exit status, -# not the order of the prints: a non-zero exit here makes the assignment fail, -# and set -e stops the script before emit_sha runs. Printing the oid after the -# signature check is belt-and-braces on top of that, which is why moving it -# earlier does not break the tests. -oid="$(python3 - "$response" <<'PY' +# Prints the new oid, or nothing when GitHub answered with errors. What a retry +# cannot fix, a failed request or an unsigned commit, exits instead. +mutate() { + # --fail-with-body writes the body and exits non-zero, and under set -e that + # exit skipped the reporting below and the trap deleted the file: an auth + # failure or a 502 showed curl's generic "returned error: NNN" and nothing + # GitHub actually said. The body is printed here instead, where it is still + # on disk. + # + # --retry covers transport failures. An error GitHub reports inside a 2xx + # body is not one; the loop below handles that. + if ! curl -sS --fail-with-body -X POST "$API/graphql" \ + --max-time 120 --retry 3 --retry-connrefused --retry-all-errors \ + -H "Authorization: bearer $GITHUB_TOKEN" \ + -H 'Content-Type: application/json' \ + --data @"$payload" > "$response"; then + log "FATAL: the GraphQL request to $API failed. What it returned:" + cat "$response" >&2 + exit 1 + fi + + # stdout carries the oid and nothing else; every diagnostic goes to stderr. + # + # What keeps an unsigned commit from reaching the caller is the exit status, + # not the order of the prints: a non-zero exit here makes the assignment + # fail, and set -e stops the script before emit_sha runs. + python3 - "$response" <<'PY' import json, sys d = json.load(open(sys.argv[1])) if d.get("errors"): - print("FATAL: " + "; ".join(e.get("message", "?") for e in d["errors"]), file=sys.stderr) - raise SystemExit(1) + print("error: " + "; ".join(e.get("message", "?") for e in d["errors"]), file=sys.stderr) + raise SystemExit(0) c = d["data"]["createCommitOnBranch"]["commit"] sig = c.get("signature") or {} print(f'committed {c["oid"][:12]} signature={sig.get("state")} valid={sig.get("isValid")}', @@ -177,5 +176,42 @@ if not sig.get("isValid"): raise SystemExit(1) print(c["oid"]) PY -)" +} + +# Prints the branch tip's oid if the tip is this commit: parented on HEAD_OID, +# carrying the staged tree, and signed. Anything else fails closed. +landed() { + curl -sS --fail-with-body --max-time 30 --retry 3 --retry-all-errors \ + -H "Authorization: bearer $GITHUB_TOKEN" \ + "$API/repos/$GITHUB_REPOSITORY/commits/heads/$BRANCH" > "$tip" || return 1 + python3 - "$tip" <<'PY' +import json, os, sys +c = json.load(open(sys.argv[1])) +if ([p["sha"] for p in c["parents"]] != [os.environ["HEAD_OID"]] + or c["commit"]["tree"]["sha"] != os.environ["EXPECTED_TREE"] + or not c["commit"]["verification"]["verified"]): + raise SystemExit(1) +print(c["sha"]) +PY +} + +# GitHub can report "Something went wrong while executing your query" for a +# mutation it carried out, so an error is checked against the branch before +# anything is retried. A retry is safe: once one attempt has landed, +# expectedHeadOid refuses the rest. The pauses give a branch that still +# resolves to its old tip time to catch up. +oid="" +for pause in 5 15 40; do + oid="$(mutate)" + [ -n "$oid" ] && break + sleep "$pause" + if oid="$(landed)"; then + log "GitHub reported an error but committed ${oid:0:12}; using it" + break + fi +done +if [ -z "$oid" ]; then + log "FATAL: no attempt committed to $BRANCH, and its tip is not this commit" + exit 1 +fi emit_sha "$oid" diff --git a/tests/run.sh b/tests/run.sh index 5e16cb6..40a6055 100755 --- a/tests/run.sh +++ b/tests/run.sh @@ -44,7 +44,7 @@ eq() { # label expected actual # assertions vanish rather than fail -- which is exactly what happened when # these groups were first lifted out of pkghaus/apt: eq() was left behind and # the suite still printed "all tests passed". Bump this when adding one. -EXPECTED_ASSERTIONS=13 +EXPECTED_ASSERTIONS=26 echo "the signed-commit payload describes every change, deletions included" ( @@ -262,6 +262,99 @@ FAKE exit $((fail > 0)) ) || fail=$((fail + 1)) +echo "an error from the mutation is checked against the branch before it fails" +( + work="$(mktemp -d)" + export GITHUB_TOKEN=fake GITHUB_REPOSITORY=pkghaus/packages + + repo="$work/repo"; mkdir -p "$repo"; cd "$repo" + git init -q -b master . + printf 'VERSION=v0.2.95\n' > package.conf + git add -A + git -c user.name=t -c user.email=t@example.invalid commit -qm base + head="$(git rev-parse HEAD)" + base_tree="$(git rev-parse 'HEAD^{tree}')" + + # The tree GitHub would build, worked out apart from the script: a real + # commit of the same change in a clone. + git clone -q "$repo" "$work/clone" + printf 'VERSION=v0.2.96\n' | tee package.conf > "$work/clone/package.conf" + git -C "$work/clone" -c user.name=t -c user.email=t@example.invalid commit -qam bump + tree="$(git -C "$work/clone" rev-parse 'HEAD^{tree}')" + + # GitHub's message from the run whose commit landed regardless. The + # backticks are its own. + # shellcheck disable=SC2016 + err='{"errors":[{"message":"Something went wrong while executing your query on 2026-11-03T23:23:25Z. Please include `0402:41EF6:164DE3:17BEFB:6AEA6CE9` when reporting this issue."}]}' + landed=2a8060afd330340e35e76998304c11f927403792 + ok_reply() { printf '{"data":{"createCommitOnBranch":{"commit":{"oid":"%s","signature":{"isValid":true,"state":"VALID"}}}}}' "$1"; } + tip() { # sha parent tree verified + printf '{"sha":"%s","parents":[{"sha":"%s"}],"commit":{"tree":{"sha":"%s"},"verification":{"verified":%s}}}' \ + "$@" > "$work/tip" + } + + # The Nth mutation answers with mutation.N if present, else mutation; any + # other request is the branch lookup. + mkdir -p "$work/bin" + cat > "$work/bin/curl" </dev/null || echo 0) + 1)); echo "\$n" > "$work/calls" + if [ -f "$work/mutation.\$n" ]; then cat "$work/mutation.\$n"; else cat "$work/mutation"; fi ;; + *) cat "$work/tip" ;; +esac +FAKE + printf '#!/bin/sh\n' > "$work/bin/sleep" + chmod +x "$work/bin/curl" "$work/bin/sleep" + + fresh() { rm -f "$work/calls" "$work"/mutation*; : > "$work/out"; } + run() { + GITHUB_OUTPUT="$work/out" PATH="$work/bin:$PATH" \ + "$ROOT/commit-branch.sh" "$repo" master "bump" >/dev/null 2>"$work/log" + } + + # The incident: GitHub answers with an error, and the commit is on master. + fresh; printf '%s' "$err" > "$work/mutation"; tip "$landed" "$head" "$tree" true + run && rc=0 || rc=$? + eq "a commit that landed despite the error is a success" "0" "$rc" + eq "and its oid reaches the caller" "sha=$landed" "$(cat "$work/out")" + eq "and it is not committed a second time" "1" "$(cat "$work/calls")" + + # The error was real: the branch has not moved, so the mutation is retried. + fresh; printf '%s' "$err" > "$work/mutation.1"; ok_reply cafebabecafebabe > "$work/mutation" + tip "$head" 0000000000000000000000000000000000000000 "$base_tree" true + run && rc=0 || rc=$? + eq "a commit that did not land is retried" "0" "$rc" + eq "and the retry's oid reaches the caller" "sha=cafebabecafebabe" "$(cat "$work/out")" + eq "after exactly one retry" "2" "$(cat "$work/calls")" + + # Our parent and tree, but unsigned: never handed to the caller. + fresh; printf '%s' "$err" > "$work/mutation"; tip "$landed" "$head" "$tree" false + run && rc=0 || rc=$? + eq "an unsigned tip is not taken for this commit" "1" "$rc" + eq "and nothing is handed to the caller" "" "$(cat "$work/out")" + case "$(cat "$work/log")" in + *"Something went wrong"*) ok "GitHub's own message reaches the log" ;; + *) no "GitHub's own message reaches the log" "got [$(cat "$work/log")]" ;; + esac + + # Someone else's commit on the same parent: a different tree. + fresh; printf '%s' "$err" > "$work/mutation"; tip "$landed" "$head" "$base_tree" true + run && rc=0 || rc=$? + eq "a tip with another tree is not taken for this commit" "1" "$rc" + eq "and nothing is handed to the caller" "" "$(cat "$work/out")" + + # The same tree on another parent: not the commit this run asked for. + fresh; printf '%s' "$err" > "$work/mutation" + tip "$landed" 1111111111111111111111111111111111111111 "$tree" true + run && rc=0 || rc=$? + eq "a tip on another parent is not taken for this commit" "1" "$rc" + eq "and nothing is handed to the caller" "" "$(cat "$work/out")" + + exit $((fail > 0)) +) || fail=$((fail + 1)) + echo ran="$(wc -l < "$TALLY")" if [ "$ran" -ne "$EXPECTED_ASSERTIONS" ]; then