Skip to content

fix(cli): bundled workspace packaging for release-runtime #128

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
stoicneko wants to merge 2 commits into
base: dev
Choose a base branch
from
+115 −31
Open

fix(cli): bundled workspace packaging for release-runtime #128

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3919fb0
Fix CLI bundled workspace packaging
stoicneko Mar 13, 2026
d9df85c
refactor(cli): address review feedback on bundled workspace packaging
stoicneko Mar 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions apps/cli/scripts/buildSharedDeps.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { execFileSync } from 'node:child_process';
import { cpSync, existsSync, readFileSync, realpathSync, writeFileSync } from 'node:fs';
import { cpSync, existsSync, readFileSync, writeFileSync } from 'node:fs';
import { dirname, resolve } from 'node:path';
import { fileURLToPath, pathToFileURL } from 'node:url';

import { bundledWorkspaceDirNames } from './bundledWorkspacePackages.mjs';

const __dirname = dirname(fileURLToPath(import.meta.url));

function findRepoRoot(startDir) {
Expand Down Expand Up @@ -51,6 +53,7 @@ export function resolveTscBin({ exists } = {}) {
}

const tscBin = resolveTscBin();
export const bundledWorkspaceDirs = bundledWorkspaceDirNames;

export function runTsc(tsconfigPath, opts) {
const exec = opts?.execFileSync ?? execFileSync;
Expand Down Expand Up @@ -78,7 +81,8 @@ export function syncBundledWorkspaceDist(opts = {}) {
const cp = opts.cpSync ?? cpSync;
const readFile = opts.readFileSync ?? readFileSync;
const writeFile = opts.writeFileSync ?? writeFileSync;
const packages = Array.isArray(opts.packages) && opts.packages.length > 0 ? opts.packages : ['agents', 'cli-common', 'protocol'];
const packages =
Array.isArray(opts.packages) && opts.packages.length > 0 ? opts.packages : bundledWorkspaceDirs;

for (const pkg of packages) {
const srcDist = resolve(repoRoot, 'packages', pkg, 'dist');
Expand Down Expand Up @@ -134,9 +138,9 @@ function sanitizeBundledWorkspacePackageJson(raw) {
}

export function main() {
runTsc(resolve(repoRoot, 'packages', 'agents', 'tsconfig.json'));
runTsc(resolve(repoRoot, 'packages', 'cli-common', 'tsconfig.json'));
runTsc(resolve(repoRoot, 'packages', 'protocol', 'tsconfig.json'));
for (const pkg of bundledWorkspaceDirs) {
runTsc(resolve(repoRoot, 'packages', pkg, 'tsconfig.json'));
}

const protocolDist = resolve(repoRoot, 'packages', 'protocol', 'dist', 'index.js');
if (!existsSync(protocolDist)) {
Expand Down
24 changes: 2 additions & 22 deletions apps/cli/scripts/bundleWorkspaceDeps.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,35 +7,15 @@ import {
findRepoRoot,
vendorBundledPackageRuntimeDependencies,
} from '../../../packages/cli-common/dist/workspaces/index.js';
import { createBundledWorkspaceBundles } from './bundledWorkspacePackages.mjs';

const __dirname = dirname(fileURLToPath(import.meta.url));

export function bundleWorkspaceDeps(opts = {}) {
const repoRoot = opts.repoRoot ?? findRepoRoot(__dirname);
const happyCliDir = opts.happyCliDir ?? resolve(repoRoot, 'apps', 'cli');

const bundles = [
{
packageName: '@happier-dev/agents',
srcDir: resolve(repoRoot, 'packages', 'agents'),
destDir: resolve(happyCliDir, 'node_modules', '@happier-dev', 'agents'),
},
{
packageName: '@happier-dev/cli-common',
srcDir: resolve(repoRoot, 'packages', 'cli-common'),
destDir: resolve(happyCliDir, 'node_modules', '@happier-dev', 'cli-common'),
},
{
packageName: '@happier-dev/protocol',
srcDir: resolve(repoRoot, 'packages', 'protocol'),
destDir: resolve(happyCliDir, 'node_modules', '@happier-dev', 'protocol'),
},
{
packageName: '@happier-dev/release-runtime',
srcDir: resolve(repoRoot, 'packages', 'release-runtime'),
destDir: resolve(happyCliDir, 'node_modules', '@happier-dev', 'release-runtime'),
},
];
const bundles = createBundledWorkspaceBundles({ repoRoot, happyCliDir });
bundleWorkspacePackages({ bundles });

for (const b of bundles) {
Expand Down
24 changes: 24 additions & 0 deletions apps/cli/scripts/bundledWorkspacePackages.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { resolve } from 'node:path';

export const bundledWorkspacePackages = Object.freeze([
{ dirName: 'agents', packageName: '@happier-dev/agents' },
{ dirName: 'cli-common', packageName: '@happier-dev/cli-common' },
{ dirName: 'protocol', packageName: '@happier-dev/protocol' },
{ dirName: 'release-runtime', packageName: '@happier-dev/release-runtime' },
]);

export const bundledWorkspaceDirNames = Object.freeze(
bundledWorkspacePackages.map(({ dirName }) => dirName),
);

export const bundledWorkspacePackageNames = Object.freeze(
bundledWorkspacePackages.map(({ packageName }) => packageName),
);

export function createBundledWorkspaceBundles({ repoRoot, happyCliDir }) {
return bundledWorkspacePackages.map(({ dirName, packageName }) => ({
packageName,
srcDir: resolve(repoRoot, 'packages', dirName),
destDir: resolve(happyCliDir, 'node_modules', '@happier-dev', dirName),
}));
}
42 changes: 42 additions & 0 deletions apps/cli/scripts/prepack-script.test.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,24 @@
import test from 'node:test';
import assert from 'node:assert/strict';
import { readFileSync } from 'node:fs';
import { resolve } from 'node:path';

import { bundledWorkspaceDirs } from './buildSharedDeps.mjs';
import {
bundledWorkspaceDirNames,
bundledWorkspacePackageNames,
createBundledWorkspaceBundles,
} from './bundledWorkspacePackages.mjs';

test('apps/cli prepack builds dist for npm pack', () => {
const pkgPath = new URL('../package.json', import.meta.url);
const pkg = JSON.parse(readFileSync(pkgPath, 'utf8'));
const prepack = String(pkg?.scripts?.prepack ?? '');
assert.ok(prepack.includes('build'), `expected scripts.prepack to include a build step, got: ${prepack || '(missing)'}`);
assert.ok(
prepack.includes('bundleWorkspaceDeps.mjs'),
`expected scripts.prepack to bundle workspace deps, got: ${prepack || '(missing)'}`,
);
});

test('apps/cli npm files list ships archives (not unpacked tools)', () => {
Expand All @@ -23,3 +35,33 @@ test('apps/cli npm files list ships archives (not unpacked tools)', () => {
assert.ok(!files.includes('tools'), 'expected not to ship entire tools/ tree (would include unpacked binaries)');
assert.ok(!files.includes('tools/unpacked'), 'expected tools/unpacked to be excluded');
});

test('apps/cli bundled workspace package definitions stay in sync', () => {
const pkgPath = new URL('../package.json', import.meta.url);
const pkg = JSON.parse(readFileSync(pkgPath, 'utf8'));
const bundledDependencies = Array.isArray(pkg?.bundledDependencies)
? pkg.bundledDependencies.map((value) => String(value)).sort()
: [];

assert.deepEqual(bundledDependencies, [...bundledWorkspacePackageNames].sort());
assert.deepEqual([...bundledWorkspaceDirs], [...bundledWorkspaceDirNames]);
Copy link
Contributor

@greptile-apps greptile-apps bot Mar 13, 2026

Choose a reason for hiding this comment

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

Tautological assertion — re-export compared to itself

bundledWorkspaceDirs in buildSharedDeps.mjs is defined as:

export const bundledWorkspaceDirs = bundledWorkspaceDirNames;

…a direct re-export of bundledWorkspaceDirNames. So when the test imports both and spreads them, it is comparing the same underlying frozen array to itself — deepEqual([...x], [...x]) — which will always pass regardless of what value x holds.

The intent of the assertion appears to be guarding against a future divergence where buildSharedDeps.mjs might override bundledWorkspaceDirs with a manually maintained subset. That guard is real and useful, but the assertion can only protect against that if the two symbols are independently defined. Consider adding a comment to the assertion explaining its forward-looking intent, or alternatively move the assertion's value closer to what it actually tests — that the main() loop in buildSharedDeps compiles every entry in the canonical list:

// Verify buildSharedDeps re-exports the canonical list without filtering.
// If buildSharedDeps.mjs ever defines bundledWorkspaceDirs independently,
// this assertion catches any divergence from the source of truth.
assert.deepEqual([...bundledWorkspaceDirs], [...bundledWorkspaceDirNames]);


const repoRoot = resolve('/tmp', 'happier-repo');
const happyCliDir = resolve(repoRoot, 'apps', 'cli');
const bundles = createBundledWorkspaceBundles({ repoRoot, happyCliDir });

assert.deepEqual(
bundles.map(({ packageName }) => packageName).sort(),
[...bundledWorkspacePackageNames].sort(),
);

const releaseRuntimeBundle = bundles.find(
({ packageName }) => packageName === '@happier-dev/release-runtime',
);
assert.ok(releaseRuntimeBundle, 'expected release-runtime to be bundled for npm publish');
assert.equal(releaseRuntimeBundle.srcDir, resolve(repoRoot, 'packages', 'release-runtime'));
assert.equal(
releaseRuntimeBundle.destDir,
resolve(happyCliDir, 'node_modules', '@happier-dev', 'release-runtime'),
);
});