fix(features): fail nested checker resolution closed

Disallow recursive ResolveFeature calls from feature checkers so direct, negating, multi-node, and concurrent cycles cannot cache enabled results or wait on one another.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1e4a1ca6-53f7-4158-af22-35d2448d0b13

Author: SamMorrowDrums

docs/feature-flags.md

@@ -70,6 +70,8 @@ branch.
 The inventory's string-based checker owns request feature state. Once installed,
 that state is authoritative; a checker stored on tool dependencies is used only
 as a fallback when handlers are invoked directly without request state.
+Feature checkers must not call `ResolveFeature`; nested resolution fails the
+owning check closed.
 
 ---
 

pkg/inventory/features.go

@@ -13,9 +13,8 @@ const maxFeatureRuleFlags = 16
 // FeatureFlag identifies a feature consistently across inventory consumers.
 type FeatureFlag string
 
-// FeatureFlagChecker resolves one feature flag for the current request. Every
-// context value needed for availability checks must be installed before the
-// inventory is resolved. Handler-only checks receive the live tool-call context.
+// FeatureFlagChecker resolves one feature flag for the current request. Checkers
+// must not call ResolveFeature; nested resolution fails the owning check closed.
 type FeatureFlagChecker func(ctx context.Context, flag string) (bool, error)
 
 // FeatureResolver returns the resolved value of a feature flag.
@@ -154,6 +153,16 @@ func (s *featureState) enabled(ctx context.Context, feature FeatureFlag) bool {
 	}
 
 	owner := resolvingFeatureFromContext(ctx)
+	if owner != nil {
+		s.mu.Lock()
+		if ownerResult := s.results[owner.flag]; ownerResult != nil {
+			ownerResult.failed = true
+		}
+		s.mu.Unlock()
+		fmt.Fprintf(os.Stderr, "Feature flag checker attempted nested resolution of %q\n", feature)
+		return false
+	}
+
 	s.mu.Lock()
 	if s.results == nil {
 		s.results = make(map[FeatureFlag]*featureResult)
@@ -165,15 +174,6 @@ func (s *featureState) enabled(ctx context.Context, feature FeatureFlag) bool {
 			s.mu.Unlock()
 			return enabled
 		}
-		if owner != nil {
-			result.failed = true
-			if ownerResult := s.results[owner.flag]; ownerResult != nil {
-				ownerResult.failed = true
-			}
-			s.mu.Unlock()
-			fmt.Fprintf(os.Stderr, "Feature flag resolution cycle detected for %q\n", feature)
-			return false
-		}
 		for !result.done {
 			s.cond.Wait()
 		}

pkg/inventory/features_test.go

@@ -146,7 +146,7 @@ func TestLazyFeatureResolutionUsesLiveContext(t *testing.T) {
 	assert.True(t, ResolveFeature(ctx, checker, "handler_only"))
 }
 
-func TestFeatureResolutionIsReentrantAcrossFlags(t *testing.T) {
+func TestNestedFeatureResolutionFailsClosed(t *testing.T) {
 	var checker FeatureFlagChecker
 	checker = func(ctx context.Context, flag string) (bool, error) {
 		if flag == "meta" {
@@ -156,7 +156,8 @@ func TestFeatureResolutionIsReentrantAcrossFlags(t *testing.T) {
 	}
 
 	ctx := WithFeatureState(context.Background(), checker)
-	assert.True(t, ResolveFeature(ctx, nil, "meta"))
+	assert.False(t, ResolveFeature(ctx, nil, "meta"))
+	assert.True(t, ResolveFeature(ctx, nil, "base"))
 }
 
 func TestDirectFeatureResolutionCycleFailsClosed(t *testing.T) {
@@ -197,6 +198,27 @@ func TestMutualFeatureResolutionCycleFailsClosed(t *testing.T) {
 	assert.False(t, ResolveFeature(ctx, nil, "b"))
 }
 
+func TestThreeNodeFeatureResolutionCycleFailsClosed(t *testing.T) {
+	var checker FeatureFlagChecker
+	checker = func(ctx context.Context, flag string) (bool, error) {
+		switch flag {
+		case "a":
+			return !ResolveFeature(ctx, checker, "b"), nil
+		case "b":
+			return !ResolveFeature(ctx, checker, "c"), nil
+		case "c":
+			return !ResolveFeature(ctx, checker, "a"), nil
+		default:
+			return false, nil
+		}
+	}
+
+	ctx := WithFeatureState(context.Background(), checker)
+	assert.False(t, ResolveFeature(ctx, nil, "a"))
+	assert.False(t, ResolveFeature(ctx, nil, "b"))
+	assert.False(t, ResolveFeature(ctx, nil, "c"))
+}
+
 func TestConcurrentFeatureResolutionIsDeduplicated(t *testing.T) {
 	var (
 		calls   int