Add scope validation feature to inventory Builder

Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com>

Author: Copilot

pkg/inventory/builder.go

@@ -2,6 +2,7 @@ package inventory
 
 import (
 	"context"
+	"fmt"
 	"sort"
 	"strings"
 )
@@ -25,6 +26,7 @@ type ToolFilter func(ctx context.Context, tool *ServerTool) (bool, error)
 //	    WithToolsets([]string{"repos", "issues"}).
 //	    WithFeatureChecker(checker).
 //	    WithFilter(myFilter).
+//	    WithRequireScopes(true).
 //	    Build()
 type Builder struct {
 	tools             []ServerTool
@@ -39,6 +41,7 @@ type Builder struct {
 	additionalTools []string // raw input, processed at Build()
 	featureChecker  FeatureFlagChecker
 	filters         []ToolFilter // filters to apply to all tools
+	requireScopes   bool         // when true, validates all tools have scope definitions
 }
 
 // NewBuilder creates a new Builder.
@@ -127,11 +130,34 @@ func (b *Builder) WithFilter(filter ToolFilter) *Builder {
 	return b
 }
 
+// WithRequireScopes enables validation that all tools have scope definitions.
+// When enabled, Build() will panic if any tool has both RequiredScopes and
+// AcceptedScopes set to nil. Empty slices ([]string{}) are allowed, as they
+// explicitly indicate the tool requires no scopes.
+// Returns self for chaining.
+func (b *Builder) WithRequireScopes(require bool) *Builder {
+	b.requireScopes = require
+	return b
+}
+
 // Build creates the final Inventory with all configuration applied.
 // This processes toolset filtering, tool name resolution, and sets up
 // the inventory for use. The returned Inventory is ready for use with
 // AvailableTools(), RegisterAll(), etc.
+//
+// If WithRequireScopes(true) was called, this method will panic if any
+// tool has both RequiredScopes and AcceptedScopes set to nil.
 func (b *Builder) Build() *Inventory {
+	// Validate scopes if required
+	if b.requireScopes {
+		for i := range b.tools {
+			tool := &b.tools[i]
+			if tool.RequiredScopes == nil && tool.AcceptedScopes == nil {
+				panic(fmt.Sprintf("tool %q missing scope definitions (both RequiredScopes and AcceptedScopes are nil)", tool.Tool.Name))
+			}
+		}
+	}
+
 	r := &Inventory{
 		tools:             b.tools,
 		resourceTemplates: b.resourceTemplates,

pkg/inventory/builder_test.go

@@ -0,0 +1,209 @@
+package inventory
+
+import (
+	"testing"
+)
+
+// mockToolWithScopes creates a mock tool with specified scope definitions
+func mockToolWithScopes(name string, toolsetID string, requiredScopes, acceptedScopes []string) ServerTool {
+	tool := mockTool(name, toolsetID, false)
+	tool.RequiredScopes = requiredScopes
+	tool.AcceptedScopes = acceptedScopes
+	return tool
+}
+
+func TestWithRequireScopes_AllToolsHaveScopes(t *testing.T) {
+	tools := []ServerTool{
+		mockToolWithScopes("tool1", "toolset1", []string{"repo"}, []string{"repo", "admin:org"}),
+		mockToolWithScopes("tool2", "toolset1", []string{"repo", "user"}, []string{"repo", "user", "admin:org"}),
+		mockToolWithScopes("tool3", "toolset2", []string{}, []string{}), // empty slices are valid
+	}
+
+	// Should not panic when all tools have scope definitions
+	defer func() {
+		if r := recover(); r != nil {
+			t.Fatalf("Build() panicked unexpectedly: %v", r)
+		}
+	}()
+
+	_ = NewBuilder().
+		SetTools(tools).
+		WithRequireScopes(true).
+		Build()
+}
+
+func TestWithRequireScopes_ToolMissingBothScopes(t *testing.T) {
+	tools := []ServerTool{
+		mockToolWithScopes("tool1", "toolset1", []string{"repo"}, []string{"repo"}),
+		mockTool("tool2", "toolset1", false), // This tool has nil RequiredScopes and AcceptedScopes
+	}
+
+	// Should panic when a tool is missing both scope definitions
+	defer func() {
+		r := recover()
+		if r == nil {
+			t.Fatal("Build() should have panicked for tool missing scope definitions")
+		}
+		errMsg, ok := r.(string)
+		if !ok {
+			t.Fatalf("Expected panic message to be a string, got %T", r)
+		}
+		expectedMsg := `tool "tool2" missing scope definitions (both RequiredScopes and AcceptedScopes are nil)`
+		if errMsg != expectedMsg {
+			t.Errorf("Expected panic message %q, got %q", expectedMsg, errMsg)
+		}
+	}()
+
+	_ = NewBuilder().
+		SetTools(tools).
+		WithRequireScopes(true).
+		Build()
+}
+
+func TestWithRequireScopes_OnlyRequiredScopesSet(t *testing.T) {
+	tool := mockTool("tool1", "toolset1", false)
+	tool.RequiredScopes = []string{"repo"}
+	tool.AcceptedScopes = nil
+
+	tools := []ServerTool{tool}
+
+	// Should not panic when only RequiredScopes is set (AcceptedScopes can be nil if RequiredScopes is set)
+	defer func() {
+		if r := recover(); r != nil {
+			t.Fatalf("Build() panicked unexpectedly: %v", r)
+		}
+	}()
+
+	_ = NewBuilder().
+		SetTools(tools).
+		WithRequireScopes(true).
+		Build()
+}
+
+func TestWithRequireScopes_OnlyAcceptedScopesSet(t *testing.T) {
+	tool := mockTool("tool1", "toolset1", false)
+	tool.RequiredScopes = nil
+	tool.AcceptedScopes = []string{"repo"}
+
+	tools := []ServerTool{tool}
+
+	// Should not panic when only AcceptedScopes is set (RequiredScopes can be nil if AcceptedScopes is set)
+	defer func() {
+		if r := recover(); r != nil {
+			t.Fatalf("Build() panicked unexpectedly: %v", r)
+		}
+	}()
+
+	_ = NewBuilder().
+		SetTools(tools).
+		WithRequireScopes(true).
+		Build()
+}
+
+func TestWithRequireScopes_EmptySlicesAllowed(t *testing.T) {
+	tools := []ServerTool{
+		mockToolWithScopes("tool1", "toolset1", []string{}, []string{}),
+		mockToolWithScopes("tool2", "toolset2", []string{}, []string{}),
+	}
+
+	// Should not panic when tools have empty slices (explicit "no scopes needed")
+	defer func() {
+		if r := recover(); r != nil {
+			t.Fatalf("Build() panicked unexpectedly: %v", r)
+		}
+	}()
+
+	_ = NewBuilder().
+		SetTools(tools).
+		WithRequireScopes(true).
+		Build()
+}
+
+func TestWithRequireScopes_False(t *testing.T) {
+	tools := []ServerTool{
+		mockTool("tool1", "toolset1", false), // Missing scope definitions
+		mockTool("tool2", "toolset1", false), // Missing scope definitions
+	}
+
+	// Should not panic when WithRequireScopes(false) or not set
+	defer func() {
+		if r := recover(); r != nil {
+			t.Fatalf("Build() panicked unexpectedly: %v", r)
+		}
+	}()
+
+	_ = NewBuilder().
+		SetTools(tools).
+		WithRequireScopes(false).
+		Build()
+}
+
+func TestWithRequireScopes_NotSet(t *testing.T) {
+	tools := []ServerTool{
+		mockTool("tool1", "toolset1", false), // Missing scope definitions
+		mockTool("tool2", "toolset1", false), // Missing scope definitions
+	}
+
+	// Should not panic when WithRequireScopes is not called (default is false)
+	defer func() {
+		if r := recover(); r != nil {
+			t.Fatalf("Build() panicked unexpectedly: %v", r)
+		}
+	}()
+
+	_ = NewBuilder().
+		SetTools(tools).
+		Build()
+}
+
+func TestWithRequireScopes_MixedTools(t *testing.T) {
+	tools := []ServerTool{
+		mockToolWithScopes("tool1", "toolset1", []string{"repo"}, []string{"repo"}),
+		mockToolWithScopes("tool2", "toolset1", []string{}, []string{}),
+		mockTool("tool3", "toolset2", false), // Missing scope definitions
+	}
+
+	// Should panic on the first tool with missing scope definitions
+	defer func() {
+		r := recover()
+		if r == nil {
+			t.Fatal("Build() should have panicked for tool missing scope definitions")
+		}
+		errMsg, ok := r.(string)
+		if !ok {
+			t.Fatalf("Expected panic message to be a string, got %T", r)
+		}
+		expectedMsg := `tool "tool3" missing scope definitions (both RequiredScopes and AcceptedScopes are nil)`
+		if errMsg != expectedMsg {
+			t.Errorf("Expected panic message %q, got %q", expectedMsg, errMsg)
+		}
+	}()
+
+	_ = NewBuilder().
+		SetTools(tools).
+		WithRequireScopes(true).
+		Build()
+}
+
+func TestWithRequireScopes_Chaining(t *testing.T) {
+	tools := []ServerTool{
+		mockToolWithScopes("tool1", "toolset1", []string{"repo"}, []string{"repo"}),
+	}
+
+	// Test that WithRequireScopes returns the builder for chaining
+	builder := NewBuilder()
+	result := builder.WithRequireScopes(true)
+
+	if result != builder {
+		t.Error("WithRequireScopes should return the same builder instance for chaining")
+	}
+
+	// Verify the build works
+	defer func() {
+		if r := recover(); r != nil {
+			t.Fatalf("Build() panicked unexpectedly: %v", r)
+		}
+	}()
+
+	_ = result.SetTools(tools).Build()
+}