[HiroshiYoshino]

Related issue: #1427
Related docs: fullstackhero/docs#268

Problem

An integration event is stored in one database (the Outbox), but its consumers can
need another. For a tenant with a dedicated database, InMemoryEventBus installed an
ID-only tenant context before resolving consumers, which dropped the dedicated
connection string. Consumers, tenant-aware DbContexts and Inbox rows then went to the
shared default database, and webhook subscriptions in the dedicated database were missed.

Fix

InMemoryEventBus now runs its handler loop inside the new IEventTenantScope.ExecuteAsync,
so tenant resolution and consumer construction share one async flow (DbContexts pick their
database at construction).

• IEventTenantScope.ExecuteAsync(tenantId, action, ct) has a default implementation over
  Begin, so custom scopes keep working.
• FinbuckleEventTenantScope.ExecuteAsync:
  • a null/empty tenant ID leaves the context untouched;
  • an ambient context that already has the same tenant ID and a connection string is reused;
  • otherwise the tenant is read from the cached IMultiTenantStore<AppTenantInfo>, falling
    back to the tenant catalog (EF store) on a miss and writing the result back to the cache;
  • a missing or inactive tenant logs a structured warning and dispatches with the ID-only
    context, as before;
  • the previous context is restored in finally.
• WebhookFanoutHandler keeps a complete context the bus already resolved; otherwise it
  installs the ID-only context as before and restores it in finally.
• Unchanged: Begin, FinbuckleEventingDrainScope, .agents/rules.

FinbuckleEventTenantScope has a single constructor.

Tests

• DedicatedTenantRoutingTests (integration, Testcontainers): dedicated-database dispatch (the
  consumer row and the Inbox row land in the tenant's database, the default has neither),
  webhook subscription lookup for a dedicated tenant, and context restored after success and
  after failure.
• Existing FinbuckleEventTenantScopeTests only adapt to the constructor.

Local run: Multitenancy.Tests 95, Webhooks.Tests 71, Architecture.Tests 64 and
DedicatedTenantRoutingTests 4, all passing.

Behaviour change

Consumers and Inbox rows now follow the event tenant's database. Outbox claim, completion and
retry updates stay in the source database. No migration or configuration change. Historical
misrouted rows are not repaired.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-14T11:15:52.080093Z	549931f	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 549931f37f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Obtain approval before changing BuildingBlocks

When no explicit approval accompanies this change, do not land the new shared interface method: the repository's AGENTS.md marks all of src/BuildingBlocks as protected and prohibits modifications without explicit approval, while this commit changes both the public eventing abstraction and the framework event bus. Record the approval or move the behavior behind a module-owned adapter before merging.

AGENTS.md reference: AGENTS.md:L82-L82

Useful? React with 👍 / 👎.

[iammukeshm]

Thanks @HiroshiYoshino, and sorry this sat for a while. The core of the fix is right. Running the consumer inside a callback-shaped ExecuteAsync, so resolution and consumer construction happen in the same async flow, is exactly the shape I wanted, and restoring the context in finally is sound. It merges cleanly with main, builds with zero warnings, and the unit suites pass.

The problem is that the PR was written before the constraints I posted on #1427, and it breaks most of them. These are the changes I need before it can go in:

1. No database query per event. EventTenantResolver runs a TenantDbContext query inside a new DI scope on every dispatch (EventTenantResolver.cs:11, FinbuckleEventTenantScope.cs:86-89). Please resolve through IMultiTenantStore<AppTenantInfo>, which is cached, and drop IEventTenantResolver, EventTenantResolver and the scope-per-event.
2. Reuse a context that's already complete. ExecuteAsync always overwrites the context and then does the lookup. If the ambient TenantInfo already has the same ID and a non-empty connection string (an in-request publish), just run the action.
3. Global events stay as they are. FinbuckleEventTenantScope.cs:73-77 installs a synthetic root tenant when tenantId is null. That changes the query filter global-event handlers see. For a null or empty tenant ID, leave the context alone like Begin does today.
4. The default drain pass stays a no-op. FinbuckleEventingDrainScope.cs:37 removes the no-op default pass and installs root with an empty connection string. Please restore the NoopScope path for TenantId is null && ConnectionString is null.
5. Unknown or inactive tenants don't throw. Lines 90 and 94 throw, which turns these events into retries and dead-letters. That's a policy decision I want to make separately. For now, log a structured warning and dispatch with the ID-only context as before.
6. WebhookFanoutHandler never throws on a mismatch (WebhookFanoutHandler.cs:57-60). Keep the context if it's already complete for this tenant, otherwise install the ID-only context as before and restore it in finally. The bus owns the context.
7. Trim the tests. The three new files add about 1,400 lines, and many cases test the behaviour above that we're not taking (retry then dead-letter, the synthetic root, the default-drain reset). I only need three things: dedicated-DB dispatch (the consumer row and the Inbox row land in the tenant's DB, default has neither), the webhook subscription lookup for a dedicated tenant, and the context restored after success and after failure. One file is enough.
8. Drop the .agents/rules edits. They document the behaviour we're not taking and mention an internal issue. One line on the final behaviour is fine if you want it.

Two smaller ones: please use a single constructor on FinbuckleEventTenantScope (the second one without IServiceScopeFactory makes ExecuteAsync throw at runtime), and leave Begin unchanged since it no longer has production callers.

On the docs PR (fullstackhero/docs#268): it currently describes the behaviour above, so it'll need a rewrite once the code changes. Please remove the "Proposed fix" callouts and the link to the fork commit, and keep the changelog to one dated bullet that links this PR, like the other entries. The new "Delivery status and recovery" section is beyond this fix, so let's leave it out.

The real change here is small: about 30 lines in FinbuckleEventTenantScope (a lookup through the cached store, a fast path for a complete context, warn and fall back), plus reverting the drain-scope and webhook changes. Once that's in, I'll merge it quickly. Thanks again for chasing this down; it's a real bug for anyone using dedicated databases.

[HiroshiYoshino]

@iammukeshm
Thanks for the detailed review. And sorry for causing you trouble because I didn’t understand properly.
I reworked the branch (one commit on current main) and rewrote the description to match.

1. Cached IMultiTenantStore: IEventTenantResolver / EventTenantResolver are gone. The lookup reads the cache store first and queries the tenant catalog only on a miss, then writes the tenant back to the cache. One deviation: a plain IMultiTenantStore<AppTenantInfo> resolve returns the last registration (the scoped EF store), so FinbuckleEventTenantScope opens a short DI scope on every event and enumerates the stores with GetServices to reach the cache store. A cache hit does no database query. If you would rather avoid even that scope, the alternative is to create it only on a cache miss; say so and I will switch.
2. A complete ambient context (same tenant ID, non-empty connection string) is reused.
3. A null or empty tenant ID leaves the context alone.
4. FinbuckleEventingDrainScope is back to main.
5. A missing or inactive tenant logs a structured warning and dispatches with the ID-only context.
6. WebhookFanoutHandler keeps a complete context, otherwise installs the ID-only one and restores it in finally.
7. Tests are down to the three you listed, in one new file (DedicatedTenantRoutingTests); the existing scope tests only adapt to the constructor.
8. .agents/rules edits are dropped.
9. Single constructor; Begin is unchanged.

Regarding the Codex note on src/BuildingBlocks: I read your comment about the ExecuteAsync shape as the go-ahead; please say if you want it recorded differently.

Docs: fullstackhero/docs#268 is updated too (one dated changelog bullet, no "Proposed fix" callouts, no recovery section).