Fix vendor source files being stripped from release package - #15
Conversation
The release rsync excluded 'src/' without a leading slash. rsync treats a pattern with no slash as matching at every depth, so this removed src/ from every vendor package, not just the plugin's own block source. The published 1.0.0 package is missing 107 PHP files: all of Guzzle, guzzle/promises, PSR-7, psr/http-client, psr/http-factory, psr/http-message and getallheaders. Two of the three files listed in Composer's generated autoload_files.php were among them, and those are require'd unconditionally, so the plugin fatals as soon as vendor/autoload.php is included: Fatal error: Uncaught Error: Failed opening required '.../ralouphie/getallheaders/src/getallheaders.php' in vendor/composer/autoload_real.php:41 bin/package-plugin.sh had the correct anchored '/src/' pattern, so local zips were fine and the discrepancy went unnoticed. Extract the staging step into bin/stage-release.sh so the local script and the deploy workflow can no longer disagree about what ships, and anchor the other plugin-root-only patterns (/composer.lock, /package.json, /package-lock.json, /phpcs.xml) that had the same latent problem. Add bin/verify-release.sh, which resolves every $vendorDir/$baseDir reference in Composer's generated autoload files against disk and then loads the autoloader to resolve the classes the plugin actually uses. The previous checks only asserted that release/mailmojo/vendor existed, which was true of the broken package. The new check reports 111 of 177 references missing on the published zip and passes on a correctly staged one. Run staging and verification on pull requests too, so packaging regressions surface before a release is tagged rather than during deploy. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Steps without an `if:` only run when all previous steps succeeded, so with the upload placed after the verification steps the artifact was produced only for packages that already passed -- exactly the opposite of when it is useful. Zip and upload before verifying, and mark the upload `!cancelled()` so it also runs on failure. The deploy step has no condition of its own, so it still only runs when every preceding step succeeded and verification continues to gate the release. Downgrade if-no-files-found to a warning: if staging itself fails there is legitimately no zip, and an upload error there would obscure the real failure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
checkout, setup-node and upload-artifact were on v4, which targets Node.js 20 and is now force-run on Node.js 24 with a deprecation warning. Taking all three to v7 rather than the minimum that clears the warning, since nothing in the intervening majors affects this usage: checkout v6 moved credential persistence to a separate file and v7 blocks fork checkouts for pull_request_target/workflow_run, neither of which applies here; setup-node v5/v6 changed automatic package manager detection, and both workflows set `cache: npm` explicitly; upload-artifact v7 adds an opt-in `archive` parameter that defaults to existing behaviour. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ed3ee58f2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Mutable tags mean a third party can change what runs in the release pipeline without any change here, and that pipeline holds the WordPress.org SVN credentials. Pin every action to a commit SHA with the version in a trailing comment. Each SHA is the commit its previous floating tag already resolved to, so this changes nothing about what executes: actions/checkout@v7 -> 3d3c42e5 (v7.0.1) actions/setup-node@v7 -> 82076278 (v7.0.0) actions/upload-artifact@v7 -> 043fb46d (v7.0.1) shivammathur/setup-php@v2 -> f3e473d1 (v2.37.2) 10up/...-deploy@2.3.0 -> 54bd289b (2.3.0) setup-php was on a floating major and the deploy action on a mutable release tag, so both were exposed the same way despite looking pinned. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary
The rsync in the deploy workflow excluded
src/without a leading slash. rsync treats a pattern containing no slash as matching at every depth, so this strippedsrc/from every vendor package — not just the plugin's own block source atmailmojo/src/.bin/package-plugin.shhad the correctly anchored/src/, so locally built zips were fine. That is why the discrepancy went unnoticed: the two exclude lists were byte-identical apart from that one character.Impact on the published 1.0.0
I downloaded the published package from WordPress.org and inspected it. 107 PHP files are missing — all of Guzzle, guzzle/promises, PSR-7, psr/http-client, psr/http-factory, psr/http-message and getallheaders. The vendor folders are present but contain only
composer.jsonandLICENSE.This does not degrade gracefully. Two of the three files listed in Composer's generated
autoload_files.phpwere stripped, and those arerequired unconditionally, so the plugin fatals as soon asvendor/autoload.phpis included — on every request where it is active. Executed against the real published zip on PHP 8.2:Changes
bin/stage-release.sh(new) — single source of truth for what ships. Uses the anchored/src/, and anchors the other plugin-root-only patterns (/composer.lock,/package.json,/package-lock.json,/phpcs.xml) that had the same latent problem.test/,tests/,docs/,*.mdstay unanchored, which is intentional for trimming vendor cruft. A comment records the anchoring rule and what breaks without it.bin/verify-release.sh(new) — resolves every$vendorDir/$baseDirreference in Composer's generated autoload files against disk (classmap targets, PSR-4 roots, always-required files — 177 for the current dependency set), then loads the autoloader and resolves the classes the plugin actually uses. The previous checks only asserted thatrelease/mailmojo/vendorexisted, which was true of the broken package. The autoload integrity check is pure bash so it runs without a local PHP toolchain; the load test runs whereverphpis onPATH.bin/package-plugin.sh/deploy-wordpress-org.yml— both now call the shared scripts, so they can no longer disagree. The deploy additionally unzips the built package and verifies that, so a bad package blocks the deploy instead of shipping.verify-package.yml(new) — runs the same staging and verification on pull requests and pushes tomain, so packaging regressions surface before a release is tagged. Uploads the candidate zip as an artifact for inspection.Verification
Built a true
--no-devvendor tree from the committedcomposer.lockand ran the full packaging sequence against it:src/patternThe broken staging and the published zip fail identically, confirming the reproduction is faithful. The zip round-trip passes. Diffing the new package against the published one: 107 files gained, 0 lost, so nothing was over-excluded.
Follow-up
WordPress.org will not accept a re-upload of an existing tag, so this needs a version bump to
1.0.1inmailmojo/mailmojo.phpand theStable taginmailmojo/readme.txt— as a separate commit, per the release process. The deploy workflow's version-validation step enforces that both match the git tag.🤖 Generated with Claude Code