Skip to content

Commit 927c016

Browse files
d-csclaude
andcommitted
perf(webapp): gate pre-gate idempotency claim by per-org mollifier flag
Addresses Devin's review on PR #3753 (idempotencyKeys.server.ts comment 2026-05-28): when TRIGGER_MOLLIFIER_ENABLED=1 globally during staged rollout, claimOrAwait was issuing a Redis SETNX for every idempotency-keyed trigger — including orgs that had not opted in. Those orgs gain nothing from the claim (the gate always returns pass_through for them, so the buffer is never written to) and PG unique constraint already deduplicates same-key races on the pass-through path. Wrap the claim block in an additional per-org check using the same in-memory Organization.featureFlags predicate that evaluateGate uses (makeResolveMollifierFlag) — no DB query. Non-opted-in orgs now skip claimOrAwait entirely; opted-in orgs still get the cross-store race coordination. Regression test added in mollifierClaimResolution.test.ts: with orgFlag=false the concern returns isCached:false with no claim, without calling claimIdempotency at all. The two prior tests now opt the fake org in (organization.featureFlags.mollifierEnabled=true) so they still exercise the resolution branches. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent a8bed79 commit 927c016

2 files changed

Lines changed: 86 additions & 3 deletions

File tree

apps/webapp/app/runEngine/concerns/idempotencyKeys.server.ts

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,16 @@ import { shouldIdempotencyKeyBeCleared } from "~/v3/taskStatus";
88
import { getMollifierBuffer } from "~/v3/mollifier/mollifierBuffer.server";
99
import { findRunByIdWithMollifierFallback } from "~/v3/mollifier/readFallback.server";
1010
import { claimOrAwait } from "~/v3/mollifier/idempotencyClaim.server";
11+
import { makeResolveMollifierFlag } from "~/v3/mollifier/mollifierGate.server";
1112
import type { TraceEventConcern, TriggerTaskRequest } from "../types";
1213

