Skip to content

feat(webapp,redis-worker): add global mollifier gate mode#3837

Draft
d-cs wants to merge 1 commit into
mainfrom
mollifier-global-gate
Draft

feat(webapp,redis-worker): add global mollifier gate mode#3837
d-cs wants to merge 1 commit into
mainfrom
mollifier-global-gate

Conversation

@d-cs
Copy link
Copy Markdown
Collaborator

@d-cs d-cs commented Jun 5, 2026

Summary

The mollifier trip gate is per-env: each environment is rate-limited independently. That structurally cannot bound the aggregate trigger rate hitting shared infrastructure (the primary database) — N environments each sitting just under their per-env threshold sum to N× the intended ceiling. This adds an opt-in global gate mode that rate-limits the fleet-wide aggregate via a single shared counter. It's behind config and off by default (existing per_env behavior unchanged).

Design

MollifierBuffer.evaluateTripGlobal(opts) reuses the existing fixed-window trip Lua but against fixed mollifier:rate:{global} / mollifier:tripped:{global} keys (hash-tagged so the two-key script stays on one slot under Redis Cluster). Every trigger increments the same counter, so the gate sees aggregate load and ignores per-env contributions entirely — by design, so a global cap can't be sidestepped by spreading load across envs.

The trip evaluator gains a mode: "per_env" | "global" option; in global mode it calls evaluateTripGlobal and reports reason: "global_rate" (a distinct metric label from per_env_rate).

Tuning is kept in separate env vars so the two regimes never share values: TRIGGER_MOLLIFIER_GATE_MODE (default per_env), plus TRIGGER_MOLLIFIER_GLOBAL_TRIP_WINDOW_MS, TRIGGER_MOLLIFIER_GLOBAL_TRIP_THRESHOLD, TRIGGER_MOLLIFIER_GLOBAL_HOLD_MS.

Test plan

  • redis-worker: evaluateTripGlobal trips once the global counter exceeds threshold, ignoring env
  • redis-worker: global counter catches aggregate load spread across envs that per-env tripping misses
  • redis-worker: global window resets after a traffic lull; tripped marker outlives the rate window (holdMs > windowMs); INCR atomic under 100 concurrent calls
  • webapp: evaluator global vs per_env mode; switching to global mid-flight starts the counter cold and is isolated from per-env counters
  • webapp: reason: "global_rate" propagates through evaluateGate to recordDecision for both shadow-log and mollify outcomes
  • existing mollifier gate + evaluator suites still green
  • reviewer: confirm the global threshold default before enabling in any environment

Manual verification (local)

Ran against a local webapp + Redis with GATE_MODE=global:

  • Global on: firing triggers created only mollifier:rate:{global} / mollifier:tripped:{global} (no per-env keys), every decision logged reason: "global_rate", and the count-vs-threshold boundary was exact (count > threshold trips).
  • Disabled (TRIGGER_MOLLIFIER_ENABLED=0): triggers passed through with zero mollifier:* keys created and no decision logs — the kill-switch short-circuits before the mode is read, so this holds identically for both per_env and global.

The per-env trip rate-limits each environment independently and cannot bound the aggregate trigger rate hitting the primary database. Global mode rate-limits the fleet-wide aggregate via a single shared Redis counter (mollifier:rate:{global}), ignoring per-env contributions. Behind config, default unchanged (per_env).

- redis-worker: MollifierBuffer.evaluateTripGlobal reuses the trip Lua against hash-tagged global keys
- trip evaluator gains a mode option; global mode reports reason "global_rate"
- separate TRIGGER_MOLLIFIER_GATE_MODE + TRIGGER_MOLLIFIER_GLOBAL_* env vars

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Jun 5, 2026

🦋 Changeset detected

Latest commit: 3a47cc7

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

This PR includes changesets to release 25 packages
Name Type
@trigger.dev/redis-worker Patch
@internal/run-engine Patch
@internal/schedule-engine Patch
@trigger.dev/build Patch
@trigger.dev/core Patch
@trigger.dev/plugins Patch
@trigger.dev/python Patch
@trigger.dev/react-hooks Patch
@trigger.dev/rsc Patch
@trigger.dev/schema-to-json Patch
@trigger.dev/sdk Patch
@trigger.dev/database Patch
@trigger.dev/otlp-importer Patch
@trigger.dev/rbac Patch
trigger.dev Patch
@internal/cache Patch
@internal/clickhouse Patch
@internal/llm-model-catalog Patch
@internal/redis Patch
@internal/replication Patch
@internal/testcontainers Patch
@internal/tracing Patch
@internal/tsql Patch
@internal/zod-worker Patch
@internal/sdk-compat-tests 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
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 5, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 231c0cdd-09c7-4d6f-963c-7fe89f0df79a

📥 Commits

Reviewing files that changed from the base of the PR and between 64151d6 and 3a47cc7.

📒 Files selected for processing (9)
  • .changeset/mollifier-global-gate.md
  • apps/webapp/app/env.server.ts
  • apps/webapp/app/v3/mollifier/mollifierGate.server.ts
  • apps/webapp/app/v3/mollifier/mollifierTelemetry.server.ts
  • apps/webapp/app/v3/mollifier/mollifierTripEvaluator.server.ts
  • apps/webapp/test/mollifierGate.test.ts
  • apps/webapp/test/mollifierTripEvaluator.test.ts
  • packages/redis-worker/src/mollifier/buffer.test.ts
  • packages/redis-worker/src/mollifier/buffer.ts
