Merge pull request #19 from HodeTech/development

Phase 02a Packet 10: architecture tests green, and the phase exit

Author: cemililik

.claude/skills/add-architecture-test/SKILL.md

@@ -14,8 +14,8 @@ description: >
 
 ## Purpose
 
-Move a rule from documentation prose into a CI-enforced check using NetArchTest /
-ArchUnitNET / Roslyn analyzers / migration scans
+Move a rule from documentation prose into a CI-enforced check — NetArchTest, reflection,
+an IL scan, a source or migration scan, or a Roslyn analyzer
 ([01-architecture-standards.md § Architecture Tests](../../../docs/standards/01-architecture-standards.md)).
 
 ## When to use
@@ -40,7 +40,7 @@ ArchUnitNET / Roslyn analyzers / migration scans
 |-------|----------|-------------|
 | Rule | Yes | Plain-English statement of the invariant. |
 | Source | Yes | ADR / standard that defines the rule. |
-| Detection mechanism | Yes | NetArchTest / ArchUnitNET (project graph) / Roslyn (syntactic) / migration scan (SQL pattern). |
+| Detection mechanism | Yes | NetArchTest (type references) / reflection / Mono.Cecil (IL) / Roslyn (syntactic) / source or migration scan (text). |
 | Failure message | Yes | Actionable: tell the author what to do, not just what is wrong. |
 
 ## Workflow
@@ -57,21 +57,26 @@ them.
 | Mechanism | Use when |
 |-----------|----------|
 | **NetArchTest** | Project-graph reference bans, naming conventions on types / namespaces. |
-| **ArchUnitNET** | Slightly richer rules (constraints across assemblies). Pick when NetArchTest can't express it. |
-| **Roslyn analyzer** | Syntactic patterns inside code (`IgnoreQueryFilters` call, raw `Guid` in command, …). |
+| **Mono.Cecil (IL scan)** | What a method body does — a constructor called, a member read — which NetArchTest's type-level rules cannot express. It ships with NetArchTest; there is no ArchUnitNET in this repository. |
+| **Roslyn analyzer** | A syntactic pattern the build itself should fail on — `throw new DomainException(...)` outside an invariant guard is `LS0001`. |
 | **Migration scan** | SQL patterns in `.cs` migration files (RLS-enable, policy presence, partition declaration). |
 | **Reflection over conventions** | Marker-attribute presence + matching property / migration shape. |
 | **String / regex scan** | When all else fails — explicit "no file under `Verticals/`", "no `english.*` permission key". |
 
 ### Step 3: Author the test
 
