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.