CmdPal: preserve host-owned proxy lifetimes

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 7a42451c-0ec4-4778-86fd-c77d7ac9d122

src/modules/cmdpal/Microsoft.CmdPal.JsonRpc/Models/JSListPageProxy.cs

@@ -36,6 +36,7 @@ internal sealed partial class JSListPageProxy : JSObservableProxyBase, IListPage
     private readonly object _stateLock = new();
     private readonly JSLazyCache<IFilters?> _filters;
     private readonly JSLazyCache<ICommandItem?> _emptyContent;
+    private readonly object _getItemsLock = new();
     private readonly object _itemCacheLock = new();
     private bool? _hasMoreItemsState;
     private bool _disposed;
@@ -108,26 +109,29 @@ internal sealed partial class JSListPageProxy : JSObservableProxyBase, IListPage
 
     public IListItem[] GetItems()
     {
-        try
+        lock (_getItemsLock)
         {
-            var response = Connection.SendRequestAsync(
-                "listPage/getItems",
-                new JsonObject { ["pageId"] = _pageId },
-                CancellationToken.None).GetAwaiter().GetResult();
-
-            if (response.Error != null)
+            try
             {
-                Logger.LogError($"GetItems error for page {_pageId}: {response.Error.Message}");
+                var response = Connection.SendRequestAsync(
+                    "listPage/getItems",
+                    new JsonObject { ["pageId"] = _pageId },
+                    CancellationToken.None).GetAwaiter().GetResult();
+
+                if (response.Error != null)
+                {
+                    Logger.LogError($"GetItems error for page {_pageId}: {response.Error.Message}");
+                    return [];
+                }
+
+                UpdatePageState(response.Result);
+                return ParseListItems(response.Result);
+            }
+            catch (Exception ex)
+            {
+                Logger.LogError($"Failed to get items for page {_pageId}: {ex.Message}");
                 return [];
             }
-
-            UpdatePageState(response.Result);
-            return ParseListItems(response.Result);
-        }
-        catch (Exception ex)
-        {
-            Logger.LogError($"Failed to get items for page {_pageId}: {ex.Message}");
-            return [];
         }
     }
 
@@ -451,7 +455,6 @@ internal sealed partial class JSListPageProxy : JSObservableProxyBase, IListPage
                 nextQueue.Enqueue(adapter);
             }
 
-            DisposeAdapters(previousCache);
             _adapterCache = nextCache;
         }
 
@@ -462,22 +465,12 @@ internal sealed partial class JSListPageProxy : JSObservableProxyBase, IListPage
     {
         lock (_itemCacheLock)
         {
-            DisposeAdapters(_adapterCache);
+            // GetItems hands these adapters to the host, so the cache only owns its
+            // references. Host-held adapters remain live until their owners release them.
             _adapterCache = new Dictionary<string, Queue<JSListItemAdapter>>(StringComparer.Ordinal);
         }
     }
 
-    private static void DisposeAdapters(Dictionary<string, Queue<JSListItemAdapter>> cache)
-    {
-        foreach (var adapters in cache.Values)
-        {
-            while (adapters.TryDequeue(out var adapter))
-            {
-                adapter.Dispose();
-            }
-        }
-    }
-
     private sealed class PageRegistry
     {
         private readonly object _subscribeLock = new();

src/modules/cmdpal/Tests/Microsoft.CmdPal.JsonRpc.UnitTests/JSAdapterLifecycleTests.cs

@@ -249,7 +249,7 @@ public partial class JSAdapterTests
     }
 
     [TestMethod]
-    public void ListPage_RemovedItemDisposesNestedProxies()
+    public void ListPage_RemovedItemKeepsHostHeldProxyLive()
     {
         using var fake = new JSFakeExtension();
         var requestCount = 0;
@@ -275,14 +275,69 @@ public partial class JSAdapterTests
 
         using var page = new JSListPageProxy("page", fake.Connection);
         var firstItems = page.GetItems();
-        _ = firstItems[0].Command;
+        var command = firstItems[0].Command;
 
         Assert.AreEqual(2, JSPropertyChangeRegistry.GetRegistrationCount(fake.Connection, "nested-command"));
 
         Assert.AreEqual(0, page.GetItems().Length);
+        Assert.AreEqual(2, JSPropertyChangeRegistry.GetRegistrationCount(fake.Connection, "nested-command"));
+
+        JSPropertyChangeRegistry.Dispatch(
+            fake.Connection,
+            ParseElement(new JsonObject
+            {
+                ["commandId"] = "nested-command",
+                ["properties"] = new JsonObject { ["name"] = "Still live" },
+            }));
+
+        Assert.AreEqual("Still live", command?.Name);
+
+        (firstItems[0] as IDisposable)?.Dispose();
         Assert.AreEqual(0, JSPropertyChangeRegistry.GetRegistrationCount(fake.Connection, "nested-command"));
     }
 
