diff --git a/.github/scripts/classify-upload.sh b/.github/scripts/classify-upload.sh new file mode 100755 index 0000000..d4ffe84 --- /dev/null +++ b/.github/scripts/classify-upload.sh @@ -0,0 +1,32 @@ +#!/usr/bin/env bash +# Classify the result of `xcrun altool --upload-app`. +# +# Sourced by the publish steps of distribute-beta.yml and distribute-release.yml +# so both paths share one definition of "did this upload actually land?" — the +# logic used to be duplicated inline in each, and drifted into the same bug twice. +# +# classify_upload → one of: +# accepted ASC took the binary +# already-present the identical version+build is already there; safe to skip +# failed anything else; the caller must fail the job +classify_upload() { + local rc="$1" out="$2" + + if [ "$rc" -eq 0 ] || grep -qE "UPLOAD SUCCEEDED|No errors uploading" <<<"$out"; then + echo accepted; return + fi + + # Benign ONLY for a true redundant upload — same version AND same build already + # on ASC, which Apple reports as ITMS-90189 / "Redundant Binary Upload". + # + # Deliberately does NOT match the looser "already been used". Apple phrases the + # -19232 build-number collision as "an attribute with a value that has already + # been used", and that is a real failure needing a higher CFBundleVersion. The + # wider pattern swallowed it as an idempotent re-run and reported the job green, + # so failed uploads looked like successful ones. + if grep -qiE "ITMS-90189|redundant binary upload" <<<"$out"; then + echo already-present; return + fi + + echo failed +} diff --git a/.github/workflows/distribute-beta.yml b/.github/workflows/distribute-beta.yml index a1b9369..3ac2ba2 100644 --- a/.github/workflows/distribute-beta.yml +++ b/.github/workflows/distribute-beta.yml @@ -448,6 +448,7 @@ jobs: ASC_ISSUER_ID: ${{ secrets.ASC_ISSUER_ID }} run: | set -uo pipefail + source .shared-ci/.github/scripts/classify-upload.sh while IFS=$'\x1f' read -r id pname sub afile aseed bd bas das dap ver mkt label rtag; do { [ "$bas" = "True" ] && [ "$das" = "True" ]; } || continue echo "::group::TestFlight upload — product=$id" @@ -461,12 +462,10 @@ jobs: RC=$? set -e echo "$OUTPUT" - if grep -qE "UPLOAD SUCCEEDED|No errors uploading" <<<"$OUTPUT"; then - echo "TestFlight upload accepted for $id"; echo "::endgroup::"; continue - fi - if grep -qiE "already been (used|uploaded)|redundant binary upload|ITMS-90189" <<<"$OUTPUT"; then - echo "Build for $id already present — treating as success."; echo "::endgroup::"; continue - fi + case "$(classify_upload "$RC" "$OUTPUT")" in + accepted) echo "TestFlight upload accepted for $id"; echo "::endgroup::"; continue ;; + already-present) echo "Build for $id already present — treating as success."; echo "::endgroup::"; continue ;; + esac echo "::error::altool failed for $id (exit ${RC})"; echo "::endgroup::"; exit 1 done < /tmp/products.tsv diff --git a/.github/workflows/distribute-release.yml b/.github/workflows/distribute-release.yml index 0422a98..2f191f8 100644 --- a/.github/workflows/distribute-release.yml +++ b/.github/workflows/distribute-release.yml @@ -412,6 +412,7 @@ jobs: ASC_ISSUER_ID: ${{ secrets.ASC_ISSUER_ID }} run: | set -uo pipefail + source .shared-ci/.github/scripts/classify-upload.sh while IFS=$'\x1f' read -r id pname sub afile aseed clfile bd bas das dap; do { [ "$bas" = "True" ] && [ "$das" = "True" ]; } || continue echo "::group::App Store upload — product=$id" @@ -425,12 +426,10 @@ jobs: RC=$? set -e echo "$OUTPUT" - if grep -qE "UPLOAD SUCCEEDED|No errors uploading" <<<"$OUTPUT"; then - echo "App Store upload accepted for $id"; echo "::endgroup::"; continue - fi - if grep -qiE "already been (used|uploaded)|redundant binary upload|ITMS-90189" <<<"$OUTPUT"; then - echo "Build for $id already present — treating as success."; echo "::endgroup::"; continue - fi + case "$(classify_upload "$RC" "$OUTPUT")" in + accepted) echo "App Store upload accepted for $id"; echo "::endgroup::"; continue ;; + already-present) echo "Build for $id already present — treating as success."; echo "::endgroup::"; continue ;; + esac echo "::error::altool failed for $id (exit ${RC})"; echo "::endgroup::"; exit 1 done < /tmp/products.tsv diff --git a/tests/run.sh b/tests/run.sh index 5053f57..0777df3 100644 --- a/tests/run.sh +++ b/tests/run.sh @@ -99,5 +99,23 @@ echo "== validation: two empty-id products → hard error ==" CAP PRODUCTS_DIR="$DUAL" python3 "$PY" discover { [ $RC -ne 0 ] && grep -q "at most one product may omit" /tmp/pd.err; } && pass "dual-bare rejected" || bad "dual-bare should fail with the one-primary error (rc=$RC)" +echo "== classify_upload: altool outcome classification ==" +# Sourced from the shipped script rather than re-implemented, so this test cannot +# drift from what the publish steps actually run. +source "$ROOT/.github/scripts/classify-upload.sh" +cls() { GOT=$(classify_upload "$2" "$3"); [ "$GOT" = "$4" ] && pass "$1" || { echo " FAIL: $1 — got '$GOT', want '$4'"; FAIL=1; }; } + +cls "clean success → accepted" 0 \ + "UPLOAD SUCCEEDED with no errors +No errors uploading archive at './App.pkg'." accepted +cls "exit 0, quiet output → accepted" 0 "Uploading... done" accepted +# Verbatim from the FrameBison run that reported green while the upload failed. +cls "build-number collision (-19232) → failed" 31 \ + "ERROR: [ContentDelivery.Uploader.7814C25280] The provided entity includes an attribute with a value that has already been used (-19232) The bundle version must be higher than the previously uploaded version: '1'. +ERROR: [altool.main] ExitFailure (31)" failed +cls "true redundant upload (ITMS-90189) → already-present" 31 \ + "ERROR: [altool] Redundant Binary Upload. There already exists a binary upload with build version '42' (ITMS-90189)" already-present +cls "opaque altool error → failed" 1 "ERROR: [altool.main] network unreachable" failed + echo -[ $FAIL -eq 0 ] && echo "ALL products.py TESTS PASSED ✅" || { echo "SOME TESTS FAILED ❌"; exit 1; } +[ $FAIL -eq 0 ] && echo "ALL TESTS PASSED ✅" || { echo "SOME TESTS FAILED ❌"; exit 1; }