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 12 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 12 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, and so is a delay given as a date rather than a duration, which could never work because the value is re-applied on every push.

The same check runs against a configured server ceiling, so a self-hosted deployment that sets RUN_ENGINE_MAXIMUM_DEBOUNCE_DURATION_MS gets the error rather than the silent failure this PR is about. With no maxDelay and no configured ceiling, which is the default, there is nothing to conflict with and nothing is rejected.

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

⚠️ No Changeset found

Latest commit: d45c149

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

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 changes in detail but omits the required issue reference, checklist, testing section, changelog section, and screenshots section. Add the template sections, link the closing issue, complete the checklist, document explicit test steps, add a changelog summary, and state whether screenshots apply.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main changes: removing the hidden debounce ceiling and rejecting unusable maxDelay values.
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.
✨ 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[bot]

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

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

This comment was marked as resolved.

@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@13da597

trigger.dev

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

@trigger.dev/core

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

@trigger.dev/python

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

@trigger.dev/react-hooks

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

@trigger.dev/redis-worker

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

@trigger.dev/rsc

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

@trigger.dev/schema-to-json

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

@trigger.dev/sdk

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

commit: 13da597

coderabbitai[bot]

This comment was marked as resolved.

…ocument the duration grammar

An empty string passed as maxDelay was falsy everywhere it was checked, so
it read as no ceiling at all rather than as the invalid value it is. The
server ceiling now also rejects zero and negative values at startup, and
the duration grammar in the errors and JSDoc lists the hr suffix and
compound forms that the parser has always accepted.
devin-ai-integration[bot]

This comment was marked as resolved.

Validation only looked at a caller-supplied maxDelay, so a deployment
that configures a server ceiling kept the original silent failure: a
delay at or above it accepted the trigger and then created a run per
trigger. The check now runs against the effective ceiling, whichever of
the two applies, and a delay given as a date is rejected as well since
the value is re-applied on every push and an absolute date can never
work.
…ation

z.coerce turns an empty string into 0, so a deployment that templates the
variable without a value would have failed the new positive check and
refused to boot. Reuse the file's existing blank-normalising pattern so
absent, empty and whitespace all mean no ceiling, while a value that is
actually set still has to be greater than zero.
devin-ai-integration[bot]

This comment was marked as resolved.

…forced limit

A trigger that sets maxDelay uses that value even when it is longer than
the configured ceiling, so the setting bounds only the triggers that omit
it. Say so where an operator reads it.
devin-ai-integration[bot]

This comment was marked as resolved.

…a boot failure

Zero was previously a legal value meaning the window closes on the first
push, so rejecting it would stop an existing deployment from starting
after an upgrade. Accepting it as a real ceiling is worse still, since
the trigger-time check would then reject every debounced trigger. Zero
and blank now both mean no ceiling, which keeps the setting switchable
off and leaves negatives and garbage rejected.

Also note in the docs that a trigger omitting maxDelay now has no bound
at all, where it previously fell back to the built-in ceiling.
The only packages change here is JSDoc, so a changeset would file a
server-side behaviour fix under the core package changelog, where
upgrading the package does nothing for it.
@matt-aitken
matt-aitken enabled auto-merge (squash) August 6, 2026 16:36
devin-ai-integration[bot]

This comment was marked as resolved.

…d run is pushed back

With no ceiling, a continuously triggered key holds its run indefinitely,
and triggerAndWait parents block on that run's waitpoint for the whole
time, holding their concurrency with it.
devin-ai-integration[bot]

This comment was marked as resolved.

…nbounded

The fallback for an unparseable maxDelay is the server ceiling, which is
now normally unset, so the old log line claimed a bound that no longer
exists. Name the real outcome instead.
@matt-aitken
matt-aitken disabled auto-merge August 6, 2026 16:46
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.

2 participants