Skip to content

Commit 4be4e04

Browse files
hotlongclaude
andauthored
fix(service-messaging): an HTTP ack binds the claim credential, so a reaped claim's late ack cannot overwrite the live re-claim (#17641)
* wip(service-messaging): reproduce the late HTTP ack overwriting a live re-claim Claude-Session: https://claude.ai/code/session_c5c0ce54-bb9c-478c-9e5b-cf44b80d4569 Co-authored-by: Claude <noreply@anthropic.com> * fix(service-messaging): an HTTP ack binds the claim credential, so a reaped claim's late ack cannot overwrite the live re-claim Claude-Session: https://claude.ai/code/session_c5c0ce54-bb9c-478c-9e5b-cf44b80d4569 Co-authored-by: Claude <noreply@anthropic.com> * docs(permissions): tenant-audit census counts the credentialed HTTP ack write (223 sites) Claude-Session: https://claude.ai/code/session_c5c0ce54-bb9c-478c-9e5b-cf44b80d4569 Co-authored-by: Claude <noreply@anthropic.com> * fix(service-messaging): keep tracker ids out of the HTTP ack refusal strings Claude-Session: https://claude.ai/code/session_c5c0ce54-bb9c-478c-9e5b-cf44b80d4569 Co-authored-by: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 1c4270f commit 4be4e04

13 files changed

