Skip to content

Commit 44286e0

Browse files
d-csclaude
andcommitted
fix(webapp): skip request-idempotency cache for mollified triggers
`saveRequestIdempotency` caches the runId returned by the trigger service against the x-trigger-request-idempotency-key header. When the gate mollifies the trigger, that runId is a synthesised cuid with no PG row; a lost-response SDK retry would lookup the cached id in `handleRequestIdempotency`, miss in PG, fall through to a fresh trigger attempt, and — for triggers without a task-level idempotency key — produce a duplicate buffer entry. Surface the divert as a typed `isMollified` flag on `TriggerTaskServiceResult` and gate the route's `saveRequestIdempotency` call on it. Retries during the buffer window are now accepted as fresh triggers; task-level idempotency keys still dedupe via the buffer's SETNX in `findBufferedRunWithIdempotency`. The behaviour is bounded by the drainer's eventual materialisation — once the PG row lands, normal request-idempotency from that point forward works as usual. Regression covered by two assertions in the existing mollifier tests: the mollify path returns `isMollified: true`, and the pass-through path leaves the flag falsy. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent f70fd90 commit 44286e0

4 files changed

Lines changed: 47 additions & 2 deletions

File tree

apps/webapp/app/routes/api.v1.tasks.$taskId.trigger.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,20 @@ const { action, loader } = createActionApiRoute(
134134
return json({ error: "Task not found" }, { status: 404 });
135135
}
136136

137-
await saveRequestIdempotency(requestIdempotencyKey, "trigger", result.run.id);
137+
// Skip request-idempotency caching when the gate diverted to the
138+
// mollifier buffer. `result.run.id` is a synthesised cuid with no
139+
// corresponding PG row, so a lost-response SDK retry that reaches
140+
// `handleRequestIdempotency` would lookup that id, miss in PG, and
141+
// fall through to a fresh trigger — producing a duplicate buffer
142+
// entry for triggers without a task-level idempotency key (the
143+
// task-level path still dedupes via the buffer's SETNX in
144+
// `findBufferedRunWithIdempotency`). Accepting the retry-as-fresh-
145+
// trigger semantics here is bounded by the drainer's eventual
146+
// materialisation: once the run lands in PG, normal request-
147+
// idempotency from that point forward works as usual.
148+
if (!result.isMollified) {
149+
await saveRequestIdempotency(requestIdempotencyKey, "trigger", result.run.id);
150+
}
138151

139152
const $responseHeaders = await responseHeaders(result.run, authentication);
140153

apps/webapp/app/runEngine/services/triggerTask.server.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -514,7 +514,13 @@ export class RunEngineTriggerTaskService {
514514
// TaskRun; the route handler only reads
515515
// `result.run.friendlyId`. traceRun flushes the PARTIAL
516516
// run-span event to ClickHouse on callback return.
517-
return result as unknown as TriggerTaskServiceResult;
517+
// `isMollified` flags the route to skip the request-
518+
// idempotency cache write — see the field's contract on
519+
// `TriggerTaskServiceResult`.
520+
return {
521+
...(result as unknown as TriggerTaskServiceResult),
522+
isMollified: true,
523+
};
518524
}
519525
if (!mollifierBuffer) {
520526
logger.warn(

apps/webapp/app/v3/services/triggerTask.server.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,14 @@ export class OutOfEntitlementError extends Error {
4646
export type TriggerTaskServiceResult = {
4747
run: TaskRun;
4848
isCached: boolean;
49+
// True when the mollifier gate diverted the trigger to the Redis
50+
// buffer and `run` is a synthesised record (no PG row exists yet).
51+
// The trigger route reads this to skip `saveRequestIdempotency` —
52+
// caching the synth runId would mean a lost-response SDK retry hits
53+
// a PG-miss in `handleRequestIdempotency` and falls through to a
54+
// fresh trigger, producing a duplicate buffer entry for trigger
55+
// calls that don't carry a task-level idempotency key.
56+
isMollified?: boolean;
4957
};
5058

5159
export const MAX_ATTEMPTS = 2;

apps/webapp/test/engine/triggerTask.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1385,6 +1385,18 @@ describe("RunEngineTriggerTaskService", () => {
13851385
expect(synthetic.notice.message).toBeTypeOf("string");
13861386
expect(synthetic.notice.docs).toBeTypeOf("string");
13871387

1388+
// The mollify branch must flag `isMollified: true` on the result so
1389+
// the trigger route can skip `saveRequestIdempotency`. Caching the
1390+
// synthetic runId in the request-idempotency table would mean a
1391+
// lost-response SDK retry (same `x-trigger-request-idempotency-key`
1392+
// header) hits a PG miss in `handleRequestIdempotency` and falls
1393+
// through to a fresh trigger — producing a duplicate buffer entry
1394+
// for trigger calls without a task-level idempotency key. The
1395+
// bounded behaviour (accept retry-as-fresh-trigger during the
1396+
// buffer window) is the deliberate choice; a stale-cache lookup
1397+
// returning null is not.
1398+
expect(result?.isMollified).toBe(true);
1399+
13881400
// buffer.accept ran — Redis has the canonical engine.trigger snapshot
13891401
// under the synthesised friendlyId. The drainer will read this and
13901402
// replay it through engine.trigger to materialise the run.
@@ -1490,6 +1502,12 @@ describe("RunEngineTriggerTaskService", () => {
14901502
// getMollifierBuffer must not be called either — the call site short-circuits
14911503
// before touching the singleton when the gate says pass_through.
14921504
expect(getBufferSpy).not.toHaveBeenCalled();
1505+
// Pass-through must NOT set `isMollified` — `result.run` is a real
1506+
// PG row, and the trigger route's `saveRequestIdempotency` is
1507+
// safe to call. Setting the flag here would silently skip the
1508+
// request-idempotency cache for every non-mollified trigger on a
1509+
// mollifier-enabled org, breaking lost-response retry dedup.
1510+
expect(result?.isMollified).toBeFalsy();
14931511

14941512
await engine.quit();
14951513
},

0 commit comments

Comments
 (0)