Skip to content

feat(identity): Avro events + per-module tenant provisioning (Phase 2)#93

Merged
phuongnse merged 4 commits into
mainfrom
feat/identity-phase2-pr93-followup
May 24, 2026
Merged

feat(identity): Avro events + per-module tenant provisioning (Phase 2)#93
phuongnse merged 4 commits into
mainfrom
feat/identity-phase2-pr93-followup

Conversation

@phuongnse

@phuongnse phuongnse commented May 24, 2026

Copy link
Copy Markdown
Owner

Summary

Completes the PR #92 follow-up. Replaces the central TenantSchemaProvisioner with an event-driven per-module flow: Identity raises 5 domain events (OrganizationVerified, UserDeactivated, UserReactivated, RoleAssigned, RoleRemoved); IdentityUnitOfWork maps them to Avro integration events and publishes via Wolverine outbox → Kafka with CloudEvents envelope (ADR-019). Each of DataModeling/FormBuilder/WorkflowBuilder/WorkflowEngine now owns an OrganizationVerifiedHandler in its own Infrastructure project that provisions its own tenant schema — extraction of a module is a redeploy, no gateway refactor (ADR-010).

Also wires AddGrpcClient<IdentityServiceClient> against Modules:Identity:GrpcUrl (self-loop in modulith), and adds JWKS-only validation as a documented P0 rule for consuming modules in patterns.md (never call IdentityDbContext per request).

Linked spec

