🎨✅ Address review comments

Signed-off-by: Evaline Ju <69598118+evaline-ju@users.noreply.github.com>

Author: evaline-ju

.github/workflows/ci.yaml

@@ -101,10 +101,10 @@ jobs:
         run: go build -v ./...
 
       # The authbridge-lite image is this same authbridge-proxy binary built
-      # with exclude_plugin_* tags (only jwt-validation + token-exchange).
+      # with the trimmed plugin set derived by authbridge/scripts/lite-tags.
       # Build AND test that tag set on every PR — build.yaml only exercises
       # it on tag/main pushes, and this guards against lite-only regressions.
-      - name: Build + test lite variant (exclude_plugin_* tags)
+      - name: Build + test lite variant
         if: matrix.binary == 'authbridge-proxy'
         run: |
           TAGS=$(go -C ../../scripts/lite-tags run .)

authbridge/cmd/README.md

@@ -76,6 +76,6 @@ inbound mechanism, and the preset fills only that one's address.
 - **Size-constrained, no protocol-aware events needed**: use the
   `authbridge-lite` image — the `authbridge-proxy` binary built with
   `exclude_plugin_*` tags from `authbridge/scripts/lite-tags` (trimmed
-  plugin set). Same listener layout, but abctl will only see denial
-  events and basic auth-level invocations for the plugins the trimmed
-  set drops.
+  plugin set). Same listener layout, but without parsers/OPA — abctl
+  will only see denial events and basic auth-level invocations, not
+  full A2A/MCP/Inference protocol context.

authbridge/scripts/lite-tags/main.go

@@ -6,10 +6,9 @@
 // New default-on plugins are excluded from lite automatically. To keep one
 // in lite, add its build-tag suffix to liteKeep.
 //
