|
| 1 | +# Mollifier idempotency — treat Redis as a second store for keys |
| 2 | + |
| 3 | +**Branch:** `mollifier-phase-3` |
| 4 | +**Date:** 2026-05-19 |
| 5 | +**Status:** Locked. (Q5 in the api-parity plan series.) |
| 6 | +**Companion docs:** Q1 listing, Q2 replay, Q3 mutation race, Q4 cancel. |
| 7 | + |
| 8 | +## The question |
| 9 | + |
| 10 | +`POST /api/v1/idempotencyKeys/{key}/reset` (SDK route) and `POST /resources/.../runs/{runParam}/idempotencyKey/reset` (dashboard route) both clear an idempotency key from matching TaskRun rows. Two adjacent concerns: |
| 11 | + |
| 12 | +1. **Reset itself.** The current `ResetIdempotencyKeyService` does `prisma.taskRun.updateMany` against PG. Buffered runs are invisible to it — a customer who resets a key during the buffered window sees the buffered run materialise *with the key still set*, defeating the reset. |
| 13 | +2. **Trigger-time dedup.** The existing `IdempotencyKeyConcern.handleTriggerRequest` does `prisma.taskRun.findFirst` against PG only. Two triggers with the same key during the buffered window both pass the check (PG has neither yet) and create duplicate runs. |
| 14 | + |
| 15 | +Both are surfaced by the same root cause: **idempotency keys live in PG today, and the buffer is invisible to the key-aware code paths.** |
| 16 | + |
| 17 | +## The principle |
| 18 | + |
| 19 | +The buffer is just another store. Keys live where the run lives. Every place the existing code consults PG for keys, also consult the buffer. Every place the existing code mutates PG keys, also mutate buffer keys. |
| 20 | + |
| 21 | +No "secondary index" component, no new helper service. Just an additional Redis lookup that lives next to the entry hash and is maintained by the same Lua scripts that manage entries. |
| 22 | + |
| 23 | +## Design |
| 24 | + |
| 25 | +### The Redis lookup |
| 26 | + |
| 27 | +``` |
| 28 | +key: mollifier:idempotency:{envId}:{taskIdentifier}:{idempotencyKey} |
| 29 | +value: runId |
| 30 | +ttl: matches the entry hash TTL |
| 31 | +``` |
| 32 | + |
| 33 | +One key per `(env, task, idempotencyKey)` combination. Resolves the same composite uniqueness PG enforces via the `findFirst` query. |
| 34 | + |
| 35 | +### `accept` — atomic with entry creation |
| 36 | + |
| 37 | +The existing `acceptMollifierEntry` Lua already serialises with the entry's lifecycle. Extend it to also write the idempotency lookup: |
| 38 | + |
| 39 | +```lua |
| 40 | +-- acceptMollifierEntry (revised) |
| 41 | +local entryKey = KEYS[1] |
| 42 | +local queueKey = KEYS[2] |
| 43 | +local orgsKey = KEYS[3] |
| 44 | +local idempotencyKey = ARGV[?] -- optional |
| 45 | +local idempotencyLookupKey = ARGV[?] -- optional, derived from envId+taskId+idempotencyKey |
| 46 | + |
| 47 | +if redis.call('EXISTS', entryKey) == 1 then |
| 48 | + return 'duplicate_run_id' |
| 49 | +end |
| 50 | + |
| 51 | +if idempotencyLookupKey then |
| 52 | + -- SETNX: refuse if the key is already taken by a buffered run. |
| 53 | + -- Returns the existing runId for the caller to use as the cached response. |
| 54 | + local existingRunId = redis.call('GET', idempotencyLookupKey) |
| 55 | + if existingRunId then |
| 56 | + return { 'duplicate_idempotency', existingRunId } |
| 57 | + end |
| 58 | + redis.call('SET', idempotencyLookupKey, runId, 'EX', ttlSeconds) |
| 59 | +end |
| 60 | + |
| 61 | +-- ... existing accept logic (HSET entry, ZADD queue, SADD orgs/orgEnvs) |
| 62 | +return 'accepted' |
| 63 | +``` |
| 64 | + |
| 65 | +The SETNX gives us **trigger-time dedup during the buffered window for free**. Two simultaneous accepts with the same key — the second's Lua sees the lookup already set, returns the existing runId. Same behaviour as PG's unique constraint, but synchronous and pre-PG-insert. |
| 66 | + |
| 67 | +### Drainer ack — atomic with materialisation |
| 68 | + |
| 69 | +The drainer's ack Lua (per Q1: `HSET materialised=true; EXPIRE +30s`) extends to clear the idempotency lookup. PG is canonical for the key after materialisation: |
| 70 | + |
| 71 | +```lua |
| 72 | +-- drainer ack (revised) |
| 73 | +HSET entryKey materialised=true |
| 74 | +EXPIRE entryKey +30s |
| 75 | +if entry.idempotencyKey then |
| 76 | + DEL idempotencyLookupKey |
| 77 | +end |
| 78 | +``` |
| 79 | + |
| 80 | +The lookup's TTL is the safety net if this DEL is missed for any reason — it'll TTL out within the same window as the entry hash itself. |
| 81 | + |
| 82 | +### Trigger-time dedup — check both stores |
| 83 | + |
| 84 | +Modify `IdempotencyKeyConcern.handleTriggerRequest`: |
| 85 | + |
| 86 | +```ts |
| 87 | +const existingRun = idempotencyKey |
| 88 | + ? await this.findExistingIdempotentRun({ |
| 89 | + runtimeEnvironmentId: request.environment.id, |
| 90 | + idempotencyKey, |
| 91 | + taskIdentifier: request.taskId, |
| 92 | + }) |
| 93 | + : undefined; |
| 94 | +// ... rest unchanged |
| 95 | +``` |
| 96 | + |
| 97 | +Where: |
| 98 | + |
| 99 | +```ts |
| 100 | +async findExistingIdempotentRun({ runtimeEnvironmentId, idempotencyKey, taskIdentifier }) { |
| 101 | + // 1. PG canonical check (existing behaviour). |
| 102 | + const pgRun = await this.prisma.taskRun.findFirst({ |
| 103 | + where: { runtimeEnvironmentId, idempotencyKey, taskIdentifier }, |
| 104 | + include: { associatedWaitpoint: true }, |
| 105 | + }); |
| 106 | + if (pgRun) return pgRun; |
| 107 | + |
| 108 | + // 2. Buffer check — the same key may belong to a buffered run. |
| 109 | + const bufferedRunId = await this.mollifierBuffer?.lookupIdempotency({ |
| 110 | + envId: runtimeEnvironmentId, |
| 111 | + taskIdentifier, |
| 112 | + idempotencyKey, |
| 113 | + }); |
| 114 | + if (!bufferedRunId) return undefined; |
| 115 | + |
| 116 | + // 3. Synthesise the TaskRun shape from the buffered snapshot using the |
| 117 | + // existing readFallback machinery. Returned shape includes all the |
| 118 | + // fields the dedup logic reads (status, idempotencyKeyExpiresAt, |
| 119 | + // associatedWaitpoint, etc.). |
| 120 | + return await synthesiseFromBuffer(bufferedRunId); |
| 121 | +} |
| 122 | +``` |
| 123 | + |
| 124 | +The synthesis path is the same one Q1 uses for listing and Q2 uses for replay. No new fallback logic — just one more caller of the existing helper. |
| 125 | + |
| 126 | +The dedup logic that follows (key expired? status indicates clear? return cached? trigger new?) runs unchanged against either source. |
| 127 | + |
| 128 | +### Reset — operate on both stores |
| 129 | + |
| 130 | +`ResetIdempotencyKeyService.call`: |
| 131 | + |
| 132 | +```ts |
| 133 | +async call(idempotencyKey, taskIdentifier, env) { |
| 134 | + // 1. PG-side (existing behaviour). |
| 135 | + const { count: pgCount } = await this.prisma.taskRun.updateMany({ |
| 136 | + where: { idempotencyKey, taskIdentifier, runtimeEnvironmentId: env.id }, |
| 137 | + data: { idempotencyKey: null, idempotencyKeyExpiresAt: null }, |
| 138 | + }); |
| 139 | + |
| 140 | + // 2. Buffer-side via a single Lua call. |
| 141 | + const { runId: clearedBufferedRunId } = await mollifierBuffer.resetIdempotency({ |
| 142 | + envId: env.id, |
| 143 | + taskIdentifier, |
| 144 | + idempotencyKey, |
| 145 | + }); |
| 146 | + |
| 147 | + const totalCount = pgCount + (clearedBufferedRunId ? 1 : 0); |
| 148 | + if (totalCount === 0) { |
| 149 | + throw new ServiceValidationError( |
| 150 | + `No runs found with idempotency key: ${idempotencyKey} and task: ${taskIdentifier}`, |
| 151 | + 404, |
| 152 | + ); |
| 153 | + } |
| 154 | + |
| 155 | + return { id: idempotencyKey }; |
| 156 | +} |
| 157 | +``` |
| 158 | + |
| 159 | +The buffer-side reset is one Lua script: |
| 160 | + |
| 161 | +```lua |
| 162 | +-- resetIdempotencyKey Lua |
| 163 | +local idempotencyLookupKey = KEYS[1] |
| 164 | +local entryPrefix = ARGV[1] |
| 165 | + |
| 166 | +local runId = redis.call('GET', idempotencyLookupKey) |
| 167 | +if not runId then return cjson.encode({}) end |
| 168 | + |
| 169 | +local entryKey = entryPrefix .. runId |
| 170 | +if redis.call('EXISTS', entryKey) == 0 then |
| 171 | + -- Stale lookup (entry expired without the lookup being cleaned up). |
| 172 | + -- Lazy cleanup. |
| 173 | + redis.call('DEL', idempotencyLookupKey) |
| 174 | + return cjson.encode({}) |
| 175 | +end |
| 176 | + |
| 177 | +-- Clear the idempotency fields on the snapshot payload. |
| 178 | +local payloadJson = redis.call('HGET', entryKey, 'payload') |
| 179 | +local payload = cjson.decode(payloadJson) |
| 180 | +payload.idempotencyKey = cjson.null |
| 181 | +payload.idempotencyKeyExpiresAt = cjson.null |
| 182 | +redis.call('HSET', entryKey, 'payload', cjson.encode(payload)) |
| 183 | + |
| 184 | +redis.call('DEL', idempotencyLookupKey) |
| 185 | +return cjson.encode({ runId = runId }) |
| 186 | +``` |
| 187 | + |
| 188 | +Single round-trip, atomic per-Redis-script. The customer sees the same `{ id: idempotencyKey }` response either way. |
| 189 | + |
| 190 | +### Dashboard reset surface |
| 191 | + |
| 192 | +`POST /resources/.../runs/{runParam}/idempotencyKey/reset` flow: |
| 193 | + |
| 194 | +1. Resolve runId → snapshot (via existing readFallback for buffer, or PG findFirst). |
| 195 | +2. Read the snapshot's `idempotencyKey` field. |
| 196 | +3. If null, return "This run does not have an idempotency key" (existing message). |
| 197 | +4. Otherwise call the same `ResetIdempotencyKeyService.call(key, taskIdentifier, env)`. The service handles both stores. |
| 198 | + |
| 199 | +No special-case for buffered vs PG runs at the route level. The service's two-store reset is the abstraction. |
| 200 | + |
| 201 | +## Why this works |
| 202 | + |
| 203 | +### Trigger-time dedup is symmetric with PG semantics |
| 204 | + |
| 205 | +The SETNX inside `acceptMollifierEntry` mirrors PG's unique-key behaviour at trigger time: |
| 206 | + |
| 207 | +- Two simultaneous PG triggers race. One wins, the other's `findFirst` sees the winner before its own insert, returns cached. |
| 208 | +- Two simultaneous buffered triggers race. One wins the SETNX, the other's accept-Lua sees the lookup set, returns the existing runId. |
| 209 | +- A buffered trigger followed by a PG trigger: PG `findFirst` returns null (the row isn't in PG), then the buffer lookup hits → return cached buffered runId. ✓ |
| 210 | +- A PG trigger followed by a buffered trigger: PG `findFirst` returns the existing PG row → return cached. ✓ |
| 211 | +- A buffered trigger followed by another buffered trigger after the first has drained: PG `findFirst` returns the (now-materialised) row → return cached. Buffer lookup was cleared at materialisation, so the second buffered trigger correctly sees PG only. ✓ |
| 212 | + |
| 213 | +### Reset is symmetric too |
| 214 | + |
| 215 | +- A key bound to a PG row: existing `updateMany` clears it. |
| 216 | +- A key bound to a buffered run: the new buffer-side reset clears it. |
| 217 | +- A key bound to both (during the in-flight window after drainer materialised but before its ack ran): existing `updateMany` clears PG; the buffer-side reset is a no-op (lookup already cleared by drainer ack). Counts to 1. |
| 218 | +- A key not bound anywhere: 404 (existing behaviour, both stores return 0). |
| 219 | + |
| 220 | +### Failure isolation |
| 221 | + |
| 222 | +Stale lookups are bounded by the TTL match — both the entry hash and the idempotency lookup TTL at the same time. If the lookup somehow persists past the entry (e.g., the drainer ack's DEL was lost to a partial Redis write), the next access through `lookupIdempotency` returns a runId for a non-existent entry. The buffer's helper detects this and lazy-cleans: |
| 223 | + |
| 224 | +```ts |
| 225 | +async lookupIdempotency({ envId, taskIdentifier, idempotencyKey }) { |
| 226 | + const runId = await this.redis.get(/*lookup key*/); |
| 227 | + if (!runId) return null; |
| 228 | + const entry = await this.getEntry(runId); |
| 229 | + if (!entry) { |
| 230 | + await this.redis.del(/*lookup key*/); // self-heal |
| 231 | + return null; |
| 232 | + } |
| 233 | + return runId; |
| 234 | +} |
| 235 | +``` |
| 236 | + |
| 237 | +## Behaviour table |
| 238 | + |
| 239 | +| Scenario | Trigger response | Reset response | |
| 240 | +|---|---|---| |
| 241 | +| Key K bound to PG run R1 | `findFirst` hits → return R1 cached | `updateMany` clears K on R1. Returns `{ id: K }` | |
| 242 | +| Key K bound to buffered run R1 | PG miss → buffer lookup hits → return R1 cached (synthesised) | Buffer Lua clears K on R1's snapshot + lookup DEL. Returns `{ id: K }` | |
| 243 | +| Key K bound to PG R1 AND buffered R2 (impossible — SETNX prevents) | n/a | n/a | |
| 244 | +| Key K bound nowhere | Returns null → new trigger proceeds | 404 (matches existing behaviour) | |
| 245 | +| Key K bound to buffered R1, R1 drains, customer triggers with K again | PG `findFirst` hits the now-materialised R1 → return cached | n/a | |
| 246 | +| Two simultaneous triggers, both with key K | One's accept-Lua wins SETNX. The other's accept-Lua sees the lookup, refuses, returns the winner's runId. Customer of the loser gets the winner's runId as their response. | n/a | |
| 247 | + |
| 248 | +## Forward-compatibility under rolling update |
| 249 | + |
| 250 | +New Redis key: `mollifier:idempotency:{envId}:{taskIdentifier}:{key}`. New Lua extension on `acceptMollifierEntry`. |
| 251 | + |
| 252 | +Rolling-update concern: if we deploy the new acceptMollifierEntry Lua before the new trigger-time dedup logic, accept will be setting lookups that nothing reads. Harmless. |
| 253 | + |
| 254 | +If we deploy the new trigger-time dedup before the new accept-Lua, the lookup will always be empty (nothing writes it), so the new check is a no-op until the new accept runs. Also harmless. |
| 255 | + |
| 256 | +Reset similarly: the buffer-side reset is independent of accept. Can deploy in either order. |
| 257 | + |
| 258 | +So the rollout is not strictly ordered — any of the three changes can ship independently and the system stays correct, just incrementally less complete until all three are deployed. |
| 259 | + |
| 260 | +## Test coverage |
| 261 | + |
| 262 | +Unit tests in `packages/redis-worker/src/mollifier/buffer.test.ts`: |
| 263 | + |
| 264 | +1. `accept` with no idempotency key — no lookup written. |
| 265 | +2. `accept` with idempotency key — lookup SET to the runId, TTL matches entry. |
| 266 | +3. `accept` with already-bound idempotency key — Lua returns `duplicate_idempotency` with the existing runId. |
| 267 | +4. `lookupIdempotency` hit / miss / stale (lookup points at expired entry — self-heals). |
| 268 | +5. `resetIdempotencyKey` — clears snapshot + lookup atomically; idempotent on already-cleared. |
| 269 | +6. Drainer ack — DELs the lookup when entry had idempotency key. |
| 270 | + |
| 271 | +Integration tests in `apps/webapp/test/idempotency-buffered.test.ts`: |
| 272 | + |
| 273 | +7. Trigger A with key K → buffered. Trigger B with same K — returns A's runId. |
| 274 | +8. Trigger A with K → buffered → drain. Trigger B with K — returns A's materialised PG row. |
| 275 | +9. Trigger A with K → buffered. Reset K. Trigger B with K — creates new buffered run B. |
| 276 | +10. Trigger A with K → buffered. Dashboard reset on A's runId clears K from snapshot. Trigger B with K — creates new buffered run B. |
| 277 | + |
| 278 | +## What this design does NOT cover |
| 279 | + |
| 280 | +- Idempotency-key expiry handling — unchanged from PG-side behaviour. The existing `handleTriggerRequest` checks `idempotencyKeyExpiresAt` against the current time and clears expired keys. The buffer-side synthesis returns the same fields, so the same logic runs against either source. No new code path. |
| 281 | +- Cross-env or cross-task idempotency — not a thing today, not introduced. |
| 282 | +- Bulk reset (resetting many keys at once) — out of scope, no existing API surface. |
| 283 | + |
| 284 | +## Files touched |
| 285 | + |
| 286 | +**Modified:** |
| 287 | +- `packages/redis-worker/src/mollifier/buffer.ts` — extend `acceptMollifierEntry` Lua, drainer ack Lua, add `lookupIdempotency` + `resetIdempotency` methods. |
| 288 | +- `apps/webapp/app/runEngine/concerns/idempotencyKeys.server.ts` — `findExistingIdempotentRun` helper checks both stores. |
| 289 | +- `apps/webapp/app/v3/services/resetIdempotencyKey.server.ts` — call buffer reset alongside PG `updateMany`. |
| 290 | +- `apps/webapp/app/v3/mollifier/readFallback.server.ts` — extend snapshot-to-TaskRun synthesis to include `idempotencyKeyExpiresAt` and `associatedWaitpoint` (if not already present) for the dedup logic. |
| 291 | + |
| 292 | +**New tests:** |
| 293 | +- `packages/redis-worker/src/mollifier/buffer.test.ts` extensions. |
| 294 | +- `apps/webapp/test/idempotency-buffered.test.ts`. |
| 295 | + |
| 296 | +## What this fixes |
| 297 | + |
| 298 | +| Bug | Today | After | |
| 299 | +|---|---|---| |
| 300 | +| Trigger-time dedup blind to buffer | Two rapid triggers with same K during burst → two runs created | One run, the second trigger returns the first's runId | |
| 301 | +| Reset can't clear buffered keys | Reset succeeds on PG; buffered run materialises with key still set | Reset clears both stores; buffered run materialises without key | |
| 302 | +| Dashboard reset on a buffered run | "Run not found" or "This run does not have an idempotency key" depending on lookup path | Resolves through readFallback, finds the key on snapshot, clears it | |
| 303 | + |
| 304 | +## Risks |
| 305 | + |
| 306 | +- **The SETNX on accept becomes load-bearing for idempotency correctness.** Previously, idempotency dedup was PG-only and happened pre-buffer; the buffer didn't participate. Now the buffer's accept-Lua is on the dedup critical path. Test coverage for the race cases (two simultaneous accepts) is the highest priority. |
| 307 | +- **TTL drift between entry hash and idempotency lookup.** Both are set with the same TTL on accept, but if the entry is requeued (`requeueMollifierEntry` after a transient drainer error), the TTL extends. The lookup's TTL doesn't extend automatically. Need to extend the requeue Lua to also EXPIRE the lookup. Tiny change; flag it explicitly. |
| 308 | +- **Migration concern.** Existing buffered runs (from prior to this change) won't have lookups in Redis. They'll fall through trigger-time dedup as if no key was bound. Acceptable transient — within the buffer TTL (10 min default), this resolves. Document in the migration notes. |
0 commit comments