Skip to content

fix(webapp,run-engine,core): drop the hidden debounce ceiling, fail fast on an unusable maxDelay - #4521

Open
matt-aitken wants to merge 2 commits into
mainfrom
fix/debounce-max-duration-ceiling
Open

fix(webapp,run-engine,core): drop the hidden debounce ceiling, fail fast on an unusable maxDelay#4521
matt-aitken wants to merge 2 commits into
mainfrom
fix/debounce-max-duration-ceiling

Conversation

@matt-aitken

@matt-aitken matt-aitken commented Aug 6, 2026

Copy link
Copy Markdown
Member

Debouncing with a delay longer 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 createdAt and defaulting to one hour. A run is only pushed back while its new execution time stays inside that ceiling, so a delay at or above it could never push anything: the waiting run was released, the trigger started its own run, and the next trigger repeated it. A delay: "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 maxDelay keeps 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 set RUN_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. Set maxDelay when the work has to happen eventually.

Failing fast on an unusable maxDelay. A caller who sets maxDelay no longer than their delay hits 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:

debounce.maxDelay (1h) must be longer than debounce.delay (12h). A debounced run is only
pushed back while it stays inside maxDelay, so with these values every trigger would create
its own run.

An unparseable maxDelay is rejected too, rather than quietly falling back to no bound at all. Only an explicit maxDelay is checked; with none set there is no ceiling to conflict with.

The docs, the TriggerOptions JSDoc and the engine option all now state that the room available to push is the gap between delay and maxDelay. 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.

…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-bot

changeset-bot Bot commented Aug 6, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: d7dd33b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 27 packages
Name Type
@trigger.dev/core Patch
@trigger.dev/build Patch
trigger.dev Patch
@trigger.dev/python Patch
@trigger.dev/redis-worker Patch
@trigger.dev/schema-to-json Patch
@trigger.dev/sdk Patch
@internal/cache Patch
@internal/clickhouse Patch
@internal/llm-model-catalog Patch
@internal/metrics-pipeline Patch
@trigger.dev/rbac Patch
@internal/redis Patch
@internal/replication Patch
@internal/run-engine Patch
@internal/run-store Patch
@internal/schedule-engine Patch
@trigger.dev/sso Patch
@internal/testcontainers Patch
@internal/tracing Patch
@internal/tsql Patch
@internal/dashboard-agent Patch
@internal/sdk-compat-tests Patch
@trigger.dev/react-hooks Patch
@trigger.dev/rsc Patch
@trigger.dev/database Patch
@trigger.dev/otlp-importer Patch

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

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

