Skip to content
Merged
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
66 changes: 57 additions & 9 deletions pkg/vmcp/aggregator/conflict_resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ func TestPriorityConflictResolver(t *testing.T) {
wantCount int
wantWinners map[string]string // tool name -> expected backend ID
wantStrategies map[string]vmcp.ConflictResolutionStrategy // tool name -> expected strategy (optional)
wantMissing []string // tool names that must not be advertised
wantErr bool
}{
{
Expand Down Expand Up @@ -182,7 +183,7 @@ func TestPriorityConflictResolver(t *testing.T) {
},
},
{
name: "backends not in priority with conflict use prefix fallback",
name: "unlisted backends with conflict are dropped",
priorityOrder: []string{"github"},
toolsByBackend: map[string][]vmcp.Tool{
"github": {
Expand All @@ -195,17 +196,58 @@ func TestPriorityConflictResolver(t *testing.T) {
{Name: "send_message", Description: "Teams message"},
},
},
wantCount: 3, // All tools included, conflicting ones prefixed
wantCount: 1, // Conflicting unrankable tools are dropped
wantWinners: map[string]string{
"create_issue": "github", // In priority list
"slack_send_message": "slack", // Not in priority, prefixed
"teams_send_message": "teams", // Not in priority, prefixed
"create_issue": "github", // In priority list, no conflict
},
wantStrategies: map[string]vmcp.ConflictResolutionStrategy{
"create_issue": vmcp.ConflictStrategyPriority, // Priority strategy used
"slack_send_message": vmcp.ConflictStrategyPrefix, // Prefix fallback used
"teams_send_message": vmcp.ConflictStrategyPrefix, // Prefix fallback used
},
{
name: "mixed listed and unlisted conflict drops all candidates",
priorityOrder: []string{"github"},
toolsByBackend: map[string][]vmcp.Tool{
"github": {
{Name: "deploy", Description: "GitHub deploy"},
},
"prod": {
{Name: "deploy", Description: "Production deploy"},
},
},
wantCount: 0,
wantWinners: map[string]string{},
wantMissing: []string{"deploy", "github_deploy", "prod_deploy"},
},
{
name: "three-way mixed conflict drops all candidates",
priorityOrder: []string{"github", "staging"},
toolsByBackend: map[string][]vmcp.Tool{
"github": {
{Name: "deploy", Description: "GitHub deploy"},
},
"staging": {
{Name: "deploy", Description: "Staging deploy"},
},
"prod": {
{Name: "deploy", Description: "Production deploy"},
},
},
wantCount: 0,
wantWinners: map[string]string{},
wantMissing: []string{"deploy", "github_deploy", "staging_deploy", "prod_deploy"},
},
{
name: "drop prevents forbid bypass via prefixed names",
priorityOrder: []string{"github"},
toolsByBackend: map[string][]vmcp.Tool{
"github": {
{Name: "deploy", Description: "GitHub deploy"},
},
"prod": {
{Name: "deploy", Description: "Production deploy"},
},
},
wantCount: 0,
wantWinners: map[string]string{},
wantMissing: []string{"deploy", "github_deploy", "prod_deploy"},
},
{
Comment thread
JAORMX marked this conversation as resolved.
name: "empty priority order",
Expand Down Expand Up @@ -267,6 +309,12 @@ func TestPriorityConflictResolver(t *testing.T) {
}
}
}

for _, toolName := range tt.wantMissing {
if _, exists := resolved[toolName]; exists {
t.Errorf("tool %q should have been dropped", toolName)
}
}
})
}
}
Expand Down
58 changes: 27 additions & 31 deletions pkg/vmcp/aggregator/priority_resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,19 @@ import (
)

