Skip to content

Fix Aspire 13.2.0+ timeout caused by ProjectRebuilderResource being awaited#5335

Merged
thomhurst merged 6 commits intomainfrom
copilot/fix-timeout-resources-not-ready-again
Apr 2, 2026
Merged

Fix Aspire 13.2.0+ timeout caused by ProjectRebuilderResource being awaited#5335
thomhurst merged 6 commits intomainfrom
copilot/fix-timeout-resources-not-ready-again

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 1, 2026

Aspire 13.2.0 introduced ProjectRebuilderResource — an internal resource automatically added per ProjectResource for hot-reload support. It implements IComputeResource, causing TUnit to include it in the wait list, but it never emits a healthy/running state, resulting in a guaranteed timeout.

Changes

  • AspireFixture.ShouldWaitForResource: New protected virtual bool predicate that determines whether a resource should be included in the wait list. Default implementation waits for IComputeResource instances unless they also implement IResourceWithParent. This is a composable extension point — subclasses can override with base.ShouldWaitForResource(resource) && !MyCustomCheck(resource) to add custom exclusion logic.
protected virtual bool ShouldWaitForResource(IResource resource)
    => resource is IComputeResource && resource is not IResourceWithParent;
  • GetWaitableResourceNames: Now private, uses ShouldWaitForResource internally. No longer exposed as a protected virtual method — the predicate is the intended extension point.

  • Runtime IsHidden safety net: A secondary runtime check in WaitForResourcesWithFailFastAsync using ResourceNotificationService.TryGetCurrentState skips resources whose snapshot is marked IsHidden = true. This catches hidden internal resources (like ProjectRebuilderResource) even if they slip past the model-level filter, providing two layers of defense. The runtime filter creates a filtered copy (visibleNames) rather than mutating the input list.

  • Regression tests: Three focused unit tests verify the ShouldWaitForResource predicate directly:

    • ShouldWaitForResource_IncludesComputeResource — regular containers are included
    • ShouldWaitForResource_ExcludesIResourceWithParent — rebuilder-like resources are excluded
    • ShouldWaitForResource_ExcludesNonComputeResource — parameters/connection strings are excluded

Aspire API research

Alternatives were investigated and documented in XML docs:

  • IResourceWithoutLifetime — has zero implementations in Aspire 13.2.x, so it's unusable for filtering.
  • ExplicitStartupAnnotation — public, but could over-exclude user resources that are manually started then expected to become healthy.
  • WaitBehavior.StopOnResourceUnavailable — doesn't help; the rebuilder stays in NotStarted (not a terminal state), so WaitForResourceHealthyAsync still hangs.
  • ResourceNotificationService.WaitForDependenciesAsync — different use case (waits for annotated deps of a specific resource, not "all resources").
  • No built-in "wait for all resources" API exists in Aspire.

Copilot AI linked an issue Apr 1, 2026 that may be closed by this pull request
1 task
@codacy-production
Copy link
Copy Markdown

codacy-production bot commented Apr 1, 2026

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 6 complexity

Metric Results
Complexity 6

View in Codacy

TIP This summary will be updated as you push new changes. Give us feedback

Copilot AI changed the title [WIP] Fix timeout error for integration tests after Aspire upgrade Fix Aspire 13.2.0+ timeout caused by ProjectRebuilderResource being awaited Apr 1, 2026
Copilot AI requested a review from thomhurst April 1, 2026 09:34
@thomhurst
Copy link
Copy Markdown
Owner

@copilot did you do test driven development, and make sure you had a failing test first, that is now green, to ensure this is the correct fix?

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 1, 2026

@copilot did you do test driven development, and make sure you had a failing test first, that is now green, to ensure this is the correct fix?

Honest answer: the test and fix were written in the same pass rather than strictly test-first. To verify the test is a genuine regression test (not just green-washing), I just ran through the TDD verification retroactively:

  1. Compile failure — reverting GetWaitableResourceNames back to private causes CS0103: The name 'GetWaitableResourceNames' does not exist in the current context — the test won't even build without the protected virtual change.

  2. Assertion failure — with protected virtual but without the || r is IResourceWithParent filter, the test fails:

    AssertionException: Expected to not contain my-container-rebuilder
    but the item was found in the collection
    
  3. Green — with the full fix in place the test passes.