The change removes the default one-hour debounce ceiling. An unset server ceiling allows matching triggers to continue extending a run. Trigger-level maxDelay overrides the server ceiling. Trigger processing now rejects invalid maxDelay values and values that are not greater than delay. Runtime handling applies the optional limit in fast and locked paths. Documentation, a changeset, and integration tests describe and verify the updated behavior.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description explains the behavior and changes in detail, but it omits the required issue reference, checklist, testing section, changelog section, and screenshots section. Use the repository template and add the issue reference, completed checklist, explicit testing steps, changelog summary, and a screenshots note if screenshots do not apply.
✅ Passed checks (4 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly identifies both primary changes: removing the hidden debounce ceiling and rejecting unusable maxDelay values.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/debounce-max-duration-ceiling

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

Open in Devin Review

Comment on lines +161 to +170
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`
);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 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.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 win

Assert 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 delayUntil increases 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

📥 Commits

Reviewing files that changed from the base of the PR and between 337dda1 and af9fbe4.

📒 Files selected for processing (9)
  • .changeset/debounce-max-duration.md
  • apps/webapp/app/env.server.ts
  • apps/webapp/app/runEngine/services/triggerTask.server.ts
  • apps/webapp/test/engine/triggerTask.debounce.test.ts
  • docs/triggering.mdx
  • internal-packages/run-engine/src/engine/index.ts
  • internal-packages/run-engine/src/engine/tests/debounce.test.ts
  • internal-packages/run-engine/src/engine/types.ts
  • packages/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 dynamic import(); 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/v3 or deprecated client.defineJob.
Add agentcrumbs while writing code using approved namespaces; mark lines with // @Crumbs or blocks with `// `#region` `@crumbs, and strip them before merging.

Files:

  • internal-packages/run-engine/src/engine/index.ts
  • internal-packages/run-engine/src/engine/tests/debounce.test.ts
  • apps/webapp/test/engine/triggerTask.debounce.test.ts
  • internal-packages/run-engine/src/engine/types.ts
  • packages/core/src/v3/types/tasks.ts
  • apps/webapp/app/runEngine/services/triggerTask.server.ts
  • apps/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.ts
  • internal-packages/run-engine/src/engine/tests/debounce.test.ts
  • apps/webapp/test/engine/triggerTask.debounce.test.ts
  • internal-packages/run-engine/src/engine/types.ts
  • packages/core/src/v3/types/tasks.ts
  • apps/webapp/app/runEngine/services/triggerTask.server.ts
  • apps/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.ts
  • internal-packages/run-engine/src/engine/tests/debounce.test.ts
  • apps/webapp/test/engine/triggerTask.debounce.test.ts
  • internal-packages/run-engine/src/engine/types.ts
  • packages/core/src/v3/types/tasks.ts
  • apps/webapp/app/runEngine/services/triggerTask.server.ts
  • apps/webapp/app/env.server.ts
internal-packages/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

For internal packages, use typecheck for verification and never use build as the correctness check.

Files:

  • internal-packages/run-engine/src/engine/index.ts
  • internal-packages/run-engine/src/engine/tests/debounce.test.ts
  • internal-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.ts
  • apps/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.ts
  • packages/core/src/v3/types/tasks.ts
  • apps/webapp/app/runEngine/services/triggerTask.server.ts
  • apps/webapp/app/env.server.ts
apps/webapp/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)

apps/webapp/**/*.{ts,tsx}: Access environment variables through the env export of env.server.ts instead of directly accessing process.env
Use subpath exports from @trigger.dev/core package instead of importing from the root @trigger.dev/core path

Do not reintroduce the removed v1 execution path; RunEngineVersion.V1 branches may only reject or finalize gracefully so v3 clients receive a clean 4xx, never a 5xx.

Files:

  • apps/webapp/test/engine/triggerTask.debounce.test.ts
  • apps/webapp/app/runEngine/services/triggerTask.server.ts
  • apps/webapp/app/env.server.ts
apps/webapp/**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)

Do not import env.server.ts directly 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 typecheck for verification and never use build as the correctness check.

Files:

  • apps/webapp/test/engine/triggerTask.debounce.test.ts
  • apps/webapp/app/runEngine/services/triggerTask.server.ts
  • apps/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/sdk in 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, json

Documentation 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 build for verification.

Files:

  • packages/core/src/v3/types/tasks.ts
packages/core/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Import @trigger.dev/core subpaths 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.
Use useCallback and useMemo only 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.ts
  • apps/webapp/app/env.server.ts
apps/webapp/app/**/*.ts

📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)

