Skip to content

Commit d12e4e9

Browse files
baywetCopilot
baywet
and
Copilot
authored
.NET: fix(workflows): preserve expression result metadata (#9457)
* fix(workflows): preserve expression result metadata Co-authored-by: Copilot <[email protected]> Copilot-Session: a1abb9bc-cfb6-4a08-ba40-c08005f37163 * fix(workflows): reconcile expression error handling after merge Co-authored-by: Copilot <[email protected]> Copilot-Session: a1abb9bc-cfb6-4a08-ba40-c08005f37163 --------- Co-authored-by: Copilot <[email protected]> Copilot-Session: a1abb9bc-cfb6-4a08-ba40-c08005f37163
1 parent 501cc6f commit d12e4e9

5 files changed

Lines changed: 214 additions & 17 deletions

File tree

‎dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/PowerFx/WorkflowExpressionEngine.cs‎

Lines changed: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
using System;
44
using System.Collections.Generic;
55
using System.Collections.Immutable;
6+
using System.Diagnostics;
67
using System.Linq;
78
using Microsoft.Agents.AI.Workflows.Declarative.Extensions;
89
using Microsoft.Agents.ObjectModel;
@@ -107,7 +108,7 @@ private EvaluationResult<bool> Evaluate(BoolExpression expression)
107108

108109
if (expressionResult.Value is BlankValue)
109110
{
110-
return new EvaluationResult<bool>(default, SensitivityLevel.None);
111+
return new EvaluationResult<bool>(default, expressionResult.Sensitivity);
111112
}
112113

113114
if (expressionResult.Value is not BooleanValue formulaValue)
@@ -327,42 +328,65 @@ private EvaluationResult<FormulaValue> EvaluateScope(ExpressionBase expression)
327328
expression.VariableReference?.ToString() :
328329
expression.ExpressionText;
329330

331+
ImmutableArray<string> sensitiveReferences = this.GetSensitiveReferences(expression);
332+
SensitivityLevel sensitivity = sensitiveReferences.IsEmpty ? SensitivityLevel.None : SensitivityLevel.Sensitive;
330333
FormulaValue result = this._state.Engine.Eval(expressionText, options: this._parserOptions);
331334

335+
if (sensitivity == SensitivityLevel.Sensitive && ContainsError(result))
336+
{
337+
Trace.TraceWarning(
338+
"Expression evaluation returned a blank result for references: {0}.",
339+
string.Join(", ", sensitiveReferences));
340+
return new(FormulaValue.NewBlank(result.Type), sensitivity);
341+
}
342+
332343
if (result is ErrorValue errorValue)
333344
{
334-
throw new DeclarativeActionException(
335-
this.GetSensitivity(expression) == SensitivityLevel.Sensitive
336-
? "Expression evaluation failed for a sensitive value."
337-
: errorValue.Format());
345+
throw new DeclarativeActionException(errorValue.Format());
338346
}
339347

340-
return new(result, this.GetSensitivity(expression));
348+
return new(result, sensitivity);
341349
}
342350