📜 Recent review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (21)
  • GitHub Check: internal / 🧪 Unit Tests: Internal (5, 8)
  • GitHub Check: internal / 🧪 Unit Tests: Internal (7, 8)
  • GitHub Check: internal / 🧪 Unit Tests: Internal (6, 8)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 8)
  • GitHub Check: internal / 🧪 Unit Tests: Internal (4, 8)
  • GitHub Check: internal / 🧪 Unit Tests: Internal (8, 8)
  • GitHub Check: internal / 🧪 Unit Tests: Internal (1, 8)
  • GitHub Check: internal / 🧪 Unit Tests: Internal (3, 8)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 8)
  • GitHub Check: internal / 🧪 Unit Tests: Internal (2, 8)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 8)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 8)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 8)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 8)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 8)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 8)
  • GitHub Check: packages / 🧪 Unit Tests: Packages (1, 1)
  • GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp
  • GitHub Check: typecheck / typecheck
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Build and publish previews
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{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

Import from @trigger.dev/sdk when writing Trigger.dev tasks. Never use @trigger.dev/sdk/v3 or deprecated client.defineJob

Files:

  • apps/webapp/app/v3/mollifier/mollifierTripEvaluator.server.ts
  • packages/redis-worker/src/mollifier/buffer.ts
  • apps/webapp/test/mollifierGate.test.ts
  • apps/webapp/app/v3/mollifier/mollifierTelemetry.server.ts
  • apps/webapp/app/env.server.ts
  • apps/webapp/test/mollifierTripEvaluator.test.ts
  • packages/redis-worker/src/mollifier/buffer.test.ts
  • apps/webapp/app/v3/mollifier/mollifierGate.server.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/v3/mollifier/mollifierTripEvaluator.server.ts
  • apps/webapp/test/mollifierGate.test.ts
  • apps/webapp/app/v3/mollifier/mollifierTelemetry.server.ts
  • apps/webapp/app/env.server.ts
  • apps/webapp/test/mollifierTripEvaluator.test.ts
  • apps/webapp/app/v3/mollifier/mollifierGate.server.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use function declarations instead of default exports

**/*.{ts,tsx,js,jsx}: Prefer static imports over dynamic imports. Only use dynamic import() when circular dependencies cannot be resolved, code splitting is needed for performance, or the module must be loaded conditionally at runtime
Import subpaths only from packages/core (@trigger.dev/core), never import from the root

Files:

  • apps/webapp/app/v3/mollifier/mollifierTripEvaluator.server.ts
  • packages/redis-worker/src/mollifier/buffer.ts
  • apps/webapp/test/mollifierGate.test.ts
  • apps/webapp/app/v3/mollifier/mollifierTelemetry.server.ts
  • apps/webapp/app/env.server.ts
  • apps/webapp/test/mollifierTripEvaluator.test.ts
  • packages/redis-worker/src/mollifier/buffer.test.ts
  • apps/webapp/app/v3/mollifier/mollifierGate.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:

  • apps/webapp/app/v3/mollifier/mollifierTripEvaluator.server.ts
  • packages/redis-worker/src/mollifier/buffer.ts
  • apps/webapp/test/mollifierGate.test.ts
  • apps/webapp/app/v3/mollifier/mollifierTelemetry.server.ts
  • apps/webapp/app/env.server.ts
  • apps/webapp/test/mollifierTripEvaluator.test.ts
  • packages/redis-worker/src/mollifier/buffer.test.ts
  • apps/webapp/app/v3/mollifier/mollifierGate.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

Use named constants for sentinel/placeholder values (e.g. const UNSET_VALUE = '__unset__') instead of raw string literals scattered across comparisons

Files:

  • apps/webapp/app/v3/mollifier/mollifierTripEvaluator.server.ts
  • apps/webapp/test/mollifierGate.test.ts
  • apps/webapp/app/v3/mollifier/mollifierTelemetry.server.ts
  • apps/webapp/app/env.server.ts
  • apps/webapp/test/mollifierTripEvaluator.test.ts
  • apps/webapp/app/v3/mollifier/mollifierGate.server.ts
apps/webapp/**/*.server.ts

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

apps/webapp/**/*.server.ts: Never use request.signal for detecting client disconnects. Use getRequestAbortSignal() from app/services/httpAsyncStorage.server.ts instead, which is wired directly to Express res.on('close') and fires reliably
Access environment variables via env export from app/env.server.ts. Never use process.env directly
Always use findFirst instead of findUnique in Prisma queries. findUnique has an implicit DataLoader that batches concurrent calls and has active bugs even in Prisma 6.x (uppercase UUIDs returning null, composite key SQL correctness issues, 5-10x worse performance). findFirst is never batched and avoids this entire class of issues

Files:

  • apps/webapp/app/v3/mollifier/mollifierTripEvaluator.server.ts
  • apps/webapp/app/v3/mollifier/mollifierTelemetry.server.ts
  • apps/webapp/app/env.server.ts
  • apps/webapp/app/v3/mollifier/mollifierGate.server.ts
**/*.{js,ts,tsx,jsx,css,json,md}

📄 CodeRabbit inference engine (AGENTS.md)

Use Prettier for code formatting and run pnpm run format before committing

Files:

  • apps/webapp/app/v3/mollifier/mollifierTripEvaluator.server.ts
  • packages/redis-worker/src/mollifier/buffer.ts
  • apps/webapp/test/mollifierGate.test.ts
  • apps/webapp/app/v3/mollifier/mollifierTelemetry.server.ts
  • apps/webapp/app/env.server.ts
  • apps/webapp/test/mollifierTripEvaluator.test.ts
  • packages/redis-worker/src/mollifier/buffer.test.ts
  • apps/webapp/app/v3/mollifier/mollifierGate.server.ts
**/*.{test,spec}.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use vitest for all tests in the Trigger.dev repository

Files:

  • apps/webapp/test/mollifierGate.test.ts
  • apps/webapp/test/mollifierTripEvaluator.test.ts
  • packages/redis-worker/src/mollifier/buffer.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

For testable code, never import env.server.ts in test files. Pass configuration as options instead (e.g., realtimeClient.server.ts takes config as constructor arg, realtimeClientGlobal.server.ts creates singleton with env config)

Files:

  • apps/webapp/test/mollifierGate.test.ts
  • apps/webapp/test/mollifierTripEvaluator.test.ts
**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.test.{ts,tsx}: Never mock anything in tests - use testcontainers instead
Test files should be placed next to source files (e.g., MyService.ts -> MyService.test.ts)

Files:

  • apps/webapp/test/mollifierGate.test.ts
  • apps/webapp/test/mollifierTripEvaluator.test.ts
  • packages/redis-worker/src/mollifier/buffer.test.ts
**/*.test.{js,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.test.{js,ts,tsx}: Test files should live beside the files under test and use descriptive describe and it blocks
Use vitest for unit testing
Tests should avoid mocks or stubs and use helpers from @internal/testcontainers when Redis or Postgres are needed

Files:

  • apps/webapp/test/mollifierGate.test.ts
  • apps/webapp/test/mollifierTripEvaluator.test.ts
  • packages/redis-worker/src/mollifier/buffer.test.ts
🧠 Learnings (46)
📓 Common learnings
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3754
File: apps/webapp/app/v3/mollifierStaleSweepWorker.server.ts:30-32
Timestamp: 2026-06-01T12:05:44.112Z
Learning: In the triggerdotdev/trigger.dev codebase, the mollifier stale-entry sweep (`initMollifierStaleSweepWorker` in `apps/webapp/app/v3/mollifierStaleSweepWorker.server.ts`) intentionally runs per-webapp instance without a distributed lease in its initial implementation. All Redis ops (cursor, counts hash, reconcile) are individually atomic and produce correct shared state even with multiple concurrent sweepers. The known limitation is that OpenTelemetry metric output (`recordStaleEntry`, `reportStaleEntrySnapshot`) multiplies by N webapp instances, mis-calibrating alert thresholds by a factor of N. A SETNX-based per-tick lease (SET NX PX on the sweep's existing Redis) is the planned follow-up fix. Until then, alert thresholds should be scaled accordingly. Do not re-raise this as a blocking correctness bug — it is a documented metric-scaling limitation with a tracked follow-up.
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3754
File: apps/webapp/app/env.server.ts:1104-1129
Timestamp: 2026-06-01T11:37:12.623Z
Learning: In triggerdotdev/trigger.dev (apps/webapp/app/env.server.ts), new background/periodic worker feature flags should hard-default to "0" (explicitly opt-in) rather than inheriting a parent feature flag (e.g., TRIGGER_MOLLIFIER_ENABLED). Inheriting a parent flag causes the new worker to auto-start on upgrade for any deployment that already has the parent flag enabled, turning on unexpected background load without an explicit rollout step. Each new worker component should require its own explicit opt-in via its own env var (e.g., TRIGGER_MOLLIFIER_STALE_SWEEP_ENABLED defaults to "0", not to process.env.TRIGGER_MOLLIFIER_ENABLED ?? "0").
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3614
File: apps/webapp/app/v3/mollifier/mollifierGate.server.ts:48-52
Timestamp: 2026-05-14T08:21:10.439Z
Learning: In triggerdotdev/trigger.dev's v3 feature-flag system, `flag()` in `apps/webapp/app/v3/featureFlags.server.ts` supports per-org gating via an `overrides` argument: callers pass `Organization.featureFlags` (a JSON column on the Org row) as overrides, which take precedence over the global `featureFlag` Prisma table row. This pattern is used by the AI beta, private-connections beta, query-access, and compute beta flags, and also by the mollifier gate (`resolveOrgFlag` in `apps/webapp/app/v3/mollifier/mollifierGate.server.ts`), where `GateInputs.orgFeatureFlags` is passed as overrides and `triggerTask.server.ts` threads `environment.organization.featureFlags` into the gate call. No schema change is required for per-org gating — use this overrides mechanism rather than adding an `orgId` field to `FlagsOptions`.
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3756
File: apps/webapp/app/v3/services/resetIdempotencyKey.server.ts:65-94
Timestamp: 2026-06-01T15:01:35.175Z
Learning: In `apps/webapp/app/v3/services/resetIdempotencyKey.server.ts` (triggerdotdev/trigger.dev), a transient `buffer.resetIdempotency()` failure when `pgCount > 0` does NOT warrant a 503 and should return success. The mollifier `ack` and `fail` Lua scripts always DEL the idempotency lookup key as part of the run's natural lifecycle (drain→ack or terminal→fail or cancel-bifurcation), so stale buffered idempotency lookups converge automatically without caller retries. Only when `pgCount === 0 && bufferResetFailed` is a 503 appropriate, because then the run's existence is genuinely unobservable (the buffer outage hides a potentially matching buffered run). The test "returns success when PG cleared >=1 run, even if the buffer reset throws" documents this contract explicitly.
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3757
File: apps/webapp/app/routes/realtime.v1.runs.$runId.ts:86-86
Timestamp: 2026-05-26T13:46:37.556Z
Learning: In `triggerdotdev/trigger.dev`, the `mollifier.stale_entries` counter and `mollifier.stale_entries.current` gauge intentionally attach `envId` as a metric dimension. This is deliberate because the inline alert recommendation in the codebase prescribes `mollifier_stale_entries_current{envId=...} > 0 for 5m` as the alert expression, making `envId` a load-bearing label selector. Do not flag these metrics for high-cardinality attribute violations — the pattern is an accepted exception to the general guideline of avoiding UUID/ID metric attributes.
📚 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/v3/mollifier/mollifierTripEvaluator.server.ts
  • packages/redis-worker/src/mollifier/buffer.ts
  • apps/webapp/test/mollifierGate.test.ts
  • apps/webapp/app/v3/mollifier/mollifierTelemetry.server.ts
  • apps/webapp/app/env.server.ts
  • apps/webapp/test/mollifierTripEvaluator.test.ts
  • packages/redis-worker/src/mollifier/buffer.test.ts
  • apps/webapp/app/v3/mollifier/mollifierGate.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:

  • apps/webapp/app/v3/mollifier/mollifierTripEvaluator.server.ts
  • packages/redis-worker/src/mollifier/buffer.ts
  • apps/webapp/test/mollifierGate.test.ts
  • apps/webapp/app/v3/mollifier/mollifierTelemetry.server.ts
  • apps/webapp/app/env.server.ts
  • apps/webapp/test/mollifierTripEvaluator.test.ts
  • packages/redis-worker/src/mollifier/buffer.test.ts
  • apps/webapp/app/v3/mollifier/mollifierGate.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:

  • apps/webapp/app/v3/mollifier/mollifierTripEvaluator.server.ts
  • packages/redis-worker/src/mollifier/buffer.ts
  • apps/webapp/test/mollifierGate.test.ts
  • apps/webapp/app/v3/mollifier/mollifierTelemetry.server.ts
  • apps/webapp/app/env.server.ts
  • apps/webapp/test/mollifierTripEvaluator.test.ts
  • packages/redis-worker/src/mollifier/buffer.test.ts
  • apps/webapp/app/v3/mollifier/mollifierGate.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:

  • apps/webapp/app/v3/mollifier/mollifierTripEvaluator.server.ts
  • packages/redis-worker/src/mollifier/buffer.ts
  • apps/webapp/test/mollifierGate.test.ts
  • apps/webapp/app/v3/mollifier/mollifierTelemetry.server.ts
  • apps/webapp/app/env.server.ts
  • apps/webapp/test/mollifierTripEvaluator.test.ts
  • packages/redis-worker/src/mollifier/buffer.test.ts
  • apps/webapp/app/v3/mollifier/mollifierGate.server.ts
📚 Learning: 2026-03-29T19:16:28.864Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 3291
File: apps/webapp/app/v3/featureFlags.ts:53-65
Timestamp: 2026-03-29T19:16:28.864Z
Learning: When reviewing TypeScript code that uses Zod v3, treat `z.coerce.*()` schemas as their direct Zod type (e.g., `z.coerce.boolean()` returns a `ZodBoolean` with `_def.typeName === "ZodBoolean"`) rather than a `ZodEffects`. Only `.preprocess()`, `.refine()`/`.superRefine()`, and `.transform()` are expected to wrap schemas in `ZodEffects`. Therefore, in reviewers’ logic like `getFlagControlType`, do not flag/unblock failures that require unwrapping `ZodEffects` when the input schema is a `z.coerce.*` schema.

Applied to files:

  • apps/webapp/app/v3/mollifier/mollifierTripEvaluator.server.ts
  • apps/webapp/app/v3/mollifier/mollifierTelemetry.server.ts
  • apps/webapp/app/v3/mollifier/mollifierGate.server.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/v3/mollifier/mollifierTripEvaluator.server.ts
  • apps/webapp/app/v3/mollifier/mollifierTelemetry.server.ts
  • apps/webapp/app/env.server.ts
  • apps/webapp/app/v3/mollifier/mollifierGate.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/v3/mollifier/mollifierTripEvaluator.server.ts
  • apps/webapp/test/mollifierGate.test.ts
  • apps/webapp/app/v3/mollifier/mollifierTelemetry.server.ts
  • apps/webapp/app/env.server.ts
  • apps/webapp/test/mollifierTripEvaluator.test.ts
  • apps/webapp/app/v3/mollifier/mollifierGate.server.ts
📚 Learning: 2026-05-14T08:21:07.614Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3614
File: apps/webapp/app/v3/mollifier/mollifierGate.server.ts:48-52
Timestamp: 2026-05-14T08:21:07.614Z
Learning: When using Trigger.dev v3 feature flags in the webapp, prefer the existing per-org gating mechanism supported by `flag()` via the `overrides` argument. Pass `Organization.featureFlags` (from `environment.organization.featureFlags`) as the `overrides` value; overrides must take precedence over the global `featureFlag` row. Do not require schema changes or add an `orgId` field to `FlagsOptions` for per-org gating—use the overrides pattern consistently (e.g., in gate flows like `resolveOrgFlag` and any server code that threads `environment.organization.featureFlags` into the gate call).

Applied to files:

  • apps/webapp/app/v3/mollifier/mollifierTripEvaluator.server.ts
  • apps/webapp/app/v3/mollifier/mollifierTelemetry.server.ts
  • apps/webapp/app/v3/mollifier/mollifierGate.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/v3/mollifier/mollifierTripEvaluator.server.ts
  • packages/redis-worker/src/mollifier/buffer.ts
  • apps/webapp/test/mollifierGate.test.ts
  • apps/webapp/app/v3/mollifier/mollifierTelemetry.server.ts
  • apps/webapp/app/env.server.ts
  • apps/webapp/test/mollifierTripEvaluator.test.ts
  • packages/redis-worker/src/mollifier/buffer.test.ts
  • apps/webapp/app/v3/mollifier/mollifierGate.server.ts
📚 Learning: 2026-06-01T12:05:44.112Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3754
File: apps/webapp/app/v3/mollifierStaleSweepWorker.server.ts:30-32
Timestamp: 2026-06-01T12:05:44.112Z
Learning: In the triggerdotdev/trigger.dev codebase, the mollifier stale-entry sweep (`initMollifierStaleSweepWorker` in `apps/webapp/app/v3/mollifierStaleSweepWorker.server.ts`) intentionally runs per-webapp instance without a distributed lease in its initial implementation. All Redis ops (cursor, counts hash, reconcile) are individually atomic and produce correct shared state even with multiple concurrent sweepers. The known limitation is that OpenTelemetry metric output (`recordStaleEntry`, `reportStaleEntrySnapshot`) multiplies by N webapp instances, mis-calibrating alert thresholds by a factor of N. A SETNX-based per-tick lease (SET NX PX on the sweep's existing Redis) is the planned follow-up fix. Until then, alert thresholds should be scaled accordingly. Do not re-raise this as a blocking correctness bug — it is a documented metric-scaling limitation with a tracked follow-up.

Applied to files:

  • packages/redis-worker/src/mollifier/buffer.ts
  • .changeset/mollifier-global-gate.md
  • apps/webapp/test/mollifierTripEvaluator.test.ts
  • packages/redis-worker/src/mollifier/buffer.test.ts
📚 Learning: 2026-06-01T15:01:35.175Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3756
File: apps/webapp/app/v3/services/resetIdempotencyKey.server.ts:65-94
Timestamp: 2026-06-01T15:01:35.175Z
Learning: In `apps/webapp/app/v3/services/resetIdempotencyKey.server.ts` (triggerdotdev/trigger.dev), a transient `buffer.resetIdempotency()` failure when `pgCount > 0` does NOT warrant a 503 and should return success. The mollifier `ack` and `fail` Lua scripts always DEL the idempotency lookup key as part of the run's natural lifecycle (drain→ack or terminal→fail or cancel-bifurcation), so stale buffered idempotency lookups converge automatically without caller retries. Only when `pgCount === 0 && bufferResetFailed` is a 503 appropriate, because then the run's existence is genuinely unobservable (the buffer outage hides a potentially matching buffered run). The test "returns success when PG cleared >=1 run, even if the buffer reset throws" documents this contract explicitly.

Applied to files:

  • packages/redis-worker/src/mollifier/buffer.ts
  • .changeset/mollifier-global-gate.md
  • apps/webapp/test/mollifierTripEvaluator.test.ts
  • packages/redis-worker/src/mollifier/buffer.test.ts
📚 Learning: 2026-03-02T12:43:43.173Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: packages/redis-worker/CLAUDE.md:0-0
Timestamp: 2026-03-02T12:43:43.173Z
Learning: Applies to packages/redis-worker/**/redis-worker/src/worker.ts : Worker loop and job processing should implement concurrency control in src/worker.ts

Applied to files:

  • packages/redis-worker/src/mollifier/buffer.ts
📚 Learning: 2026-01-12T17:18:09.451Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 2870
File: apps/webapp/app/services/redisConcurrencyLimiter.server.ts:56-66
Timestamp: 2026-01-12T17:18:09.451Z
Learning: In `apps/webapp/app/services/redisConcurrencyLimiter.server.ts`, the query concurrency limiter will not be deployed with Redis Cluster mode, so multi-key operations (keyKey and globalKey in different hash slots) are acceptable and will function correctly in standalone Redis mode.

Applied to files:

  • packages/redis-worker/src/mollifier/buffer.ts
  • packages/redis-worker/src/mollifier/buffer.test.ts
📚 Learning: 2026-05-15T08:05:57.683Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3625
File: apps/webapp/app/services/taskMetadataCache.server.ts:270-291
Timestamp: 2026-05-15T08:05:57.683Z
Learning: In the triggerdotdev/trigger.dev codebase, `populateByCurrentWorker()` in `apps/webapp/app/services/taskMetadataCache.server.ts` intentionally logs and swallows Redis errors rather than rethrowing. The design rationale: rethrowing would propagate into `ChangeCurrentDeploymentService.call` and break deploy promotion when Redis is briefly unavailable; the 24h `TASK_META_CACHE_CURRENT_ENV_TTL_SECONDS` TTL acts as the self-healing window for cache drift, and next-promotion overwrites the env key sooner in practice. A compensating DEL on failure is also not a win because if Redis is unreachable the DEL fails identically, and Lua scripts are atomic so a partial write is impossible. Do not flag this log+swallow pattern as a bug in future reviews.

Applied to files:

  • packages/redis-worker/src/mollifier/buffer.ts
  • .changeset/mollifier-global-gate.md
  • apps/webapp/test/mollifierTripEvaluator.test.ts
  • packages/redis-worker/src/mollifier/buffer.test.ts
📚 Learning: 2026-03-03T13:08:03.862Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3166
File: packages/redis-worker/src/fair-queue/index.ts:1114-1121
Timestamp: 2026-03-03T13:08:03.862Z
Learning: In packages/redis-worker/src/fair-queue/index.ts, it's acceptable for the worker queue depth cap check to allow overshooting by up to batchClaimSize messages per iteration, as the next iteration will recheck and prevent sustained growth beyond the limit.

Applied to files:

  • packages/redis-worker/src/mollifier/buffer.ts
  • packages/redis-worker/src/mollifier/buffer.test.ts
📚 Learning: 2026-03-02T12:43:43.173Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: packages/redis-worker/CLAUDE.md:0-0
Timestamp: 2026-03-02T12:43:43.173Z
Learning: Applies to packages/redis-worker/**/redis-worker/src/queue.ts : Job queue abstraction should be Redis-backed in src/queue.ts

Applied to files:

  • packages/redis-worker/src/mollifier/buffer.ts
📚 Learning: 2026-05-12T06:43:12.346Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3558
File: internal-packages/run-engine/src/run-queue/index.ts:420-424
Timestamp: 2026-05-12T06:43:12.346Z
Learning: In the triggerdotdev/trigger.dev codebase (`internal-packages/run-engine/src/run-queue/index.ts`), the established convention in `RunQueue` read-path methods (e.g., `lengthOfQueue`, `lengthOfQueues`, `currentConcurrencyOfQueues`) is to **fail open** on transient Redis pipeline errors: pipeline result errors (`baseErr`, `ctrErr`, etc.) are coerced to `0` rather than surfaced or re-thrown. This is intentional — the project treats Redis command errors the same as missing keys for these counter reads. Do not flag this pattern as a bug or suggest throwing/propagating these errors in future reviews.

Applied to files:

  • packages/redis-worker/src/mollifier/buffer.ts
  • apps/webapp/test/mollifierTripEvaluator.test.ts
  • packages/redis-worker/src/mollifier/buffer.test.ts
📚 Learning: 2026-03-03T13:07:33.177Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3166
File: internal-packages/run-engine/src/batch-queue/tests/index.test.ts:711-713
Timestamp: 2026-03-03T13:07:33.177Z
Learning: In `internal-packages/run-engine/src/batch-queue/tests/index.test.ts`, test assertions for rate limiter stubs can use `toBeGreaterThanOrEqual` rather than exact equality (`toBe`) because the consumer loop may call the rate limiter during empty pops in addition to actual item processing, and this over-calling is acceptable in integration tests.

Applied to files:

  • apps/webapp/test/mollifierGate.test.ts
  • apps/webapp/test/mollifierTripEvaluator.test.ts
  • packages/redis-worker/src/mollifier/buffer.test.ts
📚 Learning: 2026-05-04T19:14:58.481Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3499
File: apps/webapp/test/api-auth.e2e.test.ts:149-158
Timestamp: 2026-05-04T19:14:58.481Z
Learning: In triggerdotdev/trigger.dev (apps/webapp/test/api-auth.e2e.test.ts and apps/webapp/test/auth-api.e2e.full.test.ts), the `expect(res.status).not.toBe(401)` / `expect(res.status).not.toBe(403)` assertion shape for "auth passes" tests is intentional. The handler can return other 4xx/5xx codes due to downstream infra reasons in the test container (e.g., missing body validation, resource not found, infra errors), so pinning to a specific success code like 400 would cause false negatives. Do not flag these as needing tighter assertions.

Applied to files:

  • apps/webapp/test/mollifierGate.test.ts
📚 Learning: 2026-05-14T08:21:10.439Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3614
File: apps/webapp/app/v3/mollifier/mollifierGate.server.ts:48-52
Timestamp: 2026-05-14T08:21:10.439Z
Learning: In triggerdotdev/trigger.dev's v3 feature-flag system, `flag()` in `apps/webapp/app/v3/featureFlags.server.ts` supports per-org gating via an `overrides` argument: callers pass `Organization.featureFlags` (a JSON column on the Org row) as overrides, which take precedence over the global `featureFlag` Prisma table row. This pattern is used by the AI beta, private-connections beta, query-access, and compute beta flags, and also by the mollifier gate (`resolveOrgFlag` in `apps/webapp/app/v3/mollifier/mollifierGate.server.ts`), where `GateInputs.orgFeatureFlags` is passed as overrides and `triggerTask.server.ts` threads `environment.organization.featureFlags` into the gate call. No schema change is required for per-org gating — use this overrides mechanism rather than adding an `orgId` field to `FlagsOptions`.

Applied to files:

  • apps/webapp/test/mollifierGate.test.ts
  • apps/webapp/app/env.server.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/mollifierGate.test.ts
  • apps/webapp/test/mollifierTripEvaluator.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/mollifierGate.test.ts
  • apps/webapp/test/mollifierTripEvaluator.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/mollifierGate.test.ts
  • apps/webapp/test/mollifierTripEvaluator.test.ts
  • packages/redis-worker/src/mollifier/buffer.test.ts
📚 Learning: 2026-04-23T13:26:31.290Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3430
File: apps/webapp/app/presenters/v3/RunStreamPresenter.server.ts:0-0
Timestamp: 2026-04-23T13:26:31.290Z
Learning: In `apps/webapp/app/utils/sse.ts` (and callers such as `apps/webapp/app/presenters/v3/RunStreamPresenter.server.ts`), the string-reason type passed to `.abort()` is NOT load-bearing for heap retention in SSE streams on Node 20. Isolation testing (2000 requests × 200 KB payload) confirmed `.abort()` vs `.abort("string")` produces identical heap deltas once `AbortSignal.any` is removed. The actual root cause of the SSE memory leak was `AbortSignal.any`, which registers signals with a FinalizationRegistry that does not reliably release them on abort (see nodejs/node#54614 and `#55351`). Named sentinel constants (`ABORT_REASON_*`, exported from `sse.ts`) are a readability/style win per the CLAUDE.md named-constant guideline, not a correctness fix. Do not flag `.abort()` vs `.abort("reason string")` as a memory-retention issue in future reviews of this codebase.

Applied to files:

  • apps/webapp/app/v3/mollifier/mollifierTelemetry.server.ts
  • apps/webapp/app/v3/mollifier/mollifierGate.server.ts
📚 Learning: 2026-03-02T12:43:43.173Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: packages/redis-worker/CLAUDE.md:0-0
Timestamp: 2026-03-02T12:43:43.173Z
Learning: Applies to packages/redis-worker/**/*@(job|queue|worker|background).{ts,tsx} : Use trigger.dev/redis-worker for all new background job implementations, replacing graphile-worker and zodworker

Applied to files:

  • .changeset/mollifier-global-gate.md
📚 Learning: 2026-05-26T13:46:37.556Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3757
File: apps/webapp/app/routes/realtime.v1.runs.$runId.ts:86-86
Timestamp: 2026-05-26T13:46:37.556Z
Learning: In `triggerdotdev/trigger.dev`, the `mollifier.stale_entries` counter and `mollifier.stale_entries.current` gauge intentionally attach `envId` as a metric dimension. This is deliberate because the inline alert recommendation in the codebase prescribes `mollifier_stale_entries_current{envId=...} > 0 for 5m` as the alert expression, making `envId` a load-bearing label selector. Do not flag these metrics for high-cardinality attribute violations — the pattern is an accepted exception to the general guideline of avoiding UUID/ID metric attributes.

Applied to files:

  • .changeset/mollifier-global-gate.md
  • apps/webapp/app/env.server.ts
📚 Learning: 2026-05-15T08:05:54.659Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3625
File: apps/webapp/app/services/taskMetadataCache.server.ts:72-84
Timestamp: 2026-05-15T08:05:54.659Z
Learning: In `triggerdotdev/trigger.dev`, the `task-meta:*` Redis keyspace in `apps/webapp/app/services/taskMetadataCache.server.ts` is fully self-owned: `RedisTaskMetadataCache` is the sole writer and sole reader of this keyspace. Do not flag the `decode()` function (which casts parsed JSON to `EncodedEntry` and wraps in try/catch) for missing Zod schema validation. The existing `JSON.parse` + `try/catch` → `null` fallback is intentional; a `null` return triggers a safe PG fallback and cache back-fill. Adding Zod validation on every `HGET` was explicitly rejected as unnecessary CPU overhead on the hot path with no real safety benefit given the single-writer contract.

Applied to files:

  • .changeset/mollifier-global-gate.md
📚 Learning: 2026-06-01T11:37:12.623Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3754
File: apps/webapp/app/env.server.ts:1104-1129
Timestamp: 2026-06-01T11:37:12.623Z
Learning: In triggerdotdev/trigger.dev (apps/webapp/app/env.server.ts), new background/periodic worker feature flags should hard-default to "0" (explicitly opt-in) rather than inheriting a parent feature flag (e.g., TRIGGER_MOLLIFIER_ENABLED). Inheriting a parent flag causes the new worker to auto-start on upgrade for any deployment that already has the parent flag enabled, turning on unexpected background load without an explicit rollout step. Each new worker component should require its own explicit opt-in via its own env var (e.g., TRIGGER_MOLLIFIER_STALE_SWEEP_ENABLED defaults to "0", not to process.env.TRIGGER_MOLLIFIER_ENABLED ?? "0").

Applied to files:

  • .changeset/mollifier-global-gate.md
📚 Learning: 2026-05-22T15:14:11.190Z
Learnt from: myftija
Repo: triggerdotdev/trigger.dev PR: 3705
File: apps/webapp/server.ts:24-25
Timestamp: 2026-05-22T15:14:11.190Z
Learning: In `apps/webapp/server.ts` (triggerdotdev/trigger.dev), direct `process.env` reads are the established pattern for top-level server bootstrap constants (e.g. `ENABLE_CLUSTER`, `WEB_CONCURRENCY`, `CLUSTER_WORKERS`, `HTTP_KEEPALIVE_TIMEOUT_MS`). Do not flag these as violations of the "use env.server.ts" guideline — `server.ts` is a pre-Remix-bootstrap entry point where `env.server.ts` is not yet initialised.

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: 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-04-15T15:39:31.575Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2026-04-15T15:39:31.575Z
Learning: Applies to apps/webapp/**/*.{ts,tsx} : Access environment variables through the `env` export of `env.server.ts` instead of directly accessing `process.env`

Applied to files:

  • apps/webapp/app/env.server.ts
📚 Learning: 2026-03-02T12:42:47.652Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: apps/supervisor/CLAUDE.md:0-0
Timestamp: 2026-03-02T12:42:47.652Z
Learning: Applies to apps/supervisor/src/env.ts : Environment configuration should be defined in `src/env.ts`

Applied to files:

  • apps/webapp/app/env.server.ts
📚 Learning: 2026-04-16T14:19:16.330Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: apps/webapp/CLAUDE.md:0-0
Timestamp: 2026-04-16T14:19:16.330Z
Learning: Applies to apps/webapp/**/*.server.ts : Access environment variables via `env` export from `app/env.server.ts`. Never use `process.env` directly

Applied to files:

  • apps/webapp/app/env.server.ts
📚 Learning: 2026-04-15T15:39:31.575Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2026-04-15T15:39:31.575Z
Learning: Applies to apps/webapp/**/*.test.{ts,tsx} : Do not import `env.server.ts` directly or indirectly into test files; instead pass environment-dependent values through options/parameters to make code testable

Applied to files:

  • apps/webapp/app/env.server.ts
📚 Learning: 2026-05-19T13:33:07.551Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 3669
File: apps/supervisor/src/wideEvents/new.ts:26-31
Timestamp: 2026-05-19T13:33:07.551Z
Learning: In the triggerdotdev/trigger.dev supervisor codebase (`apps/supervisor/src/wideEvents/new.ts`), the `Env` type intentionally omits `instanceId` because `nodeId` (populated via the `TRIGGER_WORKER_INSTANCE_NAME` environment variable) already serves as the per-pod/per-instance identifier. Do not flag `instanceId` as missing from `Env` or `newState`.

Applied to files:

  • apps/webapp/app/env.server.ts
📚 Learning: 2026-03-27T18:11:57.032Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 3114
File: apps/supervisor/src/index.ts:83-98
Timestamp: 2026-03-27T18:11:57.032Z
Learning: In `apps/supervisor/src/index.ts`, `RESOURCE_MONITOR_ENABLED` (env var in `apps/supervisor/src/env.ts`) defaults to `false`. As a result, the local `ResourceMonitor`-based `maxResources`/`skipDequeue` gating in `preDequeue` is inactive in compute mode deployments. Do not flag local resource monitor usage in compute mode as a live bug; it has no practical impact unless `RESOURCE_MONITOR_ENABLED` is explicitly set to `true`.

Applied to files:

  • apps/webapp/app/env.server.ts
📚 Learning: 2026-04-16T13:45:22.317Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3368
File: apps/webapp/test/engine/taskIdentifierRegistry.test.ts:3-19
Timestamp: 2026-04-16T13:45:22.317Z
Learning: In `apps/webapp/test/engine/taskIdentifierRegistry.test.ts`, the `vi.mock` calls for `~/services/taskIdentifierCache.server` (stubbing `getTaskIdentifiersFromCache` and `populateTaskIdentifierCache`), `~/models/task.server` (stubbing `getAllTaskIdentifiers`), and `~/db.server` (stubbing `prisma` and `$replica`) are intentional. The suite uses real Postgres via testcontainers for all `TaskIdentifier` DB operations, but isolates the Redis cache layer and legacy query fallback as separate concerns not exercised in this test file. Do not flag these mocks as violations of the no-mocks policy in future reviews.

Applied to files:

  • apps/webapp/test/mollifierTripEvaluator.test.ts
  • packages/redis-worker/src/mollifier/buffer.test.ts
📚 Learning: 2026-03-02T12:43:25.254Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: internal-packages/run-engine/CLAUDE.md:0-0
Timestamp: 2026-03-02T12:43:25.254Z
Learning: Applies to internal-packages/run-engine/src/engine/tests/**/*.test.ts : Implement tests for RunEngine in `src/engine/tests/` using testcontainers for Redis and PostgreSQL containerization

Applied to files:

  • apps/webapp/test/mollifierTripEvaluator.test.ts
  • packages/redis-worker/src/mollifier/buffer.test.ts
📚 Learning: 2026-04-07T14:12:18.946Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3331
File: apps/webapp/test/engine/batchPayloads.test.ts:5-24
Timestamp: 2026-04-07T14:12:18.946Z
Learning: In `apps/webapp/test/engine/batchPayloads.test.ts`, using `vi.mock` for `~/v3/objectStore.server` (stubbing `hasObjectStoreClient` and `uploadPacketToObjectStore`), `~/env.server` (overriding offload thresholds), and `~/v3/tracer.server` (stubbing `startActiveSpan`) is intentional and acceptable. Simulating controlled transient upload failures (e.g., fail N times then succeed) to verify `p-retry` behavior cannot be reproduced with real services or testcontainers. This file is an explicit exception to the repo's general no-mocks policy.

Applied to files:

  • apps/webapp/test/mollifierTripEvaluator.test.ts
  • packages/redis-worker/src/mollifier/buffer.test.ts
📚 Learning: 2026-05-18T14:40:18.886Z
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:18.886Z
Learning: In the triggerdotdev/trigger.dev codebase, the "never mock — use testcontainers" rule applies **only** to integration tests that interact with real external services (e.g., Redis, Postgres, S2). Unit tests for in-memory logic — such as cache deduplication in `StandardRealtimeStreamsManager` — are explicitly allowed to use `vi.fn()` / stubbed `ApiClient` objects as module-boundary call counters. This pattern is established in `packages/core/src/v3/realtimeStreams/manager.test.ts`, `streams.test.ts`, `chat-server.test.ts`, and `chat.test.ts`. Do not flag `vi.fn()`-based mocks in these unit tests as policy violations.

Applied to files:

  • apps/webapp/test/mollifierTripEvaluator.test.ts
📚 Learning: 2026-05-04T19:14:44.097Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3499
File: apps/webapp/test/auth-api.e2e.full.test.ts:205-227
Timestamp: 2026-05-04T19:14:44.097Z
Learning: In triggerdotdev/trigger.dev's e2e auth test suite (`apps/webapp/test/auth-api.e2e.full.test.ts` and related `*.e2e.full.test.ts` files), loose negative assertions like `expect(res.status).not.toBe(200)` are intentional. External infrastructure (e.g. ClickHouse) is unreachable in the e2e test environment, so a 5xx from the route handler after auth passes is an expected and acceptable outcome. Tightening these to a specific set like `[401, 403, 404]` would incorrectly exclude valid 5xx results. Do not flag these as issues during review.

Applied to files:

  • apps/webapp/test/mollifierTripEvaluator.test.ts
📚 Learning: 2026-03-02T12:43:43.173Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: packages/redis-worker/CLAUDE.md:0-0
Timestamp: 2026-03-02T12:43:43.173Z
Learning: Applies to packages/redis-worker/**/redis-worker/**/*.{test,spec}.{ts,tsx} : Use testcontainers for Redis in test files for redis-worker

Applied to files:

  • apps/webapp/test/mollifierTripEvaluator.test.ts
  • packages/redis-worker/src/mollifier/buffer.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 this repo’s trigger.dev codebase, the “never mock — use testcontainers” guideline should only be applied to integration tests that talk to real external services (e.g., Redis, Postgres, S2). For unit tests that validate in-memory logic (e.g., deduplication/cache behavior in StandardRealtimeStreamsManager and similar module-boundary call counting), it is allowed to use Vitest mocks like `vi.fn()` and to stub/mock `ApiClient` objects to count calls or simulate in-process collaborators. Do not flag `vi.fn()`-based mocks as policy violations in these unit-test scenarios; reserve the rule for true external-service integration tests.

Applied to files:

  • packages/redis-worker/src/mollifier/buffer.test.ts
📚 Learning: 2026-03-06T14:44:55.489Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3173
File: packages/trigger-sdk/src/v3/chat.test.ts:103-104
Timestamp: 2026-03-06T14:44:55.489Z
Learning: In `packages/trigger-sdk/src/v3/chat.test.ts`, mocking `global.fetch` with `vi.fn()` is acceptable and intentional. `TriggerChatTransport` is a browser-facing SSE/HTTP client, and using testcontainers for these tests is not required. This file is an explicit exception to the repo's general no-mocks policy.

Applied to files:

  • packages/redis-worker/src/mollifier/buffer.test.ts
🔇 Additional comments (13)
packages/redis-worker/src/mollifier/buffer.ts (1)

613-637: LGTM!

packages/redis-worker/src/mollifier/buffer.test.ts (1)

129-297: LGTM!

.changeset/mollifier-global-gate.md (1)

1-6: LGTM!

apps/webapp/app/env.server.ts (1)

1107-1111: Validate global threshold defaults against live fleet headroom before enabling global mode.

TRIGGER_MOLLIFIER_GLOBAL_TRIP_THRESHOLD=1000 with a 200ms window implies ~5k trigger/s before tripping. Please verify this against current aggregate trigger.run rates and DB saturation thresholds so the first global rollout neither under-trips nor over-trips.

apps/webapp/test/mollifierTripEvaluator.test.ts (3)

63-90: LGTM!


92-112: LGTM!


114-149: LGTM!

apps/webapp/app/v3/mollifier/mollifierTelemetry.server.ts (1)

10-10: LGTM!

apps/webapp/app/v3/mollifier/mollifierGate.server.ts (2)

26-26: LGTM!


91-106: LGTM!

apps/webapp/app/v3/mollifier/mollifierTripEvaluator.server.ts (2)

6-10: LGTM!


29-37: LGTM!

apps/webapp/test/mollifierGate.test.ts (1)

185-228: LGTM!


Walkthrough

This pull request introduces a fleet-wide rate-limiting mode for the Mollifier trigger gate. Previously, rate limits were enforced per environment using separate Redis counters. The change adds MollifierBuffer.evaluateTripGlobal(), which uses a single shared fixed-window counter across all environments to limit aggregate trigger rate. A new environment variable TRIGGER_MOLLIFIER_GATE_MODE controls whether the gate uses per-env or global mode. Type definitions are extended to distinguish between diversion reasons ("per_env_rate" vs "global_rate"), and the gate conditionally routes to the appropriate Redis method based on the configured mode. Comprehensive integration tests verify mode isolation, window expiration, hold semantics, atomicity under concurrency, and correct reason propagation through the telemetry layer.

✨ 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 mollifier-global-gate

@d-cs d-cs self-assigned this Jun 5, 2026
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