apps/webapp/app/**/*.ts: Never use request.signal to detect client disconnects. Use getRequestAbortSignal() from app/services/httpAsyncStorage.server.ts, which is wired to Express response close events.
Access environment variables through the env export from app/env.server.ts; never use process.env directly.
Always use Prisma findFirst instead of findUnique.
Always use the $transaction helper from ~/db.server, never call prisma.$transaction or $replica.$transaction directly. Pass isolation levels as strings, use Serializable for 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.ts
  • apps/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.ts
  • internal-packages/run-engine/src/engine/tests/debounce.test.ts
  • apps/webapp/test/engine/triggerTask.debounce.test.ts
  • internal-packages/run-engine/src/engine/types.ts
  • packages/core/src/v3/types/tasks.ts
  • apps/webapp/app/runEngine/services/triggerTask.server.ts
  • apps/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.ts
  • internal-packages/run-engine/src/engine/tests/debounce.test.ts
  • apps/webapp/test/engine/triggerTask.debounce.test.ts
  • internal-packages/run-engine/src/engine/types.ts
  • packages/core/src/v3/types/tasks.ts
  • apps/webapp/app/runEngine/services/triggerTask.server.ts
  • apps/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.ts
  • internal-packages/run-engine/src/engine/tests/debounce.test.ts
  • apps/webapp/test/engine/triggerTask.debounce.test.ts
  • internal-packages/run-engine/src/engine/types.ts
  • packages/core/src/v3/types/tasks.ts
  • apps/webapp/app/runEngine/services/triggerTask.server.ts
  • apps/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.ts
  • internal-packages/run-engine/src/engine/tests/debounce.test.ts
  • apps/webapp/test/engine/triggerTask.debounce.test.ts
  • internal-packages/run-engine/src/engine/types.ts
  • packages/core/src/v3/types/tasks.ts
  • apps/webapp/app/runEngine/services/triggerTask.server.ts
  • apps/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.ts
  • internal-packages/run-engine/src/engine/tests/debounce.test.ts
  • apps/webapp/test/engine/triggerTask.debounce.test.ts
  • internal-packages/run-engine/src/engine/types.ts
  • packages/core/src/v3/types/tasks.ts
  • apps/webapp/app/runEngine/services/triggerTask.server.ts
  • apps/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.ts
  • internal-packages/run-engine/src/engine/tests/debounce.test.ts
  • apps/webapp/test/engine/triggerTask.debounce.test.ts
  • internal-packages/run-engine/src/engine/types.ts
  • packages/core/src/v3/types/tasks.ts
  • apps/webapp/app/runEngine/services/triggerTask.server.ts
  • apps/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.ts
  • internal-packages/run-engine/src/engine/tests/debounce.test.ts
  • apps/webapp/test/engine/triggerTask.debounce.test.ts
  • internal-packages/run-engine/src/engine/types.ts
  • packages/core/src/v3/types/tasks.ts
  • apps/webapp/app/runEngine/services/triggerTask.server.ts
  • apps/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.ts
  • internal-packages/run-engine/src/engine/tests/debounce.test.ts
  • apps/webapp/test/engine/triggerTask.debounce.test.ts
  • internal-packages/run-engine/src/engine/types.ts
  • packages/core/src/v3/types/tasks.ts
  • apps/webapp/app/runEngine/services/triggerTask.server.ts
  • apps/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.ts
  • internal-packages/run-engine/src/engine/tests/debounce.test.ts
  • apps/webapp/test/engine/triggerTask.debounce.test.ts
  • internal-packages/run-engine/src/engine/types.ts
  • packages/core/src/v3/types/tasks.ts
  • apps/webapp/app/runEngine/services/triggerTask.server.ts
  • apps/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.ts
  • 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:

  • internal-packages/run-engine/src/engine/tests/debounce.test.ts
  • apps/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.ts
  • apps/webapp/app/runEngine/services/triggerTask.server.ts
  • apps/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.ts
  • apps/webapp/app/runEngine/services/triggerTask.server.ts
  • apps/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.ts
  • apps/webapp/app/runEngine/services/triggerTask.server.ts
  • apps/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.ts
  • apps/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 & Integration

Custom 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!

Comment thread apps/webapp/app/env.server.ts Outdated
Comment on lines +1045 to +1048
RUN_ENGINE_MAXIMUM_DEBOUNCE_DURATION_MS: z.coerce
.number()
.int()
.default(60_000 * 60), // 1 hour
.default(24 * 60 * 60 * 1000),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 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.

Suggested change
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),

Comment on lines +153 to +169
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`
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 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: Include hr and 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 accept h or hr.
📍 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.
@matt-aitken matt-aitken changed the title fix(webapp,run-engine,core): raise the debounce ceiling to 24h and reject windows that cannot debounce fix(webapp,run-engine,core): drop the hidden debounce ceiling, fail fast on an unusable maxDelay Aug 6, 2026

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 3 new potential issues.

Open in Devin Review

Comment on lines +143 to +145
if (!debounce.maxDelay) {
return;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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.
Open in Devin Review

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,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 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".

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +156 to +164
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.`
);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 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.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@pkg-pr-new

