Skip to content

Check the release tag against the manifest instead of overwriting it - #271

Merged
514sid merged 2 commits into
masterfrom
check-tag-matches-manifest
Nov 4, 2026
Merged

514sid merged 2 commits into
masterfrom
check-tag-matches-manifest

Conversation

@514sid

@514sid 514sid commented Nov 4, 2026 •

Copy link
Copy Markdown
Member

This is the follow-up left open in #266. The token used there had no workflow scope, so it couldn't land.

build.yaml and test.yaml used to write the tag's version over the committed manifest during the build. That meant the version in the repo was never the one that shipped, and nothing noticed when the two drifted apart (tags at 1.6.1, manifests at 1.0.1). With this change:

  • Both workflows copy src/manifest-<platform>.json into the build unchanged.
  • build.yaml has a new check-version job that compares a tag with both manifests, and the Chrome and Firefox build jobs need it. A tag that doesn't match either manifest stops both builds before any release is created, and the error tells you to bump the manifests on master and tag that commit. The job runs on every trigger and exits early off tags, so branch and PR builds aren't skipped.
  • test.yaml copies the manifest and nothing else. It runs on tags too, but it doesn't publish anything, so one check in the workflow that creates the release is enough.
  • CONTRIBUTING.md no longer says that nothing enforces the tag and manifest staying equal.

Branch and PR builds now carry the manifest's real version instead of 0.0.0. Those artifacts never reach a store, since the release steps only run on tags.

I ran the check script on its own against the current manifests (2026.10.0). v2026.10.0 passes, v26.11.0 fails on both manifests, and branch refs pass. With only the Chrome manifest bumped to 26.11.0, v26.11.0 still fails on Firefox.

Once #270 merges, tagging v26.11.0 will pass this check. Any other tag will be stopped before a release is created.

build.yaml and test.yaml wrote the tag version over the committed manifest,
so the version in the repo was never the one that shipped and nothing
noticed when they differed. The manifests are now copied into the build
unchanged, and a tag build in build.yaml fails when the tag does not equal
the manifest version.

Branch and PR builds now carry the manifest version instead of 0.0.0.
@514sid
514sid requested review from vpetersson-bot and a lite review from Copilot November 4, 2026 06:43

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

A critical workflow flaw allows independent matrix jobs to create a partially published release when only one manifest matches the tag, so tag validation must gate both manifests before release creation.

1 open issue
What changed in this PR

This pull request makes committed platform manifests the source of truth for extension versions and validates release tags against those manifests before packaging.

Changes:

  • Copies platform manifests unchanged in build and test workflows.
  • Validates tag versions against each platform manifest.
  • Updates contributing documentation for the enforced versioning process.
  • Critical issue: Validation currently runs independently in matrix jobs, allowing one platform to publish before another fails. Release creation must depend on validation of both manifests.

This pull request requires changes.

File Summary
CONTRIBUTING.md Documents manifest-based versioning and tag validation.
.github/​workflows/​test.yaml Copies the selected platform manifest unchanged for test builds.
.github/​workflows/​build.yaml Validates tag versions and packages the committed manifest; release gating must be coordinated across both platform jobs.

Review effort: Lite

Comment thread .github/workflows/build.yaml Resolved
The check ran inside each matrix job, so a tag that matched only one
manifest let the job for that platform go on to create the release while
the other failed. A separate check-version job now compares the tag with
both manifests, and the build jobs need it. It runs on every trigger and
exits early off tags, so branch and PR builds are not skipped.
Copilot AI lite review requested due to automatic review settings November 4, 2026 06:49

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The review evidence identifies no active issues, and the documented changes address manifest version validation, preservation, and release-process guidance.

No open issues 🎉

1 resolved since last review

Review effort: Lite

@vpetersson-bot vpetersson-bot 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.

Re-checked at b81c33f. Approving.

  • The check-version preflight job plus needs: check-version on build addresses Copilot's partial-release concern: a tag that matches only one manifest now stops both matrix jobs before Create Release runs.
  • Non-tag refs exit 0 early, so PR and branch builds are unaffected. They now ship the real manifest version instead of 0.0.0, which is fine because nothing publishes them.
  • test.yaml only copies the manifest now. It doesn't publish, so leaving the check out of it is correct.
  • CI is green, including the new check job.

Nit (optional): the PR description still says "Each matrix job checks its own platform's manifest". Since the second commit, a single job checks both, so that line could be updated before merge.

Out of scope, but worth knowing: bin/common.sh, bin/run_tests.sh and bin/package_extension.sh still default VERSION to 0.0.0 and write it over the manifest for local builds. That's harmless because those builds aren't released, but it's the last place where the "manifest is the version" rule doesn't hold.

@514sid
514sid merged commit bcd5956 into master Nov 4, 2026
9 checks passed
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.

3 participants