test(architecture): the rules assert against packages that exist

services/api/src/test/kotlin/net/blueshell/api/architecture/AccessArchitectureTest.kt

@@ -9,7 +9,6 @@ import com.tngtech.archunit.lang.SimpleConditionEvent
 import com.tngtech.archunit.lang.syntax.ArchRuleDefinition.classes
 import com.tngtech.archunit.lang.syntax.ArchRuleDefinition.methods
 import com.tngtech.archunit.lang.syntax.ArchRuleDefinition.noClasses
-import com.tngtech.archunit.library.dependencies.SlicesRuleDefinition.slices
 import net.blueshell.api.architecture.support.ArchJUnitTestBase
 import org.junit.jupiter.api.Test
 import org.springframework.security.access.prepost.PreAuthorize
@@ -24,6 +23,11 @@ import org.springframework.security.access.prepost.PreAuthorize
  * the auth module ever had, and architecture ADR-003 gives every module one domain folder
  * holding both. Its cross-module half is what module verification now checks; what remains
  * intra-module is a single folder with no layer boundary left to cross.
+ *
+ * `only the web layer reaches a domain module's web package` went the same way. It selected
+ * `net.blueshell.api.domain..web..`, a grouping level the flattening removed, and
+ * [CrossModuleWebAccessArchitectureTest] now states the stronger rule against today's packages:
+ * no module reaches another module's web package at all, whatever layer the reach comes from.
  */
 class AccessArchitectureTest : ArchJUnitTestBase(ArchitecturePackages.ROOT) {
 
@@ -155,21 +159,6 @@ class AccessArchitectureTest : ArchJUnitTestBase(ArchitecturePackages.ROOT) {
                 .because("ADR-016: Persistence can depend on query objects (ADR-015), but not services/handlers/validators")
         }
 
-    @Test
-    fun `only the web layer reaches a domain module's web package`(): Unit =
-        arch("Domain web packages are reached from the web layer only") {
-            noClasses()
-                .that().resideInAnyPackage(
-                    ArchitecturePackages.APPLICATION,
-                    ArchitecturePackages.INFRASTRUCTURE,
-                    ArchitecturePackages.PLATFORM
-                )
-                // The OpenAPI schema customizer documents a response type, so it names one.
-                .and(DescribedPredicate.not(openApiConfiguration))
-                .should().dependOnClassesThat().resideInAnyPackage(ArchitecturePackages.DOMAIN_WEB)
-                .because("ADR-016: controllers, request/response types and their mappers serve one endpoint; inner layers work with entities and commands")
-        }
-
     @Test
     fun `repositories do not depend on DTOs`(): Unit =
         arch("Repositories must not depend on DTOs") {
@@ -180,20 +169,6 @@ class AccessArchitectureTest : ArchJUnitTestBase(ArchitecturePackages.ROOT) {
                 .because("ADR-016: Persistence layer should not know about web DTOs")
         }
 
-    // NOTE: Cyclic dependency test disabled - known exception exists
-    // shared → domain.user.persistence.User for audit fields is documented in ADR-016 as acceptable
-    // The cycle is: domain → shared (normal) + shared → domain.user.persistence.User (for audit)
-    // This is a conscious architectural trade-off for audit field convenience
-    // TODO: Consider adding test with ignoreDependency() when ArchUnit API is clearer
-    // @Test
-    // fun `no cyclic dependencies by top level package`(): Unit =
-    //     arch("No cyclic dependencies between top-level packages") {
-    //         slices()
-    //             .matching("${ArchitecturePackages.ROOT}.(*)..")
-    //             .should().beFreeOfCycles()
-    //             .because("Cyclic dependencies make refactoring risky and coupling invisible")
-    //     }
-
     @Test
     fun `configuration does not depend on web controllers`(): Unit =
         arch("Configuration must not depend on controllers") {
@@ -288,10 +263,5 @@ class AccessArchitectureTest : ArchJUnitTestBase(ArchitecturePackages.ROOT) {
                     )
                 )
                 .`as`("a domain module's application package other than its query objects")
-
-        val openApiConfiguration: DescribedPredicate<JavaClass> =
-            JavaClass.Predicates.resideInAnyPackage(ArchitecturePackages.PLATFORM_CONFIG)
-                .and(JavaClass.Predicates.simpleNameContaining("OpenApi"))
-                .`as`("OpenAPI configuration")
     }
 }

services/api/src/test/kotlin/net/blueshell/api/architecture/ArchitecturePackages.kt

