Skip to content

fix: auto-detect publish target; loud failure for unpublishable roots#49

Merged
lacymorrow merged 2 commits into
mainfrom
fix/auto-detect-publish-target
May 25, 2026
Merged

fix: auto-detect publish target; loud failure for unpublishable roots#49
lacymorrow merged 2 commits into
mainfrom
fix/auto-detect-publish-target

Conversation

@lacymorrow

Copy link
Copy Markdown
Owner

Summary

Closes two related disasters seen in lacy's release stream:

  • LAC-2055 — single-project mode published the root `package.json` of a monorepo (no `bin` field), breaking `npx` for several versions in a row.
  • LAC-2090 — multi-mode produced a tag + GitHub release for v1.8.16 then silently skipped `npm publish` because `hasNpm` went stale after a branch switch.

Unifying principle: `npm.cwd` is the source of truth for the package being published. Everything publish-related now reads from there, and shipx refuses to publish a structurally unpublishable package without a config fix or an explicit opt-out.

Changes

  • `src/detect.ts` — `isUnpublishablePackage` + `detectPublishTarget`. Conservative: only picks a subpackage when the root is clearly unpublishable (`private: true`, `workspaces` field, or no entry-point fields). Returns ambiguous when 2+ candidates exist instead of guessing.
  • `src/config.ts` — `loadConfig` runs detection when the user didn't set `npm.cwd` or `npm.targets`. When detection picks a subpackage, also defaults `packageJsonPaths` and `versionSource` to it so the bumped version matches the published one. Records the reason on `ResolvedConfig` so the CLI can log it.
  • `src/steps/preflight.ts` — publish-side checks (entry points, files, npm auth) now read from `npm.cwd`, not `packageJsonPaths[0]`. New `checkPackagePublishable` refuses to proceed when any npm target is structurally unpublishable.
  • `src/multi.ts` — re-evaluate `project.hasNpm` after a branch switch (LAC-2090 fix). Warn loudly for every prepared project that is about to skip npm publish — silent skips after producing a tag are the worst failure mode.
  • `src/steps/npm.ts` — `verifyPublishedArtifact` runs after each successful publish (single + multi + retry paths), `npm view`s the version, and warns when the registry artifact is missing `bin`/`main` fields the local package has. Best-effort, never fails the release.
  • Tests for detect, `loadConfig` auto-detect, `packageJsonPaths` defaulting, and user-override precedence.

After this change, a repo structured like lacy (root is private, single publishable subpackage under `packages/`) works with zero shipx config.

Test plan

  • `bun run typecheck` clean
  • `bun test src/detect.test.ts src/config.test.ts src/tests/` — 89/89 (canonical CI command `bun run test` runs `src/tests/` only and is green)
  • `bun run build` succeeds; `node dist/cli.js --help` works
  • Smoke test against lacy (`SHIPX_ROOT=~/repo/lacy node dist/cli.js --dry-run patch`):
    • logs `auto-detected npm.cwd → packages/lacy (root is marked "private": true; packages/lacy is the only publishable subpackage)`
    • registry check runs against `lacy` (the published name), not the root
    • proposes a bump from `packages/lacy/package.json`'s version
  • Regression: shipx self-release still works (root has `bin`, no auto-detect fires)
  • Regression: lash with explicit `npm.cwd: "packages/opencode"` still publishes from the configured target

Notes

  • The `bun test src/steps/` pre-existing failure (homebrew.test.ts globally mocks `../utils.ts` and corrupts `git.test.ts`) is not caused by this PR — same failure exists on `main`. Verified by stashing and re-running.
  • Does not address the broader UX concern that shipx auto-switches branches mid-release (the trigger for LAC-2090). That's a separate UX redesign — happy to follow up.

Closes two related disasters seen in lacy's release stream:

* LAC-2055 — single-project mode published the root package.json of a
  monorepo (no bin field), breaking npx for several versions in a row.
* LAC-2090 — multi-mode produced a tag + GitHub release for v1.8.16 then
  silently skipped npm publish because hasNpm went stale after a branch
  switch.

Unifying principle: npm.cwd is the source of truth for the package being
published. Everything publish-related now reads from there, and shipx
refuses to publish a structurally unpublishable package without a config
fix or an explicit opt-out.

Changes:

* src/detect.ts — isUnpublishablePackage + detectPublishTarget. Conservative:
  only picks a subpackage when the root is clearly unpublishable
  (private, workspaces field, or no entry-point fields). Returns ambiguous
  when 2+ candidates exist instead of guessing.

* src/config.ts — loadConfig runs detection when the user didn't set
  npm.cwd or npm.targets. When detection picks a subpackage, also defaults
  packageJsonPaths and versionSource to it so the bumped version matches
  the published one. Records the reason on ResolvedConfig so the cli can
  log it.

* src/steps/preflight.ts — publish-side checks (entry points, files,
  npm auth) now read from npm.cwd, not packageJsonPaths[0]. New
  checkPackagePublishable refuses to proceed when any npm target is
  structurally unpublishable.

* src/multi.ts — re-evaluate project.hasNpm after a branch switch
  (LAC-2090 fix). Warn loudly for every prepared project that is about to
  skip npm publish — silent skips after producing a tag are the worst
  failure mode.

* src/steps/npm.ts — verifyPublishedArtifact runs after each successful
  publish (single + multi + retry paths), npm view's the version, and
  warns when the registry artifact is missing bin/main fields the local
  package has. Best-effort, never fails the release.

* Tests for detect, loadConfig auto-detect, packageJsonPaths defaulting,
  and user-override precedence.

After this change, a repo structured like lacy/ (root is private, single
publishable subpackage under packages/) works with zero shipx config.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request implements automatic detection of publishable subpackages in monorepos when the root package is unpublishable (e.g., private or a workspace root). It introduces a new detection utility, enhances preflight checks to prevent shipping unpublishable targets, and adds post-publish verification to ensure registry artifacts match local entry points. The review feedback correctly identifies two instances in src/multi.ts where manual publishability checks should be replaced with the centralized isUnpublishablePackage function to ensure consistency and capture all failure modes, such as missing entry points.

Comment thread src/multi.ts Outdated
for (const target of config.npm.targets) {
try {
const pkg = readJson(resolve(target.cwd, "package.json"));
const isPublishable = pkg.private !== true && !pkg.workspaces;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The publishability check here is inconsistent with the isUnpublishablePackage function defined in src/detect.ts. Specifically, it misses the check for entry points (bin, main, exports, module). This could lead to the same "no bin field" disaster the PR aims to prevent, as multi mode might incorrectly identify a package as publishable.

Since isUnpublishablePackage is already defined as the source of truth for this logic, it should be imported and used here to ensure consistency across the codebase.

Suggested change
const isPublishable = pkg.private !== true && !pkg.workspaces;
const isPublishable = !isUnpublishablePackage(pkg);

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Done in 741343a — switched to isUnpublishablePackage(pkg) === null so the no-entry-points case is covered. Added a focused unit test (reevaluateProjectPublishability > target with no bin/main/exports/module: hasNpm becomes false) to pin the regression.

Comment thread src/multi.ts Outdated
Comment on lines +608 to +609
if (pkg.private === true) reasons.push(`${target.cwd}: private`);
else if (pkg.workspaces) reasons.push(`${target.cwd}: workspace root`);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

This manual check for skip reasons is incomplete and duplicates logic that should be centralized. It misses the "no entry point" reason, which is one of the primary failure modes this PR addresses.

Consider using isUnpublishablePackage to retrieve the specific reason for why a target is considered unpublishable.

Suggested change
if (pkg.private === true) reasons.push(`${target.cwd}: private`);
else if (pkg.workspaces) reasons.push(`${target.cwd}: workspace root`);
const reason = isUnpublishablePackage(pkg);
if (reason) reasons.push(`${target.cwd}: ${reason}`);

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Done in 741343a — now uses isUnpublishablePackage(pkg) and reports the returned reason verbatim, so private / workspace-root / no-entry-points all surface consistently.

)

Both spots in multi.ts that previously inlined a partial publishability
check now call isUnpublishablePackage from detect.ts, so private,
workspace-root, AND missing-entry-points are all caught consistently.

- reevaluateProjectPublishability: a sub-package with no bin/main/exports
  was previously treated as publishable, defeating the original fix. Now
  it falls into the skip path with a clear reason.
- Phase-2 skip-reason walk: now reports the reason verbatim from
  isUnpublishablePackage instead of a hand-rolled subset.

Plus a focused unit test for reevaluateProjectPublishability that pins
the entry-points regression case.

Both raised by Gemini Code Assist on PR #49.
@lacymorrow lacymorrow merged commit fc6b16a into main May 25, 2026
3 checks passed
@lacymorrow lacymorrow deleted the fix/auto-detect-publish-target branch May 25, 2026 19:24
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.

1 participant