Skip to content

feat(cli): implement all CLI commands with test suite - #344

Closed
alfredo1996 wants to merge 8 commits into
release/1.0from
feat/cli-scaffold
Closed

feat(cli): implement all CLI commands with test suite#344
alfredo1996 wants to merge 8 commits into
release/1.0from
feat/cli-scaffold

Conversation

@alfredo1996

@alfredo1996 alfredo1996 commented Apr 2, 2026

Copy link
Copy Markdown
Owner

Summary

Commands

Command Issue Description
neoboard doctor #303 Prerequisite checks (Docker, ports, Node, deps)
neoboard env #304 Generate/validate .env.local with secure defaults
neoboard init/start/stop/setup #305 Full lifecycle management
neoboard db migrate #306 Version-aware Drizzle migrations (--status, --dry-run)
neoboard dev #307 Local-mode Next.js dev server
neoboard status #308 Service health dashboard
neoboard db seed / demo #309 Idempotent Neo4j + PG seeding
neoboard db reset #310 Safe database reset with confirmation
neoboard db dump #311 pg_dump backup (--data-only, --output)

Test plan

  • cd cli && npm test — 142 tests pass
  • cd cli && npm run build — tsc compiles clean
  • ./bin/neoboard --version — prints version
  • ./bin/neoboard --help — lists all commands
  • ./bin/neoboard doctor — runs prerequisite checks
  • npm run test from root — all packages pass (app + component + cli)

Closes #302, Closes #303, Closes #304, Closes #305, Closes #306, Closes #307, Closes #308, Closes #309, Closes #310, Closes #311

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • New Features
    • Added a comprehensive command-line interface (neoboard CLI) with commands for project initialization, startup/shutdown, development mode, and project setup
    • Integrated database management tools including migrations, data resets, seeding, and backup/dump functionality
    • Added automatic environment configuration and validation tools for local setup
    • Included diagnostic health checks to verify system readiness

alfredorubin96 and others added 2 commits April 2, 2026 16:52
- Create cli/ package with commander, ora, chalk, dotenv
- Add stub commands: init, start, stop, dev, setup, status, doctor, demo, env
- Add db subcommand group: migrate, reset, seed, dump
- Add config system: neoboard.config.json (shared) + .neoboard.local (personal)
- Add bin/neoboard entry point shim
- Wire up root package.json scripts

