Skip to content

Commit 116c574

Browse files
majiayu000SamMorrowDrums
authored andcommitted
fix: filterToolsByName returns all matching tools for feature flag filtering
When multiple tools share the same name but have different feature flags (like GetJobLogs and ActionsGetJobLogs both named "get_job_logs"), filterToolsByName was only returning the first match. This caused the remote server to fail with "unknown tool" error when the first matching tool was disabled by feature flags, even though another variant was enabled. The fix modifies filterToolsByName to return ALL tools with matching names, allowing the feature flag filtering in AvailableTools to select the correct variant based on the enabled flags. Fixes #1714 Signed-off-by: majiayu000 <[email protected]>
1 parent 92bdc28 commit 116c574

File tree

2 files changed

+56
-4
lines changed

2 files changed

+56
-4
lines changed

pkg/inventory/filters.go

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -178,22 +178,29 @@ func (r *Inventory) AvailablePrompts(ctx context.Context) []ServerPrompt {
178178

179179
// filterToolsByName returns tools matching the given name, checking deprecated aliases.
180180
// Uses linear scan - optimized for single-lookup per-request scenarios (ForMCPRequest).
181+
// Returns ALL tools matching the name to support feature-flagged tool variants
182+
// (e.g., GetJobLogs and ActionsGetJobLogs both use name "get_job_logs" but are
183+
// controlled by different feature flags).
181184
func (r *Inventory) filterToolsByName(name string) []ServerTool {
182-
// First check for exact match
185+
var result []ServerTool
186+
// Check for exact matches - multiple tools may share the same name with different feature flags
183187
for i := range r.tools {
184188
if r.tools[i].Tool.Name == name {
185-
return []ServerTool{r.tools[i]}
189+
result = append(result, r.tools[i])
186190
}
187191
}
192+
if len(result) > 0 {
193+
return result
194+
}
188195
// Check if name is a deprecated alias
189196
if canonical, isAlias := r.deprecatedAliases[name]; isAlias {
190197
for i := range r.tools {
191198
if r.tools[i].Tool.Name == canonical {
192-
return []ServerTool{r.tools[i]}
199+
result = append(result, r.tools[i])
193200
}
194201
}
195202
}
196-
return []ServerTool{}
203+
return result
197204
}
198205

199206
// filterResourcesByURI returns resource templates matching the given URI pattern.

pkg/inventory/registry_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1643,3 +1643,48 @@ func TestFilteringOrder(t *testing.T) {
16431643
}
16441644
}
16451645
}
1646+
1647+
func TestForMCPRequest_ToolsCall_FeatureFlaggedVariants(t *testing.T) {
1648+
// Simulate the get_job_logs scenario: two tools with the same name but different feature flags
1649+
// - "get_job_logs" with FeatureFlagDisable (available when flag is OFF)
1650+
// - "get_job_logs" with FeatureFlagEnable (available when flag is ON)
1651+
tools := []ServerTool{
1652+
mockToolWithFlags("get_job_logs", "actions", true, "", "consolidated_flag"), // disabled when flag is ON
1653+
mockToolWithFlags("get_job_logs", "actions", true, "consolidated_flag", ""), // enabled when flag is ON
1654+
mockTool("other_tool", "actions", true),
1655+
}
1656+
1657+
// Test 1: Flag is OFF - first tool variant should be available
1658+
regFlagOff := NewBuilder().
1659+
SetTools(tools).
1660+
WithToolsets([]string{"all"}).
1661+
Build()
1662+
filteredOff := regFlagOff.ForMCPRequest(MCPMethodToolsCall, "get_job_logs")
1663+
availableOff := filteredOff.AvailableTools(context.Background())
1664+
if len(availableOff) != 1 {
1665+
t.Fatalf("Flag OFF: Expected 1 tool, got %d", len(availableOff))
1666+
}
1667+
if availableOff[0].FeatureFlagDisable != "consolidated_flag" {
1668+
t.Errorf("Flag OFF: Expected tool with FeatureFlagDisable, got FeatureFlagEnable=%q, FeatureFlagDisable=%q",
1669+
availableOff[0].FeatureFlagEnable, availableOff[0].FeatureFlagDisable)
1670+
}
1671+
1672+
// Test 2: Flag is ON - second tool variant should be available
1673+
checker := func(_ context.Context, flag string) (bool, error) {
1674+
return flag == "consolidated_flag", nil
1675+
}
1676+
regFlagOn := NewBuilder().
1677+
SetTools(tools).
1678+
WithToolsets([]string{"all"}).
1679+
WithFeatureChecker(checker).
1680+
Build()
1681+
filteredOn := regFlagOn.ForMCPRequest(MCPMethodToolsCall, "get_job_logs")
1682+
availableOn := filteredOn.AvailableTools(context.Background())
1683+
if len(availableOn) != 1 {
1684+
t.Fatalf("Flag ON: Expected 1 tool, got %d", len(availableOn))
1685+
}
1686+
if availableOn[0].FeatureFlagEnable != "consolidated_flag" {
1687+
t.Errorf("Flag ON: Expected tool with FeatureFlagEnable, got FeatureFlagEnable=%q, FeatureFlagDisable=%q",
1688+
availableOn[0].FeatureFlagEnable, availableOn[0].FeatureFlagDisable)
1689+
}
1690+
}

0 commit comments

Comments
 (0)