Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
7 changes: 6 additions & 1 deletion framework.manifest.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

28 changes: 27 additions & 1 deletion packages/cli/src/cmd-dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import type { CliCommand, CommandContext } from './command';
import { assetRoutes } from './dev-assets';
import type { DevDashboardInput, DevStatus } from './dev-dashboard';
import { devDashboardRoutes, devPanels } from './dev-dashboard';
import { clearLock, preflight, writeLock } from './dev-lock';
import { createStatementLedger } from './dev-n-plus-one';
import { appRoutes } from './dev-render';
import type { RunningRoles } from './dev-roles';
Expand Down Expand Up @@ -295,6 +296,13 @@ export const devCommand: CliCommand = {
DEFAULT_PORT,
);
const roles = selectRoles(flagString(ctx.args, 'role'));
// BEFORE anything boots. Both failures this catches were reachable and both reported the wrong
// thing: a taken port surfaced as X_CLI_UNEXPECTED wrapping "Is port 3000 in use?" with a `fix:`
// naming `x doctor`, and a second `x dev` on one checkout died later on X_DB_UNAVAILABLE whose
// `fix:` named `x dev`. Neither is discoverable from the message; both are trivial once the
// preflight has the state directory and the port in front of it.
const stateDir = resolveServices(root, ctx.env).stateDir;
const { clearedStale } = await preflight({ stateDir, port });
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
const server = await startDev({
root,
port,
Expand Down Expand Up @@ -344,20 +352,38 @@ export const devCommand: CliCommand = {
panels: [...server.panels],
},
lines: [
// A hard kill leaves the lock behind; clearing it is normal and worth one line, never a
// finding. First, because it happened before anything else this run reports.
...(clearedStale ? [msg('cli.dev.staleLock')] : []),
msg('cli.dev.roles', { roles: server.roles.join(', ') }),
msg('cli.dev.panels', { panels: server.panels.join(', ') }),
msg('cli.dev.manifest', { path: join(root, MANIFEST_FILENAME) }),
msg('cli.dev.introspect', { url: `${server.url}/_x` }),
],
};
await writeLock(stateDir, {
pid: process.pid,
port,
url: server.url,
startedAt: new Date().toISOString(),
});
if (ctx.args.flags.get('once') === true) {
clearLock(stateDir);
await server.stop();
return result;
}
// Long-running: `dispatch` awaits this instead of exiting, so the watcher keeps reloading and
// `/_x` stays reachable. Ctrl-C drains the web role through core's phases first and releases
// the embedded Postgres, the worker and the watcher after — a hard kill leaves the PGlite
// directory locked by a process that no longer exists.
return { ...result, hold: holdUntilShutdown('dev', () => server.stop()) };
return {
...result,
hold: holdUntilShutdown('dev', async () => {
// The lock first: a stop() that throws must not leave a file claiming this pid still owns
// the directory, because the next boot would then refuse for a process that is gone.
clearLock(stateDir);
await server.stop();
}),
};
},
};
199 changes: 199 additions & 0 deletions packages/cli/src/dev-lock.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,199 @@
// The preflight's whole job is to turn two confusing late failures into two precise early ones, so
// what matters here is that each refusal names the right cause and offers a remedy that RUNS.

