diff --git a/DEV_SETUP.md b/DEV_SETUP.md index 93c6740f95..63daacb53f 100644 --- a/DEV_SETUP.md +++ b/DEV_SETUP.md @@ -28,13 +28,36 @@ npm run build --workspace=@bastani/atomic-natives The natives build is a required one-time step (and again after pulling changes to `crates/` or `packages/natives/`). `npm ci --ignore-scripts` deliberately skips lifecycle scripts, and the workspace natives package has no install hook anyway — -only published releases ship prebuilt binaries. Without the compiled -`packages/natives/native/*.node`, the CLI still runs but silently degrades: -`pty:true` bash falls back to pipes, native grep/find/tree-sitter block -resolution fall back to slower JS paths, and several `packages/coding-agent` -tests fail (`bash-pty-native`, `search-tool-*`, `hashline-tools`). CI builds the +only published releases ship prebuilt binaries. + +**`vitest` now builds it for you when it is missing.** The `globalSetup` in +`test/global-setup-natives.ts` checks for `packages/natives/native/*.node` +before collecting any file: present and current, it returns after one stat and +costs nothing; missing, it prints what it is doing and runs the build; older +than the Rust sources, it warns and runs anyway, because `git checkout` rewrites +mtimes and blocking a suite on that evidence is worse than one build too few. +You still need a Rust toolchain — without cargo it fails with that prerequisite +rather than a compile error. + +Running the CLI is not covered by that, so build it yourself before using the +agent from a fresh checkout. Without the compiled binding, `pty:true` bash falls +back to pipes and native grep/find/tree-sitter block resolution fall back to +slower JS paths. + +What is **not** a graceful degradation is the test suites. Since the in-process +subagent runner landed, `packages/subagents` reaches the Rust control plane +through a *static* import, so a missing binding throws while the module graph is +still loading and takes roughly twenty root unit and integration files with it — +not just `bash-pty-native`, `search-tool-*`, and `hashline-tools`. The errors +name whatever imported the extension, such as `workflow-stage-bundled-resources`, +so the failure reads like a regression in an unrelated subsystem. CI builds the module explicitly for the same reason (see `.github/workflows/test.yml`). +Note that the generated napi-rs loader's own miss message suggests removing +`package-lock.json` and `node_modules` and re-running `npm i`. That advice does +not apply here: with `--ignore-scripts`, reinstalling never produces the +binding. `npm run build --workspace=@bastani/atomic-natives` is the fix. + The committed `.npmrc` applies a three-day minimum release age to anything you add with `npm install`, and pins exact versions. `package-lock.json` is the only lockfile. diff --git a/test/global-setup-natives.ts b/test/global-setup-natives.ts new file mode 100644 index 0000000000..21aba54e8c --- /dev/null +++ b/test/global-setup-natives.ts @@ -0,0 +1,183 @@ +import { existsSync, readdirSync, statSync } from "node:fs"; +import { createRequire } from "node:module"; +import { join } from "node:path"; +import { spawnSyncCollect } from "./helpers/runtime.js"; + +/** + * Make `@bastani/atomic-natives` loadable before the suites run, and say so out + * loud when it is not. + * + * `npm ci --ignore-scripts` is the mandated install (AGENTS.md) and the natives + * workspace has no install hook, so a fresh checkout or a new git worktree has + * no compiled binding. Since the in-process subagent runner landed, + * `packages/subagents` reaches the Rust control plane through a *static* + * import, so that is not a graceful degradation: the bundled extension throws + * while the module graph is still loading and takes roughly twenty root unit + * and integration files down with it. + * + * The errors name whatever imported the extension — `workflow-stage-bundled- + * resources`, the in-process runner suites — rather than the missing binding, + * so the failure reads like a regression in an unrelated subsystem. + * + * The generated napi-rs loader makes that worse. Its own miss message says to + * remove `package-lock.json` and `node_modules` and re-run `npm i`. That advice + * is wrong here: under `--ignore-scripts`, reinstalling never produces the + * binding, so a developer who follows it loops. `native/index.js` is generated + * (`@ts-nocheck`) and would lose a hand edit at the next `napi artifacts`, so + * the correct instruction has to live here. + * + * Shape borrowed from `can1357/oh-my-pi`'s `packages/natives/native/ + * loader-state.js`: report what was tried, give the exact command for the + * context you are in, and let a dev tree keep running on a stale binding rather + * than hard-failing while a rebuild is pending. Two deliberate differences — + * this runs at test setup rather than load time, so it can repair the missing + * case instead of only describing it, and staleness is a timestamp comparison + * because our `.node` carries no embedded version sentinel. + */ + +const REPO_ROOT = join(import.meta.dirname, ".."); +const NATIVE_DIR = join(REPO_ROOT, "packages", "natives", "native"); +const NATIVE_ENTRY = join(NATIVE_DIR, "index.js"); +const BUILD_COMMAND = "npm run build --workspace=@bastani/atomic-natives"; +/** Sources whose edit invalidates a compiled binding. */ +const SOURCE_ROOTS = [join(REPO_ROOT, "crates"), join(REPO_ROOT, "packages", "natives", "src")]; + +function note(lines: readonly string[]): void { + process.stderr.write(`\n${lines.join("\n")}\n\n`); +} + +/** + * Whether a binding usable *by this host* is present. + * + * Deliberately a load attempt rather than a filename scan. napi-rs resolves a + * platform-arch-libc triple through roughly seven hundred lines that also cover + * musl detection, Android, and WASI; a scan that accepted any `.node` would skip + * the build when the directory holds only a binding for another platform — a + * real state after copying a native directory between checkouts or unpacking + * release artifacts. Requiring the generated entrypoint asks the exact question + * the suites will ask, so it cannot drift from the loader. + */ +function bindingLoads(): boolean { + if (!existsSync(NATIVE_ENTRY)) return false; + try { + createRequire(import.meta.url)(NATIVE_ENTRY); + return true; + } catch { + return false; + } +} + +/** Newest mtime across Rust sources, or 0 when none can be read. */ +function newestSourceMtime(): number { + let newest = 0; + const visit = (directory: string): void => { + let entries: string[]; + try { + entries = readdirSync(directory); + } catch { + return; + } + for (const entry of entries) { + const path = join(directory, entry); + let stats: ReturnType; + try { + stats = statSync(path); + } catch { + continue; + } + if (stats.isDirectory()) { + if (entry !== "target" && entry !== "node_modules") visit(path); + continue; + } + if (entry.endsWith(".rs") || entry === "Cargo.toml") newest = Math.max(newest, stats.mtimeMs); + } + }; + for (const root of SOURCE_ROOTS) visit(root); + return newest; +} + +/** Compiled bindings on disk, for staleness reporting only. */ +function bindingFiles(): string[] { + try { + return readdirSync(NATIVE_DIR) + .filter((entry) => entry.endsWith(".node")) + .map((entry) => join(NATIVE_DIR, entry)); + } catch { + return []; + } +} + +export default function setup(): void { + if (bindingLoads()) { + // Stale is a warning, never a rebuild and never a failure. A timestamp is + // weaker evidence than a version sentinel -- `git checkout` rewrites + // mtimes, so this can cry wolf after a branch switch -- and blocking a + // suite on weak evidence is worse than running one build too few. + const newestSource = newestSourceMtime(); + const stale = bindingFiles().filter((binding) => { + try { + return statSync(binding).mtimeMs < newestSource; + } catch { + return false; + } + }); + if (stale.length > 0) { + note([ + "WARNING: the compiled native binding is older than the Rust sources.", + ...stale.map((binding) => ` stale: ${binding}`), + "", + "Tests will run against the binding already on disk. If results look", + "impossible, rebuild first:", + ` ${BUILD_COMMAND}`, + ]); + } + return; + } + + const present = bindingFiles(); + note([ + "@bastani/atomic-natives has no binding this host can load, so it is being built now.", + ` looked in: ${NATIVE_DIR}`, + ...(present.length > 0 + ? [" present but not loadable here:", ...present.map((binding) => ` ${binding}`)] + : [" no .node files found"]), + "", + "Without it packages/subagents fails at import and takes roughly twenty", + "unrelated unit and integration files down with it, naming the importer", + "rather than the binding.", + "", + "This happens once per checkout or git worktree, and takes about 35 seconds.", + ` ${BUILD_COMMAND}`, + ]); + + const result = spawnSyncCollect(["npm", "run", "build", "--workspace=@bastani/atomic-natives"], { cwd: REPO_ROOT }); + + if (!result.success) { + throw new Error( + [ + `Could not build @bastani/atomic-natives (exit ${result.exitCode}).`, + "", + `Looked for a loadable binding in: ${NATIVE_DIR}`, + ...(present.length > 0 ? ["Present but not loadable on this host:", ...present.map((b) => ` ${b}`)] : []), + "", + result.stderr.toString().trim().split("\n").slice(-12).join("\n"), + "", + "The suites cannot run without it: packages/subagents imports the Rust", + "control plane statically, so the bundled extension fails at import.", + "", + "This build needs a stable Rust toolchain with cargo (https://rustup.rs).", + "The generated napi-rs loader will instead suggest reinstalling with npm;", + "that cannot work here, because the mandated `npm ci --ignore-scripts`", + "never runs the build.", + "", + `Once cargo is available, run: ${BUILD_COMMAND}`, + ].join("\n"), + ); + } + + if (!bindingLoads()) { + throw new Error( + `${BUILD_COMMAND} reported success but produced no binding this host can load in ${NATIVE_DIR}. Run it directly to see why.`, + ); + } +} diff --git a/vitest.config.ts b/vitest.config.ts index ce65557ea2..c994bae9a7 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -11,6 +11,19 @@ export { TEST_TIMEOUT_MS }; */ const setupFiles = ["./test/setup-workflow-durability.ts"]; +/** + * Runs once per project, before any file is collected. It builds + * `@bastani/atomic-natives` only when no compiled binding exists, because a + * missing binding no longer degrades gracefully: `packages/subagents` imports + * the Rust control plane statically, so the bundled extension throws during + * module loading and takes roughly twenty unrelated files down with it under + * errors that name the importer rather than the binding. + * + * On the happy path this is a single `existsSync`, so CI — which builds the + * binding in an explicit step first — and any warm worktree pay nothing. + */ +const globalSetup = ["./test/global-setup-natives.ts"]; + const project = (name: string, directory: string) => ({ resolve: { alias: sharedAliases }, test: { @@ -21,6 +34,7 @@ const project = (name: string, directory: string) => ({ include: [`${directory}/**/*.test.ts`], exclude: ["**/node_modules/**"], setupFiles, + globalSetup, testTimeout: TEST_TIMEOUT_MS, hookTimeout: TEST_TIMEOUT_MS, },