343-
private SensitivityLevel GetSensitivity(ExpressionBase expression)
351+
private static bool ContainsError(FormulaValue value) =>
352+
value switch
353+
{
354+
ErrorValue => true,
355+
RecordValue recordValue => recordValue.OriginalFields.Any(field => ContainsError(field.Value)),
356+
TableValue tableValue => tableValue.Rows.Any(row => row.IsError || (row.IsValue && ContainsError(row.Value))),
357+
_ => false,
358+
};
359+
360+
private ImmutableArray<string> GetSensitiveReferences(ExpressionBase expression)
344361
{
345362
if (expression.VariableReference is { VariableName: string variableName })
346363
{
347-
return GetReferenceSensitivity(expression.VariableReference.NamespaceAlias, variableName);
364+
return GetReferenceSensitivity(expression.VariableReference.NamespaceAlias, variableName) == SensitivityLevel.Sensitive
365+
? [expression.VariableReference.ToString()]
366+
: [];
348367
}
349368

350369
string? expressionText = expression.ExpressionText;
351370
if (string.IsNullOrWhiteSpace(expressionText))
352371
{
353-
return SensitivityLevel.None;
372+
return [];
354373
}
355374

356375
CheckResult checkResult = this._state.Engine.Check(expressionText, options: this._parserOptions);
357376
checkResult.ThrowOnErrors();
358377

359-
SensitivityLevel sensitivity = SensitivityLevel.None;
378+
HashSet<string> sensitiveReferences = new(StringComparer.Ordinal);
360379
foreach ((string? ScopeName, string VariableName) reference in GetVariableReferences(checkResult.Parse.Root))
361380
{
362-
sensitivity = MaxSensitivity(sensitivity, GetReferenceSensitivity(reference.ScopeName, reference.VariableName));
381+
if (GetReferenceSensitivity(reference.ScopeName, reference.VariableName) == SensitivityLevel.Sensitive)
382+
{
383+
sensitiveReferences.Add(reference.ScopeName is null
384+
? reference.VariableName
385+
: $"{reference.ScopeName}.{reference.VariableName}");
386+
}
363387
}
364388

365-
return sensitivity;
389+
return [.. sensitiveReferences];
366390

367391
SensitivityLevel GetReferenceSensitivity(string? scopeName, string variableName) =>
368392
scopeName is null && VariableScopeNames.IsValidName(variableName)

‎dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,14 @@ Function approval denials complete without evaluating automatic-send settings.
4040
Older approved checkpoints without a captured setting still validate the current
4141
setting before invoking the function.
4242

43+
### Expression result handling
44+
45+
Blank expression results retain their input sensitivity metadata. Evaluation
46+
errors in sensitive expressions, including record fields and table rows, produce
47+
a blank result with the same metadata so data assignments can continue.
48+
Diagnostic warnings identify the referenced variable names, not their values.
49+
Non-sensitive expression errors retain their existing exception behavior.
50+
4351
### Function tool approval
4452

4553
`InvokeFunctionTool` requests approval when `requireApproval` evaluates to `true`

‎dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/ObjectModel/ConditionGroupExecutorTest.cs‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,29 @@ public async Task ConditionGroupRejectsSensitiveConditionAsync(string scope, str
126126
Assert.DoesNotContain(matches ? "enabled" : "disabled", exception.Message);
127127
}
128128

129+
[Theory]
130+
[InlineData(true, "If(Env.Setting, false)")]
131+
[InlineData(false, "If(Env.Setting, false)")]
132+
[InlineData(true, "If(Env.Setting, 1 / 0 = 0, false)")]
133+
[InlineData(false, "If(Env.Setting, 1 / 0 = 0, false)")]
134+
public async Task ConditionGroupValidatesConditionalResultsAsync(bool value, string expression)
135+
{
136+
// Arrange
137+
this.State.Set("Setting", FormulaValue.New(value), VariableScopeNames.Environment, SensitivityLevel.Sensitive);
138+
ConditionGroup.Builder builder = this.CreateModel(
139+
nameof(ConditionGroupValidatesConditionalResultsAsync), [true], includeElse: true).ToBuilder();
140+
builder.Conditions[0].Condition = BoolExpression.Expression(expression).ToBuilder();
141+
ConditionGroupExecutor action = new(AssignParent<ConditionGroup>(builder), this.State);
142+
143+
// Act
144+
Task ExecuteAsync() => this.ExecuteAsync(action, isDiscrete: false);
145+
146+
// Assert
147+
DeclarativeActionException exception = await Assert.ThrowsAsync<DeclarativeActionException>(ExecuteAsync);
148+
Assert.Contains("condition", exception.Message);
149+
Assert.DoesNotContain("ErrDivByZero", exception.Message);
150+
}
151+
129152
[Theory]
130153
[InlineData(true)]
131154
[InlineData(false)]

‎dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/ObjectModel/SetVariableExecutorTest.cs‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,40 @@ await this.ExecuteTestAsync(
148148
expectedValue: FormulaValue.New(42));
149149
}
150150

