PR #1201 - AltimateAI/altimate-code

Issue for this PR

Closes #1200
Closes #1202
Closes #1207

Two commits, both in resolve.ts, both blocking the same rig. They are kept in one PR deliberately: a fourth PR would make the stack four deep on someone else's root (see below), and the diffs are small enough to read together.

Stack warning — please read before merging anything in this chain.
fix/driver-load-cwd-prefix (#1201) → fix/native-driver-resolution (#1192) → fix/warehouse-driver-bootstrap (#1122) → main.

That is three deep, and the root (#1122) is @sahrizvi's, not mine. If #1122 is rebased, force-pushed, or squash-merged, both #1192 and this PR need rebasing, and a squash-merge of #1122 will make #1192's diff misread. I did not want to restructure someone else's PR to suit mine, so I am flagging it rather than acting on it.

This change genuinely depends on #1122 — it modifies resolve.ts, which #1122 creates; on main today the code path does not exist. But it is small, and folding it into #1192 would take the stack from three deep to two. I have no attachment to it being its own PR. That call belongs to whoever is driving #1192, or to Anand.

Type of change

Bug fix
New feature
Refactor / code improvement
Documentation

What does this PR do?

The bug. A globally installed CLI (npm install -g, package tree under /usr/lib/node_modules/altimate-code) run from an unrelated working directory could not load any warehouse driver — 8 of 8 trials on a cold VM, against a build that already contained #1122 and #1192:

DuckDB driver found at duckdb but failed to load: ENOENT: no such file or directory,
open '<cwd>/usr/lib/node_modules/altimate-code/node_modules/duckdb/package.json'

The file exists at that path without the <cwd> prefix. Ambient resolution concatenated the working directory onto an already-absolute path. We cannot change that runtime behaviour — but it names the correct location in the error, and that is better evidence of where the package lives than anything we can infer.

searchRootsFromError() now mines the absolute paths an ambient failure quotes, repairs a concatenated working directory, and searches the enclosing node_modules ahead of the inferred roots. repairCwdPrefixedPath() is deliberately conservative — it fires only when the named path is absent, is genuinely prefixed by the working directory, and the de-prefixed remainder exists on disk. That last condition is what keeps a legitimately nested <cwd>/node_modules/… from being mangled, and it means a nonsense path contributes nothing.

The message was the second half of the bug. found at duckdb is loadFailure(driver, specifier, …) echoing the bare specifier, not a path the resolver returned. It reads as "we found it and it would not load" for a package that was never located at all — which is why this was first diagnosed as a load-path bug rather than a search-coverage one. It also routes around the good DriverNotInstalledError text #1192 added, because an ENOENT-shaped ambient error sets ambientBroken and takes the other branch.

Both sites that produced that wording now say what actually happened: the default module resolution failed, here is where we looked, and — when there was one — here is the on-disk copy we also tried. loadFailure is kept for the case where we do have a real path.

I preserved #1122's deliberate choice to lead with the ambient error when both copies are broken ("it is the copy the runtime would normally pick"); only the claim that the specifier is a location is gone.

Second commit — concurrent installs corrupt each other (#1202). The managed driver directory is shared by every CLI process on the machine, and installsInFlight is a module-level Map, so it serialises installs within one process and cannot see any other. Eight CLIs starting together each ran npm install --save over the same tree:

npm install failed (exit 217) … ENOTEMPTY …
  rmdir /root/.local/share/altimate-code/drivers/node_modules/duckdb/…

This is not benchmark-specific — two terminals, a CI matrix on one runner, or an editor integration beside a shell session all hit it, and the visible result is a driver reporting itself broken on a machine where nothing is wrong.

withInstallLock takes a cross-process lock before mutating. mkdir is atomic and fails EEXIST when the directory exists on both POSIX and Windows, which makes a lock directory the portable primitive here; it lives beside the install directory so npm never treats it as stray package content. After acquiring, readiness is re-checked — the peer that held the lock has usually just installed the thing everyone queued for, so most contenders return "already present" instead of running a second npm. Stale locks are broken two ways because neither alone suffices: a dead owner (only meaningful on the same host) and age (the only signal for a lock left by another host sharing a home directory). On timeout the install proceeds unlocked rather than failing — a racing install is recoverable and the readiness check afterwards is authoritative, whereas refusing to install because a peer is slow turns contention into a hard error.

Also: diagnostics on load failures. Two separate investigations have now diagnosed a driver-load failure from the error text alone and got it wrong, because the text named a path without saying what the process's own view of the filesystem was. Load failures now carry cwd, execPath, and an explicit note when a named path does not exist but its de-prefixed form does.

How did you verify your code works?

In the shape it occurs in, not in unit tests alone.

A probe compiled with the production Bun.build options from script/build.ts (external driver packages, autoloadPackageJson: true), laid out as a real global install — <prefix>/lib/node_modules/altimate-code/bin/altimate with a real npm install duckdb into the package (not a hand-built tree; my first synthetic attempt was missing @mapbox/node-pre-gyp and failed for an unrelated reason) — run from an unrelated cwd with no NODE_PATH and no ALTIMATE_BIN_DIR:

--- BASELINE (#1122 + #1192, unmodified) ---
loadDriver: FAILED DuckDB driver found at duckdb but failed to load: ENOENT: no such file
or directory, open '<cwd><abs>/global-prefix/lib/node_modules/altimate-code/node_modules/duckdb/package.json'

--- WITH FIX ---
loadDriver: OK, Database is function

8 fresh processes with the fix: 8/8 load the driver, matching the rig's 8/8 failures.

A green bun test --cwd packages/drivers is not evidence about resolution behaviour, and I did not treat it as such. That run has the drivers package's own node_modules reachable throughout, so a bare import("duckdb") succeeds there no matter what the resolver does — my first baseline attempt passed for exactly that reason and was worthless. Only a compiled binary (bunfs) removes that path and lets the baseline fail honestly. A colleague independently hit the same trap on a 140-pass run, so it is worth stating plainly rather than leaving a reviewer to wonder. Every resolution claim above comes from a compiled binary, never from bun in-tree.

Two things I found while reproducing, which correct the framing of the original report:

A faithful global-install layout on its own does not reproduce it. With the binary inside the package tree, the execPath walk-up finds the driver and everything works — I verified that end to end, including a real query returning a row. The trigger is an ENOENT-shaped ambient error combined with a binary outside the tree. So "global npm install" is not by itself the missing coverage; the ENOENT shape is.
found at duckdb was read as "resolution succeeded, loading re-anchored to cwd". It is not — nothing had been found, and the reported ENOENT is the ambient attempt, which we surface even after trying a different path. The wording caused the misdiagnosis.

Gate	Result
`bun run typecheck`	13/13 successful
`analyze.ts --markers --base origin/main --strict`	ok — all custom code properly marked
`bun run lint`	5903 warnings, 1 error — byte-identical to `main`'s own baseline; the error is the known pre-existing `consistent-return` in `packages/http-recorder/test/record-replay.test.ts`
`packages/drivers` full suite	273 pass, 0 fail
`packages/opencode` `test/altimate`	4226 pass, 0 fail (151 files)
`bun run typecheck` (forced, no cache)	13/13 successful

Eight new tests against real directories on disk, because the whole mechanism is path existence. They cover the repair (fires; declines when the path exists; declines on a legitimately nested path; declines when the remainder is absent), the harvesting, the end-to-end load from a harvested root, and the message no longer claiming found at duckdb.

Concurrency (#1202), verified with real processes. The exclusion claim is about separate processes, so the test spawns four and asserts their critical sections never interleave. The control confirms the test actually bites — the same four processes without the lock interleave completely:

enter 1     enter 3     enter 0     enter 2     exit 0     exit 1     exit 2     exit 3

which the assertion rejects on the second enter. 10/10 repeat runs green. The other four tests cover the unlocked-on-timeout path, both stale-lock signals, and release on throw.

A flaky test you may see, which is not mine. test/altimate/tools/sample-setup.test.ts intermittently fails 1–3 of its 8 tests at exactly 5000ms. Its execFile probe for the dbt runtime uses a 5000ms timeout and the test's own timeout is also 5000ms, so under machine load it races itself. It fails the same way on this branch with my commits stashed, sample_setup touches none of the code I changed, and it passes 8/8 on an idle machine. Pre-existing, worth its own fix, not this PR.

Windows shapes and the multi-peer wait (review round 3). Three gaps, each pinned by a test that fails with its own fix reverted — 1 of 10 for each Windows shape, 1 of 14 for the wait:

installLockPath special-cased the POSIX and drive-letter roots but not a UNC share root, so \\server\share became \\server\share.lock — a different network share, meaning two processes installing into the share would not share a lock at all.
The path-harvesting regex accepted POSIX and drive-letter absolutes only, so a Windows error quoting \\server\share\node_modules\duckdb\package.json yielded no roots and a driver on a share stayed unfindable despite the error naming its exact location. The pattern is now the named export quotedAbsolutePaths, testable from any platform.
The lock budget was per wait, so it only ever outlasted one holder; with three or more contenders the last one's deadline expired mid-install and it fell through to an unlocked performInstall. It is now per holder, with a bounded number of extensions.

The chdir arm was hardened after review, and the reason generalises. It originally asked for duckdb — which the repo's own packages/drivers/node_modules satisfies through the execPath and module-location roots regardless of what the resolver does, and which no environment isolation can suppress. It would have passed while proving nothing. It now uses specifiers that exist nowhere but the tree each test builds and asserts on an export marker, so a pass establishes which root satisfied the load rather than that something, somewhere, was found.

Not verified by me:

Windows, on Windows. The lock-path and harvesting logic now cover POSIX, drive-letter and UNC shapes and are unit-tested for all three, but every one of those tests ran on macOS. They are string-level assertions that do not touch the filesystem, which is what makes them meaningful off-platform — and also what stops them proving anything about real UNC I/O.
The exact runtime conditions that make ambient resolution emit ENOENT rather than ERR_MODULE_NOT_FOUND. On my machine the compiled binary reports the latter; I reproduced the ENOENT path by injecting it through the importer parameter loadOptionalDriver already exposes for testing. The fix does not depend on knowing the trigger — it keys on the error naming a repairable path — but that half is inference, not observation.
Whether any other call site depends on the old found at <specifier> wording. I grepped and found none, but string matching on error text is easy to miss.
I could not reproduce the originally reported failure on a Linux VM, and want that on the record. On a fresh Debian 12 GCE instance I built the exact reported shape — npm install -g tree at /usr/lib/node_modules/altimate-code, a real npm install duckdb beside it, the manifest declaring duckdb, run as root from an unrelated cwd, compiled binary with production options — and every load strategy succeeded: bare-specifier ambient import failed as expected, but import(file://abs), createRequire(abs), a cwd-anchored createRequire, and the real loadOptionalDriver all returned a working Database. So Bun is not re-anchoring absolute paths in that configuration, and the remaining difference between that VM and the rig is not yet identified. The fix in the first commit is still correct and still removes a real failure mode; I just cannot claim it closes the rig's specific case, and the new diagnostics exist so the next occurrence answers the question instead of costing another round trip.

Screenshots / recordings

Not a UI change.

Checklist

I have tested my changes locally
I have not included unrelated changes in this PR

[!NOTE]
High Risk
Changes core driver resolution, load semantics, and shared install directory locking; workspace exclusion and argv mutation affect security-sensitive credential boundaries and global process state.

Overview
Fixes warehouse driver loading when ambient resolution fails with cwd-prefixed absolute paths (e.g. global CLI run from another directory) and when concurrent installs corrupt the managed driver tree.

Resolution and load path: After a failed ambient import, the resolver now mines quoted absolute paths from the error (searchRootsFromError), repairs <cwd> + absolute path concatenation (repairCwdPrefixedPath), walks all enclosing node_modules roots, and appends those roots after managed/trusted search roots so stale copies do not win. Workspace node_modules (including nested and symlinked targets) are never harvested, preserving the permission boundary. Loads use createRequire anchored at the resolved file plus temporary process.argv neutralization so @mapbox/node-pre-gyp does not misread --dir as --directory. safeCwd() replaces process.cwd() where a deleted cwd would mask the real failure; load errors add cwd/execPath and cwd-prefix hints.

Install concurrency: withInstallLock serializes cross-process npm install into the shared driver directory via an atomic held lock file with UUID tokens, stale recovery (rename + token checks), per-holder wait budgets, and unlocked fallback on timeout. warehouse_install_driver now requests permission for the lock container (installLockPath) alongside the driver dir.

^{Reviewed by Cursor Bugbot for commit 9a930e3e20893fe50c307152903b0a5cbd506067. Bugbot is set up for automated code reviews on this repo. Configure here.}

Summary by CodeRabbit

Bug Fixes
- Improved optional warehouse driver discovery when paths are malformed or working-directory information is unavailable.
- Prevented driver resolution from depending on the current working directory.
- Enhanced diagnostics and path handling across POSIX, Windows drive, and UNC formats.
- Ensured drivers load relative to their resolved installation location.
- Prevented concurrent installations from interfering through cross-process locking.
- Improved recovery for stale and handed-off installation locks while preserving active locks.
- Strengthened workspace boundary handling, including symlinked paths.
Permissions
- Installation approval now includes the directory used for coordination locks.