Address PR review feedback: fix deadlock, unhandled rejection, add comments

- Move flushPendingPromises() from _buildInternal to public build() wrapper
  using async closure pattern to prevent deadlock (JamesNK)
- Apply same fix in GenerateTypeClassMethod for type class build() paths
- Add .catch(() => {}) to promise.finally() in trackPromise to prevent
  unhandled rejection crashes in Node.js (JamesNK)
- Add console.warn when flushing pending promises to surface implicit flushes
- Add explanatory comment on the while loop in flushPendingPromises (JamesNK)
- Update doc comment to reflect flush location change
- Update snapshots (TwoPassScanning, transport)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: davidfowl

src/Aspire.Hosting.CodeGeneration.TypeScript/AtsTypeScriptCodeGenerator.cs

@@ -1412,7 +1412,6 @@ private void GenerateBuilderClass(BuilderModel builder)
     /// <code>
     /// /** @internal */
     /// private async _withEnvironmentInternal(name: string, value: string): Promise&lt;RedisResource&gt; {
-    ///     await this._client.flushPendingPromises();  // only emitted for build()
     ///     const rpcArgs: Record&lt;string, unknown&gt; = { builder: this._handle, name, value };
     ///     const result = await this._client.invokeCapability&lt;RedisResourceHandle&gt;('...', rpcArgs);
     ///     return new RedisResourceImpl(result, this._client);
@@ -1422,6 +1421,12 @@ private void GenerateBuilderClass(BuilderModel builder)
     ///     return new RedisResourcePromiseImpl(
     ///         this._withEnvironmentInternal(name, value), this._client);
     /// }
+    ///
+    /// // For build(), the public wrapper flushes pending promises first:
+    /// build(): DistributedApplicationPromise {
+    ///     const flushAndBuild = async () =&gt; { await this._client.flushPendingPromises(); return this._buildInternal(); };
+    ///     return new DistributedApplicationPromiseImpl(flushAndBuild(), this._client);
+    /// }
     /// </code>
     /// <para>When a parameter is a handle type, promise resolution is emitted before the RPC args
     /// (e.g. <c>db = isPromiseLike(db) ? await db : db;</c>).</para>
@@ -1552,12 +1557,6 @@ private void GenerateBuilderMethod(BuilderModel builder, AtsCapabilityInfo capab
         // Resolve any promise-like handle parameters before building rpcArgs
         GeneratePromiseResolution(capability.Parameters);
 
-        // Flush pending promises before build to ensure all un-awaited chains complete
-        if (string.Equals(capability.MethodName, "build", StringComparison.OrdinalIgnoreCase))
-        {
-            WriteLine("        await this._client.flushPendingPromises();");
-        }
-
         // Build args object with conditional inclusion
         GenerateArgsObjectWithConditionals(targetParamName, requiredParams, optionalParams);
 
@@ -1597,9 +1596,21 @@ private void GenerateBuilderMethod(BuilderModel builder, AtsCapabilityInfo capab
 
         // Forward all params to internal method
         var allParamNames = capability.Parameters.Select(p => p.Name);
-        Write($"        return new {promiseImplementationClass}(this.{internalMethodName}(");
-        Write(string.Join(", ", allParamNames));
-        WriteLine("), this._client);");
+        var internalCall = $"this.{internalMethodName}({string.Join(", ", allParamNames)})";
+
+        // For build(), flush pending promises before invoking the internal method.
+        // This must happen in the public wrapper (not _buildInternal) to avoid deadlock:
+        // the PromiseImpl constructor tracks the build promise, and if _buildInternal
+        // awaited flushPendingPromises, the flush would re-await the tracked build promise.
+        if (string.Equals(capability.MethodName, "build", StringComparison.OrdinalIgnoreCase))
+        {
+            WriteLine($"        const flushAndBuild = async () => {{ await this._client.flushPendingPromises(); return {internalCall}; }};");
+            WriteLine($"        return new {promiseImplementationClass}(flushAndBuild(), this._client);");
+        }
+        else
+        {
+            WriteLine($"        return new {promiseImplementationClass}({internalCall}, this._client);");
+        }
         WriteLine("    }");
         WriteLine();
     }
@@ -3044,9 +3055,18 @@ private void GenerateTypeClassMethod(BuilderModel model, AtsCapabilityInfo capab
                 WriteLine($"        {(IsWidenedHandleType(param.Type) ? "let" : "const")} {param.Name} = options?.{param.Name};");
             }
 
-            Write($"        return new {returnPromiseImplementationClass}(this.{internalMethodName}(");
-            Write(string.Join(", ", userParams.Select(p => p.Name)));
-            WriteLine("), this._client);");
+            var internalCall = $"this.{internalMethodName}({string.Join(", ", userParams.Select(p => p.Name))})";
+
+            // For build(), flush pending promises before invoking the internal method to avoid deadlock
+            if (string.Equals(methodName, "build", StringComparison.OrdinalIgnoreCase))
+            {
+                WriteLine($"        const flushAndBuild = async () => {{ await this._client.flushPendingPromises(); return {internalCall}; }};");
+                WriteLine($"        return new {returnPromiseImplementationClass}(flushAndBuild(), this._client);");
+            }
+            else
+            {
+                WriteLine($"        return new {returnPromiseImplementationClass}({internalCall}, this._client);");
+            }
             WriteLine("    }");
         }
         else if (isVoid)

