From 680def069df9db734c4459789dc2c7a08265c4ea Mon Sep 17 00:00:00 2001 From: Wes Mason Date: Fri, 21 Aug 2026 13:31:49 +0100 Subject: [PATCH] test(run-engine): pin the parked tag a gated variant registers with The rewind guard added alongside it was covered, but deleting the idle correction underneath it outright left all 59 vtime tests green, so the behaviour that correction exists for was still held up by a comment. A variant registering from the gated batch has to come back at its parked tag rather than at the floor, or draining under the gate hands it full credit. The comment saying so shrinks to the part the test cannot state, which is why the current score is the thing that tells an already-registered variant apart from one this call just added. --- .../run-engine/src/run-queue/index.ts | 9 ++-- .../tests/ckVtimeGatedRewind.test.ts | 51 +++++++++++++++++++ 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/internal-packages/run-engine/src/run-queue/index.ts b/internal-packages/run-engine/src/run-queue/index.ts index 9c8415e07..c2823de8a 100644 --- a/internal-packages/run-engine/src/run-queue/index.ts +++ b/internal-packages/run-engine/src/run-queue/index.ts @@ -5747,12 +5747,9 @@ if gatedPending ~= nil then end if redis.call('ZADD', unpack(gatedArgs)) > 0 then for _, ckQueueName in ipairs(gatedPending) do - -- Only the members this call put at the floor may take their parked tag back. The - -- batched ZADD reports how many it added but not which, and gatedPending can hold an - -- already-registered variant whenever the pass-1 scan was truncated, so the current - -- score is the discriminator: at the floor means it either just registered or has no - -- credit to lose either way. Without this the parked tag overwrites a live advanced - -- one and rewinds that variant's clock, which is the one thing NX exists to stop. + -- The batch ZADD reports how many it added, not which, and gatedPending can hold an + -- already-registered variant when the pass-1 scan was truncated. At the floor is the + -- discriminator: anything above it has spent credit that must not be rewound. local gateCur = redis.call('ZSCORE', ckVtimeKey, ckQueueName) if gateCur and tonumber(gateCur) <= floor then local gateIdle = redis.call('ZSCORE', ckVtimeIdleKey, ckQueueName) diff --git a/internal-packages/run-engine/src/run-queue/tests/ckVtimeGatedRewind.test.ts b/internal-packages/run-engine/src/run-queue/tests/ckVtimeGatedRewind.test.ts index f50317494..ef5334a3a 100644 --- a/internal-packages/run-engine/src/run-queue/tests/ckVtimeGatedRewind.test.ts +++ b/internal-packages/run-engine/src/run-queue/tests/ckVtimeGatedRewind.test.ts @@ -133,4 +133,55 @@ describe("CK vtime: the gated batch must not rewind a live tag", () => { await queue.quit(); } }); + + redisTest( + "a variant registering here still gets its parked tag back", + async ({ redisContainer }) => { + // The other half of the same branch. Deleting the idle correction outright used to + // leave every vtime suite green, so the rewind guard was pinned while the behaviour it + // guards was not. + const queue = createQueue(redisContainer); + try { + const t0 = Date.now() - 100_000; + + const cks = ["returner", "aa", "bb"]; + for (let i = 0; i < cks.length; i++) { + await queue.enqueueMessage({ + env: authenticatedEnvDev, + message: makeMessage({ + runId: `r-${cks[i]}`, + concurrencyKey: cks[i], + timestamp: t0 + i, + }), + workerQueue: authenticatedEnvDev.id, + skipDequeueProcessing: true, + }); + } + + const returner = variantName("returner"); + const ckVtimeKey = testOptions.keys.ckVtimeKeyFromQueue(returner); + const ckVtimeIdleKey = testOptions.keys.ckVtimeIdleKeyFromQueue(returner); + + await queue.redis.zadd(ckVtimeKey, 0, variantName("aa"), 1, variantName("bb")); + await queue.redis.zrem(ckVtimeKey, returner); + await queue.redis.zadd(ckVtimeIdleKey, 5, returner); + + await queue.updateQueueConcurrencyLimits(authenticatedEnvDev, QUEUE, 1); + for (const ck of cks) { + await queue.redis.sadd( + testOptions.keys.queueCurrentConcurrencyKeyFromQueue(variantName(ck)), + "occupant" + ); + } + + const shard = testOptions.keys.masterQueueShardForEnvironment(authenticatedEnvDev.id, 2); + await queue.testDequeueFromMasterQueue(shard, authenticatedEnvDev.id, 1); + + // Registering at the floor instead would hand it a turn it already spent. + expect(await queue.redis.zscore(ckVtimeKey, returner)).toBe("5"); + } finally { + await queue.quit(); + } + } + ); });