feat(cli): 2.L — packaging & distribution as an engine-inlined npm bundle (ADR-0051)#49
Merged
Conversation
…xt pickup 2.L PR #48 merged (2.I — the read commands `list` / `logs` / `status` / `gate list` over durable history). Flip the status surfaces: - phase-2-cli.md: §2.I heading → ✅ Done (PR #48); status header adds 2.I; the Remaining build order status line + table drop the 2.I row and renumber (1.=2.L, 2.=2.S, 3.=2.R, 4.=chat, 5.=2.J); the gate-closing backbone is now `2.L` alone (2.I closed go/no-go #2). - current.md: 2.I added to Landed; next pickup → 2.L (the last gate-closing spine PR). - CLAUDE.md + README.md: 2.I landed; next pickup → 2.L. Structural views (the work-breakdown DAG, ordered-waves table, Mermaid graphs, dependency matrix) are the from-scratch plan, not a live tracker, and keep 2.I as a graph node unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ndle (ADR-0051)
Make `relavium` publishable to npm (`npm i -g relavium`) and verify the artifact cross-OS, behind a
new packaging/distribution decision.
ADR-0051 (the bundle boundary): the published artifact is an ENGINE-INLINED ESM bundle — tsup inlines
ONLY the proprietary, unpublished `@relavium/*` engine and externalizes every third-party dependency
(declared in package.json, installed normally; prebuilt native addons included). A "fat bundle" failed
empirically (esbuild can't inline @napi-rs/keyring's native wrapper); publishing the engine would expose
proprietary code or break the install.
Implementation:
- tsup.config: `noExternal:[/^@relavium\//]` + explicit third-party `external` list; `sourcemap:false` +
`minify:true` so the published `dist/` ships no original engine source (a source map would leak it); an
`onSuccess` copies `@relavium/db`'s drizzle migrations beside the bundle (the inlined db resolves them via
`new URL('../drizzle', import.meta.url)`, which once bundled points at the CLI package).
- apps/cli/package.json: drop `private`, v0.1.0, `engines`/`license`/`repository`/`keywords`, `files`+drizzle,
a `prepack` build; move `@relavium/*` to devDependencies (build-time inputs, inlined — never a published
dep); declare the 15-package third-party runtime closure (catalog:).
- tools/bundle-closure/check.mjs (+ `lint:bundle-closure`, wired into ci.yml + the `ci` script): fails the
build if the bundle's external import closure ever drifts from the declared dependencies — the guard for
the mirrored-closure cost ADR-0051 names.
- .github/workflows/release.yml: a `v*`-tag flow — pack once (pnpm pack resolves catalog:/workspace:) →
install-smoke the EXACT tarball on ubuntu/macOS/Windows (--help, provider list, run --json exit 0,
human-gate exit 3, gate list, unknown-runId exit 2 — headless-safe, no OS credential access) → maintainer
`npm publish <tarball> --provenance` gated on green smoke.
Local verification (macOS leg): pnpm pack → `npm install -g <tarball>` in an isolated prefix → the full smoke
passes end-to-end (both prebuilt native addons load, the bundled engine runs, migrations apply, durable
history + read commands work). This caught + fixed the migrations-not-shipped bug above.
Docs: ADR-0051 + decisions index; release-a-surface.md CLI release flow; tech-stack + commands.md
distribution model; phase-2-cli §2.L references ADR-0051; current.md adds the NPM_TOKEN maintainer
obligation; apps/cli/README.md is now the npm landing page.
Toolchain green: turbo lint/typecheck/test/build (CLI 319), typecheck:tools, bundle-closure + engine-deps
guards, format, leakwatch (0). The actual npm publish + the Linux/Windows smoke legs are the maintainer/CI
obligation (gated on the matrix), per ADR-0051.
Refs: ADR-0051
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @cemililik, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds CLI thin-bundle packaging, bundle-closure validation, a tag-driven release workflow, and matching ADR, runbook, roadmap, and status documentation updates. ChangesCLI Thin-Bundle Packaging & Release Pipeline
Sequence Diagram(s)sequenceDiagram
participant Maintainer
participant GitHubActions
participant PackJob
participant SmokeJob
participant PublishJob
participant npmRegistry
Maintainer->>GitHubActions: push v* tag or run workflow_dispatch
GitHubActions->>PackJob: start pack job
PackJob->>PackJob: build CLI, pack tarball, upload artifact
GitHubActions->>SmokeJob: start smoke job after pack
SmokeJob->>SmokeJob: download artifact, install tarball, run CLI checks
SmokeJob-->>GitHubActions: smoke success
GitHubActions->>PublishJob: start publish job on v* refs
PublishJob->>npmRegistry: npm publish <tgz> --provenance
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements packaging, distribution, and install verification (Workstream 2.L) for the Relavium CLI, establishing an engine-inlined ESM bundling strategy (ADR-0051) that inlines proprietary engine packages while externalizing third-party dependencies. It also introduces a bundle-closure guard script to prevent dependency drift. The review feedback suggests cleaning up the destination directory in the build script before copying migrations to avoid stale files, copying the root LICENSE file into the package directory for compliance, and updating the regular expression in the bundle-closure guard to correctly capture static side-effect imports.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| @@ -1,15 +1,54 @@ | |||
| import { cpSync } from 'node:fs'; | |||
There was a problem hiding this comment.
Import rmSync from node:fs to clean up the destination directory before copying migrations, and ensure the LICENSE file is copied to the package directory.
Suggested change
| import { cpSync } from 'node:fs'; | |
| import { cpSync, rmSync } from 'node:fs'; |
Comment on lines
+51
to
+53
| onSuccess: async () => { | ||
| cpSync('../../packages/db/drizzle', './drizzle', { recursive: true }); | ||
| }, |
There was a problem hiding this comment.
Stale migration files can accumulate in ./drizzle over time (e.g., when migrations are deleted or renamed in the source package) because cpSync with recursive: true merges files and does not delete extraneous ones in the destination. To prevent stale migrations from being packaged into the npm bundle, clean the destination directory before copying. Additionally, copy the repository LICENSE file to the package directory so that the "license": "SEE LICENSE IN LICENSE" field in package.json resolves correctly in the published package.
onSuccess: async () => {
rmSync('./drizzle', { recursive: true, force: true });
cpSync('../../packages/db/drizzle', './drizzle', { recursive: true });
try {
cpSync('../../LICENSE', './LICENSE', { force: true });
} catch (err) {
// Ignore if root LICENSE is missing during local dev builds
}
},
Comment on lines
+40
to
+46
| const SPEC_RE = /(?:from\s*|import\s*\(\s*|require\s*\(\s*)["']([^"']+)["']/g; | ||
| const imported = new Set(); | ||
| for (const match of code.matchAll(SPEC_RE)) { | ||
| const spec = match[1]; | ||
| if (spec.startsWith('.') || spec.startsWith('/') || BUILTINS.has(spec)) continue; | ||
| imported.add(toPackageName(spec)); | ||
| } |
There was a problem hiding this comment.
The original regular expression SPEC_RE does not match static side-effect imports (e.g., import "better-sqlite3" or import "react") because they do not contain the from keyword or parenthesis. This can lead to false positives (flagging active dependencies as dead/stale) or false negatives (missing undeclared side-effect imports). Updating the regex to match both static side-effect imports and standard imports/requires resolves this issue.
Suggested change
| const SPEC_RE = /(?:from\s*|import\s*\(\s*|require\s*\(\s*)["']([^"']+)["']/g; | |
| const imported = new Set(); | |
| for (const match of code.matchAll(SPEC_RE)) { | |
| const spec = match[1]; | |
| if (spec.startsWith('.') || spec.startsWith('/') || BUILTINS.has(spec)) continue; | |
| imported.add(toPackageName(spec)); | |
| } | |
| const SPEC_RE = /(?:from|import)\\s*[\\"']([^\\"']+)[\\"']|(?:import|require)\\s*\\(\\s*[\\"']([^\\"']+)[\\"']/g; | |
| const imported = new Set(); | |
| for (const match of code.matchAll(SPEC_RE)) { | |
| const spec = match[1] || match[2]; | |
| if (spec.startsWith('.') || spec.startsWith('/') || BUILTINS.has(spec)) continue; | |
| imported.add(toPackageName(spec)); | |
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
apps/cli/package.json (1)
31-32: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winRun the bundle-closure guard from
prepacktoo.Line 32 rebuilds the bundle, but a local
pnpm packornpm publishfromapps/clistill skips the drift gate that CI and the release workflow rely on. Chaining the guard here keeps the tarball path self-validating.Suggested fix
- "prepack": "tsup", + "prepack": "tsup && node ../../tools/bundle-closure/check.mjs",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/cli/package.json` around lines 31 - 32, The prepack step for the CLI package currently rebuilds the bundle but does not run the bundle-closure guard, so local packing and publishing can bypass the same drift check used in CI and release. Update the package.json scripts around build/prepack so the prepack flow in apps/cli invokes the bundle-closure guard as part of tsup-related packaging, ensuring the tarball path self-validates before pack/publish completes..github/workflows/release.yml (1)
48-55: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winRun the bundle-closure guard after
pnpm pack’sprepackrebuild.Line 50 says
prepackrebuilds the bundle, so the guard at lines 48-49 validates the pre-pack build, not necessarily the finaldistthat was just packed. Move or repeat the guard afterpnpm packso the checked bundle matches the tarball.Proposed ordering
- - name: Bundle-closure guard - run: node tools/bundle-closure/check.mjs # `pnpm pack` (not `npm pack`) resolves catalog:/workspace: in the published manifest; prepack rebuilds # the bundle + copies the drizzle migrations beside it. The tarball is platform-independent (native # addons resolve per-OS at install), so one tarball is smoked on, and published to, every target. - name: Pack working-directory: apps/cli run: pnpm pack --pack-destination "${{ runner.temp }}/artifact" + - name: Bundle-closure guard + run: node tools/bundle-closure/check.mjs🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/release.yml around lines 48 - 55, The bundle-closure guard is running before the `pnpm pack` step that triggers `prepack`, so it may validate an out-of-date bundle instead of the final packed output. Move or repeat the `Bundle-closure guard` step to run after `Pack` in the workflow so `tools/bundle-closure/check.mjs` checks the same rebuilt `dist` that gets tarballed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release.yml:
- Around line 13-17: The release workflow currently triggers on any v* tag but
does not verify that the Git tag matches the CLI package version. Add a
validation step in the release job to compare the pushed tag (from the workflow
context) against the version in apps/cli/package.json before publishing, and
fail early if they differ. Keep the check near the existing release/publish
logic in the release workflow so it guards the npm publish path and the manual
dry-run path consistently.
In `@apps/cli/README.md`:
- Line 54: The README license reference is pointing to a local LICENSE file that
may not exist in the CLI package, so update the markdown link in the README to
use the repository-level license target instead. Locate the existing license
notice in the README and replace the current relative link with a repo-root
license path so the link resolves correctly regardless of package location.
In `@apps/cli/tsup.config.ts`:
- Around line 51-52: The CLI build step in tsup.config.ts copies the drizzle
directory on top of an existing ./drizzle folder, so stale migrations can remain
published. Update the onSuccess handler to remove or recreate ./drizzle before
calling cpSync, keeping the copy in sync with ../../packages/db/drizzle and
preventing deleted or renamed files from lingering.
In `@docs/decisions/0051-cli-distribution-thin-bundle-private-engine.md`:
- Line 69: The ADR text is inconsistent with the actual package contents: the
shipped-files statement in the decision document currently says the package
keeps files: ["dist"], but the packaging model also includes drizzle for runtime
migrations. Update the wording in the relevant ADR section to match the
implemented bundle shape, referencing the shipped-files statement in the
decision doc.
In `@docs/roadmap/current.md`:
- Around line 33-34: The relative link in the roadmap entry is broken from the
current document location, so update the reference in the markdown list item to
use the correct relative path to the runbook. Keep the surrounding ADR link
unchanged, and adjust the `release-a-surface.md` link in
`docs/roadmap/current.md` so it resolves properly from that page.
---
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 48-55: The bundle-closure guard is running before the `pnpm pack`
step that triggers `prepack`, so it may validate an out-of-date bundle instead
of the final packed output. Move or repeat the `Bundle-closure guard` step to
run after `Pack` in the workflow so `tools/bundle-closure/check.mjs` checks the
same rebuilt `dist` that gets tarballed.
In `@apps/cli/package.json`:
- Around line 31-32: The prepack step for the CLI package currently rebuilds the
bundle but does not run the bundle-closure guard, so local packing and
publishing can bypass the same drift check used in CI and release. Update the
package.json scripts around build/prepack so the prepack flow in apps/cli
invokes the bundle-closure guard as part of tsup-related packaging, ensuring the
tarball path self-validates before pack/publish completes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7aaf85f7-7705-43d8-b75a-43819a0c6388
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (17)
.github/workflows/ci.yml.github/workflows/release.yml.gitignoreCLAUDE.mdREADME.mdapps/cli/README.mdapps/cli/package.jsonapps/cli/tsup.config.tsdocs/decisions/0051-cli-distribution-thin-bundle-private-engine.mddocs/decisions/README.mddocs/reference/cli/commands.mddocs/roadmap/current.mddocs/roadmap/phases/phase-2-cli.mddocs/runbooks/release-a-surface.mddocs/tech-stack.mdpackage.jsontools/bundle-closure/check.mjs
…g-version gate, drizzle clean-copy Address the PR #49 external-review batch (2.L · ADR-0051). Each finding verified against current code; valid ones fixed, the LICENSE-copy suggestion skipped (`pnpm pack` already ships the workspace-root LICENSE — tarball carries `package/LICENSE`, no `apps/cli/LICENSE`). - release.yml: drop the workflow-level `permissions` block; declare `permissions: contents: read` per job (pack, smoke) — least privilege (Sonar). Add a tag-version-check step to `pack` (a `v*` tag must equal apps/cli/package.json version, fail-fast before the cross-OS smoke). Remove the now-redundant standalone bundle-closure step — `prepack` runs the guard during Pack. - apps/cli/package.json: `prepack` = `tsup && pnpm -w run lint:bundle-closure`, so the guard validates exactly the dist that gets tarballed. - tsup.config.ts: `rmSync('./drizzle', {recursive,force})` before `cpSync` so a stale migration set can't survive a rebuild. - tools/bundle-closure/check.mjs: the import-extraction regex now also matches the bare side-effect `import "x"` form esbuild can emit. - ADR-0051: `files: ["dist", "drizzle"]` (was `["dist"]`) + a migrations-shipping mechanics bullet (the inlined db resolves `../drizzle` relative to the bundle). - apps/cli/README.md: LICENSE link → absolute repo URL. - current.md: runbook link → ../runbooks/release-a-surface.md. Refs: ADR-0051 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
cemililik
added a commit
that referenced
this pull request
Jun 24, 2026
…xt pickup 2.S 2.L (CLI packaging, distribution & install verification) merged via PR #49, closing the last Phase-3 go/no-go exit criterion (#7) — so the Phase-2 spine is complete and all seven criteria now hold. Move 2.L from "next pickup" into the Landed/Done list and re-point the next pickup to 2.S (media host-wiring, the first additive lane — off the M3 critical path and the go/no-go). Audited every status surface; updated only the live trackers and left the static plan views (Mermaid DAGs, wave/dependency tables, exit-criteria definitions) unchanged per their own "the plan, not a live tracker" framing. - current.md: 2.L Done entry + next pickup 2.S; re-tense the NPM_TOKEN maintainer obligation ("once 2.L lands" → "now that 2.L has landed (PR #49)"). - phase-2-cli.md: §2.L heading ✅ Done badge; header status line; Remaining-build-order banner; drop the 2.L row from the pickup queue and renumber 2.S/2.R/chat/2.J to 1–4; re-tense the gate-closing-backbone, 2.K-closed, and 2.S-timing bullets. - CLAUDE.md, README.md: append the 2.L packaging deliverable to the landed Phase-2 list; next pickup 2.S; all seven Phase-3 exit criteria now hold. - runbooks/release-a-surface.md: Status "draft" → "partial — CLI (npm) flow complete (2.L)". ADR-0051's "(today a stub)" aside about the runbook is left unedited: ADRs are append-only (CLAUDE.md rule 9) and it is a harmless point-in-time authoring note. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase-2 workstream 2.L — make
relaviumpublishable to npm (npm i -g relavium) and verify the artifact cross-OS. The last gate-closing spine PR: it closes Phase-3 go/no-go #7 (a working installable binary), after which all seven exit criteria hold.Introduces ADR-0051 (the bundle boundary). Also carries the small 2.I Done bookkeeping commit (
44e368b).The decision (ADR-0051)
The published artifact is an engine-inlined ESM bundle:
tsupinlines only the proprietary, unpublished@relavium/*engine and externalizes every third-party dependency (declared inpackage.json, installed normally — prebuilt native addons included). A "fat bundle" failed empirically (esbuild can't inline@napi-rs/keyring's native wrapper); publishing the engine would expose proprietary code (public) or break the install (private registry).Implementation
tsup.config—noExternal:[/^@relavium\//]+ explicit third-partyexternal;sourcemap:false+minify:true(the publisheddist/ships no original engine source — a map would leak it); anonSuccessships@relavium/db's drizzle migrations beside the bundle (the inlined db resolves them relative to the bundle).apps/cli/package.json— dropprivate, v0.1.0,engines/license/repository/keywords,prepack,files+drizzle; move@relavium/*→devDependencies(build-time inputs, inlined — never a published dep); declare the 15-package third-party runtime closure (catalog:).tools/bundle-closure/check.mjs(+lint:bundle-closure, wired into CI) — fails the build if the bundle's external import closure drifts from the declareddependencies(the guard for ADR-0051's mirrored-closure cost)..github/workflows/release.yml— av*-tag flow: pack once (pnpm packresolvescatalog:/workspace:) → install-smoke the exact tarball on ubuntu / macOS / Windows → maintainernpm publish <tarball> --provenance, gated on green smoke. Aworkflow_dispatchrun smokes without publishing.Local verification (macOS leg)
pnpm pack→npm install -g <tarball>in an isolated prefix → the full smoke passes end-to-end:--help,provider list,run … --json(exit 0,doubled:42),logs, human-gaterun --json(exit 3),gate list, unknown-runId (exit 2). Both prebuilt native addons load, the bundled engine runs, migrations apply, durable history works. This caught and fixed a real bug — the drizzle migrations weren't shipping beside the bundle, so every DB-opening command failed on the packaged install until the build copied them.Test plan
pnpm turbo run lint typecheck test build— green (CLI 319 tests).pnpm typecheck:tools·pnpm lint:bundle-closure(15 deps match) ·pnpm lint:engine-deps·pnpm format:check· leakwatch (0).Release CLIworkflow (maintainer/CI; can't run from a dev box).Maintainer obligations (per ADR-0051 / release runbook)
NPM_TOKENrepo secret + npm 2FA; then av0.1.0tag triggers pack → cross-OS smoke → publish-with-provenance. The actualnpm publishis maintainer-gated (like branch protection).Docs
ADR-0051 + decisions index;
release-a-surface.mdCLI release flow;tech-stack.md+commands.mddistribution model;phase-2-cli §2.Lreferences ADR-0051;current.mdadds the publish obligation;apps/cli/README.mdis now the npm landing page.🤖 Generated with Claude Code
Summary by CodeRabbit
relaviumCLI now ships as an engine-inlined ESM bundle and includes database migrations for runtime use.