Fix/node prepack

[chrisdoc]

No description provided.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6de52976-4ca7-4cfb-a804-7f4648af710f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/node-prepack

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[qodo-code-review]

PR Summary by Qodo

Build Node package during prepack to fix npm packaging

🐞 Bug fix 🧪 Tests ⚙️ Configuration changes 🕐 10-20 Minutes

AI Description

• Run a build during Node package prepack to ensure dist outputs exist before packing.
• Gate packaging on server.json/package.json drift checks via server-manifest script.
• Add a unit assertion that the published package defines a build-backed prepack step.

Diagram

graph TD
  A["CI / Developer"] --> B["npm pack / publish"] --> C["prepack script"] --> D["server-manifest check"] --> E["npm run build"] --> F["dist artifacts"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Use prepublishOnly instead of prepack

• ➕ Runs before npm publish, avoiding extra work for npm pack
• ➖ Does not run for npm pack, so tarballs can still be missing dist outputs

2. Build in CI release pipeline only

• ➕ Keeps npm lifecycle scripts minimal and predictable locally
• ➖ Easy to bypass when publishing outside CI; local npm pack can still be broken

3. Switch to prepare (git installs) + prepublishOnly (publish)

• ➕ Covers installs from git and publishing explicitly
• ➖ More complex lifecycle surface; still needs care to cover npm pack behavior

Recommendation: Keep the PR’s prepack approach: it is the most direct way to guarantee npm pack/npm publish includes built dist outputs, and the added test locks the behavior in. The manifest check is a good guardrail to prevent publishing metadata drift.

Files changed (3) +4 / -0

Bug fix (1) +1 / -0

package.jsonAdd prepack hook to validate manifest and build before packing +1/-0

Add prepack hook to validate manifest and build before packing

• Adds a 'prepack' script that first runs 'scripts/server-manifest.mjs check' to detect manifest drift and then runs 'npm run build' so 'dist' is present when the package is packed/published.

packages/node/package.json

Tests (1) +1 / -0

package-readme.test.tsAssert published Node package defines a build-backed prepack script +1/-0

Assert published Node package defines a build-backed prepack script

• Extends the documentation/package metadata unit test to verify the Node package’s 'prepack' script includes 'npm run build', preventing regressions in packaging behavior.

tests/unit/package-readme.test.ts

Other (1) +2 / -0

eleven-eyes-stick.mdAdd changeset entry for Node packaging fix +2/-0

Add changeset entry for Node packaging fix

• Introduces a new changeset to record the packaging-related fix for release automation/versioning.

.changeset/eleven-eyes-stick.md

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 65 rules

✅ Skills: thermo-nuclear-code-quality-review, build-mcpb, build-mcp-server

1. Prepack suggests missing script 🐞 Bug ☼ Reliability

Description

The Node workspace prepack runs server-manifest.mjs from packages/node, but on drift the
script errors with “Run npm run sync:server-manifest” even though packages/node/package.json does
not define that script. This can block npm pack/publish with a remediation command that fails in
the current working directory and can also reduce validation scope because server-manifest derives
paths from process.cwd().

Code

packages/node/package.json[28]

+		"prepack": "node ../../scripts/server-manifest.mjs check && npm run build",

Relevance

⭐⭐⭐ High

Team recently accepted server-manifest lifecycle/script fixes; likely to fix prepack cwd/remediation
mismatch too (PR #574).

PR-#574

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new prepack executes the manifest checker from the workspace directory, but the checker’s
drift error message explicitly instructs running a root script (sync:server-manifest). Since
packages/node/package.json doesn’t define that script, following the instruction from the failing
prepack context will not work; additionally, the checker’s use of rootDir = process.cwd() means
running from packages/node changes the resolved manifest paths compared to running from repo root.

packages/node/package.json[26-31]
scripts/server-manifest.mjs[139-150]
scripts/server-manifest.mjs[174-178]
package.json[58-72]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`packages/node` now runs `node ../../scripts/server-manifest.mjs check` during `prepack`. When the check detects drift, `server-manifest.mjs` instructs users to run `npm run sync:server-manifest`, but that script exists only in the repo root, not in `packages/node`. This makes the failure message misleading and can block packaging/publishing with an un-runnable remediation.

### Issue Context
- `server-manifest.mjs` uses `rootDir = process.cwd()` and resolves manifest paths relative to that, so invoking it from `packages/node` changes which manifests are validated.

### Fix Focus Areas
- packages/node/package.json[26-31]
- scripts/server-manifest.mjs[139-150]
- scripts/server-manifest.mjs[174-178]
- package.json[58-72]

### Suggested fix
Update the workspace `prepack` to run the repo-root check script so the validation matches CI and the remediation command exists, e.g.:
- `"prepack": "npm --prefix ../.. run check:server-manifest && npm run build"`

(Alternative: add `sync:server-manifest` (and optionally `check:server-manifest`) scripts to `packages/node/package.json`, but prefer running the root check to keep the dual-manifest validation consistent.)

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-code-review]

1. Prepack suggests missing script 🐞 Bug ☼ Reliability

The Node workspace prepack runs server-manifest.mjs from packages/node, but on drift the
script errors with “Run npm run sync:server-manifest” even though packages/node/package.json does
not define that script. This can block npm pack/publish with a remediation command that fails in
the current working directory and can also reduce validation scope because server-manifest derives
paths from process.cwd().

Agent Prompt

### Issue description
`packages/node` now runs `node ../../scripts/server-manifest.mjs check` during `prepack`. When the check detects drift, `server-manifest.mjs` instructs users to run `npm run sync:server-manifest`, but that script exists only in the repo root, not in `packages/node`. This makes the failure message misleading and can block packaging/publishing with an un-runnable remediation.

### Issue Context
- `server-manifest.mjs` uses `rootDir = process.cwd()` and resolves manifest paths relative to that, so invoking it from `packages/node` changes which manifests are validated.

### Fix Focus Areas
- packages/node/package.json[26-31]
- scripts/server-manifest.mjs[139-150]
- scripts/server-manifest.mjs[174-178]
- package.json[58-72]

### Suggested fix
Update the workspace `prepack` to run the repo-root check script so the validation matches CI and the remediation command exists, e.g.:
- `"prepack": "npm --prefix ../.. run check:server-manifest && npm run build"`

(Alternative: add `sync:server-manifest` (and optionally `check:server-manifest`) scripts to `packages/node/package.json`, but prefer running the root check to keep the dual-manifest validation consistent.)

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools