From 62c62b69bf8071a1ec5559596858c49c375d400e Mon Sep 17 00:00:00 2001 From: David Ahmann Date: Wed, 11 Mar 2026 10:25:39 -0400 Subject: [PATCH] inventory: expose stable hidden tool reason codes (#2197) --- pkg/inventory/filters.go | 89 +++++++++++++++++++++++++++------- pkg/inventory/registry_test.go | 47 ++++++++++++++++++ 2 files changed, 118 insertions(+), 18 deletions(-) diff --git a/pkg/inventory/filters.go b/pkg/inventory/filters.go index 707457853..aee7a5cc6 100644 --- a/pkg/inventory/filters.go +++ b/pkg/inventory/filters.go @@ -13,6 +13,24 @@ import ( // Returns (enabled, error). If error occurs, the caller should log and treat as false. type FeatureFlagChecker func(ctx context.Context, flagName string) (bool, error) +// HiddenToolReason is a stable machine-readable reason for tool filtering decisions. +type HiddenToolReason string + +const ( + HiddenToolReasonEnabledFalse HiddenToolReason = "enabled_false" + HiddenToolReasonFeatureFlag HiddenToolReason = "feature_flag_blocked" + HiddenToolReasonReadOnlyMode HiddenToolReason = "read_only_mode" + HiddenToolReasonBuilderFilterFalse HiddenToolReason = "builder_filter_false" + HiddenToolReasonBuilderFilterError HiddenToolReason = "builder_filter_error" + HiddenToolReasonToolsetDisabled HiddenToolReason = "toolset_disabled" +) + +// HiddenTool describes a hidden tool name and its first matching hidden reason. +type HiddenTool struct { + Name string + Reason HiddenToolReason +} + // isToolsetEnabled checks if a toolset is enabled based on current filters. func (r *Inventory) isToolsetEnabled(toolsetID ToolsetID) bool { // Check enabled toolsets filter @@ -51,53 +69,59 @@ func (r *Inventory) isFeatureFlagAllowed(ctx context.Context, enableFlag, disabl return true } -// isToolEnabled checks if a specific tool is enabled based on current filters. -// Filter evaluation order: -// 1. Tool.Enabled (tool self-filtering) -// 2. FeatureFlagEnable/FeatureFlagDisable -// 3. Read-only filter -// 4. Builder filters (via WithFilter) -// 5. Toolset/additional tools -func (r *Inventory) isToolEnabled(ctx context.Context, tool *ServerTool) bool { +// toolEnabledReason evaluates the tool filter chain in order and returns hidden reason if excluded. +func (r *Inventory) toolEnabledReason(ctx context.Context, tool *ServerTool) (bool, HiddenToolReason) { // 1. Check tool's own Enabled function first if tool.Enabled != nil { enabled, err := tool.Enabled(ctx) if err != nil { fmt.Fprintf(os.Stderr, "Tool.Enabled check error for %q: %v\n", tool.Tool.Name, err) - return false + return false, HiddenToolReasonEnabledFalse } if !enabled { - return false + return false, HiddenToolReasonEnabledFalse } } // 2. Check feature flags if !r.isFeatureFlagAllowed(ctx, tool.FeatureFlagEnable, tool.FeatureFlagDisable) { - return false + return false, HiddenToolReasonFeatureFlag } // 3. Check read-only filter (applies to all tools) if r.readOnly && !tool.IsReadOnly() { - return false + return false, HiddenToolReasonReadOnlyMode } // 4. Apply builder filters for _, filter := range r.filters { allowed, err := filter(ctx, tool) if err != nil { fmt.Fprintf(os.Stderr, "Builder filter error for tool %q: %v\n", tool.Tool.Name, err) - return false + return false, HiddenToolReasonBuilderFilterError } if !allowed { - return false + return false, HiddenToolReasonBuilderFilterFalse } } // 5. Check if tool is in additionalTools (bypasses toolset filter) if r.additionalTools != nil && r.additionalTools[tool.Tool.Name] { - return true + return true, "" } - // 5. Check toolset filter + // 6. Check toolset filter if !r.isToolsetEnabled(tool.Toolset.ID) { - return false + return false, HiddenToolReasonToolsetDisabled } - return true + return true, "" +} + +// isToolEnabled checks if a specific tool is enabled based on current filters. +// Filter evaluation order: +// 1. Tool.Enabled (tool self-filtering) +// 2. FeatureFlagEnable/FeatureFlagDisable +// 3. Read-only filter +// 4. Builder filters (via WithFilter) +// 5. Toolset/additional tools +func (r *Inventory) isToolEnabled(ctx context.Context, tool *ServerTool) bool { + enabled, _ := r.toolEnabledReason(ctx, tool) + return enabled } // AvailableTools returns the tools that pass all current filters, @@ -284,3 +308,32 @@ func (r *Inventory) EnabledToolsetIDs() []ToolsetID { func (r *Inventory) FilteredTools(ctx context.Context) ([]ServerTool, error) { return r.AvailableTools(ctx), nil } + +// HiddenTools returns hidden tools and stable reason codes for why they were filtered out. +// The first matching reason in the filter evaluation order is returned per tool name. +func (r *Inventory) HiddenTools(ctx context.Context) []HiddenTool { + seen := make(map[string]bool) + result := make([]HiddenTool, 0) + for i := range r.tools { + tool := &r.tools[i] + enabled, reason := r.toolEnabledReason(ctx, tool) + if enabled { + continue + } + if seen[tool.Tool.Name] { + continue + } + seen[tool.Tool.Name] = true + result = append(result, HiddenTool{ + Name: tool.Tool.Name, + Reason: reason, + }) + } + sort.Slice(result, func(i, j int) bool { + if result[i].Name != result[j].Name { + return result[i].Name < result[j].Name + } + return result[i].Reason < result[j].Reason + }) + return result +} diff --git a/pkg/inventory/registry_test.go b/pkg/inventory/registry_test.go index 207e65dba..45d1cec92 100644 --- a/pkg/inventory/registry_test.go +++ b/pkg/inventory/registry_test.go @@ -2277,3 +2277,50 @@ func TestCreateExcludeToolsFilter(t *testing.T) { require.NoError(t, err) require.True(t, allowed, "allowed_tool should be included") } + +func TestHiddenTools_ReportsStableReasonCodes(t *testing.T) { + tools := []ServerTool{ + mockTool("read_tool", "toolset1", true), + mockTool("write_tool", "toolset1", false), + mockTool("toolset_hidden", "toolset2", true), + mockToolWithFlags("flag_hidden", "toolset1", true, "feature_x", ""), + mockTool("builder_hidden", "toolset1", true), + } + filter := func(_ context.Context, tool *ServerTool) (bool, error) { + return tool.Tool.Name != "builder_hidden", nil + } + inv := mustBuild(t, NewBuilder(). + SetTools(tools). + WithToolsets([]string{"toolset1"}). + WithReadOnly(true). + WithFeatureChecker(func(_ context.Context, _ string) (bool, error) { return false, nil }). + WithFilter(filter)) + + hidden := inv.HiddenTools(context.Background()) + reasonsByName := make(map[string]HiddenToolReason, len(hidden)) + for _, item := range hidden { + reasonsByName[item.Name] = item.Reason + } + + require.Equal(t, HiddenToolReasonReadOnlyMode, reasonsByName["write_tool"]) + require.Equal(t, HiddenToolReasonToolsetDisabled, reasonsByName["toolset_hidden"]) + require.Equal(t, HiddenToolReasonFeatureFlag, reasonsByName["flag_hidden"]) + require.Equal(t, HiddenToolReasonBuilderFilterFalse, reasonsByName["builder_hidden"]) + _, hasReadTool := reasonsByName["read_tool"] + require.False(t, hasReadTool, "read_tool should not be hidden") +} + +func TestHiddenTools_ReportsBuilderFilterErrorReason(t *testing.T) { + tool := mockTool("error_tool", "toolset1", true) + inv := mustBuild(t, NewBuilder(). + SetTools([]ServerTool{tool}). + WithToolsets([]string{"all"}). + WithFilter(func(_ context.Context, _ *ServerTool) (bool, error) { + return false, fmt.Errorf("forced filter failure") + })) + + hidden := inv.HiddenTools(context.Background()) + require.Len(t, hidden, 1) + require.Equal(t, "error_tool", hidden[0].Name) + require.Equal(t, HiddenToolReasonBuilderFilterError, hidden[0].Reason) +}