Skip to content

Commit a2451f3

Browse files
committed
fix(run-store): clear a stale records field when a new wait cycle reuses a key
The seq counter can vanish under maxmemory eviction while a wp:<n> key survives. A birth does not check seq, unlike a transition, so the counter restarts and re-mints a cycleSeq whose key still holds another cycle's records. order and count are both overwritten, so they stay consistent with each other and the mismatch check cannot see the drift. A resolver would then read a previous cycle's records paired with a new order.
1 parent 178472e commit a2451f3

2 files changed

Lines changed: 53 additions & 0 deletions

File tree

‎internal-packages/run-store/src/redisSnapshotStore.test.ts‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,54 @@ describe("append", () => {
278278
}
279279
);
280280

281+
redisTest(
282+
"a recordless new cycle clears another cycle's records off a reused key",
283+
async ({ redisOptions }) => {
284+
const store = new RedisSnapshotStore({ redisOptions, completedTtlMs: 1000 });
285+
const raw = createRedisClient(redisOptions);
286+
try {
287+
await store.append({
288+
entry: entry({ id: "snap_1" }),
289+
kind: "birth",
290+
isTerminal: false,
291+
cycle: {
292+
kind: "new",
293+
completedWaitpoints: [{ id: "w_a", index: 0 }],
294+
records: [
295+
{
296+
id: "w_a",
297+
friendlyId: "waitpoint_a",
298+
type: "MANUAL",
299+
completedAt: "2026-01-01T00:00:00.000Z",
300+
outputType: "application/json",
301+
outputIsError: false,
302+
output: { inline: "stale" },
303+
},
304+
],
305+
},
306+
});
307+
expect(await raw.hget("snap:{run_1}:wp:1", "records")).not.toBeNull();
308+
309+
// Only the counter is lost, as under maxmemory eviction. A birth does not check seq, so
310+
// the next new cycle re-mints cycleSeq 1 onto the surviving key.
311+
await raw.del("snap:{run_1}:seq");
312+
313+
await store.append({
314+
entry: entry({ id: "snap_2" }),
315+
kind: "birth",
316+
isTerminal: false,
317+
cycle: { kind: "new", completedWaitpoints: [{ id: "w_b", index: 0 }] },
318+
});
319+
320+
expect(await raw.hget("snap:{run_1}:wp:1", "order")).toBe(JSON.stringify(["w_b"]));
321+
expect(await raw.hget("snap:{run_1}:wp:1", "records")).toBeNull();
322+
} finally {
323+
raw.disconnect();
324+
await store.quit();
325+
}
326+
}
327+
);
328+
281329
redisTest(
282330
"reports a duplicate id without overwriting the original entry",
283331
async ({ redisOptions }) => {

‎internal-packages/run-store/src/redisSnapshotStore.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -571,6 +571,11 @@ export class RedisSnapshotStore {
571571
redis.call('HSET', wpKey(cycleSeq), 'order', orderJson, 'count', orderCount)
572572
if records ~= '' then
573573
redis.call('HSET', wpKey(cycleSeq), 'records', records)
574+
else
575+
-- A new cycle owns the whole key: a lost seq counter can re-mint a cycleSeq whose key
576+
-- still holds another cycle's records, and order/count stay mutually consistent so the
577+
-- mismatch check cannot see it. No-op on a fresh key.
578+
redis.call('HDEL', wpKey(cycleSeq), 'records')
574579
end
575580
elseif cycleMode == 'carry' then
576581
cycleSeq = cycleSeqIn

0 commit comments

Comments
 (0)