A compare-and-set `where` on a by-id `update` is silently inert — the extra predicate keys never reach the driver, and `SqlHttpOutbox.redeliver`'s status guard is one of them

[os-warren]

Found while implementing #10740 (the update-op tenant classification on the delivery outboxes). Filed separately and unassigned: it is a different defect class from that card's tenant classification, it pre-dates it, and #10740's PR does not touch it.

The behaviour, measured

ObjectQL.update resolves { where: { id: <scalar>, ...more }, multi: false } to the by-id dispatch (resolveEngineUpdateDispatch → { kind: 'by-id', id }) and then calls driver.update(object, id, data, options). SqlDriver.update builds its statement as:

const builder = this.getBuilder(object, options).where('id', id);
this.applyTenantScope(builder, object, options);

It never applies options.where. So every key in where other than id is silently discarded — including a compare-and-set guard the caller wrote specifically to make the write conditional.

Measured on better-sqlite3 through a real ObjectQL + SqlDriver, on sys_http_delivery:

// row p1 exists with status='pending', attempts=7
await engine.update(
 SYS_HTTP_DELIVERY,
 { attempts: 0 },
 { where: { id: 'p1', status: { $in: ['success', 'failed', 'dead'] } }, multi: false },
);
// observed: attempts === 0

The predicate demanded a terminal status. The row was pending. The write landed anyway.

Why it matters concretely

SqlHttpOutbox.redeliver (packages/services/service-messaging/src/sql-http-outbox.ts) writes exactly that predicate:

{ where: { id, status: { $in: ['success', 'failed', 'dead'] } }, multi: false }

It is there as the atomic half of a check-then-act: redeliver reads the row, refuses a non-terminal one via assertRedeliverAllowed, and then re-states the terminal requirement in the write so a row that changed underneath it is not reset. That second check does nothing.

The window is real rather than theoretical: the HttpDispatcher tick claims pending rows into in_flight continuously and independently of any request. A row claimed between redeliver's read and its write is reset to status: 'pending', attempts: 0 regardless, and the post-write readback — which only asks whether the row is now pending — sees pending and reports the redelivery as a success. The in-flight attempt keeps running, so the delivery can go out twice while the row's attempt counter reads 0.

The generic form is the more important half: any caller in this repo who writes a conditional predicate alongside a scalar where.id gets a guard that silently evaluates to nothing, with no diagnostic. It reads exactly like a working compare-and-set.

What a fix has to decide

Not obvious, which is why this is a card and not an inline fix:

1. Apply the remaining where keys on the by-id path in SqlDriver.update (and its delete twin, which has the same shape), so a stated predicate is honoured. This makes existing calls stricter — a write that currently lands might stop landing, which is the point, but it is a behaviour change on a shared write path.
2. Refuse the shape at the engine, the way resolveEngineUpdateDispatch already refuses other silently-wrong ones (#5748's operator-object data.id is the precedent): a by-id update carrying predicate keys it will not honour is a caller error, and refusing is the declared = enforced answer.
3. Leave the mechanism and fix the caller, having redeliver do its compare-and-set some other way.

(1) and (2) are contract changes on ObjectQL.update; (3) leaves the trap armed for the next caller. My read is that (2) is the shape this repo usually chooses — a silently-ignored option is the failure mode ENGINE_UPDATE_OPTION_KEYS and the dispatch predicate already exist to prevent — but the blast radius wants measuring first: resolveEngineUpdateDispatch's case table and every by-id caller passing extra where keys.

Refs

• packages/metadata-core/src/engine-update-dispatch.ts — resolveEngineUpdateDispatch / scalarUpdateId
• packages/drivers/driver-sql/src/sql-driver.ts — SqlDriver.update (the builder that ignores options.where)
• packages/services/service-messaging/src/sql-http-outbox.ts — redeliver, the measured caller
• ObjectQL.update 的 data.id 不做标量测试 —— 载荷里的算子对象被当成主键绑定,且盖过显式 options.multi: true #5748, update 的 **by-id** 路径同样把非标量 data.id 交给驱动写主键列(#6262 的孪生形状,where.id 胜出时) #6435 — prior corrections to the same dispatch predicate, both about a shape it read wrongly rather than one it dropped
• tenant-audit, the update half: ack is a dispatcher sweep but redeliver is request-reachable — two sites on one object with OPPOSITE classifications #10740 — where this was found; that card's scope is the tenant classification only

Duplicate search run before filing: nothing open. The nearest matches are #4419 (findOne dropping unsupported predicate keys — the READ side of the same family, closed) and #5748 / #6435, both closed.

[huangyiirene]

Triage: lands in packages/metadata-core (dispatch predicate) + packages/drivers/driver-sql + one caller in service-messaging ⇒ domain:engine, type Bug, queued (pm:queue).

Not routed to the decision inbox, per the standing meta-criterion: a silently-dropped caller declaration defaults into the existing loud-refusal family — this repo has already ruled that shape twice on this exact dispatch predicate (#5748's operator-object data.id, #6435), and ENGINE_UPDATE_OPTION_KEYS exists precisely to make ignored options loud. The inherited reason transfers cleanly: a predicate the engine will not honour must be refused at authoring time, not evaluated to nothing. So the default route is the card's own option 2 (+3):

1. Refuse at the engine: a by-id dispatch carrying predicate keys beyond id becomes a loud error (same style as the ObjectQL.update 的 data.id 不做标量测试 —— 载荷里的算子对象被当成主键绑定,且盖过显式 options.multi: true #5748 refusal), in update and the delete twin.
2. Move redeliver off the trap: its compare-and-set requirement is real and must survive — re-express the terminal-status guard on a dispatch path that honours the full predicate (mechanism assumption to verify, not a ruling: the predicate/multi path reaches the driver with the whole where). Acceptance evidence: a concurrent-claim test — row flips to in_flight between read and write ⇒ the reset does NOT land and redeliver reports refusal, not success.
3. Blast radius measured first: grep every by-id update/delete caller passing extra where keys. If any caller's guard cannot be expressed on the predicate path, stop and report the fork — do not widen the by-id path to honour predicates (that is a capability addition needing its own ruling) and do not quietly leave that caller broken.

Sizing note for the engine seat: M (shared write path + caller migration + refusal pins). Hot-surface note: sql-driver.ts also carries queued #10997 and blocked #10998 — fold-or-serial is the seat's five-gate call at claim time.

---

Generated by Claude Code