Duplicated condition evaluation logic with inconsistent error messages. Lines 129-139 and 140-150 both evaluatetool.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.

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:

1. Tool.Enabled (legacy)
2. Tool.EnableCondition
3. FeatureFlagEnable/FeatureFlagDisable (legacy)

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

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.

The prefix check logic is inconsistent with the key format. The code checksprefix := 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 casecase "ctx": will never match because context bool keys have the format"ctx:" (4 chars), not"ctx" (3 chars). This should becase "ctx:" to match the actual key format.

Redundant name reassignment. Line 431 reassignsname = 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.

Inconsistent prefix checking logic. InbuildRequestMask (filters.go), the code useskey[:4] == "ctx:" andkey[:3] == "ff:" which correctly matches the key formats. However, inBuildFromContext (condition_compiler.go line 422-438), the code usesprefix := 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.

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: ifEnabled returns false, the tool is excluded immediately. IfEnabled passes (or is nil), thenEnableCondition 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.