Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 28 additions & 5 deletions DEV_SETUP.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
183 changes: 183 additions & 0 deletions test/global-setup-natives.ts
Original file line number Diff line number Diff line change
@@ -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<typeof statSync>;
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.`,
);
}
}
14 changes: 14 additions & 0 deletions vitest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand All @@ -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,
},
Expand Down
Loading