Skip to content

Commit 5234b7c

Browse files
claude[bot]claude
andauthored
fix(service-queue): compare the publish idempotency window as instants, not strings (#14200)
* fix(service-queue): compare the publish idempotency window as instants, not strings (#13993) The idempotency check deduped terminal rows with a lexicographic String(row.created_at) compare against ISO text. On Postgres/MySQL the builtin audit column materialises as a JS Date whose String() starts with a weekday letter, unconditionally above the ISO window-start's digit, so the predicate was always true: terminal rows blocked re-publish forever and publish() silently enqueued nothing. Normalise created_at to an instant (the canonicalVersionInstant shape) and compare epoch ms; the pending/running arm and SQLite verdicts are unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs * test(service-queue): conform the fake engine to the double gates and register its pins The new double's update()/delete() now open with the engine's own dispatch predicates, find() bounds by presence and refuses combinators, and the engine-double-contract RETAINED ledger records the new (file, verb) pins. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 1403d94 commit 5234b7c

4 files changed

Lines changed: 303 additions & 2 deletions

File tree

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
---
2+
"@objectstack/service-queue": patch
3+
---
4+
5+
fix(service-queue): compare the publish idempotency window as instants, not strings (#13993)
6+
7+
`DbQueueAdapter#publish` deduped terminal rows with
8+
`String(row.created_at) >= windowStart` — a lexicographic compare of the raw
9+
driver value against canonical ISO text. On Postgres/MySQL the builtin audit
10+
column `created_at` comes out of the record read door as a JS `Date`, whose
11+
`String()` begins with a weekday letter, unconditionally above the ISO
12+
window-start's leading digit — so the predicate was always true: any terminal
13+
(`completed`/`dlq`) row with that idempotency key blocked re-publish forever,
14+
and `publish()` returned the old id having enqueued nothing. Silent message
15+
loss on the production default drivers; SQLite (ISO text on both sides) was
16+
always correct, which is why every existing test stayed green.
17+
18+
The check now normalises `created_at` to an instant (the #13382
19+
`canonicalVersionInstant` shape: `Date`, epoch-ms number, or absolute ISO
20+
text) and compares epoch milliseconds, so every dialect gets the declared
21+
window semantics. The `pending`/`running` arm — which blocks regardless of
22+
age — is untouched, and SQLite verdicts are unchanged. A `created_at` that
23+
denotes no instant cannot be inside a window measured on the `created_at`
24+
axis and no longer blocks.
Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,207 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#13993] The publish idempotency window, driven through every `created_at`
5+
* materialisation a driver actually hands out of the record read door.
6+
*
7+
* The defect: `DbQueueAdapter#publish` compared
8+
* `String(row.created_at) >= windowStart` — lexicographic text against
9+
* canonical ISO text. On Postgres/MySQL the builtin audit column `created_at`
10+
* comes back as a JS `Date` (pinned in `driver-sql`'s
11+
* `sql-driver-13567-audit-stamp-materialisation.test.ts`), whose `String()`
12+
* starts with a weekday LETTER (0x41–0x5A), unconditionally above the ISO
13+
* text's leading digit `'2'` (0x32) — so the predicate was TRUE for every
14+
* terminal row, the window never expired, and `publish()` returned the old id
15+
* having enqueued nothing: silent message loss on the production default
16+
* drivers. SQLite hands back ISO-Z text on both sides, so the SQLite arm was
17+
* always correct — which is why every existing test stayed green, and why the
18+
* ISO cases below are the CONTROL group: they must keep passing unchanged.
19+
*
20+
* The discriminating `Date` input exists in CI only inside
21+
* `@objectstack/driver-sql` today (#13973 point 4), so this pin lives in THIS
22+
* package driving hand-made `Date`s — deliberately NOT by widening any
23+
* required job's package set (#13567, maintainer decision).
24+
*
25+
* Assertions are DIRECTIONAL, not literal: out-of-window must stop blocking,
26+
* in-window must keep blocking, and `pending`/`running` rows must block
27+
* regardless of age (that arm bypasses the time compare entirely).
28+
*/
29+
30+
import { describe, it, expect } from 'vitest';
31+
import {
32+
assertEngineDeleteDispatch,
33+
assertEngineUpdateDispatch,
34+
} from '@objectstack/objectql';
35+
import { DbQueueAdapter } from './db-queue-adapter.js';
36+
37+
/**
38+
* Minimal engine double — only the surface `publish()` touches. `update()` and
39+
* `delete()` are unreachable from `publish()`, but they still open with the
40+
* engine's own dispatch predicates so this fake can never drift looser than
41+
* ObjectQL's contract (`check:engine-double-contract`).
42+
*/
43+
function makeFakeEngine(seed: any[] = []) {
44+
const rows: any[] = [...seed];
45+
return {
46+
rows,
47+
async find(_table: string, opts: any = {}) {
48+
const out = opts?.where
49+
? rows.filter((r) => Object.entries(opts.where).every(([k, v]) => {
50+
// Refuse combinators rather than reading them as field names.
51+
if (k.startsWith('$')) throw new Error(`fake engine: unsupported operator ${k}`);
52+
return r[k] === v;
53+
}))
54+
: [...rows];
55+
// The caller's bound, by PRESENCE — `limit: 0` must bound to zero rows.
56+
return typeof opts?.limit === 'number' ? out.slice(0, opts.limit) : out;
57+
},
58+
async insert(_table: string, data: any) {
59+
rows.push({ ...data });
60+
return { id: data.id };
61+
},
62+
async update(_table: string, data: any, options?: any): Promise<never> {
63+
assertEngineUpdateDispatch(data, options);
64+
throw new Error('not reachable from publish()');
65+
},
66+
async delete(_table: string, options?: any): Promise<never> {
67+
assertEngineDeleteDispatch(options);
68+
throw new Error('not reachable from publish()');
69+
},
70+
};
71+
}
72+
73+
/** Frozen "now" so window edges are deterministic. */
74+
const NOW_MS = Date.parse('2026-08-30T10:00:00.000Z');
75+
const WINDOW_MS = 60_000;
76+
77+
function makeAdapter(seed: any[]) {
78+
const engine = makeFakeEngine(seed);
79+
const adapter = new DbQueueAdapter({
80+
engine,
81+
clock: { now: () => new Date(NOW_MS) },
82+
options: { autoStart: false, idempotencyWindowMs: WINDOW_MS },
83+
});
84+
return { engine, adapter };
85+
}
86+
87+
function terminalRow(id: string, status: 'completed' | 'dlq', createdAt: unknown) {
88+
return {
89+
id,
90+
queue: 'q',
91+
idempotency_key: 'k',
92+
status,
93+
created_at: createdAt,
94+
};
95+
}
96+
97+
describe('[#13993] publish idempotency window vs created_at materialisation', () => {
98+
describe('Date side (Postgres/MySQL/Mongo hand the audit column back as a JS Date)', () => {
99+
it('an OUT-OF-WINDOW terminal Date row no longer blocks — publish enqueues a NEW message', async () => {
100+
// Pre-fix this row blocked FOREVER: String(Date) begins with a weekday
101+
// letter, lexicographically above the ISO windowStart's digit.
102+
const { engine, adapter } = makeAdapter([
103+
terminalRow('row_old', 'completed', new Date(NOW_MS - 2 * WINDOW_MS)),
104+
]);
105+
const id = await adapter.publish('q', { x: 1 }, { idempotencyKey: 'k' });
106+
expect(id).not.toBe('row_old');
107+
const inserted = engine.rows.find((r) => r.id === id);
108+
expect(inserted).toBeDefined();
109+
expect(inserted.status).toBe('pending');
110+
expect(engine.rows).toHaveLength(2);
111+
});
112+
113+
it('an IN-WINDOW terminal Date row still blocks — old id back, nothing enqueued', async () => {
114+
const { engine, adapter } = makeAdapter([
115+
terminalRow('row_recent', 'dlq', new Date(NOW_MS - WINDOW_MS / 2)),
116+
]);
117+
const id = await adapter.publish('q', { x: 1 }, { idempotencyKey: 'k' });
118+
expect(id).toBe('row_recent');
119+
expect(engine.rows).toHaveLength(1);
120+
});
121+
});
122+
123+
describe('ISO-text side (SQLite/turso/wasm/memory status quo — the CONTROL group)', () => {
124+
// The lexicographic compare was CORRECT on ISO-Z text (order = chronology).
125+
// These two must hold before AND after the fix; a red here is a regression
126+
// in the only arm that ever worked.
127+
it('an OUT-OF-WINDOW terminal ISO row does not block (as it never did on SQLite)', async () => {
128+
const { engine, adapter } = makeAdapter([
129+
terminalRow('row_old_iso', 'completed', new Date(NOW_MS - 2 * WINDOW_MS).toISOString()),
130+
]);
131+
const id = await adapter.publish('q', { x: 1 }, { idempotencyKey: 'k' });
132+
expect(id).not.toBe('row_old_iso');
133+
expect(engine.rows).toHaveLength(2);
134+
});
135+
136+
it('an IN-WINDOW terminal ISO row still blocks', async () => {
137+
const { engine, adapter } = makeAdapter([
138+
terminalRow('row_recent_iso', 'completed', new Date(NOW_MS - WINDOW_MS / 2).toISOString()),
139+
]);
140+
const id = await adapter.publish('q', { x: 1 }, { idempotencyKey: 'k' });
141+
expect(id).toBe('row_recent_iso');
142+
expect(engine.rows).toHaveLength(1);
143+
});
144+
});
145+
146+
describe('epoch-ms number (pre-canonical / hand-migrated SQLite column)', () => {
147+
it('windowed verdicts hold for a numeric created_at too', async () => {
148+
const outOfWindow = makeAdapter([
149+
terminalRow('row_old_num', 'completed', NOW_MS - 2 * WINDOW_MS),
150+
]);
151+
const idA = await outOfWindow.adapter.publish('q', { x: 1 }, { idempotencyKey: 'k' });
152+
expect(idA).not.toBe('row_old_num');
153+
154+
const inWindow = makeAdapter([
155+
terminalRow('row_recent_num', 'completed', NOW_MS - WINDOW_MS / 2),
156+
]);
157+
const idB = await inWindow.adapter.publish('q', { x: 1 }, { idempotencyKey: 'k' });
158+
expect(idB).toBe('row_recent_num');
159+
});
160+
});
161+
162+
describe('reverse control: the non-terminal arm bypasses the time compare', () => {
163+
// pending/running block REGARDLESS of age — prove the fix did not narrow
164+
// that arm. Both materialisations, both statuses, absurdly old stamps.
165+
it('a pending row blocks however old, Date and ISO alike', async () => {
166+
for (const createdAt of [
167+
new Date(NOW_MS - 1000 * WINDOW_MS),
168+
new Date(NOW_MS - 1000 * WINDOW_MS).toISOString(),
169+
]) {
170+
const { engine, adapter } = makeAdapter([
171+
{ id: 'row_pending', queue: 'q', idempotency_key: 'k', status: 'pending', created_at: createdAt },
172+
]);
173+
const id = await adapter.publish('q', { x: 1 }, { idempotencyKey: 'k' });
174+
expect(id).toBe('row_pending');
175+
expect(engine.rows).toHaveLength(1);
176+
}
177+
});
178+
179+
it('a running row blocks however old, Date and ISO alike', async () => {
180+
for (const createdAt of [
181+
new Date(NOW_MS - 1000 * WINDOW_MS),
182+
new Date(NOW_MS - 1000 * WINDOW_MS).toISOString(),
183+
]) {
184+
const { engine, adapter } = makeAdapter([
185+
{ id: 'row_running', queue: 'q', idempotency_key: 'k', status: 'running', created_at: createdAt },
186+
]);
187+
const id = await adapter.publish('q', { x: 1 }, { idempotencyKey: 'k' });
188+
expect(id).toBe('row_running');
189+
expect(engine.rows).toHaveLength(1);
190+
}
191+
});
192+
});
193+
194+
describe('a created_at that denotes no instant', () => {
195+
it('cannot be inside a window measured on the created_at axis — does not block', async () => {
196+
// Documented decision (createdAtInstantMs): duplicate delivery is
197+
// tolerated by contract; "suppress forever" is the defect. Pre-fix this
198+
// very value DID block forever ('n' is above '2' lexicographically).
199+
const { engine, adapter } = makeAdapter([
200+
terminalRow('row_opaque', 'completed', 'not-an-instant'),
201+
]);
202+
const id = await adapter.publish('q', { x: 1 }, { idempotencyKey: 'k' });
203+
expect(id).not.toBe('row_opaque');
204+
expect(engine.rows).toHaveLength(2);
205+
});
206+
});
207+
});

packages/services/service-queue/src/db-queue-adapter.ts

Lines changed: 62 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,57 @@ import {
2121

2222
const QUEUE_TABLE = 'sys_job_queue';
2323

24+
/**
25+
* [#13993] An ISO-8601 date-time that denotes an ABSOLUTE instant — it
26+
* carries an explicit `Z` or a numeric offset, so reading it never consults
27+
* the process timezone. Same shape as the OCC seam's `ABSOLUTE_ISO_INSTANT`
28+
* (#13382, `packages/metadata-protocol/src/protocol.ts`): a string this
29+
* pattern rejects does not denote an instant and is not guessed at.
30+
*/
31+
const ABSOLUTE_ISO_INSTANT =
32+
/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}(?::\d{2}(?:\.\d+)?)?(?:Z|[+-]\d{2}:\d{2})$/;
33+
34+
/**
35+
* [#13993] A `created_at` as a driver hands it out of the record read door,
36+
* read as epoch milliseconds — or null when the value does not denote an
37+
* instant.
38+
*
39+
* `created_at` is a builtin audit column: it is not in `datetimeFields`, so no
40+
* declared-field coercion reaches it, and the dialects genuinely disagree on
41+
* its materialisation (the domain below is the one #13382 measured and #13973
42+
* re-measured, pinned in `driver-sql`'s
43+
* `sql-driver-13567-audit-stamp-materialisation.test.ts`):
44+
*
45+
* - **JS `Date`** — Postgres / MySQL (`timestamptz` / `DATETIME(3)` are
46+
* instants and the driver materialises them as `Date` on purpose;
47+
* `SqlDriver.withPostgresCalendarDayAsText` says so in as many words).
48+
* - **canonical ISO-8601 UTC text** — SQLite and its turso / sqlite-wasm
49+
* siblings, and `driver-memory`. This adapter writes `toISOString()`.
50+
* - **`number`, epoch milliseconds** — a pre-canonical or hand-migrated
51+
* SQLite column; the legacy datetime repair is keyed on declared
52+
* `Field.datetime` columns, and the engine-injected audit columns are not
53+
* in that set.
54+
* - **anything else** — not an instant. Returns null, and the caller treats
55+
* the row as OUTSIDE the window: the dedup window is measured on the
56+
* `created_at` axis, so a row that cannot be placed on that axis cannot be
57+
* inside it (and duplicate delivery is tolerated by contract — see
58+
* `claimBatch` — while "suppress forever" is the very defect #13993
59+
* removes).
60+
*/
61+
function createdAtInstantMs(value: unknown): number | null {
62+
let ms: number;
63+
if (value instanceof Date) {
64+
ms = value.getTime();
65+
} else if (typeof value === 'number') {
66+
ms = value;
67+
} else if (typeof value === 'string' && ABSOLUTE_ISO_INSTANT.test(value.trim())) {
68+
ms = Date.parse(value.trim());
69+
} else {
70+
return null;
71+
}
72+
return Number.isFinite(ms) ? ms : null;
73+
}
74+
2475
/**
2576
* How long a `completed` row survives before the platform Reaper deletes it.
2677
*
@@ -247,7 +298,7 @@ export class DbQueueAdapter implements IQueueService {
247298
// constructor — which makes "the reaper deleted a row the dedup check
248299
// needed" unrepresentable rather than merely unlikely.
249300
if (opts.idempotencyKey) {
250-
const windowStart = new Date(now.getTime() - this.opts.idempotencyWindowMs).toISOString();
301+
const windowStartMs = now.getTime() - this.opts.idempotencyWindowMs;
251302
const existing = await this.engine.find(QUEUE_TABLE, {
252303
where: {
253304
queue,
@@ -259,7 +310,16 @@ export class DbQueueAdapter implements IQueueService {
259310
});
260311
const blocking = (existing ?? []).find((row: any) => {
261312
if (row.status === 'pending' || row.status === 'running') return true;
262-
return String(row.created_at ?? '') >= windowStart;
313+
// [#13993] Compare INSTANTS, not strings (#13382's shape). The old
314+
// `String(row.created_at) >= windowStart` was a lexicographic compare
315+
// whose left side, on Postgres/MySQL, is a `Date.toString()` starting
316+
// with a weekday LETTER — unconditionally above the ISO text's digit —
317+
// so every terminal row blocked forever and publish() silently
318+
// enqueued nothing. An instant compare gives every materialisation the
319+
// same verdict; a row whose created_at denotes no instant cannot be
320+
// inside the window (see createdAtInstantMs).
321+
const createdAtMs = createdAtInstantMs(row.created_at);
322+
return createdAtMs !== null && createdAtMs >= windowStartMs;
263323
});
264324
if (blocking) return String(blocking.id);
265325
}

scripts/engine-double-contract.pinned.json

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3256,6 +3256,16 @@
32563256
"verb": "findOne",
32573257
"pinned": 1
32583258
},
3259+
{
3260+
"file": "packages/services/service-queue/src/db-queue-adapter-13993-idempotency-window-materialisation.test.ts",
3261+
"verb": "delete",
3262+
"pinned": 1
3263+
},
3264+
{
3265+
"file": "packages/services/service-queue/src/db-queue-adapter-13993-idempotency-window-materialisation.test.ts",
3266+
"verb": "update",
3267+
"pinned": 1
3268+
},
32593269
{
32603270
"file": "packages/services/service-queue/src/db-queue-adapter.test.ts",
32613271
"verb": "delete",

0 commit comments

Comments
 (0)