ci: bump wallet-api versions in ledger-live's pnpm catalog - #604
Open
Justkant wants to merge 3 commits into
Open
ci: bump wallet-api versions in ledger-live's pnpm catalog#604Justkant wants to merge 3 commits into
Justkant wants to merge 3 commits into
Conversation
ledger-live centralised the @LedgerHQ/wallet-api-* versions in its pnpm catalog (LedgerHQ/ledger-live#20521): consumers now declare "catalog:" and the range lives once in pnpm-workspace.yaml. The sed loop matched the literal "catalog:" specifier and replaced it with ^<version>, silently reverting the indirection on every release while the five catalog entries went stale. pnpm install still succeeded, so nothing failed — the PR just quietly undid the change each time. Replace it with a node script that: - bumps the entry in the top-level catalog: block of pnpm-workspace.yaml, line by line so comments, key order and quoting survive; keys under packages:/overrides:/catalogs: are out of scope and exact-matched, so wallet-api-client does not hit wallet-api-client-react - falls back to the package.json rewrite for non-cataloged consumers, skipping node_modules and never clobbering workspace:/catalog: specifiers - fails the job when a package.json asks for "catalog:" but no catalog entry was bumped, instead of leaving that consumer pinned to the old version - skips published packages ledger-live does not consume, without counting them as bumped The bumped output, BUMP_SUMMARY and the 5x lockfile retry are unchanged.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
CremaFR
approved these changes
Aug 6, 2026
There was a problem hiding this comment.
Pull request overview
Updates the release automation that opens/updates the downstream ledger-live dependency bump PRs, adapting it to pnpm catalog-managed @ledgerhq/wallet-api-* versions so releases don’t accidentally de-catalog consumers.
Changes:
- Replaces the
sed/greploop with a Node script that bumps the top-levelcatalog:entries inpnpm-workspace.yaml(preserving comments/order/quoting) and only falls back topackage.jsonedits when no catalog entry exists. - Adds guardrails: skips
node_modules, never rewritesworkspace:/link:/file:/portal:/catalog:specifiers, and fails loudly if"catalog:"is referenced but no matching catalog entry exists. - Improves bump reporting by emitting a TSV summary (
bumped.tsv) that records where each bump landed and uses it to populateBUMP_SUMMARY.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The catalog was bumped before the references were resolved, so a leftover entry alone was enough to rewrite pnpm-workspace.yaml, count as bumped and open a PR for a version nothing resolves. Resolve the references first and gate the catalog on the package being consumed at all — by "catalog:" or by a literal range — so a stale entry is reported and left alone, while an entry backing literal package.json refs still gets refreshed. Alongside that: - support named catalogs: catalogs: blocks are now indexed and catalog:<name> (plus the catalog:default alias) resolves to the right one, instead of failing the release with "no entry in the top-level 'catalog:' block" - inherit the range operator from the entry being replaced. ledger-live pins its catalog exactly, and a ^ there would let an unrelated install drift onto a wallet-api version this release never validated - keep a separator after the colon: an entry with an empty value produced "key":^1.0.0, which pnpm's YAML parsers reject or read as a plain string - set an exit code instead of process.exit(1), which can drop the orphan diagnostic still queued on stderr — that message is the whole point of failing fast - strip the carriage returns before matching: "." never matches \r, so on a CRLF checkout every entry was invisible and the job failed claiming the catalog entry was missing
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (3)
.github/workflows/release.yml:252
- The package.json rewrite path increments
refsand records the file asrewritteneven whenrange(current, version)produces the same string (sonext === text). This makes the bump summary/location accounting inaccurate and can contribute to reporting a bump when there was no effective change.
refs++;
return `${head}${range(current, version)}${tail}`;
});
if (!refs) continue;
rewritten.push(file);
.github/workflows/release.yml:317
bumped.tsvbeing non-empty currently gates the rest of the step, but the script can still emit entries even when no files actually changed (e.g., reruns after ledger-live is already updated). Add an explicitgit diff --quietguard so the workflow exits withbumped=falsewhen there is no effective diff, avoiding no-op lockfile refreshes and PR attempts.
if [[ ! -s "$BUMPED_FILE" ]]; then
echo "No published package is consumed by ledger-live — nothing to bump."
echo "bumped=false" >> "$GITHUB_OUTPUT"
exit 0
fi
.github/workflows/release.yml:203
bumpCatalog()rewrites the catalog line and setsws.dirty = trueeven when the computed value is identical to the existing value. On workflow re-runs (or if ledger-live is already at the released version), this can cause unnecessary file writes/CRLF churn and can contribute to no-op PR creation.
This issue also appears in the following locations of the same file:
- line 248
- line 313
ws.lines[i] = `${indent}${key}${sep}${quote}${range(unquote(value), version)}${quote}${trailer}`;
}
ws.dirty = true;
return true;
An entry already on the published version rewrites to the very same line, yet still counted as a bump: re-running a release whose ledger-live PR had merged handed create-pull-request an empty diff, after regenerating the lockfile for nothing. Split "entry found" from "entry moved" — the first still drives the orphan check, the second the summary — and do the same for package.json refs, where `rewritten` had come to mean "references it", not "was rewritten". Also drop the specifier allow-list for a colon test. A semver range cannot hold one (prerelease and build identifiers are [0-9A-Za-z-]), so it covers `npm:`/`jsr:` aliases and git URLs too, which the list clobbered into a plain range.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (2)
.github/workflows/release.yml:259
- The fallback rewrite treats “contains a colon” as the only indicator of an indirection. That means non-semver specifiers without
:(e.g. git shorthands likeuser/repo#..., dist-tags likelatest, etc.) would be rewritten to a version range, which can corrupt the dependency spec.
Safer approach: only rewrite values that look like semver pins/ranges (optionally prefixed by a known operator), and leave everything else untouched.
if (current.includes(":")) return match;
refs++;
return `${head}${range(current, version)}${tail}`;
.github/workflows/release.yml:125
range()only preserves~and^. If a dependency (catalog or package.json) uses a comparator range like ">=1.2.3" / "<2.0.0", the code currently falls back to^and changes the meaning of the constraint.
Consider preserving common comparator prefixes when present (and only defaulting to ^ when the existing spec doesn’t look semver-ish).
This issue also appears on line 257 of the same file.
const range = (current, version) => {
const prefix = /^([~^]?)\d/.exec(current.trim());
return `${prefix ? prefix[1] : "^"}${version}`;
};
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.
Problem
ledger-live has centralised the
@ledgerhq/wallet-api-*versions in its pnpm catalog (LedgerHQ/ledger-live#20521, currently open). Consumers now declare:and the range lives once in
pnpm-workspace.yaml.The
update-ledger-livejob bumps versions with a sed loop over every consumingpackage.json. Against a cataloged consumer that sed matches the literal"catalog:"specifier and rewrites it to^<version>, while never touchingpnpm-workspace.yaml. Every release would silently undo the catalog indirection and leave five stale catalog entries behind.pnpm installstill succeeds, so nothing fails — the PR just quietly reverts the change each time.Change
The step now runs a node script (node is already set up in the job; the script is written to
$RUNNER_TEMPso it never lands in the ledger-live PR) that:pnpm-workspace.yaml, editing line by line so comments, key order and quoting style survive. Only the top-levelcatalog:block is in scope — same-named keys underpackages:,overrides:orcatalogs:are left alone. Keys are compared exactly after unquoting, sowallet-api-clientcannot hitwallet-api-client-react.package.jsonrewrite for published packages with no catalog entry.node_modulesis skipped andworkspace:/catalog:/link:/file:specifiers are never rewritten.package.jsonasks for"catalog:"but no catalog entry was found to bump — otherwise that consumer stays silently pinned to the old version. Nothing is written when this fires.Unchanged: the
bumpedstep output (false⇒ no PR),BUMP_SUMMARY(now also naming where each bump landed), and the 5× retry aroundpnpm install --lockfile-only.Verification
The step body was extracted from this workflow and executed under bash with a stubbed
pnpm— 43 assertions across 7 fixtures, all passing: cataloged package (workspace bumped,package.jsonuntouched), non-cataloged fallback, orphancatalog:ref (job fails, nothing written), no catalog block (legacy behaviour), prefix collisions,catalogs:/overrides:isolation,node_modulesskipped, unconsumed package skipped, and bothbumped=falseexits.Also run against the real ledger-live files:
pnpm-workspace.yaml, comments and untouched entries intact, everypackage.jsonbyte-identical,wallet-api-tools(not consumed) skipped:develop(no wallet-api catalog entries yet) — fallback path fires,pnpm-workspace.yamluntouched,workspace:^module deps left alone.So this is correct both before and after #20521 merges.