Enforce production module import boundaries
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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/")
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user