Lines changed: 829 additions & 116 deletions
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
'@objectstack/service-messaging': minor
3+
---
4+
5+
`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.
6+
7+
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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -622,6 +622,7 @@ A precise table of what the runtime promises and what it does not.
622622
| 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). |
623623
| Subscriber node crashes after persist | Row exists in `sys_http_delivery`, another node picks it up. |
624624
| 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). |
625+
| 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. |
625626
| Receiver returns 5xx | Retry per backoff schedule until the fixed 8-attempt budget is exhausted (§4.5). |
626627
| Receiver returns 4xx | Treated as terminal — no retry, status `dead` immediately. Exception: 408 / 429 are retried. |
627628
| Receiver returns 2xx | `status = success`, no more attempts. |

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

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ are reported as `undecidable` rather than assumed either way.
9898

9999
The same holds twice over for the context. An options argument spelled as a
100100
literal can be read; one spelled `options`, `{ ...opts }`, or handed through a
101-
forwarding shim cannot, and **66 of the 222 sites are spelled that way**. A
101+
forwarding shim cannot, and **67 of the 223 sites are spelled that way**. A
102102
context resolved from an inline literal or a local `const` can be tested for
103103
`isSystem`; one arriving from a helper call cannot.
104104

@@ -126,8 +126,8 @@ now **0**: nothing on this surface threads a context that provably lacks the fla
126126

127127
**"No tenant context" counted sites it had not read.** An options argument the
128128
walker could not parse was folded into the same bucket as one it had read and
129-
found empty. That published **83 sites "carrying no tenant context at all"**
130-
when 17 said so and 66 were simply unread — an over-claim in the *alarming*
129+
found empty. That published **84 sites "carrying no tenant context at all"**
130+
when 17 said so and 67 were simply unread — an over-claim in the *alarming*
131131
direction, on the very figure this page tells other cards to cite. `carries` is
132132
now three-valued, and an unreadable argument can never contribute to the
133133
provable count.
@@ -147,10 +147,10 @@ reproduce them. Where it disagrees, it disagrees on the page:
147147

148148
| carried figure | where it survives | this census |
149149
| :--- | :--- | ---: |
150-
| 175 write call sites | quoted in the merged changeset | **222** |
150+
| 175 write call sites | quoted in the merged changeset | **223** |
151151
| 24 carrying no tenant context | quoted in the merged changeset | **9** provable and tenancy-enabled; **32** more whose options argument is unreadable |
152-
| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **149 of 222** decidable, **73** undecidable |
153-
| 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 |
152+
| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **149 of 223** decidable, **74** undecidable |
153+
| 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 |
154154
| 141 and 132, two independent re-derivations | the card that filed this work ||
155155

156156
**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.
167167

168168
The fourth row is the one worth flagging to anyone citing it. **The 135 / 77%
169169
figure has no surviving corroboration anywhere in the tree.** This census reads
170-
105 of 222 (47%) as decidably elevated, with 100 more whose elevation is a
170+
105 of 223 (47%) as decidably elevated, with 101 more whose elevation is a
171171
run-time fact — so the claim is neither confirmed nor refuted, and the honest
172172
answer is that a static reading cannot settle it.
173173

174-
**Cite `9 / 222`, and say what it is**: the sites whose options argument was
174+
**Cite `9 / 223`, and say what it is**: the sites whose options argument was
175175
READ and holds no tenant context, against a decidably tenancy-enabled object.
176176
That is the control's provable yield surface. ⛔ Do not cite it as "the sites
177177
without tenant context" — **32 further sites** have an options argument this
@@ -183,31 +183,31 @@ cannot read, and they are neither in nor out.
183183

184184
| what | count |
185185
| :--- | ---: |
186-
| write call sites on the application surface | **222** |
186+
| write call sites on the application surface | **223** |
187187
| …whose object name is statically decidable | 149 |
188-
| …whose object name is chosen at run time | 73 |
188+
| …whose object name is chosen at run time | 74 |
189189
| …against an object with tenancy ENABLED | 149 |
190190
| …against an object that declares tenancy off | 0 |
191191
| threading a tenant context | 139 |
192192
| PROVABLY carrying none (options read, no context key) | **17** |
193193
| …of those, against a decidably tenancy-enabled object | **9** |
194-
| options argument UNREADABLE — may or may not carry one | 66 |
194+
| options argument UNREADABLE — may or may not carry one | 67 |
195195
| …of those, against a decidably tenancy-enabled object | 32 |
196196
| threading a decidably ELEVATED (`isSystem`) context | 105 |
197197
| threading a context that is decidably NOT elevated | 0 |
198-
| threading a context whose elevation is a run-time fact | 100 |
198+
| threading a context whose elevation is a run-time fact | 101 |
199199

200200
| how the instrument reached the site | count |
201201
| :--- | ---: |
202-
| receiver carried a readable engine type | 177 |
202+
| receiver carried a readable engine type | 178 |
203203
| receiver erased, placed by the object NAME | 19 |
204204
| receiver erased, placed by an `object: string` PARAMETER | 15 |
205205
| receiver erased, placed by an `UNTYPED_RECEIVERS` row | 11 |
206206

207207
| object name spelled inline | 109 |
208208
| object name spelled through a `const` | 40 |
209209
| object name is an `object: string` parameter | 19 |
210-
| object name is some other run-time expression | 54 |
210+
| object name is some other run-time expression | 55 |
211211

212212
The corpus walked is every tracked non-test source under `packages/services/`
213213
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;
224224
their values are not compared. The reasoning, and the measurement behind it,
225225
are in `scripts/check-tenant-audit-census.mjs`.
226226

227-
Measured on 2026-09-11 at `f368fd0aa`.
227+
Measured on 2026-09-11 at `06215b074`.
228228

229229
| corpus scale (not enforced) | count |
230230
| :--- | ---: |
231-
| tracked non-test sources scanned | 563 |
231+
| tracked non-test sources scanned | 564 |
232232
| engine-shaped types recognised | 59 |
233233
| declared objects in the registry | 300 |
234234
| same-named calls subtracted as non-engine | 137 |

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,19 +29,19 @@ silent, and `node scripts/tenant-audit-census.mjs --write` is the resolution.
2929

3030
| Measure | Value |
3131
|---|---:|
32-
| Write call sites | 222 |
32+
| Write call sites | 223 |
3333
| Object name statically decidable | 149 |
34-
| Object name chosen at run time | 73 |
34+
| Object name chosen at run time | 74 |
3535
| Against a tenancy-enabled object | 149 |
3636
| Against an object declaring tenancy off | 0 |
3737
| Threading a tenant context | 139 |
3838
| Provably carrying none | 17 |
3939
| …and decidably tenancy-enabled | 9 |
40-
| Options argument unreadable | 66 |
40+
| Options argument unreadable | 67 |
4141
| …and decidably tenancy-enabled | 32 |
4242
| Threading a decidably elevated context | 105 |
4343
| Threading a decidably non-elevated context | 0 |
44-
| Threading a context of undecidable elevation | 100 |
44+
| Threading a context of undecidable elevation | 101 |
4545

4646
## Corpus scale — present and dated, ⛔ NOT enforced
4747

@@ -52,11 +52,11 @@ holds still. They are required to be HERE and to say WHEN they were true;
5252
their values are not compared. The reasoning, and the measurement behind it,
5353
are in `scripts/check-tenant-audit-census.mjs`.
5454

55-
Measured on 2026-09-11 at `f368fd0aa`.
55+
Measured on 2026-09-11 at `06215b074`.
5656

5757
| corpus scale (not enforced) | count |
5858
| :--- | ---: |
59-
| tracked non-test sources scanned | 563 |
59+
| tracked non-test sources scanned | 564 |
6060
| engine-shaped types recognised | 59 |
6161
| declared objects in the registry | 300 |
6262
| same-named calls subtracted as non-engine | 137 |
@@ -179,7 +179,7 @@ Measured on 2026-09-11 at `f368fd0aa`.
179179
| `packages/services/service-messaging/src/messaging-service.ts` | `update` | `RECEIPT_OBJECT` | undecidable | options unreadable | 1 |
180180
| `packages/services/service-messaging/src/messaging-service.ts` | `insert` | `sys_notification` | enabled | options unreadable | 1 |
181181
| `packages/services/service-messaging/src/sql-http-outbox.ts` | `insert` | `this.objectName` | undecidable | options unreadable | 1 |
182-
| `packages/services/service-messaging/src/sql-http-outbox.ts` | `update` | `this.objectName` | undecidable | options unreadable | 4 |
182+
| `packages/services/service-messaging/src/sql-http-outbox.ts` | `update` | `this.objectName` | undecidable | options unreadable | 5 |
183183
| `packages/services/service-messaging/src/sql-outbox.ts` | `insert` | `this.objectName` | undecidable | options unreadable | 1 |
184184
| `packages/services/service-messaging/src/sql-outbox.ts` | `update` | `this.objectName` | undecidable | options unreadable | 4 |
185185
| `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

