mirror of
https://github.com/microsoft/agent-framework.git
synced 2026-06-16 21:04:09 +08:00
.NET: Fix QuestionExecutor looping after GotoAction re-entry in declarative workflows (#5635)
* Fix QuestionExecutor looping after GotoAction re-entry in declarative workflows * Addressed failing integration test and promptcount
This commit is contained in:
committed by
GitHub
Unverified
parent
6f86debb81
commit
d2e694dfe1
+24
-10
@@ -43,10 +43,11 @@ internal sealed class QuestionExecutor(Question model, ResponseAgentProvider age
|
||||
|
||||
protected override async ValueTask<object?> ExecuteAsync(IWorkflowContext context, CancellationToken cancellationToken = default)
|
||||
{
|
||||
await this._promptCount.WriteAsync(context, 0).ConfigureAwait(false);
|
||||
|
||||
InitializablePropertyPath variable = Throw.IfNull(this.Model.Variable);
|
||||
bool isValueUndefined = context.ReadState(variable.Path) is BlankValue;
|
||||
// Snapshot prior-execution state before we mutate it below so the SkipQuestionMode
|
||||
// evaluation reflects whether this is the first time the action has run.
|
||||
bool hasExecutedPreviously = await this._hasExecuted.ReadAsync(context).ConfigureAwait(false);
|
||||
bool proceed = this.Evaluator.GetValue(this.Model.AlwaysPrompt).Value;
|
||||
|
||||
if (!proceed)
|
||||
@@ -55,16 +56,23 @@ internal sealed class QuestionExecutor(Question model, ResponseAgentProvider age
|
||||
proceed =
|
||||
mode switch
|
||||
{
|
||||
SkipQuestionMode.SkipOnFirstExecutionIfVariableHasValue => isValueUndefined && !await this._hasExecuted.ReadAsync(context).ConfigureAwait(false),
|
||||
SkipQuestionMode.SkipOnFirstExecutionIfVariableHasValue => isValueUndefined || hasExecutedPreviously,
|
||||
SkipQuestionMode.AlwaysSkipIfVariableHasValue => isValueUndefined,
|
||||
SkipQuestionMode.AlwaysAsk => true,
|
||||
_ => true,
|
||||
};
|
||||
}
|
||||
|
||||
// Record that the action has executed in the same executor scope as the read above.
|
||||
// (CaptureResponseAsync runs in a different executor's state scope, so writing it there
|
||||
// would not be visible to subsequent ExecuteAsync invocations triggered by GotoAction.)
|
||||
await this._hasExecuted.WriteAsync(context, true).ConfigureAwait(false);
|
||||
|
||||
if (proceed)
|
||||
{
|
||||
await this.PromptAsync(context, cancellationToken).ConfigureAwait(false);
|
||||
// Initial prompt: count is 0 because no responses have been received yet for this turn.
|
||||
// _promptCount itself is tracked in CaptureResponseAsync's scope (see comment on _promptCount).
|
||||
await this.PromptAsync(context, actualCount: 0, cancellationToken).ConfigureAwait(false);
|
||||
}
|
||||
else
|
||||
{
|
||||
@@ -76,14 +84,18 @@ internal sealed class QuestionExecutor(Question model, ResponseAgentProvider age
|
||||
|
||||
public async ValueTask PrepareResponseAsync(IWorkflowContext context, ActionExecutorResult message, CancellationToken cancellationToken)
|
||||
{
|
||||
int count = await this._promptCount.ReadAsync(context).ConfigureAwait(false);
|
||||
ExternalInputRequest inputRequest = new(this.FormatPrompt(this.Model.Prompt));
|
||||
await context.SendMessageAsync(inputRequest, cancellationToken).ConfigureAwait(false);
|
||||
await this._promptCount.WriteAsync(context, count + 1).ConfigureAwait(false);
|
||||
}
|
||||
|
||||
public async ValueTask CaptureResponseAsync(IWorkflowContext context, ExternalInputResponse response, CancellationToken cancellationToken)
|
||||
{
|
||||
// _promptCount is tracked in this (Capture) executor's scope so reads and writes are coherent.
|
||||
// Each Capture invocation represents an attempt to satisfy the question; increment up front
|
||||
// and pass the value to PromptAsync explicitly so the retry/default decision is scope-independent.
|
||||
int promptCount = await this._promptCount.ReadAsync(context).ConfigureAwait(false) + 1;
|
||||
await this._promptCount.WriteAsync(context, promptCount).ConfigureAwait(false);
|
||||
|
||||
FormulaValue? extractedValue = null;
|
||||
if (!response.HasMessages)
|
||||
{
|
||||
@@ -106,10 +118,12 @@ internal sealed class QuestionExecutor(Question model, ResponseAgentProvider age
|
||||
|
||||
if (extractedValue is null)
|
||||
{
|
||||
await this.PromptAsync(context, cancellationToken).ConfigureAwait(false);
|
||||
await this.PromptAsync(context, promptCount, cancellationToken).ConfigureAwait(false);
|
||||
}
|
||||
else
|
||||
{
|
||||
// Reset for any subsequent Question turn (e.g. via GotoAction re-entry) so the next attempt starts fresh.
|
||||
await this._promptCount.WriteAsync(context, 0).ConfigureAwait(false);
|
||||
bool autoSend = true;
|
||||
|
||||
if (this.Model.ExtensionData?.Properties.TryGetValue("autoSend", out DataValue? autoSendValue) ?? false)
|
||||
@@ -133,7 +147,6 @@ internal sealed class QuestionExecutor(Question model, ResponseAgentProvider age
|
||||
}
|
||||
|
||||
await this.AssignAsync(Throw.IfNull(this.Model.Variable).Path, extractedValue, context).ConfigureAwait(false);
|
||||
await this._hasExecuted.WriteAsync(context, true).ConfigureAwait(false);
|
||||
await context.SendResultMessageAsync(this.Id, cancellationToken).ConfigureAwait(false);
|
||||
}
|
||||
}
|
||||
@@ -143,10 +156,9 @@ internal sealed class QuestionExecutor(Question model, ResponseAgentProvider age
|
||||
await context.RaiseCompletionEventAsync(this.Model, cancellationToken).ConfigureAwait(false);
|
||||
}
|
||||
|
||||
private async ValueTask PromptAsync(IWorkflowContext context, CancellationToken cancellationToken)
|
||||
private async ValueTask PromptAsync(IWorkflowContext context, int actualCount, CancellationToken cancellationToken)
|
||||
{
|
||||
long repeatCount = this.Evaluator.GetValue(this.Model.RepeatCount).Value;
|
||||
int actualCount = await this._promptCount.ReadAsync(context).ConfigureAwait(false);
|
||||
if (actualCount >= repeatCount)
|
||||
{
|
||||
DataValue defaultValue = DataValue.Blank();
|
||||
@@ -158,6 +170,8 @@ internal sealed class QuestionExecutor(Question model, ResponseAgentProvider age
|
||||
await this.AssignAsync(Throw.IfNull(this.Model.Variable).Path, defaultValue.ToFormula(), context).ConfigureAwait(false);
|
||||
string defaultValueResponse = this.FormatPrompt(this.Model.DefaultValueResponse);
|
||||
await context.AddEventAsync(new MessageActivityEvent(defaultValueResponse.Trim()), cancellationToken).ConfigureAwait(false);
|
||||
// Reset for any subsequent Question turn (e.g. via GotoAction re-entry) so the next attempt starts fresh.
|
||||
await this._promptCount.WriteAsync(context, 0).ConfigureAwait(false);
|
||||
await context.SendResultMessageAsync(this.Id, cancellationToken).ConfigureAwait(false);
|
||||
}
|
||||
else
|
||||
|
||||
+1
-1
@@ -33,7 +33,7 @@ public sealed class DeclarativeWorkflowTest(ITestOutputHelper output) : Workflow
|
||||
public Task ValidateScenarioAsync(string workflowFileName, string testcaseFileName, bool externalConveration = false) =>
|
||||
this.RunWorkflowAsync(GetWorkflowPath(workflowFileName, isSample: true), testcaseFileName, externalConveration);
|
||||
|
||||
[Theory(Skip = "Multi-turn tests hang in CI - needs investigation")]
|
||||
[Theory]
|
||||
[InlineData("ConfirmInput.yaml", "ConfirmInput.json", false)]
|
||||
[InlineData("RequestExternalInput.yaml", "RequestExternalInput.json", false)]
|
||||
public Task ValidateMultiTurnAsync(string workflowFileName, string testcaseFileName, bool isSample) =>
|
||||
|
||||
+17
-4
@@ -1,11 +1,15 @@
|
||||
{
|
||||
"description": "Human in the loop sample - RequestExternalInput.yaml.",
|
||||
"description": "Human in the loop sample - ConfirmInput.yaml. First response mismatches to exercise the GotoAction re-prompt path; second response matches and completes the workflow.",
|
||||
"setup": {
|
||||
"input": {
|
||||
"type": "String",
|
||||
"value": "1234"
|
||||
},
|
||||
"responses": [
|
||||
{
|
||||
"type": "String",
|
||||
"value": "9999"
|
||||
},
|
||||
{
|
||||
"type": "String",
|
||||
"value": "1234"
|
||||
@@ -14,12 +18,21 @@
|
||||
},
|
||||
"validation": {
|
||||
"conversation_count": 1,
|
||||
"min_action_count": 4,
|
||||
"min_action_count": 6,
|
||||
"max_action_count": -1,
|
||||
"min_response_count": 0,
|
||||
"min_response_count": 2,
|
||||
"max_response_count": -1,
|
||||
"min_message_count": 0,
|
||||
"max_message_count": -1,
|
||||
"actions": {
|
||||
"start": [
|
||||
"set_project"
|
||||
"set_project",
|
||||
"question_confirm"
|
||||
],
|
||||
"repeat": [
|
||||
"sendActivity_mismatch",
|
||||
"goto_again",
|
||||
"question_confirm"
|
||||
],
|
||||
"final": [
|
||||
"sendActivity_confirmed"
|
||||
|
||||
+1
-1
@@ -15,7 +15,7 @@
|
||||
"validation": {
|
||||
"conversation_count": 1,
|
||||
"min_action_count": 2,
|
||||
"min_response_count": 0,
|
||||
"min_response_count": 1,
|
||||
"min_message_count": 1,
|
||||
"actions": {
|
||||
"start": [
|
||||
|
||||
Reference in New Issue
Block a user