Refuse an occupied destination before asking, not after

.changeset/lucky-moons-shake.md

@@ -14,4 +14,6 @@ Git and the GitHub Actions workflows are questions too, defaulted yes and no res
 
 `--ci` is gone. It read as "I am running in CI" — which is what it means in `npm ci`, in Jest, and in this CLI's own `isCI` check — while actually meaning "emit GitHub Actions workflows". It is now `--workflows` / `--no-workflows`, and the subcommand is `add workflows`. `add ci` still works, undocumented, because it is written into the README of every project already scaffolded.
 
+An occupied destination is refused **before** the first question rather than by `scaffoldGame` after the last one. Being told the directory was taken after carefully giving an organization is the same insult as being told about `flutterfire` after two minutes of Flutter and pub. The message names the first three things standing in the way, which usually settles whether the path was mistyped or already scaffolded once — where "not empty" only invites an `ls`. There is deliberately no offer to delete it: every other question here is reversible and that one is not. A directory that exists and is *empty* is no longer refused at all, since people make one out of habit and cloning a repository with no first commit leaves exactly that.
+
 Also adds `--version`, and stops asking about git at all when the destination is already inside a repository, since the scaffold declines to nest one there whatever it is told.

packages/create-eigen-game/src/cli.ts

@@ -5,7 +5,7 @@ import { fileURLToPath } from "node:url";
 import { parseArgs } from "node:util";
 import { cancel, intro, isCI, isTTY, log, note, outro, taskLog } from "@clack/prompts";
 import color from "picocolors";
-import { addContinuousIntegration, applicationId, detectPackageManager, type FirebaseProblem, firebaseReadiness, insideWorkTree, normaliseTerminalWidth, type PackageManager, plainReporter, type Reporter, scaffoldGame } from "./index.js";
+import { addContinuousIntegration, applicationId, destinationProblem, detectPackageManager, type FirebaseProblem, firebaseReadiness, insideWorkTree, normaliseTerminalWidth, type PackageManager, plainReporter, type Reporter, scaffoldGame } from "./index.js";
 import { askForGit, askForOrg, askForPackageManager, askForWorkflows, askToScaffoldWithoutFirebase, DEFAULT_ORG, ORG } from "./prompt.js";
 import { summarise } from "./summary.js";
 