-// Usage:
-//
-//	go -C authbridge/scripts/lite-tags run .           # default path, CWD-independent
-//	go -C authbridge/scripts/lite-tags run . <dir>     # override the plugins dir
+// Usage: every call site uses `go -C <path>/scripts/lite-tags run .`, which
+// chdirs to this module before exec so the default plugins path resolves
+// correctly. Callers can pass an override path as the first argument.
 package main
 
 import (
@@ -31,13 +30,18 @@ var liteKeep = map[string]bool{
 	"staticinject":        true,
 }
 
-// defaultPluginsDir is relative to this script's module directory, so
-// `go -C authbridge/scripts/lite-tags run .` finds it regardless of the
-// caller's CWD. Callers with an unusual layout can pass a plugins path
-// as the first argument.
+// defaultPluginsDir is resolved relative to the process working directory
+// (filepath.Glob and os.ReadFile know nothing about module layout). It
+// works because every call site uses `go -C <this-module> run .`, which
+// chdirs before exec.
 const defaultPluginsDir = "../../cmd/authbridge-proxy"
 
-var buildTagPattern = regexp.MustCompile(`^//go:build !exclude_plugin_(\S+)$`)
+// buildTagPattern matches `!exclude_plugin_<name>` inside a //go:build
+// directive. The pattern does NOT anchor to end-of-line so compound
+// directives like `//go:build !exclude_plugin_opa && !nocgo` still yield
+// the exclude tag — otherwise the plugin would silently stay in the lite
+// build.
+var buildTagPattern = regexp.MustCompile(`^//go:build\s+!exclude_plugin_(\w+)`)
 
 func main() {
 	dir := defaultPluginsDir
@@ -63,6 +67,11 @@ func discover(dir string) ([]string, error) {
 
 	var tags []string
 	for _, path := range matches {
+		// Skip test files: `plugins_*_test.go` matches the same glob but
+		// isn't a plugin build directive.
+		if strings.HasSuffix(path, "_test.go") {
+			continue
+		}
 		name, err := extractExcludeSuffix(path)
 		if err != nil {
 			return nil, err
@@ -72,6 +81,11 @@ func discover(dir string) ([]string, error) {
 		}
 		tags = append(tags, "exclude_plugin_"+name)
 	}
+	// Fail closed if no tags were derived: an empty CSV would produce
+	// `go build -tags ""` and silently ship a full binary as "lite".
+	if len(tags) == 0 {
+		return nil, fmt.Errorf("no exclude tags derived from %s: every default-on plugin is in liteKeep, or the build-tag convention changed", dir)
+	}
 	sort.Strings(tags)
 	return tags, nil
 }

authbridge/scripts/lite-tags/main_test.go

@@ -0,0 +1,75 @@
+package main
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// TestDiscover_ExpectedTags fails when the shipped lite tag set changes —
+// a plugin added/removed or liteKeep edited. Deliberate changes update
+// the want string; unintended changes are caught here.
+func TestDiscover_ExpectedTags(t *testing.T) {
+	tags, err := discover(defaultPluginsDir)
+	if err != nil {
+		t.Fatalf("discover: %v", err)
+	}
+	got := strings.Join(tags, ",")
+	want := "exclude_plugin_a2aparser,exclude_plugin_ibac,exclude_plugin_inferenceparser,exclude_plugin_mcpparser,exclude_plugin_opa,exclude_plugin_sparc,exclude_plugin_tokenbroker,exclude_plugin_toolprune"
+	if got != want {
+		t.Errorf("output changed\n got: %s\nwant: %s", got, want)
+	}
+}
+
+// TestDiscover_FailsClosedOnEmptyTags — an empty result would flow to
+// `go build -tags ""` and ship a full binary as "lite".
+func TestDiscover_FailsClosedOnEmptyTags(t *testing.T) {
+	dir := t.TempDir()
+	writePlugin(t, dir, "plugins_jwtvalidation.go", "!exclude_plugin_jwtvalidation")
+
+	if _, err := discover(dir); err == nil {
+		t.Fatal("want error when every plugin is in liteKeep, got nil")
+	}
+}
+
+// TestDiscover_CompoundBuildDirective — before dropping the $ anchor,
+// `!exclude_plugin_X && !Y` silently failed to match and the plugin
+// stayed in the lite build.
+func TestDiscover_CompoundBuildDirective(t *testing.T) {
+	dir := t.TempDir()
+	writePlugin(t, dir, "plugins_examplecompound.go", "!exclude_plugin_examplecompound && !nocgo")
+	writePlugin(t, dir, "plugins_jwtvalidation.go", "!exclude_plugin_jwtvalidation") // keep-listed, prevents fail-closed
+
+	tags, err := discover(dir)
+	if err != nil {
+		t.Fatalf("discover: %v", err)
+	}
+	if got := strings.Join(tags, ","); got != "exclude_plugin_examplecompound" {
+		t.Errorf("got %q, want exclude_plugin_examplecompound", got)
+	}
+}
+
+// TestDiscover_SkipsTestFiles — the plugins_*.go glob would otherwise
+// scan a future plugins_foo_test.go as a plugin definition.
+func TestDiscover_SkipsTestFiles(t *testing.T) {
+	dir := t.TempDir()
+	writePlugin(t, dir, "plugins_ghost_test.go", "!exclude_plugin_ghost")
+	writePlugin(t, dir, "plugins_a2aparser.go", "!exclude_plugin_a2aparser")
+
+	tags, err := discover(dir)
+	if err != nil {
+		t.Fatalf("discover: %v", err)
+	}
+	if got := strings.Join(tags, ","); got != "exclude_plugin_a2aparser" {
+		t.Errorf("got %q, want exclude_plugin_a2aparser (test file must be skipped)", got)
+	}
+}
+
+func writePlugin(t *testing.T, dir, name, buildConstraint string) {
+	t.Helper()
+	content := "//go:build " + buildConstraint + "\n\npackage main\n"
+	if err := os.WriteFile(filepath.Join(dir, name), []byte(content), 0o644); err != nil {
+		t.Fatalf("write %s: %v", name, err)
+	}
+}