Skip to content

Commit 0d42b12

Browse files
jorgemoyaclaude
andauthored
fix(cli): prevent dependency install from hanging on link/deploy setup (#3107)
* fix(cli): prevent dependency install from hanging on link/deploy setup The install step in `catalyst project link` and the `catalyst deploy` first-run setup could hang forever on "Installing dependencies...". nypm was called with `silent: true`, which pipes the child's stdio; since nypm routes pnpm/yarn through corepack, corepack's package-manager download confirmation prompt blocked on stdin that never arrived — an invisible, unanswerable deadlock. Inherit stdio instead so corepack/pnpm prompts and progress are visible and answerable. Also detect the project's actual package manager from its lockfile in the link/deploy setup paths instead of always defaulting to pnpm, so npm/yarn/bun projects aren't forced through corepack pnpm. Refs LTRAC-1224 Co-Authored-By: Claude <noreply@anthropic.com> * fix(cli): silence child stderr flood that deadlocked dependency install The real cause of the install hang was not corepack's prompt: nypm runs the install via tinyexec, which in silent mode fully drains the child's stdout before it reads stderr. pnpm sends progress to stdout but floods stderr with Node warnings (on Node 26, thousands of "File descriptor N opened in unmanaged mode" lines). That stderr fills its ~64KB pipe buffer while tinyexec is still on stdout, pnpm blocks writing, and the install deadlocks. Inheriting stdio avoided the deadlock but surfaced the warning spam in the terminal. Keep the silent spinner UX and instead starve the flood: run the install child with NODE_NO_WARNINGS=1 (drops the internal Node warnings) and pin COREPACK_ENABLE_DOWNLOAD_PROMPT=0 defensively. Both are saved and restored so they don't leak into later CLI operations. Refs LTRAC-1224 Co-Authored-By: Claude <noreply@anthropic.com> * fix(cli): default project package manager fallback to npm Detection of an existing project's package manager fell back to pnpm when a lockfile or packageManager field couldn't be resolved. Most users have npm installed by default, so default there instead — pnpm requires an extra install step most contributors won't have. Refs LTRAC-1224 Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 552cfb5 commit 0d42b12

7 files changed

Lines changed: 144 additions & 4 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@bigcommerce/catalyst": patch
3+
---
4+
5+
Fix the dependency install step hanging indefinitely during `catalyst project link` and the `catalyst deploy` first-run setup. nypm runs the install through tinyexec, which in silent mode drains the child's stdout to completion before it reads stderr; pnpm floods stderr with Node warnings (on Node 26, thousands of `File descriptor N opened in unmanaged mode` lines), filling the stderr pipe buffer while tinyexec is still on stdout, so pnpm blocks writing and the install deadlocks. The install now runs the child with `NODE_NO_WARNINGS` (and pins `COREPACK_ENABLE_DOWNLOAD_PROMPT` off) so stderr never fills. The `link`/`deploy` setup paths also now detect the project's actual package manager from its lockfile instead of always forcing pnpm.

packages/catalyst/src/cli/commands/deploy.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -828,7 +828,7 @@ describe('transformation guard', () => {
828828
storeHash,
829829
accessToken,
830830
});
831-
expect(installDependencies).toHaveBeenCalledWith(tmpDir);
831+
expect(installDependencies).toHaveBeenCalledWith(tmpDir, expect.any(String));
832832
});
833833