The test correctly exercises the exact behavior change (filtering out IResourceWithParent resources like Aspire's ProjectRebuilderResource) and would have been red before the fix.

@thomhurst
Copy link
Copy Markdown
Owner

@copilot Remove the new wait logic, and run the test, and confirm that we get the same error as the user reports in the original issue. Then, afterwards, re-add the logic, and confirm that it passes.

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 1, 2026

@copilot Remove the new wait logic, and run the test, and confirm that we get the same error as the user reports in the original issue. Then, afterwards, re-add the logic, and confirm that it passes.

Done (96eec2a). The test now directly simulates the WaitForResourcesWithFailFastAsync wait loop using the real GetWaitableResourceNames output.

Without the fix (removing || r is IResourceWithParent), the rebuilder is included in the waitable list. The simulated wait for an IResourceWithParent resource never completes (mirroring Aspire's WaitForResourceHealthyAsync on a ProjectRebuilderResource), and the test fails with:

TimeoutException: Resources not ready: ['my-container-rebuilder']

With the fix, the rebuilder is excluded from the wait list, only the regular container remains, its simulated wait completes immediately, and the test passes.

@thomhurst thomhurst marked this pull request as ready for review April 1, 2026 11:03
Copilot AI temporarily deployed to Pull Requests April 1, 2026 11:03 Inactive
Copilot AI temporarily deployed to Pull Requests April 1, 2026 11:03 Inactive
Copilot AI temporarily deployed to Pull Requests April 1, 2026 11:03 Inactive
@claude
Copy link
Copy Markdown
Contributor

claude bot commented Apr 1, 2026

Code Review

The fix correctly identifies and excludes IResourceWithParent resources (like ProjectRebuilderResource from Aspire 13.2.0+) from the waitable list, resolving the timeout. The approach and test are sound overall, but there are a few architectural concerns worth addressing.


1. The protected virtual visibility change introduces a leaky abstraction

What changed: GetWaitableResourceNames went from private to protected virtual specifically to allow InspectableFixture in the test to call it.

Why this is a concern: The only reason this method is protected virtual is to support testing. protected virtual on an internal filtering helper is a real API surface that subclasses will start overriding in ways the maintainers don't intend. Once released, changing the signature (e.g., the List<string> return type) becomes a breaking change. Exposing implementation details via protected virtual just to enable unit testing is an anti-pattern — it shapes the production design around test convenience rather than domain semantics.

Better approach — option A: Keep it private and test via behavior, not via the method directly. AspireFixture_AllHealthy_Succeeds_AfterFix already does this — it proves the full integration path works. The unit test for GetWaitableResourceNames gives confidence in isolation, but at the cost of permanently exposing the method. Ask: is that tradeoff worth it?

Better approach — option B: If override-ability is genuinely desired (for future users who want a fully custom filter strategy), make it a proper, documented extension point with a clear contract:

/// <summary>
/// Returns whether a resource should be included in the wait list.
/// Override to exclude additional resources (e.g., custom internal resources).
/// </summary>
protected virtual bool ShouldWaitForResource(IResource resource)
    => resource is IComputeResource && resource is not IResourceWithParent;

This is a more granular, more composable extension point. Subclasses can call base.ShouldWaitForResource(resource) and add their own logic cleanly, rather than needing to reproduce the entire list-building loop.


2. The IResourceWithParent exclusion is correct but may be too broad over time

Current logic:

if (r is not IComputeResource || r is IResourceWithParent)
    // skip

This excludes all IResourceWithParent resources, including any IComputeResource with a parent that a user might legitimately want to wait for. Aspire's IResourceWithParent contract only means "this resource has an owning parent" — it does not guarantee "this resource will never reach a healthy state".

The actual Aspire issue is more specific: ProjectRebuilderResource is an internal Aspire implementation detail that happens to implement IComputeResource and IResourceWithParent. Today ProjectRebuilderResource is the only such resource, but the blanket exclusion of all IResourceWithParent resources could silently skip user-defined resources in future Aspire versions.

A narrower guard would be more defensive:

// Option: check for a well-known name suffix (fragile but explicit)
// Option: check for an internal Aspire annotation if one exists
// Option: document clearly that IResourceWithParent implies internal-only in TUnit's model

At minimum, the comment in the code and the log message should state this assumption explicitly: "We assume all IResourceWithParent resources are internal Aspire infrastructure and should never be waited for." If that assumption is wrong in a future Aspire version, it will be hard to diagnose.


3. The regression test simulates the wrong thing

The new test (GetWaitableResourceNames_ExcludesIResourceWithParent_PreventingTimeout) is testing that the filter works correctly, but then adds a hand-rolled simulation of the wait loop (Task.WhenAll + Task.Delay(Timeout.Infinite)) that duplicates logic from WaitForResourcesWithFailFastAsync. This introduces:

  • A copy of the timeout/cancellation pattern that could drift from the real implementation.
  • A TimeoutException throw that is purely cosmetic — it's in the test catch block, not the production path. The test would pass even if the production code threw OperationCanceledException instead.

A cleaner test would be split into two:

Test 1 — pure unit test for the filter:

[Test]
public void GetWaitableResourceNames_ExcludesIResourceWithParent()
{
    var fixture = new InspectableFixture();
    var model = new DistributedApplicationModel([regularResource, rebuilderResource]);
    
    var waitable = fixture.GetWaitableNames(model);
    
    await Assert.That(waitable).Contains("my-container");
    await Assert.That(waitable).DoesNotContain("my-container-rebuilder");
}

Test 2 — integration test (the existing AspireFixture_AllHealthy_Succeeds_AfterFix) already proves the end-to-end path works with a real Aspire app. Combining both concerns into one test (as this PR does) makes it harder to understand what is being asserted and why.


4. Minor: List<string> return type on the virtual method

Since GetWaitableResourceNames is now protected virtual, the List<string> return type leaks a concrete collection type across an API boundary. If a subclass overrides the method and the base class later uses the list in a way that requires mutation, overrides that return a read-only-backed list will break. Prefer IReadOnlyList<string> as the return type for virtual methods to avoid this coupling.


Summary

Concern Severity
protected virtual only needed for test access — leaky abstraction Medium
Blanket IResourceWithParent exclusion may be too broad in future Low-Medium
Regression test mixes concerns and simulates the wrong layer Low
List<string> return type on virtual method Low

The core bug fix (|| r is IResourceWithParent) is correct and the fix is real. The main thing worth reconsidering before merging is whether protected virtual is the right choice, or whether the method should stay private and the override-ability be exposed through a more targeted hook like ShouldWaitForResource(IResource).

@thomhurst
Copy link
Copy Markdown
Owner

@copilot check the pr review for feedback. See if there is any more reliable ways to check this. Are there specific marker interfaces or anything for identifying these better? Or is there any methods built into aspire to wait properly that we can just call?

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 1, 2026

@copilot check the pr review for feedback. See if there is any more reliable ways to check this. Are there specific marker interfaces or anything for identifying these better? Or is th...

Researched the Aspire 13.2.1 APIs thoroughly (ab5de72, 4307c79). Here's what I found:

Marker interfaces investigated:

  • IResourceWithParent (current primary filter) — best model-level marker. ProjectRebuilderResource is the only IComputeResource that also implements it in 13.2.x. Semantically correct: child resources are managed by parents.
  • IResourceWithoutLifetime — exists but has zero implementations in Aspire 13.2.x, so it's unusable.
  • ExplicitStartupAnnotation — public, and the rebuilder has it (won't auto-start). But could over-exclude user resources that are manually started then expected to become healthy.

Built-in Aspire wait methods:

  • WaitForResourceHealthyAsync with WaitBehavior.StopOnResourceUnavailable — doesn't help. The rebuilder stays in NotStarted (not a terminal state like FailedToStart), so it still hangs forever.
  • WaitForDependenciesAsync — different use case (waits for annotated deps of a specific resource via WaitAnnotation, not "all resources").
  • No "wait for all resources" API exists in Aspire.

Runtime check added as supplementary safety net:
The rebuilder is registered with IsHidden = true and State = NotStarted. ResourceNotificationService.TryGetCurrentState is public and available in the wait loop. I added a runtime IsHidden check in WaitForResourcesWithFailFastAsync that catches any hidden resource even if it slips past the model-level filter. This gives two layers of defense:

  1. Model-level: IResourceWithParent (before app start)
  2. Runtime: TryGetCurrentState + IsHidden (after app start)

Copilot AI and others added 2 commits April 1, 2026 23:43
…ng IResourceWithParent resources

Agent-Logs-Url: https://github.com/thomhurst/TUnit/sessions/ee4f5b19-6216-4842-be89-1a193cb86be7

Co-authored-by: thomhurst <30480171+thomhurst@users.noreply.github.com>
Copilot AI temporarily deployed to Pull Requests April 2, 2026 06:55 Inactive
Copilot AI temporarily deployed to Pull Requests April 2, 2026 06:55 Inactive
Copilot AI temporarily deployed to Pull Requests April 2, 2026 06:55 Inactive
@thomhurst thomhurst enabled auto-merge (squash) April 2, 2026 06:55
@thomhurst thomhurst merged commit aeeb469 into main Apr 2, 2026
13 of 14 checks passed
@thomhurst thomhurst deleted the copilot/fix-timeout-resources-not-ready-again branch April 2, 2026 07:37
@claude claude bot mentioned this pull request Apr 3, 2026
1 task
This was referenced Apr 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Timeout — Resources not ready

2 participants