From 3e0fdc08d5538682d74bb5762eda0f3ee3bd36bd Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Tue, 21 Jul 2026 17:08:34 -0400 Subject: [PATCH 1/2] fix: serialize next build across worker threads (#52) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Harper runs `handleApplication` in every worker thread, but `build()` only did a post-hoc dedup check — it never claimed the build before compiling. On a fresh deploy/restart there is no (or a stale) build-info record, so all workers fell through and ran `next build` concurrently into the same `.next`. `next build` clears and rewrites `.next` at the start, so one worker deletes `.next/BUILD_ID` while another reads it -> ENOENT -> the build worker exits and nothing serves (every route 404s on multi-thread, non-prebuilt deploys). Port the claim-then-wait lock from `@harperfast/vite`: a worker claims the shared `nextjs_build_info` record with status `building` before compiling; sibling workers observe the fresh claim and poll-wait, then skip, so only one worker ever builds. A stale-claim timeout reclaims the build if the holder crashed. The existing fresh-build reuse / failure-skip behavior is preserved. - Extract coordination into a testable `src/buildLock.ts` (`withBuildLock`), mirroring vite; `build()` now supplies a `runBuild` closure with the Next-version-specific `next build` call. - Add `src/buildLock.test.ts` (node --test) covering claim/wait/stale/dedup /failure paths, and a `test` script. - Keep compiled test code out of the published package (`files` allowlist + `.npmignore`), matching vite. - Document the `building` status in schema.graphql. Co-Authored-By: Claude Opus 4.8 --- .npmignore | 9 +++ package.json | 4 +- schema.graphql | 5 ++ src/buildLock.test.ts | 167 ++++++++++++++++++++++++++++++++++++++++++ src/buildLock.ts | 119 ++++++++++++++++++++++++++++++ src/plugin.ts | 49 ++++--------- 6 files changed, 318 insertions(+), 35 deletions(-) create mode 100644 .npmignore create mode 100644 src/buildLock.test.ts create mode 100644 src/buildLock.ts diff --git a/.npmignore b/.npmignore new file mode 100644 index 0000000..a5e72d4 --- /dev/null +++ b/.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.* diff --git a/package.json b/package.json index e15d0df..f199c0e 100644 --- a/package.json +++ b/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": { diff --git a/schema.graphql b/schema.graphql index cc47e4e..647bea2 100644 --- a/schema.graphql +++ b/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 diff --git a/src/buildLock.test.ts b/src/buildLock.test.ts new file mode 100644 index 0000000..6cff82f --- /dev/null +++ b/src/buildLock.test.ts @@ -0,0 +1,167 @@ +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; + + /** 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 stale "building" record as abandoned and builds anyway', async (t) => { + const events: string[] = []; + // Older than the 5-minute stale threshold → the holder is assumed crashed. + const table = makeTable(events, [building(6 * 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'); + }); +}); diff --git a/src/buildLock.ts b/src/buildLock.ts new file mode 100644 index 0000000..3d59e49 --- /dev/null +++ b/src/buildLock.ts @@ -0,0 +1,119 @@ +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. + +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; +// A `building` claim older than this is treated as abandoned — e.g. the worker holding it crashed (the +// issue notes `process.exit(1)` in a build worker is swallowed to keep Harper alive). Must exceed the +// longest expected `next build`. +const STALE_CLAIM_MS = 5 * 60 * 1000; +// How often a waiting worker re-checks the claim, and how long it waits before building anyway. +const CLAIM_POLL_MS = 150; +const CLAIM_WAIT_TIMEOUT_MS = 5 * 60 * 1000; + +/** 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; + put(key: string, value: { buildId: string | null; status: string }): Promise | 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>)?.[DATABASE]?.[TABLE]; +} + +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; +} + +/** + * 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 { + 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. + const waitStart = Date.now(); + while (true) { + const buildInfo = await table.get(key); + const age = buildInfo ? Date.now() - buildInfo.getUpdatedTime() : Infinity; + + // Another worker is actively building. Poll-wait for it to finish, then re-evaluate. + if (buildInfo?.status === 'building' && age < STALE_CLAIM_MS) { + if (Date.now() - waitStart > CLAIM_WAIT_TIMEOUT_MS) { + scope.logger.warn?.(`Timed out waiting for another worker to build ${key}; building anyway`); + break; + } + 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. + await table.put(key, { buildId: null, status: 'building' }); + + scope.logger.debug?.(`Building Next.js application at ${scope.directory}`); + + try { + const buildId = await deps.runBuild(); + await table.put(key, { buildId, status: 'success' }); + scope.logger.debug?.(`Successful build for ${key} (id ${buildId})`); + } catch (error) { + await table.put(key, { buildId: null, status: 'failure' }); + scope.logger.debug?.(`Error building ${key}`); + throw error; + } +} diff --git a/src/plugin.ts b/src/plugin.ts index fad6802..bac3043 100644 --- a/src/plugin.ts +++ b/src/plugin.ts @@ -5,6 +5,8 @@ import { parse as urlParse } from 'node:url'; import { join } from 'node:path'; import { existsSync, readFileSync } from 'node:fs'; +import { withBuildLock } from './buildLock.js'; + import type NextModule14 from 'next-14'; import type NextBuildModule14 from 'next-14/dist/cli/next-build.d.ts'; @@ -265,32 +267,17 @@ export async function handleApplication(scope: Scope) { } async function build(scope: Scope, config: NextPluginConfig, next: NextPackage) { - const buildInfo = await databases.harperfast_nextjs.nextjs_build_info.get(scope.appName); - - if (buildInfo && Date.now() - buildInfo.getUpdatedTime() < 5000) { - // If the build info record is marked as "failure" just return immediately - // avoids building (and failing) on every thread - if (buildInfo.status === 'failure') { - scope.logger.debug?.(`Failure build of ${scope.appName} detected`); - return; - } - - // If the build info record is marked as "success" - if (buildInfo.status === 'success') { - // then validate the BUILD_ID value - const buildId = getBuildId(scope); - if (buildId === buildInfo.buildId) { - scope.logger.debug?.(`Fresh build of ${scope.appName} (id: ${buildInfo.buildId}) detected`); - // fresh build - return; - } - } - } - - // Otherwise we have a stale build (or no build info at all) and now we can proceed with building - - scope.logger.debug?.(`Building Next.js application at ${scope.directory}`); + // Serialize builds across Harper worker threads (see buildLock.ts). `withBuildLock` decides whether + // this worker builds, reuses a fresh build, or waits for a sibling; it only invokes `runBuild` (and + // only on the one worker that wins the claim) when a build is actually needed. + await withBuildLock(scope, { + getBuildId: () => getBuildId(scope), + runBuild: () => runNextBuild(scope, config, next), + }); +} +/** Runs `next build` for the detected Next.js version and returns the resulting BUILD_ID. */ +async function runNextBuild(scope: Scope, config: NextPluginConfig, next: NextPackage): Promise { // --expose-internals is set in Harper's worker execArgv but is not allowed in NODE_OPTIONS. // Next.js reads process.execArgv to forward flags to its own child workers via NODE_OPTIONS, // which causes Node to reject the build worker. Strip it before building and restore after. @@ -335,16 +322,10 @@ async function build(scope: Scope, config: NextPluginConfig, next: NextPackage) break; } + // Read BUILD_ID directly (not via getBuildId) so a build that completed without one throws and is + // recorded as a failure. Trim it so it matches getBuildId() in the lock's fresh-build check. const buildIdPath = join(scope.directory, '.next', 'BUILD_ID'); - const buildId = readFileSync(buildIdPath, 'utf-8'); - // Update the build info record - await databases.harperfast_nextjs.nextjs_build_info.put(scope.appName, { buildId, status: 'success' }); - scope.logger.debug?.(`Successful build for ${scope.appName} (id ${buildId})`); - return; - } catch (error) { - await databases.harperfast_nextjs.nextjs_build_info.put(scope.appName, { buildId: null, status: 'failure' }); - scope.logger.debug?.(`Error building ${scope.appName}`); - throw error; + return readFileSync(buildIdPath, 'utf-8').trim(); } finally { if (exposeInternalsIdx !== -1) process.execArgv.splice(exposeInternalsIdx, 0, '--expose-internals'); } From ba39306ffeea67b87d7a0661ac4482dd06827e41 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Tue, 21 Jul 2026 17:22:51 -0400 Subject: [PATCH 2/2] fix: heartbeat the build claim so long builds don't defeat the lock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review feedback on the fixed build-lock timeout: a `next build` that runs longer than the stale-claim window would let waiting workers give up and build concurrently, reintroducing the ENOENT race. Instead of relying on a fixed timeout large enough for the longest build, the claiming worker now re-stamps its `building` record every 30s while building. A live build stays fresh no matter how long it takes; a crashed builder's claim still goes stale within STALE_CLAIM_MS (now 2m, purely a crash-detection bound) and is reclaimed. Waiting workers no longer have an absolute wait timeout — they wait as long as the claim keeps heartbeating and only take over once it clears or goes stale. The heartbeat is stopped (awaiting any in-flight re-stamp) before the terminal success/failure record is written, so no stray beat can revert it to `building`. Adds a unit test using mock timers. Co-Authored-By: Claude Opus 4.8 --- src/buildLock.test.ts | 43 ++++++++++++++++++++++++++++--- src/buildLock.ts | 59 +++++++++++++++++++++++++++++++++---------- 2 files changed, 85 insertions(+), 17 deletions(-) diff --git a/src/buildLock.test.ts b/src/buildLock.test.ts index 6cff82f..3135388 100644 --- a/src/buildLock.test.ts +++ b/src/buildLock.test.ts @@ -47,6 +47,9 @@ describe('withBuildLock', () => { // 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 { @@ -96,10 +99,10 @@ describe('withBuildLock', () => { assert.deepStrictEqual(table.puts, [], 'never writes a claim of its own — the sibling produced the output'); }); - it('treats a stale "building" record as abandoned and builds anyway', async (t) => { + it('treats a "building" record with no recent heartbeat as abandoned and builds anyway', async (t) => { const events: string[] = []; - // Older than the 5-minute stale threshold → the holder is assumed crashed. - const table = makeTable(events, [building(6 * 60 * 1000)]); + // 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)); @@ -164,4 +167,38 @@ describe('withBuildLock', () => { 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((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'); + }); }); diff --git a/src/buildLock.ts b/src/buildLock.ts index 3d59e49..0671993 100644 --- a/src/buildLock.ts +++ b/src/buildLock.ts @@ -7,20 +7,26 @@ import { setTimeout as sleep } from 'node:timers/promises'; // 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. +// 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; -// A `building` claim older than this is treated as abandoned — e.g. the worker holding it crashed (the -// issue notes `process.exit(1)` in a build worker is swallowed to keep Harper alive). Must exceed the -// longest expected `next build`. -const STALE_CLAIM_MS = 5 * 60 * 1000; -// How often a waiting worker re-checks the claim, and how long it waits before building anyway. +// 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; -const CLAIM_WAIT_TIMEOUT_MS = 5 * 60 * 1000; /** A build-info record as Harper returns it from `table.get`. */ interface BuildInfoRecord { @@ -40,6 +46,29 @@ function buildInfoTable(): BuildInfoTable | undefined { return (databases as unknown as Record>)?.[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 { + let stopped = false; + let inFlight: Promise = 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; @@ -66,17 +95,13 @@ export async function withBuildLock(scope: Scope, deps: BuildLockDeps): Promise< // 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. - const waitStart = Date.now(); while (true) { const buildInfo = await table.get(key); const age = buildInfo ? Date.now() - buildInfo.getUpdatedTime() : Infinity; - // Another worker is actively building. Poll-wait for it to finish, then re-evaluate. + // 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) { - if (Date.now() - waitStart > CLAIM_WAIT_TIMEOUT_MS) { - scope.logger.warn?.(`Timed out waiting for another worker to build ${key}; building anyway`); - break; - } scope.logger.debug?.(`Another worker is building ${key}; waiting`); await sleep(CLAIM_POLL_MS); continue; @@ -102,16 +127,22 @@ export async function withBuildLock(scope: Scope, deps: BuildLockDeps): Promise< } // 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. + // 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;