Skip to content

fix(npm): tolerate workspace:* deps in version bump and bun.lock patching#805

Merged
BYK merged 4 commits intomasterfrom
fix/npm-workspace-protocol
Apr 21, 2026
Merged

fix(npm): tolerate workspace:* deps in version bump and bun.lock patching#805
BYK merged 4 commits intomasterfrom
fix/npm-workspace-protocol

Conversation

@BYK
Copy link
Copy Markdown
Member

@BYK BYK commented Apr 21, 2026

Closes #804.

Context

Two bugs in NpmTarget.bumpVersion (and the publish path) when a monorepo uses "workspace:*" dependency specifiers:

Issue 1: npm version --workspaces false-negative failure

npm version <NEW> --no-git-tag-version --allow-same-version --workspaces --include-workspace-root successfully bumps the version field in every workspace package.json, then fails with EUNSUPPORTEDPROTOCOL when npm's URL validator encounters "workspace:*" deps. Craft saw the non-zero exit, fell back to per-package npm version, hit the same error in each subdir, and failed the whole release.

Upstream: npm/cli#8845 — known, unresolved. workspace: is documented as supported but npm 11.x's version command validator still rejects it.

Issue 2: stale bun.lock workspace versions

bun pm pack rewrites workspace:* specifiers to concrete versions at pack time, reading them from bun.lock, not from package.json. Neither npm version nor bun install updates those lockfile entries, so published tarballs shipped with stale dependencies pointing at old workspace versions that were never published. Install failed with ETARGET.

Changes

src/targets/npm.ts

  • Post-check on --workspaces failure: if npm version --workspaces exits non-zero, read every relevant package.json (root + every workspace package) and check version === newVersion. All match → warn (citing [DOCS] workspace: protocol documented but throws EUNSUPPORTEDPROTOCOL in npm 11.6.4 npm/cli#8845) and proceed. Mismatch → fall back to individual bumping. Private workspaces are verified too, since npm version --workspaces bumps them.
  • Post-check in the individual-bump path too: per-package npm version failures now check whether the file was bumped anyway. Match → warn and continue; mismatch → rethrow. This keeps genuine failures (bad version strings, perm errors) loud while no longer failing on workspace:* deps.
  • patchBunLock(rootDir, packages, newVersion): after successful bumps, find each workspace's path-relative key in bun.lock and rewrite the first "version": "..." line inside its block. The regex uses [^{}]*? (not [\s\S]*?) so it stops at either the workspace block's closing } or the opener of a nested object — this prevents corrupting sibling workspaces when a workspace lacks a version field, and leaves nested dependency version pins untouched regardless of field ordering. Writes via safeFs.writeFileSync for dry-run safety. No-op if bun.lock is absent. Handles Windows path separators by normalizing to /.