pkg-pr-new Bot commented Aug 6, 2026

Copy link
Copy Markdown

Open in StackBlitz

@trigger.dev/build

npm i https://pkg.pr.new/@trigger.dev/build@d7dd33b

trigger.dev

npm i https://pkg.pr.new/trigger.dev@d7dd33b

@trigger.dev/core

npm i https://pkg.pr.new/@trigger.dev/core@d7dd33b

@trigger.dev/python

npm i https://pkg.pr.new/@trigger.dev/python@d7dd33b

@trigger.dev/react-hooks

npm i https://pkg.pr.new/@trigger.dev/react-hooks@d7dd33b

@trigger.dev/redis-worker

npm i https://pkg.pr.new/@trigger.dev/redis-worker@d7dd33b

@trigger.dev/rsc

npm i https://pkg.pr.new/@trigger.dev/rsc@d7dd33b

@trigger.dev/schema-to-json

npm i https://pkg.pr.new/@trigger.dev/schema-to-json@d7dd33b

@trigger.dev/sdk

npm i https://pkg.pr.new/@trigger.dev/sdk@d7dd33b

commit: d7dd33b

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 win

Correct the maxDelay timeline.

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 lift

Move debounce.maxDelay validation into TriggerTaskRequestBody.

The request schema is the shared Zod entry point for TriggerTaskRequestBody consumers, 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

📥 Commits

Reviewing files that changed from the base of the PR and between af9fbe4 and d7dd33b.

📒 Files selected for processing (10)
  • .changeset/debounce-max-duration.md
  • apps/webapp/app/env.server.ts
  • apps/webapp/app/runEngine/services/triggerTask.server.ts
  • apps/webapp/test/engine/triggerTask.debounce.test.ts
  • docs/triggering.mdx
  • internal-packages/run-engine/src/engine/index.ts
  • internal-packages/run-engine/src/engine/systems/debounceSystem.ts
  • internal-packages/run-engine/src/engine/tests/debounce.test.ts
  • internal-packages/run-engine/src/engine/types.ts
  • packages/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 dynamic import(); 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/v3 or deprecated client.defineJob.
Add agentcrumbs while writing code using approved namespaces; mark lines with // @Crumbs or blocks with `// `#region` `@crumbs, and strip them before merging.

Files:

  • apps/webapp/app/runEngine/services/triggerTask.server.ts
  • apps/webapp/app/env.server.ts
  • internal-packages/run-engine/src/engine/systems/debounceSystem.ts
  • internal-packages/run-engine/src/engine/index.ts
  • apps/webapp/test/engine/triggerTask.debounce.test.ts
  • internal-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.ts
  • apps/webapp/app/env.server.ts
  • apps/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.ts
  • apps/webapp/app/env.server.ts
  • internal-packages/run-engine/src/engine/systems/debounceSystem.ts
  • internal-packages/run-engine/src/engine/index.ts
  • apps/webapp/test/engine/triggerTask.debounce.test.ts
  • internal-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.ts
  • apps/webapp/app/env.server.ts
  • internal-packages/run-engine/src/engine/systems/debounceSystem.ts
  • internal-packages/run-engine/src/engine/index.ts
  • apps/webapp/test/engine/triggerTask.debounce.test.ts
  • internal-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 the env export of env.server.ts instead of directly accessing process.env
Use subpath exports from @trigger.dev/core package instead of importing from the root @trigger.dev/core path

Do not reintroduce the removed v1 execution path; RunEngineVersion.V1 branches may only reject or finalize gracefully so v3 clients receive a clean 4xx, never a 5xx.

Files:

  • apps/webapp/app/runEngine/services/triggerTask.server.ts
  • apps/webapp/app/env.server.ts
  • apps/webapp/test/engine/triggerTask.debounce.test.ts
apps/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

For apps, use typecheck for verification and never use build as the correctness check.