834834
test('exits gracefully when user declines to run setup', async () => {

packages/catalyst/src/cli/commands/deploy.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
setupCommerceHosting,
1818
} from '../lib/commerce-hosting';
1919
import { getDeploymentErrorMessage } from '../lib/deployment-errors';
20+
import { detectProjectPackageManager } from '../lib/detect-package-manager';
2021
import {
2122
getStoredEnv,
2223
mergeDeploymentSecrets,
@@ -493,7 +494,11 @@ Example:
493494
await setupCommerceHosting({ projectDir, projectUuid, storeHash, accessToken });
494495
consola.success('Commerce Hosting setup complete.');
495496

496-
await installDependencies(projectDir);
497+
// Match the manager the project was scaffolded with (from its lockfile),
498+
// rather than forcing pnpm on npm/yarn/bun projects.
499+
const packageManager = await detectProjectPackageManager(projectDir);
500+
501+
await installDependencies(projectDir, packageManager);
497502
} else {
498503
// Existing Commerce Hosting users may carry artifacts incompatible with
499504
// the Cloudflare worker bundle from earlier Catalyst versions

packages/catalyst/src/cli/commands/project.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
selectOrCreateInfrastructureProject,
99
setupCommerceHosting,
1010
} from '../lib/commerce-hosting';
11+
import { detectProjectPackageManager } from '../lib/detect-package-manager';
1112
import { installDependencies } from '../lib/install-dependencies';
1213
import { consola } from '../lib/logger';
1314
import { LoginAbortedError, login as runInteractiveLogin } from '../lib/login';
@@ -59,7 +60,11 @@ async function offerCommerceHostingSetup(
5960

6061
consola.success('Commerce Hosting setup complete.');
6162

62-
await installDependencies(projectDir);
63+
// Match the manager the project was scaffolded with (from its lockfile),
64+
// rather than forcing pnpm on npm/yarn/bun projects.
65+
const packageManager = await detectProjectPackageManager(projectDir);
66+
67+
await installDependencies(projectDir, packageManager);
6368
}
6469

6570
const list = new Command('list')
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
import { mkdtempSync, rmSync, writeFileSync } from 'node:fs';
2+
import { tmpdir } from 'node:os';
3+
import { join } from 'node:path';
4+
import { detectPackageManager as detectFromDir } from 'nypm';
5+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
6+
7+
import { detectProjectPackageManager } from './detect-package-manager';
8+
9+
// nypm's own fallback sniffs process.argv[1] for a package manager's name, which
10+
// spuriously matches 'pnpm' in this pnpm-managed monorepo (test runner path
11+
// contains `.pnpm`). Spy on it so the "nothing detectable" case below tests our
12+
// fallback instead of that environment artifact.
13+
vi.mock('nypm', async (importOriginal) => {
14+
const actual = await importOriginal<typeof import('nypm')>();
15+
16+
return { ...actual, detectPackageManager: vi.fn(actual.detectPackageManager) };
17+
});
18+
19+
let projectDir: string;
20+
21+
beforeEach(() => {
22+
projectDir = mkdtempSync(join(tmpdir(), 'catalyst-detect-pm-'));
23+
});
24+
25+
afterEach(() => {
26+
rmSync(projectDir, { recursive: true, force: true });
27+
});
28+
29+
describe('detectProjectPackageManager', () => {
30+
it('detects pnpm from a pnpm-lock.yaml', async () => {
31+
writeFileSync(join(projectDir, 'pnpm-lock.yaml'), '');
32+
33+
await expect(detectProjectPackageManager(projectDir)).resolves.toBe('pnpm');
34+
});
35+
36+
it('detects npm from a package-lock.json', async () => {
37+
writeFileSync(join(projectDir, 'package-lock.json'), '{}');
38+
39+
await expect(detectProjectPackageManager(projectDir)).resolves.toBe('npm');
40+
});
41+
42+
it('detects yarn from a yarn.lock', async () => {
43+
writeFileSync(join(projectDir, 'yarn.lock'), '');
44+
45+
await expect(detectProjectPackageManager(projectDir)).resolves.toBe('yarn');
46+
});
47+
48+
it('detects bun from a bun.lock', async () => {
49+
writeFileSync(join(projectDir, 'bun.lock'), '');
50+
51+
await expect(detectProjectPackageManager(projectDir)).resolves.toBe('bun');
52+
});
53+
54+
it('honors the package.json packageManager field', async () => {
55+
writeFileSync(
56+
join(projectDir, 'package.json'),
57+
JSON.stringify({ packageManager: 'yarn@4.0.0' }),
58+
);
59+
60+
await expect(detectProjectPackageManager(projectDir)).resolves.toBe('yarn');
61+
});
62+
63+
it('falls back to npm when nothing is detectable', async () => {
64+
vi.mocked(detectFromDir).mockResolvedValueOnce(undefined);
65+
66+
await expect(detectProjectPackageManager(projectDir)).resolves.toBe('npm');
67+
});
68+
});

packages/catalyst/src/cli/lib/detect-package-manager.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { detectPackageManager as detectFromDir } from 'nypm';
2+
13
export type PackageManager = 'npm' | 'pnpm' | 'yarn' | 'bun';
24

35
// Detect the package manager that INVOKED the CLI (via npx / pnpm dlx / yarn dlx
@@ -20,3 +22,21 @@ export const detectPackageManager = (): PackageManager => {
2022

2123
return 'npm';
2224
};
25+
26+
// Detect the package manager an EXISTING project uses, from its lockfile or the
27+
// package.json `packageManager` field. Used by flows that run inside an
28+
// already-scaffolded project (e.g. `project link`), where a lockfile exists —
29+
// unlike detectPackageManager(), which infers the INVOKING manager for a freshly
30+
// extracted project that has no lockfile yet. Falls back to npm, since most
31+
// users will have it installed, when detection is inconclusive or returns a
32+
// manager we don't support.
33+
export const detectProjectPackageManager = async (projectDir: string): Promise<PackageManager> => {
34+
const detected = await detectFromDir(projectDir);
35+
const name = detected?.name;
36+
37+
if (name === 'pnpm' || name === 'yarn' || name === 'bun' || name === 'npm') {
38+
return name;
39+
}
40+
41+
return 'npm';
42+
};

packages/catalyst/src/cli/lib/install-dependencies.ts

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,43 @@ import yoctoSpinner from 'yocto-spinner';
33

44
import type { PackageManager } from './detect-package-manager';
55

6+
// Restore an env var to a captured previous value, unsetting it when it wasn't
7+
// set before (a plain assignment can't represent "absent").
8+
const restoreEnv = (key: string, previous: string | undefined) => {
9+
if (previous === undefined) {
10+
Reflect.deleteProperty(process.env, key);
11+
} else {
12+
process.env[key] = previous;
13+
}
14+
};
15+
16+
// nypm runs the install through tinyexec, which in silent mode drains the
17+
// child's stdout to completion BEFORE it starts reading stderr. pnpm sends its
18+
// progress to stdout but floods stderr with Node warnings — on Node 26 the
19+
// `File descriptor N opened in unmanaged mode` spam alone is thousands of
20+
// lines. That stderr fills its ~64KB pipe buffer while tinyexec is still busy
21+
// on stdout, pnpm blocks writing, and the whole install deadlocks (the original
22+
// "hangs forever" bug). These env vars keep the child quiet on stderr so the
23+
// buffer never fills:
24+
// - NODE_NO_WARNINGS: drop pnpm/corepack's internal Node warnings (the flood).
25+
// - COREPACK_ENABLE_DOWNLOAD_PROMPT: never block on corepack's download
26+
// confirmation (defensive; corepack already defaults this off for the
27+
// `corepack pnpm` entrypoint, but pin it so no path can stall on stdin).
28+
const withQuietInstallEnv = async <T>(run: () => Promise<T>): Promise<T> => {
29+
const prevNodeNoWarnings = process.env.NODE_NO_WARNINGS;
30+
const prevDownloadPrompt = process.env.COREPACK_ENABLE_DOWNLOAD_PROMPT;
31+
32+
process.env.NODE_NO_WARNINGS ??= '1';
33+
process.env.COREPACK_ENABLE_DOWNLOAD_PROMPT ??= '0';
34+
35+
try {
36+
return await run();
37+
} finally {
38+
restoreEnv('NODE_NO_WARNINGS', prevNodeNoWarnings);
39+
restoreEnv('COREPACK_ENABLE_DOWNLOAD_PROMPT', prevDownloadPrompt);
40+
}
41+
};
42+
643
export const installDependencies = async (
744
projectDir: string,
845
packageManager: PackageManager = 'pnpm',
@@ -12,7 +49,7 @@ export const installDependencies = async (
1249
);
1350

1451
try {
15-
await installDeps({ cwd: projectDir, silent: true, packageManager });
52+
await withQuietInstallEnv(() => installDeps({ cwd: projectDir, silent: true, packageManager }));
1653
spinner.success('Dependencies installed successfully.');
1754
} catch (error) {
1855
const message = error instanceof Error ? error.message : 'unknown error';

0 commit comments

Comments
 (0)