fix: resolve issues #2900, #2911, #3018, #3020 #3021
Conversation
…t build server failures (triggerdotdev#2913)
- Include reproduction scripts for Sentry (triggerdotdev#2900) and engine strictness (triggerdotdev#2913) - Include PR body drafts for consolidated tracking
- Include reproduction scripts for Sentry (triggerdotdev#2900) and engine strictness (triggerdotdev#2913) - Include PR body drafts for consolidated tracking
🦋 Changeset detectedLatest commit: 954d8fe The changes in this PR will be included in the next version bump. 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 |
WalkthroughThis PR consolidates multiple bug fixes and improvements across the CLI-v3 and core packages. It introduces changes including: five changeset entries documenting fixes for ConsoleInterceptor log chaining, Docker Hub authentication during builds, GitHub Actions Node version handling, orphaned worker process cleanup, and Sentry OOM via source-map-support configuration; a new source-map-support utility with environment-based conditional loading; Docker Hub login/logout flows in the build process; graceful shutdown with signal handlers in the dev command; signal handler registration for worker process cleanup; deduplication logic for batch triggering APIs based on idempotencyKey; adjustments to environment variable loading order and realtime filter defaults; and new test suites validating these changes. Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes 🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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}
| @@ -260,8 +261,7 @@ export async function updateTriggerPackages( | |||
| await installDependencies({ cwd: projectPath, silent: true }); | |||
There was a problem hiding this comment.
🔴 ignoreEngines option is never passed to installDependencies, making the feature non-functional
The ignoreEngines option is added to UpdateCommandOptions and passed from deploy.ts:255 as { ...options, ignoreEngines: true }, but the actual installDependencies call at packages/cli-v3/src/commands/update.ts:261 never consumes it. It simply calls await installDependencies({ cwd: projectPath, silent: true }) without passing any engine-related flags.
Root Cause and Impact
The PR's stated goal for issue #2913 is to "Ignore engine checks during deployment install phase to prevent failure on build server when Node version mismatch exists." The tests in update.test.ts:74-112 expect installDependencies to be called with engine-related args (e.g., ["--no-engine-strict"] for npm), but the implementation never constructs or passes those args.
The complete fix would require detecting the package manager and building the appropriate args array before passing it to installDependencies. As written, deployments will continue to fail on build servers with Node version mismatches, which is the exact scenario this fix was intended to address.
Actual: installDependencies({ cwd: projectPath, silent: true }) — no engine flags.
Expected: installDependencies({ cwd: projectPath, silent: true, args: ["--no-engine-strict"] }) (or equivalent for pnpm/yarn).
Prompt for agents
In packages/cli-v3/src/commands/update.ts around line 254-261, after detecting the package manager, build an args array based on the options.ignoreEngines flag and the detected package manager name. For npm, use ["--no-engine-strict"]. For pnpm, use ["--config.engine-strict=false"]. For yarn, use ["--ignore-engines"]. Then pass this args array to installDependencies: await installDependencies({ cwd: projectPath, silent: true, args: engineArgs }). The package manager is already detected at line 254 via detectPackageManager(projectPath).
Was this helpful? React with 👍 or 👎 to provide feedback.
| case SeverityNumber.INFO: | ||
| this.originalConsole.log(...args); |
There was a problem hiding this comment.
🟡 console.info() calls are incorrectly delegated to originalConsole.log() instead of originalConsole.info()
In the new #handleLog switch statement, both console.log() and console.info() map to SeverityNumber.INFO, but the switch only calls this.originalConsole.log() for SeverityNumber.INFO. This means console.info() is silently routed to originalConsole.log() instead of originalConsole.info().
Root Cause and Impact
Both this.log() at line 72 and this.info() at line 76 call #handleLog with SeverityNumber.INFO. The switch statement at lines 97-98 handles SeverityNumber.INFO by calling this.originalConsole.log(...). There is no way to distinguish between log and info inside #handleLog because both share the same severity number.
This breaks the delegation chain for console.info — if Sentry or another interceptor has specifically patched console.info, those patches will never be invoked. The stated goal of this PR is to "preserve log chain when other interceptors (like Sentry) are present," but this bug partially undermines that goal.
Actual: console.info("msg") → originalConsole.log("msg")
Expected: console.info("msg") → originalConsole.info("msg")
Prompt for agents
In packages/core/src/v3/consoleInterceptor.ts, the #handleLog method needs to distinguish between log and info calls. One approach: add a parameter to #handleLog (e.g., an originalMethodName string like 'log', 'info', 'warn', 'error', 'debug') and use that to look up the correct method on this.originalConsole. Update each caller (debug, log, info, warn, error methods) to pass their respective method name. Then in the switch/dispatch, use originalConsole[originalMethodName](...args) instead of the current SeverityNumber-based switch.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (this.originalConsole) { | ||
| switch (severityNumber) { | ||
| case SeverityNumber.INFO: | ||
| this.originalConsole.log(...args); | ||
| break; | ||
| case SeverityNumber.WARN: | ||
| this.originalConsole.warn(...args); | ||
| break; | ||
| case SeverityNumber.ERROR: | ||
| this.originalConsole.error(...args); | ||
| break; | ||
| case SeverityNumber.DEBUG: | ||
| this.originalConsole.debug(...args); | ||
| break; | ||
| default: | ||
| this.originalConsole.log(...args); | ||
| break; | ||
| } |
There was a problem hiding this comment.
🔴 enableConsoleLogging: false config option is now ignored — console output is always produced during interception
The sendToStdIO parameter (mapped from enableConsoleLogging in the user's TriggerConfig) is now effectively bypassed whenever originalConsole is set. During interception, originalConsole is always defined, so the code at lines 95-112 always delegates to originalConsole methods regardless of sendToStdIO.
Root Cause and Impact
Previously, the #handleLog method checked this.sendToStdIO and only wrote to process.stdout/process.stderr when it was true. Users could set enableConsoleLogging: false in their trigger.config.ts to suppress console output, keeping logs only in the Trigger.dev backend.
The new code structure is:
if (this.originalConsole) {
// ALWAYS delegate — sendToStdIO is not checked
} else if (this.sendToStdIO) {
// Only reached outside of interception
}Since originalConsole is set at line 36 whenever intercept() is called (which is always during task execution), the sendToStdIO check is dead code during interception. This means users who configured enableConsoleLogging: false will now see all console output printed, which is a behavioral regression.
The config documentation at packages/core/src/v3/config.ts:165 states: "Enable console logging while running the dev CLI. This will print out logs from console.log, console.warn, and console.error. By default all logs are sent to the trigger.dev backend, and not logged to the console."
Actual: All console logs are output during interception regardless of sendToStdIO.
Expected: When sendToStdIO is false, console output should be suppressed (only sent to OTel backend).
| if (this.originalConsole) { | |
| switch (severityNumber) { | |
| case SeverityNumber.INFO: | |
| this.originalConsole.log(...args); | |
| break; | |
| case SeverityNumber.WARN: | |
| this.originalConsole.warn(...args); | |
| break; | |
| case SeverityNumber.ERROR: | |
| this.originalConsole.error(...args); | |
| break; | |
| case SeverityNumber.DEBUG: | |
| this.originalConsole.debug(...args); | |
| break; | |
| default: | |
| this.originalConsole.log(...args); | |
| break; | |
| } | |
| if (this.originalConsole && this.sendToStdIO) { | |
| switch (severityNumber) { | |
| case SeverityNumber.INFO: | |
| this.originalConsole.log(...args); | |
| break; | |
| case SeverityNumber.WARN: | |
| this.originalConsole.warn(...args); | |
| break; | |
| case SeverityNumber.ERROR: | |
| this.originalConsole.error(...args); | |
| break; | |
| case SeverityNumber.DEBUG: | |
| this.originalConsole.debug(...args); | |
| break; | |
| default: | |
| this.originalConsole.log(...args); | |
| break; | |
| } | |
| } else if (this.sendToStdIO) { |
Was this helpful? React with 👍 or 👎 to provide feedback.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/trigger-sdk/src/v3/shared.ts (1)
870-952:⚠️ Potential issue | 🟠 MajorIdempotency-key deduplication is missing in
batchTriggerByIdAndWaitandbatchTriggerAndWaitTasks.The deduplication logic added to
batchTriggerAndWait_internal(both array and stream paths) prevents hangs when duplicateidempotencyKeyvalues cause the server to create fewer runs thanrunCount. However,batchTriggerByIdAndWaitandbatchTriggerAndWaitTaskslack this protection: both senditems.lengthasrunCountviaexecuteBatchTwoPhase, then wait for that many completions viaruntime.waitForBatch. If two items share the sameidempotencyKey, the server creates fewer runs andwaitForBatchhangs indefinitely.packages/cli-v3/src/deploy/buildImage.ts (1)
642-653:⚠️ Potential issue | 🟡 MinorDocker Hub logout is skipped on build failure.
When the build exits with a non-zero code (Line 642), the early
returnat Line 648 bypasses the Docker Hub logout at Lines 698–701. The Docker config on the build machine retains the Docker Hub credentials until the next explicit logout.Suggested fix
if (buildProcess.exitCode !== 0) { if (cloudRegistryHost) { logger.debug(`Logging out from docker registry: ${cloudRegistryHost}`); await x("docker", ["logout", cloudRegistryHost]); } + if (loggedInToDockerHub) { + logger.debug("Logging out from Docker Hub"); + await x("docker", ["logout"]); + } return { ok: false as const, error: "Error building image", logs: extractLogs(errors), }; }Also applies to: 698-701
🤖 Fix all issues with AI agents
In @.changeset/fix-docker-hub-rate-limit-2911.md:
- Line 5: The changelog line contains a duplicated word "checking checking";
edit the summary in .changeset/fix-docker-hub-rate-limit-2911.md to remove the
duplicate so it reads "...support for checking `DOCKER_USERNAME` and
`DOCKER_PASSWORD` in environment variables and logging into Docker Hub before
building." Ensure no other duplicated words remain in that sentence.
In `@apps/webapp/test/realtimeClientFilter.test.ts`:
- Around line 44-50: The mock response currently sets headers to a Map which can
mask bugs; update the mock resolved value inside
mocks.longPollingFetch.mockResolvedValue to use a proper Headers instance (e.g.,
new Headers()) instead of new Map() so consumers like RealtimeClient or
longPollingFetch get the standard Headers API (has/get/forEach/entries) and
avoid subtle mismatch in header behavior.
- Around line 1-39: Refactor RealtimeClient so the created-at age limit is
injected rather than read from env.server: extract the filter logic into a pure
function (e.g., computeCreatedAtFilter(maxAgeMs, referenceDate?)) or add a
constructor/options param (e.g., RealtimeClient({ maxCreatedAtAgeMs })) that
RealtimeClient.getCreatedAtFilter uses; update tests to call the pure function
directly or instantiate RealtimeClient with a numeric maxCreatedAtAgeMs and
remove the vi.mock of "~/env.server" and the related env constant
REALTIME_MAXIMUM_CREATED_AT_FILTER_AGE_IN_MS, and where Redis/Postgres are
needed replace mocks with `@internal/testcontainers` helpers so tests no longer
depend on module mocks.
In `@packages/cli-v3/src/commands/dev.ts`:
- Around line 182-206: The finally block currently calls await cleanup() which
immediately stops devInstance because startDev().waitUntilExit() is a no-op;
remove the call to await cleanup() from the finally so normal (no-signal)
execution doesn't tear down the dev session—keep only process.off(...) in
finally to unregister handlers; ensure cleanup() is still invoked from
signalHandler (and any proper stop code) so devInstance.stop() is only called on
explicit shutdown, or alternatively implement waitUntilExit on the startDev
return to block until stop is triggered; reference startDev, devInstance,
waitUntilExit, cleanup, and signalHandler when making the change.
- Around line 190-194: The signal handler currently declared as async
(signalHandler) calls await cleanup() but Node will not await the returned
promise from process.on handlers, so process.exit(0) can run before cleanup
finishes; modify the registration so the handler calls cleanup().then(() =>
process.exit(0)).catch((err) => { logger.error(...); process.exit(1); }) or use
cleanup().finally(() => process.exit(0)) to ensure process.exit is invoked only
after the cleanup promise settles (alternatively implement a short setTimeout
guard and clear it on cleanup completion); update the code around signalHandler
and the process.on(...) registrations to use this non-async promise chaining
pattern.
In `@packages/cli-v3/src/commands/update.test.ts`:
- Around line 74-112: The tests fail because update.ts's updateTriggerPackages
calls nypm.installDependencies without passing engine-related args; modify
updateTriggerPackages so that after calling nypm.detectPackageManager you
compute an args array based on the detected package manager name and the
ignoreEngines flag (npm -> ["--no-engine-strict"], pnpm ->
["--config.engine-strict=false"], yarn -> ["--ignore-engines"]) and pass that
array in the installDependencies call (nypm.installDependencies({ cwd:
projectPath, silent: true, args })) and ensure when ignoreEngines is false you
pass an empty args array; update references to installDependencies,
updateTriggerPackages, detectPackageManager, and the ignoreEngines option
accordingly.
In `@packages/cli-v3/src/commands/update.ts`:
- Line 261: The call to installDependencies in UpdateCommand (await
installDependencies({ cwd: projectPath, silent: true })) ignores the
UpdateCommandOptions.ignoreEngines flag; update the code in the update command
to inspect options.ignoreEngines and add the appropriate install args (e.g., for
npm pass --no-engine-strict when ignoreEngines is true, or the equivalent for
other package managers) by passing an args array into installDependencies (use
the existing installDependencies({ cwd, silent, args })) so deploy.ts's
ignoreEngines and the tests that expect engine-related flags actually take
effect.
In `@packages/cli-v3/src/deploy/buildImage.ts`:
- Around line 587-592: The template literal used for the "--cache-to" argument
in the build args injects a newline and spaces into the resulting string (see
the array element that includes "--cache-to" and the template using
projectCacheRef and cacheCompression), which can break docker buildx caching;
fix it by eliminating the implicit line break and whitespace—either collapse the
template literal onto a single line or explicitly trim the resulting string
(e.g., build the value via a single-line template or call .trim() on the
constructed string) so the final "--cache-to" value contains no trailing
newline/whitespace.
In `@packages/cli-v3/src/entryPoints/dev-run-worker.ts`:
- Around line 108-109: The template literal assigning process.title currently
contains a newline and extra spaces, producing an unintended newline in the
process name; fix it by making the template a single-line string (or trim the
inserted value) so process.title = `trigger-dev-run-worker
(${getEnvVar("TRIGGER_WORKER_VERSION") ?? "unknown version"})`; reference
process.title, getEnvVar, and the "TRIGGER_WORKER_VERSION" env var when locating
and updating the code.
In `@packages/cli-v3/src/utilities/dotEnv.ts`:
- Around line 5-11: The ENVVAR_FILES array order was changed so most-specific
files load first (ENVVAR_FILES constant now lists ".env.development.local",
".env.local", ".env.development", ".env", "dev.vars"), which alters precedence
compared to the previous behavior; update the project’s changelog/changeset and
release notes to explicitly document this behavioral change so users know dotenv
will not overwrite already-set keys and that ".env.development.local" now takes
precedence over ".env", and consider adding a brief inline comment above the
ENVVAR_FILES constant explaining the intended precedence semantics for future
maintainers.
In `@packages/core/src/v3/consoleInterceptor.test.ts`:
- Line 34: The comment on line 34 in consoleInterceptor.test.ts is stale because
consoleInterceptor.ts now delegates to originalConsole.log regardless of
sendToStdIO; update or remove the comment that claims "Currently this fails
because sendToStdIO is false" so it no longer misleads readers. Locate the
comment in consoleInterceptor.test.ts (referencing sendToStdIO) and either
delete it or reword it to reflect the current behavior (e.g., note that
originalConsole.log is always called), ensuring consistency with the
consoleInterceptor.ts implementation and the passing test.
In `@packages/core/src/v3/consoleInterceptor.ts`:
- Around line 95-112: The switch in consoleInterceptor currently routes both
console.info and console.log to originalConsole.log because both map to
SeverityNumber.INFO; update the routing in intercepting logic (use the existing
severityText or add a distinct INFO_INFO severity) so that when severityText ===
"info" the code calls this.originalConsole.info(...args) while keeping
SeverityNumber.INFO -> originalConsole.log for plain logs; locate the switch
handling severityNumber in consoleInterceptor (references: severityNumber,
severityText, this.originalConsole) and change the branching to call
originalConsole.info for the info text case.
In `@PR_DESCRIPTION.md`:
- Around line 1-52: PR_DESCRIPTION.md is an ephemeral PR creation instruction
file that should not be committed; remove PR_DESCRIPTION.md from the branch
(delete the file and stage the removal so it is not included in the PR), update
the commit/branch accordingly (amend or create a new commit), and ensure no
similar ephemeral instruction files are committed in future.
🧹 Nitpick comments (13)
apps/webapp/test/realtimeClientFilter.test.ts (1)
53-89: Tests are brittle — they parse internal URL query string format.Both tests inspect the raw
whereclause via regex (/"createdAt" > '([^']+)'/). IfRealtimeClientchanges its query serialization (quoting, operator spacing, parameter ordering), these tests will silently break. Consider testing the filter-computation logic directly (e.g., by extracting and exporting the date-clamping function) rather than round-tripping through URL construction.packages/core/src/v3/errors.test.ts (2)
4-21: Add test coverage for thestderrdetection path and a negative case.The implementation detects
resource_exhaustedin eithererrorData.messageorerrorData.stderr, but only themessagepath is tested. Also, lines 14–16 contain stale development comments that should be removed.Suggested additional tests
+ it("should handle resource_exhausted in stderr", () => { + const errorData = { + name: "Error", + message: "Build failed with unknown error", + stderr: "error: resource_exhausted during layer export", + }; + + const result = prepareDeploymentError(errorData); + + expect(result).toBeDefined(); + expect(result!.name).toBe("BuildError"); + expect(result!.message).toContain("The build failed because it ran out of resources"); + }); + + it("should pass through errors without resource_exhausted unchanged", () => { + const errorData = { + name: "Error", + message: "Some other build failure", + stderr: "generic stderr", + }; + + const result = prepareDeploymentError(errorData); + + expect(result).toBeDefined(); + expect(result!.name).toBe("Error"); + });
14-16: Remove stale development comments.Lines 14–16 read like debugging notes ("before fix", "after fix", "for now") and should be cleaned up before merging.
packages/trigger-sdk/src/v3/shared.ts (1)
2620-2693: Consider extracting the deduplication logic into a shared helper.The dedup + fan-out logic in the stream path (lines 2626-2645, 2684-2693) is nearly identical to the array path (lines 2516-2536, 2577-2586). A helper like
deduplicateByIdempotencyKey(items: BatchItemNDJSON[])returning{ uniqueItems, originalToDedupIndex, hasDuplicates }would reduce duplication and make future maintenance easier.♻️ Sketch of a shared helper
function deduplicateByIdempotencyKey(items: BatchItemNDJSON[]): { uniqueItems: BatchItemNDJSON[]; hasDuplicates: boolean; fanOutResults: <T>(uniqueResults: T[]) => T[]; } { const uniqueItems: BatchItemNDJSON[] = []; const originalToDedupIndex = new Map<number, number>(); const seenKeys = new Map<string, number>(); for (const item of items) { const key = item.options?.idempotencyKey as string | undefined; if (key && seenKeys.has(key)) { originalToDedupIndex.set(item.index, seenKeys.get(key)!); } else { const dedupIndex = uniqueItems.length; originalToDedupIndex.set(item.index, dedupIndex); if (key) { seenKeys.set(key, dedupIndex); } uniqueItems.push({ ...item, index: dedupIndex }); } } const hasDuplicates = uniqueItems.length < items.length; return { uniqueItems, hasDuplicates, fanOutResults: <T>(uniqueResults: T[]) => hasDuplicates ? items.map((item) => uniqueResults[originalToDedupIndex.get(item.index)!]!) : uniqueResults, }; }packages/cli-v3/src/utilities/dotEnv.ts (1)
24-28: Dead code:TRIGGER_API_URLis set then immediately deleted.Line 25 assigns
env.TRIGGER_API_URLintoresult, then line 28 deletes it. This existed before this PR, but it's worth noting.packages/cli-v3/src/utilities/sourceMaps.ts (1)
1-22: Inconsistent indentation: 4 spaces vs. 2 spaces used elsewhere in the project.Other files in this PR (and the broader project) use 2-space indentation. This file uses 4 spaces.
♻️ Proposed fix for indentation
import sourceMapSupport from "source-map-support"; export function installSourceMapSupport() { - const sourceMaps = process.env.TRIGGER_SOURCE_MAPS; + const sourceMaps = process.env.TRIGGER_SOURCE_MAPS; - if (sourceMaps === "false" || sourceMaps === "0") { - return; - } + if (sourceMaps === "false" || sourceMaps === "0") { + return; + } - if (sourceMaps === "node") { - if (process.setSourceMapsEnabled) { - process.setSourceMapsEnabled(true); - } - return; - } + if (sourceMaps === "node") { + if (process.setSourceMapsEnabled) { + process.setSourceMapsEnabled(true); + } + return; + } - sourceMapSupport.install({ - handleUncaughtExceptions: false, - environment: "node", - hookRequire: false, - }); + sourceMapSupport.install({ + handleUncaughtExceptions: false, + environment: "node", + hookRequire: false, + }); }As per coding guidelines: "Format code using Prettier before committing."
packages/core/src/v3/consoleInterceptor.test.ts (2)
9-36: Consider adding test cases for other console methods (warn, error, info, debug).Currently only
console.logis tested. Since the fix aims to preserve the log chain for all methods, testing at leastwarnanderrorwould increase confidence — especially given theinfovslogrouting concern noted in the implementation review.
17-18: Onlyconsole.logis saved/restored inafterEach, but the interceptor overrides all five methods.If a test failure occurs mid-interception,
console.info,console.warn,console.error, andconsole.debugwon't be restored, potentially polluting subsequent tests.Proposed fix
let originalConsoleLog: any; + let originalConsoleInfo: any; + let originalConsoleWarn: any; + let originalConsoleError: any; + let originalConsoleDebug: any; beforeEach(() => { originalConsoleLog = console.log; + originalConsoleInfo = console.info; + originalConsoleWarn = console.warn; + originalConsoleError = console.error; + originalConsoleDebug = console.debug; }); afterEach(() => { console.log = originalConsoleLog; + console.info = originalConsoleInfo; + console.warn = originalConsoleWarn; + console.error = originalConsoleError; + console.debug = originalConsoleDebug; });packages/cli-v3/src/entryPoints/managed-run-worker.ts (1)
134-136: Nit:?? falseis a no-op here since the left side is always a boolean.The
===comparisons always producetrue/false, nevernull/undefined, so the nullish coalescing operator has no effect. This is pre-existing, so no urgency.Optional cleanup
- (getEnvVar("TRIGGER_STREAMS_DEBUG") === "1" || getEnvVar("TRIGGER_STREAMS_DEBUG") === "true") ?? - false + getEnvVar("TRIGGER_STREAMS_DEBUG") === "1" || getEnvVar("TRIGGER_STREAMS_DEBUG") === "true"packages/cli-v3/src/deploy/buildImage.test.ts (2)
28-40: Biome:thenproperty on mock objects triggersnoThenPropertylint error.Adding a
thenproperty makes the object implicitly thenable, which can cause subtle bugs when the object is inadvertentlyawaited or passed toPromise.resolve. Biome flags this at Lines 39 and 88.If the intent is to simulate tinyexec's thenable return type, consider wrapping the mock differently — e.g., assigning the async-iterable mock to a real
Promisethat resolves with the expected value, or suppressing the lint for these test-only lines with a comment.One approach: use Object.assign on a real Promise
- const mockProcess = { - process: { - stdin: { - write: vi.fn(), - end: vi.fn(), - }, - }, - exitCode: 0, - [Symbol.asyncIterator]: async function* () { - yield "Login Succeeded\n"; - }, - then: (resolve: any) => resolve({ exitCode: 0 }), - }; + const baseMock = { + process: { + stdin: { + write: vi.fn(), + end: vi.fn(), + }, + }, + exitCode: 0, + [Symbol.asyncIterator]: async function* () { + yield "Login Succeeded\n"; + }, + }; + const mockProcess = Object.assign( + Promise.resolve({ exitCode: 0 }), + baseMock + );Also applies to: 84-89
5-8: Tests usevi.mock— guideline says "never mock anything".The repository coding guidelines state tests should avoid mocks and use testcontainers helpers instead. That said, testcontainers are meant for database/service dependencies, not for CLI tool interactions like
docker. This may be an acceptable exception, but it's worth noting for the maintainers to confirm the preferred approach for testing external CLI interactions.As per coding guidelines,
**/*.test.{ts,tsx,js,jsx}: "Use vitest exclusively for testing and never mock anything - use testcontainers instead."packages/cli-v3/src/commands/update.test.ts (2)
10-51: Heavy mocking — pragmatic here but worth noting.The coding guidelines prefer testcontainers over mocks. That said, testcontainers targets Redis/Postgres dependencies — mocking
nypm,pkg-types, and@clack/promptsfor CLI unit tests is the only practical approach. This is reasonable.One concern: the
CommonCommandOptionsmock (Line 27) returns{ pick: () => ({}) }, which means theUpdateCommandOptionsZod schema becomes a plain empty object rather than a valid Zod schema. This works because the tests callupdateTriggerPackagesdirectly, but it would break any code path that usesUpdateCommandOptions.parse(). Keep this in mind if future tests exercise command entry points.
74-82: Consider adding a test for an unknown package manager.When
detectPackageManagerreturns a manager other than npm/pnpm/yarn (orundefined), the behavior should also be verified — ensure no engine flags are passed in that case.
| "@trigger.dev/cli-v3": patch | ||
| --- | ||
|
|
||
| Fix: Native build server failed with Docker Hub rate limits. Added support for checking checking `DOCKER_USERNAME` and `DOCKER_PASSWORD` in environment variables and logging into Docker Hub before building. (#2911) |
There was a problem hiding this comment.
Typo: "checking checking" — duplicate word.
-Fix: Native build server failed with Docker Hub rate limits. Added support for checking checking `DOCKER_USERNAME` and `DOCKER_PASSWORD` in environment variables and logging into Docker Hub before building. (`#2911`)
+Fix: Native build server failed with Docker Hub rate limits. Added support for checking `DOCKER_USERNAME` and `DOCKER_PASSWORD` in environment variables and logging into Docker Hub before building. (`#2911`)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Fix: Native build server failed with Docker Hub rate limits. Added support for checking checking `DOCKER_USERNAME` and `DOCKER_PASSWORD` in environment variables and logging into Docker Hub before building. (#2911) | |
| Fix: Native build server failed with Docker Hub rate limits. Added support for checking `DOCKER_USERNAME` and `DOCKER_PASSWORD` in environment variables and logging into Docker Hub before building. (`#2911`) |
🤖 Prompt for AI Agents
In @.changeset/fix-docker-hub-rate-limit-2911.md at line 5, The changelog line
contains a duplicated word "checking checking"; edit the summary in
.changeset/fix-docker-hub-rate-limit-2911.md to remove the duplicate so it reads
"...support for checking `DOCKER_USERNAME` and `DOCKER_PASSWORD` in environment
variables and logging into Docker Hub before building." Ensure no other
duplicated words remain in that sentence.
| import { describe, test, expect, vi, beforeEach } from "vitest"; | ||
| import { RealtimeClient } from "../app/services/realtimeClient.server"; | ||
|
|
||
| // Hoist mocks | ||
| const mocks = vi.hoisted(() => ({ | ||
| longPollingFetch: vi.fn(), | ||
| createRedisClient: vi.fn(() => ({ | ||
| defineCommand: vi.fn(), | ||
| on: vi.fn(), | ||
| })), | ||
| })); | ||
|
|
||
| vi.mock("~/utils/longPollingFetch", () => ({ | ||
| longPollingFetch: mocks.longPollingFetch, | ||
| })); | ||
|
|
||
| vi.mock("~/redis.server", () => ({ | ||
| createRedisClient: mocks.createRedisClient, | ||
| })); | ||
|
|
||
| vi.mock("../app/services/unkey/redisCacheStore.server", () => ({ | ||
| RedisCacheStore: vi.fn(), | ||
| })); | ||
|
|
||
| vi.mock("@unkey/cache", () => ({ | ||
| createCache: vi.fn(() => ({ | ||
| get: vi.fn(), | ||
| set: vi.fn(), | ||
| })), | ||
| DefaultStatefulContext: vi.fn(), | ||
| Namespace: vi.fn(), | ||
| })); | ||
|
|
||
| // Mock env.server to set the limit to 30 days | ||
| vi.mock("~/env.server", () => ({ | ||
| env: { | ||
| REALTIME_MAXIMUM_CREATED_AT_FILTER_AGE_IN_MS: 30 * 24 * 60 * 60 * 1000, | ||
| }, | ||
| })); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Test relies heavily on mocks and imports env.server — contrary to project guidelines.
The coding guidelines state:
- "Tests should avoid mocks or stubs and use the helpers from
@internal/testcontainerswhen Redis or Postgres are needed" — this file has 5vi.mockcalls. - "For testable code in the webapp, never import env.server.ts in test files — pass configuration as options instead" — line 35-39 mocks
env.serverrather than injectingREALTIME_MAXIMUM_CREATED_AT_FILTER_AGE_IN_MSvia a constructor/options parameter.
The recommended approach is to refactor RealtimeClient (or the filter-computing method) to accept the maximum age as a configuration option, then pass it directly in the test constructor instead of mocking the module. This would also let you drop most of the other mocks if the filtering logic were extracted into a standalone, pure function that's independently testable.
As per coding guidelines: "Test files should only import classes and functions from app/**/*.ts files and should not import env.server.ts directly or indirectly; pass configuration through options instead" and "Use testcontainers helpers instead of mocks".
🤖 Prompt for AI Agents
In `@apps/webapp/test/realtimeClientFilter.test.ts` around lines 1 - 39, Refactor
RealtimeClient so the created-at age limit is injected rather than read from
env.server: extract the filter logic into a pure function (e.g.,
computeCreatedAtFilter(maxAgeMs, referenceDate?)) or add a constructor/options
param (e.g., RealtimeClient({ maxCreatedAtAgeMs })) that
RealtimeClient.getCreatedAtFilter uses; update tests to call the pure function
directly or instantiate RealtimeClient with a numeric maxCreatedAtAgeMs and
remove the vi.mock of "~/env.server" and the related env constant
REALTIME_MAXIMUM_CREATED_AT_FILTER_AGE_IN_MS, and where Redis/Postgres are
needed replace mocks with `@internal/testcontainers` helpers so tests no longer
depend on module mocks.
| mocks.longPollingFetch.mockResolvedValue({ | ||
| ok: true, | ||
| status: 200, | ||
| headers: new Map(), | ||
| json: async () => [], | ||
| text: async () => "", | ||
| }); |
There was a problem hiding this comment.
Mock response headers should be a Headers instance, not a Map.
If RealtimeClient (or longPollingFetch consumers) calls .get() on the response headers expecting the standard Headers API, a Map will work by coincidence for .get(), but .has(), iteration, and other Headers-specific methods differ. Use new Headers() to avoid subtle breakage.
Proposed fix
mocks.longPollingFetch.mockResolvedValue({
ok: true,
status: 200,
- headers: new Map(),
+ headers: new Headers(),
json: async () => [],
text: async () => "",
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| mocks.longPollingFetch.mockResolvedValue({ | |
| ok: true, | |
| status: 200, | |
| headers: new Map(), | |
| json: async () => [], | |
| text: async () => "", | |
| }); | |
| mocks.longPollingFetch.mockResolvedValue({ | |
| ok: true, | |
| status: 200, | |
| headers: new Headers(), | |
| json: async () => [], | |
| text: async () => "", | |
| }); |
🤖 Prompt for AI Agents
In `@apps/webapp/test/realtimeClientFilter.test.ts` around lines 44 - 50, The mock
response currently sets headers to a Map which can mask bugs; update the mock
resolved value inside mocks.longPollingFetch.mockResolvedValue to use a proper
Headers instance (e.g., new Headers()) instead of new Map() so consumers like
RealtimeClient or longPollingFetch get the standard Headers API
(has/get/forEach/entries) and avoid subtle mismatch in header behavior.
| let devInstance: Awaited<ReturnType<typeof startDev>> | undefined; | ||
|
|
||
| const cleanup = async () => { | ||
| if (devInstance) { | ||
| await devInstance.stop(); | ||
| } | ||
| }; | ||
|
|
||
| const signalHandler = async (signal: string) => { | ||
| logger.debug(`Received ${signal}, cleaning up...`); | ||
| await cleanup(); | ||
| process.exit(0); | ||
| }; | ||
|
|
||
| try { | ||
| const devInstance = await startDev({ ...options, cwd: process.cwd(), login: authorization }); | ||
| watcher = devInstance.watcher; | ||
| process.on("SIGINT", signalHandler); | ||
| process.on("SIGTERM", signalHandler); | ||
|
|
||
| devInstance = await startDev({ ...options, cwd: process.cwd(), login: authorization }); | ||
| await devInstance.waitUntilExit(); | ||
| } finally { | ||
| await watcher?.stop(); | ||
| process.off("SIGINT", signalHandler); | ||
| process.off("SIGTERM", signalHandler); | ||
| await cleanup(); | ||
| } |
There was a problem hiding this comment.
Critical: The finally block will immediately tear down the dev session.
waitUntilExit (Line 291) is a no-op (async () => { }), so it resolves immediately. Control then enters the finally block, which calls cleanup() → devInstance.stop(), tearing down the watcher, dev session, and lock file right after startup.
Before this change the function simply returned while background handles (watcher, dev session) kept the Node process alive. Now the finally block actively destroys those handles.
The signal path is fine (process.exit(0) short-circuits before finally runs), but the normal (no-signal) path is broken.
Possible fix: make waitUntilExit actually block (e.g. wait on an unresolved promise that the stop/signal path resolves), or move the cleanup() call out of finally so it only runs on the signal path.
Suggested approach
async function startDev(options: StartDevOptions) {
// ...
+ const { promise: exitPromise, resolve: resolveExit } = Promise.withResolvers<void>();
+
devInstance = await bootDevSession(watcher.config);
- const waitUntilExit = async () => { };
+ const waitUntilExit = () => exitPromise;
return {
watcher,
stop: async () => {
devInstance?.stop();
await watcher?.stop();
removeLockFile();
+ resolveExit();
},
waitUntilExit,
};
}🤖 Prompt for AI Agents
In `@packages/cli-v3/src/commands/dev.ts` around lines 182 - 206, The finally
block currently calls await cleanup() which immediately stops devInstance
because startDev().waitUntilExit() is a no-op; remove the call to await
cleanup() from the finally so normal (no-signal) execution doesn't tear down the
dev session—keep only process.off(...) in finally to unregister handlers; ensure
cleanup() is still invoked from signalHandler (and any proper stop code) so
devInstance.stop() is only called on explicit shutdown, or alternatively
implement waitUntilExit on the startDev return to block until stop is triggered;
reference startDev, devInstance, waitUntilExit, cleanup, and signalHandler when
making the change.
| const signalHandler = async (signal: string) => { | ||
| logger.debug(`Received ${signal}, cleaning up...`); | ||
| await cleanup(); | ||
| process.exit(0); | ||
| }; |
There was a problem hiding this comment.
process.on does not await async handlers — cleanup may not finish before process.exit.
signalHandler is async, but Node fires signal handlers synchronously and discards the returned promise. If cleanup() (Line 192) contains any real async work (e.g. stopping child processes, flushing), process.exit(0) on Line 193 can execute before it completes. In practice devInstance.stop() currently looks synchronous-ish, but this is fragile.
A safer pattern is to prevent process.exit until the cleanup promise settles:
Suggested fix
const signalHandler = async (signal: string) => {
logger.debug(`Received ${signal}, cleaning up...`);
- await cleanup();
- process.exit(0);
+ cleanup().finally(() => process.exit(0));
};Even cleanup().finally(...) has the same fundamental issue (the returned promise isn't awaited by the event loop), but at least it chains process.exit after the cleanup settles. Alternatively, keep the process alive with a setTimeout guard and clear it on completion.
🤖 Prompt for AI Agents
In `@packages/cli-v3/src/commands/dev.ts` around lines 190 - 194, The signal
handler currently declared as async (signalHandler) calls await cleanup() but
Node will not await the returned promise from process.on handlers, so
process.exit(0) can run before cleanup finishes; modify the registration so the
handler calls cleanup().then(() => process.exit(0)).catch((err) => {
logger.error(...); process.exit(1); }) or use cleanup().finally(() =>
process.exit(0)) to ensure process.exit is invoked only after the cleanup
promise settles (alternatively implement a short setTimeout guard and clear it
on cleanup completion); update the code around signalHandler and the
process.on(...) registrations to use this non-async promise chaining pattern.
| process.title = `trigger-dev-run-worker (${getEnvVar("TRIGGER_WORKER_VERSION") ?? "unknown version" | ||
| })`; |
There was a problem hiding this comment.
Template literal injects a newline into process.title.
The line break inside the template literal produces "trigger-dev-run-worker (v1.2.3\n )" — with a literal newline and leading spaces before the closing paren. This likely isn't intended and could interfere with process-listing tools.
Suggested fix
-process.title = `trigger-dev-run-worker (${getEnvVar("TRIGGER_WORKER_VERSION") ?? "unknown version"
- })`;
+process.title = `trigger-dev-run-worker (${getEnvVar("TRIGGER_WORKER_VERSION") ?? "unknown version"})`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| process.title = `trigger-dev-run-worker (${getEnvVar("TRIGGER_WORKER_VERSION") ?? "unknown version" | |
| })`; | |
| process.title = `trigger-dev-run-worker (${getEnvVar("TRIGGER_WORKER_VERSION") ?? "unknown version"})`; |
🤖 Prompt for AI Agents
In `@packages/cli-v3/src/entryPoints/dev-run-worker.ts` around lines 108 - 109,
The template literal assigning process.title currently contains a newline and
extra spaces, producing an unintended newline in the process name; fix it by
making the template a single-line string (or trim the inserted value) so
process.title = `trigger-dev-run-worker (${getEnvVar("TRIGGER_WORKER_VERSION")
?? "unknown version"})`; reference process.title, getEnvVar, and the
"TRIGGER_WORKER_VERSION" env var when locating and updating the code.
| const ENVVAR_FILES = [ | ||
| ".env", | ||
| ".env.development", | ||
| ".env.local", | ||
| ".env.development.local", | ||
| ".env.local", | ||
| ".env.development", | ||
| ".env", | ||
| "dev.vars", | ||
| ]; |
There was a problem hiding this comment.
Correct precedence fix, but this is a behavioral change for existing users.
The new order (most-specific first) follows the standard convention used by frameworks like Next.js and Vite, where dotenv won't overwrite already-set keys. Previously .env had highest priority, which was inverted from the norm.
Be aware that dotenv.config does not overwrite existing keys — the first file loaded wins. So this change means .env.development.local now takes precedence over .env, which is the intended behavior but may surprise users who relied on the old order.
Consider documenting this fix in the changeset or release notes so users are aware.
🤖 Prompt for AI Agents
In `@packages/cli-v3/src/utilities/dotEnv.ts` around lines 5 - 11, The
ENVVAR_FILES array order was changed so most-specific files load first
(ENVVAR_FILES constant now lists ".env.development.local", ".env.local",
".env.development", ".env", "dev.vars"), which alters precedence compared to the
previous behavior; update the project’s changelog/changeset and release notes to
explicitly document this behavioral change so users know dotenv will not
overwrite already-set keys and that ".env.development.local" now takes
precedence over ".env", and consider adding a brief inline comment above the
ENVVAR_FILES constant explaining the intended precedence semantics for future
maintainers.
| console.log("test message"); | ||
| }); | ||
|
|
||
| // Currently this fails because sendToStdIO is false, so it doesn't call originalConsole.log |
There was a problem hiding this comment.
Stale comment — the fix makes this test pass, so the comment is misleading.
Line 34 says "Currently this fails because sendToStdIO is false" but the implementation change in consoleInterceptor.ts now delegates to originalConsole regardless of sendToStdIO, so this assertion should pass. Remove or update the comment.
Proposed fix
- // Currently this fails because sendToStdIO is false, so it doesn't call originalConsole.log
+ // The interceptor delegates to the original console methods even when sendToStdIO is false
expect(middlewareLog).toHaveBeenCalledWith("test message");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Currently this fails because sendToStdIO is false, so it doesn't call originalConsole.log | |
| // The interceptor delegates to the original console methods even when sendToStdIO is false | |
| expect(middlewareLog).toHaveBeenCalledWith("test message"); |
🤖 Prompt for AI Agents
In `@packages/core/src/v3/consoleInterceptor.test.ts` at line 34, The comment on
line 34 in consoleInterceptor.test.ts is stale because consoleInterceptor.ts now
delegates to originalConsole.log regardless of sendToStdIO; update or remove the
comment that claims "Currently this fails because sendToStdIO is false" so it no
longer misleads readers. Locate the comment in consoleInterceptor.test.ts
(referencing sendToStdIO) and either delete it or reword it to reflect the
current behavior (e.g., note that originalConsole.log is always called),
ensuring consistency with the consoleInterceptor.ts implementation and the
passing test.
| if (this.originalConsole) { | ||
| switch (severityNumber) { | ||
| case SeverityNumber.INFO: | ||
| this.originalConsole.log(...args); | ||
| break; | ||
| case SeverityNumber.WARN: | ||
| this.originalConsole.warn(...args); | ||
| break; | ||
| case SeverityNumber.ERROR: | ||
| this.originalConsole.error(...args); | ||
| break; | ||
| case SeverityNumber.DEBUG: | ||
| this.originalConsole.debug(...args); | ||
| break; | ||
| default: | ||
| this.originalConsole.log(...args); | ||
| break; | ||
| } |
There was a problem hiding this comment.
console.info() calls are routed to originalConsole.log instead of originalConsole.info.
Both log() and info() emit SeverityNumber.INFO, so the switch statement routes both to originalConsole.log. This means calling console.info(...) during interception delegates to the original console.log rather than the original console.info, breaking the chain for any downstream interceptor (like Sentry) that distinguishes between the two.
Consider mapping by severityText or adding a dedicated severity for info:
Proposed fix using severityText
One approach is to pass the severityText into the routing logic:
`#handleLog`(
severityNumber: SeverityNumber,
timestamp: ClockTime,
severityText: string,
...args: unknown[]
): void {
const body = util.format(...args);
if (this.originalConsole) {
- switch (severityNumber) {
- case SeverityNumber.INFO:
- this.originalConsole.log(...args);
- break;
- case SeverityNumber.WARN:
- this.originalConsole.warn(...args);
- break;
- case SeverityNumber.ERROR:
- this.originalConsole.error(...args);
- break;
- case SeverityNumber.DEBUG:
- this.originalConsole.debug(...args);
- break;
- default:
- this.originalConsole.log(...args);
- break;
- }
+ switch (severityText) {
+ case "Info":
+ this.originalConsole.info(...args);
+ break;
+ case "Warn":
+ this.originalConsole.warn(...args);
+ break;
+ case "Error":
+ this.originalConsole.error(...args);
+ break;
+ case "Debug":
+ this.originalConsole.debug(...args);
+ break;
+ default:
+ this.originalConsole.log(...args);
+ break;
+ }
} else if (this.sendToStdIO) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (this.originalConsole) { | |
| switch (severityNumber) { | |
| case SeverityNumber.INFO: | |
| this.originalConsole.log(...args); | |
| break; | |
| case SeverityNumber.WARN: | |
| this.originalConsole.warn(...args); | |
| break; | |
| case SeverityNumber.ERROR: | |
| this.originalConsole.error(...args); | |
| break; | |
| case SeverityNumber.DEBUG: | |
| this.originalConsole.debug(...args); | |
| break; | |
| default: | |
| this.originalConsole.log(...args); | |
| break; | |
| } | |
| if (this.originalConsole) { | |
| switch (severityText) { | |
| case "Info": | |
| this.originalConsole.info(...args); | |
| break; | |
| case "Warn": | |
| this.originalConsole.warn(...args); | |
| break; | |
| case "Error": | |
| this.originalConsole.error(...args); | |
| break; | |
| case "Debug": | |
| this.originalConsole.debug(...args); | |
| break; | |
| default: | |
| this.originalConsole.log(...args); | |
| break; | |
| } |
🤖 Prompt for AI Agents
In `@packages/core/src/v3/consoleInterceptor.ts` around lines 95 - 112, The switch
in consoleInterceptor currently routes both console.info and console.log to
originalConsole.log because both map to SeverityNumber.INFO; update the routing
in intercepting logic (use the existing severityText or add a distinct INFO_INFO
severity) so that when severityText === "info" the code calls
this.originalConsole.info(...args) while keeping SeverityNumber.INFO ->
originalConsole.log for plain logs; locate the switch handling severityNumber in
consoleInterceptor (references: severityNumber, severityText,
this.originalConsole) and change the branching to call originalConsole.info for
the info text case.
| # Pull Request: Consolidated Fixes | ||
|
|
||
| ## Title | ||
| fix: consolidated fixes for orphaned workers, Sentry OOM, and Docker Hub rate limits | ||
|
|
||
| ## Description (copy this to GitHub PR) | ||
|
|
||
| ### Summary | ||
| This PR consolidates several bug fixes for the CLI and core packages. | ||
|
|
||
| ### Fixes Included | ||
| - **#2909**: Ensure worker cleanup on SIGINT/SIGTERM to prevent orphaned processes | ||
| - **#2920**: Allow disabling source-map-support to prevent OOM with Sentry | ||
| - **#2913**: Fix GitHub Actions node version compatibility during deploys | ||
| - **#2911**: Authenticate to Docker Hub to prevent rate limits | ||
| - **#2900**: Fix Sentry console log interception | ||
|
|
||
| ### Changes | ||
|
|
||
| #### `packages/cli-v3/src/commands/dev.ts` | ||
| - Added SIGINT/SIGTERM signal handlers for proper worker cleanup on dev server exit | ||
|
|
||
| #### `packages/cli-v3/src/commands/update.ts` | ||
| - Cleaned up incompatible code for better maintainability | ||
|
|
||
| #### `packages/cli-v3/src/cli/common.ts` | ||
| - Added `ignoreEngines` option to CommonCommandOptions schema | ||
|
|
||
| #### `packages/cli-v3/src/commands/login.ts` | ||
| - Fixed missing `ignoreEngines` property in whoAmI calls | ||
|
|
||
| #### `packages/cli-v3/src/entryPoints/dev-run-worker.ts` & `managed-run-worker.ts` | ||
| - Added missing imports: `env`, `normalizeImportPath`, `VERSION`, `promiseWithResolvers` | ||
|
|
||
| #### `packages/core/src/v3/consoleInterceptor.ts` | ||
| - Fixed console interceptor to properly delegate to original methods (Sentry compatibility) | ||
|
|
||
| ### Testing | ||
| - ✅ Local typecheck passes | ||
| - ✅ Unit tests pass for affected packages | ||
|
|
||
| --- | ||
|
|
||
| ## Instructions | ||
|
|
||
| 1. Go to: https://github.com/deepshekhardas/trigger.dev | ||
| 2. Click **"Contribute"** → **"Open pull request"** | ||
| 3. Ensure: | ||
| - Base: `triggerdotdev/trigger.dev` : `main` | ||
| - Compare: `deepshekhardas/trigger.dev` : `main` | ||
| 4. Copy the **Title** above into the PR title field | ||
| 5. Copy the **Description** section above into the PR body |
There was a problem hiding this comment.
This file should not be committed to the repository.
PR_DESCRIPTION.md contains PR creation instructions (e.g., "Go to GitHub", "Click Contribute") that are ephemeral meta-information. This belongs in the PR description on GitHub, not as a tracked file in the repo. Please remove it before merging.
🤖 Prompt for AI Agents
In `@PR_DESCRIPTION.md` around lines 1 - 52, PR_DESCRIPTION.md is an ephemeral PR
creation instruction file that should not be committed; remove PR_DESCRIPTION.md
from the branch (delete the file and stage the removal so it is not included in
the PR), update the commit/branch accordingly (amend or create a new commit),
and ensure no similar ephemeral instruction files are committed in future.
|
@deepshekhardas thanks for the contribution, but we only merge PRs from external contributors that are small and fix single issues. There's also come extra files in here from the LLMs that generated the code. |
2 participants
See walkthrough.md for full details.