From 600349e65aad72b3ab4cbc49d174fe70995ff6fa Mon Sep 17 00:00:00 2001 From: thekevinbot <248217931+thekevinbot@users.noreply.github.com> Date: Fri, 18 Sep 2026 10:30:26 -0400 Subject: [PATCH 1/4] test(rust): red contract for planning a programmatic extension path Add `extension_resolution::plan_extension_path` with its unit tests, implementation stubbed. Pushed on its own so CI is observed red before the implementation lands. The config-entry planner already lives in the core (#1119); the programmatic `extensions=[{path}]` probe is the last piece both SDKs still port from one another. Part of #1122. --- .../2026-09-18-programmatic-extension-plan.md | 8 ++ packages/rust/src/extension_resolution.rs | 85 ++++++++++++++++++- 2 files changed, 92 insertions(+), 1 deletion(-) create mode 100644 packages/rust/changelog.d/2026-09-18-programmatic-extension-plan.md diff --git a/packages/rust/changelog.d/2026-09-18-programmatic-extension-plan.md b/packages/rust/changelog.d/2026-09-18-programmatic-extension-plan.md new file mode 100644 index 000000000..07feebee7 --- /dev/null +++ b/packages/rust/changelog.d/2026-09-18-programmatic-extension-plan.md @@ -0,0 +1,8 @@ +**Added** + +`dirsql::extension_resolution::plan_extension_path` plans a single extension +path given outside a config file, the form a constructor's `extensions=[{path}]` +entry takes: path-looking values stay literal (made absolute against a base only +when asked), bare names become a package lookup with a shadow probe. The Python +and TypeScript SDKs each still carry a port of that probe; they migrate onto +this next. diff --git a/packages/rust/src/extension_resolution.rs b/packages/rust/src/extension_resolution.rs index 402765d03..a75fc3427 100644 --- a/packages/rust/src/extension_resolution.rs +++ b/packages/rust/src/extension_resolution.rs @@ -169,6 +169,24 @@ fn spec_is_bare_name(extension: &ExtensionSpec, suffixes: &[&str]) -> bool { .is_some_and(|path| is_bare_name(path, suffixes)) } +/// Plan one extension path given outside a config file. +/// +/// A constructor's `extensions=[{path}]` entry takes the same ordered probe as +/// a config entry: a path-looking value is literal, made absolute against +/// `base` only when `resolve_relative` is set (config semantics; programmatic +/// entries keep a relative path verbatim), and a bare name is a package whose +/// shadow probe sits under `base`. The entry's `entrypoint` travels with the +/// caller, so the returned entry carries none. +pub fn plan_extension_path( + path: &str, + base: &Path, + resolve_relative: bool, + suffixes: &[&str], +) -> PlanEntry { + let _ = (path, base, resolve_relative, suffixes); + unimplemented!("plan_extension_path") +} + /// Pick the single loadable file for package `name` out of a host's listing. /// /// `dirs` are the package's directories (used only for the not-found message); @@ -209,7 +227,8 @@ pub fn select_loadable( #[cfg(test)] mod tests { use super::{ - ConfigSource, PlanEntry, SelectError, is_bare_name, plan_config_extensions, select_loadable, + ConfigSource, PlanEntry, SelectError, is_bare_name, plan_config_extensions, + plan_extension_path, select_loadable, }; use std::path::{Path, PathBuf}; @@ -357,6 +376,70 @@ mod tests { ); } + #[test] + fn a_programmatic_relative_path_is_made_absolute_only_when_asked() { + let base = Path::new("/cwd"); + assert_eq!( + plan_extension_path("ext/vec0.so", base, true, PY), + PlanEntry::Literal { + path: PathBuf::from("/cwd/ext/vec0.so"), + entrypoint: None, + } + ); + assert_eq!( + plan_extension_path("ext/vec0.so", base, false, PY), + PlanEntry::Literal { + path: PathBuf::from("ext/vec0.so"), + entrypoint: None, + } + ); + } + + #[test] + fn a_programmatic_absolute_path_is_left_alone() { + assert_eq!( + plan_extension_path("/opt/vec0.so", Path::new("/cwd"), true, PY), + PlanEntry::Literal { + path: PathBuf::from("/opt/vec0.so"), + entrypoint: None, + } + ); + } + + #[test] + fn a_programmatic_bare_name_plans_a_package_shadowed_by_base() { + for resolve_relative in [true, false] { + assert_eq!( + plan_extension_path("sqlite_vec", Path::new("/cwd"), resolve_relative, PY), + PlanEntry::Package { + name: "sqlite_vec".into(), + shadow: PathBuf::from("/cwd/sqlite_vec"), + entrypoint: None, + } + ); + } + } + + #[test] + fn a_programmatic_entry_uses_the_hosts_own_suffix_list() { + let base = Path::new("/cwd"); + assert_eq!( + plan_extension_path("vec0.node", base, true, PY), + PlanEntry::Package { + name: "vec0.node".into(), + shadow: PathBuf::from("/cwd/vec0.node"), + entrypoint: None, + } + ); + assert_eq!( + plan_extension_path("vec0.node", base, true, NODE), + PlanEntry::Literal { + path: PathBuf::from("/cwd/vec0.node"), + entrypoint: None, + } + ); + } + #[test] fn select_loadable_returns_the_single_matching_file() { let dir = PathBuf::from("/site-packages/sqlite_vec"); From c05317742ecadafeabc1f5de5ccffa75fa983756 Mon Sep 17 00:00:00 2001 From: thekevinbot <248217931+thekevinbot@users.noreply.github.com> Date: Fri, 18 Sep 2026 10:37:23 -0400 Subject: [PATCH 2/4] refactor: plan the programmatic extension path in the core Implement `extension_resolution::plan_extension_path` and route both SDKs' `resolve_extension_path` / `resolveExtensionPath` through it, so a programmatic `extensions=[{path}]` entry is classified by the same code as a config entry and each host keeps only the package lookup it alone can do. The `is_bare_name` / `isBareName` binding exports were that probe's only callers, so they go; `plan_extension_path` / `planExtensionPath` replaces them. Resolved paths and error messages are unchanged. Part of #1122. --- PARITY.md | 13 +- .../2026-09-18-programmatic-extension-plan.md | 8 ++ packages/python/dirsql/_dirsql.pyi | 4 +- packages/python/dirsql/resolve_extension.py | 20 +--- .../python/dirsql/resolve_extension_test.py | 62 +++------- packages/python/src/lib.rs | 40 +++++-- packages/rust/src/extension_resolution.rs | 18 ++- .../2026-09-18-programmatic-extension-plan.md | 8 ++ packages/ts/napi/src/lib.rs | 44 +++++-- packages/ts/src/core.test.ts | 8 +- packages/ts/src/core.ts | 6 +- packages/ts/src/resolve-extension.test.ts | 113 +++++------------- packages/ts/src/resolve-extension.ts | 27 ++--- .../integration/hermetic/extensions.test.ts | 30 +++-- 14 files changed, 200 insertions(+), 201 deletions(-) create mode 100644 packages/python/changelog.d/2026-09-18-programmatic-extension-plan.md create mode 100644 packages/ts/changelog.d/2026-09-18-programmatic-extension-plan.md diff --git a/PARITY.md b/PARITY.md index 1b76c6df5..c9eea144b 100644 --- a/PARITY.md +++ b/PARITY.md @@ -143,11 +143,16 @@ since [#1119](https://github.com/thekevinscott/dirsql/issues/1119) they live once in `dirsql::extension_resolution` (`#[doc(hidden)]`, not a public Rust API). Both SDKs reach it through their binding — [#1120](https://github.com/thekevinscott/dirsql/issues/1120) for Python -(`dirsql._dirsql.plan_config_extensions` / `select_loadable` / `is_bare_name`), +(`dirsql._dirsql.plan_config_extensions` / `plan_extension_path` / +`select_loadable`), [#1121](https://github.com/thekevinscott/dirsql/issues/1121) for TypeScript -(the addon's `planConfigExtensions` / `selectLoadable` / `isBareName`). Each -host keeps only what it alone can do: reading the config files, and locating an -installed package (`importlib.util.find_spec` / `require.resolve`). +(the addon's `planConfigExtensions` / `planExtensionPath` / `selectLoadable`). +The same probe covers a *programmatic* `extensions=[{path}]` entry, planned by +`plan_extension_path` since +[#1122](https://github.com/thekevinscott/dirsql/issues/1122); the `is_bare_name` +/ `isBareName` export it replaced is gone. Each host keeps only what it alone +can do: reading the config files, and locating an installed package +(`importlib.util.find_spec` / `require.resolve`). **Config-flag scanning breadth ([#754](https://github.com/thekevinscott/dirsql/issues/754) / diff --git a/packages/python/changelog.d/2026-09-18-programmatic-extension-plan.md b/packages/python/changelog.d/2026-09-18-programmatic-extension-plan.md new file mode 100644 index 000000000..2487a953e --- /dev/null +++ b/packages/python/changelog.d/2026-09-18-programmatic-extension-plan.md @@ -0,0 +1,8 @@ +**Changed** + +- `resolve_extension_path` now plans through the core + (`_dirsql.plan_extension_path`) instead of re-implementing the ordered probe, + so a programmatic `extensions=[{path}]` entry is classified by the same code + as a config entry. `importlib` package lookup is unchanged, and so is the + resolved path. The internal `_dirsql.is_bare_name` binding export is gone, + replaced by `plan_extension_path`. No change to the published API. (#1122) diff --git a/packages/python/dirsql/_dirsql.pyi b/packages/python/dirsql/_dirsql.pyi index 62f809bb9..6504a4774 100644 --- a/packages/python/dirsql/_dirsql.pyi +++ b/packages/python/dirsql/_dirsql.pyi @@ -97,4 +97,6 @@ def plan_config_extensions( configs: list[tuple[str, str | None]], ) -> list[ExtensionPlanEntry] | None: ... def select_loadable(name: str, dirs: list[str], candidates: list[str]) -> str: ... -def is_bare_name(path: str) -> bool: ... +def plan_extension_path( + path: str, base: str, resolve_relative: bool +) -> ExtensionPlanEntry: ... diff --git a/packages/python/dirsql/resolve_extension.py b/packages/python/dirsql/resolve_extension.py index 158dea4a6..43997608d 100644 --- a/packages/python/dirsql/resolve_extension.py +++ b/packages/python/dirsql/resolve_extension.py @@ -1,7 +1,8 @@ """Resolve an extension entry's ``path`` to a concrete loadable file. -Resolution is an ordered probe (file-first, then package), so only a bare -package name reaches the package machinery: +Resolution is an ordered probe (file-first, then package), planned by the core +(``_dirsql.plan_extension_path``) and carried out here: only locating an +installed package is host-specific. 1. **Path-looking** (contains a separator, or ends in ``.so`` / ``.dylib`` / ``.dll`` / ``.pyd``) -- returned as a file path: made absolute against @@ -13,10 +14,8 @@ picked from inside it. """ -import os - -from ._dirsql import is_bare_name -from .resolve_package import _resolve_package +from ._dirsql import plan_extension_path +from .plan_entry_path import _plan_entry_path def resolve_extension_path(path, base, resolve_relative): @@ -28,11 +27,4 @@ def resolve_extension_path(path, base, resolve_relative): against ``base`` (config-file semantics); when false it is returned verbatim (programmatic semantics). """ - if not is_bare_name(path): - if resolve_relative and not os.path.isabs(path): - return os.path.join(base, path) - return path - local = os.path.join(base, path) - if os.path.isfile(local): - return local - return _resolve_package(path) + return _plan_entry_path(plan_extension_path(path, base, resolve_relative)) diff --git a/packages/python/dirsql/resolve_extension_test.py b/packages/python/dirsql/resolve_extension_test.py index e949d8525..8e526f8a8 100644 --- a/packages/python/dirsql/resolve_extension_test.py +++ b/packages/python/dirsql/resolve_extension_test.py @@ -1,8 +1,7 @@ """Unit tests for `resolve_extension_path`. -The effectful collaborators -- the filesystem probe (`os.path.isfile`) and the -package resolver -- are mocked, so these isolate the ordered probe from any -real package or disk. The core's pure `is_bare_name` stays real. +Both collaborators are mocked: the core's planner and the entry-to-path step +that owns the filesystem probe and the package lookup. """ from unittest import mock @@ -10,51 +9,26 @@ import dirsql.resolve_extension as mod -def describe_path_looking_values(): - def it_makes_a_relative_path_absolute_when_resolve_relative(): - out = mod.resolve_extension_path("ext/a.so", base="/cfg", resolve_relative=True) - assert out == "/cfg/ext/a.so" - - def it_preserves_an_absolute_path_when_resolve_relative(): - out = mod.resolve_extension_path( - "/abs/a.so", base="/cfg", resolve_relative=True - ) - assert out == "/abs/a.so" - - def it_returns_a_path_verbatim_when_not_resolve_relative(): - out = mod.resolve_extension_path( - "relative/a.so", base="/cfg", resolve_relative=False - ) - assert out == "relative/a.so" - - -def describe_bare_names(): - def it_uses_a_same_named_local_file_when_present(): +def describe_resolve_extension_path(): + def it_plans_through_the_core_and_resolves_the_planned_entry(): + entry = object() with ( - mock.patch.object(mod.os.path, "isfile", return_value=True) as isfile, - mock.patch.object(mod, "_resolve_package") as resolve_package, - ): - out = mod.resolve_extension_path("vec", base="/cfg", resolve_relative=True) - assert out == "/cfg/vec" - isfile.assert_called_once_with("/cfg/vec") - resolve_package.assert_not_called() - - def it_resolves_the_package_when_no_local_file_shadows_it(): - with ( - mock.patch.object(mod.os.path, "isfile", return_value=False), + mock.patch.object(mod, "plan_extension_path", return_value=entry) as plan, mock.patch.object( - mod, "_resolve_package", return_value="/site/vec/vec0.so" - ) as resolve_package, + mod, "_plan_entry_path", return_value="/site/vec/vec0.so" + ) as entry_path, ): - out = mod.resolve_extension_path("vec", base="/cfg", resolve_relative=True) + out = mod.resolve_extension_path("vec", base="/cfg", resolve_relative=False) + assert out == "/site/vec/vec0.so" - resolve_package.assert_called_once_with("vec") + plan.assert_called_once_with("vec", "/cfg", False) + entry_path.assert_called_once_with(entry) - def it_probes_the_shadow_file_even_when_not_resolve_relative(): + def it_passes_resolve_relative_through_to_the_planner(): with ( - mock.patch.object(mod.os.path, "isfile", return_value=True) as isfile, - mock.patch.object(mod, "_resolve_package"), + mock.patch.object(mod, "plan_extension_path") as plan, + mock.patch.object(mod, "_plan_entry_path"), ): - out = mod.resolve_extension_path("vec", base="/cfg", resolve_relative=False) - assert out == "/cfg/vec" - isfile.assert_called_once_with("/cfg/vec") + mod.resolve_extension_path("ext/a.so", base="/cfg", resolve_relative=True) + + plan.assert_called_once_with("ext/a.so", "/cfg", True) diff --git a/packages/python/src/lib.rs b/packages/python/src/lib.rs index b390e5059..af496bdab 100644 --- a/packages/python/src/lib.rs +++ b/packages/python/src/lib.rs @@ -573,10 +573,19 @@ mod python { .map_err(|err| err.to_string()) } - /// Whether an extension entry names a package rather than a file. + /// Plan one extension path given outside a config file. + /// + /// `base` is the directory a relative path and the bare-name shadow probe + /// resolve against; `resolve_relative` makes a relative path-looking value + /// absolute against it (config semantics) rather than verbatim. #[pyfunction] - fn is_bare_name(path: &str) -> bool { - extension_resolution::is_bare_name(path, PY_SUFFIXES) + fn plan_extension_path(path: &str, base: &str, resolve_relative: bool) -> PyExtensionPlanEntry { + plan_entry_to_py(extension_resolution::plan_extension_path( + path, + Path::new(base), + resolve_relative, + PY_SUFFIXES, + )) } #[pymodule] @@ -591,7 +600,7 @@ mod python { m.add_function(wrap_pyfunction!(config_paths_from_argv, m)?)?; m.add_function(wrap_pyfunction!(plan_config_extensions, m)?)?; m.add_function(wrap_pyfunction!(select_loadable, m)?)?; - m.add_function(wrap_pyfunction!(is_bare_name, m)?)?; + m.add_function(wrap_pyfunction!(plan_extension_path, m)?)?; m.add_class::()?; Ok(()) } @@ -763,11 +772,24 @@ mod python { } #[test] - fn is_bare_name_applies_the_cpython_suffix_list() { - assert!(is_bare_name("sqlite_vec")); - assert!(!is_bare_name("vec0.pyd")); - assert!(is_bare_name("vec0.node")); - assert!(!is_bare_name("ext/vec0")); + fn a_programmatic_path_marshals_as_a_literal_entry() { + let entry = plan_extension_path("ext/vec0.so", "/cwd", false); + assert_eq!(entry.path.as_deref(), Some("ext/vec0.so")); + assert_eq!(entry.package, None); + assert_eq!( + plan_extension_path("ext/vec0.so", "/cwd", true) + .path + .as_deref(), + Some("/cwd/ext/vec0.so") + ); + } + + #[test] + fn a_programmatic_entry_applies_the_cpython_suffix_list() { + let entry = plan_extension_path("vec0.node", "/cwd", false); + assert_eq!(entry.package.as_deref(), Some("vec0.node")); + assert_eq!(entry.shadow.as_deref(), Some("/cwd/vec0.node")); + assert_eq!(plan_extension_path("vec0.pyd", "/cwd", false).package, None); } #[test] diff --git a/packages/rust/src/extension_resolution.rs b/packages/rust/src/extension_resolution.rs index a75fc3427..ab5f729c7 100644 --- a/packages/rust/src/extension_resolution.rs +++ b/packages/rust/src/extension_resolution.rs @@ -183,8 +183,22 @@ pub fn plan_extension_path( resolve_relative: bool, suffixes: &[&str], ) -> PlanEntry { - let _ = (path, base, resolve_relative, suffixes); - unimplemented!("plan_extension_path") + if is_bare_name(path, suffixes) { + return PlanEntry::Package { + name: path.to_owned(), + shadow: base.join(path), + entrypoint: None, + }; + } + PlanEntry::Literal { + // `join` yields an absolute path verbatim. + path: if resolve_relative { + base.join(path) + } else { + PathBuf::from(path) + }, + entrypoint: None, + } } /// Pick the single loadable file for package `name` out of a host's listing. diff --git a/packages/ts/changelog.d/2026-09-18-programmatic-extension-plan.md b/packages/ts/changelog.d/2026-09-18-programmatic-extension-plan.md new file mode 100644 index 000000000..1bd406c31 --- /dev/null +++ b/packages/ts/changelog.d/2026-09-18-programmatic-extension-plan.md @@ -0,0 +1,8 @@ +**Changed** + +- `resolveExtensionPath` now plans through the core (the addon's + `planExtensionPath`) instead of re-implementing the ordered probe, so a + programmatic `extensions: [{ path }]` entry is classified by the same code as + a config entry. `require.resolve` package lookup is unchanged, and so is the + resolved path. The internal `isBareName` addon export is gone, replaced by + `planExtensionPath`. No change to the published API. (#1122) diff --git a/packages/ts/napi/src/lib.rs b/packages/ts/napi/src/lib.rs index 5afcac236..a9f0a296c 100644 --- a/packages/ts/napi/src/lib.rs +++ b/packages/ts/napi/src/lib.rs @@ -149,10 +149,23 @@ fn selected_loadable( .map_err(|err| err.to_string()) } -/// Whether an extension entry names a package rather than a file. -#[napi(js_name = "isBareName")] -pub fn is_bare_name(path: String) -> bool { - extension_resolution::is_bare_name(&path, NODE_SUFFIXES) +/// Plan one extension path given outside a config file. +/// +/// `base` is the directory a relative path and the bare-name shadow probe +/// resolve against; `resolveRelative` makes a relative path-looking value +/// absolute against it (config semantics) rather than verbatim. +#[napi(js_name = "planExtensionPath")] +pub fn plan_extension_path( + path: String, + base: String, + resolve_relative: bool, +) -> ExtensionPlanEntry { + plan_entry_to_js(extension_resolution::plan_extension_path( + &path, + Path::new(&base), + resolve_relative, + NODE_SUFFIXES, + )) } /// A row-level event emitted by the file watcher. @@ -938,12 +951,23 @@ mod tests { } #[test] - fn is_bare_name_applies_the_node_suffix_list() { - assert!(is_bare_name("sqlite-vec".into())); - assert!(!is_bare_name("vec0.node".into())); - assert!(!is_bare_name("vec0.so".into())); - assert!(is_bare_name("vec0.pyd".into())); - assert!(!is_bare_name("ext/vec0".into())); + fn a_programmatic_path_marshals_as_a_literal_entry() { + let entry = plan_extension_path("ext/vec0.so".into(), "/cwd".into(), false); + assert_eq!(entry.path.as_deref(), Some("ext/vec0.so")); + assert_eq!(entry.package, None); + let resolved = plan_extension_path("ext/vec0.so".into(), "/cwd".into(), true); + assert_eq!(resolved.path.as_deref(), Some("/cwd/ext/vec0.so")); + } + + #[test] + fn a_programmatic_entry_applies_the_node_suffix_list() { + let entry = plan_extension_path("vec0.pyd".into(), "/cwd".into(), false); + assert_eq!(entry.package.as_deref(), Some("vec0.pyd")); + assert_eq!(entry.shadow.as_deref(), Some("/cwd/vec0.pyd")); + assert_eq!( + plan_extension_path("vec0.node".into(), "/cwd".into(), false).package, + None + ); } fn one_row() -> HashMap { diff --git a/packages/ts/src/core.test.ts b/packages/ts/src/core.test.ts index 63410e7db..f5a1a7a62 100644 --- a/packages/ts/src/core.test.ts +++ b/packages/ts/src/core.test.ts @@ -27,7 +27,7 @@ describe("CoreModule contract", () => { const configPathsFromArgv = vi.fn((argv: string[]) => argv); const planConfigExtensions = vi.fn(() => null); const selectLoadable = vi.fn(() => "/d/ext.so"); - const isBareName = vi.fn(() => true); + const planExtensionPath = vi.fn(() => ({ path: "/d/ext.so" })); // Typed against the real interface, not cast: dropping a member from // `CoreModule` would make this literal fail to compile, which pins the // surface the SDK reaches for before the core parses argv. @@ -36,7 +36,7 @@ describe("CoreModule contract", () => { configPathsFromArgv, planConfigExtensions, selectLoadable, - isBareName, + planExtensionPath, }; expect(core.configPathsFromArgv(["-c", "a.toml"])).toEqual([ "-c", @@ -45,7 +45,9 @@ describe("CoreModule contract", () => { expect(configPathsFromArgv).toHaveBeenCalledWith(["-c", "a.toml"]); expect(core.planConfigExtensions([{ path: "a.toml" }])).toBeNull(); expect(core.selectLoadable("ext", ["/d"], ["/d/ext.so"])).toBe("/d/ext.so"); - expect(core.isBareName("ext")).toBe(true); + expect(core.planExtensionPath("ext.so", "/d", true)).toEqual({ + path: "/d/ext.so", + }); }); }); diff --git a/packages/ts/src/core.ts b/packages/ts/src/core.ts index 282f6526b..091f9f5c4 100644 --- a/packages/ts/src/core.ts +++ b/packages/ts/src/core.ts @@ -63,7 +63,11 @@ export interface CoreModule { // every config's entries itself. planConfigExtensions(configs: ConfigSource[]): ExtensionPlanEntry[] | null; selectLoadable(name: string, dirs: string[], candidates: string[]): string; - isBareName(path: string): boolean; + planExtensionPath( + path: string, + base: string, + resolveRelative: boolean, + ): ExtensionPlanEntry; } // Unit tests `vi.mock("./core.js")` to fake `getCore` directly, so diff --git a/packages/ts/src/resolve-extension.test.ts b/packages/ts/src/resolve-extension.test.ts index 84d214f4c..282f57aad 100644 --- a/packages/ts/src/resolve-extension.test.ts +++ b/packages/ts/src/resolve-extension.test.ts @@ -1,16 +1,11 @@ -import { existsSync, statSync } from "node:fs"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { getCore } from "./core.js"; import { defaultResolver } from "./default-resolver.js"; import type { PackageResolver } from "./package-dir.js"; +import { planEntryPath } from "./plan-entry-path.js"; import { resolveExtensionPath } from "./resolve-extension.js"; -import { resolvePackage } from "./resolve-package.js"; -vi.mock("node:fs", async () => ({ - ...(await vi.importActual("node:fs")), - existsSync: vi.fn(), - statSync: vi.fn(), -})); +vi.mock("./core.js"); vi.mock("./default-resolver.js", async () => ({ ...(await vi.importActual( @@ -19,98 +14,48 @@ vi.mock("./default-resolver.js", async () => ({ defaultResolver: vi.fn(), })); -vi.mock("./core.js"); - -vi.mock("./resolve-package.js", async () => ({ - ...(await vi.importActual( - "./resolve-package.js", +vi.mock("./plan-entry-path.js", async () => ({ + ...(await vi.importActual( + "./plan-entry-path.js", )), - resolvePackage: vi.fn(), + planEntryPath: vi.fn(), })); -const isBareName = vi.fn<(path: string) => boolean>(); +const planExtensionPath = vi.fn(); function fakeResolver(): PackageResolver { - return { - resolve: vi.fn(() => { - throw new Error("not resolvable"); - }), - paths: vi.fn(() => []), - }; + return { resolve: vi.fn(), paths: vi.fn(() => []) }; } describe("resolveExtensionPath", () => { beforeEach(() => { - vi.mocked(getCore).mockReturnValue({ isBareName } as never); + vi.mocked(getCore).mockReturnValue({ planExtensionPath } as never); }); afterEach(() => vi.resetAllMocks()); - describe("path-looking values", () => { - it("makes a relative path absolute when resolveRelative", () => { - isBareName.mockReturnValue(false); - expect(resolveExtensionPath("ext/a.so", "/cfg", true)).toBe( - "/cfg/ext/a.so", - ); - expect(isBareName).toHaveBeenCalledWith("ext/a.so"); - }); - - it("preserves an absolute path when resolveRelative", () => { - isBareName.mockReturnValue(false); - expect(resolveExtensionPath("/abs/a.so", "/cfg", true)).toBe("/abs/a.so"); - }); - - it("returns a path verbatim when not resolveRelative", () => { - isBareName.mockReturnValue(false); - expect(resolveExtensionPath("rel/a.so", "/cfg", false)).toBe("rel/a.so"); - }); - }); - - describe("bare-name shadowing", () => { - it("uses a same-named local file when present", () => { - isBareName.mockReturnValue(true); - vi.mocked(existsSync).mockReturnValue(true); - vi.mocked(statSync).mockReturnValue({ isFile: () => true } as ReturnType< - typeof statSync - >); - expect(resolveExtensionPath("vec", "/cfg", true)).toBe("/cfg/vec"); - expect(resolvePackage).not.toHaveBeenCalled(); - }); - - it("falls through when the same-named local entry is not a file", () => { - isBareName.mockReturnValue(true); - vi.mocked(resolvePackage).mockReturnValue("/nm/pkg/vec0.so"); - vi.mocked(existsSync).mockReturnValue(true); - vi.mocked(statSync).mockReturnValue({ isFile: () => false } as ReturnType< - typeof statSync - >); - expect(resolveExtensionPath("vec", "/cfg", true, fakeResolver())).toBe( - "/nm/pkg/vec0.so", - ); - }); + it("resolves the entry the core plans, with the injected resolver", () => { + const entry = { path: "/abs/a.so" }; + planExtensionPath.mockReturnValue(entry); + vi.mocked(planEntryPath).mockReturnValue("/abs/a.so"); + const resolver = fakeResolver(); + + expect(resolveExtensionPath("a.so", "/cfg", false, resolver)).toBe( + "/abs/a.so", + ); + expect(planExtensionPath).toHaveBeenCalledWith("a.so", "/cfg", false); + expect(planEntryPath).toHaveBeenCalledWith(entry, resolver); + expect(defaultResolver).not.toHaveBeenCalled(); }); - describe("bare-name package resolution", () => { - it("delegates to resolvePackage with the injected resolver", () => { - isBareName.mockReturnValue(true); - vi.mocked(resolvePackage).mockReturnValue("/nm/pkg/vec0.so"); - vi.mocked(existsSync).mockReturnValue(false); - const resolver = fakeResolver(); - expect(resolveExtensionPath("sqlite-vec", "/cfg", true, resolver)).toBe( - "/nm/pkg/vec0.so", - ); - expect(resolvePackage).toHaveBeenCalledWith("sqlite-vec", resolver); - expect(defaultResolver).not.toHaveBeenCalled(); - }); + it("hands defaultResolver() on when none is injected", () => { + const fallback = fakeResolver(); + planExtensionPath.mockReturnValue({ package: "vec", shadow: "/cfg/vec" }); + vi.mocked(defaultResolver).mockReturnValue(fallback); + vi.mocked(planEntryPath).mockReturnValue("/nm/vec/vec0.so"); - it("hands defaultResolver() to resolvePackage when none is injected", () => { - const fallback = fakeResolver(); - isBareName.mockReturnValue(true); - vi.mocked(defaultResolver).mockReturnValue(fallback); - vi.mocked(resolvePackage).mockReturnValue("/nm/pkg/vec0.so"); - vi.mocked(existsSync).mockReturnValue(false); - expect(resolveExtensionPath("x", "/c", true)).toBe("/nm/pkg/vec0.so"); - expect(resolvePackage).toHaveBeenCalledWith("x", fallback); - }); + expect(resolveExtensionPath("vec", "/cfg", true)).toBe("/nm/vec/vec0.so"); + expect(planExtensionPath).toHaveBeenCalledWith("vec", "/cfg", true); + expect(planEntryPath).toHaveBeenCalledWith(expect.anything(), fallback); }); }); diff --git a/packages/ts/src/resolve-extension.ts b/packages/ts/src/resolve-extension.ts index abdfe8781..2bfd9dc45 100644 --- a/packages/ts/src/resolve-extension.ts +++ b/packages/ts/src/resolve-extension.ts @@ -1,6 +1,8 @@ // Resolve an extension entry's `path` to a concrete loadable file. // -// Resolution is an ordered probe (file-first, then package): +// Resolution is an ordered probe (file-first, then package), planned by the +// core (`planExtensionPath`) and carried out here: only locating an installed +// package is host-specific. // // 1. Path-looking (contains a separator, or ends in `.so` / `.dylib` / // `.dll` / `.node`) -> returned as a file path: made absolute against @@ -8,16 +10,12 @@ // verbatim (programmatic entries). // 2. Bare package name -> a same-named local file under `base` shadows the // package; otherwise the package dir is located via `require.resolve` -// and the current platform's loadable is globbed from inside it. Zero -// matches and multiple matches are both hard errors -- disambiguate -// with a literal path. +// and the current platform's loadable is picked from inside it. -import { existsSync, statSync } from "node:fs"; -import { isAbsolute, resolve as resolvePath } from "node:path"; import { getCore } from "./core.js"; import { defaultResolver } from "./default-resolver.js"; import type { PackageResolver } from "./package-dir.js"; -import { resolvePackage } from "./resolve-package.js"; +import { planEntryPath } from "./plan-entry-path.js"; /** * Resolve an extension `path` to a concrete file. `base` is the directory a @@ -32,15 +30,8 @@ export function resolveExtensionPath( resolveRelative: boolean, resolver: PackageResolver = defaultResolver(), ): string { - if (!getCore().isBareName(path)) { - if (resolveRelative && !isAbsolute(path)) { - return resolvePath(base, path); - } - return path; - } - const local = resolvePath(base, path); - if (existsSync(local) && statSync(local).isFile()) { - return local; - } - return resolvePackage(path, resolver); + return planEntryPath( + getCore().planExtensionPath(path, base, resolveRelative), + resolver, + ); } diff --git a/packages/ts/tests/integration/hermetic/extensions.test.ts b/packages/ts/tests/integration/hermetic/extensions.test.ts index 2908f55c7..022f6ee5a 100644 --- a/packages/ts/tests/integration/hermetic/extensions.test.ts +++ b/packages/ts/tests/integration/hermetic/extensions.test.ts @@ -1,8 +1,8 @@ // Hermetic: first-party code runs for real; the mocked boundaries are // `node:fs` probes and `node:module`'s `createRequire` (behind which both -// the napi binary and package resolution sit). The bare-name test and the -// config planner now live in the core, so the fake core answers for them -// here; the real ones are covered by `tests/integration/binding/`. +// the napi binary and package resolution sit). Extension planning now lives +// in the core, so the fake core answers for it here; the real planner is +// covered by `tests/integration/binding/`. import { existsSync, readFileSync, readdirSync, statSync } from "node:fs"; import { DirSQL } from "dirsql"; @@ -13,7 +13,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; const { fakeCore } = vi.hoisted(() => ({ fakeCore: { DirSQL: { openAsync: vi.fn() }, - isBareName: vi.fn(), + planExtensionPath: vi.fn(), selectLoadable: vi.fn(), planConfigExtensions: vi.fn(), }, @@ -46,12 +46,13 @@ vi.mock("node:module", async (importOriginal) => { }); const openAsync = fakeCore.DirSQL.openAsync; -const { isBareName, selectLoadable, planConfigExtensions } = fakeCore; +const { planExtensionPath, selectLoadable, planConfigExtensions } = fakeCore; beforeEach(() => { // Reset (not just clear) so per-test fs implementations never leak. vi.resetAllMocks(); - isBareName.mockReturnValue(false); + // The core plans a path-looking entry as a literal, verbatim. + planExtensionPath.mockImplementation((path: string) => ({ path })); planConfigExtensions.mockReturnValue(null); openAsync.mockResolvedValue({ query: vi.fn().mockResolvedValue([]), @@ -84,7 +85,10 @@ describe("extensions option (hermetic, #230/#299)", () => { }); it("resolves a bare package name to the installed loadable", async () => { - isBareName.mockReturnValue(true); + planExtensionPath.mockReturnValue({ + package: "dirsql-testext-pkg", + shadow: "/cwd/dirsql-testext-pkg", + }); // No same-named local file shadows the package... vi.mocked(existsSync).mockReturnValue(false); // ...and the core picks the one loadable in the resolved package dir. @@ -104,8 +108,10 @@ describe("extensions option (hermetic, #230/#299)", () => { }); it("prefers a same-named local file over the installed package", async () => { - isBareName.mockReturnValue(true); - const cwd = vi.spyOn(process, "cwd").mockReturnValue("/cwd"); + planExtensionPath.mockReturnValue({ + package: "dirsql-testext-pkg", + shadow: "/cwd/dirsql-testext-pkg", + }); vi.mocked(existsSync).mockReturnValue(true); vi.mocked(statSync).mockReturnValue({ isFile: () => true, @@ -119,7 +125,6 @@ describe("extensions option (hermetic, #230/#299)", () => { expect(openAsync.mock.calls[0]?.[6]).toEqual([ { path: "/cwd/dirsql-testext-pkg", entrypoint: undefined }, ]); - cwd.mockRestore(); }); it("resolves config [[dirsql.extension]] package names and suppresses the core's own loading (#313)", async () => { @@ -147,7 +152,10 @@ describe("extensions option (hermetic, #230/#299)", () => { }); it("rejects ready when a bare package name is not installed", async () => { - isBareName.mockReturnValue(true); + planExtensionPath.mockReturnValue({ + package: "not-installed-pkg", + shadow: "/cwd/not-installed-pkg", + }); vi.mocked(existsSync).mockReturnValue(false); const db = new DirSQL({ From 87bcdafb0c96eefcfb639f7fedb31b97a475c8f9 Mon Sep 17 00:00:00 2001 From: thekevinbot <248217931+thekevinbot@users.noreply.github.com> Date: Fri, 18 Sep 2026 10:39:34 -0400 Subject: [PATCH 3/4] e2e attestation for claude/1122-ext-cleanup --- .../python/e2e-attestations/claude-1122-ext-cleanup.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 packages/python/e2e-attestations/claude-1122-ext-cleanup.json diff --git a/packages/python/e2e-attestations/claude-1122-ext-cleanup.json b/packages/python/e2e-attestations/claude-1122-ext-cleanup.json new file mode 100644 index 000000000..6984e823b --- /dev/null +++ b/packages/python/e2e-attestations/claude-1122-ext-cleanup.json @@ -0,0 +1,7 @@ +{ + "command": "just test-e2e", + "ran_at": 1789742374, + "exit_code": 0, + "commit": "c05317742ecadafeabc1f5de5ccffa75fa983756", + "branch": "claude/1122-ext-cleanup" +} From 9319ba94d6e0370b7ee28710a493af9eae63a324 Mon Sep 17 00:00:00 2001 From: thekevinbot <248217931+thekevinbot@users.noreply.github.com> Date: Fri, 18 Sep 2026 10:40:06 -0400 Subject: [PATCH 4/4] e2e attestation for claude/1122-ext-cleanup --- packages/ts/e2e-attestations/claude-1122-ext-cleanup.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 packages/ts/e2e-attestations/claude-1122-ext-cleanup.json diff --git a/packages/ts/e2e-attestations/claude-1122-ext-cleanup.json b/packages/ts/e2e-attestations/claude-1122-ext-cleanup.json new file mode 100644 index 000000000..5f114c55c --- /dev/null +++ b/packages/ts/e2e-attestations/claude-1122-ext-cleanup.json @@ -0,0 +1,7 @@ +{ + "command": "pnpm test:e2e", + "ran_at": 1789742406, + "exit_code": 0, + "commit": "87bcdafb0c96eefcfb639f7fedb31b97a475c8f9", + "branch": "claude/1122-ext-cleanup" +}