From 849cb3e99093836e329d86182d8c67021e7172b4 Mon Sep 17 00:00:00 2001
From: Sean Huh <sokwhan@google.com>
Date: Mon, 17 Aug 2026 15:17:01 -0700
Subject: [PATCH] Avoid concatenating superfluous empty list for aggregate
 semantics

Add more aggregate policy conformance test cases

PiperOrigin-RevId: 966209180
---
 .../java/dev/cel/policy/RuleComposer.java     | 12 +++-
 .../cel/policy/CelPolicyCompilerImplTest.java | 58 ++++++++++++++++++-
 2 files changed, 66 insertions(+), 4 deletions(-)

diff --git a/policy/src/main/java/dev/cel/policy/RuleComposer.java b/policy/src/main/java/dev/cel/policy/RuleComposer.java
index bf667cb93..f98152c62 100644
--- a/policy/src/main/java/dev/cel/policy/RuleComposer.java
+++ b/policy/src/main/java/dev/cel/policy/RuleComposer.java
@@ -148,8 +148,12 @@ private Step optimizeRule(Cel cel, CelCompiledRule compiledRule, boolean asList)
 
   private @Nullable Step createBaseStep(boolean returnList, boolean hasOptionalOutput) {
     if (returnList) {
-      // If the rule is evaluated as a list (AGGREGATE), the base case is an empty list.
-      return Step.newUnconditionalNonOptionalStep(newTrueLiteral(), newList());
+      if (hasOptionalOutput) {
+        // If a nested rule inside an aggregate context has an optional output, the last result in
+        // the ternary should return an empty list to allow concatenation with other branches.
+        return Step.newUnconditionalNonOptionalStep(newTrueLiteral(), newList());
+      }
+      return null;
     }
 
     if (hasOptionalOutput) {
@@ -280,6 +284,10 @@ private Step combineAggregate(AstMutator astMutator, Step currentStep, Step accu
       conditionalListPart = currentListPart;
     }
 
+    if (accumulatedStep == null) {
+      return Step.newUnconditionalNonOptionalStep(trueCondition, conditionalListPart);
+    }
+
     CelMutableAst concatenated =
         astMutator.newGlobalCall(
             Operator.ADD.getFunction(), conditionalListPart, accumulatedStep.expr);
diff --git a/policy/src/test/java/dev/cel/policy/CelPolicyCompilerImplTest.java b/policy/src/test/java/dev/cel/policy/CelPolicyCompilerImplTest.java
index 3fbc8720c..b5894c6df 100644
--- a/policy/src/test/java/dev/cel/policy/CelPolicyCompilerImplTest.java
+++ b/policy/src/test/java/dev/cel/policy/CelPolicyCompilerImplTest.java
@@ -261,8 +261,62 @@ public void compileYamlPolicy_aggregate_macrosPreserved() throws Exception {
     String unparsed = CelUnparserFactory.newUnparser().unparse(ast);
     assertThat(unparsed)
         .isEqualTo(
-            "(cond ? [payload.filter(x, x > 10, x).exists(y, y % 2 == 0)] : []) "
-                + "+ ([payload.all(x, x > 0)] + [])");
+            "(cond ? [payload.filter(x, x > 10).exists(y, y % 2 == 0)] : []) "
+                + "+ [payload.all(x, x > 0)]");
+  }
+
+  @Test
+  public void compileYamlPolicy_aggregateSingleMatch_noSuperfluousConcatenation() throws Exception {
+    String policySource =
+        "name: aggregate_single_match\n"
+            + "rule:\n"
+            + "  aggregate:\n"
+            + "    - condition: \"cond\"\n"
+            + "      output: \"payload.filter(x, x > 10).exists(y, y % 2 == 0)\"\n";
+    Cel cel =
+        newCel()
+            .toCelBuilder()
+            .addVar("cond", SimpleType.BOOL)
+            .addVar("payload", ListType.create(SimpleType.INT))
+            .build();
+
+    CelPolicy policy = POLICY_PARSER.parse(policySource);
+
+    CelAbstractSyntaxTree ast =
+        CelPolicyCompilerFactory.newPolicyCompiler(cel).build().compile(policy);
+
+    String unparsed = CelUnparserFactory.newUnparser().unparse(ast);
+    assertThat(unparsed).isEqualTo("cond ? [payload.filter(x, x > 10).exists(y, y % 2 == 0)] : []");
+  }
+
+  @Test
+  public void compileYamlPolicy_aggregateMultipleConditionalMatches_noSuperfluousConcatenation()
+      throws Exception {
+    String policySource =
+        "name: aggregate_multiple_conditional\n"
+            + "rule:\n"
+            + "  aggregate:\n"
+            + "    - condition: \"cond1\"\n"
+            + "      output: \"payload.all(x, x > 0)\"\n"
+            + "    - condition: \"cond2\"\n"
+            + "      output: \"payload.exists(x, x == 0)\"\n";
+    Cel cel =
+        newCel()
+            .toCelBuilder()
+            .addVar("cond1", SimpleType.BOOL)
+            .addVar("cond2", SimpleType.BOOL)
+            .addVar("payload", ListType.create(SimpleType.INT))
+            .build();
+
+    CelPolicy policy = POLICY_PARSER.parse(policySource);
+
+    CelAbstractSyntaxTree ast =
+        CelPolicyCompilerFactory.newPolicyCompiler(cel).build().compile(policy);
+
+    String unparsed = CelUnparserFactory.newUnparser().unparse(ast);
+    assertThat(unparsed)
+        .isEqualTo(
+            "(cond1 ? [payload.all(x, x > 0)] : []) + (cond2 ? [payload.exists(x, x == 0)] : [])");
   }
 
   @Test