Skip to content

Commit 880c194

Browse files
refactor: embed compiledCondition in ServerTool
Instead of maintaining a separate compiledConditions slice indexed by tool position, embed the *CompiledCondition directly in each ServerTool. This eliminates the index alignment issue where filtering to a single tool (ForMCPRequest with ToolsCall) would break the mapping between tools and their compiled conditions. Benefits: - Compiled condition travels with the tool during any filtering - No index bookkeeping needed - Simpler code in filters.go (no toolIndex parameter) - ForMCPRequest works correctly for single-tool lookups - Same O(1) bitmask evaluation performance
1 parent 3a52f7b commit 880c194

File tree

5 files changed

+33
-29
lines changed

5 files changed

+33
-29
lines changed

pkg/inventory/builder.go

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -166,9 +166,9 @@ func (b *Builder) Build() *Inventory {
166166
}
167167
}
168168

169-
// Compile EnableConditions for O(1) bitmask evaluation
170-
// Note: compileConditions uses r.tools which is now sortedTools
171-
r.conditionCompiler, r.compiledConditions = b.compileConditions(sortedTools)
169+
// Compile EnableConditions for O(1) bitmask evaluation.
170+
// This modifies sortedTools in place, embedding compiled conditions in each tool.
171+
r.conditionCompiler = b.compileConditions(sortedTools)
172172

173173
return r
174174
}
@@ -223,21 +223,20 @@ func (b *Builder) preSortPrompts() []ServerPrompt {
223223
}
224224

