Repository navigation
fix(connectors): load the native clients inside the execute path - #264
Merged
Merged
Conversation
A Worker product that imports only /catalog and /specs — to describe connectors, never to execute one — could not bundle. specs/registry reaches bundled-manifests, which re-exports every adapter so their static manifests can be read, and two adapters imported a native client at module top level. Reading manifest DATA therefore pulled @duckdb/node-api and ssh2-sftp-client's cpu-features into the bundle, and a Worker cannot load a .node file at all. Both clients now load when a query or a transfer runs. The types stay static and erase. A source-graph test walks the two entries and fails on a native import that returns, and asserts it can still see one so it cannot pass vacuously. Closes #263
tangletools
approved these changes
Aug 9, 2026
tangletools
left a comment
Collaborator
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — e5db016f
This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.
tangletools · auto-approval · reason: drewstone_author · 2026-08-09T23:42:53Z
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #263.
/catalogand/specsdescribe connectors. A Worker product reads them to render an integrations page or build a tool list, and never executes one — but both entries statically reached two native Node clients, so the bundle failed on a file the product never intended to run:The path is
specs/registry→bundled-manifests→import * as bundledAdapters from './adapters/index.js', which re-exports every adapter so their static manifests can be read.duckdb.tsimported@duckdb/node-apiandsftp.tsimportedssh2-sftp-clientat module top level, so reading manifest data pulled both clients in.The manifest is data; only execution needs a client. Both now load inside the execute path, and the types stay static — they erase.
Proof
The graph, walked before and after (from the built
dist/catalog.js+dist/specs.js):chunk-4JDM3HRZ.js -> @duckdb/node-apiA test that fails when it comes back.
src/worker-safe-subpaths.test.tswalks the source graph from both entries, skippingimport type(erases) andawait import(the point). Restoring one static import turns it red and names the file:Its third case points the same matcher at a known-bad line and asserts one hit, so the two green assertions cannot be vacuous.
Suite: 685 files / 5,158 tests passed.
pnpm typecheckexit 0,pnpm buildexit 0.What it unblocks
legal-agentis pinned to^0.52.0today purely because of this — the only product off the latest integrations. It moves to latest as soon as this publishes.