Skip to content

Commit d1f1a85

Browse files
committed
test(webapp): guards for the split-idempotency + realtime-batch review fixes
Caller-driven / unit guards for the production hardening in the base PR: claim-TTL floor, publishClaim CAS result, reacquire fail-closed 503, expired/failed global-scope recreate re-serialisation, and the realtime batch primary re-read. Split out of the base PR so the production change stays reviewable on its own.
1 parent 82b42af commit d1f1a85

6 files changed

Lines changed: 298 additions & 0 deletions

apps/webapp/test/claimTtl.test.ts

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import { describe, expect, it } from "vitest";
2+
import { computeClaimTtlSeconds } from "~/v3/mollifier/claimTtl";
3+
4+
// The mollifier idempotency claim is a serialization lock that must outlive the winner's
5+
// create-and-publish pipeline. If its TTL is derived from the customer's key TTL alone, a short key
6+
// TTL expires the claim mid-pipeline and a polling loser re-claims → a second creator → cross-DB dup.
7+
describe("computeClaimTtlSeconds", () => {
8+
const now = Date.parse("2026-01-01T00:00:00.000Z");
9+
const minTtlSeconds = 5;
10+
const maxTtlSeconds = 30;
11+
12+
it("floors a short key TTL at the pipeline minimum (claim can't expire mid-pipeline)", () => {
13+
expect(
14+
computeClaimTtlSeconds({
15+
keyExpiresAt: new Date(now + 2_000),
16+
now,
17+
minTtlSeconds,
18+
maxTtlSeconds,
19+
})
20+
).toBe(5);
21+
});
22+
23+
it("caps a long key TTL at the maximum", () => {
24+
expect(
25+
computeClaimTtlSeconds({
26+
keyExpiresAt: new Date(now + 3_600_000),
27+
now,
28+
minTtlSeconds,
29+
maxTtlSeconds,
30+
})
31+
).toBe(30);
32+
});
33+
34+
it("uses the key TTL when it sits between the floor and the cap", () => {
35+
expect(
36+
computeClaimTtlSeconds({
37+
keyExpiresAt: new Date(now + 12_000),
38+
now,
39+
minTtlSeconds,
40+
maxTtlSeconds,
41+
})
42+
).toBe(12);
43+
});
44+
45+
it("floors an already-expired key at the minimum", () => {
46+
expect(
47+
computeClaimTtlSeconds({
48+
keyExpiresAt: new Date(now - 1_000),
49+
now,
50+
minTtlSeconds,
51+
maxTtlSeconds,
52+
})
53+
).toBe(5);
54+
});
55+
});
Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
import { describe, expect, it, vi } from "vitest";
2+
3+
// Devin #2: a global-scope (or scope-absent) key whose existing run is EXPIRED/FAILED gets its key
4+
// cleared and recreated. Under the run-ops split that recreate must serialise through the claim (same
5+
// cross-DB dup risk as the claim-loser cleared path) rather than fall through to an unserialised create.
6+
vi.mock("~/db.server", () => ({
7+
prisma: {},
8+
$replica: {},
9+
runOpsNewPrisma: {},
10+
runOpsLegacyPrisma: {},
11+
runOpsNewReplica: {},
12+
runOpsLegacyReplica: {},
13+
}));
14+
15+
const h = vi.hoisted(() => ({ existingRun: null as unknown, splitEnabled: true }));
16+
17+
vi.mock("~/v3/runStore.server", () => ({
18+
runStore: { findRun: vi.fn(async () => h.existingRun) },
19+
}));
20+
vi.mock("~/v3/mollifier/mollifierBuffer.server", () => ({ getMollifierBuffer: () => null }));
21+
vi.mock("~/v3/mollifier/mollifierGate.server", () => ({
22+
makeResolveMollifierFlag: () => async () => false,
23+
}));
24+
vi.mock("~/v3/runOpsMigration/splitMode.server", () => ({
25+
isSplitEnabled: async () => h.splitEnabled,
26+
}));
27+
vi.mock("~/runEngine/concerns/idempotencyResidency.server", () => ({
28+
resolveIdempotencyDedupClient: async () => ({}),
29+
}));
30+
31+
import { IdempotencyKeyConcern } from "~/runEngine/concerns/idempotencyKeys.server";
32+
import type { TriggerTaskRequest } from "~/runEngine/types";
33+
34+
function makeRequest(): TriggerTaskRequest {
35+
return {
36+
taskId: "my-task",
37+
environment: { id: "env_a", organizationId: "org_1", organization: { featureFlags: {} } },
38+
options: {},
39+
body: { options: { idempotencyKey: "k-1" } }, // scope absent → treated as global
40+
} as unknown as TriggerTaskRequest;
41+
}
42+
43+
// handleExistingRun's documented contract for an expired/failed run: clear the key, return isCached:false.
44+
const CLEARED = {
45+
isCached: false as const,
46+
idempotencyKey: "k-1",
47+
idempotencyKeyExpiresAt: new Date(Date.now() + 60_000),
48+
};
49+
50+
describe("IdempotencyKeyConcern · expired/failed recreate re-serialisation", () => {
51+
it("routes the cleared recreate through the claim under global-scope split (no unserialised create)", async () => {
52+
h.existingRun = { id: "run_internal", friendlyId: "run_friendly" };
53+
h.splitEnabled = true;
54+
const concern = new IdempotencyKeyConcern({} as never, {} as never, {} as never);
55+
vi.spyOn(
56+
concern as never as { handleExistingRun: () => unknown },
57+
"handleExistingRun"
58+
).mockResolvedValue(CLEARED);
59+
const SENTINEL = { ...CLEARED, claim: { token: "t" } };
60+
const reacquire = vi
61+
.spyOn(
62+
concern as never as { reacquireClearedGlobalWinner: () => unknown },
63+
"reacquireClearedGlobalWinner"
64+
)
65+
.mockResolvedValue(SENTINEL);
66+
67+
const result = await concern.handleTriggerRequest(makeRequest(), undefined);
68+
69+
expect(reacquire).toHaveBeenCalledOnce();
70+
expect(result).toBe(SENTINEL);
71+
});
72+
73+
it("does NOT re-serialise when the split is off — plain recreate", async () => {
74+
h.existingRun = { id: "run_internal", friendlyId: "run_friendly" };
75+
h.splitEnabled = false;
76+
const concern = new IdempotencyKeyConcern({} as never, {} as never, {} as never);
77+
vi.spyOn(
78+
concern as never as { handleExistingRun: () => unknown },
79+
"handleExistingRun"
80+
).mockResolvedValue(CLEARED);
81+
const reacquire = vi
82+
.spyOn(
83+
concern as never as { reacquireClearedGlobalWinner: () => unknown },
84+
"reacquireClearedGlobalWinner"
85+
)
86+
.mockResolvedValue({} as never);
87+
88+
const result = await concern.handleTriggerRequest(makeRequest(), undefined);
89+
90+
expect(reacquire).not.toHaveBeenCalled();
91+
expect(result).toBe(CLEARED);
92+
});
93+
});

