Fix CI failure reporting and interrupted draft uploads

.github/workflows/ci.yml

@@ -38,15 +38,6 @@ jobs:
         with:
           name: d2chaos
           path: ./d2chaos/out
-  nofixups:
-    runs-on: ubuntu-latest
-    timeout-minutes: 5
-    steps:
-      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
-      - run: git submodule update --init
-      - run: COLOR=1 ./ci/sub/bin/nofixups.sh
-        env:
-          GITHUB_TOKEN: ${{ github.token }}
   signed:
     runs-on: ubuntu-latest
     timeout-minutes: 5

ci/release/README.md

@@ -95,10 +95,14 @@ After the workflow succeeds, review and merge the release PR, then publish the c
 draft manually on GitHub. `release.sh` rejects `--publish`, `--skip-build`, and `--rebuild`,
 and never repurposes an existing version. To recover a failed run without changing code or
 artifacts, use **Re-run failed jobs** on that run. Code or build changes require a new version,
-even if the old version is still a draft. Its Actions artifacts are immutable: a full
-rerun fails rather than replacing them. If draft upload was interrupted, the failed job
-resumes by accepting identical existing assets and attaching only missing ones. Artifacts
-are retained for 30 days; recover failures within that window.
+even if the old version is still a draft. Individual Actions artifacts are immutable, but
+reruns can create newer artifacts with the same names; downloads by name select the newest.
+Retry failed jobs to reuse completed build outputs. If draft upload was interrupted, the
+failed job accepts identical existing assets and attaches only missing ones. A GitHub upload
+failure can leave an empty `starter` asset; after rechecking the tag, draft, and asset identity,
+the uploader deletes only that zero-byte placeholder and retries its upload. Changed or
+completed assets are never deleted. Artifacts are retained for 30 days; recover failures
+within that window. Run recovery without another uploader modifying the same draft.
 
 Release-related PRs exercise the same archive and native smoke jobs and, when the WiX gate
 below is enabled, the MSI build. PRs never attest or write release assets. CI is called

ci/release/build.sh