Dry-run handling: the failure post-check short-circuits under isDryRun() because spawnProcess is already a no-op in dry-run (files wouldn't have been written by npm), and safeFs.writeFileSync natively respects dry-run.

src/__tests__/versionBump.test.ts — 10 new tests under NpmTarget.bumpVersion > workspace:* handling:

  • Post-check tolerates [DOCS] workspace: protocol documented but throws EUNSUPPORTEDPROTOCOL in npm 11.6.4 npm/cli#8845-style write-then-fail on --workspaces
  • Falls back to individual bumping when files actually weren't bumped
  • Per-package post-check tolerates the same failure mode on fallback
  • Rethrows when per-package bump fails AND file wasn't bumped (genuine error)
  • Patches bun.lock workspace versions after a successful bump
  • No-op when bun.lock is absent
  • Non-greedy regex leaves nested dependency version pins untouched
  • Regex stays within workspace block boundaries (regression test for silent-corruption bug caught in self-review)
  • Path prefix disambiguation (packages/foo vs packages/foo-bar)
  • Post-check verifies private workspace packages (regression test for over-permissive check caught in self-review)

Each regression test was verified to fail when the corresponding fix is reverted.

Backwards compatibility

Fully backwards compatible:

  • Repos without workspace:* deps: npm version succeeds as before, post-check isn't invoked.
  • Repos without bun.lock: patchBunLock is a no-op.
  • Repos with both: release actually works.

Verification

  • pnpm lint — 0 errors (7 pre-existing warnings, unchanged).
  • pnpm vitest run src/__tests__/versionBump.test.ts — all 42 tests pass (32 existing + 10 new).
  • pnpm build — clean.
  • Full pnpm test — 994 pass, 7 fail (pre-existing prepare-dry-run.e2e.test.ts failures in dumb terminals without EDITOR, documented environment issue).

References

…hing (#804)

`npm version --workspaces --include-workspace-root` successfully writes the
new version to every workspace package.json but then exits non-zero with
`EUNSUPPORTEDPROTOCOL` when any package declares a `workspace:*` dependency
(npm/cli#8845). Craft treated the non-zero exit as failure and fell back to
per-package bumping, which hit the same validator and failed the release.

Separately, `bun pm pack` rewrites `workspace:*` specifiers to concrete
versions at pack time by reading them from bun.lock — not from package.json.
Neither npm nor bun updates those lockfile entries automatically, so published
tarballs shipped with stale dependency versions and failed to install.

This change:

- After `npm version --workspaces` fails, post-checks every package.json on
  disk. If they all have the target version, we warn (citing npm/cli#8845)
  and proceed. Otherwise we fall through to individual bumping, which now
  applies the same per-package post-check before rethrowing.
- After a successful bump, patches `bun.lock` workspace `version` entries
  in-place using the workspace's path-relative key and a non-greedy regex
  that leaves nested dependency pins untouched. No-op when bun.lock is
  absent. Uses safeFs.writeFileSync for dry-run safety.

Dry-run is handled by short-circuiting the failure post-check (spawnProcess
is a no-op in dry-run so files wouldn't have been written by npm anyway) and
by safeFs.writeFileSync already respecting it.
Comment thread src/targets/npm.ts Outdated
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 2af7e84. Configure here.

Comment thread src/targets/npm.ts Outdated
Comment thread src/targets/npm.ts Outdated
…kspaces

Two correctness fixes from self-review:

1. `patchBunLock` regex used `[\s\S]*?` which walked across the closing `}`
   of a versionless workspace and silently rewrote the NEXT workspace's
   version line. For example, with `"packages/empty": { "name": "..." }`
   (no version) followed by `"packages/other": { ..., "version": "0.0.1" }`,
   a bump targeting `packages/empty` would corrupt `packages/other`'s
   version. Fixed by using `[^{}]*?` which stops at the workspace block's
   closing brace OR the opener of a nested object (e.g. "dependencies").
   This also removes the implicit assumption that the top-level `version`
   line precedes nested objects inside a workspace block.

2. `allWorkspacePackageJsonsMatch` skipped private workspace packages on
   the grounds that "npm version skips them" — but npm's `--workspaces`
   flag bumps all workspaces regardless of `private`. Skipping them made
   the post-check too permissive: if npm partially failed to bump a
   private workspace, we'd wrongly report success. Now we verify every
   workspace package.

Adds three regression tests:
- `bun.lock patching does NOT cross workspace boundaries when a workspace
  lacks a version field` (catches regex bug)
- `bun.lock patching disambiguates path prefixes (packages/foo vs
  packages/foo-bar)` (locks in the implicit guarantee from trailing `"`)
- `post-check verifies private workspace packages too (npm bumps them)`
  (catches the private-skip bug)

Each regression test was verified to fail when the corresponding fix is
reverted.
…om utils

Two bot-flagged issues addressed:

1. `patchBunLock` was called unconditionally in `bumpVersion`, which meant
   non-workspace bun projects with a bun.lock would get a spurious
   "lockfile out of sync" warning (workspaces.packages is empty → no
   entries match → warning fires). Move the call inside the
   `if (isWorkspace)` block so it only runs for monorepos.

2. The inline `escapeRegex` helper was a duplicate of the already-exported
   `escapeRegex` in `src/utils/filters.ts`. Import it from there instead.
@BYK BYK merged commit 2ec39c0 into master Apr 21, 2026
18 checks passed
@BYK BYK deleted the fix/npm-workspace-protocol branch April 21, 2026 22:03
BYK added a commit that referenced this pull request Apr 23, 2026
CHANGELOG.md is auto-generated from PR descriptions by the release
tooling (see the #793, #807, #805 entries in CHANGELOG.md for
examples where release-note titles contain `_*` sequences that
prettier wants escaped to `\*`). Every time a release cuts new
entries matching those patterns the format-check starts failing on
every open PR against master until someone runs a manual fixup.

Exclude the file from prettier so the release tooling is the sole
authority on its contents.
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.

npm target: workspace: dependencies break auto version bumping and break published tarballs

1 participant