diff --git a/docs/internal/modules.md b/docs/internal/modules.md index 6c8993b..f8124a7 100644 --- a/docs/internal/modules.md +++ b/docs/internal/modules.md @@ -181,8 +181,9 @@ payload rules are defined in the ## Production Registration -The CLI allocates one complete framework registry set and one LLM asset -registry. It invokes `internal/modules/generic/register`, +Production composition occurs through family registrars. The CLI allocates one +complete framework registry set and one LLM asset registry. It invokes +`internal/modules/generic/register`, `internal/modules/seriatim/register`, and `internal/modules/dnd/register` in that order, then exposes the matching catalog for resolution. The generic and Seriatim registrars own their production leaf registrations. The D&D registrar @@ -194,8 +195,13 @@ packages directly. A concrete family's `register` package is its composition point for specializing reusable generic implementations, while the generic registrar composes only generic children. -Framework packages must not import production extensions. Tests may compose -registries and catalogs directly with fakes. +Core and framework production packages do not import production extensions. +CLI production code imports only exact family registrar packages. Compatibility +tests in the CLI, core, and framework trees may import roots and implementation +leaves directly. White-box tests within module families retain the production +family boundaries. `internal/modules/integration` is test infrastructure: its +black-box tests may compose multiple families, but it is not a production +module family or production dependency target. ## Adding An Extension diff --git a/internal/modules/import_boundaries_test.go b/internal/modules/import_boundaries_test.go index 50cb8fa..3187d04 100644 --- a/internal/modules/import_boundaries_test.go +++ b/internal/modules/import_boundaries_test.go @@ -155,6 +155,100 @@ func TestImportBoundaryRules(t *testing.T) { sourcePackage: "cli", importPath: moduleImportPrefix + "almanac/register", }, + { + name: "production CLI cannot import family root", + filename: "internal/cli/catalog.go", + sourcePackage: "cli", + importPath: moduleImportPrefix + "almanac", + wantError: true, + }, + { + name: "production CLI cannot import concrete leaf", + filename: "internal/cli/catalog.go", + sourcePackage: "cli", + importPath: moduleImportPrefix + "almanac/extract/events", + wantError: true, + }, + { + name: "production CLI cannot import generic leaf", + filename: "internal/cli/catalog.go", + sourcePackage: "cli", + importPath: moduleImportPrefix + "generic/output/json", + wantError: true, + }, + { + name: "production CLI cannot import non-registrar package", + filename: "internal/cli/catalog.go", + sourcePackage: "cli", + importPath: moduleImportPrefix + "almanac/register/helpers", + wantError: true, + }, + { + name: "CLI test may import concrete leaf", + filename: "internal/cli/compatibility_test.go", + sourcePackage: "cli", + importPath: moduleImportPrefix + "almanac/extract/events", + }, + { + name: "framework production cannot import concrete module", + filename: "internal/framework/pipeline/runner.go", + sourcePackage: "pipeline", + importPath: moduleImportPrefix + "almanac/extract/events", + wantError: true, + }, + { + name: "framework production cannot import generic module", + filename: "internal/framework/pipeline/runner.go", + sourcePackage: "pipeline", + importPath: moduleImportPrefix + "generic/normalize/noop", + wantError: true, + }, + { + name: "core production cannot import concrete module", + filename: "internal/core/source/source.go", + sourcePackage: "source", + importPath: moduleImportPrefix + "almanac", + wantError: true, + }, + { + name: "core production cannot import generic module", + filename: "internal/core/source/source.go", + sourcePackage: "source", + importPath: moduleImportPrefix + "generic/chunk/units", + wantError: true, + }, + { + name: "framework test may import module implementation", + filename: "internal/framework/pipeline/compatibility_test.go", + sourcePackage: "pipeline", + importPath: moduleImportPrefix + "generic/chunk/units", + }, + { + name: "core test may import module implementation", + filename: "internal/core/source/compatibility_test.go", + sourcePackage: "source", + importPath: moduleImportPrefix + "almanac", + }, + { + name: "module production cannot import integration infrastructure", + filename: "internal/modules/almanac/register/register.go", + sourcePackage: "register", + importPath: moduleImportPrefix + "integration", + wantError: true, + }, + { + name: "non-module production cannot import integration infrastructure", + filename: "cmd/notarius/main.go", + sourcePackage: "main", + importPath: moduleImportPrefix + "integration/helpers", + wantError: true, + }, + { + name: "test may import integration infrastructure", + filename: "internal/cli/compatibility_test.go", + sourcePackage: "cli", + importPath: moduleImportPrefix + "integration", + }, { name: "black-box integration test may compose families", filename: "internal/modules/integration/example_test.go", @@ -183,6 +277,9 @@ func TestImportBoundaryRules(t *testing.T) { if tt.wantError && err == nil { t.Fatal("validateImport() error = nil, want boundary violation") } + if tt.wantError && (!strings.Contains(err.Error(), tt.filename) || !strings.Contains(err.Error(), tt.importPath)) { + t.Fatalf("validateImport() error = %q, want importing file and import target", err) + } if !tt.wantError && err != nil { t.Fatalf("validateImport() error = %v, want nil", err) } @@ -206,43 +303,84 @@ func checkImportBoundaries(repositoryRoot string, filename string) error { return fmt.Errorf("parse import in %s: %w", relative, err) } if err := validateImport(relative, parsed.Name.Name, importPath); err != nil { - return fmt.Errorf("%s imports %s: %w", relative, importPath, err) + return err } } return nil } func validateImport(filename string, sourcePackage string, importPath string) error { - if isBlackBoxIntegrationTest(filename, sourcePackage) { + target, ok := moduleTargetForImport(importPath) + if !ok { return nil } - targetFamily, targetChild := moduleFamilyForImport(importPath) - if targetFamily == "" { - return nil + isTest := strings.HasSuffix(filename, "_test.go") + if target.integration && !isTest { + return importBoundaryViolation(filename, importPath, "module integration infrastructure is not a production dependency target") } if isIntegrationFile(filename) { - return fmt.Errorf("module integration composition is allowed only in black-box tests") + if isBlackBoxIntegrationTest(filename, sourcePackage) { + return nil + } + return importBoundaryViolation(filename, importPath, "module integration composition is allowed only in black-box tests") + } + if !isTest && (strings.HasPrefix(filename, "internal/framework/") || strings.HasPrefix(filename, "internal/core/")) { + return importBoundaryViolation(filename, importPath, "core and framework production code must not import module implementations") + } + if !isTest && strings.HasPrefix(filename, "internal/cli/") { + if target.registrar { + return nil + } + return importBoundaryViolation(filename, importPath, "CLI production code may import only exact module family registrar packages") } sourceFamily, sourceRoot, sourceRegistrar := moduleFamilyForFile(filename) if sourceFamily == "" { return nil } - if sourceRoot && sourceFamily == targetFamily && targetChild { - return fmt.Errorf("family root must not import child packages") + if sourceRoot && sourceFamily == target.family && target.child { + return importBoundaryViolation(filename, importPath, "family root must not import child packages") } - if sourceFamily == targetFamily { + if sourceFamily == target.family { return nil } if sourceFamily == "generic" { - return fmt.Errorf("generic family must not import concrete family %q", targetFamily) + return importBoundaryViolation(filename, importPath, fmt.Sprintf("generic family must not import concrete family %q", target.family)) } - if targetFamily == "generic" { + if target.family == "generic" { if sourceRegistrar { return nil } - return fmt.Errorf("concrete family %q may import generic implementations only from its registrar", sourceFamily) + return importBoundaryViolation(filename, importPath, fmt.Sprintf("concrete family %q may import generic implementations only from its registrar", sourceFamily)) } - return fmt.Errorf("concrete family %q must not import concrete family %q", sourceFamily, targetFamily) + return importBoundaryViolation(filename, importPath, fmt.Sprintf("concrete family %q must not import concrete family %q", sourceFamily, target.family)) +} + +type moduleImportTarget struct { + family string + child bool + registrar bool + integration bool +} + +func moduleTargetForImport(importPath string) (moduleImportTarget, bool) { + if !strings.HasPrefix(importPath, moduleImportPrefix) { + return moduleImportTarget{}, false + } + remainder := strings.TrimPrefix(importPath, moduleImportPrefix) + parts := strings.Split(remainder, "/") + if len(parts) == 0 || parts[0] == "" { + return moduleImportTarget{}, false + } + return moduleImportTarget{ + family: parts[0], + child: len(parts) > 1, + registrar: len(parts) == 2 && parts[1] == "register", + integration: parts[0] == "integration", + }, true +} + +func importBoundaryViolation(filename string, importPath string, rule string) error { + return fmt.Errorf("import boundary violation: %s imports %s: %s", filename, importPath, rule) } func moduleFamilyForFile(filename string) (family string, root bool, registrar bool) { @@ -258,18 +396,6 @@ func moduleFamilyForFile(filename string) (family string, root bool, registrar b return parts[0], len(parts) == 2, len(parts) > 2 && parts[1] == "register" } -func moduleFamilyForImport(importPath string) (family string, child bool) { - if !strings.HasPrefix(importPath, moduleImportPrefix) { - return "", false - } - remainder := strings.TrimPrefix(importPath, moduleImportPrefix) - parts := strings.Split(remainder, "/") - if len(parts) == 0 || parts[0] == "" || parts[0] == "integration" { - return "", false - } - return parts[0], len(parts) > 1 -} - func isIntegrationFile(filename string) bool { return strings.HasPrefix(filename, "internal/modules/integration/") }