fix(service-messaging): an HTTP ack binds the claim credential, so a reaped claim's late ack cannot overwrite the live re-claim

.changeset/17634-http-ack-claim-credential.md

@@ -0,0 +1,7 @@
+---
+'@objectstack/service-messaging': minor
+---
+
+`IHttpOutbox.ack()` takes an optional third argument, the claim credential, and `HttpDispatcher` now always passes it (#17634). A late ack from a claim the visibility-timeout reap had taken back — a send that outran `claimTtlMs` while another dispatcher re-claimed the row — used to write its outcome by row id over that dispatcher's live attempt: a delivery still in progress could be marked `dead`, or one attempt's outcome overwrite another's. Handed the credential, `SqlHttpOutbox` and `MemoryHttpOutbox` perform the compare-and-set `INotificationOutbox.ack()` has performed since #11859: the outcome is written only while the row is still `in_flight` under the same (`claimedBy`, `claimedAt`) pair `claim()` stamped on it. A lost claim writes nothing and throws the new `HttpAckError` (`DELIVERY_NOT_ELIGIBLE`, the code this package already raises for a delivery row in the wrong state); the dispatcher logs `http-dispatcher: ack refused, claim no longer held`, carries on with the rest of its batch, and whoever holds the row re-drives the delivery.
+
+Nothing written against the two-argument `ack(id, result)` has to change. An `IHttpOutbox` implementation that does not read the third argument compiles and works as before, and a caller that does not pass it gets the by-id write it always got — that arity is deprecated, because it checks no ownership. New exports: `HttpClaimCredential` and `HttpAckError`. A subclass that overrides a built-in store's `ack()` should forward the third argument to `super.ack()`, or its dispatcher acks keep the old unchecked write.

content/docs/automation/webhooks.mdx

@@ -622,6 +622,7 @@ A precise table of what the runtime promises and what it does not.
 | Producer node crashes mid-emit           | **Not durable today.** The realtime bus (`InMemoryRealtimeAdapter`) is an unpersisted, in-process pub/sub — an event lost before Stage 3's INSERT is gone, not redelivered (see §4.1). |
 | Subscriber node crashes after persist    | Row exists in `sys_http_delivery`, another node picks it up. |
 | Dispatcher node crashes mid-HTTP         | Row stays `in_flight` with `claimed_by`; the first dispatcher tick after the claim TTL reverts it to `pending` and it is re-posted. The TTL derives from the dispatcher tick (`intervalMs`, default 500ms): `lockTtlMs = 5 × intervalMs`, `claimTtlMs = 2 × lockTtlMs` (so ~5s at defaults), all configurable via `HttpDispatcherOptions`. An idle surviving dispatcher ticks at least every `maxIdleIntervalMs` (default 30s), so recovery takes at most `claimTtlMs + maxIdleIntervalMs` (~35s at defaults). |
+| A send outlasts the claim TTL and another dispatcher re-claims the row | The re-claiming dispatcher posts the delivery again (at-least-once — the receiver can see it twice; see the last row). The first dispatcher's late outcome is **not** written over the live attempt: its ack carries the claim credential (`claimed_by`, `claimed_at`) its claim stamped, the conditional update matches nothing, and it logs `http-dispatcher: ack refused, claim no longer held`. The row records the outcome of the attempt that still holds it. |
 | Receiver returns 5xx                     | Retry per backoff schedule until the fixed 8-attempt budget is exhausted (§4.5). |
 | Receiver returns 4xx                     | Treated as terminal — no retry, status `dead` immediately. Exception: 408 / 429 are retried. |
 | Receiver returns 2xx                     | `status = success`, no more attempts.                       |

content/docs/permissions/tenant-audit-census.mdx

@@ -98,7 +98,7 @@ are reported as `undecidable` rather than assumed either way.
 
 The same holds twice over for the context. An options argument spelled as a
 literal can be read; one spelled `options`, `{ ...opts }`, or handed through a
-forwarding shim cannot, and **66 of the 222 sites are spelled that way**. A
+forwarding shim cannot, and **67 of the 223 sites are spelled that way**. A
 context resolved from an inline literal or a local `const` can be tested for
 `isSystem`; one arriving from a helper call cannot.
 
@@ -126,8 +126,8 @@ now **0**: nothing on this surface threads a context that provably lacks the fla
 
 **"No tenant context" counted sites it had not read.** An options argument the
 walker could not parse was folded into the same bucket as one it had read and
-found empty. That published **83 sites "carrying no tenant context at all"**
-when 17 said so and 66 were simply unread — an over-claim in the *alarming*
+found empty. That published **84 sites "carrying no tenant context at all"**
+when 17 said so and 67 were simply unread — an over-claim in the *alarming*
 direction, on the very figure this page tells other cards to cite. `carries` is
 now three-valued, and an unreadable argument can never contribute to the
 provable count.
@@ -147,10 +147,10 @@ reproduce them. Where it disagrees, it disagrees on the page:
 
 | carried figure | where it survives | this census |
 | :--- | :--- | ---: |
-| 175 write call sites | quoted in the merged changeset | **222** |
+| 175 write call sites | quoted in the merged changeset | **223** |
 | 24 carrying no tenant context | quoted in the merged changeset | **9** provable and tenancy-enabled; **32** more whose options argument is unreadable |
-| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **149 of 222** decidable, **73** undecidable |
-| 135 (77%) silenced by the `isSystem` guard before the posture gate | the lost issue body — **no surviving corroboration** | **not reproduced**: 105 decidably elevated, 0 decidably not, 100 undecidable |
+| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **149 of 223** decidable, **74** undecidable |
+| 135 (77%) silenced by the `isSystem` guard before the posture gate | the lost issue body — **no surviving corroboration** | **not reproduced**: 105 decidably elevated, 0 decidably not, 101 undecidable |
 | 141 and 132, two independent re-derivations | the card that filed this work | — |
 
 **The differences are not reconciled, and deliberately so.** The old census's
@@ -167,11 +167,11 @@ would report a smaller number and would not say so.
 
 The fourth row is the one worth flagging to anyone citing it. **The 135 / 77%
 figure has no surviving corroboration anywhere in the tree.** This census reads
-105 of 222 (47%) as decidably elevated, with 100 more whose elevation is a
+105 of 223 (47%) as decidably elevated, with 101 more whose elevation is a
 run-time fact — so the claim is neither confirmed nor refuted, and the honest
 answer is that a static reading cannot settle it.
 
-⇒ **Cite `9 / 222`, and say what it is**: the sites whose options argument was
+⇒ **Cite `9 / 223`, and say what it is**: the sites whose options argument was
 READ and holds no tenant context, against a decidably tenancy-enabled object.
 That is the control's provable yield surface. ⛔ Do not cite it as "the sites
 without tenant context" — **32 further sites** have an options argument this
@@ -183,31 +183,31 @@ cannot read, and they are neither in nor out.
 
 | what | count |
 | :--- | ---: |
-| write call sites on the application surface | **222** |
+| write call sites on the application surface | **223** |
 | …whose object name is statically decidable | 149 |
-| …whose object name is chosen at run time | 73 |
+| …whose object name is chosen at run time | 74 |
 | …against an object with tenancy ENABLED | 149 |
 | …against an object that declares tenancy off | 0 |
 | threading a tenant context | 139 |
 | PROVABLY carrying none (options read, no context key) | **17** |
 | …of those, against a decidably tenancy-enabled object | **9** |
-| options argument UNREADABLE — may or may not carry one | 66 |
+| options argument UNREADABLE — may or may not carry one | 67 |
 | …of those, against a decidably tenancy-enabled object | 32 |
 | threading a decidably ELEVATED (`isSystem`) context | 105 |
 | threading a context that is decidably NOT elevated | 0 |
-| threading a context whose elevation is a run-time fact | 100 |
+| threading a context whose elevation is a run-time fact | 101 |
 
 | how the instrument reached the site | count |
 | :--- | ---: |
-| receiver carried a readable engine type | 177 |
+| receiver carried a readable engine type | 178 |
 | receiver erased, placed by the object NAME | 19 |
 | receiver erased, placed by an `object: string` PARAMETER | 15 |
 | receiver erased, placed by an `UNTYPED_RECEIVERS` row | 11 |
 
 | object name spelled inline | 109 |
 | object name spelled through a `const` | 40 |
 | object name is an `object: string` parameter | 19 |
-| object name is some other run-time expression | 54 |
+| object name is some other run-time expression | 55 |
 
 The corpus walked is every tracked non-test source under `packages/services/`
 and `packages/plugins/`; calls to a same-named method on something that is not
@@ -224,11 +224,11 @@ holds still. They are required to be HERE and to say WHEN they were true;
 their values are not compared. The reasoning, and the measurement behind it,
 are in `scripts/check-tenant-audit-census.mjs`.
 
-Measured on 2026-09-11 at `f368fd0aa`.
+Measured on 2026-09-11 at `06215b074`.
 
 | corpus scale (not enforced) | count |
 | :--- | ---: |
-| tracked non-test sources scanned | 563 |
+| tracked non-test sources scanned | 564 |
 | engine-shaped types recognised | 59 |
 | declared objects in the registry | 300 |
 | same-named calls subtracted as non-engine | 137 |

docs/audits/2026-08-tenant-audit-write-call-sites.counts.md

@@ -29,19 +29,19 @@ silent, and `node scripts/tenant-audit-census.mjs --write` is the resolution.
 
 | Measure | Value |
 |---|---:|
-| Write call sites | 222 |
+| Write call sites | 223 |
 | Object name statically decidable | 149 |
-| Object name chosen at run time | 73 |
+| Object name chosen at run time | 74 |
 | Against a tenancy-enabled object | 149 |
 | Against an object declaring tenancy off | 0 |
 | Threading a tenant context | 139 |
 | Provably carrying none | 17 |
 | …and decidably tenancy-enabled | 9 |
-| Options argument unreadable | 66 |
+| Options argument unreadable | 67 |
 | …and decidably tenancy-enabled | 32 |
 | Threading a decidably elevated context | 105 |
 | Threading a decidably non-elevated context | 0 |
-| Threading a context of undecidable elevation | 100 |
+| Threading a context of undecidable elevation | 101 |
 
 ## Corpus scale — present and dated, ⛔ NOT enforced
 
@@ -52,11 +52,11 @@ holds still. They are required to be HERE and to say WHEN they were true;
 their values are not compared. The reasoning, and the measurement behind it,
 are in `scripts/check-tenant-audit-census.mjs`.
 
-Measured on 2026-09-11 at `f368fd0aa`.
+Measured on 2026-09-11 at `06215b074`.
 
 | corpus scale (not enforced) | count |
 | :--- | ---: |
-| tracked non-test sources scanned | 563 |
+| tracked non-test sources scanned | 564 |
 | engine-shaped types recognised | 59 |
 | declared objects in the registry | 300 |
 | same-named calls subtracted as non-engine | 137 |
@@ -179,7 +179,7 @@ Measured on 2026-09-11 at `f368fd0aa`.
 | `packages/services/service-messaging/src/messaging-service.ts` | `update` | `RECEIPT_OBJECT` | undecidable | options unreadable | 1 |
 | `packages/services/service-messaging/src/messaging-service.ts` | `insert` | `sys_notification` | enabled | options unreadable | 1 |
 | `packages/services/service-messaging/src/sql-http-outbox.ts` | `insert` | `this.objectName` | undecidable | options unreadable | 1 |
-| `packages/services/service-messaging/src/sql-http-outbox.ts` | `update` | `this.objectName` | undecidable | options unreadable | 4 |
+| `packages/services/service-messaging/src/sql-http-outbox.ts` | `update` | `this.objectName` | undecidable | options unreadable | 5 |
 | `packages/services/service-messaging/src/sql-outbox.ts` | `insert` | `this.objectName` | undecidable | options unreadable | 1 |
 | `packages/services/service-messaging/src/sql-outbox.ts` | `update` | `this.objectName` | undecidable | options unreadable | 4 |
 | `packages/services/service-queue/src/db-queue-adapter.ts` | `delete` | `sys_job_queue` | enabled | context, elevation undecidable | 2 |

packages/services/service-messaging/src/delivery-update-tenant-audit.integration.test.ts

@@ -25,10 +25,15 @@
  * `updateMany` and its spy below records `SqlDriver.updateMany`. Its
  * CLASSIFICATION is unchanged — declared global, now via
  * `dispatcherAckCasOptions` — which is the point of pinning the two
- * separately: the op moved, the warrant did not. Of the three sites only
- * `SqlHttpOutbox.ack` still writes by id.
+ * separately: the op moved, the warrant did not.
  *
- * The `ack` pair is declared global (`dispatcherAckOptions`, warrant in
+ * [#17634] `SqlHttpOutbox.ack` has since made the same move: handed the claim
+ * credential — which `HttpDispatcher` always hands it — its ownership test is
+ * a compare-and-set, so its op is `updateMany`, via `dispatcherAckCasOptions`,
+ * with the classification unchanged. Only its deprecated credential-less arity
+ * still writes by id, and no dispatcher tick reaches it.
+ *
+ * The `ack` pair is declared global (`dispatcherAckCasOptions`, warrant in
  * `outbox-dispatcher-scope.ts`). `redeliver` is NOT: it is served to any
  * authenticated user, so it threads the caller's tenant instead. ⛔ A
  * `bypassTenantAudit` on that third site would convert a detectable hole into
@@ -56,9 +61,9 @@
  * ## The vacuity traps closed explicitly
  *  1. **"the audit was never armed."** Every silence assertion is followed by
  *     a positive control on the SAME object through the SAME driver: an
- *     unscoped by-id `update` that MUST produce the line. The gate throttles
- *     one warning per `${object}:${op}`, so the control runs last and only
- *     fires if the production path consumed no `update` warning of its own.
+ *     unscoped write on the SAME op that MUST produce the line. The gate
+ *     throttles one warning per `${object}:${op}`, so the control runs last and
+ *     only fires if the production path consumed no warning on that op itself.
  *  2. **"a fix that touches nothing."** Row state is pinned after every write.
  *  3. **"a refusal that refuses everything."** The cross-tenant refusal is
  *     paired with a still-works leg: an in-tenant redeliver still succeeds.
@@ -84,13 +89,9 @@ let driver: SqlDriver;
 let warns: Array<{ msg: string; meta: any }>;
 /** Every `options` bag that reached `SqlDriver.update` — the `update` op only. */
 let driverUpdates: Array<{ object: string; id: unknown; options: any }>;
-/** Every `options` bag that reached `SqlDriver.updateMany` — `redeliver`'s op since #11009, and the notification `ack`'s since #11453. */
+/** Every `options` bag that reached `SqlDriver.updateMany` — `redeliver`'s op since #11009, the notification `ack`'s since #11453, and the HTTP `ack`'s since #17634. */
 let driverUpdateManys: Array<{ object: string; where: unknown; options: any }>;
 
-/** The audit line for the SINGLE-RECORD op, matched on object + op. */
-const auditedUpdate = (object: string): boolean =>
-    warns.some((w) => w.msg.includes(`[tenant-audit] update on tenant-scoped object "${object}"`));
-
 /** The audit line for the PREDICATE op — `redeliver`'s write since #11009, the notification `ack`'s since #11453. */
 const auditedUpdateMany = (object: string): boolean =>
     warns.some((w) => w.msg.includes(`[tenant-audit] updateMany on tenant-scoped object "${object}"`));
@@ -145,30 +146,6 @@ afterEach(async () => {
     else process.env.OS_TENANT_AUDIT = OLD_AUDIT;
 });
 
-/**
- * The positive control for the `update` op. A by-id write with no `tenantId`
- * and no bypass MUST produce the audit line on `object`, or this file cannot
- * tell "classified" from "the gate was never armed". Run AFTER the assertion
- * it guards — the gate throttles one warning per `${object}:${op}`.
- */
-async function controlUnscopedUpdate(object: string, existingId: string): Promise<void> {
-    // `where: { id }` with a scalar id routes through `driver.update`
-    // (`resolveEngineUpdateDispatch` → `by-id`), exactly as the production
-    // paths under test do.
-    //
-    // ⚠️ It must name a row that EXISTS. The engine's by-id branch raises
-    // `Record <id> not found` before it ever reaches the driver, so a control
-    // pointed at a missing id never arms the gate it is meant to prove is
-    // armed — it fails as an error rather than reporting a vacuous suite,
-    // which is the only reason that mistake was visible here.
-    await engine.update(object, { attempts: 99 }, { where: { id: existingId } } as any);
-    expect(
-        auditedUpdate(object),
-        `positive control failed: an unscoped by-id update on ${object} produced no [tenant-audit] `
-            + 'line, so every "no finding" assertion in this file is vacuous',
-    ).toBe(true);
-}
-
 /**
  * The positive control for the `updateMany` op — `redeliver`'s op since
  * #11009. An unscoped predicate write with no bypass MUST produce the
@@ -221,7 +198,7 @@ async function seedDeadRow(id: string, org: string): Promise<void> {
 }
 
 // ───────────────────────────────────────────────────────────────────────────
-describe('ack — the two dispatcher sites are a classified global sweep (update + updateMany ops)', () => {
+describe('ack — the two dispatcher sites are a classified global sweep (updateMany op)', () => {
     it('SqlHttpOutbox.ack records a REAL delivery in every organization, without a finding', async () => {
         // The gate's own precondition: this object really is tenant-scoped.
         expect((driver as any).resolveTenantField(SYS_HTTP_DELIVERY)).toBe('organization_id');
@@ -246,14 +223,30 @@ describe('ack — the two dispatcher sites are a classified global sweep (update
         ]);
         // ② Both organizations' rows were acked by one dispatcher — the
         // cross-organization reach is the operation's semantics.
-        const ackWrites = driverUpdates.filter((u) => u.object === SYS_HTTP_DELIVERY);
-        expect(ackWrites.map((u) => u.id).sort()).toEqual(['h_a', 'h_b']);
+        //
+        // [#17634] The dispatcher's ack hands the claim credential, so it is a
+        // compare-and-set on the predicate path and the reading moves to the
+        // `updateMany` spy — the move the notification ack made in #11453. The
+        // claim path writes there too (its reap and its atomic claim), so the
+        // filter names what an ACK write looks like: a scalar id bound to
+        // `in_flight` AND to the claiming node. That predicate IS the
+        // compare-and-set, so matching on it pins that the ack reached the
+        // driver CONDITIONAL rather than as a blind by-id write.
+        const ackWrites = driverUpdateManys.filter(
+            (u) => u.object === SYS_HTTP_DELIVERY
+                && typeof (u.where as any)?.id === 'string'
+                && (u.where as any)?.status === 'in_flight'
+                && (u.where as any)?.claimed_by === 'n1',
+        );
+        expect(ackWrites.map((u) => (u.where as any).id).sort()).toEqual(['h_a', 'h_b']);
         // ③ …under the DECLARED classification, not an accidental silence.
         expect(ackWrites.every((u) => u.options?.bypassTenantAudit === true)).toBe(true);
         expect(ackWrites.every((u) => u.options?.tenantId === undefined)).toBe(true);
-        expect(auditedUpdate(SYS_HTTP_DELIVERY)).toBe(false);
+        // …and no by-id write reached the driver: the dispatcher never takes the deprecated arity.
+        expect(driverUpdates.filter((u) => u.object === SYS_HTTP_DELIVERY)).toEqual([]);
+        expect(auditedUpdateMany(SYS_HTTP_DELIVERY)).toBe(false);
 
-        await controlUnscopedUpdate(SYS_HTTP_DELIVERY, 'h_a');
+        await controlUnscopedUpdateMany(SYS_HTTP_DELIVERY, 'h_a');
     });
 
     it('SqlNotificationOutbox.ack records a REAL delivery in every organization, without a finding', async () => {