// PriorityConflictResolver implements priority-based conflict resolution.
// The first backend in the priority order wins; conflicting tools from
// lower-priority backends are dropped.
// When every conflicting backend is listed in the priority order, the first
// backend in that order wins and lower-priority tools are dropped.
//
// For backends not in the priority list, conflicts are resolved using
// prefix strategy as a fallback (prevents data loss).
// When any conflicting backend is absent from the priority list, all candidates
// in that conflict are dropped because the conflict cannot be safely ranked.
// Dropping preserves the fail-closed policy invariant: a renamed loser would be
// advertised under a name that existing name-scoped forbid policies do not cover.
type PriorityConflictResolver struct {
// PriorityOrder defines the priority of backends (first has highest priority).
PriorityOrder []string

// priorityMap is a map from backend ID to its priority index.
priorityMap map[string]int

// prefixResolver is used as fallback for backends not in priority list.
prefixResolver *PrefixConflictResolver
}

// NewPriorityConflictResolver creates a new priority-based conflict resolver.
Expand All @@ -44,9 +43,8 @@ func NewPriorityConflictResolver(priorityOrder []string) (*PriorityConflictResol
}

return &PriorityConflictResolver{
PriorityOrder: priorityOrder,
priorityMap: priorityMap,
prefixResolver: NewPrefixConflictResolver(defaultPrefixFormat), // Fallback for unmapped backends
PriorityOrder: priorityOrder,
priorityMap: priorityMap,
}, nil
}

Expand Down Expand Up @@ -82,35 +80,24 @@ func (r *PriorityConflictResolver) ResolveToolConflicts(
continue
}

// Conflict detected - choose the highest priority backend
winner := r.selectWinner(candidates)
if winner == nil {
// All candidates are from backends not in priority list
// Use prefix strategy as fallback to avoid data loss
if r.hasUnlistedCandidate(candidates) {
// A collision involving a backend outside priorityOrder cannot be safely
// rank-compared. Drop every candidate instead of awarding the bare name
// to a listed backend or re-advertising losers under names outside
// existing name-scoped policies.
backendIDs := make([]string, len(candidates))
for i, c := range candidates {
backendIDs[i] = c.BackendID
}
slog.Debug("tool exists in backends not in priority order, using prefix fallback",
slog.Error("dropped tool conflict involving backend not in priority order",
"tool", toolName, "backends", backendIDs)
Comment thread
JAORMX marked this conversation as resolved.

// Apply prefix strategy to these unmapped backends
for _, candidate := range candidates {
prefixedName := r.prefixResolver.applyPrefix(candidate.BackendID, toolName)
resolved[prefixedName] = &ResolvedTool{
ResolvedName: prefixedName,
OriginalName: toolName,
Description: candidate.Tool.Description,
InputSchema: candidate.Tool.InputSchema,
OutputSchema: candidate.Tool.OutputSchema,
Annotations: candidate.Tool.Annotations,
BackendID: candidate.BackendID,
ConflictResolutionApplied: vmcp.ConflictStrategyPrefix, // Fallback used prefix
}
}
droppedTools += len(candidates)
continue
}

// Conflict detected among only listed backends; choose the highest priority backend.
winner := r.selectWinner(candidates)
resolved[toolName] = &ResolvedTool{
ResolvedName: toolName,
OriginalName: toolName,
Expand Down Expand Up @@ -142,8 +129,17 @@ func (r *PriorityConflictResolver) ResolveToolConflicts(
return resolved, nil
}

func (r *PriorityConflictResolver) hasUnlistedCandidate(candidates []toolWithBackend) bool {
for _, candidate := range candidates {
if _, exists := r.priorityMap[candidate.BackendID]; !exists {
return true
}
}
return false
}

// selectWinner chooses the tool from the highest-priority backend.
// Returns nil if none of the candidates are in the priority list.
// Callers should only pass candidates from backends that are in the priority list.
func (r *PriorityConflictResolver) selectWinner(candidates []toolWithBackend) *toolWithBackend {
var winner *toolWithBackend
winnerPriority := -1
Expand Down
Loading