refactor(init): use runner detection in format.ts

[wyattjoh]

Summary

• Replaces hardcoded ["npx", "prettier", ...] arrays in init/format.ts with the runner detection from lib/runners.ts. A bun project with no npx installed (e.g. Bun via Homebrew but no Node) now falls back to bunx instead of silently failing to format.
• Wraps the Bun.spawn call in try/catch, formatting is best-effort and shouldn't break init even if the runner subprocess crashes.
• Signature change: runFormatters now takes the full ProjectContext instead of just cwd, so it can read packageManager. Updates the lone caller in init/index.ts.

Stacked on #118.

Test plan

• bun run test passes (59 passed)
• Manual: on a bun-only machine without npx, run clerk init in a Prettier-using project and confirm formatting still runs (via bunx)

[wyattjoh]

Stack: init-skills

• refactor(init): use runner detection in format.ts #124 👈 this PR
  • feat(lib): add NonEmptyArray helper, tighten preferredRunner #125
    • feat: Clerk CLI Skill #126
      • feat(init): support CLERK_SKILL_SOURCE env override #127

Part of a stacked PR chain. Do not merge manually.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7a82fa97-fb78-41aa-8890-4d28eb5bc676

📥 Commits

Reviewing files that changed from the base of the PR and between 3a3375e and f7ab840.

📒 Files selected for processing (3)

• packages/cli-core/src/commands/init/format.test.ts
• packages/cli-core/src/commands/init/format.ts
• packages/cli-core/src/commands/init/index.ts

✅ Files skipped from review due to trivial changes (1)

• packages/cli-core/src/commands/init/index.ts

---

📝 Walkthrough

Walkthrough

runFormatters signature changed to accept a ProjectContext (ctx) and uses ctx.cwd and ctx.deps (falling back to reading package deps). Formatters are pre-filtered by dependency presence. Formatter configs now expose runner/args separately (binArgs) and no longer hardcode npx; available runners are detected and a preferred runner is chosen via preferredRunner(ctx.packageManager, available). Runner command and formatter args are composed into spawn invocations; spawns run with the project cwd and are wrapped in try/catch to swallow spawn errors while ignoring stdout/stderr. The caller in init/index.ts now passes ctx. A new Bun test suite for runFormatters was added.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main refactoring: switching to runner detection in format.ts, which aligns with the core change of replacing hardcoded 'npx' with dynamic runner selection.
Description check	✅ Passed	The description is directly related to the changeset, explaining the rationale for replacing hardcoded runner commands, wrapping spawn in try/catch, and the signature change to accept ProjectContext.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

missclick

[rafa-thayto]

Some comments

[rafa-thayto]

Silent mock failures make tests untrustworthy

The try/catch blocks silently swallow mock-assignment failures. If Bun makes these properties non-writable in a future version, every test runs against real globals with confusing results rather than failing fast.

Same issue in mockWhich (line 42) and mockSpawnSync (line 55) and all three restore* functions.

Suggestion — drop all try/catch blocks, verify the assignment took effect:

function setSpawn(impl: SpawnImpl) {
  (Bun as unknown as { spawn: SpawnImpl }).spawn = impl;
  if (Bun.spawn !== (impl as unknown)) {
    throw new Error("Failed to mock Bun.spawn — property may be non-writable");
  }
}

function restoreSpawn() {
  (Bun as unknown as { spawn: typeof Bun.spawn }).spawn = origSpawn;
}

Bonus — extract the cast once to reduce boilerplate (repeated 6× as-is):

const bunOverrides = Bun as unknown as {
  spawn: SpawnImpl;
  which: (bin: string) => string | null;
  spawnSync: (cmd: string[]) => { exitCode: number };
};

function setSpawn(impl: SpawnImpl) {
  bunOverrides.spawn = impl;
}
function restoreSpawn() {
  bunOverrides.spawn = origSpawn as unknown as SpawnImpl;
}
// ... etc

[rafa-thayto]

This reads like commit-message rationale rather than a code comment. The function-level docstring already covers the best-effort / silent-failure contract, and the import names (detectAvailableRunners, preferredRunner) are self-documenting.

Suggestion: delete this comment block entirely — the PR description is the right place for the "why we're switching" context.

[rafa-thayto]

Two things on this line:

1. ctx.deps && is dead code — deps is typed Record<string, string> (not optional), and gatherContext always sets it to deps ?? {}. An empty object is truthy, so the && never short-circuits. Simplify to:

const deps = Object.keys(ctx.deps).length > 0 ? ctx.deps : await readDeps(ctx.cwd);

2. The disk fallback itself is redundant in practice — gatherContext() already calls readDeps(cwd) and stores the result. If it returned null, ctx.deps becomes {}. Re-calling readDeps here will return the same null. The only scenario this fallback helps is when someone constructs a ProjectContext manually with deps: {} while a real package.json exists — which only happens in the test for this very code path.

Consider simplifying to just trust ctx.deps:

export async function runFormatters(ctx: ProjectContext, files: string[]): Promise<void> {
  if (files.length === 0) return;
  if (Object.keys(ctx.deps).length === 0) return;

  const matchingFormatters = FORMATTERS.filter((f) => f.pkg in ctx.deps);
  ...

Then update the "reads deps from disk when ctx.deps is empty" test to pass deps: { prettier: "3.0.0" } directly, and drop the "no-op when ctx.deps is empty and package.json is missing" test (already covered by "no-op when no supported formatter is in deps").

[rafa-thayto]

Missing test: non-zero formatter exit code

Tests cover spawn throwing (swallowed correctly) and spawn succeeding (exit 0), but there's no test for a formatter exiting non-zero. The current code silently ignores exit codes (await proc.exited with no check), which is the right best-effort behavior — worth locking in with a test:

test("ignores non-zero exit code from formatter (best-effort)", async () => {
  setSpawn((cmd) => {
    spawnCalls.push(cmd);
    return { exited: Promise.resolve(1) };
  });
  const ctx = makeCtx({
    cwd: tempDir,
    packageManager: "bun",
    deps: { prettier: "3.0.0", "@biomejs/biome": "1.9.0" },
  });
  await runFormatters(ctx, ["x.ts"]);
  // Both formatters attempted despite prettier exiting non-zero
  expect(spawnCalls).toHaveLength(2);
});