@@ -292,12 +292,18 @@ async function main(args: string[]): Promise<void> {
 
   const directory = positionals[0];
 
-  greet();
-
-  // `applicationId` validates the slug, so a destination that cannot be a game
-  // name fails before the first question rather than after the last one.
+  // Everything that makes the command impossible, before the greeting rather
+  // than after the last question. `applicationId` validates the slug;
+  // `destinationProblem` is the one `scaffoldGame` would raise at the far end,
+  // asked here instead — being told the directory was occupied *after* giving
+  // an organization is the same insult as being told about `flutterfire` after
+  // two minutes of Flutter and pub.
   const game = basename(resolve(directory));
   applicationId(directory);
+  const occupied = destinationProblem(directory);
+  if (occupied !== undefined) throw new Error(occupied);
+
+  greet();
 
   // Every question this run will ask, settled before any of it starts. A flag
   // is an answer already given; anything left over is asked, or — with nothing

packages/create-eigen-game/src/index.ts

@@ -1,5 +1,5 @@
 import { execFileSync } from "node:child_process";
-import { appendFileSync, cpSync, existsSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, renameSync, rmSync, writeFileSync } from "node:fs";
+import { appendFileSync, cpSync, existsSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, renameSync, rmdirSync, rmSync, statSync, writeFileSync } from "node:fs";
 import { basename, dirname, relative, resolve } from "node:path";
 import { fileURLToPath } from "node:url";
 import { ENGINE_PACKAGE, engineRange } from "./engine-range.js";
@@ -626,6 +626,43 @@ function signedOut(probe: Probe): boolean {
  * declines to nest a repository — but to stop the CLI asking a question whose
  * answer cannot matter.
  */
+/**
+ * Why this destination cannot be scaffolded into, said as a whole message, or
+ * `undefined` when there is nothing in the way.
+ *
+ * A directory that exists and is *empty* is not in the way. People make one out
+ * of habit, and cloning a repository that has not had its first commit yet
+ * leaves exactly that — so it is removed and rewritten rather than refused over
+ * nothing. Anything with contents in it is refused.
+ *
+ * Refused, and deliberately not offered as a "remove it and continue?"
+ * question, which is where several scaffolders in this family go. This one
+ * writes ninety-odd files and runs `flutter create`; the cost of getting that
+ * answer wrong against a mistyped path has no upper bound, and `rm -rf` is one
+ * command that belongs to the person who can see what is in there. Every other
+ * question this CLI asks is reversible. This one would not be.
+ *
+ * Exported so the CLI can ask it before the first question rather than after
+ * the last one — {@link scaffoldGame} asks it again, for callers that are not
+ * the CLI.
+ */
+export function destinationProblem(directory: string): string | undefined {
+  const root = resolve(directory);
+  if (!existsSync(root)) return undefined;
+
+  if (!statSync(root).isDirectory()) return `${root} already exists, and is a file.`;
+
+  const entries = readdirSync(root);
+  if (entries.length === 0) return undefined;
+
+  // Named, not counted. "not empty" invites an `ls`; the three that are
+  // actually there usually settle whether this was the wrong path or a
+  // directory that has already been scaffolded once.
+  const shown = entries.slice(0, 3).sort().join(", ");
+  const rest = entries.length > 3 ? `, and ${entries.length - 3} more` : "";
+  return `${root} already exists, and is not empty — it holds ${shown}${rest}.\n\nScaffold somewhere else, or clear it out yourself. This will not delete a directory you already had.`;
+}
+
 export function insideWorkTree(directory: string, probe: Probe = probeCommand): boolean {
   let candidate = resolve(directory);
   while (!existsSync(candidate)) {
@@ -740,7 +777,12 @@ export function scaffoldGame(options: ScaffoldOptions): ScaffoldResult {
   const reporter = options.reporter ?? plainReporter;
   const run = options.run ?? capturingRunner(reporter);
 
-  if (existsSync(root)) throw new Error(`target already exists: ${root}`);
+  const occupied = destinationProblem(root);
+  if (occupied !== undefined) throw new Error(occupied);
+  // Empty, or `destinationProblem` would have said so. Removed rather than
+  // scaffolded into, because the rename below publishes the staging directory
+  // *as* this path and cannot do that while something is standing there.
+  if (existsSync(root)) rmdirSync(root);
 
   mkdirSync(dirname(root), { recursive: true });
 

packages/create-eigen-game/test/scaffold.spec.ts

@@ -4,7 +4,7 @@ import { tmpdir } from "node:os";
 import { relative, resolve } from "node:path";
 import { buildGameContract } from "@eigeninteractive/testkit";
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
-import { addContinuousIntegration, capturingRunner, decodeUtf8, detectPackageManager, firebaseReadiness, insideWorkTree, normaliseTerminalWidth, type Probe, type Reporter, scaffoldGame } from "../src/index.js";
+import { addContinuousIntegration, capturingRunner, decodeUtf8, destinationProblem, detectPackageManager, firebaseReadiness, insideWorkTree, normaliseTerminalWidth, type Probe, type Reporter, scaffoldGame } from "../src/index.js";
 import gameModule from "../templates/worker/src/module/index.js";
 
 const temporaryParent = (): string => mkdtempSync(resolve(tmpdir(), "create-eigen-game-"));
@@ -21,6 +21,27 @@ const temporaryParent = (): string => mkdtempSync(resolve(tmpdir(), "create-eige
 const expectedEngineRange = `^${(JSON.parse(readFileSync(resolve(import.meta.dirname, "../../server/package.json"), "utf8")) as { version: string }).version}`;
 
 describe("scaffoldGame", () => {
+  it("scaffolds into a directory that exists but is empty", () => {
+    const root = resolve(temporaryParent(), "my-game");
+    mkdirSync(root);
+
+    scaffoldGame({ directory: root, bootstrap: false, packageManager: "npm" });
+
+    expect(existsSync(resolve(root, "server/package.json"))).toBe(true);
+  });
+
+  it("refuses a directory with anything in it, and leaves it exactly as it was", () => {
+    const root = resolve(temporaryParent(), "my-game");
+    mkdirSync(root);
+    writeFileSync(resolve(root, "notes.txt"), "mine");
+
+    expect(() => scaffoldGame({ directory: root, bootstrap: false, packageManager: "npm" })).toThrow(/already exists, and is not empty/);
+    // The refusal is the whole feature: nothing here is worth less than a
+    // scaffold, and only the person who can see it knows that.
+    expect(readdirSync(root)).toEqual(["notes.txt"]);
+    expect(readFileSync(resolve(root, "notes.txt"), "utf8")).toBe("mine");
+  });
+
   it("renders the canonical templates as a combined repository", () => {
     const root = resolve(temporaryParent(), "my-game");
 
@@ -429,6 +450,46 @@ describe("firebaseReadiness", () => {
   });
 });
 
+describe("destinationProblem", () => {
+  it("has nothing to say about a destination that does not exist", () => {
+    expect(destinationProblem(resolve(temporaryParent(), "my-game"))).toBeUndefined();
+  });
+
+  it("treats an existing but empty directory as free", () => {
+    // Made out of habit, or left by cloning a repository with no first commit
+    // in it. Refusing over nothing would be its own papercut.
+    const root = resolve(temporaryParent(), "my-game");
+    mkdirSync(root);
+
+    expect(destinationProblem(root)).toBeUndefined();
+  });
+
+  it("names what is in the way rather than saying it is not empty", () => {
+    const root = resolve(temporaryParent(), "my-game");
+    mkdirSync(root);
+    for (const entry of ["README.md", "app", "server", "notes.txt"]) writeFileSync(resolve(root, entry), "");
+
+    const problem = destinationProblem(root);
+
+    // Three of them, because that usually settles whether this was a mistyped
+    // path or a directory that has already been scaffolded once — where "not
+    // empty" only invites an `ls`.
+    expect(problem).toContain("README.md, app, notes.txt");
+    expect(problem).toContain("and 1 more");
+    // No offer to delete it: this run would write ninety files and call
+    // `flutter create`, and the answer being wrong against a mistyped path
+    // costs more than any other question here.
+    expect(problem).toContain("will not delete a directory you already had");
+  });
+
+  it("catches a destination that is a file", () => {
+    const root = resolve(temporaryParent(), "my-game");
+    writeFileSync(root, "");
+
+    expect(destinationProblem(root)).toContain("is a file");
+  });
+});
+
 describe("insideWorkTree", () => {
   it("asks the nearest ancestor that exists, since the destination does not yet", () => {
     const parent = temporaryParent();