feat(mcp): check_env struct-level granularity via composes selectors

Patch to v1.1.1. check_env could only reason at module granularity, so a var that lives on one struct of a multi-struct module (e.g. EINHERJAR_SERVER_CORS_ORIGINS on web.Config, not web/server.Config) couldn't be flagged dead when the app composes the sibling struct. Adds struct-level selectors to close that gap. internal/tools (check_env): - New `composes` input: exact config structs as module/subpackage/Struct selectors (e.g. web/server/Config, db-postgres/Config, core/logz/Config), alongside or instead of the coarse `modules` list. - not-composed now reasons over the union of composed vars (from modules AND struct selectors), so it surfaces struct-level dead vars. Coarse modules:["web"] keeps the prior behavior. - parseStructSelector helper; errors on malformed or non-existent selectors. Tests: struct-level dead-var catch (CORS flagged with web/server/Config but not with module web), bad-selector errors; existing calls updated to the new arg. Version bumped to v1.1.1 (badge + serverVersion). No dependency changes.
2026-08-07 19:38:00 -06:00
parent 850b63607c
commit cff5a7b4c8
6 changed files with 135 additions and 24 deletions
@@ -47,8 +47,8 @@ type Rule struct {
 type Context struct {
 	Fset    *token.FileSet
 	File    *ast.File
-	Imports map[string]string   // path → local name (e.g. "code.nochebuena.dev/einherjar/core/launcher" → "launcher")
-	Calls   []CallSite          // every function call in the file
+	Imports map[string]string // path → local name (e.g. "code.nochebuena.dev/einherjar/core/launcher" → "launcher")
+	Calls   []CallSite        // every function call in the file
 }

 // CallSite is a recorded function call. Func is the textual form
@@ -12,8 +12,9 @@ import (
 )

 type checkEnvInput struct {
-	Env     string   `json:"env" jsonschema:"the .env or .env.example contents to check (KEY=value lines; comments and blanks ignored)."`
-	Modules []string `json:"modules,omitempty" jsonschema:"the framework modules whose component configs the app composes, e.g. [\"db-postgres\",\"web\"]. Their required vars must be present; omit to only validate that EINHERJAR_* names are real."`
+	Env      string   `json:"env" jsonschema:"the .env or .env.example contents to check (KEY=value lines; comments and blanks ignored)."`
+	Modules  []string `json:"modules,omitempty" jsonschema:"module-granularity: the framework modules whose component configs the app composes, e.g. [\"db-postgres\",\"web\"]. Every env var in the module counts as composed. Convenient but coarse — a module can declare more than one config struct."`
+	Composes []string `json:"composes,omitempty" jsonschema:"struct-granularity (preferred when precise): the exact config structs the app composes, as module/subpackage/Struct selectors, e.g. [\"web/server/Config\",\"db-postgres/Config\",\"core/logz/Config\"]. Use module/Struct when the config is at the module root. This catches struct-level dead vars, e.g. flagging EINHERJAR_SERVER_CORS_ORIGINS (which lives on web.Config, not web/server/Config)."`
 }

 type envFinding struct {
@@ -34,10 +35,11 @@ func registerCheckEnv(s *mcp.Server, idx *index.Index) {
 	mcp.AddTool(s, &mcp.Tool{
 		Name: "check_env",
 		Description: "Check a .env / .env.example against the framework's real env vars. Flags EINHERJAR_* names that don't exist (invented or misspelled, e.g. EINHERJAR_PG_DATABASE), " +
-			"and — when you pass the modules the app composes — required vars that are missing, plus framework vars set for a module you don't compose (dead vars). " +
+			"and — when you declare what the app composes (via `modules` for coarse or `composes` for exact struct selectors) — required vars that are missing, plus framework vars that nothing composed reads (dead vars). " +
+			"Prefer `composes` (e.g. web/server/Config) for precision; it catches struct-level dead vars that module granularity can't. " +
 			"This is the enforcement half of building a correct global config: get_scaffold emits an accurate .env.example, get_config_env lists the truth, and check_env catches drift in an existing file.",
 	}, func(ctx context.Context, req *mcp.CallToolRequest, args checkEnvInput) (*mcp.CallToolResult, checkEnvOutput, error) {
-		findings, err := checkEnvVars(idx, args.Env, args.Modules)
+		findings, err := checkEnvVars(idx, args.Env, args.Modules, args.Composes)
 		if err != nil {
 			return errorResult(err.Error()), checkEnvOutput{}, nil
 		}
@@ -53,14 +55,17 @@ func registerCheckEnv(s *mcp.Server, idx *index.Index) {
 }

 // checkEnvVars is the pure core of check_env: given the framework index, an env
-// file body, and the modules the app composes, it returns the drift findings.
-func checkEnvVars(idx *index.Index, env string, modules []string) ([]envFinding, error) {
+// file body, the modules the app composes (coarse), and the exact config structs
+// it composes (precise selectors), it returns the drift findings. A var counts as
+// "composed" when it is declared by any listed module or any listed struct.
+func checkEnvVars(idx *index.Index, env string, modules, composes []string) ([]envFinding, error) {
 	present := parseEnvKeys(env)
 	known := envspec.KnownNames(idx)

-	// Expected vars from the composed modules, and the module each real var belongs to.
+	// The set of composed env vars (name -> Var), unioned from module-granularity
+	// and struct-granularity selectors. Its keyset is the "what this app reads" set.
 	expected := map[string]envspec.Var{}
-	composed := map[string]bool{}
+	scoped := false
 	for _, mod := range modules {
 		mod = strings.TrimSpace(mod)
 		if mod == "" {
@@ -69,11 +74,33 @@ func checkEnvVars(idx *index.Index, env string, modules []string) ([]envFinding,
 		if idx.FindModule(mod) == nil {
 			return nil, fmt.Errorf("module not found: %s", mod)
 		}
-		composed[mod] = true
+		scoped = true
 		for _, v := range envspec.ForModule(idx, mod) {
 			expected[v.Name] = v
 		}
 	}
+	for _, sel := range composes {
+		sel = strings.TrimSpace(sel)
+		if sel == "" {
+			continue
+		}
+		mod, sub, name, err := parseStructSelector(sel)
+		if err != nil {
+			return nil, err
+		}
+		if idx.FindModule(mod) == nil {
+			return nil, fmt.Errorf("module not found in selector %q: %s", sel, mod)
+		}
+		vars := envspec.FindStruct(idx, mod, sub, name)
+		if vars == nil {
+			return nil, fmt.Errorf("struct not found for selector %q (want an existing module/subpackage/Struct)", sel)
+		}
+		scoped = true
+		for _, v := range vars {
+			expected[v.Name] = v
+		}
+	}
+
 	varModule := map[string]string{}
 	for _, v := range envspec.All(idx) {
 		varModule[v.Name] = v.Module
@@ -110,17 +137,22 @@ func checkEnvVars(idx *index.Index, env string, modules []string) ([]envFinding,
 		}
 	}

-	// 3. Real framework var set for a module the app doesn't compose → dead.
-	if len(composed) > 0 {
+	// 3. A real framework var set in the file that nothing composed reads → dead.
+	//    With struct selectors this catches struct-level dead vars (a var whose
+	//    module is partly composed but whose specific struct is not).
+	if scoped {
 		for name := range present {
 			mod, real := varModule[name]
-			if !real || composed[mod] {
+			if !real {
+				continue
+			}
+			if _, composed := expected[name]; composed {
 				continue
 			}
 			findings = append(findings, envFinding{
 				Severity: "info", Kind: "not-composed", Var: name,
-				Message: name + " belongs to module " + mod + ", which this app does not list as composed — it will be ignored",
-				Hint:    "Either compose " + mod + "'s Config, or drop the var. (This is how EINHERJAR_SERVER_CORS_ORIGINS is dead unless you compose web.Config.)",
+				Message: name + " is a real " + mod + " var, but none of the composed configs read it — it will be ignored",
+				Hint:    "Compose the config that declares it, or drop the var. (E.g. EINHERJAR_SERVER_CORS_ORIGINS lives on web.Config; it is dead if you compose web/server/Config instead.)",
 			})
 		}
 	}
@@ -137,6 +169,21 @@ func checkEnvVars(idx *index.Index, env string, modules []string) ([]envFinding,
 	return findings, nil
 }

+// parseStructSelector splits a "module/Struct" or "module/subpackage/Struct"
+// selector. The struct name is always the last segment; a two-part selector
+// targets a module-root config (empty sub-package).
+func parseStructSelector(sel string) (module, subPackage, name string, err error) {
+	parts := strings.Split(sel, "/")
+	switch len(parts) {
+	case 2:
+		return parts[0], "", parts[1], nil
+	case 3:
+		return parts[0], parts[1], parts[2], nil
+	default:
+		return "", "", "", fmt.Errorf("invalid struct selector %q (want module/Struct or module/subpackage/Struct)", sel)
+	}
+}
+
 // parseEnvKeys returns the set of assigned variable names in an env file body,
 // tolerating `export KEY=`, leading whitespace, comments, and blank lines.
 func parseEnvKeys(body string) map[string]bool {
@@ -75,7 +75,7 @@ func TestRenderEnvExample(t *testing.T) {
 func TestCheckEnvUnknownVar(t *testing.T) {
 	idx := loadRealIndex(t)
 	env := "EINHERJAR_PG_DATABASE=x\nEINHERJAR_SERVER_ADDR=:8080\nAPP_ENV=local\n"
-	findings, err := checkEnvVars(idx, env, nil)
+	findings, err := checkEnvVars(idx, env, nil, nil)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -101,7 +101,7 @@ func TestCheckEnvUnknownVar(t *testing.T) {
 func TestCheckEnvMissingRequired(t *testing.T) {
 	idx := loadRealIndex(t)
 	// Compose db-postgres but set none of its required vars.
-	findings, err := checkEnvVars(idx, "APP_ENV=local\n", []string{"db-postgres"})
+	findings, err := checkEnvVars(idx, "APP_ENV=local\n", []string{"db-postgres"}, nil)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -122,7 +122,7 @@ func TestCheckEnvNotComposed(t *testing.T) {
 	idx := loadRealIndex(t)
 	// A real MinIO var while only composing db-postgres → dead (not-composed).
 	env := "EINHERJAR_MINIO_ENDPOINT=x\nEINHERJAR_PG_HOST=h\nEINHERJAR_PG_USER=u\nEINHERJAR_PG_PASSWORD=p\nEINHERJAR_PG_NAME=n\n"
-	findings, err := checkEnvVars(idx, env, []string{"db-postgres"})
+	findings, err := checkEnvVars(idx, env, []string{"db-postgres"}, nil)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -141,7 +141,7 @@ func TestCheckEnvClean(t *testing.T) {
 	idx := loadRealIndex(t)
 	// The scaffold's own output, checked against the modules it composes, is clean.
 	env := renderEnvExample(idx, "myapp")
-	findings, err := checkEnvVars(idx, env, []string{"core", "web", "db-postgres"})
+	findings, err := checkEnvVars(idx, env, []string{"core", "web", "db-postgres"}, nil)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -154,7 +154,52 @@ func TestCheckEnvClean(t *testing.T) {

 func TestCheckEnvUnknownModule(t *testing.T) {
 	idx := loadRealIndex(t)
-	if _, err := checkEnvVars(idx, "", []string{"nope"}); err == nil {
+	if _, err := checkEnvVars(idx, "", []string{"nope"}, nil); err == nil {
 		t.Error("expected error for unknown module")
 	}
 }
+
+// TestCheckEnvStructLevel proves the struct-granularity path catches a dead var
+// that module granularity cannot: EINHERJAR_SERVER_CORS_ORIGINS lives on
+// web.Config, so an app composing web/server/Config (not web.Config) never reads
+// it — module "web" would hide that, struct selectors surface it.
+func TestCheckEnvStructLevel(t *testing.T) {
+	idx := loadRealIndex(t)
+	env := "EINHERJAR_SERVER_CORS_ORIGINS=*\nEINHERJAR_PG_HOST=h\nEINHERJAR_PG_USER=u\nEINHERJAR_PG_PASSWORD=p\nEINHERJAR_PG_NAME=n\n"
+
+	// struct-level: server/Config has no CORS → CORS flagged as dead.
+	structF, err := checkEnvVars(idx, env, nil, []string{"web/server/Config", "db-postgres/Config"})
+	if err != nil {
+		t.Fatal(err)
+	}
+	dead := false
+	for _, f := range structF {
+		if f.Kind == "not-composed" && f.Var == "EINHERJAR_SERVER_CORS_ORIGINS" {
+			dead = true
+		}
+	}
+	if !dead {
+		t.Errorf("struct-level: expected EINHERJAR_SERVER_CORS_ORIGINS flagged not-composed; got %+v", structF)
+	}
+
+	// module-level ["web"] keeps CORS silent (documents the coarse behavior).
+	modF, err := checkEnvVars(idx, env, []string{"web", "db-postgres"}, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, f := range modF {
+		if f.Var == "EINHERJAR_SERVER_CORS_ORIGINS" {
+			t.Errorf("module-level should not flag CORS (it lives in module web), got %+v", f)
+		}
+	}
+}
+
+func TestCheckEnvBadSelector(t *testing.T) {
+	idx := loadRealIndex(t)
+	if _, err := checkEnvVars(idx, "", nil, []string{"web"}); err == nil {
+		t.Error("expected error for a one-segment selector")
+	}
+	if _, err := checkEnvVars(idx, "", nil, []string{"web/server/Nope"}); err == nil {
+		t.Error("expected error for a non-existent struct selector")
+	}
+}