Skip to content

Commit 54a0213

Browse files
fix: harden package-manager auto-install (review findings)
- yarn: install corepack shims into a user-owned --install-directory and prependPath it, so a NodeSource Linux install no longer hits EACCES writing to the root-owned Node bin dir. - bun: honor $BUN_INSTALL when exposing the freshly-installed bin dir, instead of hardcoding ~/.bun/bin. - orchestrator: re-check each tool after install and fail with the manual hint if it is still not on PATH, instead of letting the next step die with a confusing "command not found". - orchestrator: single-flight installs by tool name so concurrent deploy-all builds sharing a missing manager don't race the same installer. - detect: recognise bun 1.2+'s text lockfile bun.lock (not just bun.lockb). - changeset: clarify that deploy/build auto-install while mod prompts.
1 parent 9520126 commit 54a0213

6 files changed

Lines changed: 133 additions & 21 deletions

File tree

.changeset/mod-package-manager-autoinstall.md

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22
"playground-cli": minor
33
---
44

5-
`playground mod` and deploy now detect the package manager a project uses
6-
(pnpm/yarn/bun/npm) and offer to install it — plus Node.js when needed — instead
7-
of failing with a confusing error when it's missing. macOS and Linux are
8-
supported; non-interactive runs auto-install.
5+
`playground mod` now detects the package manager a project uses (pnpm/yarn/bun/npm)
6+
and, when it (or Node.js) is missing, offers to install it with one confirmation
7+
before running the project's setup, instead of failing with a confusing error.
8+
`playground deploy` and `playground build` install a missing manager automatically
9+
as part of the build step. macOS and Linux are supported; non-interactive runs
10+
install without prompting.

src/utils/build/detect.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,10 @@ describe("detectPackageManager", () => {
4949
it("picks bun when only bun.lockb is present", () => {
5050
expect(detectPackageManager(new Set(["bun.lockb"]))).toBe("bun");
5151
});
52+
53+
it("picks bun when only bun.lock (bun 1.2+ text lockfile) is present", () => {
54+
expect(detectPackageManager(new Set(["bun.lock"]))).toBe("bun");
55+
});
5256
});
5357

