From da572305dc4d5e70560f2a79465bb4b3ba88adcc Mon Sep 17 00:00:00 2001 From: Sawyer Hood Date: Thu, 27 Aug 2026 18:30:16 +0000 Subject: [PATCH 1/2] Prevent recurring native module ABI failures --- packages/bb-app/src/launcher.ts | 7 +++ .../test/server-health-identity.test.ts | 29 ++++++++++++ .../test/ensure-native-modules.test.mjs | 47 ++++++++++++++++++- scripts/ensure-native-modules.mjs | 20 ++++---- scripts/start-bb.mjs | 3 +- 5 files changed, 92 insertions(+), 14 deletions(-) diff --git a/packages/bb-app/src/launcher.ts b/packages/bb-app/src/launcher.ts index 500feb1a3a..757dbc776f 100644 --- a/packages/bb-app/src/launcher.ts +++ b/packages/bb-app/src/launcher.ts @@ -227,6 +227,7 @@ interface ResolveWorktreeRuntimePolicyArgs { } interface RunBbAppOptions { + beforeServerStart?: () => Promise | void; worktreePolicy: WorktreeRuntimePolicy | null; } @@ -395,6 +396,7 @@ interface SpawnNamedManagedProcessArgs { } interface StartFullStackServerProcessArgs { + beforeStart?: () => Promise | void; context: BbAppStartContext; env: NodeJS.ProcessEnv; outputBuffer: OutputBuffer; @@ -3076,6 +3078,8 @@ function logManagedProcessStartupFailureContext( export async function startFullStackServerProcess( args: StartFullStackServerProcessArgs, ): Promise { + await args.beforeStart?.(); + // Fresh per spawn: the probe must match this child, not any earlier one. const launchId = randomUUID(); const serverRun = spawnNamedManagedProcess({ @@ -3508,6 +3512,9 @@ export async function runBbApp( }; const startServer = (): Promise => startFullStackServerProcess({ + ...(options.beforeServerStart === undefined + ? {} + : { beforeStart: options.beforeServerStart }), context, env: serverEnv, outputBuffer, diff --git a/packages/bb-app/test/server-health-identity.test.ts b/packages/bb-app/test/server-health-identity.test.ts index db6ae1fef9..a12501174f 100644 --- a/packages/bb-app/test/server-health-identity.test.ts +++ b/packages/bb-app/test/server-health-identity.test.ts @@ -239,6 +239,35 @@ describe("startFullStackServerProcess", () => { } }); + it("runs the server preflight before it starts a child", async () => { + const serverPort = await reserveFreePort(); + const context = createStartContext({ + serverEntry: writeFakeServerEntry(), + serverPort, + }); + const processes: ManagedFullStackProcesses = { + daemonRun: null, + serverRun: null, + }; + const preflightError = new Error("native module ABI mismatch"); + + await expect( + startFullStackServerProcess({ + beforeStart: () => { + throw preflightError; + }, + context, + env: { + BB_SERVER_PORT: String(context.serverPort), + PATH: process.env.PATH, + }, + outputBuffer: silentOutputBuffer, + processes, + }), + ).rejects.toBe(preflightError); + expect(processes.serverRun).toBeNull(); + }); + it("fails startup instead of adopting a server that already owns the port", async () => { const foreign = await listen((_request, response) => { answerHealth(response, { ok: true }); diff --git a/packages/scripts/test/ensure-native-modules.test.mjs b/packages/scripts/test/ensure-native-modules.test.mjs index 86d322f7ee..db6a647a52 100644 --- a/packages/scripts/test/ensure-native-modules.test.mjs +++ b/packages/scripts/test/ensure-native-modules.test.mjs @@ -136,7 +136,7 @@ describe("ensure-native-modules", () => { expect(fake.state.constructorCalls).toBe(2); }); - it("detaches a hardlinked native binary before prebuilt repair", () => { + it("detaches a hardlinked native binary before verification", () => { const tempRoot = mkdtempSync(join(tmpdir(), "bb-native-repair-")); try { const packageJsonPath = join(tempRoot, "better-sqlite3", "package.json"); @@ -180,7 +180,50 @@ describe("ensure-native-modules", () => { statSync(otherCheckoutBinaryPath).ino, ); expect(options.log).toHaveBeenCalledWith( - "[ensure-native-modules] Detached hardlinked better-sqlite3 binary before repair", + "[ensure-native-modules] Detached hardlinked better-sqlite3 binary before verification", + ); + } finally { + rmSync(tempRoot, { recursive: true, force: true }); + } + }); + + it("detaches a matching native binary without a repair", () => { + const tempRoot = mkdtempSync(join(tmpdir(), "bb-native-verify-")); + try { + const packageJsonPath = join(tempRoot, "better-sqlite3", "package.json"); + const binaryPath = join( + dirname(packageJsonPath), + "build", + "Release", + "better_sqlite3.node", + ); + const otherCheckoutBinaryPath = join(tempRoot, "other-checkout.node"); + mkdirSync(dirname(binaryPath), { recursive: true }); + writeFileSync(packageJsonPath, "{}"); + writeFileSync(otherCheckoutBinaryPath, "abi-137"); + linkSync(otherCheckoutBinaryPath, binaryPath); + + const fake = createBetterSqliteRequire(null, packageJsonPath); + const execFileSync = vi.fn(); + const options = createEnsureOptions(fake.requireModule, execFileSync); + options.modules = [ + { + name: "better-sqlite3", + resolveFrom: "packages/db/package.json", + binaryPath: "build/Release/better_sqlite3.node", + }, + ]; + + expect(() => ensureNativeModules(options)).not.toThrow(); + + expect(statSync(binaryPath).ino).not.toBe( + statSync(otherCheckoutBinaryPath).ino, + ); + expect(readFileSync(binaryPath, "utf8")).toBe("abi-137"); + expect(readFileSync(otherCheckoutBinaryPath, "utf8")).toBe("abi-137"); + expect(execFileSync).not.toHaveBeenCalled(); + expect(options.log).toHaveBeenCalledWith( + "[ensure-native-modules] Detached hardlinked better-sqlite3 binary before verification", ); } finally { rmSync(tempRoot, { recursive: true, force: true }); diff --git a/scripts/ensure-native-modules.mjs b/scripts/ensure-native-modules.mjs index 95c2d7a491..24ea1cee64 100644 --- a/scripts/ensure-native-modules.mjs +++ b/scripts/ensure-native-modules.mjs @@ -142,23 +142,23 @@ export function ensureNativeModules({ } = {}) { for (const { name, resolveFrom, binaryPath } of modules) { const requireModule = createRequireImpl(resolve(repoRoot, resolveFrom)); + const pkgJsonPath = requireModule.resolve(`${name}/package.json`); + const pkgDir = dirname(pkgJsonPath); + if ( + binaryPath !== undefined && + detachHardlinkedBinary(resolve(pkgDir, binaryPath)) + ) { + log( + `[ensure-native-modules] Detached hardlinked ${name} binary before verification`, + ); + } try { verifyNativeModule(name, requireModule); } catch (err) { const message = formatThrownValue(err); if (!shouldRebuildNativeModule(message)) throw err; - const pkgJsonPath = requireModule.resolve(`${name}/package.json`); - const pkgDir = dirname(pkgJsonPath); const pkgRequire = createRequireImpl(pkgJsonPath); - if ( - binaryPath !== undefined && - detachHardlinkedBinary(resolve(pkgDir, binaryPath)) - ) { - log( - `[ensure-native-modules] Detached hardlinked ${name} binary before repair`, - ); - } log( `[ensure-native-modules] Installing prebuilt ${name} for Node ${process.versions.node} (ABI ${process.versions.modules})`, ); diff --git a/scripts/start-bb.mjs b/scripts/start-bb.mjs index ea24ad0821..11c26ae340 100644 --- a/scripts/start-bb.mjs +++ b/scripts/start-bb.mjs @@ -125,11 +125,10 @@ export async function main(args = process.argv.slice(2)) { const parsedArgs = parseStartBbArgs(args); await buildRuntimeArtifacts(); await buildBundledPlugins(); - ensureNativeModules({ repoRoot }); - const { resolveWorktreeRuntimePolicy, runBbApp } = await import("../packages/bb-app/src/launcher.ts"); await runBbApp(parsedArgs.cliArgs, { + beforeServerStart: () => ensureNativeModules({ repoRoot }), worktreePolicy: parsedArgs.useWorktreeRuntimePolicy ? resolveWorktreeRuntimePolicy({ env: process.env, From a6a1d6940ab4f5c9db97ca6b74bb6b46195ab485 Mon Sep 17 00:00:00 2001 From: Sawyer Hood Date: Thu, 27 Aug 2026 19:51:05 +0000 Subject: [PATCH 2/2] Fix native preflight restart lifecycle --- packages/scripts/test/start-bb.test.mjs | 172 ++++++++++++++++++------ scripts/start-bb.mjs | 28 +++- 2 files changed, 158 insertions(+), 42 deletions(-) diff --git a/packages/scripts/test/start-bb.test.mjs b/packages/scripts/test/start-bb.test.mjs index b9aea48344..108757fa24 100644 --- a/packages/scripts/test/start-bb.test.mjs +++ b/packages/scripts/test/start-bb.test.mjs @@ -1,9 +1,14 @@ import { spawn } from "node:child_process"; import { once } from "node:events"; -import { dirname, resolve } from "node:path"; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join, resolve } from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; import { afterEach, describe, expect, it } from "vitest"; -import { parseStartBbArgs } from "../../../scripts/start-bb.mjs"; +import { + parseStartBbArgs, + runNativeModulePreflight, +} from "../../../scripts/start-bb.mjs"; const testDir = dirname(fileURLToPath(import.meta.url)); const repoRoot = resolve(testDir, "..", "..", ".."); @@ -11,6 +16,7 @@ const startBbUrl = pathToFileURL( resolve(repoRoot, "scripts/start-bb.mjs"), ).href; const spawnedPids = []; +const scratchDirs = []; function isAlive(pid) { try { @@ -60,12 +66,61 @@ async function waitForExit(child, timeoutMs) { } } +async function expectSignalStopsProcessTree({ + errorLabel, + expectedPidCount, + fixtureSource, +}) { + const parent = spawn( + process.execPath, + [ + "--conditions=source", + "--import", + "tsx", + "--input-type=module", + "--eval", + fixtureSource, + ], + { + cwd: repoRoot, + env: process.env, + stdio: ["ignore", "pipe", "pipe"], + }, + ); + if (parent.pid === undefined) { + throw new Error(`${errorLabel} did not receive a pid`); + } + spawnedPids.push(parent.pid); + const stderrChunks = []; + parent.stderr.on("data", (chunk) => stderrChunks.push(String(chunk))); + const processPids = (await readFirstLine(parent.stdout)) + .split(" ") + .map(Number); + expect(processPids).toHaveLength(expectedPidCount); + spawnedPids.push(...processPids); + for (const pid of processPids) { + expect(isAlive(pid)).toBe(true); + } + + parent.kill("SIGTERM"); + const [code, signal] = await waitForExit(parent, 10_000); + if (code !== 0 || signal !== null) { + throw new Error( + `Expected clean fixture exit, got code=${String(code)} signal=${String(signal)} stderr=${stderrChunks.join("")}`, + ); + } + await waitFor(() => processPids.every((pid) => !isAlive(pid)), 5_000); +} + afterEach(async () => { for (const pid of spawnedPids.splice(0)) { if (isAlive(pid)) { process.kill(pid, "SIGKILL"); } } + for (const dir of scratchDirs.splice(0)) { + rmSync(dir, { recursive: true, force: true }); + } }); describe("start-bb", () => { @@ -82,6 +137,38 @@ describe("start-bb", () => { }); }); + it("uses a fresh process after a native binary changes", async () => { + const fixtureRoot = mkdtempSync(join(tmpdir(), "bb-native-preflight-")); + scratchDirs.push(fixtureRoot); + const binaryPath = join(fixtureRoot, "native-binary.txt"); + const observationsPath = join(fixtureRoot, "observations.txt"); + const scriptPath = join(fixtureRoot, "verify-native.mjs"); + writeFileSync( + scriptPath, + [ + 'import { appendFileSync, readFileSync } from "node:fs";', + `const binaryPath = ${JSON.stringify(binaryPath)};`, + `const observationsPath = ${JSON.stringify(observationsPath)};`, + 'appendFileSync(observationsPath, `${process.pid}:${readFileSync(binaryPath, "utf8")}\\n`);', + ].join("\n"), + ); + + writeFileSync(binaryPath, "abi-137"); + await runNativeModulePreflight({ cwd: fixtureRoot, scriptPath }); + writeFileSync(binaryPath, "abi-127"); + await runNativeModulePreflight({ cwd: fixtureRoot, scriptPath }); + + const observations = readFileSync(observationsPath, "utf8") + .trim() + .split("\n") + .map((line) => line.split(":")); + expect(observations).toEqual([ + [expect.stringMatching(/^\d+$/u), "abi-137"], + [expect.stringMatching(/^\d+$/u), "abi-127"], + ]); + expect(observations[0][0]).not.toBe(observations[1][0]); + }); + const posixIt = process.platform === "win32" ? it.skip : it; posixIt( "stops the build leader and grandchild after direct SIGTERM", @@ -99,46 +186,51 @@ describe("start-bb", () => { "});", "process.exitCode = result.code ?? (result.signal === null ? 1 : 0);", ].join("\n"); - const parent = spawn( - process.execPath, - [ - "--conditions=source", - "--import", - "tsx", - "--input-type=module", - "--eval", - fixtureSource, - ], - { - cwd: repoRoot, - env: process.env, - stdio: ["ignore", "pipe", "pipe"], - }, - ); - if (parent.pid === undefined) { - throw new Error("start-bb fixture did not receive a pid"); - } - spawnedPids.push(parent.pid); - const stderrChunks = []; - parent.stderr.on("data", (chunk) => stderrChunks.push(String(chunk))); - const [leaderPid, grandchildPid] = (await readFirstLine(parent.stdout)) - .split(" ") - .map(Number); - spawnedPids.push(leaderPid, grandchildPid); - expect(isAlive(leaderPid)).toBe(true); - expect(isAlive(grandchildPid)).toBe(true); + await expectSignalStopsProcessTree({ + errorLabel: "start-bb fixture", + expectedPidCount: 2, + fixtureSource, + }); + }, + 20_000, + ); - parent.kill("SIGTERM"); - const [code, signal] = await waitForExit(parent, 10_000); - if (code !== 0 || signal !== null) { - throw new Error( - `Expected clean fixture exit, got code=${String(code)} signal=${String(signal)} stderr=${stderrChunks.join("")}`, - ); - } - await waitFor( - () => !isAlive(leaderPid) && !isAlive(grandchildPid), - 5_000, + posixIt( + "stops a blocked native repair process group after SIGTERM", + async () => { + const fixtureRoot = mkdtempSync(join(tmpdir(), "bb-native-signal-")); + scratchDirs.push(fixtureRoot); + const scriptPath = join(fixtureRoot, "blocked-repair.mjs"); + writeFileSync( + scriptPath, + [ + 'import { execFileSync } from "node:child_process";', + "execFileSync(", + ' "sh",', + " [", + ' "-c",', + ' "sleep 300 & grandchild=$!; echo \\\"$PPID $$ $grandchild\\\"; wait \\\"$grandchild\\\"",', + " ],", + ' { stdio: "inherit" },', + ");", + ].join("\n"), ); + const fixtureSource = [ + `import { runNativeModulePreflight } from ${JSON.stringify(startBbUrl)};`, + "try {", + " await runNativeModulePreflight({", + ` cwd: ${JSON.stringify(fixtureRoot)},`, + ` scriptPath: ${JSON.stringify(scriptPath)},`, + " });", + "} catch (error) {", + ' if (!(error instanceof Error) || !error.message.includes("stopped by SIGTERM")) throw error;', + "}", + ].join("\n"); + await expectSignalStopsProcessTree({ + errorLabel: "native preflight fixture", + expectedPidCount: 3, + fixtureSource, + }); }, 20_000, ); diff --git a/scripts/start-bb.mjs b/scripts/start-bb.mjs index 11c26ae340..afc2eab928 100644 --- a/scripts/start-bb.mjs +++ b/scripts/start-bb.mjs @@ -7,7 +7,6 @@ import { stopProcessGroupLeaderFirst, supportsProcessGroups, } from "../packages/process-utils/src/index.ts"; -import { ensureNativeModules } from "./ensure-native-modules.mjs"; const scriptDir = dirname(fileURLToPath(import.meta.url)); const repoRoot = resolve(scriptDir, ".."); @@ -111,6 +110,31 @@ async function buildBundledPlugins() { ); } +export async function runNativeModulePreflight({ + cwd = repoRoot, + env = process.env, + nodePath = process.execPath, + scriptPath = resolve(repoRoot, "scripts/ensure-native-modules.mjs"), +} = {}) { + // Each check needs a fresh module cache. The process group also lets the + // launcher stop a blocked download or source build during shutdown. + const result = await runBuildProcess({ + args: [scriptPath], + command: nodePath, + cwd, + env, + }); + if (result.code === 0) { + return; + } + if (result.signal !== null) { + throw new Error(`Native module preflight stopped by ${result.signal}`); + } + throw new Error( + `Native module preflight failed with exit code ${result.code ?? 1}`, + ); +} + export function parseStartBbArgs(args) { if (args[0] !== WORKTREE_RUNTIME_POLICY_ARG) { return { cliArgs: args, useWorktreeRuntimePolicy: false }; @@ -128,7 +152,7 @@ export async function main(args = process.argv.slice(2)) { const { resolveWorktreeRuntimePolicy, runBbApp } = await import("../packages/bb-app/src/launcher.ts"); await runBbApp(parsedArgs.cliArgs, { - beforeServerStart: () => ensureNativeModules({ repoRoot }), + beforeServerStart: runNativeModulePreflight, worktreePolicy: parsedArgs.useWorktreeRuntimePolicy ? resolveWorktreeRuntimePolicy({ env: process.env,