Migrate DotnetToolResource to use RequiredCommandValidator

src/Aspire.Hosting/ApplicationModel/RequiredCommandValidator.cs

@@ -21,8 +21,8 @@ internal sealed class RequiredCommandValidator : IRequiredCommandValidator, IDis
     private readonly IInteractionService _interactionService;
     private readonly ILogger<RequiredCommandValidator> _logger;
 
-    // Track validation state per command to coalesce notifications
-    private readonly ConcurrentDictionary<string, CommandValidationState> _commandStates = new(StringComparer.OrdinalIgnoreCase);
+    // Track validation state per command/callback pair to coalesce notifications and validation work.
+    private readonly ConcurrentDictionary<CommandValidationCacheKey, CommandValidationState> _commandStates = new();
 
     public RequiredCommandValidator(
         IServiceProvider serviceProvider,
@@ -59,8 +59,9 @@ public async Task<RequiredCommandValidationResult> ValidateAsync(
             throw new InvalidOperationException($"Required command on resource '{resource.Name}' cannot be null or empty.");
         }
 
-        // Get or create state for this command
-        var state = _commandStates.GetOrAdd(command, _ => new CommandValidationState());
+        // Get or create state for this command/callback combination.
+        var cacheKey = new CommandValidationCacheKey(command, annotation.ValidationCallback);
+        var state = _commandStates.GetOrAdd(cacheKey, _ => new CommandValidationState());
 
         await state.Gate.WaitAsync(cancellationToken).ConfigureAwait(false);
         try
@@ -179,6 +180,13 @@ private sealed class CommandValidationState : IDisposable
         public void Dispose() => Gate.Dispose();
     }
 
+    /// <summary>
+    /// Cache key for command validation state.
+    /// </summary>
+    private readonly record struct CommandValidationCacheKey(
+        string Command,
+        Func<RequiredCommandValidationContext, Task<RequiredCommandValidationResult>>? Callback);
+
     /// <summary>
     /// Attempts to resolve a command (file name or path) to a full path.
     /// </summary>

src/Aspire.Hosting/DotnetToolResourceExtensions.cs

@@ -50,6 +50,7 @@ public static IResourceBuilder<T> AddDotnetTool<T>(this IDistributedApplicationB
             .WithIconName("Toolbox")
             .WithCommand("dotnet")
             .WithArgs(BuildToolExecArguments)
+            .WithRequiredCommand("dotnet", context => ValidateDotnetSdkVersionAsync(context, resource.WorkingDirectory))
             .OnBeforeResourceStarted(BeforeResourceStarted);
 
         void BuildToolExecArguments(CommandLineArgsCallbackContext x)
@@ -110,9 +111,6 @@ await rns.PublishUpdateAsync(resource, x => x with
                     new (KnownProperties.Resource.Source, resource.ToolConfiguration?.PackageId)
                     ])
             }).ConfigureAwait(false);
-
-            var version = await DotnetSdkUtils.TryGetVersionAsync(resource.WorkingDirectory).ConfigureAwait(false);
-            ValidateDotnetSdkVersion(version, resource.WorkingDirectory);
         }
 
     }
@@ -204,18 +202,15 @@ public static IResourceBuilder<T> WithToolIgnoreFailedSources<T>(this IResourceB
         return builder;
     }
 
-    internal static void ValidateDotnetSdkVersion(Version? version, string workingDirectory)
+    internal static async Task<RequiredCommandValidationResult> ValidateDotnetSdkVersionAsync(RequiredCommandValidationContext _, string workingDirectory)
     {
-        if (version is null)
-        {
-            // This most likely means something is majorly wrong with the dotnet sdk
-            // Which will show up as an error once dcp runs `dotnet tool exec`
-            return;
-        }
+        var version = await DotnetSdkUtils.TryGetVersionAsync(workingDirectory).ConfigureAwait(false);
 
-        if (version.Major < 10)
+        if (version?.Major < 10)
         {
-            throw new DistributedApplicationException($"DotnetToolResource requires dotnet SDK 10 or higher to run. Detected version: {version} for working directory {workingDirectory}");
+            return RequiredCommandValidationResult.Failure($"DotnetToolResource requires dotnet SDK 10 or higher to run. Detected version: {version}");
         }
+
+        return RequiredCommandValidationResult.Success();
     }
 }

tests/Aspire.Hosting.DotnetTool.Tests/AddDotnetToolTests.cs

