feat: composable EnableCondition with bitmask optimization #1641

SamMorrowDrums · 2025-12-18T13:21:08Z

Summary

This PR adds a composable EnableCondition system for tool filtering that encapsulates bitmask complexity behind a clean API.

Key Features

1. User-facing API (`conditions.go`)

EnableCondition interface with Evaluate(ctx) method
Primitives: FeatureFlag(), ContextBool(), Static(), Always(), Never()
Combinators: And(), Or(), Not() with short-circuit evaluation
All bitmask complexity hidden from users

2. Bitmask Compiler (`condition_compiler.go`)

Compiles conditions to O(1) bitmask evaluators at build time
RequestMask holds pre-computed uint64 bitmask per request
AND/OR of flags compile to single bitmask operations
Falls back gracefully for custom ConditionFunc

3. Pre-sorting Optimization (`builder.go`)

Tools, resources, prompts sorted once at build time
Filtering preserves order, eliminating per-request sorting
~45% faster request handling in benchmarks

4. Integration (`filters.go`, `registry.go`)

Builder.Build() compiles all EnableConditions
AvailableTools() builds RequestMask once, evaluates via bitmask
Backward compatible with legacy Enabled func and feature flags

Usage Example

// Simple feature flag
tool.EnableCondition = FeatureFlag("my_feature")

// CCA bypass pattern (common in remote server)
tool.EnableCondition = Or(
    ContextBool("is_cca"),        // CCA users bypass flag
    FeatureFlag("my_feature"),    // Others need flag enabled
)

// Complex condition
tool.EnableCondition = And(
    ContextBool("is_premium"),
    Or(
        FeatureFlag("beta_access"),
        ContextBool("is_staff"),
    ),
    Not(FeatureFlag("kill_switch")),
)

Benchmarks

1000 requests × 50 tools each:

Metric	Before	After	Improvement
Time	23.7ms	12.9ms	46% faster
Allocations	15000	12000	20% fewer

Why This Approach?

Easy to adopt: Remote server just sets EnableCondition on tools - optimization is automatic
Composable: Build complex conditions from simple primitives
Hidden complexity: Users don't need to know about bitmasks
Backward compatible: Existing Enabled func and feature flag fields still work

Supersedes #1637

Add a composable EnableCondition system for tool filtering that: 1. **User-facing API** (conditions.go): - EnableCondition interface with Evaluate(ctx) method - Primitives: FeatureFlag(), ContextBool(), Static(), Always(), Never() - Combinators: And(), Or(), Not() with short-circuit evaluation - All bitmask complexity hidden from users 2. **Bitmask compiler** (condition_compiler.go): - Compiles conditions to O(1) bitmask evaluators at build time - RequestMask holds pre-computed uint64 bitmask per request - AND/OR of flags compile to single bitmask operations - Falls back gracefully for custom ConditionFunc 3. **Pre-sorting optimization** (builder.go): - Tools, resources, prompts sorted once at build time - Filtering preserves order, eliminating per-request sorting - ~45% faster request handling in benchmarks 4. **Integration** (filters.go, registry.go): - Builder.Build() compiles all EnableConditions - AvailableTools() builds RequestMask once, evaluates via bitmask - Backward compatible with legacy Enabled func and feature flags Usage example: tool.EnableCondition = Or( ContextBool("is_cca"), // CCA users bypass flag FeatureFlag("my_feature"), // Others need flag enabled ) Benchmarks (1000 requests × 50 tools): - Before: 23.7ms (with per-request sorting) - After: 12.9ms (pre-sorted + bitmask) - Improvement: 46% faster This makes it easy for remote server to adopt - just set EnableCondition on tools and the optimization is automatic.

Copilot

Pull request overview

This PR introduces a composable EnableCondition system for tool filtering with bitmask optimization, providing a cleaner API and significant performance improvements (~46% faster in benchmarks). The implementation includes primitives (FeatureFlag, ContextBool, Static), combinators (And, Or, Not), a compile-time bitmask optimizer, and pre-sorting optimization.

Key Changes

Composable API: New EnableCondition interface with declarative condition primitives and combinators for building complex tool availability logic
Bitmask optimization: Conditions compile to O(1) bitmask evaluators at build time, with RequestMask for efficient per-request evaluation
Pre-sorting optimization: Tools/resources/prompts sorted once at build time rather than on every request

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
pkg/inventory/conditions.go	New file: Core EnableCondition API with primitives and combinators
pkg/inventory/condition_compiler.go	New file: Bitmask compiler for O(1) condition evaluation (has a critical bug)
pkg/inventory/server_tool.go	Added EnableCondition field with deprecation notices for legacy fields (documentation issues)
pkg/inventory/filters.go	Integrated EnableCondition evaluation with bitmask support and buildRequestMask
pkg/inventory/builder.go	Added condition compilation and pre-sorting at build time
pkg/inventory/registry.go	Added compiler/conditions fields and propagated them in ForMCPRequest
pkg/inventory/conditions_test.go	New file: Comprehensive unit tests for condition primitives and combinators
pkg/inventory/condition_compiler_test.go	New file: Tests for compiler logic and bitmask operations
pkg/inventory/registry_test.go	Added integration tests for EnableCondition system
pkg/inventory/enable_bench_test.go	New file: Extensive benchmarks comparing old vs new approach

Copilot · 2025-12-18T13:26:37Z

pkg/inventory/filters.go

+		} else if tool.EnableCondition != nil {
+			// Fallback to tree-based evaluation if no request mask
+			enabled, err := tool.EnableCondition.Evaluate(ctx)
+			if err != nil {
+				fmt.Fprintf(os.Stderr, "Tool.EnableCondition check error for %q: %v\n", tool.Tool.Name, err)
+				return false
+			}
+			if !enabled {
+				return false
+			}
+		}
+	} else if tool.EnableCondition != nil {
+		// Fallback to tree-based evaluation if no compiled condition
+		enabled, err := tool.EnableCondition.Evaluate(ctx)
+		if err != nil {
+			fmt.Fprintf(os.Stderr, "Tool.EnableCondition check error for %q: %v\n", tool.Tool.Name, err)
+			return false
+		}
+		if !enabled {
+			return false
+		}
+	}


