Skip to content

Commit 20fc3db

Browse files
Fix/release intent lockfile gh (#119)
## Description The release-intent bot bumps the release version in `desktop/package.json` but never updated `desktop/package-lock.json`, so after each automated release the lockfile's root version (top-level `version` and `packages[""].version`) fell behind. On `develop` it reads 0.1.1 while `package.json` reads 0.1.5, and tooling that checks the two against each other rejects the tree. The bot now writes both lockfile copies in the same single commit as the other version files, and this PR resyncs the lockfile to 0.1.5. ## Release intent <!-- Keep the fences exactly as they are; CI parses between them. --> <!-- Set each bump to none, patch, minor, or major. Leave the key names. --> <!-- `services` must be >= the highest component bump. --> <!-- Set the title and body to n/a when no bump is a release. --> <!-- The release version users see is NOT declared here: it patch-bumps by itself whenever any bump above is a release. --> <!-- Keep the changelog body to prose and bullets. A line starting with "### " ends the section and silently truncates the rest. --> <!-- pair-release-intent:v1 --> ### Changelog title n/a ### Changelog body n/a ### Bumps - services: none - nvpair-cluster-manager: none - nvpair-engine-manager: none - nvpair-errors: none - nvpair-job-scheduler: none - nvpair-manual-nodes: none - nvpair-node-info: none - nvpair-node-scanner: none - nvpair-node-settings: none - nvpair-proxy: none - nvpair-tui: none - nvpair-ui-broker: none - nvpair-workload-manager: none <!-- /pair-release-intent:v1 --> ## Scope - `scripts/release-intent/lib.py`: new `render_package_lock()` replaces the two root versions in place, like `render_package_json()`, and refuses to render if the result differs from the parsed edit in any other field. - `scripts/release-intent/apply_pr.py`: reads, renders, and commits the lockfile alongside the other three files, in both apply and `--dry-run`. `read_file_at()` now fails loudly when the contents API omits a body, which it does above 1 MB; the lockfile is about 508 KB today. - `desktop/package-lock.json`: two-line resync to 0.1.5. - Docs: the release-intent README lists the fourth file; `services/VERSIONING.md` points manual minor/major bumps at `npm version <version> --no-git-tag-version`. - No bumps: no service binary changes, so the bot applies nothing after merge and `develop` stays consistent at 0.1.5. ## Validation - `python3 scripts/release-intent/test_lib.py`: 33 tests pass, including new ones for both root versions, a drifted lockfile, unexpected layouts, and the repository's real lockfile (exactly two lines change). - `apply_pr.py --dry-run` with a sample patch release on a scratch copy: `package.json` and both lockfile versions moved 0.1.5 → 0.1.6 and the lockfile still parsed. - `validate_pr.py --skip-owned-files-check` against this description passes. ## Risk Only the release automation and lockfile metadata change. If npm ever writes a lockfile layout the in-place edit does not recognise, apply fails instead of committing, and the versions stop bumping until the script is updated. ## Checklist - [x] I have read the [Contributing Guidelines](https://github.com/NVIDIA/Personal-AI-Router/blob/main/CONTRIBUTING.md). - [x] Every commit is signed off (`git commit -s`), certifying the [Developer Certificate of Origin](https://developercertificate.org/). - [x] New or existing tests cover the change. - [x] Relevant documentation is updated. - [x] I checked the diff, changed filenames, and commit messages for credentials, private data, internal URLs, internal issue identifiers, and generated artifacts. - [x] I recorded the validation commands and results above. - [x] I declared version bumps in the release-intent block above. `services/versions.json` is written by automation — do not edit it by hand. --------- Signed-off-by: Terve <ntervalon@nvidia.com>
1 parent e95f87d commit 20fc3db

6 files changed

Lines changed: 138 additions & 15 deletions

File tree

‎desktop/package-lock.json‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎scripts/release-intent/README.md‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,11 @@ an ordering accident nobody did wrong.
4949

5050
## What apply writes
5151

52-
Three files, in **one** commit built through the git data API:
52+
Four files, in **one** commit built through the git data API:
5353

5454
- `desktop/package.json` — the release version, one PATCH forward
55+
- `desktop/package-lock.json` — the same version, at the top level and under
56+
`packages[""]`, so the lockfile never names a different release
5557
- `services/versions.json` — the declared `services` and component bumps
5658
- `CHANGELOG.md` — a new section titled with the new release version, citing
5759
the pull request number
@@ -137,5 +139,5 @@ python3 scripts/release-intent/apply_pr.py \
137139
`--dry-run` **writes the working tree** despite the name. Restore afterwards:
138140

139141
```bash
140-
git restore desktop/package.json services/versions.json CHANGELOG.md
142+
git restore desktop/package.json desktop/package-lock.json services/versions.json CHANGELOG.md
141143
```

‎scripts/release-intent/apply_pr.py‎

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,12 @@
33
# SPDX-License-Identifier: Apache-2.0
44
"""Apply a merged pull request's release-intent block to the version files.
55
6-
Writes three files in ONE commit via the git data API: desktop/package.json
7-
(the release version, patch-bumped), services/versions.json (declared services
8-
and component bumps), and CHANGELOG.md (a new section).
6+
Writes four files in ONE commit via the git data API: desktop/package.json
7+
and desktop/package-lock.json (the release version, patch-bumped),
8+
services/versions.json (declared services and component bumps), and
9+
CHANGELOG.md (a new section).
910
10-
The contents API would be one commit per file, which for three files means a
11+
The contents API would be one commit per file, which for four files means a
1112
release landing in pieces and a window where the changelog names a version that
1213
package.json does not yet carry. Building a tree and moving the ref once avoids
1314
that, and the ref update doubles as the concurrency check: a non-fast-forward
@@ -35,6 +36,7 @@
3536
BOT_COMMIT_PREFIX,
3637
CHANGELOG_PATH,
3738
PACKAGE_JSON_PATH,
39+
PACKAGE_LOCK_PATH,
3840
REPO_ROOT,
3941
VERSIONS_PATH,
4042
apply_bumps,
@@ -47,28 +49,32 @@
4749
prepend_changelog,
4850
read_release_version,
4951
render_package_json,
52+
render_package_lock,
5053
render_versions_json,
5154
)
5255

5356
VERSIONS_REPO_PATH = str(VERSIONS_PATH.relative_to(REPO_ROOT))
5457
CHANGELOG_REPO_PATH = str(CHANGELOG_PATH.relative_to(REPO_ROOT))
5558
PACKAGE_REPO_PATH = str(PACKAGE_JSON_PATH.relative_to(REPO_ROOT))
59+
PACKAGE_LOCK_REPO_PATH = str(PACKAGE_LOCK_PATH.relative_to(REPO_ROOT))
5660

5761
BLOB_MODE = '100644'
5862

5963

6064
@dataclass(frozen=True)
6165
class ReleaseUpdate:
62-
"""The three file bodies a release intent produces, and what to call it."""
66+
"""The four file bodies a release intent produces, and what to call it."""
6367

6468
package: str
69+
package_lock: str
6570
versions: str
6671
changelog: str
6772
release: str
6873

6974
def as_paths(self) -> dict[str, str]:
7075
return {
7176
PACKAGE_REPO_PATH: self.package,
77+
PACKAGE_LOCK_REPO_PATH: self.package_lock,
7278
VERSIONS_REPO_PATH: self.versions,
7379
CHANGELOG_REPO_PATH: self.changelog,
7480
}
@@ -156,12 +162,13 @@ def fetch_merged_pr(repo: str, sha: str, token: str) -> tuple[str, str] | None:
156162

157163
def compute_release_update(
158164
package_text: str,
165+
package_lock_text: str,
159166
versions_text: str,
160167
changelog_text: str,
161168
description: str,
162169
pr_ref: str,
163170
) -> ReleaseUpdate | None:
164-
"""Apply the intent to three file bodies. None when nothing changes."""
171+
"""Apply the intent to four file bodies. None when nothing changes."""
165172
versions, raw = parse_versions(versions_text, VERSIONS_REPO_PATH)
166173
intent = parse_release_intent(
167174
description, versions.bump_keys, key_policy='reject-unknown'
@@ -187,6 +194,7 @@ def compute_release_update(
187194
)
188195
return ReleaseUpdate(
189196
package=render_package_json(package_text, release_after),
197+
package_lock=render_package_lock(package_lock_text, release_after),
190198
versions=render_versions_json(updated, raw),
191199
changelog=prepend_changelog(changelog_text, entry),
192200
release=release_after,
@@ -202,7 +210,8 @@ def read_file_at(repo: str, token: str, path: str, ref: str) -> str:
202210
if not isinstance(payload, dict):
203211
raise SystemExit(f'Unexpected payload reading {path} at {ref}')
204212
content = payload.get('content')
205-
if not isinstance(content, str):
213+
# Above 1 MB the contents API sends encoding "none" and an empty content.
214+
if payload.get('encoding') != 'base64' or not isinstance(content, str):
206215
raise SystemExit(f'Incomplete payload reading {path} at {ref}')
207216
return base64.b64decode(content).decode('utf-8')
208217

@@ -289,6 +298,7 @@ def apply_release(
289298
if dry_run:
290299
update = compute_release_update(
291300
PACKAGE_JSON_PATH.read_text(encoding='utf-8'),
301+
PACKAGE_LOCK_PATH.read_text(encoding='utf-8'),
292302
VERSIONS_PATH.read_text(encoding='utf-8'),
293303
CHANGELOG_PATH.read_text(encoding='utf-8'),
294304
description,
@@ -298,12 +308,14 @@ def apply_release(
298308
print('Dry-run: nothing to apply')
299309
return
300310
PACKAGE_JSON_PATH.write_text(update.package, encoding='utf-8')
311+
PACKAGE_LOCK_PATH.write_text(update.package_lock, encoding='utf-8')
301312
VERSIONS_PATH.write_text(update.versions, encoding='utf-8')
302313
CHANGELOG_PATH.write_text(update.changelog, encoding='utf-8')
303314
print(
304-
'Dry-run MODIFIED the working tree (package.json, versions.json, '
305-
'CHANGELOG.md). Restore with: git restore '
306-
f'{PACKAGE_REPO_PATH} {VERSIONS_REPO_PATH} {CHANGELOG_REPO_PATH}'
315+
'Dry-run MODIFIED the working tree (package.json, package-lock.json, '
316+
'versions.json, CHANGELOG.md). Restore with: git restore '
317+
f'{PACKAGE_REPO_PATH} {PACKAGE_LOCK_REPO_PATH} '
318+
f'{VERSIONS_REPO_PATH} {CHANGELOG_REPO_PATH}'
307319
)
308320
print(message)
309321
return
@@ -333,6 +345,7 @@ def apply_release(
333345

334346
update = compute_release_update(
335347
read_file_at(repo, token, PACKAGE_REPO_PATH, head),
348+
read_file_at(repo, token, PACKAGE_LOCK_REPO_PATH, head),
336349
read_file_at(repo, token, VERSIONS_REPO_PATH, head),
337350
read_file_at(repo, token, CHANGELOG_REPO_PATH, head),
338351
description,

‎scripts/release-intent/lib.py‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@
7777
VERSIONS_PATH = REPO_ROOT / 'services' / 'versions.json'
7878
CHANGELOG_PATH = REPO_ROOT / 'CHANGELOG.md'
7979
PACKAGE_JSON_PATH = REPO_ROOT / 'desktop' / 'package.json'
80+
PACKAGE_LOCK_PATH = REPO_ROOT / 'desktop' / 'package-lock.json'
8081

8182
VERSIONS_COMMENT = (
8283
'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:
157158
return replaced
158159

159160

161+
_LOCK_ROOT_PACKAGE_RE = re.compile(r'"packages"\s*:\s*\{\s*""\s*:\s*\{')
162+
163+
164+
def render_package_lock(text: str, version: str) -> str:
165+
"""Set both copies of the release version in desktop/package-lock.json.
166+
167+
npm records the root package's version at the top level and again under
168+
packages[""], and both must match package.json. Each is replaced in place,
169+
as in render_package_json, and the result is compared with a parsed edit so
170+
a layout that puts some other "version" first fails here rather than
171+
committing a lockfile with the wrong entry changed.
172+
"""
173+
expected: Any = json.loads(text)
174+
if not isinstance(expected, dict):
175+
raise ValueError('package-lock.json: top-level value must be a JSON object')
176+
packages = expected.get('packages')
177+
root = packages.get('') if isinstance(packages, dict) else None
178+
if not isinstance(root, dict) or 'version' not in expected or 'version' not in root:
179+
raise ValueError('package-lock.json: missing version or packages[""].version')
180+
expected['version'] = version
181+
root['version'] = version
182+
183+
def substitute(segment: str) -> tuple[str, int]:
184+
return _PACKAGE_VERSION_RE.subn(
185+
lambda m: f'{m.group("lead")}{version}{m.group("tail")}', segment, count=1
186+
)
187+
188+
root_match = _LOCK_ROOT_PACKAGE_RE.search(text)
189+
if root_match is None:
190+
raise ValueError('package-lock.json: could not locate packages[""]')
191+
head, head_count = substitute(text[: root_match.end()])
192+
tail, tail_count = substitute(text[root_match.end() :])
193+
rendered = head + tail
194+
if head_count != 1 or tail_count != 1 or json.loads(rendered) != expected:
195+
raise ValueError(
196+
'package-lock.json: could not set the top-level and packages[""] '
197+
'versions in place'
198+
)
199+
return rendered
200+
201+
160202
def load_versions(path: Path = VERSIONS_PATH) -> tuple[VersionsManifest, dict[str, Any]]:
161203
return parse_versions(path.read_text(encoding='utf-8'), str(path))
162204

‎scripts/release-intent/test_lib.py‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
ALLOW_OWNED_FILES_MARKER,
1818
INTENT_END,
1919
INTENT_START,
20+
PACKAGE_LOCK_PATH,
2021
VERSIONS_PATH,
2122
VersionsManifest,
2223
apply_bumps,
@@ -28,6 +29,7 @@
2829
prepend_changelog,
2930
read_release_version,
3031
render_package_json,
32+
render_package_lock,
3133
render_versions_json,
3234
)
3335

@@ -92,6 +94,69 @@ def test_render_package_json_requires_a_version_field(self) -> None:
9294
with self.assertRaises(ValueError):
9395
render_package_json('{\n "name": "pair"\n}\n', '0.1.2')
9496

97+
LOCK = (
98+
'{\n'
99+
' "name": "pair",\n'
100+
' "version": "0.1.1",\n'
101+
' "lockfileVersion": 3,\n'
102+
' "requires": true,\n'
103+
' "packages": {\n'
104+
' "": {\n'
105+
' "name": "pair",\n'
106+
' "version": "0.1.1"\n'
107+
' },\n'
108+
' "node_modules/dep": {\n'
109+
' "version": "0.1.1"\n'
110+
' }\n'
111+
' }\n'
112+
'}\n'
113+
)
114+
115+
@staticmethod
116+
def _changed_lines(before: str, after: str) -> list[tuple[str, str]]:
117+
return [
118+
pair for pair in zip(before.splitlines(), after.splitlines()) if pair[0] != pair[1]
119+
]
120+
121+
def test_render_package_lock_sets_both_root_versions(self) -> None:
122+
rendered = render_package_lock(self.LOCK, '0.1.2')
123+
parsed = json.loads(rendered)
124+
self.assertEqual(parsed['version'], '0.1.2')
125+
self.assertEqual(parsed['packages']['']['version'], '0.1.2')
126+
# A dependency that happens to share the old version keeps it.
127+
self.assertEqual(parsed['packages']['node_modules/dep']['version'], '0.1.1')
128+
self.assertEqual(len(self._changed_lines(self.LOCK, rendered)), 2)
129+
130+
def test_render_package_lock_resyncs_a_drifted_lockfile(self) -> None:
131+
drifted = self.LOCK.replace('"version": "0.1.1",', '"version": "0.1.0",', 1)
132+
parsed = json.loads(render_package_lock(drifted, '0.1.2'))
133+
self.assertEqual(parsed['version'], '0.1.2')
134+
self.assertEqual(parsed['packages']['']['version'], '0.1.2')
135+
136+
def test_render_package_lock_requires_both_root_versions(self) -> None:
137+
without_root = json.dumps({'name': 'pair', 'version': '0.1.1', 'packages': {}})
138+
with self.assertRaises(ValueError):
139+
render_package_lock(without_root, '0.1.2')
140+
141+
def test_render_package_lock_rejects_an_unexpected_layout(self) -> None:
142+
"""A top-level version written after packages cannot be set in place."""
143+
reordered = json.dumps(
144+
{
145+
'name': 'pair',
146+
'packages': {'': {'name': 'pair', 'version': '0.1.1'}},
147+
'version': '0.1.1',
148+
},
149+
indent=4,
150+
)
151+
with self.assertRaises(ValueError):
152+
render_package_lock(reordered, '0.1.2')
153+
154+
def test_render_repo_package_lock(self) -> None:
155+
text = PACKAGE_LOCK_PATH.read_text(encoding='utf-8')
156+
rendered = render_package_lock(text, '9.9.9')
157+
self.assertEqual(len(self._changed_lines(text, rendered)), 2)
158+
self.assertTrue(rendered.endswith('\n'))
159+
95160
def test_missing_fences_rejected(self) -> None:
96161
with self.assertRaises(ValueError):
97162
parse_release_intent('## Summary\nno fences here', KEYS)

‎services/VERSIONING.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@ this?", not "how compatible is it?".
3131

3232
A MINOR or MAJOR release version is a deliberate manual edit at cut time. The
3333
automation only ever moves it one PATCH forward, so it cannot promote a release
34-
on its own.
34+
on its own. Make the edit with `npm version <version> --no-git-tag-version` in
35+
`desktop/`, which also moves the two copies in `desktop/package-lock.json`.
3536

3637
The release version and `services` are **not** held equal, and no attempt is
3738
made to align them. They version different artifacts.

0 commit comments

Comments
 (0)