[thomhurst]

Summary by CodeRabbit

• Bug Fixes
  • Improved Aspire resource startup monitoring so readiness and failure outcomes are handled consistently.
  • Startup failures continue to include diagnostic information, while pending startup checks are now cleaned up before the result is reported.
• Tests
  • Added coverage for startup success, failures, cancellation, and errors that occur while pending checks are being cleaned up.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: d65cd6d4-283d-4191-aea0-97f1de071e42

Commits

Reviewing files that changed from the base of the PR and between 6a2b383 and d26f325.

Files selected for processing: 3

• src/TUnit.Aspire.Core/AspireFixture.cs
• src/TUnit.Aspire.Core/ResourceStartupWaiter.cs
• tests/TUnit.Aspire.Tests/ResourceStartupWaiterTests.cs

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.

Walkthrough

Aspire startup now uses ResourceStartupWaiter to race resource readiness checks against failure watchers. The helper cancels and joins pending waits, and tests cover outcomes, cancellation, and cleanup faults.

Changes: Aspire startup wait coordination

Layer / File(s)	Summary
Coordinate and validate startup waits src/TUnit.Aspire.Core/ResourceStartupWaiter.cs, tests/TUnit.Aspire.Tests/ResourceStartupWaiterTests.cs	Adds a helper that races readiness waits against failure waits. Tests cover results, exceptions, cancellation, and cleanup behavior.
Use coordinated waits during startup src/TUnit.Aspire.Core/AspireFixture.cs	WaitForResourcesWithFailFastAsync delegates wait coordination to the helper. It retains readiness checks, ready-resource logging, and failure diagnostics.

Priority: Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant AspireFixture
  participant ResourceStartupWaiter
  participant ReadinessCallbacks
  participant FailureWatchers
  AspireFixture->>ResourceStartupWaiter: Call WaitAsync with readiness and failure waits
  par Wait for readiness
    ResourceStartupWaiter->>ReadinessCallbacks: Start readiness waits
  and Wait for failure
    ResourceStartupWaiter->>FailureWatchers: Start failure waits
  end
  ResourceStartupWaiter->>ReadinessCallbacks: Cancel and join pending waits
  ResourceStartupWaiter->>FailureWatchers: Cancel and join pending waits
  ResourceStartupWaiter-->>AspireFixture: Return failed resource name or null

Loading

Possibly related issues

• thomhurst/TUnit 7277: The issue describes cancelling and joining readiness and failure waits before startup exits. This change adds that coordination.

Merge Risk: Minimal · up to d26f3

This change makes Aspire startup cancel and join its pending resource waits instead of leaving them running. No concrete regression was identified, and tests cover the main outcomes and cleanup faults. It is low risk to merge.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title "fix(aspire): cancel and join startup waits (#7277)" matches the main change. ResourceStartupWaiter.WaitAsync now cancels and joins pending readiness and failure waits on every exit path, an…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title "fix(aspire): cancel and join startup waits (#7277)" matches the main change. ResourceStartupWaiter.WaitAsync now cancels and joins pending readiness and failure waits on every exit path, and AspireFixture delegates to it. The title names the fix, the affected area (aspire), and the behavior. The issue number in the title is a reference, not a problem. The objectives mention PR #7279 while the title cites #7277; the objectives also describe the branch as issue-7277, so the mismatch is a minor metadata inconsistency and does not affect the title's relevance to the changeset.

Full details: Docstring Coverage

Explanation

Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 4 functions across 3 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit watched the resources wake,
While readiness raced failure’s track.
The waits were joined when races were done,
And logs marked each resource one by one.
The rabbit thumped, then hopped away,
As startup finished safely that day.}

_{Comment @coderabbitai help to get the list of available commands.}

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-28T00:48:31.172339Z	d26f325	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable issues were found.

Summary

The PR cancels and joins startup waits before reporting success, failure, or cancellation.

• Aspire startup waits finish cleanup before reporting a result.

No actionable issues were found. Tests were reviewed but not run.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Start readiness and failure waits] --> B[Race all ready against first failure]
    B --> C[Keep winning outcome]
    C --> D[Cancel shared token]
    D --> E[Join waits and cancellation callbacks]
    E --> F[Return or throw original outcome]

Loading

_{Reviews (1) · Last reviewed commit: "fix(aspire): cancel and join startup wai..." · Reviewed by Greptile}

[github-actions]

Review

I read the diff but did not build the project or run the tests.

The change moves the ready-versus-failure race out of AspireFixture into ResourceStartupWaiter.WaitAsync. Losing waits are now cancelled and joined before the outcome is reported. I found no blocking issues.

• Behaviour: Failure, success, readiness faults, watcher faults and outer cancellation all keep their original outcome. The finally block cancels the linked CTS, awaits CancelAsync together with every wait, and swallows only the cleanup exceptions. The old code left the loser watchers unobserved, so this fixes that leak.
• Design: Making the waiter an internal static helper that takes delegates is a good seam. It is testable without Aspire types, and AspireFixture is shorter.
• Tests: The parameterised test covers every outcome, with and without a fault during cleanup. It also proves the join by checking that WaitAsync has not completed while the pending waits are still held.

Non-blocking suggestions:

1. Hang risk: Joining in finally means a losing wait that ignores its token would now stall startup reporting. The previous code would have reported straight away. The Aspire WaitFor* APIs honour the token, so this is acceptable. A short comment on that assumption, or a bounded join, would make the contract explicit.
2. Cleanup: Task.WhenAll(readyTask, Task.WhenAll(failureTasks)) can be Task.WhenAll(failureTasks.Append(readyTask)), or just a list of the tasks. The lambda async name => await waitForReady(...) can also drop async/await and return the task directly.
3. Test flakiness: Completed is a plain bool written on a background continuation. The awaits give ordering in practice, but volatile would be more honest.

Looks good to merge.