Skip to content

fix(messaging): classify the delivery dispatchers' updateMany sweeps as global environment sweeps - #10725

Merged
os-warren merged 2 commits into
mainfrom
claude/issue-10673-delivery-updatemany-tenant-scope
Aug 21, 2026
Merged

fix(messaging): classify the delivery dispatchers' updateMany sweeps as global environment sweeps#10725
os-warren merged 2 commits into
mainfrom
claude/issue-10673-delivery-updatemany-tenant-scope

Conversation

@os-warren

@os-warrenos-warren commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

Fixes#10673

On a walled deployment (OS_TENANCY_POSTURE=isolated|group) the SQL driver's tenant-audit gate reported both delivery objects:

[tenant-audit] updateMany on tenant-scoped object "sys_http_delivery" without options.tenantId
[tenant-audit] updateMany on tenant-scoped object "sys_notification_delivery" without options.tenantId

The card is explicit that silencing the line is not a fix — the audit exists to force a per-call-site classification. This PR does that classification and nothing else.

Call-site census

Six multi: true (⇒ driver.updateMany) writes exist on these two objects. All six are on the dispatcher claim path:

#SiteRole
1sql-outbox.tsclaim() step 1visibility-timeout reap
2sql-outbox.tsclaim() step 3atomic claim
3sql-outbox.tsclaimDigest() step 1visibility-timeout reap
4sql-outbox.tsclaimDigest() step 3atomic claim
5sql-http-outbox.tsclaim() step 1visibility-timeout reap
6sql-http-outbox.tsclaim() step 3atomic claim

The census is not a bare updateMany grep: driver.updateMany is reached through engine.update(..., { multi: true }), so the search term is multi: true scoped to the two outbox implementations, cross-checked against every engine.update in both files (the remaining three are multi: false single-record writes — ack in both outboxes and redeliver — which the driver audits under the update op, not updateMany, and which this card does not cover).

Classification: all six are global dispatcher sweeps

Not a judgement of taste — three structural facts, all checked against the tree:

  1. No request context exists to thread. The only callers are NotificationDispatcher.runPartition() and HttpDispatcher.runPartition(), driven by a setInterval tick under a cluster lock keyed notify.dispatcher.partition.N / http.dispatcher.partition.N. No HTTP request, no session, no active organization.
  2. The outbox contract has no tenant to thread even in principle.ClaimOptions / HttpClaimOptions are { nodeId, limit, partition, claimTtlMs, now }. Partitioning is hash(refId | notificationId | digestKey) mod N — a load-spreading key, deliberately not an organization key — so a partition holds rows from every organization by construction.
  3. Scoping them would break delivery, not isolate it. One outbox and one dispatcher pair are constructed per environment (messaging-service-plugin.ts) and drain the whole environment's queue. An organization_id = predicate naming a single org on the claim would strand every other organization's deliveries forever; the same predicate on the reap would leave rows a crashed node abandoned permanently in_flight for every other organization.

So each site now passes bypassTenantAudit through one documented helper, outbox-dispatcher-scope.ts, which carries that warrant in full, with a one-line pointer at each site. Per its spec the flag is diagnostics-only and "never changes what the write touches" — and the tests below pin that it changed nothing.

Verification

The card's repro (EE 4.1.0 image + composed boot + docker compose logs app) is not runnable in this container. Declared narrowing: it was not run and no composed log is claimed. What stands in its place is the instrument's own criterion — SqlDriver.auditMissingTenant — exercised end to end in delivery-claim-tenant-audit.integration.test.ts: a real SqlDriver on better-sqlite3, real syncSchemas(), the real live OS_TENANCY_POSTURE read, the production outboxes, and the driver's own logger as the assertion surface. Before the change that harness reproduces both card lines verbatim.

Each test pins three directions:

  • the classified write emits no [tenant-audit] updateMany line for its object;
  • the rows still move in every organization — both orgs' rows are claimed/reaped and their organization_id survives, so an implementation that touched nothing, or one that scoped per organization, goes red;
  • a positive control runs afterwards: a deliberately unscoped multi: true write on the same object through the same driver must still produce the audit line. Without it, a silent run would prove nothing about the code under test — it would equally match an object that stopped being tenant-scoped or a posture that stopped resolving. The control runs after the assertion it guards because the gate throttles one warning per object-and-op pair.

Ablation (signature predicted before mutating): flipping bypassTenantAudit to false in src/, with no rebuild, turns exactly the four audit assertions red with expected true to be false, and zero "positive control failed" — matching the prediction, and proving the tests read src/ rather than a built artifact. git checkout HEAD -- restored the file byte-identically (git hash-object = e8e59054e802ecd60344682d6f46d20cf8bea6c8, equal to the HEAD blob) and the suite returned to 4 passed.

Package suite at final commit 72607c0: Test Files 24 passed (24), Tests 248 passed (248); tsc --noEmit clean.

Gate union derived with node scripts/pm/dispatch-gates.mjs (no path arguments) after the final commit on a clean tree, all green at 72607c0 — including check:i18n ("OK — 9 package(s) — all bundles in sync"), check:type-check-debt --re-measure ("33 ledger entr(ies) re-measured ... none above its recorded number"), check:engine-double-contract, check:where-matcher, check:test-source-alias and check:nul-bytes. The derivation did not name check:route-envelope or check:dispatcher-error-vocabulary; both were run explicitly with --self-test and both pass. check:query-options-erasure is green, and its self-test prints packages/spec/src/migrations/registry.ts parses at --stack-size=4000 through this gate's own channel — the reported parser-stack red does not reproduce here.

Out of scope

The crm_contract half of the upstream report stays on objectstack-ai/cloud#1512 and is not addressed here; that card remains open.

@github-actionsgithub-actionsBot added size/m documentation Improvements or additions to documentation tests tooling labels Aug 21, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-messaging, touching 3 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/automation/webhooks.mdx(via in_flight (literal))
What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 45 of 221 client-bound route-ledger rows — the other 176 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 4 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 8f04d9a4ff8d50f84783a20c70b73a291be9c0c7packageMentionDocs.

Which tree this was computed on

This run read content/docs from 38981505bfa2c4b518660e0e3b17667e57c1ca54 — the merge of head 72607c032fcfddec9eac469e984ed01964455e11 into base 8f04d9a4ff8d50f84783a20c70b73a291be9c0c7, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 38981505bfa2c4b518660e0e3b17667e57c1ca54 && git checkout 38981505bfa2c4b518660e0e3b17667e57c1ca54
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 8f04d9a4ff8d50f84783a20c70b73a291be9c0c7 72607c032fcfddec9eac469e984ed01964455e11 && git checkout -B drift-repro 8f04d9a4ff8d50f84783a20c70b73a291be9c0c7 && git merge --no-ff 72607c032fcfddec9eac469e984ed01964455e11
node scripts/docs-audit/affected-docs.mjs --json 8f04d9a4ff8d50f84783a20c70b73a291be9c0c7

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 8f04d9a4ff8d50f84783a20c70b73a291be9c0c7 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@os-warren
os-warren marked this pull request as ready for review August 21, 2026 10:06
@os-warren
os-warren added this pull request to the merge queueAug 21, 2026
Merged via the queue into main with commit 8163a1cAug 21, 2026
35 checks passed
@os-warren
os-warren deleted the claude/issue-10673-delivery-updatemany-tenant-scope branch August 21, 2026 10:22
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

1 participant

@os-warren