Duplicated condition evaluation logic with inconsistent error messages. Lines 129-139 and 140-150 both evaluate tool.EnableCondition using the tree-based approach and have identical error handling, differing only in when they're triggered. This code duplication makes the logic harder to maintain and increases the risk of inconsistencies.

Consider extracting the tree-based evaluation into a helper function to reduce duplication and ensure consistent behavior across both paths.

Copilot · 2025-12-18T13:26:38Z

pkg/inventory/server_tool.go

+	// This field is checked before EnableCondition for backward compatibility.
 	FeatureFlagEnable string

 	// FeatureFlagDisable specifies a feature flag that, when enabled, causes this tool
 	// to be omitted. Used to disable tools when a feature flag is on.
+	//
+	// Deprecated: Use EnableCondition with Not(FeatureFlag()) instead for composable conditions.
+	// This field is checked before EnableCondition for backward compatibility.


Incorrect documentation about filtering order. The comment states "This field is checked before EnableCondition for backward compatibility" (lines 63 and 70), but the implementation in filters.go shows that legacy feature flags (FeatureFlagEnable/FeatureFlagDisable) are checked AFTER EnableCondition, not before.

The actual order from filters.go:isToolEnabled is:

Tool.Enabled (legacy)

Tool.EnableCondition

FeatureFlagEnable/FeatureFlagDisable (legacy)

The documentation should be corrected to reflect that these legacy fields are checked after EnableCondition.

Suggested change

// This field is checked before EnableCondition for backward compatibility.

FeatureFlagEnable string

// FeatureFlagDisable specifies a feature flag that, when enabled, causes this tool

// to be omitted. Used to disable tools when a feature flag is on.

//

// Deprecated: Use EnableCondition with Not(FeatureFlag()) instead for composable conditions.

// This field is checked before EnableCondition for backward compatibility.

// This field is checked after EnableCondition, as an additional legacy filter.

FeatureFlagEnable string

// FeatureFlagDisable specifies a feature flag that, when enabled, causes this tool

// to be omitted. Used to disable tools when a feature flag is on.

//

// Deprecated: Use EnableCondition with Not(FeatureFlag()) instead for composable conditions.

// This field is checked after EnableCondition, as an additional legacy filter.

Copilot · 2025-12-18T13:26:38Z

pkg/inventory/server_tool.go

+	// If both Enabled and EnableCondition are set, Enabled takes precedence for
+	// backward compatibility. Migrate to EnableCondition for new tools.


Incorrect documentation about precedence. The comment states "If both Enabled and EnableCondition are set, Enabled takes precedence for backward compatibility" (line 97-98), but this is misleading.

The actual behavior is that both checks are applied sequentially - Enabled is checked first, and if it passes (or is nil), then EnableCondition is checked. Neither "takes precedence" - both must pass for the tool to be enabled. The comment should clarify that both conditions must be satisfied when both are present.

Suggested change

// If both Enabled and EnableCondition are set, Enabled takes precedence for

// backward compatibility. Migrate to EnableCondition for new tools.

// When both Enabled and EnableCondition are set, Enabled is evaluated first for

// backward compatibility, and EnableCondition is then evaluated; both must pass

// for the tool to be enabled. Migrate to EnableCondition for new tools.

Copilot · 2025-12-18T13:26:39Z

pkg/inventory/condition_compiler.go

+		prefix := key[:3]
+		name := key[3:]
+
+		switch prefix {
+		case "ctx":
+			if bools != nil && bools[name] {
+				bits |= 1 << bit
+			}


The prefix check logic is inconsistent with the key format. The code checks prefix := key[:3] and then compares with "ctx", but context bool keys are formatted as "ctx:key_name" (4 characters for the prefix). This will cause all context bool keys to be incorrectly parsed.

The switch case case "ctx": will never match because context bool keys have the format "ctx:" (4 chars), not "ctx" (3 chars). This should be case "ctx:" to match the actual key format.

Copilot · 2025-12-18T13:26:39Z

pkg/inventory/condition_compiler.go

+				bits |= 1 << bit
+			}
+		case "ff:":
+			name = key[3:] // "ff:" is 3 chars


