Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions .changeset/fiery-comics-shave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
---
---
4 changes: 2 additions & 2 deletions packages/cli-core/src/commands/init/format.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { detectAvailableRunners, preferredRunner, runnerCommand } from "../../lib/runners.js";
import { isNonEmpty } from "../../lib/helpers/arrays.js";
import type { ProjectContext } from "./frameworks/types.js";

type FormatterConfig = {
Expand Down Expand Up @@ -32,9 +33,8 @@ export async function runFormatters(ctx: ProjectContext, files: string[]): Promi
if (matchingFormatters.length === 0) return;

const available = detectAvailableRunners();
if (available.length === 0) return;
if (!isNonEmpty(available)) return;
const runner = preferredRunner(ctx.packageManager, available);
if (!runner) return;

for (const formatter of matchingFormatters) {
const command = runnerCommand(runner, formatter.binArgs(files));
Expand Down
14 changes: 2 additions & 12 deletions packages/cli-core/src/commands/init/skills.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
runnerCommand,
runnerForPackageManager,
} from "../../lib/runners.js";
import { isNonEmpty } from "../../lib/helpers/arrays.js";
import type { ProjectContext } from "./frameworks/types.js";

/** Skills installed regardless of framework. */
Expand Down Expand Up @@ -87,7 +88,7 @@ export async function installSkills(

// Detect runners after the user accepts — no point probing PATH if they decline.
const available = detectAvailableRunners();
if (available.length === 0) {
if (!isNonEmpty(available)) {
const suggested = runnerForPackageManager(packageManager);
log.blank();
log.warn(
Expand All @@ -98,17 +99,6 @@ export async function installSkills(
}

const preferred = preferredRunner(packageManager, available);
if (!preferred) {
// Defensive: detectAvailableRunners returned a non-empty array above, so
// preferredRunner should always find something. This guards against any
// future change that decouples the two.
const suggested = runnerForPackageManager(packageManager);
log.blank();
log.warn(
`Could not select a package runner. Run \`${suggested.display} skills add ${SKILLS_SOURCE}\` manually.`,
);
return;
}

// Only prompt when there's an actual choice and the user is interactive.
let runner = preferred;
Expand Down
40 changes: 40 additions & 0 deletions packages/cli-core/src/lib/helpers/arrays.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { test, expect, describe } from "bun:test";
import { isNonEmpty, type NonEmptyArray } from "./arrays.ts";

describe("isNonEmpty", () => {
test("returns false for an empty array", () => {
expect(isNonEmpty([])).toBe(false);
});

test("returns true for an array with one element", () => {
expect(isNonEmpty([1])).toBe(true);
});

test("returns true for an array with multiple elements", () => {
expect(isNonEmpty(["a", "b", "c"])).toBe(true);
});

test("works with readonly arrays", () => {
const ro: readonly number[] = [1, 2, 3];
expect(isNonEmpty(ro)).toBe(true);
});

test("narrows the type so [0] is non-undefined", () => {
const arr: number[] = [42];
if (isNonEmpty(arr)) {
// This line is the test — if isNonEmpty didn't narrow, the assignment
// would be `number | undefined` and TS would error under strict mode.
const first: number = arr[0];
expect(first).toBe(42);
} else {
throw new Error("expected isNonEmpty to return true");
}
});

test("NonEmptyArray accepts a tuple literal", () => {
// Compile-time check: this would error if NonEmptyArray rejected a
// single-element tuple.
const one: NonEmptyArray<string> = ["hi"];
expect(one).toHaveLength(1);
});
});
24 changes: 24 additions & 0 deletions packages/cli-core/src/lib/helpers/arrays.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/**
* Generic array helpers and type-narrowing predicates.
*/

/** A readonly array proven to contain at least one element. */
export type NonEmptyArray<T> = readonly [T, ...T[]];

/**
* Type guard: narrows a `readonly T[]` to `NonEmptyArray<T>` when it has
* one or more elements. Use at boundaries where you want the type system
* (rather than a runtime null-check) to prove an array is non-empty.
*
* @example
* ```ts
* const items = await getItems();
* if (!isNonEmpty(items)) {
* return;
* }
* // items is now NonEmptyArray<Item> — items[0] is Item, not Item | undefined
* ```
*/
export function isNonEmpty<T>(arr: readonly T[]): arr is NonEmptyArray<T> {
return arr.length > 0;
}
43 changes: 26 additions & 17 deletions packages/cli-core/src/lib/runners.test.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { test, expect, describe, afterEach } from "bun:test";
import {
KNOWN_RUNNERS,
type Runner,
detectAvailableRunners,
preferredRunner,
runnerCommand,
runnerForPackageManager,
} from "./runners.ts";
import { isNonEmpty } from "./helpers/arrays.ts";

// Bun.which / Bun.spawnSync are native globals. We patch them directly the
// same way commands/auth/login.test.ts patches Bun.spawn — wrapped in
Expand Down Expand Up @@ -121,31 +121,35 @@ describe("preferredRunner", () => {
const pnpm = KNOWN_RUNNERS.find((r) => r.id === "pnpm")!;
const yarn = KNOWN_RUNNERS.find((r) => r.id === "yarn")!;

test("returns undefined when no runners are available", () => {
expect(preferredRunner("bun", [])).toBeUndefined();
expect(preferredRunner(undefined, [])).toBeUndefined();
});
// preferredRunner now requires a NonEmptyArray<Runner>, so the empty-array
// case is unrepresentable in the type system and doesn't need a runtime test.

test("returns the runner matching the project's package manager", () => {
const all = [bunx, npx, pnpm, yarn];
expect(preferredRunner("bun", all)?.id).toBe("bunx");
expect(preferredRunner("npm", all)?.id).toBe("npx");
expect(preferredRunner("pnpm", all)?.id).toBe("pnpm");
expect(preferredRunner("yarn", all)?.id).toBe("yarn");
const all = [bunx, npx, pnpm, yarn] as const;
expect(preferredRunner("bun", all).id).toBe("bunx");
expect(preferredRunner("npm", all).id).toBe("npx");
expect(preferredRunner("pnpm", all).id).toBe("pnpm");
expect(preferredRunner("yarn", all).id).toBe("yarn");
});

test("falls back to first available when the preferred pm runner is missing", () => {
expect(preferredRunner("bun", [npx, pnpm])?.id).toBe("npx");
expect(preferredRunner("yarn", [pnpm])?.id).toBe("pnpm");
// Project is bun but only npx is on PATH → fall back to npx (first
// available, which RUNNERS orders as bunx > npx > pnpm > yarn).
expect(preferredRunner("bun", [npx, pnpm] as const).id).toBe("npx");
// Project is yarn but only pnpm is on PATH → fall back to pnpm.
expect(preferredRunner("yarn", [pnpm] as const).id).toBe("pnpm");
});

test("returns first available when no package manager is given", () => {
expect(preferredRunner(undefined, [npx, pnpm, yarn])?.id).toBe("npx");
expect(preferredRunner(undefined, [yarn])?.id).toBe("yarn");
expect(preferredRunner(undefined, [npx, pnpm, yarn] as const).id).toBe("npx");
expect(preferredRunner(undefined, [yarn] as const).id).toBe("yarn");
});

test("preserves KNOWN_RUNNERS preference order in fallback", () => {
expect(preferredRunner(undefined, KNOWN_RUNNERS)?.id).toBe("bunx");
// Even with all four available, no pm hint → bunx wins (it's first in KNOWN_RUNNERS).
// KNOWN_RUNNERS isn't typed as NonEmptyArray, so we narrow via isNonEmpty.
if (!isNonEmpty(KNOWN_RUNNERS)) throw new Error("unreachable");
expect(preferredRunner(undefined, KNOWN_RUNNERS).id).toBe("bunx");
});
});

Expand Down Expand Up @@ -186,6 +190,8 @@ describe("detectAvailableRunners", () => {
});

test("preserves KNOWN_RUNNERS order in the output", () => {
// Even though we list yarn first in the set, the result should still
// be in KNOWN_RUNNERS preference order (bunx > npx > pnpm > yarn).
mockWhich(new Set(["yarn", "bunx", "npx"]));
mockSpawnSync(0);
const result = detectAvailableRunners();
Expand Down Expand Up @@ -217,10 +223,13 @@ describe("detectAvailableRunners", () => {
test("integrates cleanly with preferredRunner + runnerCommand", () => {
mockWhich(new Set(["npx", "pnpm"]));
const available = detectAvailableRunners();
if (!isNonEmpty(available)) throw new Error("expected at least one runner");
// After the isNonEmpty narrowing, preferredRunner returns Runner (no
// undefined) and `runner` doesn't need a cast.
const runner = preferredRunner("pnpm", available);
expect(runner?.id).toBe("pnpm");
expect(runner.id).toBe("pnpm");

const command = runnerCommand(runner as Runner, ["prettier", "--write", "src/x.ts"]);
const command = runnerCommand(runner, ["prettier", "--write", "src/x.ts"]);
expect(command).toEqual(["pnpm", "dlx", "prettier", "--write", "src/x.ts"]);
});
});
10 changes: 6 additions & 4 deletions packages/cli-core/src/lib/runners.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

import type { ProjectContext } from "../commands/init/frameworks/types.js";
import type { NonEmptyArray } from "./helpers/arrays.js";

/**
* One way to invoke an npm-published binary without installing it globally.
Expand Down Expand Up @@ -106,13 +107,14 @@ export function runnerForPackageManager(
* bunx). Otherwise falls back to the first available runner in {@link KNOWN_RUNNERS}
* order.
*
* Returns `undefined` only if `available` is empty.
* Requires a {@link NonEmptyArray} so the return type can be `Runner` (never
* undefined). Callers prove non-emptiness via `isNonEmpty()` from
* `lib/helpers/arrays.ts`.
*/
export function preferredRunner(
packageManager: ProjectContext["packageManager"] | undefined,
available: readonly Runner[],
): Runner | undefined {
if (available.length === 0) return undefined;
available: NonEmptyArray<Runner>,
): Runner {
if (packageManager) {
const preferredId = PM_TO_RUNNER[packageManager];
const match = available.find((r) => r.id === preferredId);
Expand Down
Loading