151+
[Theory]
152+
[InlineData(true)]
153+
[InlineData(false)]
154+
public async Task SetConditionalExpressionContinuesAsync(bool value)
155+
{
156+
// Arrange
157+
this.State.Set("Setting", FormulaValue.New(value), VariableScopeNames.Environment, SensitivityLevel.Sensitive);
158+
SetVariable model = this.CreateModel(
159+
nameof(SetConditionalExpressionContinuesAsync),
160+
FormatVariablePath("Result"),
161+
new ValueExpression.Builder(ValueExpression.Expression("If(Env.Setting, 1 / 0, 1)")));
162+
SetVariableExecutor action = new(model, this.State);
163+
bool continued = false;
164+
165+
// Act
166+
await this.ExecuteAsync(action, "next_action", (context, message, cancellationToken) =>
167+
{
168+
continued = true;
169+
return default;
170+
});
171+
172+
// Assert
173+
Assert.True(continued);
174+
Assert.Equal(SensitivityLevel.Sensitive, this.State.GetSensitivity("Result"));
175+
if (value)
176+
{
177+
Assert.IsType<BlankValue>(this.State.Get("Result"));
178+
}
179+
else
180+
{
181+
this.VerifyState("Result", FormulaValue.New(1));
182+
}
183+
}
184+
151185
private Task ExecuteTestAsync(
152186
string displayName,
153187
string variableName,

‎dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/PowerFx/WorkflowExpressionEngineTests.cs‎

Lines changed: 113 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
using System;
44
using System.Collections.Immutable;
5+
using System.Diagnostics;
6+
using System.IO;
57
using Microsoft.Agents.AI.Workflows.Declarative.Extensions;
68
using Microsoft.Agents.AI.Workflows.Declarative.PowerFx;
79
using Microsoft.Agents.ObjectModel;
@@ -48,20 +50,19 @@ public WorkflowExpressionEngineTests(ITestOutputHelper output)
4850
[InlineData("Value(Env.Setting)", VariableScopeNames.Environment)]
4951
[InlineData("Error(Env.Setting)", VariableScopeNames.Environment)]
5052
[InlineData("Error({Kind: ErrorKind.Custom, Message: Local.Setting})", VariableScopeNames.Local)]
51-
public void ExpressionErrorOmitsSensitiveInput(string expression, string scope)
53+
public void ExpressionErrorReturnsLabelledBlankForSensitiveInput(string expression, string scope)
5254
{
5355
// Arrange
5456
const string Value = "sample-setting";
5557
this.State.Set("Setting", FormulaValue.New(Value), scope, SensitivityLevel.Sensitive);
5658
this.State.Bind();
5759

5860
// Act
59-
DeclarativeActionException exception = Assert.Throws<DeclarativeActionException>(
60-
() => this.State.Evaluator.GetValue(ValueExpression.Expression(expression)));
61+
EvaluationResult<DataValue> result = this.State.Evaluator.GetValue(ValueExpression.Expression(expression));
6162

6263
// Assert
63-
Assert.DoesNotContain(Value, exception.ToString(), StringComparison.Ordinal);
64-
Assert.Null(exception.InnerException);
64+
Assert.Equal(DataValue.Blank(), result.Value);
65+
Assert.Equal(SensitivityLevel.Sensitive, result.Sensitivity);
6566
}
6667

6768
[Theory]
@@ -108,6 +109,113 @@ public void BoolExpressionGetValueForBlank() =>
108109
BoolExpression.Variable(PropertyPath.TopicVariable(Variables.BlankValue)),
109110
expectedValue: false);
110111

