From 68f52af4a2226085164a9567307cef537cedf93d Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Fri, 21 Aug 2026 16:38:06 +1200 Subject: [PATCH] (fix): accept generated notes on the draft release MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit createDraft requests generate_release_notes, so GitHub returns the body it was sent with the generated changelog appended. Both draft checks then compared that body for equality against what was sent, so the comparison could never hold and every release died after the tag and draft already existed — leaving exactly the half-finished state recovery has to unpick. Require the body to open with the automation markers instead. That is the property the safety check depends on, and RecoverySelector already reads the markers this way rather than demanding an exact body. Name the mismatching fields too: the message listed none of the six it compared, which is why a guaranteed failure took a release run to explain. Co-Authored-By: Claude Opus 5 --- .../scripts/src/Automation/Orchestrator.php | 20 ++++-- .../src/Automation/Repository/GitHub.php | 32 ++++++--- .../Unit/Automation/OrchestratorTest.php | 69 +++++++++++++++++++ CHANGES.md | 3 + 4 files changed, 109 insertions(+), 15 deletions(-) diff --git a/.github/scripts/src/Automation/Orchestrator.php b/.github/scripts/src/Automation/Orchestrator.php index ea18bc0..8b1129a 100644 --- a/.github/scripts/src/Automation/Orchestrator.php +++ b/.github/scripts/src/Automation/Orchestrator.php @@ -348,13 +348,21 @@ private function validateDraft( files: ['Dockerfile'], state: 'merged', ); - if ( - $draft->tag !== $tag - || $draft->body !== $body - || ! RecoverySelector::matches($draft, $merge) - ) { + $mismatched = []; + if ($draft->tag !== $tag) { + $mismatched[] = "tag {$draft->tag} is not {$tag}"; + } + if (! str_starts_with($draft->body, $body)) { + $mismatched[] = 'body does not open with the automation markers'; + } + if (! RecoverySelector::matches($draft, $merge)) { + $mismatched[] = 'release does not match its automation merge'; + } + + if ($mismatched !== []) { throw new RuntimeException( - "Draft release {$draft->identifier} is unsafe", + "Draft release {$draft->identifier} is unsafe: " + . implode(', ', $mismatched), ); } } diff --git a/.github/scripts/src/Automation/Repository/GitHub.php b/.github/scripts/src/Automation/Repository/GitHub.php index ffa11df..6807ca2 100644 --- a/.github/scripts/src/Automation/Repository/GitHub.php +++ b/.github/scripts/src/Automation/Repository/GitHub.php @@ -853,16 +853,30 @@ private function assertDraft( int $pull, string $body, ): Recovery { - if ( - $release->tag !== $tag - || $release->target !== $target - || $release->pull !== $pull - || ! $release->draft - || $release->prerelease - || $release->body !== $body - ) { + $mismatched = []; + if ($release->tag !== $tag) { + $mismatched[] = "tag {$release->tag} is not {$tag}"; + } + if ($release->target !== $target) { + $mismatched[] = "target {$release->target} is not {$target}"; + } + if ($release->pull !== $pull) { + $mismatched[] = "pull {$release->pull} is not {$pull}"; + } + if (! $release->draft) { + $mismatched[] = 'release is not a draft'; + } + if ($release->prerelease) { + $mismatched[] = 'release is a prerelease'; + } + if (! str_starts_with($release->body, $body)) { + $mismatched[] = 'body does not open with the automation markers'; + } + + if ($mismatched !== []) { throw new RuntimeException( - "Draft release {$release->identifier} is unsafe", + "Draft release {$release->identifier} is unsafe: " + . implode(', ', $mismatched), ); } diff --git a/.github/scripts/tests/Unit/Automation/OrchestratorTest.php b/.github/scripts/tests/Unit/Automation/OrchestratorTest.php index d338c8a..3935f05 100644 --- a/.github/scripts/tests/Unit/Automation/OrchestratorTest.php +++ b/.github/scripts/tests/Unit/Automation/OrchestratorTest.php @@ -515,6 +515,71 @@ public function test_recovers_concurrently_created_draft_after_422(): void self::assertSame(0, $runner->remaining()); } + public function test_accepts_a_draft_with_generated_notes_appended(): void + { + $target = str_repeat('a', 40); + $body = $this->draftBody($target, 75); + $runner = new Queue([ + $this->commandResult(1, '{"status":"404"}'), + $this->commandResult( + output: '{"data":{"repository":{"release":null}}}', + ), + $this->commandResult( + output: json_encode( + $this->release( + identifier: 10, + tag: '1.4.5', + target: $target, + draft: true, + body: $body . self::GENERATED_NOTES, + ), + JSON_THROW_ON_ERROR, + ), + ), + ]); + + $draft = $this->github($runner)->createDraft( + '1.4.5', + $target, + 75, + $body, + ); + + self::assertSame(10, $draft->identifier); + self::assertSame(0, $runner->remaining()); + } + + public function test_rejects_a_draft_whose_markers_were_rewritten(): void + { + $target = str_repeat('a', 40); + $body = $this->draftBody($target, 75); + $runner = new Queue([ + $this->commandResult(1, '{"status":"404"}'), + $this->commandResult( + output: '{"data":{"repository":{"release":null}}}', + ), + $this->commandResult( + output: json_encode( + $this->release( + identifier: 10, + tag: '1.4.5', + target: $target, + draft: true, + body: $this->draftBody($target, 76), + ), + JSON_THROW_ON_ERROR, + ), + ), + ]); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage( + 'body does not open with the automation markers', + ); + + $this->github($runner)->createDraft('1.4.5', $target, 75, $body); + } + public function test_does_not_publish_when_prepublication_target_changed(): void { $repository = $this->repository(); @@ -762,6 +827,10 @@ private function pullBody(string $head, string $base): string . "\n"; } + private const string GENERATED_NOTES = "\n\n## What's Changed" + . "\n* chore: update dependencies by @abnegate in " + . 'https://github.com/appwrite/docker-base/pull/75'; + private function draftBody(string $target, int $pull): string { return '' diff --git a/CHANGES.md b/CHANGES.md index d667df6..fb89ce9 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -14,6 +14,9 @@ * The updater rewrote `PHP_*_VERSION` and left `PHP_*_COMMIT` / `PHP_*_CHECKSUM` at the superseded release. Protobuf failed loudly on the checksum, but the git-sourced extensions did not: the build fetched the old commit and shipped, say, brotli 0.20.0 in an image labelled 0.21.0. `Dockerfile::pins()` only ever located the version variable, so no companion reference was ever a candidate for replacement. Every dependency now carries its reference variable through the catalog, resolver, selector, and rewriter, and both move together or neither does. * `git ls-remote --tags --refs` returns the *annotated tag object*, not the commit it points at — `refs/tags/6.3.0` on phpredis is `aa4302d`, while the commit is `df4fab2`. Resolving references from that output would have replaced correct commit pins with tag-object SHAs. The resolver now reads the peeled `^{}` entry when a tag is annotated and falls back to the object for lightweight tags. +* The release step could never succeed. `createDraft` asks GitHub for `generate_release_notes=true`, so the returned body is the automation's body *plus* the generated changelog — and both `assertDraft` and `validateDraft` then required the body to equal what was sent. Every run died at `Draft release is unsafe` after tagging and drafting. Both checks now require the body to *open with* the automation markers, which is what the safety property actually depends on; `RecoverySelector::matches` already worked this way. +* `Draft release is unsafe` named none of the six fields it compared, so diagnosing it needed the API and the source side by side. It now says which ones mismatched. + * A reference that has drifted from its version is now corrected on the next run even when the version itself is unchanged, so a hand-edited or stale pin self-heals instead of persisting. Every reference is resolved from upstream unconditionally — the peeled commit for git, a fresh hash of the selected tarball for PECL — rather than carrying forward whatever the file already held. A pinned tag that upstream no longer publishes now fails the run instead of passing silently. * Duplicate release builds — `build-and-push.yml` no longer triggers on `release: published`. Tag pushes already trigger it, so publishing a release rebuilt and repushed the same image a second time.