Files:

  • apps/webapp/app/runEngine/services/triggerTask.server.ts
  • apps/webapp/app/env.server.ts
  • apps/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.
Use useCallback and useMemo only 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.ts
  • apps/webapp/app/env.server.ts
apps/webapp/app/**/*.ts

📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)

apps/webapp/app/**/*.ts: Never use request.signal to detect client disconnects. Use getRequestAbortSignal() from app/services/httpAsyncStorage.server.ts, which is wired to Express response close events.
Access environment variables through the env export from app/env.server.ts; never use process.env directly.
Always use Prisma findFirst instead of findUnique.
Always use the $transaction helper from ~/db.server, never call prisma.$transaction or $replica.$transaction directly. Pass isolation levels as strings, use Serializable for 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.ts
  • apps/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 typecheck for verification and never use build as the correctness check.

Files:

  • internal-packages/run-engine/src/engine/systems/debounceSystem.ts
  • internal-packages/run-engine/src/engine/index.ts
  • internal-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/sdk in 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, json

Documentation 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.ts directly 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.ts
  • apps/webapp/app/env.server.ts
  • internal-packages/run-engine/src/engine/systems/debounceSystem.ts
  • internal-packages/run-engine/src/engine/index.ts
  • apps/webapp/test/engine/triggerTask.debounce.test.ts
  • internal-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.ts
  • apps/webapp/app/env.server.ts
  • internal-packages/run-engine/src/engine/systems/debounceSystem.ts
  • internal-packages/run-engine/src/engine/index.ts
  • apps/webapp/test/engine/triggerTask.debounce.test.ts
  • internal-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.ts
  • apps/webapp/app/env.server.ts
  • internal-packages/run-engine/src/engine/systems/debounceSystem.ts
  • internal-packages/run-engine/src/engine/index.ts
  • apps/webapp/test/engine/triggerTask.debounce.test.ts
  • internal-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.ts
  • apps/webapp/app/env.server.ts
  • internal-packages/run-engine/src/engine/systems/debounceSystem.ts
  • internal-packages/run-engine/src/engine/index.ts
  • apps/webapp/test/engine/triggerTask.debounce.test.ts
  • internal-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.ts
  • apps/webapp/app/env.server.ts
  • internal-packages/run-engine/src/engine/systems/debounceSystem.ts
  • internal-packages/run-engine/src/engine/index.ts
  • apps/webapp/test/engine/triggerTask.debounce.test.ts
  • internal-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.ts
  • apps/webapp/app/env.server.ts
  • internal-packages/run-engine/src/engine/systems/debounceSystem.ts
  • internal-packages/run-engine/src/engine/index.ts
  • apps/webapp/test/engine/triggerTask.debounce.test.ts
  • internal-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.ts
  • apps/webapp/app/env.server.ts
  • internal-packages/run-engine/src/engine/systems/debounceSystem.ts
  • internal-packages/run-engine/src/engine/index.ts
  • apps/webapp/test/engine/triggerTask.debounce.test.ts
  • internal-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.ts
  • apps/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.ts
  • apps/webapp/app/env.server.ts
  • apps/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.ts
  • apps/webapp/app/env.server.ts
  • apps/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.ts
  • apps/webapp/app/env.server.ts
  • apps/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.ts
  • apps/webapp/app/env.server.ts
  • internal-packages/run-engine/src/engine/systems/debounceSystem.ts
  • internal-packages/run-engine/src/engine/index.ts
  • apps/webapp/test/engine/triggerTask.debounce.test.ts
  • internal-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.ts
  • apps/webapp/app/env.server.ts
  • internal-packages/run-engine/src/engine/systems/debounceSystem.ts
  • internal-packages/run-engine/src/engine/index.ts
  • apps/webapp/test/engine/triggerTask.debounce.test.ts
  • internal-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 hr and compound forms such as 2h30m. 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!

Comment on lines +143 to +153
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`
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 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 only debounce.maxDelay === undefined for 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 with Invalid 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-L594
  • apps/webapp/test/engine/triggerTask.debounce.test.ts#L528-L547

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant