Skip to content

fix: detect shrinkwrap siblings by workspace, not @bitgo/ prefix - #9422

Merged
zahin-mohammad merged 1 commit into
masterfrom
WCN-1818-phase-1-shrinkwrap-fix
Aug 5, 2026
Merged

fix: detect shrinkwrap siblings by workspace, not @bitgo/ prefix#9422
zahin-mohammad merged 1 commit into
masterfrom
WCN-1818-phase-1-shrinkwrap-fix

Conversation

@lokesh-bitgo

Copy link
Copy Markdown
Contributor

Ticket: WCI-1818

Summary

Phase 1 of WCN-1818: fixes two latent bugs in scripts/generate-bitgo-shrinkwrap.ts (sibling detection breaks on alpha/beta, the resolved-entry safety check was weaker than it should be), adds a bounded retry for ordinary registry propagation lag, and adds a new standalone script (set-umbrella-publishable.ts) that phase 2 will use to implement the actual two-phase publish split. This PR does not change any workflow behavior and does not fix the underlying race condition on its own — see "What this PR does not fix" below.

Background

PR #9404 (WCI-1200) fixed npm install bitgo shipping with zero @bitgo/* siblings by making generate-bitgo-shrinkwrap.ts actually resolve siblings into the shrinkwrap instead of stripping them. That PR assumed lerna publish publishes packages in dependency-topological order, so siblings would already be live on the registry by the time bitgo is packed.

That assumption is false. Lerna runs every package's lifecycle hooks (including bitgo's prepack) before uploading any of them. The next prod release after #9404 merged failed exactly this way:

npm error notarget No matching version found for @bitgo/abstract-lightning@^8.2.3
lerna ERR! lifecycle "prepack" errored in "bitgo", exiting 1

Zero packages published in that run. WCN-1818 designed the real fix: split publishing into two passes — siblings first (with bitgo temporarily held out via a private flag), bitgo second, once siblings are actually live. This PR is the first of two phases implementing that design.

What changed

1. scripts/generate-bitgo-shrinkwrap.ts

  • Sibling detection: was Object.keys(bitgoPackageJson.dependencies).filter(name => name.startsWith('@bitgo/')). Now reads every modules/*/package.json name from disk (getWorkspacePackageNames()) and intersects that with bitgo's dependencies. On the real @bitgo/* scope this produces the same 89 siblings as before (no behavior change). On alpha/beta, where prepare-release.ts rescopes everything to @bitgo-beta/*, the old check matched zero siblings — meaning the WCI-1200 safety check was silently verifying nothing on those channels. The new check is scope-agnostic because it reads the actual current names instead of assuming a fixed prefix.
  • Stronger resolved-entry guard: was checking only that a packages["node_modules/<name>"] key existed. Now requires version, resolved, and integrity to each be present as strings. A key that exists but is missing install-relevant fields now correctly fails the check instead of passing it.
  • Bounded retry on resolution (npmInstallWithRetry): wraps npm install --package-lock-only --ignore-scripts with up to 5 attempts / 5s backoff, but only when the failure looks like ETARGET/E404 (i.e., "not found yet," not "will never exist"). This exists for phase 2: once siblings publish in their own pass immediately before bitgo's pass, there's a real possibility bitgo's resolution runs before the registry has fully propagated a just-published sibling. Any other error type is rethrown immediately with zero retries — confirmed in local testing.
  • Updated the file's header comment to state the real ordering constraint instead of the disproven "topological order" claim, and to describe how phase 2's two-pass split is expected to satisfy it.

2. scripts/set-umbrella-publishable.ts (new)

Toggles private on modules/bitgo/package.json:

npx tsx scripts/set-umbrella-publishable.ts false   # mark private — lerna skips packing it
npx tsx scripts/set-umbrella-publishable.ts true    # restore — lerna packs it normally

This is the mechanism phase 2 will use to hold bitgo out of lerna's pack step during the siblings-only pass (lerna's filterPrivatePkgUpdates filters private packages before packing, confirmed against lerna 9.0.0's source in node_modules/lerna/dist/commands/publish/index.js). Deliberately a scripted temporary toggle rather than lerna's built-in --include-private (which would require bitgo to be permanently private: true in the committed manifest) — three separate pipeline checks (pre-publish existence check, recovery verification, beta verification/auto-retry) enumerate non-private packages, and a permanently-private bitgo would silently stop being covered by all three. The script itself holds no state and does not decide when to run — the workflow (phase 2) is responsible for calling the restore (true) under always() so a failed pass 1 can't leave the manifest stuck flipped.

What this fixes

  • The alpha/beta blind spot where the sibling-detection safety check verified nothing (silently), regardless of whether the underlying shrinkwrap was actually correct.
  • A weaker-than-necessary resolved-entry check that could pass on a partially-specified entry.
  • Provides the primitive (set-umbrella-publishable.ts) phase 2 needs, and hardens the resolution step against a known-expected side effect of phase 2's design (propagation lag) ahead of time.

What this PR does not fix

The original race condition — bitgo's prepack trying to resolve siblings before they're published, because both currently happen inside one combined lerna publish — is unchanged by this PR. Neither .github/workflows/publish.yml nor .github/workflows/npmjs-release.yml are touched here. If a prod release ran today with only this PR merged, it would still fail the same way as before (this is expected — see "Upcoming: Phase 2" below).

How I tested this locally

All of the following were actually run, not just reasoned about. Working tree was clean before and after (temporary edits made during testing were reverted/diffed to confirm exact restoration).

1. Type-check and lint the two files:

npx tsc --noEmit --skipLibCheck --target es2020 --module commonjs --esModuleInterop --resolveJsonModule \
  scripts/generate-bitgo-shrinkwrap.ts scripts/set-umbrella-publishable.ts
npx eslint scripts/generate-bitgo-shrinkwrap.ts scripts/set-umbrella-publishable.ts

Result: 0 errors on both. Only pre-existing no-sync/no-console style warnings, same kind/count pattern as the rest of scripts/.

2. set-umbrella-publishable.ts — both directions, plus invalid input:

npx tsx scripts/set-umbrella-publishable.ts false   # -> "private": true appears
npx tsx scripts/set-umbrella-publishable.ts true    # -> "private" key removed
git diff --stat modules/bitgo/package.json          # -> empty; exact round-trip, no corruption
npx tsx scripts/set-umbrella-publishable.ts maybe   # -> throws: Expected a single argument "true" or "false", got: "maybe"

3. Workspace-based sibling detection against the real repo (standalone script comparing old vs. new detection logic against the actual modules/ directory):

npx tsx /tmp/test-workspace-detection.ts

Result: both old and new logic found the same 89 siblings on the real @bitgo/* scope — confirms no regression.

4. Simulated beta-rescoped state — reproducing the actual bug (small script simulating prepare-release.ts's rescoping of a few sample names/deps to @bitgo-beta/* in memory, no files touched):

npx tsx /tmp/test-beta-rescope.ts

Result: old prefix-based logic found 0/3 siblings on the simulated beta scope (the bug). New workspace-membership logic found 3/3 correctly.

5. Retry/backoff logic, mocked runner, three scenarios:

npx tsx /tmp/test-retry-logic.ts
  • Fails twice with ETARGET-like text, then succeeds -> recovers on attempt 3.
  • Fails with ETARGET-like text every time -> exhausts all 5 attempts, fails cleanly (no infinite loop).
  • Fails with an unrelated error -> fails on attempt 1, zero wasted retries.

6. Real end-to-end run against the live npm registry (safe: resolution/read-only against the registry, only local temp-dir + modules/bitgo/npm-shrinkwrap.json writes, no publish):

BITGO_GENERATE_SHRINKWRAP=true npx tsx scripts/generate-bitgo-shrinkwrap.ts
  • First attempt failed on an unrelated local issue: Node 20.18.3 vs. the .nvmrc-required 24.13.0, surfaced as EBADENGINE from a transitive dependency. Notably, this triggered zero retry attempts — real-world confirmation that the retry logic correctly classifies non-transient errors and doesn't waste time on them.
  • After nvm install 24.13.0 && nvm use 24.13.0 and re-running: exit code 0. Real npm-shrinkwrap.json generated. Spot-checked several sibling entries directly in the output — all had real version/resolved/integrity:
    @bitgo/sdk-core       -> version: 38.5.1  | resolved: true | integrity: true
    @bitgo/sdk-api        -> version: 2.2.2   | resolved: true | integrity: true
    @bitgo/sdk-coin-ada   -> version: 5.0.6   | resolved: true | integrity: true
    @bitgo/statics        -> version: 59.2.0  | resolved: true | integrity: true
    
    150 total node_modules/@bitgo/* entries (89 direct siblings + their own transitive @bitgo/* deps). Deleted the generated file afterward — it's a build artifact, not meant to be committed here.

7. Fault injection — proving the strengthened guard actually catches breakage:

cp modules/bitgo/package.json /tmp/bitgo-package.json.bak
# edited package.json: set the @bitgo/sdk-core dependency to the nonexistent "999.999.999"
BITGO_GENERATE_SHRINKWRAP=true npx tsx scripts/generate-bitgo-shrinkwrap.ts

Result: exit code 1. Because ETARGET looks like it could be propagation lag, retried 4 times (~20s) before correctly giving up and reporting the real error:

npm error notarget No matching version found for @bitgo/sdk-core@999.999.999.

Confirms the retry logic has a bounded cost even in the worst case (a version that will never resolve) and never masks a genuine failure — it just adds a small, fixed delay before reporting accurately.

cp /tmp/bitgo-package.json.bak modules/bitgo/package.json
diff /tmp/bitgo-package.json.bak modules/bitgo/package.json   # confirmed exact match
rm -f /tmp/bitgo-package.json.bak /tmp/test-*.ts /tmp/shrinkwrap-*.log

How to verify this after merging

Since this PR doesn't change when/how publishing happens, there's no new end-to-end release behavior to verify yet. What's verifiable post-merge:

  1. Confirm it's still inert on beta. The next normal publish.yml dispatch should still show:
    BITGO_GENERATE_SHRINKWRAP not set to "true" — skipping npm-shrinkwrap.json generation.
    
    confirming this PR didn't change the no-op path.
  2. If a prod release runs before phase 2 lands, it will still fail the original way (siblings not live yet) — that's expected, not a regression. What to check in that failure's logs specifically:
    • The "Resolving N workspace siblings" line should list all 89 sibling names (proves detection still works correctly in the real prepack context, not just locally).
    • If it fails with ETARGET, the log should show a few "retrying in 5000ms" lines before the final failure (proves the retry wrapper is wired in and behaving as tested).
  3. Full confirmation of the original bug being fixed requires phase 2 (the two-pass workflow split) to also land, then a real alpha dispatch or prod release actually succeeding with _hasShrinkwrap: true and a clean-directory npm install bitgo working. That's out of scope for this PR.

Upcoming: Phase 2

Not included in this PR. Tracked in WCN-1818, planned as a follow-up PR:

  1. .github/workflows/publish.yml (alpha/beta): split the single Lerna Publish step into:
    • set-umbrella-publishable false -> lerna publish from-package (siblings only, bitgo skipped)
    • set-umbrella-publishable true (if: always(), so a failed pass 1 can't leave bitgo stuck private)
    • lerna publish from-package with BITGO_GENERATE_SHRINKWRAP=true (bitgo only, siblings now live)
  2. .github/workflows/npmjs-release.yml (prod): same split, applied to both the normal publish path and the recovery-mode path, on top of the existing (unchanged) lerna version step.
  3. Validation plan for phase 2:
    • Dispatch an alpha publish from the phase 2 branch first — cheapest way to exercise the real code path against the real registry (BITGO_GENERATE_SHRINKWRAP would need to be set on that path for the test, unlike today's beta).
    • Confirm the published alpha bitgo has _hasShrinkwrap: true with fully-resolved entries for every sibling.
    • npm install @bitgo-beta/bitgo@alpha in a clean, empty directory — confirm siblings install and require() succeeds.
    • Only after that passes, attempt a real prod release — in recovery mode, not a normal run, since rel/latest already has an orphaned 52.4.2 version bump/tags from the original failed release that a normal run would skip past.
  4. Known gap to track separately: scripts/verify-release.ts's recovery-retry path doesn't set BITGO_GENERATE_SHRINKWRAP, so a beta package republished via that specific path would ship without a shrinkwrap. Not exploitable today (beta never ships one anyway, and prod doesn't use that script), but worth its own follow-up ticket rather than leaving it undocumented indefinitely.
  5. Branch hygiene: rel/latest and master have diverged (one unpublished version-bump commit on rel/latest, three unrelated commits on master it doesn't have) and need reconciling before any real release attempt — tracked as a separate, pre-release-time task, not blocking this PR or phase 2's code review.

Ticket: WCN-1818

@linear-code

linear-code Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

WCN-1818

@lokesh-bitgo lokesh-bitgo self-assigned this Aug 5, 2026
Comment thread scripts/generate-bitgo-shrinkwrap.ts
@zahin-mohammad
zahin-mohammad merged commit 835483e into master Aug 5, 2026
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants