chore: align version metadata + version-consistency CI guard (#107, #119) - #118
Merged
Conversation
appinfo/info.xml is the single source of truth for the app version (the release tarball script already derives the version from it). Align the stale package.json / package-lock.json (1.0.0) to info.xml's 1.1.0-alpha.2 so the metadata no longer drifts. Bump EtherpadClient::DEFAULT_API_VERSION 1.2.15 -> 1.3.0 to match current Etherpad deployments. The constant is only the auto-detection fallback used when no etherpad_api_version is stored; tests reference it symbolically and stay green. Closes #107.
The vite license plugin embeds package.json's version into the js/*.license SPDX sidecars. Bumping the version to 1.1.0-alpha.2 changes those generated files; the .mjs bundles themselves are unchanged. Commit them so the stale-js CI guard (#101) passes.
Revert the fallback bump. The constant is a failsafe used only when auto-detection fails and no version is stored; Etherpad's API version list is cumulative, so a *lower* requested version is accepted by every server that supports a higher one (and by older servers too), while a higher request is rejected. A low fallback therefore maximises compatibility in exactly the degraded path where it matters, and the plugin uses no method newer than ~1.2.1. Documented the rationale inline so it is not 'bumped' again. The package.json/info.xml alignment from this PR stays.
Jaggob
changed the title
appinfo/info.xml is the single source of truth for the app version, but package.json / package-lock.json are aligned by hand and drifted (1.0.0 vs 1.1.0-alpha.2). Add a CI job that fails when the three disagree, so the metadata can't silently diverge again on the next bump. Closes #119.
Jaggob
changed the title
There was a problem hiding this comment.
Pull request overview
Aligns version metadata across the PHP app manifest and Node tooling, regenerates version-stamped license sidecars, documents the intentionally conservative Etherpad API fallback, and adds CI to prevent future version drift.
Changes:
- Bumps
package.json/package-lock.jsonversion to matchappinfo/info.xml(1.1.0-alpha.2). - Regenerates
js/*.licensesidecars to reflect the updated package version metadata. - Adds a
version-consistencyCI job and documents the rationale for keepingEtherpadClient::DEFAULT_API_VERSIONconservative.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| package.json | Updates Node package version metadata to match appinfo/info.xml. |
| package-lock.json | Updates lockfile version fields to match appinfo/info.xml. |
| lib/Service/EtherpadClient.php | Adds inline documentation explaining why the fallback API version is intentionally low. |
| js/oc-compat-hVqZy-MX.chunk.mjs.license | Regenerated license sidecar reflecting the new app/package version. |
| js/fetch-helpers-C4MxuNvt.chunk.mjs.license | Regenerated license sidecar reflecting the new app/package version. |
| js/etherpad_nextcloud-viewer-main.mjs.license | Regenerated license sidecar reflecting the new app/package version. |
| js/etherpad_nextcloud-files-main.mjs.license | Regenerated license sidecar reflecting the new app/package version. |
| js/etherpad_nextcloud-embed-main.mjs.license | Regenerated license sidecar reflecting the new app/package version. |
| js/etherpad_nextcloud-embed-create-main.mjs.license | Regenerated license sidecar reflecting the new app/package version. |
| js/etherpad_nextcloud-admin-settings.mjs.license | Regenerated license sidecar reflecting the new app/package version. |
| js/api-client-BXEMiUh7.chunk.mjs.license | Regenerated license sidecar reflecting the new app/package version. |
| .github/workflows/lint-info-xml.yml | Adds a CI job to fail PRs when info.xml, package.json, and package-lock.json versions drift. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+55
to
+67
| INFO_VERSION="$(grep -oE '<version>[^<]+</version>' appinfo/info.xml | sed -E 's|</?version>||g' | head -n1)" | ||
| PKG_VERSION="$(jq -r '.version' package.json)" | ||
| LOCK_VERSION="$(jq -r '.version' package-lock.json)" | ||
| echo "info.xml: $INFO_VERSION" | ||
| echo "package.json: $PKG_VERSION" | ||
| echo "package-lock.json: $LOCK_VERSION" | ||
| if [ -z "$INFO_VERSION" ]; then | ||
| echo "::error::could not read <version> from appinfo/info.xml" >&2 | ||
| exit 1 | ||
| fi | ||
| if [ "$PKG_VERSION" != "$INFO_VERSION" ] || [ "$LOCK_VERSION" != "$INFO_VERSION" ]; then | ||
| echo "::error::Version mismatch. appinfo/info.xml is the source of truth ($INFO_VERSION); align package.json and package-lock.json to it." >&2 | ||
| exit 1 |
Address review feedback on #118: npm stores the project version twice in the lockfile (top-level .version and .packages[""].version). The drift guard only checked the former, so an inconsistent root package entry could slip through. Validate both lockfile fields against info.xml.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
appinfo/info.xmlis the single source of truth for the app version (the release tarball script already derives the version from<version>). Aligned the stalepackage.json/package-lock.json(1.0.0) to info.xml's1.1.0-alpha.2.package.json's version isprivateand not consumed by the Vite build, so this is purely metadata-consistency.js/*.licenseSPDX sidecars — the Vite license plugin embedspackage.json's version into them, so the bump changes those generated files (the.mjsbundles are unchanged). Needed for the CI: fail the build when committed js/ assets are stale #101 stale-js guard.EtherpadClient::DEFAULT_API_VERSIONstays at1.2.15(see below) with an inline comment documenting why it is intentionally conservative.lint-info-xml.yml→version-consistencyjob): fails the build wheninfo.xml,package.jsonandpackage-lock.jsonversions disagree, so the metadata can't silently drift again. This folds in the tech-debt: keep package.json version in sync with appinfo/info.xml #119 follow-up directly.Why not bump the API fallback
DEFAULT_API_VERSIONis a failsafe, used only when auto-detection fails and noetherpad_api_versionis stored. Etherpad's API version list is cumulative: a newer server accepts any older requested version, while a request higher than the server supports is rejected. So a lower fallback maximises compatibility in exactly the degraded path where the constant matters, and the plugin uses no method newer than ~1.2.1 (it never passes theauthorIdparam 1.3.0 added). Bumping the failsafe would trade a tiny compatibility loss for zero functional gain. In normal operation the detected/stored version wins anyway. Documented inline so it isn't re-bumped.Acceptance
package.json/info.xml(both1.1.0-alpha.2)DEFAULT_API_VERSIONreviewed — intentionally kept at the conservative1.2.15, with rationale documentedVerification
info=pkg=lock=1.1.0-alpha.2).Closes #107.
Closes #119.