14+
// In-memory per-org mollifier-enabled check, shared with `evaluateGate`
15+
// (same `Organization.featureFlags` JSON, no DB read). Used to gate the
16+
// pre-gate claim's Redis round-trip so non-mollifier orgs don't pay it
17+
// during staged rollout — see the comment above the claim block in
18+
// handleTriggerRequest.
19+
const resolveOrgMollifierFlag = makeResolveMollifierFlag();
20+
1321
// Claim ownership context returned to the caller when the
1422
// IdempotencyKeyConcern won a pre-gate claim. Caller MUST publish the
1523
// winning runId on pipeline success (`publishClaim`) or release the
@@ -227,7 +235,32 @@ export class IdempotencyKeyConcern {
227235
// PG-pass-through vs mollify. Skipped for triggerAndWait
228236
// (resumeParentOnCompletion) — that path bypasses the gate entirely
229237
// and its existing PG-side dedup is sufficient.
230-
if (!request.body.options?.resumeParentOnCompletion) {
238+
//
239+
// Also gated on the same per-org mollifier flag the gate uses: when
240+
// `TRIGGER_MOLLIFIER_ENABLED=1` globally for staged rollout, the buffer
241+
// singleton is constructed and `claimOrAwait` would otherwise issue a
242+
// Redis SETNX for EVERY idempotency-keyed trigger — including orgs
243+
// that haven't opted in. Those orgs never enter the mollify branch
244+
// (the gate always returns pass_through for them), so there's no
245+
// buffer activity to serialise against; PG's unique constraint
246+
// already deduplicates concurrent same-key races. Resolving the org
247+
// flag is a pure in-memory read of `Organization.featureFlags` — no
248+
// DB query, same predicate the gate uses — keeping the claim's Redis
249+
// RTT off the hot path for non-opted-in orgs during incremental
250+
// rollout.
251+
const claimEligible =
252+
!request.body.options?.resumeParentOnCompletion &&
253+
(await resolveOrgMollifierFlag({
254+
envId: request.environment.id,
255+
orgId: request.environment.organizationId,
256+
taskId: request.taskId,
257+
orgFeatureFlags:
258+
((request.environment.organization?.featureFlags as
259+
| Record<string, unknown>
260+
| null
261+
| undefined) ?? null),
262+
}));
263+
if (claimEligible) {
231264
const ttlSeconds = Math.max(
232265
1,
233266
Math.min(

apps/webapp/test/mollifierClaimResolution.test.ts

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,18 @@ vi.mock("~/db.server", () => ({ prisma: {}, $replica: {} }));
1010
// global mollifier buffer (`getMollifierBuffer`), shared by both
1111
// `claimOrAwait` and `findBufferedRunWithIdempotency`. Control it via a
1212
// hoisted handle so each test can script the claim/lookup responses.
13-
const h = vi.hoisted(() => ({ buffer: null as unknown }));
13+
const h = vi.hoisted(() => ({ buffer: null as unknown, orgFlag: true }));
1414
vi.mock("~/v3/mollifier/mollifierBuffer.server", () => ({
1515
getMollifierBuffer: () => h.buffer,
1616
}));
17+
// Stub `mollifierGate.server` so loading the concern doesn't drag in
18+
// `env.server` (which fails to parse without a populated environment in
19+
// CI). The concern only uses `makeResolveMollifierFlag` to gate the
20+
// claim; tests flip `h.orgFlag` to cover both opted-in and opted-out
21+
// orgs without touching real env or feature-flag wiring.
22+
vi.mock("~/v3/mollifier/mollifierGate.server", () => ({
23+
makeResolveMollifierFlag: () => async () => h.orgFlag,
24+
}));
1725

1826
import type { MollifierBuffer } from "@trigger.dev/redis-worker";
1927
import { IdempotencyKeyConcern } from "~/runEngine/concerns/idempotencyKeys.server";
@@ -30,7 +38,16 @@ function makeConcern(prisma: { findFirst: () => Promise<unknown> }) {
3038
function makeRequest(): TriggerTaskRequest {
3139
return {
3240
taskId: "my-task",
33-
environment: { id: "env_a", organizationId: "org_1" },
41+
environment: {
42+
id: "env_a",
43+
organizationId: "org_1",
44+
// The pre-gate claim is gated by the per-org mollifier flag
45+
// (mirroring evaluateGate's gating) so non-opted-in orgs don't pay
46+
// the Redis SETNX. Tests covering the claim path must opt this
47+
// fake org in, otherwise the concern skips claimOrAwait entirely
48+
// and the resolution branches under test never run.
49+
organization: { featureFlags: { mollifierEnabled: true } },
50+
},
3451
options: {},
3552
body: { options: { idempotencyKey: "k-1" } },
3653
} as unknown as TriggerTaskRequest;
@@ -90,4 +107,37 @@ describe("IdempotencyKeyConcern · claim resolution", () => {
90107
expect(result.run).toBe(winner);
91108
}
92109
});
110+
111+
it("non-opted-in org skips claimOrAwait entirely (no buffer round-trip, no claim held)", async () => {
112+
// Regression guard for the per-org gating that keeps the claim's
113+
// Redis SETNX off the hot path for orgs that haven't opted into the
114+
// mollifier — even when `TRIGGER_MOLLIFIER_ENABLED=1` globally and
115+
// the buffer singleton exists. The concern should NOT touch
116+
// `claimIdempotency` for these orgs; PG's unique constraint already
117+
// deduplicates same-key races on the pass-through path.
118+
h.orgFlag = false;
119+
const claimIdempotency = vi.fn(async () => ({ kind: "claimed" as const }));
120+
const lookupIdempotency = vi.fn(async () => null);
121+
h.buffer = {
122+
claimIdempotency,
123+
lookupIdempotency,
124+
} as unknown as MollifierBuffer;
125+
126+
const findFirst = vi.fn(async () => null);
127+
const concern = makeConcern({ findFirst });
128+
129+
try {
130+
const result = await concern.handleTriggerRequest(makeRequest(), undefined);
131+
expect(result.isCached).toBe(false);
132+
if (result.isCached === false) {
133+
// No claim returned — the caller must NOT publish/release.
134+
expect(result.claim).toBeUndefined();
135+
expect(result.idempotencyKey).toBe("k-1");
136+
}
137+
// The headline guarantee: zero Redis claim activity for this org.
138+
expect(claimIdempotency).not.toHaveBeenCalled();
139+
} finally {
140+
h.orgFlag = true; // restore for any later tests in this file
141+
}
142+
});
93143
});

0 commit comments

Comments
 (0)