Skip to content

[release/13.0] Make IReportingStep and IReportingTask completion idempotent #12613

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Nov 3, 2025
Merged

[release/13.0] Make IReportingStep and IReportingTask completion idempotent #12613

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0963ce8
Initial plan
Copilot Nov 1, 2025
1c346ec
Make state changes on IReportingStep and IReportingTask idempotent
Copilot Nov 1, 2025
c43799f
Make ALL terminal state transitions idempotent (noop)
Copilot Nov 1, 2025
d543033
Extract helper methods for clearing activities and asserting no activ…
Copilot Nov 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/Aspire.Hosting/Pipelines/PipelineActivityReporter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,10 @@ public async Task CompleteStepAsync(ReportingStep step, string completionText, C
{
lock (step)
{
// Prevent double completion if the step is already complete
// If the step is already in a terminal state, this is a noop (idempotent)
if (step.CompletionState != CompletionState.InProgress)
{
throw new InvalidOperationException($"Cannot complete step '{step.Id}' with state '{step.CompletionState}'. Only 'InProgress' steps can be completed.");
return;
}

step.CompletionState = completionState;
Expand Down Expand Up @@ -196,9 +196,10 @@ public async Task CompleteTaskAsync(ReportingTask task, CompletionState completi
throw new InvalidOperationException($"Parent step with ID '{task.StepId}' does not exist.");
}

// If the task is already in a terminal state, this is a noop (idempotent)
if (task.CompletionState != CompletionState.InProgress)
{
throw new InvalidOperationException($"Cannot complete task '{task.Id}' with state '{task.CompletionState}'. Only 'InProgress' tasks can be completed.");
return;
}

lock (parentStep)
Expand Down
267 changes: 248 additions & 19 deletions tests/Aspire.Hosting.Tests/Publishing/PipelineActivityReporterTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ public async Task CompleteTaskAsync_WithNullCompletionMessage_SetsEmptyString()
}

[Fact]
public async Task CompleteTaskAsync_ThrowsWhenTaskAlreadyCompleted()
public async Task CompleteTaskAsync_IdempotentWhenCompletedWithSameState()
{
// Arrange
var reporter = CreatePublishingReporter();
Expand All @@ -387,16 +387,46 @@ public async Task CompleteTaskAsync_ThrowsWhenTaskAlreadyCompleted()
// Complete the task first time
await task.CompleteAsync(null, cancellationToken: CancellationToken.None);

// Act & Assert - Try to complete the same task again
var exception = await Assert.ThrowsAsync<InvalidOperationException>(
() => task.CompleteAsync(null, cancellationToken: CancellationToken.None));
// Clear activities
ClearActivities(reporter);

// Act - Try to complete the same task again with the same state (should be idempotent, no exception)
await task.CompleteAsync(null, cancellationToken: CancellationToken.None);

// Assert - No new activity should be emitted (noop)
AssertNoActivitiesEmitted(reporter);

var taskInternal = Assert.IsType<ReportingTask>(task);
Assert.Contains($"Cannot complete task '{taskInternal.Id}' with state 'Completed'. Only 'InProgress' tasks can be completed.", exception.Message);
Assert.Equal(CompletionState.Completed, taskInternal.CompletionState);
}

[Fact]
public async Task CompleteStepAsync_ThrowsWhenStepAlreadyCompleted()
public async Task CompleteTaskAsync_IdempotentWhenCompletedWithDifferentState()
{
// Arrange
var reporter = CreatePublishingReporter();

var step = await reporter.CreateStepAsync("Test Step", CancellationToken.None);
var task = await step.CreateTaskAsync("Test Task", CancellationToken.None);

// Complete the task first time successfully
await task.CompleteAsync(null, CompletionState.Completed, cancellationToken: CancellationToken.None);

// Clear activities
ClearActivities(reporter);

// Act - Try to complete with a different state (should be idempotent, no exception)
await task.CompleteAsync("Error", CompletionState.CompletedWithError, cancellationToken: CancellationToken.None);

// Assert - No new activity should be emitted (noop)
AssertNoActivitiesEmitted(reporter);

var taskInternal = Assert.IsType<ReportingTask>(task);
Assert.Equal(CompletionState.Completed, taskInternal.CompletionState); // Original state is retained
}

[Fact]
public async Task CompleteStepAsync_IdempotentWhenCompletedWithSameState()
{
// Arrange
var reporter = CreatePublishingReporter();
Expand All @@ -406,12 +436,43 @@ public async Task CompleteStepAsync_ThrowsWhenStepAlreadyCompleted()
// Complete the step first time
await step.CompleteAsync("Complete", cancellationToken: CancellationToken.None);

// Act & Assert - Try to complete the same step again
var exception = await Assert.ThrowsAsync<InvalidOperationException>(
() => step.CompleteAsync("Complete again", cancellationToken: CancellationToken.None));
// Clear activities
ClearActivities(reporter);

// Act - Try to complete the same step again with the same state (should be idempotent, no exception)
await step.CompleteAsync("Complete again", cancellationToken: CancellationToken.None);

// Assert - No new activity should be emitted (noop)
AssertNoActivitiesEmitted(reporter);

var stepInternal = Assert.IsType<ReportingStep>(step);
Assert.Contains($"Cannot complete step '{stepInternal.Id}' with state 'Completed'. Only 'InProgress' steps can be completed.", exception.Message);
Assert.Equal(CompletionState.Completed, stepInternal.CompletionState);
Assert.Equal("Complete", stepInternal.CompletionText); // Original completion text is retained
}

[Fact]
public async Task CompleteStepAsync_IdempotentWhenCompletedWithDifferentState()
{
// Arrange
var reporter = CreatePublishingReporter();

var step = await reporter.CreateStepAsync("Test Step", CancellationToken.None);

// Complete the step first time successfully
await step.CompleteAsync("Complete", CompletionState.Completed, cancellationToken: CancellationToken.None);

// Clear activities
ClearActivities(reporter);

// Act - Try to complete with a different state (should be idempotent, no exception)
await step.CompleteAsync("Error", CompletionState.CompletedWithError, cancellationToken: CancellationToken.None);

// Assert - No new activity should be emitted (noop)
AssertNoActivitiesEmitted(reporter);

var stepInternal = Assert.IsType<ReportingStep>(step);
Assert.Equal(CompletionState.Completed, stepInternal.CompletionState); // Original state is retained
Assert.Equal("Complete", stepInternal.CompletionText); // Original completion text is retained
}

[Fact]
Expand All @@ -436,10 +497,9 @@ public async Task CompleteStepAsync_KeepsStepInDictionaryForAggregation()
var taskInternal = Assert.IsType<ReportingTask>(task);
Assert.Contains($"Cannot update task '{taskInternal.Id}' because its parent step", updateException.Message);

// For CompleteTaskAsync, it will first check if the task is already completed, so we expect that error instead
var completeException = await Assert.ThrowsAsync<InvalidOperationException>(
() => task.CompleteAsync(null, cancellationToken: CancellationToken.None));
Assert.Contains($"Cannot complete task '{taskInternal.Id}' with state 'Completed'. Only 'InProgress' tasks can be completed.", completeException.Message);
// For CompleteTaskAsync, since task is already completed, attempting to complete with same or different state should be idempotent
await task.CompleteAsync(null, cancellationToken: CancellationToken.None); // Should not throw
await task.CompleteAsync("Error", CompletionState.CompletedWithError, cancellationToken: CancellationToken.None); // Should also not throw (noop)

// Creating new tasks for the completed step should also fail because the step is complete
var createException = await Assert.ThrowsAsync<InvalidOperationException>(
Expand Down Expand Up @@ -630,7 +690,7 @@ public async Task DisposeAsync_StepWithCompletedTasks_CompletesWithSuccessState(
await task2.SucceedAsync(null, CancellationToken.None);

// Clear previous activities
while (reporter.ActivityItemUpdated.Reader.TryRead(out _)) { }
ClearActivities(reporter);

// Act
await step.DisposeAsync();
Expand Down Expand Up @@ -661,13 +721,13 @@ public async Task DisposeAsync_StepAlreadyCompleted_DoesNotCompleteAgain()
await step.CompleteAsync("Step completed manually", CompletionState.Completed, CancellationToken.None);

// Clear activities
while (reporter.ActivityItemUpdated.Reader.TryRead(out _)) { }
ClearActivities(reporter);

// Act - Dispose should not cause another completion
await step.DisposeAsync();

// Assert - No new activities should be emitted
Assert.False(reporter.ActivityItemUpdated.Reader.TryRead(out _));
AssertNoActivitiesEmitted(reporter);
var stepInternal = Assert.IsType<ReportingStep>(step);
Assert.Equal(CompletionState.Completed, stepInternal.CompletionState);
Assert.Equal("Step completed manually", stepInternal.CompletionText);
Expand Down Expand Up @@ -876,13 +936,13 @@ public async Task Log_DoesNothingWhenStepIsComplete()
await step.CompleteAsync("Completed", CompletionState.Completed, CancellationToken.None);

// Clear activities
while (reporter.ActivityItemUpdated.Reader.TryRead(out _)) { }
ClearActivities(reporter);

// Act - Step is completed, so logging should be a no-op
step.Log(LogLevel.Information, "Test log", enableMarkdown: false);

// Assert - No new activity should be emitted
Assert.False(reporter.ActivityItemUpdated.Reader.TryRead(out _));
AssertNoActivitiesEmitted(reporter);
}

[Fact]
Expand Down Expand Up @@ -913,6 +973,175 @@ public async Task CompleteTaskAsync_WithMarkdownCompletionMessage_PreservesMarkd
Assert.Equal(markdownCompletionMessage, activity.Data.CompletionMessage);
}

[Fact]
public async Task CompleteTaskAsync_IdempotentWithWarning()
{
// Arrange
var reporter = CreatePublishingReporter();

var step = await reporter.CreateStepAsync("Test Step", CancellationToken.None);
var task = await step.CreateTaskAsync("Test Task", CancellationToken.None);

// Complete the task with warning
await task.CompleteAsync("Warning message", CompletionState.CompletedWithWarning, CancellationToken.None);

// Clear activities
ClearActivities(reporter);

// Act - Try to complete again with same state (should be idempotent)
await task.CompleteAsync("Different warning message", CompletionState.CompletedWithWarning, CancellationToken.None);

// Assert - No new activity should be emitted
AssertNoActivitiesEmitted(reporter);

var taskInternal = Assert.IsType<ReportingTask>(task);
Assert.Equal(CompletionState.CompletedWithWarning, taskInternal.CompletionState);
}

[Fact]
public async Task CompleteTaskAsync_IdempotentWithError()
{
// Arrange
var reporter = CreatePublishingReporter();

var step = await reporter.CreateStepAsync("Test Step", CancellationToken.None);
var task = await step.CreateTaskAsync("Test Task", CancellationToken.None);

// Complete the task with error
await task.CompleteAsync("Error message", CompletionState.CompletedWithError, CancellationToken.None);

// Clear activities
ClearActivities(reporter);

// Act - Try to complete again with same state (should be idempotent)
await task.CompleteAsync("Different error message", CompletionState.CompletedWithError, CancellationToken.None);

// Assert - No new activity should be emitted
AssertNoActivitiesEmitted(reporter);

var taskInternal = Assert.IsType<ReportingTask>(task);
Assert.Equal(CompletionState.CompletedWithError, taskInternal.CompletionState);
}

[Fact]
public async Task CompleteStepAsync_IdempotentWithWarning()
{
// Arrange
var reporter = CreatePublishingReporter();

var step = await reporter.CreateStepAsync("Test Step", CancellationToken.None);

// Complete the step with warning
await step.CompleteAsync("Warning", CompletionState.CompletedWithWarning, CancellationToken.None);

// Clear activities
ClearActivities(reporter);

// Act - Try to complete again with same state (should be idempotent)
await step.CompleteAsync("Different warning", CompletionState.CompletedWithWarning, CancellationToken.None);

// Assert - No new activity should be emitted
AssertNoActivitiesEmitted(reporter);

var stepInternal = Assert.IsType<ReportingStep>(step);
Assert.Equal(CompletionState.CompletedWithWarning, stepInternal.CompletionState);
}

[Fact]
public async Task CompleteStepAsync_IdempotentWithError()
{
// Arrange
var reporter = CreatePublishingReporter();

var step = await reporter.CreateStepAsync("Test Step", CancellationToken.None);

// Complete the step with error
await step.CompleteAsync("Error", CompletionState.CompletedWithError, CancellationToken.None);

// Clear activities
ClearActivities(reporter);

// Act - Try to complete again with same state (should be idempotent)
await step.CompleteAsync("Different error", CompletionState.CompletedWithError, CancellationToken.None);

// Assert - No new activity should be emitted
AssertNoActivitiesEmitted(reporter);

var stepInternal = Assert.IsType<ReportingStep>(step);
Assert.Equal(CompletionState.CompletedWithError, stepInternal.CompletionState);
}

[Theory]
[InlineData(CompletionState.Completed, CompletionState.CompletedWithWarning)]
[InlineData(CompletionState.Completed, CompletionState.CompletedWithError)]
[InlineData(CompletionState.CompletedWithWarning, CompletionState.Completed)]
[InlineData(CompletionState.CompletedWithWarning, CompletionState.CompletedWithError)]
[InlineData(CompletionState.CompletedWithError, CompletionState.Completed)]
[InlineData(CompletionState.CompletedWithError, CompletionState.CompletedWithWarning)]
public async Task CompleteTaskAsync_IdempotentWhenTransitioningBetweenTerminalStates(CompletionState firstState, CompletionState secondState)
{
// Arrange
var reporter = CreatePublishingReporter();

var step = await reporter.CreateStepAsync("Test Step", CancellationToken.None);
var task = await step.CreateTaskAsync("Test Task", CancellationToken.None);

// Complete the task with first state
await task.CompleteAsync("First completion", firstState, CancellationToken.None);

// Clear activities
ClearActivities(reporter);

// Act - Try to complete with different state (should be idempotent, no exception)
await task.CompleteAsync("Second completion", secondState, CancellationToken.None);

// Assert - No new activity should be emitted (noop)
AssertNoActivitiesEmitted(reporter);

var taskInternal = Assert.IsType<ReportingTask>(task);
Assert.Equal(firstState, taskInternal.CompletionState); // Original state is retained
}

[Theory]
[InlineData(CompletionState.Completed, CompletionState.CompletedWithWarning)]
[InlineData(CompletionState.Completed, CompletionState.CompletedWithError)]
[InlineData(CompletionState.CompletedWithWarning, CompletionState.Completed)]
[InlineData(CompletionState.CompletedWithWarning, CompletionState.CompletedWithError)]
[InlineData(CompletionState.CompletedWithError, CompletionState.Completed)]
[InlineData(CompletionState.CompletedWithError, CompletionState.CompletedWithWarning)]
public async Task CompleteStepAsync_IdempotentWhenTransitioningBetweenTerminalStates(CompletionState firstState, CompletionState secondState)
{
// Arrange
var reporter = CreatePublishingReporter();

var step = await reporter.CreateStepAsync("Test Step", CancellationToken.None);

// Complete the step with first state
await step.CompleteAsync("First completion", firstState, CancellationToken.None);

// Clear activities
ClearActivities(reporter);

// Act - Try to complete with different state (should be idempotent, no exception)
await step.CompleteAsync("Second completion", secondState, CancellationToken.None);

// Assert - No new activity should be emitted (noop)
AssertNoActivitiesEmitted(reporter);

var stepInternal = Assert.IsType<ReportingStep>(step);
Assert.Equal(firstState, stepInternal.CompletionState); // Original state is retained
}

private static void ClearActivities(PipelineActivityReporter reporter)
{
while (reporter.ActivityItemUpdated.Reader.TryRead(out _)) { }
}

private static void AssertNoActivitiesEmitted(PipelineActivityReporter reporter)
{
Assert.False(reporter.ActivityItemUpdated.Reader.TryRead(out _));
}

private PipelineActivityReporter CreatePublishingReporter()
{
return new PipelineActivityReporter(_interactionService, NullLogger<PipelineActivityReporter>.Instance);
Expand Down