Closes #302

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Implement all 9 remaining CLI issues (#303#311) on top of the
existing scaffold (#302). The CLI provides a Supabase-inspired
zero-friction developer experience: `neoboard init → start → demo`.

Commands implemented:
- doctor: prerequisite checks (Docker, ports, Node, deps, env)
- env: generate/validate app/.env.local with secure defaults
- init/start/stop/setup: full lifecycle management
- dev: local-mode Next.js dev server
- status: service health dashboard
- demo: one-command demo environment with seed data
- db migrate: version-aware Drizzle migrations (--status, --dry-run)
- db seed: idempotent Neo4j + PostgreSQL seeding
- db dump: pg_dump backup (--data-only, --output)
- db reset: safe database reset with confirmation

Shared utilities: exec (child_process wrapper), docker (compose ops),
health (polling), ports (availability), prompt (confirmation).

142 Vitest tests across 20 test files, all passing. Tests mock at
one layer up (commands mock exec.ts, not child_process directly).

Closes #303, Closes #304, Closes #305, Closes #306, Closes #307,
Closes #308, Closes #309, Closes #310, Closes #311

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@alfredo1996 alfredo1996 added the enhancement New feature or request label Apr 2, 2026
@coderabbitai

coderabbitai Bot commented Apr 2, 2026

Copy link
Copy Markdown

Walkthrough

Introduces a complete CLI package for NeoBoard with commander-based command structure, supporting core lifecycle commands (init, start, stop, setup), database operations (migrate, reset, seed, dump), development utilities (dev, status, doctor, demo, env), and comprehensive configuration/utility libraries with full test coverage.

Changes

Cohort / File(s) Summary
CLI Package Setup
cli/package.json, cli/tsconfig.json, cli/vitest.config.ts
Created new @neoboard/cli package with ESM configuration, build scripts, test framework setup, and dependencies (commander, ora, chalk, dotenv).
CLI Core Libraries
cli/src/lib/config.ts, cli/src/lib/docker.ts, cli/src/lib/exec.ts, cli/src/lib/health.ts, cli/src/lib/output.ts, cli/src/lib/ports.ts, cli/src/lib/prompt.ts
Implemented utility modules for project configuration (root detection, path resolution, config file I/O), Docker compose orchestration, command execution with error handling, service health polling, styled console output, port availability checks, and user confirmation prompts.
Lifecycle Commands
cli/src/commands/init.ts, cli/src/commands/start.ts, cli/src/commands/stop.ts, cli/src/commands/setup.ts
Implemented initialization (config generation, mode selection, local dependency installation), service startup with health checks and migrations, service teardown, and convenience setup alias combining init+start.
Database Commands
cli/src/commands/db/migrate.ts, cli/src/commands/db/reset.ts, cli/src/commands/db/seed.ts, cli/src/commands/db/dump.ts
Implemented version-aware migration management with dry-run support, database reset with confirmation and re-seeding, Neo4j/Postgres seeding with idempotency checks, and pg_dump backup creation with timestamp naming.
Development & Status Commands
cli/src/commands/dev.ts, cli/src/commands/status.ts, cli/src/commands/doctor.ts, cli/src/commands/demo.ts, cli/src/commands/env.ts
Implemented Next.js dev server launcher (mode-aware), service health dashboard with container/migration state reporting, prerequisite checker for Docker/Node/ports, full demo setup with seeding and login credentials, and env file generation/validation with secret generation.
CLI Entry Point & Tests
cli/src/index.ts, cli/src/__tests__/**/*.test.ts
Created commander program with all command registration and dynamic imports; added comprehensive Vitest test suites covering all commands and utility libraries with mocked dependencies.
Root Integration
bin/neoboard, .gitignore, package.json, neoboard.config.json, sonar-project.properties
Added root-level Node shebang script, updated gitignore patterns, root package.json scripts for CLI testing/building/execution, project configuration file with port/credential defaults, and extended SonarQube analysis scope.

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant CLI as neoboard setup
    participant Init as runInit()
    participant Config as config.ts
    participant Docker as docker.ts
    participant Start as runStart()
    participant Migrate as runDbMigrate()
    
    User->>CLI: Execute 'neoboard setup'
    CLI->>Init: Call runInit(opts)
    Init->>Config: Write neoboard.config.json
    Init->>Config: Write .neoboard.local
    Init-->>CLI: Init complete
    CLI->>Start: Call runStart()
    Start->>Docker: Call runDoctor()
    Docker-->>Start: Checks passed
    Start->>Docker: composeUp({full: true/false})
    Docker-->>Start: Services started
    Start->>Docker: waitForHealth(PostgreSQL)
    Docker-->>Start: Postgres ready
    Start->>Docker: waitForHealth(Neo4j)
    Docker-->>Start: Neo4j ready
    Start->>Migrate: runDbMigrate({})
    Migrate-->>Start: Migrations applied
    Start-->>User: Setup complete!
Loading
sequenceDiagram
    actor User
    participant CLI as neoboard db reset
    participant Prompt as confirm()
    participant Config as getMode()
    participant Exec as run/dockerExec
    participant Migrate as runDbMigrate()
    participant Seed as runDbSeed()
    
    User->>CLI: Execute 'neoboard db reset'
    CLI->>Prompt: Confirm with user
    Prompt-->>CLI: User confirmed
    CLI->>Config: Check DATABASE_URL localhost
    Config-->>CLI: Valid
    CLI->>Exec: Execute DROP/CREATE database
    Exec-->>CLI: Database recreated
    CLI->>Migrate: runDbMigrate({})
    Migrate-->>CLI: Migrations replayed
    CLI->>Seed: runDbSeed()
    Seed-->>CLI: Seed data applied
    CLI-->>User: Database reset complete
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested labels

type: feature, testing, chore

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 4.55% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(cli): implement all CLI commands with test suite' accurately summarizes the main change—comprehensive CLI implementation with full test coverage.
Linked Issues check ✅ Passed All linked objectives are met: doctor, env, init/start/stop/setup, db migrate/seed/reset/dump, dev, status, and demo commands implemented; comprehensive test suite (142 tests across 20 files); supporting utilities (exec, docker, health, ports, prompt, config); CI and SonarCloud integration; all tests passing.
Out of Scope Changes check ✅ Passed All changes are in-scope: CLI package structure, command implementations, test suite, library utilities, configuration files, CI/SonarCloud updates, and root package.json scripts are all required for the PR objectives.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/cli-scaffold

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

- Add cli/ to CI path triggers, install, test, and coverage upload
- Add cli/src to sonar.sources and sonar.tests
- Add cli/coverage/lcov.info to sonar coverage report paths

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment thread cli/src/lib/config.ts Fixed
@alfredo1996
alfredo1996 changed the base branch from dev to release/1.0 April 2, 2026 22:02
alfredorubin96 and others added 5 commits April 3, 2026 00:06
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…swords

These are local development defaults matching docker-compose, not production credentials.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
CLI commands are internally constructed, not user-supplied input.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Will resolve security hotspots via SonarCloud UI instead.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@sonarqubecloud

sonarqubecloud Bot commented Apr 3, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
1 Security Hotspot
C Security Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 16

🧹 Nitpick comments (19)
cli/src/lib/config.ts (1)

105-121: Silent parse errors may hide configuration issues.

Both readProjectConfig() and readLocalConfig() silently return defaults on JSON parse errors. Consider logging a warning so users know their config file has syntax errors.

💡 Suggested improvement
+import { warn } from "./output.js";
+
 export function readProjectConfig(): ProjectConfig {
   if (!existsSync(paths.projectConfig)) return DEFAULT_PROJECT_CONFIG;
   try {
     return JSON.parse(readFileSync(paths.projectConfig, "utf-8"));
-  } catch {
+  } catch (e) {
+    warn(`Failed to parse ${paths.projectConfig}, using defaults`);
     return DEFAULT_PROJECT_CONFIG;
   }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cli/src/lib/config.ts` around lines 105 - 121, Modify readProjectConfig and
readLocalConfig to surface JSON parse errors instead of failing silently: in
each catch block capture the error (catch (err)), log a warning that includes
the config path (paths.projectConfig / paths.localConfig) and the error message
(use the project logger if available or console.warn), then return the
DEFAULT_*_CONFIG as before; update functions readProjectConfig and
readLocalConfig to include these warning logs so users know their config file
has syntax issues.
cli/src/__tests__/commands/stop.test.ts (1)

21-29: Add an assertion for the success message call.

You already mock success; asserting it was called will better protect command UX regressions.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cli/src/__tests__/commands/stop.test.ts` around lines 21 - 29, Add an
assertion that the mocked success function is called after runStop to verify the
command prints the success message; in the tests referencing runStop and
mockComposeDown (the two it blocks), add expect(success).toHaveBeenCalled() (or
expect(success).toHaveBeenCalledWith(<expected message>) if you want to assert
the exact text) after the existing composeDown assertions so both cases verify
success() was invoked.
neoboard.config.json (1)

8-16: Avoid committed default DB credentials in shared config.

Line 10 and Line 15 hardcode predictable passwords. Even for local-first workflows, this weakens security when ports are exposed or configs are reused. Prefer generating per-init credentials and storing them in local env/state files instead of VCS-tracked defaults.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@neoboard.config.json` around lines 8 - 16, The config currently commits
default DB credentials for "postgres" (user/password) and "neo4j"
(user/password); remove these hardcoded passwords and replace them with
references to environment-driven or generated secrets (e.g., read from env vars
or a local state file) so credentials are not stored in VCS; update the
"postgres" and "neo4j" entries to load their "password" (and optionally "user")
from a secure source and document the init step to generate/store per-instance
credentials instead of committing them.
cli/package.json (1)

23-23: Align @types/node with the Node 22 runtime.

The cli package uses @types/node@^25.5.0 (Node.js 24 types) while CI runs Node 22, causing type-checking to permit APIs unavailable at runtime. Align with the runtime version:

Suggested adjustment
-    "@types/node": "^25.5.0",
+    "@types/node": "^22.0.0",
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cli/package.json` at line 23, Update the CLI package.json dependency for
"@types/node" to match the Node 22 runtime (e.g., change "@types/node" to a
Node-22-aligned version such as "^22.0.0") so type-checking reflects the actual
runtime; update package.json's dependencies entry for "@types/node" and
reinstall/update lockfile (npm/yarn) to ensure the new types are used during CI
and local type checks.
cli/src/__tests__/commands/start.test.ts (1)

77-85: Consider strengthening ordering assertion for migrations.

The test verifies waitForHealth is called twice and runDbMigrate is called, but doesn't assert that migrations run after health checks pass. For critical sequencing, consider using toHaveBeenCalledBefore:

  it("runs migrations after health checks pass", async () => {
    await runStart();
+   expect(mockWaitForHealth).toHaveBeenCalledBefore(mockRunDbMigrate);
    expect(mockRunDbMigrate).toHaveBeenCalledWith({});
  });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cli/src/__tests__/commands/start.test.ts` around lines 77 - 85, The test
needs an ordering assertion to ensure migrations run after health checks: in the
"runs migrations after health checks pass" spec (where runStart,
mockWaitForHealth, and mockRunDbMigrate are used), add an assertion that
mockWaitForHealth was called before mockRunDbMigrate — e.g., use jest-extended's
toHaveBeenCalledBefore(mockRunDbMigrate) or assert invocation order via
mockWaitForHealth.mock.invocationCallOrder[0] <
mockRunDbMigrate.mock.invocationCallOrder[0]; this ensures runDbMigrate() occurs
after waitForHealth() when runStart() is executed.
cli/src/commands/start.ts (1)

24-27: Health checks could run in parallel.

PostgreSQL and Neo4j health checks are independent. Running them with Promise.all would reduce startup time.

♻️ Parallel health checks
-  await waitForHealth({ check: isPgReady, label: "PostgreSQL" });
-  await waitForHealth({ check: isNeo4jReady, label: "Neo4j" });
+  await Promise.all([
+    waitForHealth({ check: isPgReady, label: "PostgreSQL" }),
+    waitForHealth({ check: isNeo4jReady, label: "Neo4j" }),
+  ]);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cli/src/commands/start.ts` around lines 24 - 27, The two independent health
checks called via waitForHealth (using isPgReady and isNeo4jReady) should run in
parallel to reduce startup time: after calling readProjectConfig(), replace the
sequential awaits for waitForHealth({ check: isPgReady, label: "PostgreSQL" })
and waitForHealth({ check: isNeo4jReady, label: "Neo4j" }) with a single
Promise.all that awaits both calls concurrently so both checks execute in
parallel.
cli/src/commands/status.ts (1)

57-62: "stopped" may be misleading when service is running but unhealthy.

If the database container is running but the readiness check fails (e.g., Postgres is starting up), displaying "stopped" is inaccurate. Consider using "unhealthy" or "not ready" for consistency with the app health output.

♻️ Suggested terminology fix
  info(
-    `PostgreSQL   ${pgHealthy ? "healthy" : "stopped"} (localhost:${config.ports.postgres})`,
+    `PostgreSQL   ${pgHealthy ? "healthy" : "not ready"} (localhost:${config.ports.postgres})`,
  );
  info(
-    `Neo4j        ${neo4jHealthy ? "healthy" : "stopped"} (localhost:${config.ports.neo4j_bolt})`,
+    `Neo4j        ${neo4jHealthy ? "healthy" : "not ready"} (localhost:${config.ports.neo4j_bolt})`,
  );
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cli/src/commands/status.ts` around lines 57 - 62, The status output uses
"stopped" when a readiness check fails which is misleading; update the messages
in the info(...) calls that reference pgHealthy and neo4jHealthy so they display
"unhealthy" or "not ready" (e.g., "unhealthy") instead of "stopped" while
keeping the port info (config.ports.postgres and config.ports.neo4j_bolt) and
the existing conditional logic intact; locate the two info(...) calls that
format `PostgreSQL   ${pgHealthy ? "healthy" : "stopped"}
(localhost:${config.ports.postgres})` and `Neo4j        ${neo4jHealthy ?
"healthy" : "stopped"} (localhost:${config.ports.neo4j_bolt})` and change the
false branch text to the new term.
cli/src/commands/db/reset.ts (2)

25-27: Consider adding ::1 for IPv6 localhost.

The isLocalhost check handles localhost and 127.0.0.1 but misses ::1 (IPv6 localhost).

♻️ Add IPv6 localhost
 function isLocalhost(host: string): boolean {
-  return host === "localhost" || host === "127.0.0.1";
+  return host === "localhost" || host === "127.0.0.1" || host === "::1";
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cli/src/commands/db/reset.ts` around lines 25 - 27, The isLocalhost function
currently only checks "localhost" and "127.0.0.1"; update isLocalhost(host:
string) to also treat the IPv6 loopback "::1" as local by including "::1" in the
comparison (or normalizing/using a set of loopback addresses) so calls to
isLocalhost correctly recognize IPv6 localhost.

15-23: Regex may miss edge cases in DATABASE_URL parsing.

The regex /DATABASE_URL=.*@([^:/]+)/ doesn't account for URL-encoded credentials or IPv6 addresses. Consider using the URL constructor for robust parsing.

♻️ More robust URL parsing
 function getDatabaseHost(): string {
   try {
     const content = readFileSync(paths.envFile, "utf-8");
-    const match = content.match(/DATABASE_URL=.*@([^:/]+)/);
-    return match?.[1] ?? "localhost";
+    const match = content.match(/DATABASE_URL=["']?([^"'\s]+)/);
+    if (!match?.[1]) return "localhost";
+    const url = new URL(match[1]);
+    return url.hostname || "localhost";
   } catch {
     return "localhost";
   }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cli/src/commands/db/reset.ts` around lines 15 - 23, The getDatabaseHost
function currently uses a fragile regex to parse DATABASE_URL; instead read the
env file (readFileSync(paths.envFile, "utf-8")), extract the DATABASE_URL entry,
pass its value into the standard URL constructor (new URL(...)) to reliably
obtain hostname (which correctly handles URL-encoded credentials and IPv6
bracketed hosts), and return url.hostname or fallback to "localhost" on any
parse/missing errors; update getDatabaseHost to implement this flow and preserve
the existing try/catch behavior.
cli/src/index.ts (2)

150-155: Fragile direct-run detection.

The check process.argv[1]?.endsWith("index.js") || process.argv[1]?.endsWith("neoboard") won't match if invoked via a symlink with a different name, via npx, or after bundling. Consider using a more robust pattern like checking for a sentinel env var or using import.meta.url comparison.

💡 Alternative approach
-const isDirectRun =
-  process.argv[1]?.endsWith("index.js") ||
-  process.argv[1]?.endsWith("neoboard");
+const isDirectRun = process.env.VITEST === undefined;

Or alternatively, move program.parse() to a separate bin/neoboard.js file that imports and calls it explicitly, keeping index.ts purely as a module export.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cli/src/index.ts` around lines 150 - 155, The direct-run detection using
process.argv[1] in the isDirectRun variable is brittle; replace it with a robust
mechanism such as checking an explicit sentinel environment variable or using
import.meta.url to detect direct invocation, or move program.parse() out of
index.ts into a dedicated bin entry (e.g., bin/neoboard.js) that imports the
module and calls program.parse() explicitly. Update references to isDirectRun
and the conditional that calls program.parse() so it uses the chosen approach
(sentinel env var check or import.meta.url comparison) or remove the check
entirely and perform parsing only from the new bin/neoboard.js entrypoint.

11-13: Synchronous file read at module load time.

Reading package.json synchronously at import time blocks the event loop and can cause issues if the file is missing or the path is incorrect at runtime.

💡 Consider lazy initialization or error handling
-const pkg = JSON.parse(
-  readFileSync(join(__dirname, "..", "package.json"), "utf-8"),
-);
+let pkg: { version: string };
+try {
+  pkg = JSON.parse(
+    readFileSync(join(__dirname, "..", "package.json"), "utf-8"),
+  );
+} catch {
+  pkg = { version: "0.0.0-unknown" };
+}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cli/src/index.ts` around lines 11 - 13, The synchronous read of package.json
using readFileSync/JSON.parse at module load (the pkg constant built from
readFileSync(join(__dirname, "..", "package.json"), "utf-8")) should be replaced
with a lazy or async initialization and error handling: move the file-read into
an async helper (e.g., getPackageJson or initCliConfig) or use
fs.promises.readFile so the event loop isn't blocked, wrap the read/JSON.parse
in try/catch to handle missing/invalid files and provide a sensible fallback or
propagate a clear error, and update any callers to await the helper instead of
relying on the top-level pkg constant.
cli/src/commands/db/migrate.ts (1)

70-74: --to flag accepted but not functional.

The flag is parsed and a warning is emitted, but it doesn't affect behavior. This is documented in the PR objectives as "not yet supported" — consider using .hideHelp() or removing the option until implemented.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cli/src/commands/db/migrate.ts` around lines 70 - 74, The CLI accepts a
non-functional --to flag (checked via opts.to and only emits warn(...)) which
can be misleading; either remove the --to option registration or mark it hidden
until implemented (call .hideHelp() on the option definition) and remove the
opts.to handling/warning in migrate.ts (or ensure the option actually affects
behavior). Locate the code that defines/parses the --to option and the warn(...)
block in migrate.ts and either delete the option + the opts.to branch or update
the option declaration to .hideHelp() and keep or adjust the warning accordingly
so the CLI surface matches actual functionality.
cli/src/commands/env.ts (1)

67-68: writeFileSync can throw without error handling.

If the app/ directory doesn't exist or permissions are wrong, this will crash. Consider wrapping with try/catch and reporting a user-friendly error.

🛡️ Add error handling
-  writeFileSync(paths.envFile, lines.join("\n"));
-  success("Generated app/.env.local");
+  try {
+    writeFileSync(paths.envFile, lines.join("\n"));
+    success("Generated app/.env.local");
+  } catch (err) {
+    logError(`Failed to write ${paths.envFile}: ${(err as Error).message}`);
+    process.exitCode = 1;
+    return;
+  }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cli/src/commands/env.ts` around lines 67 - 68, The writeFileSync call can
throw if the target directory or permissions are wrong; wrap
writeFileSync(paths.envFile, lines.join("\n")) in a try/catch around the write
and only call success("Generated app/.env.local") on success; in the catch block
log a clear, user-friendly error that includes paths.envFile and the caught
error message (e.g., via console.error or the existing error logging helper) and
exit non-zero (process.exit(1)) so the CLI fails cleanly.
cli/src/__tests__/commands/doctor.test.ts (1)

83-89: Test relies on runtime Node version — potentially flaky.

This test passes only if the CI/dev environment runs Node ≥ 20. If someone runs tests on Node 18, the assertion will fail unexpectedly. Consider mocking process.version or making the assertion unconditional based on the expected behavior.

💡 Mock process.version for deterministic test
 describe("checkNodeVersion", () => {
-  it("returns ok for current node (>= 20)", () => {
-    const result = checkNodeVersion();
-    const major = parseInt(process.version.slice(1), 10);
-    expect(result.status).toBe(major >= 20 ? "ok" : "fail");
+  it("returns ok for node >= 20", () => {
+    const original = process.version;
+    Object.defineProperty(process, 'version', { value: 'v20.0.0', configurable: true });
+    const result = checkNodeVersion();
+    expect(result.status).toBe("ok");
+    Object.defineProperty(process, 'version', { value: original, configurable: true });
+  });
+
+  it("returns fail for node < 20", () => {
+    const original = process.version;
+    Object.defineProperty(process, 'version', { value: 'v18.0.0', configurable: true });
+    const result = checkNodeVersion();
+    expect(result.status).toBe("fail");
+    Object.defineProperty(process, 'version', { value: original, configurable: true });
   });
 });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cli/src/__tests__/commands/doctor.test.ts` around lines 83 - 89, The test is
flaky because it reads the real runtime Node version; update the test for
checkNodeVersion to make it deterministic by mocking process.version (or spying)
to a controlled value before calling checkNodeVersion and restoring it after;
e.g. use jest.spyOn(process, "version", "get") or Object.defineProperty to
return a specific version string (like "v20.0.0" and a negative case if needed),
call checkNodeVersion(), assert on result.status, then restore the original
getter so other tests are unaffected.
cli/src/__tests__/commands/env.test.ts (3)

86-96: Missing assertion for NEXTAUTH_URL in generated content.

Per the implementation, generateEnvFile writes NEXTAUTH_URL to the file, but this test doesn't verify it. Since NEXTAUTH_URL is in REQUIRED_VARS, it's worth asserting.

Suggested addition
     expect(content).toContain("ENCRYPTION_KEY=");
     expect(content).toContain("NEXTAUTH_SECRET=");
+    expect(content).toContain("NEXTAUTH_URL=");
     expect(content).toContain("ADMIN_BOOTSTRAP_TOKEN=");
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cli/src/__tests__/commands/env.test.ts` around lines 86 - 96, The test for
generateEnvFile is missing an assertion that the generated env content includes
NEXTAUTH_URL; update the "generates file when none exists" test (which calls
generateEnvFile and inspects mockWriteFileSync.mock.calls[0][1]) to also assert
that the content contains "NEXTAUTH_URL=" so that all REQUIRED_VARS produced by
generateEnvFile are verified.

68-75: Consider verifying all missing required vars.

The test only asserts ENCRYPTION_KEY and NEXTAUTH_SECRET are in missing, but NEXTAUTH_URL is also in REQUIRED_VARS per the implementation. Adding that assertion would strengthen coverage.

Suggested addition
     expect(result.ok).toBe(false);
     expect(result.missing).toContain("ENCRYPTION_KEY");
     expect(result.missing).toContain("NEXTAUTH_SECRET");
+    expect(result.missing).toContain("NEXTAUTH_URL");
   });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cli/src/__tests__/commands/env.test.ts` around lines 68 - 75, The test for
validateEnv currently checks for ENCRYPTION_KEY and NEXTAUTH_SECRET but omits
NEXTAUTH_URL which is listed in REQUIRED_VARS; update the test in env.test.ts
(the "reports specific missing vars" case) to also assert that result.missing
contains "NEXTAUTH_URL" (e.g. add
expect(result.missing).toContain("NEXTAUTH_URL")) so the test verifies all
required missing variables reported by validateEnv.

128-135: Add assertion for process.exitCode in this validation test.

The runEnv function sets process.exitCode = 1 on validation failure (line 94 of env.ts), but this test only verifies the error logging. Add expect(process.exitCode).toBe(1); to ensure the exit code side effect is covered.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cli/src/__tests__/commands/env.test.ts` around lines 128 - 135, The test for
runEnv's validation path is missing an assertion for the side-effect on
process.exitCode; after invoking runEnv({ validate: true }) and asserting
logError, add expect(process.exitCode).toBe(1); to verify that runEnv sets
process.exitCode to 1 on validation failure. Ensure this assertion is placed in
the "validates when --validate flag is set" test alongside the existing
expect(logError) check so the process.exitCode behavior is covered.
cli/src/commands/doctor.ts (2)

80-100: Consider handling config read errors gracefully.

If readProjectConfig() throws (e.g., missing or malformed neoboard.config.json), the error propagates uncaught. Wrapping this in a try-catch with a friendly "run neoboard init first" message would improve DX.

Suggested approach
 export async function runDoctor(): Promise<CheckResult[]> {
-  const config = readProjectConfig();
+  let config;
+  try {
+    config = readProjectConfig();
+  } catch {
+    return [
+      {
+        name: "Project config",
+        status: "fail",
+        message: "neoboard.config.json missing or invalid — run 'neoboard init'",
+      },
+    ];
+  }
   const results: CheckResult[] = [
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cli/src/commands/doctor.ts` around lines 80 - 100, Wrap the call to
readProjectConfig() in runDoctor() with a try-catch to gracefully handle missing
or malformed configuration: call readProjectConfig() inside try, and in catch
create/return a failing CheckResult (matching the CheckResult shape used by
checkDockerRunning()/checkNodeVersion()) that contains a clear user-facing
message like "Could not read project config — run `neoboard init` first or fix
neoboard.config.json" and include the caught error details for logging; this
ensures runDoctor() does not throw and provides a helpful actionable result
instead of crashing.

102-114: Return value semantics may confuse callers.

printResults returns true when failures exist, which is inverted from typical "success" semantics. A caller writing if (printResults(results)) { abort(); } is correct but reads awkwardly. Consider renaming to printResultsAndCheckForFailures or inverting to return allPassed.

Alternative: invert to return success boolean
-export function printResults(results: CheckResult[]): boolean {
-  let hasFailure = false;
+export function printResults(results: CheckResult[]): boolean {
+  let allPassed = true;
   for (const r of results) {
     if (r.status === "ok") {
       success(r.message);
     } else if (r.status === "warn") {
       warn(r.message);
     } else {
       logError(r.message);
-      hasFailure = true;
+      allPassed = false;
     }
   }
-  return hasFailure;
+  return allPassed;
 }

Callers would then use if (!printResults(results)) { abort(); } which reads more naturally.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cli/src/commands/doctor.ts` around lines 102 - 114, The function printResults
currently returns true when failures exist (hasFailure), which is
counterintuitive; change it to return a success boolean (e.g., allPassed)
instead: initialize allPassed = true, set allPassed = false when any CheckResult
has non-"ok" status, keep calls to
success(r.message)/warn(r.message)/logError(r.message) unchanged, and update any
callers of printResults to expect true on success (or alternatively rename the
function to printResultsAndCheckForFailures if you prefer keeping current
semantics). Ensure you update references to the hasFailure symbol and any
boolean checks that assume the old inverted meaning.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@bin/neoboard`:
- Around line 1-2: The top-level entry script imports "../cli/dist/index.js"
which doesn't exist and the root package.json lacks a "bin" registration; either
remove this duplicate entry script or make the CLI build available and register
it: 1) Delete the top-level entry that does import("../cli/dist/index.js") and
remove any corresponding root "bin" entry, or 2) ensure the CLI is built to
produce cli/dist/index.js during your build (or copy it into place) and add a
"bin": { "neoboard": "./bin/neoboard" } (or point root package.json to the
compiled CLI) so the entry matches cli/package.json's "neoboard":
"./dist/index.js". Ensure the chosen fix keeps the cli/package.json "neoboard"
target consistent with the published entry point.

In `@cli/src/commands/db/dump.ts`:
- Around line 25-43: The spinner started by createSpinner("Dumping database...")
is not stopped if run() throws; wrap the logic that calls run(), writeFileSync
and statSync in a try/catch/finally around the block using spinner.start(), call
spinner.succeed(...) on success and spinner.fail(...) (or spinner.stop()) in the
catch/finally to ensure the spinner is stopped, rethrow or surface the error
after cleanup so the command exits appropriately; update the block containing
getMode(), run(...), writeFileSync(outPath, sql), statSync(outPath) and
spinner.succeed to use this try/catch/finally pattern.
- Around line 30-38: The shell commands in the db dump branches interpolate
unescaped config.postgres.user and config.postgres.database into run(...) calls
(the docker and non-docker branches around the conditional on mode ===
"docker"), enabling command injection; fix by passing arguments as a safe array
or using an execFile/spawn-based helper instead of building a single
interpolated string in run, or by applying a proper shell-escaping utility to
config.postgres.user and config.postgres.database before interpolation; update
the call sites in this file (the run(...) invocations) and mirror the same safe
approach used for CLI docker helpers (see run and any wrapper in
cli/src/lib/docker.ts) so both docker exec and local pg_dump invocations use
argument vectors or escaped values.

In `@cli/src/commands/db/migrate.ts`:
- Around line 41-51: The showDryRun function currently lists all migrations from
readJournal() which is misleading because drizzle-kit only applies pending
migrations; update showDryRun to query the database's __drizzle_migrations table
(or call an existing function that fetches applied migrations) to compute
pending migrations by diffing journal.entries against applied entries and then
log only those pending tags with info; if querying the DB isn't possible here,
change the message text in showDryRun to clearly state that it shows all
migrations from the journal rather than pending ones.

In `@cli/src/commands/db/reset.ts`:
- Around line 61-75: The commands in reset.ts interpolate config values user and
database directly into shell strings (used in dockerExec and run), creating
command-injection risk; validate and sanitize these values before use (e.g.,
enforce a strict whitelist regex like only alphanumerics, dashes, underscores),
and reject or throw when invalid, and where possible replace string-interpolated
shell execution with argument-based exec/spawn calls that pass user and database
as separate args (or use a shell-escaping utility) so dockerExec and run no
longer execute raw untrusted strings.

In `@cli/src/commands/db/seed.ts`:
- Around line 39-40: The seed command is vulnerable because config.seed.script
is concatenated into a shell command in run(`node
${paths.root}/${config.seed.script}`, ...) — validate and sanitize that value:
resolve it with path.resolve(paths.root, config.seed.script) or path.join and
ensure the resolved path is inside paths.root (e.g., startsWith or compare paths
after path.resolve) or restrict to a whitelist of allowed filenames; then call
the runner without shell interpolation (use execFile/spawn with argv array or
pass the resolved absolute path only) so untrusted `../../../` segments cannot
escape the project root; update run invocation in seed.ts (and keep
spinner.succeed) to use the validated resolved path.
- Around line 27-30: The dockerExec call is using a hardcoded cypher file path;
update the call in seed.ts (the dockerExec invocation) to use
config.seed.neo4j_cypher instead of "/var/lib/neo4j/import/init.cypher",
ensuring the value is interpolated into the command string passed to dockerExec
(and correctly quoted/escaped if needed) so the container path comes from
configuration rather than being hardcoded.
- Around line 8-14: The command in seed.ts uses dockerExec("neoboard-neo4j",
`cypher-shell -u ${config.neo4j.user} -p ${config.neo4j.password} "...`) which
interpolates the Neo4j password into a shell string and creates a shell
injection risk; update dockerExec to accept RunOptions (including an env map) or
call the underlying run() directly and pass the password via environment
variables (e.g., set NEO4J_AUTH or CYHER_SHELL_PASSWORD in env) and replace the
inline password interpolation in the dockerExec call in seed.ts (and similarly
fix isPgReady() and isNeo4jReady() in docker.ts) so credentials are passed
through env rather than embedded in the shell command.

In `@cli/src/commands/dev.ts`:
- Around line 18-25: The process-level signal handlers registered around the
child process (cleanup and process.on("SIGINT"/"SIGTERM")) are never removed,
causing listener leaks when runDev() is called multiple times; change cleanup to
a named function (e.g., const cleanup = () => child.kill()) and store the two
handler references (e.g., sigintHandler and sigtermHandler or reuse cleanup)
when calling process.on, then on child.on("close") call process.off or
process.removeListener for those two handlers to deregister them (and also call
process.off/removeListener in the cleanup function path if the child is killed)
so the listeners are removed when the child exits.

In `@cli/src/commands/env.ts`:
- Around line 22-34: The parseEnvFile function currently leaves surrounding
quotes in values (e.g., DATABASE_URL="...") which breaks consumers; update
parseEnvFile to, after computing value in the loop, detect if value begins and
ends with the same quote char (single or double) and if so remove those outer
quotes and unescape common escape sequences (at minimum handle escaped quotes
and backslashes, and standard escapes like \n,\r,\t) before assigning to
vars[key]; ensure this logic is applied to the local variable value inside
parseEnvFile so vars[key] stores the unquoted, unescaped string.

In `@cli/src/commands/init.ts`:
- Around line 21-30: The init command currently always calls writeLocalConfig({
mode }) which silently overwrites .neoboard.local on every run; change init to
only write the local config when a mode was explicitly provided (the CLI flag
that sets the local "mode") or when no local config exists, instead of
unconditionally; locate the writeLocalConfig call in init.ts and wrap it with a
conditional that checks the presence of the mode flag (or checks for an existing
.neoboard.local via existsSync or readLocalConfig) so rerunning init without
--mode does not reset the mode, or alternatively add explicit support for
reconfiguring mode only when a --mode value is passed.

In `@cli/src/commands/stop.ts`:
- Around line 4-6: runStop always calls composeDown without passing mode info so
full-stack (docker) services started by start may not be torn down; update
runStop to pass a mode flag (e.g., full: true/false) into composeDown based on
the same mode determination used by start, and update composeDown in
cli/src/lib/docker.ts to accept an options object with full?: boolean and call
composeFile(opts?.full) when building the docker-compose invocation; ensure
runStop continues to pass volumes via the existing volumes option when calling
composeDown.

In `@cli/src/lib/config.ts`:
- Around line 23-40: The loop termination in findProjectRoot uses while (dir !==
"/"), which fails on Windows roots like C:\; change the termination test to
detect when we've reached the filesystem root (e.g., use while (dir !==
dirname(dir)) or compare against path.parse(dir).root) so the loop stops
correctly across platforms; update the loop condition and keep existing logic
that walks up via dirname(dir) and throws the same error if not found.

In `@cli/src/lib/docker.ts`:
- Around line 60-78: The docker-related functions (dockerExec, isPgReady,
isNeo4jReady) interpolate config values directly into shell commands which
allows special characters or injection; update them to safely pass arguments
(container, usernames, passwords, commands) without shell interpolation — either
by using a command API that accepts an args array (e.g., spawn/execFile) or by
applying a robust shell-escaping utility to each interpolated value (escape
container, config.postgres.user, config.neo4j.user, config.neo4j.password and
any command strings) before constructing the docker exec call; ensure dockerExec
and the readiness checks use the new safe-call pattern so usernames/passwords
cannot break the shell invocation.

In `@cli/src/lib/health.ts`:
- Around line 16-20: Wrap the call to check() inside the health-wait loop in a
try-catch so any thrown errors (e.g., from isPgReady/isNeo4jReady) are caught;
on catch call spinner.fail(`${label} failed: ${err.message}`) (or a similar
descriptive failure message) to ensure the spinner is not left indeterminate,
then rethrow the error (or return) so the caller still sees the failure. Locate
the loop that calls check(), spinner, and label in the health wait logic and add
this try-catch around check().

In `@cli/src/lib/output.ts`:
- Around line 24-34: banner currently calls Math.max(...lines.map(...)) which
throws -Infinity for an empty array; update the banner function (the maxLen
calculation used by top/bottom and the loop that pads lines) to handle an empty
lines array by setting maxLen to 0 when lines.length === 0 (e.g., use a
conditional or default value) so the top/bottom border construction
("\u2554"/"\u2557"/"\u255A"/"\u255D" with "\u2550".repeat(maxLen + 2)) and the
for loop behave correctly for empty input.

---

Nitpick comments:
In `@cli/package.json`:
- Line 23: Update the CLI package.json dependency for "@types/node" to match the
Node 22 runtime (e.g., change "@types/node" to a Node-22-aligned version such as
"^22.0.0") so type-checking reflects the actual runtime; update package.json's
dependencies entry for "@types/node" and reinstall/update lockfile (npm/yarn) to
ensure the new types are used during CI and local type checks.

In `@cli/src/__tests__/commands/doctor.test.ts`:
- Around line 83-89: The test is flaky because it reads the real runtime Node
version; update the test for checkNodeVersion to make it deterministic by
mocking process.version (or spying) to a controlled value before calling
checkNodeVersion and restoring it after; e.g. use jest.spyOn(process, "version",
"get") or Object.defineProperty to return a specific version string (like
"v20.0.0" and a negative case if needed), call checkNodeVersion(), assert on
result.status, then restore the original getter so other tests are unaffected.

In `@cli/src/__tests__/commands/env.test.ts`:
- Around line 86-96: The test for generateEnvFile is missing an assertion that
the generated env content includes NEXTAUTH_URL; update the "generates file when
none exists" test (which calls generateEnvFile and inspects
mockWriteFileSync.mock.calls[0][1]) to also assert that the content contains
"NEXTAUTH_URL=" so that all REQUIRED_VARS produced by generateEnvFile are
verified.
- Around line 68-75: The test for validateEnv currently checks for
ENCRYPTION_KEY and NEXTAUTH_SECRET but omits NEXTAUTH_URL which is listed in
REQUIRED_VARS; update the test in env.test.ts (the "reports specific missing
vars" case) to also assert that result.missing contains "NEXTAUTH_URL" (e.g. add
expect(result.missing).toContain("NEXTAUTH_URL")) so the test verifies all
required missing variables reported by validateEnv.
- Around line 128-135: The test for runEnv's validation path is missing an
assertion for the side-effect on process.exitCode; after invoking runEnv({
validate: true }) and asserting logError, add expect(process.exitCode).toBe(1);
to verify that runEnv sets process.exitCode to 1 on validation failure. Ensure
this assertion is placed in the "validates when --validate flag is set" test
alongside the existing expect(logError) check so the process.exitCode behavior
is covered.

In `@cli/src/__tests__/commands/start.test.ts`:
- Around line 77-85: The test needs an ordering assertion to ensure migrations
run after health checks: in the "runs migrations after health checks pass" spec
(where runStart, mockWaitForHealth, and mockRunDbMigrate are used), add an
assertion that mockWaitForHealth was called before mockRunDbMigrate — e.g., use
jest-extended's toHaveBeenCalledBefore(mockRunDbMigrate) or assert invocation
order via mockWaitForHealth.mock.invocationCallOrder[0] <
mockRunDbMigrate.mock.invocationCallOrder[0]; this ensures runDbMigrate() occurs
after waitForHealth() when runStart() is executed.

In `@cli/src/__tests__/commands/stop.test.ts`:
- Around line 21-29: Add an assertion that the mocked success function is called
after runStop to verify the command prints the success message; in the tests
referencing runStop and mockComposeDown (the two it blocks), add
expect(success).toHaveBeenCalled() (or
expect(success).toHaveBeenCalledWith(<expected message>) if you want to assert
the exact text) after the existing composeDown assertions so both cases verify
success() was invoked.

In `@cli/src/commands/db/migrate.ts`:
- Around line 70-74: The CLI accepts a non-functional --to flag (checked via
opts.to and only emits warn(...)) which can be misleading; either remove the
--to option registration or mark it hidden until implemented (call .hideHelp()
on the option definition) and remove the opts.to handling/warning in migrate.ts
(or ensure the option actually affects behavior). Locate the code that
defines/parses the --to option and the warn(...) block in migrate.ts and either
delete the option + the opts.to branch or update the option declaration to
.hideHelp() and keep or adjust the warning accordingly so the CLI surface
matches actual functionality.

In `@cli/src/commands/db/reset.ts`:
- Around line 25-27: The isLocalhost function currently only checks "localhost"
and "127.0.0.1"; update isLocalhost(host: string) to also treat the IPv6
loopback "::1" as local by including "::1" in the comparison (or
normalizing/using a set of loopback addresses) so calls to isLocalhost correctly
recognize IPv6 localhost.
- Around line 15-23: The getDatabaseHost function currently uses a fragile regex
to parse DATABASE_URL; instead read the env file (readFileSync(paths.envFile,
"utf-8")), extract the DATABASE_URL entry, pass its value into the standard URL
constructor (new URL(...)) to reliably obtain hostname (which correctly handles
URL-encoded credentials and IPv6 bracketed hosts), and return url.hostname or
fallback to "localhost" on any parse/missing errors; update getDatabaseHost to
implement this flow and preserve the existing try/catch behavior.

In `@cli/src/commands/doctor.ts`:
- Around line 80-100: Wrap the call to readProjectConfig() in runDoctor() with a
try-catch to gracefully handle missing or malformed configuration: call
readProjectConfig() inside try, and in catch create/return a failing CheckResult
(matching the CheckResult shape used by checkDockerRunning()/checkNodeVersion())
that contains a clear user-facing message like "Could not read project config —
run `neoboard init` first or fix neoboard.config.json" and include the caught
error details for logging; this ensures runDoctor() does not throw and provides
a helpful actionable result instead of crashing.
- Around line 102-114: The function printResults currently returns true when
failures exist (hasFailure), which is counterintuitive; change it to return a
success boolean (e.g., allPassed) instead: initialize allPassed = true, set
allPassed = false when any CheckResult has non-"ok" status, keep calls to
success(r.message)/warn(r.message)/logError(r.message) unchanged, and update any
callers of printResults to expect true on success (or alternatively rename the
function to printResultsAndCheckForFailures if you prefer keeping current
semantics). Ensure you update references to the hasFailure symbol and any
boolean checks that assume the old inverted meaning.

In `@cli/src/commands/env.ts`:
- Around line 67-68: The writeFileSync call can throw if the target directory or
permissions are wrong; wrap writeFileSync(paths.envFile, lines.join("\n")) in a
try/catch around the write and only call success("Generated app/.env.local") on
success; in the catch block log a clear, user-friendly error that includes
paths.envFile and the caught error message (e.g., via console.error or the
existing error logging helper) and exit non-zero (process.exit(1)) so the CLI
fails cleanly.

In `@cli/src/commands/start.ts`:
- Around line 24-27: The two independent health checks called via waitForHealth
(using isPgReady and isNeo4jReady) should run in parallel to reduce startup
time: after calling readProjectConfig(), replace the sequential awaits for
waitForHealth({ check: isPgReady, label: "PostgreSQL" }) and waitForHealth({
check: isNeo4jReady, label: "Neo4j" }) with a single Promise.all that awaits
both calls concurrently so both checks execute in parallel.

In `@cli/src/commands/status.ts`:
- Around line 57-62: The status output uses "stopped" when a readiness check
fails which is misleading; update the messages in the info(...) calls that
reference pgHealthy and neo4jHealthy so they display "unhealthy" or "not ready"
(e.g., "unhealthy") instead of "stopped" while keeping the port info
(config.ports.postgres and config.ports.neo4j_bolt) and the existing conditional
logic intact; locate the two info(...) calls that format `PostgreSQL  
${pgHealthy ? "healthy" : "stopped"} (localhost:${config.ports.postgres})` and
`Neo4j        ${neo4jHealthy ? "healthy" : "stopped"}
(localhost:${config.ports.neo4j_bolt})` and change the false branch text to the
new term.

In `@cli/src/index.ts`:
- Around line 150-155: The direct-run detection using process.argv[1] in the
isDirectRun variable is brittle; replace it with a robust mechanism such as
checking an explicit sentinel environment variable or using import.meta.url to
detect direct invocation, or move program.parse() out of index.ts into a
dedicated bin entry (e.g., bin/neoboard.js) that imports the module and calls
program.parse() explicitly. Update references to isDirectRun and the conditional
that calls program.parse() so it uses the chosen approach (sentinel env var
check or import.meta.url comparison) or remove the check entirely and perform
parsing only from the new bin/neoboard.js entrypoint.
- Around line 11-13: The synchronous read of package.json using
readFileSync/JSON.parse at module load (the pkg constant built from
readFileSync(join(__dirname, "..", "package.json"), "utf-8")) should be replaced
with a lazy or async initialization and error handling: move the file-read into
an async helper (e.g., getPackageJson or initCliConfig) or use
fs.promises.readFile so the event loop isn't blocked, wrap the read/JSON.parse
in try/catch to handle missing/invalid files and provide a sensible fallback or
propagate a clear error, and update any callers to await the helper instead of
relying on the top-level pkg constant.

In `@cli/src/lib/config.ts`:
- Around line 105-121: Modify readProjectConfig and readLocalConfig to surface
JSON parse errors instead of failing silently: in each catch block capture the
error (catch (err)), log a warning that includes the config path
(paths.projectConfig / paths.localConfig) and the error message (use the project
logger if available or console.warn), then return the DEFAULT_*_CONFIG as
before; update functions readProjectConfig and readLocalConfig to include these
warning logs so users know their config file has syntax issues.

In `@neoboard.config.json`:
- Around line 8-16: The config currently commits default DB credentials for
"postgres" (user/password) and "neo4j" (user/password); remove these hardcoded
passwords and replace them with references to environment-driven or generated
secrets (e.g., read from env vars or a local state file) so credentials are not
stored in VCS; update the "postgres" and "neo4j" entries to load their
"password" (and optionally "user") from a secure source and document the init
step to generate/store per-instance credentials instead of committing them.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 06b15876-32b5-4b68-bd7a-f937c07ab2dc

📥 Commits

Reviewing files that changed from the base of the PR and between 8610931 and 2f5d015.

⛔ Files ignored due to path filters (1)
  • cli/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (50)
  • .github/workflows/ci.yml
  • .gitignore
  • bin/neoboard
  • cli/package.json
  • cli/src/__tests__/commands/db/dump.test.ts
  • cli/src/__tests__/commands/db/migrate.test.ts
  • cli/src/__tests__/commands/db/reset.test.ts
  • cli/src/__tests__/commands/db/seed.test.ts
  • cli/src/__tests__/commands/demo.test.ts
  • cli/src/__tests__/commands/dev.test.ts
  • cli/src/__tests__/commands/doctor.test.ts
  • cli/src/__tests__/commands/env.test.ts
  • cli/src/__tests__/commands/init.test.ts
  • cli/src/__tests__/commands/setup.test.ts
  • cli/src/__tests__/commands/start.test.ts
  • cli/src/__tests__/commands/status.test.ts
  • cli/src/__tests__/commands/stop.test.ts
  • cli/src/__tests__/lib/config.test.ts
  • cli/src/__tests__/lib/docker.test.ts
  • cli/src/__tests__/lib/exec.test.ts
  • cli/src/__tests__/lib/health.test.ts
  • cli/src/__tests__/lib/output.test.ts
  • cli/src/__tests__/lib/ports.test.ts
  • cli/src/__tests__/program.test.ts
  • cli/src/commands/db/dump.ts
  • cli/src/commands/db/migrate.ts
  • cli/src/commands/db/reset.ts
  • cli/src/commands/db/seed.ts
  • cli/src/commands/demo.ts
  • cli/src/commands/dev.ts
  • cli/src/commands/doctor.ts
  • cli/src/commands/env.ts
  • cli/src/commands/init.ts
  • cli/src/commands/setup.ts
  • cli/src/commands/start.ts
  • cli/src/commands/status.ts
  • cli/src/commands/stop.ts
  • cli/src/index.ts
  • cli/src/lib/config.ts
  • cli/src/lib/docker.ts
  • cli/src/lib/exec.ts
  • cli/src/lib/health.ts
  • cli/src/lib/output.ts
  • cli/src/lib/ports.ts
  • cli/src/lib/prompt.ts
  • cli/tsconfig.json
  • cli/vitest.config.ts
  • neoboard.config.json
  • package.json
  • sonar-project.properties

Comment thread bin/neoboard
Comment on lines +1 to +2
#!/usr/bin/env node
import("../cli/dist/index.js");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify the main-module detection logic in cli/src/index.ts
rg -n -A5 'isDirectRun' cli/src/index.ts

# Check if there are any tests covering the entry point behavior
rg -l 'bin/neoboard' --type ts

Repository: alfredo1996/neoboard

Length of output: 236


🏁 Script executed:

#!/bin/bash
# Check package.json for bin entry
rg -A5 '"bin"' package.json

# Verify the import path resolves correctly
# From bin/neoboard, ../cli/dist/index.js should resolve to cli/dist/index.js at root
fd -t f 'index.js' cli/dist 2>/dev/null || echo "cli/dist/index.js does not exist"

# Check the actual directory structure
ls -la bin/neoboard 2>/dev/null
head -2 bin/neoboard

# Look for any build/dist configuration in package.json
rg -A10 '"scripts"' package.json | head -20

Repository: alfredo1996/neoboard

Length of output: 741


🏁 Script executed:

#!/bin/bash
# Get full package.json bin field
cat package.json | jq '.bin' 2>/dev/null || grep -A3 '"bin"' package.json

# Check if cli directory has its own package.json (monorepo)
ls -la cli/package.json 2>/dev/null || echo "cli/package.json not found"

# Check if cli/dist directory exists
ls -la cli/dist 2>/dev/null || echo "cli/dist directory does not exist"

# Check if there's a build output anywhere
find cli -type d -name 'dist' -o -name 'build' 2>/dev/null | head -10

# Check cli/src structure
ls -la cli/src/ 2>/dev/null | head -20

Repository: alfredo1996/neoboard

Length of output: 506


🏁 Script executed:

#!/bin/bash
# Check cli/package.json build configuration
cat cli/package.json

# Check if there's a tsconfig for cli
ls -la cli/tsconfig.json 2>/dev/null || echo "No tsconfig.json in cli"

# Check the build scripts in cli/package.json more thoroughly
cat cli/package.json | jq '.scripts' 2>/dev/null || grep -A20 '"scripts"' cli/package.json

Repository: alfredo1996/neoboard

Length of output: 841


Remove or fix the broken entry point.

The bin/neoboard file references ../cli/dist/index.js, which doesn't exist (the CLI package hasn't been compiled). Additionally, the root package.json has no "bin" field to register this entry point. The cli/package.json already correctly defines "neoboard": "./dist/index.js", so either remove the duplicate root entry or ensure the CLI package is built as part of the build process.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@bin/neoboard` around lines 1 - 2, The top-level entry script imports
"../cli/dist/index.js" which doesn't exist and the root package.json lacks a
"bin" registration; either remove this duplicate entry script or make the CLI
build available and register it: 1) Delete the top-level entry that does
import("../cli/dist/index.js") and remove any corresponding root "bin" entry, or
2) ensure the CLI is built to produce cli/dist/index.js during your build (or
copy it into place) and add a "bin": { "neoboard": "./bin/neoboard" } (or point
root package.json to the compiled CLI) so the entry matches cli/package.json's
"neoboard": "./dist/index.js". Ensure the chosen fix keeps the cli/package.json
"neoboard" target consistent with the published entry point.

Comment on lines +25 to +43
const spinner = createSpinner("Dumping database...");
spinner.start();

const mode = getMode();
let sql: string;
if (mode === "docker") {
sql = run(
`docker exec neoboard-postgres pg_dump -U ${config.postgres.user} ${config.postgres.database}${dataFlag}`,
);
} else {
sql = run(
`pg_dump -h localhost -p ${config.ports.postgres} -U ${config.postgres.user} ${config.postgres.database}${dataFlag}`,
);
}

writeFileSync(outPath, sql);
const size = statSync(outPath).size;
spinner.succeed(`Backup saved to ${outPath} (${formatSize(size)})`);
success("Database dump complete");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Spinner not stopped on error path.

If run() throws, the spinner keeps spinning. Wrap in try/catch to ensure cleanup:

Proposed fix
   const spinner = createSpinner("Dumping database...");
   spinner.start();
 
+  let sql: string;
+  try {
     const mode = getMode();
-  let sql: string;
     if (mode === "docker") {
       sql = run(
         `docker exec neoboard-postgres pg_dump -U ${config.postgres.user} ${config.postgres.database}${dataFlag}`,
       );
     } else {
       sql = run(
         `pg_dump -h localhost -p ${config.ports.postgres} -U ${config.postgres.user} ${config.postgres.database}${dataFlag}`,
       );
     }
+  } catch (err) {
+    spinner.fail("Database dump failed");
+    throw err;
+  }
 
   writeFileSync(outPath, sql);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const spinner = createSpinner("Dumping database...");
spinner.start();
const mode = getMode();
let sql: string;
if (mode === "docker") {
sql = run(
`docker exec neoboard-postgres pg_dump -U ${config.postgres.user} ${config.postgres.database}${dataFlag}`,
);
} else {
sql = run(
`pg_dump -h localhost -p ${config.ports.postgres} -U ${config.postgres.user} ${config.postgres.database}${dataFlag}`,
);
}
writeFileSync(outPath, sql);
const size = statSync(outPath).size;
spinner.succeed(`Backup saved to ${outPath} (${formatSize(size)})`);
success("Database dump complete");
const spinner = createSpinner("Dumping database...");
spinner.start();
let sql: string;
try {
const mode = getMode();
if (mode === "docker") {
sql = run(
`docker exec neoboard-postgres pg_dump -U ${config.postgres.user} ${config.postgres.database}${dataFlag}`,
);
} else {
sql = run(
`pg_dump -h localhost -p ${config.ports.postgres} -U ${config.postgres.user} ${config.postgres.database}${dataFlag}`,
);
}
} catch (err) {
spinner.fail("Database dump failed");
throw err;
}
writeFileSync(outPath, sql);
const size = statSync(outPath).size;
spinner.succeed(`Backup saved to ${outPath} (${formatSize(size)})`);
success("Database dump complete");
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cli/src/commands/db/dump.ts` around lines 25 - 43, The spinner started by
createSpinner("Dumping database...") is not stopped if run() throws; wrap the
logic that calls run(), writeFileSync and statSync in a try/catch/finally around
the block using spinner.start(), call spinner.succeed(...) on success and
spinner.fail(...) (or spinner.stop()) in the catch/finally to ensure the spinner
is stopped, rethrow or surface the error after cleanup so the command exits
appropriately; update the block containing getMode(), run(...),
writeFileSync(outPath, sql), statSync(outPath) and spinner.succeed to use this
try/catch/finally pattern.

Comment on lines +30 to +38
if (mode === "docker") {
sql = run(
`docker exec neoboard-postgres pg_dump -U ${config.postgres.user} ${config.postgres.database}${dataFlag}`,
);
} else {
sql = run(
`pg_dump -h localhost -p ${config.ports.postgres} -U ${config.postgres.user} ${config.postgres.database}${dataFlag}`,
);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if config validation exists elsewhere
rg -n "postgres\.(user|database)" --type=ts -C3 | head -80

Repository: alfredo1996/neoboard

Length of output: 2583


🏁 Script executed:

# Find readProjectConfig implementation
rg -n "readProjectConfig" --type=ts -A20 | head -100

Repository: alfredo1996/neoboard

Length of output: 5489


🏁 Script executed:

# Check config parsing and validation
fd "config" --type=ts | grep -E "(config|\.ts)" | head -20

Repository: alfredo1996/neoboard

Length of output: 233


🏁 Script executed:

# Look at the config test file to understand validation
cat -n cli/src/__tests__/lib/config.test.ts | head -100

Repository: alfredo1996/neoboard

Length of output: 3737


🏁 Script executed:

# Find and read the config.ts implementation
find cli/src -name "config.ts" -type f | head -5

Repository: alfredo1996/neoboard

Length of output: 85


🏁 Script executed:

# Read the config.ts file
cat -n cli/src/lib/config.ts | head -150

Repository: alfredo1996/neoboard

Length of output: 4234


Unescaped config values in shell commands enable command injection.

config.postgres.user and config.postgres.database are interpolated directly into shell commands without escaping. Users who edit neoboard.config.json could inject shell metacharacters to execute arbitrary commands.

This pattern also appears in cli/src/lib/docker.ts:68 and affects cli/src/commands/env.ts:53 (though in a connection string context).

Apply shell escaping or use an argument array approach. Example:

Suggested fix
+function shellEscape(str: string): string {
+  return `'${str.replace(/'/g, "'\\''")}'`;
+}
+
 if (mode === "docker") {
   sql = run(
-    `docker exec neoboard-postgres pg_dump -U ${config.postgres.user} ${config.postgres.database}${dataFlag}`,
+    `docker exec neoboard-postgres pg_dump -U ${shellEscape(config.postgres.user)} ${shellEscape(config.postgres.database)}${dataFlag}`,
   );
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cli/src/commands/db/dump.ts` around lines 30 - 38, The shell commands in the
db dump branches interpolate unescaped config.postgres.user and
config.postgres.database into run(...) calls (the docker and non-docker branches
around the conditional on mode === "docker"), enabling command injection; fix by
passing arguments as a safe array or using an execFile/spawn-based helper
instead of building a single interpolated string in run, or by applying a proper
shell-escaping utility to config.postgres.user and config.postgres.database
before interpolation; update the call sites in this file (the run(...)
invocations) and mirror the same safe approach used for CLI docker helpers (see
run and any wrapper in cli/src/lib/docker.ts) so both docker exec and local
pg_dump invocations use argument vectors or escaped values.

Comment on lines +41 to +51
export function showDryRun(): void {
const journal = readJournal();
if (!journal) {
warn("No migration journal found.");
return;
}
info(`Would apply ${journal.entries.length} migration(s):`);
for (const entry of journal.entries) {
info(` - ${entry.tag}`);
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check how drizzle-kit tracks applied migrations - it uses a __drizzle_migrations table
rg -n "drizzle_migrations" --type=sql --type=ts

Repository: alfredo1996/neoboard

Length of output: 46


🏁 Script executed:

fd -i "journal" --type f

Repository: alfredo1996/neoboard

Length of output: 105


🏁 Script executed:

rg -n "readJournal" -B 5 -A 10

Repository: alfredo1996/neoboard

Length of output: 2359


🏁 Script executed:

rg -n "migrations" --type ts -A 3 -B 3 | head -100

Repository: alfredo1996/neoboard

Length of output: 5869


🏁 Script executed:

cat app/drizzle/migrations/meta/_journal.json

Repository: alfredo1996/neoboard

Length of output: 418


🏁 Script executed:

rg -n "drizzle-kit" --type ts -B 2 -A 2

Repository: alfredo1996/neoboard

Length of output: 1446


🏁 Script executed:

cat -n cli/src/__tests__/commands/db/migrate.test.ts | head -100

Repository: alfredo1996/neoboard

Length of output: 3565


showDryRun displays all available migrations, not just pending ones.

The journal contains all migrations in the migrations directory. Since drizzle-kit applies only pending migrations (tracked in the database's __drizzle_migrations table), the --dry-run output will be misleading if migrations have already been applied. Consider querying the database to filter and display only pending migrations, or clarify in the output message.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cli/src/commands/db/migrate.ts` around lines 41 - 51, The showDryRun function
currently lists all migrations from readJournal() which is misleading because
drizzle-kit only applies pending migrations; update showDryRun to query the
database's __drizzle_migrations table (or call an existing function that fetches
applied migrations) to compute pending migrations by diffing journal.entries
against applied entries and then log only those pending tags with info; if
querying the DB isn't possible here, change the message text in showDryRun to
clearly state that it shows all migrations from the journal rather than pending
ones.

Comment on lines +61 to +75
dockerExec(
"neoboard-postgres",
`psql -U ${user} -d postgres -c "DROP DATABASE IF EXISTS ${database}"`,
);
dockerExec(
"neoboard-postgres",
`psql -U ${user} -d postgres -c "CREATE DATABASE ${database}"`,
);
} else {
run(
`psql -h localhost -U ${user} -d postgres -c "DROP DATABASE IF EXISTS ${database}"`,
);
run(
`psql -h localhost -U ${user} -d postgres -c "CREATE DATABASE ${database}"`,
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Unsanitized config values in shell commands.

user and database from config are interpolated directly into shell commands. If neoboard.config.json contains malicious values (e.g., user: "x; rm -rf /"), it could lead to command injection. While this is a local dev tool, consider validating these values match expected patterns.

🛡️ Suggested validation
 const { user, database } = config.postgres;
+
+// Validate identifiers contain only safe characters
+const identifierPattern = /^[a-zA-Z_][a-zA-Z0-9_]*$/;
+if (!identifierPattern.test(user) || !identifierPattern.test(database)) {
+  logError("Invalid postgres user or database name in config");
+  process.exitCode = 1;
+  return;
+}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cli/src/commands/db/reset.ts` around lines 61 - 75, The commands in reset.ts
interpolate config values user and database directly into shell strings (used in
dockerExec and run), creating command-injection risk; validate and sanitize
these values before use (e.g., enforce a strict whitelist regex like only
alphanumerics, dashes, underscores), and reject or throw when invalid, and where
possible replace string-interpolated shell execution with argument-based
exec/spawn calls that pass user and database as separate args (or use a
shell-escaping utility) so dockerExec and run no longer execute raw untrusted
strings.

Comment thread cli/src/commands/stop.ts
Comment on lines +4 to +6
export async function runStop(opts?: { volumes?: boolean }): Promise<void> {
composeDown({ volumes: opts?.volumes });
success("NeoBoard services stopped");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

stop is not mode-aware, so full-stack services may remain running.

Line 5 always routes through default composeDown, which uses docker-compose.yml. If start launched a full/docker mode stack, stop may not tear it down completely.

💡 Suggested direction
- export async function runStop(opts?: { volumes?: boolean }): Promise<void> {
-   composeDown({ volumes: opts?.volumes });
+ export async function runStop(opts?: { volumes?: boolean }): Promise<void> {
+   const mode = readLocalConfig().mode;
+   composeDown({ volumes: opts?.volumes, full: mode === "docker" });
    success("NeoBoard services stopped");
 }

And extend composeDown in cli/src/lib/docker.ts to accept { full?: boolean } and call composeFile(opts?.full).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cli/src/commands/stop.ts` around lines 4 - 6, runStop always calls
composeDown without passing mode info so full-stack (docker) services started by
start may not be torn down; update runStop to pass a mode flag (e.g., full:
true/false) into composeDown based on the same mode determination used by start,
and update composeDown in cli/src/lib/docker.ts to accept an options object with
full?: boolean and call composeFile(opts?.full) when building the docker-compose
invocation; ensure runStop continues to pass volumes via the existing volumes
option when calling composeDown.

Comment thread cli/src/lib/config.ts
Comment on lines +23 to +40
export function findProjectRoot(startDir?: string): string {
let dir = startDir ?? dirname(fileURLToPath(import.meta.url));
while (dir !== "/") {
const pkgPath = join(dir, "package.json");
if (existsSync(pkgPath)) {
try {
const pkg = JSON.parse(readFileSync(pkgPath, "utf-8"));
if (pkg.name === "neoboard") return dir;
} catch {
/* skip */
}
}
dir = dirname(dir);
}
throw new Error(
"Could not find NeoBoard project root (package.json with name 'neoboard')",
);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Windows path compatibility.

while (dir !== "/") won't terminate on Windows where roots are like C:\. If cross-platform support is needed, consider checking dir === dirname(dir) instead.

🛡️ Proposed cross-platform fix
 export function findProjectRoot(startDir?: string): string {
   let dir = startDir ?? dirname(fileURLToPath(import.meta.url));
-  while (dir !== "/") {
+  const prevDir = () => dirname(dir);
+  while (dir !== prevDir()) {
     const pkgPath = join(dir, "package.json");
     if (existsSync(pkgPath)) {

Or more simply:

-  while (dir !== "/") {
+  while (dir !== dirname(dir)) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export function findProjectRoot(startDir?: string): string {
let dir = startDir ?? dirname(fileURLToPath(import.meta.url));
while (dir !== "/") {
const pkgPath = join(dir, "package.json");
if (existsSync(pkgPath)) {
try {
const pkg = JSON.parse(readFileSync(pkgPath, "utf-8"));
if (pkg.name === "neoboard") return dir;
} catch {
/* skip */
}
}
dir = dirname(dir);
}
throw new Error(
"Could not find NeoBoard project root (package.json with name 'neoboard')",
);
}
export function findProjectRoot(startDir?: string): string {
let dir = startDir ?? dirname(fileURLToPath(import.meta.url));
const prevDir = () => dirname(dir);
while (dir !== prevDir()) {
const pkgPath = join(dir, "package.json");
if (existsSync(pkgPath)) {
try {
const pkg = JSON.parse(readFileSync(pkgPath, "utf-8"));
if (pkg.name === "neoboard") return dir;
} catch {
/* skip */
}
}
dir = dirname(dir);
}
throw new Error(
"Could not find NeoBoard project root (package.json with name 'neoboard')",
);
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cli/src/lib/config.ts` around lines 23 - 40, The loop termination in
findProjectRoot uses while (dir !== "/"), which fails on Windows roots like C:\;
change the termination test to detect when we've reached the filesystem root
(e.g., use while (dir !== dirname(dir)) or compare against path.parse(dir).root)
so the loop stops correctly across platforms; update the loop condition and keep
existing logic that walks up via dirname(dir) and throws the same error if not
found.

Comment thread cli/src/lib/docker.ts
Comment on lines +60 to +78
export function dockerExec(container: string, cmd: string): string {
return run(`docker exec ${container} ${cmd}`);
}

export function isPgReady(): boolean {
const config = readProjectConfig();
return (
runOrNull(
`docker exec neoboard-postgres pg_isready -U ${config.postgres.user}`,
) !== null
);
}

export function isNeo4jReady(): boolean {
const config = readProjectConfig();
return (
runOrNull(
`docker exec neoboard-neo4j cypher-shell -u ${config.neo4j.user} -p ${config.neo4j.password} "RETURN 1"`,
) !== null

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Escape interpolated shell arguments in docker exec calls.

Lines 60-78 interpolate values from config directly into shell commands. Special characters in usernames/passwords can break readiness checks and allow shell-token injection via modified config.

💡 Proposed hardening
+function shellQuote(v: string): string {
+  return `'${v.replace(/'/g, `'\\''`)}'`;
+}
+
 export function dockerExec(container: string, cmd: string): string {
-  return run(`docker exec ${container} ${cmd}`);
+  return run(`docker exec ${shellQuote(container)} ${cmd}`);
 }
@@
     runOrNull(
-      `docker exec neoboard-postgres pg_isready -U ${config.postgres.user}`,
+      `docker exec neoboard-postgres pg_isready -U ${shellQuote(config.postgres.user)}`,
     ) !== null
@@
     runOrNull(
-      `docker exec neoboard-neo4j cypher-shell -u ${config.neo4j.user} -p ${config.neo4j.password} "RETURN 1"`,
+      `docker exec neoboard-neo4j cypher-shell -u ${shellQuote(config.neo4j.user)} -p ${shellQuote(config.neo4j.password)} "RETURN 1"`,
     ) !== null
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cli/src/lib/docker.ts` around lines 60 - 78, The docker-related functions
(dockerExec, isPgReady, isNeo4jReady) interpolate config values directly into
shell commands which allows special characters or injection; update them to
safely pass arguments (container, usernames, passwords, commands) without shell
interpolation — either by using a command API that accepts an args array (e.g.,
spawn/execFile) or by applying a robust shell-escaping utility to each
interpolated value (escape container, config.postgres.user, config.neo4j.user,
config.neo4j.password and any command strings) before constructing the docker
exec call; ensure dockerExec and the readiness checks use the new safe-call
pattern so usernames/passwords cannot break the shell invocation.

Comment thread cli/src/lib/health.ts
Comment on lines +16 to +20
while (Date.now() < deadline) {
if (check()) {
spinner.succeed(`${label} is ready`);
return;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Consider wrapping check() in try-catch.

If the health check function throws (e.g., execSync failure in isPgReady/isNeo4jReady), the exception propagates and leaves the spinner in an indeterminate state.

🛡️ Proposed fix
   while (Date.now() < deadline) {
-    if (check()) {
-      spinner.succeed(`${label} is ready`);
-      return;
+    try {
+      if (check()) {
+        spinner.succeed(`${label} is ready`);
+        return;
+      }
+    } catch {
+      // Check failed, continue polling
     }
     await new Promise((r) => setTimeout(r, interval));
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
while (Date.now() < deadline) {
if (check()) {
spinner.succeed(`${label} is ready`);
return;
}
while (Date.now() < deadline) {
try {
if (check()) {
spinner.succeed(`${label} is ready`);
return;
}
} catch {
// Check failed, continue polling
}
await new Promise((r) => setTimeout(r, interval));
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cli/src/lib/health.ts` around lines 16 - 20, Wrap the call to check() inside
the health-wait loop in a try-catch so any thrown errors (e.g., from
isPgReady/isNeo4jReady) are caught; on catch call spinner.fail(`${label} failed:
${err.message}`) (or a similar descriptive failure message) to ensure the
spinner is not left indeterminate, then rethrow the error (or return) so the
caller still sees the failure. Locate the loop that calls check(), spinner, and
label in the health wait logic and add this try-catch around check().

Comment thread cli/src/lib/output.ts
Comment on lines +24 to +34
export function banner(lines: string[]): void {
const maxLen = Math.max(...lines.map((l) => l.length));
const top = "\u2554" + "\u2550".repeat(maxLen + 2) + "\u2557";
const bottom = "\u255A" + "\u2550".repeat(maxLen + 2) + "\u255D";

console.log(top);
for (const line of lines) {
console.log("\u2551 " + line.padEnd(maxLen) + " \u2551");
}
console.log(bottom);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Handle empty lines array in banner().

Math.max(...[]) returns -Infinity, which would produce malformed output if called with an empty array.

🛡️ Proposed fix
 export function banner(lines: string[]): void {
+  if (lines.length === 0) return;
   const maxLen = Math.max(...lines.map((l) => l.length));
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export function banner(lines: string[]): void {
const maxLen = Math.max(...lines.map((l) => l.length));
const top = "\u2554" + "\u2550".repeat(maxLen + 2) + "\u2557";
const bottom = "\u255A" + "\u2550".repeat(maxLen + 2) + "\u255D";
console.log(top);
for (const line of lines) {
console.log("\u2551 " + line.padEnd(maxLen) + " \u2551");
}
console.log(bottom);
}
export function banner(lines: string[]): void {
if (lines.length === 0) return;
const maxLen = Math.max(...lines.map((l) => l.length));
const top = "\u2554" + "\u2550".repeat(maxLen + 2) + "\u2557";
const bottom = "\u255A" + "\u2550".repeat(maxLen + 2) + "\u255D";
console.log(top);
for (const line of lines) {
console.log("\u2551 " + line.padEnd(maxLen) + " \u2551");
}
console.log(bottom);
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cli/src/lib/output.ts` around lines 24 - 34, banner currently calls
Math.max(...lines.map(...)) which throws -Infinity for an empty array; update
the banner function (the maxLen calculation used by top/bottom and the loop that
pads lines) to handle an empty lines array by setting maxLen to 0 when
lines.length === 0 (e.g., use a conditional or default value) so the top/bottom
border construction ("\u2554"/"\u2557"/"\u255A"/"\u255D" with
"\u2550".repeat(maxLen + 2)) and the for loop behave correctly for empty input.

@alfredo1996

Copy link
Copy Markdown
Owner Author

Merged into aggregated PR #349 (release/1.0-all-fixes)

@alfredo1996 alfredo1996 closed this Apr 3, 2026
@alfredo1996
alfredo1996 deleted the feat/cli-scaffold branch April 7, 2026 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment