Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions pkg/inventory/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package inventory

import (
"context"
"fmt"
"sort"
"strings"
)
Expand All @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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,
Expand Down
209 changes: 209 additions & 0 deletions pkg/inventory/builder_test.go
Original file line number Diff line number Diff line change
@@ -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()
}