diff --git a/packages/drivers/src/resolve.ts b/packages/drivers/src/resolve.ts index 1d061ca7e..6e5c1329d 100644 --- a/packages/drivers/src/resolve.ts +++ b/packages/drivers/src/resolve.ts @@ -156,6 +156,185 @@ function nodeModulesUpward(start: string): string[] { return found } +/** + * Repair a path that had the working directory concatenated onto an + * already-absolute path, e.g. `/usr/lib/node_modules/…` for a package + * that really lives at `/usr/lib/node_modules/…`. + * + * Observed from a globally installed CLI (`npm install -g`, package tree under + * `/usr/lib/node_modules/altimate-code`) run from an unrelated directory: the + * runtime's own resolution reported + * `ENOENT … open '/usr/lib/node_modules/altimate-code/node_modules/duckdb/package.json'` + * while the file existed at that path without the `` prefix. + * + * Deliberately conservative — it fires only when the named path is absent, is + * genuinely prefixed by the working directory, and the de-prefixed remainder + * exists. That last check is what keeps a legitimately nested + * `/node_modules/…` from being mangled. + */ +export function repairCwdPrefixedPath(candidate: string, cwd = safeCwd()): string | undefined { + if (!candidate || !cwd || fs.existsSync(candidate)) return undefined + if (!candidate.startsWith(cwd)) return undefined + // POSIX concatenation yields `/usr/…`, whose remainder carries the + // separator. Windows has no separator to carry: `C:\work` + `C:\global\…` + // concatenates to `C:\workC:\global\…`, and a join-shaped `C:\work\C:\global\…` + // leaves a stray leading separator on the remainder. Try each shape and + // accept only a remainder that is absolute and exists, so a near-miss such + // as cwd `/work` against `/workspace/…` contributes nothing. + const rest = candidate.slice(cwd.length) + for (const remainder of rest.startsWith(path.sep) ? [rest, rest.slice(1)] : [rest]) { + if (!remainder || !path.isAbsolute(remainder)) continue + if (fs.existsSync(remainder)) return remainder + } + return undefined +} + +/** + * `process.cwd()` throws ENOENT when the working directory has been removed out + * from under the process. Every caller here is formatting an error or repairing + * a path, where throwing would replace the driver failure the caller is trying + * to report with an unrelated ENOENT — losing the actual diagnosis. + */ +function safeCwd(): string | undefined { + try { + return process.cwd() + } catch { + return undefined + } +} + +/** + * The `node_modules` content `driverSearchRoots()` deliberately refuses to + * search: the working directory tree, and the project and ancestor + * `node_modules` above it. + */ +function workspaceScope(): { cwd: string | undefined; ancestors: string[] } { + const cwd = safeCwd() + if (!cwd) return { cwd: undefined, ancestors: [] } + const resolved = realPath(cwd) + return { cwd: resolved, ancestors: nodeModulesUpward(resolved).map(realPath) } +} + +/** + * Absolute *real* path, falling back to the lexical one when the link cannot be + * followed. + * + * The containment check below must compare real paths. `isDirectory` follows + * symlinks, so a symlinked `node_modules` whose lexical path sits outside the + * working directory but whose target sits inside it would pass a lexical + * exclusion and be imported — reintroducing the workspace-controlled code the + * exclusion exists to keep out. + */ +function realPath(candidate: string): string { + try { + return fs.realpathSync(path.resolve(candidate)) + } catch { + return path.resolve(candidate) + } +} + +/** + * Preserve order, drop repeats. A harvested root often names a directory the + * inferred roots already cover, and listing it twice makes the searched-location + * count in the failure message overstate where we actually looked. + */ +function dedupeRoots(roots: readonly string[]): string[] { + return [...new Set(roots)] +} + +/** + * Every `node_modules` directory enclosing `candidate`, innermost first. + * + * All of them, not just the innermost: a quoted path often runs through a + * driver's own dependency — `/opt/node_modules/duckdb/node_modules/node-addon-api/…` + * — where the innermost root holds the dependency and the *outer* one holds the + * driver we are actually looking for. Returning only the innermost left the + * driver unfindable in exactly the nested case. + * + * `sep` is a parameter so the Windows behaviour is testable from a POSIX host. + * It matters because Windows quotes both `C:\…` and `C:/…` in errors, while the + * marker is built from the platform separator — a forward-slash path would + * never match a backslash marker, and nothing would be harvested at all. + * Rewriting separators is length-preserving, so the slice offsets still hold. + */ +export function enclosingNodeModulesRoots(candidate: string, sep: string = path.sep): string[] { + const normalized = sep === "\\" ? candidate.replace(/\//g, "\\") : candidate + const marker = `${sep}node_modules${sep}` + const roots: string[] = [] + for (let at = normalized.lastIndexOf(marker); at !== -1; at = normalized.lastIndexOf(marker, at - 1)) { + roots.push(normalized.slice(0, at + marker.length - 1)) + if (at === 0) break + } + return roots +} + +/** True when `root` is workspace-controlled and must not be imported from. */ +function isWorkspaceRoot(root: string, scope: { cwd: string | undefined; ancestors: string[] }): boolean { + // Real paths on both sides: a symlink pointing into the workspace must not + // slip past a purely lexical comparison. + const resolved = realPath(root) + if (scope.ancestors.includes(resolved)) return true + // Fail closed. With no working directory there is nothing to compare against, + // and treating that as "not workspace content" would admit every root an error + // happens to name — turning the one case where the process cannot see its own + // filesystem into the case with no boundary at all. + if (!scope.cwd) return true + const rel = path.relative(scope.cwd, resolved) + return rel === "" || (!rel.startsWith("..") && !path.isAbsolute(rel)) +} + +/** + * `node_modules` directories named by an error the runtime raised while trying + * to resolve a package itself. + * + * When ambient resolution fails it often names the exact absolute location it + * was reaching for. That location is better evidence than anything we can + * infer, so it is worth searching — after repairing a concatenated working + * directory, which is the failure this exists for. Returns roots only when they + * exist on disk, so a nonsense path contributes nothing. + * + * Workspace-controlled roots are never returned. `driverSearchRoots()` refuses + * project and ancestor `node_modules` because importing a matching SDK during a + * warehouse read/test would bypass the permission boundary and can expose + * resolved credentials; mining a path out of an error message must not become a + * way around that invariant. + */ +/** + * Absolute paths a runtime quoted inside an error message. + * + * Three absolute shapes, and the third is the one that is easy to leave out: + * POSIX `/…`, a drive letter `C:\…`, and a UNC share `\\server\share\…`. A + * Windows error naming a driver on a share yields no roots at all without it, + * so a driver stays unfindable even though the error named its exact location. + */ +export function quotedAbsolutePaths(message: string): string[] { + const found: string[] = [] + const pattern = /['"`]((?:\/|[A-Za-z]:[\\/]|\\\\[^\\/'"`\n]+[\\/])[^'"`\n]+)['"`]/g + for (const match of message.matchAll(pattern)) { + const named = match[1] + if (named) found.push(named) + } + return found +} + +export function searchRootsFromError(error: unknown): string[] { + const message = error instanceof Error ? error.message : String(error) + const scope = workspaceScope() + const roots: string[] = [] + for (const named of quotedAbsolutePaths(message)) { + for (const candidate of [named, repairCwdPrefixedPath(named)]) { + if (!candidate) continue + // Walk back to every enclosing node_modules directory, innermost first. + for (const root of enclosingNodeModulesRoots(candidate)) { + if (!isDirectory(root)) continue + if (isWorkspaceRoot(root, scope)) continue + if (!roots.includes(root)) roots.push(root) + } + } + } + return roots +} + /** * Directories to search for an optional SDK, most specific first. * @@ -224,7 +403,13 @@ export function packageNameOf(specifier: string): string { */ export function resolveOptionalPackage(specifier: string, roots = driverSearchRoots()): string | undefined { const pkg = packageNameOf(specifier) - const require = createRequire(pathToFileURL(path.join(process.cwd(), "noop.js")).href) + // The anchor only has to be some absolute file URL — resolution is driven by + // the explicit `paths` below, not by this base. So it must not be the one + // thing that can throw: process.cwd() raises ENOENT when the working + // directory has been removed, and letting that escape here replaces every + // driver diagnosis with an unrelated uv_cwd error. + const anchor = safeCwd() ?? os.tmpdir() + const require = createRequire(pathToFileURL(path.join(anchor, "noop.js")).href) for (const root of roots) { const pkgDir = path.join(root, pkg) @@ -308,6 +493,101 @@ function entryFromManifest(pkgDir: string, specifier: string, pkg: string): stri * * @throws {DriverNotInstalledError} when the package is genuinely absent. */ +/** + * Load an already-resolved package from its own absolute location. + * + * `import(pathToFileURL(abs))` is not enough. The ESM loader reads the + * package's manifest to decide the module's type and exports, and in a compiled + * binary that lookup has been observed resolving against the *process working + * directory* rather than the module's own directory — measured twice + * independently, from a global install run under `--dir`: + * + * ENOENT ... open '/usr/lib/.../duckdb/package.json' + * + * while the file exists at that path without the `` prefix. Supplying that + * concatenated path is sufficient to make the load succeed, and running with + * cwd `/` — which makes the concatenation a no-op — clears it too. + * + * A CommonJS require anchored at the resolved file makes every nested lookup, + * the manifest included, relative to the driver's own directory, so the working + * directory is never an input. The drivers we load this way are CommonJS; an + * ESM-only package still needs the loader, so that path remains as a fallback + * and is taken only for the error that specifically means "this is ESM". + * + * `process.chdir()` around the load would also neutralise the concatenation and + * is deliberately not used: it is global mutable state, and these loads happen + * under concurrency, so it would corrupt resolution for unrelated work + * non-deterministically — worse than the fault it patches. + */ +/** + * Run `fn` with this process's command line hidden from it. + * + * `@mapbox/node-pre-gyp`, which packages several native drivers, resolves a + * module's manifest by parsing the **host application's** `process.argv` — + * `find()` passes `argv: process.argv` into its own `Run`, `nopt` + * abbreviation-matches whatever it sees against node-pre-gyp's option list, and + * `node-pre-gyp.js:164` then does: + * + * package_json_path = path.join(this.opts.directory, package_json_path) + * + * `path.join`, not `path.resolve`, so an absolute manifest path is **not** + * discarded. Our `--dir` abbreviates to node-pre-gyp's `--directory`, so + * `altimate-code run --dir ` made the driver look for its manifest at + * `` + the manifest's own absolute path, and the load failed with an + * ENOENT naming a path that had never existed. + * + * Nothing about that is specific to us, to DuckDB, or to a compiled binary: any + * CLI that embeds a node-pre-gyp-packaged module and accepts a flag + * abbreviating to `--directory` is exposed. Reported upstream; this keeps our + * users working in the meantime. + * + * **Why swapping a global here is safe when `process.chdir()` would not be.** + * `fn` is a synchronous `require`. JavaScript is single-threaded and there is + * no `await` between the swap and the restore, so no other task can run while + * the command line is hidden and no concurrent load can observe it. The same + * trick around an awaited dynamic `import()` would be a genuine hazard, and is + * deliberately not done below. + */ +function withNeutralArgv(fn: () => T): T { + const saved = process.argv + // Keep argv[0] and argv[1] — the executable and the entry script. nopt only + // parses what follows, and node-pre-gyp expects those two to be present. + process.argv = saved.slice(0, 2) + try { + return fn() + } finally { + process.argv = saved + } +} + +function requireFromLocation(resolved: string): unknown { + const requireFrom = createRequire(pathToFileURL(resolved).href) + return withNeutralArgv(() => requireFrom(resolved)) +} + +/** True when a require failed only because the target is an ES module. */ +function isRequireOfEsm(error: unknown): boolean { + const code = error && typeof error === "object" && "code" in error ? (error as { code?: unknown }).code : undefined + if (code === "ERR_REQUIRE_ESM") return true + const message = error instanceof Error ? error.message : String(error) + return /require\(\) of ES Module|Cannot use import statement outside a module|Unexpected token 'export'/i.test(message) +} + +/** + * Load from `resolved`, preferring the cwd-independent path. + * + * The injected `importer` is still used for the ESM fallback so tests keep a + * seam over the loader. + */ +async function loadFromLocation(resolved: string, importer: (spec: string) => Promise): Promise { + try { + return requireFromLocation(resolved) + } catch (requireError) { + if (!isRequireOfEsm(requireError)) throw requireError + return await importer(pathToFileURL(resolved).href) + } +} + export async function loadOptionalDriver( driver: DriverName, specifier: string, @@ -317,22 +597,47 @@ export async function loadOptionalDriver( return await importer(specifier) } catch (ambientError) { const ambientBroken = !isModuleNotFound(ambientError, specifier) - const roots = driverSearchRoots() + // Trusted roots first, then the location the runtime itself named. When + // ambient resolution fails it frequently quotes the absolute path it was + // reaching for — including the case where it concatenated the working + // directory onto an already-absolute path — and that is the only evidence + // available when nothing else resolves. But it is evidence about wherever + // the runtime happened to point, which may be a stale or broken copy, so it + // must not preempt the managed installation: `driverSearchRoots()` puts the + // driver we installed first precisely so it wins over a stale copy + // elsewhere. Appending keeps that order and still recovers the failure this + // exists for, because a harvested root is reached whenever the roots ahead + // of it resolve nothing. + const roots = dedupeRoots([...driverSearchRoots(), ...searchRootsFromError(ambientError)]) const resolved = resolveOptionalPackage(specifier, roots) if (!resolved) { - // A broken ambient copy is a load failure, not an absence. - if (ambientBroken) throw loadFailure(driver, specifier, ambientError) + // Nothing was found anywhere, so there is no location to name. Saying + // "found at " here would report the bare specifier as though + // it were a place on disk, which reads as a load failure at a known path + // and sends the reader looking for a file that was never located. + if (ambientBroken) throw ambientLoadFailure(driver, ambientError, describeSearched(roots)) throw new DriverNotInstalledError(driver, DRIVER_PACKAGES[driver], roots) } try { - return await importer(pathToFileURL(resolved).href) + return await loadFromLocation(resolved, importer) } catch (loadError) { // On disk but will not load — a half-installed copy, or a native addon // built for another platform. When an ambient copy was also broken, // report that one: it is the copy the runtime would normally pick. - throw loadFailure(driver, ambientBroken ? specifier : resolved, ambientBroken ? ambientError : loadError) + if (ambientBroken) { + // Both copies are unusable. Lead with the ambient one — it is the copy + // the runtime would normally pick — but name the on-disk path we also + // tried, rather than passing the specifier off as a location. + throw ambientLoadFailure( + driver, + ambientError, + `A copy at ${resolved} was also tried and failed to load: ` + + (loadError instanceof Error ? loadError.message : String(loadError)), + ) + } + throw loadFailure(driver, resolved, loadError) } } } @@ -368,6 +673,60 @@ function loadFailure(driver: DriverName, where: string, error: unknown): Error { ) } +/** + * The ambient import failed for a reason other than "not installed". + * + * We do not know where that copy lives — the runtime resolved it, not us — so + * the specifier must not be reported as though it were a location on disk. The + * previous wording, `found at duckdb but failed to load`, read as a load + * failure at a known path for a package that had in fact never been located, + * and sent readers looking for a file that was not there. + */ +function ambientLoadFailure(driver: DriverName, error: unknown, detail: string): Error { + return new Error( + `${DRIVER_LABELS[driver]} driver failed to load from the default module resolution: ` + + `${error instanceof Error ? error.message : String(error)}\n${detail}${loadDiagnostics(error)}`, + ) +} + +/** + * Context a reader needs to tell a resolution fault apart from a broken + * package, appended to load failures. + * + * Driver-load failures have twice been diagnosed from the error text alone and + * twice been diagnosed wrong, because the text named a path without saying what + * the process's own view of the filesystem was. The expensive question each + * time was "is this absolute path being re-anchored to the working directory?" + * — which is answerable on the spot, and only from inside the failing process. + */ +function loadDiagnostics(error: unknown): string { + // A deleted working directory makes process.cwd() throw. This runs while a + // driver failure is being formatted, so letting that escape would replace the + // fault the reader needs with an unrelated ENOENT from the reporting path. + const cwd = safeCwd() + const lines = [`cwd=${cwd ?? ""}`, `execPath=${process.execPath}`] + const message = error instanceof Error ? error.message : String(error) + for (const match of message.matchAll(/['"`]((?:\/|[A-Za-z]:[\\/])[^'"`\n]+)['"`]/g)) { + const named = match[1] + if (!named) continue + const repaired = repairCwdPrefixedPath(named) + if (repaired) { + lines.push( + `NOTE: "${named}" does not exist, but "${repaired}" does — the working ` + + `directory appears to have been concatenated onto an absolute path.`, + ) + } + } + return `\n(${lines.join("; ")})` +} + +function describeSearched(searched: readonly string[]): string { + return searched.length + ? `It was not found in any searchable location. Searched ${searched.length} ` + + `location${searched.length === 1 ? "" : "s"}: ${searched.join(", ")}` + : "It was not found in any searchable location, and no driver directory exists yet." +} + /** * Import an optional package that is not a warehouse driver, returning * undefined when it is unavailable. @@ -381,9 +740,12 @@ export async function loadOptionalPackage(specifier: string): Promise import(/* @vite-ignore */ spec)) } } @@ -712,7 +1074,25 @@ async function installOptionalDriverInternal( if (!options.force && installed(driver)) { return { driver, packages, dir, installed: true, alreadyPresent: true } } - return performInstall(driver, packages, dir, options) + // Take the cross-process lock, then check readiness again. The peer that + // held it has usually just installed the very thing we queued for, so + // most contenders return "already present" instead of running a second + // npm over the same tree — which is what produced the ENOTEMPTY races. + return withInstallLock( + dir, + async (acquired) => { + if (acquired && !options.force && installed(driver)) { + return { driver, packages, dir, installed: true, alreadyPresent: true } + } + return performInstall(driver, packages, dir, options) + }, + // Outlast one peer's install. A lock wait shorter than the install it is + // waiting on means a contender gives up while the holder's npm is still + // running and then installs unlocked over the same tree, which is the + // race this lock exists to stop. The two timeouts were independent + // constants, so raising the install timeout alone silently broke it. + { timeoutMs: (options.timeoutMs ?? 180_000) + 60_000 }, + ) }) installsInFlight.set(dir, run) try { @@ -725,6 +1105,358 @@ async function installOptionalDriverInternal( /** In-flight installs keyed by target directory (see the note above). */ const installsInFlight = new Map>() +// --------------------------------------------------------------------------- +// Cross-process install lock +// --------------------------------------------------------------------------- + +/** + * `installsInFlight` serialises installs inside one process. It cannot see + * other processes, and the managed driver directory is shared by all of them, + * so N CLIs starting together each run `npm install` over the same tree: + * + * npm install failed (exit 217) … ENOTEMPTY … + * rmdir /root/.local/share/altimate-code/drivers/node_modules/duckdb/… + * + * That is not benchmark-specific — any concurrent use of the CLI hits it. + * + * `mkdir` is atomic and fails with EEXIST when the directory exists, on both + * POSIX and Windows, which makes a lock directory the portable primitive here. + * The lock lives beside the install directory rather than inside it so npm + * never sees it as stray package content. + */ +export function installLockPath(dir: string): string { + const trimmed = dir.replace(/[\\/]+$/, "") + const separator = dir.includes("\\") ? "\\" : "/" + // Trailing separators are stripped so `/` and `` agree on one lock. + // A filesystem root is the exception: stripping there destroys the root, and + // the result names something *outside* the directory being locked, so two + // processes installing into the same place take different locks. Roots keep + // their separator and the lock goes inside them. + // + // / -> /.lock not the relative .lock + // C:\ -> C:\.lock not drive-relative C:.lock + // \\server\share\ -> \\server\share\.lock not a *different share* + if (isFilesystemRoot(trimmed)) return `${trimmed}${separator}.lock` + return `${trimmed}.lock` +} + +/** + * True when `candidate` — trailing separators already stripped — names a + * filesystem root rather than a directory inside one. + * + * The UNC case is the one that is easy to miss: a share root is a root in + * exactly the way a drive letter is, and appending `.lock` to it names an + * unrelated share rather than anything under the directory being locked. + */ +function isFilesystemRoot(candidate: string): boolean { + if (candidate === "") return true // POSIX "/" strips to "" + if (/^[A-Za-z]:$/.test(candidate)) return true // "C:\" strips to "C:" + return /^\\\\[^\\/]+\\[^\\/]+$/.test(candidate) // "\\server\share" +} + +/** + * The atomically-created lock itself, which lives *inside* the container + * `installLockPath()` names. + * + * Two directories rather than one so every path this mechanism writes sits + * under a single approved prefix. Stale recovery renames the held lock aside + * before deleting it, and that destination has to be somewhere the install tool + * asked permission for; a sibling of the container would be outside the + * `.lock/*` pattern the tool brokers. + */ +function heldLockPath(dir: string): string { + return path.join(installLockPath(dir), "held") +} + +interface LockHolder { + pid: number + hostname: string + /** Absent when the owner file is malformed; age then falls back to mtime. */ + startedAt?: number + /** Identifies one acquisition, so a holder only ever releases its own lock. */ + token?: string +} + +function readLockHolder(lockDir: string): LockHolder | undefined { + try { + const raw = fs.readFileSync(path.join(lockDir, "owner.json"), "utf8") + const parsed: unknown = JSON.parse(raw) + if (!parsed || typeof parsed !== "object") return undefined + const holder = parsed as Partial + if (typeof holder.pid !== "number") return undefined + return { + pid: holder.pid, + hostname: typeof holder.hostname === "string" ? holder.hostname : "", + startedAt: typeof holder.startedAt === "number" ? holder.startedAt : undefined, + token: typeof holder.token === "string" ? holder.token : undefined, + } + } catch { + return undefined + } +} + +/** + * True when a lock cannot belong to a live install any more. + * + * The two signals are not interchangeable, and which one applies depends on + * whether liveness is decidable at all: + * + * - **Owner on this host.** Liveness is decidable, so it is the only thing that + * counts. Age must *not* also apply here: npm can legitimately run longer + * than any duration we pick — a native build such as `oracledb` or `duckdb`, + * or a caller that raised its own install timeout — and breaking a live + * owner's lock puts two `npm install` runs over the same tree, which is the + * exact corruption this lock exists to prevent. + * - **No readable owner, or an owner on another host** (a shared home + * directory). Liveness cannot be established, so age is the only signal + * available and the lock ages out. + */ +function isStaleLock( + lockDir: string, + holder: LockHolder | undefined, + maxAgeMs: number, + hardMaxAgeMs: number, +): boolean { + const age = lockAgeMs(lockDir, holder) + if (holder && holder.hostname === os.hostname()) { + if (!processExists(holder.pid)) return true + // `processExists` answers "some process holds this pid", not "our installer + // is still running": a crashed owner's pid can be recycled by an unrelated + // long-lived process, and liveness alone would then keep the lock forever, + // making every later install wait out its timeout and run unlocked. So a + // live same-host owner is protected, but only up to a backstop far beyond + // any real npm run — long enough never to interrupt an install, short + // enough that a recycled pid cannot wedge the directory permanently. + return age !== undefined && age > hardMaxAgeMs + } + // No readable owner, or an owner on another host sharing a home directory. + // Liveness is not decidable, so age is the only signal there is. + return age !== undefined && age > maxAgeMs +} + +/** How long the lock has been held, by owner record or directory mtime. */ +function lockAgeMs(lockDir: string, holder: LockHolder | undefined): number | undefined { + const startedAt = holder?.startedAt + if (typeof startedAt === "number") return Date.now() - startedAt + try { + return Date.now() - fs.statSync(lockDir).mtimeMs + } catch { + // Vanished between checks — someone else released it, so it is not stale. + return undefined + } +} + +/** True when two owner records describe the same acquisition. */ +function sameHolder(a: LockHolder | undefined, b: LockHolder | undefined): boolean { + if (!a || !b) return a === b + return a.pid === b.pid && a.hostname === b.hostname && a.startedAt === b.startedAt && a.token === b.token +} + +/** + * Take ownership of a lock judged stale, atomically, and remove it. + * + * Two processes can both judge the same lock stale. If each simply deleted the + * pathname, the first would delete the dead lock and acquire a fresh one, and + * the second would then delete *that* live lock and acquire its own — putting + * both inside the critical section, which is the failure the lock exists to + * prevent. `rename` is atomic: exactly one process can move a given directory, + * and only that process goes on to delete it. The loser's rename fails and it + * simply retries against whatever state now exists. + */ +function claimStaleLock(lockDir: string, judged: LockHolder | undefined): boolean { + // The staleness verdict was formed before this call, and the owner can have + // released the lock and a peer re-taken it since. Renaming blindly would move + // a *live* lock aside and put two installs over the same tree. Check the owner + // record still matches the one judged stale before touching anything. + if (!sameHolder(judged, readLockHolder(lockDir))) return false + + const claimed = path.join(path.dirname(lockDir), `stale-${process.pid}-${Date.now().toString(36)}`) + try { + fs.renameSync(lockDir, claimed) + } catch { + // Another process claimed it first, the owner released it, or the parent + // does not permit rename. Nothing of ours to clean up. + return false + } + + // The check above narrows the window but cannot close it — nothing makes + // "read the owner" and "rename" one operation. So confirm what was actually + // moved, and put it back if a peer had re-taken the lock in between. + if (!sameHolder(judged, readLockHolder(claimed))) { + try { + fs.renameSync(claimed, lockDir) + return false + } catch { + // Cannot restore — a third process has already re-created the lock. Fall + // through and remove what we moved rather than leaking it. + } + } + + try { + fs.rmSync(claimed, { recursive: true, force: true }) + } catch { + // The rename already made the lock unreachable, so a leftover directory + // costs nothing but disk. + } + return true +} + +/** + * Release a lock this process acquired, but only while it is still ours. + * + * A lock we hold can be broken as stale and re-taken by a peer while `fn` is + * still running — an install that outlives `staleAfterMs` on a machine whose + * owner record is unreadable, say. Removing it by pathname would then delete + * the successor's live lock and admit a third process. The token is written + * when the lock is taken, so a mismatch means the directory is somebody else's. + */ +function releaseInstallLock(lockDir: string, token: string, ino: number | undefined): void { + const holder = readLockHolder(lockDir) + if (holder?.token !== undefined) { + if (holder.token !== token) return + } else if (ino !== undefined) { + // No token to compare against. There is a real window for this: the lock + // directory is created before `owner.json` is written, so a successor that + // re-took the lock in between holds a live lock carrying no token, and + // removing it by pathname would admit a third process. The directory's own + // identity settles it — a different inode is a different lock. + try { + if (fs.statSync(lockDir).ino !== ino) return + } catch { + return + } + } + try { + fs.rmSync(lockDir, { recursive: true, force: true }) + } catch { + // Leaving it behind is safe: the next contender ages it out as stale. + } +} + +/** + * A value that changes when the lock changes hands. + * + * The owner's token when it published one; otherwise the lock directory's own + * identity, which a fresh `mkdir` changes even when the owner file is missing + * or unreadable. `undefined` means we could not tell, and the caller then + * treats the holder as unchanged rather than inventing progress. + */ +function lockIdentity(lockDir: string, holder: LockHolder | undefined): string | undefined { + if (holder?.token) return holder.token + try { + const stat = fs.statSync(lockDir) + return `${stat.ino}:${stat.mtimeMs}` + } catch { + return undefined + } +} + +/** + * Run `fn` while holding an exclusive lock on `dir`, across processes. + * + * On timeout the work runs anyway rather than failing. A driver install that + * races is recoverable — npm is largely idempotent here and the readiness check + * afterwards is authoritative — whereas refusing to install because a lock + * could not be taken turns a slow peer into a hard failure. + */ +export async function withInstallLock( + dir: string, + fn: (acquired: boolean) => Promise, + options: { timeoutMs?: number; staleAfterMs?: number; hardStaleAfterMs?: number; pollMs?: number } = {}, +): Promise { + const lockDir = heldLockPath(dir) + const timeoutMs = options.timeoutMs ?? 240_000 + const staleAfterMs = options.staleAfterMs ?? 300_000 + const hardStaleAfterMs = options.hardStaleAfterMs ?? Math.max(staleAfterMs, 3_600_000) + const pollMs = options.pollMs ?? 100 + let deadline = Date.now() + timeoutMs + // The budget is per *holder*, not per wait. Every process counts its deadline + // from its own start, so a single budget only ever outlasts one peer: with + // three or more contenders the last one's deadline expires part-way through + // somebody else's install and it falls through to an unlocked performInstall + // — the concurrent npm mutation this lock exists to prevent. Seeing the lock + // change hands is proof the queue is moving rather than wedged, so each new + // holder gets its own budget. Extensions are capped so a machine that keeps + // feeding in contenders cannot block a caller indefinitely. + const maxHandovers = 32 + let handovers = 0 + let lastHolder: string | undefined + const token = `${process.pid}-${Date.now().toString(36)}-${Math.random().toString(36).slice(2, 10)}` + let acquired = false + let ino: number | undefined + + // The container must exist before the atomic mkdir inside it can land. On a + // cold machine nothing has created the XDG data directory yet — + // `performInstall` is the first thing that does, and it runs *after* this — so + // a non-recursive mkdir would fail ENOENT, take the "cannot lock" branch, and + // drop every caller straight into an unlocked install. That is precisely the + // cold-start stampede this lock exists to prevent. + try { + fs.mkdirSync(path.dirname(lockDir), { recursive: true }) + } catch { + // Genuinely unwritable. The acquire below then fails too and we proceed + // unlocked, which is the documented degradation. + } + + for (;;) { + try { + fs.mkdirSync(lockDir, { recursive: false }) + acquired = true + try { + ino = fs.statSync(lockDir).ino + } catch { + // Identity check is skipped on release; the token check still applies. + } + break + } catch (e) { + // Anything but "already held" — an unwritable parent, say — means we + // cannot lock at all, so proceed unlocked rather than block forever. + const code = e && typeof e === "object" && "code" in e ? (e as { code?: unknown }).code : undefined + if (code !== "EEXIST") break + const holder = readLockHolder(lockDir) + const identity = lockIdentity(lockDir, holder) + if (identity !== undefined && identity !== lastHolder) { + if (lastHolder !== undefined && handovers < maxHandovers) { + handovers++ + deadline = Date.now() + timeoutMs + } + lastHolder = identity + } + if (isStaleLock(lockDir, holder, staleAfterMs, hardStaleAfterMs)) { + // A claim can fail persistently — a lock owned by another user, or a + // container that permits inspection but not rename. Retrying such a + // claim without yielding spins at full CPU and never reaches the + // deadline, so only a claim that actually succeeded skips the wait. + if (claimStaleLock(lockDir, holder)) continue + } + if (Date.now() >= deadline) break + await sleep(pollMs) + } + } + + if (acquired) { + try { + fs.writeFileSync( + path.join(lockDir, "owner.json"), + JSON.stringify({ + pid: process.pid, + hostname: os.hostname(), + startedAt: Date.now(), + token, + } satisfies LockHolder), + ) + } catch { + // Diagnostics only — the lock is the directory, not the file in it. + } + } + + try { + return await fn(acquired) + } finally { + if (acquired) releaseInstallLock(lockDir, token, ino) + } +} + async function performInstall( driver: DriverName, packages: readonly string[], diff --git a/packages/drivers/test/install-lock.test.ts b/packages/drivers/test/install-lock.test.ts new file mode 100644 index 000000000..de3f5dcd9 --- /dev/null +++ b/packages/drivers/test/install-lock.test.ts @@ -0,0 +1,355 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test" +import fs from "node:fs" +import os from "node:os" +import path from "node:path" +import { fileURLToPath } from "node:url" + +import { installLockPath, withInstallLock } from "../src/resolve" + +// The managed driver directory is shared by every CLI process on the machine, +// and `installsInFlight` only serialises within one process. Eight CLIs +// starting together each ran `npm install` over the same tree: +// +// npm install failed (exit 217) … ENOTEMPTY … +// rmdir /root/.local/share/altimate-code/drivers/node_modules/duckdb/… +// +// The exclusion claim is about separate processes, so the central test spawns +// separate processes. An in-process test cannot establish it. + +const resolveModule = fileURLToPath(new URL("../src/resolve.ts", import.meta.url)) + +/** The atomic lock inside the container, which is what contention is fought over. */ +const heldPath = (target: string) => path.join(installLockPath(target), "held") + +/** Plant a lock with a given owner record, as a peer process would leave it. */ +function plantLock(target: string, holder: Record) { + const held = heldPath(target) + fs.mkdirSync(held, { recursive: true }) + fs.writeFileSync(path.join(held, "owner.json"), JSON.stringify(holder)) + return held +} + +let dir = "" + +beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), "install-lock-")) +}) + +afterEach(() => { + if (dir) fs.rmSync(dir, { recursive: true, force: true }) +}) + +describe("install lock path", () => { + test("agrees on one lock whether or not the directory has a trailing separator", () => { + expect(installLockPath("/a/drivers/")).toBe(installLockPath("/a/drivers")) + }) + + test("keeps a filesystem root intact", () => { + // Stripping the separator from a root turns the lock into a *relative* + // path, so processes with different working directories would take + // different locks while installing into the same directory. + expect(installLockPath("/")).toBe("/.lock") + expect(path.isAbsolute(installLockPath("/"))).toBe(true) + expect(installLockPath("C:\\")).toBe("C:\\.lock") + }) +}) + +describe("cross-process install lock", () => { + test("excludes concurrent processes from the critical section", async () => { + const target = path.join(dir, "drivers") + fs.mkdirSync(target, { recursive: true }) + const log = path.join(dir, "log.txt") + const ready = path.join(dir, "ready") + fs.mkdirSync(ready) + + // A start barrier, because without one the test can pass vacuously: if the + // scheduler happens to run the children serially — each acquiring, holding, + // and exiting before the next starts — the "no overlapping bracket" check + // is satisfied even by a completely broken lock. Every child announces + // itself and waits until all four are ready, so they contend for real. + const child = path.join(dir, "child.ts") + fs.writeFileSync( + child, + `import fs from "node:fs" +import { withInstallLock } from ${JSON.stringify(resolveModule)} +const [target, log, ready, id] = process.argv.slice(2) +fs.writeFileSync(\`\${ready}/\${id}\`, "1") +const deadline = Date.now() + 20000 +while (fs.readdirSync(ready).length < 4 && Date.now() < deadline) { + await new Promise((r) => setTimeout(r, 5)) +} +await withInstallLock(target, async (acquired) => { + if (!acquired) { fs.appendFileSync(log, \`timeout \${id}\\n\`); return } + fs.appendFileSync(log, \`enter \${id}\\n\`) + await new Promise((r) => setTimeout(r, 120)) + fs.appendFileSync(log, \`exit \${id}\\n\`) +}, { timeoutMs: 30000 }) +process.exit(0) +`, + ) + + const kids = Array.from({ length: 4 }, (_, i) => + Bun.spawn(["bun", child, target, log, ready, String(i)], { stdout: "ignore", stderr: "ignore" }), + ) + const codes = await Promise.all(kids.map((k) => k.exited)) + expect(codes).toEqual([0, 0, 0, 0]) + + const events = fs.readFileSync(log, "utf8").trim().split("\n").filter(Boolean) + // All four contended and all four got in; nobody fell through unlocked. + expect(events.filter((e) => e.startsWith("timeout")).length).toBe(0) + expect(events.filter((e) => e.startsWith("enter")).length).toBe(4) + // Every enter must be followed by its own exit before the next enter. + let inside: string | undefined + for (const line of events) { + const [kind, id] = line.split(" ") + if (kind === "enter") { + expect(inside).toBeUndefined() + inside = id + } else if (kind === "exit") { + expect(inside).toBe(id) + inside = undefined + } + } + }, 60_000) + + test("takes the lock when its container does not exist yet", async () => { + // The cold-start shape this change exists for. The lock sits beside the + // managed directory, and on a fresh machine nothing has created the XDG data + // directory yet — `performInstall` is the first thing that does, and it runs + // *after* the lock is taken. A non-recursive mkdir would fail ENOENT, take + // the "cannot lock" branch, and drop every concurrent CLI into an unlocked + // install: exactly the stampede the lock is meant to stop. + const target = path.join(dir, "fresh", "xdg", "altimate-code", "drivers") + expect(fs.existsSync(installLockPath(target))).toBe(false) + + let sawAcquired: boolean | undefined + await withInstallLock( + target, + async (acquired) => { + sawAcquired = acquired + }, + { timeoutMs: 5000, pollMs: 20 }, + ) + expect(sawAcquired).toBe(true) + }) + + test("waits behind more than one peer without falling through unlocked", async () => { + // Every process counts its deadline from its own start, so a single budget + // only ever outlasts ONE holder. With three contenders the last one's + // deadline expires part-way through somebody else's install and it runs + // performInstall unlocked — the concurrent npm mutation the lock exists to + // prevent. The hold below is deliberately longer than the timeout so the + // test fails unless the wait is extended each time the lock changes hands. + const target = path.join(dir, "drivers") + fs.mkdirSync(target, { recursive: true }) + const log = path.join(dir, "peers.txt") + + const child = path.join(dir, "peer.ts") + fs.writeFileSync( + child, + `import fs from "node:fs" +import { withInstallLock } from ${JSON.stringify(resolveModule)} +const [target, log, id] = process.argv.slice(2) +await withInstallLock(target, async (acquired) => { + fs.appendFileSync(log, \`\${acquired ? "locked" : "UNLOCKED"} \${id}\\n\`) + if (acquired) await new Promise((r) => setTimeout(r, 400)) +}, { timeoutMs: 600, pollMs: 20 }) +process.exit(0) +`, + ) + + const kids = Array.from({ length: 3 }, (_, i) => + Bun.spawn(["bun", child, target, log, String(i)], { stdout: "ignore", stderr: "ignore" }), + ) + expect(await Promise.all(kids.map((k) => k.exited))).toEqual([0, 0, 0]) + + const events = fs.readFileSync(log, "utf8").trim().split("\n").filter(Boolean) + expect(events.length).toBe(3) + // Three holds of 400ms against a 600ms budget: the third can only succeed + // if watching the lock change hands renewed its wait. + expect(events.filter((e) => e.startsWith("UNLOCKED"))).toEqual([]) + }, 60_000) + + test("reports the section ran unlocked when the lock cannot be taken in time", async () => { + const target = path.join(dir, "drivers") + fs.mkdirSync(target, { recursive: true }) + // Hold the lock with a live owner so it cannot be judged stale. + const held = plantLock(target, { pid: process.pid, hostname: os.hostname(), startedAt: Date.now() }) + + let sawAcquired: boolean | undefined + await withInstallLock( + target, + async (acquired) => { + sawAcquired = acquired + }, + { timeoutMs: 200, pollMs: 20 }, + ) + // The work still runs — refusing to install because a peer is slow would + // turn contention into a hard failure — but it knows it was unlocked. + expect(sawAcquired).toBe(false) + // A lock we did not take must not be deleted on the way out. + expect(fs.existsSync(held)).toBe(true) + }) + + test("breaks a lock whose owner is gone", async () => { + const target = path.join(dir, "drivers") + fs.mkdirSync(target, { recursive: true }) + // PID 0x7FFFFFFF is not a live process on any platform we run on. + plantLock(target, { pid: 0x7fffffff, hostname: os.hostname(), startedAt: Date.now() }) + + let sawAcquired: boolean | undefined + await withInstallLock( + target, + async (acquired) => { + sawAcquired = acquired + }, + { timeoutMs: 5000, pollMs: 20 }, + ) + expect(sawAcquired).toBe(true) + }) + + test("breaks a lock left by another host once it has outlived any plausible install", async () => { + // Age is the only signal available for a lock written by a different host + // sharing a home directory, because its pid means nothing here. + const target = path.join(dir, "drivers") + fs.mkdirSync(target, { recursive: true }) + plantLock(target, { + pid: process.pid, + hostname: `${os.hostname()}-other`, + startedAt: Date.now() - 10 * 60_000, + }) + + let sawAcquired: boolean | undefined + await withInstallLock( + target, + async (acquired) => { + sawAcquired = acquired + }, + { timeoutMs: 5000, staleAfterMs: 60_000, pollMs: 20 }, + ) + expect(sawAcquired).toBe(true) + }) + + test("does not age out a live owner on this host", async () => { + // npm can legitimately run longer than any age we pick — a native build such + // as oracledb, or a caller that raised its own install timeout. Breaking a + // live owner's lock would put two npm runs over the same tree, which is the + // corruption this lock exists to prevent. Where liveness is decidable it is + // what counts. + const target = path.join(dir, "drivers") + fs.mkdirSync(target, { recursive: true }) + const held = plantLock(target, { + pid: process.pid, + hostname: os.hostname(), + startedAt: Date.now() - 60 * 60_000, + }) + + let sawAcquired: boolean | undefined + await withInstallLock( + target, + async (acquired) => { + sawAcquired = acquired + }, + { timeoutMs: 200, staleAfterMs: 1_000, hardStaleAfterMs: 24 * 60 * 60_000, pollMs: 20 }, + ) + // Waited, then proceeded unlocked rather than stealing a running install. + expect(sawAcquired).toBe(false) + expect(fs.existsSync(held)).toBe(true) + }) + + test("breaks a live-looking lock once past the hard backstop", async () => { + // `processExists` answers "some process holds this pid", not "our installer + // is still running". A crashed owner's pid can be recycled by an unrelated + // long-lived process, and liveness alone would then keep that lock forever — + // every later install waiting out its timeout and running unlocked. The + // backstop bounds that without interrupting any real install. + const target = path.join(dir, "drivers") + fs.mkdirSync(target, { recursive: true }) + plantLock(target, { pid: process.pid, hostname: os.hostname(), startedAt: Date.now() - 48 * 60 * 60_000 }) + + let sawAcquired: boolean | undefined + await withInstallLock( + target, + async (acquired) => { + sawAcquired = acquired + }, + { timeoutMs: 5000, staleAfterMs: 1_000, hardStaleAfterMs: 60_000, pollMs: 20 }, + ) + expect(sawAcquired).toBe(true) + }) + + test("gives up by the deadline when a stale lock cannot be claimed", async () => { + // A claim can fail persistently: a lock owned by another user, or a + // container that permits inspection but not rename. Retrying that without + // yielding spins at full CPU and never reaches the deadline, so this test + // hangs rather than fails if the bound is lost. + const target = path.join(dir, "drivers") + fs.mkdirSync(target, { recursive: true }) + plantLock(target, { pid: 0x7fffffff, hostname: os.hostname(), startedAt: Date.now() }) + const container = installLockPath(target) + fs.chmodSync(container, 0o555) + + const started = Date.now() + try { + await withInstallLock(target, async () => {}, { timeoutMs: 300, pollMs: 20 }) + } finally { + fs.chmodSync(container, 0o755) + } + // Bounded either way: root can still rename and simply acquires the lock. + expect(Date.now() - started).toBeLessThan(10_000) + }, 30_000) + + test("does not delete a lock that has been re-taken by a peer", async () => { + // A lock we hold can be broken as stale and re-acquired by someone else + // while our critical section is still running. Releasing by pathname would + // then delete the successor's live lock and admit a third process, so the + // release only removes a lock still carrying our own token. + const target = path.join(dir, "drivers") + fs.mkdirSync(target, { recursive: true }) + const held = heldPath(target) + + await withInstallLock(target, async (acquired) => { + expect(acquired).toBe(true) + // A peer breaks our lock and takes its own. + fs.writeFileSync( + path.join(held, "owner.json"), + JSON.stringify({ pid: process.pid, hostname: os.hostname(), startedAt: Date.now(), token: "successor" }), + ) + }) + + expect(fs.existsSync(held)).toBe(true) + }) + + test("does not delete a successor lock that has no owner record yet", async () => { + // The lock directory is created before `owner.json` is written, so a + // successor can hold a live lock carrying no token at all. Identity of the + // directory itself is what settles ownership in that window. + const target = path.join(dir, "drivers") + fs.mkdirSync(target, { recursive: true }) + const held = heldPath(target) + + await withInstallLock(target, async (acquired) => { + expect(acquired).toBe(true) + // Replace the lock with a different directory carrying no owner record. + fs.rmSync(held, { recursive: true, force: true }) + fs.mkdirSync(held, { recursive: true }) + }) + + expect(fs.existsSync(held)).toBe(true) + }) + + test("releases the lock when the critical section throws", async () => { + const target = path.join(dir, "drivers") + fs.mkdirSync(target, { recursive: true }) + let thrown = "" + try { + await withInstallLock(target, async () => { + throw new Error("boom") + }) + } catch (e) { + thrown = e instanceof Error ? e.message : String(e) + } + expect(thrown).toBe("boom") + expect(fs.existsSync(heldPath(target))).toBe(false) + }) +}) diff --git a/packages/drivers/test/resolve-argv-isolation.test.ts b/packages/drivers/test/resolve-argv-isolation.test.ts new file mode 100644 index 000000000..196738c14 --- /dev/null +++ b/packages/drivers/test/resolve-argv-isolation.test.ts @@ -0,0 +1,109 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test" +import fs from "node:fs" +import os from "node:os" +import path from "node:path" + +import { loadOptionalDriver } from "../src/resolve" + +// `@mapbox/node-pre-gyp` resolves a native module's manifest by parsing the +// HOST APPLICATION's `process.argv`. `find()` passes `argv: process.argv` into +// its own `Run`, `nopt` abbreviation-matches our flags against node-pre-gyp's +// option list, and `node-pre-gyp.js:164` then does +// +// package_json_path = path.join(this.opts.directory, package_json_path) +// +// `path.join`, not `path.resolve` — so an absolute manifest path is not +// discarded. Our `--dir` abbreviates to `--directory`, and the driver ended up +// looking for its manifest at `<--dir value>` + the manifest's absolute path. +// +// The fixture below reproduces exactly that arithmetic. It does not stand in +// for node-pre-gyp in general; it stands in for the one line that broke. + +const FIXTURE = "altimate-argv-fixture" +const savedArgv = process.argv + +let root = "" +let nodeModules = "" +let pkgDir = "" + +beforeEach(() => { + root = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), "resolve-argv-"))) + nodeModules = path.join(root, "node_modules") + pkgDir = path.join(nodeModules, FIXTURE) + fs.mkdirSync(pkgDir, { recursive: true }) + fs.writeFileSync( + path.join(pkgDir, "package.json"), + JSON.stringify({ name: FIXTURE, version: "1.0.0", main: "index.js" }), + ) + fs.writeFileSync( + path.join(pkgDir, "index.js"), + [ + "const path = require('path')", + "const i = process.argv.indexOf('--dir')", + "const directory = i !== -1 ? process.argv[i + 1] : undefined", + "const manifest = path.join(__dirname, 'package.json')", + "module.exports = {", + " sawDirFlag: directory !== undefined,", + " manifestPath: directory ? path.join(directory, manifest) : manifest,", + "}", + "", + ].join("\n"), + ) +}) + +afterEach(() => { + process.argv = savedArgv + if (root) fs.rmSync(root, { recursive: true, force: true }) +}) + +/** Reach the fixture the way the real failure does: via the harvested root. */ +function importerThrowingAt(target: string) { + const ambient = Object.assign(new Error(`ENOENT: no such file or directory, open '${target}'`), { code: "ENOENT" }) + return async () => { + throw ambient + } +} + +describe("a driver load does not see the host's command line", () => { + test("the loaded module cannot observe --dir", async () => { + process.argv = [savedArgv[0], savedArgv[1], "run", "--dir", "/some/project", "--print-logs"] + + const loaded: any = await loadOptionalDriver( + "duckdb", + FIXTURE, + importerThrowingAt(path.join(pkgDir, "package.json")), + ) + const mod = loaded?.default ?? loaded + + // Without argv neutralisation the fixture sees the flag and joins, which is + // precisely what sent the driver after a manifest that never existed. + expect(mod.sawDirFlag).toBe(false) + expect(mod.manifestPath).toBe(path.join(pkgDir, "package.json")) + expect(mod.manifestPath.startsWith("/some/project")).toBe(false) + }) + + test("restores the command line afterwards", async () => { + const argv = [savedArgv[0], savedArgv[1], "run", "--dir", "/some/project"] + process.argv = argv + + await loadOptionalDriver("duckdb", FIXTURE, importerThrowingAt(path.join(pkgDir, "package.json"))) + + expect(process.argv).toEqual(argv) + }) + + test("restores the command line even when the load throws", async () => { + fs.writeFileSync(path.join(pkgDir, "index.js"), "throw new Error('broken driver')\n") + const argv = [savedArgv[0], savedArgv[1], "run", "--dir", "/some/project"] + process.argv = argv + + let failed = false + try { + await loadOptionalDriver("duckdb", FIXTURE, importerThrowingAt(path.join(pkgDir, "package.json"))) + } catch { + failed = true + } + + expect(failed).toBe(true) + expect(process.argv).toEqual(argv) + }) +}) diff --git a/packages/drivers/test/resolve-chdir.test.ts b/packages/drivers/test/resolve-chdir.test.ts new file mode 100644 index 000000000..650ea0464 --- /dev/null +++ b/packages/drivers/test/resolve-chdir.test.ts @@ -0,0 +1,136 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test" +import fs from "node:fs" +import os from "node:os" +import path from "node:path" + +import { createRequire } from "node:module" +import { pathToFileURL } from "node:url" + +import { driverSearchRoots, loadOptionalDriver, resolveOptionalPackage } from "../src/resolve" + +// Specifiers that exist nowhere but the tree each test builds. Asking for a +// real driver name would let the repo's own `packages/drivers/node_modules` +// satisfy the lookup through the execPath and module-location roots — which no +// environment isolation can suppress — so the test would pass while proving +// nothing about which root actually won. +const FIXTURE = "altimate-chdir-fixture" +const CWD_ONLY = "altimate-cwd-only-fixture" +const MARKER = "resolved-from-the-fixture-tree" + +// Six pilots were spent on a driver-load failure that only appeared under +// `--dir`, which calls `process.chdir()` (cli/cmd/run.ts) before any driver is +// loaded. Every local verification — and the rig's own pre-flight probe — ran +// without a chdir, so a green suite said nothing about the configuration that +// actually failed. +// +// This arm exists so that stops being true. Resolution must not depend on the +// working directory the process happens to hold when a driver is loaded, and a +// regression that reintroduces a cwd anchor has to fail here. + +let root = "" +let pkgRoot = "" +let nodeModules = "" +let elsewhere = "" +const originalCwd = process.cwd() + +function writePackage(dir: string, name: string, main: string, body: string) { + const pkgDir = path.join(dir, name) + fs.mkdirSync(pkgDir, { recursive: true }) + fs.writeFileSync(path.join(pkgDir, "package.json"), JSON.stringify({ name, version: "1.0.0", main })) + fs.writeFileSync(path.join(pkgDir, main), body) + return pkgDir +} + +beforeEach(() => { + root = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), "resolve-chdir-"))) + pkgRoot = path.join(root, "lib", "node_modules", "altimate-code") + nodeModules = path.join(pkgRoot, "node_modules") + fs.mkdirSync(nodeModules, { recursive: true }) + writePackage(nodeModules, FIXTURE, "index.js", `module.exports = { marker: ${JSON.stringify(MARKER)} }\n`) + elsewhere = path.join(root, "unrelated-run-dir") + fs.mkdirSync(elsewhere, { recursive: true }) +}) + +afterEach(() => { + process.chdir(originalCwd) + if (root) fs.rmSync(root, { recursive: true, force: true }) +}) + +describe("resolution does not depend on the working directory", () => { + test("resolves the same package before and after a chdir", () => { + const before = resolveOptionalPackage(FIXTURE, [nodeModules]) + expect(before).toBeDefined() + + process.chdir(elsewhere) + const after = resolveOptionalPackage(FIXTURE, [nodeModules]) + expect(after).toBe(before) + }) + + test("resolves from a directory that is not an ancestor of the package", () => { + // `elsewhere` shares only the temp root with the package tree, so nothing + // about it can contribute to resolution. This is the rig's shape: the run + // directory and the install tree are unrelated. + process.chdir(elsewhere) + const resolved = resolveOptionalPackage(FIXTURE, [nodeModules]) + expect(resolved).toBeDefined() + expect(resolved!.startsWith(nodeModules)).toBe(true) + // Load it and read the marker, so the test reports which root satisfied the + // lookup rather than merely that something was found. + const loaded = createRequire(pathToFileURL(resolved!).href)(resolved!) + expect(loaded.marker).toBe(MARKER) + }) + + test("does not resolve out of the working directory's own node_modules", () => { + // A package present only under cwd must stay invisible: project trees are + // workspace-controlled executable content and are deliberately not searched. + const cwdModules = path.join(elsewhere, "node_modules") + fs.mkdirSync(cwdModules, { recursive: true }) + writePackage(cwdModules, CWD_ONLY, "index.js", `module.exports = { marker: ${JSON.stringify(MARKER)} }\n`) + + process.chdir(elsewhere) + // The specifier exists nowhere else on the machine, so this is absence with + // a known cause: anything but undefined means the lookup reached into the + // working directory. + expect(resolveOptionalPackage(CWD_ONLY, driverSearchRoots())).toBeUndefined() + }) + + test("loads from the package's own directory while cwd is somewhere else", async () => { + // Resolution being cwd-independent is not enough: the *load* consults the + // package manifest too, and in a compiled binary that lookup was observed + // resolving against the process working directory — + // `ENOENT ... open '/usr/lib/.../duckdb/package.json'` for a file that + // exists at that path without the prefix. This pins the load itself. + // + // The fixture reports its own __dirname, so the assertion is about which + // directory the module was loaded from rather than merely that it loaded. + const pkgDir = path.join(nodeModules, FIXTURE) + fs.writeFileSync(path.join(pkgDir, "index.js"), "module.exports = { dir: __dirname }\n") + + process.chdir(elsewhere) + + // The only route to the fixture root is the path named in the ambient + // failure, which is how the real failure surfaces it. + const named = path.join(pkgDir, "package.json") + const ambient = Object.assign(new Error(`ENOENT: no such file or directory, open '${named}'`), { + code: "ENOENT", + }) + const importer = async () => { + throw ambient + } + + const loaded: any = await loadOptionalDriver("duckdb", FIXTURE, importer) + const mod = loaded?.default ?? loaded + expect(mod.dir).toBe(fs.realpathSync(pkgDir)) + }) + + test("a chdir between resolve and re-resolve does not change the answer", () => { + process.chdir(elsewhere) + const first = resolveOptionalPackage(FIXTURE, [nodeModules]) + process.chdir(originalCwd) + const second = resolveOptionalPackage(FIXTURE, [nodeModules]) + process.chdir(root) + const third = resolveOptionalPackage(FIXTURE, [nodeModules]) + expect(second).toBe(first) + expect(third).toBe(first) + }) +}) diff --git a/packages/drivers/test/resolve-cwd-prefix.test.ts b/packages/drivers/test/resolve-cwd-prefix.test.ts new file mode 100644 index 000000000..2045e38a6 --- /dev/null +++ b/packages/drivers/test/resolve-cwd-prefix.test.ts @@ -0,0 +1,370 @@ +import { afterAll, afterEach, beforeAll, describe, expect, test } from "bun:test" +import fs from "node:fs" +import os from "node:os" +import path from "node:path" + +import { + enclosingNodeModulesRoots, + loadOptionalDriver, + repairCwdPrefixedPath, + searchRootsFromError, +} from "../src/resolve" + +// A globally installed CLI (`npm install -g`, package tree under +// /usr/lib/node_modules/altimate-code) run from an unrelated directory reported: +// +// DuckDB driver found at duckdb but failed to load: ENOENT … open +// '/usr/lib/node_modules/altimate-code/node_modules/duckdb/package.json' +// +// Two defects in one line. The runtime concatenated the working directory onto +// an already-absolute path, and our message then named the bare specifier as +// though it were a location on disk — so it read as "found it, could not load +// it" when in fact nothing had been found at all. +// +// Real directories on disk here, because the whole mechanism is path existence. +// +// The end-to-end tests deliberately use specifiers that exist *nowhere* except +// the tree the test builds. `driverSearchRoots()` includes roots derived from +// execPath and this module's own location, which in-tree reach the repository's +// own `packages/drivers/node_modules` — where a real `duckdb` is installed. A +// test asking for `duckdb` would therefore resolve it from the repository no +// matter what the harvesting code did, and pass while proving nothing. A unique +// specifier cannot be satisfied by any root but the harvested one. + +let root = "" +let pkgRoot = "" +let nodeModules = "" + +const HARVESTED_PKG = "altimate-harvest-probe" +const ABSENT_PKG = "altimate-absent-probe" + +/** Build a minimal but real installed package tree. */ +function writePackage(dir: string, name: string, main: string, body: string) { + const pkgDir = path.join(dir, name) + fs.mkdirSync(pkgDir, { recursive: true }) + fs.writeFileSync(path.join(pkgDir, "package.json"), JSON.stringify({ name, version: "1.0.0", main })) + fs.writeFileSync(path.join(pkgDir, main), body) + return pkgDir +} + +/** An ENOENT of the reported shape, naming `real` with the cwd concatenated on. */ +function cwdPrefixedEnoent(real: string) { + return Object.assign(new Error(`ENOENT: no such file or directory, open '${process.cwd()}${real}'`), { + code: "ENOENT", + }) +} + +describe("cwd concatenated onto an absolute path", () => { + beforeAll(() => { + root = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), "resolve-cwd-"))) + pkgRoot = path.join(root, "lib", "node_modules", "altimate-code") + nodeModules = path.join(pkgRoot, "node_modules") + fs.mkdirSync(nodeModules, { recursive: true }) + writePackage(nodeModules, "duckdb", "index.js", "module.exports = { Database: function () {} }\n") + writePackage(nodeModules, HARVESTED_PKG, "index.js", "module.exports = { marker: 'from-harvested-root' }\n") + }) + + afterAll(() => { + if (root) fs.rmSync(root, { recursive: true, force: true }) + }) + + test("repairs a path whose absolute form exists", () => { + const real = path.join(nodeModules, "duckdb", "package.json") + const mangled = process.cwd() + real + expect(repairCwdPrefixedPath(mangled)).toBe(real) + }) + + test("leaves a path that exists alone", () => { + const real = path.join(nodeModules, "duckdb", "package.json") + expect(repairCwdPrefixedPath(real)).toBeUndefined() + }) + + test("leaves a legitimately nested path under cwd alone", () => { + // `/node_modules/x` de-prefixes to `/node_modules/x`, which does not + // exist — so the repair must decline rather than invent a root. + const nested = path.join(process.cwd(), "node_modules", "definitely-not-here", "package.json") + expect(repairCwdPrefixedPath(nested)).toBeUndefined() + }) + + test("declines when the de-prefixed path does not exist either", () => { + const mangled = process.cwd() + path.join(root, "nope", "package.json") + expect(repairCwdPrefixedPath(mangled)).toBeUndefined() + }) + + test("reports the driver failure when the working directory is unavailable", async () => { + // process.cwd() throws ENOENT once the working directory is removed out from + // under the process. Repair and diagnostics both run *while a driver failure + // is being formatted*, so an unavailable cwd must degrade to "no repair" + // rather than replace the fault the reader needs with an unrelated ENOENT + // raised by the reporting path itself. + // + // The condition is forced rather than staged: deleting a real working + // directory does not make process.cwd() throw on macOS, so a test that + // removed a directory would silently assert nothing on this platform. + const realCwd = process.cwd.bind(process) + process.cwd = () => { + throw Object.assign(new Error("ENOENT: no such file or directory, uv_cwd"), { code: "ENOENT" }) + } + try { + const real = path.join(nodeModules, "duckdb", "package.json") + expect(repairCwdPrefixedPath(`/somewhere${real}`)).toBeUndefined() + + const ambient = Object.assign(new Error("ENOENT: no such file or directory, open '/nowhere/pkg.json'"), { + code: "ENOENT", + }) + let message = "" + try { + await loadOptionalDriver("duckdb", ABSENT_PKG, async () => { + throw ambient + }) + } catch (e) { + message = e instanceof Error ? e.message : String(e) + } + // The driver fault survives, and the diagnostics say plainly that the + // process could not see its own working directory. + expect(message).toContain("failed to load from the default module resolution") + expect(message).toContain("cwd=") + expect(message).not.toContain("uv_cwd") + } finally { + process.cwd = realCwd + } + }) + + test("does not mistake a sibling directory sharing a prefix for the cwd", () => { + // cwd `/a/work` against `/a/workspace/…` shares a textual prefix but is a + // different directory; de-prefixing there would invent a nonsense path. + const sibling = `${process.cwd()}space` + expect(repairCwdPrefixedPath(path.join(sibling, "lib", "pkg.json"))).toBeUndefined() + }) + + test("harvests the node_modules root the runtime named", () => { + const error = cwdPrefixedEnoent(path.join(nodeModules, "duckdb", "package.json")) + expect(searchRootsFromError(error)).toContain(nodeModules) + }) + + test("harvests nothing from an error naming no usable path", () => { + expect(searchRootsFromError(new Error("something went wrong"))).toEqual([]) + expect(searchRootsFromError(new Error("open '/no/such/place/pkg/package.json'"))).toEqual([]) + }) + + test("loads a package from the location the failing runtime named", async () => { + // Reproduces the reported failure exactly: ambient resolution throws ENOENT + // naming the correct absolute path with cwd concatenated on, and nothing + // else on this machine can see that tree. + const ambient = cwdPrefixedEnoent(path.join(nodeModules, HARVESTED_PKG, "package.json")) + + let call = 0 + const importer = async (spec: string) => { + call++ + if (call === 1) throw ambient + return await import(/* @vite-ignore */ spec) + } + + const mod: any = await loadOptionalDriver("duckdb", HARVESTED_PKG, importer) + // Identity, not shape: proves the harvested root is what satisfied the load. + expect((mod.default ?? mod).marker).toBe("from-harvested-root") + }) + + test("does not claim a location when nothing was found", async () => { + const ambient = Object.assign(new Error("ENOENT: no such file or directory, open '/nowhere/pkg.json'"), { + code: "ENOENT", + }) + const importer = async () => { + throw ambient + } + + let message = "" + try { + await loadOptionalDriver("duckdb", ABSENT_PKG, importer) + } catch (e) { + message = e instanceof Error ? e.message : String(e) + } + // The old text was `found at duckdb but failed to load: …`, naming the bare + // specifier as a place on disk. + expect(message).not.toContain(`found at ${ABSENT_PKG}`) + expect(message).toContain("failed to load from the default module resolution") + }) +}) + +describe("enclosing node_modules roots", () => { + test("finds the root in a platform-native path", () => { + expect(enclosingNodeModulesRoots("/a/node_modules/pkg/index.js", "/")).toEqual(["/a/node_modules"]) + }) + + test("returns every enclosing root, innermost first", () => { + // A quoted path often runs through a driver's own dependency. The innermost + // root holds that dependency; the *outer* one holds the driver being looked + // for, so returning only the innermost left the driver unfindable. + expect(enclosingNodeModulesRoots("/opt/node_modules/duckdb/node_modules/node-addon-api/x.js", "/")).toEqual([ + "/opt/node_modules/duckdb/node_modules", + "/opt/node_modules", + ]) + }) + + test("returns nothing when the path names no node_modules", () => { + expect(enclosingNodeModulesRoots("/a/b/index.js", "/")).toEqual([]) + }) + + test("handles a Windows path quoted with forward slashes", () => { + // Windows runtimes quote both shapes. The marker is built from the platform + // separator, so without normalisation a forward-slash path would never match + // a backslash marker and nothing would be harvested at all. + expect(enclosingNodeModulesRoots("C:/app/node_modules/pkg/index.js", "\\")).toEqual(["C:\\app\\node_modules"]) + }) + + test("handles a Windows path quoted with backslashes", () => { + expect(enclosingNodeModulesRoots("C:\\app\\node_modules\\pkg\\index.js", "\\")).toEqual(["C:\\app\\node_modules"]) + }) + + test("handles a Windows path with mixed separators", () => { + expect(enclosingNodeModulesRoots("C:\\app/node_modules\\pkg/index.js", "\\")).toEqual(["C:\\app\\node_modules"]) + }) +}) + +describe("harvested roots do not preempt the managed installation", () => { + const PRIORITY_PKG = "altimate-priority-probe" + let managedRoot = "" + let strayRoot = "" + let savedDriverDir: string | undefined + + beforeAll(() => { + savedDriverDir = process.env["ALTIMATE_DRIVER_DIR"] + managedRoot = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), "resolve-managed-"))) + strayRoot = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), "resolve-stray-"))) + writePackage(path.join(managedRoot, "node_modules"), PRIORITY_PKG, "index.js", "module.exports={marker:'managed'}\n") + writePackage(path.join(strayRoot, "node_modules"), PRIORITY_PKG, "index.js", "module.exports={marker:'stray'}\n") + process.env["ALTIMATE_DRIVER_DIR"] = managedRoot + }) + + afterAll(() => { + if (savedDriverDir === undefined) delete process.env["ALTIMATE_DRIVER_DIR"] + else process.env["ALTIMATE_DRIVER_DIR"] = savedDriverDir + for (const dir of [managedRoot, strayRoot]) if (dir) fs.rmSync(dir, { recursive: true, force: true }) + }) + + test("prefers the driver we installed over a copy the error happened to name", async () => { + // A harvested root is evidence about wherever the runtime pointed, which may + // be a stale or broken copy. driverSearchRoots() puts the managed install + // first precisely so a driver we installed wins; harvesting must not undo + // that by jumping the queue. + const ambient = Object.assign( + new Error( + `ENOENT: no such file or directory, open '${path.join(strayRoot, "node_modules", PRIORITY_PKG, "package.json")}'`, + ), + { code: "ENOENT" }, + ) + let call = 0 + const importer = async (spec: string) => { + call++ + if (call === 1) throw ambient + return await import(/* @vite-ignore */ spec) + } + + const mod: any = await loadOptionalDriver("duckdb", PRIORITY_PKG, importer) + expect((mod.default ?? mod).marker).toBe("managed") + }) +}) + +describe("harvested roots respect the workspace boundary", () => { + let workspace = "" + let outside = "" + let outsideModules = "" + const originalCwd = process.cwd() + + beforeAll(() => { + // Its own tree: the first describe's afterAll has already removed that one + // by the time this block runs. + outside = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), "resolve-outside-"))) + outsideModules = path.join(outside, "node_modules") + writePackage(outsideModules, "duckdb", "index.js", "module.exports = {}\n") + }) + + afterAll(() => { + if (outside) fs.rmSync(outside, { recursive: true, force: true }) + }) + + afterEach(() => { + process.chdir(originalCwd) + if (workspace) fs.rmSync(workspace, { recursive: true, force: true }) + workspace = "" + }) + + test("refuses a node_modules inside the working directory", () => { + // driverSearchRoots() deliberately never searches project node_modules: + // importing a workspace-controlled SDK during a warehouse read/test would + // bypass the permission boundary and can expose resolved credentials. + // Mining a path out of an error message must not route around that. + workspace = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), "resolve-ws-"))) + const projectModules = path.join(workspace, "node_modules") + writePackage(projectModules, "duckdb", "index.js", "module.exports = {}\n") + process.chdir(workspace) + + const error = new Error(`ENOENT: no such file or directory, open '${path.join(projectModules, "duckdb", "package.json")}'`) + expect(searchRootsFromError(error)).not.toContain(projectModules) + expect(searchRootsFromError(error)).toEqual([]) + }) + + test("refuses a node_modules in an ancestor of the working directory", () => { + workspace = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), "resolve-ws-"))) + const ancestorModules = path.join(workspace, "node_modules") + writePackage(ancestorModules, "duckdb", "index.js", "module.exports = {}\n") + const nested = path.join(workspace, "packages", "app") + fs.mkdirSync(nested, { recursive: true }) + process.chdir(nested) + + const error = new Error(`ENOENT: no such file or directory, open '${path.join(ancestorModules, "duckdb", "package.json")}'`) + expect(searchRootsFromError(error)).not.toContain(ancestorModules) + }) + + test("refuses a symlinked root whose target is inside the working directory", () => { + // The containment check must compare real paths. `isDirectory` follows + // symlinks, so a link whose lexical path sits outside the workspace but + // whose target sits inside it would otherwise pass a lexical exclusion and + // import workspace-controlled code anyway. + workspace = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), "resolve-ws-"))) + const real = path.join(workspace, "inside") + writePackage(path.join(real, "node_modules"), "duckdb", "index.js", "module.exports = {}\n") + // The link lives outside the workspace and points back into it. + const linkHome = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), "resolve-link-"))) + const link = path.join(linkHome, "node_modules") + fs.symlinkSync(path.join(real, "node_modules"), link, "dir") + process.chdir(workspace) + + try { + const error = new Error(`ENOENT: no such file or directory, open '${path.join(link, "duckdb", "package.json")}'`) + expect(searchRootsFromError(error)).toEqual([]) + } finally { + fs.rmSync(linkHome, { recursive: true, force: true }) + } + }) + + test("harvests nothing at all when the working directory cannot be established", () => { + // Fail closed. With no cwd there is nothing to compare a root against, and + // treating that as "not workspace content" would admit every root an error + // names — turning the one case where the process cannot see its own + // filesystem into the case with no boundary at all. + const realCwd = process.cwd.bind(process) + process.cwd = () => { + throw Object.assign(new Error("ENOENT: no such file or directory, uv_cwd"), { code: "ENOENT" }) + } + try { + const error = new Error( + `ENOENT: no such file or directory, open '${path.join(outsideModules, "duckdb", "package.json")}'`, + ) + expect(searchRootsFromError(error)).toEqual([]) + } finally { + process.cwd = realCwd + } + }) + + test("still harvests a root outside the workspace", () => { + // The exclusion must not swallow the case the harvesting exists for. + workspace = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), "resolve-ws-"))) + process.chdir(workspace) + + const error = new Error( + `ENOENT: no such file or directory, open '${path.join(outsideModules, "duckdb", "package.json")}'`, + ) + expect(searchRootsFromError(error)).toContain(outsideModules) + }) +}) diff --git a/packages/drivers/test/resolve-windows-shapes.test.ts b/packages/drivers/test/resolve-windows-shapes.test.ts new file mode 100644 index 000000000..c36e0d426 --- /dev/null +++ b/packages/drivers/test/resolve-windows-shapes.test.ts @@ -0,0 +1,81 @@ +import { describe, expect, test } from "bun:test" + +import { enclosingNodeModulesRoots, installLockPath, quotedAbsolutePaths } from "../src/resolve" + +// Windows path shapes, unit-testable from any platform because none of these +// touch the filesystem. UNC is the shape that keeps getting left out: a share +// root behaves like a drive root, and code that only special-cases `C:` gets it +// wrong in a way that is invisible until someone runs from a network share. + +describe("installLockPath keeps a root intact", () => { + test("puts the lock inside a POSIX root", () => { + // Stripping the separator would leave "", making the lock the *relative* + // ".lock" — a different file for every working directory. + expect(installLockPath("/")).toBe("/.lock") + }) + + test("puts the lock inside a drive root", () => { + // "C:.lock" is drive-relative, not the root of C:. + expect(installLockPath("C:\\")).toBe("C:\\.lock") + }) + + test("puts the lock inside a UNC share root", () => { + // The bug this pins: "\\\\server\\share" + ".lock" names the *share* + // "\\\\server\\share.lock", a different network location entirely, so two + // processes installing into the share would not share a lock. + expect(installLockPath("\\\\server\\share\\")).toBe("\\\\server\\share\\.lock") + expect(installLockPath("\\\\server\\share")).toBe("\\\\server\\share\\.lock") + }) + + test("still strips a trailing separator on an ordinary directory", () => { + expect(installLockPath("/home/u/drivers/")).toBe("/home/u/drivers.lock") + expect(installLockPath("/home/u/drivers")).toBe("/home/u/drivers.lock") + expect(installLockPath("\\\\server\\share\\drivers\\")).toBe("\\\\server\\share\\drivers.lock") + }) + + test("a directory deeper than the share root is not treated as a root", () => { + expect(installLockPath("\\\\server\\share\\a")).toBe("\\\\server\\share\\a.lock") + }) +}) + +describe("quotedAbsolutePaths covers every absolute shape", () => { + test("POSIX", () => { + expect(quotedAbsolutePaths(`open '/usr/lib/node_modules/x/package.json'`)).toEqual([ + "/usr/lib/node_modules/x/package.json", + ]) + }) + + test("drive letter, both separators", () => { + expect(quotedAbsolutePaths(`open "C:\\app\\node_modules\\x\\package.json"`)).toEqual([ + "C:\\app\\node_modules\\x\\package.json", + ]) + expect(quotedAbsolutePaths(`open "C:/app/node_modules/x/package.json"`)).toEqual([ + "C:/app/node_modules/x/package.json", + ]) + }) + + test("UNC share", () => { + // Without the UNC alternative this yields nothing, so a driver on a share + // stays unfindable even though the error named its exact location. + expect(quotedAbsolutePaths(`ENOENT: open '\\\\server\\share\\node_modules\\duckdb\\package.json'`)).toEqual([ + "\\\\server\\share\\node_modules\\duckdb\\package.json", + ]) + }) + + test("ignores relative paths and bare words", () => { + expect(quotedAbsolutePaths(`Cannot find package 'duckdb' from 'lib/x.js'`)).toEqual([]) + }) +}) + +describe("enclosingNodeModulesRoots on Windows separators", () => { + test("walks back through a UNC path, innermost first", () => { + const roots = enclosingNodeModulesRoots( + "\\\\server\\share\\app\\node_modules\\a\\node_modules\\duckdb\\package.json", + "\\", + ) + expect(roots).toEqual([ + "\\\\server\\share\\app\\node_modules\\a\\node_modules", + "\\\\server\\share\\app\\node_modules", + ]) + }) +}) diff --git a/packages/opencode/src/altimate/tools/warehouse-install-driver.ts b/packages/opencode/src/altimate/tools/warehouse-install-driver.ts index 7b52b523b..7a1387129 100644 --- a/packages/opencode/src/altimate/tools/warehouse-install-driver.ts +++ b/packages/opencode/src/altimate/tools/warehouse-install-driver.ts @@ -8,6 +8,7 @@ import { loadOptionalDriver, driverInstallDir, driverLabel, + installLockPath, installOptionalDriver, isDriverInstalled, npmInstallArgs, @@ -85,6 +86,14 @@ export const WarehouseInstallDriverTool = Tool.define("warehouse_install_driver" const packages = DRIVER_PACKAGES[driver].join(" ") const externalPattern = FSUtil.normalizePathPattern(path.join(dir, "*")) + // The cross-process install lock is a sibling of the managed directory, not + // a child of it — it lives outside so npm never treats it as stray package + // content. That puts it outside the pattern above, so it has to be approved + // explicitly: this tool creates, writes and removes that directory, and + // asking for `/*` alone would mutate an external path the user never + // agreed to. + const lockPattern = FSUtil.normalizePathPattern(path.join(installLockPath(dir), "*")) + const externalPatterns = [externalPattern, lockPattern] const installCommand = ["npm", ...npmInstallArgs(DRIVER_PACKAGES[driver])].join(" ") // This tool bypasses the bash and edit tools, so it must broker the same @@ -93,9 +102,9 @@ export const WarehouseInstallDriverTool = Tool.define("warehouse_install_driver" // choose only a driver enum, never shell text or package names. await ctx.ask({ permission: "external_directory", - patterns: [externalPattern], - always: [externalPattern], - metadata: { driver, dir }, + patterns: externalPatterns, + always: externalPatterns, + metadata: { driver, dir, lockDir: installLockPath(dir) }, }) await ctx.ask({ permission: "bash", diff --git a/packages/opencode/test/altimate/warehouse-install-driver-permission.test.ts b/packages/opencode/test/altimate/warehouse-install-driver-permission.test.ts index d4b6bf549..a844ad11a 100644 --- a/packages/opencode/test/altimate/warehouse-install-driver-permission.test.ts +++ b/packages/opencode/test/altimate/warehouse-install-driver-permission.test.ts @@ -51,13 +51,19 @@ describe("warehouse_install_driver permissions", () => { ) const dir = DriverResolve.driverInstallDir() + // The cross-process install lock is a sibling of the managed directory, not + // a child of it, so `/*` does not cover it. The tool creates, writes + // and removes that directory, so it has to be approved explicitly rather + // than mutated as an unapproved external path. + const lockDir = DriverResolve.installLockPath(dir) + expect(lockDir).toBe(`${dir}.lock`) expect(events).toEqual(["ask:external_directory", "ask:bash", "install"]) expect(requests).toEqual([ { permission: "external_directory", - patterns: [path.join(dir, "*")], - always: [path.join(dir, "*")], - metadata: { driver: "postgres", dir }, + patterns: [path.join(dir, "*"), path.join(lockDir, "*")], + always: [path.join(dir, "*"), path.join(lockDir, "*")], + metadata: { driver: "postgres", dir, lockDir }, }, { permission: "bash",