-In `backend/tests/LearnStack.Tests.Architecture/`:
+In `backend/tests/LearnStack.Tests.Architecture/`. The block below illustrates the
+shape and is **not** a rule to add: its subject ships as
+`ModuleDomain_DoesNotDependOn_OtherModuleDomain` in `ModuleDependencyTests.cs`, and a
+rule's name comes from
+[the catalogue](../../../docs/standards/21-architecture-tests-catalogue.md), never from
+an example.
 
 ```csharp
-public sealed class ModuleDependencyTests
+public sealed class ExampleBoundaryTests
 {
     [Fact]
-    public void Modules_DoNot_Reference_OtherModule_Domain()
+    public void Example_Module_Does_Not_Reference_Another_Modules_Domain()
     {
         // pseudo-code; replace `GetOwningModuleNamespace(...)` with the real
         // owning-namespace resolver helper that lives next to the test. The
@@ -110,32 +115,36 @@ Patterns to follow:
 
 ### Step 4: Common architecture-test families
 
-**The shipped set is fourteen files, not a family per topic.** Add yours to the one whose
+**The shipped set is eighteen files, not a family per topic.** Add yours to the one whose
 subject it shares:
 
 | File | What it covers |
 |------|----------------|
-| `ModuleDependencyTests.cs` | Dependency direction between module packages, plus a planted-violation meta test that proves the scanner still detects one. |
-| `PersistenceConventionTests.cs` | `row_version` mapping, ambient-unit-of-work enlistment, the Docker trait, and the `migrate` recipe's chain coverage and credential redaction. |
-| `TenancyConventionTests.cs` | The ADR-0036 **request-edge** rules — what may read a host, where the effective host and `app.resolving_host` are computed, and the assertion budget's independence from `ICacheService`. Source scans, because three of the four banned inputs appear only as string literals. |
-| `TenantScopingTests.cs` | The correspondence between the `[TenantOwned]` / `[OrganizationScoped]` markers, the EF global query filters, and the Row Level Security policies. |
+| `ModuleDependencyTests.cs` | The dependency matrix — each module layer, and the core `Application`, an allow-list over its IL and its project references — the Domain's one EF Core exception for Vogen's emitted converters, plus a planted-violation meta test that proves the scanner still detects one. |
+| `PersistenceConventionTests.cs` | `row_version` mapping, ambient-unit-of-work enlistment and the ban on fanning out over its connection, the Docker trait, and the `migrate` recipe's chain coverage and credential redaction. |
+| `TenancyConventionTests.cs` | The ADR-0036 **request-edge** rules — what may read a host, where the effective host and `app.resolving_host` are computed, the assertion budget's independence from `ICacheService`, and the host resolver's lack of outbound dependencies — and who may set a session variable: the out-of-band setters' read-only transactions and the ban on setting `app.scope`. Mostly source scans, because most of the banned inputs are header names and SQL that appear only as string literals. |
+| `TenantScopingTests.cs` | The correspondence between the `[TenantOwned]` / `[OrganizationScoped]` markers, the EF global query filters, and the Row Level Security policies, down to the tenant term in both of a policy's clauses. |
 | `TenantContextConstructionTests.cs` | How a tenant context comes into existence and who may write it: the factory's single entry point, the constructor's one call site, the enumerated accessor writers, and the composite-key organization read. |
 | `ApiConventionTests.cs` | Live majors, forwarded headers, required `Deployment:Mode`, unversioned route prefixes. |
-| `CrossCuttingFoundationTests.cs` | Pipeline order, `Result<T>` returns, topic naming, and the direct-reference bans (Sentry, `DeploymentMode`, `IEventBus`, provider SDK exceptions). |
+| `CrossCuttingFoundationTests.cs` | Pipeline order, `Result<T>` returns, topic naming, and the direct-reference bans (Sentry, `DeploymentMode`, `IEventBus`, the cache clients, the Hub, provider SDK exceptions). |
 | `RequestSurfaceTests.cs` | What the step-4 authority ceiling admits: the two request markers, their permitted sets, the shape of the attributes themselves, and the ban on request shapes MediatR runs with no pipeline. |
 | `PlatformAdminScopeConventionTests.cs` | The single sanctioned `BYPASSRLS` path: who may resolve the keyed platform data source, where connection strings are read, the entry gate, and what the scope must not touch. |
-| `RepositoryLayoutTests.cs` | `No_Source_Folder_Named_Verticals` and the single-frontend-app rule. |
+| `RepositoryLayoutTests.cs` | `No_Source_Folder_Named_Verticals`, the single-frontend-app rule, and the commit-subject grammar — checked by running the `commit-msg` hook. |
 | `AggregateWriteTests.cs` | The one sanctioned cross-aggregate write, and the count of aggregate roots a handler's ports can write that keeps it at one. |
 | `CustomizationRegistryTests.cs` | The closed renderer registries that exist twice — in C# and in TypeScript — and stop the two drifting. |
-| `AuditConventionTests.cs` | The audit aggregate's shape, the closed-set columns against their `CHECK`s, append-only enforcement at the source level, and the `OperationType` table. |
+| `AuditConventionTests.cs` | The audit aggregate's shape, the closed-set columns against their `CHECK`s, append-only enforcement at the source level, the closed set of `audit_log` writers, and the `OperationType` table. |
+| `EntitlementConventionTests.cs` | The entitlement socket: which types may name `platform_entitlement_cache`'s entity, and whose SQL may touch its rows. |
+| `EntitlementKeyTests.cs` | The entitlement vocabulary: where a feature, limit or killswitch key may be spelled, and which keys the tenant's own flag table admits. |
+| `DomainModelTests.cs` | The shape every aggregate carries: identity equality declared once, and an aggregate root's id generated rather than hand-written. |
+| `DomainGenericityTests.cs` | The genericity guarantee: no name the platform ships — backend, database, audit slug, key, renderer or frontend — carries a domain term. |
 | `AuditCoverageTests.cs` | The catalogue ↔ matrix join, keyed on request types discovered from every backend assembly, in both directions — plus the module-has-a-matrix rule. |
 
-Rules for surfaces no file covers yet — permissions, entitlement, event bus, Hub
-contract — are **Registered** in
+Rules for surfaces no file covers yet — permissions, the Hub adapters and endpoints — are
+**Registered** in
 [the catalogue](../../../docs/standards/21-architecture-tests-catalogue.md) against
 the phase that ships the code they inspect. Check its Status line before assuming a
 net is under you, and create a new file only when your rule's subject is not one of
-the fourteen above.
+the eighteen above.
 
 > **The tenancy rules live in three files, and the split is by subject, not by ADR.**
 > All three cite ADR-0036, so "put it with the other ADR-0036 rules" is not a usable

.claude/skills/add-audit-coverage/SKILL.md

@@ -492,10 +492,11 @@ passes even when every policy is inert. See
     Implemented (`AuditConventionTests`), and `Every_Module_With_An_Aggregate_Or_A_Request_Has_A_Matrix`
     — Implemented (`AuditCoverageTests`) — fails a module that ships an aggregate or a
     request with none.
-  - `Modules_Do_Not_Write_AuditLog_Directly` (no module assembly outside
-    `LearnStack.Modules.Audit.*` names `audit_log` or `AuditEntry`; the
-    `LearnStack.SharedKernel.Audit` ports are explicitly out of scope) — Registered,
-    Packet 10.
+  - `Modules_Do_Not_Write_AuditLog_Directly` (the types that name `AuditEntry` are the
+    entity, its configuration and `AuditDbContext`; the only SQL inserting into
+    `audit_log` is `PostgresAuditStore`'s; no module but Audit names the table; the
+    `LearnStack.SharedKernel.Audit` ports are explicitly out of scope) — Implemented
+    (`AuditConventionTests`).
   - `No_Set_Based_Write_Bypasses_The_Audit_Capture` (no `ExecuteUpdate`,
     `ExecuteDelete` or `ExecuteSql*` in backend source — the capture sees only the change
     tracker, so the operation you are classifying must write through its aggregate and

.claude/skills/add-backend-module/SKILL.md

@@ -285,9 +285,12 @@ See [add-ef-migration](../add-ef-migration/SKILL.md) for migration conventions
 ## Validation
 
 - `dotnet build` succeeds for all four projects.
-- `LearnStack.Tests.Architecture` is green; specifically the two rules that
-  actually run, `ModuleDomain_DoesNotDependOn_OtherModuleDomain` and
-  `ModuleDomain_DoesNotDependOn_AnyApplicationOrInfrastructure`, for `<Name>`.
+- `LearnStack.Tests.Architecture` is green; specifically the dependency matrix for
+  `<Name>` — `ModuleDomain_DoesNotDependOn_OtherModuleDomain`,
+  `ModuleDomain_DoesNotDependOn_AnyApplicationOrInfrastructure`,
+  `ModuleApplication_References_Only_Its_Own_Layers_And_Other_Contracts`,
+  `ModuleInfrastructure_References_Only_Its_Own_Layers_And_Core_Infrastructure` and
+  `ModuleContracts_Reference_Only_SharedKernel`.
 - `dotnet ef migrations script` for the module shows the expected baseline schema.
 - The module appears in [03-module-boundaries.md](../../../docs/architecture/03-module-boundaries.md)
   module map and in [docs/glossary.md](../../../docs/glossary.md) if it owns any

.claude/skills/add-feature-key/SKILL.md

@@ -438,11 +438,13 @@ public async Task RefreshAsync_AppliesAnEqualGenerationPush()
 - `dotnet build` and `dotnet test` pass.
 - Architecture tests, canonical names from
   [21-architecture-tests-catalogue.md](../../../docs/standards/21-architecture-tests-catalogue.md):
-  - `FeatureKey_AllReferences_AreInRegistry` (Roslyn) — every `IFeatureFlags` call
-    references a registered key. There is no separate `LimitKey_*` spelling; this rule
-    covers both key types.
+  - `FeatureKey_AllReferences_AreInRegistry` (an IL scan) — every `FeatureKey`,
+    `LimitKey` and `KillswitchKey` a call site names is a member of its registry. There
+    is no separate `LimitKey_*` spelling; this one rule covers all three key types.
   - `PlanProjected_Keys_NotInTenantFlags` — plan-projected keys never appear in
-    `tenant_feature_flags`.
+    `tenant_feature_flags`. `Tenant.SetFeatureFlag` takes a `FeatureKey` and refuses every
+    key whose descriptor is not `FeatureSource.TenantFlag`, so a new plan key needs no
+    change here and a new tenant flag is admitted by its descriptor alone.
   - `Modules_Do_Not_Read_Entitlement_Cache_Directly` — the only sanctioned reader **and**
     writer of `platform_entitlement_cache` is an `IEntitlementProvider` implementation;
     no module, Tenancy included, may query it.

.claude/skills/add-integration-event/SKILL.md

@@ -110,8 +110,13 @@ await outbox.EnqueueAsync(new EnrollmentCreatedIntegrationEventV1
     TenantId = tenantContext.TenantId.Value,   // the envelope carries a Guid
     EnrollmentId = enrollment.Id.Value,
     LearnerId = request.LearnerId.Value,
-    CourseVersionId = request.CourseVersionId.Value,
-    CohortId = request.CohortId?.Value,
+    // Three types, two of them the same. The contract carries a module-local id as a
+    // `Guid` (add-mediatr-handler § Step 1) and this event declares one, so the request's
+    // value assigns directly — `request.CourseVersionId.Value` does not compile, because a
+    // `Guid` has no `Value`. The handler's own `courseVersionId` is the TYPED id it built
+    // one layer in, so that spelling needs `courseVersionId.Value`.
+    CourseVersionId = request.CourseVersionId,
+    CohortId = request.CohortId,
     Source = request.Source.ToString().ToLowerInvariant(),
 }, cancellationToken);
 
@@ -120,9 +125,12 @@ await db.SaveChangesAsync(cancellationToken);   // atomic
 
 Rules:
 
-- `IOutbox.EnqueueAsync` enrolls in the **ambient** `DbContext`. Do not open a new
-  transaction.
-- `SaveChangesAsync` commits the aggregate change and the outbox row together.
+- `IOutbox.EnqueueAsync` enlists in the **ambient unit of work** — the one connection and
+  transaction `IUnitOfWork` owns ([ADR-0040](../../../docs/decisions/0040-ambient-unit-of-work.md)),
+  not a module's `DbContext`. Do not open a second transaction.
+- `SaveChangesAsync` **flushes** the context; it does not commit. `TransactionBehavior` owns
+  the commit boundary, so the aggregate change and the outbox row land together because they
+  share that transaction — not because one `SaveChanges` wrote both.
 
 ### Step 3: Topic declaration
 
@@ -178,10 +186,14 @@ public sealed class CreateAuditEntryOnEnrollmentCreated(
 Rules:
 
 - **Every** `IIntegrationEventHandler` invokes `IInboxGuard.IsAlreadyProcessedAsync`
-  before any business logic. Architecture test
-  `Integration_Event_Handlers_Use_InboxGuard` enforces this.
-- `MarkAsProcessed` enrolls in the same `DbContext`; the inbox marker and the
-  business write commit atomically.
+  before any business logic. The architecture test
+  `Integration_Event_Handlers_Use_InboxGuard` holds every handler to it from
+  [Phase 02b](../../../docs/roadmap/phase-02b-events-auth.md), which ships the guard; it
+  is registered, not running, until then.
+- `MarkAsProcessed` writes on the same **transaction** as the business write — the ambient
+  one the transport opens per delivery through `IUnitOfWork`
+  ([ADR-0040](../../../docs/decisions/0040-ambient-unit-of-work.md)) — so the inbox marker
+  and the business write commit atomically.
 - The **transport** — not middleware — restores tenant context from
   `@event.TenantId` before your handler runs, and puts the publisher's own back
   afterwards. Read it through `ITenantContext` as usual; don't read it from
@@ -232,10 +244,12 @@ Two tests minimum:
 ## Validation
 
 - `dotnet build` and `dotnet test` pass.
-- `LearnStack.Tests.Architecture` is green; specifically
-  `Integration_Events_Inherit_From_IntegrationEventBase`,
-  `Integration_Event_Handlers_Use_InboxGuard`,
-  `Integration_Event_TopicNames_FollowConvention`.
+- `LearnStack.Tests.Architecture` is green — `Integration_Event_TopicNames_FollowConvention`
+  runs today; `Integration_Events_Inherit_From_IntegrationEventBase` and
+  `Integration_Event_Handlers_Use_InboxGuard` are registered for
+  [Phase 02b](../../../docs/roadmap/phase-02b-events-auth.md) and run from there. Check
+  [the catalogue](../../../docs/standards/21-architecture-tests-catalogue.md) for their
+  status rather than assuming a net is under you.
 - An integration test confirms the round-trip: handler enqueues → outbox row
   created → outbox processor dispatches → consumer handles + writes business
   state + inbox row.

.claude/skills/add-integration-test/SKILL.md

@@ -69,7 +69,7 @@ architecture test) plus any other invariant the change touches. See
 > what almost every test wants: **every** migration chain applied — through
 > `MigrationChains.ApplyAllAsync`, never a list a fixture keeps itself — and every
 > table seeded for two tenants, with a second organization under tenant A. There
-> are three chains and fourteen tables as of Packet 8; a fixture that names them is
+> are four chains and seventeen tables as of Packet 9; a fixture that names them is
 > a fixture that goes stale, which is the defect below. Share it with `[Collection(SharedSchema.Name)]` rather than
 > `IClassFixture<>`, so one container serves the whole schema suite.
 >
@@ -173,11 +173,11 @@ public async Task Unsetting_tenant_context_returns_zero_rows_through_RLS()
 }
 ```
 
-Nothing throws `TenantContextMissingException` today — the type itself shipped in
-Packet 3, in `LearnStack.SharedKernel/Errors/`. The `DbCommandInterceptor` that
-throws it is described in Standards 05 and 11 and lands in **Packet 7**, which owns
-it. Until it does, the fail-closed behaviour is the empty result, which is what to
-assert. From Packet 7 the same read **through a module `DbContext`** is a loud
+A raw connection with no announcement fails closed as the empty result, which is what
+to assert. `TenantContextMissingException` — the type shipped in Packet 3, in
+`LearnStack.SharedKernel/Errors/` — is thrown from Packet 7 on by the
+`TenantContextGuardInterceptor` Standards 05 and 11 describe, and since Packet 9 by
+`FeatureFlags` when asked with no tenant. The same read **through a module `DbContext`** is a loud
 `TenantContextMissingException` — the interceptor is an EF `DbCommandInterceptor`
 keyed on the marker a sanctioned setter stamps, so it never sees a raw
 `NpgsqlCommand`. The case above opens its own connection from `PostgresFixture` and
@@ -212,7 +212,8 @@ and neither write does. Without that case both `AS RESTRICTIVE` guards can be
 deleted with the suite green — measured, in Packet 6. Set the variable in the test
 itself; nothing sets it at runtime, because the flag derives from the actor's role
 and roles arrive in
-[Phase 02b](../../../docs/roadmap/phase-02b-events-auth.md). See
+[Phase 03](../../../docs/roadmap/phase-03-identity-admin.md), after Phase 02b's
+authenticated principal. See
 `TenancySchemaTests.TheTenantScopeHatchWidensReadsAndNeitherWrite`.
 
 The Packet 7 half of these cases goes through the **request**, and it needs no
@@ -339,7 +340,7 @@ cases assert on stay what they were.
 ## Validation
 
 - `dotnet test backend/tests/LearnStack.Tests.Integration` passes the new test.
-- The mandatory isolation pair (`TenantA_cannot_read_TenantB`,
+- The mandatory isolation pair (`Tenant_A_cannot_read_Tenant_B_data`,
   `Org_X_cannot_read_Org_Y_within_TenantA`) is present for every new
   tenant-owned / org-scoped entity.
 - A negative test confirms RLS-effective behaviour (empty result when