@@ -48,10 +48,14 @@ object ArchitecturePackages {
     const val REPOSITORY = "$ROOT..persistence.repository.."
     const val SPECIFICATION = "$ROOT..persistence.spec.."
 
-    /** Infrastructure - Cross-cutting concerns */
-    const val INFRASTRUCTURE = "$ROOT.infrastructure.."
-    const val SECURITY = "$ROOT.infrastructure.security.."
-    const val PERMISSION = "$ROOT.infrastructure.security.permission.."
+    /**
+     * Cross-cutting security. Architecture ADR-003 makes this a top-level module of its own;
+     * the `infrastructure` package these used to name was removed by the flattening.
+     */
+    const val SECURITY = "$ROOT.security.."
+
+    /** ADR-007: only the base and composite evaluator stay here, never a `*Permission`. */
+    const val PERMISSION = "$ROOT.security.permission.."
 
     /** Platform - Integration with external systems */
     const val PLATFORM = "$ROOT.platform.."

services/api/src/test/kotlin/net/blueshell/api/architecture/SpringPracticesArchitectureTest.kt

@@ -15,13 +15,10 @@ class SpringPracticesArchitectureTest : ArchJUnitTestBase(ArchitecturePackages.R
     @Test
     fun `no field injection in application code`(): Unit =
         arch("No @Autowired field injection - use constructor injection") {
+            // The old layer list named packages the ADR-003 flattening removed, so it selected
+            // a fraction of the code; constructor injection is wanted everywhere regardless.
             noFields()
-                .that().areDeclaredInClassesThat().resideInAnyPackage(
-                    ArchitecturePackages.WEB,
-                    ArchitecturePackages.APPLICATION,
-                    ArchitecturePackages.INFRASTRUCTURE,
-                    ArchitecturePackages.PLATFORM
-                )
+                .that().areDeclaredInClassesThat().resideInAnyPackage("${ArchitecturePackages.ROOT}..")
                 .should().beAnnotatedWith(Autowired::class.java)
                 .because("Constructor injection is preferred: immutable dependencies, simpler tests, clearer contracts")
         }

services/api/src/test/kotlin/net/blueshell/api/architecture/VendorClientArchitectureTest.kt

@@ -0,0 +1,111 @@
+package net.blueshell.api.architecture
+
+import net.blueshell.api.architecture.support.ArchJUnitTestBase
+import net.blueshell.api.architecture.support.ArchModules
+import org.assertj.core.api.Assertions.assertThat
+import org.junit.jupiter.api.Test
+
+/**
+ * API ADR-019: an external model does not enter a domain. A generated vendor client under
+ * `net.blueshell.clients` is that external model in its rawest form, so exactly one module owns
+ * each vendor and wraps it behind an interface the rest of the application speaks. Anyone else
+ * calls that interface.
+ *
+ * The record still describes the vendor wrappers as living in `platform/integration/{system}`,
+ * packages the architecture ADR-003 flattening removed; ownership is by module now, which is what
+ * [VENDOR_OWNERS] states. Modulith polices `net.blueshell.api` and stops at the module boundary,
+ * so nothing but this rule looks at a third-party import.
+ *
+ * The reaches that exist are pinned in [PINNED] rather than fixed here, the way
+ * [CrossModuleWebAccessArchitectureTest] pins its own. Each is a line in the file, so dropping one
+ * is a visible diff. Pinned at six reaches, all made by one class.
+ */
+class VendorClientArchitectureTest : ArchJUnitTestBase(ArchitecturePackages.ROOT) {
+
+    private companion object {
+        const val VENDOR_ROOT = "net.blueshell.clients"
+
+        /**
+         * The module that owns each vendor client and wraps it. A vendor absent from this map has
+         * no owner, which is itself a violation — adding a client means deciding who wraps it.
+         */
+        val VENDOR_OWNERS = mapOf(
+            "brevo" to "contact",
+            "discord" to "sync",
+        )
+
+        /**
+         * Vendor reaches from outside the owning module that existed when this rule landed, as
+         * `<reaching module> -> <vendor type>`.
+         */
+        val PINNED = setOf(
+            // DEBT. BrevoTargetStrategy drives ContactsApi for the list catalog and folder names
+            // while pushing membership through contact's ContactListAdapter — one integration
+            // behind two ports, one of them raw. Removing these means publishing the catalog side
+            // through contact :: api so cohort speaks only to the wrapper. Tracked separately.
+            "cohort -> net.blueshell.clients.brevo.api.ContactsApi",
+            "cohort -> net.blueshell.clients.brevo.model.GetContactsSortParameter",
+            "cohort -> net.blueshell.clients.brevo.model.GetFolder",
+            "cohort -> net.blueshell.clients.brevo.model.GetFolders200Response",
+            "cohort -> net.blueshell.clients.brevo.model.GetLists200Response",
+            "cohort -> net.blueshell.clients.brevo.model.GetLists200ResponseListsInner",
+        )
+    }
+
+    @Test
+    fun `only the owning module imports a vendor client`() {
+        val offenders = measureReaches()
+            .filterKeys { it !in PINNED }
+            .flatMap { (reach, origins) -> origins.map { "$reach   from $it" } }
+            .sorted()
+
+        assertThat(offenders)
+            .describedAs(
+                "API ADR-019: a generated vendor client belongs to the module that wraps it. Call " +
+                    "that module's published interface instead, or add the reach to PINNED if it " +
+                    "is being cleaned up separately",
+            )
+            .isEmpty()
+    }
+
+    @Test
+    fun `no pinned reach outlives the code that made it`() {
+        val measured = measureReaches().keys
+
+        val stale = PINNED.filterNot { it in measured }.sorted()
+
+        assertThat(stale)
+            .describedAs(
+                "these reaches are gone — drop them from PINNED so the ratchet cannot slip back",
+            )
+            .isEmpty()
+    }
+
+    /**
+     * Every `<module> -> <vendor type>` reach from outside the owning module, mapped to the classes
+     * that make it. A type directly under the base package belongs to no module and owns no vendor,
+     * so its reaches count too.
+     */
+    private fun measureReaches(): Map<String, Set<String>> {
+        val reaches = mutableMapOf<String, MutableSet<String>>()
+
+        importedClasses.forEach { origin ->
+            val originModule = ArchModules.moduleOf(origin)
+            origin.directDependenciesFromSelf.forEach { dependency ->
+                val target = dependency.targetClass
+                val vendor = vendorOf(target.packageName) ?: return@forEach
+                if (VENDOR_OWNERS[vendor] == originModule) return@forEach
+                reaches.getOrPut("${originModule ?: "<root>"} -> ${target.fullName}") { mutableSetOf() }
+                    .add(origin.fullName)
+            }
+        }
+
+        return reaches
+    }
+
+    private fun vendorOf(packageName: String): String? =
+        packageName.removePrefix("$VENDOR_ROOT.")
+            .takeIf { packageName.startsWith("$VENDOR_ROOT.") }
+            ?.substringBefore('.')
+            ?.takeIf { it.isNotEmpty() }
+}