Merge pull request #2351 from Molecule-AI/auto/issue-2344-architecture-lint

test(arch): codify 4 module boundaries as architecture tests (#2344)

workspace-server/internal/db/architecture_test.go

@@ -0,0 +1,63 @@
+package db_test
+
+// Architecture test (#2344): db is a leaf — DB pool + migrations + raw
+// SQL helpers, no business-logic dependencies. The DB layer must be
+// testable with sqlmock in isolation. If db starts importing handlers
+// or provisioner, every db unit test would need to bring up that
+// subsystem, and the layering becomes circular.
+//
+// If this test fails: you put business logic in the db package. Move
+// it to a higher-tier package that imports db, not the reverse.
+
+import (
+	"go/parser"
+	"go/token"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+const moduleInternalPrefix = "github.com/Molecule-AI/molecule-monorepo/platform/internal/"
+
+func TestDBHasNoInternalDependencies(t *testing.T) {
+	t.Parallel()
+	for path, file := range listImports(t, ".") {
+		if strings.HasPrefix(path, moduleInternalPrefix) {
+			t.Errorf(
+				"db must not import other internal packages "+
+					"(found %q in %s) — db is the foundation layer and a "+
+					"reverse dep creates a cycle (everything imports db). "+
+					"See workspace-server/internal/db/architecture_test.go.",
+				path, file,
+			)
+		}
+	}
+}
+
+func listImports(t *testing.T, dir string) map[string]string {
+	t.Helper()
+	fset := token.NewFileSet()
+	entries, err := os.ReadDir(dir)
+	if err != nil {
+		t.Fatalf("read %s: %v", dir, err)
+	}
+	out := make(map[string]string)
+	for _, e := range entries {
+		name := e.Name()
+		if e.IsDir() || !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") {
+			continue
+		}
+		f, err := parser.ParseFile(fset, filepath.Join(dir, name), nil, parser.ImportsOnly)
+		if err != nil {
+			t.Fatalf("parse %s: %v", name, err)
+		}
+		for _, imp := range f.Imports {
+			path := strings.Trim(imp.Path.Value, "\"")
+			if _, seen := out[path]; !seen {
+				out[path] = name
+			}
+		}
+	}
+	return out
+}

workspace-server/internal/models/architecture_test.go

@@ -0,0 +1,63 @@
+package models_test
+
+// Architecture test (#2344): models is a leaf — it carries pure type
+// definitions and must not import any other internal/* package. Almost
+// every package in workspace-server depends on models; if models grew a
+// reverse dep, the import graph would cycle.
+//
+// If this test fails: you put behavior inside models. Move the behavior
+// to whichever package actually owns it (handlers, provisioner, db, …)
+// and have *that* package import models, not the reverse.
+
+import (
+	"go/parser"
+	"go/token"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+const moduleInternalPrefix = "github.com/Molecule-AI/molecule-monorepo/platform/internal/"
+
+func TestModelsHasNoInternalDependencies(t *testing.T) {
+	t.Parallel()
+	for path, file := range listImports(t, ".") {
+		if strings.HasPrefix(path, moduleInternalPrefix) {
+			t.Errorf(
+				"models must not import other internal packages "+
+					"(found %q in %s) — models is the pure-types leaf and any "+
+					"reverse dep creates an import cycle since most packages "+
+					"depend on models. See workspace-server/internal/models/architecture_test.go.",
+				path, file,
+			)
+		}
+	}
+}
+
+func listImports(t *testing.T, dir string) map[string]string {
+	t.Helper()
+	fset := token.NewFileSet()
+	entries, err := os.ReadDir(dir)
+	if err != nil {
+		t.Fatalf("read %s: %v", dir, err)
+	}
+	out := make(map[string]string)
+	for _, e := range entries {
+		name := e.Name()
+		if e.IsDir() || !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") {
+			continue
+		}
+		f, err := parser.ParseFile(fset, filepath.Join(dir, name), nil, parser.ImportsOnly)
+		if err != nil {
+			t.Fatalf("parse %s: %v", name, err)
+		}
+		for _, imp := range f.Imports {
+			path := strings.Trim(imp.Path.Value, "\"")
+			if _, seen := out[path]; !seen {
+				out[path] = name
+			}
+		}
+	}
+	return out
+}

workspace-server/internal/provisioner/architecture_test.go

@@ -0,0 +1,80 @@
+package provisioner_test
+
+// Architecture test (#2344): provisioner is below handlers/router in
+// the layer hierarchy. handlers wires provisioner into HTTP routes;
+// the reverse direction (provisioner reaching back into handlers or
+// the router) creates a cycle and tangles infra-orchestration with
+// transport.
+//
+// Note: provisioner CURRENTLY imports db (for the runtime-image
+// lookup). That's a known coupling — see PR #2276 review thread on
+// where image resolution should live. The narrower rule we enforce
+// here is "no upward import to handlers/router," which is the harder
+// rule to keep clean.
+//
+// If this test fails: you reached "up" the stack. Pass whatever you
+// need from handlers down through a constructor parameter or a
+// function-typed callback instead of importing the package directly.
+
+import (
+	"go/parser"
+	"go/token"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+const moduleInternalPrefix = "github.com/Molecule-AI/molecule-monorepo/platform/internal/"
+
+var provisionerForbiddenImports = []string{
+	moduleInternalPrefix + "handlers",
+	moduleInternalPrefix + "router",
+}
+
+func TestProvisionerDoesNotImportUpstreamLayers(t *testing.T) {
+	t.Parallel()
+	imports := listImports(t, ".")
+	for path, file := range imports {
+		for _, forbidden := range provisionerForbiddenImports {
+			if path == forbidden || strings.HasPrefix(path, forbidden+"/") {
+				t.Errorf(
+					"provisioner must not import %q (found in %s) — "+
+						"provisioner sits below handlers/router in the layer "+
+						"hierarchy and a reverse dep creates a cycle. Pass "+
+						"what you need down via constructor params or "+
+						"function-typed callbacks. See workspace-server/internal/"+
+						"provisioner/architecture_test.go.",
+					path, file,
+				)
+			}
+		}
+	}
+}
+
+func listImports(t *testing.T, dir string) map[string]string {
+	t.Helper()
+	fset := token.NewFileSet()
+	entries, err := os.ReadDir(dir)
+	if err != nil {
+		t.Fatalf("read %s: %v", dir, err)
+	}
+	out := make(map[string]string)
+	for _, e := range entries {
+		name := e.Name()
+		if e.IsDir() || !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") {
+			continue
+		}
+		f, err := parser.ParseFile(fset, filepath.Join(dir, name), nil, parser.ImportsOnly)
+		if err != nil {
+			t.Fatalf("parse %s: %v", name, err)
+		}
+		for _, imp := range f.Imports {
+			path := strings.Trim(imp.Path.Value, "\"")
+			if _, seen := out[path]; !seen {
+				out[path] = name
+			}
+		}
+	}
+	return out
+}

workspace-server/internal/wsauth/architecture_test.go

@@ -0,0 +1,68 @@
+package wsauth_test
+
+// Architecture test (#2344): wsauth is a leaf package — it must not import
+// any other internal/* package. The auth layer is below business logic;
+// importing handlers, db, or any cousin package would force every wsauth
+// test to spin up that subsystem, defeating the unit-test boundary that
+// makes the auth code reviewable.
+//
+// If this test fails: you added an import that crosses a layer. Either
+// move the dependency the other direction (consumer wires wsauth into
+// itself), accept the boundary by inlining what you need, or — if the
+// new coupling is genuinely correct — explicitly update this test with
+// the new allowed import + a comment explaining why.
+
+import (
+	"go/parser"
+	"go/token"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+const moduleInternalPrefix = "github.com/Molecule-AI/molecule-monorepo/platform/internal/"
+
+func TestWsauthHasNoInternalDependencies(t *testing.T) {
+	t.Parallel()
+	for path, file := range listImports(t, ".") {
+		if strings.HasPrefix(path, moduleInternalPrefix) {
+			t.Errorf(
+				"wsauth must not import other internal packages "+
+					"(found %q in %s) — wsauth is the auth leaf and must stay "+
+					"unit-testable without spinning up other subsystems. "+
+					"See workspace-server/internal/wsauth/architecture_test.go for context.",
+				path, file,
+			)
+		}
+	}
+}
+
+// listImports returns import-path → first-file-where-seen for non-test
+// .go files in dir. Used by every architecture_test.go in this tree.
+func listImports(t *testing.T, dir string) map[string]string {
+	t.Helper()
+	fset := token.NewFileSet()
+	entries, err := os.ReadDir(dir)
+	if err != nil {
+		t.Fatalf("read %s: %v", dir, err)
+	}
+	out := make(map[string]string)
+	for _, e := range entries {
+		name := e.Name()
+		if e.IsDir() || !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") {
+			continue
+		}
+		f, err := parser.ParseFile(fset, filepath.Join(dir, name), nil, parser.ImportsOnly)
+		if err != nil {
+			t.Fatalf("parse %s: %v", name, err)
+		}
+		for _, imp := range f.Imports {
+			path := strings.Trim(imp.Path.Value, "\"")
+			if _, seen := out[path]; !seen {
+				out[path] = name
+			}
+		}
+	}
+	return out
+}