MemberJunction / MemberJunction/MJ

Audit MJ for the #3677 bug class: dynamic imports of runtime-configured or undeclared specifiers that break under pnpm

Open
#3,741 3 comments 0 reactions 0 assignees View on GitHub
Dominant language
TSQL
Stars
29
Forks
6
Avg merge
2d 1h
Merged PRs (30d)
323

Description

PR #3717 fixed the two instances of this class that block Open Apps (server-bootstrap registration, open-app-engine migrations), but an adversarial audit of that PR found the **class persists elsewhere**. This issue requests a **full repo-wide audit**, not just fixes for the sites below.

## The bug class

A dynamic `import()` whose specifier is runtime-configured (or simply undeclared by the importing package) resolves from the **importing package**, which under pnpm's strict layout only sees what it declares. npm's hoisting masked every instance of this; pnpm surfaces them one by one as packages are exercised on 6.x. Failure modes: hard `ERR_MODULE_NOT_FOUND` at runtime, or — worse — silent wrong-copy loading when a stale registry copy shadows the intended one.

## Known instances (verified on next @ PR #3717's tip)

| Site | Specifier source | Risk |
|---|---|---|
| `packages/AI/Engine/src/AIEngine.ts:1535` | DB-configured `DriverImportPath` (`await import(driverModuleName)`) — AI Engine declares no driver packages | **High** — the most likely next real-world hit; any AI driver configured via metadata fails to load under pnpm exactly like #3677 |
| `packages/CLICore/src/plugin-registry.ts:98` | bare-specifier fallback for plugin loading | Medium |
| `packages/TestingFramework/CLI/src/utils/check-module-loader.ts:63` | bare-specifier fallback | Low (tooling) |
| `packages/Actions/Runtime/harness/run-demos.ts:50` | `await import(pkg)` over a package list | Low (demo harness) |

## Requested audit scope

1. **Every non-literal dynamic import** across `packages/`: `grep -rnE "await import\((?!['\"])" --include='*.ts'` (plus template-literal and variable-specifier forms), classifying each as: (a) declared by the importer → fine, (b) runtime-configured names → needs host-anchored resolution, (c) undeclared static contract → needs a declaration.
2. **Undeclared static imports** masked by hoisting — the existing dependency-check CI catches some; confirm its coverage includes every `"type": "module"` package and dev-only entry points.
3. Decide the shared mechanism: PR #3717 established two patterns — declare (optionalDependencies for optional runtime deps) and **host-anchored resolution** (`server-bootstrap/src/host-import.ts`: bare import first, then `createRequire` retries anchored at the config that named the package → cwd → process entrypoint, with resolution/evaluation separated so a found module's own failure is never masked). If three or more packages need the latter, it likely deserves a shared utility package rather than more keep-in-sync copies (there are already two: `host-import.ts` and `migration-runner.ts`'s predicate).

## Acceptance

- An inventory of every dynamic-import site with its classification (including the ones judged fine, so coverage is explicit)
- Fixes or tracked follow-ups for every (b)/(c) site
- A CI guard so new sites of this class fail review instead of shipping (the existing dependency-check bot catches static undeclared imports; non-literal dynamic imports currently pass silently)

Found during the audit of PR #3717 (fixes #3677); reported from MJDev.

Contributor guide

Open the contributing guide

Research direction

Start with the listed dynamic-import sites, especially packages/AI/Engine/src/AIEngine.ts, packages/CLICore/src/plugin-registry.ts, packages/TestingFramework/CLI/src/utils/check-module-loader.ts, and packages/Actions/Runtime/harness/run-demos.ts. Run the requested grep across packages/ and review server-bootstrap/src/host-import.ts plus the dependency-check CI coverage. Done means a complete classified inventory, fixes or follow-ups for every problematic site, and a CI guard for new non-literal imports.

Written by the indexing model from the issue text.

Assessment

Tech stack
node.js, typescript
Domain
build-system, tooling
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.