Skip to content

fix(cli): setup scripts use docker mode + increase health timeout #397

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
alfredo1996 wants to merge 6 commits into from
Closed

fix(cli): setup scripts use docker mode + increase health timeout #397

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3010a1b
fix(cli): setup scripts use docker mode, increase health timeout
alfredorubin96 Apr 6, 2026
af31a5e
fix(cli): reliable setup — resolve DATABASE_URL, DBs-only compose, fa…
alfredorubin96 Apr 6, 2026
13eecfa
fix(cli): use execSync for dockerExec to preserve quoted Cypher queries
alfredorubin96 Apr 6, 2026
ab6ea2d
fix(cli): demo runs full Docker stack, setup runs DBs only
alfredorubin96 Apr 6, 2026
2eebf39
fix(cli): seed with Docker-internal hostnames for full-stack mode
alfredorubin96 Apr 6, 2026
7addc1d
fix(cli): harden DATABASE_URL parsing — strip quotes, URI-encode cred…
alfredorubin96 Apr 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 69 additions & 21 deletions cli/src/__tests__/commands/db/migrate.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@ vi.mock("../../../lib/exec.js", () => ({
run: vi.fn(),
}));

vi.mock("../../../lib/docker.js", () => ({
dockerExec: vi.fn(),
}));

vi.mock("../../../lib/config.js", () => ({
paths: {
journalPath: "/project/app/drizzle/migrations/meta/_journal.json",
appDir: "/project/app",
envFile: "/project/app/.env.local",
},
getMode: vi.fn(() => "local"),
readProjectConfig: vi.fn(() => ({
ports: { postgres: 5432 },
postgres: { user: "neoboard", password: "neoboard", database: "neoboard" },
})),
}));

