From 39735d47c0a382ba626a40936668158b4680d6fc Mon Sep 17 00:00:00 2001 From: Terve Date: Wed, 23 Sep 2026 16:56:51 -0400 Subject: [PATCH 1/2] fix(release-intent): bump the lockfile version with package.json The release-intent bot moved the release version in desktop/package.json but never touched desktop/package-lock.json, so the lockfile's root version (top-level and packages[""]) stayed behind after every automated release and disagreed with package.json. Apply now rewrites both lockfile copies in place in the same single commit as the other version files, and fails rather than commit if the edit would change anything else. Reading a file through the contents API now also fails loudly when GitHub omits the body, which it does above 1 MB; the lockfile is already about half that. VERSIONING.md points manual minor or major bumps at npm version so the lockfile moves with them too. Signed-off-by: Terve --- scripts/release-intent/README.md | 6 ++- scripts/release-intent/apply_pr.py | 33 ++++++++++----- scripts/release-intent/lib.py | 42 +++++++++++++++++++ scripts/release-intent/test_lib.py | 65 ++++++++++++++++++++++++++++++ services/VERSIONING.md | 3 +- 5 files changed, 136 insertions(+), 13 deletions(-) diff --git a/scripts/release-intent/README.md b/scripts/release-intent/README.md index ce99e76b..9c5a1bb3 100644 --- a/scripts/release-intent/README.md +++ b/scripts/release-intent/README.md @@ -49,9 +49,11 @@ an ordering accident nobody did wrong. ## What apply writes -Three files, in **one** commit built through the git data API: +Four files, in **one** commit built through the git data API: - `desktop/package.json` — the release version, one PATCH forward +- `desktop/package-lock.json` — the same version, at the top level and under + `packages[""]`, so the lockfile never names a different release - `services/versions.json` — the declared `services` and component bumps - `CHANGELOG.md` — a new section titled with the new release version, citing the pull request number @@ -137,5 +139,5 @@ python3 scripts/release-intent/apply_pr.py \ `--dry-run` **writes the working tree** despite the name. Restore afterwards: ```bash -git restore desktop/package.json services/versions.json CHANGELOG.md +git restore desktop/package.json desktop/package-lock.json services/versions.json CHANGELOG.md ``` diff --git a/scripts/release-intent/apply_pr.py b/scripts/release-intent/apply_pr.py index 759ac270..9510c445 100644 --- a/scripts/release-intent/apply_pr.py +++ b/scripts/release-intent/apply_pr.py @@ -3,11 +3,12 @@ # SPDX-License-Identifier: Apache-2.0 """Apply a merged pull request's release-intent block to the version files. -Writes three files in ONE commit via the git data API: desktop/package.json -(the release version, patch-bumped), services/versions.json (declared services -and component bumps), and CHANGELOG.md (a new section). +Writes four files in ONE commit via the git data API: desktop/package.json +and desktop/package-lock.json (the release version, patch-bumped), +services/versions.json (declared services and component bumps), and +CHANGELOG.md (a new section). -The contents API would be one commit per file, which for three files means a +The contents API would be one commit per file, which for four files means a release landing in pieces and a window where the changelog names a version that package.json does not yet carry. Building a tree and moving the ref once avoids that, and the ref update doubles as the concurrency check: a non-fast-forward @@ -35,6 +36,7 @@ BOT_COMMIT_PREFIX, CHANGELOG_PATH, PACKAGE_JSON_PATH, + PACKAGE_LOCK_PATH, REPO_ROOT, VERSIONS_PATH, apply_bumps, @@ -47,21 +49,24 @@ prepend_changelog, read_release_version, render_package_json, + render_package_lock, render_versions_json, ) VERSIONS_REPO_PATH = str(VERSIONS_PATH.relative_to(REPO_ROOT)) CHANGELOG_REPO_PATH = str(CHANGELOG_PATH.relative_to(REPO_ROOT)) PACKAGE_REPO_PATH = str(PACKAGE_JSON_PATH.relative_to(REPO_ROOT)) +PACKAGE_LOCK_REPO_PATH = str(PACKAGE_LOCK_PATH.relative_to(REPO_ROOT)) BLOB_MODE = '100644' @dataclass(frozen=True) class ReleaseUpdate: - """The three file bodies a release intent produces, and what to call it.""" + """The four file bodies a release intent produces, and what to call it.""" package: str + package_lock: str versions: str changelog: str release: str @@ -69,6 +74,7 @@ class ReleaseUpdate: def as_paths(self) -> dict[str, str]: return { PACKAGE_REPO_PATH: self.package, + PACKAGE_LOCK_REPO_PATH: self.package_lock, VERSIONS_REPO_PATH: self.versions, CHANGELOG_REPO_PATH: self.changelog, } @@ -156,12 +162,13 @@ def fetch_merged_pr(repo: str, sha: str, token: str) -> tuple[str, str] | None: def compute_release_update( package_text: str, + package_lock_text: str, versions_text: str, changelog_text: str, description: str, pr_ref: str, ) -> ReleaseUpdate | None: - """Apply the intent to three file bodies. None when nothing changes.""" + """Apply the intent to four file bodies. None when nothing changes.""" versions, raw = parse_versions(versions_text, VERSIONS_REPO_PATH) intent = parse_release_intent( description, versions.bump_keys, key_policy='reject-unknown' @@ -187,6 +194,7 @@ def compute_release_update( ) return ReleaseUpdate( package=render_package_json(package_text, release_after), + package_lock=render_package_lock(package_lock_text, release_after), versions=render_versions_json(updated, raw), changelog=prepend_changelog(changelog_text, entry), release=release_after, @@ -202,7 +210,8 @@ def read_file_at(repo: str, token: str, path: str, ref: str) -> str: if not isinstance(payload, dict): raise SystemExit(f'Unexpected payload reading {path} at {ref}') content = payload.get('content') - if not isinstance(content, str): + # Above 1 MB the contents API sends encoding "none" and an empty content. + if payload.get('encoding') != 'base64' or not isinstance(content, str): raise SystemExit(f'Incomplete payload reading {path} at {ref}') return base64.b64decode(content).decode('utf-8') @@ -289,6 +298,7 @@ def apply_release( if dry_run: update = compute_release_update( PACKAGE_JSON_PATH.read_text(encoding='utf-8'), + PACKAGE_LOCK_PATH.read_text(encoding='utf-8'), VERSIONS_PATH.read_text(encoding='utf-8'), CHANGELOG_PATH.read_text(encoding='utf-8'), description, @@ -298,12 +308,14 @@ def apply_release( print('Dry-run: nothing to apply') return PACKAGE_JSON_PATH.write_text(update.package, encoding='utf-8') + PACKAGE_LOCK_PATH.write_text(update.package_lock, encoding='utf-8') VERSIONS_PATH.write_text(update.versions, encoding='utf-8') CHANGELOG_PATH.write_text(update.changelog, encoding='utf-8') print( - 'Dry-run MODIFIED the working tree (package.json, versions.json, ' - 'CHANGELOG.md). Restore with: git restore ' - f'{PACKAGE_REPO_PATH} {VERSIONS_REPO_PATH} {CHANGELOG_REPO_PATH}' + 'Dry-run MODIFIED the working tree (package.json, package-lock.json, ' + 'versions.json, CHANGELOG.md). Restore with: git restore ' + f'{PACKAGE_REPO_PATH} {PACKAGE_LOCK_REPO_PATH} ' + f'{VERSIONS_REPO_PATH} {CHANGELOG_REPO_PATH}' ) print(message) return @@ -333,6 +345,7 @@ def apply_release( update = compute_release_update( read_file_at(repo, token, PACKAGE_REPO_PATH, head), + read_file_at(repo, token, PACKAGE_LOCK_REPO_PATH, head), read_file_at(repo, token, VERSIONS_REPO_PATH, head), read_file_at(repo, token, CHANGELOG_REPO_PATH, head), description, diff --git a/scripts/release-intent/lib.py b/scripts/release-intent/lib.py index 9eaf5d7b..9f02f4f7 100644 --- a/scripts/release-intent/lib.py +++ b/scripts/release-intent/lib.py @@ -77,6 +77,7 @@ VERSIONS_PATH = REPO_ROOT / 'services' / 'versions.json' CHANGELOG_PATH = REPO_ROOT / 'CHANGELOG.md' PACKAGE_JSON_PATH = REPO_ROOT / 'desktop' / 'package.json' +PACKAGE_LOCK_PATH = REPO_ROOT / 'desktop' / 'package-lock.json' VERSIONS_COMMENT = ( 'Single source of truth for all version numbers. See VERSIONING.md for bump rules.' @@ -157,6 +158,47 @@ def render_package_json(text: str, version: str) -> str: return replaced +_LOCK_ROOT_PACKAGE_RE = re.compile(r'"packages"\s*:\s*\{\s*""\s*:\s*\{') + + +def render_package_lock(text: str, version: str) -> str: + """Set both copies of the release version in desktop/package-lock.json. + + npm records the root package's version at the top level and again under + packages[""], and both must match package.json. Each is replaced in place, + as in render_package_json, and the result is compared with a parsed edit so + a layout that puts some other "version" first fails here rather than + committing a lockfile with the wrong entry changed. + """ + expected: Any = json.loads(text) + if not isinstance(expected, dict): + raise ValueError('package-lock.json: top-level value must be a JSON object') + packages = expected.get('packages') + root = packages.get('') if isinstance(packages, dict) else None + if not isinstance(root, dict) or 'version' not in expected or 'version' not in root: + raise ValueError('package-lock.json: missing version or packages[""].version') + expected['version'] = version + root['version'] = version + + def substitute(segment: str) -> tuple[str, int]: + return _PACKAGE_VERSION_RE.subn( + lambda m: f'{m.group("lead")}{version}{m.group("tail")}', segment, count=1 + ) + + root_match = _LOCK_ROOT_PACKAGE_RE.search(text) + if root_match is None: + raise ValueError('package-lock.json: could not locate packages[""]') + head, head_count = substitute(text[: root_match.end()]) + tail, tail_count = substitute(text[root_match.end() :]) + rendered = head + tail + if head_count != 1 or tail_count != 1 or json.loads(rendered) != expected: + raise ValueError( + 'package-lock.json: could not set the top-level and packages[""] ' + 'versions in place' + ) + return rendered + + def load_versions(path: Path = VERSIONS_PATH) -> tuple[VersionsManifest, dict[str, Any]]: return parse_versions(path.read_text(encoding='utf-8'), str(path)) diff --git a/scripts/release-intent/test_lib.py b/scripts/release-intent/test_lib.py index 664b7757..25ca925f 100644 --- a/scripts/release-intent/test_lib.py +++ b/scripts/release-intent/test_lib.py @@ -17,6 +17,7 @@ ALLOW_OWNED_FILES_MARKER, INTENT_END, INTENT_START, + PACKAGE_LOCK_PATH, VERSIONS_PATH, VersionsManifest, apply_bumps, @@ -28,6 +29,7 @@ prepend_changelog, read_release_version, render_package_json, + render_package_lock, render_versions_json, ) @@ -92,6 +94,69 @@ def test_render_package_json_requires_a_version_field(self) -> None: with self.assertRaises(ValueError): render_package_json('{\n "name": "pair"\n}\n', '0.1.2') + LOCK = ( + '{\n' + ' "name": "pair",\n' + ' "version": "0.1.1",\n' + ' "lockfileVersion": 3,\n' + ' "requires": true,\n' + ' "packages": {\n' + ' "": {\n' + ' "name": "pair",\n' + ' "version": "0.1.1"\n' + ' },\n' + ' "node_modules/dep": {\n' + ' "version": "0.1.1"\n' + ' }\n' + ' }\n' + '}\n' + ) + + @staticmethod + def _changed_lines(before: str, after: str) -> list[tuple[str, str]]: + return [ + pair for pair in zip(before.splitlines(), after.splitlines()) if pair[0] != pair[1] + ] + + def test_render_package_lock_sets_both_root_versions(self) -> None: + rendered = render_package_lock(self.LOCK, '0.1.2') + parsed = json.loads(rendered) + self.assertEqual(parsed['version'], '0.1.2') + self.assertEqual(parsed['packages']['']['version'], '0.1.2') + # A dependency that happens to share the old version keeps it. + self.assertEqual(parsed['packages']['node_modules/dep']['version'], '0.1.1') + self.assertEqual(len(self._changed_lines(self.LOCK, rendered)), 2) + + def test_render_package_lock_resyncs_a_drifted_lockfile(self) -> None: + drifted = self.LOCK.replace('"version": "0.1.1",', '"version": "0.1.0",', 1) + parsed = json.loads(render_package_lock(drifted, '0.1.2')) + self.assertEqual(parsed['version'], '0.1.2') + self.assertEqual(parsed['packages']['']['version'], '0.1.2') + + def test_render_package_lock_requires_both_root_versions(self) -> None: + without_root = json.dumps({'name': 'pair', 'version': '0.1.1', 'packages': {}}) + with self.assertRaises(ValueError): + render_package_lock(without_root, '0.1.2') + + def test_render_package_lock_rejects_an_unexpected_layout(self) -> None: + """A top-level version written after packages cannot be set in place.""" + reordered = json.dumps( + { + 'name': 'pair', + 'packages': {'': {'name': 'pair', 'version': '0.1.1'}}, + 'version': '0.1.1', + }, + indent=4, + ) + with self.assertRaises(ValueError): + render_package_lock(reordered, '0.1.2') + + def test_render_repo_package_lock(self) -> None: + text = PACKAGE_LOCK_PATH.read_text(encoding='utf-8') + rendered = render_package_lock(text, '9.9.9') + self.assertEqual(len(self._changed_lines(text, rendered)), 2) + self.assertTrue(rendered.endswith('\n')) + def test_missing_fences_rejected(self) -> None: with self.assertRaises(ValueError): parse_release_intent('## Summary\nno fences here', KEYS) diff --git a/services/VERSIONING.md b/services/VERSIONING.md index 04c74257..61a9b720 100644 --- a/services/VERSIONING.md +++ b/services/VERSIONING.md @@ -31,7 +31,8 @@ this?", not "how compatible is it?". A MINOR or MAJOR release version is a deliberate manual edit at cut time. The automation only ever moves it one PATCH forward, so it cannot promote a release -on its own. +on its own. Make the edit with `npm version --no-git-tag-version` in +`desktop/`, which also moves the two copies in `desktop/package-lock.json`. The release version and `services` are **not** held equal, and no attempt is made to align them. They version different artifacts. From f434e98a043c67281bc4af25d7b2bde10553d56a Mon Sep 17 00:00:00 2001 From: Terve Date: Wed, 23 Sep 2026 16:56:51 -0400 Subject: [PATCH 2/2] fix(desktop): resync package-lock.json to release 0.1.5 Automated release bumps left the lockfile's root version at 0.1.1 while package.json reached 0.1.5. Set both lockfile copies to match. Signed-off-by: Terve --- desktop/package-lock.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/desktop/package-lock.json b/desktop/package-lock.json index d6191db3..84f69905 100644 --- a/desktop/package-lock.json +++ b/desktop/package-lock.json @@ -1,12 +1,12 @@ { "name": "nvpair", - "version": "0.1.1", + "version": "0.1.5", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "nvpair", - "version": "0.1.1", + "version": "0.1.5", "hasInstallScript": true, "license": "Apache-2.0", "dependencies": {