@@ -132,12 +132,12 @@ main() {
     runjob windows/amd64 'OS=windows ARCH=amd64 build'
     runjob windows/arm64 'OS=windows ARCH=arm64 build'
   else
-    runjob linux/amd64 'OS=linux ARCH=amd64 build' &
-    runjob linux/arm64 'OS=linux ARCH=arm64 build' &
-    runjob macos/amd64 'OS=macos ARCH=amd64 build' &
-    runjob macos/arm64 'OS=macos ARCH=arm64 build' &
-    runjob windows/amd64 'OS=windows ARCH=amd64 build' &
-    runjob windows/arm64 'OS=windows ARCH=arm64 build' &
+    runjob_bg linux/amd64 'OS=linux ARCH=amd64 build'
+    runjob_bg linux/arm64 'OS=linux ARCH=arm64 build'
+    runjob_bg macos/amd64 'OS=macos ARCH=amd64 build'
+    runjob_bg macos/arm64 'OS=macos ARCH=arm64 build'
+    runjob_bg windows/amd64 'OS=windows ARCH=amd64 build'
+    runjob_bg windows/arm64 'OS=windows ARCH=arm64 build'
     waitjobs
   fi
   write_checksums

ci/release/upload-draft.py

@@ -94,6 +94,25 @@ def existing_asset(release, path, expected_digest, expected_size, allow_pending=
     return True
 
 
+def starter_asset_id(asset):
+    if (asset.get("state") == "starter" and type(asset.get("size")) is int and asset["size"] == 0
+            and asset.get("digest") is None and type(asset.get("id")) is int and asset["id"] > 0):
+        return asset["id"]
+    return None
+
+
+def preflight_assets(release, assets):
+    starters = {}
+    for asset in assets:
+        matches = [item for item in release["assets"] if item["name"] == asset[0].name]
+        asset_id = starter_asset_id(matches[0]) if len(matches) == 1 else None
+        if asset_id is not None:
+            starters[asset[0].name] = asset_id
+            continue
+        existing_asset(release, *asset)
+    return starters
+
+
 def upload(repository, version, commit, assets):
     verify_tag(repository, version, commit)
     release = find_release(repository, version)
@@ -111,9 +130,18 @@ def upload(repository, version, commit, assets):
     verify_draft(release, version)
     release_id = release["id"]
     prerelease = release["prerelease"]
-    # Preflight every existing asset before the first upload. Retries accept only identical bytes.
-    for asset in assets:
-        existing_asset(release, *asset)
+    # A failed GitHub upload can leave an empty starter. Never delete completed assets.
+    starters = preflight_assets(release, assets)
+    for name, asset_id in starters.items():
+        release = api(f"repos/{repository}/releases/{release_id}")
+        verify_draft(release, version, release_id, prerelease)
+        if preflight_assets(release, assets).get(name) != asset_id:
+            raise ValueError(f"starter asset changed before cleanup: {name}")
+        asset = api(f"repos/{repository}/releases/assets/{asset_id}")
+        if asset.get("name") != name or starter_asset_id(asset) != asset_id:
+            raise ValueError(f"starter asset changed before cleanup: {name}")
+        verify_tag(repository, version, commit)
+        gh("api", "--method", "DELETE", f"repos/{repository}/releases/assets/{asset_id}")
     for path, expected_digest, expected_size in assets:
         verify_tag(repository, version, commit)
         release = api(f"repos/{repository}/releases/{release_id}")

ci/release/upload_draft_test.py

@@ -25,17 +25,38 @@ def __init__(self, assets=(), published=False):
         self.commit = COMMIT
         self.writes = []
         self.change_after_upload = None
+        self.change_before_release_read = None
+        self.change_before_asset_read = None
+        self.fail_upload_once = False
+        self.next_asset_id = 1000
         for asset in assets:
             self.add_asset(*asset)
 
     def add_asset(self, path, digest, size):
-        self.release["assets"].append({"name": path.name, "state": "uploaded", "digest": digest, "size": size})
+        asset = {"id": self.next_asset_id, "name": path.name, "state": "uploaded", "digest": digest, "size": size}
+        self.next_asset_id += 1
+        self.release["assets"].append(asset)
+        return asset
+
+    def add_starter(self, path):
+        asset = self.add_asset(path, None, 0)
+        asset["state"] = "starter"
+        return asset
 
     def api(self, path):
         if "/git/ref/tags/" in path:
             return {"object": {"type": "commit", "sha": self.commit}}
         if path.endswith("/releases/123"):
+            if self.change_before_release_read:
+                self.change_before_release_read(self)
+                self.change_before_release_read = None
             return copy.deepcopy(self.release)
+        if "/releases/assets/" in path:
+            asset = next(asset for asset in self.release["assets"] if asset["id"] == int(path.rsplit("/", 1)[1]))
+            if self.change_before_asset_read:
+                self.change_before_asset_read(asset)
+                self.change_before_asset_read = None
+            return copy.deepcopy(asset)
         raise AssertionError(path)
 
     def gh(self, *args):
@@ -46,9 +67,19 @@ def gh(self, *args):
             self.release = FakeGitHub().release
         elif args[:2] == ("release", "upload"):
             path = Path(args[3])
+            assert "--clobber" not in args
+            assert not any(asset["name"] == path.name for asset in self.release["assets"])
+            if self.fail_upload_once:
+                self.fail_upload_once = False
+                self.add_starter(path)
+                self.writes.append(args)
+                raise RuntimeError("502 upload failure left an empty starter")
             self.add_asset(path, uploader.digest(path), path.stat().st_size)
             if self.change_after_upload:
                 self.change_after_upload(self)
+        elif args[:3] == ("api", "--method", "DELETE"):
+            asset_id = int(args[3].rsplit("/", 1)[1])
+            self.release["assets"] = [asset for asset in self.release["assets"] if asset["id"] != asset_id]
         else:
             raise AssertionError(args)
         self.writes.append(args)
@@ -123,6 +154,97 @@ def test_conflict_preflight_prevents_partial_upload(self):
             self.run_upload(fake)
         self.assertEqual(fake.writes, [])
 
+    def test_starter_cleanup_recovers_repeated_upload_failure(self):
+        fake = FakeGitHub(self.assets[:2])
+        completed = copy.deepcopy(fake.release["assets"])
+        first_id = fake.add_starter(self.assets[2][0])["id"]
+        fake.fail_upload_once = True
+        with self.assertRaisesRegex(RuntimeError, "502"):
+            self.run_upload(fake)
+        second_id = fake.release["assets"][-1]["id"]
+        self.assertNotEqual(first_id, second_id)
+        self.run_upload(fake)
+        writes = copy.deepcopy(fake.writes)
+        self.run_upload(fake)
+        self.assertEqual(fake.writes, writes)
+        self.assertEqual(fake.release["assets"][:2], completed)
+        self.assertEqual(len(fake.release["assets"]), 9)
+        self.assertEqual([call[3] for call in fake.writes if call[:3] == ("api", "--method", "DELETE")],
+                         [f"repos/test/repo/releases/assets/{first_id}", f"repos/test/repo/releases/assets/{second_id}"])
+
+    def test_unsafe_starter_metadata_is_never_deleted(self):
+        for field, value in (("state", "uploading"), ("state", "uploaded"), ("size", 1), ("size", False),
+                             ("digest", "sha256:" + "b" * 64), ("id", "1000"), ("id", -1), ("id", True), ("id", None)):
+            with self.subTest(field=field, value=value):
+                fake = FakeGitHub()
+                fake.add_starter(self.assets[0][0])[field] = value
+                with self.assertRaisesRegex(ValueError, "incomplete metadata"):
+                    self.run_upload(fake)
+                self.assertEqual(fake.writes, [])
+
+    def test_starter_change_before_cleanup_prevents_deletion(self):
+        path, digest, size = self.assets[0]
+        changes = ({"id": 456}, {"state": "uploading"}, {"size": 1}, {"digest": digest},
+                   {"state": "uploaded", "size": size, "digest": digest})
+        for phase in ("release", "asset"):
+            for change in changes:
+                with self.subTest(phase=phase, change=change):
+                    fake = FakeGitHub()
+                    starter = fake.add_starter(path)
+                    if phase == "release":
+                        fake.change_before_release_read = lambda state: starter.update(change)
+                    else:
+                        fake.change_before_asset_read = lambda asset: asset.update(change)
+                    with self.assertRaises(ValueError):
+                        self.run_upload(fake)
+                    self.assertEqual(fake.writes, [])
+
+    def test_starter_name_change_at_exact_asset_read_prevents_deletion(self):
+        fake = FakeGitHub()
+        fake.add_starter(self.assets[0][0])
+        fake.change_before_asset_read = lambda asset: asset.update(name="unrelated.tar.gz")
+        with self.assertRaisesRegex(ValueError, "starter asset changed"):
+            self.run_upload(fake)
+        self.assertEqual(fake.writes, [])
+
+    def test_conflict_elsewhere_prevents_starter_cleanup(self):
+        for phase in ("preflight", "cleanup"):
+            with self.subTest(phase=phase):
+                fake = FakeGitHub([self.assets[-1]])
+                fake.add_starter(self.assets[0][0])
+                def conflict(state):
+                    state.release["assets"][0]["digest"] = "sha256:" + "b" * 64
+                if phase == "preflight":
+                    conflict(fake)
+                else:
+                    fake.change_before_release_read = conflict
+                with self.assertRaisesRegex(ValueError, "different bytes"):
+                    self.run_upload(fake)
+                self.assertEqual(fake.writes, [])
+
+    def test_duplicate_starters_are_never_deleted(self):
+        fake = FakeGitHub()
+        fake.add_starter(self.assets[0][0])
+        fake.add_starter(self.assets[0][0])
+        with self.assertRaisesRegex(ValueError, "incomplete metadata"):
+            self.run_upload(fake)
+        self.assertEqual(fake.writes, [])
+
+    def test_release_or_tag_change_before_cleanup_prevents_deletion(self):
+        for field, value in (("id", 456), ("prerelease", True), ("draft", False), ("tag_name", "v9.9.9"), ("commit", "b" * 40)):
+            with self.subTest(field=field):
+                fake = FakeGitHub()
+                fake.add_starter(self.assets[0][0])
+                def change(state):
+                    if field == "commit":
+                        state.commit = value
+                    else:
+                        state.release[field] = value
+                fake.change_before_release_read = change
+                with self.assertRaises(ValueError):
+                    self.run_upload(fake)
+                self.assertEqual(fake.writes, [])
+
     def test_published_release_is_never_modified(self):
         fake = FakeGitHub(published=True)
         with self.assertRaisesRegex(ValueError, "remain a draft"):