Fix StartupFormResourceAnnotation resolved at registration time instead of evaluation time#16
Conversation
…gate Co-authored-by: oising <1844001+oising@users.noreply.github.com>
* clean up RR publishing * Update Src/Nivot.Aspire.Hosting.ProjectCommander/DistributedApplicationBuilderExtensions.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Initial plan (#15) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> * Initial plan (#17) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Oisin Grehan <oising@gmail.com> * Fix StartupFormResourceAnnotation resolved at registration time instead of evaluation time (#16) * Initial plan * Resolve StartupFormResourceAnnotation dynamically in UpdateState delegate Co-authored-by: oising <1844001+oising@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: oising <1844001+oising@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: oising <1844001+oising@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a timing issue where StartupFormResourceAnnotation was captured at registration time in WithProjectCommands, causing commands to remain permanently enabled when WithProjectManifest (which adds the annotation) was called after WithProjectCommands. The fix resolves the annotation dynamically on each state evaluation, making the call order between these methods irrelevant.
Changes:
- Modified
WithProjectCommandsto resolveStartupFormResourceAnnotationdynamically in theUpdateStatecallback instead of capturing it once at registration time - Commands now correctly check form completion state regardless of whether
WithProjectCommandsorWithProjectManifestis called first
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| UpdateState = _ => | ||
| { | ||
| var annotation = builder.Resource.Annotations | ||
| .OfType<StartupFormResourceAnnotation>() | ||
| .FirstOrDefault(); | ||
| return annotation != null && !annotation.StartupFormResource.IsCompleted | ||
| ? ResourceCommandState.Disabled | ||
| : ResourceCommandState.Enabled; | ||
| } |
There was a problem hiding this comment.
This change modifies the core behavior of command state management based on startup forms, but there are no tests covering this logic. Consider adding tests that verify commands are disabled when a startup form exists and is incomplete, enabled when the form is completed, and that the annotation is resolved dynamically to handle different call orders of WithProjectCommands and WithProjectManifest.
| UpdateState = _ => | ||
| { | ||
| var annotation = builder.Resource.Annotations | ||
| .OfType<StartupFormResourceAnnotation>() | ||
| .FirstOrDefault(); | ||
| return annotation != null && !annotation.StartupFormResource.IsCompleted | ||
| ? ResourceCommandState.Disabled | ||
| : ResourceCommandState.Enabled; | ||
| } |
There was a problem hiding this comment.
The new logic always provides an UpdateState function, whereas the old code set UpdateState to null when no StartupFormResourceAnnotation existed at registration time. While the new behavior correctly handles dynamic annotation resolution (the main fix), it also means commands without startup forms now have their state explicitly managed to return Enabled on every check, rather than having no UpdateState callback. Verify this behavior change is intentional and doesn't have unintended side effects in the Aspire framework's command state management.
WithProjectCommandscapturedStartupFormResourceAnnotationonce at call time, meaning ifWithProjectManifestwas called afterward, the annotation would never be found and commands would stay permanently enabled regardless of form completion state.Changes
ResourceBuilderProjectCommanderExtensions.cs: Removed the upfront annotation lookup; moved it inside theUpdateStatedelegate so it resolves frombuilder.Resource.Annotationson every state evaluation💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.