From 73f560deee96493b8234e65fee5383faa2422722 Mon Sep 17 00:00:00 2001 From: Imani Manyara Date: Sat, 7 Nov 2026 23:28:08 -0500 Subject: [PATCH 1/2] Add a shared check that no workflow leaves a release as a draft Deleting a tag does not delete its release -- it reverts it to a draft. The release keeps its body and assets and stops being visible, and the state outlives the tag: laranail/validation still holds v1.0.0 and v1.0.1 as drafts from tags withdrawn on 2026-08-24, and neither tag exists on the remote any more. `gh release edit` leaves that flag exactly as it finds it, so an edit after a tag is deleted and re-pushed republishes nothing on a run that still goes green. Nothing else reports it: the tag resolves, composer installs, and verify-tag-currency.sh next door passes, because none of them reads the release. It was found by reading one. The check takes the caller's path the way verify-action-pins.py does, so the reusable hygiene workflow can point it at another repository, and it needs no network and no token. Most repositories pass by having no `gh release edit` at all, because softprops/action-gh-release publishes an existing draft on its own. So "found nothing" is the normal result, and a matcher that silently stopped matching would report the whole estate clean. The script runs five built-in controls on every invocation and refuses to report anything when one misbehaves; the workflow-file floor covers a glob that goes empty. The test sabotages the pattern in a copy of the script and asserts the controls catch it. --- CHANGELOG.md | 23 ++ scripts/verify-release-publish.py | 216 ++++++++++++++++++ .../VerifyReleasePublishScriptTest.php | 161 +++++++++++++ 3 files changed, 400 insertions(+) create mode 100755 scripts/verify-release-publish.py create mode 100644 tests/Feature/VerifyReleasePublishScriptTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 790b968..ee9a554 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,29 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- **`scripts/verify-release-publish.py` — no workflow may leave a GitHub release as a draft.** + Deleting a tag does not delete its release, it reverts it to a *draft*: the release keeps its body + and assets and stops being visible, and the state outlives the tag — `laranail/validation` still + holds `v1.0.0` and `v1.0.1` as drafts from tags withdrawn on 2026-08-24, neither of which exists + on the remote any more. `gh release edit` leaves that flag exactly as it finds it, so an edit after + a tag is deleted and re-pushed republishes nothing on a run that still goes **green**. Nothing else + reports it: the tag resolves, `composer` installs, and `verify-tag-currency.sh` next door passes, + because none of them reads the release. Found by reading one, in `laranail/validation-js`, whose + release had to be published by hand. Measured 2026-11-08 across the 286 workflow files in + `laranail/packages`: four hand-rolled call sites in three repositories, all with green runs behind + them. The check takes the caller's path like `verify-action-pins.py`, so the reusable hygiene + workflow can point it at another repository, and it needs no network and no token. + + **Most repositories pass by having no `gh release edit` at all** — `softprops/action-gh-release` + publishes an existing draft unless told to keep it — so "found nothing" is the normal result, and a + matcher that silently stopped matching would report the whole estate clean. The script therefore + drives its matcher against five built-in controls on every invocation and refuses to report + anything if one misbehaves, with the workflow-file floor covering a glob that goes empty. + `tests/Feature/VerifyReleasePublishScriptTest.php` sabotages the pattern in a copy of the script + and asserts the controls catch it. + ### Changed - `scripts/verify-tag-currency.sh` follows the immutable-tag model. A live-line or highest tag that diff --git a/scripts/verify-release-publish.py b/scripts/verify-release-publish.py new file mode 100755 index 0000000..4530c64 --- /dev/null +++ b/scripts/verify-release-publish.py @@ -0,0 +1,216 @@ +#!/usr/bin/env python3 +"""Check that no workflow can leave a GitHub release sitting as a draft. + +Deleting a tag does not delete its release -- it reverts it to a DRAFT. The release keeps +its body and its assets and simply stops being visible, and the state outlives the tag: +`laranail/validation` still holds `v1.0.0` and `v1.0.1` as drafts from tags withdrawn on +2026-08-24, neither of which exists on the remote any more. + +`gh release edit` leaves that flag exactly as it finds it. So an edit after a tag is deleted +and re-pushed -- which is how the family moves a pre-stable tag wherever `git push --force` +is unavailable -- republishes nothing on a run that still goes GREEN: correct tag, invisible +release. + +Nothing else reports that state. The tag resolves, `composer` installs, and +`verify-tag-currency.sh` next door passes, because none of them reads the release. It was +found by reading one: `laranail/validation-js` had a live release sitting invisible after a +green run, and it had to be published by hand (laranail/validation-js#35). + +Measured 2026-11-08 across the 286 workflow files in `laranail/packages`: four hand-rolled +call sites in three repositories, every one with green runs behind it. + +The fix is one flag, `--draft=false`, on every `gh release edit`. Most repositories never +need it, because they use `softprops/action-gh-release`, which publishes an existing draft +unless its `draft` input is true -- "omit it to publish after upload", in its own words. + +A REPOSITORY WITH NO `gh release edit` IS THEREFORE CLEAN, NOT SUSPICIOUS, and that is the +one way this check could lie: if the matcher stopped matching, every repository would pass +and the check would report a clean estate it had not read. So the matcher is driven against +built-in positive and negative controls on every invocation, and the run fails if any control +does not behave. The file floor covers the other direction, a workflows glob that goes empty. + +Not checked, deliberately: `gh release create --draft`. A release created as a draft on +purpose, to have assets uploaded before it is published, is the pattern the action's own +documentation describes, so flagging it would report the correct case. + +No network and no token: this is a text scan, so it is the cheapest job in the hygiene +workflow and it reports the same thing locally as it does on a runner. +""" + +from __future__ import annotations + +import re +import sys +from pathlib import Path + +# Every `gh release edit` invocation, after comments are dropped and shell line +# continuations are joined. +EDIT = re.compile(r"gh release edit[^\n]*") + +# A trailing backslash and the indentation of the next line: joined so a flag written on +# the following line still counts as part of the same command. +CONTINUATION = re.compile(r"\\\n\s*") + +# The prose beside a command names it too ("`gh release edit` takes no assets"), so comment +# lines are removed before matching rather than filtered out afterwards. +COMMENT = re.compile(r"^\s*#") + +# gh needs the `=` form for a false boolean: `--draft false` would be read as a positional +# argument, so this is the exact spelling that publishes. +PUBLISHES = "--draft=false" + +# A workflows directory that has gone empty, or a glob that has stopped matching, must fail +# rather than report a clean repository -- the same floor verify-action-pins.py carries. +MIN_WORKFLOW_FILES = 1 + +# (label, sample, expected commands found, expected failures). A scanner that can only ever +# say "clean" proves nothing, so these run every time: the first two prove it can fail and +# can pass, the third that a continuation is joined, and the last two that it does not match +# prose about the command or a different gh subcommand. +CONTROLS: tuple[tuple[str, str, int, int], ...] = ( + ( + "an edit with no flag is caught", + ' gh release edit "$TAG" --notes-file body.md\n', + 1, + 1, + ), + ( + "an edit that publishes passes", + ' gh release edit "$TAG" --draft=false --notes-file body.md\n', + 1, + 0, + ), + ( + "a flag on the next line counts", + ' gh release edit "$TAG" \\\n --draft=false --notes-file body.md\n', + 1, + 0, + ), + ( + "prose about the command is not a command", + " # `gh release edit` takes no assets, so the upload is explicit\n", + 0, + 0, + ), + ( + "another gh subcommand is not this one", + ' gh release create "$TAG" --notes-file body.md\n', + 0, + 0, + ), +) + + +def commands(text: str) -> list[str]: + """Every `gh release edit` invocation in one workflow's text.""" + code = "\n".join(line for line in text.split("\n") if not COMMENT.match(line)) + + return EDIT.findall(CONTINUATION.sub(" ", code)) + + +def failures_in(text: str) -> list[str]: + """The invocations in one workflow's text that would leave a release as a draft.""" + return [c for c in commands(text) if PUBLISHES not in c] + + +def broken_controls() -> list[str]: + """Any control the matcher no longer handles, which makes every result meaningless.""" + out: list[str] = [] + for label, sample, found, bad in CONTROLS: + got_found, got_bad = len(commands(sample)), len(failures_in(sample)) + if (got_found, got_bad) != (found, bad): + out.append( + f" CONTROL {label}: expected {found} command(s) and {bad} failure(s), " + f"got {got_found} and {got_bad}" + ) + + return out + + +def workflows_dir(argv: list[str]) -> Path: + """Which `.github/workflows` to check. + + Defaults to the repository this script lives in, which is what a package running its + own copy wants. An explicit path argument is what makes the script shareable: the + reusable hygiene workflow checks this repository out beside the caller's and points the + script at the caller's workflows. + + Deriving the path from `__file__` alone silently scanned the wrong repository the moment + the script stopped being a local copy. + """ + if len(argv) > 1: + return Path(argv[1]).resolve() / ".github" / "workflows" + + return Path(__file__).resolve().parent.parent / ".github" / "workflows" + + +def main() -> int: + workflows = workflows_dir(sys.argv) + + print() + print(f" Release publishing: {workflows}") + print() + + broken = broken_controls() + if broken: + print(" The matcher is broken, so nothing below can be trusted:") + print() + for line in broken: + print(line) + print() + print(" Fix the matcher before reading any result from this script.") + return 1 + + if not workflows.is_dir(): + print(f" No workflows directory at {workflows}.") + return 1 + + # Both extensions: GitHub reads either, and globbing one silently skipped the other. + workflow_files = sorted(p for p in workflows.iterdir() if p.suffix in (".yml", ".yaml")) + + if len(workflow_files) < MIN_WORKFLOW_FILES: + print( + f" Refusing to report a clean repository: {len(workflow_files)} workflow " + f"files in {workflows}, floor is {MIN_WORKFLOW_FILES}." + ) + return 1 + + found = 0 + failures: list[str] = [] + + for workflow in workflow_files: + for command in commands(workflow.read_text()): + found += 1 + if PUBLISHES in command: + print(f" OK {workflow.name}: {command.strip()}") + else: + failures.append( + f" DRAFT {workflow.name}: {command.strip()}\n" + f" add {PUBLISHES}, or a deleted-and-re-pushed tag leaves " + f"this release invisible on a green run" + ) + + print() + if failures: + for line in failures: + print(line) + print() + print(f" {len(failures)} of {found} release edit(s) would leave a draft behind.") + return 1 + + if found: + print( + f" {len(workflow_files)} workflow files, {found} release edit(s): every one " + f"publishes the release it edits." + ) + else: + print( + f" {len(workflow_files)} workflow files, no hand-rolled release edit: nothing " + f"here can leave a draft behind." + ) + + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/Feature/VerifyReleasePublishScriptTest.php b/tests/Feature/VerifyReleasePublishScriptTest.php new file mode 100644 index 0000000..64d816c --- /dev/null +++ b/tests/Feature/VerifyReleasePublishScriptTest.php @@ -0,0 +1,161 @@ +run(); + + return $process; +} + +function releaseStep(string ...$lines): string +{ + return "jobs:\n release:\n steps:\n - run: |\n" . implode('', array_map( + static fn (string $line): string => " {$line}\n", + $lines, + )); +} + +it('fails an edit that does not publish the release', function (): void { + $process = runReleasePublish(releaseStep('gh release edit "$TAG" --notes-file body.md')); + + expect($process->getExitCode())->toBe(1) + ->and($process->getOutput())->toContain('DRAFT release.yml') + ->toContain('add --draft=false') + ->toContain('1 of 1 release edit(s) would leave a draft behind'); +})->skipOnWindows(); + +it('passes an edit that publishes the release', function (): void { + $process = runReleasePublish(releaseStep('gh release edit "$TAG" --draft=false --notes-file body.md')); + + expect($process->getExitCode())->toBe(0) + ->and($process->getOutput())->toContain('1 release edit(s): every one publishes'); +})->skipOnWindows(); + +it('joins a shell line continuation, so a flag on the next line still counts', function (): void { + // Matching line by line would call this a failure and send someone to add a flag already there. + $process = runReleasePublish(releaseStep( + 'gh release edit "$TAG" \\', + ' --draft=false \\', + ' --notes-file body.md', + )); + + expect($process->getExitCode())->toBe(0); +})->skipOnWindows(); + +it('does not read prose about the command as the command', function (): void { + // Every repository that carried this defect also carried a comment naming `gh release edit`. + $process = runReleasePublish(releaseStep( + '# `gh release edit` takes no assets, so the upload below is explicit', + 'gh release edit "$TAG" --draft=false --notes-file body.md', + )); + + expect($process->getExitCode())->toBe(0) + ->and($process->getOutput())->toContain('1 release edit(s)'); +})->skipOnWindows(); + +it('leaves other gh subcommands alone', function (): void { + $process = runReleasePublish(releaseStep( + 'gh release create "$TAG" --notes-file body.md', + 'gh release upload "$TAG" sbom.cdx.json --clobber', + 'gh release view "$TAG" --json isDraft', + )); + + expect($process->getExitCode())->toBe(0) + ->and($process->getOutput())->toContain('no hand-rolled release edit'); +})->skipOnWindows(); + +it('passes a repository that edits no release, which is most of them', function (): void { + // softprops/action-gh-release publishes an existing draft unless told to keep it, so a + // repository using it has nothing to fix. Finding nothing is the normal pass, not a warning. + $process = runReleasePublish("jobs:\n release:\n steps:\n - uses: softprops/action-gh-release@v2\n"); + + expect($process->getExitCode())->toBe(0) + ->and($process->getOutput())->toContain('no hand-rolled release edit'); +})->skipOnWindows(); + +it('reads a .yaml workflow, which globbing *.yml silently skipped', function (): void { + $process = runReleasePublish( + releaseStep('gh release edit "$TAG" --notes-file body.md'), + 'release.yaml', + ); + + expect($process->getExitCode())->toBe(1) + ->and($process->getOutput())->toContain('release.yaml'); +})->skipOnWindows(); + +it('refuses to report a clean repository when the workflows directory is empty', function (): void { + // With "no edits found" as the normal pass, an empty or unmatched directory would otherwise be + // indistinguishable from a repository that is genuinely clean. + $process = runReleasePublish(''); + + expect($process->getExitCode())->toBe(1) + ->and($process->getOutput())->toContain('Refusing to report a clean repository'); +})->skipOnWindows(); + +it('counts every workflow file, not only the ones holding a command', function (): void { + $root = dirname(__DIR__, 2); + $repo = sys_get_temp_dir() . '/release-publish-' . bin2hex(random_bytes(6)); + mkdir($repo . '/.github/workflows', 0o755, true); + file_put_contents($repo . '/.github/workflows/ci.yml', "jobs:\n a:\n steps: []\n"); + file_put_contents( + $repo . '/.github/workflows/release.yml', + releaseStep('gh release edit "$TAG" --draft=false --notes-file body.md'), + ); + + $process = new Process(['python3', $root . '/scripts/verify-release-publish.py', $repo]); + $process->run(); + + expect($process->getExitCode())->toBe(0) + ->and($process->getOutput())->toContain('2 workflow files, 1 release edit(s)'); +})->skipOnWindows(); + +it('fails on its own controls when the matcher stops matching', function (): void { + // The one way this check can lie: most repositories pass by having no command at all, so a + // broken matcher would report the estate clean. Sabotage the pattern in a copy and the controls + // must refuse to report anything -- and must say the matcher, not the repository, is at fault. + $root = dirname(__DIR__, 2); + $sabotaged = sys_get_temp_dir() . '/verify-release-publish-broken-' . bin2hex(random_bytes(6)) . '.py'; + $source = (string) file_get_contents($root . '/scripts/verify-release-publish.py'); + $broken = str_replace('r"gh release edit[^\n]*"', 'r"gh release editx[^\n]*"', $source); + + expect($broken)->not->toBe($source, 'the sabotage must actually change the pattern'); + + file_put_contents($sabotaged, $broken); + + $process = runReleasePublish( + releaseStep('gh release edit "$TAG" --notes-file body.md'), + 'release.yml', + $sabotaged, + ); + + expect($process->getExitCode())->toBe(1) + ->and($process->getOutput())->toContain('The matcher is broken') + ->toContain('CONTROL') + ->and($process->getOutput())->not->toContain('DRAFT release.yml'); +})->skipOnWindows(); From 6ee871cabdf911693153a8b9d471e0d58a0f0635 Mon Sep 17 00:00:00 2001 From: Imani Manyara Date: Sat, 7 Nov 2026 23:43:32 -0500 Subject: [PATCH 2/2] Annotate two arrow functions Rector 2.8 asks for Neither file is touched by this branch's own change. The repository tracks no lock file, so CI resolves fresh on every run: it got Rector 2.8.4 and PHPStan 2.3.5 while the local vendor directory held 2.6.7 and 2.2.17, and AddArrowFunctionReturnTypeRector in the newer Rector wants `fn (): never` on an arrow function whose body only throws. So the static-analysis job went red on code that has been on main since before this branch, and the last run on main predates the Rector release. A `composer update -W` locally reproduces both findings exactly; this is what Rector itself writes. It lands here rather than in its own pull request because the branch cannot go green without it, and the family's convention is one pull request per repository rather than a trickle. --- tests/Feature/BootPackageAboutSectionsTest.php | 2 +- tests/Unit/HasConsoleWrapperTest.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Feature/BootPackageAboutSectionsTest.php b/tests/Feature/BootPackageAboutSectionsTest.php index 98562f0..751f42a 100644 --- a/tests/Feature/BootPackageAboutSectionsTest.php +++ b/tests/Feature/BootPackageAboutSectionsTest.php @@ -29,7 +29,7 @@ public function configurePackage(Package $package): void ->field('Enum', SeederExecutionMode::Queued) // → "queued" ->field('Bool', true) // → "true" ->field('Missing', null) // → "null" - ->field('Boom', fn () => throw new RuntimeException('unmigrated')) + ->field('Boom', fn (): never => throw new RuntimeException('unmigrated')) ->fallback('n/a (unavailable)'), ) ->hasAboutSection( diff --git a/tests/Unit/HasConsoleWrapperTest.php b/tests/Unit/HasConsoleWrapperTest.php index 9ca8885..5f33ffb 100644 --- a/tests/Unit/HasConsoleWrapperTest.php +++ b/tests/Unit/HasConsoleWrapperTest.php @@ -76,7 +76,7 @@ public function it_respects_and_when_callable_condition(): void public function it_handles_exceptions_gracefully(): void { $result = $this->shouldRunInConsole( - callback: fn () => throw new Exception('Test error'), + callback: fn (): never => throw new Exception('Test error'), expectReturn: true, default: 'fallback', );