Measured while writing the contract for the ruled hard recall() on #10753 (that half was forked, see PR #11450's last section). Two related gaps on one surface, filed together because a fix to either has to decide the other.
1. There is no cancellation expression
INotificationOutbox (packages/services/service-messaging/src/outbox.ts:115-129) declares exactly:
enqueue | claim | ack | list | claimDigest
No cancel, no delete, no way to move a pending row out of the queue. Any feature that means "stop this delivery before it goes out" — retraction, a recalled notification, a superseded digest — has nothing to call.
2. ack() looks like the workaround, and it is a trap
ack(id, { success: false, suppressed: true }) flips a row to the terminal suppressed state, so it reads like the missing cancel. Neither implementation checks that the row is claimed:
MemoryNotificationOutbox.ack (memory-outbox.ts:118) — looks the row up by id and mutates it, no status precondition.SqlNotificationOutbox.ack (sql-outbox.ts:212) — reads only ['attempts'] by id, no status precondition.
So ack on an unclaimed pending row works, and two things go wrong quietly:
- It races the dispatcher. Between the caller's
list() and its ack(), claim() can take the row (that call is atomic by contract, this one is not part of it), the dispatcher sends it, and acks it back to success — or the reverse order suppresses a delivery already on the wire. There is no compare-and-set to express "suppress only if still pending". - It corrupts
attempts.ack increments attempts unconditionally. A row cancelled this way records an attempt that never happened — and attempts === 0 on a terminal row is a load-bearing predicate elsewhere on the sibling HTTP outbox (assertHttpRedeliverable, http-outbox.ts:315+, distinguishes "parked, never sent" from "sent and failed").
ack is the dispatcher's completion callback. Using it as a cancel primitive is the kind of reuse that reads correct in review and is wrong under concurrency.
What a fix has to decide
Whether cancellation is a new INotificationOutbox member (widening the contract plus both implementations, and SqlNotificationOutbox needs a conditional update so the transition is atomic against claim), or whether ack grows a status precondition and a documented cancel discriminator. Either way it is a surface decision, not a repair — which is why this is filed rather than done.
Links
Measured while writing the contract for the ruled hard
recall()on #10753 (that half was forked, see PR #11450's last section). Two related gaps on one surface, filed together because a fix to either has to decide the other.1. There is no cancellation expression
INotificationOutbox(packages/services/service-messaging/src/outbox.ts:115-129) declares exactly:No
cancel, nodelete, no way to move apendingrow out of the queue. Any feature that means "stop this delivery before it goes out" — retraction, a recalled notification, a superseded digest — has nothing to call.2.
ack()looks like the workaround, and it is a trapack(id, { success: false, suppressed: true })flips a row to the terminalsuppressedstate, so it reads like the missing cancel. Neither implementation checks that the row is claimed:MemoryNotificationOutbox.ack(memory-outbox.ts:118) — looks the row up by id and mutates it, no status precondition.SqlNotificationOutbox.ack(sql-outbox.ts:212) — reads only['attempts']by id, no status precondition.So
ackon an unclaimedpendingrow works, and two things go wrong quietly:list()and itsack(),claim()can take the row (that call is atomic by contract, this one is not part of it), the dispatcher sends it, and acks it back tosuccess— or the reverse order suppresses a delivery already on the wire. There is no compare-and-set to express "suppress only if still pending".attempts.ackincrementsattemptsunconditionally. A row cancelled this way records an attempt that never happened — andattempts === 0on a terminal row is a load-bearing predicate elsewhere on the sibling HTTP outbox (assertHttpRedeliverable,http-outbox.ts:315+, distinguishes "parked, never sent" from "sent and failed").ackis the dispatcher's completion callback. Using it as a cancel primitive is the kind of reuse that reads correct in review and is wrong under concurrency.What a fix has to decide
Whether cancellation is a new
INotificationOutboxmember (widening the contract plus both implementations, andSqlNotificationOutboxneeds a conditional update so the transition is atomic againstclaim), or whetherackgrows a status precondition and a documented cancel discriminator. Either way it is a surface decision, not a repair — which is why this is filed rather than done.Links
recall()cannot express "cancel pending/unclaimed deliveries" without this)