Skip to content

fix(sync): cover plugin PHP and block-theme version stamps#284

Merged
adekbadek merged 2 commits into
mainfrom
fix/sync-pin-all-version-stamps
Jun 11, 2026
Merged

fix(sync): cover plugin PHP and block-theme version stamps#284
adekbadek merged 2 commits into
mainfrom
fix/sync-pin-all-version-stamps

Conversation

@adekbadek

Copy link
Copy Markdown
Member

Changes proposed in this Pull Request:

Follow-up to #282. A code review of that fix surfaced that restore_release_artifacts only protected a subset of release-stamped files, leaving the same downgrade-on-clean-merge bug open for:

  • Every plugin's main PHP file - the Version: header (the version WordPress and publishers actually read, e.g. newspack-popups.php Version: 3.12.1) plus any *_VERSION constant kept in lockstep with it (NEWSPACK_BLOCKS__VERSION, NEWSPACK_ADS_VERSION). Stamped via config/release.js files: [ phpFile ]. None were restored, so a legacy plugin hotfix would downgrade the WP-visible plugin version.
  • The block theme - src/scss/_theme-description.scss (the leading underscore slipped past the theme-description.scss regex) and functions.php (Version: header), both committed by its release config's gitCommitStep.

Classic themes were already fully covered (they commit only theme-description.scss + CHANGELOG.md).

Fix:

  • Broaden the wholesale restore to _?theme-description.scss (covers the block theme).
  • Add a surgical pass that resets only the Version: header and *_VERSION constants in the main plugin PHP file / theme functions.php to HEAD's version - these are real source, so the rest of the file (the actual fix the hotfix carried) is preserved.
  • Harden the embedded node calls: each is now the if condition, so a single malformed file is skipped instead of aborting the whole sync under set -e (also drops a redundant git cat-file + git show double-fetch).

How to test the changes in this Pull Request:

  1. bash bin/migration/test-restore-release-artifacts.sh - 18/18 assertions across all three release-stamped shapes (classic theme, plugin with PHP header + version constant, block theme with functions.php + underscore SCSS). Each shape asserts both "stamp restored to monorepo version" and "real source change kept".
  2. Red/green check: the new plugin/block-theme assertions fail against the pre-fix function and pass against this one (the classic-theme assertions stay green in both).
  3. bash -n bin/migration/lib.sh - syntax check.

Other information:

  • Have you written new tests for your changes, as applicable?
  • Have you successfully run tests with your changes locally?

🤖 Generated with Claude Code

restore_release_artifacts only protected CHANGELOG, classic-theme
theme-description.scss, and package.json name/version. A late legacy
hotfix release also stamps a regressed version into files it missed,
which a clean "legacy wins" merge then lands unchallenged:

- every plugin's main PHP file Version: header (the version WordPress and
  publishers actually read) plus any *_VERSION constant kept in lockstep
- the block theme's underscore-prefixed src/scss/_theme-description.scss
  (missed by the theme-description.scss regex) and functions.php Version:

Broaden the wholesale restore to _?theme-description.scss, and add a
surgical pass that resets only the Version: header and *_VERSION constants
in the main plugin PHP file / theme functions.php to HEAD's value, leaving
real source the hotfix carried intact. Node calls are now the `if`
condition so one malformed file skips rather than aborting the sync under
set -e. Test extended with plugin and block-theme fixtures.
Copilot AI review requested due to automatic review settings June 11, 2026 10:10
@adekbadek adekbadek requested a review from a team as a code owner June 11, 2026 10:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review any files in this pull request.

Guard the surgical PHP version pin against an unmerged file: editing and
`git add`-ing one would mark it resolved with conflict markers still
inside, hiding it from the post-merge check that escalates. Skip it so the
escalation fires. Add a conflict-path test that drives an actual
`git merge --no-commit -Xtheirs` (the prior cases staged a post-merge
state directly), plus a note that package.json passes through three
sequential rewriters.
@adekbadek adekbadek merged commit 3c55ef1 into main Jun 11, 2026
7 checks passed
@adekbadek adekbadek deleted the fix/sync-pin-all-version-stamps branch June 11, 2026 10:23
@github-actions

Copy link
Copy Markdown

Hey @adekbadek, good job getting this PR merged! 🎉

Now, the needs-changelog label has been added to it.

Please check if this PR needs to be included in the "Upcoming Changes" and "Release Notes" doc. If it doesn't, simply remove the label.

If it does, please add an entry to our shared document, with screenshots and testing instructions if applicable, then remove the label.

Thank you! ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants