fix(pack): include host-migrations files in npm tarball - #1622
Conversation
PR #1619 added the host-migrations framework but `package.json` `files` array did not include `scripts/postinstall-migrations.js` nor `src/migrations/`. Result on published 4.260503.5: $ tar -tzf @automagik-genie-4.260503.5.tgz | grep migration package/src/db/migrations/... # only DB migrations shipped # scripts/postinstall-migrations.js MISSING # src/migrations/{runner,discover,store,steps/*}.ts MISSING Symptom on `bun add -g @automagik/genie@next && genie update --next`: - postinstall chain has `node scripts/postinstall-migrations.js` but the file is absent (silent failure under bun's default trust policy). - `genie migrate --status` returns "No migrations discovered." because `discoverMigrations()` scans a non-existent directory. Wish acceptance bullet from `.genie/wishes/genie-host-migrations`: "Auto-runs on `bun add -g @automagik/genie@latest` via postinstall hook so users get fixes transparently." This made the bullet impossible. Two-line fix to make it actually work in the next release. Verified via `npm pack --dry-run`: npm notice 2.2kB scripts/postinstall-migrations.js npm notice 1.8kB src/migrations/discover.ts npm notice 3.8kB src/migrations/index.ts npm notice 2.4kB src/migrations/runner.ts npm notice 3.4kB src/migrations/steps/001-pm2-env-databaseurl-bake.ts npm notice 3.8kB src/migrations/steps/002-kill-embedded-pgserve-legacy.ts npm notice 2.6kB src/migrations/store.ts npm notice 2.8kB src/migrations/README.md Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Code Review
This pull request updates package.json to include the postinstall-migrations.js script and the src/migrations/ directory in the published package. Feedback highlights a potential runtime failure where the post-install script might incorrectly use Node.js to execute a Bun-targeted bundle, and a path resolution issue in the migrations logic that could lead to incorrect version reporting in production.
| "plugins/genie/", | ||
| "scripts/postinstall-tmux.js", | ||
| "scripts/postinstall-hook-binary.js", | ||
| "scripts/postinstall-migrations.js", |
There was a problem hiding this comment.
The postinstall script scripts/postinstall-migrations.js (added here to the npm package) uses process.execPath to execute the genie.js bundle. Since the postinstall script in package.json is explicitly invoked with node, process.execPath will point to the Node.js binary. However, the bundle is built with --target bun and likely depends on Bun-specific APIs (as indicated by the bun engine requirement and externalized bun dependency). Running this bundle with Node.js will likely fail, preventing migrations from running during installation. Consider detecting and using bun to execute the bundle if it is a Bun-only artifact.
| "scripts/sec-fix.cjs", | ||
| "scripts/tmux/", | ||
| "src/db/migrations/", | ||
| "src/migrations/", |
There was a problem hiding this comment.
While including src/migrations/ is necessary for dynamic discovery, note that src/migrations/index.ts calculates the package.json path as ../../package.json. This works in the source tree but is incorrect for the bundled dist/genie.js (where __dirname is dist/). In the bundled state, the path should be ../package.json. This will cause getGenieVersion() to return "unknown" in production migration logs.
Summary
Two-line fix to
package.jsonfilesarray. PR #1619 added the host-migrations framework but thefileswhitelist was not updated, so the published tarball excluded both the postinstall hook script and the entiresrc/migrations/source directory.Symptom on the just-released
4.260503.5After
genie update --nexton a host:Verified via tarball inspection:
So the
migratecommand in the bundleddist/genie.jsruns, butdiscoverMigrations()scans asrc/migrations/steps/path that doesn't exist in the installed package → empty list → silent "no migrations." The whole framework is non-functional in published artifacts. Postinstall chain... && node scripts/postinstall-migrations.jsalso silently fails (bun's default trust policy blocks it on global installs anyway).Wish acceptance bullet broken
.genie/wishes/genie-host-migrations/WISH.md:This was impossible until both files ship in the tarball.
Diff
Verification
Test plan
bun add -g @automagik/genie@nexton a fresh host → tarball includes both new entriesgenie migrate --statuslists the 2 shipped steps (or "no pending" if already applied)genie update --next→ post-update maintenance section shows a[ok]/[fix]/[!!] migrationsline (if/when the post-update flow is taught to surface migrations status — separate follow-up)Out of scope
src/migrations/__tests__/orchestrator.test.ts(3.7kB) — minor waste, not worth a glob exclusionmigrationsline to the post-update maintenance summary — separate cosmetic follow-up🤖 Generated with Claude Code