mirror of
https://github.com/cloudflare/vinext.git
synced 2026-09-14 19:04:59 +08:00
9277b101b2
* feat(dev): add dev server lock file Write the running dev server's PID, port, and URL into `node_modules/.cache/vinext/dev-lock.json`. When a second `vinext dev` process starts in the same project directory, vinext reads the lock file and prints an actionable error: Another vinext dev server is already running. - Local: http://localhost:3000 - PID: 12345 - Dir: /path/to/project You can access the existing server at http://localhost:3000, or run `kill 12345` to stop it and start a new one. This is especially useful for AI coding agents, which frequently attempt to start `vinext dev` without knowing a server is already running. The structured error gives the agent the PID to kill the existing process or the URL to connect to it — no manual intervention required. If the recorded PID is dead (stale lock from a crashed run), the new process takes over the lock automatically. Set `VINEXT_NO_DEV_LOCK=1` to opt out entirely. Behavior modeled on Next.js' dev server lock file: https://github.com/vercel/next.js/blob/canary/packages/next/src/build/lockfile.ts vinext uses a JSON file + PID liveness check (`process.kill(pid, 0)`) rather than a native `flock()`. The race window on acquisition is small and benign: at worst, two dev servers race and one fails to bind its port. * chore(dev-lockfile): unexport types only used internally knip flagged FormatErrorOptions, AcquireOptions, AcquireSuccess, AcquireFailure, and AcquireResult as unused exports. These are only referenced inside dev-lockfile.ts itself, so drop the `export` keyword to keep the public surface minimal. * review(dev-lockfile): address self-review and bot feedback - Capture ownerPid at acquire time and use it in release() instead of reading info.pid. Decouples release from update() so a future caller passing a different PID through update() can't trick release() into deleting another process's lock. Added a regression test. - Document the TOCTOU window between readLockfile() and writeLockfile() inline so future contributors don't try to 'fix' the intentionally accepted race. - Document the unreachable existing: undefined branch in formatAlreadyRunningError as a defensive fallback. - Document the process.on('exit', ...) signal semantics — exit fires on graceful shutdown and after default SIGINT/SIGTERM handlers, but not on hard crashes. The next vinext dev will take over the stale lock. - Set lock file mode to 0o600. Defense-in-depth: the PID is also discoverable via ps, so this isn't load-bearing. - Use Vite's server.resolvedUrls.local[0] in cli.ts when building the post-listen appUrl. Vite substitutes 'localhost' for wildcard binds (0.0.0.0) so the URL is actually clickable. Also substitute 'localhost' in the pre-listen initial info for the same reason. - Add tests: * exit listener is registered/deregistered correctly * update() with a different PID doesn't affect release() ownership * lock file is created with 0o600 on POSIX * review(dev-lockfile): release on listen failure, preserve startedAt Two issues caught in the bot re-review: 1. If `server.listen()` throws (e.g. strictPort and the port is taken), the dev() function previously threw without releasing the lock. The process.on('exit') handler still cleaned up afterwards, but in the brief window between listen failure and process exit, the lock file claimed a server was running at a port nobody was listening on. A concurrent `vinext dev` in that window would have shown a misleading 'already running' error. Wrap createServer + listen in try/catch and call lockfile.release() on failure. The exit listener is still registered as a safety net for unexpected exit paths. 2. update() after server.listen() was setting `startedAt: Date.now()`, which overwrote the original acquisition timestamp. `startedAt` is meant to reflect when the process started (for 'how long has this server been running?' debugging), not when the URL was resolved. Capture `startedAt` once at acquisition time and pass the same value into update(). Added a regression test. * feat(dev-lockfile): move lock file to .vinext/dev/lock.json Previously the lock file lived at node_modules/.cache/vinext/dev-lock.json. .vinext/ is already the established convention for project-local vinext state — the fonts plugin uses .vinext/fonts/ to cache self-hosted Google Fonts, and the monorepo's own .gitignore already excludes .vinext. Benefits: - Doesn't pollute node_modules (which package managers may aggressively clear during install / pnpm prune). - Co-located with other vinext state, so users find it where they expect. - Mirrors Next.js' .next/dev/lock layout more closely. Follow-up: vinext init does not currently add .vinext to user gitignores. That's a separate hygiene improvement also relevant to the fonts cache.
359 lines
12 KiB
TypeScript
359 lines
12 KiB
TypeScript
/**
|
|
* Tests for the dev server lock file.
|
|
*
|
|
* Behavior modeled on Next.js' dev lock file:
|
|
* https://github.com/vercel/next.js/blob/canary/packages/next/src/build/lockfile.ts
|
|
*/
|
|
|
|
import fs from "node:fs";
|
|
import os from "node:os";
|
|
import path from "node:path";
|
|
import { afterEach, beforeEach, describe, expect, it } from "vite-plus/test";
|
|
|
|
import {
|
|
type DevServerInfo,
|
|
formatAlreadyRunningError,
|
|
getLockfilePath,
|
|
isPidAlive,
|
|
readLockfile,
|
|
tryAcquireLockfile,
|
|
} from "../packages/vinext/src/server/dev-lockfile.js";
|
|
|
|
function makeTempRoot(): string {
|
|
return fs.mkdtempSync(path.join(os.tmpdir(), "vinext-lockfile-"));
|
|
}
|
|
|
|
function cleanup(root: string): void {
|
|
try {
|
|
fs.rmSync(root, { recursive: true, force: true });
|
|
} catch {
|
|
// best-effort
|
|
}
|
|
}
|
|
|
|
function baseInfo(overrides: Partial<DevServerInfo> = {}): DevServerInfo {
|
|
return {
|
|
pid: process.pid,
|
|
port: 3000,
|
|
hostname: "localhost",
|
|
appUrl: "http://localhost:3000",
|
|
startedAt: Date.now(),
|
|
cwd: "/tmp/example",
|
|
...overrides,
|
|
};
|
|
}
|
|
|
|
describe("getLockfilePath", () => {
|
|
it("places the lock file under .vinext/dev/", () => {
|
|
const root = "/projects/my-app";
|
|
expect(getLockfilePath(root)).toBe(
|
|
path.join("/projects/my-app", ".vinext", "dev", "lock.json"),
|
|
);
|
|
});
|
|
});
|
|
|
|
describe("isPidAlive", () => {
|
|
it("returns true for the current process", () => {
|
|
expect(isPidAlive(process.pid)).toBe(true);
|
|
});
|
|
|
|
it("returns false for an obviously dead pid", () => {
|
|
// PID 0 / negative / non-integer are never valid.
|
|
expect(isPidAlive(0)).toBe(false);
|
|
expect(isPidAlive(-1)).toBe(false);
|
|
expect(isPidAlive(Number.NaN)).toBe(false);
|
|
});
|
|
|
|
it("returns false for a very large unused pid", () => {
|
|
// PIDs above the typical kernel max are extremely unlikely to exist.
|
|
expect(isPidAlive(2_147_000_000)).toBe(false);
|
|
});
|
|
});
|
|
|
|
describe("readLockfile", () => {
|
|
let root: string;
|
|
|
|
beforeEach(() => {
|
|
root = makeTempRoot();
|
|
});
|
|
|
|
afterEach(() => {
|
|
cleanup(root);
|
|
});
|
|
|
|
it("returns undefined when the file doesn't exist", () => {
|
|
expect(readLockfile(getLockfilePath(root))).toBeUndefined();
|
|
});
|
|
|
|
it("returns undefined for invalid JSON", () => {
|
|
const lockPath = getLockfilePath(root);
|
|
fs.mkdirSync(path.dirname(lockPath), { recursive: true });
|
|
fs.writeFileSync(lockPath, "{ not json");
|
|
expect(readLockfile(lockPath)).toBeUndefined();
|
|
});
|
|
|
|
it("returns undefined for JSON missing required fields", () => {
|
|
const lockPath = getLockfilePath(root);
|
|
fs.mkdirSync(path.dirname(lockPath), { recursive: true });
|
|
fs.writeFileSync(lockPath, JSON.stringify({ pid: 1 }));
|
|
expect(readLockfile(lockPath)).toBeUndefined();
|
|
});
|
|
|
|
it("round-trips valid server info", () => {
|
|
const info = baseInfo({ pid: 42, port: 4321 });
|
|
const lockPath = getLockfilePath(root);
|
|
fs.mkdirSync(path.dirname(lockPath), { recursive: true });
|
|
fs.writeFileSync(lockPath, JSON.stringify(info));
|
|
expect(readLockfile(lockPath)).toEqual(info);
|
|
});
|
|
});
|
|
|
|
describe("tryAcquireLockfile", () => {
|
|
let root: string;
|
|
|
|
beforeEach(() => {
|
|
root = makeTempRoot();
|
|
});
|
|
|
|
afterEach(() => {
|
|
cleanup(root);
|
|
});
|
|
|
|
it("writes the lock file when none exists", () => {
|
|
const info = baseInfo({ cwd: root });
|
|
const result = tryAcquireLockfile({ root, info, unlockOnExit: false });
|
|
expect(result.ok).toBe(true);
|
|
if (!result.ok) return;
|
|
const stored = readLockfile(getLockfilePath(root));
|
|
expect(stored).toEqual(info);
|
|
result.lockfile.release();
|
|
expect(fs.existsSync(getLockfilePath(root))).toBe(false);
|
|
});
|
|
|
|
it("fails when an existing lock file references a live PID", () => {
|
|
// Write a lock file pointing at the current (live) process.
|
|
const existing = baseInfo({ pid: process.pid, cwd: root });
|
|
const lockPath = getLockfilePath(root);
|
|
fs.mkdirSync(path.dirname(lockPath), { recursive: true });
|
|
fs.writeFileSync(lockPath, JSON.stringify(existing));
|
|
|
|
const result = tryAcquireLockfile({
|
|
root,
|
|
info: baseInfo({ pid: process.pid + 1, cwd: root }),
|
|
unlockOnExit: false,
|
|
});
|
|
expect(result.ok).toBe(false);
|
|
if (result.ok) return;
|
|
expect(result.existing).toEqual(existing);
|
|
expect(result.lockfilePath).toBe(lockPath);
|
|
});
|
|
|
|
it("takes over the lock when the existing entry is a dead PID", () => {
|
|
// Use a definitely-dead pid (very large).
|
|
const stale = baseInfo({ pid: 2_147_000_000, cwd: root });
|
|
const lockPath = getLockfilePath(root);
|
|
fs.mkdirSync(path.dirname(lockPath), { recursive: true });
|
|
fs.writeFileSync(lockPath, JSON.stringify(stale));
|
|
|
|
const info = baseInfo({ pid: process.pid, port: 4000, cwd: root });
|
|
const result = tryAcquireLockfile({ root, info, unlockOnExit: false });
|
|
expect(result.ok).toBe(true);
|
|
if (!result.ok) return;
|
|
expect(readLockfile(lockPath)).toEqual(info);
|
|
result.lockfile.release();
|
|
});
|
|
|
|
it("does not take over stale entries when takeOverStale is false", () => {
|
|
const stale = baseInfo({ pid: 2_147_000_000, cwd: root });
|
|
const lockPath = getLockfilePath(root);
|
|
fs.mkdirSync(path.dirname(lockPath), { recursive: true });
|
|
fs.writeFileSync(lockPath, JSON.stringify(stale));
|
|
|
|
const result = tryAcquireLockfile({
|
|
root,
|
|
info: baseInfo({ cwd: root }),
|
|
takeOverStale: false,
|
|
unlockOnExit: false,
|
|
});
|
|
expect(result.ok).toBe(false);
|
|
});
|
|
|
|
it("update() rewrites the lock file contents", () => {
|
|
const result = tryAcquireLockfile({
|
|
root,
|
|
info: baseInfo({ cwd: root, port: 3000 }),
|
|
unlockOnExit: false,
|
|
});
|
|
expect(result.ok).toBe(true);
|
|
if (!result.ok) return;
|
|
const updated = baseInfo({ cwd: root, port: 5173, appUrl: "http://localhost:5173" });
|
|
result.lockfile.update(updated);
|
|
expect(readLockfile(getLockfilePath(root))).toEqual(updated);
|
|
result.lockfile.release();
|
|
});
|
|
|
|
it("update() preserves startedAt when callers pass the original value", () => {
|
|
// Documents the CLI contract: `startedAt` is meant to reflect when the
|
|
// process started, not when the URL was resolved. The dev() command in
|
|
// cli.ts captures startedAt at acquire time and passes the same value
|
|
// into update() so the lock file's startedAt stays stable across the
|
|
// pre-listen → post-listen rewrite.
|
|
const startedAt = Date.now() - 60_000; // pretend the process started a minute ago
|
|
const result = tryAcquireLockfile({
|
|
root,
|
|
info: baseInfo({ cwd: root, port: 3000, startedAt }),
|
|
unlockOnExit: false,
|
|
});
|
|
expect(result.ok).toBe(true);
|
|
if (!result.ok) return;
|
|
result.lockfile.update(
|
|
baseInfo({
|
|
cwd: root,
|
|
port: 5173,
|
|
appUrl: "http://localhost:5173",
|
|
startedAt, // caller's responsibility to thread this through
|
|
}),
|
|
);
|
|
expect(readLockfile(getLockfilePath(root))?.startedAt).toBe(startedAt);
|
|
result.lockfile.release();
|
|
});
|
|
|
|
it("release() is idempotent", () => {
|
|
const result = tryAcquireLockfile({
|
|
root,
|
|
info: baseInfo({ cwd: root }),
|
|
unlockOnExit: false,
|
|
});
|
|
expect(result.ok).toBe(true);
|
|
if (!result.ok) return;
|
|
result.lockfile.release();
|
|
result.lockfile.release();
|
|
expect(fs.existsSync(getLockfilePath(root))).toBe(false);
|
|
});
|
|
|
|
it("release() does not delete a lock file owned by a different PID", () => {
|
|
const result = tryAcquireLockfile({
|
|
root,
|
|
info: baseInfo({ cwd: root, pid: process.pid }),
|
|
unlockOnExit: false,
|
|
});
|
|
expect(result.ok).toBe(true);
|
|
if (!result.ok) return;
|
|
|
|
// Simulate another process taking over.
|
|
const lockPath = getLockfilePath(root);
|
|
fs.writeFileSync(lockPath, JSON.stringify(baseInfo({ cwd: root, pid: process.pid + 12345 })));
|
|
|
|
result.lockfile.release();
|
|
expect(fs.existsSync(lockPath)).toBe(true);
|
|
});
|
|
|
|
it("release() ignores stale ownerPid even after update() rewrites with different info", () => {
|
|
// Acquire with current PID, then update() with a different PID in the
|
|
// payload. release() must still check against the *original* ownerPid
|
|
// (the acquire-time PID), not the update-time PID. Without that guarantee
|
|
// a malicious or buggy caller could trick release() into deleting another
|
|
// process's lock by passing a fake PID through update().
|
|
const lockPath = getLockfilePath(root);
|
|
const result = tryAcquireLockfile({
|
|
root,
|
|
info: baseInfo({ cwd: root, pid: process.pid }),
|
|
unlockOnExit: false,
|
|
});
|
|
expect(result.ok).toBe(true);
|
|
if (!result.ok) return;
|
|
|
|
// Pretend update() was called with a different PID (simulating a future
|
|
// bug or hostile caller). The lock file now claims to be owned by some
|
|
// other PID.
|
|
const otherPid = process.pid + 99999;
|
|
result.lockfile.update(baseInfo({ cwd: root, pid: otherPid }));
|
|
expect(readLockfile(lockPath)?.pid).toBe(otherPid);
|
|
|
|
// release() must NOT delete the file, because ownerPid (captured at
|
|
// acquire) doesn't match what's on disk.
|
|
result.lockfile.release();
|
|
expect(fs.existsSync(lockPath)).toBe(true);
|
|
});
|
|
|
|
it("registers an exit listener when unlockOnExit is true and removes it on release()", () => {
|
|
const before = process.listenerCount("exit");
|
|
const result = tryAcquireLockfile({
|
|
root,
|
|
info: baseInfo({ cwd: root }),
|
|
unlockOnExit: true,
|
|
});
|
|
expect(result.ok).toBe(true);
|
|
if (!result.ok) return;
|
|
expect(process.listenerCount("exit")).toBe(before + 1);
|
|
result.lockfile.release();
|
|
expect(process.listenerCount("exit")).toBe(before);
|
|
});
|
|
|
|
it("registers no exit listener when unlockOnExit is false", () => {
|
|
const before = process.listenerCount("exit");
|
|
const result = tryAcquireLockfile({
|
|
root,
|
|
info: baseInfo({ cwd: root }),
|
|
unlockOnExit: false,
|
|
});
|
|
expect(result.ok).toBe(true);
|
|
if (!result.ok) return;
|
|
expect(process.listenerCount("exit")).toBe(before);
|
|
result.lockfile.release();
|
|
});
|
|
|
|
it("write sets restrictive file permissions on POSIX", () => {
|
|
// mode bits aren't meaningfully enforced on Windows, skip there.
|
|
if (process.platform === "win32") return;
|
|
const result = tryAcquireLockfile({
|
|
root,
|
|
info: baseInfo({ cwd: root }),
|
|
unlockOnExit: false,
|
|
});
|
|
expect(result.ok).toBe(true);
|
|
if (!result.ok) return;
|
|
const stat = fs.statSync(getLockfilePath(root));
|
|
// Only the user permission bits should be set; group/other should be 0.
|
|
expect(stat.mode & 0o077).toBe(0);
|
|
result.lockfile.release();
|
|
});
|
|
});
|
|
|
|
describe("formatAlreadyRunningError", () => {
|
|
it("includes PID, URL, and dir when existing info is available", () => {
|
|
const existing = baseInfo({
|
|
pid: 12345,
|
|
port: 3000,
|
|
appUrl: "http://localhost:3000",
|
|
cwd: "/path/to/project",
|
|
});
|
|
const msg = formatAlreadyRunningError({
|
|
existing,
|
|
cwd: "/path/to/project",
|
|
lockfilePath: "/path/to/project/.vinext/dev/lock.json",
|
|
});
|
|
expect(msg).toContain("Another vinext dev server is already running");
|
|
expect(msg).toContain("- Local: http://localhost:3000");
|
|
expect(msg).toContain("- PID: 12345");
|
|
expect(msg).toContain("- Dir: /path/to/project");
|
|
|
|
// Platform-aware kill instructions.
|
|
if (process.platform === "win32") {
|
|
expect(msg).toContain("taskkill /PID 12345 /F");
|
|
} else {
|
|
expect(msg).toContain("kill 12345");
|
|
}
|
|
});
|
|
|
|
it("falls back to a generic message when the lock file is corrupt", () => {
|
|
const msg = formatAlreadyRunningError({
|
|
existing: undefined,
|
|
cwd: "/path/to/project",
|
|
lockfilePath: "/path/to/project/.vinext/dev/lock.json",
|
|
});
|
|
expect(msg).toContain("Stale lock file");
|
|
expect(msg).toContain(".vinext/dev/lock.json");
|
|
});
|
|
});
|