Requirements & rules followed

  • Spec → code — clears the deferred line Deferred (PR #93 follow-up): Avro user/role lifecycle events, OrganizationVerified Kafka flow, gateway REST → gRPC client wiring, JWKS-only validation doc hardening from PROGRESS.md and E02 README. No new ACs added; no new deferred lines introduced (existing F01 US-003 retry/UI deferred lines preserved).
  • Gate 0 — Implementation Status callouts updated in 5 epic READMEs (E01/E02/E03/E04/E05/E06).
  • Gate 1dotnet build Axis.sln (0 errors, 0 warnings); dotnet test Axis.sln (Api 94 / Identity Infra 40 / Identity App 85 / Identity Domain 68 / all other module Infra + unit tests passing); dotnet format --verify-no-changes exit 0.
  • Gate 2 — Docs in same PR: PROGRESS.md, E01-E06 READMEs, patterns.md Pattern 3 (JWKS-only). ./scripts/check-doc-drift.sh green.
  • Gate 3 — Added durable rule to patterns.md § Rules (P0): "JWT validation is JWKS-only. Consuming modules verify JWTs against Identity's JWKS endpoint locally; never call IdentityDbContext or any Identity service per request just to authenticate a user."
  • No new TODO / FIXME / NotImplementedException / placeholder / stub under src/, tests/, frontend/src/

Summary by CodeRabbit

  • New Features

    • Event-driven tenant provisioning: tenant schemas are provisioned asynchronously via Kafka/Avro events; each module now provisions its own tenant schema.
    • Cross-module gRPC: added GetUserPermissions gRPC client wiring for cross-module permission checks.
    • Identity lifecycle events: identity now emits OrganizationVerified, UserDeactivated/Reactivated, RoleAssigned/Removed events.
  • Bug Fixes / Changes

    • Removed centralized provisioning path and scheduler; modules handle provisioning independently.
  • Documentation

    • Updated docs and playbooks to reflect event-driven provisioning, schema naming, JWKS-only validation, and wiring.

Review Change Stack

phuongnse and others added 2 commits May 24, 2026 07:37
Completes the PR #92 follow-up by replacing the central
TenantSchemaProvisioner with an event-driven per-module flow:

- Identity raises 5 domain events (OrganizationVerified, UserDeactivated,
  UserReactivated, RoleAssigned, RoleRemoved). IdentityUnitOfWork maps
  them to Avro integration events and publishes via Wolverine outbox →
  Kafka with CloudEvents envelope (ADR-019).
- Each of DataModeling/FormBuilder/WorkflowBuilder/WorkflowEngine owns
  an OrganizationVerifiedHandler that provisions its own tenant schema
  (CREATE SCHEMA IF NOT EXISTS + MigrateAsync) when the event arrives.
- Removed: central TenantSchemaProvisioner, ProvisionTenantHandler,
  ProvisionTenantMessage, ITenantProvisioningScheduler,
  WolverineTenantProvisioningScheduler. Extraction of a module is now a
  redeploy of that module's own handler + provisioner — no refactor of
  the gateway (ADR-010).
- Axis.Api wires AddGrpcClient<IdentityServiceClient> against
  Modules:Identity:GrpcUrl (defaults to self-loop for modulith). Same
  call site survives extraction.
- Added patterns.md § Pattern 3 (JWKS-only JWT validation) as the rule
  for consuming modules: never call IdentityDbContext per request;
  validate locally via Identity's JWKS endpoint.

Closes the deferred line in PROGRESS.md and E02 README.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…04/E05/E06

PR #93 moved tenant schema provisioning from a central provisioner in
Axis.Api to per-module OrganizationVerifiedHandler instances in each
module's Infrastructure project. Update the implementation-status tables
in the touched epic READMEs so docs reflect the new ownership.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented May 24, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: bd4a632a-f6a6-4968-bc86-8835e230b83e

📥 Commits

Reviewing files that changed from the base of the PR and between c066a56 and d0d207f.

📒 Files selected for processing (7)
  • docs/PROGRESS.md
  • docs/playbooks/local-dev.md
  • src/Modules/DataModeling/Axis.DataModeling.Infrastructure/Messaging/OrganizationVerifiedHandler.cs
  • src/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Messaging/OrganizationVerifiedHandler.cs
  • src/Modules/Identity/Axis.Identity.Infrastructure/Persistence/IdentityUnitOfWork.cs
  • src/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Messaging/OrganizationVerifiedHandler.cs
  • src/Modules/WorkflowEngine/Axis.WorkflowEngine.Infrastructure/Messaging/OrganizationVerifiedHandler.cs
✅ Files skipped from review due to trivial changes (2)
  • docs/playbooks/local-dev.md
  • docs/PROGRESS.md

📝 Walkthrough

Walkthrough

This PR replaces centralized tenant provisioning with Identity domain events, Avro/Kafka publication, and per-module OrganizationVerifiedHandler schema provisioning. It also updates API gRPC wiring, removes the old scheduler/provisioner path, and refreshes tests and documentation.

Changes

Event-driven tenant provisioning

Layer / File(s) Summary
Identity events and contracts
src/Modules/Identity/Axis.Identity.Domain/Events/*, src/Modules/Identity/Axis.Identity.Domain/Aggregates/User.cs, src/Modules/Identity/Axis.Identity.Contracts/Schemas/*, src/Modules/Identity/Axis.Identity.Contracts/IdentityEventExtensions.cs, src/Modules/Identity/Axis.Identity.Contracts/IdentityKafkaTopics.cs, src/Modules/Identity/Axis.Identity.Contracts/Axis.Identity.Contracts.csproj
Adds Identity domain events, Avro schemas, Kafka topic constants, and GUID accessors, and updates User to raise the new events.
Identity publish pipeline
src/Modules/Identity/Axis.Identity.Infrastructure/Messaging/IdentityEventMapper.cs, src/Modules/Identity/Axis.Identity.Infrastructure/Persistence/IdentityUnitOfWork.cs, src/Modules/Identity/Axis.Identity.Infrastructure/Extensions/IdentityInfrastructureExtensions.cs, src/Modules/Identity/Axis.Identity.Application/Commands/VerifyEmail/VerifyEmailHandler.cs, src/Modules/Identity/Axis.Identity.Infrastructure/Services/WolverineTenantProvisioningScheduler.cs (removed)
Maps domain events to Avro integration events, publishes them from IdentityUnitOfWork, removes scheduler registration and implementation, and simplifies VerifyEmailHandler to persist the verified user state only.
Per-module tenant provisioning
src/Shared/Axis.Shared.Infrastructure/Tenancy/FixedTenantContext.cs, src/Modules/DataModeling/Axis.DataModeling.Infrastructure/Messaging/OrganizationVerifiedHandler.cs, src/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Messaging/OrganizationVerifiedHandler.cs, src/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Messaging/OrganizationVerifiedHandler.cs, src/Modules/WorkflowEngine/Axis.WorkflowEngine.Infrastructure/Messaging/OrganizationVerifiedHandler.cs, corresponding *.csproj files
Moves FixedTenantContext to shared infrastructure and adds module-local OrganizationVerifiedHandler consumers that create schemas and run EF Core migrations for each tenant.
API and central wiring
src/Axis.Api/Program.cs, src/Axis.Api/Axis.Api.csproj, Directory.Packages.props, src/Axis.Api/Infrastructure/Handlers/ProvisionTenantHandler.cs (removed), src/Axis.Api/Infrastructure/TenantSchemaProvisioner.cs (removed), src/Shared/Axis.Shared.Application/Tenancy/* (removed)
Updates API gRPC and Wolverine/Kafka configuration, adds gRPC package references, and removes the centralized tenant provisioning handler, provisioner, message, and scheduler interface.
Tests and docs
tests/Api/Axis.Api.Tests/Helpers/*, tests/Modules/Identity/Axis.Identity.Application.Tests/Commands/VerifyEmailHandlerTests.cs, docs/PROGRESS.md, docs/epics/E01-06/*, docs/playbooks/*
Updates tests for domain-event expectations and refreshes implementation-status, playbook, and local-dev documentation for the Kafka-driven provisioning and JWKS-only validation flow.

Sequence Diagram(s)

sequenceDiagram
  participant User as User
  participant IdentityUnitOfWork as IdentityUnitOfWork
  participant IdentityEventMapper as IdentityEventMapper
  participant Wolverine as Wolverine
  participant Kafka as Kafka
  participant OrganizationVerifiedHandler as OrganizationVerifiedHandler
  participant PostgreSQL as PostgreSQL

  User->>IdentityUnitOfWork: VerifyEmail / other domain changes
  IdentityUnitOfWork->>IdentityEventMapper: ToIntegrationEvent(domainEvent)
  IdentityEventMapper-->>IdentityUnitOfWork: Avro event payload
  IdentityUnitOfWork->>Wolverine: PublishAsync(mapped event)
  Wolverine->>Kafka: publish topic message
  Kafka->>OrganizationVerifiedHandler: OrganizationVerifiedEvent
  OrganizationVerifiedHandler->>PostgreSQL: CREATE SCHEMA IF NOT EXISTS
  OrganizationVerifiedHandler->>PostgreSQL: Database.MigrateAsync()
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • phuong-labs/axis#84: Touches the same Program.cs Wolverine/Kafka wiring that this PR expands.
  • phuong-labs/axis#92: Adds the gRPC contract/client surface that this PR consumes for IdentityServiceClient.
  • phuong-labs/axis#87: Related Identity unit-of-work/event publication work that this PR builds on.

Poem

🐰 I thumped a path from old to new,
Event seeds grew across the blue.
Kafka hummed, and schemas bloomed,
Each module dressed in tenant-tuned.
Hop-hop—Identity sang it through.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(identity): Avro events + per-module tenant provisioning (Phase 2)' is clear, concise, and accurately summarizes the main change: introducing Avro-based event-driven architecture and decentralized tenant provisioning in the Identity module.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/identity-phase2-pr93-followup

Comment @coderabbitai help to get the list of available commands and usage tips.

Markdown link check on PR #93 failed because `docs/playbooks/local-dev.md`
linked to `src/Axis.Api/Infrastructure/TenantSchemaProvisioner.cs`, which
was deleted when ownership moved into each module. Replace the dead link
and rewrite the surrounding paragraphs in `local-dev.md`,
`patterns.md` § Tenant schema provisioning, and
`docs/epics/E01-platform-foundation/features/F01-tenant-registration.md`
to describe the new per-module OrganizationVerifiedHandler flow.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Nitpick comments (1)
src/Axis.Api/Program.cs (1)

320-325: ⚡ Quick win

Replace the fully-qualified gRPC client type with a using alias.

Line 322 hardcodes the nested type name in the registration. Please add a top-level alias/import and register that alias instead so this stays consistent with the repo's C# import style.

♻️ Proposed cleanup
+using IdentityServiceClient = Axis.Identity.Contracts.Grpc.IdentityService.IdentityServiceClient;
...
-builder.Services.AddGrpcClient<Axis.Identity.Contracts.Grpc.IdentityService.IdentityServiceClient>(opts =>
+builder.Services.AddGrpcClient<IdentityServiceClient>(opts =>
 {
     opts.Address = new Uri(identityGrpcUrl);
 });

As per coding guidelines, "Use using declarations instead of fully-qualified names for imports in C# files."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Axis.Api/Program.cs` around lines 320 - 325, The registration currently
uses the fully-qualified nested type
Axis.Identity.Contracts.Grpc.IdentityService.IdentityServiceClient; add a
top-level using alias (e.g., a using declaration alias for
IdentityServiceClient) and replace the fully-qualified type in AddGrpcClient
with that alias (referencing the identityGrpcUrl variable and the AddGrpcClient
registration) so the file follows the repo's import style and avoids the long
nested type name.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/PROGRESS.md`:
- Line 74: Replace the nonconforming deferred callout "**Deferred:**" in the
PROGRESS.md entry that starts with "**Verify email → provision (PR `#93`):**" with
the required format; update it to "**Deferred (PR `#93` follow-up):**" (or
"**Deferred (follow-up PR):**" if no PR number is intended) so it matches the
coding guideline, ensuring the rest of the sentence ("retry/backoff/alert on
provision failures, provisioning wait UI, Admin role on verify per E01 US-003")
remains unchanged; look for the literal "**Deferred:**" in the Verify email →
provision paragraph and perform this single-token replacement.

In
`@src/Modules/DataModeling/Axis.DataModeling.Infrastructure/Messaging/OrganizationVerifiedHandler.cs`:
- Around line 28-38: The migration block can run concurrently for the same
organization causing migration/DDL races; wrap schema creation and await
context.Database.MigrateAsync(...) behind a per-organization lock (e.g., acquire
a PostgreSQL advisory lock on a hash of organizationId using the existing
NpgsqlConnection before CREATE SCHEMA and MigrateAsync, or use a distributed
lock/Redis keyed by organizationId), hold the lock for the duration of schema
creation + new DataModelingDbContext/Database.MigrateAsync, implement a
timeout/retry and ensure the lock is always released in a finally block;
reference the existing symbols NpgsqlConnection connection, organizationId,
schema, FixedTenantContext and DataModelingDbContext when adding the lock.

In
`@src/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Messaging/OrganizationVerifiedHandler.cs`:
- Around line 27-37: The migration path in OrganizationVerifiedHandler currently
only guards CREATE SCHEMA but allows concurrent calls to
FormBuilderDbContext.Database.MigrateAsync leading to migration/DDL races; wrap
the schema creation and the call to Database.MigrateAsync in a per-organization
lock/dedupe (e.g. a Postgres advisory lock using the organizationId, or an
application-level concurrent dictionary/semaphore keyed by organizationId) so
only one handler runs migration for a given tenant at a time; apply the lock
around the block that opens the NpgsqlConnection, executes
createSchema.CommandText and invokes new
FormBuilderDbContext(...).Database.MigrateAsync to ensure exclusive provisioning
for that FixedTenantContext/organization.

In
`@src/Modules/Identity/Axis.Identity.Infrastructure/Persistence/IdentityUnitOfWork.cs`:
- Line 57: The PublishAsync call in IdentityUnitOfWork.cs does not forward the
CancellationToken, so message publishing continues after SaveChangesAsync is
cancelled; update the call site that currently invokes
bus.PublishAsync(integrationEvent) to pass the CancellationToken parameter from
the surrounding async method (e.g., use the method's cancellationToken argument)
so it becomes bus.PublishAsync(integrationEvent, cancellationToken), ensuring
the token is propagated through PublishAsync.

In
`@src/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Messaging/OrganizationVerifiedHandler.cs`:
- Around line 27-37: The schema provisioning and migration in
OrganizationVerifiedHandler is not serialized and can race when the same
OrganizationVerifiedEvent is redelivered; wrap the CREATE SCHEMA +
Database.MigrateAsync sequence with a per-organization lock/dedupe (for example
acquire a Postgres advisory lock derived from organizationId or use a
distributed mutex keyed by organizationId) before opening/using the
NpgsqlConnection and releasing it after migrations complete; ensure the lock is
held across the createSchema/WorkflowBuilderDbContext.Database.MigrateAsync
calls (symbols: NpgsqlConnection, createSchema.CommandText, FixedTenantContext,
WorkflowBuilderDbContext, Database.MigrateAsync) and make the lock acquisition
retry-safe and cancellable with the existing cancellationToken.

In
`@src/Modules/WorkflowEngine/Axis.WorkflowEngine.Infrastructure/Messaging/OrganizationVerifiedHandler.cs`:
- Around line 27-37: The CREATE SCHEMA + Database.MigrateAsync call in
OrganizationVerifiedHandler can run concurrently for the same organization; wrap
schema provisioning and context.Database.MigrateAsync in a per-organization lock
(or dedupe) to serialize provisioning. Specifically, before running the CREATE
SCHEMA / new FixedTenantContext + WorkflowEngineDbContext and calling
context.Database.MigrateAsync, acquire a lock keyed by organizationId (for
example a Postgres advisory lock using organizationId, or an
in-memory/distributed mutex keyed by organizationId), only proceed if the lock
is held, and ensure the lock is released after migration (with proper
timeout/error handling and retry/backoff on contention). Keep the CREATE SCHEMA,
FixedTenantContext, optionsBuilder.UseNpgsql, and context.Database.MigrateAsync
logic inside the locked section so two handlers for the same organization cannot
run migrations in parallel.

---

Nitpick comments:
In `@src/Axis.Api/Program.cs`:
- Around line 320-325: The registration currently uses the fully-qualified
nested type Axis.Identity.Contracts.Grpc.IdentityService.IdentityServiceClient;
add a top-level using alias (e.g., a using declaration alias for
IdentityServiceClient) and replace the fully-qualified type in AddGrpcClient
with that alias (referencing the identityGrpcUrl variable and the AddGrpcClient
registration) so the file follows the repo's import style and avoids the long
nested type name.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ad1aea43-8532-4412-8b3d-44ef5a57ec82

📥 Commits

Reviewing files that changed from the base of the PR and between c792a2f and 40b05d1.

⛔ Files ignored due to path filters (5)
  • src/Modules/Identity/Axis.Identity.Contracts/Generated/axis/identity/events/OrganizationVerifiedEvent.cs is excluded by !**/generated/**
  • src/Modules/Identity/Axis.Identity.Contracts/Generated/axis/identity/events/RoleAssignedEvent.cs is excluded by !**/generated/**
  • src/Modules/Identity/Axis.Identity.Contracts/Generated/axis/identity/events/RoleRemovedEvent.cs is excluded by !**/generated/**
  • src/Modules/Identity/Axis.Identity.Contracts/Generated/axis/identity/events/UserDeactivatedEvent.cs is excluded by !**/generated/**
  • src/Modules/Identity/Axis.Identity.Contracts/Generated/axis/identity/events/UserReactivatedEvent.cs is excluded by !**/generated/**
📒 Files selected for processing (47)
  • Directory.Packages.props
  • docs/PROGRESS.md
  • docs/epics/E01-platform-foundation/README.md
  • docs/epics/E02-identity-access/README.md
  • docs/epics/E03-data-modeling/README.md
  • docs/epics/E04-workflow-builder/README.md
  • docs/epics/E05-form-builder/README.md
  • docs/epics/E06-workflow-engine/README.md
  • docs/playbooks/patterns.md
  • src/Axis.Api/Axis.Api.csproj
  • src/Axis.Api/Infrastructure/Handlers/ProvisionTenantHandler.cs
  • src/Axis.Api/Infrastructure/TenantSchemaProvisioner.cs
  • src/Axis.Api/Program.cs
  • src/Modules/DataModeling/Axis.DataModeling.Infrastructure/Axis.DataModeling.Infrastructure.csproj
  • src/Modules/DataModeling/Axis.DataModeling.Infrastructure/Messaging/OrganizationVerifiedHandler.cs
  • src/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Axis.FormBuilder.Infrastructure.csproj
  • src/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Messaging/OrganizationVerifiedHandler.cs
  • src/Modules/Identity/Axis.Identity.Application/Commands/VerifyEmail/VerifyEmailHandler.cs
  • src/Modules/Identity/Axis.Identity.Application/Services/ITenantProvisioningScheduler.cs
  • src/Modules/Identity/Axis.Identity.Contracts/Axis.Identity.Contracts.csproj
  • src/Modules/Identity/Axis.Identity.Contracts/IdentityEventExtensions.cs
  • src/Modules/Identity/Axis.Identity.Contracts/IdentityKafkaTopics.cs
  • src/Modules/Identity/Axis.Identity.Contracts/Schemas/OrganizationVerifiedEvent.avsc
  • src/Modules/Identity/Axis.Identity.Contracts/Schemas/RoleAssignedEvent.avsc
  • src/Modules/Identity/Axis.Identity.Contracts/Schemas/RoleRemovedEvent.avsc
  • src/Modules/Identity/Axis.Identity.Contracts/Schemas/UserDeactivatedEvent.avsc
  • src/Modules/Identity/Axis.Identity.Contracts/Schemas/UserReactivatedEvent.avsc
  • src/Modules/Identity/Axis.Identity.Domain/Aggregates/User.cs
  • src/Modules/Identity/Axis.Identity.Domain/Events/OrganizationVerified.cs
  • src/Modules/Identity/Axis.Identity.Domain/Events/RoleAssigned.cs
  • src/Modules/Identity/Axis.Identity.Domain/Events/RoleRemoved.cs
  • src/Modules/Identity/Axis.Identity.Domain/Events/UserDeactivated.cs
  • src/Modules/Identity/Axis.Identity.Domain/Events/UserReactivated.cs
  • src/Modules/Identity/Axis.Identity.Infrastructure/Extensions/IdentityInfrastructureExtensions.cs
  • src/Modules/Identity/Axis.Identity.Infrastructure/Messaging/IdentityEventMapper.cs
  • src/Modules/Identity/Axis.Identity.Infrastructure/Persistence/IdentityUnitOfWork.cs
  • src/Modules/Identity/Axis.Identity.Infrastructure/Services/WolverineTenantProvisioningScheduler.cs
  • src/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Axis.WorkflowBuilder.Infrastructure.csproj
  • src/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Messaging/OrganizationVerifiedHandler.cs
  • src/Modules/WorkflowEngine/Axis.WorkflowEngine.Infrastructure/Axis.WorkflowEngine.Infrastructure.csproj
  • src/Modules/WorkflowEngine/Axis.WorkflowEngine.Infrastructure/Messaging/OrganizationVerifiedHandler.cs
  • src/Shared/Axis.Shared.Application/Tenancy/ITenantSchemaProvisioner.cs
  • src/Shared/Axis.Shared.Application/Tenancy/ProvisionTenantMessage.cs
  • src/Shared/Axis.Shared.Infrastructure/Tenancy/FixedTenantContext.cs
  • tests/Api/Axis.Api.Tests/Helpers/ApiTestFixture.cs
  • tests/Api/Axis.Api.Tests/Helpers/NoOpTenantSchemaProvisioner.cs
  • tests/Modules/Identity/Axis.Identity.Application.Tests/Commands/VerifyEmailHandlerTests.cs
💤 Files with no reviewable changes (9)
  • src/Modules/Identity/Axis.Identity.Infrastructure/Extensions/IdentityInfrastructureExtensions.cs
  • src/Shared/Axis.Shared.Application/Tenancy/ProvisionTenantMessage.cs
  • src/Modules/Identity/Axis.Identity.Application/Services/ITenantProvisioningScheduler.cs
  • src/Shared/Axis.Shared.Application/Tenancy/ITenantSchemaProvisioner.cs
  • src/Axis.Api/Infrastructure/Handlers/ProvisionTenantHandler.cs
  • tests/Api/Axis.Api.Tests/Helpers/NoOpTenantSchemaProvisioner.cs
  • src/Axis.Api/Infrastructure/TenantSchemaProvisioner.cs
  • tests/Api/Axis.Api.Tests/Helpers/ApiTestFixture.cs
  • src/Modules/Identity/Axis.Identity.Infrastructure/Services/WolverineTenantProvisioningScheduler.cs

Comment thread docs/PROGRESS.md Outdated
{
object? integrationEvent = IdentityEventMapper.ToIntegrationEvent(evt);
if (integrationEvent is not null)
await bus.PublishAsync(integrationEvent);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Forward CancellationToken to PublishAsync.

The PublishAsync call does not forward the CancellationToken parameter. When SaveChangesAsync is cancelled, the message publishing will continue executing instead of being cancelled. As per coding guidelines, always forward CancellationToken through async method calls.

🔧 Proposed fix
-                await bus.PublishAsync(integrationEvent);
+                await bus.PublishAsync(integrationEvent, ct);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
await bus.PublishAsync(integrationEvent);
await bus.PublishAsync(integrationEvent, ct);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/Modules/Identity/Axis.Identity.Infrastructure/Persistence/IdentityUnitOfWork.cs`
at line 57, The PublishAsync call in IdentityUnitOfWork.cs does not forward the
CancellationToken, so message publishing continues after SaveChangesAsync is
cancelled; update the call site that currently invokes
bus.PublishAsync(integrationEvent) to pass the CancellationToken parameter from
the surrounding async method (e.g., use the method's cancellationToken argument)
so it becomes bus.PublishAsync(integrationEvent, cancellationToken), ensuring
the token is propagated through PublishAsync.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
docs/playbooks/local-dev.md (1)

86-86: ⚡ Quick win

Consider breaking this into multiple sentences for readability.

This single sentence contains three distinct concepts: Identity bootstrap, per-tenant provisioning, and the trigger mechanism. Breaking it into 2-3 shorter sentences would improve scannability.

📝 Suggested rewrite
-On the next boot, `IdentityDbContext.Database.MigrateAsync()` runs at startup (dev only — see [`src/Axis.Api/Program.cs`](../../src/Axis.Api/Program.cs)) to recreate Identity + OpenIddict tables. Per-tenant module schemas (`tenant_{org-id}`) are provisioned on demand by each module's `OrganizationVerifiedHandler` (e.g. [`src/Modules/DataModeling/Axis.DataModeling.Infrastructure/Messaging/OrganizationVerifiedHandler.cs`](../../src/Modules/DataModeling/Axis.DataModeling.Infrastructure/Messaging/OrganizationVerifiedHandler.cs)), triggered by Identity's `OrganizationVerifiedEvent` (Kafka, ADR-019).
+On the next boot, `IdentityDbContext.Database.MigrateAsync()` runs at startup (dev only — see [`src/Axis.Api/Program.cs`](../../src/Axis.Api/Program.cs)) to recreate Identity + OpenIddict tables.
+
+Per-tenant module schemas (`tenant_{org-id}`) are provisioned on demand by each module's `OrganizationVerifiedHandler` (e.g. [`src/Modules/DataModeling/Axis.DataModeling.Infrastructure/Messaging/OrganizationVerifiedHandler.cs`](../../src/Modules/DataModeling/Axis.DataModeling.Infrastructure/Messaging/OrganizationVerifiedHandler.cs)). This handler is triggered by Identity's `OrganizationVerifiedEvent` published via Kafka (ADR-019).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/playbooks/local-dev.md` at line 86, The long sentence describing startup
migration, per-tenant schema provisioning, and the event trigger should be split
into 2–3 clearer sentences: one stating that
IdentityDbContext.Database.MigrateAsync() runs at startup in dev (reference
Program.cs), a second describing that per-tenant module schemas
(tenant_{org-id}) are provisioned on demand by each module's
OrganizationVerifiedHandler (e.g. OrganizationVerifiedHandler class), and a
third noting that this is triggered by Identity's OrganizationVerifiedEvent
(Kafka, ADR-019); preserve the existing links and examples while improving
scannability.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@docs/playbooks/local-dev.md`:
- Line 86: The long sentence describing startup migration, per-tenant schema
provisioning, and the event trigger should be split into 2–3 clearer sentences:
one stating that IdentityDbContext.Database.MigrateAsync() runs at startup in
dev (reference Program.cs), a second describing that per-tenant module schemas
(tenant_{org-id}) are provisioned on demand by each module's
OrganizationVerifiedHandler (e.g. OrganizationVerifiedHandler class), and a
third noting that this is triggered by Identity's OrganizationVerifiedEvent
(Kafka, ADR-019); preserve the existing links and examples while improving
scannability.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8c0498d5-102f-4032-a2b5-8208588fccca

📥 Commits

Reviewing files that changed from the base of the PR and between 40b05d1 and c066a56.

📒 Files selected for processing (3)
  • docs/epics/E01-platform-foundation/features/F01-tenant-registration.md
  • docs/playbooks/local-dev.md
  • docs/playbooks/patterns.md
✅ Files skipped from review due to trivial changes (1)
  • docs/epics/E01-platform-foundation/features/F01-tenant-registration.md

Findings actioned:

1. **(Major × 4) Race on __EFMigrationsHistory under Kafka redelivery.**
   Each module's OrganizationVerifiedHandler now acquires a session-scoped
   Postgres advisory lock keyed by 'axis.tenant.{module}:{org-id}' before
   CREATE SCHEMA + MigrateAsync, serializing concurrent provisioning for
   the same org. The lock auto-releases when the guard connection closes.

2. **(Minor) PROGRESS.md deferred-callout format.** Replaced bare
   '**Deferred:**' with '**Deferred (F01 US-003 follow-up):**' per the
   CLAUDE.md Definition of Done convention.

3. **(Nitpick) Long sentence in docs/playbooks/local-dev.md.**
   Split into two sentences for scannability — startup migration first,
   then per-tenant provisioning flow.

4. **(Major, NOT actioned — false positive)**
   'Forward CancellationToken to PublishAsync' in IdentityUnitOfWork.cs.
   Wolverine 5.39.3's IMessageBus.PublishAsync signature is
   (object, DeliveryOptions?) — no CancellationToken overload exists.
   PublishAsync is fire-and-forget onto the outbox. Added an inline
   comment mirroring the one already in DataModelingUnitOfWork so this
   doesn't get re-flagged.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@phuongnse

Copy link
Copy Markdown
Owner Author

@coderabbitai Thanks for the review. Addressed in d0d207f:

  1. Schema provisioning races — all 4 module OrganizationVerifiedHandler files now wrap CREATE SCHEMA + MigrateAsync in a session-scoped Postgres advisory lock (pg_advisory_lock(hashtextextended('axis.tenant.{module}:{org-id}', 0))) on the guard connection. Concurrent redelivery for the same org blocks at the DB level; lock auto-releases when the connection closes.

  2. Deferred: format in PROGRESS.md — fixed to **Deferred (F01 US-003 follow-up):** per CLAUDE.md.

  3. Long sentence in local-dev.md — split into two for scannability.

  4. Forward CancellationToken to PublishAsync — NOT actioned (false positive). WolverineFx 5.39.3's IMessageBus.PublishAsync signature is PublishAsync(object, DeliveryOptions?). There is no CancellationToken overload — adding ct produces CS1503: cannot convert from 'CancellationToken' to 'DeliveryOptions?'. PublishAsync is fire-and-forget onto the outbox; the actual dispatch happens later out of the handler's scope. Added an inline comment in IdentityUnitOfWork.cs mirroring the one already in DataModelingUnitOfWork.cs so this doesn't get re-flagged. Same issue was raised + resolved on PR refactor(shared): inline UnitOfWork + AxisDbContext per module (ADR-017) #87 (commit b348cb2).

@coderabbitai

coderabbitai Bot commented May 24, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

@phuongnse Thanks for the thorough follow-up!

  1. Advisory lock on schema provisioning — the pg_advisory_lock(hashtextextended(...)) approach is solid; session-scoped means the lock is held for the full CREATE SCHEMA + MigrateAsync window and releases automatically on connection close, which is exactly right for this pattern.

2–3. Formatting & prose fixes — noted, all good.

  1. PublishAsync / CancellationToken — confirmed false positive on my end. The WolverineFx 5.39.3 IMessageBus.PublishAsync(object, DeliveryOptions?) signature doesn't accept a CancellationToken, and the outbox dispatch is intentionally fire-and-forget outside the handler's scope. The inline comment in IdentityUnitOfWork.cs is a good guard against future re-flags.

🐇 ✨


✏️ Learnings added
Learnt from: phuongnse
URL: https://github.com/phuong-labs/axis/pull/93

Timestamp: 2026-05-24T00:58:00.736Z
Learning: In this codebase (phuong-labs/axis), WolverineFx 5.39.3's `IMessageBus.PublishAsync` signature is `PublishAsync(object, DeliveryOptions?)` — there is no `CancellationToken` overload. Suggesting to pass a CancellationToken to `PublishAsync` is a false positive; PublishAsync is fire-and-forget onto the Wolverine outbox and the actual dispatch happens later outside the handler's scope. This was confirmed in PR `#93` (commit d0d207f) and previously in PR `#87` (commit b348cb2).

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

@phuongnse phuongnse merged commit e229e82 into main May 24, 2026
6 checks passed
phuongnse added a commit that referenced this pull request May 24, 2026
Per PR review discussion: in-file Resolved log is mostly noise.
- Git history (`git log -p docs/WORKAROUNDS.md`) is the audit trail.
- Durable lessons belong in patterns.md / ADRs where future readers
  actually look, not in a graveyard at the bottom of the inventory.

Updates the "Resolving a workaround" protocol to say: delete the entry
when the trigger fires, and only promote the lesson to patterns.md or
an ADR if it is durable. Removes the seeded central-tenant-schema-
provisioner entry — the PR #93 commit message + patterns.md § Tenant
schema provisioning already cover it.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
phuongnse added a commit that referenced this pull request May 24, 2026
CodeRabbit caught the unfilled placeholder. Per CLAUDE.md the literal
"PR `#N`" is the template form; when actually using the callout, the
substitution should be a real PR number or the responsible feature/US
(matches the pattern from PR #93 docs, e.g. "Deferred (F01 US-003
follow-up)").

The deferred items (real IScriptExecutor + INotificationSender) belong
to E06 F02-step-handlers — that's where the Script and Notification
step semantics are specified.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant