From f5b01bf95fa76a2e5f9fe376fb9217bf11791f90 Mon Sep 17 00:00:00 2001 From: alfredorubin96 Date: Tue, 28 Jul 2026 14:36:01 +0200 Subject: [PATCH 1/3] fix(cli): make the published package work outside the monorepo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit release.yml publishes @neoboard/cli on every `v*` tag, so the first tag makes `npx @neoboard/cli setup` real for strangers. It failed immediately, twice over. **Root detection required a checkout.** findProjectRoot walked up from the CLI's own install location looking for the monorepo; under npx that is an npm cache directory, and it THREW. A throw that deep in a path helper surfaced as an unrelated-looking crash — setup died in root detection, reporting nothing about standalone mode. It now returns null and resolveRoot decides: the checkout when there is one, else $NEOBOARD_DIR, else ./neoboard, created on demand. The monorepo always wins, so a stray NEOBOARD_DIR cannot silently redirect a contributor. **The package shipped nothing to start.** `files` listed only dist/README/ LICENSE. Adding "docker" to it does NOTHING — npm resolves `files` relative to the PACKAGE root and docker/ lives one level up, at the REPO root. npm pack succeeds and ships no compose file, silently. That matters more than the fix: my first packaging test asserted `pkg.files.includes("docker")`. It passed. It proved nothing, and would have let this ship a second time. The test now runs `npm pack --dry-run --json` and asserts on what would actually be published. A prepack step stages the four assets a standalone install can use into cli/docker/ (gitignored, removed by postpack). **Commands that genuinely need the source say so.** dev, db seed and the plugin commands run scripts/ tooling that is deliberately not shipped — 568K to make one command work is the wrong trade. assertCheckout names the command, gives the clone commands, and lists what does work standalone. **Errors no longer surface as stack traces.** program.parse() left a rejected action to Node's unhandled-rejection dump: the message buried under a trace rooted in dist/, reading as a crash rather than as the CLI telling you something. parseAsync().catch prints the message and exits 1. Two existing tests asserted the throw that is now a null. The #991 one still proves what it was written for — a non-terminating loop would hang rather than return null — so only the assertion changed, and the comment says so. Verified against a real packed tarball installed outside the monorepo: neoboard --help works neoboard config list works, creates ./neoboard/ NEOBOARD_DIR=... config list honours the override neoboard status exit 0 neoboard dev clean message, exit 1, no stack node cli/dist/index.js dev in-monorepo path unaffected tarball contains docker/docker-compose.prod-full.yml Also fixes the prepack script logging to stdout, which corrupted `npm pack --json` for every consumer, not just this test. Closes #1315 Co-Authored-By: Claude Opus 5 --- .gitignore | 3 + cli/package.json | 5 +- cli/scripts/stage-compose.mjs | 40 ++++ cli/src/__tests__/commands/config.test.ts | 1 + cli/src/__tests__/commands/db/dump.test.ts | 1 + cli/src/__tests__/commands/db/migrate.test.ts | 1 + cli/src/__tests__/commands/db/reset.test.ts | 1 + cli/src/__tests__/commands/db/seed.test.ts | 1 + cli/src/__tests__/commands/demo.test.ts | 1 + cli/src/__tests__/commands/dev.test.ts | 1 + cli/src/__tests__/commands/doctor.test.ts | 1 + cli/src/__tests__/commands/env.test.ts | 1 + cli/src/__tests__/commands/init.test.ts | 1 + cli/src/__tests__/commands/logs.test.ts | 1 + .../__tests__/commands/plugin-hints.test.ts | 1 + cli/src/__tests__/commands/plugin.test.ts | 1 + cli/src/__tests__/commands/start.test.ts | 1 + cli/src/__tests__/commands/status.test.ts | 1 + .../__tests__/lib/bootstrap-status.test.ts | 1 + cli/src/__tests__/lib/config.test.ts | 18 +- .../__tests__/lib/credential-probe.test.ts | 1 + cli/src/__tests__/lib/docker-env.test.ts | 1 + cli/src/__tests__/lib/docker.test.ts | 1 + cli/src/__tests__/lib/standalone.test.ts | 176 ++++++++++++++++++ cli/src/commands/db/seed.ts | 3 +- cli/src/commands/dev.ts | 3 +- cli/src/commands/plugin.ts | 11 +- cli/src/index.ts | 10 +- cli/src/lib/config.ts | 62 +++++- 29 files changed, 331 insertions(+), 19 deletions(-) create mode 100644 cli/scripts/stage-compose.mjs create mode 100644 cli/src/__tests__/lib/standalone.test.ts diff --git a/.gitignore b/.gitignore index c8c34c1f..7865e295 100644 --- a/.gitignore +++ b/.gitignore @@ -94,3 +94,6 @@ videos/ # Per-install secrets for the full-stack compose (generated by the CLI, #970) docker/.env + +# Staged by cli/scripts/stage-compose.mjs at prepack; never committed (#1315) +cli/docker/ diff --git a/cli/package.json b/cli/package.json index 09fee11b..2d932ea2 100644 --- a/cli/package.json +++ b/cli/package.json @@ -28,6 +28,7 @@ }, "files": [ "dist", + "docker", "README.md", "LICENSE" ], @@ -41,7 +42,9 @@ "test": "SKIP_INTEGRATION=1 vitest run", "test:integration": "vitest run src/__tests__/integration", "test:watch": "vitest", - "test:coverage": "SKIP_INTEGRATION=1 vitest run --coverage" + "test:coverage": "SKIP_INTEGRATION=1 vitest run --coverage", + "prepack": "node scripts/stage-compose.mjs", + "postpack": "rm -rf docker" }, "dependencies": { "chalk": "^5.0.0", diff --git a/cli/scripts/stage-compose.mjs b/cli/scripts/stage-compose.mjs new file mode 100644 index 00000000..e9b5a504 --- /dev/null +++ b/cli/scripts/stage-compose.mjs @@ -0,0 +1,40 @@ +#!/usr/bin/env node +/** + * Copy the compose files the standalone CLI needs into the package directory, + * so `npm pack` can include them. + * + * npm's `files` is resolved relative to the PACKAGE root, and `docker/` lives + * at the REPO root — one level up. Listing "docker" in `files` therefore does + * nothing at all, silently: `npm pack` succeeds, ships no compose file, and a + * manifest-only test asserting `files.includes("docker")` passes while proving + * nothing. That is how #1315's second half would have shipped twice. + * + * Run by `prepack`, removed by `postpack`. `cli/docker/` is gitignored. + */ +import { mkdirSync, copyFileSync, rmSync } from "node:fs"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; + +const cliDir = dirname(dirname(fileURLToPath(import.meta.url))); +const repoDocker = join(dirname(cliDir), "docker"); +const dest = join(cliDir, "docker"); + +// Only what a standalone install can actually use. prod-full pulls +// ghcr.io/... rather than building from source, which a standalone user has +// none of; the dev compose files reference build contexts that will not exist. +const FILES = [ + "docker-compose.prod-full.yml", + "docker-compose.prod.yml", + "neo4j/init.cypher", + "postgres/init.sql", +]; + +rmSync(dest, { recursive: true, force: true }); +for (const rel of FILES) { + const to = join(dest, rel); + mkdirSync(dirname(to), { recursive: true }); + copyFileSync(join(repoDocker, rel), to); +} +// stderr, not stdout: prepack output is interleaved with `npm pack --json`, +// and a stray line there makes the JSON unparseable for every consumer. +console.error(`staged ${FILES.length} compose assets into cli/docker/`); diff --git a/cli/src/__tests__/commands/config.test.ts b/cli/src/__tests__/commands/config.test.ts index 7150abb7..18172a0d 100644 --- a/cli/src/__tests__/commands/config.test.ts +++ b/cli/src/__tests__/commands/config.test.ts @@ -8,6 +8,7 @@ const mockConfig = { }; vi.mock("../../lib/config.js", () => ({ + assertCheckout: vi.fn(), paths: { projectConfig: "/project/neoboard.config.json" }, readProjectConfig: vi.fn(() => ({ ...mockConfig })), writeProjectConfig: vi.fn(), diff --git a/cli/src/__tests__/commands/db/dump.test.ts b/cli/src/__tests__/commands/db/dump.test.ts index 1e3065dd..38ba5cd9 100644 --- a/cli/src/__tests__/commands/db/dump.test.ts +++ b/cli/src/__tests__/commands/db/dump.test.ts @@ -7,6 +7,7 @@ vi.mock("node:child_process", () => ({ })); vi.mock("../../../lib/config.js", () => ({ + assertCheckout: vi.fn(), paths: { root: "/project" }, readProjectConfig: vi.fn(() => ({ ports: { postgres: 5432 }, diff --git a/cli/src/__tests__/commands/db/migrate.test.ts b/cli/src/__tests__/commands/db/migrate.test.ts index bf0e09ca..d4896833 100644 --- a/cli/src/__tests__/commands/db/migrate.test.ts +++ b/cli/src/__tests__/commands/db/migrate.test.ts @@ -20,6 +20,7 @@ vi.mock("../../../lib/exec.js", () => ({ })); vi.mock("../../../lib/config.js", () => ({ + assertCheckout: vi.fn(), paths: { journalPath: "/project/app/drizzle/migrations/meta/_journal.json", appDir: "/project/app", diff --git a/cli/src/__tests__/commands/db/reset.test.ts b/cli/src/__tests__/commands/db/reset.test.ts index df4d39b0..102515dc 100644 --- a/cli/src/__tests__/commands/db/reset.test.ts +++ b/cli/src/__tests__/commands/db/reset.test.ts @@ -13,6 +13,7 @@ vi.mock("../../../lib/exec.js", () => ({ })); vi.mock("../../../lib/config.js", () => ({ + assertCheckout: vi.fn(), paths: { envFile: "/project/app/.env.local" }, readProjectConfig: vi.fn(() => ({ postgres: { user: "neoboard", password: "neoboard", database: "neoboard" }, diff --git a/cli/src/__tests__/commands/db/seed.test.ts b/cli/src/__tests__/commands/db/seed.test.ts index afce6b33..949fc6f3 100644 --- a/cli/src/__tests__/commands/db/seed.test.ts +++ b/cli/src/__tests__/commands/db/seed.test.ts @@ -11,6 +11,7 @@ vi.mock("../../../lib/exec.js", () => ({ })); vi.mock("../../../lib/config.js", () => ({ + assertCheckout: vi.fn(), paths: { root: "/project" }, getMode: vi.fn(() => "docker"), readProjectConfig: vi.fn(() => ({ diff --git a/cli/src/__tests__/commands/demo.test.ts b/cli/src/__tests__/commands/demo.test.ts index 3fb2a20f..0d72fc43 100644 --- a/cli/src/__tests__/commands/demo.test.ts +++ b/cli/src/__tests__/commands/demo.test.ts @@ -29,6 +29,7 @@ vi.mock("../../lib/prompt.js", () => ({ })); vi.mock("../../lib/config.js", () => ({ + assertCheckout: vi.fn(), paths: { root: "/repo" }, getMode: vi.fn(() => "local"), readProjectConfig: vi.fn(() => ({ ports: { app: 3000 } })), diff --git a/cli/src/__tests__/commands/dev.test.ts b/cli/src/__tests__/commands/dev.test.ts index 678082f9..8fdd6e72 100644 --- a/cli/src/__tests__/commands/dev.test.ts +++ b/cli/src/__tests__/commands/dev.test.ts @@ -8,6 +8,7 @@ vi.mock("../../lib/exec.js", () => ({ })); vi.mock("../../lib/config.js", () => ({ + assertCheckout: vi.fn(), paths: { appDir: "/project/app" }, getMode: vi.fn(() => "local"), readProjectConfig: vi.fn(() => ({ diff --git a/cli/src/__tests__/commands/doctor.test.ts b/cli/src/__tests__/commands/doctor.test.ts index 2aa5eb58..ca70360f 100644 --- a/cli/src/__tests__/commands/doctor.test.ts +++ b/cli/src/__tests__/commands/doctor.test.ts @@ -9,6 +9,7 @@ vi.mock("../../lib/ports.js", () => ({ })); vi.mock("../../lib/config.js", () => ({ + assertCheckout: vi.fn(), paths: { root: "/project", appDir: "/project/app", diff --git a/cli/src/__tests__/commands/env.test.ts b/cli/src/__tests__/commands/env.test.ts index dae5ec1d..ba27a1aa 100644 --- a/cli/src/__tests__/commands/env.test.ts +++ b/cli/src/__tests__/commands/env.test.ts @@ -13,6 +13,7 @@ vi.mock("node:crypto", () => ({ })); vi.mock("../../lib/config.js", () => ({ + assertCheckout: vi.fn(), paths: { envFile: "/project/app/.env.local", envExample: "/project/.env.example", diff --git a/cli/src/__tests__/commands/init.test.ts b/cli/src/__tests__/commands/init.test.ts index dfe49e28..1a4f5386 100644 --- a/cli/src/__tests__/commands/init.test.ts +++ b/cli/src/__tests__/commands/init.test.ts @@ -10,6 +10,7 @@ vi.mock("../../lib/exec.js", () => ({ })); vi.mock("../../lib/config.js", () => ({ + assertCheckout: vi.fn(), paths: { root: "/project", appDir: "/project/app", diff --git a/cli/src/__tests__/commands/logs.test.ts b/cli/src/__tests__/commands/logs.test.ts index 0ed1a1a2..e2870c73 100644 --- a/cli/src/__tests__/commands/logs.test.ts +++ b/cli/src/__tests__/commands/logs.test.ts @@ -10,6 +10,7 @@ vi.mock("../../lib/docker.js", () => ({ })); vi.mock("../../lib/config.js", () => ({ + assertCheckout: vi.fn(), paths: { root: "/project" }, })); diff --git a/cli/src/__tests__/commands/plugin-hints.test.ts b/cli/src/__tests__/commands/plugin-hints.test.ts index 375f8074..9bde46d6 100644 --- a/cli/src/__tests__/commands/plugin-hints.test.ts +++ b/cli/src/__tests__/commands/plugin-hints.test.ts @@ -18,6 +18,7 @@ import { pathToFileURL } from "node:url"; vi.mock("../../lib/exec.js", () => ({ run: vi.fn(), runFile: vi.fn() })); vi.mock("../../lib/config.js", () => ({ + assertCheckout: vi.fn(), findProjectRoot: vi.fn(() => "/project"), })); diff --git a/cli/src/__tests__/commands/plugin.test.ts b/cli/src/__tests__/commands/plugin.test.ts index c06e6d3e..fb246955 100644 --- a/cli/src/__tests__/commands/plugin.test.ts +++ b/cli/src/__tests__/commands/plugin.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; vi.mock("../../lib/config.js", () => ({ + assertCheckout: vi.fn(), findProjectRoot: vi.fn(() => "/project"), })); diff --git a/cli/src/__tests__/commands/start.test.ts b/cli/src/__tests__/commands/start.test.ts index 089a10b4..57a3be34 100644 --- a/cli/src/__tests__/commands/start.test.ts +++ b/cli/src/__tests__/commands/start.test.ts @@ -12,6 +12,7 @@ vi.mock("../../lib/health.js", () => ({ })); vi.mock("../../lib/config.js", () => ({ + assertCheckout: vi.fn(), readProjectConfig: vi.fn(() => ({ ports: { app: 3000, postgres: 5432, neo4j_http: 7474, neo4j_bolt: 7687 }, })), diff --git a/cli/src/__tests__/commands/status.test.ts b/cli/src/__tests__/commands/status.test.ts index 089d6320..7f78114f 100644 --- a/cli/src/__tests__/commands/status.test.ts +++ b/cli/src/__tests__/commands/status.test.ts @@ -14,6 +14,7 @@ vi.mock("../../lib/exec.js", () => ({ })); vi.mock("../../lib/config.js", () => ({ + assertCheckout: vi.fn(), paths: { journalPath: "/project/app/drizzle/migrations/meta/_journal.json", root: "/project", diff --git a/cli/src/__tests__/lib/bootstrap-status.test.ts b/cli/src/__tests__/lib/bootstrap-status.test.ts index 7311b406..53aec2b8 100644 --- a/cli/src/__tests__/lib/bootstrap-status.test.ts +++ b/cli/src/__tests__/lib/bootstrap-status.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; vi.mock("../../lib/exec.js", () => ({ runOrNull: vi.fn() })); vi.mock("../../lib/config.js", () => ({ + assertCheckout: vi.fn(), readProjectConfig: vi.fn(() => ({ ports: { app: 3000 } })), })); diff --git a/cli/src/__tests__/lib/config.test.ts b/cli/src/__tests__/lib/config.test.ts index e7e32cef..d25d62b5 100644 --- a/cli/src/__tests__/lib/config.test.ts +++ b/cli/src/__tests__/lib/config.test.ts @@ -50,20 +50,24 @@ describe("findProjectRoot", () => { expect(findProjectRoot("/a/b/c")).toBe("/a"); }); - it("throws when no project root found", () => { + it("returns null when no project root is found", () => { + // Was `toThrow`. Changed deliberately in #1315: under `npx`, the CLI lives + // in an npm cache directory with no monorepo above it, and throwing this + // deep in a path helper surfaced as an unrelated-looking crash. Absence is + // a normal state now; only the caller knows whether it is a problem. mockExistsSync.mockReturnValue(false); - expect(() => findProjectRoot("/nowhere")).toThrow( - "Could not find NeoBoard project root", - ); + expect(findProjectRoot("/nowhere")).toBeNull(); }); it("terminates instead of looping when run from a Windows drive root (#991)", () => { // dirname("C:\\") === "C:\\" — the old `while (dir !== "/")` loop // never terminated. The fixed loop stops when dirname stops changing. + // + // Returning null still proves termination: a non-terminating loop would + // hang the test rather than return anything. The assertion changed with + // #1315; what it protects did not. mockExistsSync.mockReturnValue(false); - expect(() => findProjectRoot("C:\\")).toThrow( - "Could not find NeoBoard project root", - ); + expect(findProjectRoot("C:\\")).toBeNull(); }); }); diff --git a/cli/src/__tests__/lib/credential-probe.test.ts b/cli/src/__tests__/lib/credential-probe.test.ts index 0e946c00..6c6172dc 100644 --- a/cli/src/__tests__/lib/credential-probe.test.ts +++ b/cli/src/__tests__/lib/credential-probe.test.ts @@ -7,6 +7,7 @@ vi.mock("../../lib/exec.js", () => ({ })); vi.mock("../../lib/config.js", () => ({ + assertCheckout: vi.fn(), readProjectConfig: vi.fn(() => ({ ports: { app: 3000, postgres: 5432, neo4j_http: 7474, neo4j_bolt: 7687 }, postgres: { user: "neoboard", password: "neoboard", database: "neoboard" }, diff --git a/cli/src/__tests__/lib/docker-env.test.ts b/cli/src/__tests__/lib/docker-env.test.ts index 502295c3..4529baec 100644 --- a/cli/src/__tests__/lib/docker-env.test.ts +++ b/cli/src/__tests__/lib/docker-env.test.ts @@ -13,6 +13,7 @@ vi.mock("node:crypto", () => ({ })); vi.mock("../../lib/config.js", () => ({ + assertCheckout: vi.fn(), paths: { root: "/project" }, getMode: vi.fn(() => "docker"), })); diff --git a/cli/src/__tests__/lib/docker.test.ts b/cli/src/__tests__/lib/docker.test.ts index 9bc95f76..86084b79 100644 --- a/cli/src/__tests__/lib/docker.test.ts +++ b/cli/src/__tests__/lib/docker.test.ts @@ -16,6 +16,7 @@ vi.mock("../../lib/exec.js", () => ({ })); vi.mock("../../lib/config.js", () => ({ + assertCheckout: vi.fn(), paths: { root: "/project", dockerDir: "/project/docker", diff --git a/cli/src/__tests__/lib/standalone.test.ts b/cli/src/__tests__/lib/standalone.test.ts new file mode 100644 index 00000000..ce19d587 --- /dev/null +++ b/cli/src/__tests__/lib/standalone.test.ts @@ -0,0 +1,176 @@ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { execFileSync } from "node:child_process"; + +/** + * The published package is what strangers run. `.github/workflows/release.yml` + * publishes @neoboard/cli on every `v*` tag, so the first tag makes + * `npx @neoboard/cli setup` real — and today it throws before doing anything, + * because root detection walks up from the CLI's own install location looking + * for the monorepo, and under npx that is an npm cache directory (#1315). + * + * These tests run against a REAL temp directory tree rather than a mocked fs: + * the bug is entirely about what exists on disk where, which a mocked + * existsSync would happily lie about. + */ + +let tmp: string; + +async function loadConfig() { + const mod = await import("../../lib/config.js"); + mod._setRootForTesting(null); + return mod; +} + +beforeEach(() => { + tmp = mkdtempSync(join(tmpdir(), "nb-standalone-")); +}); + +afterEach(async () => { + const mod = await import("../../lib/config.js"); + mod._setRootForTesting(null); + delete process.env.NEOBOARD_DIR; + rmSync(tmp, { recursive: true, force: true }); +}); + +describe("findProjectRoot (#1315)", () => { + it("finds the monorepo root when one is above the start dir", () => { + // The contributor path, and the regression risk: every existing command + // depends on this working exactly as it does today. + writeFileSync( + join(tmp, "package.json"), + JSON.stringify({ name: "neoboard" }), + ); + const nested = join(tmp, "cli", "dist", "lib"); + mkdirSync(nested, { recursive: true }); + + return loadConfig().then(({ findProjectRoot }) => { + expect(findProjectRoot(nested)).toBe(tmp); + }); + }); + + it("returns null instead of throwing when there is no monorepo above", async () => { + // Throwing deep in a path helper is why this surfaced as an unrelated + // -looking crash: `neoboard setup` died in root detection, reporting + // nothing about npx or standalone mode. + const nested = join(tmp, "npm", "_npx", "abc123", "node_modules"); + mkdirSync(nested, { recursive: true }); + const { findProjectRoot } = await loadConfig(); + expect(findProjectRoot(nested)).toBeNull(); + }); + + it("ignores a package.json that is not the monorepo root", async () => { + writeFileSync( + join(tmp, "package.json"), + JSON.stringify({ name: "some-other-project" }), + ); + const nested = join(tmp, "node_modules", "@neoboard", "cli"); + mkdirSync(nested, { recursive: true }); + const { findProjectRoot } = await loadConfig(); + expect(findProjectRoot(nested)).toBeNull(); + }); + + it("survives an unparseable package.json on the way up", async () => { + writeFileSync(join(tmp, "package.json"), "{ not json"); + const nested = join(tmp, "a"); + mkdirSync(nested, { recursive: true }); + const { findProjectRoot } = await loadConfig(); + expect(findProjectRoot(nested)).toBeNull(); + }); +}); + +describe("resolveRoot — standalone working directory (#1315)", () => { + it("uses NEOBOARD_DIR when set, creating it", async () => { + const dir = join(tmp, "custom-workdir"); + process.env.NEOBOARD_DIR = dir; + const { resolveRoot, isStandalone } = await loadConfig(); + expect(resolveRoot(join(tmp, "nowhere"))).toBe(dir); + expect(isStandalone(join(tmp, "nowhere"))).toBe(true); + }); + + it("prefers the monorepo root over NEOBOARD_DIR when inside a checkout", async () => { + // A contributor with NEOBOARD_DIR exported for another install must not + // have their checkout silently redirected. + writeFileSync( + join(tmp, "package.json"), + JSON.stringify({ name: "neoboard" }), + ); + const nested = join(tmp, "cli", "dist"); + mkdirSync(nested, { recursive: true }); + process.env.NEOBOARD_DIR = join(tmp, "elsewhere"); + + const { resolveRoot, isStandalone } = await loadConfig(); + expect(resolveRoot(nested)).toBe(tmp); + expect(isStandalone(nested)).toBe(false); + }); + + it("falls back to ./neoboard in the current directory", async () => { + const { resolveRoot } = await loadConfig(); + const resolved = resolveRoot(join(tmp, "npm-cache")); + expect(resolved).toBe(join(process.cwd(), "neoboard")); + }); +}); + +describe("assertCheckout (#1315)", () => { + it("passes silently inside a checkout", async () => { + // The CLI's own module lives inside this monorepo when the tests run, so + // isStandalone() is genuinely false here — no mocking needed. + const { assertCheckout } = await loadConfig(); + expect(() => assertCheckout("dev")).not.toThrow(); + }); + + it("names the command and points at the clone, not a missing file", async () => { + // Without this, `dev` and `db seed` fail on a missing script deep inside + // the command, which reads as a bug rather than "not for this install". + const { assertCheckout } = await loadConfig(); + const outside = join(tmp, "npx-cache"); + mkdirSync(outside, { recursive: true }); + + expect(() => assertCheckout("db seed", outside)).toThrow(/db seed/); + expect(() => assertCheckout("db seed", outside)).toThrow(/git clone/); + // Says what DOES work, so the reader is not left guessing. + expect(() => assertCheckout("db seed", outside)).toThrow( + /setup, start, stop/, + ); + }); +}); + +describe("packaged files (#1315)", () => { + // Asserting on `package.json.files` is NOT enough, and believing it was is + // how this would have shipped broken twice. npm resolves `files` relative to + // the PACKAGE root; `docker/` lives at the REPO root, one level up. Listing + // "docker" there does nothing, silently — npm pack succeeds and ships no + // compose file, while a manifest test passes. + // + // So this runs the real `npm pack --dry-run` and reads what would actually + // be published. + const packedFiles = (): string[] => { + const cliDir = new URL("../../..", import.meta.url).pathname; + const out = execFileSync("npm", ["pack", "--dry-run", "--json"], { + cwd: cliDir, + encoding: "utf8", + stdio: ["pipe", "pipe", "pipe"], + }); + return JSON.parse(out)[0].files.map((f: { path: string }) => f.path); + }; + + it("ships a compose file a standalone install can start from", () => { + const files = packedFiles(); + // prod-full pulls ghcr.io/... rather than building from source, which a + // standalone user does not have. + expect(files).toContain("docker/docker-compose.prod-full.yml"); + }, 60_000); + + it("ships the CLI entrypoint", () => { + expect(packedFiles()).toContain("dist/index.js"); + }, 60_000); + + it("does NOT ship scripts/ — those commands decline instead", () => { + // scripts/ is 568K and only `db seed` / `demo` need it. Shipping a + // checkout's worth of tooling to make one command work is the wrong + // trade; assertCheckout tells those commands to say so instead. + expect(packedFiles().some((f) => f.startsWith("scripts/"))).toBe(false); + }, 60_000); +}); diff --git a/cli/src/commands/db/seed.ts b/cli/src/commands/db/seed.ts index 3432d6d6..f1f87990 100644 --- a/cli/src/commands/db/seed.ts +++ b/cli/src/commands/db/seed.ts @@ -2,7 +2,7 @@ import { existsSync } from "node:fs"; import { buildSeedEnv } from "../../lib/docker-env.js"; import { resolve, normalize } from "node:path"; import { run, dockerExec } from "../../lib/exec.js"; -import { paths, readProjectConfig } from "../../lib/config.js"; +import { paths, readProjectConfig , assertCheckout } from "../../lib/config.js"; import { success, createSpinner } from "../../lib/output.js"; /** Validate a seed script path stays within the project root. */ @@ -83,6 +83,7 @@ export async function runDbSeed(opts?: { neo4j?: boolean; demo?: boolean; }): Promise { + assertCheckout("db seed"); const seedNeo4jOnly = opts?.neo4j && !opts?.demo; const seedDemoOnly = opts?.demo && !opts?.neo4j; const seedBoth = (!opts?.neo4j && !opts?.demo) || (opts?.neo4j && opts?.demo); diff --git a/cli/src/commands/dev.ts b/cli/src/commands/dev.ts index f6e051f4..583199f7 100644 --- a/cli/src/commands/dev.ts +++ b/cli/src/commands/dev.ts @@ -1,11 +1,12 @@ import { spawn } from "../lib/exec.js"; -import { paths, getMode, readProjectConfig } from "../lib/config.js"; +import { paths, getMode, readProjectConfig , assertCheckout } from "../lib/config.js"; import { info, warn, banner } from "../lib/output.js"; import { isPgReady, isNeo4jReady, composeUp } from "../lib/docker.js"; import { waitForHealth } from "../lib/health.js"; import { validateEnv } from "./env.js"; export async function runDev(): Promise { + assertCheckout("dev"); const mode = getMode(); const config = readProjectConfig(); diff --git a/cli/src/commands/plugin.ts b/cli/src/commands/plugin.ts index 392b8248..03654fb8 100644 --- a/cli/src/commands/plugin.ts +++ b/cli/src/commands/plugin.ts @@ -1,5 +1,5 @@ import { join } from "node:path"; -import { findProjectRoot } from "../lib/config.js"; +import { findProjectRoot, assertCheckout } from "../lib/config.js"; import { run, runFile } from "../lib/exec.js"; import { success, @@ -29,7 +29,8 @@ export async function runPluginAdd( packageName: string, opts?: { override?: boolean; export?: string }, ): Promise { - const root = findProjectRoot(); + assertCheckout("plugin"); + const root = findProjectRoot() as string; const overrides = opts?.override ?? false; const exportName = opts?.export ?? "default"; @@ -141,7 +142,8 @@ export async function runPluginAdd( * List all registered plugins (built-in chart types + external from manifests). */ export function runPluginList(): void { - const root = findProjectRoot(); + assertCheckout("plugin"); + const root = findProjectRoot() as string; // Keep in sync with app/src/plugins/chart-types.ts const builtInCharts = [ @@ -219,7 +221,8 @@ export function runPluginList(): void { * Remove an external plugin by package name and uninstall it. */ export async function runPluginRemove(packageName: string): Promise { - const root = findProjectRoot(); + assertCheckout("plugin"); + const root = findProjectRoot() as string; // Try both manifests let removed = removeFromManifest( diff --git a/cli/src/index.ts b/cli/src/index.ts index 218818e9..860d9c1b 100644 --- a/cli/src/index.ts +++ b/cli/src/index.ts @@ -4,6 +4,7 @@ import { Command } from "commander"; import { readFileSync } from "node:fs"; import { fileURLToPath } from "node:url"; import { dirname, join } from "node:path"; +import { error as logError } from "./lib/output.js"; const __filename = fileURLToPath(import.meta.url); const __dirname = dirname(__filename); @@ -302,5 +303,12 @@ const isDirectRun = process.argv[1]?.endsWith("neoboard"); if (isDirectRun) { - program.parse(); + // A rejected command action otherwise surfaces as Node's unhandled-rejection + // dump: the message buried under a stack trace rooted in dist/, which reads + // as a crash rather than as the CLI telling you something (#1315). Commands + // throw to say "you cannot do that here"; print that and nothing else. + program.parseAsync().catch((err: unknown) => { + logError(err instanceof Error ? err.message : String(err)); + process.exitCode = 1; + }); } diff --git a/cli/src/lib/config.ts b/cli/src/lib/config.ts index f634a84c..a86fdff2 100644 --- a/cli/src/lib/config.ts +++ b/cli/src/lib/config.ts @@ -1,6 +1,6 @@ import { fileURLToPath } from "node:url"; import { dirname, join } from "node:path"; -import { readFileSync, writeFileSync, existsSync } from "node:fs"; +import { readFileSync, writeFileSync, existsSync, mkdirSync } from "node:fs"; // Types export interface ProjectConfig { @@ -20,7 +20,16 @@ export interface LocalConfig { } // Project root detection -export function findProjectRoot(startDir?: string): string { +/** + * Walk up looking for the monorepo root. Returns null when there isn't one. + * + * Returns rather than throws (#1315): under `npx @neoboard/cli`, the CLI lives + * in an npm cache directory with no `neoboard` package.json above it, and a + * throw this deep in a path helper surfaced as an unrelated-looking crash — + * `setup` died in root detection, reporting nothing about standalone mode. + * The caller is the only place that knows whether the absence is a problem. + */ +export function findProjectRoot(startDir?: string): string | null { let dir = startDir ?? dirname(fileURLToPath(import.meta.url)); // Terminate when dirname stops changing — "/" on POSIX, "C:\\" on Windows. // The old `while (dir !== "/")` looped forever on Windows (#991). @@ -38,15 +47,60 @@ export function findProjectRoot(startDir?: string): string { if (parent === dir) break; dir = parent; } + return null; +} + +/** + * The directory the CLI operates on, in either situation: + * + * inside the monorepo the checkout root (contributors) — always wins, so a + * stray NEOBOARD_DIR cannot silently redirect a + * contributor's checkout somewhere else + * installed standalone a working directory: $NEOBOARD_DIR, else + * ./neoboard under the current directory + * + * Created if missing — a standalone user has nowhere to put config, .env or + * generated secrets otherwise. + */ +export function resolveRoot(startDir?: string): string { + const monorepo = findProjectRoot(startDir); + if (monorepo) return monorepo; + + const dir = process.env.NEOBOARD_DIR || join(process.cwd(), "neoboard"); + if (!existsSync(dir)) mkdirSync(dir, { recursive: true }); + return dir; +} + +/** + * Abort with an explanation when a command genuinely needs the source tree. + * + * `dev`, `db seed` and the plugin commands run scripts that only exist in a + * checkout, and scripts/ is deliberately not shipped in the published package. + * Without this they fail on a missing file deep inside the command, which + * reads as a bug rather than as "this command is not for this install". + */ +export function assertCheckout(command: string, startDir?: string): void { + if (!isStandalone(startDir)) return; throw new Error( - "Could not find NeoBoard project root (package.json with name 'neoboard')", + `\`neoboard ${command}\` needs the NeoBoard source tree, and this is a ` + + `standalone install (no checkout found).\n\n` + + `Clone the repo and run it from there:\n` + + ` git clone https://github.com/alfredo1996/neoboard.git\n` + + ` cd neoboard && npm install\n\n` + + `Everything that manages a running instance — setup, start, stop, ` + + `status, doctor, logs, config, env — works standalone.`, ); } +/** True when there is no monorepo checkout — i.e. an npx/global install. */ +export function isStandalone(startDir?: string): boolean { + return findProjectRoot(startDir) === null; +} + // Path constants (lazy-initialized) let _root: string | null = null; function root(): string { - if (!_root) _root = findProjectRoot(); + if (!_root) _root = resolveRoot(); return _root; } From 88564e8b8930599b99b27bf887fded77cd56f49e Mon Sep 17 00:00:00 2001 From: alfredorubin96 Date: Tue, 28 Jul 2026 15:03:31 +0200 Subject: [PATCH 2/3] fix(cli): build in prepack, and make postpack cwd-independent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI caught what my local run had not: the packaging test asserts on the real `npm pack` output, and CI has no cli/dist, so the tarball genuinely lacked dist/index.js. The test was right — a package packed without a build IS broken, and the release workflow only escapes it by running an explicit build step first. prepack now builds, which is what prepack is for, so `npm pack` produces a complete package for anyone rather than only for the one workflow that happens to build first. prepublishOnly's build is redundant and removed. Also replaces `postpack: rm -rf docker`. That path is relative to whatever directory npm runs it from, and one level up is the repo's real docker/. Both paths now come from import.meta.url, so cwd cannot make it delete the wrong tree. Verified from a clean dist with the exact invocation release.yml uses: `npm publish --workspace=@neoboard/cli --dry-run` stages the compose files, ships dist/index.js, and removes cli/docker afterwards. Checked and dismissed: that dry-run prints `"bin[neoboard]" script name dist/index.js was invalid and removed`. It is pre-existing on release/1.4 and harmless — the packed manifest keeps bin, and installing the tarball produces a working node_modules/.bin/neoboard (`neoboard --version` -> 1.0.0). npm is normalising an in-memory manifest for the dry-run, not the package. Refs #1315 Co-Authored-By: Claude Opus 5 --- cli/package.json | 5 ++--- cli/scripts/stage-compose.mjs | 16 +++++++++++++++- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/cli/package.json b/cli/package.json index 2d932ea2..52992317 100644 --- a/cli/package.json +++ b/cli/package.json @@ -38,13 +38,12 @@ "scripts": { "build": "tsc", "dev": "tsc --watch", - "prepublishOnly": "npm run build", "test": "SKIP_INTEGRATION=1 vitest run", "test:integration": "vitest run src/__tests__/integration", "test:watch": "vitest", "test:coverage": "SKIP_INTEGRATION=1 vitest run --coverage", - "prepack": "node scripts/stage-compose.mjs", - "postpack": "rm -rf docker" + "prepack": "npm run build && node scripts/stage-compose.mjs", + "postpack": "node scripts/stage-compose.mjs --clean" }, "dependencies": { "chalk": "^5.0.0", diff --git a/cli/scripts/stage-compose.mjs b/cli/scripts/stage-compose.mjs index e9b5a504..fc083cf5 100644 --- a/cli/scripts/stage-compose.mjs +++ b/cli/scripts/stage-compose.mjs @@ -9,7 +9,15 @@ * manifest-only test asserting `files.includes("docker")` passes while proving * nothing. That is how #1315's second half would have shipped twice. * - * Run by `prepack`, removed by `postpack`. `cli/docker/` is gitignored. + * Run by `prepack` (after the build — a package without dist/ is as broken as + * one without compose files, and prepack is the hook that guarantees BOTH for + * anyone who packs, not just for the release workflow which happens to build + * first). `--clean`, from `postpack`, removes the staged copy. + * + * Both paths are computed from import.meta.url, never from cwd. A bare + * `rm -rf docker` in postpack would be relative to whatever directory npm + * happened to run it from — and one level up is the repo's real docker/. + * `cli/docker/` is gitignored. */ import { mkdirSync, copyFileSync, rmSync } from "node:fs"; import { dirname, join } from "node:path"; @@ -30,6 +38,12 @@ const FILES = [ ]; rmSync(dest, { recursive: true, force: true }); + +if (process.argv.includes("--clean")) { + console.error("removed staged cli/docker/"); + process.exit(0); +} + for (const rel of FILES) { const to = join(dest, rel); mkdirSync(dirname(to), { recursive: true }); From 69bf4ade3116c246f1841444610b086cdda0c23c Mon Sep 17 00:00:00 2001 From: alfredorubin96 Date: Tue, 28 Jul 2026 15:26:44 +0200 Subject: [PATCH 3/3] fix(cli): resolve the checkout from where the user stands, not where the CLI lives MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four CodeRabbit findings, all valid. The Major one is a genuine behavioural gap: resolveRoot and isStandalone searched from import.meta.url only, so a globally installed or npx'd CLI run INSIDE a checkout was treated as standalone — `dev` and `db seed` refused, and `config list` created ./neoboard inside the user's own source tree with the repo right there. Now searches process.cwd() first and falls back to the module path, which still covers the monorepo's own `node cli/dist/index.js` run from elsewhere. Verified with the real installed binary, both directions: from inside the checkout config get -> 3000, dev proceeds, no ./neoboard from a bare temp dir config get -> 3000, dev declines, ./neoboard made The three Minor ones: the standalone-root tests asserted only the RETURNED PATH, so deleting mkdirSync would still have passed — the loose-assertion shape I have been cataloguing all session, in my own test. They now assert the directory exists. afterEach deleted NEOBOARD_DIR unconditionally, clobbering a value the developer or CI may have set. Restores it now. `new URL(...).pathname` keeps the leading slash and percent-encodes spaces, so a checkout under "Program Files" would pass a broken cwd to execFileSync. fileURLToPath instead — this repo has already been bitten by a Windows path bug (#991). Refs #1315 Co-Authored-By: Claude Opus 5 --- cli/src/__tests__/lib/standalone.test.ts | 70 +++++++++++++++++++++++- cli/src/lib/config.ts | 19 ++++++- 2 files changed, 84 insertions(+), 5 deletions(-) diff --git a/cli/src/__tests__/lib/standalone.test.ts b/cli/src/__tests__/lib/standalone.test.ts index ce19d587..3058a48d 100644 --- a/cli/src/__tests__/lib/standalone.test.ts +++ b/cli/src/__tests__/lib/standalone.test.ts @@ -1,7 +1,14 @@ import { describe, it, expect, beforeEach, afterEach } from "vitest"; -import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from "node:fs"; +import { + mkdtempSync, + mkdirSync, + writeFileSync, + rmSync, + existsSync, +} from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; +import { fileURLToPath } from "node:url"; import { execFileSync } from "node:child_process"; /** @@ -17,6 +24,7 @@ import { execFileSync } from "node:child_process"; */ let tmp: string; +let originalNeoboardDir: string | undefined; async function loadConfig() { const mod = await import("../../lib/config.js"); @@ -26,12 +34,16 @@ async function loadConfig() { beforeEach(() => { tmp = mkdtempSync(join(tmpdir(), "nb-standalone-")); + originalNeoboardDir = process.env.NEOBOARD_DIR; }); afterEach(async () => { const mod = await import("../../lib/config.js"); mod._setRootForTesting(null); - delete process.env.NEOBOARD_DIR; + // Restore rather than delete — a developer or CI process may have set it, + // and clobbering it leaks this suite's state outward. + if (originalNeoboardDir === undefined) delete process.env.NEOBOARD_DIR; + else process.env.NEOBOARD_DIR = originalNeoboardDir; rmSync(tmp, { recursive: true, force: true }); }); @@ -87,6 +99,10 @@ describe("resolveRoot — standalone working directory (#1315)", () => { process.env.NEOBOARD_DIR = dir; const { resolveRoot, isStandalone } = await loadConfig(); expect(resolveRoot(join(tmp, "nowhere"))).toBe(dir); + // Asserting the returned path alone would still pass with mkdirSync + // removed — and a standalone install has nowhere to write config, .env or + // generated secrets if the directory is not actually created. + expect(existsSync(dir)).toBe(true); expect(isStandalone(join(tmp, "nowhere"))).toBe(true); }); @@ -110,6 +126,50 @@ describe("resolveRoot — standalone working directory (#1315)", () => { const { resolveRoot } = await loadConfig(); const resolved = resolveRoot(join(tmp, "npm-cache")); expect(resolved).toBe(join(process.cwd(), "neoboard")); + expect(existsSync(resolved)).toBe(true); + rmSync(resolved, { recursive: true, force: true }); + }); +}); + +describe("invocation directory wins over install location (#1315)", () => { + const withCwd = async (dir: string, fn: () => Promise | void) => { + const prev = process.cwd(); + process.chdir(dir); + try { + await fn(); + } finally { + process.chdir(prev); + } + }; + + it("uses the checkout you are standing in, not ./neoboard", async () => { + // A globally installed or npx'd CLI run from inside a checkout must use + // that checkout. Searching only from the CLI's install location would call + // it standalone, refuse `dev`, and create ./neoboard inside the user's own + // source tree with the repo right there. + writeFileSync( + join(tmp, "package.json"), + JSON.stringify({ name: "neoboard" }), + ); + const { resolveRoot, isStandalone, assertCheckout } = await loadConfig(); + + await withCwd(tmp, () => { + expect(resolveRoot()).toBe(process.cwd()); + expect(isStandalone()).toBe(false); + expect(() => assertCheckout("dev")).not.toThrow(); + expect(existsSync(join(tmp, "neoboard"))).toBe(false); + }); + }); + + it("falls back to the install location when cwd has no checkout", async () => { + // The monorepo's own `node cli/dist/index.js` run from an unrelated + // directory still has to find its root. + const { isStandalone } = await loadConfig(); + await withCwd(tmp, () => { + // This suite's own module lives inside the monorepo, so the fallback + // finds it even though cwd is a bare temp dir. + expect(isStandalone()).toBe(false); + }); }); }); @@ -147,7 +207,11 @@ describe("packaged files (#1315)", () => { // So this runs the real `npm pack --dry-run` and reads what would actually // be published. const packedFiles = (): string[] => { - const cliDir = new URL("../../..", import.meta.url).pathname; + // fileURLToPath, not .pathname: the latter keeps the leading slash and + // percent-encodes spaces, so a checkout under "Program Files" would pass a + // broken cwd. This repo has already been bitten by a Windows path bug + // (#991). + const cliDir = fileURLToPath(new URL("../../..", import.meta.url)); const out = execFileSync("npm", ["pack", "--dry-run", "--json"], { cwd: cliDir, encoding: "utf8", diff --git a/cli/src/lib/config.ts b/cli/src/lib/config.ts index a86fdff2..5ee57249 100644 --- a/cli/src/lib/config.ts +++ b/cli/src/lib/config.ts @@ -63,7 +63,7 @@ export function findProjectRoot(startDir?: string): string | null { * generated secrets otherwise. */ export function resolveRoot(startDir?: string): string { - const monorepo = findProjectRoot(startDir); + const monorepo = findCheckout(startDir); if (monorepo) return monorepo; const dir = process.env.NEOBOARD_DIR || join(process.cwd(), "neoboard"); @@ -94,7 +94,22 @@ export function assertCheckout(command: string, startDir?: string): void { /** True when there is no monorepo checkout — i.e. an npx/global install. */ export function isStandalone(startDir?: string): boolean { - return findProjectRoot(startDir) === null; + return findCheckout(startDir) === null; +} + +/** + * Look for a checkout from where the user is STANDING first, then from where + * the CLI is installed. + * + * cwd matters because a globally installed or npx'd CLI run from inside a + * checkout should use that checkout — otherwise `neoboard dev` would refuse, + * and `config list` would create ./neoboard inside the user's own source tree, + * with the repo right there. The module path is the fallback that covers the + * monorepo's own `node cli/dist/index.js` from an unrelated directory. + */ +function findCheckout(startDir?: string): string | null { + if (startDir !== undefined) return findProjectRoot(startDir); + return findProjectRoot(process.cwd()) ?? findProjectRoot(); } // Path constants (lazy-initialized)