src/Aspire.Hosting.CodeGeneration.TypeScript/Resources/transport.ts

@@ -761,10 +761,15 @@ export class AspireClient implements AspireClientRpc {
 
     trackPromise(promise: Promise<unknown>): void {
         this._pendingPromises.add(promise);
-        promise.finally(() => this._pendingPromises.delete(promise));
+        promise.finally(() => this._pendingPromises.delete(promise)).catch(() => {});
     }
 
     async flushPendingPromises(): Promise<void> {
+        if (this._pendingPromises.size > 0) {
+            console.warn(`Flushing ${this._pendingPromises.size} pending promise(s). Consider awaiting fluent calls to avoid implicit flushing.`);
+        }
+        // Loop because awaiting existing promises may cause new ones to be enqueued.
+        // Awaiting a tracked promise also removes it from the set via the .finally() handler.
         while (this._pendingPromises.size > 0) {
             await Promise.all(this._pendingPromises);
         }

tests/Aspire.Hosting.CodeGeneration.TypeScript.Tests/Snapshots/TwoPassScanningGeneratedAspire.verified.ts

@@ -4055,7 +4055,8 @@ class DistributedApplicationBuilderImpl implements DistributedApplicationBuilder
     }
 
     build(): DistributedApplicationPromise {
-        return new DistributedApplicationPromiseImpl(this._buildInternal(), this._client);
+        const flushAndBuild = async () => { await this._client.flushPendingPromises(); return this._buildInternal(); };
+        return new DistributedApplicationPromiseImpl(flushAndBuild(), this._client);
     }
 
     /** Adds a connection string with a reference expression */

tests/Aspire.Hosting.CodeGeneration.TypeScript.Tests/Snapshots/transport.verified.ts

@@ -1,4 +1,4 @@
-// transport.ts - ATS transport layer: RPC, Handle, errors, callbacks
+// transport.ts - ATS transport layer: RPC, Handle, errors, callbacks
 import * as net from 'net';
 import * as rpc from 'vscode-jsonrpc/node.js';
 
@@ -761,10 +761,15 @@ export class AspireClient implements AspireClientRpc {
 
     trackPromise(promise: Promise<unknown>): void {
         this._pendingPromises.add(promise);
-        promise.finally(() => this._pendingPromises.delete(promise));
+        promise.finally(() => this._pendingPromises.delete(promise)).catch(() => {});
     }
 
     async flushPendingPromises(): Promise<void> {
+        if (this._pendingPromises.size > 0) {
+            console.warn(`Flushing ${this._pendingPromises.size} pending promise(s). Consider awaiting fluent calls to avoid implicit flushing.`);
+        }
+        // Loop because awaiting existing promises may cause new ones to be enqueued.
+        // Awaiting a tracked promise also removes it from the set via the .finally() handler.
         while (this._pendingPromises.size > 0) {
             await Promise.all(this._pendingPromises);
         }