import { describe, expect, test } from 'bun:test';
import { mkdtempSync, rmSync, writeFileSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import {
clearLock,
DEV_LOCK_FILE,
DevAlreadyRunningError,
DevPortInUseError,
isProcessAlive,
lockPath,
parseLock,
preflight,
suggestPort,
writeLock,
} from './dev-lock';

const scratch = (): string => mkdtempSync(join(tmpdir(), 'ultimate-dev-lock-'));

const LOCK = {
pid: 4242,
port: 3000,
url: 'http://localhost:3000',
startedAt: '2026-08-20T00:00:00.000Z',
};

describe('parseLock', () => {
test('reads a lock this module wrote', () => {
expect(parseLock(JSON.stringify(LOCK))).toEqual(LOCK);
});

test('a truncated or hand-edited file is a stale lock, never a crash', () => {
// This runs on the path whose entire job is to make a confusing failure clear; throwing here
// would replace one bad message with a worse one.
for (const raw of ['', '{', 'null', '[]', '{"pid":"nope"}', '{"port":3000}']) {
expect(parseLock(raw)).toBe(null);
}
});

test('a lock missing its url still resolves to one, from the port', () => {
expect(parseLock('{"pid":1,"port":4311}')?.url).toBe('http://localhost:4311');
});
});

describe('isProcessAlive', () => {
test('this process is alive', () => {
expect(isProcessAlive(process.pid)).toBe(true);
});

test('a pid that cannot exist is not', () => {
expect(isProcessAlive(0)).toBe(false);
expect(isProcessAlive(-1)).toBe(false);
expect(isProcessAlive(2 ** 31)).toBe(false);
});
});

describe('suggestPort', () => {
test('the next port up, except at the top of the range', () => {
expect(suggestPort(3000)).toBe(3001);
// 65536 is not a port, and a `fix:` that cannot run is the failure this module exists to end.
expect(suggestPort(65535)).toBe(65534);
});
});

describe('preflight', () => {
test('a clean directory and a free port pass, clearing nothing', async () => {
const dir = scratch();
try {
expect(await preflight({ stateDir: dir, port: 3000, portBound: () => false })).toEqual({
clearedStale: false,
});
} finally {
rmSync(dir, { recursive: true, force: true });
}
});

test('a live lock is refused, and the refusal names the pid holding the directory', async () => {
const dir = scratch();
try {
await writeLock(dir, LOCK);
const thrown = await preflight({
stateDir: dir,
port: 3000,
portBound: () => false,
alive: () => true,
}).catch((error: unknown) => error);
expect(thrown).toBeInstanceOf(DevAlreadyRunningError);
const error = thrown as DevAlreadyRunningError;
expect(error.code).toBe('X_DEV_ALREADY_RUNNING');
expect(error.cause).toContain('4242');
expect(error.cause).toContain('single-writer');
// The remedy is the running server or stopping it — never `x dev`, which is what the
// framework's own X_DB_UNAVAILABLE used to say when this exact thing happened.
expect(error.fix).toContain('kill 4242');
expect(error.fix).not.toMatch(/^x dev/);
} finally {
rmSync(dir, { recursive: true, force: true });
}
});

test('a stale lock is cleared and the boot continues — a hard kill must not block the next one', async () => {
const dir = scratch();
try {
await writeLock(dir, LOCK);
const result = await preflight({
stateDir: dir,
port: 3000,
portBound: () => false,
alive: () => false,
});
expect(result.clearedStale).toBe(true);
expect(await Bun.file(lockPath(dir)).exists()).toBe(false);
} finally {
rmSync(dir, { recursive: true, force: true });
}
});

test('an unparseable lock is treated as stale, not as a live owner', async () => {
const dir = scratch();
try {
writeFileSync(join(dir, DEV_LOCK_FILE), 'not json');
const result = await preflight({ stateDir: dir, port: 3000, portBound: () => false });
expect(result.clearedStale).toBe(true);
} finally {
rmSync(dir, { recursive: true, force: true });
}
});

test('a taken port is refused with a runnable --port, and names the holder when the OS says', async () => {
const dir = scratch();
try {
const thrown = await preflight({
stateDir: dir,
port: 3000,
portBound: () => true,
holder: () => ({ pid: 99, command: 'docker-pr' }),
}).catch((error: unknown) => error);
expect(thrown).toBeInstanceOf(DevPortInUseError);
const error = thrown as DevPortInUseError;
expect(error.code).toBe('X_PORT_IN_USE');
expect(error.cause).toContain('docker-pr');
expect(error.cause).toContain('99');
expect(error.fix).toContain('x dev --port 3001');
} finally {
rmSync(dir, { recursive: true, force: true });
}
});

test('an unidentifiable holder still gets a refusal with a remedy — root-owned ports are common', async () => {
const dir = scratch();
try {
const thrown = (await preflight({
stateDir: dir,
port: 3000,
portBound: () => true,
holder: () => ({}),
}).catch((error: unknown) => error)) as DevPortInUseError;
expect(thrown.cause).toContain('another process');
expect(thrown.fix).toBe('x dev --port 3001');
} finally {
rmSync(dir, { recursive: true, force: true });
}
});

test('the lock check runs BEFORE the port check — a second x dev is not a port problem', async () => {
// Moving a second `x dev` to another port would still fail on the single-writer database, so
// reporting the port first would send the reader down the wrong path entirely.
const dir = scratch();
try {
await writeLock(dir, LOCK);
const thrown = await preflight({
stateDir: dir,
port: 3000,
portBound: () => true,
alive: () => true,
}).catch((error: unknown) => error);
expect(thrown).toBeInstanceOf(DevAlreadyRunningError);
} finally {
rmSync(dir, { recursive: true, force: true });
}
});
});

describe('clearLock', () => {
test('removes the file, and is safe to call again — shutdown paths overlap', async () => {
const dir = scratch();
try {
await writeLock(dir, LOCK);
clearLock(dir);
expect(await Bun.file(lockPath(dir)).exists()).toBe(false);
expect(() => clearLock(dir)).not.toThrow();
} finally {
rmSync(dir, { recursive: true, force: true });
}
});
});
Loading