From 8b517e2268c1068abb0c4bbe20e2315c932c7d99 Mon Sep 17 00:00:00 2001 From: blessdyb Date: Tue, 3 Nov 2026 22:52:20 -0800 Subject: [PATCH] Avoid duplicate release test runs Require successful exact-commit PR CI and CodeQL evidence before signing, then reuse those reviewed results instead of running the same newest-Xcode test suite again in Release. Keep signing, notarization, Gatekeeper, checksums, publication, and Homebrew validation unchanged. Co-Authored-By: Claude Code --- .github/workflows/ci.yml | 3 +- .github/workflows/release.yml | 76 ++++++--------------- docs/DEVELOPMENT.md | 17 ++--- scripts/check_release_checks.py | 117 ++++++++++++++++++++++++++++++++ scripts/test_release_checks.py | 89 ++++++++++++++++++++++++ 5 files changed, 237 insertions(+), 65 deletions(-) create mode 100644 scripts/check_release_checks.py create mode 100644 scripts/test_release_checks.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 110d77c4..703c89ec 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -178,12 +178,13 @@ jobs: - name: Generate the project and reject drift run: scripts/check_xcode_project.sh - - name: Check bounded notarization retries, catalog policy, and warning gate + - name: Check release helpers, catalog policy, and warning gate if: matrix.xcode == 'newest' run: | scripts/test-notarize.sh scripts/test-rule-pack-policy.sh scripts/test-swift-warnings.sh + python3 scripts/test_release_checks.py # This compiles a different product graph and activation path from the Developer ID build. The build clears # protected entitlements because CI has no Store profiles; the next step re-signs the isolated product with the diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 0584fbcd..990160c3 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -22,6 +22,8 @@ on: permissions: contents: write + checks: read + pull-requests: read concurrency: group: release-${{ github.ref }} @@ -77,6 +79,24 @@ jobs: exit 1 fi + # The release runs at the immutable tag after its pull request gates have passed. Re-running the same test suite + # here used another macOS runner for no new evidence. Resolve the checked-out tag to its commit instead, then + # require the latest trusted CI and CodeQL results plus an open or merged main-targeting pull request at that SHA. + - name: Require reviewed checks for the exact release commit + env: + GH_TOKEN: ${{ github.token }} + run: | + set -euo pipefail + RELEASE_SHA=$(git rev-parse HEAD) + CHECKS="$RUNNER_TEMP/release-checks.json" + PULLS="$RUNNER_TEMP/release-pulls.json" + gh api --paginate --slurp \ + "repos/${{ github.repository }}/commits/$RELEASE_SHA/check-runs?per_page=100" > "$CHECKS" + gh api --paginate --slurp -H 'Accept: application/vnd.github+json' \ + "repos/${{ github.repository }}/commits/$RELEASE_SHA/pulls?per_page=100" > "$PULLS" + python3 -I scripts/check_release_checks.py \ + --repo "${{ github.repository }}" --sha "$RELEASE_SHA" --checks "$CHECKS" --pulls "$PULLS" + # The newest Xcode on the image, named in the log so a release says which compiler built it. - name: Select Xcode run: | @@ -100,62 +120,6 @@ jobs: fi done - - name: Test - timeout-minutes: 40 - run: | - # GitHub starts run blocks with bash -e. This step records the first attempt's status so it can retry a - # timeout, then exits explicitly if the final attempt failed. - set +e - set -uo pipefail - # xcodebuild occasionally never returns here — the same suite passes locally and on most runs, and the - # per-test allowance below doesn't always catch it, so the whole invocation gets a watchdog. One retry: - # a hang that clears on a second attempt is a flake, and one that repeats is a real failure worth seeing. - run_tests() { - local log="$1" limit=900 - xcodebuild test -project Flowlight.xcodeproj -scheme Flowlight -destination 'platform=macOS' \ - -test-timeouts-enabled YES \ - -default-test-execution-time-allowance 120 \ - -maximum-test-execution-time-allowance 300 \ - CODE_SIGN_IDENTITY=- CODE_SIGN_STYLE=Manual DEVELOPMENT_TEAM= CODE_SIGN_ENTITLEMENTS= > "$log" 2>&1 & - local pid=$! - local waited=0 - while kill -0 "$pid" 2>/dev/null; do - if [ "$waited" -ge "$limit" ]; then - echo "::warning::xcodebuild still running after ${limit}s — killing it." - kill -9 "$pid" 2>/dev/null - pkill -9 -f "xcodebuild|XCTestCore|testmanagerd" 2>/dev/null - wait "$pid" 2>/dev/null - return 124 - fi - sleep 5 - waited=$((waited + 5)) - done - wait "$pid" - } - - LOG="$RUNNER_TEMP/test.log" - run_tests "$LOG" - status=$? - grep -E "Test Case .* (passed|failed)|Executed .* tests|\*\* TEST" "$LOG" | tail -25 - - if [ "$status" -eq 124 ]; then - echo "::group::Where it stopped (last 60 lines before the kill)"; tail -60 "$LOG"; echo "::endgroup::" - echo "Retrying once." - LOG="$RUNNER_TEMP/test-retry.log" - run_tests "$LOG" - status=$? - grep -E "Executed .* tests|\*\* TEST" "$LOG" | tail -10 - fi - - if [ "$status" -ne 0 ]; then - echo "::group::Last 300 lines"; tail -300 "$LOG"; echo "::endgroup::" - echo "Why it failed:" - grep -E "error:|Testing failed:|XCTAssert.*failed|exceeded execution time allowance" "$LOG" | head -40 - exit 1 - fi - set -e - scripts/check-swift-warnings.sh "$LOG" - # A keychain of its own, unlocked only for this job, so nothing lands in the runner's login keychain. - name: Import the signing certificates env: diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md index 92c1ecc0..becbcf49 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -451,17 +451,18 @@ comes after publication so the site can only ever describe something downloadabl force-push or move a pushed version. If a tag is wrong or a release needs another change, bump the patch version and begin a new release branch and tag. -A tag push starts `release.yml` automatically in dry-run-only mode; push-triggered runs cannot publish. It must build, -sign, notarize and pass Gatekeeper verification before an explicit production dispatch for the immutable tag. After -production succeeds, verify the GitHub release page and downloaded DMG, PKG and -`SHA256SUMS.txt` checksums before merging the release PR. +A tag push starts `release.yml` automatically in dry-run-only mode; push-triggered runs cannot publish. It first +requires an open or merged `main` pull request and the latest successful CI and CodeQL checks at the exact tagged +commit, rather than running the same test suite a second time. It must then build, sign, notarize and pass Gatekeeper +verification before an explicit production dispatch for the immutable tag. After production succeeds, verify the +GitHub release page and downloaded DMG, PKG and `SHA256SUMS.txt` checksums before merging the release PR. ## Releasing from CI -`.github/workflows/release.yml` checks the tag matches `MARKETING_VERSION`, runs the tests, signs, notarizes and -verifies with `spctl`. A tag push always stops there; an explicit dispatch for that tag publishes the release and updates -the Homebrew cask. `.github/workflows/ci.yml` builds and tests every push and pull request, and fails if `docs/` is out -of date with `site/`. +`.github/workflows/release.yml` checks the tag matches `MARKETING_VERSION`, verifies the exact commit already passed +the required PR CI and CodeQL checks, then signs, notarizes and verifies with `spctl`. A tag push always stops there; +an explicit dispatch for that tag publishes the release and updates the Homebrew cask. `.github/workflows/ci.yml` +builds and tests every push and pull request, and fails if `docs/` is out of date with `site/`. ```sh scripts/ci-secrets.sh # upload the secrets once diff --git a/scripts/check_release_checks.py b/scripts/check_release_checks.py new file mode 100644 index 00000000..587e608c --- /dev/null +++ b/scripts/check_release_checks.py @@ -0,0 +1,117 @@ +#!/usr/bin/env python3 +"""Require reviewed exact-commit CI evidence before a release consumes signing credentials.""" + +import argparse +import json +import pathlib +import sys + + +REQUIRED_CHECKS = ( + ("github-actions", "changes"), + ("github-actions", "site"), + ("github-actions", "test (oldest Xcode)"), + ("github-actions", "test (newest Xcode)"), + ("github-actions", "analyze (swift)"), + ("github-actions", "analyze (python)"), + ("github-actions", "analyze (actions)"), + ("github-advanced-security", "CodeQL"), +) + + +def load(path): + return json.loads(pathlib.Path(path).read_text()) + + +def check_runs(document): + pages = document if isinstance(document, list) else [document] + runs = [] + for page in pages: + if not isinstance(page, dict) or not isinstance(page.get("check_runs"), list): + raise ValueError("check-run response has an unexpected shape") + runs.extend(page["check_runs"]) + return runs + + +def pull_requests(document): + pages = document if isinstance(document, list) else [document] + pulls = [] + for page in pages: + if isinstance(page, list): + pulls.extend(page) + elif isinstance(page, dict) and "number" in page: + pulls.append(page) + else: + raise ValueError("pull-request response has an unexpected shape") + return pulls + + +def verify(repo, sha, checks_document, pulls_document): + errors = [] + eligible_pulls = [] + for pull in pull_requests(pulls_document): + head_sha = pull.get("head", {}).get("sha") + base_ref = pull.get("base", {}).get("ref") + state = pull.get("state") + merged = bool(pull.get("merged_at")) + if head_sha == sha and base_ref == "main" and (state == "open" or merged): + eligible_pulls.append(pull) + if not eligible_pulls: + errors.append(f"commit {sha} has no open or merged pull request targeting main at that exact head") + + latest = {} + repo_prefix = f"https://github.com/{repo}/" + for run in check_runs(checks_document): + app = run.get("app", {}).get("slug") + name = run.get("name") + if (app, name) not in REQUIRED_CHECKS: + continue + details_url = run.get("details_url", "") + if not details_url.startswith(repo_prefix): + continue + key = (app, name) + if int(run.get("id", 0)) > int(latest.get(key, {}).get("id", 0)): + latest[key] = run + + for key in REQUIRED_CHECKS: + run = latest.get(key) + label = f"{key[1]} ({key[0]})" + if not run: + errors.append(f"missing required check: {label}") + elif run.get("status") != "completed" or run.get("conclusion") != "success": + errors.append( + f"required check is not successful: {label} " + f"status={run.get('status')} conclusion={run.get('conclusion')}" + ) + + if errors: + return errors, [] + return [], [int(pull["number"]) for pull in eligible_pulls] + + +def main(): + parser = argparse.ArgumentParser() + parser.add_argument("--repo", required=True) + parser.add_argument("--sha", required=True) + parser.add_argument("--checks", type=pathlib.Path, required=True) + parser.add_argument("--pulls", type=pathlib.Path, required=True) + args = parser.parse_args() + + try: + errors, pull_numbers = verify(args.repo, args.sha, load(args.checks), load(args.pulls)) + except (OSError, ValueError, json.JSONDecodeError) as error: + print(f"release check evidence could not be read: {error}", file=sys.stderr) + return 1 + + if errors: + for error in errors: + print(f"release check rejected: {error}", file=sys.stderr) + return 1 + + joined = ", ".join(f"#{number}" for number in pull_numbers) + print(f"Exact commit {args.sha} has reviewed release evidence from {joined} and all required checks passed.") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/test_release_checks.py b/scripts/test_release_checks.py new file mode 100644 index 00000000..1e627a02 --- /dev/null +++ b/scripts/test_release_checks.py @@ -0,0 +1,89 @@ +#!/usr/bin/env python3 + +import copy +import unittest + +from check_release_checks import REQUIRED_CHECKS, verify + + +REPO = "xinbetween/flowlight" +SHA = "a" * 40 + + +def successful_checks(): + runs = [] + for index, (app, name) in enumerate(REQUIRED_CHECKS, start=1): + path = "actions/runs/1/job/2" if app == "github-actions" else "runs/3" + runs.append({ + "id": index, + "name": name, + "status": "completed", + "conclusion": "success", + "details_url": f"https://github.com/{REPO}/{path}", + "app": {"slug": app}, + }) + return [{"total_count": len(runs), "check_runs": runs}] + + +def reviewed_pull(**changes): + pull = { + "number": 76, + "state": "open", + "merged_at": None, + "base": {"ref": "main"}, + "head": {"sha": SHA}, + } + pull.update(changes) + return [[pull]] + + +class ReleaseCheckTests(unittest.TestCase): + def test_accepts_exact_reviewed_commit_with_all_checks(self): + errors, pulls = verify(REPO, SHA, successful_checks(), reviewed_pull()) + self.assertEqual(errors, []) + self.assertEqual(pulls, [76]) + + def test_accepts_merged_pull(self): + pulls = reviewed_pull(state="closed", merged_at="2026-11-04T00:00:00Z") + errors, _ = verify(REPO, SHA, successful_checks(), pulls) + self.assertEqual(errors, []) + + def test_rejects_missing_check(self): + checks = successful_checks() + checks[0]["check_runs"].pop() + errors, _ = verify(REPO, SHA, checks, reviewed_pull()) + self.assertTrue(any("missing required check" in error for error in errors)) + + def test_newer_failure_cannot_hide_behind_older_success(self): + checks = successful_checks() + failed = copy.deepcopy(checks[0]["check_runs"][0]) + failed.update(id=999, status="completed", conclusion="failure") + checks[0]["check_runs"].append(failed) + errors, _ = verify(REPO, SHA, checks, reviewed_pull()) + self.assertTrue(any("not successful" in error for error in errors)) + + def test_rejects_check_from_untrusted_app(self): + checks = successful_checks() + checks[0]["check_runs"][0]["app"]["slug"] = "untrusted" + errors, _ = verify(REPO, SHA, checks, reviewed_pull()) + self.assertTrue(any("missing required check" in error for error in errors)) + + def test_rejects_check_linked_to_another_repository(self): + checks = successful_checks() + checks[0]["check_runs"][0]["details_url"] = "https://github.com/other/repo/actions/runs/1" + errors, _ = verify(REPO, SHA, checks, reviewed_pull()) + self.assertTrue(any("missing required check" in error for error in errors)) + + def test_rejects_wrong_pull_head(self): + pulls = reviewed_pull(head={"sha": "b" * 40}) + errors, _ = verify(REPO, SHA, successful_checks(), pulls) + self.assertTrue(any("no open or merged pull request" in error for error in errors)) + + def test_rejects_closed_unmerged_pull(self): + pulls = reviewed_pull(state="closed", merged_at=None) + errors, _ = verify(REPO, SHA, successful_checks(), pulls) + self.assertTrue(any("no open or merged pull request" in error for error in errors)) + + +if __name__ == "__main__": + unittest.main()