Skip to content

Commit b1ab893

Browse files
authored
bug fix (#1775)
1 parent c061804 commit b1ab893

File tree

2 files changed

+60
-3
lines changed

2 files changed

+60
-3
lines changed

pkg/inventory/builder.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -149,11 +149,14 @@ func (b *Builder) Build() *Inventory {
149149
if len(b.additionalTools) > 0 {
150150
r.additionalTools = make(map[string]bool, len(b.additionalTools))
151151
for _, name := range b.additionalTools {
152-
// Resolve deprecated aliases to canonical names
152+
// Always include the original name - this handles the case where
153+
// the tool exists but is controlled by a feature flag that's OFF.
154+
r.additionalTools[name] = true
155+
// Also include the canonical name if this is a deprecated alias.
156+
// This handles the case where the feature flag is ON and only
157+
// the new consolidated tool is available.
153158
if canonical, isAlias := b.deprecatedAliases[name]; isAlias {
154159
r.additionalTools[canonical] = true
155-
} else {
156-
r.additionalTools[name] = true
157160
}
158161
}
159162
}

pkg/inventory/registry_test.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1688,3 +1688,57 @@ func TestForMCPRequest_ToolsCall_FeatureFlaggedVariants(t *testing.T) {
16881688
availableOn[0].FeatureFlagEnable, availableOn[0].FeatureFlagDisable)
16891689
}
16901690
}
1691+
1692+
// TestWithTools_DeprecatedAliasAndFeatureFlag tests that deprecated aliases work correctly
1693+
// when the old tool is controlled by a feature flag. This covers the scenario where:
1694+
// - Old tool "old_tool" has FeatureFlagDisable="my_flag" (available when flag is OFF)
1695+
// - New tool "new_tool" has FeatureFlagEnable="my_flag" (available when flag is ON)
1696+
// - Deprecated alias maps "old_tool" -> "new_tool"
1697+
// - User specifies --tools=old_tool
1698+
// Expected behavior:
1699+
// - Flag OFF: old_tool should be available (not the new_tool via alias)
1700+
// - Flag ON: new_tool should be available (via alias resolution)
1701+
func TestWithTools_DeprecatedAliasAndFeatureFlag(t *testing.T) {
1702+
oldTool := mockToolWithFlags("old_tool", "actions", true, "", "my_flag")
1703+
newTool := mockToolWithFlags("new_tool", "actions", true, "my_flag", "")
1704+
tools := []ServerTool{oldTool, newTool}
1705+
1706+
deprecatedAliases := map[string]string{
1707+
"old_tool": "new_tool",
1708+
}
1709+
1710+
// Test 1: Flag OFF - old_tool should be available via direct name match
1711+
// (not via alias resolution to new_tool, since old_tool still exists)
1712+
regFlagOff := NewBuilder().
1713+
SetTools(tools).
1714+
WithDeprecatedAliases(deprecatedAliases).
1715+
WithToolsets([]string{}). // No toolsets enabled
1716+
WithTools([]string{"old_tool"}). // Explicitly request old tool
1717+
Build()
1718+
availableOff := regFlagOff.AvailableTools(context.Background())
1719+
if len(availableOff) != 1 {
1720+
t.Fatalf("Flag OFF: Expected 1 tool, got %d", len(availableOff))
1721+
}
1722+
if availableOff[0].Tool.Name != "old_tool" {
1723+
t.Errorf("Flag OFF: Expected old_tool, got %s", availableOff[0].Tool.Name)
1724+
}
1725+
1726+
// Test 2: Flag ON - new_tool should be available via alias resolution
1727+
checker := func(_ context.Context, flag string) (bool, error) {
1728+
return flag == "my_flag", nil
1729+
}
1730+
regFlagOn := NewBuilder().
1731+
SetTools(tools).
1732+
WithDeprecatedAliases(deprecatedAliases).
1733+
WithToolsets([]string{}). // No toolsets enabled
1734+
WithTools([]string{"old_tool"}). // Request old tool name
1735+
WithFeatureChecker(checker).
1736+
Build()
1737+
availableOn := regFlagOn.AvailableTools(context.Background())
1738+
if len(availableOn) != 1 {
1739+
t.Fatalf("Flag ON: Expected 1 tool, got %d", len(availableOn))
1740+
}
1741+
if availableOn[0].Tool.Name != "new_tool" {
1742+
t.Errorf("Flag ON: Expected new_tool (via alias), got %s", availableOn[0].Tool.Name)
1743+
}
1744+
}

0 commit comments

Comments
 (0)