+    [TestMethod]
+    public async Task ListPage_ConcurrentGetItemsRequestsAreSerialized()
+    {
+        using var fake = new JSFakeExtension();
+        var requestCount = 0;
+        var firstRequestStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
+        var releaseFirstRequest = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
+        fake.OnRequestAsync("listPage/getItems", async _ =>
+        {
+            var request = Interlocked.Increment(ref requestCount);
+            if (request == 1)
+            {
+                firstRequestStarted.SetResult();
+                await releaseFirstRequest.Task;
+            }
+
+            return new JsonObject
+            {
+                ["items"] = new JsonArray
+                {
+                    new JsonObject { ["id"] = "row", ["title"] = $"Response {request}" },
+                },
+            };
+        });
+
+        using var page = new JSListPageProxy("page", fake.Connection);
+        var firstGetItems = Task.Run(page.GetItems);
+        await firstRequestStarted.Task.WaitAsync(Timeout);
+        var secondGetItems = Task.Run(page.GetItems);
+
+        await Task.Delay(100);
+        Assert.AreEqual(1, Volatile.Read(ref requestCount));
+
+        releaseFirstRequest.SetResult();
+        var firstItems = await firstGetItems.WaitAsync(Timeout);
+        var secondItems = await secondGetItems.WaitAsync(Timeout);
+
+        Assert.AreEqual("Response 2", secondItems[0].Title);
+        Assert.AreSame(firstItems[0], secondItems[0]);
+        Assert.AreEqual("Response 2", firstItems[0].Title);
+    }
+
     [TestMethod]
     public void ProviderSettings_ConcurrentReadsReturnOneProxy()
     {

src/modules/cmdpal/Tests/Microsoft.CmdPal.JsonRpc.UnitTests/JSFakeExtension.cs

@@ -28,8 +28,10 @@ internal sealed class JSFakeExtension : IDisposable
     private readonly Stream _extensionReads;
     private readonly Stream _extensionWrites;
     private readonly ConcurrentDictionary<string, Func<JsonElement, JsonNode?>> _handlers = new(StringComparer.Ordinal);
+    private readonly ConcurrentDictionary<string, Func<JsonElement, Task<JsonNode?>>> _asyncHandlers = new(StringComparer.Ordinal);
     private readonly ConcurrentDictionary<string, (int Code, string Message)> _errors = new(StringComparer.Ordinal);
     private readonly CancellationTokenSource _cts = new();
+    private readonly SemaphoreSlim _writeLock = new(1, 1);
     private readonly Task _pump;
     private bool _isDisposed;
 
@@ -47,6 +49,8 @@ internal sealed class JSFakeExtension : IDisposable
 
     public void OnRequest(string method, Func<JsonElement, JsonNode?> handler) => _handlers[method] = handler;
 
+    public void OnRequestAsync(string method, Func<JsonElement, Task<JsonNode?>> handler) => _asyncHandlers[method] = handler;
+
     public void OnResult(string method, string resultJson) => _handlers[method] = _ => JsonNode.Parse(resultJson);
 
     // Answers a request method with a JSON-RPC error so tests can drive proxy
@@ -115,6 +119,12 @@ internal sealed class JSFakeExtension : IDisposable
                     continue;
                 }
 
+                if (_asyncHandlers.TryGetValue(method, out var asyncHandler))
+                {
+                    _ = RespondAsync(id, asyncHandler(parameters), cancellationToken);
+                    continue;
+                }
+
                 JsonNode? result = null;
                 if (_handlers.TryGetValue(method, out var handler))
                 {
@@ -150,6 +160,20 @@ internal sealed class JSFakeExtension : IDisposable
         await WriteFramedAsync(message.ToJsonString(), cancellationToken);
     }
 
+    private async Task RespondAsync(int id, Task<JsonNode?> resultTask, CancellationToken cancellationToken)
+    {
+        try
+        {
+            await RespondAsync(id, await resultTask, cancellationToken);
+        }
+        catch (OperationCanceledException)
+        {
+        }
+        catch (ObjectDisposedException)
+        {
+        }
+    }
+
     private async Task RespondErrorAsync(int id, int code, string message, CancellationToken cancellationToken)
     {
         var envelope = new JsonObject
@@ -168,14 +192,22 @@ internal sealed class JSFakeExtension : IDisposable
 
     private async Task WriteFramedAsync(string json, CancellationToken cancellationToken)
     {
-        var body = Encoding.UTF8.GetBytes(json);
-        var header = Encoding.ASCII.GetBytes($"Content-Length: {body.Length}\r\n\r\n");
-        var buffer = new byte[header.Length + body.Length];
-        Buffer.BlockCopy(header, 0, buffer, 0, header.Length);
-        Buffer.BlockCopy(body, 0, buffer, header.Length, body.Length);
+        await _writeLock.WaitAsync(cancellationToken);
+        try
+        {
+            var body = Encoding.UTF8.GetBytes(json);
+            var header = Encoding.ASCII.GetBytes($"Content-Length: {body.Length}\r\n\r\n");
+            var buffer = new byte[header.Length + body.Length];
+            Buffer.BlockCopy(header, 0, buffer, 0, header.Length);
+            Buffer.BlockCopy(body, 0, buffer, header.Length, body.Length);
 
-        await _extensionWrites.WriteAsync(buffer, cancellationToken);
-        await _extensionWrites.FlushAsync(cancellationToken);
+            await _extensionWrites.WriteAsync(buffer, cancellationToken);
+            await _extensionWrites.FlushAsync(cancellationToken);
+        }
+        finally
+        {
+            _writeLock.Release();
+        }
     }
 
     private static async Task<string> ReadFramedAsync(Stream stream, CancellationToken cancellationToken)