Fix/release intent lockfile gh

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
 ```

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,28 +49,32 @@
     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
 
     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,

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))
 

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)

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 <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.