Redundant name reassignment. Line 431 reassigns name = key[3:] even though it was already set on line 423. Since the "ff:" prefix is 3 characters, the name extraction on line 423 is already correct for feature flags. This redundant assignment is unnecessary and could cause confusion.

Suggested change

name = key[3:] // "ff:" is 3 chars

Copilot · 2025-12-18T13:26:39Z

pkg/inventory/filters.go

+	for key, bit := range r.conditionCompiler.keyToBit {
+		// Keys are formatted as "ctx:key_name" or "ff:flag_name"
+		if len(key) < 4 { // Minimum: "ff:x" or "ctx:" prefix + 1 char
+			continue
+		}
+
+		switch {
+		case len(key) > 4 && key[:4] == "ctx:":
+			// Context bool: "ctx:key_name"
+			name := key[4:]
+			if bools != nil && bools[name] {
+				bits |= 1 << bit
+			}
+		case len(key) > 3 && key[:3] == "ff:":
+			// Feature flag: "ff:flag_name"
+			name := key[3:]
+			if checker != nil {
+				enabled, err := checker(ctx, name)
+				if err == nil && enabled {
+					bits |= 1 << bit
+				}
+			}
+		}
+	}


Inconsistent prefix checking logic. In buildRequestMask (filters.go), the code uses key[:4] == "ctx:" and key[:3] == "ff:" which correctly matches the key formats. However, in BuildFromContext (condition_compiler.go line 422-438), the code uses prefix := key[:3] and then compares with string literals, which is inconsistent and error-prone.

The two implementations should use the same approach for consistency and maintainability. Consider extracting this key parsing logic into a shared helper function.

Copilot · 2025-12-18T13:26:39Z

pkg/inventory/registry_test.go

+}
+
+func TestEnableConditionAndLegacyEnabledInteraction(t *testing.T) {
+	// When both Enabled and EnableCondition are set, both must pass


The test comment incorrectly states the behavior. The comment says "When both Enabled and EnableCondition are set, both must pass", but the implementation in filters.go shows that if either check fails, the tool is excluded. The two checks are sequential, not a logical AND at the same level.

The actual behavior is: if Enabled returns false, the tool is excluded immediately. If Enabled passes (or is nil), then EnableCondition is checked. This is more of a filter chain than a logical AND. Consider clarifying the comment to reflect the sequential nature of the checks.

Suggested change

// When both Enabled and EnableCondition are set, both must pass

// When both Enabled and EnableCondition are set, Enabled is checked first and must pass;

// if it does, EnableCondition is then evaluated and must also pass for the tool to be included.

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

Copilot AI review requested due to automatic review settings December 18, 2025 13:21

SamMorrowDrums requested a review from a team as a code owner December 18, 2025 13:21

Copilot started reviewing on behalf of SamMorrowDrums December 18, 2025 13:21 View session

SamMorrowDrums force-pushed the perf/composable-enable-conditions branch from d64e314 to 3a52f7b Compare December 18, 2025 13:25

Copilot AI reviewed Dec 18, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: composable EnableCondition with bitmask optimization #1641

feat: composable EnableCondition with bitmask optimization #1641

Uh oh!

SamMorrowDrums commented Dec 18, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 18, 2025

Uh oh!

Copilot AI Dec 18, 2025

Uh oh!

Copilot AI Dec 18, 2025

Uh oh!

Copilot AI Dec 18, 2025

Uh oh!

Copilot AI Dec 18, 2025

Uh oh!

Copilot AI Dec 18, 2025

Uh oh!

Copilot AI Dec 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		// If both Enabled and EnableCondition are set, Enabled takes precedence for
		// backward compatibility. Migrate to EnableCondition for new tools.

-	// If both Enabled and EnableCondition are set, Enabled takes precedence for
-	// backward compatibility. Migrate to EnableCondition for new tools.
+	// When both Enabled and EnableCondition are set, Enabled is evaluated first for
+	// backward compatibility, and EnableCondition is then evaluated; both must pass
+	// for the tool to be enabled. Migrate to EnableCondition for new tools.

	// When both Enabled and EnableCondition are set, both must pass
	// When both Enabled and EnableCondition are set, Enabled is checked first and must pass;
	// if it does, EnableCondition is then evaluated and must also pass for the tool to be included.

feat: composable EnableCondition with bitmask optimization #1641

Are you sure you want to change the base?

feat: composable EnableCondition with bitmask optimization #1641

Uh oh!

Conversation

SamMorrowDrums commented Dec 18, 2025

Summary

Key Features

1. User-facing API (conditions.go)

2. Bitmask Compiler (condition_compiler.go)

3. Pre-sorting Optimization (builder.go)

4. Integration (filters.go, registry.go)

Usage Example

Benchmarks

Why This Approach?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

1. User-facing API (`conditions.go`)

2. Bitmask Compiler (`condition_compiler.go`)

3. Pre-sorting Optimization (`builder.go`)

4. Integration (`filters.go`, `registry.go`)