112+
[Theory]
113+
[InlineData(true)]
114+
[InlineData(false)]
115+
public void BoolExpressionGetValueForConditionalBlankPreservesSensitivity(bool value)
116+
{
117+
// Arrange
118+
this.State.Set("Setting", FormulaValue.New(value), VariableScopeNames.Environment, SensitivityLevel.Sensitive);
119+
this.State.Bind();
120+
121+
// Act & Assert
122+
this.EvaluateExpression(
123+
BoolExpression.Expression("If(Env.Setting, false)"),
124+
expectedValue: false,
125+
expectedSensitivity: SensitivityLevel.Sensitive);
126+
}
127+
128+
[Theory]
129+
[InlineData(true)]
130+
[InlineData(false)]
131+
public void ValueExpressionGetValueForConditionalErrorPreservesSensitivity(bool value)
132+
{
133+
// Arrange
134+
this.State.Set("Setting", FormulaValue.New(value), VariableScopeNames.Environment, SensitivityLevel.Sensitive);
135+
this.State.Bind();
136+
137+
// Act
138+
EvaluationResult<DataValue> result =
139+
this.State.Evaluator.GetValue(ValueExpression.Expression("If(Env.Setting, 1 / 0, 1)"));
140+
141+
// Assert
142+
Assert.Equal(SensitivityLevel.Sensitive, result.Sensitivity);
143+
Assert.Equal(value ? DataValue.Blank() : NumberDataValue.Create(1), result.Value);
144+
}
145+
146+
[Theory]
147+
[InlineData(VariableScopeNames.Environment, "Env")]
148+
[InlineData(VariableScopeNames.Local, "Local")]
149+
public void StringExpressionGetValueForConversionErrorPreservesSensitivity(string scope, string alias)
150+
{
151+
// Arrange
152+
this.State.Set("Setting", FormulaValue.New("not-a-number"), scope, SensitivityLevel.Sensitive);
153+
this.State.Bind();
154+
155+
// Act & Assert
156+
this.EvaluateExpression(
157+
StringExpression.Expression($"Text(Value({alias}.Setting))"),
158+
expectedValue: string.Empty,
159+
expectedSensitivity: SensitivityLevel.Sensitive);
160+
}
161+
162+
[Fact]
163+
public void ValueExpressionGetValueForNonSensitiveErrorStillThrows()
164+
{
165+
// Arrange
166+
ValueExpression expression = ValueExpression.Expression("1 / 0");
167+
168+
// Act & Assert
169+
Assert.Throws<DeclarativeActionException>(() => this.State.Evaluator.GetValue(expression));
170+
}
171+
172+
[Theory]
173+
[InlineData("{ Item: Value(Env.Setting) }")]
174+
[InlineData("[Value(Env.Setting)]")]
175+
[InlineData("Table({ Item: Value(Env.Setting) })")]
176+
[InlineData("{ Nested: Table({ Item: Value(Env.Setting) }) }")]
177+
public void ValueExpressionGetValueForNestedErrorPreservesSensitivity(string expression)
178+
{
179+
// Arrange
180+
this.State.Set("Setting", FormulaValue.New("not-a-number"), VariableScopeNames.Environment, SensitivityLevel.Sensitive);
181+
this.State.Bind();
182+
183+
// Act & Assert
184+
this.EvaluateExpression(
185+
ValueExpression.Expression(expression),
186+
expectedValue: DataValue.Blank(),
187+
expectedSensitivity: SensitivityLevel.Sensitive);
188+
}
189+
190+
[Theory]
191+
[InlineData("Value(Env.Setting)", VariableScopeNames.Environment)]
192+
[InlineData("Error(Env.Setting)", VariableScopeNames.Environment)]
193+
[InlineData("Error({Kind: ErrorKind.Custom, Message: Local.Setting})", VariableScopeNames.Local)]
194+
public void ExpressionErrorDiagnosticUsesReferenceName(string expression, string scope)
195+
{
196+
// Arrange
197+
this.State.Set("Setting", FormulaValue.New("not-a-number"), scope, SensitivityLevel.Sensitive);
198+
this.State.Bind();
199+
using StringWriter output = new();
200+
using TextWriterTraceListener listener = new(output);
201+
Trace.Listeners.Add(listener);
202+
203+
try
204+
{
205+
// Act
206+
this.State.Evaluator.GetValue(ValueExpression.Expression(expression));
207+
listener.Flush();
208+
209+
// Assert
210+
Assert.Contains($"{scope}.Setting", output.ToString());
211+
Assert.DoesNotContain("not-a-number", output.ToString());
212+
}
213+
finally
214+
{
215+
Trace.Listeners.Remove(listener);
216+
}
217+
}
218+
111219
[Fact]
112220
public void BoolExpressionGetValueForVariable()
113221
{

0 commit comments

Comments
 (0)