diff --git a/.changeset/dev-watcher-restart-honesty.md b/.changeset/dev-watcher-restart-honesty.md new file mode 100644 index 0000000000..d9c1a025a8 --- /dev/null +++ b/.changeset/dev-watcher-restart-honesty.md @@ -0,0 +1,37 @@ +--- +"@objectstack/cli": patch +--- + +fix(cli): `objectstack dev` restarts the server after each rebuild — the running server can no longer silently disagree with `dist/objectstack.json` (#5148) + +The dev watcher rebuilt `dist/objectstack.json` on every source change and +printed `✓ recompiled — server will auto-reload`, but the running serve child +only **partially** received the rebuilt artifact: MetadataPlugin's own +artifact watcher re-ingests the metadata registry, syncs DDL + seeds for +newly-appearing objects and broadcasts the SSE HMR event — while hook bodies +and already-registered view metadata from the compiled bundle are applied at +**boot only**. #5148 measured both staying stale: the old hook kept executing +and `/api/v1/meta/views` kept serving the old view after a confirmed on-disk +rebuild, with no error and no warning. That made every dev edit/verify loop +capable of a false conclusion in either direction ("my fix doesn't work" / +"this code isn't load-bearing"). + +`objectstack dev` now supervises its serve child nodemon-style: + +- **Auto-restart (default-on).** After a rebuild lands on disk, the serve + child is SIGTERMed (the kernel shuts down gracefully), and a replacement is + spawned once it exits — boot-time load is the one path that applies the + whole artifact. Restarts coalesce (rapid saves produce one restart), a + child that ignores SIGTERM is force-killed after 8s, a replacement that + fails to come up is loud and exits dev, and parent SIGINT/SIGTERM is + forwarded to the child so no orphan server outlives dev. +- **`--no-restart` opt-out.** The watcher then only rebuilds the artifact — + and every rebuild now says explicitly that the running server keeps the + build it booted with. The watch banner states the active mode instead of + implying a hot reload the runtime only partially performs. +- **Boot-time staleness warning (#5148 startup variant).** When + `objectstack.config.ts` / `src/**` are newer than the artifact at boot + (edited while the server was down), dev warns loudly that the boot serves + the stale build, names the newest source file and the remedy + (`objectstack build`, `--compile`, or save a watched file). The boot is + never gated — the silence is removed, not the start. diff --git a/content/docs/getting-started/your-first-project.mdx b/content/docs/getting-started/your-first-project.mdx index f00d3fa459..953e1b0066 100644 --- a/content/docs/getting-started/your-first-project.mdx +++ b/content/docs/getting-started/your-first-project.mdx @@ -179,7 +179,7 @@ Postgres / MySQL / MongoDB for production — [no code changes](/docs/data-model ## 3. Run it ```bash -npm run dev # objectstack dev — hot reload +npm run dev # objectstack dev — rebuild + server restart on change ``` Or with the visual Console (data browser, metadata explorer, API docs): diff --git a/packages/cli/src/commands/dev.ts b/packages/cli/src/commands/dev.ts index 1870e5ae3a..5046a2fbd6 100644 --- a/packages/cli/src/commands/dev.ts +++ b/packages/cli/src/commands/dev.ts @@ -9,6 +9,12 @@ import os from 'os'; import path from 'path'; import { printHeader, printKV, printStep, printError } from '../utils/format.js'; import { redactConnectionUrl } from '../utils/connection-display.js'; +import { + DEV_WATCH_IGNORED, + ServeRestartCoordinator, + assessArtifactStaleness, + formatMtimeGap, +} from '../utils/dev-restart.js'; import { readEnvWithDeprecation, isMcpServerEnabled } from '@objectstack/types'; /** @@ -45,7 +51,8 @@ export function resolveDefaultDevDbUrl(opts: { } export default class Dev extends Command { - static override description = 'Start development mode with hot-reload'; + static override description = + 'Start development mode — watch sources, rebuild the artifact, and restart the server on change'; static override args = { package: Args.string({ description: 'Package name or filter pattern', default: 'all', required: false }), @@ -68,6 +75,12 @@ export default class Dev extends Command { default: false, allowNo: true, }), + restart: Flags.boolean({ + description: + 'Restart the server after each successful rebuild so the running server always matches dist/objectstack.json (#5148). With --no-restart the watcher only rebuilds the artifact — the running server keeps the build it booted with until you restart it yourself, and every rebuild says so.', + default: true, + allowNo: true, + }), // ── Runtime overrides (mirror `os start`) ──────────────────────── // These let `os dev` consume a pre-built artifact and arbitrary @@ -178,6 +191,35 @@ export default class Dev extends Command { } } + // ── Startup staleness warning (#5148 startup variant) ─────────── + // Without --compile, `os dev` boots whatever dist/objectstack.json + // holds. When the sources are NEWER than the artifact (edited while + // the server was down, stale checkout), the boot silently serves the + // old build — measured in #5148: the config had gained a capability + // and dev still printed `Plugins: 38 loaded` until a manual build. + // Warn loudly and name the remedy; never gate the boot (per triage: + // remove the silence, not the start). + const watchActive = flags.watch !== false && !flags.artifact && configExists; + if (!needsCompile && !flags.artifact && configExists) { + const stale = assessArtifactStaleness({ + artifactPath, + configPath, + srcDir: path.resolve(process.cwd(), 'src'), + }); + if (stale) { + const relArtifact = path.relative(process.cwd(), artifactPath); + const relSource = path.relative(process.cwd(), stale.newestSourcePath); + const gap = formatMtimeGap(stale.newestSourceMtimeMs - stale.artifactMtimeMs); + console.log(chalk.yellow.bold(` ⚠ ${relArtifact} is OLDER than your sources — this boot serves the STALE build.`)); + console.log(chalk.yellow(` newest source: ${relSource} (${gap} newer than the artifact)`)); + console.log(chalk.yellow( + ' fix: run `objectstack build` or start with `--compile`' + + (watchActive ? ', or save a watched file to trigger a rebuild' + (flags.restart ? ' + restart' : '') : '') + + '.', + )); + } + } + printStep('Starting dev server (local mode)...'); const environmentId = flags['environment-id'] ?? process.env.OS_ENVIRONMENT_ID ?? 'env_local'; @@ -287,81 +329,123 @@ export default class Dev extends Command { const port = flags.port ?? readEnvWithDeprecation('OS_PORT', 'PORT', { silent: true }); const binPath = process.argv[1]; - const serveChild = spawn( - process.execPath, - [ - binPath, - 'serve', - '--dev', - ...(port ? ['--port', port] : []), - ...(flags.ui ? ['--ui'] : []), - ...(flags.verbose ? ['--verbose'] : []), - ...(flags['log-level'] ? ['--log-level', flags['log-level']] : []), - ...(flags.preset ? ['--preset', flags.preset] : []), - ], - // 'ipc' adds a message channel so the serve child can report the - // port it ACTUALLY bound (dev auto-shifts off a busy port). Without - // this, the parent only knows the requested port. - { stdio: ['inherit', 'inherit', 'inherit', 'ipc'], env: localEnv }, - ); - - // ── Learn the actually-bound port from the serve child ────────── - // The child emits `{ type: 'objectstack:listening', port, url }` once - // its HTTP server is up. We surface it so the printed URL is correct - // even when the port was auto-shifted (e.g. 3000 busy → 3001). const requestedPort = port ?? '3000'; - serveChild.on('message', (msg: any) => { - if (msg?.type === 'objectstack:listening' && msg.port) { - const actual = String(msg.port); - if (actual !== requestedPort) { - console.log(chalk.dim(` ↪ server bound to port ${actual} (requested ${requestedPort})`)); - } - // ── MCP connect hint (#3167) ──────────────────────────────── - // The app IS an MCP server: the dispatcher serves /api/v1/mcp - // per-request, default-on (isMcpServerEnabled). Print how a - // coding agent (Claude Code, Cursor, any MCP client) attaches so - // the "AI builds the app it's running" loop is discoverable at the - // moment it's most useful — dev boot. An opted-out deployment - // (OS_MCP_SERVER_ENABLED=false) advertises nothing, mirroring the - // connect-UI / discovery gates that follow the same switch. - if (isMcpServerEnabled()) { - const base = - typeof msg.url === 'string' && msg.url ? msg.url.replace(/\/+$/, '') : `http://localhost:${actual}`; - const name = path.basename(process.cwd()) || 'objectstack'; - console.log(); - console.log(chalk.cyan(' 🤖 MCP server — connect a coding agent:')); - console.log(` Endpoint ${base}/api/v1/mcp`); - console.log(` Skill ${base}/api/v1/mcp/skill`); - console.log(chalk.dim(` Connect claude mcp add --transport http ${name} ${base}/api/v1/mcp`)); - console.log(chalk.dim(' Disable OS_MCP_SERVER_ENABLED=false')); + + // ── Serve child under restart supervision (#5148) ─────────────── + // The watcher below rebuilds dist/objectstack.json, but a running + // server only PARTIALLY receives a rebuilt artifact: MetadataPlugin's + // own artifact watcher re-ingests the registry, syncs DDL + seeds for + // new objects and broadcasts the SSE HMR event — while hook bodies + // and already-registered view metadata from the compiled bundle are + // applied at boot only (#5148 measured both staying stale). Boot-time + // load is the one path that applies the whole artifact, so the + // coordinator restarts the serve child after each successful rebuild + // (nodemon-style, default-on; opt out with --no-restart). + const spawnServeChild = (info: { restartIndex: number }) => { + const child = spawn( + process.execPath, + [ + binPath, + 'serve', + '--dev', + ...(port ? ['--port', port] : []), + ...(flags.ui ? ['--ui'] : []), + ...(flags.verbose ? ['--verbose'] : []), + ...(flags['log-level'] ? ['--log-level', flags['log-level']] : []), + ...(flags.preset ? ['--preset', flags.preset] : []), + ], + // 'ipc' adds a message channel so the serve child can report the + // port it ACTUALLY bound (dev auto-shifts off a busy port). Without + // this, the parent only knows the requested port. + { stdio: ['inherit', 'inherit', 'inherit', 'ipc'], env: localEnv }, + ); + + // ── Learn the actually-bound port from the serve child ──────── + // The child emits `{ type: 'objectstack:listening', port, url }` once + // its HTTP server is up. We surface it so the printed URL is correct + // even when the port was auto-shifted (e.g. 3000 busy → 3001). + child.on('message', (msg: any) => { + if (msg?.type === 'objectstack:listening' && msg.port) { + const actual = String(msg.port); + if (actual !== requestedPort) { + console.log(chalk.dim(` ↪ server bound to port ${actual} (requested ${requestedPort})`)); + } + if (info.restartIndex > 0) { + // A restarted child is listening — the running server matches + // dist/objectstack.json again. The MCP hint below is boot + // noise on a restart, so it prints on the initial boot only. + console.log(chalk.green(' ✓ server restarted — the new build is live')); + return; + } + // ── MCP connect hint (#3167) ──────────────────────────────── + // The app IS an MCP server: the dispatcher serves /api/v1/mcp + // per-request, default-on (isMcpServerEnabled). Print how a + // coding agent (Claude Code, Cursor, any MCP client) attaches so + // the "AI builds the app it's running" loop is discoverable at the + // moment it's most useful — dev boot. An opted-out deployment + // (OS_MCP_SERVER_ENABLED=false) advertises nothing, mirroring the + // connect-UI / discovery gates that follow the same switch. + if (isMcpServerEnabled()) { + const base = + typeof msg.url === 'string' && msg.url ? msg.url.replace(/\/+$/, '') : `http://localhost:${actual}`; + const name = path.basename(process.cwd()) || 'objectstack'; + console.log(); + console.log(chalk.cyan(' 🤖 MCP server — connect a coding agent:')); + console.log(` Endpoint ${base}/api/v1/mcp`); + console.log(` Skill ${base}/api/v1/mcp/skill`); + console.log(chalk.dim(` Connect claude mcp add --transport http ${name} ${base}/api/v1/mcp`)); + console.log(chalk.dim(' Disable OS_MCP_SERVER_ENABLED=false')); + } } - } + }); + return child; + }; + + const coordinator = new ServeRestartCoordinator({ + spawnChild: spawnServeChild, + exitParent: (code) => process.exit(code), + log: (line) => { + const trimmed = line.trimStart(); + if (trimmed.startsWith('✗')) console.log(chalk.red(line)); + else if (trimmed.startsWith('⚠')) console.log(chalk.yellow(line)); + else console.log(chalk.dim(line)); + }, }); + // Forward parent signals to the child (covers non-TTY parents where no + // process group delivers Ctrl-C to the child for us), and never leave + // an orphaned serve child behind whatever path ends the parent — e.g. + // --fresh's SIGINT handler calls process.exit before later SIGINT + // listeners run, but 'exit' listeners still do. + process.on('SIGINT', () => coordinator.beginShutdown('SIGINT')); + process.on('SIGTERM', () => coordinator.beginShutdown('SIGTERM')); + process.on('exit', () => coordinator.killChildOnParentExit()); + coordinator.start(); // ── Watch-recompile loop ──────────────────────────────────────── // When the agent edits an objectstack source file (config or - // src/**), debounce-rebuild dist/objectstack.json. The server - // (MetadataPlugin) watches the artifact path directly and - // broadcasts the HMR event to UI consumers (ADR-0008 PR-8); no POST - // ping required. + // src/**), debounce-rebuild dist/objectstack.json, then ask the + // coordinator to restart the serve child so the running server + // matches the artifact (#5148). With --no-restart the rebuild still + // lands on disk and the messaging says, on every rebuild, that the + // running server keeps the old build. // // Skipped when: // - --watch=false (user opted out) // - --artifact was passed (no source to watch) // - the environment has no objectstack.config.ts - if (flags.watch !== false && !flags.artifact && configExists) { + if (watchActive) { this.startWatchRecompile({ cwd: process.cwd(), configPath, artifactPath, binPath, verbose: flags.verbose, + autoRestart: flags.restart, + onRebuildLanded: flags.restart + ? (label) => coordinator.requestRestart(label) + : undefined, }); } - - serveChild.on('exit', (code) => { - process.exit(code ?? 0); - }); return; } @@ -396,14 +480,24 @@ export default class Dev extends Command { * Watch objectstack source files (config + src/**) and on change: * 1. Debounce 250ms * 2. Run `os compile` to rebuild `dist/objectstack.json` + * 3. Report the rebuild via `onRebuildLanded` so the restart + * coordinator can restart the serve child (#5148) * - * The server (MetadataPlugin) watches `dist/objectstack.json` - * directly and broadcasts the HMR event to UI consumers (ADR-0008 PR-8); - * the CLI no longer POSTs `/api/v1/dev/metadata-events`. That POST - * endpoint remains available for external trigger sources (cloud - * webhooks, git hooks, ad-hoc curl) but is not used here. + * Why a restart and not in-place reload: the server (MetadataPlugin) + * does watch `dist/objectstack.json` directly (ADR-0008 PR-8) — but that + * reload path only re-ingests the metadata registry, syncs DDL + seeds + * for NEW objects, and broadcasts the SSE HMR event to UI consumers. + * Hook bodies and already-registered view metadata from the compiled + * bundle are applied at boot only, so #5148 measured a rebuilt artifact + * with the running server still executing the old hooks and serving the + * old views — silently. Until the runtime can apply a full artifact + * in-place (ADR-0008's target state), the restart is what keeps the + * running server and the artifact from disagreeing without a word. + * The `/api/v1/dev/metadata-events` POST endpoint remains available for + * external trigger sources (cloud webhooks, git hooks, ad-hoc curl) but + * is not used here. * - * The watcher runs in this parent process; the serve child stays untouched. + * The watcher runs in this parent process, across serve child restarts. */ private startWatchRecompile(opts: { cwd: string; @@ -411,6 +505,10 @@ export default class Dev extends Command { artifactPath: string; binPath: string; verbose?: boolean; + /** Whether a successful rebuild auto-restarts the serve child (#5148). */ + autoRestart: boolean; + /** Called after a rebuild landed on disk (label = what changed). */ + onRebuildLanded?: (label: string) => void; }): void { void (async () => { const chokidar = (await import('chokidar')).default; @@ -419,13 +517,9 @@ export default class Dev extends Command { if (fs.existsSync(srcDir)) watchPaths.push(srcDir); const watcher = chokidar.watch(watchPaths, { - ignored: [ - /node_modules/, - /\.git/, - /\.objectstack\//, - /\bdist\b/, - /\.test\.[jt]sx?$/, - ], + // Shared with the boot-time staleness check (#5148) so both judge + // the same source surface. + ignored: DEV_WATCH_IGNORED, ignoreInitial: true, persistent: true, // Use polling to avoid `fs.watch` EMFILE on macOS when other @@ -481,14 +575,21 @@ export default class Dev extends Command { if (r.status !== 0) { const stderr = r.stderr?.toString().trim(); console.log(chalk.red(` ✗ compile failed (${dt}ms)${stderr ? '\n' + stderr : ''}`)); + // A failed compile writes no artifact — the running server still + // matches dist/objectstack.json, so no restart and no warning. } else { - // ADR-0008 PR-8: the server now watches the artifact file - // directly via MetadataPlugin and reloads + broadcasts - // HMR events autonomously. The CLI no longer needs to POST - // /api/v1/dev/metadata-events. The endpoint remains - // available for external trigger sources (cloud webhooks, - // git hooks, ad-hoc curl). - console.log(chalk.green(` ✓ recompiled in ${dt}ms — server will auto-reload`)); + if (opts.autoRestart) { + console.log(chalk.green(` ✓ recompiled in ${dt}ms`)); + } else { + // --no-restart: the artifact and the running server now + // disagree — say so on EVERY rebuild (#5148: the old + // "server will auto-reload" line advertised a hot reload the + // runtime only partially performs). + console.log(chalk.green(` ✓ recompiled in ${dt}ms — artifact updated on disk`)); + console.log(chalk.yellow( + ' ⚠ auto-restart is off (--no-restart): the running server keeps the build it booted with — restart dev to apply', + )); + } const objectsNow = readArtifactObjects(); if (objectsNow) { const prior = knownObjects; @@ -497,11 +598,12 @@ export default class Dev extends Command { if (fresh.length > 0) { console.log( chalk.cyan( - ` ✚ new object(s): ${fresh.join(', ')} — table & seeds sync on reload`, + ` ✚ new object(s): ${fresh.join(', ')} — table & seeds sync on ${opts.autoRestart ? 'restart' : 'reload'}`, ), ); } } + opts.onRebuildLanded?.(label); } inFlight = false; if (queued) { queued = false; setTimeout(compileAndPing, 50); } @@ -517,7 +619,13 @@ export default class Dev extends Command { watcher.on('add', schedule); watcher.on('unlink', schedule); watcher.on('ready', () => { - console.log(chalk.dim(` 👁 watching ${watchPaths.map(p => path.relative(opts.cwd, p) || '.').join(', ')} for changes`)); + // Honest banner (#5148): say what a rebuild actually does to the + // running server in the current mode, instead of implying a hot + // reload the runtime only partially performs. + const what = opts.autoRestart + ? 'rebuild + restart on change' + : 'rebuild on change (auto-restart off — running server keeps its build)'; + console.log(chalk.dim(` 👁 watching ${watchPaths.map(p => path.relative(opts.cwd, p) || '.').join(', ')} — ${what}`)); }); // Clean up on process exit diff --git a/packages/cli/src/utils/dev-restart.test.ts b/packages/cli/src/utils/dev-restart.test.ts new file mode 100644 index 0000000000..242b732131 --- /dev/null +++ b/packages/cli/src/utils/dev-restart.test.ts @@ -0,0 +1,334 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect, afterEach } from 'vitest'; +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import { + ServeRestartCoordinator, + type ServeChildLike, + assessArtifactStaleness, + formatMtimeGap, + isDevWatchIgnored, +} from './dev-restart.js'; + +// ──────────────────────────────────────────────────────────────────────────── +// assessArtifactStaleness — the #5148 startup variant: booting on an artifact +// older than the sources must be detectable (loudly warned by dev.ts), and a +// current artifact must produce NO report. +// ──────────────────────────────────────────────────────────────────────────── + +describe('assessArtifactStaleness', () => { + const tmpDirs: string[] = []; + const mkProject = () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'os-dev-stale-')); + tmpDirs.push(dir); + const configPath = path.join(dir, 'objectstack.config.ts'); + const srcDir = path.join(dir, 'src'); + const artifactPath = path.join(dir, 'dist', 'objectstack.json'); + fs.mkdirSync(srcDir, { recursive: true }); + fs.mkdirSync(path.dirname(artifactPath), { recursive: true }); + fs.writeFileSync(configPath, '// config'); + return { dir, configPath, srcDir, artifactPath }; + }; + const touch = (p: string, epochSec: number) => { + fs.mkdirSync(path.dirname(p), { recursive: true }); + if (!fs.existsSync(p)) fs.writeFileSync(p, '// x'); + fs.utimesSync(p, epochSec, epochSec); + }; + afterEach(() => { + for (const d of tmpDirs.splice(0)) { + try { fs.rmSync(d, { recursive: true, force: true }); } catch { /* noop */ } + } + }); + + const T = 1_700_000_000; // arbitrary fixed epoch base (seconds) + + it('returns null when the artifact is missing (dev compiles it fresh)', () => { + const p = mkProject(); + touch(p.configPath, T + 100); + expect( + assessArtifactStaleness({ artifactPath: p.artifactPath, configPath: p.configPath, srcDir: p.srcDir }), + ).toBeNull(); + }); + + it('returns null when the artifact is newer than every source', () => { + const p = mkProject(); + touch(p.configPath, T); + touch(path.join(p.srcDir, 'views', 'a.view.ts'), T + 10); + touch(p.artifactPath, T + 100); + expect( + assessArtifactStaleness({ artifactPath: p.artifactPath, configPath: p.configPath, srcDir: p.srcDir }), + ).toBeNull(); + }); + + it('reports the config file when it is newer than the artifact', () => { + const p = mkProject(); + touch(p.artifactPath, T); + touch(p.configPath, T + 60); + const report = assessArtifactStaleness({ + artifactPath: p.artifactPath, configPath: p.configPath, srcDir: p.srcDir, + }); + expect(report).not.toBeNull(); + expect(report!.newestSourcePath).toBe(p.configPath); + expect(report!.newestSourceMtimeMs).toBeGreaterThan(report!.artifactMtimeMs); + }); + + it('reports a nested src file when it is the newest source', () => { + const p = mkProject(); + touch(p.configPath, T); + touch(p.artifactPath, T + 50); + const hook = path.join(p.srcDir, 'hooks', 'rating.hook.ts'); + touch(hook, T + 120); + const report = assessArtifactStaleness({ + artifactPath: p.artifactPath, configPath: p.configPath, srcDir: p.srcDir, + }); + expect(report?.newestSourcePath).toBe(hook); + }); + + it('ignores the same paths the watcher ignores (node_modules, tests, dist)', () => { + const p = mkProject(); + touch(p.configPath, T); + touch(p.artifactPath, T + 50); + // All newer than the artifact, all outside the watched surface: + touch(path.join(p.srcDir, 'node_modules', 'dep', 'index.ts'), T + 500); + touch(path.join(p.srcDir, 'views', 'a.view.test.ts'), T + 500); + touch(path.join(p.srcDir, 'dist', 'out.ts'), T + 500); + expect( + assessArtifactStaleness({ artifactPath: p.artifactPath, configPath: p.configPath, srcDir: p.srcDir }), + ).toBeNull(); + }); + + it('works without a src dir (config-only project)', () => { + const p = mkProject(); + fs.rmSync(p.srcDir, { recursive: true, force: true }); + touch(p.artifactPath, T); + touch(p.configPath, T + 60); + const report = assessArtifactStaleness({ + artifactPath: p.artifactPath, configPath: p.configPath, srcDir: p.srcDir, + }); + expect(report?.newestSourcePath).toBe(p.configPath); + }); +}); + +describe('isDevWatchIgnored', () => { + it('matches the watcher ignore surface', () => { + expect(isDevWatchIgnored('/proj/src/node_modules/x.ts')).toBe(true); + expect(isDevWatchIgnored('/proj/src/a.view.test.ts')).toBe(true); + expect(isDevWatchIgnored('/proj/dist/objectstack.json')).toBe(true); + expect(isDevWatchIgnored('/proj/.objectstack/data/dev.db')).toBe(true); + expect(isDevWatchIgnored('/proj/src/views/a.view.ts')).toBe(false); + expect(isDevWatchIgnored('/proj/objectstack.config.ts')).toBe(false); + // \bdist\b must not swallow lookalike segments: + expect(isDevWatchIgnored('/proj/src/distribution/a.ts')).toBe(false); + }); +}); + +describe('formatMtimeGap', () => { + it('renders humane gap sizes', () => { + expect(formatMtimeGap(500)).toBe('1s'); + expect(formatMtimeGap(42_000)).toBe('42s'); + expect(formatMtimeGap(3 * 60_000)).toBe('3m'); + expect(formatMtimeGap(5 * 3_600_000)).toBe('5h'); + expect(formatMtimeGap(3 * 86_400_000)).toBe('3d'); + }); +}); + +// ──────────────────────────────────────────────────────────────────────────── +// ServeRestartCoordinator — the restart decision seam. +// ──────────────────────────────────────────────────────────────────────────── + +class FakeChild implements ServeChildLike { + pid = 4242; + signals: (NodeJS.Signals | undefined)[] = []; + private exitListeners: Array<(code: number | null, signal: NodeJS.Signals | null) => void> = []; + kill(signal?: NodeJS.Signals): boolean { + this.signals.push(signal); + return true; + } + once(_event: 'exit', listener: (code: number | null, signal: NodeJS.Signals | null) => void): this { + this.exitListeners.push(listener); + return this; + } + emitExit(code: number | null, signal: NodeJS.Signals | null = null): void { + for (const l of this.exitListeners.splice(0)) l(code, signal); + } +} + +function makeHarness(opts: { forceKillAfterMs?: number; failSpawnAt?: number } = {}) { + const children: FakeChild[] = []; + const spawnInfos: Array<{ restartIndex: number }> = []; + const exits: number[] = []; + const logs: string[] = []; + const coordinator = new ServeRestartCoordinator({ + spawnChild: (info) => { + spawnInfos.push(info); + if (opts.failSpawnAt !== undefined && spawnInfos.length - 1 === opts.failSpawnAt) { + throw new Error('spawn boom'); + } + const c = new FakeChild(); + children.push(c); + return c; + }, + exitParent: (code) => { exits.push(code); }, + log: (line) => { logs.push(line); }, + forceKillAfterMs: opts.forceKillAfterMs, + }); + return { coordinator, children, spawnInfos, exits, logs }; +} + +const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); + +describe('ServeRestartCoordinator', () => { + it('start() boots exactly one child with restartIndex 0', () => { + const h = makeHarness(); + h.coordinator.start(); + h.coordinator.start(); // idempotent + expect(h.children).toHaveLength(1); + expect(h.spawnInfos).toEqual([{ restartIndex: 0 }]); + expect(h.coordinator.getState()).toBe('running'); + }); + + it('requestRestart: SIGTERMs the child, then spawns a replacement on exit', () => { + const h = makeHarness(); + h.coordinator.start(); + h.coordinator.requestRestart('src/a.view.ts'); + expect(h.children[0].signals).toEqual(['SIGTERM']); + expect(h.coordinator.getState()).toBe('stopping'); + h.children[0].emitExit(0); + expect(h.children).toHaveLength(2); + expect(h.spawnInfos[1]).toEqual({ restartIndex: 1 }); + expect(h.coordinator.getState()).toBe('running'); + expect(h.exits).toEqual([]); // the parent survives a restart + }); + + it('coalesces restart requests that arrive while a restart is tearing down', () => { + const h = makeHarness(); + h.coordinator.start(); + h.coordinator.requestRestart('first'); + h.coordinator.requestRestart('second'); // absorbed — pending boot reads newest artifact + h.coordinator.requestRestart('third'); // absorbed + expect(h.children[0].signals).toEqual(['SIGTERM']); // exactly one kill + h.children[0].emitExit(0); + expect(h.children).toHaveLength(2); // exactly one replacement + }); + + it('a rebuild landing while the replacement runs triggers a fresh restart cycle', () => { + const h = makeHarness(); + h.coordinator.start(); + h.coordinator.requestRestart('a'); + h.children[0].emitExit(0); + h.coordinator.requestRestart('b'); // replacement is 'running' again + expect(h.children[1].signals).toEqual(['SIGTERM']); + h.children[1].emitExit(0); + expect(h.children).toHaveLength(3); + expect(h.exits).toEqual([]); + }); + + it('a child exit the coordinator did not initiate exits the parent with code ?? 0', () => { + const h = makeHarness(); + h.coordinator.start(); + h.children[0].emitExit(7); + expect(h.exits).toEqual([7]); + expect(h.children).toHaveLength(1); // no respawn + // signal-killed child (code null) keeps the pre-#5148 `code ?? 0` contract: + const h2 = makeHarness(); + h2.coordinator.start(); + h2.children[0].emitExit(null, 'SIGKILL'); + expect(h2.exits).toEqual([0]); + }); + + it('is loud when a restarted child dies on its own', () => { + const h = makeHarness(); + h.coordinator.start(); + h.coordinator.requestRestart('a'); + h.children[0].emitExit(0); + h.children[1].emitExit(1); // the restarted child crashed (e.g. bad new build at boot) + expect(h.exits).toEqual([1]); + expect(h.logs.some((l) => l.includes('server exited'))).toBe(true); + }); + + it('beginShutdown forwards the signal and the child exit then ends the parent — no respawn', () => { + const h = makeHarness(); + h.coordinator.start(); + h.coordinator.beginShutdown('SIGTERM'); + expect(h.children[0].signals).toEqual(['SIGTERM']); + h.children[0].emitExit(0); + expect(h.exits).toEqual([0]); + expect(h.children).toHaveLength(1); + // and restart requests after shutdown are ignored: + h.coordinator.requestRestart('late'); + expect(h.children).toHaveLength(1); + expect(h.children[0].signals).toEqual(['SIGTERM']); + }); + + it('shutdown during a restart teardown wins over the respawn', () => { + const h = makeHarness(); + h.coordinator.start(); + h.coordinator.requestRestart('a'); + h.coordinator.beginShutdown('SIGINT'); + h.children[0].emitExit(0); + expect(h.children).toHaveLength(1); // no replacement spawned + expect(h.exits).toEqual([0]); + }); + + it('requestRestart before start() is a no-op (first boot reads the fresh artifact)', () => { + const h = makeHarness(); + h.coordinator.requestRestart('early'); + expect(h.children).toHaveLength(0); + h.coordinator.start(); + expect(h.children).toHaveLength(1); + }); + + it('escalates to SIGKILL when the child ignores SIGTERM, then still respawns', async () => { + const h = makeHarness({ forceKillAfterMs: 20 }); + h.coordinator.start(); + h.coordinator.requestRestart('a'); + expect(h.children[0].signals).toEqual(['SIGTERM']); + await sleep(60); + expect(h.children[0].signals).toEqual(['SIGTERM', 'SIGKILL']); + expect(h.logs.some((l) => l.includes('force-killing'))).toBe(true); + h.children[0].emitExit(null, 'SIGKILL'); + expect(h.children).toHaveLength(2); // replacement still comes up + expect(h.exits).toEqual([]); + }); + + it('does not force-kill a child that exited in time', async () => { + const h = makeHarness({ forceKillAfterMs: 20 }); + h.coordinator.start(); + h.coordinator.requestRestart('a'); + h.children[0].emitExit(0); // graceful, well inside the window + await sleep(60); + expect(h.children[0].signals).toEqual(['SIGTERM']); // no SIGKILL + }); + + it('a failed respawn is loud and exits the parent', () => { + const h = makeHarness({ failSpawnAt: 1 }); + h.coordinator.start(); + h.coordinator.requestRestart('a'); + h.children[0].emitExit(0); + expect(h.exits).toEqual([1]); + expect(h.logs.some((l) => l.includes('failed to restart server'))).toBe(true); + }); + + it('killChildOnParentExit SIGTERMs a live child and is a no-op after exit', () => { + const h = makeHarness(); + h.coordinator.start(); + h.coordinator.killChildOnParentExit(); + expect(h.children[0].signals).toEqual(['SIGTERM']); + h.children[0].emitExit(0); // (counts as self-exit → parent exit recorded) + h.coordinator.killChildOnParentExit(); + expect(h.children[0].signals).toEqual(['SIGTERM']); // no second signal + }); + + it('ignores a late exit event from an already-replaced child', () => { + const h = makeHarness(); + h.coordinator.start(); + h.coordinator.requestRestart('a'); + h.children[0].emitExit(0); + expect(h.children).toHaveLength(2); + h.children[0].emitExit(1); // stale double-fire from the dead child + expect(h.exits).toEqual([]); // replacement unaffected, parent alive + expect(h.coordinator.getState()).toBe('running'); + }); +}); diff --git a/packages/cli/src/utils/dev-restart.ts b/packages/cli/src/utils/dev-restart.ts new file mode 100644 index 0000000000..8099081706 --- /dev/null +++ b/packages/cli/src/utils/dev-restart.ts @@ -0,0 +1,348 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Testable seams for `objectstack dev`'s watch → rebuild → restart pipeline + * (#5148). + * + * Background: the dev watcher rebuilds `dist/objectstack.json` on source + * changes, but the running serve child only *partially* receives a rebuilt + * artifact (new-object DDL + seeds sync and an SSE broadcast fire; hook + * bodies and already-registered view metadata from the compiled bundle are + * boot-time-only). #5148 measured the result: the artifact on disk and the + * behaviour of the server disagree silently, so every dev edit/verify loop + * can produce a false conclusion in either direction. Boot-time load is the + * one path that applies the whole artifact, so the fix is nodemon-style: on + * each successful rebuild, restart the serve child ({@link ServeRestartCoordinator}), + * and at boot warn when the artifact is already older than the sources + * ({@link assessArtifactStaleness}). + */ + +import fs from 'fs'; +import path from 'path'; + +/** + * Paths the dev watcher does NOT consider objectstack sources. Shared between + * the chokidar watcher (which triggers rebuilds) and the boot-time staleness + * walk (which warns about a stale artifact), so both judge exactly the same + * source surface — a file that can trigger a rebuild is a file that can make + * the artifact stale, and vice versa. + */ +export const DEV_WATCH_IGNORED: RegExp[] = [ + /node_modules/, + /\.git/, + /\.objectstack\//, + /\bdist\b/, + /\.test\.[jt]sx?$/, +]; + +/** True when the path is outside the dev watcher's source surface. */ +export function isDevWatchIgnored(p: string): boolean { + return DEV_WATCH_IGNORED.some((re) => re.test(p)); +} + +export interface ArtifactStalenessReport { + /** mtime (ms) of the compiled artifact. */ + artifactMtimeMs: number; + /** mtime (ms) of the newest watched source file. */ + newestSourceMtimeMs: number; + /** Absolute path of the newest watched source file. */ + newestSourcePath: string; +} + +/** + * Boot-time staleness check (#5148 startup variant): compare the compiled + * artifact against the sources that compile into it (`objectstack.config.ts` + * + `src/**`, minus {@link DEV_WATCH_IGNORED}). + * + * Returns a report when at least one source is strictly NEWER than the + * artifact — i.e. booting now silently serves a stale build — and `null` + * when the artifact is current, missing (dev auto-compiles a missing + * artifact), or unreadable. This is a diagnostic: it must never throw and + * never gate the boot. + */ +export function assessArtifactStaleness(opts: { + artifactPath: string; + configPath: string; + srcDir?: string; + /** + * Walk budget — directory entries visited before the walk stops. A dev + * project's `src/` is small; the cap only guards against pathological + * trees. Best-effort by design: a capped walk can miss staleness, never + * invent it. + */ + maxEntries?: number; +}): ArtifactStalenessReport | null { + let artifactMtimeMs: number; + try { + artifactMtimeMs = fs.statSync(opts.artifactPath).mtimeMs; + } catch { + return null; // no artifact → dev compiles it fresh; nothing stale to report + } + + let newestPath: string | null = null; + let newestMtimeMs = -Infinity; + const consider = (p: string) => { + try { + const st = fs.statSync(p); + if (st.isFile() && st.mtimeMs > newestMtimeMs) { + newestMtimeMs = st.mtimeMs; + newestPath = p; + } + } catch { + /* raced deletion — skip */ + } + }; + + consider(opts.configPath); + + const budget = opts.maxEntries ?? 20_000; + let visited = 0; + const walk = (dir: string) => { + if (visited >= budget) return; + let entries: fs.Dirent[]; + try { + entries = fs.readdirSync(dir, { withFileTypes: true }); + } catch { + return; + } + for (const ent of entries) { + if (visited++ >= budget) return; + const p = path.join(dir, ent.name); + if (isDevWatchIgnored(p)) continue; + if (ent.isSymbolicLink()) continue; // no cycles + if (ent.isDirectory()) walk(p); + else if (ent.isFile()) consider(p); + } + }; + if (opts.srcDir) { + try { + if (fs.statSync(opts.srcDir).isDirectory()) walk(opts.srcDir); + } catch { + /* no src dir — config-only project */ + } + } + + if (newestPath !== null && newestMtimeMs > artifactMtimeMs) { + return { artifactMtimeMs, newestSourceMtimeMs: newestMtimeMs, newestSourcePath: newestPath }; + } + return null; +} + +/** Human-readable rendering of an mtime gap, for the staleness warning. */ +export function formatMtimeGap(ms: number): string { + const s = Math.max(1, Math.round(ms / 1000)); + if (s < 90) return `${s}s`; + const m = Math.round(s / 60); + if (m < 90) return `${m}m`; + const h = Math.round(m / 60); + if (h < 36) return `${h}h`; + return `${Math.round(h / 24)}d`; +} + +/** + * The slice of `child_process.ChildProcess` the coordinator needs — narrow so + * tests can drive the state machine with a fake child. + */ +export interface ServeChildLike { + readonly pid?: number; + kill(signal?: NodeJS.Signals): boolean; + once( + event: 'exit', + listener: (code: number | null, signal: NodeJS.Signals | null) => void, + ): unknown; +} + +export interface ServeRestartCoordinatorOptions { + /** + * Spawn a serve child. `restartIndex` is 0 for the initial boot and 1+ + * for restarts, so the caller can vary its messaging (e.g. print the MCP + * connect hint only once). + */ + spawnChild: (info: { restartIndex: number }) => ServeChildLike; + /** Terminate the dev parent process with this exit code. */ + exitParent: (code: number) => void; + /** Line sink for coordinator messages (default: console.log). */ + log?: (line: string) => void; + /** + * SIGTERM → SIGKILL escalation delay for a child that ignores the + * graceful stop. The kernel's shutdown handler normally exits well within + * this window; the escalation only exists so a wedged child cannot stall + * the restart loop forever. + */ + forceKillAfterMs?: number; +} + +export type ServeRestartState = 'idle' | 'running' | 'stopping' | 'shutdown'; + +/** + * Supervises the single serve child of `objectstack dev` (#5148, nodemon + * style). + * + * States: `idle` → `running` ⇄ `stopping` → `shutdown`. + * + * - {@link requestRestart} (called after each successful rebuild lands on + * disk): SIGTERM the child; when it exits, spawn a replacement, which + * reads the newest artifact at boot. Requests arriving while a restart is + * already tearing down are absorbed — the upcoming boot picks up the + * newest build anyway, so consecutive rebuilds coalesce into one restart. + * - A child exit the coordinator did NOT initiate keeps the pre-#5148 + * contract: the parent follows with the child's exit code (`code ?? 0`), + * loudly when the exit follows a restart. + * - {@link beginShutdown} (parent got SIGINT/SIGTERM): forward the signal to + * the child; its exit then ends the parent — the same path a Ctrl-C took + * before, but now also covered when no TTY process group delivers the + * signal to the child for us. + */ +export class ServeRestartCoordinator { + private state: ServeRestartState = 'idle'; + private child: ServeChildLike | null = null; + /** Number of spawns so far — 0 until start(), 1 after the initial boot. */ + private spawnCount = 0; + private forceKillTimer: ReturnType | null = null; + + constructor(private readonly opts: ServeRestartCoordinatorOptions) {} + + getState(): ServeRestartState { + return this.state; + } + + /** Boot the initial serve child. No-op unless idle. */ + start(): void { + if (this.state !== 'idle') return; + this.spawn(); + } + + /** + * A successful rebuild landed on disk — restart the serve child so the + * running server matches `dist/objectstack.json` again. + */ + requestRestart(reason: string): void { + if (this.state === 'shutdown' || this.state === 'idle') return; + if (this.state === 'stopping') { + // Coalesce: the restart already in flight boots from disk, which + // now holds the newer build. + this.log(' ↻ restart already in progress — the pending boot picks up the newest build'); + return; + } + this.state = 'stopping'; + this.log(` ↻ restarting server to apply the new build (${reason})...`); + const child = this.child; + if (!child) { + // Defensive: running-with-no-child cannot happen; recover by booting. + this.spawn(); + return; + } + this.armForceKill(child); + try { + child.kill('SIGTERM'); + } catch { + /* child raced its own exit — the exit handler takes over */ + } + } + + /** + * The dev parent received `signal`: forward it to the child and let the + * child's exit end the parent (pre-existing dev semantics). + */ + beginShutdown(signal: NodeJS.Signals): void { + if (this.state === 'shutdown') return; + this.state = 'shutdown'; + if (this.child) { + try { + this.child.kill(signal); + } catch { + /* already gone */ + } + } + } + + /** + * Last-resort orphan guard for `process.on('exit')`: whatever path ends + * the parent (a `--fresh` SIGINT handler's early `process.exit`, an + * uncaught error), the serve child must not linger. Sync-only. + */ + killChildOnParentExit(): void { + if (this.child) { + try { + this.child.kill('SIGTERM'); + } catch { + /* noop */ + } + } + } + + private log(line: string): void { + (this.opts.log ?? console.log)(line); + } + + private spawn(): void { + const info = { restartIndex: this.spawnCount }; + let child: ServeChildLike; + try { + child = this.opts.spawnChild(info); + } catch (e: any) { + this.log( + ` ✗ failed to ${info.restartIndex > 0 ? 'restart' : 'start'} server: ${e?.message ?? e}`, + ); + this.state = 'shutdown'; + this.opts.exitParent(1); + return; + } + this.child = child; + this.spawnCount++; + this.state = 'running'; + child.once('exit', (code, signal) => this.onChildExit(child, code, signal)); + } + + private onChildExit( + child: ServeChildLike, + code: number | null, + signal: NodeJS.Signals | null, + ): void { + if (child !== this.child) return; // late event from an already-replaced child + this.clearForceKill(); + this.child = null; + if (this.state === 'shutdown') { + this.opts.exitParent(code ?? 0); + return; + } + if (this.state === 'stopping') { + // We asked it to stop — bring up the replacement on the fresh artifact. + this.spawn(); + return; + } + // Exited on its own (crash, or killed externally). Keep the + // pre-#5148 contract — the parent follows the child — and say so + // when the dead child was a restarted one, so a rebuild that boots + // into a crash is loud instead of a silently vanished dev session. + this.state = 'shutdown'; + if (this.spawnCount > 1) { + this.log( + ` ✗ server exited (${signal ? `signal ${signal}` : `code ${code}`}) — stopping dev`, + ); + } + this.opts.exitParent(code ?? 0); + } + + private armForceKill(child: ServeChildLike): void { + this.clearForceKill(); + const ms = this.opts.forceKillAfterMs ?? 8000; + const t = setTimeout(() => { + this.log(` ⚠ server did not exit within ${ms}ms — force-killing (SIGKILL)`); + try { + child.kill('SIGKILL'); + } catch { + /* gone */ + } + }, ms); + (t as { unref?: () => void }).unref?.(); + this.forceKillTimer = t; + } + + private clearForceKill(): void { + if (this.forceKillTimer) { + clearTimeout(this.forceKillTimer); + this.forceKillTimer = null; + } + } +}