feat(cli): warn on createRequire packages missing from deployed images - #4851
Conversation
Packages loaded via createRequire(import.meta.url)("pkg") are invisible
to the bundler: they are neither bundled nor installed into the deployed
image, and the deploy succeeds silently before failing at runtime with a
module-not-found error. Deploy builds now scan user source files for such
loads, cross-check against the packages actually installed in the image,
and warn with file and line, suggesting the additionalPackages build
extension. Deploys also surface the bundler's own warnings for user files
instead of discarding them.
🦋 Changeset detectedLatest commit: b1a98d5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 27 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 4 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (44)
🧰 Additional context used📓 Path-based instructions (8)We use vitest exclusively. **Never mock anything** - use testcontainers instead.📄 CodeRabbit inference engine (AGENTS.md) Files:
**Prefer static imports over dynamic imports.** Only use dynamic `import()` when:📄 CodeRabbit inference engine (AGENTS.md) Files:
Add crumbs as you write code — not just when debugging. Mark lines with📄 CodeRabbit inference engine (AGENTS.md) Files:
Bundle worker code using the build system in `src/build/` based on configuration from `trigger.config.ts`📄 CodeRabbit inference engine (packages/cli-v3/CLAUDE.md) Files:
Use vitest for all tests in the Trigger.dev repository📄 CodeRabbit inference engine (.github/copilot-instructions.md) Files:
Use function declarations instead of default exports📄 CodeRabbit inference engine (.github/copilot-instructions.md) Files:
Use types over interfaces for TypeScript📄 CodeRabbit inference engine (.github/copilot-instructions.md) Files:
When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc) Files:
🧠 Learnings (1)📚 Learning: 2026-06-04T18:16:35.386ZApplied to files:
🪛 ast-grep (0.45.2)packages/cli-v3/src/build/createRequireWarnings.ts[warning] 79-82: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns. (regexp-from-variable) [warning] 88-91: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns. (regexp-from-variable) [warning] 127-130: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns. (regexp-from-variable) [warning] 131-134: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns. (regexp-from-variable) [warning] 145-145: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns. (regexp-from-variable) [warning] 156-156: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns. (regexp-from-variable) [warning] 157-157: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns. (regexp-from-variable) [warning] 158-161: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns. (regexp-from-variable) 🔇 Additional comments (7)
WalkthroughDeploy builds scan user source for packages loaded through Merge Risk: 🔵 Low · up to Configured externals can suppress the new warning without adding a createRequire-only package to deployed dependencies, so affected deployments may still fail at runtime without a diagnostic. The PR is mergeable with explicit owner awareness or follow-up on this bounded deployment risk. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description provides a detailed summary, implementation scope, configuration guidance, testing coverage, and end-to-end verification. It does not use every template section, including the issue reference, checklist, changelog, and screenshots, but the core PR information is complete.
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…nt noise Match createRequire(fileURLToPath(import.meta.url)) by allowing one level of nested parens in the argument, skip hits on commented-out lines, and only scan files that import the module builtin so unrelated functions named createRequire never warn.
@trigger.dev/build
trigger.dev
@trigger.dev/core
@trigger.dev/python
@trigger.dev/react-hooks
@trigger.dev/redis-worker
@trigger.dev/rsc
@trigger.dev/schema-to-json
@trigger.dev/sdk
commit: |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…warning The warning now carries a note with a copy-pasteable additionalPackages snippet naming trigger.config.ts, instead of only linking the docs.
…eploys won't have The same createRequire scan now runs during dev builds and warns on every build that the package works locally but will be missing from the deployed image, so the problem surfaces while writing the code instead of after a deploy. additionalPackages declares its packages as deploy externals, so a configured fix suppresses the warning in both dev and deploy.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…tics-only suppression Adds the scan/suppression halves of the dev warning (the devSession wiring landed in the previous commit): nested-arg matching feeds both targets, and a usage only warns when the package is missing from the resolved externals and every configured external. additionalPackages declares its packages via a new diagnostics-only BuildExtension field, installedPackagesForTarget, so a configured fix silences the warning without changing bundling output.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…uire detection Scanning now runs on a comment-stripped, template-blanked copy of the source (string-aware, offsets preserved), so commented-out code never registers require names, closed inline comments don't hide real calls, and // inside a string is not mistaken for a comment. Calls are only recognized when the createRequire binding provably comes from the module builtin (named import, namespace member, or CJS destructure), typed require variables and two-level-nested createRequire arguments are matched, whitespace before require( is accepted, and the node_modules skip matches path segments instead of substrings.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…anner
Deploy suppression now uses only the manifest externals (the actual image
contents when the warning runs); configured externals like build.external
no longer silence warnings for packages that are never installed. Dev
suppression adds extension-declared packages and stays silent when those
can't be determined (a hook throws, or an older additionalPackages lacks
the declaration hook), so it never makes a false deploys-will-fail claim,
and additionalPackages skips unparseable entries instead of throwing into
dev startup. The lexer blanks regex-literal bodies and records quoted
string spans so string contents can't false-positive, dynamic
import("node:module") bindings and declare-then-assign variables are
recognized, require functions exported from one file and imported into
another are followed, dev rebuilds cache per-file scans by mtime and stat
in parallel, and the dev/deploy pipelines share one warning builder.
Completes the change described in the prior commit message: the lexer, binding, cross-module and caching work in the scanner, the corrected suppression sources, and the never-throw additionalPackages declaration.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
… createRequire warning Dev warnings stay silent when any hook-bearing extension declares no installed packages (layer installs are invisible in dev), and both targets stay silent when a build-layer command runs a JS package manager, since those installs never reach the manifest externals. The division-vs-regex heuristic handles postfix increments, non-null assertions and JSX closers, query-suffixed metafile inputs are scanned once, cross-file require functions match only when the import path resolves to the exporting file, file reads are concurrency-capped with per-file scans reused instead of recomputed, helper duplication with externals.ts is removed, and the changeset is rewritten as a single user-facing sentence.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…nstall-command suppression Replaces the hand-rolled lexer and regex scanner with a @babel/parser scan (typescript/jsx with fallbacks, parse failures skip the file), eliminating the comment/string/regex/JSX misparse class outright; template-literal specifiers now also match. Cross-file require functions resolve through the metafile's own import records, so index files and path aliases work. Build-layer install commands suppress only the packages they actually name instead of silencing the whole feature, first-party extensions that install no node packages declare that so dev warnings stay active for them, extension matchers are computed before internal extensions are prepended, deploy warning output honors plain mode and keeps location-less messages with a segment-exact node_modules filter, and additionalPackages only claims packages for the deploy target.
…ackagesForTarget externalsForTarget is synchronous; the example showed async, which the build consumes incorrectly and TypeScript rejects. Adds a section for the new diagnostics-only installedPackagesForTarget hook.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…anner edge fixes
Inverts the extension-declaration default: an extension that declares no
installed packages is assumed to install none, so dev warnings stay live
for third-party and yet-to-declare extensions instead of silently
disabling the feature (proven twice by first-party sweeps missing
extensions). The incomplete guard remains only where false warnings are
genuinely likely: a throwing declaration hook, or an additionalPackages
extension too old to declare, and the engine-only prisma mode now
declares @prisma/engines. Also: specifier-form exports (export { req })
are followed cross-file, npm-alias install tokens suppress the aliased
name, literal Windows path specifiers never warn, the bare-specifier
check reuses isBareModuleImport, collector usages swap atomically per
build, exported-name mention checks use identifier boundaries, and
signature-miss re-reads run under the concurrency cap with failures
logged.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
… scanning Removes cross-file require-function following and its collector machinery (two-phase scan, exports-signature cache, metafile import resolution, export-specifier tracking): a require helper imported from another module is no longer followed, which just leaves that pattern unwarned as before this feature. Each file now scans independently with a simple per-file mtime cache. With undeclared extensions assumed to install nothing, the empty installedPackagesForTarget stubs on built-in extensions are removed too; only extensions that actually install packages declare (additionalPackages and the engine-only prisma mode).
Files using decorators parse instead of being skipped, and npm install -g commands no longer suppress the warning for packages task code can't resolve.
2 participants
Summary
The
trigger.dev deployandtrigger.dev devcommands now warn (with the suggested fix) when your code loads a package throughcreateRequire()that won't be available in the deployed image. Previously it would fail at runtime in production to load the package. Deploys also now show bundler warnings for your code instead of discarding them.A package loaded with
createRequire(import.meta.url)("pkg")is invisible to esbuild: the call is never resolved, so the package is neither bundled nor collected as an external to install in the deployed image. The deploy succeeds with zero diagnostics and the task fails at runtime with a module-not-found error, which can surface as something far more confusing when a library maps errors coarsely (a database driver loaded this way can look exactly like a connection failure). It also works fine intrigger devbecause the localnode_modulesexists, making the production-only failure extra misleading.Both
deployanddevbuilds now warn about this, pointing at the exact file and line, with a note showing the exact config that fixes it:In
devthe message instead explains that the code works locally but deploys of it will fail, so the problem is caught while the code is being written rather than after a deploy.How it works
An esbuild plugin scans the bundle's input files outside
node_modulesfor string-literal specifiers passed tocreateRequire-created require functions:createRequire(...)("pkg"),const req = createRequire(...); req("pkg"),req.resolve("pkg"), aliased imports, namespace access, CJS destructuring, and dynamicimport("node:module")bindings. Sources are parsed with@babel/parser(already in the dependency tree), so comments, strings, templates, regex literals and JSX can't confuse the scan; a file that fails to parse is skipped. Relative paths and node builtins never warn.A usage only warns when the package will actually be missing from the image. On deploys the resolved manifest externals are the source of truth (extension-installed layers are already merged in when the warning runs);
build.externalalone deliberately does not suppress, because marking a package external installs nothing when nothing statically imports it. In dev, which predicts a future deploy, suppression additionally trusts what extensions declare they install, and stays silent entirely when that can't be determined (an extension hook throws, or an older@trigger.dev/build's additionalPackages predates the declaration hook), so dev never makes a false "deploys will fail" claim.additionalPackagesdeclares its packages via a new diagnostics-onlyBuildExtensionfield,installedPackagesForTarget, which the bundler ignores: bundling output is unchanged for existing projects.Detection is name-based, module-level, and deliberately per-file: computed specifiers, shadowed names, and require helpers imported from other files are not followed (those degrade to today's behavior, an unwarned runtime failure), and scanning is scoped to user code because bundled libraries legitimately use optional-require patterns that would drown real findings in noise. Packages named in build-layer install commands (
RUN npm install ...) are suppressed individually.Deploys also now surface esbuild's own bundle warnings for user files (for example
require()with a non-literal argument), which were previously discarded on the deploy path;trigger devalready showed them.Verification
Beyond the unit suite (54 tests, including real esbuild builds through the collector plugin), verified end to end against the hello-world reference project with the CLI linked to this branch:
trigger dev: a task loadingmssqlviacreateRequireproduced the dev-phrased warning with the exact file:line code frame and the fix note during "Building local worker", and the local worker started normally. The project's real extensions (lightpanda,syncEnvVars, a custom inline extension) did not suppress it, and none of the project's other task files produced spurious warnings.trigger deploy --dry-run, three passes: with the createRequire task it printed the deploy-phrased warning and still completed; after addingadditionalPackages({ packages: ["mssql"] })the warning disappeared and the build was clean; with the task removed and the config reverted, a pristine build produced zero warnings.