fix(webapp,run-engine,core): drop the hidden debounce ceiling, fail fast on an unusable maxDelay - #4521
fix(webapp,run-engine,core): drop the hidden debounce ceiling, fail fast on an unusable maxDelay#4521matt-aitken wants to merge 2 commits into
Conversation
…ject windows that cannot debounce A debounced run is only pushed later while the new execution time stays inside maxDelay (or the server maximum) measured from the first trigger, so the room to push is the gap between the two. A delay at or above that ceiling meant the first extension was already out of bounds: every trigger created its own run, with no error and nothing on the run to show the debounce had been ignored. The default ceiling moves from 1 hour to 24 hours, and a delay that leaves no room is now rejected at trigger time with a message naming both values and how to fix them. debounce.delay must also be a duration rather than a date, since it is re-applied on every extension.
🦋 Changeset detectedLatest commit: d7dd33b 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 |
WalkthroughThe change removes the default one-hour debounce ceiling. An unset server ceiling allows matching triggers to continue extending a run. Trigger-level 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 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 |
| const maxDelayMs = debounce.maxDelay | ||
| ? parseNaturalLanguageDurationInMs(debounce.maxDelay) | ||
| : undefined; | ||
|
|
||
| if (debounce.maxDelay && maxDelayMs === undefined) { | ||
| throw new ServiceValidationError( | ||
| `Invalid debounce maxDelay: ${debounce.maxDelay}. ` + | ||
| `Supported formats: {number}s, {number}m, {number}h, {number}d, {number}w` | ||
| ); | ||
| } |
There was a problem hiding this comment.
🔍 Malformed debounce.maxDelay now hard-fails at trigger time instead of falling back
The engine treats an unparseable debounce.maxDelay as a warning and silently falls back to the server ceiling (internal-packages/run-engine/src/engine/systems/debounceSystem.ts:825-836 and the fast-path at :628-633). The new validation rejects the trigger outright with a ServiceValidationError. This is a stricter contract than the engine's, so callers that previously succeeded with a typo'd maxDelay (e.g. "1hour") will now receive a 4xx. Reasonable, but it is an API behavior change not called out in the PR description; worth confirming it is intended and worth a release note.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
internal-packages/run-engine/src/engine/tests/debounce.test.ts (1)
3186-3251: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert that the debounce deadline advances.
The four triggers can execute in one quantization bucket. The test then proves run reuse, but it does not prove a repeated trigger extends
delayUntil.Disable quantization for this test, or advance time between triggers. Assert that the persisted
delayUntilincreases after a later trigger.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9f48ffae-0e59-4318-8790-e56605fc3dee
📒 Files selected for processing (9)
.changeset/debounce-max-duration.mdapps/webapp/app/env.server.tsapps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/test/engine/triggerTask.debounce.test.tsdocs/triggering.mdxinternal-packages/run-engine/src/engine/index.tsinternal-packages/run-engine/src/engine/tests/debounce.test.tsinternal-packages/run-engine/src/engine/types.tspackages/core/src/v3/types/tasks.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (19)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (12, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (11, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (10, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (9, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 12)
- GitHub Check: e2e / 🧪 CLI v3 tests (warp-windows-latest-x64-8x - npm)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 12)
- GitHub Check: e2e / 🧪 CLI v3 tests (warp-windows-latest-x64-8x - pnpm)
- GitHub Check: runops-guard / runops-guard
- GitHub Check: packages / 🧪 Unit Tests: Packages (3, 3)
- GitHub Check: internal / 🧪 Unit Tests: Internal
- GitHub Check: packages / 🧪 Unit Tests: Packages (1, 3)
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp
🧰 Additional context used
📓 Path-based instructions (17)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Prefer static imports over dynamicimport(); use dynamic imports only for unresolvable circular dependencies, genuine performance code splitting, or conditional runtime loading.
Import Trigger.dev tasks from@trigger.dev/sdk; never use@trigger.dev/sdk/v3or deprecatedclient.defineJob.
Add agentcrumbs while writing code using approved namespaces; mark lines with//@Crumbsor blocks with `// `#region` `@crumbs, and strip them before merging.
Files:
internal-packages/run-engine/src/engine/index.tsinternal-packages/run-engine/src/engine/tests/debounce.test.tsapps/webapp/test/engine/triggerTask.debounce.test.tsinternal-packages/run-engine/src/engine/types.tspackages/core/src/v3/types/tasks.tsapps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
internal-packages/run-engine/src/engine/index.tsinternal-packages/run-engine/src/engine/tests/debounce.test.tsapps/webapp/test/engine/triggerTask.debounce.test.tsinternal-packages/run-engine/src/engine/types.tspackages/core/src/v3/types/tasks.tsapps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: 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
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
internal-packages/run-engine/src/engine/index.tsinternal-packages/run-engine/src/engine/tests/debounce.test.tsapps/webapp/test/engine/triggerTask.debounce.test.tsinternal-packages/run-engine/src/engine/types.tspackages/core/src/v3/types/tasks.tsapps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.ts
internal-packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
For internal packages, use
typecheckfor verification and never usebuildas the correctness check.
Files:
internal-packages/run-engine/src/engine/index.tsinternal-packages/run-engine/src/engine/tests/debounce.test.tsinternal-packages/run-engine/src/engine/types.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use vitest for all tests in the Trigger.dev repository
**/*.{test,spec}.{ts,tsx}: Use Vitest exclusively and never mock dependencies; use Testcontainers for integration dependencies.
Place test files next to the source files they test.
Files:
internal-packages/run-engine/src/engine/tests/debounce.test.tsapps/webapp/test/engine/triggerTask.debounce.test.ts
internal-packages/run-engine/src/engine/tests/**/*.test.ts
📄 CodeRabbit inference engine (internal-packages/run-engine/CLAUDE.md)
Implement tests for RunEngine in
src/engine/tests/using testcontainers for Redis and PostgreSQL containerization
Files:
internal-packages/run-engine/src/engine/tests/debounce.test.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/test/engine/triggerTask.debounce.test.tspackages/core/src/v3/types/tasks.tsapps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: Access environment variables through theenvexport ofenv.server.tsinstead of directly accessingprocess.env
Use subpath exports from@trigger.dev/corepackage instead of importing from the root@trigger.dev/corepathDo not reintroduce the removed v1 execution path;
RunEngineVersion.V1branches may only reject or finalize gracefully so v3 clients receive a clean 4xx, never a 5xx.
Files:
apps/webapp/test/engine/triggerTask.debounce.test.tsapps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.ts
apps/webapp/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Do not import
env.server.tsdirectly or indirectly into test files; instead pass environment-dependent values through options/parameters to make code testable
Files:
apps/webapp/test/engine/triggerTask.debounce.test.ts
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
For apps, use
typecheckfor verification and never usebuildas the correctness check.
Files:
apps/webapp/test/engine/triggerTask.debounce.test.tsapps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.ts
apps/webapp/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Test files must not import
app/env.server.ts; pass configuration as options instead.
Files:
apps/webapp/test/engine/triggerTask.debounce.test.ts
docs/**/*.mdx
📄 CodeRabbit inference engine (docs/CLAUDE.md)
docs/**/*.mdx: MDX documentation pages must include frontmatter with title (required), description (required), and sidebarTitle (optional) in YAML format
Use Mintlify components for structured content: , , , , , , /, /
Always import from@trigger.dev/sdkin code examples (never from@trigger.dev/sdk/v3)
Code examples must be complete and runnable where possible
Use language tags in code fences:typescript,bash,jsonDocumentation in
docs/uses MDX conventions defined by the documentation guidance.
Files:
docs/triggering.mdx
packages/core/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/core/CLAUDE.md)
Never import the root package (
@trigger.dev/core). Always use subpath imports such as@trigger.dev/core/v3,@trigger.dev/core/v3/utils,@trigger.dev/core/logger, or@trigger.dev/core/schemas
Files:
packages/core/src/v3/types/tasks.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
For public packages, use
buildfor verification.
Files:
packages/core/src/v3/types/tasks.ts
packages/core/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Import
@trigger.dev/coresubpaths only; never import from the package root.
Files:
packages/core/src/v3/types/tasks.ts
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
apps/webapp/app/**/*.{ts,tsx}: For dashboard changes, visually verify the running Remix app with Chrome DevTools MCP, using snapshots, screenshots, interaction, and console-message checks as appropriate.
UseuseCallbackanduseMemoonly for context provider values, expensive derived data used as a dependency, or stable references required by dependency arrays; do not wrap ordinary event handlers or trivial computations.
Use named constants for sentinel or placeholder values instead of scattering raw string literals across comparisons.
Files:
apps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.ts
apps/webapp/app/**/*.ts
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
apps/webapp/app/**/*.ts: Never userequest.signalto detect client disconnects. UsegetRequestAbortSignal()fromapp/services/httpAsyncStorage.server.ts, which is wired to Express response close events.
Access environment variables through theenvexport fromapp/env.server.ts; never useprocess.envdirectly.
Always use PrismafindFirstinstead offindUnique.
Always use the$transactionhelper from~/db.server, never callprisma.$transactionor$replica.$transactiondirectly. Pass isolation levels as strings, useSerializablefor correctness-critical read-then-write invariants, and guard possibly undefined helper results when a definite value is required.
Files:
apps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.ts
🧠 Learnings (23)
📚 Learning: 2026-03-22T13:26:12.060Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: apps/webapp/app/components/code/TextEditor.tsx:81-86
Timestamp: 2026-03-22T13:26:12.060Z
Learning: In the triggerdotdev/trigger.dev codebase, do not flag `navigator.clipboard.writeText(...)` calls for `missing-await`/`unhandled-promise` issues. These clipboard writes are intentionally invoked without `await` and without `catch` handlers across the project; keep that behavior consistent when reviewing TypeScript/TSX files (e.g., usages like in `apps/webapp/app/components/code/TextEditor.tsx`).
Applied to files:
internal-packages/run-engine/src/engine/index.tsinternal-packages/run-engine/src/engine/tests/debounce.test.tsapps/webapp/test/engine/triggerTask.debounce.test.tsinternal-packages/run-engine/src/engine/types.tspackages/core/src/v3/types/tasks.tsapps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.ts
📚 Learning: 2026-03-22T19:24:14.403Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3187
File: apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.ts:200-204
Timestamp: 2026-03-22T19:24:14.403Z
Learning: In the triggerdotdev/trigger.dev codebase, webhook URLs are not expected to contain embedded credentials/secrets (e.g., fields like `ProjectAlertWebhookProperties` should only hold credential-free webhook endpoints). During code review, if you see logging or inclusion of raw webhook URLs in error messages, do not automatically treat it as a credential-leak/secrets-in-logs issue by default—first verify the URL does not contain embedded credentials (for example, no username/password in the URL, no obvious secret/token query params or fragments). If the URL is credential-free per this project’s conventions, allow the logging.
Applied to files:
internal-packages/run-engine/src/engine/index.tsinternal-packages/run-engine/src/engine/tests/debounce.test.tsapps/webapp/test/engine/triggerTask.debounce.test.tsinternal-packages/run-engine/src/engine/types.tspackages/core/src/v3/types/tasks.tsapps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.ts
📚 Learning: 2026-05-18T08:21:27.694Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3632
File: apps/webapp/sentry.server.ts:4-21
Timestamp: 2026-05-18T08:21:27.694Z
Learning: When handling Prisma error P1001 ("Can't reach database server") in TypeScript, don’t assume a single error shape. Prisma can surface P1001 via two different error classes/fields: `PrismaClientKnownRequestError` exposes it as `err.code === "P1001"` (common during mid-query connection drops), while `PrismaClientInitializationError` exposes it as `err.errorCode === "P1001"` (common on client startup failure). Therefore, predicates should use `err.code === "P1001" || err.errorCode === "P1001"`. Do not flag `err.code === "P1001"` as “unreachable/never matches,” as it is expected in production.
Applied to files:
internal-packages/run-engine/src/engine/index.tsinternal-packages/run-engine/src/engine/tests/debounce.test.tsapps/webapp/test/engine/triggerTask.debounce.test.tsinternal-packages/run-engine/src/engine/types.tspackages/core/src/v3/types/tasks.tsapps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.ts
📚 Learning: 2026-05-18T08:21:27.694Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3632
File: apps/webapp/sentry.server.ts:4-21
Timestamp: 2026-05-18T08:21:27.694Z
Learning: When handling Prisma errors for P1001 ("Can't reach database server"), do not assume it only appears under a single property name. Prisma may surface P1001 via either `PrismaClientKnownRequestError` (`err.code === "P1001"`, e.g., mid-query connection drops) or `PrismaClientInitializationError` (`err.errorCode === "P1001"`, e.g., client startup connection failure). To reliably detect the condition, check `err.code === "P1001" || err.errorCode === "P1001"`, and avoid review rules that would incorrectly flag `err.code === "P1001"` as unreachable/never-matching.
Applied to files:
internal-packages/run-engine/src/engine/index.tsinternal-packages/run-engine/src/engine/tests/debounce.test.tsapps/webapp/test/engine/triggerTask.debounce.test.tsinternal-packages/run-engine/src/engine/types.tspackages/core/src/v3/types/tasks.tsapps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.ts
📚 Learning: 2026-06-13T19:53:13.759Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3937
File: packages/trigger-sdk/skills/realtime-and-frontend/SKILL.md:258-260
Timestamp: 2026-06-13T19:53:13.759Z
Learning: When reviewing code that uses `trigger.dev/react-hooks`’s `useRealtimeRun`, preserve the call signature where the first argument is the full realtime handle object (not `handle.id`). This is intentional to maintain type-safety and is consistent with the official docs; do not suggest changing the first argument from the handle object to `handle.id`.
Applied to files:
internal-packages/run-engine/src/engine/index.tsinternal-packages/run-engine/src/engine/tests/debounce.test.tsapps/webapp/test/engine/triggerTask.debounce.test.tsinternal-packages/run-engine/src/engine/types.tspackages/core/src/v3/types/tasks.tsapps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.ts
📚 Learning: 2026-06-17T17:13:49.929Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3948
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.bulk-actions.$bulkActionParam/route.tsx:48-62
Timestamp: 2026-06-17T17:13:49.929Z
Learning: In triggerdotdev/trigger.dev, within `dashboardLoader`/`dashboardAction` (or similar context resolver code) whenever you resolve an organization ID from an organization slug for RBAC/enterprise authorization scope, always read from the primary Prisma client (`prisma`), not `$replica`. Using `$replica` can hit replica-lag and cause the RBAC lookup/authorization to run without the correct org scope (bypassing intended role enforcement). Implement the slug→org lookup with `prisma.organization.findFirst(...)` (or equivalent primary-client query) and add an inline comment documenting why the primary client is required (replica lag could lead to unscoped RBAC checks).
Applied to files:
internal-packages/run-engine/src/engine/index.tsinternal-packages/run-engine/src/engine/tests/debounce.test.tsapps/webapp/test/engine/triggerTask.debounce.test.tsinternal-packages/run-engine/src/engine/types.tspackages/core/src/v3/types/tasks.tsapps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.ts
📚 Learning: 2026-06-23T13:04:21.413Z
Learnt from: carderne
Repo: triggerdotdev/trigger.dev PR: 4023
File: apps/webapp/app/services/upsertBranch.server.ts:14-18
Timestamp: 2026-06-23T13:04:21.413Z
Learning: In TypeScript, it’s valid to `import { type X }` and then use `typeof X` in a type-only position, e.g. `type Alias = z.infer<typeof X>`. The `type` modifier suppresses the runtime import, but the type checker still has the full exported type so `z.infer<typeof X>` can resolve correctly. In code reviews, don’t flag this as a TypeScript compile error as long as `typeof X` is used in a type context (e.g., with `z.infer`, `type` aliases, generics), not as a runtime value.
Applied to files:
internal-packages/run-engine/src/engine/index.tsinternal-packages/run-engine/src/engine/tests/debounce.test.tsapps/webapp/test/engine/triggerTask.debounce.test.tsinternal-packages/run-engine/src/engine/types.tspackages/core/src/v3/types/tasks.tsapps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.ts
📚 Learning: 2026-06-04T18:16:35.386Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 3836
File: apps/supervisor/src/backpressure/backpressureMonitor.ts:3-5
Timestamp: 2026-06-04T18:16:35.386Z
Learning: When reviewing TypeScript in this repo, apply the rule “prefer type aliases over interfaces” only to data/object shapes and union/intersection type modeling. If an interface is being used as a behavioral contract for collaborators to implement (e.g., method-shape interfaces that define required behavior, such as `BackpressureLogger` / `BackpressureSignalSource` in `apps/supervisor/src/backpressure/backpressureMonitor.ts`), keep it as an `interface` and do not flag it as a type-alias-vs-interface violation.
Applied to files:
internal-packages/run-engine/src/engine/index.tsinternal-packages/run-engine/src/engine/tests/debounce.test.tsapps/webapp/test/engine/triggerTask.debounce.test.tsinternal-packages/run-engine/src/engine/types.tspackages/core/src/v3/types/tasks.tsapps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.ts
📚 Learning: 2026-06-09T17:58:04.699Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 3879
File: apps/webapp/app/models/vercelIntegration.server.ts:619-630
Timestamp: 2026-06-09T17:58:04.699Z
Learning: In this codebase, outbound raw `fetch` calls should typically rely on Node/undici’s default request timeout (about ~300s) rather than adding a per-call `AbortController` + `setTimeout` wrapper inside individual functions (e.g. in files like `apps/webapp/app/models/vercelIntegration.server.ts`). During code review, do not flag the absence of a per-call timeout on a single `fetch` as an issue; if per-call timeouts are needed, they should be implemented via a codebase-wide convention (e.g., a shared fetch wrapper or documented pattern) rather than ad-hoc per-function changes.
Applied to files:
internal-packages/run-engine/src/engine/index.tsinternal-packages/run-engine/src/engine/tests/debounce.test.tsapps/webapp/test/engine/triggerTask.debounce.test.tsinternal-packages/run-engine/src/engine/types.tspackages/core/src/v3/types/tasks.tsapps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.ts
📚 Learning: 2026-05-18T14:40:02.173Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3658
File: packages/core/src/v3/realtimeStreams/manager.test.ts:1-147
Timestamp: 2026-05-18T14:40:02.173Z
Learning: In the triggerdotdev/trigger.dev repo, the policy “Never mock anything — use testcontainers instead” should only be enforced for integration tests that interact with real external services (e.g., Redis, Postgres) via actual infrastructure. For unit tests that exercise pure in-memory logic (e.g., cache semantics) it is OK to stub collaborators such as `ApiClient` using Vitest (`vi.fn()`) to assert call counts or control behavior. Do not flag `vi.fn()`-based `ApiClient` stubs in unit tests as violations of the testcontainers policy.
Applied to files:
internal-packages/run-engine/src/engine/tests/debounce.test.tsapps/webapp/test/engine/triggerTask.debounce.test.ts
📚 Learning: 2026-06-16T09:19:47.637Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3960
File: apps/webapp/test/prismaInfrastructureErrorCapture.test.ts:0-0
Timestamp: 2026-06-16T09:19:47.637Z
Learning: In this repo’s Vitest setup, `vitest.config.ts` uses `globals: true`, so identifiers like `vi`, `describe`, `it`, and `expect` are available as globals in Vitest test files. During code review, do not flag missing `vi`/`describe`/`it`/`expect` imports as a runtime error or correctness issue when they’re used in `*.test.ts/tsx` or `*.spec.ts/tsx` files. Explicit imports are still preferred for consistency, but they’re not required for runtime behavior.
Applied to files:
internal-packages/run-engine/src/engine/tests/debounce.test.tsapps/webapp/test/engine/triggerTask.debounce.test.ts
📚 Learning: 2026-05-07T12:25:18.271Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3531
File: apps/webapp/test/sentryTraceContext.server.test.ts:9-47
Timestamp: 2026-05-07T12:25:18.271Z
Learning: In the triggerdotdev/trigger.dev webapp test suite, it is acceptable to leave `createInMemoryTracing()` calls that register a global `NodeTracerProvider` without `afterEach`/`afterAll` teardown. Do not flag this as a test-ordering risk when the code follows the established pattern used across webapp tests (e.g., replication service/benchmark/backfiller tests). This is considered safe because `trace.getActiveSpan()` when called outside a `context.with(...)` block reads `AsyncLocalStorage.getStore()` (undefined when no `run()` scope exists), so it falls back to `ROOT_CONTEXT` with no attached span—regardless of which provider is registered.
Applied to files:
apps/webapp/test/engine/triggerTask.debounce.test.ts
📚 Learning: 2026-05-28T20:02:10.647Z
Learnt from: myftija
Repo: triggerdotdev/trigger.dev PR: 3772
File: apps/webapp/test/findOrCreateBackgroundWorker.test.ts:1-1
Timestamp: 2026-05-28T20:02:10.647Z
Learning: In the triggerdotdev/trigger.dev monorepo, for the `apps/webapp` package use the established convention of storing Vitest tests (unit, integration, and e2e) under `apps/webapp/test/` rather than colocating them next to source files. Do not flag files located in `apps/webapp/test/` as violating any rule that says to colocate tests with source.
Applied to files:
apps/webapp/test/engine/triggerTask.debounce.test.ts
📚 Learning: 2026-07-30T18:43:56.874Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 4426
File: apps/webapp/test/memberDevEnvironments.server.test.ts:124-125
Timestamp: 2026-07-30T18:43:56.874Z
Learning: In the `apps/webapp` test suite (`apps/webapp/test/**`), respect the established test harness in `apps/webapp/test/setup.ts`: it loads `.env` and provides default values for required environment variables so that transitive imports (e.g., `~/env.server`) work without production-style wiring.
During code review, do not require dependency injection/refactoring solely to avoid this existing import path. Only introduce configuration injection if it delivers production-level value (for example, a more general `createEnvironment` abstraction that improves runtime behavior beyond test setup).
Applied to files:
apps/webapp/test/engine/triggerTask.debounce.test.ts
📚 Learning: 2026-05-12T21:04:05.815Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3542
File: apps/webapp/app/components/sessions/v1/SessionStatus.tsx:1-3
Timestamp: 2026-05-12T21:04:05.815Z
Learning: In this Remix + TypeScript codebase, do not flag a server/client boundary violation when a file imports only types from a module matching `*.server`.
Specifically, it’s safe to import types using `import type { Foo } from "*.server"` or `import { type Foo } from "*.server"` because TypeScript erases type-only imports at compile time and they emit no JavaScript, so they won’t cross the Remix server/client bundle boundary.
Only raise the boundary concern for value imports (e.g., `import { Foo }` without `type`, or `import Foo`), since those produce JavaScript output.
Applied to files:
apps/webapp/test/engine/triggerTask.debounce.test.tsapps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.ts
📚 Learning: 2026-06-25T18:21:51.905Z
Learnt from: carderne
Repo: triggerdotdev/trigger.dev PR: 4039
File: apps/webapp/app/routes/invite-revoke.tsx:0-0
Timestamp: 2026-06-25T18:21:51.905Z
Learning: During the Zod v4 migration in the triggerdotdev/trigger.dev webapp, ensure any imports from `conform-to/zod` use the Zod-4 subpath: `conform-to/zod/v4` (e.g., `import { parseWithZod } from "conform-to/zod/v4"`). Do not import from the package root `conform-to/zod`, because it is the Zod 3 implementation and may load Zod-3-only symbols (e.g., `ZodBranded`, `ZodEffects`), which can throw at module load (notably with `zod4.4.3`). This should be enforced across `apps/webapp/**/*` where helpers like `parseWithZod` and `conformZodMessage` are used.
Applied to files:
apps/webapp/test/engine/triggerTask.debounce.test.tsapps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.ts
📚 Learning: 2026-07-03T17:10:21.498Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 4148
File: apps/webapp/app/models/orgMember.server.ts:149-168
Timestamp: 2026-07-03T17:10:21.498Z
Learning: In triggerdotdev/trigger.dev, `User.email` (Prisma schema: `internal-packages/database/prisma/schema.prisma`) currently does NOT use `citext` and does NOT have a `lower(email)` functional unique index. Therefore, do not introduce Prisma queries like `where: { email: { equals: <value>, mode: "insensitive" } }` (or any case-insensitive lookup) against `User.email`, because it can force sequential scans of the `users` table under load. During review, ensure email is normalized (e.g., lowercased/trimmed) before both writes and subsequent lookups, and if true case-insensitive behavior/uniqueness is required, implement it via a separate app-wide migration (e.g., switch to `citext` and/or add a functional unique index with backfill) rather than bolting it onto individual feature PRs.
Applied to files:
apps/webapp/test/engine/triggerTask.debounce.test.tsapps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.ts
📚 Learning: 2026-03-10T12:44:14.176Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 3200
File: docs/config/config-file.mdx:353-368
Timestamp: 2026-03-10T12:44:14.176Z
Learning: In the trigger.dev repo, docs PRs are often companions to implementation PRs. When reviewing docs PRs (MDX files under docs/), check the PR description for any companion/related PR references and verify that the documented features exist in those companion PRs before flagging missing implementations. This ensures docs stay in sync with code changes across related PRs.
Applied to files:
docs/triggering.mdx
📚 Learning: 2026-04-30T20:30:29.458Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3226
File: docs/ai-chat/quick-start.mdx:13-13
Timestamp: 2026-04-30T20:30:29.458Z
Learning: In this repo’s documentation MDX files (`docs/**/*.mdx`), use `ts` and `tsx` (not `typescript`) as the code-fence language tags for TypeScript/TSX snippets. Do not flag `ts`/`tsx` code-fence language tags as incorrect in any docs MDX file, since this is the site-wide Mintlify-compatible convention.
Applied to files:
docs/triggering.mdx
📚 Learning: 2026-05-05T09:38:02.512Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3523
File: apps/webapp/app/routes/api.v3.batches.ts:178-181
Timestamp: 2026-05-05T09:38:02.512Z
Learning: When reviewing code that catches `ServiceValidationError` in `*.server.ts` files, do not blindly forward `error.status` to HTTP responses, because SVEs may be thrown with non-default statuses (e.g., 400/500) and forwarding them can cause client-visible behavioral regressions (e.g., surfacing 500s to clients). Prefer a safe default response status of `error.status ?? 422`, but only after confirming via the reachable call graph that the caught `ServiceValidationError` instances are expected to carry those non-default statuses; otherwise, normalize to `422` to avoid unexpected client-visible 5xx behavior.
Applied to files:
apps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.ts
📚 Learning: 2026-05-22T11:50:56.079Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3333
File: apps/webapp/app/runEngine/services/triggerFailedTask.server.ts:76-80
Timestamp: 2026-05-22T11:50:56.079Z
Learning: When reviewing changes related to org-scoped ClickHouse / org reassignment, treat ClickHouse as the source of truth for the affected read paths (e.g., run lists, span detail, logs) with no Postgres fallback. Reassigning an org from one ClickHouse cluster to another must be done by migrating that org’s existing ClickHouse data between clusters first—do not assume it’s sufficient to update only the OrganizationDataStore entry. If any implementation/change would make org reassignment possible, ensure the required migration design/implementation is included (work tracked under Linear TRI-9659, sub-issue of TRI-7994). During the initial rollout of org-scoped ClickHouse (PR `#3333`), no production org has an override configured, so the limitation is not yet reachable in production; don’t introduce production-dependent assumptions that rely on overrides being active.
Applied to files:
apps/webapp/app/runEngine/services/triggerTask.server.ts
📚 Learning: 2026-05-20T17:21:18.543Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3678
File: apps/webapp/app/entry.server.tsx:0-0
Timestamp: 2026-05-20T17:21:18.543Z
Learning: In env.server.ts (Zod env schema), any environment variable you plan to access via the typed `env` export (e.g., `env.SENTRY_DSN`) must be explicitly declared in the schema. For `SENTRY_DSN`, include `SENTRY_DSN: z.string().optional()`; otherwise switching from `process.env.SENTRY_DSN` to `env.SENTRY_DSN` will fail TypeScript typechecking.
Applied to files:
apps/webapp/app/env.server.ts
📚 Learning: 2026-06-01T11:37:08.569Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3754
File: apps/webapp/app/env.server.ts:1104-1129
Timestamp: 2026-06-01T11:37:08.569Z
Learning: In apps/*/app/env.server.ts, any new background/periodic worker feature flag should hard-default to "0" (explicit opt-in) rather than inheriting from a parent flag (e.g., avoid defaulting to process.env.TRIGGER_MOLLIFIER_ENABLED ?? "0"). Inheriting can cause the new worker to auto-start on upgrade for deployments that already enabled the parent flag, turning on unexpected background load without an explicit rollout. Each worker component must require its own dedicated env var and default it explicitly to "0" (e.g., TRIGGER_MOLLIFIER_STALE_SWEEP_ENABLED defaults to "0" unless explicitly set to enable that worker).
Applied to files:
apps/webapp/app/env.server.ts
🪛 LanguageTool
docs/triggering.mdx
[grammar] ~887-~887: Use a hyphen to join words.
Context: ...s delay. With delay: "5s" and the 24 hour default, a key can be pushed for al...
(QB_NEW_EN_HYPHEN)
🔇 Additional comments (7)
internal-packages/run-engine/src/engine/index.ts (1)
387-387: 🗄️ Data Integrity & IntegrationCustom server debounce ceilings are passed through.
internal-packages/run-engine/src/engine/types.ts (1)
171-178: LGTM!packages/core/src/v3/types/tasks.ts (1)
943-950: LGTM!Also applies to: 1004-1009
.changeset/debounce-max-duration.md (1)
1-17: LGTM!docs/triggering.mdx (1)
876-876: LGTM!Also applies to: 885-917, 919-921
apps/webapp/app/runEngine/services/triggerTask.server.ts (1)
10-18: LGTM!Also applies to: 94-94, 114-152, 172-196, 343-344
apps/webapp/test/engine/triggerTask.debounce.test.ts (1)
462-554: LGTM!
| RUN_ENGINE_MAXIMUM_DEBOUNCE_DURATION_MS: z.coerce | ||
| .number() | ||
| .int() | ||
| .default(60_000 * 60), // 1 hour | ||
| .default(24 * 60 * 60 * 1000), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject non-positive debounce ceilings at startup.
RUN_ENGINE_MAXIMUM_DEBOUNCE_DURATION_MS accepts zero and negative values. These values make every non-negative debounce.delay reach or exceed the ceiling. Add .positive() so invalid ceilings fail during environment validation.
Proposed fix
RUN_ENGINE_MAXIMUM_DEBOUNCE_DURATION_MS: z.coerce
.number()
.int()
+ .positive()
.default(24 * 60 * 60 * 1000),📝 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.
| RUN_ENGINE_MAXIMUM_DEBOUNCE_DURATION_MS: z.coerce | |
| .number() | |
| .int() | |
| .default(60_000 * 60), // 1 hour | |
| .default(24 * 60 * 60 * 1000), | |
| RUN_ENGINE_MAXIMUM_DEBOUNCE_DURATION_MS: z.coerce | |
| .number() | |
| .int() | |
| .positive() | |
| .default(24 * 60 * 60 * 1000), |
| if (delayMs === undefined) { | ||
| throw new ServiceValidationError( | ||
| `Invalid debounce delay: ${debounce.delay}. ` + | ||
| `debounce.delay must be a duration, not a date. ` + | ||
| `Supported formats: {number}s, {number}m, {number}h, {number}d, {number}w` | ||
| ); | ||
| } | ||
|
|
||
| const maxDelayMs = debounce.maxDelay | ||
| ? parseNaturalLanguageDurationInMs(debounce.maxDelay) | ||
| : undefined; | ||
|
|
||
| if (debounce.maxDelay && maxDelayMs === undefined) { | ||
| throw new ServiceValidationError( | ||
| `Invalid debounce maxDelay: ${debounce.maxDelay}. ` + | ||
| `Supported formats: {number}s, {number}m, {number}h, {number}d, {number}w` | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Document the accepted duration grammar consistently.
parseNaturalLanguageDurationInMs accepts hr and compound durations such as "2h30m". The new validation errors and SDK JSDoc list only single-unit forms with h. This gives users incomplete guidance.
apps/webapp/app/runEngine/services/triggerTask.server.ts#L153-L169: Includehrand compound duration examples in both invalid-format errors.packages/core/src/v3/types/tasks.ts#L975-L981: State that duration components can be combined and that hours accepthorhr.
📍 Affects 2 files
apps/webapp/app/runEngine/services/triggerTask.server.ts#L153-L169(this comment)packages/core/src/v3/types/tasks.ts#L975-L981
…ast on an unusable maxDelay The engine applied a server-side ceiling on how long a debounced run could be pushed back, defaulting to an hour and documented nowhere. Any delay at or above it could never push its run, so every trigger created its own run with no error and nothing on the run to show the debounce key had been ignored. The ceiling is now unset by default, so a key keeps collapsing triggers for as long as they arrive and maxDelay is the only bound. Self-hosters can still set one. Callers who pass a maxDelay that is not longer than their delay hit the same dead end, so that pair is rejected at trigger time rather than silently doing nothing.
| if (!debounce.maxDelay) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
🟡 Debounce still silently does nothing when the operator-configured maximum is shorter than the requested delay
A debounce request whose wait time is longer than the server-wide maximum is accepted without complaint (#validateDebounceMaxDelay at apps/webapp/app/runEngine/services/triggerTask.server.ts:140-165 only looks at a caller-supplied maxDelay), so on any deployment that configures a maximum the original silent failure remains.
Impact: On installations that set the server-side debounce ceiling, users asking for a long debounce still get one separate run per trigger with no error telling them why.
Why the guard misses the server ceiling
The engine resolves the effective window as debounce.maxDelay if present, else the server ceiling (#resolveMaxDurationMs, internal-packages/run-engine/src/engine/systems/debounceSystem.ts:581-597). It then releases the existing run and returns max_duration_exceeded when now + delay > createdAt + maxDurationMs (internal-packages/run-engine/src/engine/systems/debounceSystem.ts:846-864).
RUN_ENGINE_MAXIMUM_DEBOUNCE_DURATION_MS is now optional (apps/webapp/app/env.server.ts:1044) and is passed through in apps/webapp/app/v3/runEngine.server.ts:245. When an operator sets it (e.g. to 1h) and a caller triggers with delay: "12h" and no maxDelay, the new validator returns early at line 143-145 and the trigger succeeds, but every subsequent trigger immediately exceeds the ceiling and creates its own run — exactly the failure mode this PR set out to surface. The JSDoc claim on line 137-138 ("with no maxDelay there is no ceiling to conflict with") is only true when the env var is unset.
A fix would compare delay against the effective ceiling (debounce.maxDelay ?? env.RUN_ENGINE_MAXIMUM_DEBOUNCE_DURATION_MS) rather than only against an explicit maxDelay.
Prompt for agents
In apps/webapp/app/runEngine/services/triggerTask.server.ts, #validateDebounceMaxDelay only rejects a debounce window when the caller passes an explicit maxDelay. The run engine also applies a server-side ceiling (DebounceSystem#resolveMaxDurationMs falls back to maxDebounceDurationMs, wired from env.RUN_ENGINE_MAXIMUM_DEBOUNCE_DURATION_MS in apps/webapp/app/v3/runEngine.server.ts). When that env var is configured and a caller sends a debounce delay at or above it with no maxDelay, the engine releases the delayed run on every subsequent trigger (max_duration_exceeded), producing one run per trigger with no error — the exact silent failure this PR is fixing. Consider validating the delay against the effective ceiling (explicit maxDelay if given, otherwise the configured server ceiling when set), keeping the current behaviour of no validation when neither is set.
Was this helpful? React with 👍 or 👎 to provide feedback.
| executionSnapshotSystem: this.executionSnapshotSystem, | ||
| delayedRunSystem: this.delayedRunSystem, | ||
| maxDebounceDurationMs: options.debounce?.maxDebounceDurationMs ?? 60 * 60 * 1000, // Default 1 hour | ||
| maxDebounceDurationMs: options.debounce?.maxDebounceDurationMs, |
There was a problem hiding this comment.
🔍 PR title and description describe a 24h default ceiling, but the code removes the ceiling entirely
The PR title ("raise the debounce ceiling to 24h") and the author description ("The default ceiling moves from 1 hour to 24 hours", plus an error message naming "the maximum debounce duration of 1d") do not match the committed code: RUN_ENGINE_MAXIMUM_DEBOUNCE_DURATION_MS is now optional() with no default, internal-packages/run-engine/src/engine/index.ts:387 passes options.debounce?.maxDebounceDurationMs straight through, and DebounceSystem treats undefined as "no ceiling". The changeset and docs are consistent with the code (no limit), so the description/title appear stale — worth confirming which behaviour is intended before merge, since "unbounded by default" is a meaningfully different operational posture from "24h default".
Was this helpful? React with 👍 or 👎 to provide feedback.
| const delayMs = parseNaturalLanguageDurationInMs(debounce.delay); | ||
|
|
||
| if (delayMs !== undefined && maxDelayMs <= delayMs) { | ||
| throw new ServiceValidationError( | ||
| `debounce.maxDelay (${debounce.maxDelay}) must be longer than debounce.delay (${debounce.delay}). ` + | ||
| `A debounced run is only pushed back while it stays inside maxDelay, so with these values ` + | ||
| `every trigger would create its own run.` | ||
| ); | ||
| } |
There was a problem hiding this comment.
🔍 debounce.delay given as a date is still accepted and still silently breaks debouncing
The author description says "debounce.delay must also be a duration rather than a date" and the JSDoc in packages/core/src/v3/types/tasks.ts:975 now states "Must be a duration, not a date", but nothing enforces it. parseDelay (used at apps/webapp/app/runEngine/services/triggerTask.server.ts:302-311) accepts an absolute date, and the new #validateDebounceMaxDelay skips the comparison entirely when parseNaturalLanguageDurationInMs(debounce.delay) returns undefined (line 158). At reschedule time #computeQuantizedDelayUntil returns null for a date string, which drives claimKeyForNewRun and therefore a fresh run per trigger — the same silent failure the PR targets, now contradicted by the docs.
Was this helpful? React with 👍 or 👎 to provide feedback.
@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: |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/triggering.mdx (1)
917-929: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCorrect the
maxDelaytimeline.At 26s, the new target is 31s and exceeds the 30s ceiling. The runtime leaves Run A scheduled for 29s, clears its debounce key, and creates Run B. It does not execute Run A at the 28s trigger. Update the table and the surrounding explanation.
🧹 Nitpick comments (1)
apps/webapp/app/runEngine/services/triggerTask.server.ts (1)
140-165: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftMove
debounce.maxDelayvalidation intoTriggerTaskRequestBody.The request schema is the shared Zod entry point for
TriggerTaskRequestBodyconsumers, but this parse/comparison check still happens only in the service. Add a.superRefine()on the schema so every request consumer receives the same validation contract.Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: cbe9df1c-f2b3-450c-8139-39d6a0c724eb
📒 Files selected for processing (10)
.changeset/debounce-max-duration.mdapps/webapp/app/env.server.tsapps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/test/engine/triggerTask.debounce.test.tsdocs/triggering.mdxinternal-packages/run-engine/src/engine/index.tsinternal-packages/run-engine/src/engine/systems/debounceSystem.tsinternal-packages/run-engine/src/engine/tests/debounce.test.tsinternal-packages/run-engine/src/engine/types.tspackages/core/src/v3/types/tasks.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/core/src/v3/types/tasks.ts
- internal-packages/run-engine/src/engine/tests/debounce.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (15)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (9, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (10, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (11, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (12, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 12)
- GitHub Check: packages / 🧪 Unit Tests: Packages (3, 3)
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp
- GitHub Check: internal / 🧪 Unit Tests: Internal
🧰 Additional context used
📓 Path-based instructions (14)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Prefer static imports over dynamicimport(); use dynamic imports only for unresolvable circular dependencies, genuine performance code splitting, or conditional runtime loading.
Import Trigger.dev tasks from@trigger.dev/sdk; never use@trigger.dev/sdk/v3or deprecatedclient.defineJob.
Add agentcrumbs while writing code using approved namespaces; mark lines with//@Crumbsor blocks with `// `#region` `@crumbs, and strip them before merging.
Files:
apps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.tsinternal-packages/run-engine/src/engine/systems/debounceSystem.tsinternal-packages/run-engine/src/engine/index.tsapps/webapp/test/engine/triggerTask.debounce.test.tsinternal-packages/run-engine/src/engine/types.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.tsapps/webapp/test/engine/triggerTask.debounce.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.tsinternal-packages/run-engine/src/engine/systems/debounceSystem.tsinternal-packages/run-engine/src/engine/index.tsapps/webapp/test/engine/triggerTask.debounce.test.tsinternal-packages/run-engine/src/engine/types.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: 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
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
apps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.tsinternal-packages/run-engine/src/engine/systems/debounceSystem.tsinternal-packages/run-engine/src/engine/index.tsapps/webapp/test/engine/triggerTask.debounce.test.tsinternal-packages/run-engine/src/engine/types.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: Access environment variables through theenvexport ofenv.server.tsinstead of directly accessingprocess.env
Use subpath exports from@trigger.dev/corepackage instead of importing from the root@trigger.dev/corepathDo not reintroduce the removed v1 execution path;
RunEngineVersion.V1branches may only reject or finalize gracefully so v3 clients receive a clean 4xx, never a 5xx.
Files:
apps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.tsapps/webapp/test/engine/triggerTask.debounce.test.ts
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
For apps, use
typecheckfor verification and never usebuildas the correctness check.
Files:
apps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.tsapps/webapp/test/engine/triggerTask.debounce.test.ts
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
apps/webapp/app/**/*.{ts,tsx}: For dashboard changes, visually verify the running Remix app with Chrome DevTools MCP, using snapshots, screenshots, interaction, and console-message checks as appropriate.
UseuseCallbackanduseMemoonly for context provider values, expensive derived data used as a dependency, or stable references required by dependency arrays; do not wrap ordinary event handlers or trivial computations.
Use named constants for sentinel or placeholder values instead of scattering raw string literals across comparisons.
Files:
apps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.ts
apps/webapp/app/**/*.ts
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
apps/webapp/app/**/*.ts: Never userequest.signalto detect client disconnects. UsegetRequestAbortSignal()fromapp/services/httpAsyncStorage.server.ts, which is wired to Express response close events.
Access environment variables through theenvexport fromapp/env.server.ts; never useprocess.envdirectly.
Always use PrismafindFirstinstead offindUnique.
Always use the$transactionhelper from~/db.server, never callprisma.$transactionor$replica.$transactiondirectly. Pass isolation levels as strings, useSerializablefor correctness-critical read-then-write invariants, and guard possibly undefined helper results when a definite value is required.
Files:
apps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.ts
internal-packages/run-engine/src/engine/systems/**/*.ts
📄 CodeRabbit inference engine (internal-packages/run-engine/CLAUDE.md)
Integrate OpenTelemetry tracer and meter instrumentation in RunEngine systems for observability
Files:
internal-packages/run-engine/src/engine/systems/debounceSystem.ts
internal-packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
For internal packages, use
typecheckfor verification and never usebuildas the correctness check.
Files:
internal-packages/run-engine/src/engine/systems/debounceSystem.tsinternal-packages/run-engine/src/engine/index.tsinternal-packages/run-engine/src/engine/types.ts
docs/**/*.mdx
📄 CodeRabbit inference engine (docs/CLAUDE.md)
docs/**/*.mdx: MDX documentation pages must include frontmatter with title (required), description (required), and sidebarTitle (optional) in YAML format
Use Mintlify components for structured content: , , , , , , /, /
Always import from@trigger.dev/sdkin code examples (never from@trigger.dev/sdk/v3)
Code examples must be complete and runnable where possible
Use language tags in code fences:typescript,bash,jsonDocumentation in
docs/uses MDX conventions defined by the documentation guidance.
Files:
docs/triggering.mdx
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use vitest for all tests in the Trigger.dev repository
**/*.{test,spec}.{ts,tsx}: Use Vitest exclusively and never mock dependencies; use Testcontainers for integration dependencies.
Place test files next to the source files they test.
Files:
apps/webapp/test/engine/triggerTask.debounce.test.ts
apps/webapp/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Do not import
env.server.tsdirectly or indirectly into test files; instead pass environment-dependent values through options/parameters to make code testable
Files:
apps/webapp/test/engine/triggerTask.debounce.test.ts
apps/webapp/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Test files must not import
app/env.server.ts; pass configuration as options instead.
Files:
apps/webapp/test/engine/triggerTask.debounce.test.ts
🧠 Learnings (24)
📚 Learning: 2026-03-22T13:26:12.060Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: apps/webapp/app/components/code/TextEditor.tsx:81-86
Timestamp: 2026-03-22T13:26:12.060Z
Learning: In the triggerdotdev/trigger.dev codebase, do not flag `navigator.clipboard.writeText(...)` calls for `missing-await`/`unhandled-promise` issues. These clipboard writes are intentionally invoked without `await` and without `catch` handlers across the project; keep that behavior consistent when reviewing TypeScript/TSX files (e.g., usages like in `apps/webapp/app/components/code/TextEditor.tsx`).
Applied to files:
apps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.tsinternal-packages/run-engine/src/engine/systems/debounceSystem.tsinternal-packages/run-engine/src/engine/index.tsapps/webapp/test/engine/triggerTask.debounce.test.tsinternal-packages/run-engine/src/engine/types.ts
📚 Learning: 2026-03-22T19:24:14.403Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3187
File: apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.ts:200-204
Timestamp: 2026-03-22T19:24:14.403Z
Learning: In the triggerdotdev/trigger.dev codebase, webhook URLs are not expected to contain embedded credentials/secrets (e.g., fields like `ProjectAlertWebhookProperties` should only hold credential-free webhook endpoints). During code review, if you see logging or inclusion of raw webhook URLs in error messages, do not automatically treat it as a credential-leak/secrets-in-logs issue by default—first verify the URL does not contain embedded credentials (for example, no username/password in the URL, no obvious secret/token query params or fragments). If the URL is credential-free per this project’s conventions, allow the logging.
Applied to files:
apps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.tsinternal-packages/run-engine/src/engine/systems/debounceSystem.tsinternal-packages/run-engine/src/engine/index.tsapps/webapp/test/engine/triggerTask.debounce.test.tsinternal-packages/run-engine/src/engine/types.ts
📚 Learning: 2026-05-18T08:21:27.694Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3632
File: apps/webapp/sentry.server.ts:4-21
Timestamp: 2026-05-18T08:21:27.694Z
Learning: When handling Prisma error P1001 ("Can't reach database server") in TypeScript, don’t assume a single error shape. Prisma can surface P1001 via two different error classes/fields: `PrismaClientKnownRequestError` exposes it as `err.code === "P1001"` (common during mid-query connection drops), while `PrismaClientInitializationError` exposes it as `err.errorCode === "P1001"` (common on client startup failure). Therefore, predicates should use `err.code === "P1001" || err.errorCode === "P1001"`. Do not flag `err.code === "P1001"` as “unreachable/never matches,” as it is expected in production.
Applied to files:
apps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.tsinternal-packages/run-engine/src/engine/systems/debounceSystem.tsinternal-packages/run-engine/src/engine/index.tsapps/webapp/test/engine/triggerTask.debounce.test.tsinternal-packages/run-engine/src/engine/types.ts
📚 Learning: 2026-05-18T08:21:27.694Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3632
File: apps/webapp/sentry.server.ts:4-21
Timestamp: 2026-05-18T08:21:27.694Z
Learning: When handling Prisma errors for P1001 ("Can't reach database server"), do not assume it only appears under a single property name. Prisma may surface P1001 via either `PrismaClientKnownRequestError` (`err.code === "P1001"`, e.g., mid-query connection drops) or `PrismaClientInitializationError` (`err.errorCode === "P1001"`, e.g., client startup connection failure). To reliably detect the condition, check `err.code === "P1001" || err.errorCode === "P1001"`, and avoid review rules that would incorrectly flag `err.code === "P1001"` as unreachable/never-matching.
Applied to files:
apps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.tsinternal-packages/run-engine/src/engine/systems/debounceSystem.tsinternal-packages/run-engine/src/engine/index.tsapps/webapp/test/engine/triggerTask.debounce.test.tsinternal-packages/run-engine/src/engine/types.ts
📚 Learning: 2026-06-13T19:53:13.759Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3937
File: packages/trigger-sdk/skills/realtime-and-frontend/SKILL.md:258-260
Timestamp: 2026-06-13T19:53:13.759Z
Learning: When reviewing code that uses `trigger.dev/react-hooks`’s `useRealtimeRun`, preserve the call signature where the first argument is the full realtime handle object (not `handle.id`). This is intentional to maintain type-safety and is consistent with the official docs; do not suggest changing the first argument from the handle object to `handle.id`.
Applied to files:
apps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.tsinternal-packages/run-engine/src/engine/systems/debounceSystem.tsinternal-packages/run-engine/src/engine/index.tsapps/webapp/test/engine/triggerTask.debounce.test.tsinternal-packages/run-engine/src/engine/types.ts
📚 Learning: 2026-06-17T17:13:49.929Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3948
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.bulk-actions.$bulkActionParam/route.tsx:48-62
Timestamp: 2026-06-17T17:13:49.929Z
Learning: In triggerdotdev/trigger.dev, within `dashboardLoader`/`dashboardAction` (or similar context resolver code) whenever you resolve an organization ID from an organization slug for RBAC/enterprise authorization scope, always read from the primary Prisma client (`prisma`), not `$replica`. Using `$replica` can hit replica-lag and cause the RBAC lookup/authorization to run without the correct org scope (bypassing intended role enforcement). Implement the slug→org lookup with `prisma.organization.findFirst(...)` (or equivalent primary-client query) and add an inline comment documenting why the primary client is required (replica lag could lead to unscoped RBAC checks).
Applied to files:
apps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.tsinternal-packages/run-engine/src/engine/systems/debounceSystem.tsinternal-packages/run-engine/src/engine/index.tsapps/webapp/test/engine/triggerTask.debounce.test.tsinternal-packages/run-engine/src/engine/types.ts
📚 Learning: 2026-06-23T13:04:21.413Z
Learnt from: carderne
Repo: triggerdotdev/trigger.dev PR: 4023
File: apps/webapp/app/services/upsertBranch.server.ts:14-18
Timestamp: 2026-06-23T13:04:21.413Z
Learning: In TypeScript, it’s valid to `import { type X }` and then use `typeof X` in a type-only position, e.g. `type Alias = z.infer<typeof X>`. The `type` modifier suppresses the runtime import, but the type checker still has the full exported type so `z.infer<typeof X>` can resolve correctly. In code reviews, don’t flag this as a TypeScript compile error as long as `typeof X` is used in a type context (e.g., with `z.infer`, `type` aliases, generics), not as a runtime value.
Applied to files:
apps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.tsinternal-packages/run-engine/src/engine/systems/debounceSystem.tsinternal-packages/run-engine/src/engine/index.tsapps/webapp/test/engine/triggerTask.debounce.test.tsinternal-packages/run-engine/src/engine/types.ts
📚 Learning: 2026-05-05T09:38:02.512Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3523
File: apps/webapp/app/routes/api.v3.batches.ts:178-181
Timestamp: 2026-05-05T09:38:02.512Z
Learning: When reviewing code that catches `ServiceValidationError` in `*.server.ts` files, do not blindly forward `error.status` to HTTP responses, because SVEs may be thrown with non-default statuses (e.g., 400/500) and forwarding them can cause client-visible behavioral regressions (e.g., surfacing 500s to clients). Prefer a safe default response status of `error.status ?? 422`, but only after confirming via the reachable call graph that the caught `ServiceValidationError` instances are expected to carry those non-default statuses; otherwise, normalize to `422` to avoid unexpected client-visible 5xx behavior.
Applied to files:
apps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.ts
📚 Learning: 2026-05-12T21:04:05.815Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3542
File: apps/webapp/app/components/sessions/v1/SessionStatus.tsx:1-3
Timestamp: 2026-05-12T21:04:05.815Z
Learning: In this Remix + TypeScript codebase, do not flag a server/client boundary violation when a file imports only types from a module matching `*.server`.
Specifically, it’s safe to import types using `import type { Foo } from "*.server"` or `import { type Foo } from "*.server"` because TypeScript erases type-only imports at compile time and they emit no JavaScript, so they won’t cross the Remix server/client bundle boundary.
Only raise the boundary concern for value imports (e.g., `import { Foo }` without `type`, or `import Foo`), since those produce JavaScript output.
Applied to files:
apps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.tsapps/webapp/test/engine/triggerTask.debounce.test.ts
📚 Learning: 2026-06-25T18:21:51.905Z
Learnt from: carderne
Repo: triggerdotdev/trigger.dev PR: 4039
File: apps/webapp/app/routes/invite-revoke.tsx:0-0
Timestamp: 2026-06-25T18:21:51.905Z
Learning: During the Zod v4 migration in the triggerdotdev/trigger.dev webapp, ensure any imports from `conform-to/zod` use the Zod-4 subpath: `conform-to/zod/v4` (e.g., `import { parseWithZod } from "conform-to/zod/v4"`). Do not import from the package root `conform-to/zod`, because it is the Zod 3 implementation and may load Zod-3-only symbols (e.g., `ZodBranded`, `ZodEffects`), which can throw at module load (notably with `zod4.4.3`). This should be enforced across `apps/webapp/**/*` where helpers like `parseWithZod` and `conformZodMessage` are used.
Applied to files:
apps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.tsapps/webapp/test/engine/triggerTask.debounce.test.ts
📚 Learning: 2026-07-03T17:10:21.498Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 4148
File: apps/webapp/app/models/orgMember.server.ts:149-168
Timestamp: 2026-07-03T17:10:21.498Z
Learning: In triggerdotdev/trigger.dev, `User.email` (Prisma schema: `internal-packages/database/prisma/schema.prisma`) currently does NOT use `citext` and does NOT have a `lower(email)` functional unique index. Therefore, do not introduce Prisma queries like `where: { email: { equals: <value>, mode: "insensitive" } }` (or any case-insensitive lookup) against `User.email`, because it can force sequential scans of the `users` table under load. During review, ensure email is normalized (e.g., lowercased/trimmed) before both writes and subsequent lookups, and if true case-insensitive behavior/uniqueness is required, implement it via a separate app-wide migration (e.g., switch to `citext` and/or add a functional unique index with backfill) rather than bolting it onto individual feature PRs.
Applied to files:
apps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.tsapps/webapp/test/engine/triggerTask.debounce.test.ts
📚 Learning: 2026-05-22T11:50:56.079Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3333
File: apps/webapp/app/runEngine/services/triggerFailedTask.server.ts:76-80
Timestamp: 2026-05-22T11:50:56.079Z
Learning: When reviewing changes related to org-scoped ClickHouse / org reassignment, treat ClickHouse as the source of truth for the affected read paths (e.g., run lists, span detail, logs) with no Postgres fallback. Reassigning an org from one ClickHouse cluster to another must be done by migrating that org’s existing ClickHouse data between clusters first—do not assume it’s sufficient to update only the OrganizationDataStore entry. If any implementation/change would make org reassignment possible, ensure the required migration design/implementation is included (work tracked under Linear TRI-9659, sub-issue of TRI-7994). During the initial rollout of org-scoped ClickHouse (PR `#3333`), no production org has an override configured, so the limitation is not yet reachable in production; don’t introduce production-dependent assumptions that rely on overrides being active.
Applied to files:
apps/webapp/app/runEngine/services/triggerTask.server.ts
📚 Learning: 2026-06-04T18:16:35.386Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 3836
File: apps/supervisor/src/backpressure/backpressureMonitor.ts:3-5
Timestamp: 2026-06-04T18:16:35.386Z
Learning: When reviewing TypeScript in this repo, apply the rule “prefer type aliases over interfaces” only to data/object shapes and union/intersection type modeling. If an interface is being used as a behavioral contract for collaborators to implement (e.g., method-shape interfaces that define required behavior, such as `BackpressureLogger` / `BackpressureSignalSource` in `apps/supervisor/src/backpressure/backpressureMonitor.ts`), keep it as an `interface` and do not flag it as a type-alias-vs-interface violation.
Applied to files:
apps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.tsinternal-packages/run-engine/src/engine/systems/debounceSystem.tsinternal-packages/run-engine/src/engine/index.tsapps/webapp/test/engine/triggerTask.debounce.test.tsinternal-packages/run-engine/src/engine/types.ts
📚 Learning: 2026-06-09T17:58:04.699Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 3879
File: apps/webapp/app/models/vercelIntegration.server.ts:619-630
Timestamp: 2026-06-09T17:58:04.699Z
Learning: In this codebase, outbound raw `fetch` calls should typically rely on Node/undici’s default request timeout (about ~300s) rather than adding a per-call `AbortController` + `setTimeout` wrapper inside individual functions (e.g. in files like `apps/webapp/app/models/vercelIntegration.server.ts`). During code review, do not flag the absence of a per-call timeout on a single `fetch` as an issue; if per-call timeouts are needed, they should be implemented via a codebase-wide convention (e.g., a shared fetch wrapper or documented pattern) rather than ad-hoc per-function changes.
Applied to files:
apps/webapp/app/runEngine/services/triggerTask.server.tsapps/webapp/app/env.server.tsinternal-packages/run-engine/src/engine/systems/debounceSystem.tsinternal-packages/run-engine/src/engine/index.tsapps/webapp/test/engine/triggerTask.debounce.test.tsinternal-packages/run-engine/src/engine/types.ts
📚 Learning: 2026-05-20T17:21:18.543Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3678
File: apps/webapp/app/entry.server.tsx:0-0
Timestamp: 2026-05-20T17:21:18.543Z
Learning: In env.server.ts (Zod env schema), any environment variable you plan to access via the typed `env` export (e.g., `env.SENTRY_DSN`) must be explicitly declared in the schema. For `SENTRY_DSN`, include `SENTRY_DSN: z.string().optional()`; otherwise switching from `process.env.SENTRY_DSN` to `env.SENTRY_DSN` will fail TypeScript typechecking.
Applied to files:
apps/webapp/app/env.server.ts
📚 Learning: 2026-06-01T11:37:08.569Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3754
File: apps/webapp/app/env.server.ts:1104-1129
Timestamp: 2026-06-01T11:37:08.569Z
Learning: In apps/*/app/env.server.ts, any new background/periodic worker feature flag should hard-default to "0" (explicit opt-in) rather than inheriting from a parent flag (e.g., avoid defaulting to process.env.TRIGGER_MOLLIFIER_ENABLED ?? "0"). Inheriting can cause the new worker to auto-start on upgrade for deployments that already enabled the parent flag, turning on unexpected background load without an explicit rollout. Each worker component must require its own dedicated env var and default it explicitly to "0" (e.g., TRIGGER_MOLLIFIER_STALE_SWEEP_ENABLED defaults to "0" unless explicitly set to enable that worker).
Applied to files:
apps/webapp/app/env.server.ts
📚 Learning: 2025-12-18T14:09:01.965Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 2794
File: internal-packages/run-engine/src/engine/systems/debounceSystem.ts:390-397
Timestamp: 2025-12-18T14:09:01.965Z
Learning: In internal-packages/run-engine/src/engine/systems/debounceSystem.ts, do not allow debounce delays shorter than 1 second. The parseNaturalLanguageDuration function currently supports only weeks, days, hours, minutes, and seconds (w/d/hr/h/m/s). Millisecond delays are not supported. Update any input validation, error messages, and related tests to reflect a minimum 1s debounce and restrict inputs to the supported units.
Applied to files:
internal-packages/run-engine/src/engine/systems/debounceSystem.ts
📚 Learning: 2026-03-10T12:44:14.176Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 3200
File: docs/config/config-file.mdx:353-368
Timestamp: 2026-03-10T12:44:14.176Z
Learning: In the trigger.dev repo, docs PRs are often companions to implementation PRs. When reviewing docs PRs (MDX files under docs/), check the PR description for any companion/related PR references and verify that the documented features exist in those companion PRs before flagging missing implementations. This ensures docs stay in sync with code changes across related PRs.
Applied to files:
docs/triggering.mdx
📚 Learning: 2026-04-30T20:30:29.458Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3226
File: docs/ai-chat/quick-start.mdx:13-13
Timestamp: 2026-04-30T20:30:29.458Z
Learning: In this repo’s documentation MDX files (`docs/**/*.mdx`), use `ts` and `tsx` (not `typescript`) as the code-fence language tags for TypeScript/TSX snippets. Do not flag `ts`/`tsx` code-fence language tags as incorrect in any docs MDX file, since this is the site-wide Mintlify-compatible convention.
Applied to files:
docs/triggering.mdx
📚 Learning: 2026-05-07T12:25:18.271Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3531
File: apps/webapp/test/sentryTraceContext.server.test.ts:9-47
Timestamp: 2026-05-07T12:25:18.271Z
Learning: In the triggerdotdev/trigger.dev webapp test suite, it is acceptable to leave `createInMemoryTracing()` calls that register a global `NodeTracerProvider` without `afterEach`/`afterAll` teardown. Do not flag this as a test-ordering risk when the code follows the established pattern used across webapp tests (e.g., replication service/benchmark/backfiller tests). This is considered safe because `trace.getActiveSpan()` when called outside a `context.with(...)` block reads `AsyncLocalStorage.getStore()` (undefined when no `run()` scope exists), so it falls back to `ROOT_CONTEXT` with no attached span—regardless of which provider is registered.
Applied to files:
apps/webapp/test/engine/triggerTask.debounce.test.ts
📚 Learning: 2026-05-28T20:02:10.647Z
Learnt from: myftija
Repo: triggerdotdev/trigger.dev PR: 3772
File: apps/webapp/test/findOrCreateBackgroundWorker.test.ts:1-1
Timestamp: 2026-05-28T20:02:10.647Z
Learning: In the triggerdotdev/trigger.dev monorepo, for the `apps/webapp` package use the established convention of storing Vitest tests (unit, integration, and e2e) under `apps/webapp/test/` rather than colocating them next to source files. Do not flag files located in `apps/webapp/test/` as violating any rule that says to colocate tests with source.
Applied to files:
apps/webapp/test/engine/triggerTask.debounce.test.ts
📚 Learning: 2026-07-30T18:43:56.874Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 4426
File: apps/webapp/test/memberDevEnvironments.server.test.ts:124-125
Timestamp: 2026-07-30T18:43:56.874Z
Learning: In the `apps/webapp` test suite (`apps/webapp/test/**`), respect the established test harness in `apps/webapp/test/setup.ts`: it loads `.env` and provides default values for required environment variables so that transitive imports (e.g., `~/env.server`) work without production-style wiring.
During code review, do not require dependency injection/refactoring solely to avoid this existing import path. Only introduce configuration injection if it delivers production-level value (for example, a more general `createEnvironment` abstraction that improves runtime behavior beyond test setup).
Applied to files:
apps/webapp/test/engine/triggerTask.debounce.test.ts
📚 Learning: 2026-05-18T14:40:02.173Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3658
File: packages/core/src/v3/realtimeStreams/manager.test.ts:1-147
Timestamp: 2026-05-18T14:40:02.173Z
Learning: In the triggerdotdev/trigger.dev repo, the policy “Never mock anything — use testcontainers instead” should only be enforced for integration tests that interact with real external services (e.g., Redis, Postgres) via actual infrastructure. For unit tests that exercise pure in-memory logic (e.g., cache semantics) it is OK to stub collaborators such as `ApiClient` using Vitest (`vi.fn()`) to assert call counts or control behavior. Do not flag `vi.fn()`-based `ApiClient` stubs in unit tests as violations of the testcontainers policy.
Applied to files:
apps/webapp/test/engine/triggerTask.debounce.test.ts
📚 Learning: 2026-06-16T09:19:47.637Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3960
File: apps/webapp/test/prismaInfrastructureErrorCapture.test.ts:0-0
Timestamp: 2026-06-16T09:19:47.637Z
Learning: In this repo’s Vitest setup, `vitest.config.ts` uses `globals: true`, so identifiers like `vi`, `describe`, `it`, and `expect` are available as globals in Vitest test files. During code review, do not flag missing `vi`/`describe`/`it`/`expect` imports as a runtime error or correctness issue when they’re used in `*.test.ts/tsx` or `*.spec.ts/tsx` files. Explicit imports are still preferred for consistency, but they’re not required for runtime behavior.
Applied to files:
apps/webapp/test/engine/triggerTask.debounce.test.ts
🪛 LanguageTool
.changeset/debounce-max-duration.md
[style] ~7-~7: To elevate your writing, try using more formal phrasing here.
Context: ...ng its run back for as long as triggers keep arriving, which means it never executes while th...
(CONTINUE_TO_VB)
🔇 Additional comments (10)
apps/webapp/app/env.server.ts (1)
1044-1044: Reject non-positive debounce ceilings.Zero and negative values make the configured ceiling unusable. Add
.positive()before.optional().apps/webapp/app/runEngine/services/triggerTask.server.ts (2)
149-153: Describe all accepted duration forms.The parser accepts
hrand compound forms such as2h30m. Include these forms in the invalid-format guidance.
313-313: LGTM!internal-packages/run-engine/src/engine/index.ts (1)
382-390: LGTM!internal-packages/run-engine/src/engine/systems/debounceSystem.ts (2)
56-61: LGTM!Also applies to: 121-121
655-661: LGTM!Also applies to: 846-851
internal-packages/run-engine/src/engine/types.ts (1)
171-181: LGTM!docs/triggering.mdx (1)
876-890: LGTM!Also applies to: 931-937
.changeset/debounce-max-duration.md (1)
5-17: LGTM!apps/webapp/test/engine/triggerTask.debounce.test.ts (1)
464-464: LGTM!
| if (!debounce.maxDelay) { | ||
| return; | ||
| } | ||
|
|
||
| const maxDelayMs = parseNaturalLanguageDurationInMs(debounce.maxDelay); | ||
|
|
||
| if (maxDelayMs === undefined) { | ||
| throw new ServiceValidationError( | ||
| `Invalid debounce maxDelay: ${debounce.maxDelay}. ` + | ||
| `Supported formats: {number}s, {number}m, {number}h, {number}d, {number}w` | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reject empty debounce.maxDelay values.
z.string().optional() accepts "". Both falsy checks treat that invalid value as an omitted ceiling. This bypasses trigger validation and can disable the intended debounce bound.
apps/webapp/app/runEngine/services/triggerTask.server.ts#L143-L153: test onlydebounce.maxDelay === undefinedfor absence, then parse and reject"".internal-packages/run-engine/src/engine/systems/debounceSystem.ts#L581-L594: use the same undefined check so direct engine callers cannot bypass the ceiling.apps/webapp/test/engine/triggerTask.debounce.test.ts#L528-L547: add an empty-string case that rejects withInvalid debounce maxDelay.
📍 Affects 3 files
apps/webapp/app/runEngine/services/triggerTask.server.ts#L143-L153(this comment)internal-packages/run-engine/src/engine/systems/debounceSystem.ts#L581-L594apps/webapp/test/engine/triggerTask.debounce.test.ts#L528-L547
Debouncing with a
delaylonger than an hour did nothing at all.The engine applied a server-side ceiling on how long a debounced run could be pushed back, measured from the run's
createdAtand defaulting to one hour. A run is only pushed back while its new execution time stays inside that ceiling, so adelayat or above it could never push anything: the waiting run was released, the trigger started its own run, and the next trigger repeated it. Adelay: "12h"produced one run per trigger, each correctly delayed by 12h, with no error raised and nothing on the run to show the debounce key had been ignored.The ceiling is now unset by default. A debounce key with no
maxDelaykeeps collapsing triggers for as long as they keep arriving, which is what the docs have always described. Self-hosters who want a bound can still setRUN_ENGINE_MAXIMUM_DEBOUNCE_DURATION_MS.That has a consequence worth stating plainly, so the docs now carry a warning for it: with no
maxDelay, a continuously triggered key never executes. SetmaxDelaywhen the work has to happen eventually.Failing fast on an unusable
maxDelay. A caller who setsmaxDelayno longer than theirdelayhits exactly the dead end described above, so that pair is now rejected at trigger time instead of silently behaving as if no debounce were set:An unparseable
maxDelayis rejected too, rather than quietly falling back to no bound at all. Only an explicitmaxDelayis checked; with none set there is no ceiling to conflict with.The docs, the
TriggerOptionsJSDoc and the engine option all now state that the room available to push is the gap betweendelayandmaxDelay. The run engine suite gains the case that motivated this: four triggers on one key with a 12h delay now collapse to a single run.