@@ -378,29 +378,16 @@ public async Task AddDotnetToolGeneratesCorrectManifest()
         Assert.Equal(expectedManifest, manifest.ToString());
     }
 
-    [Theory]
-    [InlineData("11.1.0", true)]
-    [InlineData("10.0.0", true)]
-    [InlineData("9.0.999", false)]
-    public void ValidateDotnetSdkVersion_ValidatesVersionCorrectly(string versionString, bool isAllowed)
-    {
-        var version = Version.Parse(versionString);
-
-        if (isAllowed)
-        {
-            DotnetToolResourceExtensions.ValidateDotnetSdkVersion(version, "");
-        }
-        else
-        {
-            Assert.Throws<DistributedApplicationException>(() =>
-                DotnetToolResourceExtensions.ValidateDotnetSdkVersion(version, ""));
-        }
-    }
-
     [Fact]
-    public void ValidateDotnetSdkVersion_WithNullVersion_DoesNotThrow()
+    public void AddDotnetTool_IncludesRequiredCommandAnnotation()
     {
-        // Should not throw - null is treated as "unable to determine version"
-        DotnetToolResourceExtensions.ValidateDotnetSdkVersion(null, "");
+        var builder = DistributedApplication.CreateBuilder();
+        var tool = builder.AddDotnetTool("mytool", "dotnet-ef");
+
+#pragma warning disable ASPIRECOMMAND001
+        var annotation = Assert.Single(tool.Resource.Annotations.OfType<RequiredCommandAnnotation>());
+#pragma warning restore ASPIRECOMMAND001
+        Assert.Equal("dotnet", annotation.Command);
+        Assert.NotNull(annotation.ValidationCallback);
     }
 }

tests/Aspire.Hosting.Tests/RequiredCommandAnnotationTests.cs

@@ -318,15 +318,48 @@ public async Task RequiredCommandValidationLifecycleHook_CachesSuccessfulValidat
         var command = OperatingSystem.IsWindows() ? "cmd" : "sh";
         var callbackCount = 0;
 
+        Task<RequiredCommandValidationResult> callback(RequiredCommandValidationContext _)
+        {
+            Interlocked.Increment(ref callbackCount);
+            return Task.FromResult(RequiredCommandValidationResult.Success());
+        }
+
         builder.AddContainer("test1", "image")
-            .WithRequiredCommand(command, ctx =>
+            .WithRequiredCommand(command, callback);
+
+        builder.AddContainer("test2", "image")
+            .WithRequiredCommand(command, callback);
+
+        await using var app = builder.Build();
+        await SubscribeHooksAsync(app);
+
+        var appModel = app.Services.GetRequiredService<DistributedApplicationModel>();
+        var resource1 = appModel.Resources.Single(r => r.Name == "test1");
+        var resource2 = appModel.Resources.Single(r => r.Name == "test2");
+        var eventing = app.Services.GetRequiredService<IDistributedApplicationEventing>();
+
+        await eventing.PublishAsync(new BeforeResourceStartedEvent(resource1, app.Services));
+        await eventing.PublishAsync(new BeforeResourceStartedEvent(resource2, app.Services));
+
+        Assert.Equal(1, callbackCount);
+    }
+
+    [Fact]
+    public async Task RequiredCommandValidationLifecycleHook_DoesNotCacheAcrossDifferentValidationCallbacks()
+    {
+        var builder = DistributedApplication.CreateBuilder();
+        var command = OperatingSystem.IsWindows() ? "cmd" : "sh";
+        var callbackCount = 0;
+
+        builder.AddContainer("test1", "image")
+            .WithRequiredCommand(command, _ =>
             {
                 Interlocked.Increment(ref callbackCount);
                 return Task.FromResult(RequiredCommandValidationResult.Success());
             });
 
         builder.AddContainer("test2", "image")
-            .WithRequiredCommand(command, ctx =>
+            .WithRequiredCommand(command, _ =>
             {
                 Interlocked.Increment(ref callbackCount);
                 return Task.FromResult(RequiredCommandValidationResult.Success());
@@ -343,7 +376,7 @@ public async Task RequiredCommandValidationLifecycleHook_CachesSuccessfulValidat
         await eventing.PublishAsync(new BeforeResourceStartedEvent(resource1, app.Services));
         await eventing.PublishAsync(new BeforeResourceStartedEvent(resource2, app.Services));
 
-        Assert.Equal(1, callbackCount);
+        Assert.Equal(2, callbackCount);
     }
 
     [Fact]