apps/webapp/test/mollifierClaimResolution.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,4 +156,30 @@ describe("IdempotencyKeyConcern · claim resolution", () => {
156156
h.orgFlag = true; // restore for any later tests in this file
157157
}
158158
});
159+
160+
it("floors the claim TTL for a short idempotency-key TTL so the claim can't expire mid-pipeline", async () => {
161+
// A 2s customer key TTL must NOT shrink the claim below the pipeline floor — else the claim expires
162+
// while the winner is still creating and a polling loser re-claims (cross-DB dup under the split).
163+
// Capture the ttlSeconds the concern hands the buffer's claim.
164+
let capturedTtl: number | undefined;
165+
h.buffer = {
166+
claimIdempotency: vi.fn(async (input: { ttlSeconds: number }) => {
167+
capturedTtl = input.ttlSeconds;
168+
return { kind: "claimed" as const };
169+
}),
170+
lookupIdempotency: vi.fn(async () => null),
171+
} as unknown as MollifierBuffer;
172+
173+
const findFirst = vi.fn(async () => null);
174+
const concern = makeConcern({ findFirst });
175+
176+
const request = makeRequest();
177+
(request.body.options as { idempotencyKeyTTL?: string }).idempotencyKeyTTL = "2s";
178+
179+
const result = await concern.handleTriggerRequest(request, undefined);
180+
181+
expect(result.isCached).toBe(false);
182+
// Floored at TRIGGER_MOLLIFIER_CLAIM_MIN_TTL_SECONDS (default 5), NOT the ~2s key TTL.
183+
expect(capturedTtl).toBeGreaterThanOrEqual(5);
184+
});
159185
});
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
import { describe, expect, it, vi } from "vitest";
2+
import { publishClaim } from "~/v3/mollifier/idempotencyClaim.server";
3+
import type { MollifierBuffer } from "@trigger.dev/redis-worker";
4+
5+
// publishClaim compare-and-sets on the caller's token; a `false` result means a stale claimant already
6+
// moved in (the winner's claim expired mid-pipeline), so the publish no-op'd. The caller must be able to
7+
// SEE that rather than silently assuming its run is canonical.
8+
function fakeBuffer(publishResult: boolean): MollifierBuffer {
9+
return { publishClaim: vi.fn(async () => publishResult) } as unknown as MollifierBuffer;
10+
}
11+
12+
const base = {
13+
envId: "env_1",
14+
taskIdentifier: "task",
15+
idempotencyKey: "k",
16+
token: "tok",
17+
runId: "run_1",
18+
ttlSeconds: 30,
19+
};
20+
21+
describe("publishClaim result", () => {
22+
it("returns false when the buffer compare-and-set no-ops (a stale claimant already moved in)", async () => {
23+
expect(await publishClaim({ ...base, buffer: fakeBuffer(false) })).toBe(false);
24+
});
25+
26+
it("returns true when the publish sets the winner", async () => {
27+
expect(await publishClaim({ ...base, buffer: fakeBuffer(true) })).toBe(true);
28+
});
29+
30+
it("returns true when the mollifier buffer is unavailable (nothing to converge)", async () => {
31+
expect(await publishClaim({ ...base, buffer: null })).toBe(true);
32+
});
33+
});
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
import { describe, expect, it, vi } from "vitest";
2+
3+
// Force every reacquire attempt to RESOLVE to a winner (never `claimed`/`timed_out`); the spied
4+
// handleExistingRun keeps reporting that winner as cleared, so the bounded loop advances to exhaustion.
5+
vi.mock("~/v3/mollifier/idempotencyClaim.server", () => ({
6+
resetResolvedClaim: vi.fn(async () => {}),
7+
claimOrAwait: vi.fn(async () => ({ kind: "resolved", runId: "run_cleared" })),
8+
}));
9+
10+
import { IdempotencyKeyConcern } from "~/runEngine/concerns/idempotencyKeys.server";
11+
12+
describe("reacquireClearedGlobalWinner — fail closed on exhaustion", () => {
13+
it("throws a retryable 503 after exhausting reacquires (never falls through to an unserialised create)", async () => {
14+
const concern = new IdempotencyKeyConcern({} as never, {} as never, {} as never);
15+
// Winner is findable but perpetually cleared → each pass advances staleRunId, exhausting the bound.
16+
vi.spyOn(
17+
concern as unknown as { resolveWinnerAcrossDbs: () => unknown },
18+
"resolveWinnerAcrossDbs"
19+
).mockResolvedValue({ friendlyId: "run_cleared" });
20+
vi.spyOn(
21+
concern as unknown as { handleExistingRun: () => unknown },
22+
"handleExistingRun"
23+
).mockResolvedValue({ isCached: false });
24+
25+
const request = { environment: { id: "env_1" }, taskId: "task" } as never;
26+
const ctx = {
27+
idempotencyKey: "k",
28+
idempotencyKeyExpiresAt: new Date(Date.now() + 60_000),
29+
dedupClient: {} as never,
30+
ttlSeconds: 30,
31+
clearedRunId: "run_cleared",
32+
safetyNetMs: 5_000,
33+
pollStepMs: 25,
34+
};
35+
36+
await expect(
37+
(
38+
concern as unknown as {
39+
reacquireClearedGlobalWinner: (...a: unknown[]) => Promise<unknown>;
40+
}
41+
).reacquireClearedGlobalWinner(request, undefined, ctx)
42+
).rejects.toMatchObject({ name: "ServiceValidationError", status: 503 });
43+
});
44+
});
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
import { describe, expect, it, vi } from "vitest";
2+
import { resolveBatchTaskRunForRealtime } from "~/v3/realtime/resolveBatchForRealtime.server";
3+
4+
// The realtime batch route reads the batch client-less (replica). Under replica lag a just-created batch
5+
// misses; `shouldRetryNotFound` covers the zodfetch GET, but the Electric ShapeStream consumer
6+
// (self-hosters) ignores `x-should-retry`, so the route must re-read the owning PRIMARY on a miss.
7+
// Passing a (non-replica) writer client flips each store leg to its own primary.
8+
function laggingStore(batch: { id: string; friendlyId: string }) {
9+
return {
10+
findBatchTaskRunByFriendlyId: vi.fn(
11+
async (_friendlyId: string, _envId: string, _args: unknown, client?: unknown) =>
12+
client ? batch : null
13+
),
14+
};
15+
}
16+
17+
describe("resolveBatchTaskRunForRealtime", () => {
18+
it("re-reads the primary when the replica misses a fresh batch", async () => {
19+
const store = laggingStore({ id: "b_1", friendlyId: "batch_1" });
20+
const found = await resolveBatchTaskRunForRealtime("batch_1", "env_1", {
21+
store: store as never,
22+
writer: {} as never,
23+
});
24+
expect(found).toEqual({ id: "b_1", friendlyId: "batch_1" });
25+
expect(store.findBatchTaskRunByFriendlyId).toHaveBeenCalledTimes(2);
26+
});
27+
28+
it("returns null when the batch is genuinely absent on both replica and primary", async () => {
29+
const store = { findBatchTaskRunByFriendlyId: vi.fn(async () => null) };
30+
const found = await resolveBatchTaskRunForRealtime("nope", "env_1", {
31+
store: store as never,
32+
writer: {} as never,
33+
});
34+
expect(found).toBeNull();
35+
});
36+
37+
it("does not re-read the primary when the replica already has the batch", async () => {
38+
const store = {
39+
findBatchTaskRunByFriendlyId: vi.fn(async () => ({ id: "b_2", friendlyId: "batch_2" })),
40+
};
41+
await resolveBatchTaskRunForRealtime("batch_2", "env_1", {
42+
store: store as never,
43+
writer: {} as never,
44+
});
45+
expect(store.findBatchTaskRunByFriendlyId).toHaveBeenCalledTimes(1);
46+
});
47+
});

0 commit comments

Comments
 (0)