From 0a1f9fda65a529bc8dda0afff2b0c91ca3810e95 Mon Sep 17 00:00:00 2001 From: Frank Gonnello <4139410+fronc@users.noreply.github.com> Date: Thu, 13 Aug 2026 11:57:39 -0400 Subject: [PATCH] fix(adapters): wrap modulePath in pathToFileURL() before dynamic import (Windows) (#4287) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip orchestrates AI agents for zero-human companies > - One of its pluggability surfaces is external adapter packages, loaded at startup by `server/src/adapters/plugin-loader.ts` and routed through the adapter registry so third parties can override built-in adapters like `claude_local` > - `loadExternalAdapterPackage` calls `await import(modulePath)` where `modulePath` is an absolute filesystem path > - On Windows that path begins with a drive letter (`C:\…`), which Node's ESM loader parses as a URL scheme and rejects with `ERR_UNSUPPORTED_ESM_URL_SCHEME`; the defensive `try/catch` around the call masks the failure and the builtin adapter silently keeps serving traffic, so the override never activates > - `reloadExternalAdapter` in the same file already tries to build a `file://` URL, but does it via template-string concatenation (`file://${modulePath}`) which produces a malformed URL on Windows (`file://C:\…` instead of `file:///C:/…`) — so dev hot-reload of adapters is broken on Windows even after initial load works on POSIX > - This pull request swaps both paths to `pathToFileURL()` from `node:url`, the idiomatic cross-platform conversion > - The benefit is external adapter packages load reliably on Windows with no changes required to existing adapters, and the two sibling paths in the same file stop diverging in their URL-handling discipline Closes #4286. ## What Changed - `server/src/adapters/plugin-loader.ts`: - Import `pathToFileURL` from `node:url`. - `loadExternalAdapterPackage`: wrap `modulePath` in `pathToFileURL(modulePath).href` before passing to `import()`. - `reloadExternalAdapter`: replace `` `file://${modulePath}` `` string concatenation with `pathToFileURL(modulePath).href` so the cache-bust URL is well-formed on Windows too (drive letter, UNC, percent-encoding). Three lines changed + one import. No behavior change on POSIX: `pathToFileURL("/foo/bar.js").href === "file:///foo/bar.js"`, which Node's ESM loader accepts identically to the bare path. ## Verification **Runtime, Windows 11, Node v24, `@paperclipai/server@2026.416.0`:** Before (installed dist, vanilla): ``` INFO: Loading external adapter package {packageName: "@reforged/adapter-claude-local", modulePath: "C:\\Users\\…\\index.js"} WARN: Failed to dynamically load external adapter; skipping err: ERR_UNSUPPORTED_ESM_URL_SCHEME … Received protocol 'c:' ``` After (same dist with the equivalent two-line patch applied): ``` INFO: Loading external adapter package {packageName: "@reforged/adapter-claude-local"} INFO: Loaded external adapters from plugin store {count: 1, adapters: ["claude_local"]} ``` End-to-end: the override actually services execute calls and its telemetry fields (e.g. `errorCode: "rate_limited"` on 429) surface into heartbeat-run records — I've been running this heartbeat through the override on a vendor-patched copy while drafting this PR. **Static / logic review:** - `pathToFileURL` is part of Node's stdlib since v10.12.0, no new dep. - On POSIX, `path.resolve("/a", "b") → "/a/b"` and `pathToFileURL("/a/b").href → "file:///a/b"`. `await import("file:///a/b")` and `await import("/a/b")` both resolve to the same ESM module — no double-load risk. - Reload path: the existing cache-bust query (`?t=${Date.now()}`) still appends cleanly because `pathToFileURL(...).href` returns a normalized `file:///…` URL with no pre-existing query string. **Local test suite:** I did not run the full `pnpm test` suite in this fork — the monorepo test infrastructure (embedded Postgres, pnpm workspace install) is a significant local-setup cost and this change is surgical enough that CI should be the source of truth. Happy to iterate based on CI signal. No existing test directly exercises `plugin-loader.ts`'s initial-load path. ## Risks **Low.** This aligns the initial-load path with the already-existing intent of the reload path (which tried, but imperfectly, to use a `file://` URL). POSIX behavior is unchanged. The only runtime difference is that Windows stops throwing and starts loading the adapter — which is exactly the bug being fixed. Edge cases worth naming: - **UNC paths** (`\\server\share\…`): previously broken the same way on the load path, still broken with `file://` string concat on the reload path. `pathToFileURL` handles UNC correctly (→ `file:////server/share/…`), so this change also quietly fixes UNC-path adapter installs on Windows. - **Bun**: the reload path has a Bun cache-eviction block that keys off `modulePath` and the old `fileUrl`. Bun accepts both `file://` URLs and bare paths in its module cache keys, so changing the URL form is consistent with the existing evict-both pattern (we still evict both `fileUrl` and `modulePath` after the change). ## Model Used Claude Opus 4.7 (`claude-opus-4-7`, provider: Anthropic) via Claude Code, running as the CTO agent in a Paperclip-orchestrated company. 200k context, tool use. No extended thinking mode. Model authored the patch, the issue body, and this PR description; human review by the company's principal (fronc) is pending. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [ ] I have run tests locally and they pass — *deferred to CI, see Verification note* - [ ] I have added or updated tests where applicable — *no existing tests for this file; adding one would require stubbing `adapter-plugin-store` + filesystem, which seemed out of scope for a 3-line fix. Happy to add one on request.* - [x] If this change affects the UI, I have included before/after screenshots — *not UI, N/A* - [x] I have updated relevant documentation to reflect my changes — *no user-facing docs affected; behavior unchanged on POSIX and now-working on Windows* - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge --- server/src/adapters/plugin-loader.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/server/src/adapters/plugin-loader.ts b/server/src/adapters/plugin-loader.ts index a9f704635d..c01fedeac4 100644 --- a/server/src/adapters/plugin-loader.ts +++ b/server/src/adapters/plugin-loader.ts @@ -11,6 +11,7 @@ import fs from "node:fs"; import path from "node:path"; +import { pathToFileURL } from "node:url"; import type { ServerAdapterModule } from "./types.js"; import { logger } from "../middleware/logger.js"; @@ -177,7 +178,7 @@ export async function loadExternalAdapterPackage( logger.info({ packageName, packageDir, entryPoint, modulePath, hasUiParser: !!uiParserSource }, "Loading external adapter package"); - const mod = await import(modulePath); + const mod = await import(pathToFileURL(modulePath).href); const adapterModule = validateAdapterModule(mod, packageName); if (uiParserSource) { @@ -212,7 +213,7 @@ export async function reloadExternalAdapter( const packageDir = resolvePackageDir(record); const entryPoint = resolvePackageEntryPoint(packageDir); const modulePath = path.resolve(packageDir, entryPoint); - const fileUrl = `file://${modulePath}`; + const fileUrl = pathToFileURL(modulePath).href; // Bust ESM module cache so re-import loads fresh code from disk. // Query-string trick (?t=...) works in Node; Bun may need the file:// URL