225225
// compileConditions compiles all EnableConditions into bitmask-based evaluators.
226-
// Returns the compiler (for building request masks) and compiled conditions slice.
227-
// Takes the sorted tools slice to ensure compiled conditions align with sorted order.
228-
func (b *Builder) compileConditions(sortedTools []ServerTool) (*ConditionCompiler, []*CompiledCondition) {
226+
// Modifies sortedTools in place, embedding compiled conditions in each tool.
227+
// Returns the compiler (for building request masks at runtime).
228+
func (b *Builder) compileConditions(sortedTools []ServerTool) *ConditionCompiler {
229229
compiler := NewConditionCompiler()
230-
compiled := make([]*CompiledCondition, len(sortedTools))
231230

232231
for i := range sortedTools {
233232
if sortedTools[i].EnableCondition != nil {
234-
compiled[i] = compiler.Compile(sortedTools[i].EnableCondition)
233+
sortedTools[i].compiledCondition = compiler.Compile(sortedTools[i].EnableCondition)
235234
}
236235
// nil means no condition (always enabled from condition perspective)
237236
}
238237

239238
compiler.Freeze()
240-
return compiler, compiled
239+
return compiler
241240
}
242241

243242
// processToolsets processes the toolsetIDs configuration and returns:

pkg/inventory/filters.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ func (r *Inventory) buildRequestMask(ctx context.Context) *RequestMask {
103103
// 4. Read-only filter
104104
// 5. Builder filters (via WithFilter)
105105
// 6. Toolset/additional tools
106-
func (r *Inventory) isToolEnabled(ctx context.Context, tool *ServerTool, toolIndex int, rm *RequestMask) bool {
106+
func (r *Inventory) isToolEnabled(ctx context.Context, tool *ServerTool, rm *RequestMask) bool {
107107
// 1. Check tool's legacy Enabled function first (for backward compatibility)
108108
if tool.Enabled != nil {
109109
enabled, err := tool.Enabled(ctx)
@@ -116,17 +116,18 @@ func (r *Inventory) isToolEnabled(ctx context.Context, tool *ServerTool, toolInd
116116
}
117117
}
118118
// 2. Check tool's EnableCondition via compiled bitmask (O(1) evaluation)
119-
if toolIndex >= 0 && toolIndex < len(r.compiledConditions) && r.compiledConditions[toolIndex] != nil {
119+
// The compiled condition is embedded in the tool, so it travels with filtering.
120+
if tool.compiledCondition != nil {
120121
if rm != nil {
121-
enabled, err := r.compiledConditions[toolIndex].Evaluate(rm)
122+
enabled, err := tool.compiledCondition.Evaluate(rm)
122123
if err != nil {
123124
fmt.Fprintf(os.Stderr, "Tool.EnableCondition check error for %q: %v\n", tool.Tool.Name, err)
124125
return false
125126
}
126127
if !enabled {
127128
return false
128129
}
129-
} else if tool.EnableCondition != nil {
130+
} else {
130131
// Fallback to tree-based evaluation if no request mask
131132
enabled, err := tool.EnableCondition.Evaluate(ctx)
132133
if err != nil {
@@ -191,7 +192,7 @@ func (r *Inventory) AvailableTools(ctx context.Context) []ServerTool {
191192
var result []ServerTool
192193
for i := range r.tools {
193194
tool := &r.tools[i]
194-
if r.isToolEnabled(ctx, tool, i, rm) {
195+
if r.isToolEnabled(ctx, tool, rm) {
195196
result = append(result, *tool)
196197
}
197198
}

pkg/inventory/registry.go

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,8 @@ type Inventory struct {
4141
defaultToolsetIDs []ToolsetID // sorted list of default toolset IDs
4242
toolsetDescriptions map[ToolsetID]string // toolset ID -> description
4343

44-
// Compiled conditions for O(1) EnableCondition evaluation (set during Build)
45-
// Maps tool index → compiled condition (nil means always enabled)
46-
compiledConditions []*CompiledCondition
47-
// conditionCompiler used to compile conditions (shared across inventory)
44+
// conditionCompiler used to compile conditions and build request masks.
45+
// The compiled conditions themselves are embedded in each ServerTool.
4846
conditionCompiler *ConditionCompiler
4947

5048
// Filters - these control what's returned by Available* methods
@@ -107,7 +105,9 @@ const (
107105
func (r *Inventory) ForMCPRequest(method string, itemName string) *Inventory {
108106
// Create a shallow copy with shared filter settings
109107
// Note: lazy-init maps (toolsByName, etc.) are NOT copied - the new Registry
110-
// will initialize its own maps on first use if needed
108+
// will initialize its own maps on first use if needed.
109+
// Compiled conditions are embedded in each ServerTool, so they travel with the tool
110+
// during filtering - no index alignment issues.
111111
result := &Inventory{
112112
tools: r.tools,
113113
resourceTemplates: r.resourceTemplates,
@@ -117,9 +117,8 @@ func (r *Inventory) ForMCPRequest(method string, itemName string) *Inventory {
117117
enabledToolsets: r.enabledToolsets, // shared, not modified
118118
additionalTools: r.additionalTools, // shared, not modified
119119
featureChecker: r.featureChecker,
120-
filters: r.filters, // shared, not modified
121-
compiledConditions: r.compiledConditions, // shared, not modified
122-
conditionCompiler: r.conditionCompiler, // shared, not modified
120+
filters: r.filters, // shared, not modified
121+
conditionCompiler: r.conditionCompiler,
123122
unrecognizedToolsets: r.unrecognizedToolsets,
124123
}
125124

pkg/inventory/registry_test.go

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1904,7 +1904,6 @@ func TestCompiledConditionsIntegration(t *testing.T) {
19041904

19051905
// Verify compiler was created
19061906
assert.NotNil(t, reg.conditionCompiler, "Condition compiler should be created")
1907-
assert.NotNil(t, reg.compiledConditions, "Compiled conditions should be created")
19081907

19091908
// Test without context bools - only flag-based tools should pass
19101909
ctx := context.Background()
@@ -1954,9 +1953,9 @@ func TestCompiledConditionsANDBitmask(t *testing.T) {
19541953
WithToolsets([]string{"all"}).
19551954
Build()
19561955

1957-
// Verify it was compiled to bitmask AND
1958-
assert.NotNil(t, reg.compiledConditions[0])
1959-
compiled := reg.compiledConditions[0]
1956+
// Verify it was compiled to bitmask AND (condition embedded in tool)
1957+
assert.NotNil(t, reg.tools[0].compiledCondition)
1958+
compiled := reg.tools[0].compiledCondition
19601959
assert.Equal(t, evalBitmaskAnd, compiled.evalType, "AND of flags should compile to bitmaskAnd")
19611960

19621961
// Test evaluation
@@ -1985,9 +1984,9 @@ func TestCompiledConditionsORBitmask(t *testing.T) {
19851984
WithToolsets([]string{"all"}).
19861985
Build()
19871986

1988-
// Verify it was compiled to bitmask OR
1989-
assert.NotNil(t, reg.compiledConditions[0])
1990-
compiled := reg.compiledConditions[0]
1987+
// Verify it was compiled to bitmask OR (condition embedded in tool)
1988+
assert.NotNil(t, reg.tools[0].compiledCondition)
1989+
compiled := reg.tools[0].compiledCondition
19911990
assert.Equal(t, evalBitmaskOr, compiled.evalType, "OR of flags should compile to bitmaskOr")
19921991

19931992
// Test evaluation - should pass because flag_a is enabled

pkg/inventory/server_tool.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,12 @@ type ServerTool struct {
9797
// If both Enabled and EnableCondition are set, Enabled takes precedence for
9898
// backward compatibility. Migrate to EnableCondition for new tools.
9999
Enabled func(ctx context.Context) (bool, error)
100+
101+
// compiledCondition is the pre-compiled bitmask evaluator for EnableCondition.
102+
// Set at build time by compileConditions(). nil means always enabled.
103+
// This is embedded in the tool so it travels with the tool during filtering,
104+
// eliminating index alignment issues when filtering to single tools.
105+
compiledCondition *CompiledCondition
100106
}
101107

102108
// IsReadOnly returns true if this tool is marked as read-only via annotations.

0 commit comments

Comments
 (0)