Lines changed: 33 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,15 @@
2525
* `updateMany` and its spy below records `SqlDriver.updateMany`. Its
2626
* CLASSIFICATION is unchanged — declared global, now via
2727
* `dispatcherAckCasOptions` — which is the point of pinning the two
28-
* separately: the op moved, the warrant did not. Of the three sites only
29-
* `SqlHttpOutbox.ack` still writes by id.
28+
* separately: the op moved, the warrant did not.
3029
*
31-
* The `ack` pair is declared global (`dispatcherAckOptions`, warrant in
30+
* [#17634] `SqlHttpOutbox.ack` has since made the same move: handed the claim
31+
* credential — which `HttpDispatcher` always hands it — its ownership test is
32+
* a compare-and-set, so its op is `updateMany`, via `dispatcherAckCasOptions`,
33+
* with the classification unchanged. Only its deprecated credential-less arity
34+
* still writes by id, and no dispatcher tick reaches it.
35+
*
36+
* The `ack` pair is declared global (`dispatcherAckCasOptions`, warrant in
3237
* `outbox-dispatcher-scope.ts`). `redeliver` is NOT: it is served to any
3338
* authenticated user, so it threads the caller's tenant instead. ⛔ A
3439
* `bypassTenantAudit` on that third site would convert a detectable hole into
@@ -56,9 +61,9 @@
5661
* ## The vacuity traps closed explicitly
5762
* 1. **"the audit was never armed."** Every silence assertion is followed by
5863
* a positive control on the SAME object through the SAME driver: an
59-
* unscoped by-id `update` that MUST produce the line. The gate throttles
60-
* one warning per `${object}:${op}`, so the control runs last and only
61-
* fires if the production path consumed no `update` warning of its own.
64+
* unscoped write on the SAME op that MUST produce the line. The gate
65+
* throttles one warning per `${object}:${op}`, so the control runs last and
66+
* only fires if the production path consumed no warning on that op itself.
6267
* 2. **"a fix that touches nothing."** Row state is pinned after every write.
6368
* 3. **"a refusal that refuses everything."** The cross-tenant refusal is
6469
* paired with a still-works leg: an in-tenant redeliver still succeeds.
@@ -84,13 +89,9 @@ let driver: SqlDriver;
8489
let warns: Array<{ msg: string; meta: any }>;
8590
/** Every `options` bag that reached `SqlDriver.update` — the `update` op only. */
8691
let driverUpdates: Array<{ object: string; id: unknown; options: any }>;
87-
/** Every `options` bag that reached `SqlDriver.updateMany` — `redeliver`'s op since #11009, and the notification `ack`'s since #11453. */
92+
/** 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. */
8893
let driverUpdateManys: Array<{ object: string; where: unknown; options: any }>;
8994

90-
/** The audit line for the SINGLE-RECORD op, matched on object + op. */
91-
const auditedUpdate = (object: string): boolean =>
92-
warns.some((w) => w.msg.includes(`[tenant-audit] update on tenant-scoped object "${object}"`));
93-
9495
/** The audit line for the PREDICATE op — `redeliver`'s write since #11009, the notification `ack`'s since #11453. */
9596
const auditedUpdateMany = (object: string): boolean =>
9697
warns.some((w) => w.msg.includes(`[tenant-audit] updateMany on tenant-scoped object "${object}"`));
@@ -145,30 +146,6 @@ afterEach(async () => {
145146
else process.env.OS_TENANT_AUDIT = OLD_AUDIT;
146147
});
147148

148-
/**
149-
* The positive control for the `update` op. A by-id write with no `tenantId`
150-
* and no bypass MUST produce the audit line on `object`, or this file cannot
151-
* tell "classified" from "the gate was never armed". Run AFTER the assertion
152-
* it guards — the gate throttles one warning per `${object}:${op}`.
153-
*/
154-
async function controlUnscopedUpdate(object: string, existingId: string): Promise<void> {
155-
// `where: { id }` with a scalar id routes through `driver.update`
156-
// (`resolveEngineUpdateDispatch` → `by-id`), exactly as the production
157-
// paths under test do.
158-
//
159-
// ⚠️ It must name a row that EXISTS. The engine's by-id branch raises
160-
// `Record <id> not found` before it ever reaches the driver, so a control
161-
// pointed at a missing id never arms the gate it is meant to prove is
162-
// armed — it fails as an error rather than reporting a vacuous suite,
163-
// which is the only reason that mistake was visible here.
164-
await engine.update(object, { attempts: 99 }, { where: { id: existingId } } as any);
165-
expect(
166-
auditedUpdate(object),
167-
`positive control failed: an unscoped by-id update on ${object} produced no [tenant-audit] `
168-
+ 'line, so every "no finding" assertion in this file is vacuous',
169-
).toBe(true);
170-
}
171-
172149
/**
173150
* The positive control for the `updateMany` op — `redeliver`'s op since
174151
* #11009. An unscoped predicate write with no bypass MUST produce the
@@ -221,7 +198,7 @@ async function seedDeadRow(id: string, org: string): Promise<void> {
221198
}
222199

223200
// ───────────────────────────────────────────────────────────────────────────
224-
describe('ack — the two dispatcher sites are a classified global sweep (update + updateMany ops)', () => {
201+
describe('ack — the two dispatcher sites are a classified global sweep (updateMany op)', () => {
225202
it('SqlHttpOutbox.ack records a REAL delivery in every organization, without a finding', async () => {
226203
// The gate's own precondition: this object really is tenant-scoped.
227204
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
246223
]);
247224
// ② Both organizations' rows were acked by one dispatcher — the
248225
// cross-organization reach is the operation's semantics.
249-
const ackWrites = driverUpdates.filter((u) => u.object === SYS_HTTP_DELIVERY);
250-
expect(ackWrites.map((u) => u.id).sort()).toEqual(['h_a', 'h_b']);
226+
//
227+
// [#17634] The dispatcher's ack hands the claim credential, so it is a
228+
// compare-and-set on the predicate path and the reading moves to the
229+
// `updateMany` spy — the move the notification ack made in #11453. The
230+
// claim path writes there too (its reap and its atomic claim), so the
231+
// filter names what an ACK write looks like: a scalar id bound to
232+
// `in_flight` AND to the claiming node. That predicate IS the
233+
// compare-and-set, so matching on it pins that the ack reached the
234+
// driver CONDITIONAL rather than as a blind by-id write.
235+
const ackWrites = driverUpdateManys.filter(
236+
(u) => u.object === SYS_HTTP_DELIVERY
237+
&& typeof (u.where as any)?.id === 'string'
238+
&& (u.where as any)?.status === 'in_flight'
239+
&& (u.where as any)?.claimed_by === 'n1',
240+
);
241+
expect(ackWrites.map((u) => (u.where as any).id).sort()).toEqual(['h_a', 'h_b']);
251242
// ③ …under the DECLARED classification, not an accidental silence.
252243
expect(ackWrites.every((u) => u.options?.bypassTenantAudit === true)).toBe(true);
253244
expect(ackWrites.every((u) => u.options?.tenantId === undefined)).toBe(true);
254-
expect(auditedUpdate(SYS_HTTP_DELIVERY)).toBe(false);
245+
// …and no by-id write reached the driver: the dispatcher never takes the deprecated arity.
246+
expect(driverUpdates.filter((u) => u.object === SYS_HTTP_DELIVERY)).toEqual([]);
247+
expect(auditedUpdateMany(SYS_HTTP_DELIVERY)).toBe(false);
255248

256-
await controlUnscopedUpdate(SYS_HTTP_DELIVERY, 'h_a');
249+
await controlUnscopedUpdateMany(SYS_HTTP_DELIVERY, 'h_a');
257250
});
258251

259252
it('SqlNotificationOutbox.ack records a REAL delivery in every organization, without a finding', async () => {

0 commit comments

Comments
 (0)