5458
describe("detectBuildConfig", () => {

src/utils/build/detect.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,21 @@ import { DEFAULT_BUILD_DIR } from "../../config.js";
2424

2525
export type PackageManager = "pnpm" | "yarn" | "bun" | "npm";
2626

27-
/** Files we inspect on disk to infer the package manager. */
27+
/** Canonical lockfile basename per package manager. */
2828
export const PM_LOCKFILES: Record<PackageManager, string> = {
2929
pnpm: "pnpm-lock.yaml",
3030
yarn: "yarn.lock",
3131
bun: "bun.lockb",
3232
npm: "package-lock.json",
3333
};
3434

35+
// Bun 1.2+ defaults to a TEXT lockfile (`bun.lock`); older bun wrote the binary
36+
// `bun.lockb`. Detect either so modern bun projects aren't mis-detected as npm.
37+
const BUN_TEXT_LOCKFILE = "bun.lock";
38+
39+
/** Every lockfile basename to probe on disk (some PMs have more than one). */
40+
export const PM_LOCKFILES_ALL: string[] = [...Object.values(PM_LOCKFILES), BUN_TEXT_LOCKFILE];
41+
3542
export interface BuildConfig {
3643
/** Binary + args to spawn. */
3744
cmd: string;
@@ -80,7 +87,7 @@ export class BuildDetectError extends Error {
8087
export function detectPackageManager(lockfiles: Set<string>): PackageManager {
8188
if (lockfiles.has(PM_LOCKFILES.pnpm)) return "pnpm";
8289
if (lockfiles.has(PM_LOCKFILES.yarn)) return "yarn";
83-
if (lockfiles.has(PM_LOCKFILES.bun)) return "bun";
90+
if (lockfiles.has(PM_LOCKFILES.bun) || lockfiles.has(BUN_TEXT_LOCKFILE)) return "bun";
8491
return "npm";
8592
}
8693

src/utils/build/runner.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import { runStreamed } from "../process.js";
2525
import {
2626
detectBuildConfig,
2727
detectInstallConfig,
28-
PM_LOCKFILES,
28+
PM_LOCKFILES_ALL,
2929
type BuildConfig,
3030
type DetectInput,
3131
type InstallConfig,
@@ -56,7 +56,7 @@ export function loadDetectInput(projectDir: string): DetectInput {
5656
: null;
5757

5858
const lockfiles = new Set<string>();
59-
for (const name of Object.values(PM_LOCKFILES)) {
59+
for (const name of PM_LOCKFILES_ALL) {
6060
if (existsSync(join(root, name))) lockfiles.add(name);
6161
}
6262

src/utils/packageManagers.test.ts

Lines changed: 55 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -125,9 +125,10 @@ describe("install command builders", () => {
125125
expect(bunInstallCommand()).toContain("bun.sh/install");
126126
});
127127

128-
it("installs yarn via corepack (its official path)", () => {
129-
expect(yarnInstallCommand()).toContain("corepack");
130-
expect(yarnInstallCommand()).toContain("yarn@stable");
128+
it("installs yarn via corepack into a user-owned dir (avoids EACCES on Linux)", () => {
129+
const cmd = yarnInstallCommand("/home/u/.corepack/bin");
130+
expect(cmd).toContain('corepack enable --install-directory "/home/u/.corepack/bin"');
131+
expect(cmd).toContain("yarn@stable");
131132
});
132133
});
133134

@@ -147,12 +148,16 @@ describe("PM_TOOLS — which tools each PM needs", () => {
147148
});
148149

149150
function fakeTool(label: string, installed: boolean, installSpy: string[]): PmTool {
151+
// Models a real tool: `check()` flips to true once `install()` has run, so
152+
// the orchestrator's post-install re-check passes.
153+
let present = installed;
150154
return {
151155
name: label,
152156
label,
153-
check: async () => installed,
157+
check: async () => present,
154158
install: async () => {
155159
installSpy.push(label);
160+
present = true;
156161
},
157162
manualHint: `install ${label}`,
158163
};
@@ -213,6 +218,52 @@ describe("ensurePackageManagerForTools", () => {
213218
// pnpm comes after the failed Node install, so it must not have run.
214219
expect(installed).toEqual([]);
215220
});
221+
222+
it("throws when a tool is still missing after its install ran (PATH mismatch)", async () => {
223+
const installed: string[] = [];
224+
// install() "succeeds" but check() never flips — models an installer that
225+
// wrote its binary somewhere not on PATH.
226+
const stubborn: PmTool = {
227+
name: "Node.js",
228+
label: "Node.js",
229+
check: async () => false,
230+
install: async () => {
231+
installed.push("Node.js");
232+
},
233+
manualHint: "https://nodejs.org/en/download",
234+
};
235+
await expect(ensurePackageManagerForTools("npm", [stubborn], {})).rejects.toThrow(
236+
/still not on PATH/,
237+
);
238+
expect(installed).toEqual(["Node.js"]); // it did attempt the install
239+
});
240+
241+
it("collapses concurrent installs of the same tool onto one run (single-flight)", async () => {
242+
let installCount = 0;
243+
let present = false;
244+
let release!: () => void;
245+
const gate = new Promise<void>((r) => {
246+
release = r;
247+
});
248+
const make = (): PmTool => ({
249+
name: "pnpm",
250+
label: "pnpm",
251+
check: async () => present,
252+
install: async () => {
253+
installCount++;
254+
await gate;
255+
present = true;
256+
},
257+
manualHint: "x",
258+
});
259+
// Two concurrent ensures racing the same missing tool (the deploy-all case).
260+
const a = ensurePackageManagerForTools("pnpm", [make()], {});
261+
const b = ensurePackageManagerForTools("pnpm", [make()], {});
262+
await new Promise((r) => setTimeout(r, 10)); // let both reach the install
263+
release();
264+
await Promise.all([a, b]);
265+
expect(installCount).toBe(1);
266+
});
216267
});
217268

218269
describe("planPackageManagerForTools", () => {

src/utils/packageManagers.ts

Lines changed: 57 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
import { existsSync, readFileSync } from "node:fs";
2828
import { homedir, platform } from "node:os";
2929
import { join, resolve } from "node:path";
30-
import { detectPackageManager, PM_LOCKFILES, type PackageManager } from "./build/detect.js";
30+
import { detectPackageManager, PM_LOCKFILES_ALL, type PackageManager } from "./build/detect.js";
3131
import { detectReferencedPackageManagers } from "./mod/packageManager.js";
3232
import { runShell } from "./process.js";
3333
import { sudo } from "./sudo.js";
@@ -98,9 +98,15 @@ export function pnpmInstallCommand(): string {
9898
return "curl -fsSL https://get.pnpm.io/install.sh | sh -";
9999
}
100100

101-
/** yarn's official path: corepack ships with Node and activates a yarn shim. */
102-
export function yarnInstallCommand(): string {
103-
return "corepack enable && corepack prepare yarn@stable --activate";
101+
/**
102+
* yarn's official path: corepack (ships with Node) writes a yarn shim. We point
103+
* `--install-directory` at a user-owned dir instead of corepack's default (the
104+
* Node bin dir), because on a NodeSource Linux install that dir is root-owned
105+
* and a plain `corepack enable` hits EACCES. The caller prepends `installDir`
106+
* to PATH so the shim resolves. Works on macOS and Linux without sudo.
107+
*/
108+
export function yarnInstallCommand(installDir: string): string {
109+
return `corepack enable --install-directory "${installDir}" && corepack prepare yarn@stable --activate`;
104110
}
105111

106112
/**
@@ -157,7 +163,14 @@ const YARN_TOOL: PmTool = {
157163
name: "yarn",
158164
label: "yarn",
159165
check: () => commandExists("yarn"),
160-
install: (onData) => runShell(yarnInstallCommand(), onData, { description: "install yarn" }),
166+
install: async (onData) => {
167+
const binDir = resolve(homedir(), ".corepack/bin");
168+
await runShell(yarnInstallCommand(binDir), onData, { description: "install yarn" });
169+
// corepack wrote the yarn shim into our user-owned --install-directory
170+
// (avoids EACCES on a root-owned NodeSource bin dir); expose it now so the
171+
// very next step resolves `yarn`.
172+
prependPath(binDir);
173+
},
161174
manualHint: "https://yarnpkg.com/getting-started/install",
162175
};
163176

@@ -167,8 +180,10 @@ const BUN_TOOL: PmTool = {
167180
check: () => commandExists("bun"),
168181
install: async (onData) => {
169182
await runShell(bunInstallCommand(), onData, { description: "install bun" });
170-
// bun's installer drops the binary in ~/.bun/bin and edits shell rc files.
171-
prependPath(resolve(homedir(), ".bun/bin"));
183+
// bun's installer writes the binary to $BUN_INSTALL/bin (default ~/.bun),
184+
// and edits shell rc files that don't reach the running process. Honor
185+
// BUN_INSTALL (the installer does) and expose the bin dir now.
186+
prependPath(resolve(process.env.BUN_INSTALL ?? resolve(homedir(), ".bun"), "bin"));
172187
},
173188
manualHint: "https://bun.sh/docs/installation",
174189
};
@@ -219,6 +234,23 @@ export class PackageManagerUnavailableError extends Error {
219234
}
220235
}
221236

237+
// Process-wide single-flight for installs, keyed by tool name. `deploy-all`
238+
// runs builds for several apps concurrently in ONE process; when they share a
239+
// missing PM, every worker would otherwise launch the same installer at once
240+
// (two `get.pnpm.io | sh` runs, or racing `sudo apt`/`dpkg` locks). Collapsing
241+
// concurrent installs of the same tool onto one promise makes that safe.
242+
const inFlightInstalls = new Map<string, Promise<void>>();
243+
244+
function installOnce(tool: PmTool, onData?: (line: string) => void): Promise<void> {
245+
const existing = inFlightInstalls.get(tool.name);
246+
if (existing) return existing;
247+
const p = Promise.resolve(tool.install(onData)).finally(() => {
248+
inFlightInstalls.delete(tool.name);
249+
});
250+
inFlightInstalls.set(tool.name, p);
251+
return p;
252+
}
253+
222254
/** Core orchestration, parameterized on the tool list so it is trivially testable. */
223255
export async function ensurePackageManagerForTools(
224256
pm: PackageManager,
@@ -239,7 +271,23 @@ export async function ensurePackageManagerForTools(
239271

240272
for (const tool of missing) {
241273
opts.onData?.(`> installing ${tool.label}`);
242-
await tool.install(opts.onData);
274+
await installOnce(tool, opts.onData);
275+
}
276+
277+
// Re-verify: an installer can exit 0 yet leave the binary off the running
278+
// process's PATH (e.g. a prependPath target that doesn't match where the
279+
// installer actually wrote). Surface that here with the manual hint, instead
280+
// of letting the next build/setup step die with a confusing "command not
281+
// found" — the exact scary-error class this whole flow exists to kill.
282+
const stillMissing: PmTool[] = [];
283+
for (const tool of missing) {
284+
if (!(await tool.check())) stillMissing.push(tool);
285+
}
286+
if (stillMissing.length > 0) {
287+
const names = stillMissing.map((t) => t.label).join(", ");
288+
throw new Error(
289+
`Installed ${names} but it is still not on PATH. ${packageManagerManualHint(pm, stillMissing)}`,
290+
);
243291
}
244292
return pm;
245293
}
@@ -284,7 +332,7 @@ export function loadPackageManagerSnapshot(projectDir: string): PmSnapshot {
284332
}
285333
}
286334
const lockfiles = new Set<string>();
287-
for (const name of Object.values(PM_LOCKFILES)) {
335+
for (const name of PM_LOCKFILES_ALL) {
288336
if (existsSync(join(projectDir, name))) lockfiles.add(name);
289337
}
290338
const setupPath = join(projectDir, "setup.sh");

0 commit comments

Comments
 (0)