fix: serialize next build across worker threads (#52)

.npmignore

@@ -0,0 +1,9 @@
+# Belt-and-suspenders backstop. The published file list is controlled primarily by the "files"
+# allowlist in package.json, which ships `dist/**` minus `dist/**/*.test.*` — so any new source module
+# is included automatically while compiled test code is excluded.
+#
+# This .npmignore is only a secondary guard: an `.npmignore` cannot subtract from a directory that the
+# "files" allowlist force-includes, so it has no effect while that allowlist is in place. It exists to
+# keep compiled test code out of the package if the allowlist is ever loosened or removed. The pattern
+# matches every test artifact — `.js`, `.d.ts`, `.js.map`, `.d.ts.map` — at any depth.
+*.test.*

package.json

@@ -24,7 +24,8 @@
 	},
 	"license": "Apache-2.0",
 	"files": [
-		"dist",
+		"dist/**",
+		"!dist/**/*.test.*",
 		"config.yaml",
 		"schema.graphql"
 	],
@@ -38,6 +39,7 @@
 	"scripts": {
 		"build": "tsc -p tsconfig.build.json",
 		"install:fixtures": "node scripts/install-fixtures.js",
+		"test": "npm run build && node --test \"dist/**/*.test.js\"",
 		"test:integration": "playwright test --config integrationTests/playwright.config.ts"
 	},
 	"engines": {

schema.graphql

@@ -1,3 +1,8 @@
+# Coordinates production builds across Harper worker threads (and processes sharing a database) so the
+# app is built once per instance rather than once per worker. A worker claims the per-app record with
+# status "building" before compiling; sibling workers observe the fresh claim and wait rather than
+# building into the same .next concurrently. Once built, `status` is "success" (with the resulting
+# `buildId`) or "failure". See the build() coordination logic in src/plugin.ts.
 type NextBuildInfo @table(database: "harperfast_nextjs", table: "nextjs_build_info") {
 	appName: String @primaryKey
 	buildId: String

src/buildLock.test.ts

@@ -0,0 +1,204 @@
+import { databases, type Scope } from 'harper';
+import { describe, it } from 'node:test';
+import assert from 'node:assert';
+
+import { withBuildLock, type BuildLockDeps } from './buildLock.js';
+
+// The build lock coordinates through `databases.harperfast_nextjs.nextjs_build_info` (imported from
+// `harper`). These unit tests run "outside Harper", so clear any real `harperfast_nextjs` database a local
+// Harper install may have opened on import, then inject a mock table per test. Because `databases` is a
+// shared singleton, assigning at that path is what makes the module under test see the mock — this is why
+// `buildLock` imports `databases` instead of reading `globalThis`.
+(databases as any).harperfast_nextjs = {};
+
+const TABLE = 'nextjs_build_info';
+
+describe('withBuildLock', () => {
+	/** Records as Harper returns them from `table.get`: a status/buildId plus the time last written. */
+	const building = (ageMs = 0) => ({ status: 'building', buildId: null, getUpdatedTime: () => Date.now() - ageMs });
+	const success = (buildId: string, ageMs = 0) => ({ status: 'success', buildId, getUpdatedTime: () => Date.now() - ageMs });
+	const failure = (ageMs = 0) => ({ status: 'failure', buildId: null, getUpdatedTime: () => Date.now() - ageMs });
+
+	/**
+	 * A stand-in for the Harper build-info table. `get` returns the scripted records in order (the last one
+	 * repeats); every `put` is appended to `events` so a test can assert the exact claim → build → record
+	 * interleaving.
+	 */
+	function makeTable(events: string[], getReturns: any[] = [undefined]) {
+		let i = 0;
+		return {
+			puts: [] as Array<{ key: string; value: any }>,
+			async get(_key: string) {
+				return getReturns[Math.min(i++, getReturns.length - 1)];
+			},
+			async put(key: string, value: any) {
+				this.puts.push({ key, value });
+				events.push(`put:${value.status}`);
+			},
+		};
+	}
+
+	/** Install `table` as the build-info table for one test, removing it afterward so tests don't leak state. */
+	function useTable(t: any, table: unknown) {
+		(databases as any).harperfast_nextjs[TABLE] = table;
+		t.after(() => delete (databases as any).harperfast_nextjs[TABLE]);
+	}
+
+	// A minimal Scope: `withBuildLock` only touches `appName`/`directory` (for logs) and the `logger`.
+	const scope = { appName: 'test-app', directory: '/test/dir', logger: {} } as unknown as Scope;
+
+	/** Yield to the microtask/macrotask queue so awaited puts settle (setImmediate is not mock-timed here). */
+	const tick = () => new Promise((resolve) => setImmediate(resolve));
+
+	/** Build deps whose `runBuild` records that it ran and resolves with a BUILD_ID. */
+	function makeDeps(events: string[], buildId = 'new-build', getBuildId: () => string | null = () => 'on-disk'): BuildLockDeps {
+		return {
+			getBuildId,
+			runBuild: async () => {
+				events.push('build');
+				return buildId;
+			},
+		};
+	}
+
+	it('runs the build directly when no Harper table is available (e.g. unit tests / outside Harper)', async () => {
+		delete (databases as any).harperfast_nextjs[TABLE];
+		const events: string[] = [];
+
+		await withBuildLock(scope, makeDeps(events));
+
+		assert.deepStrictEqual(events, ['build'], 'builds without any locking when there is no table');
+	});
+
+	it('claims the build, runs it, then records success when no record exists', async (t) => {
+		const events: string[] = [];
+		const table = makeTable(events); // get → undefined (no existing record)
+		useTable(t, table);
+
+		await withBuildLock(scope, makeDeps(events, 'abc123'));
+
+		assert.deepStrictEqual(
+			events,
+			['put:building', 'build', 'put:success'],
+			'claims (building) before building and records success (with the new BUILD_ID) after'
+		);
+		assert.deepStrictEqual(table.puts.map((p) => p.value.status), ['building', 'success']);
+		assert.strictEqual(table.puts[1].value.buildId, 'abc123', 'stores the BUILD_ID returned by the build');
+		assert.deepStrictEqual(table.puts.map((p) => p.key), ['test-app', 'test-app'], 'keyed by appName');
+	});
+
+	it('waits without building when another worker holds a fresh claim, returning once it succeeds', async (t) => {
+		const events: string[] = [];
+		// Gate sees a fresh "building" claim; after one poll the sibling has finished with a matching build.
+		const table = makeTable(events, [building(), success('done')]);
+		useTable(t, table);
+
+		await withBuildLock(scope, makeDeps(events, 'ignored', () => 'done'));
+
+		assert.ok(!events.includes('build'), 'does not build while another worker holds the claim');
+		assert.deepStrictEqual(table.puts, [], 'never writes a claim of its own — the sibling produced the output');
+	});
+
+	it('treats a "building" record with no recent heartbeat as abandoned and builds anyway', async (t) => {
+		const events: string[] = [];
+		// No heartbeat for longer than the stale threshold → the holder is assumed crashed.
+		const table = makeTable(events, [building(5 * 60 * 1000)]);
+		useTable(t, table);
+
+		await withBuildLock(scope, makeDeps(events));
+
+		assert.deepStrictEqual(events, ['put:building', 'build', 'put:success'], 'reclaims and builds past a stale claim');
+	});
+
+	it('reuses a fresh successful build without building when the on-disk BUILD_ID matches', async (t) => {
+		const events: string[] = [];
+		const table = makeTable(events, [success('match')]);
+		useTable(t, table);
+
+		await withBuildLock(scope, makeDeps(events, 'ignored', () => 'match'));
+
+		assert.deepStrictEqual(events, [], 'no build and no writes — the existing build is reused');
+	});
+
+	it('rebuilds when a fresh successful record does not match the on-disk BUILD_ID', async (t) => {
+		const events: string[] = [];
+		const table = makeTable(events, [success('old')]);
+		useTable(t, table);
+
+		await withBuildLock(scope, makeDeps(events, 'fresh', () => 'different'));
+
+		assert.deepStrictEqual(events, ['put:building', 'build', 'put:success'], 'a BUILD_ID mismatch forces a rebuild');
+	});
+
+	it('rebuilds when the last successful build is older than the fresh-build window', async (t) => {
+		const events: string[] = [];
+		// Matching BUILD_ID but older than 5s → not reusable; a restart should rebuild.
+		const table = makeTable(events, [success('match', 6000)]);
+		useTable(t, table);
+
+		await withBuildLock(scope, makeDeps(events, 'fresh', () => 'match'));
+
+		assert.deepStrictEqual(events, ['put:building', 'build', 'put:success'], 'a stale success record is rebuilt');
+	});
+
+	it('skips building when a sibling just recorded a failure (avoids failing on every thread)', async (t) => {
+		const events: string[] = [];
+		const table = makeTable(events, [failure()]);
+		useTable(t, table);
+
+		await withBuildLock(scope, makeDeps(events));
+
+		assert.deepStrictEqual(events, [], 'does not rebuild while a fresh failure is recorded');
+	});
+
+	it('records failure and rethrows when the build it runs throws', async (t) => {
+		const events: string[] = [];
+		const table = makeTable(events);
+		useTable(t, table);
+		const deps: BuildLockDeps = {
+			getBuildId: () => null,
+			runBuild: async () => {
+				events.push('build');
+				throw new Error('boom');
+			},
+		};
+
+		await assert.rejects(withBuildLock(scope, deps), /boom/);
+
+		assert.deepStrictEqual(events, ['put:building', 'build', 'put:failure'], 'claims, fails, then records the failure');
+	});
+
+	it('re-stamps the claim on a heartbeat while a long build runs, then records success', async (t) => {
+		// Mock only setInterval — the heartbeat's timer. This path never reaches the waiter's sleep().
+		t.mock.timers.enable({ apis: ['setInterval'] });
+		const events: string[] = [];
+		const table = makeTable(events); // get → undefined (no existing record)
+		useTable(t, table);
+
+		let finishBuild!: () => void;
+		const deps: BuildLockDeps = {
+			getBuildId: () => null,
+			runBuild: () => new Promise<string>((resolve) => (finishBuild = () => resolve('hb-id'))),
+		};
+
+		const done = withBuildLock(scope, deps);
+		await tick(); // claim the build, start the build, and arm the heartbeat
+		assert.deepStrictEqual(events, ['put:building'], 'claims once up front');
+
+		// Simulate a build long enough to cross two heartbeat intervals (30s each).
+		t.mock.timers.tick(30_000);
+		await tick();
+		t.mock.timers.tick(30_000);
+		await tick();
+		assert.deepStrictEqual(
+			events,
+			['put:building', 'put:building', 'put:building'],
+			'each heartbeat re-stamps the building claim so a live build never looks abandoned'
+		);
+
+		finishBuild();
+		await done;
+		assert.strictEqual(events.at(-1), 'put:success', 'the terminal record is the last write — no heartbeat re-stamp after it');
+		assert.strictEqual(table.puts.at(-1)?.value.buildId, 'hb-id');
+	});
+});

src/buildLock.ts

@@ -0,0 +1,150 @@
+import { databases, type Scope } from 'harper';
+import { setTimeout as sleep } from 'node:timers/promises';
+
+// Coordinates production builds across Harper worker threads (and processes sharing a database). Harper
+// runs `handleApplication` in every worker, so without coordination each worker runs its own `next build`
+// concurrently into the same `.next` directory. `next build` clears and rewrites `.next` at the start, so
+// one worker deletes `.next/BUILD_ID` while another is reading it → `ENOENT` → the build worker exits and
+// nothing serves (issue #52). To serialize builds, a worker claims the shared build-info record with
+// status `building` before compiling; sibling workers observe the fresh claim and wait for it to finish
+// rather than building in parallel. This mirrors the `@harperfast/vite` `buildLock` pattern, plus a
+// heartbeat: the builder re-stamps its claim on an interval so an in-progress build of any length stays
+// fresh, while a crashed builder's claim still goes stale (and is reclaimed) within STALE_CLAIM_MS.
+
+const DATABASE = 'harperfast_nextjs';
+const TABLE = 'nextjs_build_info';
+
+// A completed build (`success`/`failure`) is reused for this long; after it, a restart triggers a rebuild.
+const FRESH_BUILD_MS = 5000;
+// While building, the claiming worker re-stamps its `building` record on this interval (a heartbeat) so a
+// live build never looks abandoned, no matter how long `next build` takes.
+const HEARTBEAT_MS = 30 * 1000;
+// A `building` claim is treated as abandoned only once it has gone this long without a heartbeat — i.e. the
+// worker holding it crashed (the issue notes `process.exit(1)` in a build worker is swallowed to keep
+// Harper alive). This bounds crash detection; it does NOT need to exceed the build duration, because the
+// heartbeat keeps a live claim fresh. Must be comfortably larger than HEARTBEAT_MS to tolerate the event
+// loop being busy during a build.
+const STALE_CLAIM_MS = 2 * 60 * 1000;
+// How often a waiting worker re-checks the claim.
+const CLAIM_POLL_MS = 150;
+
+/** A build-info record as Harper returns it from `table.get`. */
+interface BuildInfoRecord {
+	buildId: string | null;
+	status: string;
+	getUpdatedTime(): number;
+}
+
+/** The subset of the Harper build-info table the lock uses. */
+interface BuildInfoTable {
+	get(key: string): Promise<BuildInfoRecord | undefined>;
+	put(key: string, value: { buildId: string | null; status: string }): Promise<unknown> | unknown;
+}
+
+/** The build-info table, or `undefined` when running outside Harper (e.g. unit tests). */
+function buildInfoTable(): BuildInfoTable | undefined {
+	return (databases as unknown as Record<string, Record<string, BuildInfoTable>>)?.[DATABASE]?.[TABLE];
+}
+
+/**
+ * Re-stamp the `building` claim on an interval so a live (possibly long) build never looks abandoned to
+ * waiting workers. Returns a stop function that halts the heartbeat and awaits any in-flight re-stamp, so
+ * the caller can then write the terminal (`success`/`failure`) record as the last word on the claim.
+ */
+function startClaimHeartbeat(table: BuildInfoTable, key: string, scope: Scope): () => Promise<void> {
+	let stopped = false;
+	let inFlight: Promise<unknown> = Promise.resolve();
+	const timer = setInterval(() => {
+		if (stopped) return;
+		inFlight = Promise.resolve(table.put(key, { buildId: null, status: 'building' })).catch((error) => {
+			scope.logger.debug?.(`Heartbeat for ${key} build claim failed`, error);
+		});
+	}, HEARTBEAT_MS);
+	// Don't let the heartbeat timer keep the process alive on its own.
+	timer.unref?.();
+	return async () => {
+		stopped = true;
+		clearInterval(timer);
+		await inFlight;
+	};
+}
+
+export interface BuildLockDeps {
+	/** Reads the current on-disk `.next/BUILD_ID` (or `null` if absent). Validates a fresh `success` record. */
+	getBuildId: () => string | null;
+	/** Runs `next build` and resolves with the resulting BUILD_ID. Rejects if the build fails. */
+	runBuild: () => Promise<string>;
+}
+
+/**
+ * Run `deps.runBuild` once across the workers sharing this app's build-info record. Resolves once a usable
+ * build exists — whether this worker built it, reused a fresh one, or waited for a sibling — so the caller
+ * can proceed to serve. Rejects only if the build this worker itself ran fails.
+ *
+ * Outside Harper (no coordination table), it simply runs the build.
+ */
+export async function withBuildLock(scope: Scope, deps: BuildLockDeps): Promise<void> {
+	const table = buildInfoTable();
+	if (!table) {
+		await deps.runBuild();
+		return;
+	}
+
+	const key = scope.appName;
+
+	// Wait for any build already in progress on a sibling worker, then decide whether we still need to
+	// build. Looping means that once a claim clears we re-read the record it produced (fresh success or
+	// failure) instead of racing straight into our own build.
+	while (true) {
+		const buildInfo = await table.get(key);
+		const age = buildInfo ? Date.now() - buildInfo.getUpdatedTime() : Infinity;
+
+		// Another worker is actively building, and its heartbeat is fresh. Poll-wait however long the build
+		// takes; we only stop waiting once the claim clears (success/failure) or goes stale (builder died).
+		if (buildInfo?.status === 'building' && age < STALE_CLAIM_MS) {
+			scope.logger.debug?.(`Another worker is building ${key}; waiting`);
+			await sleep(CLAIM_POLL_MS);
+			continue;
+		}
+
+		if (age < FRESH_BUILD_MS) {
+			// A sibling just finished a failed build — return immediately to avoid building (and failing)
+			// on every thread.
+			if (buildInfo?.status === 'failure') {
+				scope.logger.debug?.(`Failure build of ${key} detected`);
+				return;
+			}
+
+			// A sibling just finished a successful build — reuse it if the on-disk BUILD_ID matches.
+			if (buildInfo?.status === 'success' && deps.getBuildId() === buildInfo.buildId) {
+				scope.logger.debug?.(`Fresh build of ${key} (id: ${buildInfo.buildId}) detected`);
+				return;
+			}
+		}
+
+		// No fresh build and nobody is building — it's our turn.
+		break;
+	}
+
+	// Claim the build so sibling workers wait (above) instead of compiling into the same `.next`
+	// concurrently. `getUpdatedTime()` on this record is what the checks above compare against, and the
+	// heartbeat keeps it fresh for the duration of the build.
+	await table.put(key, { buildId: null, status: 'building' });
+
+	scope.logger.debug?.(`Building Next.js application at ${scope.directory}`);
+
+	const stopHeartbeat = startClaimHeartbeat(table, key, scope);
+	try {
+		const buildId = await deps.runBuild();
+		// Stop the heartbeat before writing the terminal record so no stray re-stamp can revert it to
+		// `building` afterward.
+		await stopHeartbeat();
+		await table.put(key, { buildId, status: 'success' });
+		scope.logger.debug?.(`Successful build for ${key} (id ${buildId})`);
+	} catch (error) {
+		await stopHeartbeat();
+		await table.put(key, { buildId: null, status: 'failure' });
+		scope.logger.debug?.(`Error building ${key}`);
+		throw error;
+	}
+}