vi.mock("../../../lib/output.js", () => ({
Expand All @@ -33,8 +33,6 @@ vi.mock("node:fs", () => ({
}));

import { run } from "../../../lib/exec.js";
import { dockerExec } from "../../../lib/docker.js";
import { getMode } from "../../../lib/config.js";
import { info, warn } from "../../../lib/output.js";
import { existsSync, readFileSync } from "node:fs";
import {
Expand All @@ -44,8 +42,6 @@ import {
} from "../../../commands/db/migrate.js";

const mockRun = vi.mocked(run);
const mockDockerExec = vi.mocked(dockerExec);
const mockGetMode = vi.mocked(getMode);
const mockExistsSync = vi.mocked(existsSync);
const mockReadFileSync = vi.mocked(readFileSync);

Expand All @@ -59,12 +55,15 @@ const SAMPLE_JOURNAL = JSON.stringify({

beforeEach(() => {
vi.clearAllMocks();
mockGetMode.mockReturnValue("local");
// Default: .env.local exists with a DATABASE_URL
mockExistsSync.mockReturnValue(true);
mockReadFileSync.mockReturnValue(
"DATABASE_URL=postgresql://neoboard:neoboard@localhost:5432/neoboard\n",
);
});

describe("showMigrationStatus", () => {
it("displays migration entries", () => {
mockExistsSync.mockReturnValue(true);
mockReadFileSync.mockReturnValue(SAMPLE_JOURNAL);
showMigrationStatus();
expect(info).toHaveBeenCalledWith("Migrations: 2 available");
Expand All @@ -79,7 +78,6 @@ describe("showMigrationStatus", () => {

describe("showDryRun", () => {
it("shows pending migrations without applying", () => {
mockExistsSync.mockReturnValue(true);
mockReadFileSync.mockReturnValue(SAMPLE_JOURNAL);
showDryRun();
expect(info).toHaveBeenCalledWith("Would apply 2 migration(s):");
Expand All @@ -89,34 +87,37 @@ describe("showDryRun", () => {

describe("runDbMigrate", () => {
it("shows status when --status flag set", async () => {
mockExistsSync.mockReturnValue(true);
mockReadFileSync.mockReturnValue(SAMPLE_JOURNAL);
await runDbMigrate({ status: true });
expect(info).toHaveBeenCalledWith("Migrations: 2 available");
expect(mockRun).not.toHaveBeenCalled();
});

it("shows dry run when --dry-run flag set", async () => {
mockExistsSync.mockReturnValue(true);
mockReadFileSync.mockReturnValue(SAMPLE_JOURNAL);
await runDbMigrate({ dryRun: true });
expect(mockRun).not.toHaveBeenCalled();
});

it("runs migrations in local mode", async () => {
it("runs migrations locally with DATABASE_URL from .env.local", async () => {
await runDbMigrate({});
expect(mockRun).toHaveBeenCalledWith("npx drizzle-kit migrate", {
cwd: "/project/app",
env: expect.objectContaining({
DATABASE_URL: "postgresql://neoboard:neoboard@localhost:5432/neoboard",
}),
});
});

it("runs migrations via docker exec in docker mode", async () => {
mockGetMode.mockReturnValue("docker");
it("falls back to config-derived DATABASE_URL when .env.local missing", async () => {
mockExistsSync.mockReturnValue(false);
await runDbMigrate({});
expect(mockDockerExec).toHaveBeenCalledWith(
"neoboard-app",
"npx drizzle-kit migrate",
);
expect(mockRun).toHaveBeenCalledWith("npx drizzle-kit migrate", {
cwd: "/project/app",
env: expect.objectContaining({
DATABASE_URL: "postgresql://neoboard:neoboard@localhost:5432/neoboard",
}),
});
});

it("prints backup warning", async () => {
Expand All @@ -126,6 +127,53 @@ describe("runDbMigrate", () => {
);
});

it("strips double quotes from DATABASE_URL in .env.local", async () => {
mockReadFileSync.mockReturnValue(
'DATABASE_URL="postgresql://neoboard:neoboard@localhost:5432/neoboard"\n',
);
await runDbMigrate({});
expect(mockRun).toHaveBeenCalledWith("npx drizzle-kit migrate", {
cwd: "/project/app",
env: expect.objectContaining({
DATABASE_URL: "postgresql://neoboard:neoboard@localhost:5432/neoboard",
}),
});
});

it("strips single quotes from DATABASE_URL in .env.local", async () => {
mockReadFileSync.mockReturnValue(
"DATABASE_URL='postgresql://neoboard:neoboard@localhost:5432/neoboard'\n",
);
await runDbMigrate({});
expect(mockRun).toHaveBeenCalledWith("npx drizzle-kit migrate", {
cwd: "/project/app",
env: expect.objectContaining({
DATABASE_URL: "postgresql://neoboard:neoboard@localhost:5432/neoboard",
}),
});
});

it("URI-encodes special characters in config fallback credentials", async () => {
mockExistsSync.mockReturnValue(false);
const { readProjectConfig } = await import("../../../lib/config.js");
vi.mocked(readProjectConfig).mockReturnValue({
ports: { postgres: 5432 },
postgres: {
user: "neo@board",
password: "p@ss:word",
database: "neo board",
},
} as ReturnType<typeof readProjectConfig>);
await runDbMigrate({});
expect(mockRun).toHaveBeenCalledWith("npx drizzle-kit migrate", {
cwd: "/project/app",
env: expect.objectContaining({
DATABASE_URL:
"postgresql://neo%40board:p%40ss%3Aword@localhost:5432/neo%20board",
}),
});
});

it("warns about --to flag limitation", async () => {
await runDbMigrate({ to: "1.0.0" });
expect(warn).toHaveBeenCalledWith(expect.stringContaining("--to 1.0.0"));
Expand Down
18 changes: 16 additions & 2 deletions cli/src/__tests__/commands/db/seed.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,25 @@ describe("seedNeo4j", () => {
});

describe("seedPostgres", () => {
it("runs seed script", async () => {
it("runs seed script with host env", async () => {
await seedPostgres();
expect(mockRun).toHaveBeenCalledWith(
"node /project/scripts/seed-demo.mjs",
{ cwd: "/project" },
{ cwd: "/project", env: process.env },
);
});

it("passes Docker hostnames when dockerNetwork is true", async () => {
await seedPostgres(true);
expect(mockRun).toHaveBeenCalledWith(
"node /project/scripts/seed-demo.mjs",
{
cwd: "/project",
env: expect.objectContaining({
NEO4J_HOST: "neoboard-neo4j",
PG_HOST: "neoboard-postgres",
}),
},
);
});
});
Expand Down
10 changes: 7 additions & 3 deletions cli/src/__tests__/commands/demo.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,18 @@ describe("runDemo", () => {
expect(mockRunSetup).toHaveBeenCalledBefore(mockRunDbSeed);
});

it("passes mode to setup", async () => {
it("passes mode and full=true to setup", async () => {
await runDemo({ mode: "local" });
expect(mockRunSetup).toHaveBeenCalledWith({ mode: "local" });
expect(mockRunSetup).toHaveBeenCalledWith({ mode: "local", full: true });
});

it("seeds both neo4j and demo data", async () => {
await runDemo();
expect(mockRunDbSeed).toHaveBeenCalledWith({ neo4j: true, demo: true });
expect(mockRunDbSeed).toHaveBeenCalledWith({
neo4j: true,
demo: true,
dockerNetwork: true,
});
});

it("shows login credentials", async () => {
Expand Down
4 changes: 2 additions & 2 deletions cli/src/__tests__/commands/start.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@ describe("runStart", () => {
expect(mockComposeUp).not.toHaveBeenCalled();
});

it("starts containers with full stack in docker mode", async () => {
it("starts DB containers (not full stack) in docker mode", async () => {
await runStart();
expect(mockComposeUp).toHaveBeenCalledWith({ full: true });
expect(mockComposeUp).toHaveBeenCalledWith({ full: false });
});

it("skips composeUp in local mode", async () => {
Expand Down
10 changes: 4 additions & 6 deletions cli/src/__tests__/lib/docker.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,15 +174,13 @@ describe("isPgReady", () => {
});

describe("isNeo4jReady", () => {
it("returns true when cypher-shell succeeds", () => {
mockDockerExec.mockReturnValue("1");
it("returns true when docker inspect reports healthy", () => {
mockRunOrNull.mockReturnValue("healthy");
expect(isNeo4jReady()).toBe(true);
});

it("returns false when cypher-shell fails", () => {
mockDockerExec.mockImplementation(() => {
throw new Error("not ready");
});
it("returns false when docker inspect reports starting", () => {
mockRunOrNull.mockReturnValue("starting");
expect(isNeo4jReady()).toBe(false);
});
});
48 changes: 40 additions & 8 deletions cli/src/commands/db/migrate.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,40 @@
import { existsSync, readFileSync } from "node:fs";
import { run } from "../../lib/exec.js";
import { dockerExec } from "../../lib/docker.js";
import { paths, getMode } from "../../lib/config.js";
import { paths, readProjectConfig } from "../../lib/config.js";
import { info, success, warn, createSpinner } from "../../lib/output.js";

/**
* Resolve the DATABASE_URL for migrations.
* Priority: 1) .env.local 2) built from neoboard.config.json
* This works regardless of where the DB runs (Docker, local, remote).
*/
function resolveDatabaseUrl(): string {

Check failure on line 11 in cli/src/commands/db/migrate.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this function to reduce its Cognitive Complexity from 16 to the 15 allowed. 


See more on https://sonarcloud.io/project/issues?id=alfredo1996_neoboard&issues=AZ1k1RI32PhC6Ateg9mh&open=AZ1k1RI32PhC6Ateg9mh&pullRequest=397
// Check .env.local first — user may have a custom DB host
if (existsSync(paths.envFile)) {
const content = readFileSync(paths.envFile, "utf-8");
for (const line of content.split("\n")) {
const trimmed = line.trim();
const m = trimmed.match(/^DATABASE_URL\s*=\s*(.+)$/);
if (m) {
let url = m[1].trim();
if (
(url.startsWith('"') && url.endsWith('"')) ||
(url.startsWith("'") && url.endsWith("'"))
) {
url = url.slice(1, -1);
}
if (url) return url;
}
}
}
// Fallback: build from config (assumes DB is on localhost via Docker port mapping)
const config = readProjectConfig();
const user = encodeURIComponent(config.postgres.user);
const pass = encodeURIComponent(config.postgres.password);
const db = encodeURIComponent(config.postgres.database);
return `postgresql://${user}:${pass}@localhost:${config.ports.postgres}/${db}`;
}

interface JournalEntry {
idx: number;
tag: string;
Expand Down Expand Up @@ -76,12 +107,13 @@
const spinner = createSpinner("Running migrations...");
spinner.start();

const mode = getMode();
if (mode === "docker") {
dockerExec("neoboard-app", "npx drizzle-kit migrate");
} else {
run("npx drizzle-kit migrate", { cwd: paths.appDir });
}
// Resolve DATABASE_URL: use .env.local if set, otherwise build from config.
// This works regardless of where the DB runs (Docker, local, remote).
const dbUrl = resolveDatabaseUrl();
run("npx drizzle-kit migrate", {
cwd: paths.appDir,
env: { ...process.env, DATABASE_URL: dbUrl },
});
Comment on lines +113 to +116

@coderabbitai coderabbitai Bot Apr 6, 2026

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

🌐 Web query:

For the drizzle-kit version used by this repository, does drizzle-kit migrate acquire a PostgreSQL advisory lock (or equivalent concurrency lock) to prevent concurrent migration runners?

💡 Result:

I can’t tell what drizzle-kit version “this repository” uses unless you share the repo URL or paste the drizzle-kit entry from package.json / lockfile.

That said, Drizzle Kit’s migrate has historically not taken a PostgreSQL advisory lock (e.g. pg_advisory_lock / pg_advisory_xact_lock) to serialize concurrent runners—the Drizzle team explicitly described a future “proper lock mechanism” as something they will implement, implying it wasn’t present at the time of that design note/discussion. [1] The locks people commonly observe during migrate are standard PostgreSQL DDL locks (e.g., from CREATE TABLE IF NOT EXISTS ... __drizzle_migrations and other DDL), not an explicit migration-runner mutex. [2]

If you paste your repo’s drizzle-kit version, I can verify more precisely for that version and (if needed) point you to how to confirm by searching the installed CLI code for pg_advisory_*.

Sources

  1. Drizzle discussion noting “Proper Lock Mechanism” as a planned improvement for simultaneous migrate() executions: “We will implement a proper lock mechanism…” [1]
  2. Report of DDL locking behavior during Drizzle migrations (table creation causing locks), not advisory-lock coordination: [2]

🏁 Script executed:

fd -t f "package.json" | head -5

Repository: alfredo1996/neoboard

Length of output: 162

🏁 Script executed:

cat package.json | grep -A 2 -B 2 "drizzle"

Repository: alfredo1996/neoboard

Length of output: 46

🏁 Script executed:

# Also check for any advisory lock usage in the codebase
rg "pg_advisory|advisory.*lock" -i

Repository: alfredo1996/neoboard

Length of output: 46

🏁 Script executed:

cat cli/package.json | grep -E "(drizzle|migration)" -i

Repository: alfredo1996/neoboard

Length of output: 46

🏁 Script executed:

# Check for lockfiles to get exact version
fd -t f "(pnpm-lock|package-lock|yarn.lock)" | head -1

Repository: alfredo1996/neoboard

Length of output: 85

🏁 Script executed:

# Search for drizzle across all package files
rg "drizzle-kit" --type json

Repository: alfredo1996/neoboard

Length of output: 561

🏁 Script executed:

# Check cli/package.json for drizzle dependencies
cat cli/package.json

Repository: alfredo1996/neoboard

Length of output: 636

🏁 Script executed:

# Check the full context of the migration command at lines 103-106
cat -n cli/src/commands/db/migrate.ts | sed -n '95,115p'

Repository: alfredo1996/neoboard

Length of output: 643

🏁 Script executed:

# Search for any migration-related files that might have custom locking
fd -t f "migrat" cli/src/ | head -10

Repository: alfredo1996/neoboard

Length of output: 140

🏁 Script executed:

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

Repository: alfredo1996/neoboard

Length of output: 1635

Add advisory locking to prevent concurrent migration races.

drizzle-kit migrate (version 0.31.10) does not implement PostgreSQL advisory locking. Running migrations concurrently can cause race conditions. Wrap the migration call with pg_advisory_xact_lock() or implement an equivalent serialization mechanism to comply with NeoBoard migration requirements.

🤖 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 103 - 106, Wrap the existing
run("npx drizzle-kit migrate", ...) call with an advisory lock: create a
Postgres client, connect, call SELECT pg_advisory_lock(<constant_bigint>) to
acquire a session lock, then execute the run(...) command while the lock is
held, and finally release the lock (SELECT pg_advisory_unlock(...)) and close
the client in a try/finally so the lock is always released; reference the
existing run(...) invocation in migrate.ts and use a single fixed bigint key
(e.g., 1234567890) for pg_advisory_lock/unlock to serialize concurrent
migrations.


spinner.succeed("Migrations applied");
success("Database is up to date");
Expand Down
21 changes: 18 additions & 3 deletions cli/src/commands/db/seed.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,19 +57,34 @@ export async function seedNeo4j(): Promise<void> {
spinner.succeed("Neo4j seeded with demo data");
}

export async function seedPostgres(): Promise<void> {
export async function seedPostgres(dockerNetwork = false): Promise<void> {
const config = readProjectConfig();
assertSafePath(config.seed.script, "seed.script");
const spinner = createSpinner("Seeding PostgreSQL demo data...");
spinner.start();

run(`node ${paths.root}/${config.seed.script}`, { cwd: paths.root });
// When the app runs inside Docker, seed with Docker-internal hostnames
// so the stored connection URIs resolve inside the container network.
const env = dockerNetwork
? {
...process.env,
NEO4J_HOST: "neoboard-neo4j",
PG_HOST: "neoboard-postgres",
}
: process.env;

run(`node ${paths.root}/${config.seed.script}`, {
cwd: paths.root,
env,
});
spinner.succeed("PostgreSQL seeded with demo data");
}

export async function runDbSeed(opts?: {
neo4j?: boolean;
demo?: boolean;
/** When true, seed connection URIs use Docker-internal hostnames. */
dockerNetwork?: boolean;
}): Promise<void> {
const seedNeo4jOnly = opts?.neo4j && !opts?.demo;
const seedDemoOnly = opts?.demo && !opts?.neo4j;
Expand All @@ -80,7 +95,7 @@ export async function runDbSeed(opts?: {
}

if (seedBoth || seedDemoOnly) {
await seedPostgres();
await seedPostgres(opts?.dockerNetwork ?? false);
}

success("Seeding complete");
Expand Down
5 changes: 3 additions & 2 deletions cli/src/commands/demo.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ import { success, banner } from "../lib/output.js";
export async function runDemo(opts?: {
mode?: "docker" | "local";
}): Promise<void> {
await runSetup(opts);
await runDbSeed({ neo4j: true, demo: true });
// Demo always starts the full stack (app + DBs in Docker)
await runSetup({ ...opts, full: true });
await runDbSeed({ neo4j: true, demo: true, dockerNetwork: true });

banner([
"Demo environment ready!",
Expand Down
4 changes: 3 additions & 1 deletion cli/src/commands/setup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@ import { success } from "../lib/output.js";

export async function runSetup(opts?: {
mode?: "docker" | "local";
/** Start the full stack (app + DBs) or just DBs? */
full?: boolean;
}): Promise<void> {
await runInit(opts);
await runStart();
await runStart({ full: opts?.full });
success("Setup complete!");
}
Loading
Loading