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