Skip to content

Commit d76ca28

Browse files
docs: improve filter evaluation order and FilteredTools documentation
- Add numbered filter evaluation order to isToolEnabled function doc - Number inline comments for each filter step (1-5) - Clarify FilteredTools error return is for future extensibility - Document that library consumers may need to surface recoverable errors Addresses review feedback on PR #1620
1 parent 7070d03 commit d76ca28

File tree

1 file changed

+20
-7
lines changed

1 file changed

+20
-7
lines changed

pkg/registry/filters.go

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,14 @@ func (r *Registry) isFeatureFlagAllowed(ctx context.Context, enableFlag, disable
5151
}
5252

5353
// isToolEnabled checks if a specific tool is enabled based on current filters.
54+
// Filter evaluation order:
55+
// 1. Tool.Enabled (tool self-filtering)
56+
// 2. FeatureFlagEnable/FeatureFlagDisable
57+
// 3. Read-only filter
58+
// 4. Builder filters (via WithFilter)
59+
// 5. Toolset/additional tools
5460
func (r *Registry) isToolEnabled(ctx context.Context, tool *ServerTool) bool {
55-
// Check tool's own Enabled function first
61+
// 1. Check tool's own Enabled function first
5662
if tool.Enabled != nil {
5763
enabled, err := tool.Enabled(ctx)
5864
if err != nil {
@@ -63,15 +69,15 @@ func (r *Registry) isToolEnabled(ctx context.Context, tool *ServerTool) bool {
6369
return false
6470
}
6571
}
66-
// Check feature flags
72+
// 2. Check feature flags
6773
if !r.isFeatureFlagAllowed(ctx, tool.FeatureFlagEnable, tool.FeatureFlagDisable) {
6874
return false
6975
}
70-
// Check read-only filter (applies to all tools)
76+
// 3. Check read-only filter (applies to all tools)
7177
if r.readOnly && !tool.IsReadOnly() {
7278
return false
7379
}
74-
// Apply builder filters
80+
// 4. Apply builder filters
7581
for _, filter := range r.filters {
7682
allowed, err := filter(ctx, tool)
7783
if err != nil {
@@ -82,11 +88,11 @@ func (r *Registry) isToolEnabled(ctx context.Context, tool *ServerTool) bool {
8288
return false
8389
}
8490
}
85-
// Check if tool is in additionalTools (bypasses toolset filter)
91+
// 5. Check if tool is in additionalTools (bypasses toolset filter)
8692
if r.additionalTools != nil && r.additionalTools[tool.Tool.Name] {
8793
return true
8894
}
89-
// Check toolset filter
95+
// 5. Check toolset filter
9096
if !r.isToolsetEnabled(tool.Toolset.ID) {
9197
return false
9298
}
@@ -269,7 +275,14 @@ func (r *Registry) EnabledToolsetIDs() []ToolsetID {
269275
}
270276

271277
// FilteredTools returns tools filtered by the Enabled function and builder filters.
272-
// This is an alias for AvailableTools for clarity when focusing on filtering behavior.
278+
// This provides an explicit API for accessing filtered tools, currently implemented
279+
// as an alias for AvailableTools.
280+
//
281+
// The error return is currently always nil but is included for future extensibility.
282+
// Library consumers (e.g., remote server implementations) may need to surface
283+
// recoverable filter errors rather than silently logging them. Having the error
284+
// return in the API now avoids breaking changes later.
285+
//
273286
// The context is used for Enabled function evaluation and builder filter checks.
274287
func (r *Registry) FilteredTools(ctx context.Context) ([]ServerTool, error) {
275288
return r.AvailableTools(ctx), nil

0 commit comments

Comments
 (0)